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

Using multiple PropertyPlaceholderConfigurer breaks @Value default value behavior [SPR-9989] #14623

Closed
spring-issuemaster opened this Issue Nov 14, 2012 · 18 comments

Comments

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

spring-issuemaster commented Nov 14, 2012

Johann Vanackere opened SPR-9989 and commented

When using multiple PropertyPlaceholderConfigurer in conjunction with @Value annotation and default value for placeholders syntax (ie ${key:defaultValue}), only the first PropertyPlaceholderConfigurer is used. If this configurer does not contain the desired value, it falls back to @Value default even if the second PropertyPlaceholderConfigurer contains the value.

You'll find attached a JUnit test case with Spring xml context to reproduce the issue.


Affects: 3.1.3

Attachments:

Issue Links:

  • #15247 Default resolved property value is not working properly ("is duplicated by")
  • #14873 Multiple PropertySourcesPlaceholderConfigurers do not work with placeholder defaults ("is duplicated by")
  • #15282 Improve property placeholder resolution when multiple configurers are declared ("is duplicated by")

Referenced from: commits spring-projects/spring-framework-issues@0017731

41 votes, 43 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 27, 2012

Stevo Slavić commented

As temporary workaround one can use different value separator in each PPC (e.g. : and ~ )

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Jan 22, 2013

Phil Webb commented

Hi Johann,

Do you actually need to add multiple property placeholder resolvers? Will the following work for you?

<bean class="org.springframework.context.support.PropertySourcesPlaceholderConfigurer">
	<property name="ignoreUnresolvablePlaceholders" value="true"/>
	<property name="propertiesArray">
		<array>
			<ref bean="properties1"/>
			<ref bean="properties2"/>
		</array>
	</property>
</bean>

<util:properties id="properties1">
	<prop key="prop1">value1</prop>
</util:properties>

<util:properties id="properties2">
	<prop key="prop2">value2</prop>
</util:properties>
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Jan 28, 2013

Johann Vanackere commented

Hi Phil,

I've used a single PropertySourcesPlaceholderConfigurer with classpath scanning as a temporary workaround.

Anyway, Spring supports using multiple property placeholder resolvers, so @Value behavior with default value is still incorrect and should be fixed.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Jan 31, 2013

Paul Tomlin commented

I've created a workaround for XML users which simply registers the PropertySource with the Environment, leaving the placeholder replacement as a task for an otherwise configured PropertySourcesPlaceholderConfigurer.

<environment:property-source location=".."/>

https://github.com/ptomli/spring-environment

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Apr 17, 2014

Sergei Ivanov commented

We have recently run into a similar issue. In our case we have a class that extends PropertyPlaceholderConfigurer and retrieves properties from our in-house configuration service. We use old school XML configuration approach, and typically each of our reusable components or libraries provides a pre-canned Spring XML configuration, which can be imported into the master Spring XML context and used with minimal or no adjustments. Some of the imported contexts may also define their own instances of PropertyPlaceholderConfigurer.

The problem arises if some of those imported contexts (as well as possibly the master context) use default values for properties. In that case, the order of imports has a dramatic and unpredictable effect on the way how and when default values are used. Basically, the first PropertyPlaceholderConfigurer in the traversal order is always going to use default values for all the properties that it does not know about, leaving no work to other configurers.

In answer to Phil's question: we cannot aggregate all instances of PropertyPlaceholderConfigurer inside the master context, because it defeats the purpose of information encapsulation and stops us from being able to simply import another context without any additional configuration steps.

I am attaching a simplified test case for the case that we have got, only that the imported contexts sit in modules of their own. I could expand it into a Maven reactor project, but for the demonstration purposes it should suffice.

I took a look at the sources of Spring framework and spent some time inside them. The whole logic sits inside PropertyPlaceholderHelper#parseStringValue() method and I believe I can see the problem that the current implementation is facing. If we introduce a configuration option to skip over the unresolvable placeholders, then we need an additional pass at the end to use the default values, and then (keeping in mind that the placeholders can be nested) we may have to repeat the whole process a number of times until no further replacements can be made. This is going to be massively complicated.

I think I'll exploit Paul Tomlin's idea of injecting property sources directly into the environment (need to make sure the order is set correctly so that property injection is always done up-front) and then including a singular bog-standard empty <context:property-placeholder/> into the master context to perform all substitutions.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Jul 14, 2014

Oleksandr Gavenko commented

Problem also discussed at http://stackoverflow.com/questions/16892752/property-not-found-with-multiple-contextproperty-placeholder :

Each context:property-placeholder creates a new instance of PropertyPlaceholderConfigurer - it gets messy easily.

