Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Minor
    • Resolution: Complete
    • Affects Version/s: 3.0.6
    • Fix Version/s: 4.2 RC1
    • Component/s: Web
    • Labels:
    • Last commented by a User:
      false

      Description

      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.

        Issue Links

          Activity

          Hide
          bclozel Brian Clozel added a comment -

          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?

          Show
          bclozel Brian Clozel added a comment - 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?
          Hide
          corey.engelman.1 Corey Engelman added a comment -

          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 - https://github.com/spring-projects/spring-framework/blob/f9ce11eef8b05e7e31b45a428d63ae35eed8ed42/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java#L156

          commit - https://github.com/spring-projects/spring-framework/commit/f9ce11eef8b05e7e31b45a428d63ae35eed8ed42

          Any help would be greatly appreciated

          Show
          corey.engelman.1 Corey Engelman added a comment - 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 - https://github.com/spring-projects/spring-framework/blob/f9ce11eef8b05e7e31b45a428d63ae35eed8ed42/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/HttpEntityMethodProcessor.java#L156 commit - https://github.com/spring-projects/spring-framework/commit/f9ce11eef8b05e7e31b45a428d63ae35eed8ed42 Any help would be greatly appreciated
          Hide
          bclozel Brian Clozel added a comment -

          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).

          Show
          bclozel Brian Clozel added a comment - 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 ).
          Hide
          corey.engelman.1 Corey Engelman added a comment -

          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

          Show
          corey.engelman.1 Corey Engelman added a comment - 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
          Hide
          rstoya05-aop Rossen Stoyanchev added a comment -

          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.

          Show
          rstoya05-aop Rossen Stoyanchev added a comment - 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.

            People

            • Assignee:
              bclozel Brian Clozel
              Reporter:
              habuma Craig Walls
              Last updater:
              Rossen Stoyanchev
            • Votes:
              10 Vote for this issue
              Watchers:
              17 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Days since last comment:
                2 weeks, 6 days ago