martyf
commented
2 years ago This is really good - I'll look at it more over the weekend too.
When Sitemapamic had some updates that made it get rather confusing to work with, I added a central Sitemapamic support file that helped with getting and knowing what the cache keys were (and simplifying the Controller a bit more too). It meant that the Controller and the commands could all use this centralised logic for cache keys.
I'm wondering if now could be the time to re-think the approach and do some larger scale refactoring more for the longevity of it all. This PR could be a good "now" solution but I think I'll also focus on a larger rework for longer-term code maintainability.
Thoughts?
This PR fixes correct cache invalidation for the new multi-site feature. It was possible to define custom locale for different sites, but the caching mechanism was not yet updated. That led to incorrect behavior, especially when using the 'current' keyword: all sites' feeds showed the same content due to the same cache key.
The cache key depends on the locales setting. To correctly invalidate the cache, the correct key has to be used. This is not a trivial task, as the locales configuration can mean different things.
Let's assume we have the following sites and feed URLs:
Currently the following scenarios for multi-site feeds are supported:
1. Show feeds of all sites on every site:
'locales' => '*' // the default behavior, same as not defining locales at allAll sites share the same cache key, something like
feedamic.news.*, so invalidation is fairly easy.2. Each site shows its own feeds
'locales' => 'current'Due to the dynamic nature of the 'current' locale, every sites' feed has a unique cache key:
Cache invalidation by EntrySaved event is pretty straightforward. But when invalidating the cache via CLI, it gets complicated. Instead of introducing new CLI parameters, the cache for all locales of the selected feeds are invalidated. This is not ideal, as probably only a single locales' cache should be invalidated. I've chosen to do it the easy and invalidate the cache keys of all locales for this feed.
3. Every site shows the same combination of locales
'locales' => ['com', 'fr']Similar to 1. all feeds of all sites show the same content:
Cache invalidation is basically the same as in 1.
In hindsight there is much that can be improved, e. g. define for every locale which locale feeds to show, so that something like this can be possible:
The user configuration for this could get a little complicated, so I decided to skip this feature for now.
Because cache invalidation is the main focus of this PR, I've tried to unify the cache key handling for caching and cache invalidation. The main Feedamic.php class is extended by some helper functions that can be used throughout the addon. Additionally a facade for this class has been introduced to make it easier to work with it.
These are a lot of changes, but I hope to make the multi-site support of this addon better as it did not work as expected with the last changes.