Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide controller method level alternative to WebContentInterceptor [SPR-8550] #13194

Closed
spring-issuemaster opened this issue Jul 20, 2011 · 10 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Jul 20, 2011

Craig Walls opened SPR-8550 and commented

Even though WebContentInterceptor can be used to declare when and how cache-control headers are set in a response, it isn't as straightforward or consistent with the @Controller model.

I propose an annotation-based option for declaring when cache-control headers are added to a response. For example, a general-purpose @CachePolicy annotation might be used like this:

@CachePolicy(maxAge=60)
@RequestMapping(value="/headlines", method=RequestMethod.GET)
public String showHeadlines() { ... }

Also, perhaps a more specific-purpose @PreventCaching annotation might declare that a response include the headers currently added by WebContentGenerator's preventCaching() method.

These two annotations are just suggestions--I'd be interested in any solution that allows for declarative cache policies at the request method level.


Affects: 3.0.6

This issue is a sub-task of #16413

Issue Links:

  • #13886 WebContentInterceptor.preHandle(request, response, handler), handler of type HandlerMethod not controller instance
  • #7466 HTTP caching should be decoupled from WebContentGenerator
  • #11789 Add support for public/private Cache-Control HTTP header
  • #18290 Regression: ShallowEtagHeaderFilter does not add Etag header for ResponseEntity returning methods

Referenced from: commits f9ce11e

10 votes, 17 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 1, 2011

Rossen Stoyanchev commented

This would be useful in cases where configuring an interceptor is not ideal (e.g. a framework built on Spring MVC). For Spring MVC applications WebContentInterceptor should be sufficient and more centralized.

Instead of @PreventCaching, we could take maxAge=0 to mean the same. That would also be consistent with how WebContentGenerator works.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 28, 2012

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 16, 2014

Rossen Stoyanchev commented

I've updated the title (was: "Declarative cache policy for @Controller handler methods") to be more flexible with regards to the final solution. Here is one alternative approach for example that is more in line with the way we support not modified with etag for controller methods. Also the new ResponseEntity builder option #16374 may be a natural place to consider adding cache control support.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 23, 2015

Brian Clozel commented

There are now several ways to handle HTTP caching at the controller level:

  • using ResponseEntity when such a type is returned by the controller
  • using WebRequest otherwise

This is described in the dedicated section of the reference documentation.
See also the new CacheControl class Javadoc.

See #16413 for more details.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 27, 2016

Michael Osipov commented

I fail to see how this was reasonably resolved. There is no annotation for this. Using ResponseEntity along with CacheControl means to change all method signatures. Is that the proposed solution as an alternative to WebContentInterceptor?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 27, 2016

Brian Clozel commented

Hi Michael Osipov

Here is my current understanding about this:

  • if your application needs a broad, static cache configuration, then the WebContentInterceptor is probably your best bet; you can configure common cache directives for parts of your URL space. This configuration happens in a single place and is easily understandable.
  • if your application needs more fine-grained cache configuration, then ResponseEntity is the way to go. Indeed, many cache directives depend on the resource itself: "Etag", "Last-Modified", "Cache-Control: public/private/max-age/etc". Many of those choices can't be made without making big assumptions about the served resources and the decision is quite local actually.

Now we were considering @CacheControl annotations but we thought that this would be limiting quite quickly, since cache directives values are often derived from the served resource. How would you support that, besides very complex and error-prone SpEL expressions?

There are also other ways to globally alter those cache directives (by using @SessionAttributes for example), and giving a way to override that at the controller level is probably the best way to give the full power to the developers, and not wonder which cache directives will eventually show up in the response.

What are you missing with the current model?
How would you address this?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 18, 2017

Corey Engelman commented

Hi,

Just curious if the change from outputMessage.getBody() to outputMessage.flush() was intentional? This has a potential to have a pretty large impact. For example if using a javax.servlet.Filter, you cannot make any changes to the response body/headers after calling FilterChain.doFilter, because the flush() call commits the ServeltResponse/forces it to be writtent to the client. If it was intentional and developers shouldn't be doing that (ie everything must be set before calling doFilter), then thats fine, but just wanted to confirm if this is desired behavior, especially since I am not sure exactly how it relates to the issue tracked by this ticket. FWIW this was an issue discovered when upgrading from spring 4.1.x -> 4.3.x

line in question -

commit - f9ce11e

Any help would be greatly appreciated

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 18, 2017

Brian Clozel commented

Hi Corey Engelman,

What you're describing is not specific to this commit at all. In general, you should consider that once calling doFilter, the response might be committed by other filters or the Servlet itself (and this can happen without even calling flush, just writing to the body is enough).

Spring Framework is using a ContentCachingResponseWrapper for such use cases (see for example, the ShallowEtagHeaderFilter).

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 18, 2017

Corey Engelman commented

Thanks for clarifying. I can update the code on my end to assume the response could be committed by doFilter().

Is it worth noting that anyone else who incorrectly made the assumption that it was ok to try to modify headers after doFilter() will also have similar issues? I guess my main point is: whether the code on my end is incorrect or not, it worked with spring 4.1.x and does not work with spring 4.2.x and beyond. Not sure if there is a preferred place to put such notes so other developers don't run into the same issue when upgrading.

Either way, thanks again, this is very helpful (y)

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 27, 2017

Rossen Stoyanchev commented

Note that you can use ResponseBodyAdvice if you want to intercept @ResponseBody methods before the response is committed (or likewise HandlerInterceptor for HTML controller methods). Aside from that the fact that it worked in 4.1.x is by chance, even without an explicit flush, servers will auto-flush once the response buffer fills up, and a flush may also come from serialization libraries such as Jackson.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.