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

Spring Boot not working well with the new ResourceTransformer in Spring 4.1 #2360

Closed
csavory opened this issue Jan 14, 2015 · 11 comments
Closed
Assignees

Comments

@csavory
Copy link
Contributor

csavory commented Jan 14, 2015

Recently I introduced the new ResourceTransformer into our application for JS and CSS files. In my WebConfig I overrode the addResourceHandlers method. and added the following code:

    @Override
    public void addResourceHandlers( ResourceHandlerRegistry registry ) {

        boolean localMode = this.env.acceptsProfiles( "local" );

        //String location = localMode ? "file:///" + getProjectRootRequired() + "/client/src/" : "classpath:/static/"; 
        String location = "classpath:/static/";
        Integer cachePeriod = this.resourceProperties.getCachePeriod();
        boolean useResourceCache = !localMode;
        String version = localMode ? "local" : this.appVersion;

        AppCacheManifestTransformer appCacheTransformer = new AppCacheManifestTransformer();
        LessLinkResourceTransformer lessLinkResourceTransformer = new LessLinkResourceTransformer();
        VersionResourceResolver versionResolver = new VersionResourceResolver()
                //.addFixedVersionStrategy( version, "/**/*.js" ) //Enable this if we use a JavaScript module loader
                .addContentVersionStrategy( "/**" );

        //A Handler With Versioning
        registry.addResourceHandler( "/**/*.js", "/**/*.css", "/**/*.less", "/**/*.png", "/**/*.gif", "/**/*.jpg", "/**/*.map" )
                .addResourceLocations( location )
                .setCachePeriod( this.fingerprintedCachePeriod )
                .resourceChain( useResourceCache )
                .addResolver( versionResolver )
                .addTransformer( appCacheTransformer )
                .addTransformer( lessLinkResourceTransformer );
    }

I added the ResourceUrlEncodingFilter as it says to do in the Spring 4.1 docs. Then when I tested my app I realized that the CSS files were being versioned, but the JS files were not being versioned. After a lot of debugging, I realized that the problem was in ResourceUrlEncodingFilter.getForLookupPath. It loops through all the handler mappings and uses the first one that matches with AntMatcher. My CSS links were being matched to my mapping that I added in my WebConfig, but my JS links were always getting picked up by another mapping ("/") that did not include a version transformer. I finally tracked this mapping to the one that was added in WebMvcAutoConfiguration.WebMvcAutoConfigurationAdapter.addResourceHandlers. This was a huge pain since the ResourceHandlerRegistry API does not allow you to remove existing handlers, but only add new ones. I didn't want a to overwrite "/" since I didn't want all my links transformed. The only temporary solution I could come up with was to make my WebConfig extend WebMvcAutoConfiguration.WebMvcAutoConfigurationAdapter so that WebMvcAutoConfiguration.WebMvcAutoConfigurationAdapter.addResourceHandlers would not get called. Then I added another ResourceHandler to my addResourceHandlers for the static resources I did not want versioned.

I don't think this is a good long term solution for us, but it works for the mean time.

@philwebb
Copy link
Member

/cc @bclozel

@philwebb
Copy link
Member

@bclozel Do you think this an auto-configure problem or something that needs to change in Spring Framework?

@bclozel
Copy link
Member

bclozel commented Jan 16, 2015

Hi @csavory

I'm not sure I understand what's happening here. A few questions:

  1. Is this issue related to SPR-12628? How?
  2. How come all files but js files are matched by your custom handler, while js files are matched by /**? This may be a bug, can you reproduce it? (in Spring Framework, we're using AbstractUrlHandlerMapping to select the best matching pattern, so /** should be a fallback anyway)
  3. I'm not sure what kind of change you'd like to fix this. Would you rather disable all resourcehandler autoconfiguration if you define your own?

@csavory
Copy link
Contributor Author

csavory commented Jan 16, 2015

#1 - As far as I know, it's only related in that I was working on the ResourceTransformer integration this week and I created two issues in Spring Jira and one on the Spring Boot Github.

#2 - I think I narrowed down the problem to ResourceUrlProvider.getForLookupPath line 198. When it loops through the handlerMap there is no order to the keySet. So /** was always showing up 2nd and //_.js was 3rd in the list. So the AntMatcher would always match to /_* first and it did not have a Transformer associated with it since that was the one defined in WebMvcAutoConfiguration.WebMvcAutoConfigurationAdapter.addResourceHandlers
/
was not acting as a fall back in my case.

Regarding #3, I think we could fix it two ways. Allow a priority for the handlerMap in ResourceUrlProvider that way I can set my mappings as a priority over Boot's mappings. The other way we could fix it would be to allow removal of mappings in the ResourceHandlerRegistry API.

@csavory
Copy link
Contributor Author

csavory commented Jan 16, 2015

@bclozel this could be related to https://jira.spring.io/browse/SPR-12630

@rstoyanchev
Copy link
Contributor

I've opened https://jira.spring.io/browse/SPR-12647. Would that address your concerns here?

@csavory
Copy link
Contributor Author

csavory commented Jan 20, 2015

@rstoyanchev, yes I think it would. As long as the ResouceHandler created by spring boot was ordered later in the list.

@csavory
Copy link
Contributor Author

csavory commented Jan 27, 2015

@bclozel I'm trying to do some planning on our end. Do you have an anticipated milestone for this issue?

@bclozel
Copy link
Member

bclozel commented Jan 28, 2015

@csavory This should be fixed in Spring Framework 4.1.5 and 4.2.0, see SPR-12647 for more details.
Note that to get this fix you'll probably have to override the Spring version in your Boot app or wait for the next Boot release.

I'll close this issue as it is confirmed to be a Framework issue; we'll open a new PR/issue against Boot if some changes are necessary.

@bclozel bclozel closed this as completed Jan 28, 2015
@wilkinsona
Copy link
Member

#2413 is tracking Boot's upgrade to Spring Framework 4.1.5

@csavory
Copy link
Contributor Author

csavory commented Apr 14, 2015

@bclozel thanks for all the fixes! We were finally able to upgrade to 1.2.2 and bring these changes in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants