Make cache invalidation more generic #11
Conversation
| @@ -0,0 +1,28 @@ | |||
| <?php | |||
|
|
|||
| namespace FOS\HttpCacheBundle\HttpCache\Invalidation; | |||
There was a problem hiding this comment.
i wonder if the HttpCache namespace makes much sense. this already is the HttpCacheBundle, so simply Invalidation would be enough imho
There was a problem hiding this comment.
The idea behind the HttpCache namespace was that the Varnish (and Nginx, etc.) classes are clients to those HTTP caches that now do invalidation only, but can do more in the future (such as getting configuration from Varnish). If we can make sure the Varnish etc. classes will only do invalidation tasks, we can put them in the Invalidation namespace. And if we do that, should we have the interfaces in the same directory, which may be a bit messy, or move them to a Strategy or Capability or ... directory?
There was a problem hiding this comment.
ok, i see. how much code would the other tasks share? can you create an issue with the ideas what else the varnish client might be able to do, so i get an idea? i think keeping the invalidator implementation separate from a configuration tool could make sense, if the other part is more than a small utitliy thing.
or could we use some different namespace? what we have are clients for a caching proxy server. CachingProxy maybe?
i can live with the namespace if we find nothing better, but its not very obvious for me. http cache is about the whole thing, not only about the invalidation and our clients that trigger invalidation.
There was a problem hiding this comment.
I agree we should change the namespace, so it should be either CachingProxy or Invalidation, depending on whether we want to support additional features beside invalidating. We can control Varnish through the CLI, which has some more options. I'm not sure yet whether we should add these options to our bundle or it makes more sense to just run the commands yourself without the bundle. What do you think?
There was a problem hiding this comment.
even when we support CLI things (or the varnish management port #14) this is still all about invalidation. i don't think there is much else. warmup could be a topic, but there i think it could be a separate service, or we just get a bit inconsistent in the naming and live with it :-)
until we release a 1.0, we can still move that around, so chose what you like better and we can revisit the question when we see what 1.0 is doing.
regarding CLI, i propose we discuss in #14 or you open a new issue if that is something different.
There was a problem hiding this comment.
Used Invalidation for now. Yep, let's discuss additional (possibly non-invalidation) features separately.
|
cool, this is coming great! i commented like crazy, hope you don't mind :-) |
|
and merry christmas, by the way ;-) |
| @@ -1,21 +1,31 @@ | |||
| <?php | |||
|
|
|||
| namespace Driebit\HttpCacheBundle\HttpCache; | |||
| namespace FOS\HttpCacheBundle\HttpCache; | |||
There was a problem hiding this comment.
i would put this in the Invalidation namespace as well. maybe we could call the class VarnishInvalidator, just to be more explicit.
|
Merry Christmas to you, too! And thanks for the detailed comments. 🎄 |
| * match in order to be invalidated (optional). | ||
| * This can be part of a content type or regex, | ||
| * for instance, "text" | ||
| * @param array $hosts Host that cached responses must match in |
|
Should we merge this first, and discuss supporting multiple default hosts separately? |
|
okay for merging, we can still break things as we want :-) the one piece i would love to move is the regex calculation logic - that should be pushed into the DI class. if you are unfamiliar with that, we can also merge as is and i do a PR afterwards. |
Make cache invalidation more generic
Resolve #4.