Solution with custom PropertyPlaceholderConfigurer described at blog:

http://rostislav-matl.blogspot.cz/2013/06/resolving-properties-with-spring.html

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 18, 2014

Mauro Molinari commented

This is a serious issue :-(
I hope a solution will come up shortly. I'm going to try the use of different default value separator in each PropertyPlaceholderConfigurer, which seems the best workaround option in my case, although this property is not changeable when using <context:property-placeholder>.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 18, 2014

Sergei Ivanov commented

We have actually implemented the idea that I described in the earlier comment.

I haven't got the implementation at hand now, but from the best of my recollection, we converted our custom implementation of PropertyPlaceholderConfigurer into a post-processor that injected a PropertySource into the current Environment. Each module contributed its own property source post-processor(s), and then there was a single top-level <context:property-placeholder/> that actually performed the substitutions. We possibly needed to manipulate the post-processor order property to make sure that the placeholder configurer always ran last.

Basically, for the whole set-up to work, you need to split the functionality into two stages:

  1. retrieve all properties and inject them as property sources into the environment
  2. perform substitutions for all available properties at once

The first stage will require a new tag, something like <context:property-sources/>, where one would be able to specify property resource location(s), or, perhaps, specify properties directly as an alternative or a fallback option. The second stage is covered by the existing <context:property-placeholder/>.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 19, 2014

Mauro Molinari commented

IMHO the impossibility to add property sources to the environment using built-in tags or beans is a limit and an inconsistency of Spring as of now. However, I think that, independently of this, if the framework allows to define more than one PropertyPlaceholderConfigurer in the context, the behaviour should be consistent, also with regards to applying defaults. So Sergei approach seems to me a valid alternative approach, but not, strictly speaking, a workaround to this problem.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 19, 2014

Sergei Ivanov commented

Default values for properties were also a late addition: I may be wrong, but I believe they were only introduced in Spring 3.x. Likewise, the behaviour of <context:property-placeholder/> handler was changed in Spring 4.x to instantiate PropertySourcesPlaceholderConfigurer by default, instead of PropertyPlaceholderConfigurer in Spring 3.x and earlier.
I am not sure if the current logic of placeholder substitution can be fixed without making it incredibly complicated. A more sensible approach might be to offer new configuration options and deprecating the old ones with clear documentation of known side effects. I personally think that the concept of Environment and PropertySources was a step in the right direction.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Mar 13, 2015

Juergen Hoeller commented

I'm leaning towards just documenting such scenarios. The use of multiple PropertyPlaceholderConfigurers is tricky since they are fundamentally individual processors then, and it's quite old-school in the meantime.

The only real point of multiple PropertyPlaceholderConfigurers in a modern-day configuration environment is to have individual placeholder syntax for each of them, translating specific kinds of placeholders only.

In general, it's definitely preferable to configure multiple source locations for a single placeholder configurer, or to register property sources with the Environment and rely on PropertySourcesPlaceholderConfigurer.

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Apr 9, 2015

Scott Feldstein commented

One more scenario to consider, we overlay our spring context by importing an applicationContext.xml from another project archive which uses the PropertyPlaceholderConfigurer. I had to work around this issue by Autowiring the existing PropertyPlaceholderConfigurer into our @Bean method like this:

@Bean
@Autowired(required = false)
public PropertyPlaceholderConfigurer properties(PropertyPlaceholderConfigurer existing)
        throws FileNotFoundException, IOException {
    PropertyPlaceholderConfigurer ppc = (existing == null) ? new PropertyPlaceholderConfigurer() : existing;
    Properties props = getProperties();
    ppc.setProperties(props);
    return ppc;
}

One issue worth noting with this approach is that the PropertyPlaceholderConfigurer doesn't accept additional locations after the initial locations have been set without overriding the existing locations. Not sure if that will have consequences or not.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Apr 14, 2015

Scott Feldstein commented

I apologize, my example above does not seem to be completely working, the required flag is not being respected when i do not overlay the other xml context for integration test purposes. Therefore when the bean doesn't exist a NoSuchBeanDefinitionException is thrown.

I am using spring boot version 1.2.1.RELEASE (org.springframework:spring-core:4.1.4.RELEASE). Does anyone know if that is a known issue or should I file a separate bug?

Exception in thread "main" org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'properties' ...
	at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:749)
	at org.springframework.beans.factory.support.ConstructorResolver.instantiateUsingFactoryMethod(ConstructorResolver.java:464)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.instantiateUsingFactoryMethod(AbstractAutowireCapableBeanFactory.java:1111)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1006)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:504)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:476)
	at org.springframework.beans.factory.support.AbstractBeanFactory$1.getObject(AbstractBeanFactory.java:303)
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:230)
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:299)
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:199)
	at org.springframework.context.support.PostProcessorRegistrationDelegate.invokeBeanFactoryPostProcessors(PostProcessorRegistrationDelegate.java:150)
	at org.springframework.context.support.AbstractApplicationContext.invokeBeanFactoryPostProcessors(AbstractApplicationContext.java:606)
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:462)
	at org.springframework.boot.context.embedded.EmbeddedWebApplicationContext.refresh(EmbeddedWebApplicationContext.java:118)
	at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:691)
	at org.springframework.boot.SpringApplication.run(SpringApplication.java:321)
	at myspringapp.main(MySpringApp.java:86)
