Hi,
While integrating the plugin together with the providers added in #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:
|
private function buildOrderCacheKey(OrderInterface $order): string |
|
{ |
|
$shippingAddress = $order->getShippingAddress(); |
|
if (!$shippingAddress instanceof AddressInterface) { |
|
throw new RuntimeException(sprintf( |
|
'Shipping address was not found for order #%s', |
|
$order->getNumber() |
|
)); |
|
} |
- 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?
Hi,
While integrating the plugin together with the providers added in #63, we encountered some issues with the caching mechanism:
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:
SyliusPickupPointPlugin/src/Provider/CachedProvider.php
Lines 96 to 104 in eeacac9
To overcome all these issues, I suggest the following changes:
WDYT?