PropertyPlaceholderConfigurer always tries to replace maps due to incorrect modification check in BeanDefinitionVisitor [SPR-5318]

[spring-projects-issues]

Sid Fischer opened SPR-5318 and commented

I'm programmatically creating bean definitions. One bean definition has a property value of type Map<String,OtherType>. The map keys don't contain placeholders. PropertyPlaceholderConfigurer nevertheless tries to replace the map entries because under the hood BeanDefinitionVisitor's modification check for the map keys (in visitMap()) uses an identity check and the resolveStringValue() method returns a different instance of the same string. Thus the original map is cleared and refilled with its entries despite neither the keys nor the values really changed. This is not only unnecessary, but can also lead to exceptions, e.g. when you intentionally use an unmodifiable Map implementation this will lead to the following error:

java.util.Collections$UnmodifiableMap.clear(Collections.java:1294)
org.springframework.beans.factory.config.BeanDefinitionVisitor.visitMap(BeanDefinitionVisitor.java:250)
org.springframework.beans.factory.config.BeanDefinitionVisitor.resolveValue(BeanDefinitionVisitor.java:192)
org.springframework.beans.factory.config.BeanDefinitionVisitor.visitPropertyValues(BeanDefinitionVisitor.java:142)
org.springframework.beans.factory.config.BeanDefinitionVisitor.visitBeanDefinition(BeanDefinitionVisitor.java:82)
org.springframework.beans.factory.config.PropertyPlaceholderConfigurer.processProperties(PropertyPlaceholderConfigurer.java:265)
org.springframework.beans.factory.config.PropertyResourceConfigurer.postProcessBeanFactory(PropertyResourceConfigurer.java:75)
org.springframework.context.support.AbstractApplicationContext.invokeBeanFactoryPostProcessors(AbstractApplicationContext.java:553)

---

Affects: 2.5.6

Referenced from: commits eb1631f

[spring-projects-issues]

Juergen Hoeller commented

Fixed in 3.0 M2 through consistently returning the original String if not modified. We still do an identity check for map keys which should work for all cases now.

Juergen