diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java index 5af02175f1..c307c3a56a 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2018 the original author or authors. + * Copyright 2012-2019 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. @@ -97,8 +97,10 @@ class JavaBeanBinder implements BeanBinder { /** * The bean being bound. + * + * @param the bean type */ - private static class Bean { + static class Bean { private static Bean cached; @@ -111,23 +113,36 @@ class JavaBeanBinder implements BeanBinder { Bean(ResolvableType type, Class resolvedType) { this.type = type; this.resolvedType = resolvedType; - putProperties(resolvedType); + addProperties(resolvedType); } - private void putProperties(Class type) { + private void addProperties(Class type) { while (type != null && !Object.class.equals(type)) { - for (Method method : type.getDeclaredMethods()) { - if (isCandidate(method)) { - addMethod(method); - } - } - for (Field field : type.getDeclaredFields()) { - addField(field); - } + Method[] declaredMethods = type.getDeclaredMethods(); + Field[] declaredFields = type.getDeclaredFields(); + addProperties(declaredMethods, declaredFields); type = type.getSuperclass(); } } + protected void addProperties(Method[] declaredMethods, Field[] declaredFields) { + for (int i = 0; i < declaredMethods.length; i++) { + if (!isCandidate(declaredMethods[i])) { + declaredMethods[i] = null; + } + } + for (Method method : declaredMethods) { + addMethodIfPossible(method, "get", 0, BeanProperty::addGetter); + } + for (Method method : declaredMethods) { + addMethodIfPossible(method, "is", 0, BeanProperty::addGetter); + addMethodIfPossible(method, "set", 1, BeanProperty::addSetter); + } + for (Field field : declaredFields) { + addField(field); + } + } + private boolean isCandidate(Method method) { int modifiers = method.getModifiers(); return Modifier.isPublic(modifiers) && !Modifier.isAbstract(modifiers) @@ -136,15 +151,9 @@ class JavaBeanBinder implements BeanBinder { && !Class.class.equals(method.getDeclaringClass()); } - private void addMethod(Method method) { - addMethodIfPossible(method, "get", 0, BeanProperty::addGetter); - addMethodIfPossible(method, "is", 0, BeanProperty::addGetter); - addMethodIfPossible(method, "set", 1, BeanProperty::addSetter); - } - private void addMethodIfPossible(Method method, String prefix, int parameterCount, BiConsumer consumer) { - if (method.getParameterCount() == parameterCount + if (method != null && method.getParameterCount() == parameterCount && method.getName().startsWith(prefix) && method.getName().length() > prefix.length()) { String propertyName = Introspector @@ -250,7 +259,7 @@ class JavaBeanBinder implements BeanBinder { /** * A bean property being bound. */ - private static class BeanProperty { + static class BeanProperty { private final String name; @@ -274,11 +283,16 @@ class JavaBeanBinder implements BeanBinder { } public void addSetter(Method setter) { - if (this.setter == null) { + if (this.setter == null || isBetterSetter(setter)) { this.setter = setter; } } + private boolean isBetterSetter(Method setter) { + return this.getter != null + && this.getter.getReturnType().equals(setter.getParameterTypes()[0]); + } + public void addField(Field field) { if (this.field == null) { this.field = field; diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/JavaBeanBinderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/JavaBeanBinderTests.java index b818fde8b2..1d43ff11aa 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/JavaBeanBinderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/bind/JavaBeanBinderTests.java @@ -16,6 +16,8 @@ package org.springframework.boot.context.properties.bind; +import java.lang.reflect.Field; +import java.lang.reflect.Method; import java.time.LocalDate; import java.util.ArrayList; import java.util.Collection; @@ -28,11 +30,14 @@ import java.util.Set; import org.junit.Before; import org.junit.Test; +import org.springframework.boot.context.properties.bind.JavaBeanBinder.Bean; +import org.springframework.boot.context.properties.bind.JavaBeanBinder.BeanProperty; import org.springframework.boot.context.properties.bind.handler.IgnoreErrorsBindHandler; import org.springframework.boot.context.properties.source.ConfigurationPropertyName; import org.springframework.boot.context.properties.source.ConfigurationPropertySource; import org.springframework.boot.context.properties.source.MockConfigurationPropertySource; import org.springframework.boot.convert.Delimiter; +import org.springframework.core.ResolvableType; import org.springframework.format.annotation.DateTimeFormat; import static org.assertj.core.api.Assertions.assertThat; @@ -44,6 +49,7 @@ import static org.assertj.core.api.Assertions.entry; * * @author Phillip Webb * @author Madhura Bhave + * @author Andy Wilkinson */ public class JavaBeanBinderTests { @@ -507,6 +513,56 @@ public class JavaBeanBinderTests { assertThat(bean.getBooleans().get("b").getValue()).isEqualTo(true); } + public void bindToClassWithOverloadedSetterShouldUseSetterThatMatchesField() { + // gh-16206 + MockConfigurationPropertySource source = new MockConfigurationPropertySource(); + source.put("foo.property", "some string"); + this.sources.add(source); + PropertyWithOverloadedSetter bean = this.binder + .bind("foo", Bindable.of(PropertyWithOverloadedSetter.class)).get(); + assertThat(bean.getProperty()).isEqualTo("some string"); + } + + @Test + public void beanProperiesPreferMatchingType() { + // gh-16206 + + ResolvableType type = ResolvableType.forClass(PropertyWithOverloadedSetter.class); + Bean bean = new Bean( + type, type.resolve()) { + + @Override + protected void addProperties(Method[] declaredMethods, + Field[] declaredFields) { + // We override here because we need a specific order of the declared + // methods and the JVM doesn't give us one + int intSetter = -1; + int stringSetter = -1; + for (int i = 0; i < declaredMethods.length; i++) { + Method method = declaredMethods[i]; + if (method.getName().equals("setProperty")) { + if (method.getParameters()[0].getType().equals(int.class)) { + intSetter = i; + } + else { + stringSetter = i; + } + } + } + if (intSetter > stringSetter) { + Method method = declaredMethods[intSetter]; + declaredMethods[intSetter] = declaredMethods[stringSetter]; + declaredMethods[stringSetter] = method; + } + super.addProperties(declaredMethods, declaredFields); + } + + }; + BeanProperty property = bean.getProperties().get("property"); + PropertyWithOverloadedSetter target = new PropertyWithOverloadedSetter(); + property.setValue(() -> target, "some string"); + } + public static class ExampleValueBean { private int intValue; @@ -957,4 +1013,22 @@ public class JavaBeanBinderTests { } + public static class PropertyWithOverloadedSetter { + + private String property; + + public void setProperty(int property) { + this.property = String.valueOf(property); + } + + public void setProperty(String property) { + this.property = property; + } + + public String getProperty() { + return this.property; + } + + } + }