WebContentInterceptor.preHandle(request, response, handler), handler of type HandlerMethod not controller instance [SPR-9248]

[spring-projects-issues]

tam sayers opened SPR-9248 and commented

In WebContentInterceptor.preHandle(request, response, handler), the handler type is evaluated for instanceof LastModified to eventually determine whether to add the must-revalidate Cache-Control header or not. The given handler instance is of type HandlerMethod, not the controller instance, so even if the controller implements LastModified when the point is reached to add the header it never is!

---

Affects: 3.1.1

Issue Links:

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

Referenced from: commits 68d4a70

[spring-projects-issues]

tam sayers commented

The MVC configuration is done using:

<mvc:annotation-config />

interceptor is added in:

mvc:interceptors

Controller has @Controller annotation
Controller methods have @RequestMapping and @ResponseBody annotations

[spring-projects-issues]

Rossen Stoyanchev commented

What's the outcome you see? Your comment seems to imply the header is not added. As far as I can see it comes down to WebContentGenerator.cacheForSeconds(..), line 316:

if (mustRevalidate) {
	headerValue += ", must-revalidate";
}

with the javadoc above:

* @param mustRevalidate whether the client should revalidate the resource
* (typically only necessary for controllers with last-modified support)

Even if the controller implemented LastModified, the HandlerAdapter for annotated controllers always returns -1. In this respect there is no change between 3.0 and 3.1. And 3.1 is where HandlerMethod was introduced.

In other words I don't think this has to do with the fact the handler is of type HandlerMethod. Rather it has to do with the fact that LastModified applies to a whole class whereas there can be multiple @RequestMapping methods per class. Actually the way to do it with @MVC controllers is documented on the javadoc for WebRequest.checkNotModified(..).

Have a look and let me know if that makes sense.

[spring-projects-issues]

tam sayers commented

Basically the must-revalidate header is not added to response. Looking at the following code in WebContentHandler:

public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) {
...
    checkAndPrepare(request, response, cacheSeconds, handler instanceof LastModified);
...
}

, the value of handler instanceof LastModified that gets passed as mustRevalidate to the method documented in the previous comment, suggest that if the handler implements LastModified then the header will be added.

If the handler is of type HandlerMethod how can I provide one that implements LastModified so the correct header is added when using the WebContentInterceptor? The AnnotationMethodHandlerAdapter always passes true as the lastModified parameter so the header is added, but doesn't allow the fine grained cache configuration. Is there an alternative to the following configuration that will do the same when using annotation configuration:

<mvc:interceptors>
  <bean id="webContentInterceptor" class="org.springframework.web.servlet.mvc.WebContentInterceptor">
    <property name="useExpiresHeader" value="true" />
    <property name="useCacheControlHeader" value="true" />
    <property name="useCacheControlNoStore" value="true" />
    <property name="cacheMappings">
      <util:properties>
        <prop key="/**/one/**">1</prop>
        <prop key="/**/two/**">2</prop>
        <prop key="/**/three/**">3</prop>
      </util:properties>
    </property>
  </bean>
</mvc:interceptors>

[spring-projects-issues]

Rossen Stoyanchev commented

LastModified can only be implemented by a class so when using annotated @RequestMapping methods that approach doesn't apply. The javadoc of WebRequest.checkNotModified(..) shows what an @RequestMapping method can do instead.

That aside I believe your question is how to set up cache headers to include "; mustRevalidate". Since @RequestMapping methods invoke WebRequest.checkNotModified(..) directly (and LastModified just doesn't apply), the WebContentInterceptor has no good way to tell whether to add mustRevalidate or not.

I'm just curious if this was a regression? In other words did you have a way to make it work in conjunction with @RequestMapping prior to 3.1 (where HandlerMethod was introduced)?

I suspect the answer is no. We need to introduce some declarative way to indicate if a @RequestMapping method does a not-modified check. Perhaps along the lines of #13194.

[spring-projects-issues]

tam sayers commented

I have added the WebContentInterceptor since upgrading from spring 3.0.X so I am not sure if this is a regression or not.

Something along the lines of #13194 would be good but i personally prefer doing that kind of configuration with XML and properties. If a property could be added to the WebContentInterceptor to add must-revalidate to the Cache-Control header irrespective of whether the handler implements LastModified that would solve my problem.

In the meantime I have got round the issue by subclassing WebContentInterceptor and wrapping the response so must-revalidate gets added when the Cache-Control header gets added, but it is a bit of a hack!

[spring-projects-issues]

Rossen Stoyanchev commented

Something along the lines of #13194 would be good but i personally prefer doing that kind of configuration with XML and properties.

The interceptor would still be configured as now. But there needs to be some way for it to know whether a particular @RequestMapping method should be revalidated or not. One approach would be to always add mustRevalidate but I'm not sure that would always work.

[spring-projects-issues]

Rossen Stoyanchev commented

I think we can extend the instanceof LastModified check with an additional check for instanceof HandlerMethod. That would be consistent with the behavior of RequestMappingHandlerMapping, which always passes true to the checkAndPrepare method.

[spring-projects-issues]

Rossen Stoyanchev commented

WebContentInterceptor now has a property called alwaysMustRevalidate.