Carsten Ziegeler wrote:
, then some more changes are needed to take into account that an expires header is present: if so, there should be a way to both generate the key
and cache
the output and to update/remove entries from the cache if the expires header is changed or removed from the pipeline. What would be the best way to do that? I'm thinking about having an object that generates key on behalf of non cachable components, would that be enough?
Hmm, I haven't looked yet into your code. What happens now if an expires date is set, but the pipeline does not have a cacheable generator?
Actually nothing. My code relies and performs expires logic on the CachedResponse object, so if no one will be created, there is nothing I can do ATM.
What happens, then, is that my patches are good for already cachable pipelines, where there is a cachedresponse that is used right away without any further checking. The next step would be to modify AbstractCachingProcessingPipeline#generateCachingKey(): as of now (roughly) it checks if components are cacheable and if not it gives up; it should instead check for an explicit expires setting and, if that is the case, provide a key by itself for the whole pipeline and cache the whole response right away. But it should also take into account the case where the expires header is removed (modifications wouldn't be a problem since they're already handled). And well, I must confess that I still have some troubles figuring out exactly the caching process. Will work on that, and any help/suggestion is most welcome. :-)
Ah ok, does this mean that if an expires date is set, you want to cache the pipeline, regardless if all components in this pipeline support the Cacheable... interface?
Exactly. Meaning that the "expires" setting is a way to tell the pipeline engine that you know better if and how long a resource is valid.
I think, it is much better to completly remove this expires handling from the current caching implementation and great an own expires pipeline implementation, that simply checks for the expire parameter and don't care about the key/validity handling. This is why we now have the pipelines as own components, so we can make different implementations and don't need to put everything into one implementation.
I've been thinking about it, and indeed it might make sense. My point, however, is that there are two sides to consider:
1. this ExpiresEnabledCachingProcessingPipeline would rely *a lot* on the CachingProcessingPipeline, meaning that it would just be an extension built on top of it and "falling back" to the default implementation if an expires is not (anymore?) provided, so I'm not dead sure that having a separate implementation is the best way to go, also because of usability concerns;
2. HTTP expires: it should be in theory possible to set them from any pipeline (and definitely this is what happens, since the code is in AbstractProcessingPipeline), but I'm afraid it would be somehow misleading to have a different behaviour (no internal use) depending on the actual caching pipeline implementation.
Agreed, anyhow: it would be cleaner, and I'll think about it. It would also be the case to understand if and how this code might be integrated in the CachingPointProcessingPipeline. Suggestions?
Ciao,
-- Gianugo Rabellino Pro-netics s.r.l. http://www.pro-netics.com
