From 00a69394e69a7a2944b2fd1d611faa958cf42f37 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 --- org.springframework.core/.classpath | 1 + org.springframework.core/ivy.xml | 1 + .../env/PropertySourcesPropertyResolver.java | 7 ++- .../util/PropertyPlaceholderHelper.java | 5 +- .../PropertySourcesPropertyResolverTests.java | 47 +++++++++++++++---- 5 files changed, 49 insertions(+), 12 deletions(-) diff --git a/org.springframework.core/.classpath b/org.springframework.core/.classpath index dc1edd94f2..72121a3711 100644 --- a/org.springframework.core/.classpath +++ b/org.springframework.core/.classpath @@ -11,6 +11,7 @@ + diff --git a/org.springframework.core/ivy.xml b/org.springframework.core/ivy.xml index 0538fed5e3..a092dae401 100644 --- a/org.springframework.core/ivy.xml +++ b/org.springframework.core/ivy.xml @@ -32,6 +32,7 @@ + diff --git a/org.springframework.core/src/main/java/org/springframework/core/env/PropertySourcesPropertyResolver.java b/org.springframework.core/src/main/java/org/springframework/core/env/PropertySourcesPropertyResolver.java index 333056f3cb..ceb60f1efd 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/env/PropertySourcesPropertyResolver.java +++ b/org.springframework.core/src/main/java/org/springframework/core/env/PropertySourcesPropertyResolver.java @@ -1,11 +1,11 @@ /* - * 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. * You may obtain a copy of the License at * - * http://www.apache.org/licenses/LICENSE-2.0 + * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, @@ -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/org.springframework.core/src/main/java/org/springframework/util/PropertyPlaceholderHelper.java b/org.springframework.core/src/main/java/org/springframework/util/PropertyPlaceholderHelper.java index 4b1b2c37b1..6598f7ce8c 100644 --- a/org.springframework.core/src/main/java/org/springframework/util/PropertyPlaceholderHelper.java +++ b/org.springframework.core/src/main/java/org/springframework/util/PropertyPlaceholderHelper.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2010 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. @@ -170,7 +170,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(placeholder); diff --git a/org.springframework.core/src/test/java/org/springframework/core/env/PropertySourcesPropertyResolverTests.java b/org.springframework.core/src/test/java/org/springframework/core/env/PropertySourcesPropertyResolverTests.java index 4d02d12ef4..c51fd358c8 100644 --- a/org.springframework.core/src/test/java/org/springframework/core/env/PropertySourcesPropertyResolverTests.java +++ b/org.springframework.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 { } }