From 59ea7c11f6bfd089ce1044c9083e51fc2f2c6276 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Thu, 5 Nov 2020 15:24:14 +0100 Subject: [PATCH] Use most specific getter when generating metadata This commit makes sure to use the most specific getter if more than one candidate exists. Closes gh-24002 --- .../PropertyDescriptorResolver.java | 18 +++++-- .../TypeElementMembers.java | 51 +++++++++++-------- ...ationMetadataAnnotationProcessorTests.java | 13 ++++- .../specific/BoxingPojo.java | 12 ++++- .../DeprecatedLessPreciseTypePojo.java | 50 ++++++++++++++++++ 5 files changed, 118 insertions(+), 26 deletions(-) create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/specific/DeprecatedLessPreciseTypePojo.java diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/PropertyDescriptorResolver.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/PropertyDescriptorResolver.java index fc8572d131a0..a4bc06dd2d54 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/PropertyDescriptorResolver.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/PropertyDescriptorResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 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. @@ -91,10 +91,12 @@ Stream> resolveJavaBeanProperties(TypeElement type, Execut TypeElementMembers members) { // First check if we have regular java bean properties there Map> candidates = new LinkedHashMap<>(); - members.getPublicGetters().forEach((name, getter) -> { + members.getPublicGetters().forEach((name, getters) -> { + VariableElement field = members.getFields().get(name); + ExecutableElement getter = findMatchingGetter(members, getters, field); TypeMirror propertyType = getter.getReturnType(); - register(candidates, new JavaBeanPropertyDescriptor(type, factoryMethod, getter, name, propertyType, - members.getFields().get(name), members.getPublicSetter(name, propertyType))); + register(candidates, new JavaBeanPropertyDescriptor(type, factoryMethod, getter, name, propertyType, field, + members.getPublicSetter(name, propertyType))); }); // Then check for Lombok ones members.getFields().forEach((name, field) -> { @@ -107,6 +109,14 @@ Stream> resolveJavaBeanProperties(TypeElement type, Execut return candidates.values().stream(); } + private ExecutableElement findMatchingGetter(TypeElementMembers members, List candidates, + VariableElement field) { + if (candidates.size() > 1 && field != null) { + return members.getMatchingGetter(candidates, field.asType()); + } + return candidates.get(0); + } + private void register(Map> candidates, PropertyDescriptor descriptor) { if (!candidates.containsKey(descriptor.getName()) && isCandidate(descriptor)) { candidates.put(descriptor.getName(), descriptor); diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeElementMembers.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeElementMembers.java index 53ec3d2da69b..f51290c654b7 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeElementMembers.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/main/java/org/springframework/boot/configurationprocessor/TypeElementMembers.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Function; import javax.lang.model.element.Element; import javax.lang.model.element.ExecutableElement; @@ -48,7 +49,7 @@ class TypeElementMembers { private final Map fields = new LinkedHashMap<>(); - private final Map publicGetters = new LinkedHashMap<>(); + private final Map> publicGetters = new LinkedHashMap<>(); private final Map> publicSetters = new LinkedHashMap<>(); @@ -74,8 +75,14 @@ private void process(TypeElement element) { private void processMethod(ExecutableElement method) { if (isPublic(method)) { String name = method.getSimpleName().toString(); - if (isGetter(method) && !this.publicGetters.containsKey(getAccessorName(name))) { - this.publicGetters.put(getAccessorName(name), method); + if (isGetter(method)) { + String propertyName = getAccessorName(name); + List matchingGetters = this.publicGetters.computeIfAbsent(propertyName, + (k) -> new ArrayList<>()); + TypeMirror returnType = method.getReturnType(); + if (getMatchingGetter(matchingGetters, returnType) == null) { + matchingGetters.add(method); + } } else if (isSetter(method)) { String propertyName = getAccessorName(name); @@ -95,10 +102,19 @@ private boolean isPublic(ExecutableElement method) { && !modifiers.contains(Modifier.STATIC); } + ExecutableElement getMatchingGetter(List candidates, TypeMirror type) { + return getMatchingAccessor(candidates, type, ExecutableElement::getReturnType); + } + private ExecutableElement getMatchingSetter(List candidates, TypeMirror type) { + return getMatchingAccessor(candidates, type, (candidate) -> candidate.getParameters().get(0).asType()); + } + + private ExecutableElement getMatchingAccessor(List candidates, TypeMirror type, + Function typeExtractor) { for (ExecutableElement candidate : candidates) { - TypeMirror paramType = candidate.getParameters().get(0).asType(); - if (this.env.getTypeUtils().isSameType(paramType, type)) { + TypeMirror candidateType = typeExtractor.apply(candidate); + if (this.env.getTypeUtils().isSameType(candidateType, type)) { return candidate; } } @@ -151,35 +167,30 @@ Map getFields() { return Collections.unmodifiableMap(this.fields); } - Map getPublicGetters() { + Map> getPublicGetters() { return Collections.unmodifiableMap(this.publicGetters); } ExecutableElement getPublicGetter(String name, TypeMirror type) { - ExecutableElement candidate = this.publicGetters.get(name); - if (candidate != null) { - TypeMirror returnType = candidate.getReturnType(); - if (this.env.getTypeUtils().isSameType(returnType, type)) { - return candidate; - } - TypeMirror alternative = this.env.getTypeUtils().getWrapperOrPrimitiveFor(type); - if (alternative != null && this.env.getTypeUtils().isSameType(returnType, alternative)) { - return candidate; - } - } - return null; + List candidates = this.publicGetters.get(name); + return getPublicAccessor(candidates, type, (specificType) -> getMatchingGetter(candidates, specificType)); } ExecutableElement getPublicSetter(String name, TypeMirror type) { List candidates = this.publicSetters.get(name); + return getPublicAccessor(candidates, type, (specificType) -> getMatchingSetter(candidates, specificType)); + } + + private ExecutableElement getPublicAccessor(List candidates, TypeMirror type, + Function matchingAccessorExtractor) { if (candidates != null) { - ExecutableElement matching = getMatchingSetter(candidates, type); + ExecutableElement matching = matchingAccessorExtractor.apply(type); if (matching != null) { return matching; } TypeMirror alternative = this.env.getTypeUtils().getWrapperOrPrimitiveFor(type); if (alternative != null) { - return getMatchingSetter(candidates, alternative); + return matchingAccessorExtractor.apply(alternative); } } return null; diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java index fe89c466a00e..3ea74d757d86 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationprocessor/ConfigurationMetadataAnnotationProcessorTests.java @@ -38,6 +38,7 @@ import org.springframework.boot.configurationsample.specific.AnnotatedGetter; import org.springframework.boot.configurationsample.specific.BoxingPojo; import org.springframework.boot.configurationsample.specific.BuilderPojo; +import org.springframework.boot.configurationsample.specific.DeprecatedLessPreciseTypePojo; import org.springframework.boot.configurationsample.specific.DeprecatedUnrelatedMethodPojo; import org.springframework.boot.configurationsample.specific.DoubleRegistrationProperties; import org.springframework.boot.configurationsample.specific.EmptyDefaultValueProperties; @@ -192,12 +193,22 @@ void deprecatedOnUnrelatedSetter() { } @Test - void boxingOnSetter() { + void deprecatedWithLessPreciseType() { + Class type = DeprecatedLessPreciseTypePojo.class; + ConfigurationMetadata metadata = compile(type); + assertThat(metadata).has(Metadata.withGroup("not.deprecated").fromSource(type)); + assertThat(metadata).has(Metadata.withProperty("not.deprecated.flag", Boolean.class).withDefaultValue(false) + .withNoDeprecation().fromSource(type)); + } + + @Test + void typBoxing() { Class type = BoxingPojo.class; ConfigurationMetadata metadata = compile(type); assertThat(metadata).has(Metadata.withGroup("boxing").fromSource(type)); assertThat(metadata) .has(Metadata.withProperty("boxing.flag", Boolean.class).withDefaultValue(false).fromSource(type)); + assertThat(metadata).has(Metadata.withProperty("boxing.another-flag", Boolean.class).fromSource(type)); assertThat(metadata).has(Metadata.withProperty("boxing.counter", Integer.class).fromSource(type)); } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/specific/BoxingPojo.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/specific/BoxingPojo.java index 00ab84ff1be9..5484bb941cbe 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/specific/BoxingPojo.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/specific/BoxingPojo.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 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. @@ -29,6 +29,8 @@ public class BoxingPojo { private boolean flag; + private Boolean anotherFlag; + private Integer counter; public boolean isFlag() { @@ -40,6 +42,14 @@ public void setFlag(Boolean flag) { this.flag = flag; } + public boolean isAnotherFlag() { + return Boolean.TRUE.equals(this.anotherFlag); + } + + public void setAnotherFlag(boolean anotherFlag) { + this.anotherFlag = anotherFlag; + } + public Integer getCounter() { return this.counter; } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/specific/DeprecatedLessPreciseTypePojo.java b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/specific/DeprecatedLessPreciseTypePojo.java new file mode 100644 index 000000000000..ff1e2421ef21 --- /dev/null +++ b/spring-boot-project/spring-boot-tools/spring-boot-configuration-processor/src/test/java/org/springframework/boot/configurationsample/specific/DeprecatedLessPreciseTypePojo.java @@ -0,0 +1,50 @@ +/* + * Copyright 2012-2020 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 + * + * https://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 org.springframework.boot.configurationsample.ConfigurationProperties; + +/** + * Demonstrate that deprecating accessor with not the same type is not taken into account + * to detect the deprecated flag. + * + * @author Stephane Nicoll + */ +@ConfigurationProperties("not.deprecated") +public class DeprecatedLessPreciseTypePojo { + + private boolean flag; + + @Deprecated + public Boolean getFlag() { + return this.flag; + } + + public boolean isFlag() { + return this.flag; + } + + public void setFlag(boolean flag) { + this.flag = flag; + } + + @Deprecated + public void setFlag(Boolean flag) { + this.flag = flag; + } + +}