Skip to content

product repository cache does not load the product when storeId is null #38632

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: 2.4-develop
Choose a base branch
from
Open

product repository cache does not load the product when storeId is null #38632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
20c2e5a
Ensure default store is set if no store is set in the getById method
Apr 15, 2024
119e6e3
Remove the cache mechanism from the product getList method as it has …
Apr 17, 2024
01bb478
Ensured the get method set the store id to the current store so that …
digitalrisedorset Apr 17, 2024
1122cdc
Added integration tests
digitalrisedorset Apr 23, 2024
f386122
cast storeId in int to ensure reuse of cache is valid + added tests t…
digitalrisedorset Apr 25, 2024
3001112
added tests when editmode is passed but not storeId
digitalrisedorset Apr 25, 2024
111b11f
Removed unused method
digitalrisedorset Apr 25, 2024
9ba6d45
Merge branch '2.4-develop' into feature/product-repository-cache-issue
digitalrisedorset May 8, 2024
3f96885
Merge branch '2.4-develop' into feature/product-repository-cache-issue
digitalrisedorset Jun 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 9 additions & 20 deletions app/code/Magento/Catalog/Model/ProductRepository.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,10 @@ public function __construct(
*/
public function get($sku, $editMode = false, $storeId = null, $forceReload = false)
{
$cacheKey = $this->getCacheKey([$editMode, $storeId === null ? $storeId : (int) $storeId]);
if ($storeId === null) {
$storeId = $this->storeManager->getStore()->getId();
}
$cacheKey = $this->getCacheKey([$editMode, (int) $storeId]);
$cachedProduct = $this->getProductFromLocalCache($sku, $cacheKey);
if ($cachedProduct === null || $forceReload) {
$product = $this->productFactory->create();
Expand All @@ -291,9 +294,6 @@ public function get($sku, $editMode = false, $storeId = null, $forceReload = fal
if ($editMode) {
$product->setData('_edit_mode', true);
}
if ($storeId !== null) {
$product->setData('store_id', (int) $storeId);
}
$product->load($productId);
$this->cacheProduct($cacheKey, $product);
$cachedProduct = $product;
Expand All @@ -307,15 +307,16 @@ public function get($sku, $editMode = false, $storeId = null, $forceReload = fal
*/
public function getById($productId, $editMode = false, $storeId = null, $forceReload = false)
{
$cacheKey = $this->getCacheKey([$editMode, $storeId === null ? $storeId : (int) $storeId]);
if ($storeId === null) {
$storeId = $this->storeManager->getStore()->getId();
}
$cacheKey = $this->getCacheKey([$editMode, (int) $storeId]);

if (!isset($this->instancesById[$productId][$cacheKey]) || $forceReload) {
$product = $this->productFactory->create();
if ($editMode) {
$product->setData('_edit_mode', true);
}
if ($storeId !== null) {
$product->setData('store_id', $storeId);
}
$product->load($productId);
if (!$product->getId()) {
throw new NoSuchEntityException(
Expand Down Expand Up @@ -714,18 +715,6 @@ public function getList(SearchCriteriaInterface $searchCriteria)
$searchResult->setItems($collection->getItems());
$searchResult->setTotalCount($collection->getSize());

foreach ($collection->getItems() as $product) {
$this->cacheProduct(
$this->getCacheKey(
[
false,
$product->getStoreId()
]
),
$product
);
}

return $searchResult;
}

Expand Down
134 changes: 134 additions & 0 deletions dev/tests/integration/testsuite/Magento/Catalog/Model/ProductRepositoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,21 @@ public function skuDataProvider(): array
];
}

/**
* Get list of stores values possible
*
* @return array
*/
public function storeDataProvider(): array
{
return [
['storeId' => null],
['storeId' => 'default'],
['storeId' => '0'],
['storeId' => '1'],
];
}

/**
* Test save product with gallery image
*
Expand Down Expand Up @@ -517,6 +532,125 @@ public function testConsecutiveProductsUpdateWithDifferentAttributeSets(): void
);
}

/**
* Test get and getById methods share the ProductRepository cache in both cases with a storeId null or not
*
* @return void
* @magentoDataFixture Magento/Catalog/_files/product_simple.php
* @dataProvider storeDataProvider
*/
public function testConsecutiveCallToGetAndGetByIdShareCacheWithEditMode(?string $storeId): void
{
$this->testConsecutiveCallToGetAndGetByIdShareCache($storeId, false);
$this->testConsecutiveCallToGetAndGetByIdShareCache($storeId, true);
}

/**
* Test get and getById methods share the ProductRepository cache
*/
public function testConsecutiveCallToGetAndGetByIdShareCache(?string $storeId, bool $editMode): void
{
$sku = 'simple';

/** @var ProductRepositoryInterface $productRepository */
$this->productRepository = $this->objectManager->get(ProductRepositoryInterface::class);
$this->productRepository->cleanCache();

$product = $this->productRepository->get($sku, $editMode, $storeId);
if ($storeId!== null && (int) $storeId == $storeId) { // this will be testing the use case where storeId is a string such as '0', '1', '2'..
$product3 = $this->productRepository->get($sku, $editMode, (int) $storeId);
$product4 = $this->productRepository->getById($product->getId(), $editMode, (int) $storeId);
}

$product2 = $this->productRepository->getById($product->getId(), $editMode, $storeId);
$this->assertSame($product->getName(), $product2->getName());

$reflection = new \ReflectionClass($this->productRepository);
$reflection_property = $reflection->getProperty('instancesById');
$reflection_property->setAccessible(true);
$cacheIdContent = $reflection_property->getValue($this->productRepository);

$reflection = new \ReflectionClass($this->productRepository);
$reflection_property = $reflection->getProperty('instances');
$reflection_property->setAccessible(true);
$cacheSkuContent = $reflection_property->getValue($this->productRepository);

$this->assertEquals(1, count($cacheIdContent[$product->getId()]));
$this->assertEquals(1, count($cacheSkuContent[$sku]));
}

/**
* Test get and getById methods share the ProductRepository cache in both cases with when storeId is not passed and with editMode values
*
* @return void
* @magentoDataFixture Magento/Catalog/_files/product_simple.php
*/
public function testConsecutiveCallToGetAndGetByIdShareCacheInEdgeCase(): void
{
$sku = 'simple';

/** @var ProductRepositoryInterface $productRepository */
$this->productRepository = $this->objectManager->get(ProductRepositoryInterface::class);
$this->productRepository->cleanCache();

$product = $this->productRepository->get($sku);
$product2 = $this->productRepository->getById($product->getId());
$product3 = $this->productRepository->get($sku, false);
$product4 = $this->productRepository->getById($product->getId(), false);
$this->assertSame($product->getName(), $product2->getName());

$reflection = new \ReflectionClass($this->productRepository);
$reflection_property = $reflection->getProperty('instancesById');
$reflection_property->setAccessible(true);
$cacheIdContent = $reflection_property->getValue($this->productRepository);

$reflection = new \ReflectionClass($this->productRepository);
$reflection_property = $reflection->getProperty('instances');
$reflection_property->setAccessible(true);
$cacheSkuContent = $reflection_property->getValue($this->productRepository);

$this->assertEquals(1, count($cacheIdContent[$product->getId()]));
$this->assertEquals(1, count($cacheSkuContent[$sku]));

$product3 = $this->productRepository->get($sku, true);
$this->assertEquals(1, count($cacheIdContent[$product->getId()]));
$this->assertEquals(1, count($cacheSkuContent[$sku]));
$product4 = $this->productRepository->getById($product->getId(), true);
$this->assertEquals(1, count($cacheIdContent[$product->getId()]));
$this->assertEquals(1, count($cacheSkuContent[$sku]));
}

/**
* Test getList method does not store the result in the ProductRepository cache
*
* @magentoDataFixture Magento/Catalog/Model/ResourceModel/_files/product_simple.php
* @return void
*/
public function testGetListDoesNoUseCache(): void
{
/** @var ProductRepositoryInterface $productRepository */
$this->productRepository = $this->objectManager->get(ProductRepositoryInterface::class);
$this->productRepository->cleanCache();

$searchCriteria = $this->searchCriteriaBuilder->create();
$list = $this->productRepository->getList($searchCriteria);
$count = $list->getTotalCount();
$this->assertGreaterThanOrEqual(1, $count);

$reflection = new \ReflectionClass($this->productRepository);
$reflection_property = $reflection->getProperty('instancesById');
$reflection_property->setAccessible(true);
$cacheIdContent = $reflection_property->getValue($this->productRepository);

$reflection = new \ReflectionClass($this->productRepository);
$reflection_property = $reflection->getProperty('instances');
$reflection_property->setAccessible(true);
$cacheSkuContent = $reflection_property->getValue($this->productRepository);

$this->assertEmpty($cacheIdContent);
$this->assertEmpty($cacheSkuContent);
}

/**
* Get Simple Product Data
*
Expand Down