automatic registration of PSPC when @PropertySource is used [SPR-8539]

[spring-projects-issues]

Josh Long opened SPR-8539 and commented

Can we please register a PropertySourcesPlaceholderConfigurer when @PropertySource is used so that everything just "works" in @Configuration classes? I realize that I could also do a @Bean public static PropertySourcesPlaceholderConfigurer pspc(){ }, but it is surprising that it doesn't work by default. It's one more place for users to stub their toes when I can't think of any reason not to support it. Thanks for your consideration.

---

Reference URL: http://blog.springsource.com/2011/06/10/spring-3-1-m2-configuration-enhancements/#comment-195857

Issue Links:

• Document default EmbeddedValueResolver support for property placeholders [SPR-17212] #21745 Document default EmbeddedValueResolver support for property placeholders
• @Enable-style annotation is desired for automatic registration of PropertySourcesPlaceholderConfigurer [SPR-9904] #14537 @Enable-style annotation is desired for automatic registration of PropertySourcesPlaceholderConfigurer
• Placeholder resolution for annotation attributes is not active by default [SPR-14140] #18712 Placeholder resolution for annotation attributes is not active by default
• @Component class using @Value cannot access @PropertySource proeprties [SPR-8516] #13161 @Component class using @Value cannot access @PropertySource proeprties

Referenced from: commits 690d33e

1 votes, 9 watchers

[spring-projects-issues]

Chris Beams commented

After some consideration, resolving this as Won't Fix for the following reasons:

1. it's inconsistent.
   @PropertySource is the declarative counterpart to ConfigurableEnvironment#addPropertySource. We do not add a PropertySourcesPlaceholderConfigurer in the latter case, and it would be inconsistent to do so in the former.
2. it will not be what the user intended in every (or even most) cases.
   It is entirely possible, and even recommended that @Configuration class users forego ${...} property replacement entirely, in favor of Environment#getProperty lookups within @Bean methods. For users following this recommendation, the automatic registration of a PropertySorucesPlaceholderConfigurer would be confusing when noticed, and generally undesirable as it's one more moving part. Yes, it's presence is benign, but not cost-free. a PSPC must visit every bean definition in the container to interrogate PropertyValues, only to do nothing in cases where users are going with the Environment#getProperty approach.
3. it is solvable (and already solved) by documentation.
   Proper use of @PropertySource, PropertySourcesPlaceholderConfigurer and other components is pretty comprehensively documented in the Javadoc for @Configuration already, and reference documentation is soon to follow.

[spring-projects-issues]

Chris Beams commented

