Full Page Cache Logic in Catalog Module

[apiuser]

Feature request from sshymko, posted on GitHub Jan 13, 2015

Classes Magento\Catalog\Plugin\Model\Indexer\Category\Product\Execute and Magento\Catalog\Plugin\Model\Resource\Attribute\Save implement flushing the Full Page Cache. They should belong to the Magento_PageCache module, not the Magento_Catalog. Utilization of the constant Magento\PageCache\Model\Cache\Type::TYPE_IDENTIFIER rather than literal full_page would expose that dependency on the code level.

[apiuser]

Comment from orlangur, posted on GitHub Jan 14, 2015

They should belong to the PageCache module, not the Catalog

This suggestion is wrong. PageCache module composed in such a way would contradict to "true modularity": the intention to have granular and loosely coupled system components.

PageCache module in Magento 2 introduces common mechanism for any other module to implement page caching. It must not contain any domain-specific logic.

Other modules, like Catalog, may involve this mechanism and implement their own page caching domain-specific logic. Thus Catalog module should have soft dependency on PageCache meaning that caching ability is optional: if PageCache module is present it works and if no nothing bad happens.

Surely, it is possible to go even further and implement such caching as separate module like CatalogPageCache. Or collect caching for a bunch of modules in one with no other module depending on this new module. But, again, such code must not be placed in PageCache module itself.

[apiuser]

Comment from sshymko, posted on GitHub Jan 14, 2015

Correct, if a dependency of Catalog on PageCache is permitted, than implementation is correct except for implicit reference to the Full Page Cache constant. Good job on breaking down the monolithic FPC module, didn't notice it at first!

[apiuser]

Comment from sshymko, posted on GitHub Jan 14, 2015

Still, constant Magento\PageCache\Model\Cache\Type::TYPE_IDENTIFIER needs to be used to make dependency explicit.

[apiuser]

Comment from orlangur, posted on GitHub Jan 14, 2015

No objections on this from my side. Catalog already depends on PageCache due to classes used anyway.

This dependency is logically soft but declared as hard because there is no way to tell "I use classes of some module in plugins only" in dependency declaration currently.

[apiuser]

Comment from verklov, posted on GitHub Jan 23, 2015

MAGETWO-33021