MAGETWO-66480: [GITHUB] URL key for specified store already exists #6671

- refactoring per code review

Author: cpartica

app/code/Magento/CatalogUrlRewrite/Observer/CategoryProcessUrlRewriteMovingObserver.php

@@ -19,29 +19,25 @@
 
 /**
  * Generates Category Url Rewrites after move/save and Products Url Rewrites assigned to the category that's being saved
- *
- * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class CategoryProcessUrlRewriteMovingObserver implements ObserverInterface
 {
-    /** @var CategoryUrlRewriteGenerator */
+    /** @var \Magento\CatalogUrlRewrite\Model\CategoryUrlRewriteGenerator */
     protected $categoryUrlRewriteGenerator;
 
-    /** @var UrlPersistInterface */
+    /** @var \Magento\UrlRewrite\Model\UrlPersistInterface */
     protected $urlPersist;
 
-    /** @var ScopeConfigInterface */
+    /** @var \Magento\Framework\App\Config\ScopeConfigInterface */
     protected $scopeConfig;
 
-    /** @var UrlRewriteHandler */
+    /** @var \Magento\CatalogUrlRewrite\Observer\UrlRewriteHandler */
     protected $urlRewriteHandler;
 
-    /**
-     * @var UrlRewriteBunchReplacer
-     */
+    /** @var \Magento\CatalogUrlRewrite\Model\UrlRewriteBunchReplacer */
     private $urlRewriteBunchReplacer;
 
-    /** @var DatabaseMapPool */
+    /** @var \Magento\CatalogUrlRewrite\Model\Map\DatabaseMapPool */
     private $databaseMapPool;
 
     /** @var string[] */
@@ -53,7 +49,7 @@ class CategoryProcessUrlRewriteMovingObserver implements ObserverInterface
      * @param ScopeConfigInterface $scopeConfig
      * @param UrlRewriteHandler $urlRewriteHandler
      * @param UrlRewriteBunchReplacer $urlRewriteBunchReplacer
-     * @param DatabaseMapPool $databaseMapPool
+     * @param \Magento\CatalogUrlRewrite\Model\Map\DatabaseMapPool $databaseMapPool
      * @param string[] $dataUrlRewriteClassNames
      */
     public function __construct(

app/code/Magento/CatalogUrlRewrite/Observer/CategoryProcessUrlRewriteSavingObserver.php

@@ -18,16 +18,16 @@
  */
 class CategoryProcessUrlRewriteSavingObserver implements ObserverInterface
 {
-    /** @var CategoryUrlRewriteGenerator */
+    /** @var \Magento\CatalogUrlRewrite\Model\CategoryUrlRewriteGenerator */
     private $categoryUrlRewriteGenerator;
 
-    /** @var UrlRewriteBunchReplacer */
+    /** @var \Magento\CatalogUrlRewrite\Model\UrlRewriteBunchReplacer */
     private $urlRewriteBunchReplacer;
 
-    /** @var UrlRewriteHandler */
+    /** @var \Magento\CatalogUrlRewrite\Observer\UrlRewriteHandler */
     private $urlRewriteHandler;
 
-    /** @var DatabaseMapPool */
+    /** @var \Magento\CatalogUrlRewrite\Model\Map\DatabaseMapPool */
     private $databaseMapPool;
 
     /** @var string[] */
@@ -71,15 +71,23 @@ public function execute(\Magento\Framework\Event\Observer $observer)
         if ($category->getParentId() == Category::TREE_ROOT_ID) {
             return;
         }
+
+        $mapsGenerated = false;
         if ($category->dataHasChangedFor('url_key')
             || $category->dataHasChangedFor('is_anchor')
             || $category->getIsChangedProductList()
         ) {
-            $categoryUrlRewriteResult = $this->categoryUrlRewriteGenerator->generate($category);
-            $this->urlRewriteBunchReplacer->doBunchReplace($categoryUrlRewriteResult);
+            if ($category->dataHasChangedFor('url_key')) {
+                $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
+            $mapsGenerated = true;
+        }
+
+        //frees memory for maps that are self-initialized in multiple classes that were called by the generators
+        if ($mapsGenerated) {
             $this->resetUrlRewritesDataMaps($category);
         }
     }

app/code/Magento/UrlRewrite/Model/Message/UrlRewriteExceptionMessageFactory.php

@@ -14,7 +14,7 @@
 
 class UrlRewriteExceptionMessageFactory implements ExceptionMessageFactoryInterface
 {
-    const URL_DUPLICATE_MESSAGE = 'urlDuplicateMessage';
+    const URL_DUPLICATE_MESSAGE_MAP_ID = 'urlDuplicateMessageMapId';
 
     /** @var Factory */
     private $messageFactory;
@@ -52,7 +52,7 @@ public function createMessage(\Exception $exception, $type = MessageInterface::T
                 }
             }
             return $this->messageFactory->create($type)
-                ->setIdentifier(self::URL_DUPLICATE_MESSAGE)
+                ->setIdentifier(self::URL_DUPLICATE_MESSAGE_MAP_ID)
                 ->setText($exception->getMessage())
                 ->setData(['urls' => $generatedUrls]);
         }

app/code/Magento/UrlRewrite/Test/Unit/Model/Message/UrlRewriteExceptionMessageFactoryTest.php

@@ -67,7 +67,7 @@ public function testCreateMessage()
 
         $message->expects($this->once())
             ->method('setIdentifier')
-            ->with(UrlRewriteExceptionMessageFactory::URL_DUPLICATE_MESSAGE)
+            ->with(UrlRewriteExceptionMessageFactory::URL_DUPLICATE_MESSAGE_MAP_ID)
             ->willReturnSelf();
 
         $message->expects($this->once())

app/code/Magento/UrlRewrite/etc/adminhtml/di.xml

@@ -16,7 +16,7 @@
     <type name="Magento\Framework\View\Element\Message\MessageConfigurationsPool">
         <arguments>
             <argument name="configurationsMap" xsi:type="array">
-                <item name="urlDuplicateMessage" xsi:type="array">
+                <item name="urlDuplicateMessageMapId" xsi:type="array">
                     <item name="renderer" xsi:type="const">\Magento\Framework\View\Element\Message\Renderer\BlockRenderer::CODE</item>
                     <item name="data" xsi:type="array">
                         <item name="template" xsi:type="string">Magento_UrlRewrite::messages/url_duplicate_message.phtml</item>