From ab15b8e26d91e0c6f931d055741e21814be444c4 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 14 May 2019 20:55:51 -0700 Subject: [PATCH] Support overloaded setters when binding beans Update `JavaBeanBinder` so that overloaded setters can be used when binding. Prior to this commit the setter picked would depend on the order that the JVM returned the declared methods. We now consistently prefer using the setter with a parameter type that matches the getter. Closes gh-16206 --- .../properties/bind/JavaBeanBinder.java | 56 +++++++++----- .../properties/bind/JavaBeanBinderTests.java | 76 ++++++++++++++++++- 2 files changed, 110 insertions(+), 22 deletions(-) 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 5af02175f19..c307c3a56ab 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 240c20e8381..4055e28373c 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 @@ -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. @@ -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 { @@ -505,6 +511,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; @@ -955,4 +1011,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; + } + + } + }