PropertySourcesPlaceholderConfigurer's "ignoreUnresolvablePlaceholders" setting reliably applies to nested placeholders as well

Issue: SPR-10549
(cherry picked from commit 127b91f)
This commit is contained in:
Juergen Hoeller 2013-07-31 17:50:44 +02:00
parent 2f8dfb3e52
commit b40263e06b
8 changed files with 132 additions and 57 deletions

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2012 the original author or authors. * Copyright 2002-2013 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -108,8 +108,8 @@ public class PropertySourcesPlaceholderConfigurer extends PlaceholderConfigurerS
* <p>Processing occurs by replacing ${...} placeholders in bean definitions by resolving each * <p>Processing occurs by replacing ${...} placeholders in bean definitions by resolving each
* against this configurer's set of {@link PropertySources}, which includes: * against this configurer's set of {@link PropertySources}, which includes:
* <ul> * <ul>
* <li>all {@linkplain org.springframework.core.env.ConfigurableEnvironment#getPropertySources environment property sources}, if an * <li>all {@linkplain org.springframework.core.env.ConfigurableEnvironment#getPropertySources
* {@code Environment} {@linkplain #setEnvironment is present} * environment property sources}, if an {@code Environment} {@linkplain #setEnvironment is present}
* <li>{@linkplain #mergeProperties merged local properties}, if {@linkplain #setLocation any} * <li>{@linkplain #mergeProperties merged local properties}, if {@linkplain #setLocation any}
* {@linkplain #setLocations have} {@linkplain #setProperties been} * {@linkplain #setLocations have} {@linkplain #setProperties been}
* {@linkplain #setPropertiesArray specified} * {@linkplain #setPropertiesArray specified}
@ -135,7 +135,7 @@ public class PropertySourcesPlaceholderConfigurer extends PlaceholderConfigurerS
} }
try { try {
PropertySource<?> localPropertySource = PropertySource<?> localPropertySource =
new PropertiesPropertySource(LOCAL_PROPERTIES_PROPERTY_SOURCE_NAME, this.mergeProperties()); new PropertiesPropertySource(LOCAL_PROPERTIES_PROPERTY_SOURCE_NAME, mergeProperties());
if (this.localOverride) { if (this.localOverride) {
this.propertySources.addFirst(localPropertySource); this.propertySources.addFirst(localPropertySource);
} }
@ -148,7 +148,7 @@ public class PropertySourcesPlaceholderConfigurer extends PlaceholderConfigurerS
} }
} }
this.processProperties(beanFactory, new PropertySourcesPropertyResolver(this.propertySources)); processProperties(beanFactory, new PropertySourcesPropertyResolver(this.propertySources));
} }
/** /**

View File

@ -153,6 +153,25 @@ public class PropertyResourceConfigurerIntegrationTests {
} }
} }
@Test
public void testPropertyPlaceholderConfigurerWithNestedUnresolvableReference() {
StaticApplicationContext ac = new StaticApplicationContext();
MutablePropertyValues pvs = new MutablePropertyValues();
pvs.add("name", "name${var}");
ac.registerSingleton("tb1", TestBean.class, pvs);
pvs = new MutablePropertyValues();
pvs.add("properties", "var=${m}var\nm=${var2}\nvar2=${m2}");
ac.registerSingleton("configurer1", PropertyPlaceholderConfigurer.class, pvs);
try {
ac.refresh();
fail("Should have thrown BeanDefinitionStoreException");
}
catch (BeanDefinitionStoreException ex) {
// expected
ex.printStackTrace();
}
}
@Ignore // this test was breaking after the 3.0 repackaging @Ignore // this test was breaking after the 3.0 repackaging
@Test @Test
public void testPropertyPlaceholderConfigurerWithAutowireByType() { public void testPropertyPlaceholderConfigurerWithAutowireByType() {

View File

@ -39,8 +39,6 @@ import org.springframework.mock.env.MockPropertySource;
import org.springframework.tests.sample.beans.TestBean; import org.springframework.tests.sample.beans.TestBean;
/** /**
* Unit tests for {@link PropertySourcesPlaceholderConfigurer}.
*
* @author Chris Beams * @author Chris Beams
* @since 3.1 * @since 3.1
*/ */
@ -139,7 +137,9 @@ public class PropertySourcesPlaceholderConfigurerTests {
PropertySourcesPlaceholderConfigurer pc = new PropertySourcesPlaceholderConfigurer(); PropertySourcesPlaceholderConfigurer pc = new PropertySourcesPlaceholderConfigurer();
pc.setPropertySources(propertySources); pc.setPropertySources(propertySources);
pc.setProperties(new Properties() {{ put("my.name", "local"); }}); pc.setProperties(new Properties() {{
put("my.name", "local");
}});
pc.setIgnoreUnresolvablePlaceholders(true); pc.setIgnoreUnresolvablePlaceholders(true);
pc.postProcessBeanFactory(bf); pc.postProcessBeanFactory(bf);
assertThat(bf.getBean(TestBean.class).getName(), equalTo("${my.name}")); assertThat(bf.getBean(TestBean.class).getName(), equalTo("${my.name}"));
@ -172,6 +172,38 @@ public class PropertySourcesPlaceholderConfigurerTests {
assertThat(bf.getBean(TestBean.class).getName(), equalTo("${my.name}")); assertThat(bf.getBean(TestBean.class).getName(), equalTo("${my.name}"));
} }
@Test(expected=BeanDefinitionStoreException.class)
public void nestedUnresolvablePlaceholder() {
DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
bf.registerBeanDefinition("testBean",
genericBeanDefinition(TestBean.class)
.addPropertyValue("name", "${my.name}")
.getBeanDefinition());
PropertySourcesPlaceholderConfigurer pc = new PropertySourcesPlaceholderConfigurer();
pc.setProperties(new Properties() {{
put("my.name", "${bogus}");
}});
pc.postProcessBeanFactory(bf); // should throw
}
@Test
public void ignoredNestedUnresolvablePlaceholder() {
DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
bf.registerBeanDefinition("testBean",
genericBeanDefinition(TestBean.class)
.addPropertyValue("name", "${my.name}")
.getBeanDefinition());
PropertySourcesPlaceholderConfigurer pc = new PropertySourcesPlaceholderConfigurer();
pc.setProperties(new Properties() {{
put("my.name", "${bogus}");
}});
pc.setIgnoreUnresolvablePlaceholders(true);
pc.postProcessBeanFactory(bf);
assertThat(bf.getBean(TestBean.class).getName(), equalTo("${bogus}"));
}
@Test @Test
public void withNonEnumerablePropertySource() { public void withNonEnumerablePropertySource() {
DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
@ -213,7 +245,8 @@ public class PropertySourcesPlaceholderConfigurerTests {
ppc.postProcessBeanFactory(bf); ppc.postProcessBeanFactory(bf);
if (override) { if (override) {
assertThat(bf.getBean(TestBean.class).getName(), equalTo("local")); assertThat(bf.getBean(TestBean.class).getName(), equalTo("local"));
} else { }
else {
assertThat(bf.getBean(TestBean.class).getName(), equalTo("enclosing")); assertThat(bf.getBean(TestBean.class).getName(), equalTo("enclosing"));
} }
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2012 the original author or authors. * Copyright 2002-2013 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -16,25 +16,22 @@
package org.springframework.core.env; package org.springframework.core.env;
import static java.lang.String.format;
import static org.springframework.util.SystemPropertyUtils.PLACEHOLDER_PREFIX;
import static org.springframework.util.SystemPropertyUtils.PLACEHOLDER_SUFFIX;
import static org.springframework.util.SystemPropertyUtils.VALUE_SEPARATOR;
import java.util.LinkedHashSet; import java.util.LinkedHashSet;
import java.util.Set; import java.util.Set;
import org.apache.commons.logging.Log; import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory; import org.apache.commons.logging.LogFactory;
import org.springframework.core.convert.support.ConfigurableConversionService; import org.springframework.core.convert.support.ConfigurableConversionService;
import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.core.convert.support.DefaultConversionService;
import org.springframework.util.PropertyPlaceholderHelper; import org.springframework.util.PropertyPlaceholderHelper;
import org.springframework.util.PropertyPlaceholderHelper.PlaceholderResolver; import org.springframework.util.SystemPropertyUtils;
/** /**
* Abstract base class for resolving properties against any underlying source. * Abstract base class for resolving properties against any underlying source.
* *
* @author Chris Beams * @author Chris Beams
* @author Juergen Hoeller
* @since 3.1 * @since 3.1
*/ */
public abstract class AbstractPropertyResolver implements ConfigurablePropertyResolver { public abstract class AbstractPropertyResolver implements ConfigurablePropertyResolver {
@ -44,15 +41,20 @@ public abstract class AbstractPropertyResolver implements ConfigurablePropertyRe
protected ConfigurableConversionService conversionService = new DefaultConversionService(); protected ConfigurableConversionService conversionService = new DefaultConversionService();
private PropertyPlaceholderHelper nonStrictHelper; private PropertyPlaceholderHelper nonStrictHelper;
private PropertyPlaceholderHelper strictHelper; private PropertyPlaceholderHelper strictHelper;
private boolean ignoreUnresolvableNestedPlaceholders = false; private boolean ignoreUnresolvableNestedPlaceholders = false;
private String placeholderPrefix = PLACEHOLDER_PREFIX; private String placeholderPrefix = SystemPropertyUtils.PLACEHOLDER_PREFIX;
private String placeholderSuffix = PLACEHOLDER_SUFFIX;
private String valueSeparator = VALUE_SEPARATOR; private String placeholderSuffix = SystemPropertyUtils.PLACEHOLDER_SUFFIX;
private String valueSeparator = SystemPropertyUtils.VALUE_SEPARATOR;
private final Set<String> requiredProperties = new LinkedHashSet<String>(); private final Set<String> requiredProperties = new LinkedHashSet<String>();
public ConfigurableConversionService getConversionService() { public ConfigurableConversionService getConversionService() {
return this.conversionService; return this.conversionService;
} }
@ -63,12 +65,12 @@ public abstract class AbstractPropertyResolver implements ConfigurablePropertyRe
public String getProperty(String key, String defaultValue) { public String getProperty(String key, String defaultValue) {
String value = getProperty(key); String value = getProperty(key);
return value == null ? defaultValue : value; return (value != null ? value : defaultValue);
} }
public <T> T getProperty(String key, Class<T> targetType, T defaultValue) { public <T> T getProperty(String key, Class<T> targetType, T defaultValue) {
T value = getProperty(key, targetType); T value = getProperty(key, targetType);
return value == null ? defaultValue : value; return (value != null ? value : defaultValue);
} }
public void setRequiredProperties(String... requiredProperties) { public void setRequiredProperties(String... requiredProperties) {
@ -92,7 +94,7 @@ public abstract class AbstractPropertyResolver implements ConfigurablePropertyRe
public String getRequiredProperty(String key) throws IllegalStateException { public String getRequiredProperty(String key) throws IllegalStateException {
String value = getProperty(key); String value = getProperty(key);
if (value == null) { if (value == null) {
throw new IllegalStateException(format("required key [%s] not found", key)); throw new IllegalStateException(String.format("required key [%s] not found", key));
} }
return value; return value;
} }
@ -100,7 +102,7 @@ public abstract class AbstractPropertyResolver implements ConfigurablePropertyRe
public <T> T getRequiredProperty(String key, Class<T> valueType) throws IllegalStateException { public <T> T getRequiredProperty(String key, Class<T> valueType) throws IllegalStateException {
T value = getProperty(key, valueType); T value = getProperty(key, valueType);
if (value == null) { if (value == null) {
throw new IllegalStateException(format("required key [%s] not found", key)); throw new IllegalStateException(String.format("required key [%s] not found", key));
} }
return value; return value;
} }
@ -130,17 +132,17 @@ public abstract class AbstractPropertyResolver implements ConfigurablePropertyRe
} }
public String resolvePlaceholders(String text) { public String resolvePlaceholders(String text) {
if (nonStrictHelper == null) { if (this.nonStrictHelper == null) {
nonStrictHelper = createPlaceholderHelper(true); this.nonStrictHelper = createPlaceholderHelper(true);
} }
return doResolvePlaceholders(text, nonStrictHelper); return doResolvePlaceholders(text, this.nonStrictHelper);
} }
public String resolveRequiredPlaceholders(String text) throws IllegalArgumentException { public String resolveRequiredPlaceholders(String text) throws IllegalArgumentException {
if (strictHelper == null) { if (this.strictHelper == null) {
strictHelper = createPlaceholderHelper(false); this.strictHelper = createPlaceholderHelper(false);
} }
return doResolvePlaceholders(text, strictHelper); return doResolvePlaceholders(text, this.strictHelper);
} }
/** /**
@ -154,15 +156,19 @@ public abstract class AbstractPropertyResolver implements ConfigurablePropertyRe
/** /**
* Resolve placeholders within the given string, deferring to the value of * Resolve placeholders within the given string, deferring to the value of
* {@link #setIgnoreUnresolvableNestedPlaceholders(boolean)} to determine whether any * {@link #setIgnoreUnresolvableNestedPlaceholders} to determine whether any
* unresolvable placeholders should raise an exception or be ignored. * unresolvable placeholders should raise an exception or be ignored.
* <p>Invoked from {@link #getProperty} and its variants, implicitly resolving
* nested placeholders. In contrast, {@link #resolvePlaceholders} and
* {@link #resolveRequiredPlaceholders} do <emphasis>not</emphasis> delegate
* to this method but rather perform their own handling of unresolvable
* placeholders, as specified by each of those methods.
* @since 3.2 * @since 3.2
* @see #setIgnoreUnresolvableNestedPlaceholders(boolean) * @see #setIgnoreUnresolvableNestedPlaceholders
*/ */
protected String resolveNestedPlaceholders(String value) { protected String resolveNestedPlaceholders(String value) {
return this.ignoreUnresolvableNestedPlaceholders ? return this.ignoreUnresolvableNestedPlaceholders ?
this.resolvePlaceholders(value) : resolvePlaceholders(value) : resolveRequiredPlaceholders(value);
this.resolveRequiredPlaceholders(value);
} }
private PropertyPlaceholderHelper createPlaceholderHelper(boolean ignoreUnresolvablePlaceholders) { private PropertyPlaceholderHelper createPlaceholderHelper(boolean ignoreUnresolvablePlaceholders) {
@ -171,11 +177,19 @@ public abstract class AbstractPropertyResolver implements ConfigurablePropertyRe
} }
private String doResolvePlaceholders(String text, PropertyPlaceholderHelper helper) { private String doResolvePlaceholders(String text, PropertyPlaceholderHelper helper) {
return helper.replacePlaceholders(text, new PlaceholderResolver() { return helper.replacePlaceholders(text, new PropertyPlaceholderHelper.PlaceholderResolver() {
public String resolvePlaceholder(String placeholderName) { public String resolvePlaceholder(String placeholderName) {
return getProperty(placeholderName); return getPropertyAsRawString(placeholderName);
} }
}); });
} }
/**
* Retrieve the specified property as a raw String,
* i.e. without resolution of nested placeholders.
* @param key the property name to resolve
* @return the property value or {@code null} if none found
*/
protected abstract String getPropertyAsRawString(String key);
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2012 the original author or authors. * Copyright 2002-2013 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -74,8 +74,9 @@ public interface PropertyResolver {
/** /**
* Convert the property value associated with the given key to a {@code Class} * Convert the property value associated with the given key to a {@code Class}
* of type {@code T} or {@code null} if the key cannot be resolved. * of type {@code T} or {@code null} if the key cannot be resolved.
* @throws ConversionException if class specified by property value cannot be found * @throws org.springframework.core.convert.ConversionException if class specified
* or loaded or if targetType is not assignable from class specified by property value * by property value cannot be found or loaded or if targetType is not assignable
* from class specified by property value
* @see #getProperty(String, Class) * @see #getProperty(String, Class)
*/ */
<T> Class<T> getPropertyAsClass(String key, Class<T> targetType); <T> Class<T> getPropertyAsClass(String key, Class<T> targetType);
@ -113,8 +114,9 @@ public interface PropertyResolver {
* no default value will cause an IllegalArgumentException to be thrown. * no default value will cause an IllegalArgumentException to be thrown.
* @return the resolved String (never {@code null}) * @return the resolved String (never {@code null})
* @throws IllegalArgumentException if given text is {@code null} * @throws IllegalArgumentException if given text is {@code null}
* @throws IllegalArgumentException if any placeholders are unresolvable * or if any placeholders are unresolvable
* @see org.springframework.util.SystemPropertyUtils#resolvePlaceholders(String, boolean) * @see org.springframework.util.SystemPropertyUtils#resolvePlaceholders(String, boolean)
*/ */
String resolveRequiredPlaceholders(String text) throws IllegalArgumentException; String resolveRequiredPlaceholders(String text) throws IllegalArgumentException;
} }

View File

@ -57,11 +57,20 @@ public class PropertySourcesPropertyResolver extends AbstractPropertyResolver {
@Override @Override
public String getProperty(String key) { public String getProperty(String key) {
return getProperty(key, String.class); return getProperty(key, String.class, true);
} }
@Override @Override
public <T> T getProperty(String key, Class<T> targetValueType) { public <T> T getProperty(String key, Class<T> targetValueType) {
return getProperty(key, targetValueType, true);
}
@Override
protected String getPropertyAsRawString(String key) {
return getProperty(key, String.class, false);
}
protected <T> T getProperty(String key, Class<T> targetValueType, boolean resolveNestedPlaceholders) {
boolean debugEnabled = logger.isDebugEnabled(); boolean debugEnabled = logger.isDebugEnabled();
if (logger.isTraceEnabled()) { if (logger.isTraceEnabled()) {
logger.trace(String.format("getProperty(\"%s\", %s)", key, targetValueType.getSimpleName())); logger.trace(String.format("getProperty(\"%s\", %s)", key, targetValueType.getSimpleName()));
@ -74,8 +83,8 @@ public class PropertySourcesPropertyResolver extends AbstractPropertyResolver {
Object value; Object value;
if ((value = propertySource.getProperty(key)) != null) { if ((value = propertySource.getProperty(key)) != null) {
Class<?> valueType = value.getClass(); Class<?> valueType = value.getClass();
if (String.class.equals(valueType)) { if (resolveNestedPlaceholders && value instanceof String) {
value = this.resolveNestedPlaceholders((String) value); value = resolveNestedPlaceholders((String) value);
} }
if (debugEnabled) { if (debugEnabled) {
logger.debug(String.format("Found key '%s' in [%s] with type [%s] and value '%s'", logger.debug(String.format("Found key '%s' in [%s] with type [%s] and value '%s'",
@ -86,7 +95,7 @@ public class PropertySourcesPropertyResolver extends AbstractPropertyResolver {
"Cannot convert value [%s] from source type [%s] to target type [%s]", "Cannot convert value [%s] from source type [%s] to target type [%s]",
value, valueType.getSimpleName(), targetValueType.getSimpleName())); value, valueType.getSimpleName(), targetValueType.getSimpleName()));
} }
return conversionService.convert(value, targetValueType); return this.conversionService.convert(value, targetValueType);
} }
} }
} }
@ -115,10 +124,10 @@ public class PropertySourcesPropertyResolver extends AbstractPropertyResolver {
Class<?> clazz; Class<?> clazz;
if (value instanceof String) { if (value instanceof String) {
try { try {
clazz = ClassUtils.forName((String)value, null); clazz = ClassUtils.forName((String) value, null);
} }
catch (Exception ex) { catch (Exception ex) {
throw new ClassConversionException((String)value, targetValueType, ex); throw new ClassConversionException((String) value, targetValueType, ex);
} }
} }
else if (value instanceof Class) { else if (value instanceof Class) {

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2012 the original author or authors. * Copyright 2002-2013 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -98,8 +98,8 @@ public class PropertyPlaceholderHelper {
/** /**
* Replaces all placeholders of format {@code ${name}} with the corresponding property * Replaces all placeholders of format {@code ${name}} with the corresponding
* from the supplied {@link Properties}. * property from the supplied {@link Properties}.
* @param value the value containing the placeholders to be replaced. * @param value the value containing the placeholders to be replaced.
* @param properties the {@code Properties} to use for replacement. * @param properties the {@code Properties} to use for replacement.
* @return the supplied value with placeholders replaced inline. * @return the supplied value with placeholders replaced inline.
@ -114,8 +114,8 @@ public class PropertyPlaceholderHelper {
} }
/** /**
* Replaces all placeholders of format {@code ${name}} with the value returned from the supplied * Replaces all placeholders of format {@code ${name}} with the value returned
* {@link PlaceholderResolver}. * from the supplied {@link PlaceholderResolver}.
* @param value the value containing the placeholders to be replaced. * @param value the value containing the placeholders to be replaced.
* @param placeholderResolver the {@code PlaceholderResolver} to use for replacement. * @param placeholderResolver the {@code PlaceholderResolver} to use for replacement.
* @return the supplied value with placeholders replaced inline. * @return the supplied value with placeholders replaced inline.
@ -216,8 +216,8 @@ public class PropertyPlaceholderHelper {
/** /**
* Resolves the supplied placeholder name into the replacement value. * Resolves the supplied placeholder name into the replacement value.
* @param placeholderName the name of the placeholder to resolve. * @param placeholderName the name of the placeholder to resolve
* @return the replacement value or {@code null} if no replacement is to be made. * @return the replacement value or {@code null} if no replacement is to be made
*/ */
String resolvePlaceholder(String placeholderName); String resolvePlaceholder(String placeholderName);
} }

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2012 the original author or authors. * Copyright 2002-2013 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -30,8 +30,6 @@ import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*; import static org.junit.Assert.*;
/** /**
* Unit tests for {@link PropertySourcesPropertyResolver}.
*
* @author Chris Beams * @author Chris Beams
* @since 3.1 * @since 3.1
*/ */
@ -387,8 +385,8 @@ public class PropertySourcesPropertyResolverTests {
try { try {
pr.getProperty("pL"); pr.getProperty("pL");
} }
catch (StackOverflowError ex) { catch (IllegalArgumentException ex) {
// no explicit handling for cyclic references for now assertTrue(ex.getMessage().toLowerCase().contains("circular"));
} }
} }