-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Expose product variants via export API #160
Changes from all commits
6c9f6f6
b5b8abf
893181d
0af8deb
071aecd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento\CatalogExport\Model\Data; | ||
|
||
use Magento\CatalogExportApi\Api\Data\VariantInterface; | ||
use Magento\Framework\Model\AbstractModel; | ||
|
||
/** | ||
* Variant entity. | ||
*/ | ||
class Variant extends AbstractModel implements VariantInterface | ||
{ | ||
private const ID = 'id'; | ||
private const PARENT_ID = 'parent_id'; | ||
private const PRODUCT_ID = 'product_id'; | ||
private const DEFAULT_QTY = 'default_qty'; | ||
private const QTY_MUTABILITY = 'qty_mutability'; | ||
private const OPTION_VALUE_IDS = 'option_value_ids'; | ||
private const PRICE = 'price'; | ||
|
||
/** | ||
* @inheritDoc | ||
*/ | ||
public function getId() | ||
{ | ||
return $this->getData(self::ID); | ||
} | ||
|
||
/** | ||
* @inheritDoc | ||
*/ | ||
public function getParentId(): string | ||
{ | ||
return $this->getData(self::PARENT_ID); | ||
} | ||
|
||
/** | ||
* @inheritDoc | ||
*/ | ||
public function getProductId(): string | ||
{ | ||
return $this->getData(self::PRODUCT_ID); | ||
} | ||
|
||
/** | ||
* @inheritDoc | ||
*/ | ||
public function getDefaultQty(): float | ||
{ | ||
return $this->getData(self::DEFAULT_QTY); | ||
} | ||
|
||
/** | ||
* @inheritDoc | ||
*/ | ||
public function getQtyMutability(): bool | ||
{ | ||
return $this->getData(self::QTY_MUTABILITY); | ||
} | ||
|
||
/** | ||
* @inheritDoc | ||
*/ | ||
public function getOptionValueIds(): array | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DTO and schema does not match: https://github.com/magento/saas-export/pull/67/files#diff-f3c59568cea39e5d8221a349960c1da1R118. (it will lead to error when you will try to use Export API) I propose to create correct et_shema fist, and then generate classes from schema, so we will have one source of true. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. schema doesn't exists yet. You are referencing on the PR that isn't under review yet. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shema should be created first as a source of true as was agreed with @akaplya please check comment in #160 (review) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Manual work has already done. I don't see any needs to undo this work. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You may see after connecting 2 parts and use them together to propagate data from one system to another. if something you can automate, just do it, don't spend your time on routine work |
||
{ | ||
return $this->getData(self::OPTION_VALUE_IDS); | ||
} | ||
|
||
/** | ||
* @inheritDoc | ||
*/ | ||
public function getPrice(): float | ||
{ | ||
return $this->getData(self::PRICE); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento\CatalogExport\Model\Data; | ||
|
||
/** | ||
* Variant factory. | ||
*/ | ||
class VariantFactory | ||
{ | ||
/** | ||
* Object Manager | ||
* | ||
* @var \Magento\Framework\ObjectManagerInterface | ||
*/ | ||
private $objectManager; | ||
|
||
/** | ||
* @param \Magento\Framework\ObjectManagerInterface $objectManager | ||
*/ | ||
public function __construct(\Magento\Framework\ObjectManagerInterface $objectManager) | ||
{ | ||
$this->objectManager = $objectManager; | ||
} | ||
|
||
/** | ||
* Create instance of Variant. | ||
* | ||
* @param array $data | ||
* @return \Magento\CatalogExportApi\Api\Data\VariantInterface | ||
*/ | ||
public function create(array $data) | ||
{ | ||
return $this->objectManager->create( | ||
\Magento\CatalogExportApi\Api\Data\VariantInterface::class, | ||
$data | ||
); | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add empty line at the end of the file |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
namespace Magento\CatalogExport\Model\Indexer; | ||
|
||
use Magento\CatalogDataExporter\Model\Indexer\IndexerCallbackInterface; | ||
use Magento\CatalogMessageBroker\Model\SerializerInterface; | ||
use Magento\Framework\MessageQueue\PublisherInterface; | ||
use Psr\Log\LoggerInterface; | ||
|
||
|
@@ -18,7 +19,8 @@ class IndexerCallback implements IndexerCallbackInterface | |
{ | ||
private const BATCH_SIZE = 100; | ||
|
||
private const TOPIC_NAME = 'catalog.export.product.data'; | ||
public const PRODUCT_ENTITY = 'product'; | ||
public const PRODUCT_VARIANT_ENTITY = 'product_variant'; | ||
Comment on lines
+22
to
+23
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's better to add empty line between constants to match class codestile |
||
|
||
/** | ||
* @var PublisherInterface | ||
|
@@ -30,32 +32,65 @@ class IndexerCallback implements IndexerCallbackInterface | |
*/ | ||
private $logger; | ||
|
||
/** | ||
* @var array | ||
*/ | ||
private $topicMap; | ||
|
||
/** | ||
* @var SerializerInterface | ||
*/ | ||
private $serializer; | ||
|
||
/** | ||
* @param PublisherInterface $queuePublisher | ||
* @param LoggerInterface $logger | ||
* @param SerializerInterface $serializer | ||
* @param array $topicMap | ||
*/ | ||
public function __construct( | ||
PublisherInterface $queuePublisher, | ||
LoggerInterface $logger | ||
LoggerInterface $logger, | ||
SerializerInterface $serializer, | ||
array $topicMap | ||
) { | ||
$this->queuePublisher = $queuePublisher; | ||
$this->logger = $logger; | ||
$this->serializer = $serializer; | ||
$this->topicMap = $topicMap; | ||
} | ||
|
||
/** | ||
* @inheritdoc | ||
*/ | ||
public function execute(array $ids) | ||
public function execute(array $ids, string $entityType) | ||
{ | ||
$this->validateEntityType($entityType); | ||
foreach (array_chunk($ids, self::BATCH_SIZE) as $idsChunk) { | ||
if (!empty($idsChunk)) { | ||
try { | ||
// @todo understand why string[] doesn't work | ||
$this->queuePublisher->publish(self::TOPIC_NAME, json_encode($idsChunk)); | ||
} catch (\Exception $e) { | ||
$this->queuePublisher->publish( | ||
$this->topicMap[$entityType], | ||
$this->serializer->serialize($idsChunk) | ||
); | ||
} catch (\Throwable $e) { | ||
$this->logger->critical($e); | ||
} | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Check entity type on satisfaction to domain values. | ||
* | ||
* @param string $entityType | ||
* @return void | ||
*/ | ||
private function validateEntityType(string $entityType): void | ||
{ | ||
if (!isset($this->topicMap[$entityType])) { | ||
throw new \DomainException("'$entityType' entity type doesn't supported."); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,124 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento\CatalogExport\Model; | ||
|
||
use Magento\CatalogExportApi\Api\VariantRepositoryInterface; | ||
|
||
/** | ||
* Default implementation of product variants repository. | ||
*/ | ||
class VariantRepository implements VariantRepositoryInterface | ||
{ | ||
/** | ||
* Constant value for setting max items in response. | ||
*/ | ||
private const MAX_ITEMS_IN_RESPONSE = 250; | ||
|
||
/** | ||
* Config path of max numbers of variants allowed in response. | ||
*/ | ||
private const CATALOG_EXPORT_MAX_VARIANTS_IN_RESPONSE = 'catalog_export/max_variants_in_response'; | ||
|
||
/** | ||
* @var \Magento\DataExporter\Model\FeedInterface | ||
*/ | ||
private $variantFeed; | ||
|
||
/** | ||
* @var LoggerInterface | ||
*/ | ||
private $logger; | ||
|
||
/** | ||
* @var \Magento\Framework\App\DeploymentConfig | ||
*/ | ||
private $deploymentConfig; | ||
|
||
/** | ||
* @var \Magento\CatalogExport\Model\Data\VariantFactory | ||
*/ | ||
private $variantFactory; | ||
|
||
/** | ||
* @param \Magento\DataExporter\Model\FeedInterface $variantFeed | ||
* @param \Magento\CatalogExport\Model\Data\VariantFactory $variantFactory | ||
* @param \Magento\Framework\App\DeploymentConfig $deploymentConfig | ||
* @param LoggerInterface $logger | ||
*/ | ||
public function __construct( | ||
\Magento\DataExporter\Model\FeedInterface $variantFeed, | ||
\Magento\CatalogExport\Model\Data\VariantFactory $variantFactory, | ||
\Magento\Framework\App\DeploymentConfig $deploymentConfig, | ||
LoggerInterface $logger | ||
) { | ||
$this->variantFeed = $variantFeed; | ||
$this->variantFactory = $variantFactory; | ||
$this->deploymentConfig = $deploymentConfig; | ||
$this->logger = $logger; | ||
} | ||
|
||
/** | ||
* @inheritDoc | ||
*/ | ||
public function get(array $ids): array | ||
{ | ||
$this->validateIds($ids); | ||
|
||
$variants = []; | ||
foreach ($this->fetchData($ids) as $feedItem) { | ||
$feedItem['id'] = $feedItem['productId']; | ||
$variants[] = $this->variantFactory->create($feedItem); | ||
} | ||
|
||
return $variants; | ||
} | ||
|
||
/** | ||
* Get max items in response. | ||
* | ||
* @return int | ||
*/ | ||
private function getMaxItemsInResponse() | ||
{ | ||
$maxItemsInResponse = (int) $this->deploymentConfig->get(self::CATALOG_EXPORT_MAX_VARIANTS_IN_RESPONSE); | ||
return $maxItemsInResponse ?: self::MAX_ITEMS_IN_RESPONSE; | ||
} | ||
|
||
/** | ||
* Validate ids input array. | ||
* | ||
* @param string[] $ids | ||
* @throws \InvalidArgumentException | ||
*/ | ||
private function validateIds(array $ids): void | ||
{ | ||
if (count($ids) > $this->getMaxItemsInResponse()) { | ||
throw new \InvalidArgumentException( | ||
'Max items in the response can\'t exceed ' | ||
. $this->getMaxItemsInResponse() | ||
. '.' | ||
); | ||
} | ||
} | ||
|
||
/** | ||
* Retrieve list of variants' data from Variant feed by ids. | ||
* | ||
* @param string[] $ids | ||
* @return array | ||
*/ | ||
private function fetchData(array $ids): array | ||
{ | ||
$feedData = $this->variantFeed->getFeedByIds($ids); | ||
if (empty($feedData['feed'])) { | ||
$this->logger->error(\sprintf('Cannot find Variants in feed with ids "%s"', $ids)); | ||
} | ||
|
||
return $feedData['feed']; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
<?php | ||
/** | ||
* Copyright © Magento, Inc. All rights reserved. | ||
* See COPYING.txt for license details. | ||
*/ | ||
declare(strict_types=1); | ||
|
||
namespace Magento\CatalogExport\Model; | ||
|
||
use Magento\CatalogExportApi\Api\Data\VariantInterface; | ||
use Magento\CatalogExportApi\Api\VariantRepositoryInterface; | ||
use Magento\Framework\Reflection\DataObjectProcessor; | ||
|
||
/** | ||
* Decorate product variants repository to return data as plain array. | ||
*/ | ||
class VariantRepositoryArrayDecorator implements VariantRepositoryInterface | ||
{ | ||
/** | ||
* @var VariantRepositoryInterface | ||
*/ | ||
private $variantRepository; | ||
|
||
/** | ||
* @var DataObjectProcessor | ||
*/ | ||
private $dataObjectProcessor; | ||
|
||
/** | ||
* @param VariantRepositoryInterface $variantRepository | ||
* @param DataObjectProcessor $dataObjectProcessor | ||
*/ | ||
public function __construct( | ||
VariantRepositoryInterface $variantRepository, | ||
DataObjectProcessor $dataObjectProcessor | ||
) { | ||
$this->variantRepository = $variantRepository; | ||
$this->dataObjectProcessor = $dataObjectProcessor; | ||
} | ||
|
||
/** | ||
* @inheritDoc | ||
*/ | ||
public function get(array $ids): array | ||
{ | ||
$data = []; | ||
$products = $this->variantRepository->get($ids); | ||
foreach ($products as $product) { | ||
$data[] = $this->dataObjectProcessor->buildOutputDataArray($product, VariantInterface::class); | ||
} | ||
return $data; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our static tests will possible failed for inheritDoc (with camel case)