From 2ec7834124dfa32d7b90afbb23805433a4567bf5 Mon Sep 17 00:00:00 2001 From: Chris Beams Date: Thu, 5 Jul 2012 15:45:35 +0200 Subject: [PATCH] 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 --- .../env/PropertySourcesPropertyResolver.java | 3 ++ .../util/PropertyPlaceholderHelper.java | 3 +- .../PropertySourcesPropertyResolverTests.java | 47 +++++++++++++++---- 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/env/PropertySourcesPropertyResolver.java b/spring-core/src/main/java/org/springframework/core/env/PropertySourcesPropertyResolver.java index b390d4ddaba..cee4db51a6a 100644 --- a/spring-core/src/main/java/org/springframework/core/env/PropertySourcesPropertyResolver.java +++ b/spring-core/src/main/java/org/springframework/core/env/PropertySourcesPropertyResolver.java @@ -72,6 +72,9 @@ public class PropertySourcesPropertyResolver extends AbstractPropertyResolver { Object value; if ((value = propertySource.getProperty(key)) != null) { Class valueType = value.getClass(); + if (String.class.equals(valueType)) { + value = this.resolveRequiredPlaceholders((String) value); + } if (debugEnabled) { logger.debug( format("Found key '%s' in [%s] with type [%s] and value '%s'", diff --git a/spring-core/src/main/java/org/springframework/util/PropertyPlaceholderHelper.java b/spring-core/src/main/java/org/springframework/util/PropertyPlaceholderHelper.java index 5c769fae5ce..0bea53ea6a4 100644 --- a/spring-core/src/main/java/org/springframework/util/PropertyPlaceholderHelper.java +++ b/spring-core/src/main/java/org/springframework/util/PropertyPlaceholderHelper.java @@ -171,7 +171,8 @@ public class PropertyPlaceholderHelper { startIndex = buf.indexOf(this.placeholderPrefix, endIndex + this.placeholderSuffix.length()); } else { - throw new IllegalArgumentException("Could not resolve placeholder '" + placeholder + "'"); + throw new IllegalArgumentException("Could not resolve placeholder '" + + placeholder + "'" + " in string value [" + strVal + "]"); } visitedPlaceholders.remove(originalPlaceholder); diff --git a/spring-core/src/test/java/org/springframework/core/env/PropertySourcesPropertyResolverTests.java b/spring-core/src/test/java/org/springframework/core/env/PropertySourcesPropertyResolverTests.java index 4d02d12ef45..c51fd358c82 100644 --- a/spring-core/src/test/java/org/springframework/core/env/PropertySourcesPropertyResolverTests.java +++ b/spring-core/src/test/java/org/springframework/core/env/PropertySourcesPropertyResolverTests.java @@ -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"); * you may not use this file except in compliance with the License. @@ -16,22 +16,20 @@ 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.Map; import java.util.Properties; +import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Test; + import org.springframework.core.convert.ConversionException; import org.springframework.mock.env.MockPropertySource; +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + /** * Unit tests for {@link PropertySourcesPropertyResolver}. * @@ -352,6 +350,39 @@ public class PropertySourcesPropertyResolverTests { 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 class SpecificType implements SomeType { } }