From b3220985bb8f639f4d5eaf0793c22deff222fbd8 Mon Sep 17 00:00:00 2001 From: Hrishikesh Joshi Date: Sat, 26 Nov 2016 23:31:30 +1100 Subject: [PATCH 1/2] Detect default values defined in the parent Closes gh-5434 --- ...figurationMetadataAnnotationProcessor.java | 14 +---- .../TypeElementMembers.java | 33 +++++++++++- ...ationMetadataAnnotationProcessorTests.java | 12 +++++ .../simple/ClassWithNestedProperties.java | 54 +++++++++++++++++++ 4 files changed, 100 insertions(+), 13 deletions(-) create mode 100644 spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/simple/ClassWithNestedProperties.java diff --git a/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java b/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java index efb848a7aa..b824ef20ae 100644 --- a/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java +++ b/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java @@ -20,7 +20,6 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.PrintWriter; import java.io.StringWriter; -import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; @@ -192,23 +191,14 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor private void processTypeElement(String prefix, TypeElement element, ExecutableElement source) { - TypeElementMembers members = new TypeElementMembers(this.processingEnv, element); - Map fieldValues = getFieldValues(element); + TypeElementMembers members = new TypeElementMembers(this.processingEnv, this.fieldValuesParser, element); + Map fieldValues = members.getFieldValues(); processSimpleTypes(prefix, element, source, members, fieldValues); processSimpleLombokTypes(prefix, element, source, members, fieldValues); processNestedTypes(prefix, element, source, members); processNestedLombokTypes(prefix, element, source, members); } - private Map getFieldValues(TypeElement element) { - try { - return this.fieldValuesParser.getFieldValues(element); - } - catch (Exception ex) { - return Collections.emptyMap(); - } - } - private void processSimpleTypes(String prefix, TypeElement element, ExecutableElement source, TypeElementMembers members, Map fieldValues) { diff --git a/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeElementMembers.java b/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeElementMembers.java index 30811ea847..466d356ca7 100644 --- a/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeElementMembers.java +++ b/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeElementMembers.java @@ -31,6 +31,9 @@ import javax.lang.model.element.VariableElement; import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; import javax.lang.model.util.ElementFilter; +import javax.tools.Diagnostic; + +import org.springframework.boot.configurationprocessor.fieldvalues.FieldValuesParser; /** * Provides access to relevant {@link TypeElement} members. @@ -53,10 +56,16 @@ class TypeElementMembers { private final Map> publicSetters = new LinkedHashMap>(); - TypeElementMembers(ProcessingEnvironment env, TypeElement element) { + private final Map fieldValues = new LinkedHashMap(); + + private final FieldValuesParser fieldValuesParser; + + TypeElementMembers(ProcessingEnvironment env, FieldValuesParser fieldValuesParser, TypeElement element) { this.env = env; this.typeUtils = new TypeUtils(this.env); + this.fieldValuesParser = fieldValuesParser; process(element); + processFieldValues(element); } private void process(TypeElement element) { @@ -165,4 +174,26 @@ class TypeElementMembers { return null; } + private void processFieldValues(TypeElement element) { + try { + this.fieldValues.putAll(this.fieldValuesParser.getFieldValues(element)); + } + catch (Exception ex) { + logWarning("Could not get values for type :" + element.getSimpleName().toString()); + } + + Element superType = this.env.getTypeUtils().asElement(element.getSuperclass()); + if (superType != null && superType instanceof TypeElement && !Object.class.getName().equals(superType.toString())) { + processFieldValues((TypeElement) superType); + } + } + + private void logWarning(String message) { + this.env.getMessager().printMessage(Diagnostic.Kind.WARNING, message); + } + + public Map getFieldValues() { + return Collections.unmodifiableMap(this.fieldValues); + } + } diff --git a/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java b/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java index c9a3033a6e..eef11d0283 100644 --- a/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java +++ b/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java @@ -47,6 +47,7 @@ import org.springframework.boot.configurationsample.method.EmptyTypeMethodConfig import org.springframework.boot.configurationsample.method.InvalidMethodConfig; import org.springframework.boot.configurationsample.method.MethodAndClassConfig; import org.springframework.boot.configurationsample.method.SimpleMethodConfig; +import org.springframework.boot.configurationsample.simple.ClassWithNestedProperties; import org.springframework.boot.configurationsample.simple.DeprecatedSingleProperty; import org.springframework.boot.configurationsample.simple.HierarchicalProperties; import org.springframework.boot.configurationsample.simple.NotAnnotated; @@ -767,6 +768,17 @@ public class ConfigurationMetadataAnnotationProcessorTests { } } + @Test + public void nestedClassChildProperties() throws Exception { + ConfigurationMetadata metadata = compile(ClassWithNestedProperties.class); + assertThat(metadata).has(Metadata.withGroup("nestedChildProps") + .fromSource(ClassWithNestedProperties.NestedChildClass.class)); + assertThat(metadata).has(Metadata.withProperty("nestedChildProps.child-class-property", Integer.class) + .fromSource(ClassWithNestedProperties.NestedChildClass.class).withDefaultValue(20)); + assertThat(metadata).has(Metadata.withProperty("nestedChildProps.parent-class-property", Integer.class) + .fromSource(ClassWithNestedProperties.NestedChildClass.class).withDefaultValue(10)); + } + private static class AdditionalMetadata { } diff --git a/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/simple/ClassWithNestedProperties.java b/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/simple/ClassWithNestedProperties.java new file mode 100644 index 0000000000..80aaa6c2f2 --- /dev/null +++ b/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/simple/ClassWithNestedProperties.java @@ -0,0 +1,54 @@ +/* + * Copyright 2012-2014 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.configurationsample.simple; + +import org.springframework.boot.configurationsample.ConfigurationProperties; + +/** + * Class with nested configuration properties. + * + * @author Hrishikesh Joshi + */ +public class ClassWithNestedProperties { + + public static class NestedParentClass { + + private int parentClassProperty = 10; + + public int getParentClassProperty() { + return this.parentClassProperty; + } + + public void setParentClassProperty(int parentClassProperty) { + this.parentClassProperty = parentClassProperty; + } + } + + @ConfigurationProperties(prefix = "nestedChildProps") + public static class NestedChildClass extends NestedParentClass { + + private int childClassProperty = 20; + + public int getChildClassProperty() { + return this.childClassProperty; + } + + public void setChildClassProperty(int childClassProperty) { + this.childClassProperty = childClassProperty; + } + } +} From f84fd19eaae7af4bde2cdacb99722a6f4faf251b Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Tue, 29 Nov 2016 15:19:54 +0100 Subject: [PATCH 2/2] Polish contribution Closes gh-7491 --- ...figurationMetadataAnnotationProcessor.java | 3 +- .../TypeElementMembers.java | 33 ++++++++----------- ...ationMetadataAnnotationProcessorTests.java | 22 ++++++------- 3 files changed, 26 insertions(+), 32 deletions(-) diff --git a/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java b/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java index b824ef20ae..8ee56e1d78 100644 --- a/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java +++ b/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessor.java @@ -191,7 +191,8 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor private void processTypeElement(String prefix, TypeElement element, ExecutableElement source) { - TypeElementMembers members = new TypeElementMembers(this.processingEnv, this.fieldValuesParser, element); + TypeElementMembers members = new TypeElementMembers(this.processingEnv, + this.fieldValuesParser, element); Map fieldValues = members.getFieldValues(); processSimpleTypes(prefix, element, source, members, fieldValues); processSimpleLombokTypes(prefix, element, source, members, fieldValues); diff --git a/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeElementMembers.java b/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeElementMembers.java index 466d356ca7..7d6c662f10 100644 --- a/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeElementMembers.java +++ b/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeElementMembers.java @@ -31,7 +31,6 @@ import javax.lang.model.element.VariableElement; import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; import javax.lang.model.util.ElementFilter; -import javax.tools.Diagnostic; import org.springframework.boot.configurationprocessor.fieldvalues.FieldValuesParser; @@ -65,7 +64,6 @@ class TypeElementMembers { this.typeUtils = new TypeUtils(this.env); this.fieldValuesParser = fieldValuesParser; process(element); - processFieldValues(element); } private void process(TypeElement element) { @@ -77,6 +75,19 @@ class TypeElementMembers { .fieldsIn(element.getEnclosedElements())) { processField(field); } + try { + Map fieldValues = this.fieldValuesParser + .getFieldValues(element); + for (Map.Entry entry : fieldValues.entrySet()) { + if (!this.fieldValues.containsKey(entry.getKey())) { + this.fieldValues.put(entry.getKey(), entry.getValue()); + } + } + } + catch (Exception ex) { + // continue + } + Element superType = this.env.getTypeUtils().asElement(element.getSuperclass()); if (superType != null && superType instanceof TypeElement && !OBJECT_CLASS_NAME.equals(superType.toString())) { @@ -174,24 +185,6 @@ class TypeElementMembers { return null; } - private void processFieldValues(TypeElement element) { - try { - this.fieldValues.putAll(this.fieldValuesParser.getFieldValues(element)); - } - catch (Exception ex) { - logWarning("Could not get values for type :" + element.getSimpleName().toString()); - } - - Element superType = this.env.getTypeUtils().asElement(element.getSuperclass()); - if (superType != null && superType instanceof TypeElement && !Object.class.getName().equals(superType.toString())) { - processFieldValues((TypeElement) superType); - } - } - - private void logWarning(String message) { - this.env.getMessager().printMessage(Diagnostic.Kind.WARNING, message); - } - public Map getFieldValues() { return Collections.unmodifiableMap(this.fieldValues); } diff --git a/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java b/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java index eef11d0283..bf3ffba589 100644 --- a/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java +++ b/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java @@ -352,6 +352,17 @@ public class ConfigurationMetadataAnnotationProcessorTests { assertThat(metadata).isNotEqualTo(Metadata.withProperty("specific.foo")); } + @Test + public void nestedClassChildProperties() throws Exception { + ConfigurationMetadata metadata = compile(ClassWithNestedProperties.class); + assertThat(metadata).has(Metadata.withGroup("nestedChildProps") + .fromSource(ClassWithNestedProperties.NestedChildClass.class)); + assertThat(metadata).has(Metadata.withProperty("nestedChildProps.child-class-property", Integer.class) + .fromSource(ClassWithNestedProperties.NestedChildClass.class).withDefaultValue(20)); + assertThat(metadata).has(Metadata.withProperty("nestedChildProps.parent-class-property", Integer.class) + .fromSource(ClassWithNestedProperties.NestedChildClass.class).withDefaultValue(10)); + } + @Test public void builderPojo() throws IOException { ConfigurationMetadata metadata = compile(BuilderPojo.class); @@ -768,17 +779,6 @@ public class ConfigurationMetadataAnnotationProcessorTests { } } - @Test - public void nestedClassChildProperties() throws Exception { - ConfigurationMetadata metadata = compile(ClassWithNestedProperties.class); - assertThat(metadata).has(Metadata.withGroup("nestedChildProps") - .fromSource(ClassWithNestedProperties.NestedChildClass.class)); - assertThat(metadata).has(Metadata.withProperty("nestedChildProps.child-class-property", Integer.class) - .fromSource(ClassWithNestedProperties.NestedChildClass.class).withDefaultValue(20)); - assertThat(metadata).has(Metadata.withProperty("nestedChildProps.parent-class-property", Integer.class) - .fromSource(ClassWithNestedProperties.NestedChildClass.class).withDefaultValue(10)); - } - private static class AdditionalMetadata { }