From fec72085e230c66e90d0b12134f9e79c515b13b7 Mon Sep 17 00:00:00 2001 From: Cristian Partica <cpartica@magento.com> Date: Mon, 9 Jan 2017 22:38:52 -0600 Subject: [PATCH] MAGETWO-58924: SQL error wait timeout error when saving categories - fix issues with code review, add phpdocs for renaming and fixing unit tests --- .../Category/ChildrenUrlRewriteGenerator.php | 5 +- .../CurrentUrlRewritesRegenerator.php | 7 +- .../Model/CategoryUrlRewriteGenerator.php | 5 +- .../Model/Map/DataCategoryHashMap.php | 22 +++---- .../Map/DataCategoryUrlRewriteDatabaseMap.php | 3 +- .../Map/DataCategoryUsedInProductsHashMap.php | 65 ++++++++++--------- .../Model/Map/DataProductHashMap.php | 51 +++++++-------- .../Map/DataProductUrlRewriteDatabaseMap.php | 3 +- .../Model/Map/DatabaseMapInterface.php | 4 +- .../Model/Map/DatabaseMapPool.php | 5 ++ .../Model/Map/HashMapInterface.php | 5 +- .../Model/Map/HashMapPool.php | 9 +-- .../Model/Map/UrlRewriteFinder.php | 17 ++--- .../Product/CurrentUrlRewritesRegenerator.php | 5 +- .../Model/ProductScopeRewriteGenerator.php | 5 +- .../Observer/AfterImportDataObserver.php | 5 +- ...ategoryProcessUrlRewriteSavingObserver.php | 17 +---- .../Observer/UrlRewriteHandler.php | 5 +- .../Model/CategoryUrlPathGeneratorTest.php | 3 +- .../Test/Unit/Model/Map/HashMapPoolTest.php | 16 ++++- .../Unit/Model/Map/UrlRewriteFinderTest.php | 8 ++- app/code/Magento/CatalogUrlRewrite/etc/di.xml | 8 +++ .../Test/Unit/Model/MergeDataProviderTest.php | 6 +- .../Framework/DB/TemporaryTableService.php | 8 ++- 24 files changed, 162 insertions(+), 125 deletions(-) diff --git a/app/code/Magento/CatalogUrlRewrite/Model/Category/ChildrenUrlRewriteGenerator.php b/app/code/Magento/CatalogUrlRewrite/Model/Category/ChildrenUrlRewriteGenerator.php index 920da739c8e..de647a34619 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/Category/ChildrenUrlRewriteGenerator.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/Category/ChildrenUrlRewriteGenerator.php @@ -34,8 +34,9 @@ class ChildrenUrlRewriteGenerator ) { $this->childrenCategoriesProvider = $childrenCategoriesProvider; $this->categoryUrlRewriteGeneratorFactory = $categoryUrlRewriteGeneratorFactory; - $mergeDataProviderFactory = $mergeDataProviderFactory ?: ObjectManager::getInstance() - ->get(MergeDataProviderFactory::class); + if (!isset($mergeDataProviderFactory)) { + $mergeDataProviderFactory = ObjectManager::getInstance()->get(MergeDataProviderFactory::class); + } $this->mergeDataProviderPrototype = $mergeDataProviderFactory->create(); } diff --git a/app/code/Magento/CatalogUrlRewrite/Model/Category/CurrentUrlRewritesRegenerator.php b/app/code/Magento/CatalogUrlRewrite/Model/Category/CurrentUrlRewritesRegenerator.php index eb3fd3954bd..c994b424641 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/Category/CurrentUrlRewritesRegenerator.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/Category/CurrentUrlRewritesRegenerator.php @@ -57,8 +57,11 @@ class CurrentUrlRewritesRegenerator $this->urlFinder = $urlFinder; $this->urlRewriteFinder = $urlRewriteFinder ?: \Magento\Framework\App\ObjectManager::getInstance() ->get(\Magento\CatalogUrlRewrite\Model\Map\UrlRewriteFinder::class); - $mergeDataProviderFactory = $mergeDataProviderFactory ?: \Magento\Framework\App\ObjectManager::getInstance() - ->get(\Magento\UrlRewrite\Model\MergeDataProviderFactory::class); + if (!isset($mergeDataProviderFactory)) { + $mergeDataProviderFactory = \Magento\Framework\App\ObjectManager::getInstance()->get( + \Magento\UrlRewrite\Model\MergeDataProviderFactory::class + ); + } $this->mergeDataProviderPrototype = $mergeDataProviderFactory->create(); } diff --git a/app/code/Magento/CatalogUrlRewrite/Model/CategoryUrlRewriteGenerator.php b/app/code/Magento/CatalogUrlRewrite/Model/CategoryUrlRewriteGenerator.php index f95c72515c4..e565760f334 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/CategoryUrlRewriteGenerator.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/CategoryUrlRewriteGenerator.php @@ -67,8 +67,9 @@ class CategoryUrlRewriteGenerator $this->childrenUrlRewriteGenerator = $childrenUrlRewriteGenerator; $this->currentUrlRewritesRegenerator = $currentUrlRewritesRegenerator; $this->categoryRepository = $categoryRepository; - $mergeDataProviderFactory = $mergeDataProviderFactory ?: ObjectManager::getInstance() - ->get(MergeDataProviderFactory::class); + if (!isset($mergeDataProviderFactory)) { + $mergeDataProviderFactory = ObjectManager::getInstance()->get(MergeDataProviderFactory::class); + } $this->mergeDataProviderPrototype = $mergeDataProviderFactory->create(); } diff --git a/app/code/Magento/CatalogUrlRewrite/Model/Map/DataCategoryHashMap.php b/app/code/Magento/CatalogUrlRewrite/Model/Map/DataCategoryHashMap.php index acb8c4a8971..616e120ebaf 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/Map/DataCategoryHashMap.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/Map/DataCategoryHashMap.php @@ -47,10 +47,7 @@ class DataCategoryHashMap implements HashMapInterface */ public function getAllData($categoryId) { - if (!isset($this->hashMap[$categoryId])) { - $this->hashMap[$categoryId] = $this->generateData($categoryId); - } - return $this->hashMap[$categoryId]; + return $this->generateData($categoryId); } /** @@ -58,25 +55,28 @@ class DataCategoryHashMap implements HashMapInterface */ public function getData($categoryId, $key) { - $this->getAllData($categoryId); - return $this->hashMap[$categoryId][$key]; + $categorySpecificData = $this->generateData($categoryId); + return $categorySpecificData[$key]; } /** - * Queries the database and returns results + * Returns an array of categories ids that includes category identified by $categoryId and all its subcategories * * @param int $categoryId * @return array */ private function generateData($categoryId) { - $category = $this->categoryRepository->get($categoryId); - return $this->collection->addIdFilter($this->getAllCategoryChildrenIds($category)) - ->getAllIds(); + if (!isset($this->hashMap[$categoryId])) { + $category = $this->categoryRepository->get($categoryId); + $this->hashMap[$categoryId] = $this->collection->addIdFilter($this->getAllCategoryChildrenIds($category)) + ->getAllIds(); + } + return $this->hashMap[$categoryId]; } /** - * Queries sub-categories ids from a category + * Queries the database for sub-categories ids from a category * * @param CategoryInterface $category * @return int[] diff --git a/app/code/Magento/CatalogUrlRewrite/Model/Map/DataCategoryUrlRewriteDatabaseMap.php b/app/code/Magento/CatalogUrlRewrite/Model/Map/DataCategoryUrlRewriteDatabaseMap.php index bcc82ce8e44..032ed5e1302 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/Map/DataCategoryUrlRewriteDatabaseMap.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/Map/DataCategoryUrlRewriteDatabaseMap.php @@ -58,7 +58,8 @@ class DataCategoryUrlRewriteDatabaseMap implements DatabaseMapInterface } /** - * Queries the database and returns the name of the temporary table where data is stored + * Queries the database for all category url rewrites that are affected by the category identified by $categoryId + * It returns the name of the temporary table where the resulting data is stored * * @param int $categoryId * @return string diff --git a/app/code/Magento/CatalogUrlRewrite/Model/Map/DataCategoryUsedInProductsHashMap.php b/app/code/Magento/CatalogUrlRewrite/Model/Map/DataCategoryUsedInProductsHashMap.php index 0a3a99f8a0e..8864226d069 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/Map/DataCategoryUsedInProductsHashMap.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/Map/DataCategoryUsedInProductsHashMap.php @@ -38,10 +38,7 @@ class DataCategoryUsedInProductsHashMap implements HashMapInterface */ public function getAllData($categoryId) { - if (!isset($this->hashMap[$categoryId])) { - $this->hashMap[$categoryId] = $this->generateData($categoryId); - } - return $this->hashMap[$categoryId]; + return $this->generateData($categoryId); } /** @@ -49,45 +46,49 @@ class DataCategoryUsedInProductsHashMap implements HashMapInterface */ public function getData($categoryId, $key) { - $this->getAllData($categoryId); - return $this->hashMap[$categoryId][$key]; + $categorySpecificData = $this->generateData($categoryId); + return $categorySpecificData[$key]; } /** - * Queries the database and returns results + * Returns an array of product ids for all DataProductHashMap list, + * that occur in other categories not part of DataCategoryHashMap list * * @param int $categoryId * @return array */ private function generateData($categoryId) { - $productsLinkConnection = $this->connection->getConnection(); - $select = $productsLinkConnection->select() - ->from($this->connection->getTableName('catalog_category_product'), ['category_id']) - ->where( - $productsLinkConnection->prepareSqlCondition( - 'product_id', - [ - 'in' => $this->hashMapPool->getDataMap( - DataProductHashMap::class, - $categoryId - )->getAllData($categoryId) - ] - ) - ) - ->where( - $productsLinkConnection->prepareSqlCondition( - 'category_id', - [ - 'nin' => $this->hashMapPool->getDataMap( - DataCategoryHashMap::class, - $categoryId - )->getAllData($categoryId) - ] + if (!isset($this->hashMap[$categoryId])) { + $productsLinkConnection = $this->connection->getConnection(); + $select = $productsLinkConnection->select() + ->from($this->connection->getTableName('catalog_category_product'), ['category_id']) + ->where( + $productsLinkConnection->prepareSqlCondition( + 'product_id', + [ + 'in' => $this->hashMapPool->getDataMap( + DataProductHashMap::class, + $categoryId + )->getAllData($categoryId) + ] + ) ) - )->group('category_id'); + ->where( + $productsLinkConnection->prepareSqlCondition( + 'category_id', + [ + 'nin' => $this->hashMapPool->getDataMap( + DataCategoryHashMap::class, + $categoryId + )->getAllData($categoryId) + ] + ) + )->group('category_id'); - return $productsLinkConnection->fetchCol($select); + $this->hashMap[$categoryId] = $productsLinkConnection->fetchCol($select); + } + return $this->hashMap[$categoryId]; } /** diff --git a/app/code/Magento/CatalogUrlRewrite/Model/Map/DataProductHashMap.php b/app/code/Magento/CatalogUrlRewrite/Model/Map/DataProductHashMap.php index 74731a7d40e..a20b837e409 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/Map/DataProductHashMap.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/Map/DataProductHashMap.php @@ -45,10 +45,7 @@ class DataProductHashMap implements HashMapInterface */ public function getAllData($categoryId) { - if (!isset($this->hashMap[$categoryId])) { - $this->hashMap[$categoryId] = $this->generateData($categoryId); - } - return $this->hashMap[$categoryId]; + return $this->generateData($categoryId); } /** @@ -56,38 +53,40 @@ class DataProductHashMap implements HashMapInterface */ public function getData($categoryId, $key) { - $this->getAllData($categoryId); - return $this->hashMap[$categoryId][$key]; + $categorySpecificData = $this->generateData($categoryId); + return $categorySpecificData[$key]; } /** - * Queries the database and returns results + * Returns an array of ids of all visible products and assigned to a category and all its subcategories * * @param int $categoryId * @return array */ private function generateData($categoryId) { - $productsCollection = $this->collectionFactory->create(); - $productsCollection->getSelect() - ->joinInner( - ['cp' => $this->connection->getTableName('catalog_category_product')], - 'cp.product_id = e.entity_id', - [] - ) - ->where( - $productsCollection->getConnection()->prepareSqlCondition( - 'cp.category_id', - [ - 'in' => $this->hashMapPool->getDataMap( - DataCategoryHashMap::class, - $categoryId - )->getAllData($categoryId) - ] + if (!isset($this->hashMap[$categoryId])) { + $productsCollection = $this->collectionFactory->create(); + $productsCollection->getSelect() + ->joinInner( + ['cp' => $this->connection->getTableName('catalog_category_product')], + 'cp.product_id = e.entity_id', + [] ) - )->group('e.entity_id'); - - return $productsCollection->getAllIds(); + ->where( + $productsCollection->getConnection()->prepareSqlCondition( + 'cp.category_id', + [ + 'in' => $this->hashMapPool->getDataMap( + DataCategoryHashMap::class, + $categoryId + )->getAllData($categoryId) + ] + ) + )->group('e.entity_id'); + $this->hashMap[$categoryId] = $productsCollection->getAllIds(); + } + return $this->hashMap[$categoryId]; } /** diff --git a/app/code/Magento/CatalogUrlRewrite/Model/Map/DataProductUrlRewriteDatabaseMap.php b/app/code/Magento/CatalogUrlRewrite/Model/Map/DataProductUrlRewriteDatabaseMap.php index bca04d727ae..c369e2d62a6 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/Map/DataProductUrlRewriteDatabaseMap.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/Map/DataProductUrlRewriteDatabaseMap.php @@ -72,7 +72,8 @@ class DataProductUrlRewriteDatabaseMap implements DatabaseMapInterface } /** - * Queries the database and returns the name of the temporary table where data is stored + * Queries the database for all category url rewrites that are affected by the category identified by $categoryId + * It returns the name of the temporary table where the resulting data is stored * * @param int $categoryId * @return string diff --git a/app/code/Magento/CatalogUrlRewrite/Model/Map/DatabaseMapInterface.php b/app/code/Magento/CatalogUrlRewrite/Model/Map/DatabaseMapInterface.php index 32f81e24615..c2a9a89a33f 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/Map/DatabaseMapInterface.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/Map/DatabaseMapInterface.php @@ -5,10 +5,11 @@ */ namespace Magento\CatalogUrlRewrite\Model\Map; -use \Magento\Framework\DB\Select; +use Magento\Framework\DB\Select; /** * Interface for a mysql data type of a map + * * Is used to get data by a unique key from a temporary table in mysql to prevent memory usage * It internally holds the knowledge the creation of the actual data and it initializes itself when we call getData * We should always call destroyTableAdapter when we don't need anymore the temporary tables @@ -17,6 +18,7 @@ interface DatabaseMapInterface { /** * Gets data by key from a map identified by a category Id + * * The key is a unique identifier that matches the values of the index used to build the temporary table * * Example "1_2" where ids would correspond to store_id entity_id diff --git a/app/code/Magento/CatalogUrlRewrite/Model/Map/DatabaseMapPool.php b/app/code/Magento/CatalogUrlRewrite/Model/Map/DatabaseMapPool.php index 5dbe1ed12b8..30732013eac 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/Map/DatabaseMapPool.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/Map/DatabaseMapPool.php @@ -50,6 +50,11 @@ class DatabaseMapPool 'category' => $categoryId ] ); + if (!$this->dataArray[$key] instanceof DatabaseMapInterface) { + throw new \InvalidArgumentException( + $instanceName . ' does not implement interface ' . DatabaseMapInterface::class + ); + } } return $this->dataArray[$key]; } diff --git a/app/code/Magento/CatalogUrlRewrite/Model/Map/HashMapInterface.php b/app/code/Magento/CatalogUrlRewrite/Model/Map/HashMapInterface.php index 3e806fcbd80..341cdb1a4fd 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/Map/HashMapInterface.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/Map/HashMapInterface.php @@ -5,11 +5,12 @@ */ namespace Magento\CatalogUrlRewrite\Model\Map; -use \Magento\Framework\DB\Select; +use Magento\Framework\DB\Select; /** * Interface for a hash data map - * It is used for classes tht would build hash maps and store them into memory + * + * It is used for classes that will build hash maps and store them into memory * The initialization is done transparently whenever getAllData or getData is called * The map, upon initialization, might have a dependency on some other DataMapInterfaces * The map has to free memory after we're done using it diff --git a/app/code/Magento/CatalogUrlRewrite/Model/Map/HashMapPool.php b/app/code/Magento/CatalogUrlRewrite/Model/Map/HashMapPool.php index 74ede5393cf..bc6be808d4d 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/Map/HashMapPool.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/Map/HashMapPool.php @@ -44,10 +44,6 @@ class HashMapPool public function getDataMap($instanceName, $categoryId) { $key = $instanceName . '-' . $categoryId; - $reflectionClass = new \ReflectionClass($instanceName); - if (!$reflectionClass->implementsInterface(HashMapInterface::class)) { - throw new \Exception($instanceName . ' does not implement interface ' . HashMapInterface::class); - } if (!isset($this->dataArray[$key])) { $this->dataArray[$key] = $this->objectManager->create( $instanceName, @@ -55,6 +51,11 @@ class HashMapPool 'category' => $categoryId ] ); + if (!$this->dataArray[$key] instanceof HashMapInterface) { + throw new \InvalidArgumentException( + $instanceName . ' does not implement interface ' . HashMapInterface::class + ); + } } return $this->dataArray[$key]; } diff --git a/app/code/Magento/CatalogUrlRewrite/Model/Map/UrlRewriteFinder.php b/app/code/Magento/CatalogUrlRewrite/Model/Map/UrlRewriteFinder.php index dfa8d87644a..bcadfd848e8 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/Map/UrlRewriteFinder.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/Map/UrlRewriteFinder.php @@ -11,8 +11,11 @@ use Magento\UrlRewrite\Service\V1\Data\UrlRewrite; use Magento\UrlRewrite\Service\V1\Data\UrlRewriteFactory; /** - * Allows query to Category and Product UrlRewrite Database Map or UrlFinderInterface by identifiers + * Finds specific queried url rewrites identified by specific fields * + * A group of identifiers specifies a query consumed by the client to retrieve existing url rewrites from the database + * Clients will query a map of DatabaseMapInterface type through this class resulting into a set of url rewrites results + * Each map type will fallback to a UrlFinderInterface by identifiers for unmapped values */ class UrlRewriteFinder { @@ -29,7 +32,7 @@ class UrlRewriteFinder private $urlRewritePrototype; /** @var array */ - private $urlRewriteClassNames; + private $urlRewriteClassNames = []; /** * @param DatabaseMapPool $databaseMapPool @@ -41,10 +44,7 @@ class UrlRewriteFinder DatabaseMapPool $databaseMapPool, UrlFinderInterface $urlFinder, UrlRewriteFactory $urlRewriteFactory, - $urlRewriteClassNames = [ - self::ENTITY_TYPE_PRODUCT => DataProductUrlRewriteDatabaseMap::class, - self::ENTITY_TYPE_CATEGORY => DataCategoryUrlRewriteDatabaseMap::class - ] + array $urlRewriteClassNames = [] ) { $this->databaseMapPool = $databaseMapPool; $this->urlFinder = $urlFinder; @@ -53,7 +53,8 @@ class UrlRewriteFinder } /** - * Queries by identifiers from maps or falls-back to UrlFinderInterface + * Retrieves existing url rewrites filtered by identifiers from prebuild database maps + * This method will fall-back to by using UrlFinderInterface when map type is not found in configured list * * @param int $entityId * @param int $storeId @@ -86,7 +87,7 @@ class UrlRewriteFinder } /** - * Transfer array values to url rewrite object values + * Transfers an array values to url rewrite object values * * @param array $data * @return UrlRewrite[] diff --git a/app/code/Magento/CatalogUrlRewrite/Model/Product/CurrentUrlRewritesRegenerator.php b/app/code/Magento/CatalogUrlRewrite/Model/Product/CurrentUrlRewritesRegenerator.php index b801a84cc48..dee95f23684 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/Product/CurrentUrlRewritesRegenerator.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/Product/CurrentUrlRewritesRegenerator.php @@ -75,8 +75,9 @@ class CurrentUrlRewritesRegenerator $this->urlRewriteFactory = $urlRewriteFactory; $this->urlRewritePrototype = $urlRewriteFactory->create(); $this->urlRewriteFinder = $urlRewriteFinder ?: ObjectManager::getInstance()->get(UrlRewriteFinder::class); - $mergeDataProviderFactory = $mergeDataProviderFactory ?: ObjectManager::getInstance() - ->get(MergeDataProviderFactory::class); + if (!isset($mergeDataProviderFactory)) { + $mergeDataProviderFactory = ObjectManager::getInstance()->get(MergeDataProviderFactory::class); + } $this->mergeDataProviderPrototype = $mergeDataProviderFactory->create(); } diff --git a/app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php b/app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php index 4df9fde0f26..6e357459de8 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/ProductScopeRewriteGenerator.php @@ -88,8 +88,9 @@ class ProductScopeRewriteGenerator $this->categoriesUrlRewriteGenerator = $categoriesUrlRewriteGenerator; $this->currentUrlRewritesRegenerator = $currentUrlRewritesRegenerator; $this->anchorUrlRewriteGenerator = $anchorUrlRewriteGenerator; - $mergeDataProviderFactory = $mergeDataProviderFactory ?: ObjectManager::getInstance() - ->get(MergeDataProviderFactory::class); + if (!isset($mergeDataProviderFactory)) { + $mergeDataProviderFactory = ObjectManager::getInstance()->get(MergeDataProviderFactory::class); + } $this->mergeDataProviderPrototype = $mergeDataProviderFactory->create(); } diff --git a/app/code/Magento/CatalogUrlRewrite/Observer/AfterImportDataObserver.php b/app/code/Magento/CatalogUrlRewrite/Observer/AfterImportDataObserver.php index 27053c21cff..f76db7bef44 100644 --- a/app/code/Magento/CatalogUrlRewrite/Observer/AfterImportDataObserver.php +++ b/app/code/Magento/CatalogUrlRewrite/Observer/AfterImportDataObserver.php @@ -134,8 +134,9 @@ class AfterImportDataObserver implements ObserverInterface $this->storeManager = $storeManager; $this->urlRewriteFactory = $urlRewriteFactory; $this->urlFinder = $urlFinder; - $mergeDataProviderFactory = $mergeDataProviderFactory ?: ObjectManager::getInstance() - ->get(MergeDataProviderFactory::class); + if (!isset($mergeDataProviderFactory)) { + $mergeDataProviderFactory = ObjectManager::getInstance()->get(MergeDataProviderFactory::class); + } $this->mergeDataProviderPrototype = $mergeDataProviderFactory->create(); } diff --git a/app/code/Magento/CatalogUrlRewrite/Observer/CategoryProcessUrlRewriteSavingObserver.php b/app/code/Magento/CatalogUrlRewrite/Observer/CategoryProcessUrlRewriteSavingObserver.php index b864285b06a..dd0250049a4 100644 --- a/app/code/Magento/CatalogUrlRewrite/Observer/CategoryProcessUrlRewriteSavingObserver.php +++ b/app/code/Magento/CatalogUrlRewrite/Observer/CategoryProcessUrlRewriteSavingObserver.php @@ -71,32 +71,17 @@ class CategoryProcessUrlRewriteSavingObserver implements ObserverInterface || $category->dataHasChangedFor('is_anchor') || $category->getIsChangedProductList() ) { - $this->initializeUrlRewritesDataMaps($category); - $categoryUrlRewriteResult = $this->categoryUrlRewriteGenerator->generate($category); $this->urlRewriteBunchReplacer->doBunchReplace($categoryUrlRewriteResult); $productUrlRewriteResult = $this->urlRewriteHandler->generateProductUrlRewrites($category); $this->urlRewriteBunchReplacer->doBunchReplace($productUrlRewriteResult); + //frees memory for maps that are self-initialized in multiple classes that were called by the generators $this->resetUrlRewritesDataMaps($category); } } - /** - * Initializes data maps to be further used - * - * @param Category $category - * @return void - */ - private function initializeUrlRewritesDataMaps($category) - { - foreach ($this->dataUrlRewriteClassNames as $className) { - $this->databaseMapPool->getDataMap($className, $category->getEntityId()); - } - - } - /** * Resets used data maps to free up memory and temporary tables * diff --git a/app/code/Magento/CatalogUrlRewrite/Observer/UrlRewriteHandler.php b/app/code/Magento/CatalogUrlRewrite/Observer/UrlRewriteHandler.php index 464824e7eef..727dd744cd8 100644 --- a/app/code/Magento/CatalogUrlRewrite/Observer/UrlRewriteHandler.php +++ b/app/code/Magento/CatalogUrlRewrite/Observer/UrlRewriteHandler.php @@ -64,8 +64,9 @@ class UrlRewriteHandler $this->productUrlRewriteGenerator = $productUrlRewriteGenerator; $this->urlPersist = $urlPersist; $this->productCollectionFactory = $productCollectionFactory; - $mergeDataProviderFactory = $mergeDataProviderFactory ?: ObjectManager::getInstance() - ->get(MergeDataProviderFactory::class); + if (!isset($mergeDataProviderFactory)) { + $mergeDataProviderFactory = $mergeDataProviderFactory = ObjectManager::getInstance()->get(MergeDataProviderFactory::class); + } $this->mergeDataProviderPrototype = $mergeDataProviderFactory->create(); } diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/CategoryUrlPathGeneratorTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/CategoryUrlPathGeneratorTest.php index d5abe69ff79..f36799f789f 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/CategoryUrlPathGeneratorTest.php +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/CategoryUrlPathGeneratorTest.php @@ -5,8 +5,7 @@ */ namespace Magento\CatalogUrlRewrite\Test\Unit\Model; -use \Magento\CatalogUrlRewrite\Model\CategoryUrlPathGenerator; - +use Magento\CatalogUrlRewrite\Model\CategoryUrlPathGenerator; use Magento\Catalog\Model\Category; use Magento\Store\Model\ScopeInterface; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Map/HashMapPoolTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Map/HashMapPoolTest.php index d9351b4e426..78b114f60da 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Map/HashMapPoolTest.php +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Map/HashMapPoolTest.php @@ -47,8 +47,22 @@ class HashMapPoolTest extends \PHPUnit_Framework_TestCase ->method('create') ->willReturnOnConsecutiveCalls($dataCategoryMapMock, $dataProductMapMock, $dataProductMapMockOtherCategory); $this->assertEquals($dataCategoryMapMock, $this->model->getDataMap(DataCategoryHashMap::class, 1)); - $this->assertEquals($dataCategoryMapMock, $this->model->getDataMap(DataCategoryHashMap::class, 1)); + $this->assertSame($dataCategoryMapMock, $this->model->getDataMap(DataCategoryHashMap::class, 1)); $this->assertEquals($dataProductMapMock, $this->model->getDataMap(DataProductHashMap::class, 1)); $this->assertEquals($dataProductMapMockOtherCategory, $this->model->getDataMap(DataCategoryHashMap::class, 2)); } + + /** + * Tests getDataMap with exception + */ + public function testGetDataMapException() + { + $nonInterface = $this->getMock(HashMapPool::class, [], [], '', false); + + $this->objectManagerMock->expects($this->any()) + ->method('create') + ->willReturn($nonInterface); + $this->setExpectedException(\InvalidArgumentException::class); + $this->model->getDataMap(HashMapPool::class, 1); + } } diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Map/UrlRewriteFinderTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Map/UrlRewriteFinderTest.php index 2a8149960c6..b0c4500dab3 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Map/UrlRewriteFinderTest.php +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Map/UrlRewriteFinderTest.php @@ -45,12 +45,18 @@ class UrlRewriteFinderTest extends \PHPUnit_Framework_TestCase ->method('create') ->willReturn($this->urlRewritePrototypeMock); + $urlRewriteClassesNamesArray = [ + UrlRewriteFinder::ENTITY_TYPE_PRODUCT => DataProductUrlRewriteDatabaseMap::class, + UrlRewriteFinder::ENTITY_TYPE_CATEGORY => DataCategoryUrlRewriteDatabaseMap::class + ]; + $this->model = (new ObjectManager($this))->getObject( UrlRewriteFinder::class, [ 'databaseMapPool' => $this->databaseMapPoolMock, 'urlFinder' => $this->urlFinderMock, - 'urlRewriteFactory' => $this->urlRewriteFactoryMock + 'urlRewriteFactory' => $this->urlRewriteFactoryMock, + 'urlRewriteClassNames' => $urlRewriteClassesNamesArray ] ); } diff --git a/app/code/Magento/CatalogUrlRewrite/etc/di.xml b/app/code/Magento/CatalogUrlRewrite/etc/di.xml index 5a8974fc52d..7f0fc718083 100644 --- a/app/code/Magento/CatalogUrlRewrite/etc/di.xml +++ b/app/code/Magento/CatalogUrlRewrite/etc/di.xml @@ -23,4 +23,12 @@ <type name="Magento\UrlRewrite\Model\StorageInterface"> <plugin name="storage_plugin" type="Magento\CatalogUrlRewrite\Model\Category\Plugin\Storage"/> </type> + <type name="Magento\CatalogUrlRewrite\Model\Map\UrlRewriteFinder"> + <arguments> + <argument name="urlRewriteClassNames" xsi:type="array"> + <item name="product" xsi:type="string">Magento\CatalogUrlRewrite\Model\Map\DataProductUrlRewriteDatabaseMap</item> + <item name="category" xsi:type="string">Magento\CatalogUrlRewrite\Model\Map\DataCategoryUrlRewriteDatabaseMap</item> + </argument> + </arguments> + </type> </config> diff --git a/app/code/Magento/UrlRewrite/Test/Unit/Model/MergeDataProviderTest.php b/app/code/Magento/UrlRewrite/Test/Unit/Model/MergeDataProviderTest.php index 3e57c7176b7..76aa5d368be 100644 --- a/app/code/Magento/UrlRewrite/Test/Unit/Model/MergeDataProviderTest.php +++ b/app/code/Magento/UrlRewrite/Test/Unit/Model/MergeDataProviderTest.php @@ -35,7 +35,7 @@ class MergeDataProviderTest extends \PHPUnit_Framework_TestCase * @param array $urlRewriteMockArray * @param String $expectedData * @param int $arrayCount - * @dataProvider getMergeTestParameters + * @dataProvider mergeDataProvider * @return void */ public function testMerge($urlRewriteMockArray, $expectedData, $arrayCount) @@ -56,11 +56,11 @@ class MergeDataProviderTest extends \PHPUnit_Framework_TestCase } /** - * Data provider for testMerge + * Data provider for testMerge * * @return array */ - public function getMergeTestParameters() + public function mergeDataProvider() { $urlRewriteMock1 = $this->getMock(UrlRewrite::class, [], [], '', false); diff --git a/lib/internal/Magento/Framework/DB/TemporaryTableService.php b/lib/internal/Magento/Framework/DB/TemporaryTableService.php index afc4d884638..62d1a0d65cf 100644 --- a/lib/internal/Magento/Framework/DB/TemporaryTableService.php +++ b/lib/internal/Magento/Framework/DB/TemporaryTableService.php @@ -71,7 +71,11 @@ class TemporaryTableService foreach ($indexes as $indexName => $columns) { $renderedColumns = implode(',', array_map([$adapter, 'quoteIdentifier'], $columns)); - $indexType = sprintf('INDEX %s USING %s', $adapter->quoteIdentifier($indexName), "{$indexMethod}"); + $indexType = sprintf( + 'INDEX %s USING %s', + $adapter->quoteIdentifier($indexName), + $indexMethod + ); if ($indexName === 'PRIMARY') { $indexType = 'PRIMARY KEY'; @@ -86,7 +90,7 @@ class TemporaryTableService 'CREATE TEMPORARY TABLE %s %s ENGINE=%s IGNORE (%s)', $adapter->quoteIdentifier($name), $indexStatements ? '(' . implode(',', $indexStatements) . ')' : '', - "{$engine}", + $adapter->quoteIdentifier($engine), "{$select}" ); -- GitLab