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

Allow must-revalidate to be suppressed in ResourceHttpRequestHandler [SPR-10464] #15097

Closed
spring-issuemaster opened this issue Apr 16, 2013 · 10 comments

Comments

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

commented Apr 16, 2013

Neale Upstone opened SPR-10464 and commented

I'd like the following code snippet to be able to work, but I cannot set alwaysMustRevalidate:

@Configuration
@EnableWebMvc
public class ResourcesServletConfig extends WebMvcConfigurerAdapter {

    @Autowired
    private ResourceVersionSource versionSource;

    @Override
    public void addResourceHandlers(ResourceHandlerRegistry registry) {

        String cachebustPaths = "/" + versionSource.getVersion() + "/**";

        registry.addResourceHandler(cachebustPaths)
                .addResourceLocations("/")
                .setCachePeriod(365 * 24 * 3600)
                .setMustRevalidate(false);
    }
}

I would be good to have control over other properties of WebContentGenerator too, hence the +others in the summary.


Affects: 3.2.2

This issue is a sub-task of #16413

Referenced from: commits 38f32e3

9 votes, 13 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 16, 2013

Neale Upstone commented

Rossen:

It's worth noting that the property alone will not do the job. I've just tried the following, but ResourceHttpRequestHandler calls checkAndPrepare with lastModified=true, but this then gets turned into a call of applyCacheSeconds(..., mustRevalidate=true).

    @Bean
    public AbstractHandlerMapping resourceHandlerMapping(ResourceVersionSource resourceVersionSource,
            ApplicationContext applicationContext, ServletContext servletContext) {

        String cachebustPaths = "/" + resourceVersionSource.getVersion() + "/**";
        ResourceHttpRequestHandler requestHandler = new ResourceHttpRequestHandler();
        requestHandler.setApplicationContext(applicationContext);
        requestHandler.setServletContext(servletContext);

        requestHandler.setLocations(Collections.singletonList(applicationContext.getResource("/")));
        requestHandler.setCacheSeconds(CACHE_PERIOD);
        requestHandler.setAlwaysMustRevalidate(false);

        SimpleUrlHandlerMapping handlerMapping = new SimpleUrlHandlerMapping();
        handlerMapping.setUrlMap(Collections.singletonMap(cachebustPaths, requestHandler));
        return handlerMapping;

To make it work, I have to override ResourceHttpRequestHandler.setHeaders() and set Cache-Control manually, and override handleRequest() to duplicate but without the If-Modified-Since logic, so that browser doesn't get a Last-Modified header

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 1, 2013

Konrad Garus commented

How about this:

  • Make the ResourceHttpRequestHandler configurable with regards to must-revalidate.
  • Extend the ResourceHandlerRegistry "DSL" to have an easy way to set this configuration.
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 2, 2013

Rossen Stoyanchev commented

Sounds like a good idea. Would anyone have time to put together a pull request?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 8, 2013

Brian Clozel commented

Hi Neale Upstone,

If I understand correcly, you want browsers to cache those resources but you don't want them to revalidate them with GET requests/304 responses, I assume for front-end performance reasons.

I was wondering if you considered setUseExpiresHeader(true) and setUseCacheControlHeader(false) - in that case, client would only try to fetch resources if URLs changed (no GET/304). Could you tell me a little bit more about your use case?

Thanks!

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 11, 2013

Neale Upstone commented

Hi,

It is for a cache busting implementation. I no longer have access to the client issue tracker as to the reasoning, as it was a good while ago. We needed the headers as specified, but I don't recall the detail of what the issues were. It is possibly due to having Apache in front of the server, or CDN related issues.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 12, 2013

Brian Clozel commented

I see, so it's got nothing to do with 304/Last-modified etc.
In that case, removing "must-revalidate" from the Cache-Control header authorizes proxy-caches (such as Apache, Varnish, CDNs) to serve stale resources for a while without hitting the origin server.

You can also take a look at #11789 and #13194 - those may interest you.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 12, 2013

Neale Upstone commented

Yes. That's the scenario.

Looking at the other two, I'd say that what would be useful is for ResourceHandlerRegistration to capture the intent in a readable manner, and warn or error on invalid/unlogical combinations of these.

Perhaps it might be worth investigating how some of these requirements play out via the more opinionated world of Spring Boot. For example, Spring Boot might provide public/private based on part of the path (e.g. public/static, secure/static).

It'd be interesting to get views of others, but I suspect 4.1.0.M1 might be a better place to get feedback on changes for these issues now we're closing in on GA for 4.0...

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 12, 2013

Brian Clozel commented

Thanks for your feedback Neale Upstone.

I'll move this issue to the backlog and we will fix it along the other issues.
I'll definitely ping you then to get your feedback.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 17, 2014

Christopher Smith commented

Support for this would be helpful for me as well. (In my case, the cache is CloudFlare, which will serve an offline static copy of your site if Something Bad happens, but the HTTP spec says that must-validate requires expiration on schedule.)

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 23, 2015

Brian Clozel commented

As of 4.2, the "must-revalidate" directive is now disabled by default when serving resources with ResourceHttpRequestHandler.

A new CacheControl API allows new custom configurations for the "Cache-Control" HTTP response header:

@Override
public void addResourceHandlers(ResourceHandlerRegistry registry) {
    registry.addResourceHandler(cachebustPaths)
                .addResourceLocations("/")
                .setCacheControl(CacheControl.maxAge(365, TimeUnit.DAYS));
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.