Note that for completeness of documentation (#3 above), I've added the following to @PropertySource Javadoc:

commit aa650e9e37954b8a53bfce635555ab120aa8543f (HEAD, master)
Author: Chris Beams <cbeams@vmware.com>
Date:   Tue Dec 6 18:06:48 2011 +0100

    Update @PropertySource Javadoc re: ${...} placeholders
    
    Issue: SPR-8539

diff --git org.springframework.context/src/main/java/org/springframework/context/annotation/PropertySource.java org.springframework.context/src/main/java/org/springframework/cont
index a928d9b..e67f6aa 100644
--- org.springframework.context/src/main/java/org/springframework/context/annotation/PropertySource.java
+++ org.springframework.context/src/main/java/org/springframework/context/annotation/PropertySource.java
@@ -54,7 +54,17 @@ import java.lang.annotation.Target;
  * configuration class and then used when populating the {@code TestBean} object. Given
  * the configuration above, a call to {@code testBean.getName()} will return "myTestBean".
  *
- * <h3>Resolving placeholders within @PropertySource resource locations</h3>
+ * <h3>Resolving ${...} placeholders in {@code <bean>} and {@code @Value} annotations</h3>
+ * In order to resolve ${...} placeholders in {@code <bean>} definitions or {@code @Value}
+ * annotations using properties from a {@code PropertySource}, one must register
+ * a {@code PropertySourcesPlaceholderConfigurer}. This happens automatically when using
+ * {@code <context:property-placeholder>} in XML, but must be explicitly registered using
+ * a {@code static} {@code @Bean} method when using {@code @Configuration} classes. See
+ * the "Working with externalized values" section of @{@link Configuration} Javadoc and
+ * "a note on BeanFactoryPostProcessor-returning @Bean methods" of @{@link Bean} Javadoc
+ * for details and examples.
+ *
+ * <h3>Resolving ${...} placeholders within {@code @PropertySource} resource locations</h3>
  * Any ${...} placeholders present in a {@code @PropertySource} {@linkplain #value()
  * resource location} will be resolved against the set of property sources already
  * registered against the environment.  For example:

[spring-projects-issues]

Eugen Paraschiv commented

Reading through the notes on this issue, I am somewhat unclear as to why Environment#getProperty is favored. At first glance, it would make more sense not to use the lower level and manual lookup but instead to just wire in properties directly, with @Value for instance (or in XML).
Also, regarding the context:property-placeholder - that does register the bean, so the argument about the performance penalty of having the bean in the context should apply to this case as well.
And lastly, I do think that the expectation of most users is to have property resolution working by default, especially once a new property source is defined, and especially seeing how in XML, it does work by default.
Thanks,
Eugen.

[spring-projects-issues]

Chris Beams commented

Eugen,

The recommendation about using Environment#getProperty over @Value within @Configuration classes has several motivations:

1. Decreased verbosity
   In all but the simplest situations, users will likely have not one, but many properties to replace. For example, a JDBC driver typically has four property values to inject. In the @Value style, you end up with four @Value annotations and four otherwise unnecessary fields:

@Value("${jdbc.driverClass}") String jdbcDriverClass;
@Value("${jdbc.url}") String jdbcUrl;
@Value("${jdbc.username}") String jdbcUsername;
@Value("${jdbc.password}") String jdbcPassword;

these values are of course then referenced within the @Bean method that creates and returns the JDBC DataSource.
Now on the other hand, injecting the Environment requires only one line of code in the @Configuration class:

@Inject Environment env;

and then calls to env.getProperty("jdbc.url"), etc are made within the @Bean method. This amounts to reduced verbosity, particularly when you have many properties to deal with.

1. Imperative style
   The @Configuration approach is all about clarity, discoverability, type-safety, etc. Working against the Environment API directly removes a bit of magic (how "${...}" tokens are replaced), and generally fits with the imperative (vs. declarative) style favored by @Bean methods.
2. Fewer moving parts
   As mentioned elsewhere in this thread, eliminating Property(Sources)PlaceholderConfigurer from the mix is a good thing, in that it is one fewer moving part to reason about. This has only minor benefit at the actual container performance level, but the cognitive benefit is more interesting -- it's simply one less thing to need to understand when working with the Spring container. Experienced Spring folks know PPC (and the whole BFPP approach) by heart; however folks newer to Spring do not. BFPP and friends arose in a Spring where configuration was dominated by XML. In code-based configuration, these components are much less necessary, given that the user has programmatic control once again.
3. Greater flexibility
   The Environment, via its PropertyResolver superinterface provides a wealth of methods that allow the user to express exactly what they want and need out of property resolution, including type conversion, default value fallbacks, getProperty vs getRequiredProperty variants that allow and disallow missing properties respectively, and so on. See the full list here: http://static.springsource.org/spring/docs/3.1.x/javadoc-api/org/springframework/core/env/PropertyResolver.html

Hope that helps!

[spring-projects-issues]

Eugen Paraschiv commented

Yes, most of these make sense - it does require a bit of a shift in the mindset with which you're approaching configuration, which is fine. The only downside of making this choice, in my view, is that you're going against the behaviour that you would expect - meaning that you'd expect this to work out of the box and it won't. There is clearly good reason it doesn't - probably making the reference clear about why this is the way it is, and what the best practice is in this area would also be good.
Thanks for taking the time to cover this.

[spring-projects-issues]

Andrey Rubtsov commented

For those cases when registering a PSPC is still not so bad idea, can it be done via some @Enable-style annotation provided by Spring?
Personally I think that PSPC @Bean looks slightly unnatural among app specific beans because it is related to Spring internals rather than carrying application component semantics.

[spring-projects-issues]

Chris Beams commented

Andrey Rubtsov, feel free to create a new issue for this. We can see if it gets much in the way of interest, votes, etc. There's a fine line between what merits an @Enable annotation, and what calls for a simple @Bean definition. This may well fall into the former, but let's see. Thanks!

[spring-projects-issues]

Andrey Rubtsov commented

Ok, created #14537.

[spring-projects-issues]

Sam Brannen commented

FYI: this has effectively been superseded by #18712.