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

VersionResourceResolver does not delegate path resolution to the chain [SPR-15372] #19936

Closed
spring-issuemaster opened this Issue Mar 22, 2017 · 3 comments

Comments

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

commented Mar 22, 2017

Dave Syer opened SPR-15372 and commented

It's easy to reproduce. Take the petclinic. Observe that webjars paths have versions in them in the UI

Then set

spring.resources.chain.strategy.content.enabled=true
spring.resources.chain.strategy.content.paths=/resources/**

and observe that the local resources get versions, but the webjars lose theirs.

It seems like a logic error in the ResourceResolverChain. There is a webjar resource resolver there, but it never gets a chance to vote on the path.


No further details from SPR-15372

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 22, 2017

Dave Syer commented

I think maybe the problem is in Spring Boot, but you can decide. In this code: https://github.com/spring-projects/spring-boot/blob/1.5.x/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/WebMvcAutoConfiguration.java#L284 the ResourceHandlerRegistrationCustomizer (which is a Spring Boot internal) is applied to /webjars/** and to the static resource path without checking to see if it is appropriate. Maybe webjars do not need the additional VersionResourceResolver that they get unconditionally this way?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 22, 2017

Dave Syer commented

So this works in the petclinic:

spring.resources.add-mappings=false
spring.resources.chain.strategy.content.enabled=true
spring.resources.chain.strategy.content.paths=/resources/**

(which switches off the Spring Boot resource handling, but keeps the configuration properties) if you also add a WebMvcConfigurerAdapter that does this:

    @Override
    public void addResourceHandlers(ResourceHandlerRegistry registry) {
        Integer cachePeriod = this.resourceProperties.getCachePeriod();
        String staticPathPattern = this.mvcProperties.getStaticPathPattern();
        ResourceProperties.Chain properties = this.resourceProperties.getChain();
        registry.addResourceHandler("/webjars/**")
                .addResourceLocations("classpath:/META-INF/resources/webjars/")
                .setCachePeriod(cachePeriod).resourceChain(properties.isCache());
        registry.addResourceHandler(staticPathPattern)
                .addResourceLocations(this.resourceProperties.getStaticLocations())
                .setCachePeriod(cachePeriod).resourceChain(properties.isCache())
                .addResolver(getResourceResolver());
    }

    private VersionResourceResolver getResourceResolver() {
        VersionResourceResolver resolver = new VersionResourceResolver();
        if (resourceProperties.getChain().getStrategy().getContent().isEnabled()) {
            String[] paths = resourceProperties.getChain().getStrategy().getContent()
                    .getPaths();
            resolver.addContentVersionStrategy(paths);
        }
        return resolver;
    }

I'm not really sure why I had to add the webjars bits there, but it doesn't work without them.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 24, 2017

Brian Clozel commented

Found it!
It's actually the VersionResourceResolver's faults. When calling its resolveUrlPath method, it always returns null if no version strategy is configured for the current path, which does not give a chance for a WebJarResourceResolver configured later in the chain to contribute to the result of the path resolution.

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.