Skip to content
This repository was archived by the owner on May 20, 2019. It is now read-only.

The user passed for async api referenced correct WebAPI user #22

Merged
merged 12 commits into from
Oct 10, 2018
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
*/
interface BulkSummaryInterface extends \Magento\Framework\Bulk\BulkSummaryInterface
{
const USER_TYPE = 'user_type';

/**
* Retrieve existing extension attributes object.
*
Expand All @@ -31,4 +33,19 @@ public function getExtensionAttributes();
public function setExtensionAttributes(
\Magento\AsynchronousOperations\Api\Data\BulkSummaryExtensionInterface $extensionAttributes
);

/**
* Get user type
*
* @return int
*/
public function getUserType();

/**
* Set user type
*
* @param int $userType
* @return $this
*/
public function setUserType($userType);
}
17 changes: 16 additions & 1 deletion app/code/Magento/AsynchronousOperations/Model/BulkManagement.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
namespace Magento\AsynchronousOperations\Model;

use Magento\Framework\App\ObjectManager;
use Magento\Framework\App\ResourceConnection;
use Magento\AsynchronousOperations\Api\Data\BulkSummaryInterface;
use Magento\AsynchronousOperations\Api\Data\BulkSummaryInterfaceFactory;
Expand All @@ -13,6 +14,7 @@
use Magento\Framework\EntityManager\EntityManager;
use Magento\Framework\EntityManager\MetadataPool;
use Magento\AsynchronousOperations\Model\ResourceModel\Operation\CollectionFactory;
use Magento\Authorization\Model\UserContextInterface;

/**
* Class BulkManagement
Expand Down Expand Up @@ -51,6 +53,11 @@ class BulkManagement implements \Magento\Framework\Bulk\BulkManagementInterface
*/
private $resourceConnection;

/**
* @var \Magento\Authorization\Model\UserContextInterface
*/
private $userContext;

