From d012e761111ceb1fd0492c241f8dde7f1f6f8b4b Mon Sep 17 00:00:00 2001 From: Cristiano Casciotti Date: Fri, 17 Mar 2017 17:17:13 +0100 Subject: [PATCH 1/5] Lowercase-ing category name used in path to avoid duplication of URL keys due to both uppercase and lowercase categories' name --- .../Model/Import/Product.php | 3 ++ .../Import/Product/CategoryProcessor.php | 46 ++++++++++++++++--- .../Import/Product/CategoryProcessorTest.php | 18 +++++++- 3 files changed, 60 insertions(+), 7 deletions(-) diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Product.php b/app/code/Magento/CatalogImportExport/Model/Import/Product.php index 2ad5bcb8e65ae..64f91243e31f4 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Product.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Product.php @@ -1529,6 +1529,9 @@ protected function _saveProducts() $existingImages = $this->getExistingImages($bunch); foreach ($bunch as $rowNum => $rowData) { + // reset category processor's failed categories array + $this->categoryProcessor->clearFailedCategories(); + if (!$this->validateRow($rowData, $rowNum)) { continue; } diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php b/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php index f327aca49b781..7bc2f2e39a3ae 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php @@ -79,7 +79,10 @@ protected function initCategories() for ($i = 1; $i < $pathSize; $i++) { $path[] = $collection->getItemById((int)$structure[$i])->getName(); } - $index = implode(self::DELIMITER_CATEGORY, $path); + /** @var string $index */ + $index = $this->standardizeString( + implode(self::DELIMITER_CATEGORY, $path) + ); $this->categories[$index] = $category->getId(); } } @@ -123,13 +126,16 @@ protected function createCategory($name, $parentId) */ protected function upsertCategory($categoryPath) { - if (!isset($this->categories[$categoryPath])) { + /** @var string $index */ + $index = $this->standardizeString($categoryPath); + + if (!isset($this->categories[$index])) { $pathParts = explode(self::DELIMITER_CATEGORY, $categoryPath); $parentId = \Magento\Catalog\Model\Category::TREE_ROOT_ID; $path = ''; foreach ($pathParts as $pathPart) { - $path .= $pathPart; + $path .= $this->standardizeString($pathPart); if (!isset($this->categories[$path])) { $this->categories[$path] = $this->createCategory($pathPart, $parentId); } @@ -138,7 +144,7 @@ protected function upsertCategory($categoryPath) } } - return $this->categories[$categoryPath]; + return $this->categories[$index]; } /** @@ -171,7 +177,7 @@ public function upsertCategories($categoriesString, $categoriesSeparator) * @param string $category * @param \Magento\Framework\Exception\AlreadyExistsException $exception * - * @return array + * @return $this */ private function addFailedCategory($category, $exception) { @@ -190,7 +196,18 @@ private function addFailedCategory($category, $exception) */ public function getFailedCategories() { - return $this->failedCategories; + return $this->failedCategories; + } + + /** + * Resets failed categories' array + * + * @return $this + */ + public function clearFailedCategories() + { + $this->failedCategories = []; + return $this; } /** @@ -204,4 +221,21 @@ public function getCategoryById($categoryId) { return isset($this->categoriesCache[$categoryId]) ? $this->categoriesCache[$categoryId] : null; } + + /** + * Standardize a string by lowercase-ing it + * + * @param string $string + * @return string + */ + protected function standardizeString(string $string) + { + if (function_exists('mb_strtolower')) { + $string = mb_strtolower($string); + } else { + $string = strtolower($string); + } + + return $string; + } } diff --git a/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/Product/CategoryProcessorTest.php b/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/Product/CategoryProcessorTest.php index f7abb2d788806..1557d4ab67855 100644 --- a/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/Product/CategoryProcessorTest.php +++ b/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/Product/CategoryProcessorTest.php @@ -44,7 +44,7 @@ protected function setUp() ->disableOriginalConstructor() ->getMock(); $childCategory->method('getId')->will($this->returnValue(self::CHILD_CATEGORY_ID)); - $childCategory->method('getName')->will($this->returnValue('Child')); + $childCategory->method('getName')->will($this->returnValue(self::CHILD_CATEGORY_NAME)); $childCategory->method('getPath')->will($this->returnValue( self::PARENT_CATEGORY_ID . CategoryProcessor::DELIMITER_CATEGORY . self::CHILD_CATEGORY_ID @@ -115,6 +115,22 @@ public function testUpsertCategories() $this->assertArrayHasKey(self::CHILD_CATEGORY_ID, array_flip($categoryIds)); } + public function testClearFailedCategories() + { + $dummyFailedCategory = [ + [ + 'category' => 'dummy category', + 'exception' => 'dummy exception', + ] + ]; + + $this->setPropertyValue($this->categoryProcessor, 'failedCategories', $dummyFailedCategory); + $this->assertCount(count($dummyFailedCategory), $this->categoryProcessor->getFailedCategories()); + + $this->categoryProcessor->clearFailedCategories(); + $this->assertEmpty($this->categoryProcessor->getFailedCategories()); + } + /** * Cover getCategoryById(). * From 5a2ddf339e1dfb1e10ac6cd0d67ef1c7723e2491 Mon Sep 17 00:00:00 2001 From: Cristiano Casciotti Date: Mon, 20 Mar 2017 09:28:56 +0100 Subject: [PATCH 2/5] removed type hint for php 5.6 compatibility --- .../Model/Import/Product/CategoryProcessor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php b/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php index 7bc2f2e39a3ae..6c4c67f7347f2 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php @@ -228,7 +228,7 @@ public function getCategoryById($categoryId) * @param string $string * @return string */ - protected function standardizeString(string $string) + protected function standardizeString($string) { if (function_exists('mb_strtolower')) { $string = mb_strtolower($string); From 718edfe074fe79acc833e763871ec80c7af5ae1c Mon Sep 17 00:00:00 2001 From: Cristiano Casciotti Date: Mon, 20 Mar 2017 12:00:02 +0100 Subject: [PATCH 3/5] enhanced code quality --- .../Model/Import/Product/CategoryProcessor.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php b/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php index 6c4c67f7347f2..43c66f3a40576 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php @@ -231,11 +231,9 @@ public function getCategoryById($categoryId) protected function standardizeString($string) { if (function_exists('mb_strtolower')) { - $string = mb_strtolower($string); - } else { - $string = strtolower($string); + return mb_strtolower($string); } - return $string; + return strtolower($string); } } From ea1dd49b49f1c68db3e29069e6610dd61b7618db Mon Sep 17 00:00:00 2001 From: Cristiano Casciotti Date: Fri, 24 Mar 2017 13:02:39 +0100 Subject: [PATCH 4/5] removed check for mbstring function --- .../Model/Import/Product/CategoryProcessor.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php b/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php index 43c66f3a40576..b902430ef3210 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php @@ -230,10 +230,6 @@ public function getCategoryById($categoryId) */ protected function standardizeString($string) { - if (function_exists('mb_strtolower')) { - return mb_strtolower($string); - } - - return strtolower($string); + return mb_strtolower($string); } } From 5bbc9082c91fff27caed80f5aff9c46336394291 Mon Sep 17 00:00:00 2001 From: Cristiano Casciotti Date: Fri, 24 Mar 2017 15:42:13 +0100 Subject: [PATCH 5/5] make method private, edited docblock --- .../Model/Import/Product/CategoryProcessor.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php b/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php index b902430ef3210..d84543dfef903 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Product/CategoryProcessor.php @@ -223,12 +223,14 @@ public function getCategoryById($categoryId) } /** - * Standardize a string by lowercase-ing it + * Standardize a string. + * For now it performs only a lowercase action, this method is here to include more complex checks in the future + * if needed. * * @param string $string * @return string */ - protected function standardizeString($string) + private function standardizeString($string) { return mb_strtolower($string); }