SyliusPickupPointPlugin icon indicating copy to clipboard operation
SyliusPickupPointPlugin copied to clipboard

Cache generation issues

Open vvasiloi opened this issue 2 years ago • 0 comments

Hi,

While integrating the plugin together with the providers added in https://github.com/Setono/SyliusPickupPointPlugin/pull/63, we encountered some issues with the caching mechanism:

  1. Enabling the cache completely breaks the provider because in our checkout flow there's no shipping address at that point. The customers first enter the billing address, then they either add a shipping address or select a pickup point based on the billing address. Ref: https://github.com/Setono/SyliusPickupPointPlugin/blob/eeacac97ebdc185f4669c3e9fc6a636b3d252fc0/src/Provider/CachedProvider.php#L96-L104
  2. It completely breaks because the exceptions thrown during the cache key generation are not caught
  3. Because the cache key generation is in private methods, changing that logic would mean copy-pasting the entire class and replacing the service.
  4. The CacheProvider decorators have a relatively high priority of 256, which created a bit of confusion because we already decorated the providers with the default priority of 0.

To overcome all these issues, I suggest the following changes:

  1. Extract the cache key generation logic in separate classes, so it's easy to change per project or even per provider.
  2. Catch errors from cache key generation, log them and bypass the cache.
  3. Register the cache decorators with a negative priority.
  4. Fallback to the billing address when there's no shipping address on cache key generation.

WDYT?

vvasiloi avatar Jun 20 '22 10:06 vvasiloi