search

vuestorefront / vue-storefront-api

Vue.js storefront for Magento2 (and not only) - data backend
https://www.vuestorefront.io
MIT License
348 stars 338 forks source link

Attributes not being cached in Redis #579

Closed didkan closed 2 years ago

didkan commented 2 years ago

I'm seeing an error in api-error.log saying:

TypeError: Cannot read property 'forEach' of undefined
    at TagCache.set (/home/www/vuestorefront/codebase/vue-storefront-api/node_modules/redis-tag-cache/dist/index.js:60:12)
    at process._tickCallback (internal/process/next_tick.js:68:7)

I think I found the problem here: https://github.com/vuestorefront/vue-storefront-api/blob/eefe4efca1d98230addb9eeb265cf52c2eafb990/src/api/attribute/service.ts#L63-L66

When trying to set the attribute data to the redis cache, the third parameter (tags) is omitted resulting in the error above. The correct signature of the method is cache.set(key, data, tags)(https://www.npmjs.com/package/redis-tag-cache#cachesetkey-value-tags).

Not sure which tag to use here though. There is a tag called attribute in config.server.availableCacheTags. Not sure if VSBridge is using that tag to invalidate when attributes are updated in Magento, but it is my best bet atm...

didkan commented 2 years ago

Found another issue with the attribute cache logic. It does not use any index identifier in the cache key so only the attributes for one storeview can be held in cache at any one time. The effect is that all storeviews get the translation/language of the one in cache. Adding e.g. indexName, to the cache key would solve this.

didkan commented 2 years ago

@Fifciu I would argue that this is a pretty serious bug. Is there any roadmap/milstone list this could be added to?

Fifciu commented 2 years ago

Today I will look into this one @didkan . Thanks for reporting

didkan commented 2 years ago

Thanks! I created a PR with how I solved here: https://github.com/vuestorefront/vue-storefront-api/pull/583