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

MessageSourceAutoConfiguration fails to read messages.properties when running an executable jar with name starting with "spring-boot-" #4811

Closed
britter opened this issue Dec 18, 2015 · 8 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@britter
Copy link
Contributor

britter commented Dec 18, 2015

When MessageSourceAutoConfiguration looks for messages.properties it tries to load all classpath resources using MessageSourceAutoConfiguration$SkipPatternPathMatchingResourcePatternResolver.doFindAllClassPathResources(String). In my case I'm working on an application called spring-boot-heroku-demo, so it will also filter the root classpath resource URL [jar:file:/Users/bene/workspace/projects/spring-boot-heroku-demo/target/spring-boot-heroku-demo-0.0.1-SNAPSHOT.jar!/] because file name returned by that resource is "spring-boot-heroku-demo-0.0.1-SNAPSHOT.jar!/" which starts with "spring-boot".
It works when running the application from an IDE for example, because in this case resources will be loaded from the file system and not from the jar, so a completely different code path is taken to find the messages.properties file.

Possible solutions:

  • Document that "spring-boot-" is a reserved project prefix which must not be used.
  • List all spring-boot projects with full name in the SKIPPED array in MessageSourceAutoConfiguration$SkipPatternPathMatchingResourcePatternResolver
  • Match against the full path of each resource being checked - spring boot project jars will always be included in the lib/ directory.
  • None of the above - the described use case is unlikely to affect any other users.

I'm happy to contribute a patch. Please let me know which solution you'd like to use.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 18, 2015
@britter
Copy link
Contributor Author

britter commented Dec 19, 2015

A possible workaround for this is to set build.finalName in a maven based build to something not starting with "spring-boot-".

britter added a commit to britter/spring-boot-heroku-demo that referenced this issue Dec 20, 2015
This is a workaround for
spring-projects/spring-boot#4811
which causes MessageSourceAutoConfiguration to fail to read
message.properties when running an executable jar.
@snicoll snicoll added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 22, 2015
@snicoll snicoll added this to the 1.2.9 milestone Dec 22, 2015
@snicoll snicoll added type: bug A general bug and removed type: enhancement A general enhancement labels Dec 22, 2015
@snicoll
Copy link
Member

snicoll commented Dec 22, 2015

I am not sure, the current situation looks quite wrong to me. Here are my thoughts on your proposals:

  1. Not an option (per the bug label added unless someone else disagrees)
  2. Maybe
  3. It doesn't change the fact you may have a library with that prefix that you include in your spring boot app. To me it fixes your particular use case but not the underlying problem.

I guess the wildcard scanning is the problem in the first place. Maybe we can change that or we could bring a configuration option that would whitelist a given name (spring-boot-heroku-demo). Yet, having to do that is very weird (you have to know about it!).

I am afraid we'll have to give it more thoughts as fixing this one does not look straightforward to me.

@snicoll snicoll added the for: team-attention An issue we'd like other members of the team to review label Dec 22, 2015
@britter
Copy link
Contributor Author

britter commented Dec 22, 2015

Option 2 might become a maintenance nightmare. One has to remember to extend the SKIPPED array when new spring-boot projects are added. So although I've suggested it, I don't think it is a good fix.

It doesn't change the fact you may have a library with that prefix that you include in your spring boot app. To me it fixes your particular use case but not the underlying problem.

Good point, didn't think about that.

Maybe we can change that or we could bring a configuration option that would whitelist a given name (spring-boot-heroku-demo). Yet, having to do that is very weird (you have to know about it!).

Yes, and the list might become long if you happen to use several project with the spring-boot prefix (or any other prefix listed in the SKIPPED array).

I am afraid we'll have to give it more thoughts as fixing this one does not look straightforward to me.

Fine by me. The workaround I described above, works for me. Thank you for looking into this on short notice.

@philwebb
Copy link
Member

My vote would be a variation on 2) and switch SKIPPED to a set of regex patterns that we can tighten up. The reason that these were added in the first place was as a performance optimization, as long as for production apps we skip common library jars, we shouldn't see any performance regression.

@snicoll
Copy link
Member

snicoll commented Dec 24, 2015

The reason that these were added in the first place was as a performance optimization

Maybe we could try to fix that instead (moving the location or how it is discovered).

@philwebb philwebb self-assigned this Jan 20, 2016
@philwebb philwebb modified the milestones: 1.3.2, 1.2.9 Jan 20, 2016
@wilkinsona wilkinsona removed the for: team-attention An issue we'd like other members of the team to review label Jan 20, 2016
@philwebb
Copy link
Member

See #4930

philwebb added a commit that referenced this issue Jan 21, 2016
Update `ResourceBundleCondition` to only enable the messages source
if `messages.properties` (and not `messages*.properties`) exists. This
operation is much faster that performing a pattern match since a full
jar entry scan is not required.

Since adding `messages.properties` is good practice and this change
allows us to delete the custom `PathMatchingResourcePatternResolver`
it seems like a fine compromise to make.

Fixes gh-4930
See gh-4811
@philwebb
Copy link
Member

We've decided to change the message.properties detection logic removing the need entirely for the custom pattern resolver. Fixed in e520c47

@britter
Copy link
Contributor Author

britter commented Jan 22, 2016

Nice, thanks!

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

No branches or pull requests

5 participants