ACP2E-1201: Incorrect usage of link field (row_id) in app/code/Magento/CatalogUrlRewrite/Model/ResourceModel/Category/GetDefaultUrlKey.php

arnobsh · arnobsh · commit 0ed8060bd8f4 · 2022-11-01T02:09:07.000-05:00
- with test
diff --git a/app/code/Magento/CatalogUrlRewrite/Model/ResourceModel/Category/GetDefaultUrlKey.php b/app/code/Magento/CatalogUrlRewrite/Model/ResourceModel/Category/GetDefaultUrlKey.php
@@ -13,6 +13,7 @@
 use Magento\Framework\App\ResourceConnection;
 use Magento\Framework\DB\Select;
 use Magento\Framework\EntityManager\MetadataPool;
+use Magento\Framework\Exception\LocalizedException;
 use Magento\Store\Model\Store;
 
 /**
@@ -53,18 +54,19 @@ public function __construct(
     /**
      * Retrieve 'url_key' value for default store.
      *
-     * @param int $categoryId
+     * @param int $id
      * @return string|null
+     * @throws LocalizedException
      */
-    public function execute(int $categoryId): ?string
+    public function execute(int $id): ?string
     {
         $metadata = $this->metadataPool->getMetadata(CategoryInterface::class);
         $entityTypeId = $this->eavConfig->getEntityType(Category::ENTITY)->getId();
         $linkField = $metadata->getLinkField();
         $whereConditions = [
             'e.entity_type_id = ' . $entityTypeId,
             "e.attribute_code = 'url_key'",
-            'c.' . $linkField . ' = ' . $categoryId,
+            'c.' . $linkField . ' = ' . $id,
             'c.store_id = ' . Store::DEFAULT_STORE_ID,
         ];
         $connection = $this->resourceConnection->getConnection();
diff --git a/app/code/Magento/CatalogUrlRewrite/Observer/CategoryUrlPathAutogeneratorObserver.php b/app/code/Magento/CatalogUrlRewrite/Observer/CategoryUrlPathAutogeneratorObserver.php
@@ -8,11 +8,14 @@
 namespace Magento\CatalogUrlRewrite\Observer;
 
 use Magento\Catalog\Api\CategoryRepositoryInterface;
+use Magento\Catalog\Api\Data\CategoryInterface;
 use Magento\Catalog\Model\Category;
 use Magento\CatalogUrlRewrite\Model\Category\ChildrenCategoriesProvider;
 use Magento\CatalogUrlRewrite\Model\CategoryUrlPathGenerator;
 use Magento\CatalogUrlRewrite\Model\ResourceModel\Category\GetDefaultUrlKey;
 use Magento\CatalogUrlRewrite\Service\V1\StoreViewService;
+use Magento\Framework\App\ObjectManager;
+use Magento\Framework\EntityManager\MetadataPool;
 use Magento\Framework\Event\Observer;
 use Magento\Framework\Event\ObserverInterface;
 use Magento\Framework\Exception\LocalizedException;
@@ -22,6 +25,8 @@
 
 /**
  * Class for set or update url path.
+ *
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class CategoryUrlPathAutogeneratorObserver implements ObserverInterface
 {
@@ -56,28 +61,37 @@ class CategoryUrlPathAutogeneratorObserver implements ObserverInterface
      */
     private $getDefaultUrlKey;
 
+    /**
+     * @var MetadataPool
+     */
+    private $metadataPool;
+
     /**
      * @param CategoryUrlPathGenerator $categoryUrlPathGenerator
      * @param ChildrenCategoriesProvider $childrenCategoriesProvider
      * @param StoreViewService $storeViewService
      * @param CategoryRepositoryInterface $categoryRepository
      * @param CompositeUrlKey $compositeUrlValidator
      * @param GetDefaultUrlKey $getDefaultUrlKey
+     * @param MetadataPool|null $metadataPool
      */
     public function __construct(
         CategoryUrlPathGenerator $categoryUrlPathGenerator,
         ChildrenCategoriesProvider $childrenCategoriesProvider,
         StoreViewService $storeViewService,
         CategoryRepositoryInterface $categoryRepository,
         CompositeUrlKey $compositeUrlValidator,
-        GetDefaultUrlKey $getDefaultUrlKey
+        GetDefaultUrlKey $getDefaultUrlKey,
+        ?MetadataPool $metadataPool = null
     ) {
         $this->categoryUrlPathGenerator = $categoryUrlPathGenerator;
         $this->childrenCategoriesProvider = $childrenCategoriesProvider;
         $this->storeViewService = $storeViewService;
         $this->categoryRepository = $categoryRepository;
         $this->compositeUrlValidator = $compositeUrlValidator;
         $this->getDefaultUrlKey = $getDefaultUrlKey;
+        $this->metadataPool = $metadataPool ?: ObjectManager::getInstance()
+            ->get(MetadataPool::class);
     }
 
     /**
@@ -101,9 +115,14 @@ public function execute(Observer $observer)
                 $this->updateUrlKey($category, $resultUrlKey);
             }
             if ($category->hasChildren()) {
-                $defaultUrlKey = $this->getDefaultUrlKey->execute((int)$category->getId());
-                if ($defaultUrlKey) {
-                    $this->updateUrlKey($category, $defaultUrlKey);
+                $metadata = $this->metadataPool->getMetadata(CategoryInterface::class);
+                $linkField = $metadata->getLinkField();
+                $id = $category->getData($linkField);
+                if ($id) {
+                    $defaultUrlKey = $this->getDefaultUrlKey->execute((int)$id);
+                    if ($defaultUrlKey) {
+                        $this->updateUrlKey($category, $defaultUrlKey);
+                    }
                 }
             }
             $category->setUrlKey(null)->setUrlPath(null);
diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/CategoryUrlPathAutogeneratorObserverTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Observer/CategoryUrlPathAutogeneratorObserverTest.php
@@ -7,13 +7,16 @@
 
 namespace Magento\CatalogUrlRewrite\Test\Unit\Observer;
 
+use Magento\Catalog\Api\Data\CategoryInterface;
 use Magento\Catalog\Model\Category;
 use Magento\Catalog\Model\ResourceModel\Category as CategoryResource;
 use Magento\CatalogUrlRewrite\Model\Category\ChildrenCategoriesProvider;
 use Magento\CatalogUrlRewrite\Model\CategoryUrlPathGenerator;
 use Magento\CatalogUrlRewrite\Model\ResourceModel\Category\GetDefaultUrlKey;
 use Magento\CatalogUrlRewrite\Observer\CategoryUrlPathAutogeneratorObserver;
 use Magento\CatalogUrlRewrite\Service\V1\StoreViewService;
+use Magento\Framework\EntityManager\EntityMetadataInterface;
+use Magento\Framework\EntityManager\MetadataPool;
 use Magento\Framework\Event\Observer;
 use Magento\Framework\Exception\LocalizedException;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper;
@@ -24,6 +27,8 @@
 
 /**
  * Unit tests for \Magento\CatalogUrlRewrite\Observer\CategoryUrlPathAutogeneratorObserver class.
+ *
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class CategoryUrlPathAutogeneratorObserverTest extends TestCase
 {
@@ -72,6 +77,16 @@ class CategoryUrlPathAutogeneratorObserverTest extends TestCase
      */
     private $getDefaultUrlKey;
 
+    /**
+     * @var MetadataPool|MockObject
+     */
+    private $metadataPool;
+
+    /**
+     * @var EntityMetadataInterface|MockObject
+     */
+    private $entityMetaDataInterface;
+
     /**
      * @inheritDoc
      */
@@ -83,17 +98,21 @@ protected function setUp(): void
             ->disableOriginalConstructor()
             ->getMock();
         $this->categoryResource = $this->createMock(CategoryResource::class);
-        $this->category = $this->createPartialMock(
-            Category::class,
-            [
-                'dataHasChangedFor',
-                'getResource',
-                'getStoreId',
-                'formatUrlKey',
-                'getId',
-                'hasChildren',
-            ]
-        );
+        $this->category = $this->getMockBuilder(Category::class)
+            ->onlyMethods(
+                [
+                    'dataHasChangedFor',
+                    'getResource',
+                    'getStoreId',
+                    'formatUrlKey',
+                    'hasChildren',
+                    'getData',
+                    'getUrlKey'
+                ]
+            )
+            ->addMethods(['getUrlPath'])
+            ->disableOriginalConstructor()
+            ->getMock();
         $this->category->expects($this->any())->method('getResource')->willReturn($this->categoryResource);
         $this->observer->expects($this->any())->method('getEvent')->willReturnSelf();
         $this->observer->expects($this->any())->method('getCategory')->willReturn($this->category);
@@ -112,6 +131,14 @@ protected function setUp(): void
             ->onlyMethods(['execute'])
             ->getMock();
 
+        $this->metadataPool = $this->getMockBuilder(MetadataPool::class)
+            ->disableOriginalConstructor()
+            ->onlyMethods(['getMetadata'])
+            ->getMock();
+
+        $this->entityMetaDataInterface = $this->getMockBuilder(EntityMetadataInterface::class)
+            ->getMockForAbstractClass();
+
         $this->categoryUrlPathAutogeneratorObserver = (new ObjectManagerHelper($this))->getObject(
             CategoryUrlPathAutogeneratorObserver::class,
             [
@@ -120,6 +147,7 @@ protected function setUp(): void
                 'storeViewService' => $this->storeViewService,
                 'compositeUrlValidator' => $this->compositeUrlValidator,
                 'getDefaultUrlKey' => $this->getDefaultUrlKey,
+                'metadataPool' => $this->metadataPool
             ]
         );
     }
