Revise generics support in BeanUtils.copyProperties()

Closes gh-24187

Author: sbrannen

spring-beans/src/main/java/org/springframework/beans/BeanUtils.java

@@ -19,7 +19,6 @@
 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.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 static void copyProperties(Object source, Object target, String... ignore
 	}
 
 	/**
-	 * 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.
 	 * <p>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.
+	 * <p>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 @@ private static void copyProperties(Object source, Object target, @Nullable Class
 			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);
 						}
 					}
 				}

spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java

@@ -174,6 +174,28 @@ void copyPropertiesWithDifferentTypes2() throws Exception {
 		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 @@ void isNotSimpleProperty(Class<?> type) {
 	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<Integer> list = new ArrayList<>();
 
-		b.getList().forEach(n -> assertThat(n).isInstanceOf(Long.class));
-		assertThat(b.getList()).isEmpty();	
-		
+		public List<Integer> getList() {
+			return list;
+		}
+
+		public void setList(List<Integer> list) {
+			this.list = list;
+		}
 	}
-	
-	class A {
+
+	@SuppressWarnings("unused")
+	private static class IntegerListHolder2 {
 
 		private List<Integer> list = new ArrayList<>();
 
@@ -365,9 +387,10 @@ public List<Integer> getList() {
 		public void setList(List<Integer> list) {
 			this.list = list;
 		}
-		
 	}
-	class B {
+
+	@SuppressWarnings("unused")
+	private static class LongListHolder {
 
 		private List<Long> list = new ArrayList<>();
 
@@ -378,7 +401,6 @@ public List<Long> getList() {
 		public void setList(List<Long> list) {
 			this.list = list;
 		}
-		
 	}