Merge branch '2.1.x'

Closes gh-16859

Author: philwebb

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 @@ else if (value == null || !bound.equals(value.get())) {
 
 	/**
 	 * The bean being bound.
+	 *
+	 * @param <T> the bean type
 	 */
-	private static class Bean<T> {
+	static class Bean<T> {
 
 		private static Bean<?> cached;
 
@@ -111,23 +113,36 @@ private static class Bean<T> {
 		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 @@ private boolean isCandidate(Method method) {
 					&& !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<BeanProperty, Method> 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 @@ public T get() {
 	/**
 	 * A bean property being bound.
 	 */
-	private static class BeanProperty {
+	static class BeanProperty {
 
 		private final String name;
 
@@ -274,11 +283,16 @@ public void addGetter(Method getter) {
 		}
 
 		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;

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 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 @@
  *
  * @author Phillip Webb
  * @author Madhura Bhave
+ * @author Andy Wilkinson
  */
 public class JavaBeanBinderTests {
 
@@ -507,6 +513,56 @@ public void bindToClassShouldCacheWithGenerics() {
 		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<PropertyWithOverloadedSetter> bean = new Bean<PropertyWithOverloadedSetter>(
+				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 void setValue(T value) {
 
 	}
 
+	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;
+		}
+
+	}
+
 }