Spring Framework
  1. Spring Framework
  2. SPR-8539

automatic registration of PSPC when @PropertySource is used

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Won't Fix
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Core
    • Labels:
      None

      Description

      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.

        Issue Links

          Activity

          Hide
          Chris Beams added a comment -

          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.
          Show
          Chris Beams added a comment - After some consideration, resolving this as Won't Fix for the following reasons: 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. 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. 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.
          Hide
          Chris Beams added a comment -

          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:
          
          Show
          Chris Beams added a comment - 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:
          Hide
          Eugen Paraschiv added a comment -

          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.

          Show
          Eugen Paraschiv added a comment - 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.
          Hide
          Chris Beams added a comment - - edited

          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.

          2. 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.

          3. 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.
          4. 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!

          Show
          Chris Beams added a comment - - edited Eugen, The recommendation about using Environment#getProperty over @Value within @Configuration classes has several motivations: 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. 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. 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. 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!
          Hide
          Eugen Paraschiv added a comment -

          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.

          Show
          Eugen Paraschiv added a comment - 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.
          Hide
          Andrii Rubtsov added a comment -

          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.

          Show
          Andrii Rubtsov added a comment - 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.
          Hide
          Chris Beams added a comment -

          Andrii 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!

          Show
          Chris Beams added a comment - Andrii 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!
          Hide
          Andrii Rubtsov added a comment -

          Ok, created SPR-9904.

          Show
          Andrii Rubtsov added a comment - Ok, created SPR-9904 .

            People

            • Assignee:
              Chris Beams
              Reporter:
              Josh Long
              Last updater:
              Chris Beams
            • Votes:
              1 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Days since last comment:
                1 year, 26 weeks, 3 days ago