From fb2013753007eb6d3043e517ed743f962a8ca9c1 Mon Sep 17 00:00:00 2001 From: Dmytro Horytskyi <dhorytskyi@magento.com> Date: Thu, 28 Sep 2017 18:03:43 +0300 Subject: [PATCH 1/9] MAGETWO-71554: Category edit performance issue - for 2.2 --- .../Category/Product/PositionResolver.php | 59 +++++++++ .../Category/Product/PositionResolverTest.php | 112 ++++++++++++++++++ .../CatalogRule/Plugin/Indexer/Category.php | 2 +- .../Unit/Model/Indexer/IndexBuilderTest.php | 86 ++++++++++++-- .../Observer/UrlRewriteHandler.php | 73 ++++++------ .../Unit/Observer/UrlRewriteHandlerTest.php | 11 +- 6 files changed, 298 insertions(+), 45 deletions(-) create mode 100644 app/code/Magento/Catalog/Model/Category/Product/PositionResolver.php create mode 100644 app/code/Magento/Catalog/Test/Unit/Model/Category/Product/PositionResolverTest.php diff --git a/app/code/Magento/Catalog/Model/Category/Product/PositionResolver.php b/app/code/Magento/Catalog/Model/Category/Product/PositionResolver.php new file mode 100644 index 0000000000000..97941f2d23b9f --- /dev/null +++ b/app/code/Magento/Catalog/Model/Category/Product/PositionResolver.php @@ -0,0 +1,59 @@ +<?php +/** + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. + */ +namespace Magento\Catalog\Model\Category\Product; + +/** + * Resolver to get product positions by ids assigned to specific category + */ +class PositionResolver extends \Magento\Framework\Model\ResourceModel\Db\AbstractDb +{ + /** + * @param \Magento\Framework\Model\ResourceModel\Db\Context $context + * @param string $connectionName + */ + public function __construct( + \Magento\Framework\Model\ResourceModel\Db\Context $context, + $connectionName = null + ) { + parent::__construct($context, $connectionName); + } + + /** + * Initialize resource model + * + * @return void + */ + protected function _construct() + { + $this->_init('catalog_product_entity', 'entity_id'); + } + + /** + * Get category product positions + * + * @param int $categoryId + * @return array + */ + public function getPositions(int $categoryId) + { + $connection = $this->getConnection(); + + $select = $connection->select()->from( + ['cpe' => $this->getTable('catalog_product_entity')], + 'entity_id' + )->joinLeft( + ['ccp' => $this->getTable('catalog_category_product')], + 'ccp.product_id=cpe.entity_id' + )->where( + 'ccp.category_id = ?', + $categoryId + )->order( + 'ccp.position ' . \Magento\Framework\DB\Select::SQL_ASC + ); + + return array_flip($connection->fetchCol($select)); + } +} diff --git a/app/code/Magento/Catalog/Test/Unit/Model/Category/Product/PositionResolverTest.php b/app/code/Magento/Catalog/Test/Unit/Model/Category/Product/PositionResolverTest.php new file mode 100644 index 0000000000000..fda905321d841 --- /dev/null +++ b/app/code/Magento/Catalog/Test/Unit/Model/Category/Product/PositionResolverTest.php @@ -0,0 +1,112 @@ +<?php +/** + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. + */ +namespace Magento\Catalog\Test\Unit\Model\Category\Product; + +use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; +use Magento\Catalog\Model\Category\Product\PositionResolver; +use Magento\Framework\Model\ResourceModel\Db\Context; +use Magento\Framework\App\ResourceConnection; +use Magento\Framework\DB\Adapter\AdapterInterface; +use Magento\Framework\DB\Select; + +class PositionResolverTest extends \PHPUnit\Framework\TestCase +{ + /** + * @var Context|\PHPUnit_Framework_MockObject_MockObject + */ + private $context; + + /** + * @var ResourceConnection|\PHPUnit_Framework_MockObject_MockObject + */ + private $resources; + + /** + * @var AdapterInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $connection; + + /** + * @var Select|\PHPUnit_Framework_MockObject_MockObject + */ + private $select; + + /** + * @var PositionResolver + */ + private $model; + + /** + * @var array + */ + private $positions = [ + '3' => 100, + '2' => 101, + '1' => 102 + ]; + + private $flippedPositions = [ + '100' => 3, + '101' => 2, + '102' => 1 + ]; + + protected function setUp() + { + $this->context = $this->getMockBuilder(Context::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->resources = $this->getMockBuilder(ResourceConnection::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->connection = $this->getMockBuilder(AdapterInterface::class) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + + $this->select = $this->getMockBuilder(Select::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->model = (new ObjectManager($this))->getObject( + PositionResolver::class, + [ + 'context' => $this->context, + null, + '_resources' => $this->resources + ] + ); + } + + public function testGetPositions() + { + $this->resources->expects($this->once()) + ->method('getConnection') + ->willReturn($this->connection); + + $this->connection->expects($this->once()) + ->method('select') + ->willReturn($this->select); + $this->select->expects($this->once()) + ->method('from') + ->willReturnSelf(); + $this->select->expects($this->once()) + ->method('where') + ->willReturnSelf(); + $this->select->expects($this->once()) + ->method('order') + ->willReturnSelf(); + $this->select->expects($this->once()) + ->method('joinLeft') + ->willReturnSelf(); + $this->connection->expects($this->once()) + ->method('fetchCol') + ->willReturn($this->positions); + + $this->assertEquals($this->flippedPositions, $this->model->getPositions(1)); + } +} diff --git a/app/code/Magento/CatalogRule/Plugin/Indexer/Category.php b/app/code/Magento/CatalogRule/Plugin/Indexer/Category.php index 8bcc1fd8d6834..0ea0fdda31958 100644 --- a/app/code/Magento/CatalogRule/Plugin/Indexer/Category.php +++ b/app/code/Magento/CatalogRule/Plugin/Indexer/Category.php @@ -36,7 +36,7 @@ public function afterSave( ) { /** @var \Magento\Catalog\Model\Category $result */ $productIds = $result->getAffectedProductIds(); - if ($productIds) { + if ($productIds && !$this->productRuleProcessor->isIndexerScheduled()) { $this->productRuleProcessor->reindexList($productIds); } return $result; diff --git a/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php b/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php index 997cf704a8657..97cf050130e37 100644 --- a/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php +++ b/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php @@ -6,6 +6,12 @@ namespace Magento\CatalogRule\Test\Unit\Model\Indexer; +use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; +use Magento\Catalog\Api\ProductRepositoryInterface; +use Magento\Framework\Api\SearchCriteriaBuilder; +use Magento\Catalog\Api\Data\ProductSearchResultsInterface; +use Magento\Framework\Api\SearchCriteria; + /** * @SuppressWarnings(PHPMD.CouplingBetweenObjects) * @SuppressWarnings(PHPMD.TooManyFields) @@ -112,6 +118,26 @@ class IndexBuilderTest extends \PHPUnit\Framework\TestCase */ protected $backend; + /** + * @var ProductRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $productRepository; + + /** + * @var SearchCriteriaBuilder|\PHPUnit_Framework_MockObject_MockObject + */ + private $searchCriteriaBuilder; + + /** + * @var ProductSearchResultsInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $productSearchResults; + + /** + * @var SearchCriteria|\PHPUnit_Framework_MockObject_MockObject + */ + private $searchCriteria; + /** * @var \PHPUnit_Framework_MockObject_MockObject */ @@ -171,28 +197,53 @@ protected function setUp() $this->rules->expects($this->any())->method('getId')->will($this->returnValue(1)); $this->rules->expects($this->any())->method('getWebsiteIds')->will($this->returnValue([1])); $this->rules->expects($this->any())->method('getCustomerGroupIds')->will($this->returnValue([1])); + $this->ruleCollectionFactory->expects($this->any())->method('create')->will($this->returnSelf()); $this->ruleCollectionFactory->expects($this->any())->method('addFieldToFilter')->will( $this->returnValue([$this->rules]) ); + $this->product->expects($this->any())->method('load')->will($this->returnSelf()); $this->product->expects($this->any())->method('getId')->will($this->returnValue(1)); $this->product->expects($this->any())->method('getWebsiteIds')->will($this->returnValue([1])); + $this->rules->expects($this->any())->method('validate')->with($this->product)->willReturn(true); $this->attribute->expects($this->any())->method('getBackend')->will($this->returnValue($this->backend)); $this->productFactory->expects($this->any())->method('create')->will($this->returnValue($this->product)); - $this->indexBuilder = new \Magento\CatalogRule\Model\Indexer\IndexBuilder( - $this->ruleCollectionFactory, - $this->priceCurrency, - $this->resource, - $this->storeManager, - $this->logger, - $this->eavConfig, - $this->dateFormat, - $this->dateTime, - $this->productFactory + $this->productRepository = $this->getMockBuilder(ProductRepositoryInterface::class) + ->disableOriginalConstructor() + ->getMock(); + $this->searchCriteriaBuilder = $this->getMockBuilder(SearchCriteriaBuilder::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->productSearchResults = $this->getMockBuilder(ProductSearchResultsInterface::class) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + + $this->searchCriteria = $this->getMockBuilder(SearchCriteria::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->indexBuilder = (new ObjectManager($this))->getObject( + \Magento\CatalogRule\Model\Indexer\IndexBuilder::class, + [ + 'ruleCollectionFactory' => $this->ruleCollectionFactory, + 'priceCurrency' => $this->priceCurrency, + 'resource' => $this->resource, + 'storeManager' => $this->storeManager, + 'logger' => $this->logger, + 'eavConfig' => $this->eavConfig, + 'dateFormat' => $this->dateFormat, + 'dateTime' => $this->dateTime, + 'productFactory' => $this->productFactory, + 1000, + 'productRepository' => $this->productRepository, + 'searchCriteriaBuilder' => $this->searchCriteriaBuilder + ] ); + $this->reindexRuleProductPrice = $this->getMockBuilder(\Magento\CatalogRule\Model\Indexer\ReindexRuleProductPrice::class) ->disableOriginalConstructor() @@ -235,6 +286,21 @@ public function testUpdateCatalogRuleGroupWebsiteData() ->method('getBackend') ->will($this->returnValue($backendModelMock)); + $iterator = new \ArrayIterator([$this->product]); + $this->searchCriteriaBuilder->expects($this->once()) + ->method('addFilter') + ->willReturnSelf(); + $this->searchCriteriaBuilder->expects($this->once()) + ->method('create') + ->willReturn($this->searchCriteria); + $this->productRepository->expects($this->once()) + ->method('getList') + ->with($this->searchCriteria) + ->willReturn($this->productSearchResults); + $this->productSearchResults->expects($this->once()) + ->method('getItems') + ->willReturn($iterator); + $this->reindexRuleProductPrice->expects($this->once())->method('execute')->willReturn(true); $this->reindexRuleGroupWebsite->expects($this->once())->method('execute')->willReturn(true); diff --git a/app/code/Magento/CatalogUrlRewrite/Observer/UrlRewriteHandler.php b/app/code/Magento/CatalogUrlRewrite/Observer/UrlRewriteHandler.php index 5a6777f94e2d5..964e0a9afb6e9 100644 --- a/app/code/Magento/CatalogUrlRewrite/Observer/UrlRewriteHandler.php +++ b/app/code/Magento/CatalogUrlRewrite/Observer/UrlRewriteHandler.php @@ -104,42 +104,49 @@ public function generateProductUrlRewrites(\Magento\Catalog\Model\Category $cate $this->isSkippedProduct[$category->getEntityId()] = []; $saveRewriteHistory = $category->getData('save_rewrites_history'); $storeId = $category->getStoreId(); - if ($category->getAffectedProductIds()) { - $this->isSkippedProduct[$category->getEntityId()] = $category->getAffectedProductIds(); - /* @var \Magento\Catalog\Model\ResourceModel\Product\Collection $collection */ - $collection = $this->productCollectionFactory->create() - ->setStoreId($storeId) - ->addIdFilter($category->getAffectedProductIds()) - ->addAttributeToSelect('visibility') - ->addAttributeToSelect('name') - ->addAttributeToSelect('url_key') - ->addAttributeToSelect('url_path'); - - $collection->setPageSize(1000); - $pageCount = $collection->getLastPageNumber(); - $currentPage = 1; - while ($currentPage <= $pageCount) { - $collection->setCurPage($currentPage); - foreach ($collection as $product) { - $product->setStoreId($storeId); - $product->setData('save_rewrites_history', $saveRewriteHistory); - $mergeDataProvider->merge( - $this->productUrlRewriteGenerator->generate($product, $category->getEntityId()) - ); + + if ($category->dataHasChangedFor('name') + || $category->dataHasChangedFor('url_key') + || $category->dataHasChangedFor('url_path') + ) { + if ($category->getAffectedProductIds()) { + $this->isSkippedProduct[$category->getEntityId()] = $category->getAffectedProductIds(); + /* @var \Magento\Catalog\Model\ResourceModel\Product\Collection $collection */ + $collection = $this->productCollectionFactory->create() + ->setStoreId($storeId) + ->addIdFilter($category->getAffectedProductIds()) + ->addAttributeToSelect('visibility') + ->addAttributeToSelect('name') + ->addAttributeToSelect('url_key') + ->addAttributeToSelect('url_path'); + + $collection->setPageSize(1000); + $pageCount = $collection->getLastPageNumber(); + $currentPage = 1; + while ($currentPage <= $pageCount) { + $collection->setCurPage($currentPage); + foreach ($collection as $product) { + $product->setStoreId($storeId); + $product->setData('save_rewrites_history', $saveRewriteHistory); + $mergeDataProvider->merge( + $this->productUrlRewriteGenerator->generate($product, $category->getEntityId()) + ); + } + $collection->clear(); + $currentPage++; } - $collection->clear(); - $currentPage++; + } else { + $mergeDataProvider->merge( + $this->getCategoryProductsUrlRewrites( + $category, + $storeId, + $saveRewriteHistory, + $category->getEntityId() + ) + ); } - } else { - $mergeDataProvider->merge( - $this->getCategoryProductsUrlRewrites( - $category, - $storeId, - $saveRewriteHistory, - $category->getEntityId() - ) - ); } + foreach ($this->childrenCategoriesProvider->getChildren($category, true) as $childCategory) { $mergeDataProvider->merge( $this->getCategoryProductsUrlRewrites( diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/UrlRewriteHandlerTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/UrlRewriteHandlerTest.php index 747e0cfa771fd..441a26da9dd12 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/UrlRewriteHandlerTest.php +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/UrlRewriteHandlerTest.php @@ -127,7 +127,13 @@ public function testGenerateProductUrlRewrites() { /* @var \Magento\Catalog\Model\Category|\PHPUnit_Framework_MockObject_MockObject $category */ $category = $this->getMockBuilder(\Magento\Catalog\Model\Category::class) - ->setMethods(['getEntityId', 'getStoreId', 'getData', 'getAffectedProductIds']) + ->setMethods([ + 'getEntityId', + 'getStoreId', + 'getData', + 'getAffectedProductIds', + 'dataHasChangedFor', + ]) ->disableOriginalConstructor() ->getMock(); $category->expects($this->any()) @@ -136,6 +142,9 @@ public function testGenerateProductUrlRewrites() $category->expects($this->any()) ->method('getStoreId') ->willReturn(1); + $category->expects($this->atLeastOnce()) + ->method('dataHasChangedFor') + ->willReturn('true'); $category->expects($this->any()) ->method('getData') ->with('save_rewrites_history') From 252a706b652f2d44c65110a68f7dadb4ef39f427 Mon Sep 17 00:00:00 2001 From: Dmytro Horytskyi <dhorytskyi@magento.com> Date: Thu, 28 Sep 2017 19:00:25 +0300 Subject: [PATCH 2/9] MAGETWO-71554: Category edit performance issue - for 2.2 --- .../CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php b/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php index 97cf050130e37..4d87658136e44 100644 --- a/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php +++ b/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php @@ -152,6 +152,7 @@ class IndexBuilderTest extends \PHPUnit\Framework\TestCase * Set up test * * @return void + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ protected function setUp() { From d56861a58b3315c1fdbf693d9d910cd1c2de8c2c Mon Sep 17 00:00:00 2001 From: Dmytro Horytskyi <dhorytskyi@magento.com> Date: Tue, 3 Oct 2017 17:14:43 +0300 Subject: [PATCH 3/9] MAGETWO-71554: Category edit performance issue - for 2.2 --- app/code/Magento/Catalog/Model/Category.php | 2 + .../Catalog/Model/ResourceModel/Category.php | 2 + .../Model/Indexer/IndexBuilder.php | 17 +++- .../Indexer/IndexBuilder/ProductLoader.php | 53 ++++++++++ .../IndexBuilder/ProductLoaderTest.php | 96 +++++++++++++++++++ .../Unit/Model/Indexer/IndexBuilderTest.php | 69 +++---------- ...ategoryProcessUrlRewriteSavingObserver.php | 2 +- .../Observer/UrlRewriteHandler.php | 71 +++++++------- 8 files changed, 215 insertions(+), 97 deletions(-) create mode 100644 app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder/ProductLoader.php create mode 100644 app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilder/ProductLoaderTest.php diff --git a/app/code/Magento/Catalog/Model/Category.php b/app/code/Magento/Catalog/Model/Category.php index c4d6742a02dd3..40677b2cad1f7 100644 --- a/app/code/Magento/Catalog/Model/Category.php +++ b/app/code/Magento/Catalog/Model/Category.php @@ -28,6 +28,8 @@ * @method Category setUrlPath(string $urlPath) * @method Category getSkipDeleteChildren() * @method Category setSkipDeleteChildren(boolean $value) + * @method Category setChangedProductIds(array $categoryIds) Set products ids that inserted or deleted for category + * @method array getChangedProductIds() Get products ids that inserted or deleted for category * * @SuppressWarnings(PHPMD.LongVariable) * @SuppressWarnings(PHPMD.ExcessivePublicCount) diff --git a/app/code/Magento/Catalog/Model/ResourceModel/Category.php b/app/code/Magento/Catalog/Model/ResourceModel/Category.php index a9c705697b268..46faa47c1bc0c 100644 --- a/app/code/Magento/Catalog/Model/ResourceModel/Category.php +++ b/app/code/Magento/Catalog/Model/ResourceModel/Category.php @@ -423,6 +423,8 @@ protected function _saveCategoryProducts($category) 'catalog_category_change_products', ['category' => $category, 'product_ids' => $productIds] ); + + $category->setChangedProductIds($productIds); } if (!empty($insert) || !empty($update) || !empty($delete)) { diff --git a/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php b/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php index 34b4bff5a060e..9501a0080a058 100644 --- a/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php +++ b/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php @@ -11,6 +11,7 @@ use Magento\CatalogRule\Model\Rule; use Magento\Framework\App\ObjectManager; use Magento\Framework\Pricing\PriceCurrencyInterface; +use Magento\CatalogRule\Model\Indexer\IndexBuilder\ProductLoader; /** * @api @@ -135,6 +136,11 @@ class IndexBuilder */ private $activeTableSwitcher; + /** + * @var ProductLoader + */ + private $productLoader; + /** * @param RuleCollectionFactory $ruleCollectionFactory * @param PriceCurrencyInterface $priceCurrency @@ -153,6 +159,7 @@ class IndexBuilder * @param \Magento\CatalogRule\Model\Indexer\ReindexRuleProductPrice|null $reindexRuleProductPrice * @param \Magento\CatalogRule\Model\Indexer\RuleProductPricesPersistor|null $pricesPersistor * @param \Magento\Catalog\Model\ResourceModel\Indexer\ActiveTableSwitcher|null $activeTableSwitcher + * @param ProductLoader|null $productLoader * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ public function __construct( @@ -172,7 +179,8 @@ public function __construct( \Magento\CatalogRule\Model\Indexer\RuleProductsSelectBuilder $ruleProductsSelectBuilder = null, \Magento\CatalogRule\Model\Indexer\ReindexRuleProductPrice $reindexRuleProductPrice = null, \Magento\CatalogRule\Model\Indexer\RuleProductPricesPersistor $pricesPersistor = null, - \Magento\Catalog\Model\ResourceModel\Indexer\ActiveTableSwitcher $activeTableSwitcher = null + \Magento\Catalog\Model\ResourceModel\Indexer\ActiveTableSwitcher $activeTableSwitcher = null, + ProductLoader $productLoader = null ) { $this->resource = $resource; $this->connection = $resource->getConnection(); @@ -207,6 +215,8 @@ public function __construct( $this->activeTableSwitcher = $activeTableSwitcher ?: ObjectManager::getInstance()->get( \Magento\Catalog\Model\ResourceModel\Indexer\ActiveTableSwitcher::class ); + $this->productLoader = $productLoader ?: + ObjectManager::getInstance()->get(ProductLoader::class); } /** @@ -251,9 +261,10 @@ protected function doReindexByIds($ids) { $this->cleanByIds($ids); + $products = $this->productLoader->getProducts($ids); foreach ($this->getActiveRules() as $rule) { - foreach ($ids as $productId) { - $this->applyRule($rule, $this->getProduct($productId)); + foreach ($products as $product) { + $this->applyRule($rule, $product); } } } diff --git a/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder/ProductLoader.php b/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder/ProductLoader.php new file mode 100644 index 0000000000000..bce2bea2fd952 --- /dev/null +++ b/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder/ProductLoader.php @@ -0,0 +1,53 @@ +<?php +/** + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. + */ +namespace Magento\CatalogRule\Model\Indexer\IndexBuilder; + +use Magento\Catalog\Api\ProductRepositoryInterface; +use Magento\Framework\Api\SearchCriteriaBuilder; +use Magento\Catalog\Api\Data\ProductInterface; + +/** + * Product loader that gets products by ids via product repository + */ +class ProductLoader +{ + /** + * @var ProductRepositoryInterface + */ + private $productRepository; + + /** + * @var SearchCriteriaBuilder + */ + private $searchCriteriaBuilder; + + /** + * @param ProductRepositoryInterface $productRepository + * @param SearchCriteriaBuilder $searchCriteriaBuilder + */ + public function __construct( + ProductRepositoryInterface $productRepository, + SearchCriteriaBuilder $searchCriteriaBuilder + ) { + $this->productRepository = $productRepository; + $this->searchCriteriaBuilder = $searchCriteriaBuilder; + } + + /** + * Get products by ids + * + * @param array $productIds + * @return ProductInterface[] + */ + public function getProducts($productIds) + { + $this->searchCriteriaBuilder->addFilter('entity_id', $productIds, 'in'); + $searchCriteria = $this->searchCriteriaBuilder->create(); + $products = $this->productRepository->getList($searchCriteria)->getItems(); + + return $products; + } +} diff --git a/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilder/ProductLoaderTest.php b/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilder/ProductLoaderTest.php new file mode 100644 index 0000000000000..3509128da79da --- /dev/null +++ b/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilder/ProductLoaderTest.php @@ -0,0 +1,96 @@ +<?php +/** + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. + */ + +namespace Magento\CatalogRule\Test\Unit\Model\Indexer\IndexBuilder; + +use Magento\CatalogRule\Model\Indexer\IndexBuilder\ProductLoader; +use Magento\Catalog\Api\ProductRepositoryInterface; +use Magento\Framework\Api\SearchCriteriaBuilder; +use Magento\Catalog\Api\Data\ProductSearchResultsInterface; +use Magento\Catalog\Model\Product; +use Magento\Framework\Api\SearchCriteria; + +class ProductLoaderTest extends \PHPUnit\Framework\TestCase +{ + /** + * @var ProductLoader + */ + protected $productLoader; + + /** + * @var ProductRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $productRepository; + + /** + * @var SearchCriteriaBuilder|\PHPUnit_Framework_MockObject_MockObject + */ + private $searchCriteriaBuilder; + + /** + * @var ProductSearchResultsInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $productSearchResultsInterface; + + /** + * @var \Magento\Framework\Api\SearchCriteria|\PHPUnit_Framework_MockObject_MockObject + */ + private $searchCriteria; + + /** + * @var Product|\PHPUnit_Framework_MockObject_MockObject + */ + protected $product; + + /** + * Set up test + * + * @return void + */ + protected function setUp() + { + $this->productRepository = $this->getMockBuilder(ProductRepositoryInterface::class) + ->disableOriginalConstructor() + ->getMock(); + $this->searchCriteriaBuilder = $this->getMockBuilder(SearchCriteriaBuilder::class) + ->disableOriginalConstructor() + ->getMock(); + $this->productSearchResultsInterface = $this->getMockBuilder(ProductSearchResultsInterface::class) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + $this->searchCriteria = $this->getMockBuilder(SearchCriteria::class) + ->disableOriginalConstructor() + ->getMock(); + $this->product = $this->getMockBuilder(Product::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->productLoader = new ProductLoader( + $this->productRepository, + $this->searchCriteriaBuilder + ); + } + + public function testGetProducts() + { + $this->searchCriteriaBuilder->expects($this->once()) + ->method('addFilter') + ->willReturnSelf(); + $this->searchCriteriaBuilder->expects($this->once()) + ->method('create') + ->willReturn($this->searchCriteria); + $this->productRepository->expects($this->once()) + ->method('getList') + ->with($this->searchCriteria) + ->willReturn($this->productSearchResultsInterface); + $iterator = new \ArrayIterator([$this->product]); + $this->productSearchResultsInterface->expects($this->once()) + ->method('getItems') + ->willReturn($iterator); + + $this->assertSame($iterator, $this->productLoader->getProducts([1])); + } +} diff --git a/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php b/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php index 4d87658136e44..b54af35a571f5 100644 --- a/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php +++ b/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php @@ -6,11 +6,8 @@ namespace Magento\CatalogRule\Test\Unit\Model\Indexer; +use Magento\CatalogRule\Model\Indexer\IndexBuilder\ProductLoader; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; -use Magento\Catalog\Api\ProductRepositoryInterface; -use Magento\Framework\Api\SearchCriteriaBuilder; -use Magento\Catalog\Api\Data\ProductSearchResultsInterface; -use Magento\Framework\Api\SearchCriteria; /** * @SuppressWarnings(PHPMD.CouplingBetweenObjects) @@ -118,26 +115,6 @@ class IndexBuilderTest extends \PHPUnit\Framework\TestCase */ protected $backend; - /** - * @var ProductRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject - */ - private $productRepository; - - /** - * @var SearchCriteriaBuilder|\PHPUnit_Framework_MockObject_MockObject - */ - private $searchCriteriaBuilder; - - /** - * @var ProductSearchResultsInterface|\PHPUnit_Framework_MockObject_MockObject - */ - private $productSearchResults; - - /** - * @var SearchCriteria|\PHPUnit_Framework_MockObject_MockObject - */ - private $searchCriteria; - /** * @var \PHPUnit_Framework_MockObject_MockObject */ @@ -148,6 +125,11 @@ class IndexBuilderTest extends \PHPUnit\Framework\TestCase */ private $reindexRuleGroupWebsite; + /** + * @var ProductLoader|\PHPUnit_Framework_MockObject_MockObject + */ + private $productLoader; + /** * Set up test * @@ -211,19 +193,7 @@ protected function setUp() $this->rules->expects($this->any())->method('validate')->with($this->product)->willReturn(true); $this->attribute->expects($this->any())->method('getBackend')->will($this->returnValue($this->backend)); $this->productFactory->expects($this->any())->method('create')->will($this->returnValue($this->product)); - - $this->productRepository = $this->getMockBuilder(ProductRepositoryInterface::class) - ->disableOriginalConstructor() - ->getMock(); - $this->searchCriteriaBuilder = $this->getMockBuilder(SearchCriteriaBuilder::class) - ->disableOriginalConstructor() - ->getMock(); - - $this->productSearchResults = $this->getMockBuilder(ProductSearchResultsInterface::class) - ->disableOriginalConstructor() - ->getMockForAbstractClass(); - - $this->searchCriteria = $this->getMockBuilder(SearchCriteria::class) + $this->productLoader = $this->getMockBuilder(ProductLoader::class) ->disableOriginalConstructor() ->getMock(); @@ -240,19 +210,18 @@ protected function setUp() 'dateTime' => $this->dateTime, 'productFactory' => $this->productFactory, 1000, - 'productRepository' => $this->productRepository, - 'searchCriteriaBuilder' => $this->searchCriteriaBuilder + 'productLoader' => $this->productLoader ] ); $this->reindexRuleProductPrice = $this->getMockBuilder(\Magento\CatalogRule\Model\Indexer\ReindexRuleProductPrice::class) - ->disableOriginalConstructor() - ->getMock(); + ->disableOriginalConstructor() + ->getMock(); $this->reindexRuleGroupWebsite = $this->getMockBuilder(\Magento\CatalogRule\Model\Indexer\ReindexRuleGroupWebsite::class) - ->disableOriginalConstructor() - ->getMock(); + ->disableOriginalConstructor() + ->getMock(); $this->setProperties($this->indexBuilder, [ 'metadataPool' => $this->metadataPool, 'reindexRuleProductPrice' => $this->reindexRuleProductPrice, @@ -288,18 +257,8 @@ public function testUpdateCatalogRuleGroupWebsiteData() ->will($this->returnValue($backendModelMock)); $iterator = new \ArrayIterator([$this->product]); - $this->searchCriteriaBuilder->expects($this->once()) - ->method('addFilter') - ->willReturnSelf(); - $this->searchCriteriaBuilder->expects($this->once()) - ->method('create') - ->willReturn($this->searchCriteria); - $this->productRepository->expects($this->once()) - ->method('getList') - ->with($this->searchCriteria) - ->willReturn($this->productSearchResults); - $this->productSearchResults->expects($this->once()) - ->method('getItems') + $this->productLoader->expects($this->once()) + ->method('getProducts') ->willReturn($iterator); $this->reindexRuleProductPrice->expects($this->once())->method('execute')->willReturn(true); diff --git a/app/code/Magento/CatalogUrlRewrite/Observer/CategoryProcessUrlRewriteSavingObserver.php b/app/code/Magento/CatalogUrlRewrite/Observer/CategoryProcessUrlRewriteSavingObserver.php index 92a46facbe71c..539e5c3f42f15 100644 --- a/app/code/Magento/CatalogUrlRewrite/Observer/CategoryProcessUrlRewriteSavingObserver.php +++ b/app/code/Magento/CatalogUrlRewrite/Observer/CategoryProcessUrlRewriteSavingObserver.php @@ -85,7 +85,7 @@ public function execute(\Magento\Framework\Event\Observer $observer) $mapsGenerated = false; if ($category->dataHasChangedFor('url_key') || $category->dataHasChangedFor('is_anchor') - || $category->getIsChangedProductList() + || $category->getChangedProductIds() ) { if ($category->dataHasChangedFor('url_key')) { $categoryUrlRewriteResult = $this->categoryUrlRewriteGenerator->generate($category); diff --git a/app/code/Magento/CatalogUrlRewrite/Observer/UrlRewriteHandler.php b/app/code/Magento/CatalogUrlRewrite/Observer/UrlRewriteHandler.php index 964e0a9afb6e9..9892465d1538a 100644 --- a/app/code/Magento/CatalogUrlRewrite/Observer/UrlRewriteHandler.php +++ b/app/code/Magento/CatalogUrlRewrite/Observer/UrlRewriteHandler.php @@ -105,46 +105,41 @@ public function generateProductUrlRewrites(\Magento\Catalog\Model\Category $cate $saveRewriteHistory = $category->getData('save_rewrites_history'); $storeId = $category->getStoreId(); - if ($category->dataHasChangedFor('name') - || $category->dataHasChangedFor('url_key') - || $category->dataHasChangedFor('url_path') - ) { - if ($category->getAffectedProductIds()) { - $this->isSkippedProduct[$category->getEntityId()] = $category->getAffectedProductIds(); - /* @var \Magento\Catalog\Model\ResourceModel\Product\Collection $collection */ - $collection = $this->productCollectionFactory->create() - ->setStoreId($storeId) - ->addIdFilter($category->getAffectedProductIds()) - ->addAttributeToSelect('visibility') - ->addAttributeToSelect('name') - ->addAttributeToSelect('url_key') - ->addAttributeToSelect('url_path'); - - $collection->setPageSize(1000); - $pageCount = $collection->getLastPageNumber(); - $currentPage = 1; - while ($currentPage <= $pageCount) { - $collection->setCurPage($currentPage); - foreach ($collection as $product) { - $product->setStoreId($storeId); - $product->setData('save_rewrites_history', $saveRewriteHistory); - $mergeDataProvider->merge( - $this->productUrlRewriteGenerator->generate($product, $category->getEntityId()) - ); - } - $collection->clear(); - $currentPage++; + if ($category->getChangedProductIds()) { + $this->isSkippedProduct[$category->getEntityId()] = $category->getAffectedProductIds(); + /* @var \Magento\Catalog\Model\ResourceModel\Product\Collection $collection */ + $collection = $this->productCollectionFactory->create() + ->setStoreId($storeId) + ->addIdFilter($category->getAffectedProductIds()) + ->addAttributeToSelect('visibility') + ->addAttributeToSelect('name') + ->addAttributeToSelect('url_key') + ->addAttributeToSelect('url_path'); + + $collection->setPageSize(1000); + $pageCount = $collection->getLastPageNumber(); + $currentPage = 1; + while ($currentPage <= $pageCount) { + $collection->setCurPage($currentPage); + foreach ($collection as $product) { + $product->setStoreId($storeId); + $product->setData('save_rewrites_history', $saveRewriteHistory); + $mergeDataProvider->merge( + $this->productUrlRewriteGenerator->generate($product, $category->getEntityId()) + ); } - } else { - $mergeDataProvider->merge( - $this->getCategoryProductsUrlRewrites( - $category, - $storeId, - $saveRewriteHistory, - $category->getEntityId() - ) - ); + $collection->clear(); + $currentPage++; } + } else { + $mergeDataProvider->merge( + $this->getCategoryProductsUrlRewrites( + $category, + $storeId, + $saveRewriteHistory, + $category->getEntityId() + ) + ); } foreach ($this->childrenCategoriesProvider->getChildren($category, true) as $childCategory) { From 79f6a9590a04b71d3d00ebdb22885af053d77fb5 Mon Sep 17 00:00:00 2001 From: Dmytro Horytskyi <dhorytskyi@magento.com> Date: Tue, 3 Oct 2017 17:59:17 +0300 Subject: [PATCH 4/9] MAGETWO-71554: Category edit performance issue - for 2.2 --- .../CatalogRule/Model/Indexer/IndexBuilder.php | 1 + .../Test/Unit/Observer/UrlRewriteHandlerTest.php | 11 +---------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php b/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php index 9501a0080a058..a11f8ffe997a1 100644 --- a/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php +++ b/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php @@ -161,6 +161,7 @@ class IndexBuilder * @param \Magento\Catalog\Model\ResourceModel\Indexer\ActiveTableSwitcher|null $activeTableSwitcher * @param ProductLoader|null $productLoader * @SuppressWarnings(PHPMD.ExcessiveParameterList) + * @SuppressWarnings(PHPMD.NPathComplexity) */ public function __construct( RuleCollectionFactory $ruleCollectionFactory, diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/UrlRewriteHandlerTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/UrlRewriteHandlerTest.php index 441a26da9dd12..b18597a42bf94 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/UrlRewriteHandlerTest.php +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/UrlRewriteHandlerTest.php @@ -127,13 +127,7 @@ public function testGenerateProductUrlRewrites() { /* @var \Magento\Catalog\Model\Category|\PHPUnit_Framework_MockObject_MockObject $category */ $category = $this->getMockBuilder(\Magento\Catalog\Model\Category::class) - ->setMethods([ - 'getEntityId', - 'getStoreId', - 'getData', - 'getAffectedProductIds', - 'dataHasChangedFor', - ]) + ->setMethods(['getEntityId', 'getStoreId', 'getData', 'getChangedProductIds']) ->disableOriginalConstructor() ->getMock(); $category->expects($this->any()) @@ -142,9 +136,6 @@ public function testGenerateProductUrlRewrites() $category->expects($this->any()) ->method('getStoreId') ->willReturn(1); - $category->expects($this->atLeastOnce()) - ->method('dataHasChangedFor') - ->willReturn('true'); $category->expects($this->any()) ->method('getData') ->with('save_rewrites_history') From 30a123bfe9e89d96ad30cce8b400c08c91f5734c Mon Sep 17 00:00:00 2001 From: Dmytro Horytskyi <dhorytskyi@magento.com> Date: Wed, 4 Oct 2017 12:00:56 +0300 Subject: [PATCH 5/9] MAGETWO-71554: Category edit performance issue - for 2.2 --- .../Catalog/Model/ResourceModel/Category.php | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Catalog/Model/ResourceModel/Category.php b/app/code/Magento/Catalog/Model/ResourceModel/Category.php index 46faa47c1bc0c..6c9867359d40b 100644 --- a/app/code/Magento/Catalog/Model/ResourceModel/Category.php +++ b/app/code/Magento/Catalog/Model/ResourceModel/Category.php @@ -410,9 +410,18 @@ protected function _saveCategoryProducts($category) * Update product positions in category */ if (!empty($update)) { + $newPositions = []; foreach ($update as $productId => $position) { - $where = ['category_id = ?' => (int)$id, 'product_id = ?' => (int)$productId]; - $bind = ['position' => (int)$position]; + $delta = $position - $oldProducts[$productId]; + if (!isset($newPositions[$delta])) { + $newPositions[$delta] = []; + } + $newPositions[$delta][] = $productId; + } + + foreach ($newPositions as $delta => $productIds) { + $bind = ['position' => new \Zend_Db_Expr("position + ({$delta})")]; + $where = ['category_id = ?' => (int)$id, 'product_id IN (?)' => $productIds]; $connection->update($this->getCategoryProductTable(), $bind, $where); } } From 8c13297a36489b467e7fb1da331990a7f847c635 Mon Sep 17 00:00:00 2001 From: Dmytro Horytskyi <dhorytskyi@magento.com> Date: Fri, 6 Oct 2017 12:10:45 +0300 Subject: [PATCH 6/9] MAGETWO-71554: Category edit performance issue - for 2.2 --- .../Model/Category/Product/PositionResolverTest.php | 10 +++++++++- .../Test/Unit/Model/Indexer/IndexBuilderTest.php | 3 +-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/Catalog/Test/Unit/Model/Category/Product/PositionResolverTest.php b/app/code/Magento/Catalog/Test/Unit/Model/Category/Product/PositionResolverTest.php index fda905321d841..9545e5eb4b37d 100644 --- a/app/code/Magento/Catalog/Test/Unit/Model/Category/Product/PositionResolverTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Model/Category/Product/PositionResolverTest.php @@ -48,12 +48,20 @@ class PositionResolverTest extends \PHPUnit\Framework\TestCase '1' => 102 ]; + /** + * @var array + */ private $flippedPositions = [ '100' => 3, '101' => 2, '102' => 1 ]; + /** + * @var int + */ + private $categoryId = 1; + protected function setUp() { $this->context = $this->getMockBuilder(Context::class) @@ -107,6 +115,6 @@ public function testGetPositions() ->method('fetchCol') ->willReturn($this->positions); - $this->assertEquals($this->flippedPositions, $this->model->getPositions(1)); + $this->assertEquals($this->flippedPositions, $this->model->getPositions($this->categoryId)); } } diff --git a/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php b/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php index b54af35a571f5..8252b512e7810 100644 --- a/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php +++ b/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php @@ -209,8 +209,7 @@ protected function setUp() 'dateFormat' => $this->dateFormat, 'dateTime' => $this->dateTime, 'productFactory' => $this->productFactory, - 1000, - 'productLoader' => $this->productLoader + 'productLoader' => $this->productLoader, ] ); From 57100982dea89985fd3aa21b7192f03b89ac5ec3 Mon Sep 17 00:00:00 2001 From: Dmytro Horytskyi <dhorytskyi@magento.com> Date: Tue, 10 Oct 2017 13:03:46 +0300 Subject: [PATCH 7/9] MAGETWO-71554: Category edit performance issue - for 2.2 --- .../Model/Indexer/IndexBuilder.php | 82 +++++++++---------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php b/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php index a11f8ffe997a1..82be37049912f 100644 --- a/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php +++ b/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php @@ -102,32 +102,32 @@ class IndexBuilder protected $connection; /** - * @var \Magento\CatalogRule\Model\Indexer\ProductPriceCalculator + * @var ProductPriceCalculator */ private $productPriceCalculator; /** - * @var \Magento\CatalogRule\Model\Indexer\ReindexRuleProduct + * @var ReindexRuleProduct */ private $reindexRuleProduct; /** - * @var \Magento\CatalogRule\Model\Indexer\ReindexRuleGroupWebsite + * @var ReindexRuleGroupWebsite */ private $reindexRuleGroupWebsite; /** - * @var \Magento\CatalogRule\Model\Indexer\RuleProductsSelectBuilder + * @var RuleProductsSelectBuilder */ private $ruleProductsSelectBuilder; /** - * @var \Magento\CatalogRule\Model\Indexer\ReindexRuleProductPrice + * @var ReindexRuleProductPrice */ private $reindexRuleProductPrice; /** - * @var \Magento\CatalogRule\Model\Indexer\RuleProductPricesPersistor + * @var RuleProductPricesPersistor */ private $pricesPersistor; @@ -152,16 +152,15 @@ class IndexBuilder * @param \Magento\Framework\Stdlib\DateTime\DateTime $dateTime * @param \Magento\Catalog\Model\ProductFactory $productFactory * @param int $batchCount - * @param \Magento\CatalogRule\Model\Indexer\ProductPriceCalculator|null $productPriceCalculator - * @param \Magento\CatalogRule\Model\Indexer\ReindexRuleProduct|null $reindexRuleProduct - * @param \Magento\CatalogRule\Model\Indexer\ReindexRuleGroupWebsite|null $reindexRuleGroupWebsite - * @param \Magento\CatalogRule\Model\Indexer\RuleProductsSelectBuilder|null $ruleProductsSelectBuilder - * @param \Magento\CatalogRule\Model\Indexer\ReindexRuleProductPrice|null $reindexRuleProductPrice - * @param \Magento\CatalogRule\Model\Indexer\RuleProductPricesPersistor|null $pricesPersistor + * @param ProductPriceCalculator|null $productPriceCalculator + * @param ReindexRuleProduct|null $reindexRuleProduct + * @param ReindexRuleGroupWebsite|null $reindexRuleGroupWebsite + * @param RuleProductsSelectBuilder|null $ruleProductsSelectBuilder + * @param ReindexRuleProductPrice|null $reindexRuleProductPrice + * @param RuleProductPricesPersistor|null $pricesPersistor * @param \Magento\Catalog\Model\ResourceModel\Indexer\ActiveTableSwitcher|null $activeTableSwitcher * @param ProductLoader|null $productLoader * @SuppressWarnings(PHPMD.ExcessiveParameterList) - * @SuppressWarnings(PHPMD.NPathComplexity) */ public function __construct( RuleCollectionFactory $ruleCollectionFactory, @@ -174,12 +173,12 @@ public function __construct( \Magento\Framework\Stdlib\DateTime\DateTime $dateTime, \Magento\Catalog\Model\ProductFactory $productFactory, $batchCount = 1000, - \Magento\CatalogRule\Model\Indexer\ProductPriceCalculator $productPriceCalculator = null, - \Magento\CatalogRule\Model\Indexer\ReindexRuleProduct $reindexRuleProduct = null, - \Magento\CatalogRule\Model\Indexer\ReindexRuleGroupWebsite $reindexRuleGroupWebsite = null, - \Magento\CatalogRule\Model\Indexer\RuleProductsSelectBuilder $ruleProductsSelectBuilder = null, - \Magento\CatalogRule\Model\Indexer\ReindexRuleProductPrice $reindexRuleProductPrice = null, - \Magento\CatalogRule\Model\Indexer\RuleProductPricesPersistor $pricesPersistor = null, + ProductPriceCalculator $productPriceCalculator = null, + ReindexRuleProduct $reindexRuleProduct = null, + ReindexRuleGroupWebsite $reindexRuleGroupWebsite = null, + RuleProductsSelectBuilder $ruleProductsSelectBuilder = null, + ReindexRuleProductPrice $reindexRuleProductPrice = null, + RuleProductPricesPersistor $pricesPersistor = null, \Magento\Catalog\Model\ResourceModel\Indexer\ActiveTableSwitcher $activeTableSwitcher = null, ProductLoader $productLoader = null ) { @@ -195,29 +194,30 @@ public function __construct( $this->productFactory = $productFactory; $this->batchCount = $batchCount; - $this->productPriceCalculator = $productPriceCalculator ?: ObjectManager::getInstance()->get( - \Magento\CatalogRule\Model\Indexer\ProductPriceCalculator::class + $this->productPriceCalculator = $productPriceCalculator ?? ObjectManager::getInstance()->get( + ProductPriceCalculator::class ); - $this->reindexRuleProduct = $reindexRuleProduct ?: ObjectManager::getInstance()->get( - \Magento\CatalogRule\Model\Indexer\ReindexRuleProduct::class + $this->reindexRuleProduct = $reindexRuleProduct ?? ObjectManager::getInstance()->get( + ReindexRuleProduct::class ); - $this->reindexRuleGroupWebsite = $reindexRuleGroupWebsite ?: ObjectManager::getInstance()->get( - \Magento\CatalogRule\Model\Indexer\ReindexRuleGroupWebsite::class + $this->reindexRuleGroupWebsite = $reindexRuleGroupWebsite ?? ObjectManager::getInstance()->get( + ReindexRuleGroupWebsite::class ); - $this->ruleProductsSelectBuilder = $ruleProductsSelectBuilder ?: ObjectManager::getInstance()->get( - \Magento\CatalogRule\Model\Indexer\RuleProductsSelectBuilder::class + $this->ruleProductsSelectBuilder = $ruleProductsSelectBuilder ?? ObjectManager::getInstance()->get( + RuleProductsSelectBuilder::class ); - $this->reindexRuleProductPrice = $reindexRuleProductPrice ?: ObjectManager::getInstance()->get( - \Magento\CatalogRule\Model\Indexer\ReindexRuleProductPrice::class + $this->reindexRuleProductPrice = $reindexRuleProductPrice ?? ObjectManager::getInstance()->get( + ReindexRuleProductPrice::class ); - $this->pricesPersistor = $pricesPersistor ?: ObjectManager::getInstance()->get( - \Magento\CatalogRule\Model\Indexer\RuleProductPricesPersistor::class + $this->pricesPersistor = $pricesPersistor ?? ObjectManager::getInstance()->get( + RuleProductPricesPersistor::class ); - $this->activeTableSwitcher = $activeTableSwitcher ?: ObjectManager::getInstance()->get( + $this->activeTableSwitcher = $activeTableSwitcher ?? ObjectManager::getInstance()->get( \Magento\Catalog\Model\ResourceModel\Indexer\ActiveTableSwitcher::class ); - $this->productLoader = $productLoader ?: - ObjectManager::getInstance()->get(ProductLoader::class); + $this->productLoader = $productLoader ?? ObjectManager::getInstance()->get( + ProductLoader::class + ); } /** @@ -432,7 +432,7 @@ protected function getTable($tableName) * @param Rule $rule * @return $this * @deprecated 100.2.0 - * @see \Magento\CatalogRule\Model\Indexer\ReindexRuleProduct::execute + * @see ReindexRuleProduct::execute */ protected function updateRuleProductData(Rule $rule) { @@ -458,8 +458,8 @@ protected function updateRuleProductData(Rule $rule) * @throws \Exception * @return $this * @deprecated 100.2.0 - * @see \Magento\CatalogRule\Model\Indexer\ReindexRuleProductPrice::execute - * @see \Magento\CatalogRule\Model\Indexer\ReindexRuleGroupWebsite::execute + * @see ReindexRuleProductPrice::execute + * @see ReindexRuleGroupWebsite::execute */ protected function applyAllRules(Product $product = null) { @@ -473,7 +473,7 @@ protected function applyAllRules(Product $product = null) * * @return $this * @deprecated 100.2.0 - * @see \Magento\CatalogRule\Model\Indexer\ReindexRuleGroupWebsite::execute + * @see ReindexRuleGroupWebsite::execute */ protected function updateCatalogRuleGroupWebsiteData() { @@ -497,7 +497,7 @@ protected function deleteOldData() * @param null $productData * @return float * @deprecated 100.2.0 - * @see \Magento\CatalogRule\Model\Indexer\ProductPriceCalculator::calculate + * @see ProductPriceCalculator::calculate */ protected function calcRuleProductPrice($ruleData, $productData = null) { @@ -510,7 +510,7 @@ protected function calcRuleProductPrice($ruleData, $productData = null) * @return \Zend_Db_Statement_Interface * @throws \Magento\Framework\Exception\LocalizedException * @deprecated 100.2.0 - * @see \Magento\CatalogRule\Model\Indexer\RuleProductsSelectBuilder::build + * @see RuleProductsSelectBuilder::build */ protected function getRuleProductsStmt($websiteId, Product $product = null) { @@ -522,7 +522,7 @@ protected function getRuleProductsStmt($websiteId, Product $product = null) * @return $this * @throws \Exception * @deprecated 100.2.0 - * @see \Magento\CatalogRule\Model\Indexer\RuleProductPricesPersistor::execute + * @see RuleProductPricesPersistor::execute */ protected function saveRuleProductPrices($arrData) { From aa82a78272c882adbf4b358c16055b650f22a85e Mon Sep 17 00:00:00 2001 From: Dmytro Horytskyi <dhorytskyi@magento.com> Date: Thu, 28 Sep 2017 18:03:43 +0300 Subject: [PATCH 8/9] MAGETWO-71554: Category edit performance issue - for 2.2 --- app/code/Magento/Catalog/Model/Category.php | 2 + .../Category/Product/PositionResolver.php | 59 +++++++++ .../Catalog/Model/ResourceModel/Category.php | 15 ++- .../Category/Product/PositionResolverTest.php | 120 ++++++++++++++++++ .../Model/Indexer/IndexBuilder.php | 94 ++++++++------ .../Indexer/IndexBuilder/ProductLoader.php | 53 ++++++++ .../CatalogRule/Plugin/Indexer/Category.php | 2 +- .../IndexBuilder/ProductLoaderTest.php | 96 ++++++++++++++ .../Unit/Model/Indexer/IndexBuilderTest.php | 53 ++++++-- ...ategoryProcessUrlRewriteSavingObserver.php | 2 +- .../Observer/UrlRewriteHandler.php | 4 +- .../Unit/Observer/UrlRewriteHandlerTest.php | 2 +- 12 files changed, 441 insertions(+), 61 deletions(-) create mode 100644 app/code/Magento/Catalog/Model/Category/Product/PositionResolver.php create mode 100644 app/code/Magento/Catalog/Test/Unit/Model/Category/Product/PositionResolverTest.php create mode 100644 app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder/ProductLoader.php create mode 100644 app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilder/ProductLoaderTest.php diff --git a/app/code/Magento/Catalog/Model/Category.php b/app/code/Magento/Catalog/Model/Category.php index c4d6742a02dd3..40677b2cad1f7 100644 --- a/app/code/Magento/Catalog/Model/Category.php +++ b/app/code/Magento/Catalog/Model/Category.php @@ -28,6 +28,8 @@ * @method Category setUrlPath(string $urlPath) * @method Category getSkipDeleteChildren() * @method Category setSkipDeleteChildren(boolean $value) + * @method Category setChangedProductIds(array $categoryIds) Set products ids that inserted or deleted for category + * @method array getChangedProductIds() Get products ids that inserted or deleted for category * * @SuppressWarnings(PHPMD.LongVariable) * @SuppressWarnings(PHPMD.ExcessivePublicCount) diff --git a/app/code/Magento/Catalog/Model/Category/Product/PositionResolver.php b/app/code/Magento/Catalog/Model/Category/Product/PositionResolver.php new file mode 100644 index 0000000000000..97941f2d23b9f --- /dev/null +++ b/app/code/Magento/Catalog/Model/Category/Product/PositionResolver.php @@ -0,0 +1,59 @@ +<?php +/** + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. + */ +namespace Magento\Catalog\Model\Category\Product; + +/** + * Resolver to get product positions by ids assigned to specific category + */ +class PositionResolver extends \Magento\Framework\Model\ResourceModel\Db\AbstractDb +{ + /** + * @param \Magento\Framework\Model\ResourceModel\Db\Context $context + * @param string $connectionName + */ + public function __construct( + \Magento\Framework\Model\ResourceModel\Db\Context $context, + $connectionName = null + ) { + parent::__construct($context, $connectionName); + } + + /** + * Initialize resource model + * + * @return void + */ + protected function _construct() + { + $this->_init('catalog_product_entity', 'entity_id'); + } + + /** + * Get category product positions + * + * @param int $categoryId + * @return array + */ + public function getPositions(int $categoryId) + { + $connection = $this->getConnection(); + + $select = $connection->select()->from( + ['cpe' => $this->getTable('catalog_product_entity')], + 'entity_id' + )->joinLeft( + ['ccp' => $this->getTable('catalog_category_product')], + 'ccp.product_id=cpe.entity_id' + )->where( + 'ccp.category_id = ?', + $categoryId + )->order( + 'ccp.position ' . \Magento\Framework\DB\Select::SQL_ASC + ); + + return array_flip($connection->fetchCol($select)); + } +} diff --git a/app/code/Magento/Catalog/Model/ResourceModel/Category.php b/app/code/Magento/Catalog/Model/ResourceModel/Category.php index a9c705697b268..6c9867359d40b 100644 --- a/app/code/Magento/Catalog/Model/ResourceModel/Category.php +++ b/app/code/Magento/Catalog/Model/ResourceModel/Category.php @@ -410,9 +410,18 @@ protected function _saveCategoryProducts($category) * Update product positions in category */ if (!empty($update)) { + $newPositions = []; foreach ($update as $productId => $position) { - $where = ['category_id = ?' => (int)$id, 'product_id = ?' => (int)$productId]; - $bind = ['position' => (int)$position]; + $delta = $position - $oldProducts[$productId]; + if (!isset($newPositions[$delta])) { + $newPositions[$delta] = []; + } + $newPositions[$delta][] = $productId; + } + + foreach ($newPositions as $delta => $productIds) { + $bind = ['position' => new \Zend_Db_Expr("position + ({$delta})")]; + $where = ['category_id = ?' => (int)$id, 'product_id IN (?)' => $productIds]; $connection->update($this->getCategoryProductTable(), $bind, $where); } } @@ -423,6 +432,8 @@ protected function _saveCategoryProducts($category) 'catalog_category_change_products', ['category' => $category, 'product_ids' => $productIds] ); + + $category->setChangedProductIds($productIds); } if (!empty($insert) || !empty($update) || !empty($delete)) { diff --git a/app/code/Magento/Catalog/Test/Unit/Model/Category/Product/PositionResolverTest.php b/app/code/Magento/Catalog/Test/Unit/Model/Category/Product/PositionResolverTest.php new file mode 100644 index 0000000000000..9545e5eb4b37d --- /dev/null +++ b/app/code/Magento/Catalog/Test/Unit/Model/Category/Product/PositionResolverTest.php @@ -0,0 +1,120 @@ +<?php +/** + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. + */ +namespace Magento\Catalog\Test\Unit\Model\Category\Product; + +use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; +use Magento\Catalog\Model\Category\Product\PositionResolver; +use Magento\Framework\Model\ResourceModel\Db\Context; +use Magento\Framework\App\ResourceConnection; +use Magento\Framework\DB\Adapter\AdapterInterface; +use Magento\Framework\DB\Select; + +class PositionResolverTest extends \PHPUnit\Framework\TestCase +{ + /** + * @var Context|\PHPUnit_Framework_MockObject_MockObject + */ + private $context; + + /** + * @var ResourceConnection|\PHPUnit_Framework_MockObject_MockObject + */ + private $resources; + + /** + * @var AdapterInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $connection; + + /** + * @var Select|\PHPUnit_Framework_MockObject_MockObject + */ + private $select; + + /** + * @var PositionResolver + */ + private $model; + + /** + * @var array + */ + private $positions = [ + '3' => 100, + '2' => 101, + '1' => 102 + ]; + + /** + * @var array + */ + private $flippedPositions = [ + '100' => 3, + '101' => 2, + '102' => 1 + ]; + + /** + * @var int + */ + private $categoryId = 1; + + protected function setUp() + { + $this->context = $this->getMockBuilder(Context::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->resources = $this->getMockBuilder(ResourceConnection::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->connection = $this->getMockBuilder(AdapterInterface::class) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + + $this->select = $this->getMockBuilder(Select::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->model = (new ObjectManager($this))->getObject( + PositionResolver::class, + [ + 'context' => $this->context, + null, + '_resources' => $this->resources + ] + ); + } + + public function testGetPositions() + { + $this->resources->expects($this->once()) + ->method('getConnection') + ->willReturn($this->connection); + + $this->connection->expects($this->once()) + ->method('select') + ->willReturn($this->select); + $this->select->expects($this->once()) + ->method('from') + ->willReturnSelf(); + $this->select->expects($this->once()) + ->method('where') + ->willReturnSelf(); + $this->select->expects($this->once()) + ->method('order') + ->willReturnSelf(); + $this->select->expects($this->once()) + ->method('joinLeft') + ->willReturnSelf(); + $this->connection->expects($this->once()) + ->method('fetchCol') + ->willReturn($this->positions); + + $this->assertEquals($this->flippedPositions, $this->model->getPositions($this->categoryId)); + } +} diff --git a/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php b/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php index 34b4bff5a060e..82be37049912f 100644 --- a/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php +++ b/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php @@ -11,6 +11,7 @@ use Magento\CatalogRule\Model\Rule; use Magento\Framework\App\ObjectManager; use Magento\Framework\Pricing\PriceCurrencyInterface; +use Magento\CatalogRule\Model\Indexer\IndexBuilder\ProductLoader; /** * @api @@ -101,32 +102,32 @@ class IndexBuilder protected $connection; /** - * @var \Magento\CatalogRule\Model\Indexer\ProductPriceCalculator + * @var ProductPriceCalculator */ private $productPriceCalculator; /** - * @var \Magento\CatalogRule\Model\Indexer\ReindexRuleProduct + * @var ReindexRuleProduct */ private $reindexRuleProduct; /** - * @var \Magento\CatalogRule\Model\Indexer\ReindexRuleGroupWebsite + * @var ReindexRuleGroupWebsite */ private $reindexRuleGroupWebsite; /** - * @var \Magento\CatalogRule\Model\Indexer\RuleProductsSelectBuilder + * @var RuleProductsSelectBuilder */ private $ruleProductsSelectBuilder; /** - * @var \Magento\CatalogRule\Model\Indexer\ReindexRuleProductPrice + * @var ReindexRuleProductPrice */ private $reindexRuleProductPrice; /** - * @var \Magento\CatalogRule\Model\Indexer\RuleProductPricesPersistor + * @var RuleProductPricesPersistor */ private $pricesPersistor; @@ -135,6 +136,11 @@ class IndexBuilder */ private $activeTableSwitcher; + /** + * @var ProductLoader + */ + private $productLoader; + /** * @param RuleCollectionFactory $ruleCollectionFactory * @param PriceCurrencyInterface $priceCurrency @@ -146,13 +152,14 @@ class IndexBuilder * @param \Magento\Framework\Stdlib\DateTime\DateTime $dateTime * @param \Magento\Catalog\Model\ProductFactory $productFactory * @param int $batchCount - * @param \Magento\CatalogRule\Model\Indexer\ProductPriceCalculator|null $productPriceCalculator - * @param \Magento\CatalogRule\Model\Indexer\ReindexRuleProduct|null $reindexRuleProduct - * @param \Magento\CatalogRule\Model\Indexer\ReindexRuleGroupWebsite|null $reindexRuleGroupWebsite - * @param \Magento\CatalogRule\Model\Indexer\RuleProductsSelectBuilder|null $ruleProductsSelectBuilder - * @param \Magento\CatalogRule\Model\Indexer\ReindexRuleProductPrice|null $reindexRuleProductPrice - * @param \Magento\CatalogRule\Model\Indexer\RuleProductPricesPersistor|null $pricesPersistor + * @param ProductPriceCalculator|null $productPriceCalculator + * @param ReindexRuleProduct|null $reindexRuleProduct + * @param ReindexRuleGroupWebsite|null $reindexRuleGroupWebsite + * @param RuleProductsSelectBuilder|null $ruleProductsSelectBuilder + * @param ReindexRuleProductPrice|null $reindexRuleProductPrice + * @param RuleProductPricesPersistor|null $pricesPersistor * @param \Magento\Catalog\Model\ResourceModel\Indexer\ActiveTableSwitcher|null $activeTableSwitcher + * @param ProductLoader|null $productLoader * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ public function __construct( @@ -166,13 +173,14 @@ public function __construct( \Magento\Framework\Stdlib\DateTime\DateTime $dateTime, \Magento\Catalog\Model\ProductFactory $productFactory, $batchCount = 1000, - \Magento\CatalogRule\Model\Indexer\ProductPriceCalculator $productPriceCalculator = null, - \Magento\CatalogRule\Model\Indexer\ReindexRuleProduct $reindexRuleProduct = null, - \Magento\CatalogRule\Model\Indexer\ReindexRuleGroupWebsite $reindexRuleGroupWebsite = null, - \Magento\CatalogRule\Model\Indexer\RuleProductsSelectBuilder $ruleProductsSelectBuilder = null, - \Magento\CatalogRule\Model\Indexer\ReindexRuleProductPrice $reindexRuleProductPrice = null, - \Magento\CatalogRule\Model\Indexer\RuleProductPricesPersistor $pricesPersistor = null, - \Magento\Catalog\Model\ResourceModel\Indexer\ActiveTableSwitcher $activeTableSwitcher = null + ProductPriceCalculator $productPriceCalculator = null, + ReindexRuleProduct $reindexRuleProduct = null, + ReindexRuleGroupWebsite $reindexRuleGroupWebsite = null, + RuleProductsSelectBuilder $ruleProductsSelectBuilder = null, + ReindexRuleProductPrice $reindexRuleProductPrice = null, + RuleProductPricesPersistor $pricesPersistor = null, + \Magento\Catalog\Model\ResourceModel\Indexer\ActiveTableSwitcher $activeTableSwitcher = null, + ProductLoader $productLoader = null ) { $this->resource = $resource; $this->connection = $resource->getConnection(); @@ -186,27 +194,30 @@ public function __construct( $this->productFactory = $productFactory; $this->batchCount = $batchCount; - $this->productPriceCalculator = $productPriceCalculator ?: ObjectManager::getInstance()->get( - \Magento\CatalogRule\Model\Indexer\ProductPriceCalculator::class + $this->productPriceCalculator = $productPriceCalculator ?? ObjectManager::getInstance()->get( + ProductPriceCalculator::class ); - $this->reindexRuleProduct = $reindexRuleProduct ?: ObjectManager::getInstance()->get( - \Magento\CatalogRule\Model\Indexer\ReindexRuleProduct::class + $this->reindexRuleProduct = $reindexRuleProduct ?? ObjectManager::getInstance()->get( + ReindexRuleProduct::class ); - $this->reindexRuleGroupWebsite = $reindexRuleGroupWebsite ?: ObjectManager::getInstance()->get( - \Magento\CatalogRule\Model\Indexer\ReindexRuleGroupWebsite::class + $this->reindexRuleGroupWebsite = $reindexRuleGroupWebsite ?? ObjectManager::getInstance()->get( + ReindexRuleGroupWebsite::class ); - $this->ruleProductsSelectBuilder = $ruleProductsSelectBuilder ?: ObjectManager::getInstance()->get( - \Magento\CatalogRule\Model\Indexer\RuleProductsSelectBuilder::class + $this->ruleProductsSelectBuilder = $ruleProductsSelectBuilder ?? ObjectManager::getInstance()->get( + RuleProductsSelectBuilder::class ); - $this->reindexRuleProductPrice = $reindexRuleProductPrice ?: ObjectManager::getInstance()->get( - \Magento\CatalogRule\Model\Indexer\ReindexRuleProductPrice::class + $this->reindexRuleProductPrice = $reindexRuleProductPrice ?? ObjectManager::getInstance()->get( + ReindexRuleProductPrice::class ); - $this->pricesPersistor = $pricesPersistor ?: ObjectManager::getInstance()->get( - \Magento\CatalogRule\Model\Indexer\RuleProductPricesPersistor::class + $this->pricesPersistor = $pricesPersistor ?? ObjectManager::getInstance()->get( + RuleProductPricesPersistor::class ); - $this->activeTableSwitcher = $activeTableSwitcher ?: ObjectManager::getInstance()->get( + $this->activeTableSwitcher = $activeTableSwitcher ?? ObjectManager::getInstance()->get( \Magento\Catalog\Model\ResourceModel\Indexer\ActiveTableSwitcher::class ); + $this->productLoader = $productLoader ?? ObjectManager::getInstance()->get( + ProductLoader::class + ); } /** @@ -251,9 +262,10 @@ protected function doReindexByIds($ids) { $this->cleanByIds($ids); + $products = $this->productLoader->getProducts($ids); foreach ($this->getActiveRules() as $rule) { - foreach ($ids as $productId) { - $this->applyRule($rule, $this->getProduct($productId)); + foreach ($products as $product) { + $this->applyRule($rule, $product); } } } @@ -420,7 +432,7 @@ protected function getTable($tableName) * @param Rule $rule * @return $this * @deprecated 100.2.0 - * @see \Magento\CatalogRule\Model\Indexer\ReindexRuleProduct::execute + * @see ReindexRuleProduct::execute */ protected function updateRuleProductData(Rule $rule) { @@ -446,8 +458,8 @@ protected function updateRuleProductData(Rule $rule) * @throws \Exception * @return $this * @deprecated 100.2.0 - * @see \Magento\CatalogRule\Model\Indexer\ReindexRuleProductPrice::execute - * @see \Magento\CatalogRule\Model\Indexer\ReindexRuleGroupWebsite::execute + * @see ReindexRuleProductPrice::execute + * @see ReindexRuleGroupWebsite::execute */ protected function applyAllRules(Product $product = null) { @@ -461,7 +473,7 @@ protected function applyAllRules(Product $product = null) * * @return $this * @deprecated 100.2.0 - * @see \Magento\CatalogRule\Model\Indexer\ReindexRuleGroupWebsite::execute + * @see ReindexRuleGroupWebsite::execute */ protected function updateCatalogRuleGroupWebsiteData() { @@ -485,7 +497,7 @@ protected function deleteOldData() * @param null $productData * @return float * @deprecated 100.2.0 - * @see \Magento\CatalogRule\Model\Indexer\ProductPriceCalculator::calculate + * @see ProductPriceCalculator::calculate */ protected function calcRuleProductPrice($ruleData, $productData = null) { @@ -498,7 +510,7 @@ protected function calcRuleProductPrice($ruleData, $productData = null) * @return \Zend_Db_Statement_Interface * @throws \Magento\Framework\Exception\LocalizedException * @deprecated 100.2.0 - * @see \Magento\CatalogRule\Model\Indexer\RuleProductsSelectBuilder::build + * @see RuleProductsSelectBuilder::build */ protected function getRuleProductsStmt($websiteId, Product $product = null) { @@ -510,7 +522,7 @@ protected function getRuleProductsStmt($websiteId, Product $product = null) * @return $this * @throws \Exception * @deprecated 100.2.0 - * @see \Magento\CatalogRule\Model\Indexer\RuleProductPricesPersistor::execute + * @see RuleProductPricesPersistor::execute */ protected function saveRuleProductPrices($arrData) { diff --git a/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder/ProductLoader.php b/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder/ProductLoader.php new file mode 100644 index 0000000000000..bce2bea2fd952 --- /dev/null +++ b/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder/ProductLoader.php @@ -0,0 +1,53 @@ +<?php +/** + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. + */ +namespace Magento\CatalogRule\Model\Indexer\IndexBuilder; + +use Magento\Catalog\Api\ProductRepositoryInterface; +use Magento\Framework\Api\SearchCriteriaBuilder; +use Magento\Catalog\Api\Data\ProductInterface; + +/** + * Product loader that gets products by ids via product repository + */ +class ProductLoader +{ + /** + * @var ProductRepositoryInterface + */ + private $productRepository; + + /** + * @var SearchCriteriaBuilder + */ + private $searchCriteriaBuilder; + + /** + * @param ProductRepositoryInterface $productRepository + * @param SearchCriteriaBuilder $searchCriteriaBuilder + */ + public function __construct( + ProductRepositoryInterface $productRepository, + SearchCriteriaBuilder $searchCriteriaBuilder + ) { + $this->productRepository = $productRepository; + $this->searchCriteriaBuilder = $searchCriteriaBuilder; + } + + /** + * Get products by ids + * + * @param array $productIds + * @return ProductInterface[] + */ + public function getProducts($productIds) + { + $this->searchCriteriaBuilder->addFilter('entity_id', $productIds, 'in'); + $searchCriteria = $this->searchCriteriaBuilder->create(); + $products = $this->productRepository->getList($searchCriteria)->getItems(); + + return $products; + } +} diff --git a/app/code/Magento/CatalogRule/Plugin/Indexer/Category.php b/app/code/Magento/CatalogRule/Plugin/Indexer/Category.php index 8bcc1fd8d6834..0ea0fdda31958 100644 --- a/app/code/Magento/CatalogRule/Plugin/Indexer/Category.php +++ b/app/code/Magento/CatalogRule/Plugin/Indexer/Category.php @@ -36,7 +36,7 @@ public function afterSave( ) { /** @var \Magento\Catalog\Model\Category $result */ $productIds = $result->getAffectedProductIds(); - if ($productIds) { + if ($productIds && !$this->productRuleProcessor->isIndexerScheduled()) { $this->productRuleProcessor->reindexList($productIds); } return $result; diff --git a/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilder/ProductLoaderTest.php b/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilder/ProductLoaderTest.php new file mode 100644 index 0000000000000..3509128da79da --- /dev/null +++ b/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilder/ProductLoaderTest.php @@ -0,0 +1,96 @@ +<?php +/** + * Copyright © Magento, Inc. All rights reserved. + * See COPYING.txt for license details. + */ + +namespace Magento\CatalogRule\Test\Unit\Model\Indexer\IndexBuilder; + +use Magento\CatalogRule\Model\Indexer\IndexBuilder\ProductLoader; +use Magento\Catalog\Api\ProductRepositoryInterface; +use Magento\Framework\Api\SearchCriteriaBuilder; +use Magento\Catalog\Api\Data\ProductSearchResultsInterface; +use Magento\Catalog\Model\Product; +use Magento\Framework\Api\SearchCriteria; + +class ProductLoaderTest extends \PHPUnit\Framework\TestCase +{ + /** + * @var ProductLoader + */ + protected $productLoader; + + /** + * @var ProductRepositoryInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $productRepository; + + /** + * @var SearchCriteriaBuilder|\PHPUnit_Framework_MockObject_MockObject + */ + private $searchCriteriaBuilder; + + /** + * @var ProductSearchResultsInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $productSearchResultsInterface; + + /** + * @var \Magento\Framework\Api\SearchCriteria|\PHPUnit_Framework_MockObject_MockObject + */ + private $searchCriteria; + + /** + * @var Product|\PHPUnit_Framework_MockObject_MockObject + */ + protected $product; + + /** + * Set up test + * + * @return void + */ + protected function setUp() + { + $this->productRepository = $this->getMockBuilder(ProductRepositoryInterface::class) + ->disableOriginalConstructor() + ->getMock(); + $this->searchCriteriaBuilder = $this->getMockBuilder(SearchCriteriaBuilder::class) + ->disableOriginalConstructor() + ->getMock(); + $this->productSearchResultsInterface = $this->getMockBuilder(ProductSearchResultsInterface::class) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + $this->searchCriteria = $this->getMockBuilder(SearchCriteria::class) + ->disableOriginalConstructor() + ->getMock(); + $this->product = $this->getMockBuilder(Product::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->productLoader = new ProductLoader( + $this->productRepository, + $this->searchCriteriaBuilder + ); + } + + public function testGetProducts() + { + $this->searchCriteriaBuilder->expects($this->once()) + ->method('addFilter') + ->willReturnSelf(); + $this->searchCriteriaBuilder->expects($this->once()) + ->method('create') + ->willReturn($this->searchCriteria); + $this->productRepository->expects($this->once()) + ->method('getList') + ->with($this->searchCriteria) + ->willReturn($this->productSearchResultsInterface); + $iterator = new \ArrayIterator([$this->product]); + $this->productSearchResultsInterface->expects($this->once()) + ->method('getItems') + ->willReturn($iterator); + + $this->assertSame($iterator, $this->productLoader->getProducts([1])); + } +} diff --git a/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php b/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php index 997cf704a8657..8252b512e7810 100644 --- a/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php +++ b/app/code/Magento/CatalogRule/Test/Unit/Model/Indexer/IndexBuilderTest.php @@ -6,6 +6,9 @@ namespace Magento\CatalogRule\Test\Unit\Model\Indexer; +use Magento\CatalogRule\Model\Indexer\IndexBuilder\ProductLoader; +use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; + /** * @SuppressWarnings(PHPMD.CouplingBetweenObjects) * @SuppressWarnings(PHPMD.TooManyFields) @@ -122,10 +125,16 @@ class IndexBuilderTest extends \PHPUnit\Framework\TestCase */ private $reindexRuleGroupWebsite; + /** + * @var ProductLoader|\PHPUnit_Framework_MockObject_MockObject + */ + private $productLoader; + /** * Set up test * * @return void + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ protected function setUp() { @@ -171,36 +180,47 @@ protected function setUp() $this->rules->expects($this->any())->method('getId')->will($this->returnValue(1)); $this->rules->expects($this->any())->method('getWebsiteIds')->will($this->returnValue([1])); $this->rules->expects($this->any())->method('getCustomerGroupIds')->will($this->returnValue([1])); + $this->ruleCollectionFactory->expects($this->any())->method('create')->will($this->returnSelf()); $this->ruleCollectionFactory->expects($this->any())->method('addFieldToFilter')->will( $this->returnValue([$this->rules]) ); + $this->product->expects($this->any())->method('load')->will($this->returnSelf()); $this->product->expects($this->any())->method('getId')->will($this->returnValue(1)); $this->product->expects($this->any())->method('getWebsiteIds')->will($this->returnValue([1])); + $this->rules->expects($this->any())->method('validate')->with($this->product)->willReturn(true); $this->attribute->expects($this->any())->method('getBackend')->will($this->returnValue($this->backend)); $this->productFactory->expects($this->any())->method('create')->will($this->returnValue($this->product)); + $this->productLoader = $this->getMockBuilder(ProductLoader::class) + ->disableOriginalConstructor() + ->getMock(); - $this->indexBuilder = new \Magento\CatalogRule\Model\Indexer\IndexBuilder( - $this->ruleCollectionFactory, - $this->priceCurrency, - $this->resource, - $this->storeManager, - $this->logger, - $this->eavConfig, - $this->dateFormat, - $this->dateTime, - $this->productFactory + $this->indexBuilder = (new ObjectManager($this))->getObject( + \Magento\CatalogRule\Model\Indexer\IndexBuilder::class, + [ + 'ruleCollectionFactory' => $this->ruleCollectionFactory, + 'priceCurrency' => $this->priceCurrency, + 'resource' => $this->resource, + 'storeManager' => $this->storeManager, + 'logger' => $this->logger, + 'eavConfig' => $this->eavConfig, + 'dateFormat' => $this->dateFormat, + 'dateTime' => $this->dateTime, + 'productFactory' => $this->productFactory, + 'productLoader' => $this->productLoader, + ] ); + $this->reindexRuleProductPrice = $this->getMockBuilder(\Magento\CatalogRule\Model\Indexer\ReindexRuleProductPrice::class) - ->disableOriginalConstructor() - ->getMock(); + ->disableOriginalConstructor() + ->getMock(); $this->reindexRuleGroupWebsite = $this->getMockBuilder(\Magento\CatalogRule\Model\Indexer\ReindexRuleGroupWebsite::class) - ->disableOriginalConstructor() - ->getMock(); + ->disableOriginalConstructor() + ->getMock(); $this->setProperties($this->indexBuilder, [ 'metadataPool' => $this->metadataPool, 'reindexRuleProductPrice' => $this->reindexRuleProductPrice, @@ -235,6 +255,11 @@ public function testUpdateCatalogRuleGroupWebsiteData() ->method('getBackend') ->will($this->returnValue($backendModelMock)); + $iterator = new \ArrayIterator([$this->product]); + $this->productLoader->expects($this->once()) + ->method('getProducts') + ->willReturn($iterator); + $this->reindexRuleProductPrice->expects($this->once())->method('execute')->willReturn(true); $this->reindexRuleGroupWebsite->expects($this->once())->method('execute')->willReturn(true); diff --git a/app/code/Magento/CatalogUrlRewrite/Observer/CategoryProcessUrlRewriteSavingObserver.php b/app/code/Magento/CatalogUrlRewrite/Observer/CategoryProcessUrlRewriteSavingObserver.php index 92a46facbe71c..539e5c3f42f15 100644 --- a/app/code/Magento/CatalogUrlRewrite/Observer/CategoryProcessUrlRewriteSavingObserver.php +++ b/app/code/Magento/CatalogUrlRewrite/Observer/CategoryProcessUrlRewriteSavingObserver.php @@ -85,7 +85,7 @@ public function execute(\Magento\Framework\Event\Observer $observer) $mapsGenerated = false; if ($category->dataHasChangedFor('url_key') || $category->dataHasChangedFor('is_anchor') - || $category->getIsChangedProductList() + || $category->getChangedProductIds() ) { if ($category->dataHasChangedFor('url_key')) { $categoryUrlRewriteResult = $this->categoryUrlRewriteGenerator->generate($category); diff --git a/app/code/Magento/CatalogUrlRewrite/Observer/UrlRewriteHandler.php b/app/code/Magento/CatalogUrlRewrite/Observer/UrlRewriteHandler.php index 5a6777f94e2d5..9892465d1538a 100644 --- a/app/code/Magento/CatalogUrlRewrite/Observer/UrlRewriteHandler.php +++ b/app/code/Magento/CatalogUrlRewrite/Observer/UrlRewriteHandler.php @@ -104,7 +104,8 @@ public function generateProductUrlRewrites(\Magento\Catalog\Model\Category $cate $this->isSkippedProduct[$category->getEntityId()] = []; $saveRewriteHistory = $category->getData('save_rewrites_history'); $storeId = $category->getStoreId(); - if ($category->getAffectedProductIds()) { + + if ($category->getChangedProductIds()) { $this->isSkippedProduct[$category->getEntityId()] = $category->getAffectedProductIds(); /* @var \Magento\Catalog\Model\ResourceModel\Product\Collection $collection */ $collection = $this->productCollectionFactory->create() @@ -140,6 +141,7 @@ public function generateProductUrlRewrites(\Magento\Catalog\Model\Category $cate ) ); } + foreach ($this->childrenCategoriesProvider->getChildren($category, true) as $childCategory) { $mergeDataProvider->merge( $this->getCategoryProductsUrlRewrites( diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/UrlRewriteHandlerTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/UrlRewriteHandlerTest.php index 747e0cfa771fd..b18597a42bf94 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/UrlRewriteHandlerTest.php +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/UrlRewriteHandlerTest.php @@ -127,7 +127,7 @@ public function testGenerateProductUrlRewrites() { /* @var \Magento\Catalog\Model\Category|\PHPUnit_Framework_MockObject_MockObject $category */ $category = $this->getMockBuilder(\Magento\Catalog\Model\Category::class) - ->setMethods(['getEntityId', 'getStoreId', 'getData', 'getAffectedProductIds']) + ->setMethods(['getEntityId', 'getStoreId', 'getData', 'getChangedProductIds']) ->disableOriginalConstructor() ->getMock(); $category->expects($this->any()) From 5945195e1ed58f167d50b0eaf366d19c54a98687 Mon Sep 17 00:00:00 2001 From: Dmytro Horytskyi <dhorytskyi@magento.com> Date: Tue, 10 Oct 2017 18:47:16 +0300 Subject: [PATCH 9/9] MAGETWO-71554: Category edit performance issue - for 2.2 --- app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php b/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php index 82be37049912f..f2dd8968a903d 100644 --- a/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php +++ b/app/code/Magento/CatalogRule/Model/Indexer/IndexBuilder.php @@ -195,7 +195,7 @@ public function __construct( $this->batchCount = $batchCount; $this->productPriceCalculator = $productPriceCalculator ?? ObjectManager::getInstance()->get( - ProductPriceCalculator::class + ProductPriceCalculator::class ); $this->reindexRuleProduct = $reindexRuleProduct ?? ObjectManager::getInstance()->get( ReindexRuleProduct::class