Output cache tagging does not work correctly for CMS content / other custom entities

[lauraseidler]

When output cache tagging is enabled, cache entries are tagged with the first letter of the entity + the respective ID, e.g. C1234:

https://github.com/DivanteLtd/vue-storefront-api/blob/master/src/api/catalog.ts#L130

Given that the entities category, cms_block and cms_page all start with the same letter, this breaks the output cache tagging for these entities + any other custom entities that might share the same first letter.

[pkarw]

Fair point :) Can you please propose a pull request with a change?

[lauraseidler]

@pkarw I can raise a PR, sure, but this will affect pretty much the entire ecosystem as far as I can see. If we take the example of a CMS block, let's say we would want to use the entire entity name and generate a tag cms_block1234:

• whenever the API returns data, this tag needs to be added (I can PR that here)
• whenever a CMS block is rendered, this tag needs to be added (this would probably have to happen here: https://github.com/DivanteLtd/vue-storefront/blob/master/core/modules/cms/store/block/actions.ts)
• indexers need to invalidate the correct cache tags (for example here: https://github.com/DivanteLtd/magento2-vsbridge-indexer/blob/master/src/module-vsbridge-indexer-cms/Model/Indexer/CmsBlock.php)

You would need to align how you're generating the cache tags, e.g. snake_case as is the entity name vs. camelCase as is already in use for example here: https://github.com/DivanteLtd/vue-storefront/blob/master/core/pages/CmsPage.js#L24

We have for us now currently fixed this as follows: On the API changed the line I linked above to the following:

const defaultEntities = ['product', 'category', 'attribute', 'taxrule', 'review']

const tagPrefix = defaultEntities.indexOf(entityType) > -1
    ? entityType[0].toUpperCase() // first letter of entity name: P, T, A ...
    : entityType

We then adapted the cache tagging for CMS blocks and pages to add the respective cache tags (cms_blockXXX and cms_pageXXX) and removed the existing one for cmsPage.

On the indexer side, we added custom handlers for after save events and are invalidating the cache manually - this could be included instead in the indexer, which has already happened for the 2.x branch, but is also using the camel case version, as implemented by @philippsander in https://github.com/DivanteLtd/magento2-vsbridge-indexer/pull/289 - for exactly the same reason of that already being in use in the frontend.

We could convert snake_case to camelCase on API side too, to reduce the changes necessary on other sides, but I'm not sure if that's a good idea either - just using the entity name (on API) / index identifier (on indexer, as proposed by @afirlejczyk on the PR linked above) sounds the safest and most stable to me.

I don't mind PRing the changes, but then please align before on the strategy you want to go with :)

[pkarw]

Hi @lauraseidler. Great job - thanks for the investigation. I think that anyway - which is coherent along with the indexers and the VSF / VSF-API is a good way.

My input here is:

• we should use the camelCasedEntityNames like cmsBlock or product for tag names, instead of just P, T etc to limit the number of changes. To be perfectly honest if you think it's more developer's friendly we can use entity names cms_block etc. It doesn't make much difference as for me - but the important thing is: it must be just aligned
• we need to have these changes in VSF, VSF-API, magento2-vsbridge-indexer and (probably - not sure if it indexes the CMS stuff) magento1-vsbridge-indexer

Totally agree with the rest of your comment, including the extension points to add/invalidate the tags

Looking forward for the PRs! Great job

[lauraseidler]

@pkarw just to make sure I understand you correctly, you're suggesting to get rid of the P, C, etc. all together and always use the full name? So we would have something like category1234 as well instead of C1234?

[pkarw]

Yes, because of keeping it coherent