From 2826b1470bbf1c21cbf8249686b67e2c49645644 Mon Sep 17 00:00:00 2001 From: Mohamed El Mrabet Date: Sun, 7 Dec 2025 08:40:47 +0100 Subject: [PATCH 1/4] Remove legacy catalog_category_product_index query --- .../ResourceModel/Category/Collection.php | 42 +--- .../ResourceModel/Category/CollectionTest.php | 86 +++++++- .../ResourceModel/Category/CollectionTest.php | 198 +++++++++++++++++- 3 files changed, 277 insertions(+), 49 deletions(-) diff --git a/app/code/Magento/Catalog/Model/ResourceModel/Category/Collection.php b/app/code/Magento/Catalog/Model/ResourceModel/Category/Collection.php index 9aa073ceacb85..4ffaa84c5e176 100644 --- a/app/code/Magento/Catalog/Model/ResourceModel/Category/Collection.php +++ b/app/code/Magento/Catalog/Model/ResourceModel/Category/Collection.php @@ -1,6 +1,6 @@ getProductsCountQuery($categoryIds, (bool)$websiteId); - $categoryProductsCount = $this->_conn->fetchPairs($countSelect); $countFromCategoryTable = []; if (count($categoryIds) > self::BULK_PROCESSING_LIMIT) { $countFromCategoryTable = $this->getCountFromCategoryTableBulk($categoryIds, (int)$websiteId); @@ -350,15 +347,11 @@ public function loadProductCount($items, $countRegular = true, $countAnchor = tr foreach ($anchor as $item) { $productsCount = 0; if (count($categoryIds) > self::BULK_PROCESSING_LIMIT) { - if (isset($categoryProductsCount[$item->getId()])) { - $productsCount = (int)$categoryProductsCount[$item->getId()]; - } elseif (isset($countFromCategoryTable[$item->getId()])) { + if (isset($countFromCategoryTable[$item->getId()])) { $productsCount = (int)$countFromCategoryTable[$item->getId()]; } } else { - $productsCount = isset($categoryProductsCount[$item->getId()]) - ? (int)$categoryProductsCount[$item->getId()] - : $this->getProductsCountFromCategoryTable($item, $websiteId); + $productsCount = $this->getProductsCountFromCategoryTable($item, (int)$websiteId); } $item->setProductCount($productsCount); } @@ -455,7 +448,7 @@ private function getCountFromCategoryTableBulk( * @param string $websiteId * @return int */ - private function getProductsCountFromCategoryTable(Category $item, string $websiteId): int + private function getProductsCountFromCategoryTable(Category $item, int $websiteId): int { $productCount = 0; @@ -659,29 +652,4 @@ public function getProductTable() return $this->_productTable; } - /** - * Get query for retrieve count of products per category - * - * @param array $categoryIds - * @param bool $addVisibilityFilter - * @return Select - */ - private function getProductsCountQuery(array $categoryIds, $addVisibilityFilter = true): Select - { - $categoryTable = $this->_resource->getTableName('catalog_category_product_index'); - $select = $this->_conn->select() - ->from( - ['cat_index' => $categoryTable], - ['category_id' => 'cat_index.category_id', 'count' => 'count(cat_index.product_id)'] - ) - ->where('cat_index.category_id in (?)', \array_map('\intval', $categoryIds)); - if (true === $addVisibilityFilter) { - $select->where('cat_index.visibility in (?)', $this->catalogProductVisibility->getVisibleInSiteIds()); - } - if (count($categoryIds) > 1) { - $select->group('cat_index.category_id'); - } - - return $select; - } } diff --git a/app/code/Magento/Catalog/Test/Unit/Model/ResourceModel/Category/CollectionTest.php b/app/code/Magento/Catalog/Test/Unit/Model/ResourceModel/Category/CollectionTest.php index f29bfedc48511..da730794282ef 100644 --- a/app/code/Magento/Catalog/Test/Unit/Model/ResourceModel/Category/CollectionTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Model/ResourceModel/Category/CollectionTest.php @@ -203,19 +203,93 @@ public function setUp(): void ); } - public function testLoadProductCount() : void + /** + * Test that loadProductCount works correctly for regular (non-anchor) categories + */ + public function testLoadProductCountForRegularCategories() : void { - $this->select->expects($this->exactly(1)) + $categoryId = 1; + $websiteId = 1; + $storeId = 1; + + $category = $this->getMockBuilder(Category::class) + ->addMethods(['getIsAnchor']) + ->onlyMethods(['getId', 'setProductCount']) + ->disableOriginalConstructor() + ->getMock(); + $category->method('getId')->willReturn($categoryId); + $category->method('getIsAnchor')->willReturn(false); + $category->expects($this->once())->method('setProductCount')->with(5); + + $items = [$categoryId => $category]; + + $storeMock = $this->createMock(Store::class); + $storeMock->method('getWebsiteId')->willReturn($websiteId); + $this->storeManager->method('getStore')->with($storeId)->willReturn($storeMock); + + $this->select->expects($this->once()) ->method('from') ->willReturnSelf(); - $this->select->expects($this->exactly(1)) + $this->select->expects($this->atLeastOnce()) ->method('where') ->willReturnSelf(); - $this->connection->expects($this->exactly(1)) + $this->select->expects($this->once()) + ->method('group') + ->willReturnSelf(); + $this->connection->expects($this->once()) ->method('fetchPairs') ->with($this->select) - ->willReturn([]); - $this->collection->loadProductCount([]); + ->willReturn([$categoryId => 5]); + + $this->collection->setProductStoreId($storeId); + $this->collection->loadProductCount($items, true, false); + } + + /** + * Test that loadProductCount works correctly for anchor categories (small count) + */ + public function testLoadProductCountForAnchorCategories() : void + { + $categoryId = 1; + $websiteId = 1; + $storeId = 1; + + $category = $this->getMockBuilder(Category::class) + ->addMethods(['getIsAnchor']) + ->onlyMethods(['getId', 'setProductCount', 'getAllChildren', 'getPath']) + ->disableOriginalConstructor() + ->getMock(); + $category->method('getId')->willReturn($categoryId); + $category->method('getIsAnchor')->willReturn(true); + $category->method('getAllChildren')->willReturn('1'); + $category->method('getPath')->willReturn('1/2'); + $category->expects($this->once())->method('setProductCount')->with(3); + + $items = [$categoryId => $category]; + + $storeMock = $this->createMock(Store::class); + $storeMock->method('getWebsiteId')->willReturn($websiteId); + $this->storeManager->method('getStore')->with($storeId)->willReturn($storeMock); + + $this->select->expects($this->atLeastOnce()) + ->method('from') + ->willReturnSelf(); + $this->select->expects($this->atLeastOnce()) + ->method('where') + ->willReturnSelf(); + $this->select->expects($this->atLeastOnce()) + ->method('joinInner') + ->willReturnSelf(); + $this->select->expects($this->atLeastOnce()) + ->method('join') + ->willReturnSelf(); + $this->connection->expects($this->once()) + ->method('fetchOne') + ->with($this->select) + ->willReturn(3); + + $this->collection->setProductStoreId($storeId); + $this->collection->loadProductCount($items, false, true); } /** diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Model/ResourceModel/Category/CollectionTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Model/ResourceModel/Category/CollectionTest.php index e8ba45e028f87..87f70c9db4211 100644 --- a/dev/tests/integration/testsuite/Magento/Catalog/Model/ResourceModel/Category/CollectionTest.php +++ b/dev/tests/integration/testsuite/Magento/Catalog/Model/ResourceModel/Category/CollectionTest.php @@ -7,13 +7,32 @@ namespace Magento\Catalog\Model\ResourceModel\Category; +use Magento\Catalog\Model\Category; +use Magento\Catalog\Test\Fixture\Category as CategoryFixture; +use Magento\Catalog\Test\Fixture\Product as ProductFixture; +use Magento\Store\Model\Store; +use Magento\Store\Test\Fixture\Group as StoreGroupFixture; +use Magento\Store\Test\Fixture\Store as StoreFixture; +use Magento\Store\Test\Fixture\Website as WebsiteFixture; +use Magento\TestFramework\Fixture\DataFixture; +use Magento\TestFramework\Fixture\DataFixtureBeforeTransaction; +use Magento\TestFramework\Fixture\DataFixtureStorage; +use Magento\TestFramework\Fixture\DataFixtureStorageManager; +use Magento\TestFramework\Fixture\DbIsolation; +use Magento\TestFramework\Helper\Bootstrap; + class CollectionTest extends \PHPUnit\Framework\TestCase { /** - * @var \Magento\Catalog\Model\ResourceModel\Category\Collection + * @var Collection */ private $collection; + /** + * @var DataFixtureStorage + */ + private $fixtures; + /** * Sets up the fixture, for example, opens a network connection. * This method is called before a test is executed. @@ -21,11 +40,12 @@ class CollectionTest extends \PHPUnit\Framework\TestCase protected function setUp(): void { $this->collection = \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->create( - \Magento\Catalog\Model\ResourceModel\Category\Collection::class + Collection::class ); + $this->fixtures = DataFixtureStorageManager::getStorage(); } - protected function setDown() + protected function tearDown(): void { /* Refresh stores memory cache after store deletion */ \Magento\TestFramework\Helper\Bootstrap::getObjectManager()->get( @@ -42,7 +62,7 @@ public function testJoinUrlRewriteOnDefault() { $categories = $this->collection->joinUrlRewrite()->addPathFilter('1/2/3'); $this->assertCount(1, $categories); - /** @var $category \Magento\Catalog\Model\Category */ + /** @var $category Category */ $category = $categories->getFirstItem(); $this->assertStringEndsWith('category.html', $category->getUrl()); } @@ -55,12 +75,178 @@ public function testJoinUrlRewriteOnDefault() public function testJoinUrlRewriteNotOnDefaultStore() { $store = \Magento\TestFramework\Helper\Bootstrap::getObjectManager() - ->create(\Magento\Store\Model\Store::class); + ->create(Store::class); $storeId = $store->load('second_category_store', 'code')->getId(); $categories = $this->collection->setStoreId($storeId)->joinUrlRewrite()->addPathFilter('1/2/3'); $this->assertCount(1, $categories); - /** @var $category \Magento\Catalog\Model\Category */ + /** @var $category Category */ $category = $categories->getFirstItem(); $this->assertStringEndsWith('category-3-on-2.html', $category->getUrl()); } + + /** + * Test that product counts are loaded correctly for categories + */ + #[ + DbIsolation(true), + DataFixture(CategoryFixture::class, ['name' => 'Test Category'], as: 'category'), + DataFixture( + ProductFixture::class, + [ + 'sku' => 'simple-product-1', + 'category_ids' => ['$category.id$'] + ], + as: 'product1' + ), + DataFixture( + ProductFixture::class, + [ + 'sku' => 'simple-product-2', + 'category_ids' => ['$category.id$'] + ], + as: 'product2' + ) + ] + public function testLoadProductCount() + { + $category = $this->fixtures->get('category'); + $objectManager = Bootstrap::getObjectManager(); + $collection = $objectManager->create( + Collection::class + ); + + $collection->addIdFilter([$category->getId()]); + $collection->setLoadProductCount(true); + $collection->load(); + + $this->assertCount(1, $collection); + /** @var $loadedCategory Category */ + $loadedCategory = $collection->getFirstItem(); + $this->assertEquals($category->getId(), $loadedCategory->getId()); + $this->assertEquals(2, $loadedCategory->getProductCount(), 'Category should have 2 products'); + } + + /** + * Test that product counts respect website filtering + */ + #[ + DbIsolation(true), + DataFixture(CategoryFixture::class, ['name' => 'Test Category 2'], as: 'category2'), + DataFixture( + ProductFixture::class, + [ + 'sku' => 'simple-product-3', + 'category_ids' => ['$category2.id$'] + ], + as: 'product3' + ), + DataFixture( + ProductFixture::class, + [ + 'sku' => 'simple-product-4', + 'category_ids' => ['$category2.id$'] + ], + as: 'product4' + ) + ] + public function testLoadProductCountWithStoreFilter() + { + $category = $this->fixtures->get('category2'); + $objectManager = Bootstrap::getObjectManager(); + $collection = $objectManager->create( + Collection::class + ); + + $collection->addIdFilter([$category->getId()]); + $collection->setProductStoreId(Store::DEFAULT_STORE_ID); + $collection->setLoadProductCount(true); + $collection->load(); + + $this->assertCount(1, $collection); + /** @var $loadedCategory Category */ + $loadedCategory = $collection->getFirstItem(); + $this->assertEquals($category->getId(), $loadedCategory->getId()); + $this->assertEquals( + 2, + $loadedCategory->getProductCount(), + 'Category should have 2 products for default store' + ); + } + + /** + * Test that product counts work correctly with multi-website setup + */ + #[ + DbIsolation(true), + DataFixtureBeforeTransaction(WebsiteFixture::class, as: 'website2'), + DataFixtureBeforeTransaction(StoreGroupFixture::class, ['website_id' => '$website2.id$'], as: 'group2'), + DataFixtureBeforeTransaction( + StoreFixture::class, + ['website_id' => '$website2.id$', 'group_id' => '$group2.id$'], + as: 'store2' + ), + DataFixture(CategoryFixture::class, ['name' => 'Multi-Website Category'], as: 'category3'), + DataFixture( + ProductFixture::class, + [ + 'sku' => 'product-website-1', + 'category_ids' => ['$category3.id$'], + 'extension_attributes' => [ + // Assign to default website (ID: 1) + 'website_ids' => [1] + ] + ], + as: 'product_web1' + ), + DataFixture( + ProductFixture::class, + [ + 'sku' => 'product-website-2', + 'category_ids' => ['$category3.id$'], + 'extension_attributes' => [ + 'website_ids' => ['$website2.id$'] + ] + ], + as: 'product_web2' + ) + ] + public function testLoadProductCountMultiWebsite() + { + $category = $this->fixtures->get('category3'); + $store2 = $this->fixtures->get('store2'); + $objectManager = Bootstrap::getObjectManager(); + + // Test product count for default website (should have 1 product) + // Using DEFAULT_STORE_ID which corresponds to the default website + $collection1 = $objectManager->create(Collection::class); + $collection1->addIdFilter([$category->getId()]); + $collection1->setProductStoreId(Store::DEFAULT_STORE_ID); + $collection1->setLoadProductCount(true); + $collection1->load(); + + $this->assertCount(1, $collection1); + /** @var $loadedCategory1 Category */ + $loadedCategory1 = $collection1->getFirstItem(); + $this->assertEquals( + 1, + $loadedCategory1->getProductCount(), + 'Category should have 1 product for default website' + ); + + // Test product count for second website (should have 1 product) + $collection2 = $objectManager->create(Collection::class); + $collection2->addIdFilter([$category->getId()]); + $collection2->setProductStoreId($store2->getId()); + $collection2->setLoadProductCount(true); + $collection2->load(); + + $this->assertCount(1, $collection2); + /** @var $loadedCategory2 Category */ + $loadedCategory2 = $collection2->getFirstItem(); + $this->assertEquals( + 1, + $loadedCategory2->getProductCount(), + 'Category should have 1 product for second website' + ); + } } From 7c41879361cbca5cd431b3e6527268e63adfa832 Mon Sep 17 00:00:00 2001 From: Mohamed El Mrabet Date: Sun, 7 Dec 2025 10:25:27 +0100 Subject: [PATCH 2/4] Fix issues checks --- .../Model/ResourceModel/Category/Collection.php | 17 ++++++++--------- .../ResourceModel/Category/CollectionTest.php | 4 ++-- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/app/code/Magento/Catalog/Model/ResourceModel/Category/Collection.php b/app/code/Magento/Catalog/Model/ResourceModel/Category/Collection.php index 4ffaa84c5e176..8768a3d047ee5 100644 --- a/app/code/Magento/Catalog/Model/ResourceModel/Category/Collection.php +++ b/app/code/Magento/Catalog/Model/ResourceModel/Category/Collection.php @@ -341,7 +341,7 @@ public function loadProductCount($items, $countRegular = true, $countAnchor = tr $categoryIds = array_keys($anchor); $countFromCategoryTable = []; if (count($categoryIds) > self::BULK_PROCESSING_LIMIT) { - $countFromCategoryTable = $this->getCountFromCategoryTableBulk($categoryIds, (int)$websiteId); + $countFromCategoryTable = $this->getCountFromCategoryTableBulk($categoryIds, $websiteId); } foreach ($anchor as $item) { @@ -351,7 +351,7 @@ public function loadProductCount($items, $countRegular = true, $countAnchor = tr $productsCount = (int)$countFromCategoryTable[$item->getId()]; } } else { - $productsCount = $this->getProductsCountFromCategoryTable($item, (int)$websiteId); + $productsCount = $this->getProductsCountFromCategoryTable($item, $websiteId); } $item->setProductCount($productsCount); } @@ -363,13 +363,13 @@ public function loadProductCount($items, $countRegular = true, $countAnchor = tr * Get products number for each category with bulk query * * @param array $categoryIds - * @param int $websiteId + * @param string|int|null $websiteId * @return array * @throws \Zend_Db_Exception */ private function getCountFromCategoryTableBulk( array $categoryIds, - int $websiteId + string|int|null $websiteId ) : array { $connection = $this->_conn; $tempTableName = 'temp_category_descendants_' . uniqid(); @@ -423,7 +423,7 @@ private function getCountFromCategoryTableBulk( 'cp.category_id = t.descendant_id', ['product_count' => 'COUNT(DISTINCT cp.product_id)'] ); - if ($websiteId) { + if ($websiteId !== null) { $select->join( ['w' => $this->getProductWebsiteTable()], 'cp.product_id = w.product_id', @@ -445,10 +445,10 @@ private function getCountFromCategoryTableBulk( * Get products count using catalog_category_entity table * * @param Category $item - * @param string $websiteId + * @param string|int|null $websiteId * @return int */ - private function getProductsCountFromCategoryTable(Category $item, int $websiteId): int + private function getProductsCountFromCategoryTable(Category $item, string|int|null $websiteId): int { $productCount = 0; @@ -465,7 +465,7 @@ private function getProductsCountFromCategoryTable(Category $item, int $websiteI )->where( '(e.entity_id = :entity_id OR e.path LIKE :c_path)' ); - if ($websiteId) { + if ($websiteId !== null) { $select->join( ['w' => $this->getProductWebsiteTable()], 'main_table.product_id = w.product_id', @@ -651,5 +651,4 @@ public function getProductTable() } return $this->_productTable; } - } diff --git a/app/code/Magento/Catalog/Test/Unit/Model/ResourceModel/Category/CollectionTest.php b/app/code/Magento/Catalog/Test/Unit/Model/ResourceModel/Category/CollectionTest.php index da730794282ef..f7a5723356f6a 100644 --- a/app/code/Magento/Catalog/Test/Unit/Model/ResourceModel/Category/CollectionTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Model/ResourceModel/Category/CollectionTest.php @@ -289,7 +289,7 @@ public function testLoadProductCountForAnchorCategories() : void ->willReturn(3); $this->collection->setProductStoreId($storeId); - $this->collection->loadProductCount($items, false, true); + $this->collection->loadProductCount($items, false); } /** @@ -348,6 +348,6 @@ public function testLoadProductCountCallsBulkMethodForLargeCategoryCount() ->with($this->isInstanceOf(Select::class)) ->willReturn($counts); $this->collection->setProductStoreId($storeId); - $this->collection->loadProductCount($items, false, true); + $this->collection->loadProductCount($items, false); } } From 682f07bc4bfb9e9f76b7be94fa001748ef9637eb Mon Sep 17 00:00:00 2001 From: Mohamed El Mrabet Date: Sun, 7 Dec 2025 14:19:43 +0100 Subject: [PATCH 3/4] Fix unit review --- .../Test/Unit/Model/ResourceModel/Category/CollectionTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/code/Magento/Catalog/Test/Unit/Model/ResourceModel/Category/CollectionTest.php b/app/code/Magento/Catalog/Test/Unit/Model/ResourceModel/Category/CollectionTest.php index f7a5723356f6a..f08c708930d49 100644 --- a/app/code/Magento/Catalog/Test/Unit/Model/ResourceModel/Category/CollectionTest.php +++ b/app/code/Magento/Catalog/Test/Unit/Model/ResourceModel/Category/CollectionTest.php @@ -280,8 +280,7 @@ public function testLoadProductCountForAnchorCategories() : void $this->select->expects($this->atLeastOnce()) ->method('joinInner') ->willReturnSelf(); - $this->select->expects($this->atLeastOnce()) - ->method('join') + $this->select->method('join') ->willReturnSelf(); $this->connection->expects($this->once()) ->method('fetchOne') From e3cc90364b5f9af2cc9e8108ca0bf1cacd7bdfb1 Mon Sep 17 00:00:00 2001 From: Mohamed El Mrabet Date: Sun, 7 Dec 2025 17:43:02 +0100 Subject: [PATCH 4/4] Fix functional tests --- .../Catalog/Model/ResourceModel/Category/Collection.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/Catalog/Model/ResourceModel/Category/Collection.php b/app/code/Magento/Catalog/Model/ResourceModel/Category/Collection.php index 8768a3d047ee5..072286376e539 100644 --- a/app/code/Magento/Catalog/Model/ResourceModel/Category/Collection.php +++ b/app/code/Magento/Catalog/Model/ResourceModel/Category/Collection.php @@ -423,7 +423,7 @@ private function getCountFromCategoryTableBulk( 'cp.category_id = t.descendant_id', ['product_count' => 'COUNT(DISTINCT cp.product_id)'] ); - if ($websiteId !== null) { + if ($websiteId) { $select->join( ['w' => $this->getProductWebsiteTable()], 'cp.product_id = w.product_id', @@ -465,7 +465,7 @@ private function getProductsCountFromCategoryTable(Category $item, string|int|nu )->where( '(e.entity_id = :entity_id OR e.path LIKE :c_path)' ); - if ($websiteId !== null) { + if ($websiteId) { $select->join( ['w' => $this->getProductWebsiteTable()], 'main_table.product_id = w.product_id',