/**
* @var \Psr\Log\LoggerInterface
*/
Expand All @@ -65,6 +72,7 @@ class BulkManagement implements \Magento\Framework\Bulk\BulkManagementInterface
* @param MetadataPool $metadataPool
* @param ResourceConnection $resourceConnection
* @param \Psr\Log\LoggerInterface $logger
* @param UserContextInterface $userContext
*/
public function __construct(
EntityManager $entityManager,
Expand All @@ -73,7 +81,8 @@ public function __construct(
BulkPublisherInterface $publisher,
MetadataPool $metadataPool,
ResourceConnection $resourceConnection,
\Psr\Log\LoggerInterface $logger
\Psr\Log\LoggerInterface $logger,
UserContextInterface $userContext = null
) {
$this->entityManager = $entityManager;
$this->bulkSummaryFactory= $bulkSummaryFactory;
Expand All @@ -82,6 +91,7 @@ public function __construct(
$this->resourceConnection = $resourceConnection;
$this->publisher = $publisher;
$this->logger = $logger;
$this->userContext = $userContext ?: ObjectManager::getInstance()->get(UserContextInterface::class);
}

/**
Expand All @@ -93,13 +103,18 @@ public function scheduleBulk($bulkUuid, array $operations, $description, $userId
$connection = $this->resourceConnection->getConnectionByName($metadata->getEntityConnectionName());
// save bulk summary and related operations
$connection->beginTransaction();
$userType = $this->userContext->getUserType();
if ($userType === null) {
$userType = UserContextInterface::USER_TYPE_ADMIN;
}
try {
/** @var \Magento\AsynchronousOperations\Api\Data\BulkSummaryInterface $bulkSummary */
$bulkSummary = $this->bulkSummaryFactory->create();
$this->entityManager->load($bulkSummary, $bulkUuid);
$bulkSummary->setBulkId($bulkUuid);
$bulkSummary->setDescription($description);
$bulkSummary->setUserId($userId);
$bulkSummary->setUserType($userType);
$bulkSummary->setOperationCount((int)$bulkSummary->getOperationCount() + count($operations));

$this->entityManager->save($bulkSummary);
Expand Down
16 changes: 16 additions & 0 deletions app/code/Magento/AsynchronousOperations/Model/BulkSummary.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,22 @@ public function setUserId($userId)
return $this->setData(self::USER_ID, $userId);
}

/**
* @inheritDoc
*/
public function getUserType()
{
return $this->getData(self::USER_TYPE);
}

/**
* @inheritDoc
*/
public function setUserType($userType)
{
return $this->setData(self::USER_TYPE, $userType);
}

/**
* @inheritDoc
*/
Expand Down
26 changes: 21 additions & 5 deletions app/code/Magento/AsynchronousOperations/Model/MassSchedule.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

namespace Magento\AsynchronousOperations\Model;

use Magento\Framework\App\ObjectManager;
use Magento\Framework\DataObject\IdentityGeneratorInterface;
use Magento\Framework\Exception\LocalizedException;
use Magento\AsynchronousOperations\Api\Data\ItemStatusInterfaceFactory;
Expand All @@ -18,9 +19,12 @@
use Magento\Framework\Exception\BulkException;
use Psr\Log\LoggerInterface;
use Magento\AsynchronousOperations\Model\ResourceModel\Operation\OperationRepository;
use Magento\Authorization\Model\UserContextInterface;

/**
* Class MassSchedule used for adding multiple entities as Operations to Bulk Management with the status tracking
*
* @SuppressWarnings(PHPMD.CouplingBetweenObjects) Suppressed without refactoring to not introduce BiC
*/
class MassSchedule
{
Expand Down Expand Up @@ -54,6 +58,11 @@ class MassSchedule
*/
private $operationRepository;

/**
* @var \Magento\Authorization\Model\UserContextInterface
*/
private $userContext;

/**
* Initialize dependencies.
*
Expand All @@ -63,30 +72,33 @@ class MassSchedule
* @param BulkManagementInterface $bulkManagement
* @param LoggerInterface $logger
* @param OperationRepository $operationRepository
* @param UserContextInterface $userContext
*/
public function __construct(
IdentityGeneratorInterface $identityService,
ItemStatusInterfaceFactory $itemStatusInterfaceFactory,
AsyncResponseInterfaceFactory $asyncResponseFactory,
BulkManagementInterface $bulkManagement,
LoggerInterface $logger,
OperationRepository $operationRepository
OperationRepository $operationRepository,
UserContextInterface $userContext = null
) {
$this->identityService = $identityService;
$this->itemStatusInterfaceFactory = $itemStatusInterfaceFactory;
$this->asyncResponseFactory = $asyncResponseFactory;
$this->bulkManagement = $bulkManagement;
$this->logger = $logger;
$this->operationRepository = $operationRepository;
$this->userContext = $userContext ?: ObjectManager::getInstance()->get(UserContextInterface::class);
}

/**
* Schedule new bulk operation based on the list of entities
*
* @param $topicName
* @param $entitiesArray
* @param null $groupId
* @param null $userId
* @param string $topicName
* @param array $entitiesArray
* @param string $groupId
* @param string $userId
* @return AsyncResponseInterface
* @throws BulkException
* @throws LocalizedException
Expand All @@ -95,6 +107,10 @@ public function publishMass($topicName, array $entitiesArray, $groupId = null, $
{
$bulkDescription = __('Topic %1', $topicName);

if ($userId == null) {
$userId = $this->userContext->getUserId();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User ID is not unique identifier and should be used in conjunction with user type.

There could be integration, admin and customer with the same ID. I believe bulk operation could at least be scheduled by the admin or integration.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right the user ID is not unique.

@anvasiliev @nuzil How do you think if we save the user type?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @paliarush , @vrann
We though about 2 possible solutions, maybe we have to discuss it separately:

  1. We will add prefixes to users, like "admin_ID, customer_ID" .....
  2. We will add new column to this table with user_type.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@munkhulzii @nuzil it would be a cleaner solution if we add another column, with the default value set to the admin. Otherwise it will be backward incompatible with the rest of the system

}

if ($groupId == null) {
$groupId = $this->identityService->generateId();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ public function testScheduleBulk()
$bulkUuid = 'bulk-001';
$description = 'Bulk summary description...';
$userId = 1;
$userType = \Magento\Authorization\Model\UserContextInterface::USER_TYPE_ADMIN;
$connectionName = 'default';
$topicNames = ['topic.name.0', 'topic.name.1'];
$operation = $this->getMockBuilder(\Magento\AsynchronousOperations\Api\Data\OperationInterface::class)
Expand All @@ -131,6 +132,7 @@ public function testScheduleBulk()
$bulkSummary->expects($this->once())->method('setBulkId')->with($bulkUuid)->willReturnSelf();
$bulkSummary->expects($this->once())->method('setDescription')->with($description)->willReturnSelf();
$bulkSummary->expects($this->once())->method('setUserId')->with($userId)->willReturnSelf();
$bulkSummary->expects($this->once())->method('setUserType')->with($userType)->willReturnSelf();
$bulkSummary->expects($this->once())->method('getOperationCount')->willReturn(1);
$bulkSummary->expects($this->once())->method('setOperationCount')->with(3)->willReturnSelf();
$this->entityManager->expects($this->once())->method('save')->with($bulkSummary)->willReturn($bulkSummary);
Expand Down
1 change: 0 additions & 1 deletion app/code/Magento/AsynchronousOperations/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"magento/module-authorization": "*",
"magento/module-backend": "*",
"magento/module-ui": "*",
"magento/module-user": "*",
"php": "~7.1.3||~7.2.0"
},
"suggest": {
Expand Down
8 changes: 5 additions & 3 deletions app/code/Magento/AsynchronousOperations/etc/db_schema.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
<column xsi:type="varbinary" name="uuid" nullable="true" length="39"
comment="Bulk UUID (can be exposed to reference bulk entity)"/>
<column xsi:type="int" name="user_id" padding="10" unsigned="true" nullable="true" identity="false"
comment="ID of the user that performed an action"/>
comment="ID of the WebAPI user that performed an action"/>
<column xsi:type="int" name="user_type" nullable="true" comment="Which type of user"/>
<column xsi:type="varchar" name="description" nullable="true" length="255" comment="Bulk Description"/>
<column xsi:type="int" name="operation_count" padding="10" unsigned="true" nullable="false" identity="false"
comment="Total number of operations scheduled within this bulk"/>
Expand All @@ -23,11 +24,12 @@
<constraint xsi:type="primary" name="PRIMARY">
<column name="id"/>
</constraint>
<constraint xsi:type="foreign" name="MAGENTO_BULK_USER_ID_ADMIN_USER_USER_ID" table="magento_bulk"
column="user_id" referenceTable="admin_user" referenceColumn="user_id" onDelete="CASCADE"/>
<constraint xsi:type="unique" name="MAGENTO_BULK_UUID">
<column name="uuid"/>
</constraint>
<index name="MAGENTO_BULK_USER_ID_ADMIN_USER_USER_ID" indexType="btree">
<column name="user_id"/>
</index>
</table>
<table name="magento_operation" resource="default" engine="innodb" comment="Operation entity">
<column xsi:type="int" name="id" padding="10" unsigned="true" nullable="false" identity="true"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@
"id": true,
"uuid": true,
"user_id": true,
"user_type": true,
"description": true,
"operation_count": true,
"start_time": true
},
"index": {
"MAGENTO_BULK_USER_ID_ADMIN_USER_USER_ID": true,
"MAGENTO_BULK_USER_ID": true
},
"constraint": {
"PRIMARY": true,
"MAGENTO_BULK_USER_ID_ADMIN_USER_USER_ID": true,
"MAGENTO_BULK_UUID": true
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,30 +18,35 @@
'not_started' => [
'uuid' => 'bulk-uuid-1',
'user_id' => 1,
'user_type' => \Magento\Authorization\Model\UserContextInterface::USER_TYPE_ADMIN,
'description' => 'Bulk Description',
'operation_count' => 1,
],
'in_progress_success' => [
'uuid' => 'bulk-uuid-2',
'user_id' => 1,
'user_type' => \Magento\Authorization\Model\UserContextInterface::USER_TYPE_ADMIN,
'description' => 'Bulk Description',
'operation_count' => 3,
],
'in_progress_failed' => [
'uuid' => 'bulk-uuid-3',
'user_id' => 1,
'user_type' => \Magento\Authorization\Model\UserContextInterface::USER_TYPE_ADMIN,
'description' => 'Bulk Description',
'operation_count' => 2,
],
'finish_success' => [
'uuid' => 'bulk-uuid-4',
'user_id' => 1,
'user_type' => \Magento\Authorization\Model\UserContextInterface::USER_TYPE_ADMIN,
'description' => 'Bulk Description',
'operation_count' => 1,
],
'finish_failed' => [
'uuid' => 'bulk-uuid-5',
'user_id' => 1,
'user_type' => \Magento\Authorization\Model\UserContextInterface::USER_TYPE_ADMIN,
'description' => 'Bulk Description',
'operation_count' => 2,
],
Expand Down Expand Up @@ -90,8 +95,8 @@
],
];

$bulkQuery = "INSERT INTO {$bulkTable} (`uuid`, `user_id`, `description`, `operation_count`)"
. " VALUES (:uuid, :user_id, :description, :operation_count);";
$bulkQuery = "INSERT INTO {$bulkTable} (`uuid`, `user_id`, `user_type`, `description`, `operation_count`)"
. " VALUES (:uuid, :user_id, :user_type, :description, :operation_count);";
foreach ($bulks as $bulk) {
$connection->query($bulkQuery, $bulk);
}
Expand Down