request tagging mechanisms

[dbu]

why is there a difference between annotations and the configuration for creating tag headers? could these not just be 2 different ways to tell the same tools what they have to do? or is that already the case?

[ddeboer]

It seems we have some confusion here. At this moment, there's no way to set tag headers through configuration, only through annotations.

To clarify the current situation:

Functionality	Annotations	Configuration
Set Cache-Control headers	(SensioFrameworkExtraBundle)	rules
Tagging and invalidating tags	@Tag	-
Invalidate paths	@InvalidatePath	-
Invalidate routes	@InvalidateRoute	invalidators

The last one, invalidating routes, is the only one that can be done through both annotations and configuration. They do tell the same tools what to do: they are both read by the InvalidationListener, which calls the CacheManager to do the actual work.

I guess we should add functionality to:

• set tags, and tags to be invalidated, through configuration on a per-route/path basis (not sure how this works with ExpressionLanguage)
• extend the route invalidation through configuration to understand paths, too.

There then remains, however, the divide in the bundle between the (Liip) cache-control functionality and the (Driebit) invalidation functionality. Should we integrate these two more, for instance by moving the invalidation options (invalidate_routes) to the rules matching section?

[dbu]

given this some thought. i think adding tags over whole url patterns would be useful. maybe just allow to specify arbitrary headers in addition to the cache control headers. then i could tag a whole area of /blog.* and /news.* as my "blog" and invalidate everything related to blog by tag. (if its just one path, i can just as well ban a path pattern).

what if the invalidators configuration could additionally/alternatively be told to invalidate a tag or a path instead of a route?

so we would have:

• invalidators that configure when to invalidate what (path/route, tag) - per route, and these will be non GET routes
• cache headers configuration that use the symfony RequestMatcher to add headers - this will be on GET routes

the cache control uses the symfony core RequestMatcher mechanism. maybe if we would use the same mechanism for the invalidators, we could get the invalidation based on anything (host, path, route, ...) quite cheap. even if we do, should we mix output headers and invalidation actions or keep them 2 separate configurations? i tend for the second, to make it easier to read things. then again, in a REST application one might indeed have both overlap strongly.

[ddeboer]

Keeping cache-control and invalidation together also has its benefits, as it makes clear how the cached content is related to the way it is invalidated.

If we move the invalidator config into the rules section, and take into account #35, config would look like:

fos_http_cache:
  rules:
    - 
      match:
        host: ^/blog.*$
      controls: { public: true, max_age: 15, s_maxage: 30, last_modified: "-1 hour" }
      vary: [Accept-Encoding, Accept-Language]
      invalidation:
        routes: 
          other_route: {}
          yet_another_route: {}
        paths: 
          - "/some/path"
        tags: [ blog, other_stuff ]

Should we rename controls to cache-control? Although cache may be a bit redundant, Cache-Control is the HTTP header that everyone recognizes. It could also be grouped together with vary:

fos_http_cache:
  rules:
    - 
      match:
        host: ^/blog.*$
      headers:
        controls: { public: true, max_age: 15, s_maxage: 30, last_modified: "-1 hour" }
        vary: [Accept-Encoding, Accept-Language]
      invalidation:
      ...     

[dbu]

this is starting to look a lot better, yes. i just started wondering how we actually handle the http methods here, and start to fear that we currently send out caching headers for POST / PUT / DELETE requests which would make no sense. when we put invalidation into the same ruleset, we will have the oposite issue that we really don't want to invalidate on GET / HEAD requests. if you always need to put the "method" into the match section, things become kind of pointless again. should we hardcode that the cache headers only apply on GET / HEAD and the invalidation on everything else? or a configurable set of operations to not break on somebody sending some nonstandard method to explode our cache?

[ddeboer]

In the TagListener, we already make a hard-coded differentiation between safe and non-safe requests. So the tags under invalidation in the config above is already ambiguous: it sets tags blog, other_stuff on safe requests, and invalidates them on unsafe ones.

In my opinion, setting caching headers based on configuration should work in the same way: set caching headers such as Cache-Control only on safe requests. To accommodate non-standard HTTP methods, we could

• either have an global optional safe_methods and/or unsafe_methods config param that can be overridden
• or have an optional methods parameters under each config subsection. For controls, vary etc. the param defaults to the safe methods, for the invalidation section it defaults to the unsafe methods.

These params could either default to specific values (['GET', 'HEAD'] for safe, ['POST', 'DELETE', 'PATCH', 'PUT'] for unsafe) or to null. In the latter case, we use Symfony's Request::isMethodSafe() method to make decisions. I prefer null as default.

By the way, we can just use coded logic using isMethodSafe() for now, and postpone the config param solution until after 1.0 and when we're really sure users need it. Let's remove the methods param in the meantime to reduce confusion.

[dbu]

yeah, makes sense. if somebody has a real use case where he needs something more complicated, he can contribute further flexibility. that will make more sense than us trying to dream what would be needed.

[dbu]

will have a go at this today

[dbu]

see #65

[ddeboer]

I don’t think we’ve fully solved the invalidators configuration (route_invalidators). They now stand separate from the rest of the config.

What about moving them into rules, so we can re-use the matching mechanism (match) for invalidation rules, including safe and cacheable params etc.? If we want to maintain the invalidation by routes (besides request params) functionality, we do need to add matching by route to the match section (which may be somewhat complex if our current event subscribers fire before the router is available).

Reopening this because entails a possible BC break, so we may want to reach a decision before 1.0.

[dbu]

sounds right, yes. we should probably look at a full configuration example and re-check everything in there.

[dbu]

i think the reason i did not put those under rules is that they are not about matching requests but specific routes. but indeed, we could refactor the whole beast to be part of the matches instead of origin_routes and have an invalidate_routes section of routes that are executed in that case. origin routes will usually be unsafe routes, right?

looking at the code i see that the InvalidationSubscriber also handles annotations. so we could follow the same pattern as TagSubscriber with the AbstractRuleSubscriber.

however, i wonder if we should move all rule matching into a single rule subscriber, regardless whether its tags, invalidation or cache headers. then we see what actions we have for the rule and do them. if we have overlaps (mostly between tags and headers i guess) we match the same rules repeatedly. though, if we do that one rule for one thing would stop the processing for all 3 types. hm, tricky thing. with the current configuration, you would actually expect that to happen but it would not. say i have

- rule:
  match: a
  tags: x
- rule:
  match: a
  headers: z

both would be executed. if we unify the matcher, we would only do the first rule and miss the header. this might or might not be desired.

maybe we really should do header_rules, tag_rules and invalidator_rules to make this more clear and readable. if people use the same match criteria, they can use yml aliases with the match: &alias_name notation to reuse configuration blocks.

[dbu]

we can still reuse the code in the Configuration class and the di extension to avoid duplication

[dbu]

so, what about this configuration?

fos_http_cache:
    cache_control:
        -
            match:
                ...
            headers:
                ...
    tags:
        -
            match:
                path: ^/news
            tags: [news-section]

    invalidators:
        -
            match:
                ...
            routes:
                ...

the match part would be the same method in Configuration, reused in the DI extension and also the same base class for the listeners.

[ddeboer]

(I think the rule under invalidators is a mistake?)

I like it, as it offers a clear separation between the three types of rules offered by the bundle (caching headers, tags, invalidation) but still allows users to write those rules in a consistent manner. Of course, when people start using this bundle in real projects a different config structure may turn out to be more useful: but that's for 2.0.

[dbu]

yep, indeed, the rule is an artifact by the console config dump command. edited the comment. so i will do a PR with this, as soon as #90 and #92 are merged.