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

Consider if we should always register ResourceUrlEncodingFilter #3353

Closed
philwebb opened this issue Jun 29, 2015 · 4 comments
Closed

Consider if we should always register ResourceUrlEncodingFilter #3353

philwebb opened this issue Jun 29, 2015 · 4 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@philwebb
Copy link
Member

Commit dd561d1 add ResourceUrlEncodingFilter beans for ThymeleafAutoConfiguration and VelocityAutoConfiguration. Should we push this up an always register it? Will it do any harm if it's always on?

@philwebb philwebb added this to the 1.3.0.M2 milestone Jun 29, 2015
@philwebb
Copy link
Member Author

/cc @bclozel @snicoll

@philwebb
Copy link
Member Author

Should also be conditional on ResourceProperties.chain.enabled

@bclozel
Copy link
Member

bclozel commented Jul 1, 2015

That filter wraps ResourceUrlProvider, which assists with writing URLs in templates with the resource chain.

Now this particular filter ResourceUrlEncodingFilter is only useful for template engines that use HttpServletResponse.encodeURL, namely JSP, Thymeleaf, Velocity (and others?).
Registering globally such a filter would not do any harm, but doing so:

  • will only help JSP users
  • will be a (small) performance penalty for all other webapps

Registering it conditionally with ResourceProperties.chain.enabled is indeed a good solution: it's also registering that filter less aggressively for other template engines. So yes!

I'm wondering if we need to spend time and resources finding solutions for other template engines. Right now, I don't know if Spring Boot wants to be in the business of providing template helpers/macros for the ones that aren't supported yet (JMustache, Groovy Markup Template, etc).

Some template engines don't natively support helpers/macros, and you basically have to register those in the modelMap for all views - see this example for Groovy Markup Template.

@philwebb philwebb modified the milestones: 1.3.0.RC1, 1.3.0.M2 Jul 7, 2015
@snicoll snicoll self-assigned this Jul 28, 2015
@snicoll snicoll added type: enhancement A general enhancement and removed discussion labels Jul 28, 2015
@philwebb philwebb modified the milestones: 1.3.0.RC1, 1.3.0.M3 Aug 5, 2015
@snicoll
Copy link
Member

snicoll commented Aug 19, 2015

It's much more challenging now. spring.resources.chain.enabled is actually enabled if either that property is set or if any of the strategy is enabled (that is spring.resources.chain.fixed.enabled or spring.resources.chain.content.enabled). In practice, we'd need the ResourceProperites in a condition to figure out if it applies or not. Seems like quite a lot of work at this point.

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

No branches or pull requests

3 participants