Add child identities to products instead of parent identities

[jasperzeinstra]

Description

For Bundle and Configurable products the parent identities are added to the default identities with a plugin afterGetIdentities. From my point of view you don't need to have the identities from the parent products on the frontend but from the child products.

We've got a shop with photo cameras. We have a lot of bundle products (570) that include a specific memory card. This specific memory card is also connected as a related product to a lot of products. Currently when you do getIdentities on this simple product you will get a very large list of cache tags. And when you ask one of the bundle products for its identities you don't get any of the child products because those aren't added by the plugin. Only the parent identities are added and the bundle itself doesn't have any parent products.

From my point of view you don't need to know that a simple product has 570 parent identities, but you do want to know that a bundle has, for example, 5 child identities.

All the identities on a page are also being used by Full Page Cache to create the header X-Magento-Tags. This header is also being used by Varnish. But when this header is to big Varnish will stop working. In article https://devdocs.magento.com/guides/v2.2/config-guide/varnish/tshoot-varnish-503.html is described to allow larges headers for Varnish. In this case that isn't very stable. Because when I create 100 more bundle products where this simple product (memory card) is being used i get 100 more cache tags in my header.

On a vanilla Magento 2.2.6 shop I duplicated the bundle product 570 times with a SCV import. The result of the getIdentities is shown below in a screenshot. Notice that the bundle product Sprite Yoga Companion Kit doesn't contain the identities of the child products. But the simple product Sprite Foam Roller contains all 470 bundle products it's connected to.

After the fix I propose the simple product will not show the identities of all the products it's connected to anymore and the bundle product will contain all the identities of the products connected to that product.

Now the identities you're expecting are being returned and the X-Magento-Tags isn't polluted by a lot of cache tags that don't influence the content of the page. You can still run in to the problem that the server won't allow a large header for X-Magento-Tags like described in https://devdocs.magento.com/guides/v2.2/config-guide/varnish/tshoot-varnish-503.html. But at least now the correct cache tags are being returned instead of all the parents of a simple product.

This same issue is also the case for configurable products. For example when a simple product is visible in the catalog and connected to a lot of configurable products this simple product will return a lot of identities and the configurable product won't return the identities of its child products.

Manual testing scenarios

1. Install vanilla Magento with sample data
2. Import products to create 570 bundle products catalog_product_20181120_114259.csv.zip
3. Edit the file Magento_Catalog::view/frontend/templates/product/list.phtml and output the entities for each product <?php var_dump($block->getIdentities()); ?>
4. Notice that there are more identities being used for the simple product then needed.

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)
• All automated tests passed successfully (all builds on Travis CI are green)

[magento-engcom-team]

Hi @jasperzeinstra. Thank you for your contribution
Here is some useful tips 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-engcom-team give me test instance - deploy test instance based on PR changes
• @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

[jasperzeinstra]

@sivaschenko based on our discussion in #19281 i've made this new Pull Request.

[sivaschenko]

Thank you for contribution. Please see the review notes

[magento-engcom-team]

Hi @sivaschenko, thank you for the review.
ENGCOM-3517 has been created to process this Pull Request

[sivaschenko]

Hi @jasperzeinstra , it will be great if you can cover the changed functionality with an integration test

[jasperzeinstra]

@sivaschenko yeah sure, how can I help with that?

[sivaschenko]

@jasperzeinstra it will be great if you can add a test to this pull request that checks the tags provided by block and model for configurable and bundle product

[jasperzeinstra]

@sivaschenko i've added the Unit Test classes.

[sivaschenko]

Thanks for the updates @jasperzeinstra ! Your contribution is extremely valuable and very important for Magento performance and cache management improvement. However, during the group code review activity we decideded that we cannot accept the contribution without proper integration tests that are covering:

• Cache tags rendered for simple product that is a child of bundle/configurable
• Cache tags rendered for bundle/configurable product
• Tags raised by cache invalidation triggered when saving child product
• Tags raised by cache invalidation triggered when saving bundle/configurable product

I could understand that this is a strict requiremet for you to introduce such tests, however, it would be awesome if you can do this. Otherwise, we will seek internal resources to implement those, however, it can delay the processing of the pull request.

Please let me know if you will be able to introduce the integration tests.

[jasperzeinstra]

Hello @sivaschenko,

Thanks for your reply. I can understand the wishes you have about integration test, but it feels out of scope for this pull request. This PR focuses on configurable and bundle products and the cache tags they are returning.

As far as I can see there are no tests for saving a configurable or bundle product. I'm not comfortable with writing them my own.

Sorry that I can't help you with finishing this PR in the way you would like to see.

[sivaschenko]

@jasperzeinstra thanks for the reply, we will ensure a satisfying test coverage and process the pull request as soon as we can

[ghost]

Hi @jasperzeinstra, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

[magento-engcom-team]

Hi @jasperzeinstra. Thank you for your contribution.
We will aim to release these changes as part of 2.3.1.
Please check the release notes for final confirmation.

[peterjaap]

🎉 !!!