@@ -134,13 +162,20 @@ public function testShouldFormatUrlKeyAndGenerateUrlPathIfUrlKeyIsNotUsingDefaul
         $expectedUrlKey = 'formatted_url_key';
         $expectedUrlPath = 'generated_url_path';
         $categoryData = ['use_default' => ['url_key' => 0], 'url_key' => 'some_key', 'url_path' => ''];
+        $this->category->expects($this->any())
+            ->method('getUrlKey')
+            ->willReturnOnConsecutiveCalls($categoryData['url_key'], null, $expectedUrlKey);
+        $this->category->expects($this->any())
+            ->method('getUrlPath')
+            ->willReturnOnConsecutiveCalls($categoryData['url_path'], $expectedUrlPath);
         $this->category->setData($categoryData);
         $this->category->isObjectNew($isObjectNew);
         $this->categoryUrlPathGenerator->expects($this->once())->method('getUrlKey')->willReturn($expectedUrlKey);
         $this->categoryUrlPathGenerator->expects($this->once())->method('getUrlPath')->willReturn($expectedUrlPath);
         $this->assertEquals($categoryData['url_key'], $this->category->getUrlKey());
         $this->assertEquals($categoryData['url_path'], $this->category->getUrlPath());
-        $this->compositeUrlValidator->expects($this->once())->method('validate')->with('formatted_url_key')->willReturn([]);
+        $this->compositeUrlValidator->expects($this->once())->method('validate')
+            ->with('formatted_url_key')->willReturn([]);
         $this->categoryUrlPathAutogeneratorObserver->execute($this->observer);
         $this->assertEquals($expectedUrlKey, $this->category->getUrlKey());
         $this->assertEquals($expectedUrlPath, $this->category->getUrlPath());
