From e74f868a2397def6b3f25c96fe08fcec36c36e26 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Mon, 13 Apr 2020 15:11:23 +0200 Subject: [PATCH] Revise generics support in BeanUtils.copyProperties() Closes gh-24187 --- .../org/springframework/beans/BeanUtils.java | 47 +++++++--------- .../springframework/beans/BeanUtilsTests.java | 56 +++++++++++++------ 2 files changed, 60 insertions(+), 43 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java b/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java index a6730b3243..99cb076903 100644 --- a/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java +++ b/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java @@ -19,7 +19,6 @@ package org.springframework.beans; import java.beans.PropertyDescriptor; import java.beans.PropertyEditor; import java.lang.reflect.Constructor; -import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Modifier; @@ -46,7 +45,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.core.KotlinDetector; import org.springframework.core.MethodParameter; -import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.ResolvableType; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -683,11 +682,12 @@ public abstract class BeanUtils { } /** - * Copy the property values of the given source bean into the given target bean - * and ignored if + * Copy the property values of the given source bean into the given target bean. *

Note: The source and target classes do not have to match or even be derived * from each other, as long as the properties match. Any bean properties that the * source bean exposes but the target bean does not will silently be ignored. + *

As of Spring Framework 5.3, this method honors generic type information + * when matching properties in the source and target objects. * @param source the source bean * @param target the target bean * @param editable the class (or interface) to restrict property setting to @@ -717,30 +717,25 @@ public abstract class BeanUtils { if (writeMethod != null && (ignoreList == null || !ignoreList.contains(targetPd.getName()))) { PropertyDescriptor sourcePd = getPropertyDescriptor(source.getClass(), targetPd.getName()); if (sourcePd != null) { - Field sourcefield = ReflectionUtils.findField(source.getClass(), sourcePd.getName()); - Field targetfield = ReflectionUtils.findField(target.getClass(), targetPd.getName()); - - TypeDescriptor sourceTypeDescriptor = new TypeDescriptor(sourcefield); - TypeDescriptor targetTypeDescriptor = new TypeDescriptor(targetfield); - Method readMethod = sourcePd.getReadMethod(); - - if (readMethod != null && - ClassUtils.isAssignable(writeMethod.getParameterTypes()[0], readMethod.getReturnType()) && - sourceTypeDescriptor.getResolvableType().equals(targetTypeDescriptor.getResolvableType())) { - try { - if (!Modifier.isPublic(readMethod.getDeclaringClass().getModifiers())) { - readMethod.setAccessible(true); + if (readMethod != null) { + ResolvableType sourceResolvableType = ResolvableType.forMethodReturnType(readMethod); + ResolvableType targetResolvableType = ResolvableType.forMethodParameter(writeMethod, 0); + if (targetResolvableType.isAssignableFrom(sourceResolvableType)) { + try { + if (!Modifier.isPublic(readMethod.getDeclaringClass().getModifiers())) { + readMethod.setAccessible(true); + } + Object value = readMethod.invoke(source); + if (!Modifier.isPublic(writeMethod.getDeclaringClass().getModifiers())) { + writeMethod.setAccessible(true); + } + writeMethod.invoke(target, value); + } + catch (Throwable ex) { + throw new FatalBeanException( + "Could not copy property '" + targetPd.getName() + "' from source to target", ex); } - Object value = readMethod.invoke(source); - if (!Modifier.isPublic(writeMethod.getDeclaringClass().getModifiers())) { - writeMethod.setAccessible(true); - } - writeMethod.invoke(target, value); - } - catch (Throwable ex) { - throw new FatalBeanException( - "Could not copy property '" + targetPd.getName() + "' from source to target", ex); } } } diff --git a/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java b/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java index 50dd7a3c74..1590e9f90d 100644 --- a/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java @@ -174,6 +174,28 @@ class BeanUtilsTests { assertThat(tb2.getTouchy().equals(tb.getTouchy())).as("Touchy copied").isTrue(); } + @Test + void copyPropertiesHonorsGenericTypeMatches() { + IntegerListHolder1 integerListHolder1 = new IntegerListHolder1(); + integerListHolder1.getList().add(42); + IntegerListHolder2 integerListHolder2 = new IntegerListHolder2(); + + BeanUtils.copyProperties(integerListHolder1, integerListHolder2); + assertThat(integerListHolder1.getList()).containsOnly(42); + assertThat(integerListHolder2.getList()).containsOnly(42); + } + + @Test + void copyPropertiesDoesNotHonorGenericTypeMismatches() { + IntegerListHolder1 integerListHolder = new IntegerListHolder1(); + integerListHolder.getList().add(42); + LongListHolder longListHolder = new LongListHolder(); + + BeanUtils.copyProperties(integerListHolder, longListHolder); + assertThat(integerListHolder.getList()).containsOnly(42); + assertThat(longListHolder.getList()).isEmpty(); + } + @Test void testCopyPropertiesWithEditable() throws Exception { TestBean tb = new TestBean(); @@ -337,24 +359,24 @@ class BeanUtilsTests { private void assertSignatureEquals(Method desiredMethod, String signature) { assertThat(BeanUtils.resolveSignature(signature, MethodSignatureBean.class)).isEqualTo(desiredMethod); } - - @Test - void testCopiedParametersType() { - - A a = new A(); - a.getList().add(42); - B b = new B(); - BeanUtils.copyProperties(a, b); - assertThat(a.getList()).containsOnly(42); + @SuppressWarnings("unused") + private static class IntegerListHolder1 { + + private List list = new ArrayList<>(); - b.getList().forEach(n -> assertThat(n).isInstanceOf(Long.class)); - assertThat(b.getList()).isEmpty(); - + public List getList() { + return list; + } + + public void setList(List list) { + this.list = list; + } } - - class A { + + @SuppressWarnings("unused") + private static class IntegerListHolder2 { private List list = new ArrayList<>(); @@ -365,9 +387,10 @@ class BeanUtilsTests { public void setList(List list) { this.list = list; } - } - class B { + + @SuppressWarnings("unused") + private static class LongListHolder { private List list = new ArrayList<>(); @@ -378,7 +401,6 @@ class BeanUtilsTests { public void setList(List list) { this.list = list; } - }