From 15ba11f3022b057af5c9972ebeeef35ca4072926 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Thu, 11 Jul 2013 09:50:07 +0100 Subject: [PATCH] [bs-167] Fixed YamlProcessor to not create a key for an array [Fixes #51968679] YamlPropertiesFactoryBean creates unbindable keys (the toString() of the whole map for instance) --- .../bootstrap/config/YamlProcessor.java | 23 ++----- .../bootstrap/config/YamlProcessorTests.java | 65 +++++++++++++++++++ .../YamlPropertiesFactoryBeanTests.java | 3 - ...ileApplicationContextInitializerTests.java | 5 +- .../EnableConfigurationPropertiesTests.java | 17 ++++- .../src/test/resources/testyaml.yml | 1 + 6 files changed, 93 insertions(+), 21 deletions(-) create mode 100644 spring-bootstrap/src/test/java/org/springframework/bootstrap/config/YamlProcessorTests.java diff --git a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlProcessor.java b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlProcessor.java index 059fa3964c6..f62a77f7459 100644 --- a/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlProcessor.java +++ b/spring-bootstrap/src/main/java/org/springframework/bootstrap/config/YamlProcessor.java @@ -148,8 +148,7 @@ public class YamlProcessor { } this.logger.info("Loaded " + count + " document" + (count > 1 ? "s" : "") + " from YAML resource: " + resource); - } - catch (IOException ex) { + } catch (IOException ex) { handleProcessError(resource, ex); } return count > 0; @@ -177,8 +176,7 @@ public class YamlProcessor { if (this.documentMatchers.isEmpty()) { this.logger.debug("Merging document (no matchers set)" + map); callback.process(properties, map); - } - else { + } else { boolean valueFound = false; MatchStatus result = MatchStatus.ABSTAIN; for (DocumentMatcher matcher : this.documentMatchers) { @@ -196,8 +194,7 @@ public class YamlProcessor { if (result == MatchStatus.ABSTAIN && this.matchDefault) { this.logger.debug("Matched document with default matcher: " + map); callback.process(properties, map); - } - else if (!valueFound) { + } else if (!valueFound) { this.logger.debug("Unmatched document"); return false; } @@ -212,34 +209,28 @@ public class YamlProcessor { if (StringUtils.hasText(path)) { if (key.startsWith("[")) { key = path + key; - } - else { + } else { key = path + "." + key; } } Object value = entry.getValue(); if (value instanceof String) { properties.put(key, value); - } - else if (value instanceof Map) { + } else if (value instanceof Map) { // Need a compound key @SuppressWarnings("unchecked") Map map = (Map) value; assignProperties(properties, map, key); - } - else if (value instanceof Collection) { + } else if (value instanceof Collection) { // Need a compound key @SuppressWarnings("unchecked") Collection collection = (Collection) value; - properties.put(key, - StringUtils.collectionToCommaDelimitedString(collection)); int count = 0; for (Object object : collection) { assignProperties(properties, Collections.singletonMap("[" + (count++) + "]", object), key); } - } - else { + } else { properties.put(key, value == null ? "" : value); } } diff --git a/spring-bootstrap/src/test/java/org/springframework/bootstrap/config/YamlProcessorTests.java b/spring-bootstrap/src/test/java/org/springframework/bootstrap/config/YamlProcessorTests.java new file mode 100644 index 00000000000..cd3d8911a90 --- /dev/null +++ b/spring-bootstrap/src/test/java/org/springframework/bootstrap/config/YamlProcessorTests.java @@ -0,0 +1,65 @@ +/* + * Copyright 2012-2013 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 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.bootstrap.config; + +import java.util.Map; +import java.util.Properties; + +import org.junit.Test; +import org.springframework.bootstrap.config.YamlProcessor.MatchCallback; +import org.springframework.core.io.ByteArrayResource; +import org.springframework.core.io.Resource; + +import static org.junit.Assert.assertEquals; + +/** + * @author Dave Syer + * + */ +public class YamlProcessorTests { + + private YamlProcessor processor = new YamlProcessor(); + + @Test + public void arrayConvertedToIndexedBeanReference() { + this.processor.setResources(new Resource[] { new ByteArrayResource( + "foo: bar\nbar: [1,2,3]".getBytes()) }); + this.processor.process(new MatchCallback() { + @Override + public void process(Properties properties, Map map) { + assertEquals(1, properties.get("bar[0]")); + assertEquals(2, properties.get("bar[1]")); + assertEquals(3, properties.get("bar[2]")); + assertEquals(4, properties.size()); + } + }); + } + + @Test + public void mapConvertedToIndexedBeanReference() { + this.processor.setResources(new Resource[] { new ByteArrayResource( + "foo: bar\nbar:\n spam: bucket".getBytes()) }); + this.processor.process(new MatchCallback() { + @Override + public void process(Properties properties, Map map) { + // System.err.println(properties); + assertEquals("bucket", properties.get("bar.spam")); + assertEquals(2, properties.size()); + } + }); + } + +} diff --git a/spring-bootstrap/src/test/java/org/springframework/bootstrap/config/YamlPropertiesFactoryBeanTests.java b/spring-bootstrap/src/test/java/org/springframework/bootstrap/config/YamlPropertiesFactoryBeanTests.java index 6b6815037a2..07cb50d48b1 100644 --- a/spring-bootstrap/src/test/java/org/springframework/bootstrap/config/YamlPropertiesFactoryBeanTests.java +++ b/spring-bootstrap/src/test/java/org/springframework/bootstrap/config/YamlPropertiesFactoryBeanTests.java @@ -22,7 +22,6 @@ import java.util.Properties; import org.junit.Ignore; import org.junit.Test; -import org.springframework.bootstrap.config.YamlPropertiesFactoryBean; import org.springframework.bootstrap.config.YamlProcessor.DocumentMatcher; import org.springframework.bootstrap.config.YamlProcessor.MatchStatus; import org.springframework.bootstrap.config.YamlProcessor.ResolutionMethod; @@ -185,7 +184,6 @@ public class YamlPropertiesFactoryBeanTests { Properties properties = factory.getObject(); assertEquals("bar", properties.get("foo[0]")); assertEquals("baz", properties.get("foo[1]")); - assertEquals("bar,baz", properties.get("foo")); } @Test @@ -200,7 +198,6 @@ public class YamlPropertiesFactoryBeanTests { assertEquals("baz", properties.get("foo[1]")); assertEquals("two", properties.get("foo[2].one")); assertEquals("four", properties.get("foo[2].three")); - assertEquals("{bar={spam=crap}},baz,{one=two, three=four}", properties.get("foo")); } @SuppressWarnings("unchecked") diff --git a/spring-bootstrap/src/test/java/org/springframework/bootstrap/context/initializer/ConfigFileApplicationContextInitializerTests.java b/spring-bootstrap/src/test/java/org/springframework/bootstrap/context/initializer/ConfigFileApplicationContextInitializerTests.java index c80d70bec5e..3d3cde6be4d 100644 --- a/spring-bootstrap/src/test/java/org/springframework/bootstrap/context/initializer/ConfigFileApplicationContextInitializerTests.java +++ b/spring-bootstrap/src/test/java/org/springframework/bootstrap/context/initializer/ConfigFileApplicationContextInitializerTests.java @@ -20,12 +20,12 @@ import java.util.HashMap; import java.util.Map; import org.junit.Test; -import org.springframework.bootstrap.context.initializer.ConfigFileApplicationContextInitializer; import org.springframework.context.support.StaticApplicationContext; import org.springframework.core.env.MapPropertySource; import org.springframework.core.env.SimpleCommandLinePropertySource; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertThat; /** @@ -54,6 +54,9 @@ public class ConfigFileApplicationContextInitializerTests { this.initializer.initialize(this.context); String property = this.context.getEnvironment().getProperty("my.property"); assertThat(property, equalTo("fromyamlfile")); + assertThat(this.context.getEnvironment().getProperty("my.array[0]"), equalTo("1")); + assertThat(this.context.getEnvironment().getProperty("my.array"), + nullValue(String.class)); } @Test diff --git a/spring-bootstrap/src/test/java/org/springframework/bootstrap/context/properties/EnableConfigurationPropertiesTests.java b/spring-bootstrap/src/test/java/org/springframework/bootstrap/context/properties/EnableConfigurationPropertiesTests.java index 738f3f9ab09..49f29e10bee 100644 --- a/spring-bootstrap/src/test/java/org/springframework/bootstrap/context/properties/EnableConfigurationPropertiesTests.java +++ b/spring-bootstrap/src/test/java/org/springframework/bootstrap/context/properties/EnableConfigurationPropertiesTests.java @@ -16,7 +16,9 @@ package org.springframework.bootstrap.context.properties; +import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import javax.annotation.PostConstruct; @@ -62,6 +64,14 @@ public class EnableConfigurationPropertiesTests { assertEquals(3, this.context.getBean(TestProperties.class).getArray().length); } + @Test + public void testCollectionPropertiesBindingFromYamlArray() { + this.context.register(TestConfiguration.class); + TestUtils.addEnviroment(this.context, "name:foo", "list[0]:1", "list[1]:2"); + this.context.refresh(); + assertEquals(2, this.context.getBean(TestProperties.class).getList().size()); + } + @Test public void testPropertiesBindingWithoutAnnotation() { this.context.register(MoreConfiguration.class); @@ -100,7 +110,7 @@ public class EnableConfigurationPropertiesTests { // Maybe we could relax the condition that causes this exception but Spring makes it // difficult because it is impossible for DefaultConfiguration to override a bean - // definition created with a direct regsistration (as opposed to a @Bean) + // definition created with a direct registration (as opposed to a @Bean) @Test(expected = BeanCreationException.class) public void testPropertiesBindingWithDefaults() { this.context.register(TestConfiguration.class, DefaultConfiguration.class); @@ -201,6 +211,7 @@ public class EnableConfigurationPropertiesTests { protected static class TestProperties { private String name; private int[] array; + private List list = new ArrayList(); public String getName() { return this.name; @@ -217,6 +228,10 @@ public class EnableConfigurationPropertiesTests { public int[] getArray() { return this.array; } + + public List getList() { + return this.list; + } } protected static class MoreProperties { diff --git a/spring-bootstrap/src/test/resources/testyaml.yml b/spring-bootstrap/src/test/resources/testyaml.yml index 37811ec747e..ffce1bd3092 100644 --- a/spring-bootstrap/src/test/resources/testyaml.yml +++ b/spring-bootstrap/src/test/resources/testyaml.yml @@ -1,3 +1,4 @@ --- my: property: fromyamlfile + array: [1,2,3]