search

contao / core-bundle

[READ-ONLY] Contao Core Bundle
GNU Lesser General Public License v3.0
123 stars 58 forks source link

Cache tags for new abstract controllers #1612

Closed Toflar closed 6 years ago

Toflar commented 6 years ago

Added support for automatically tagging the elements created using the new abstract controllers. That was still missing 😄

Toflar commented 6 years ago

Ping for review @aschempp

aschempp commented 6 years ago

Should we tag for the current page ID (and possibly article) as well, as a change to them might affect the response? Or is this done in another place? I assume the tagResponse method can be called multiple times? I don't really like the name though, as it's not obvious we're tagging the final response and not the one generated by this controller…

Toflar commented 6 years ago

Should we tag for the current page ID (and possibly article) as well, as a change to them might affect the response? Or is this done in another place?

This should be done in the respective places. The idea of tags is that you don't have to worry about in what context it is used. If someone else adds another tag to your response then that's just fine. So nothing to do here.

I assume the tagResponse method can be called multiple times?

Yes, that's why it's not called setTags() or similar.

I don't really like the name though, as it's not obvious we're tagging the final response and not the one generated by this controller…

You're not. Tags are added to all master requests (so ESI as well) during the kernel.response event. So not necessarily the "final response".

leofeyer commented 6 years ago

Thank you @Toflar.