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

Call to addResourceLocations should not always be mandatory [SPR-12133] #16749

Closed
spring-projects-issues opened this issue Aug 28, 2014 · 8 comments
Closed
Assignees
Labels
in: web type: enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Aug 28, 2014

Guillaume DROUET opened SPR-12133 and commented

Since the user can specify its own PathResourceResolver (#16740), its underlying implementation may not always require resource locations to be configured. In that case, an unecessary call to addResourceLocations must be done.


Affects: 4.1 RC2

Referenced from: commits 2ef20f6, 4df05d1

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 29, 2014

Rossen Stoyanchev commented

Do you mean a call to addResourceLocations? As in your (related) comment here?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 29, 2014

Guillaume DROUET commented

Yes exactly. Here is an example: https://github.com/wuic/wuic-samples/blob/wuic-0.5.x-snapshot/spring-sample/src/main/java/com/github/wuic/spring/WuicWebConfig.java#L194-L198

WuicPathResourceResolver does not care if there is resources or not. It uses the underlying framework mechanism so I should be able to configure the registry without calling addResourceLocations(). This is a requirement which should be indicated by the resolver itself.

What do you think?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 29, 2014

Rossen Stoyanchev commented

Modified title (was: "Call to addResourceResolver is not always mandatory")

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 29, 2014

Rossen Stoyanchev commented

I'll have a look. In the very least we can move the assertion or change it to a warning in PathResourceResolver. The trade-off there is that this check is at runtime vs startup. However I see your point that with the addition of a resource resolver chain in 4.1, the assumption that locations are required no longer holds. So that needs to be fixed.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 29, 2014

Rossen Stoyanchev commented

I've removed the assertion for a non-empty list of resource locations and improved the text for a pre-existing warning in ResourceHttpRequestHandler (in afterPropertiesSet).

Having looked at WuicPathResourceResolver, I am wondering does it still need to extend PathResourceResolver? It seems that with the current state of affairs it wouldn't care even if a PathResourceResolver was added after it.

Aside from that the integration looks pretty straight forward. Can't wait to play with the sample!

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 29, 2014

Guillaume DROUET commented

Ok perfect.

Even if you check at runtime it's not important: the server cache will skip this instruction after the first resolution. And if the user really wants to do it when server starts, he can execute resolution of key resources programmatically in order to directly put the result in cache. This is a strategy that WUIC offers but in spring integration the user have to do it by hand.

Regarding the inheritance of PathResourceResolver, yes it's not mandatory now. The issue was that custom resolver was executed before the VersionStrategy. However, I think I should keep things like that in order to no create a useless instance of PathResourceResolver that will never get executed.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 1, 2014

Guillaume DROUET commented

The issue is not resolved as indicated in this comment: 4df05d1#commitcomment-7599874

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 2, 2014

Brian Clozel commented

See commit 2ef20f63.

@spring-projects-issues spring-projects-issues added type: enhancement in: web labels Jan 11, 2019
@spring-projects-issues spring-projects-issues added this to the 4.1 GA milestone Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web type: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants