From 5863e6f78c78f840127b2ae64ea9c422a06df9f3 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Fri, 28 Oct 2016 11:47:20 +0200 Subject: [PATCH] Fix class name in generated meta-data Previously, the algorithm that computes the String representation of a class reference and a property type was shared. This lead to generic information for group's `type` and `sourceType` property. This commit separates that logic in two: `getQualifiedName` is now responsible to generate a fully qualified class name while the existing `getType` is solely responsible to generate a type representation for the property. Only the latter has generic information. Closes gh-7236 --- .../appendix-configuration-metadata.adoc | 13 ++- ...figurationMetadataAnnotationProcessor.java | 15 +-- .../MetadataCollector.java | 2 +- .../configurationprocessor/TypeUtils.java | 42 ++++++- ...ationMetadataAnnotationProcessorTests.java | 30 +++++ .../specific/GenericConfig.java | 107 ++++++++++++++++++ 6 files changed, 190 insertions(+), 19 deletions(-) create mode 100644 spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/specific/GenericConfig.java diff --git a/spring-boot-docs/src/main/asciidoc/appendix-configuration-metadata.adoc b/spring-boot-docs/src/main/asciidoc/appendix-configuration-metadata.adoc index f89f3cb85d8..4f324c53fb2 100644 --- a/spring-boot-docs/src/main/asciidoc/appendix-configuration-metadata.adoc +++ b/spring-boot-docs/src/main/asciidoc/appendix-configuration-metadata.adoc @@ -164,12 +164,13 @@ The JSON object contained in the `properties` array can contain the following at |`type` | String -| The class name of the data type of the property. For example, `java.lang.String`. This - attribute can be used to guide the user as to the types of values that they can enter. - For consistency, the type of a primitive is specified using its wrapper counterpart, - i.e. `boolean` becomes `java.lang.Boolean`. Note that this class may be a complex type - that gets converted from a String as values are bound. May be omitted if the type is - not known. +| The full signature of the data type of the property. For example, `java.lang.String` + but also a full generic type such as `java.util.Map`. + This attribute can be used to guide the user as to the types of values that they can + enter. For consistency, the type of a primitive is specified using its wrapper + counterpart, i.e. `boolean` becomes `java.lang.Boolean`. Note that this class may be + a complex type that gets converted from a String as values are bound. May be omitted + if the type is not known. |`description` | String 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 4875ab8fe7d..890ca669b0d 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 @@ -161,7 +161,7 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor } private void processAnnotatedTypeElement(String prefix, TypeElement element) { - String type = this.typeUtils.getType(element); + String type = this.typeUtils.getQualifiedName(element); this.metadataCollector.add(ItemMetadata.newGroup(prefix, type, type, null)); processTypeElement(prefix, element); } @@ -173,8 +173,9 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor .asElement(element.getReturnType()); if (returns instanceof TypeElement) { this.metadataCollector.add( - ItemMetadata.newGroup(prefix, this.typeUtils.getType(returns), - this.typeUtils.getType(element.getEnclosingElement()), + ItemMetadata.newGroup(prefix, + this.typeUtils.getQualifiedName(returns), + this.typeUtils.getQualifiedName(element.getEnclosingElement()), element.toString())); processTypeElement(prefix, (TypeElement) returns); } @@ -215,7 +216,7 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor boolean isCollection = this.typeUtils.isCollectionOrMap(returnType); if (!isExcluded && !isNested && (setter != null || isCollection)) { String dataType = this.typeUtils.getType(returnType); - String sourceType = this.typeUtils.getType(element); + String sourceType = this.typeUtils.getQualifiedName(element); String description = this.typeUtils.getJavaDoc(field); Object defaultValue = fieldValues.get(name); boolean deprecated = isDeprecated(getter) || isDeprecated(setter) @@ -258,7 +259,7 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor boolean hasSetter = hasLombokSetter(field, element); if (!isExcluded && !isNested && (hasSetter || isCollection)) { String dataType = this.typeUtils.getType(returnType); - String sourceType = this.typeUtils.getType(element); + String sourceType = this.typeUtils.getQualifiedName(element); String description = this.typeUtils.getJavaDoc(field); Object defaultValue = fieldValues.get(name); boolean deprecated = isDeprecated(field) || isDeprecated(element); @@ -315,8 +316,8 @@ public class ConfigurationMetadataAnnotationProcessor extends AbstractProcessor && annotation == null && isNested) { String nestedPrefix = ConfigurationMetadata.nestedPrefix(prefix, name); this.metadataCollector.add(ItemMetadata.newGroup(nestedPrefix, - this.typeUtils.getType(returnElement), - this.typeUtils.getType(element), + this.typeUtils.getQualifiedName(returnElement), + this.typeUtils.getQualifiedName(element), (getter == null ? null : getter.toString()))); processTypeElement(nestedPrefix, (TypeElement) returnElement); } diff --git a/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/MetadataCollector.java b/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/MetadataCollector.java index 7b7ff324573..04e2321cabd 100644 --- a/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/MetadataCollector.java +++ b/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/MetadataCollector.java @@ -69,7 +69,7 @@ public class MetadataCollector { private void markAsProcessed(Element element) { if (element instanceof TypeElement) { - this.processedSourceTypes.add(this.typeUtils.getType(element)); + this.processedSourceTypes.add(this.typeUtils.getQualifiedName(element)); } } diff --git a/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeUtils.java b/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeUtils.java index 59d02f551d7..197671b808d 100644 --- a/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeUtils.java +++ b/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeUtils.java @@ -93,10 +93,34 @@ class TypeUtils { } } - public String getType(Element element) { - return getType(element == null ? null : element.asType()); + /** + * Return the qualified name of the specified element. + * @param element the element to handle + * @return the fully qualified name of the element, suitable for a call + * to {@link Class#forName(String)} + */ + public String getQualifiedName(Element element) { + if (element == null) { + return null; + } + TypeElement enclosingElement = getEnclosingTypeElement(element.asType()); + if (enclosingElement != null) { + return getQualifiedName(enclosingElement) + "$" + + ((DeclaredType) element.asType()).asElement().getSimpleName().toString(); + } + if (element instanceof TypeElement) { + return ((TypeElement) element).getQualifiedName().toString(); + } + throw new IllegalStateException("Could not extract qualified name from " + + element); } + /** + * Return the type of the specified {@link TypeMirror} including all its generic + * information. + * @param type the type to handle + * @return a representation of the type including all its generic information + */ public String getType(TypeMirror type) { if (type == null) { return null; @@ -105,15 +129,23 @@ class TypeUtils { if (wrapper != null) { return wrapper.getName(); } + TypeElement enclosingElement = getEnclosingTypeElement(type); + if (enclosingElement != null) { + return getQualifiedName(enclosingElement) + "$" + + ((DeclaredType) type).asElement().getSimpleName().toString(); + } + return type.toString(); + } + + private TypeElement getEnclosingTypeElement(TypeMirror type) { if (type instanceof DeclaredType) { DeclaredType declaredType = (DeclaredType) type; Element enclosingElement = declaredType.asElement().getEnclosingElement(); if (enclosingElement != null && enclosingElement instanceof TypeElement) { - return getType(enclosingElement) + "$" - + declaredType.asElement().getSimpleName().toString(); + return (TypeElement) enclosingElement; } } - return type.toString(); + return null; } public boolean isCollectionOrMap(TypeMirror type) { 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 d7e9c15f130..1fd3aff0e80 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 @@ -57,6 +57,7 @@ import org.springframework.boot.configurationsample.specific.BoxingPojo; import org.springframework.boot.configurationsample.specific.BuilderPojo; import org.springframework.boot.configurationsample.specific.DeprecatedUnrelatedMethodPojo; import org.springframework.boot.configurationsample.specific.ExcludedTypesPojo; +import org.springframework.boot.configurationsample.specific.GenericConfig; import org.springframework.boot.configurationsample.specific.InnerClassAnnotatedGetterConfig; import org.springframework.boot.configurationsample.specific.InnerClassProperties; import org.springframework.boot.configurationsample.specific.InnerClassRootConfig; @@ -339,6 +340,35 @@ public class ConfigurationMetadataAnnotationProcessorTests { assertThat(metadata.getItems(), hasSize(1)); } + @Test + public void genericTypes() throws IOException { + ConfigurationMetadata metadata = compile(GenericConfig.class); + assertThat(metadata, containsGroup("generic").ofType( + "org.springframework.boot.configurationsample.specific.GenericConfig")); + assertThat(metadata, containsGroup("generic.foo").ofType( + "org.springframework.boot.configurationsample.specific.GenericConfig$Foo")); + assertThat(metadata, containsGroup("generic.foo.bar").ofType( + "org.springframework.boot.configurationsample.specific.GenericConfig$Bar")); + assertThat(metadata, containsGroup("generic.foo.bar.biz").ofType( + "org.springframework.boot.configurationsample.specific.GenericConfig$Bar$Biz")); + assertThat(metadata, containsProperty("generic.foo.name") + .ofType(String.class) + .fromSource(GenericConfig.Foo.class)); + assertThat(metadata, containsProperty("generic.foo.string-to-bar") + .ofType("java.util.Map>") + .fromSource(GenericConfig.Foo.class)); + assertThat(metadata, containsProperty("generic.foo.string-to-integer") + .ofType("java.util.Map") + .fromSource(GenericConfig.Foo.class)); + assertThat(metadata, containsProperty("generic.foo.bar.name") + .ofType("java.lang.String") + .fromSource(GenericConfig.Bar.class)); + assertThat(metadata, containsProperty("generic.foo.bar.biz.name") + .ofType("java.lang.String") + .fromSource(GenericConfig.Bar.Biz.class)); + assertThat(metadata.getItems(), hasSize(9)); + } + @Test public void lombokDataProperties() throws Exception { ConfigurationMetadata metadata = compile(LombokSimpleDataProperties.class); diff --git a/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/specific/GenericConfig.java b/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/specific/GenericConfig.java new file mode 100644 index 00000000000..7649109b0fc --- /dev/null +++ b/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/specific/GenericConfig.java @@ -0,0 +1,107 @@ +/* + * Copyright 2012-2016 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.specific; + +import java.util.HashMap; +import java.util.Map; + +import org.springframework.boot.configurationsample.ConfigurationProperties; +import org.springframework.boot.configurationsample.NestedConfigurationProperty; + +/** + * Demonstrate that only relevant generics are stored in the metadata. + * + * @author Stephane Nicoll + */ +@ConfigurationProperties("generic") +public class GenericConfig { + + private final Foo foo = new Foo(); + + public Foo getFoo() { + return this.foo; + } + + public static class Foo { + + private String name; + + @NestedConfigurationProperty + private final Bar bar = new Bar(); + + private final Map> stringToBar = + new HashMap>(); + + private final Map stringToInteger = + new HashMap(); + + public String getName() { + return this.name; + } + + public void setName(String name) { + this.name = name; + } + + public Bar getBar() { + return this.bar; + } + + public Map> getStringToBar() { + return this.stringToBar; + } + + public Map getStringToInteger() { + return this.stringToInteger; + } + } + + public static class Bar { + + private String name; + + @NestedConfigurationProperty + private final Biz biz = new Biz(); + + public String getName() { + return this.name; + } + + public void setName(String name) { + this.name = name; + } + + public Biz getBiz() { + return this.biz; + } + + public static class Biz { + + private String name; + + public String getName() { + return this.name; + } + + public void setName(String name) { + this.name = name; + } + } + + } + +}