Cache tags not being added on redirect response.

[thisispaul]

The X-Cache-Tags header is not being added to Redirect Responses and I think it should be. I have a service which generates screenshots based on some data and I need to cache the redirect to the file it generates and then invalidate when the data is updated. Tags seem to be the way to do this however the tags are not added on redirect responses.

The offending code is on line 70 TagSubscriber.php where it checks to see if the response is successful

        if ($response->isSuccessful()) {
            $tags = $this->getAnnotationTags($request);
        }

I'm proposing a simple change to this so it also tags redirect responses.

        if ($response->isSuccessful() || $response->isRedirection()) {
            $tags = $this->getAnnotationTags($request);
        }

Is there any reason this isn't like this already/a different way to get it to tag redirect response (HTTP Code 302).

I'm happy to submit a pull request if this is fine.

[dbu]

this request is somewhat similar to #279. @wickedOne said he wanted to work on #69 which would solve this by using matchers to decide what tags and what triggers invalidation.

for tagging the response, i am actually not sure why we don't tag everything. usually, error pages or temporary redirects will not be cached at all. but if they are, a change that triggers a tag invalidation might also fix the error, so there is no harm in tagging them.

@ddeboer what is your opinion on this? why should we not just tag every response regardles of status? with the current code, a redirect response to a POST would also not invalidate the tags... we did have the discussion to not invalidate on fail responses - though if we make things more flexible here, we might want the default more agressively invalidating. it is better to err on the side of invalidating too much than too little.

[ddeboer]

I’m in favour of removing the $response->isSuccessful() check when setting tags, as it will make the behaviour more predictable. When invalidating tags, we can do so for redirect responses.

However, I’m not so sure about invalidating in case of error responses. When we start invalidating tags even in case of 400 errors (for instance), it becomes too easy for clients to swamp the server.

What about using the same logic as we have in InvalidationSubscriber: >= 200 && < 400?

[ddeboer]

Fixed in #354.