From 887389d3413c4a1036095cff6f9cdebb34af87fe Mon Sep 17 00:00:00 2001
From: Sam Brannen <sbrannen@vmware.com>
Date: Sat, 12 Mar 2022 18:12:23 +0100
Subject: [PATCH] Clarify behavior for generics support in
 BeanUtils.copyProperties()

Since Spring Framework 5.3, BeanUtils.copyProperties() honors generics
in the source and target property types (see gh-24187); however, this
refinement of the contract was not properly documented prior to this
commit. In addition, the refinement can be a breaking change for users
who were relying on the previous unreliable behavior.

This commit therefore clarifies the behavior for generics support in
BeanUtils.copyProperties() and introduces a table of example matches
and mismatches when generics are involved.

Closes gh-27259
---
 .../org/springframework/beans/BeanUtils.java  |  35 +++-
 .../springframework/beans/BeanUtilsTests.java | 181 +++++++++++++++++-
 2 files changed, 209 insertions(+), 7 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 77aff7ffe08..61b9541a12d 100644
--- a/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java
+++ b/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2021 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -691,7 +691,25 @@ public abstract class BeanUtils {
 	 * 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>This is just a convenience method. For more complex transfer needs,
-	 * consider using a full BeanWrapper.
+	 * consider using a full {@link BeanWrapper}.
+	 * <p>As of Spring Framework 5.3, this method honors generic type information
+	 * when matching properties in the source and target objects.
+	 * <p>The following table provides a non-exhaustive set of examples of source
+	 * and target property types that can be copied as well as source and target
+	 * property types that cannot be copied.
+	 * <table border="1">
+	 * <tr><th>source property type</th><th>target property type</th><th>copy supported</th></tr>
+	 * <tr><td>{@code Integer}</td><td>{@code Integer}</td><td>yes</td></tr>
+	 * <tr><td>{@code Integer}</td><td>{@code Number}</td><td>yes</td></tr>
+	 * <tr><td>{@code List<Integer>}</td><td>{@code List<Integer>}</td><td>yes</td></tr>
+	 * <tr><td>{@code List<?>}</td><td>{@code List<?>}</td><td>yes</td></tr>
+	 * <tr><td>{@code List<Integer>}</td><td>{@code List<?>}</td><td>yes</td></tr>
+	 * <tr><td>{@code List<Integer>}</td><td>{@code List<? extends Number>}</td><td>yes</td></tr>
+	 * <tr><td>{@code String}</td><td>{@code Integer}</td><td>no</td></tr>
+	 * <tr><td>{@code Number}</td><td>{@code Integer}</td><td>no</td></tr>
+	 * <tr><td>{@code List<Integer>}</td><td>{@code List<Long>}</td><td>no</td></tr>
+	 * <tr><td>{@code List<Integer>}</td><td>{@code List<Number>}</td><td>no</td></tr>
+	 * </table>
 	 * @param source the source bean
 	 * @param target the target bean
 	 * @throws BeansException if the copying failed
@@ -708,7 +726,10 @@ public abstract class BeanUtils {
 	 * 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>This is just a convenience method. For more complex transfer needs,
-	 * consider using a full BeanWrapper.
+	 * consider using a full {@link BeanWrapper}.
+	 * <p>As of Spring Framework 5.3, this method honors generic type information
+	 * when matching properties in the source and target objects. See the
+	 * documentation for {@link #copyProperties(Object, Object)} for details.
 	 * @param source the source bean
 	 * @param target the target bean
 	 * @param editable the class (or interface) to restrict property setting to
@@ -726,7 +747,10 @@ public abstract class BeanUtils {
 	 * 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>This is just a convenience method. For more complex transfer needs,
-	 * consider using a full BeanWrapper.
+	 * consider using a full {@link BeanWrapper}.
+	 * <p>As of Spring Framework 5.3, this method honors generic type information
+	 * when matching properties in the source and target objects. See the
+	 * documentation for {@link #copyProperties(Object, Object)} for details.
 	 * @param source the source bean
 	 * @param target the target bean
 	 * @param ignoreProperties array of property names to ignore
@@ -743,7 +767,8 @@ public abstract class BeanUtils {
 	 * 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.
+	 * when matching properties in the source and target objects. See the
+	 * documentation for {@link #copyProperties(Object, Object)} for details.
 	 * @param source the source bean
 	 * @param target the target bean
 	 * @param editable the class (or interface) to restrict property setting to
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 b90fd515dc3..711189cdf1c 100644
--- a/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java
+++ b/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2021 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -203,8 +203,25 @@ class BeanUtilsTests {
 		assertThat(tb2.getTouchy().equals(tb.getTouchy())).as("Touchy copied").isTrue();
 	}
 
+	/**
+	 * {@code Integer} can be copied to {@code Number}.
+	 */
 	@Test
-	void copyPropertiesHonorsGenericTypeMatches() {
+	void copyPropertiesFromSubTypeToSuperType() {
+		IntegerHolder integerHolder = new IntegerHolder();
+		integerHolder.setNumber(42);
+		NumberHolder numberHolder = new NumberHolder();
+
+		BeanUtils.copyProperties(integerHolder, numberHolder);
+		assertThat(integerHolder.getNumber()).isEqualTo(42);
+		assertThat(numberHolder.getNumber()).isEqualTo(42);
+	}
+
+	/**
+	 * {@code List<Integer>} can be copied to {@code List<Integer>}.
+	 */
+	@Test
+	void copyPropertiesHonorsGenericTypeMatchesFromIntegerToInteger() {
 		IntegerListHolder1 integerListHolder1 = new IntegerListHolder1();
 		integerListHolder1.getList().add(42);
 		IntegerListHolder2 integerListHolder2 = new IntegerListHolder2();
@@ -214,6 +231,68 @@ class BeanUtilsTests {
 		assertThat(integerListHolder2.getList()).containsOnly(42);
 	}
 
+	/**
+	 * {@code List<?>} can be copied to {@code List<?>}.
+	 */
+	@Test
+	void copyPropertiesHonorsGenericTypeMatchesFromWildcardToWildcard() {
+		List<?> list = Arrays.asList("foo", 42);
+		WildcardListHolder1 wildcardListHolder1 = new WildcardListHolder1();
+		wildcardListHolder1.setList(list);
+		WildcardListHolder2 wildcardListHolder2 = new WildcardListHolder2();
+		assertThat(wildcardListHolder2.getList()).isEmpty();
+
+		BeanUtils.copyProperties(wildcardListHolder1, wildcardListHolder2);
+		assertThat(wildcardListHolder1.getList()).isEqualTo(list);
+		assertThat(wildcardListHolder2.getList()).isEqualTo(list);
+	}
+
+	/**
+	 * {@code List<Integer>} can be copied to {@code List<?>}.
+	 */
+	@Test
+	void copyPropertiesHonorsGenericTypeMatchesFromIntegerToWildcard() {
+		IntegerListHolder1 integerListHolder1 = new IntegerListHolder1();
+		integerListHolder1.getList().add(42);
+		WildcardListHolder2 wildcardListHolder2 = new WildcardListHolder2();
+
+		BeanUtils.copyProperties(integerListHolder1, wildcardListHolder2);
+		assertThat(integerListHolder1.getList()).containsOnly(42);
+		assertThat(wildcardListHolder2.getList()).isEqualTo(Arrays.asList(42));
+	}
+
+	/**
+	 * {@code List<Integer>} can be copied to {@code List<? extends Number>}.
+	 */
+	@Test
+	void copyPropertiesHonorsGenericTypeMatchesForUpperBoundedWildcard() {
+		IntegerListHolder1 integerListHolder1 = new IntegerListHolder1();
+		integerListHolder1.getList().add(42);
+		NumberUpperBoundedWildcardListHolder numberListHolder = new NumberUpperBoundedWildcardListHolder();
+
+		BeanUtils.copyProperties(integerListHolder1, numberListHolder);
+		assertThat(integerListHolder1.getList()).containsOnly(42);
+		assertThat(numberListHolder.getList()).hasSize(1);
+		assertThat(numberListHolder.getList().contains(Integer.valueOf(42))).isTrue();
+	}
+
+	/**
+	 * {@code Number} can NOT be copied to {@code Integer}.
+	 */
+	@Test
+	void copyPropertiesDoesNotCopyeFromSuperTypeToSubType() {
+		NumberHolder numberHolder = new NumberHolder();
+		numberHolder.setNumber(Integer.valueOf(42));
+		IntegerHolder integerHolder = new IntegerHolder();
+
+		BeanUtils.copyProperties(numberHolder, integerHolder);
+		assertThat(numberHolder.getNumber()).isEqualTo(42);
+		assertThat(integerHolder.getNumber()).isNull();
+	}
+
+	/**
+	 * {@code List<Integer>} can NOT be copied to {@code List<Long>}.
+	 */
 	@Test
 	void copyPropertiesDoesNotHonorGenericTypeMismatches() {
 		IntegerListHolder1 integerListHolder = new IntegerListHolder1();
@@ -225,6 +304,20 @@ class BeanUtilsTests {
 		assertThat(longListHolder.getList()).isEmpty();
 	}
 
+	/**
+	 * {@code List<Integer>} can NOT be copied to {@code List<Number>}.
+	 */
+	@Test
+	void copyPropertiesDoesNotHonorGenericTypeMismatchesFromSubTypeToSuperType() {
+		IntegerListHolder1 integerListHolder = new IntegerListHolder1();
+		integerListHolder.getList().add(42);
+		NumberListHolder numberListHolder = new NumberListHolder();
+
+		BeanUtils.copyProperties(integerListHolder, numberListHolder);
+		assertThat(integerListHolder.getList()).containsOnly(42);
+		assertThat(numberListHolder.getList()).isEmpty();
+	}
+
 	@Test  // gh-26531
 	void copyPropertiesIgnoresGenericsIfSourceOrTargetHasUnresolvableGenerics() throws Exception {
 		Order original = new Order("test", Arrays.asList("foo", "bar"));
@@ -413,6 +506,90 @@ class BeanUtilsTests {
 	}
 
 
+	@SuppressWarnings("unused")
+	private static class NumberHolder {
+
+		private Number number;
+
+		public Number getNumber() {
+			return number;
+		}
+
+		public void setNumber(Number number) {
+			this.number = number;
+		}
+	}
+
+	@SuppressWarnings("unused")
+	private static class IntegerHolder {
+
+		private Integer number;
+
+		public Integer getNumber() {
+			return number;
+		}
+
+		public void setNumber(Integer number) {
+			this.number = number;
+		}
+	}
+
+	@SuppressWarnings("unused")
+	private static class WildcardListHolder1 {
+
+		private List<?> list = new ArrayList<>();
+
+		public List<?> getList() {
+			return list;
+		}
+
+		public void setList(List<?> list) {
+			this.list = list;
+		}
+	}
+
+	@SuppressWarnings("unused")
+	private static class WildcardListHolder2 {
+
+		private List<?> list = new ArrayList<>();
+
+		public List<?> getList() {
+			return list;
+		}
+
+		public void setList(List<?> list) {
+			this.list = list;
+		}
+	}
+
+	@SuppressWarnings("unused")
+	private static class NumberUpperBoundedWildcardListHolder {
+
+		private List<? extends Number> list = new ArrayList<>();
+
+		public List<? extends Number> getList() {
+			return list;
+		}
+
+		public void setList(List<? extends Number> list) {
+			this.list = list;
+		}
+	}
+
+	@SuppressWarnings("unused")
+	private static class NumberListHolder {
+
+		private List<Number> list = new ArrayList<>();
+
+		public List<Number> getList() {
+			return list;
+		}
+
+		public void setList(List<Number> list) {
+			this.list = list;
+		}
+	}
+
 	@SuppressWarnings("unused")
 	private static class IntegerListHolder1 {