@@ -179,11 +214,32 @@ public function testShouldResetUrlPathAndUrlKeyIfUrlKeyIsUsingDefaultValue(bool
         $this->category->expects($this->once())
             ->method('hasChildren')
             ->willReturn(false);
+        $this->metadataPool->expects($this->any())
+            ->method('getMetadata')
+            ->with(CategoryInterface::class)
+            ->willReturn($this->entityMetaDataInterface);
+        $this->entityMetaDataInterface->expects($this->any())
+            ->method('getLinkField')
+            ->willReturn('row_id');
+        $this->category->expects($this->any())
+            ->method('getUrlKey')
+            ->willReturn($categoryData['url_key']);
+        $this->category->expects($this->any())
+            ->method('getUrlPath')
+            ->willReturn($categoryData['url_path']);
+        $this->category->expects($this->any())
+            ->method('getData')
+            ->willReturnMap(
+                [
+                    ['use_default', null, ['url_key' => 1]],
+                    ['row_id', null, null],
+                ]
+            );
         $this->assertEquals($categoryData['url_key'], $this->category->getUrlKey());
         $this->assertEquals($categoryData['url_path'], $this->category->getUrlPath());
         $this->categoryUrlPathAutogeneratorObserver->execute($this->observer);
-        $this->assertNull($this->category->getUrlKey());
-        $this->assertNull($this->category->getUrlPath());
+        $this->assertNotEmpty($this->category->getUrlKey());
+        $this->assertNotEmpty($this->category->getUrlPath());
     }
 
     /**
@@ -201,15 +257,18 @@ public function shouldResetUrlPathAndUrlKeyIfUrlKeyIsUsingDefaultValueDataProvid
 
     /**
      * @return void
+     * @throws LocalizedException
      */
     public function testShouldUpdateUrlPathForChildrenIfUrlKeyIsUsingDefaultValueForSpecificStore(): void
     {
         $storeId = 1;
         $categoryId = 1;
+        $rowId = 1;
         $categoryData = [
             'use_default' => ['url_key' => 1],
             'url_key' => null,
             'url_path' => 'some_path',
+            'row_id' => 1
         ];
 
         $this->category->setData($categoryData);
@@ -220,9 +279,24 @@ public function testShouldUpdateUrlPathForChildrenIfUrlKeyIsUsingDefaultValueFor
         $this->category->expects($this->once())
             ->method('hasChildren')
             ->willReturn(true);
-        $this->category->expects($this->exactly(2))
-            ->method('getId')
-            ->willReturn($categoryId);
+        $this->metadataPool->expects($this->any())
+            ->method('getMetadata')
+            ->with(CategoryInterface::class)
+            ->willReturn($this->entityMetaDataInterface);
+        $this->entityMetaDataInterface->expects($this->any())
+            ->method('getLinkField')
+            ->willReturn('row_id');
+        $this->category->expects($this->any())
+            ->method('getUrlKey')
+            ->willReturn(false);
+        $this->category->expects($this->any())
+            ->method('getData')
+            ->willReturnMap(
+                [
+                    ['use_default', null, ['url_key' => 1]],
+                    ['row_id', null, $rowId],
+                ]
+            );
         $this->getDefaultUrlKey->expects($this->once())
             ->method('execute')
             ->with($categoryId)
@@ -262,7 +336,7 @@ public function testShouldUpdateUrlPathForChildrenIfUrlKeyIsUsingDefaultValueFor
             ->willReturn([$childCategory]);
 
         $this->categoryUrlPathAutogeneratorObserver->execute($this->observer);
-        $this->assertNull($this->category->getUrlKey());
+        $this->assertFalse($this->category->getUrlKey());
         $this->assertNull($this->category->getUrlPath());
     }
 