Caused by: org.springframework.beans.factory.NoSuchBeanDefinitionException: No qualifying bean of type [org.springframework.beans.factory.config.PropertyPlaceholderConfigurer] found for dependency: expected at least 1 bean which qualifies as autowire candidate for this dependency. Dependency annotations: {}
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.raiseNoSuchBeanDefinitionException(DefaultListableBeanFactory.java:1308)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.doResolveDependency(DefaultListableBeanFactory.java:1054)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.resolveDependency(DefaultListableBeanFactory.java:949)
	at org.springframework.beans.factory.support.ConstructorResolver.resolveAutowiredArgument(ConstructorResolver.java:813)
	at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:741)
	... 16 more
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Mar 30, 2016

Duane Zamrok commented

There seems to be a lack of understanding as to why you would want multiple PropertyConfigurators. For me, the answer is division of interests. I have an EAR file with several WAR files using a shared context. The shared context, effectively the parent context, has a PropertyConfigurator which encompasses properties loaded and used in the top level context. My WAR files however also have their own context files. I would like the WAR context files to load their own set of properties, specific to the individual WAR. However, because of this particular bug I am unable to do so.

That said, I have looked into the bug personally, tracing the code with the debugger. I understand why it's an issue, and I understand what makes the bug difficult to correct given the current framework and implementation. However, I just wanted to throw in my two cents since I'm seeing a lot of "who would do that?!" comments.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Dec 12, 2016

Veit Guna commented

I would also like to give an example, why multiple PSPC's can be handy. In my case, I'm using two PSPC's in one context to allow reading multiple properties - one optional for overrides, and one required base one with default configs like so:

<context:property-placeholder location="classpath:custom.properties" ignore-resource-not-found="true" order="1" ignore-unresolvable="true" />
<context:property-placeholder location="classpath:application.properties" order="2" />

So, that reads defaults from application.properties and allows overriding any property from an optional custom.properties. It basically works, but as described, using defaults syntax like this in application.properties use the default instead of using the overwritten value in custom.properties:

db.jdbc.url=${DB_JDBC_URL:jdbc:postgresql://${db.host}:${db.port}/${db.name}}

So having db.name=test in custom.properties and getting the value of db.jdbc.url will return the default value of db.name configured in application.properties.

Are there alternatives to get this behavior with existing spring features?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Jan 25, 2017

Michel commented

I've posted an article Here that describes this Gotcha and how to deal with it.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Oct 5, 2017

NathanXu commented

After quite some experimenting (trying to limit the refactoring scope, though big refactoring seems inevitable for this ticket), I think I've found an ideal solution (see master...NathanQingyangXu:SPR-9989):

  • tweak PropertyPlaceholderHelper's implementation to accept a new boolean parameter (ignoreDefault) to control whether default value will be used for a specific StringValueResolver;
  • change AbstractBeanFactory's internal implementation of 'resolveStringValue(String)' to dictate that only the last StringValueResolver will go about default value return (if needed) and ignore default value for all the other ones, by passing appropriate boolean parameter to the above tweaked PropertyPlaceHolderHelper's public method;
  • a new unit testing method named 'resolveEmbeddedDefaultValue' was added to DefaultListableBeanFactoryTests as following:
       @Test
	public void resolveEmbeddedDefaultValue() {
		DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
		Properties props1 = new Properties();
		Properties props2 = new Properties();
		props2.put("A", "B");
		bf.addEmbeddedValueResolver(buildDefaultIgnorableStringValueResolver(props1));
		bf.addEmbeddedValueResolver(buildDefaultIgnorableStringValueResolver(props2));

		String resolvedValue = bf.resolveEmbeddedValue("${A:defaultValue}");

		assertEquals("B", resolvedValue);
	}

Seems the ideal solution to this tricky refactoring ticket.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Jan 12, 2019

Bulk closing outdated, unresolved issues. Please, reopen if still relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment