From 6316620cca1482d4f56fd4f1e5d3482e6b1f6deb Mon Sep 17 00:00:00 2001 From: Erfan Date: Mon, 8 Feb 2021 11:59:54 +0800 Subject: [PATCH 1/3] Github #31984: fix for array offset bug during some 404s (PHP 7.4) --- .../CatalogUrlRewrite/Model/Storage/DynamicStorage.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/CatalogUrlRewrite/Model/Storage/DynamicStorage.php b/app/code/Magento/CatalogUrlRewrite/Model/Storage/DynamicStorage.php index d9e9705ac039d..9da95d4270b74 100644 --- a/app/code/Magento/CatalogUrlRewrite/Model/Storage/DynamicStorage.php +++ b/app/code/Magento/CatalogUrlRewrite/Model/Storage/DynamicStorage.php @@ -179,13 +179,16 @@ private function findProductRewriteByRequestPath(array $data): ?array unset($data[UrlRewrite::IS_AUTOGENERATED]); $categoryFromDb = $this->connection->fetchRow($this->prepareSelect($data)); + if ($categoryFromDb === false) { + return null; + } + if ($categoryFromDb[UrlRewrite::REDIRECT_TYPE]) { $productFromDb[UrlRewrite::REDIRECT_TYPE] = OptionProvider::PERMANENT; $categoryPath = str_replace($categorySuffix, '', $categoryFromDb[UrlRewrite::TARGET_PATH]); } - if ($categoryFromDb === false - || !$productResource->canBeShowInCategory( + if (!$productResource->canBeShowInCategory( $productFromDb[UrlRewrite::ENTITY_ID], $categoryFromDb[UrlRewrite::ENTITY_ID] ) From 6da61dffbfd21df167192c9ae4a528ffe682024b Mon Sep 17 00:00:00 2001 From: Erfan Date: Fri, 5 Mar 2021 15:37:04 +0800 Subject: [PATCH 2/3] Github #31984: covered method with unit test --- .../Unit/Model/Storage/DynamicStorageTest.php | 327 ++++++++++++++++++ 1 file changed, 327 insertions(+) create mode 100644 app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Storage/DynamicStorageTest.php diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Storage/DynamicStorageTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Storage/DynamicStorageTest.php new file mode 100644 index 0000000000000..3931397731949 --- /dev/null +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Storage/DynamicStorageTest.php @@ -0,0 +1,327 @@ +urlRewriteFactory = $this->getMockBuilder(UrlRewriteFactory::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->dataObjectHelper = $this->getMockBuilder(DataObjectHelper::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->connection = $this->getMockBuilder(AdapterInterface::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->select = $this->getMockBuilder(Select::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->resourceConnection = $this->getMockBuilder(ResourceConnection::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->connection + ->method('select') + ->willReturn($this->select); + + $this->resourceConnection + ->method('getConnection') + ->willReturn($this->connection); + + $this->scopeConfig = $this->getMockBuilder(ScopeConfigInterface::class) + ->getMock(); + + $this->productResource = $this->getMockBuilder(Product::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->productFactory = $this->getMockBuilder(ProductFactory::class) + ->disableOriginalConstructor() + ->getMock(); + + $this->productFactory + ->method('create') + ->willReturn($this->productResource); + + $this->object = new DynamicStorage( + $this->urlRewriteFactory, + $this->dataObjectHelper, + $this->resourceConnection, + $this->scopeConfig, + $this->productFactory + ); + } + + /** + * @param $data + * @param $productFromDb + * @param $categorySuffix + * @param $categoryFromDb + * @param $canBeShownInCategory + * @param $expectedProductRewrite + * @throws \ReflectionException + * @dataProvider findProductRewriteByRequestPathDataProvider + */ + public function testFindProductRewriteByRequestPath( + $data, + $productFromDb, + $categorySuffix, + $categoryFromDb, + $canBeShownInCategory, + $expectedProductRewrite + ) { + $this->connection->expects($this->any()) + ->method('fetchRow') + ->will($this->onConsecutiveCalls($productFromDb, $categoryFromDb)); + + $scopeConfigMap = [ + [ + CategoryUrlPathGenerator::XML_PATH_CATEGORY_URL_SUFFIX, + ScopeInterface::SCOPE_STORE, + $data['store_id'], + $categorySuffix + ] + ]; + + $this->scopeConfig + ->method('getValue') + ->willReturnMap($scopeConfigMap); + + $this->productResource + ->method('canBeShowInCategory') + ->willReturn($canBeShownInCategory); + + $method = new ReflectionMethod($this->object, 'findProductRewriteByRequestPath'); + $method->setAccessible(true); + + $this->assertSame($expectedProductRewrite, $method->invoke($this->object, $data)); + } + + /** + * @return array + */ + public function findProductRewriteByRequestPathDataProvider(): array + { + return [ + [ + // Non-existing product + [ + 'request_path' => 'test.html', + 'store_id' => 1 + ], + false, + null, + null, + null, + null + ], + [ + // Non-existing category + [ + 'request_path' => 'a/test.html', + 'store_id' => 1 + ], + [ + 'entity_type' => 'product', + 'entity_id' => '1', + 'request_path' => 'test.html', + 'target_path' => 'catalog/product/view/id/1', + 'redirect_type' => '0', + ], + '.html', + false, + null, + null + ], + [ + // Existing category + [ + 'request_path' => 'shop/test.html', + 'store_id' => 1 + ], + [ + 'entity_type' => 'product', + 'entity_id' => '1', + 'request_path' => 'test.html', + 'target_path' => 'catalog/product/view/id/1', + 'redirect_type' => '0', + ], + '.html', + [ + 'entity_type' => 'category', + 'entity_id' => '3', + 'request_path' => 'shop.html', + 'target_path' => 'catalog/category/view/id/3', + 'redirect_type' => '0', + ], + true, + [ + 'entity_type' => 'product', + 'entity_id' => '1', + 'request_path' => 'shop/test.html', + 'target_path' => 'catalog/product/view/id/1/category/3', + 'redirect_type' => '0', + ] + ], + [ + // Existing category, but can't be shown in category + [ + 'request_path' => 'shop/test.html', + 'store_id' => 1 + ], + [ + 'entity_type' => 'product', + 'entity_id' => '1', + 'request_path' => 'test.html', + 'target_path' => 'catalog/product/view/id/1', + 'redirect_type' => '0', + ], + '.html', + [ + 'entity_type' => 'category', + 'entity_id' => '3', + 'request_path' => 'shop.html', + 'target_path' => 'catalog/category/view/id/3', + 'redirect_type' => '0', + ], + false, + null + ], + [ + // Existing category, with product 301 redirect type + [ + 'request_path' => 'shop/test.html', + 'store_id' => 1 + ], + [ + 'entity_type' => 'product', + 'entity_id' => '1', + 'request_path' => 'test.html', + 'target_path' => 'test-new.html', + 'redirect_type' => OptionProvider::PERMANENT, + ], + '.html', + [ + 'entity_type' => 'category', + 'entity_id' => '3', + 'request_path' => 'shop.html', + 'target_path' => 'catalog/category/view/id/3', + 'redirect_type' => '0', + ], + true, + [ + 'entity_type' => 'product', + 'entity_id' => '1', + 'request_path' => 'shop/test.html', + 'target_path' => 'shop/test-new.html', + 'redirect_type' => OptionProvider::PERMANENT, + ] + ], + [ + // Existing category, with category 301 redirect type + [ + 'request_path' => 'shop/test.html', + 'store_id' => 1 + ], + [ + 'entity_type' => 'product', + 'entity_id' => '1', + 'request_path' => 'test.html', + 'target_path' => 'catalog/product/view/id/1', + 'redirect_type' => '0', + ], + '.html', + [ + 'entity_type' => 'category', + 'entity_id' => '3', + 'request_path' => 'shop.html', + 'target_path' => 'shop-new.html', + 'redirect_type' => OptionProvider::PERMANENT, + ], + true, + [ + 'entity_type' => 'product', + 'entity_id' => '1', + 'request_path' => 'shop/test.html', + 'target_path' => 'shop-new/test.html', + 'redirect_type' => OptionProvider::PERMANENT, + ] + ], + ]; + } +} From ae62eef387aef2bc925db0e6cc81aeeb2e9f2b27 Mon Sep 17 00:00:00 2001 From: Erfan Date: Sat, 6 Mar 2021 00:17:55 +0800 Subject: [PATCH 3/3] Github #31984: improved unit test from feedback --- .../Unit/Model/Storage/DynamicStorageTest.php | 90 +++++++++---------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Storage/DynamicStorageTest.php b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Storage/DynamicStorageTest.php index 3931397731949..4389498a3297b 100644 --- a/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Storage/DynamicStorageTest.php +++ b/app/code/Magento/CatalogUrlRewrite/Test/Unit/Model/Storage/DynamicStorageTest.php @@ -33,119 +33,119 @@ class DynamicStorageTest extends TestCase /** * @var UrlRewriteFactory|MockObject */ - private $urlRewriteFactory; + private $urlRewriteFactoryMock; /** * @var DataObjectHelper|MockObject */ - private $dataObjectHelper; + private $dataObjectHelperMock; /** * @var AdapterInterface|MockObject */ - private $connection; + private $connectionMock; /** * @var Select|MockObject */ - private $select; + private $selectMock; /** * @var ResourceConnection|MockObject */ - private $resourceConnection; + private $resourceConnectionMock; /** * @var ScopeConfigInterface|MockObject */ - private $scopeConfig; + private $scopeConfigMock; /** * @var Product|MockObject */ - private $productResource; + private $productResourceMock; /** * @var ProductFactory|MockObject */ - private $productFactory; + private $productFactoryMock; /** * @inheritdoc */ protected function setUp(): void { - $this->urlRewriteFactory = $this->getMockBuilder(UrlRewriteFactory::class) + $this->urlRewriteFactoryMock = $this->getMockBuilder(UrlRewriteFactory::class) ->disableOriginalConstructor() ->getMock(); - $this->dataObjectHelper = $this->getMockBuilder(DataObjectHelper::class) + $this->dataObjectHelperMock = $this->getMockBuilder(DataObjectHelper::class) ->disableOriginalConstructor() ->getMock(); - $this->connection = $this->getMockBuilder(AdapterInterface::class) + $this->connectionMock = $this->getMockBuilder(AdapterInterface::class) ->disableOriginalConstructor() ->getMock(); - $this->select = $this->getMockBuilder(Select::class) + $this->selectMock = $this->getMockBuilder(Select::class) ->disableOriginalConstructor() ->getMock(); - $this->resourceConnection = $this->getMockBuilder(ResourceConnection::class) + $this->resourceConnectionMock = $this->getMockBuilder(ResourceConnection::class) ->disableOriginalConstructor() ->getMock(); - $this->connection + $this->connectionMock ->method('select') - ->willReturn($this->select); + ->willReturn($this->selectMock); - $this->resourceConnection + $this->resourceConnectionMock ->method('getConnection') - ->willReturn($this->connection); + ->willReturn($this->connectionMock); - $this->scopeConfig = $this->getMockBuilder(ScopeConfigInterface::class) + $this->scopeConfigMock = $this->getMockBuilder(ScopeConfigInterface::class) ->getMock(); - $this->productResource = $this->getMockBuilder(Product::class) + $this->productResourceMock = $this->getMockBuilder(Product::class) ->disableOriginalConstructor() ->getMock(); - $this->productFactory = $this->getMockBuilder(ProductFactory::class) + $this->productFactoryMock = $this->getMockBuilder(ProductFactory::class) ->disableOriginalConstructor() ->getMock(); - $this->productFactory + $this->productFactoryMock ->method('create') - ->willReturn($this->productResource); + ->willReturn($this->productResourceMock); $this->object = new DynamicStorage( - $this->urlRewriteFactory, - $this->dataObjectHelper, - $this->resourceConnection, - $this->scopeConfig, - $this->productFactory + $this->urlRewriteFactoryMock, + $this->dataObjectHelperMock, + $this->resourceConnectionMock, + $this->scopeConfigMock, + $this->productFactoryMock ); } /** - * @param $data - * @param $productFromDb - * @param $categorySuffix - * @param $categoryFromDb - * @param $canBeShownInCategory - * @param $expectedProductRewrite - * @throws \ReflectionException * @dataProvider findProductRewriteByRequestPathDataProvider + * @param array $data + * @param array|false $productFromDb + * @param string $categorySuffix + * @param array|false $categoryFromDb + * @param bool $canBeShownInCategory + * @param array|null $expectedProductRewrite + * @throws \ReflectionException */ public function testFindProductRewriteByRequestPath( - $data, + array $data, $productFromDb, - $categorySuffix, + string $categorySuffix, $categoryFromDb, - $canBeShownInCategory, - $expectedProductRewrite - ) { - $this->connection->expects($this->any()) + bool $canBeShownInCategory, + ?array $expectedProductRewrite + ): void { + $this->connectionMock->expects($this->any()) ->method('fetchRow') ->will($this->onConsecutiveCalls($productFromDb, $categoryFromDb)); @@ -158,11 +158,11 @@ public function testFindProductRewriteByRequestPath( ] ]; - $this->scopeConfig + $this->scopeConfigMock ->method('getValue') ->willReturnMap($scopeConfigMap); - $this->productResource + $this->productResourceMock ->method('canBeShowInCategory') ->willReturn($canBeShownInCategory); @@ -185,9 +185,9 @@ public function findProductRewriteByRequestPathDataProvider(): array 'store_id' => 1 ], false, + '', null, - null, - null, + true, null ], [ @@ -205,7 +205,7 @@ public function findProductRewriteByRequestPathDataProvider(): array ], '.html', false, - null, + true, null ], [