@@ -312,7 +386,8 @@ public function testUrlPathAttributeUpdating()
         $this->categoryUrlPathGenerator->expects($this->any())->method('getUrlPath')->willReturn($expectedUrlPath);
         $this->categoryResource->expects($this->once())->method('saveAttribute')->with($this->category, 'url_path');
         $this->category->expects($this->once())->method('dataHasChangedFor')->with('url_path')->willReturn(false);
-        $this->compositeUrlValidator->expects($this->once())->method('validate')->with('formatted_url_key')->willReturn([]);
+        $this->compositeUrlValidator->expects($this->once())->method('validate')
+            ->with('formatted_url_key')->willReturn([]);
         $this->categoryUrlPathAutogeneratorObserver->execute($this->observer);
     }
 
@@ -368,7 +443,8 @@ public function testChildrenUrlPathAttributeUpdatingForSpecificStore()
         $this->childrenCategoriesProvider->expects($this->once())->method('getChildren')->willReturn([$childCategory]);
         $childCategory->expects($this->once())->method('setUrlPath')->with('generated_url_path')->willReturnSelf();
         $childCategoryResource->expects($this->once())->method('saveAttribute')->with($childCategory, 'url_path');
-        $this->compositeUrlValidator->expects($this->once())->method('validate')->with('generated_url_key')->willReturn([]);
+        $this->compositeUrlValidator->expects($this->once())->method('validate')
+            ->with('generated_url_key')->willReturn([]);
 
         $this->categoryUrlPathAutogeneratorObserver->execute($this->observer);
     }
