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

${} placeholders are not resolved in @ImportResource [SPR-10686] #15314

Closed
spring-issuemaster opened this issue Jun 25, 2013 · 5 comments

Comments

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

commented Jun 25, 2013

Alexey Kolyagin opened SPR-10686 and commented

${} placeholders are not substituted with values from Environment's PropertySources.

This can be easily fixed by changing line 216 of org.springframework.context.annotation.ConfigurationClassParser

from:

configClass.addImportedResource(resource, readerClass);

to:

configClass.addImportedResource(environment.resolveRequiredPlaceholders(resource), readerClass);

Would very much appreciate it if the fix was included into the upcoming 3.2.4 release.


Affects: 3.2.3

Referenced from: commits 1a8f0d6, 67f41b1

1 votes, 4 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 25, 2013

Vladimir Berezniker commented

IMHO the proposed change seems to violate IoC principle in that ConfigurationClassParser would be taking information from somewhere fixed (environment) rather than being given a means to achive the desired: e.g. resource resolver

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 25, 2013

Alexey Kolyagin commented

Let's leave it to Spring Gurus to decide :) I only proposed the simplest possible solution.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 26, 2013

Chris Beams commented

Looks like a fine solution at a glance, and probably an oversight that it was not already this way. Phil Webb, please take a look, thanks.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 27, 2013

Alexey Kolyagin commented

Regarding the issue being reclassified as Improvement: it was an improvement at the time when Environment concept was introduced (3.1) and the feature was reported as implemented by Chris Beams in #12139:

Currently, ${...} placeholder resolution against the current Spring Environment and its set of PropertySources is supported in <import/> and @ImportResource locations.

So from that moment on I'd consider it a Bug. Do you disagree?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 27, 2013

Phil Webb commented

Nope, I don't disagree. Bug it is :)

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.