Resolve nested placeholders via PropertyResolver

Prior to this change, PropertySourcesPropertyResolver (and therefore
all AbstractEnvironment) implementations failed to resolve nested
placeholders as in the following example:

    p1=v1
    p2=v2
    p3=${v1}:{$v2}

Calls to PropertySource#getProperty for keys 'p1' and 'v1' would
successfully return their respective values, but for 'p3' the return
value would be the unresolved placeholders. This behavior is
inconsistent with that of PropertyPlaceholderConfigurer.

PropertySourcesPropertyResolver #getProperty variants now resolve any
nested placeholders recursively, throwing IllegalArgumentException for
any unresolvable placeholders (as is the default behavior for
PropertyPlaceholderConfigurer). See SPR-9569 for an enhancement that
will intoduce an 'ignoreUnresolvablePlaceholders' switch to make this
behavior configurable.

This commit also improves error output in
PropertyPlaceholderHelper#parseStringValue by including the original
string in which an unresolvable placeholder was found.

Issue: SPR-9473, SPR-9569
This commit is contained in:
Chris Beams 2012-07-05 15:45:35 +02:00
parent b9786ccaca
commit 2ec7834124
3 changed files with 44 additions and 9 deletions

View File

@ -72,6 +72,9 @@ 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)) {
value = this.resolveRequiredPlaceholders((String) value);
}
if (debugEnabled) { if (debugEnabled) {
logger.debug( logger.debug(
format("Found key '%s' in [%s] with type [%s] and value '%s'", format("Found key '%s' in [%s] with type [%s] and value '%s'",

View File

@ -171,7 +171,8 @@ public class PropertyPlaceholderHelper {
startIndex = buf.indexOf(this.placeholderPrefix, endIndex + this.placeholderSuffix.length()); startIndex = buf.indexOf(this.placeholderPrefix, endIndex + this.placeholderSuffix.length());
} }
else { else {
throw new IllegalArgumentException("Could not resolve placeholder '" + placeholder + "'"); throw new IllegalArgumentException("Could not resolve placeholder '" +
placeholder + "'" + " in string value [" + strVal + "]");
} }
visitedPlaceholders.remove(originalPlaceholder); visitedPlaceholders.remove(originalPlaceholder);

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2002-2011 the original author or authors. * Copyright 2002-2012 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,22 +16,20 @@
package org.springframework.core.env; package org.springframework.core.env;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import java.util.HashMap; import java.util.HashMap;
import java.util.Map; import java.util.Map;
import java.util.Properties; import java.util.Properties;
import org.hamcrest.Matchers;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.springframework.core.convert.ConversionException; import org.springframework.core.convert.ConversionException;
import org.springframework.mock.env.MockPropertySource; import org.springframework.mock.env.MockPropertySource;
import static org.hamcrest.CoreMatchers.*;
import static org.junit.Assert.*;
/** /**
* Unit tests for {@link PropertySourcesPropertyResolver}. * Unit tests for {@link PropertySourcesPropertyResolver}.
* *
@ -352,6 +350,39 @@ public class PropertySourcesPropertyResolverTests {
propertyResolver.validateRequiredProperties(); propertyResolver.validateRequiredProperties();
} }
@Test
public void resolveNestedPropertyPlaceholders() {
MutablePropertySources ps = new MutablePropertySources();
ps.addFirst(new MockPropertySource()
.withProperty("p1", "v1")
.withProperty("p2", "v2")
.withProperty("p3", "${p1}:${p2}") // nested placeholders
.withProperty("p4", "${p3}") // deeply nested placeholders
.withProperty("p5", "${p1}:${p2}:${bogus}") // unresolvable placeholder
.withProperty("p6", "${p1}:${p2}:${bogus:def}") // unresolvable w/ default
.withProperty("pL", "${pR}") // cyclic reference left
.withProperty("pR", "${pL}") // cyclic reference right
);
PropertySourcesPropertyResolver pr = new PropertySourcesPropertyResolver(ps);
assertThat(pr.getProperty("p1"), equalTo("v1"));
assertThat(pr.getProperty("p2"), equalTo("v2"));
assertThat(pr.getProperty("p3"), equalTo("v1:v2"));
assertThat(pr.getProperty("p4"), equalTo("v1:v2"));
try {
pr.getProperty("p5");
} catch (IllegalArgumentException ex) {
assertThat(ex.getMessage(), Matchers.containsString(
"Could not resolve placeholder 'bogus' in string value [${p1}:${p2}:${bogus}]"));
}
assertThat(pr.getProperty("p6"), equalTo("v1:v2:def"));
try {
pr.getProperty("pL");
} catch (StackOverflowError ex) {
// no explicit handling for cyclic references for now
}
}
static interface SomeType { } static interface SomeType { }
static class SpecificType implements SomeType { } static class SpecificType implements SomeType { }
} }