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 but from the child products.

We've got a shop with photo cameras. We sell a lot of (570) bundle products 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 identites 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($_product->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-cicd2]

All committers have signed the CLA.

[sivaschenko]

Hi @jasperzeinstra thanks for your contribution!

Identities that are provided by Models are used for cache invalidation purpose.

The reason simple product connected to many configurable/bundle products has a lot of cache tags is that when saving that simple product all of the configurable/bundle products should be updated (cache invalidated).

While saving a bundle product does not have any influence on child products (that's why it shouldn't provide any child identities).

The identities that are included during rendering are provided by blocks (see \Magento\Catalog\Block\Product\View::getIdentities , \Magento\Catalog\Block\Product\ListProduct::getIdentities for example).

I think it may have sense to update the implementation of block's getIdentities methods to minimize the tags associated with the page for full page cache. I agree that a block that is rendering a simple product should not provide all the complex products identities where that simple product is involved.

[jasperzeinstra]

@sivaschenko Thanks for your feedback. I know about cache tags and cache invalidation. And indeed when you look at this from a AdminHtml point of view you do want to know all the bundle/configurable identities. But from the frontend point of view you don't want to know this.

So it would be better if all the blocks were using a different method to retrieve identities then how the model is returning it now. Or the getIdentities method should have a parameter loadParent / loadChild. But then you can't use an after plugin, but should use an around plugin.

Difficult issue to resolve. I will also talk this over with our team at MediaCT.

[sivaschenko]

@jasperzeinstra you are right, please pay attention that blocks do have a separate method for retrieving identities, however, by default it just proxies to Model methods. So I meant it may be necessary to consider block's getIdentities method implementation changes instead of model's

[jasperzeinstra]

@sivaschenko Indeed the method of the blocks is being used for FPC and this proxies to model. The following blocks retrieve the identities by using the method of the model.

\Magento\Catalog\Block\Product\ListProduct::getIdentities
\Magento\Catalog\Block\Product\Price::getIdentities
\Magento\Catalog\Block\Product\View::getIdentities
\Magento\Catalog\Block\Product\ProductList\Related::getIdentities
\Magento\Catalog\Block\Product\ProductList\Upsell::getIdentities
\Magento\CatalogInventory\Block\Qtyincrements::getIdentities
\Magento\CatalogInventory\Block\Stockqty\DefaultStockqty::getIdentities
\Magento\CatalogWidget\Block\Product\ProductsList::getIdentities
\Magento\Checkout\Block\Cart\Item\Renderer::getIdentities
\Magento\ConfigurableProduct\Block\Cart\Item\Renderer\Configurable::getIdentities
\Magento\GoogleOptimizer\Block\Code\Product::getIdentities
\Magento\GroupedProduct\Block\Cart\Item\Renderer\Grouped::getIdentities
\Magento\GroupedProduct\Block\Stockqty\Type\Grouped::getIdentities
\Magento\Reports\Block\Product\Viewed::getIdentities
\Magento\Swatches\Block\Product\Renderer\Configurable::getIdentities

Writing new code in all these methods isn't a clean approach. So the question is how does the plugin for configurable and bundle products know if to add the parent or child products.

Adding a parameter to \Magento\Catalog\Model\Product::getIdentities only to be used in the plugin and changing the plugin to an around plugin feels a little bit strange.

We could split up the plugins from defined in the global scope app/code/Magento/Bundle/etc/di.xml to dividing it too two plugins. One for adminhtml area app/code/Magento/Bundle/etc/adminhtml/di.xml and one for frontend area app/code/Magento/Bundle/etc/frontend/di.xml. I think this is the most clean approach and has minimum impact on code change.

How do you feel about this approach?

[sivaschenko]

@jasperzeinstra Splitting plugins between frontend and adminhtml sounds like a solution to me. I think it would be even better to just introduce new plugins for the frontend (as we are sure that products are only rendered (and not updated) on the frontend, and there are other areas, like cron jobs, etc., in scope of which a product can be updated).

[sivaschenko]

@jasperzeinstra can you please port (or recreate) this pull request for 2.3-develop branch, as we are not accepting new pull requests for 2.2-develop branch anymore. (See the contribution notice in blue here: https://devdocs.magento.com/guides/v2.2/contributor-guide/contributing.html#rules)