From 69946cc36635e85f61b33fea040e28989165b3f8 Mon Sep 17 00:00:00 2001 From: Matias Hidalgo Date: Wed, 4 Mar 2020 16:14:17 -0300 Subject: [PATCH 1/6] Added getScopeCacheKey in order to validate the store ID and return the right cache key --- .../Catalog/Model/CategoryRepository.php | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/Catalog/Model/CategoryRepository.php b/app/code/Magento/Catalog/Model/CategoryRepository.php index a8636306f5e5b..293b0a527c4f9 100644 --- a/app/code/Magento/Catalog/Model/CategoryRepository.php +++ b/app/code/Magento/Catalog/Model/CategoryRepository.php @@ -9,8 +9,10 @@ use Magento\Framework\Exception\CouldNotSaveException; use Magento\Framework\Exception\NoSuchEntityException; +use Magento\Framework\Exception\SerializationException; use Magento\Framework\Exception\StateException; use Magento\Catalog\Api\Data\CategoryInterface; +use Magento\Framework\Phrase; /** * Repository for categories. @@ -128,10 +130,11 @@ public function save(\Magento\Catalog\Api\Data\CategoryInterface $category) /** * @inheritdoc + * @throws SerializationException */ public function get($categoryId, $storeId = null) { - $cacheKey = $storeId ?? 'all'; + $cacheKey = $this->getScopeCacheKey($storeId); if (!isset($this->instances[$categoryId][$cacheKey])) { /** @var Category $category */ $category = $this->categoryFactory->create(); @@ -238,4 +241,26 @@ private function getMetadataPool() } return $this->metadataPool; } + + /** + * Returns a cache key based on scope + * + * @param string|integer|null $storeId + * + * @throws SerializationException + * @return integer + */ + private function getScopeCacheKey($storeId = null) + { + if (null !== $storeId && !is_numeric($storeId)) { + throw new SerializationException( + new Phrase( + 'The "%value" value\'s type is invalid. The "%type" type was expected. ' + . 'Verify and try again.', + ['value' => $storeId, 'type' => 'int'] + ) + ); + } + return $storeId ?? 'all'; + } } From 4872d683c5b3d6bcf988244f2e93b6b69a97987c Mon Sep 17 00:00:00 2001 From: Matias Hidalgo Date: Fri, 6 Mar 2020 10:41:03 -0300 Subject: [PATCH 2/6] Fix for Functional tests based on the code updates --- app/code/Magento/Catalog/Model/CategoryRepository.php | 9 +++++---- .../Magento/Catalog/Model/CategoryRepositoryTest.php | 11 +++++++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/Catalog/Model/CategoryRepository.php b/app/code/Magento/Catalog/Model/CategoryRepository.php index 293b0a527c4f9..034e1cdbf4203 100644 --- a/app/code/Magento/Catalog/Model/CategoryRepository.php +++ b/app/code/Magento/Catalog/Model/CategoryRepository.php @@ -7,11 +7,11 @@ namespace Magento\Catalog\Model; +use Magento\Catalog\Api\Data\CategoryInterface; use Magento\Framework\Exception\CouldNotSaveException; use Magento\Framework\Exception\NoSuchEntityException; use Magento\Framework\Exception\SerializationException; use Magento\Framework\Exception\StateException; -use Magento\Catalog\Api\Data\CategoryInterface; use Magento\Framework\Phrase; /** @@ -130,6 +130,7 @@ public function save(\Magento\Catalog\Api\Data\CategoryInterface $category) /** * @inheritdoc + * * @throws SerializationException */ public function get($categoryId, $storeId = null) @@ -245,10 +246,10 @@ private function getMetadataPool() /** * Returns a cache key based on scope * - * @param string|integer|null $storeId + * @param string|int|null $storeId * * @throws SerializationException - * @return integer + * @return int|string */ private function getScopeCacheKey($storeId = null) { @@ -261,6 +262,6 @@ private function getScopeCacheKey($storeId = null) ) ); } - return $storeId ?? 'all'; + return $storeId === null ? 'all' : (int)$storeId; } } diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryRepositoryTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryRepositoryTest.php index c4cabe46f5b32..e952d58b6f470 100644 --- a/dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryRepositoryTest.php +++ b/dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryRepositoryTest.php @@ -12,6 +12,7 @@ use Magento\Catalog\Model\ResourceModel\Category\CollectionFactory as CategoryCollectionFactory; use Magento\Catalog\Model\ResourceModel\Product\CollectionFactory; use Magento\Framework\Exception\LocalizedException; +use Magento\Store\Model\StoreFactory; use Magento\TestFramework\Catalog\Model\CategoryLayoutUpdateManager; use Magento\TestFramework\Helper\Bootstrap; use PHPUnit\Framework\TestCase; @@ -46,6 +47,11 @@ class CategoryRepositoryTest extends TestCase */ private $categoryCollectionFactory; + /** + * @var StoreFactory + */ + private $storeFactory; + /** * Sets up common objects. * @@ -57,6 +63,7 @@ protected function setUp() $this->layoutManager = Bootstrap::getObjectManager()->get(CategoryLayoutUpdateManager::class); $this->productCollectionFactory = Bootstrap::getObjectManager()->get(CollectionFactory::class); $this->categoryCollectionFactory = Bootstrap::getObjectManager()->create(CategoryCollectionFactory::class); + $this->storeFactory = Bootstrap::getObjectManager()->create(StoreFactory::class); } /** @@ -151,14 +158,14 @@ public function testGetCategoryForProvidedStore() $categoryFirstStore = $categoryRepository->get( self::FIXTURE_TWO_STORES_CATEGORY_ID, - self::FIXTURE_FIRST_STORE_CODE + $this->storeFactory->create()->load(self::FIXTURE_FIRST_STORE_CODE)->getId() ); $this->assertSame('category-defaultstore', $categoryFirstStore->getUrlKey()); $categorySecondStore = $categoryRepository->get( self::FIXTURE_TWO_STORES_CATEGORY_ID, - self::FIXTURE_SECOND_STORE_CODE + $this->storeFactory->create()->load(self::FIXTURE_SECOND_STORE_CODE)->getId() ); $this->assertSame('category-fixturestore', $categorySecondStore->getUrlKey()); From 7949329cbf1fb68f81ab8f13e7f74e2814bfe399 Mon Sep 17 00:00:00 2001 From: Matias Hidalgo Date: Tue, 10 Mar 2020 13:26:47 -0300 Subject: [PATCH 3/6] Unit test and Functional test for get store id validation on category repository --- .../Test/Unit/Model/CategoryRepositoryTest.php | 11 +++++++++++ .../Magento/Catalog/Model/CategoryRepositoryTest.php | 12 ++++++++++++ 2 files changed, 23 insertions(+) diff --git a/app/code/Magento/Catalog/Test/Unit/Model/CategoryRepositoryTest.php b/app/code/Magento/Catalog/Test/Unit/Model/CategoryRepositoryTest.php index 864b91b20d017..14d03dc2b5f4a 100644 --- a/app/code/Magento/Catalog/Test/Unit/Model/CategoryRepositoryTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Model/CategoryRepositoryTest.php @@ -140,6 +140,17 @@ public function testGetWhenCategoryDoesNotExist() $this->assertEquals($categoryMock, $this->model->get($categoryId)); } + /** + * @expectedException \Magento\Framework\Exception\SerializationException + * @expectedExceptionMessage The "default" value's type is invalid. The "int" type was expected. Verify and try again. + */ + public function testGetWithStoreCodeException() + { + $categoryId = 5; + $categoryMock = $this->createMock(\Magento\Catalog\Model\Category::class); + $this->assertEquals($categoryMock, $this->model->get($categoryId, 'default')); + } + /** * @return array */ diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryRepositoryTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryRepositoryTest.php index e952d58b6f470..4e94d58d21ab6 100644 --- a/dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryRepositoryTest.php +++ b/dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryRepositoryTest.php @@ -12,6 +12,7 @@ use Magento\Catalog\Model\ResourceModel\Category\CollectionFactory as CategoryCollectionFactory; use Magento\Catalog\Model\ResourceModel\Product\CollectionFactory; use Magento\Framework\Exception\LocalizedException; +use Magento\Framework\Exception\SerializationException; use Magento\Store\Model\StoreFactory; use Magento\TestFramework\Catalog\Model\CategoryLayoutUpdateManager; use Magento\TestFramework\Helper\Bootstrap; @@ -169,5 +170,16 @@ public function testGetCategoryForProvidedStore() ); $this->assertSame('category-fixturestore', $categorySecondStore->getUrlKey()); + + $caughtException = false; + try { + $categoryRepository->get( + self::FIXTURE_TWO_STORES_CATEGORY_ID, + self::FIXTURE_FIRST_STORE_CODE + ); + } catch (SerializationException $exception) { + $caughtException = true; + } + $this->assertTrue($caughtException); } } From ffda40bae2ccb0239d2cbf0e27e3c315d5cd89d7 Mon Sep 17 00:00:00 2001 From: Matias Hidalgo Date: Tue, 10 Mar 2020 15:15:31 -0300 Subject: [PATCH 4/6] Coding standard adjustment --- .../Magento/Catalog/Test/Unit/Model/CategoryRepositoryTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/code/Magento/Catalog/Test/Unit/Model/CategoryRepositoryTest.php b/app/code/Magento/Catalog/Test/Unit/Model/CategoryRepositoryTest.php index 14d03dc2b5f4a..5d5f5b2a33d39 100644 --- a/app/code/Magento/Catalog/Test/Unit/Model/CategoryRepositoryTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Model/CategoryRepositoryTest.php @@ -142,7 +142,8 @@ public function testGetWhenCategoryDoesNotExist() /** * @expectedException \Magento\Framework\Exception\SerializationException - * @expectedExceptionMessage The "default" value's type is invalid. The "int" type was expected. Verify and try again. + * @expectedExceptionMessage The "default" value's type is invalid. The "int" type was expected. Verify and + * try again. */ public function testGetWithStoreCodeException() { From fa481b10944ee57ee7567d4667da985f1ff4a198 Mon Sep 17 00:00:00 2001 From: engcom-Echo Date: Tue, 12 May 2020 13:02:56 +0300 Subject: [PATCH 5/6] fix unit test --- .../Test/Unit/Model/CategoryRepositoryTest.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/Catalog/Test/Unit/Model/CategoryRepositoryTest.php b/app/code/Magento/Catalog/Test/Unit/Model/CategoryRepositoryTest.php index 57f8fa72f257c..3c8515f411fb3 100644 --- a/app/code/Magento/Catalog/Test/Unit/Model/CategoryRepositoryTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Model/CategoryRepositoryTest.php @@ -152,15 +152,14 @@ public function testGetWhenCategoryDoesNotExist() $this->assertEquals($categoryMock, $this->model->get($categoryId)); } - /** - * @expectedException \Magento\Framework\Exception\SerializationException - * @expectedExceptionMessage The "default" value's type is invalid. The "int" type was expected. Verify and - * try again. - */ public function testGetWithStoreCodeException() { $categoryId = 5; - $categoryMock = $this->createMock(\Magento\Catalog\Model\Category::class); + $categoryMock = $this->createMock(CategoryModel::class); + $this->expectException('\Magento\Framework\Exception\SerializationException'); + $this->expectExceptionMessage( + 'The "default" value\'s type is invalid. The "int" type was expected. Verify and try again.' + ); $this->assertEquals($categoryMock, $this->model->get($categoryId, 'default')); } From eba266474778e69119fdce8539b55da2278954cb Mon Sep 17 00:00:00 2001 From: Matias Hidalgo Date: Fri, 8 Jan 2021 09:41:38 -0300 Subject: [PATCH 6/6] Update dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryRepositoryTest.php Co-authored-by: Ihor Sviziev --- .../Magento/Catalog/Model/CategoryRepositoryTest.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryRepositoryTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryRepositoryTest.php index f1641fa30f05c..d02835956d8be 100644 --- a/dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryRepositoryTest.php +++ b/dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryRepositoryTest.php @@ -147,10 +147,7 @@ public function testGetCategoryForProvidedStore(): void $caughtException = false; try { - $categoryRepository->get( - categoryId, - 'default' - ); + $this->categoryRepository->get($categoryId, 'default'); } catch (SerializationException $exception) { $caughtException = true; }