Fix #105

[ddeboer]

• configured tags can access request.attributes with simple expression vars (id)
• response is no longer available as a value for expressions; but there’s no use case
  for that anyway
• update docs

[ddeboer]

Unused.

[ddeboer]

Before we merge this: is there a mismatch between expressions in TagSubscriber and tag_expressions in Configuration? Or is this intentional?

[dbu]

+1 for the PR

for the naming: this is remapped in FOSHttpCacheExtension::loadTagRules. so it works correctly atm. not sure what makes sense - maybe go for expression everywhere, to be consistent with the field in the @Tag annotation?

[ddeboer]

I’m okay with tag_expressions as it’s a bit more explicit. Just added a functional test here to validate the behaviour. This can be merged when green.

[dbu]

green and merged :-)

[stof]

this does not have the Request itself anymore

[dbu]

i think it also did not have the request previously, no?

[stof]

depends. On line 81 it was passed. On line 122 it was not (and this call looks invalid by passing a Request in an argument expecting an array)

[ddeboer]

@stof What do you mean exactly? evaluateTag(string $expression, Request $request) wraps a call to $expressionLanguage->evaluate(string $expression, array $values). As far as I can see evaluateTag is called twice, both times with a Request object:

https://github.com/FriendsOfSymfony/FOSHttpCacheBundle/blob/master/EventListener/TagSubscriber.php#L80
https://github.com/FriendsOfSymfony/FOSHttpCacheBundle/blob/master/EventListener/TagSubscriber.php#L127

[stof]

@ddeboer the comment to which I replied was refering to the previous code

[stof]

@ddeboer well, SensioFRameworkExtraBundle is inconsistent. For @Security, you get more stuff, like the request.

@dbu the behavior for @Security is that known params always win. this way, you always know that request is the Request object. If you have a request attribute with the same name, you can always get it from the Request object.

[dbu]

what if we just add the request for advanced users, mention it in the doc but do not have any examples using the request, to keep them simple? the strategy to overwrite a possible request attribute with the Request means that there would be a BC break if we would add this later.

[ddeboer]

@stof Good point, and your use case makes sense. So let’s just give access to Request, too (while keeping easy access to request attributes; after all, that is what this PR was for). @dbu Can you create a PR for adding Request? To me this is no blocker for 1.0, as it won’t break BC.

[ddeboer]

@dbu Ok.

[dbu]

see #144