SyliusPickupPointPlugin
SyliusPickupPointPlugin copied to clipboard
Cache generation issues
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:
- 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
- It completely breaks because the exceptions thrown during the cache key generation are not caught
- Because the cache key generation is in private methods, changing that logic would mean copy-pasting the entire class and replacing the service.
- 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:
- Extract the cache key generation logic in separate classes, so it's easy to change per project or even per provider.
- Catch errors from cache key generation, log them and bypass the cache.
- Register the cache decorators with a negative priority.
- Fallback to the billing address when there's no shipping address on cache key generation.
WDYT?