From 484c72cd1949811c4d531488cafd6d81aec706e1 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Mon, 6 Mar 2017 16:43:16 -0800 Subject: [PATCH] Add OriginTrackedYamlLoader Update the YAML parser so that origin information can be tracked. Line and column numbers are now available for each loaded property value. Fixes gh-8142 --- .../boot/env/OriginTrackedYamlLoader.java | 153 ++++++++++++++++++ .../boot/env/YamlPropertySourceLoader.java | 65 +------- .../yaml/SpringProfileDocumentMatcher.java | 30 ++-- .../env/OriginTrackedYamlLoaderTests.java | 110 +++++++++++++ .../env/YamlPropertySourceLoaderTests.java | 12 ++ .../springframework/boot/env/test-yaml.yml | 27 ++++ 6 files changed, 320 insertions(+), 77 deletions(-) create mode 100644 spring-boot/src/main/java/org/springframework/boot/env/OriginTrackedYamlLoader.java create mode 100644 spring-boot/src/test/java/org/springframework/boot/env/OriginTrackedYamlLoaderTests.java create mode 100644 spring-boot/src/test/resources/org/springframework/boot/env/test-yaml.yml diff --git a/spring-boot/src/main/java/org/springframework/boot/env/OriginTrackedYamlLoader.java b/spring-boot/src/main/java/org/springframework/boot/env/OriginTrackedYamlLoader.java new file mode 100644 index 0000000000..8ca5e35078 --- /dev/null +++ b/spring-boot/src/main/java/org/springframework/boot/env/OriginTrackedYamlLoader.java @@ -0,0 +1,153 @@ +/* + * Copyright 2012-2017 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.boot.env; + +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Properties; +import java.util.regex.Pattern; + +import org.yaml.snakeyaml.DumperOptions; +import org.yaml.snakeyaml.Yaml; +import org.yaml.snakeyaml.constructor.BaseConstructor; +import org.yaml.snakeyaml.constructor.Constructor; +import org.yaml.snakeyaml.error.Mark; +import org.yaml.snakeyaml.nodes.Node; +import org.yaml.snakeyaml.nodes.ScalarNode; +import org.yaml.snakeyaml.nodes.Tag; +import org.yaml.snakeyaml.representer.Representer; +import org.yaml.snakeyaml.resolver.Resolver; + +import org.springframework.beans.factory.config.YamlProcessor; +import org.springframework.boot.env.TextResourcePropertyOrigin.Location; +import org.springframework.boot.yaml.SpringProfileDocumentMatcher; +import org.springframework.core.io.Resource; + +/** + * Class to load {@code .yml} files into a map of {@code String} -> + * {@link OriginTrackedValue}. + * + * @author Madhura Bhave + * @author Phillip Webb + */ +class OriginTrackedYamlLoader extends YamlProcessor { + + private final Resource resource; + + OriginTrackedYamlLoader(Resource resource, String profile) { + this.resource = resource; + if (profile == null) { + setMatchDefault(true); + setDocumentMatchers(new OriginTrackedSpringProfileDocumentMatcher()); + } + else { + setMatchDefault(false); + setDocumentMatchers(new OriginTrackedSpringProfileDocumentMatcher(profile)); + } + setResources(resource); + } + + @Override + protected Yaml createYaml() { + BaseConstructor constructor = new OriginTrackingConstructor(); + Representer representer = new Representer(); + DumperOptions dumperOptions = new DumperOptions(); + LimitedResolver resolver = new LimitedResolver(); + return new Yaml(constructor, representer, dumperOptions, resolver); + } + + public Map load() { + final Map result = new LinkedHashMap(); + process(new MatchCallback() { + + @Override + public void process(Properties properties, Map map) { + result.putAll(getFlattenedMap(map)); + } + + }); + return result; + } + + /** + * {@link Constructor}. + */ + private class OriginTrackingConstructor extends StrictMapAppenderConstructor { + + @Override + protected Object constructObject(Node node) { + if (node instanceof ScalarNode) { + return constructTrackedObject(node, super.constructObject(node)); + } + return super.constructObject(node); + } + + private Object constructTrackedObject(Node node, Object value) { + PropertyOrigin origin = getOrigin(node); + return OriginTrackedValue.of(value, origin); + } + + private PropertyOrigin getOrigin(Node node) { + Mark mark = node.getStartMark(); + Location location = new Location(mark.getLine(), mark.getColumn()); + return new TextResourcePropertyOrigin(OriginTrackedYamlLoader.this.resource, + location); + } + + } + + /** + * {@link Resolver} that limits {@link Tag#TIMESTAMP} tags. + */ + private static class LimitedResolver extends Resolver { + + @Override + public void addImplicitResolver(Tag tag, Pattern regexp, String first) { + if (tag == Tag.TIMESTAMP) { + return; + } + super.addImplicitResolver(tag, regexp, first); + } + + } + + private static class OriginTrackedSpringProfileDocumentMatcher + extends SpringProfileDocumentMatcher { + + OriginTrackedSpringProfileDocumentMatcher(String... profiles) { + super(profiles); + } + + @Override + protected List extractSpringProfiles(Properties properties) { + Properties springProperties = new Properties(); + for (Map.Entry entry : properties.entrySet()) { + if (String.valueOf(entry.getKey()).startsWith("spring.")) { + Object value = entry.getValue(); + if (value instanceof OriginTrackedValue) { + value = ((OriginTrackedValue) value).getValue(); + } + springProperties.put(entry.getKey(), value); + } + } + return super.extractSpringProfiles(springProperties); + } + + } + +} diff --git a/spring-boot/src/main/java/org/springframework/boot/env/YamlPropertySourceLoader.java b/spring-boot/src/main/java/org/springframework/boot/env/YamlPropertySourceLoader.java index 8573108fa3..28db8f3ca6 100755 --- a/spring-boot/src/main/java/org/springframework/boot/env/YamlPropertySourceLoader.java +++ b/spring-boot/src/main/java/org/springframework/boot/env/YamlPropertySourceLoader.java @@ -17,21 +17,8 @@ package org.springframework.boot.env; import java.io.IOException; -import java.util.LinkedHashMap; import java.util.Map; -import java.util.Properties; -import java.util.regex.Pattern; -import org.yaml.snakeyaml.DumperOptions; -import org.yaml.snakeyaml.Yaml; -import org.yaml.snakeyaml.nodes.Tag; -import org.yaml.snakeyaml.representer.Representer; -import org.yaml.snakeyaml.resolver.Resolver; - -import org.springframework.beans.factory.config.YamlProcessor; -import org.springframework.beans.factory.config.YamlPropertiesFactoryBean; -import org.springframework.boot.yaml.SpringProfileDocumentMatcher; -import org.springframework.core.env.MapPropertySource; import org.springframework.core.env.PropertySource; import org.springframework.core.io.Resource; import org.springframework.util.ClassUtils; @@ -54,59 +41,13 @@ public class YamlPropertySourceLoader implements PropertySourceLoader { public PropertySource load(String name, Resource resource, String profile) throws IOException { if (ClassUtils.isPresent("org.yaml.snakeyaml.Yaml", null)) { - Processor processor = new Processor(resource, profile); - Map source = processor.process(); + Map source = new OriginTrackedYamlLoader(resource, profile) + .load(); if (!source.isEmpty()) { - return new MapPropertySource(name, source); + return new OriginTrackedMapPropertySource(name, source); } } return null; } - /** - * {@link YamlProcessor} to create a {@link Map} containing the property values. - * Similar to {@link YamlPropertiesFactoryBean} but retains the order of entries. - */ - private static class Processor extends YamlProcessor { - - Processor(Resource resource, String profile) { - if (profile == null) { - setMatchDefault(true); - setDocumentMatchers(new SpringProfileDocumentMatcher()); - } - else { - setMatchDefault(false); - setDocumentMatchers(new SpringProfileDocumentMatcher(profile)); - } - setResources(resource); - } - - @Override - protected Yaml createYaml() { - return new Yaml(new StrictMapAppenderConstructor(), new Representer(), - new DumperOptions(), new Resolver() { - @Override - public void addImplicitResolver(Tag tag, Pattern regexp, - String first) { - if (tag == Tag.TIMESTAMP) { - return; - } - super.addImplicitResolver(tag, regexp, first); - } - }); - } - - public Map process() { - final Map result = new LinkedHashMap<>(); - process(new MatchCallback() { - @Override - public void process(Properties properties, Map map) { - result.putAll(getFlattenedMap(map)); - } - }); - return result; - } - - } - } diff --git a/spring-boot/src/main/java/org/springframework/boot/yaml/SpringProfileDocumentMatcher.java b/spring-boot/src/main/java/org/springframework/boot/yaml/SpringProfileDocumentMatcher.java index 26340bdba4..b197ef033e 100644 --- a/spring-boot/src/main/java/org/springframework/boot/yaml/SpringProfileDocumentMatcher.java +++ b/spring-boot/src/main/java/org/springframework/boot/yaml/SpringProfileDocumentMatcher.java @@ -52,9 +52,6 @@ public class SpringProfileDocumentMatcher implements DocumentMatcher { private String[] activeProfiles = new String[0]; - public SpringProfileDocumentMatcher() { - } - public SpringProfileDocumentMatcher(String... profiles) { addActiveProfiles(profiles); } @@ -68,7 +65,21 @@ public class SpringProfileDocumentMatcher implements DocumentMatcher { @Override public MatchStatus matches(Properties properties) { - List profiles = extractSpringProfiles(properties); + return matches(extractSpringProfiles(properties)); + } + + protected List extractSpringProfiles(Properties properties) { + SpringProperties springProperties = new SpringProperties(); + MutablePropertySources propertySources = new MutablePropertySources(); + propertySources.addFirst(new PropertiesPropertySource("profiles", properties)); + PropertyValues propertyValues = new PropertySourcesPropertyValues( + propertySources); + new RelaxedDataBinder(springProperties, "spring").bind(propertyValues); + List profiles = springProperties.getProfiles(); + return profiles; + } + + private MatchStatus matches(List profiles) { ProfilesMatcher profilesMatcher = getProfilesMatcher(); Set negative = extractProfiles(profiles, ProfileType.NEGATIVE); Set positive = extractProfiles(profiles, ProfileType.POSITIVE); @@ -83,17 +94,6 @@ public class SpringProfileDocumentMatcher implements DocumentMatcher { return profilesMatcher.matches(positive); } - private List extractSpringProfiles(Properties properties) { - SpringProperties springProperties = new SpringProperties(); - MutablePropertySources propertySources = new MutablePropertySources(); - propertySources.addFirst(new PropertiesPropertySource("profiles", properties)); - PropertyValues propertyValues = new PropertySourcesPropertyValues( - propertySources); - new RelaxedDataBinder(springProperties, "spring").bind(propertyValues); - List profiles = springProperties.getProfiles(); - return profiles; - } - private ProfilesMatcher getProfilesMatcher() { return this.activeProfiles.length == 0 ? new EmptyProfilesMatcher() : new ActiveProfilesMatcher( diff --git a/spring-boot/src/test/java/org/springframework/boot/env/OriginTrackedYamlLoaderTests.java b/spring-boot/src/test/java/org/springframework/boot/env/OriginTrackedYamlLoaderTests.java new file mode 100644 index 0000000000..1d0bedf0a8 --- /dev/null +++ b/spring-boot/src/test/java/org/springframework/boot/env/OriginTrackedYamlLoaderTests.java @@ -0,0 +1,110 @@ +/* + * Copyright 2012-2017 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.boot.env; + +import java.util.Map; + +import org.junit.Before; +import org.junit.Test; + +import org.springframework.core.io.ClassPathResource; +import org.springframework.core.io.Resource; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link OriginTrackedYamlLoader}. + * + * @author Madhura Bhave + * @author Phillip Webb + */ +public class OriginTrackedYamlLoaderTests { + + private OriginTrackedYamlLoader loader; + + private Map result; + + @Before + public void setUp() throws Exception { + Resource resource = new ClassPathResource("test-yaml.yml", getClass()); + this.loader = new OriginTrackedYamlLoader(resource, null); + } + + @Test + public void processSimpleKey() throws Exception { + OriginTrackedValue value = getValue("name"); + assertThat(value.toString()).isEqualTo("Martin D'vloper"); + assertThat(getLocation(value)).isEqualTo("3:7"); + } + + @Test + public void processMap() throws Exception { + OriginTrackedValue perl = getValue("languages.perl"); + OriginTrackedValue python = getValue("languages.python"); + OriginTrackedValue pascal = getValue("languages.pascal"); + assertThat(perl.toString()).isEqualTo("Elite"); + assertThat(getLocation(perl)).isEqualTo("13:11"); + assertThat(python.toString()).isEqualTo("Elite"); + assertThat(getLocation(python)).isEqualTo("14:13"); + assertThat(pascal.toString()).isEqualTo("Lame"); + assertThat(getLocation(pascal)).isEqualTo("15:13"); + } + + @Test + public void processCollection() throws Exception { + OriginTrackedValue apple = getValue("foods[0]"); + OriginTrackedValue orange = getValue("foods[1]"); + OriginTrackedValue strawberry = getValue("foods[2]"); + OriginTrackedValue mango = getValue("foods[3]"); + assertThat(apple.toString()).isEqualTo("Apple"); + assertThat(getLocation(apple)).isEqualTo("8:7"); + assertThat(orange.toString()).isEqualTo("Orange"); + assertThat(getLocation(orange)).isEqualTo("9:7"); + assertThat(strawberry.toString()).isEqualTo("Strawberry"); + assertThat(getLocation(strawberry)).isEqualTo("10:7"); + assertThat(mango.toString()).isEqualTo("Mango"); + assertThat(getLocation(mango)).isEqualTo("11:7"); + } + + @Test + public void processMultiline() throws Exception { + OriginTrackedValue education = getValue("education"); + assertThat(education.toString()) + .isEqualTo("4 GCSEs\n3 A-Levels\nBSc in the Internet of Things\n"); + assertThat(getLocation(education)).isEqualTo("16:12"); + } + + @Test + public void processWithActiveProfile() throws Exception { + Resource resource = new ClassPathResource("test-yaml.yml", getClass()); + this.loader = new OriginTrackedYamlLoader(resource, "development"); + Map result = this.loader.load(); + assertThat(result.get("name").toString()).isEqualTo("Test Name"); + } + + private OriginTrackedValue getValue(String name) { + if (this.result == null) { + this.result = this.loader.load(); + } + return (OriginTrackedValue) this.result.get(name); + } + + private String getLocation(OriginTrackedValue value) { + return ((TextResourcePropertyOrigin) value.getOrigin()).getLocation().toString(); + } + +} diff --git a/spring-boot/src/test/java/org/springframework/boot/env/YamlPropertySourceLoaderTests.java b/spring-boot/src/test/java/org/springframework/boot/env/YamlPropertySourceLoaderTests.java index fa824a1770..b4f6e34012 100644 --- a/spring-boot/src/test/java/org/springframework/boot/env/YamlPropertySourceLoaderTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/env/YamlPropertySourceLoaderTests.java @@ -24,6 +24,8 @@ import org.junit.Test; import org.springframework.core.env.EnumerablePropertySource; import org.springframework.core.env.PropertySource; import org.springframework.core.io.ByteArrayResource; +import org.springframework.core.io.ClassPathResource; +import org.springframework.core.io.Resource; import static org.assertj.core.api.Assertions.assertThat; @@ -84,4 +86,14 @@ public class YamlPropertySourceLoaderTests { assertThat(source.getProperty("foo")).isEqualTo("2015-01-28"); } + @Test + public void loadOriginAware() throws Exception { + Resource resource = new ClassPathResource("test-yaml.yml", getClass()); + PropertySource source = this.loader.load("resource", resource, null); + EnumerablePropertySource enumerableSource = (EnumerablePropertySource) source; + for (String name : enumerableSource.getPropertyNames()) { + System.out.println(name + " = " + enumerableSource.getProperty(name)); + } + } + } diff --git a/spring-boot/src/test/resources/org/springframework/boot/env/test-yaml.yml b/spring-boot/src/test/resources/org/springframework/boot/env/test-yaml.yml new file mode 100644 index 0000000000..0adebf9261 --- /dev/null +++ b/spring-boot/src/test/resources/org/springframework/boot/env/test-yaml.yml @@ -0,0 +1,27 @@ +# http://docs.ansible.com/ansible/YAMLSyntax.html + +name: Martin D'vloper +job: Developer +skill: Elite +employed: True +foods: + - Apple + - Orange + - Strawberry + - Mango +languages: + perl: Elite + python: Elite + pascal: Lame +education: | + 4 GCSEs + 3 A-Levels + BSc in the Internet of Things +--- + +spring: + profiles: development +name: Test Name + +--- +