Commit 59ea7c11 authored by Stephane Nicoll's avatar Stephane Nicoll

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
parent 006d4bc3
/* /*
* 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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
...@@ -91,10 +91,12 @@ class PropertyDescriptorResolver { ...@@ -91,10 +91,12 @@ class PropertyDescriptorResolver {
TypeElementMembers members) { TypeElementMembers members) {
// First check if we have regular java bean properties there // First check if we have regular java bean properties there
Map<String, PropertyDescriptor<?>> candidates = new LinkedHashMap<>(); Map<String, PropertyDescriptor<?>> 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(); TypeMirror propertyType = getter.getReturnType();
register(candidates, new JavaBeanPropertyDescriptor(type, factoryMethod, getter, name, propertyType, register(candidates, new JavaBeanPropertyDescriptor(type, factoryMethod, getter, name, propertyType, field,
members.getFields().get(name), members.getPublicSetter(name, propertyType))); members.getPublicSetter(name, propertyType)));
}); });
// Then check for Lombok ones // Then check for Lombok ones
members.getFields().forEach((name, field) -> { members.getFields().forEach((name, field) -> {
...@@ -107,6 +109,14 @@ class PropertyDescriptorResolver { ...@@ -107,6 +109,14 @@ class PropertyDescriptorResolver {
return candidates.values().stream(); return candidates.values().stream();
} }
private ExecutableElement findMatchingGetter(TypeElementMembers members, List<ExecutableElement> candidates,
VariableElement field) {
if (candidates.size() > 1 && field != null) {
return members.getMatchingGetter(candidates, field.asType());
}
return candidates.get(0);
}
private void register(Map<String, PropertyDescriptor<?>> candidates, PropertyDescriptor<?> descriptor) { private void register(Map<String, PropertyDescriptor<?>> candidates, PropertyDescriptor<?> descriptor) {
if (!candidates.containsKey(descriptor.getName()) && isCandidate(descriptor)) { if (!candidates.containsKey(descriptor.getName()) && isCandidate(descriptor)) {
candidates.put(descriptor.getName(), descriptor); candidates.put(descriptor.getName(), descriptor);
......
...@@ -22,6 +22,7 @@ import java.util.LinkedHashMap; ...@@ -22,6 +22,7 @@ import java.util.LinkedHashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
import java.util.function.Function;
import javax.lang.model.element.Element; import javax.lang.model.element.Element;
import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.ExecutableElement;
...@@ -48,7 +49,7 @@ class TypeElementMembers { ...@@ -48,7 +49,7 @@ class TypeElementMembers {
private final Map<String, VariableElement> fields = new LinkedHashMap<>(); private final Map<String, VariableElement> fields = new LinkedHashMap<>();
private final Map<String, ExecutableElement> publicGetters = new LinkedHashMap<>(); private final Map<String, List<ExecutableElement>> publicGetters = new LinkedHashMap<>();
private final Map<String, List<ExecutableElement>> publicSetters = new LinkedHashMap<>(); private final Map<String, List<ExecutableElement>> publicSetters = new LinkedHashMap<>();
...@@ -74,8 +75,14 @@ class TypeElementMembers { ...@@ -74,8 +75,14 @@ class TypeElementMembers {
private void processMethod(ExecutableElement method) { private void processMethod(ExecutableElement method) {
if (isPublic(method)) { if (isPublic(method)) {
String name = method.getSimpleName().toString(); String name = method.getSimpleName().toString();
if (isGetter(method) && !this.publicGetters.containsKey(getAccessorName(name))) { if (isGetter(method)) {
this.publicGetters.put(getAccessorName(name), method); String propertyName = getAccessorName(name);
List<ExecutableElement> matchingGetters = this.publicGetters.computeIfAbsent(propertyName,
(k) -> new ArrayList<>());
TypeMirror returnType = method.getReturnType();
if (getMatchingGetter(matchingGetters, returnType) == null) {
matchingGetters.add(method);
}
} }
else if (isSetter(method)) { else if (isSetter(method)) {
String propertyName = getAccessorName(name); String propertyName = getAccessorName(name);
...@@ -95,10 +102,19 @@ class TypeElementMembers { ...@@ -95,10 +102,19 @@ class TypeElementMembers {
&& !modifiers.contains(Modifier.STATIC); && !modifiers.contains(Modifier.STATIC);
} }
ExecutableElement getMatchingGetter(List<ExecutableElement> candidates, TypeMirror type) {
return getMatchingAccessor(candidates, type, ExecutableElement::getReturnType);
}
private ExecutableElement getMatchingSetter(List<ExecutableElement> candidates, TypeMirror type) { private ExecutableElement getMatchingSetter(List<ExecutableElement> candidates, TypeMirror type) {
return getMatchingAccessor(candidates, type, (candidate) -> candidate.getParameters().get(0).asType());
}
private ExecutableElement getMatchingAccessor(List<ExecutableElement> candidates, TypeMirror type,
Function<ExecutableElement, TypeMirror> typeExtractor) {
for (ExecutableElement candidate : candidates) { for (ExecutableElement candidate : candidates) {
TypeMirror paramType = candidate.getParameters().get(0).asType(); TypeMirror candidateType = typeExtractor.apply(candidate);
if (this.env.getTypeUtils().isSameType(paramType, type)) { if (this.env.getTypeUtils().isSameType(candidateType, type)) {
return candidate; return candidate;
} }
} }
...@@ -151,35 +167,30 @@ class TypeElementMembers { ...@@ -151,35 +167,30 @@ class TypeElementMembers {
return Collections.unmodifiableMap(this.fields); return Collections.unmodifiableMap(this.fields);
} }
Map<String, ExecutableElement> getPublicGetters() { Map<String, List<ExecutableElement>> getPublicGetters() {
return Collections.unmodifiableMap(this.publicGetters); return Collections.unmodifiableMap(this.publicGetters);
} }
ExecutableElement getPublicGetter(String name, TypeMirror type) { ExecutableElement getPublicGetter(String name, TypeMirror type) {
ExecutableElement candidate = this.publicGetters.get(name); List<ExecutableElement> candidates = this.publicGetters.get(name);
if (candidate != null) { return getPublicAccessor(candidates, type, (specificType) -> getMatchingGetter(candidates, specificType));
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;
} }
ExecutableElement getPublicSetter(String name, TypeMirror type) { ExecutableElement getPublicSetter(String name, TypeMirror type) {
List<ExecutableElement> candidates = this.publicSetters.get(name); List<ExecutableElement> candidates = this.publicSetters.get(name);
return getPublicAccessor(candidates, type, (specificType) -> getMatchingSetter(candidates, specificType));
}
private ExecutableElement getPublicAccessor(List<ExecutableElement> candidates, TypeMirror type,
Function<TypeMirror, ExecutableElement> matchingAccessorExtractor) {
if (candidates != null) { if (candidates != null) {
ExecutableElement matching = getMatchingSetter(candidates, type); ExecutableElement matching = matchingAccessorExtractor.apply(type);
if (matching != null) { if (matching != null) {
return matching; return matching;
} }
TypeMirror alternative = this.env.getTypeUtils().getWrapperOrPrimitiveFor(type); TypeMirror alternative = this.env.getTypeUtils().getWrapperOrPrimitiveFor(type);
if (alternative != null) { if (alternative != null) {
return getMatchingSetter(candidates, alternative); return matchingAccessorExtractor.apply(alternative);
} }
} }
return null; return null;
......
...@@ -38,6 +38,7 @@ import org.springframework.boot.configurationsample.simple.SimpleTypeProperties; ...@@ -38,6 +38,7 @@ import org.springframework.boot.configurationsample.simple.SimpleTypeProperties;
import org.springframework.boot.configurationsample.specific.AnnotatedGetter; import org.springframework.boot.configurationsample.specific.AnnotatedGetter;
import org.springframework.boot.configurationsample.specific.BoxingPojo; import org.springframework.boot.configurationsample.specific.BoxingPojo;
import org.springframework.boot.configurationsample.specific.BuilderPojo; 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.DeprecatedUnrelatedMethodPojo;
import org.springframework.boot.configurationsample.specific.DoubleRegistrationProperties; import org.springframework.boot.configurationsample.specific.DoubleRegistrationProperties;
import org.springframework.boot.configurationsample.specific.EmptyDefaultValueProperties; import org.springframework.boot.configurationsample.specific.EmptyDefaultValueProperties;
...@@ -192,12 +193,22 @@ class ConfigurationMetadataAnnotationProcessorTests extends AbstractMetadataGene ...@@ -192,12 +193,22 @@ class ConfigurationMetadataAnnotationProcessorTests extends AbstractMetadataGene
} }
@Test @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; Class<?> type = BoxingPojo.class;
ConfigurationMetadata metadata = compile(type); ConfigurationMetadata metadata = compile(type);
assertThat(metadata).has(Metadata.withGroup("boxing").fromSource(type)); assertThat(metadata).has(Metadata.withGroup("boxing").fromSource(type));
assertThat(metadata) assertThat(metadata)
.has(Metadata.withProperty("boxing.flag", Boolean.class).withDefaultValue(false).fromSource(type)); .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)); assertThat(metadata).has(Metadata.withProperty("boxing.counter", Integer.class).fromSource(type));
} }
......
/* /*
* 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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
...@@ -29,6 +29,8 @@ public class BoxingPojo { ...@@ -29,6 +29,8 @@ public class BoxingPojo {
private boolean flag; private boolean flag;
private Boolean anotherFlag;
private Integer counter; private Integer counter;
public boolean isFlag() { public boolean isFlag() {
...@@ -40,6 +42,14 @@ public class BoxingPojo { ...@@ -40,6 +42,14 @@ public class BoxingPojo {
this.flag = flag; this.flag = flag;
} }
public boolean isAnotherFlag() {
return Boolean.TRUE.equals(this.anotherFlag);
}
public void setAnotherFlag(boolean anotherFlag) {
this.anotherFlag = anotherFlag;
}
public Integer getCounter() { public Integer getCounter() {
return this.counter; return this.counter;
} }
......
/*
* 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;
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment