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

[digitalrisedorset]

Description (*)

At the current time, the behaviour of the product repository is like below:
The \Magento\Catalog\Model\ProductRepository::getById method and the \Magento\Catalog\Model\ProductRepository::getList method do not share a consistent behaviour regarding the storeId fallback value. When the storeId parameter in these 2 method is null, the behaviour is:

1. The getList method uses the method \Magento\Catalog\Model\ResourceModel\Collection\AbstractCollection::getStoreId to read the store id. This method has a fallback that sets the storeId with the storeManager storeId value.
2. The getById method instead leaves the storeId unchanged even if the parameter in the method is null.
3. The get method instead leaves the storeId unchanged even if the parameter in the method is null.

For a same product id and the same default store, this implementation currently triggers 2 different hashes in the product repository local cache.
Yet, in the getById method, when the product factory creates a product object, this sets the store id with the storeManager store id.
As a consequence, although the hash for the cache is different to the getList method for the same product id, the product store is eventually the same and therefore this sets in the cache the same product on 2 separate cache hashes.
The same issue occurs with the get method when the storeId is null.

The fix is twofold:

1. the cache on the getList method was removed
2. both methods get and getById now sets the storeId to the current store if the parameter storeId is null so that the cache they use can be shared

The benefits with the second change is that both get and getById now share their cache even if storeId is not passed

Related Pull Requests

none

Fixed Issues (if relevant)

none

1. Fixes [Performance] ProductRepository cache issue #38572

Manual testing scenarios (*)

Test 1

for a valid product id in the catalog, trigger the search for the product id using the product repository getList method
and verify the product is not assigned to a local cache

Test 2

for the same product id as the step above, trigger the product load using the product repository getById method and verify the product is assigned to the local cache

Test 3

for the same product as the step above, trigger the product load using the product repository get method and verify the product data come from the local cache

Test 4

change the default store in the backend and iterate both steps above and verify the product is loaded as expected and uses only one hash for the local cache

Test 5

Trigger the search for a product using get method on new request and follow with loading the same product using the method getById. Verify the product is only loaded once
and the cache used is the one created during the first call with get method

Questions or comments

There is a possible backward compatibility concern to verify: I have removed the lines that set the storeId against the product in both get and getById methods. The reasoning for doing this change is that the storeId is set when the productFactory creates the product and with the changes, the storeId is now a lot more consistent in terms of type.
However, should a core code rely on storeId being a string, this change could break some other places.

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @digitalrisedorset. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.

Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests
13. Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

---

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.