ResponseCacheFilter should support content negotiation

[GoogleCodeExporter]

Currently ResponseCacheFilter uses HttpServletRequest#getRequestURI() to 
determine the cache key. There are two problems with this approach:
- the filter offers no hook for potential sub classes to provide their own 
cache key implementation based on HttpServletRequest
- for APIs that are built around HTTP 'Accept' header based content negotiation 
the ResponseCacheFilter is useless because it does not respect the HTTP 'Vary' 
headers

Example (empty cache):
a) request for '/foo' with 'Accept=application/json' hits the API, response is 
generated, HTTP header 'Vary=Accept' is added -> response cached
b) request for '/foo' with 'Accept=application/vnd.geo+json' does NOT hit API 
since the request URI is a valid cache key -> invalid response is returned

Original issue reported on code.google.com by marcel@frightanic.com on 10 Nov 2014 at 8:21

[GoogleCodeExporter]

Pity I cannot edit the issue...just realized that by mentioning the 'Vary' 
header I made things more complicated to understand than necessary, sorry. 
Since this header is set in the response one can forget about it in the context 
of cache key calculation.

Original comment by marcel@frightanic.com on 10 Nov 2014 at 9:15

[GoogleCodeExporter]

It would be good to have the protected getCacheKey or something for potential 
subclasses to decide it. However currently can it also be done using /foo/json 
and /food/geojson to make the unique URL if that is the option?

Original comment by rr.patil...@gmail.com on 11 Nov 2014 at 1:34

[GoogleCodeExporter]

> currently can it also be done using /foo/json and /food/geojson to make the 
unique URL

Sure, but I don't want the limitations of the caching library to influence my 
API design. For now I just copy/pasted the entire filter and made the necessary 
adjustments.

I also suggest that you offer an option to include 
HttpServletRequest#getQueryString() in the cache key in the default 
implementation.

Original comment by marcel@frightanic.com on 11 Nov 2014 at 6:19

[GoogleCodeExporter]

If your API is REST then it makes sense to have unique URLs describing what 
format you accessing/returning than get query params or other post/header 
params so that to make it truly REpresentational State Transfer compliant. 

Anyways, having protected getCacheKey() that would by default use of 
getRequestURI() internally and sub classes can always override to their need 
would be good I feel.

Original comment by rr.patil...@gmail.com on 11 Nov 2014 at 6:25

[GoogleCodeExporter]

> If your API is REST then it makes sense to have unique URLs describing what 
format you accessing/returning than get query params or other post/header 
params so that to make it truly REpresentational State Transfer compliant. 

That's highly subjective. The REST paradigm as described by its inventor Roy 
Fielding doesn't favor one style over the other. So, you can't claim that a 
system based on accept header content negotiation be /not/ truly RESTful:
- http://programmers.stackexchange.com/a/170409
- http://www.xml.com/pub/a/2004/08/11/rest.html -> 'Deliver Correct Resource 
Representation'

> having protected getCacheKey() that would by default use of getRequestURI() 
internally and sub classes can always override to their need would be good I 
feel

a) I would call it calculateCacheKey rather than getCacheKey
b) The reason why I recommend you support a config parameter to include 
HttpServletRequest#getQueryString() in the /default/ implementation is that 
it's quite common for caching proxies and CDNs to either do this out-of-the-box 
or at least offer to configure this.

Original comment by marcel@frightanic.com on 11 Nov 2014 at 7:40

[GoogleCodeExporter]

All right 

Original comment by rr.patil...@gmail.com on 12 Nov 2014 at 5:26

• Changed state: Accepted
• Added labels: Milestone-0.0.8