Test detection of original generic method for CGLIB bridge method

Includes isBridgedCandidateFor optimization (aligned with 6.0.x)

See gh-32888

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * Copyright 2002-2024 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.
@@ -42,6 +42,8 @@ import org.springframework.beans.propertyeditors.CustomDateEditor;
 import org.springframework.beans.testfixture.beans.DerivedTestBean;
 import org.springframework.beans.testfixture.beans.ITestBean;
 import org.springframework.beans.testfixture.beans.TestBean;
+import org.springframework.cglib.proxy.Enhancer;
+import org.springframework.cglib.proxy.MethodInterceptor;
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.ResourceEditor;
 import org.springframework.lang.Nullable;
@@ -52,7 +54,7 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException
 import static org.assertj.core.api.SoftAssertions.assertSoftly;
 
 /**
- * Unit tests for {@link BeanUtils}.
+ * Tests for {@link BeanUtils}.
  *
  * @author Juergen Hoeller
  * @author Rob Harrop
@@ -136,7 +138,7 @@ class BeanUtilsTests {
 		PropertyDescriptor[] actual = Introspector.getBeanInfo(TestBean.class).getPropertyDescriptors();
 		PropertyDescriptor[] descriptors = BeanUtils.getPropertyDescriptors(TestBean.class);
 		assertThat(descriptors).as("Descriptors should not be null").isNotNull();
-		assertThat(descriptors.length).as("Invalid number of descriptors returned").isEqualTo(actual.length);
+		assertThat(descriptors).as("Invalid number of descriptors returned").hasSameSizeAs(actual);
 	}
 
 	@Test
@@ -162,13 +164,13 @@ class BeanUtilsTests {
 		tb.setAge(32);
 		tb.setTouchy("touchy");
 		TestBean tb2 = new TestBean();
-		assertThat(tb2.getName() == null).as("Name empty").isTrue();
-		assertThat(tb2.getAge() == 0).as("Age empty").isTrue();
-		assertThat(tb2.getTouchy() == null).as("Touchy empty").isTrue();
+		assertThat(tb2.getName()).as("Name empty").isNull();
+		assertThat(tb2.getAge()).as("Age empty").isEqualTo(0);
+		assertThat(tb2.getTouchy()).as("Touchy empty").isNull();
 		BeanUtils.copyProperties(tb, tb2);
-		assertThat(tb2.getName().equals(tb.getName())).as("Name copied").isTrue();
-		assertThat(tb2.getAge() == tb.getAge()).as("Age copied").isTrue();
-		assertThat(tb2.getTouchy().equals(tb.getTouchy())).as("Touchy copied").isTrue();
+		assertThat(tb2.getName()).as("Name copied").isEqualTo(tb.getName());
+		assertThat(tb2.getAge()).as("Age copied").isEqualTo(tb.getAge());
+		assertThat(tb2.getTouchy()).as("Touchy copied").isEqualTo(tb.getTouchy());
 	}
 
 	@Test
@@ -178,13 +180,13 @@ class BeanUtilsTests {
 		tb.setAge(32);
 		tb.setTouchy("touchy");
 		TestBean tb2 = new TestBean();
-		assertThat(tb2.getName() == null).as("Name empty").isTrue();
-		assertThat(tb2.getAge() == 0).as("Age empty").isTrue();
-		assertThat(tb2.getTouchy() == null).as("Touchy empty").isTrue();
+		assertThat(tb2.getName()).as("Name empty").isNull();
+		assertThat(tb2.getAge()).as("Age empty").isEqualTo(0);
+		assertThat(tb2.getTouchy()).as("Touchy empty").isNull();
 		BeanUtils.copyProperties(tb, tb2);
-		assertThat(tb2.getName().equals(tb.getName())).as("Name copied").isTrue();
-		assertThat(tb2.getAge() == tb.getAge()).as("Age copied").isTrue();
-		assertThat(tb2.getTouchy().equals(tb.getTouchy())).as("Touchy copied").isTrue();
+		assertThat(tb2.getName()).as("Name copied").isEqualTo(tb.getName());
+		assertThat(tb2.getAge()).as("Age copied").isEqualTo(tb.getAge());
+		assertThat(tb2.getTouchy()).as("Touchy copied").isEqualTo(tb.getTouchy());
 	}
 
 	@Test
@@ -194,13 +196,13 @@ class BeanUtilsTests {
 		tb.setAge(32);
 		tb.setTouchy("touchy");
 		DerivedTestBean tb2 = new DerivedTestBean();
-		assertThat(tb2.getName() == null).as("Name empty").isTrue();
-		assertThat(tb2.getAge() == 0).as("Age empty").isTrue();
-		assertThat(tb2.getTouchy() == null).as("Touchy empty").isTrue();
+		assertThat(tb2.getName()).as("Name empty").isNull();
+		assertThat(tb2.getAge()).as("Age empty").isEqualTo(0);
+		assertThat(tb2.getTouchy()).as("Touchy empty").isNull();
 		BeanUtils.copyProperties(tb, tb2);
-		assertThat(tb2.getName().equals(tb.getName())).as("Name copied").isTrue();
-		assertThat(tb2.getAge() == tb.getAge()).as("Age copied").isTrue();
-		assertThat(tb2.getTouchy().equals(tb.getTouchy())).as("Touchy copied").isTrue();
+		assertThat(tb2.getName()).as("Name copied").isEqualTo(tb.getName());
+		assertThat(tb2.getAge()).as("Age copied").isEqualTo(tb.getAge());
+		assertThat(tb2.getTouchy()).as("Touchy copied").isEqualTo(tb.getTouchy());
 	}
 
 	/**
@@ -227,8 +229,8 @@ class BeanUtilsTests {
 		IntegerListHolder2 integerListHolder2 = new IntegerListHolder2();
 
 		BeanUtils.copyProperties(integerListHolder1, integerListHolder2);
-		assertThat(integerListHolder1.getList()).containsOnly(42);
-		assertThat(integerListHolder2.getList()).containsOnly(42);
+		assertThat(integerListHolder1.getList()).containsExactly(42);
+		assertThat(integerListHolder2.getList()).containsExactly(42);
 	}
 
 	/**
@@ -257,7 +259,7 @@ class BeanUtilsTests {
 		WildcardListHolder2 wildcardListHolder2 = new WildcardListHolder2();
 
 		BeanUtils.copyProperties(integerListHolder1, wildcardListHolder2);
-		assertThat(integerListHolder1.getList()).containsOnly(42);
+		assertThat(integerListHolder1.getList()).containsExactly(42);
 		assertThat(wildcardListHolder2.getList()).isEqualTo(Arrays.asList(42));
 	}
 
@@ -271,9 +273,8 @@ class BeanUtilsTests {
 		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();
+		assertThat(integerListHolder1.getList()).containsExactly(42);
+		assertThat(numberListHolder.getList()).isEqualTo(Arrays.asList(42));
 	}
 
 	/**
@@ -282,7 +283,7 @@ class BeanUtilsTests {
 	@Test
 	void copyPropertiesDoesNotCopyFromSuperTypeToSubType() {
 		NumberHolder numberHolder = new NumberHolder();
-		numberHolder.setNumber(Integer.valueOf(42));
+		numberHolder.setNumber(42);
 		IntegerHolder integerHolder = new IntegerHolder();
 
 		BeanUtils.copyProperties(numberHolder, integerHolder);
@@ -300,7 +301,7 @@ class BeanUtilsTests {
 		LongListHolder longListHolder = new LongListHolder();
 
 		BeanUtils.copyProperties(integerListHolder, longListHolder);
-		assertThat(integerListHolder.getList()).containsOnly(42);
+		assertThat(integerListHolder.getList()).containsExactly(42);
 		assertThat(longListHolder.getList()).isEmpty();
 	}
 
@@ -314,7 +315,7 @@ class BeanUtilsTests {
 		NumberListHolder numberListHolder = new NumberListHolder();
 
 		BeanUtils.copyProperties(integerListHolder, numberListHolder);
-		assertThat(integerListHolder.getList()).containsOnly(42);
+		assertThat(integerListHolder.getList()).containsExactly(42);
 		assertThat(numberListHolder.getList()).isEmpty();
 	}
 
@@ -323,12 +324,13 @@ class BeanUtilsTests {
 		Order original = new Order("test", Arrays.asList("foo", "bar"));
 
 		// Create a Proxy that loses the generic type information for the getLineItems() method.
-		OrderSummary proxy = proxyOrder(original);
+		OrderSummary proxy = (OrderSummary) Proxy.newProxyInstance(getClass().getClassLoader(),
+				new Class<?>[] {OrderSummary.class}, new OrderInvocationHandler(original));
 		assertThat(OrderSummary.class.getDeclaredMethod("getLineItems").toGenericString())
-			.contains("java.util.List<java.lang.String>");
+				.contains("java.util.List<java.lang.String>");
 		assertThat(proxy.getClass().getDeclaredMethod("getLineItems").toGenericString())
-			.contains("java.util.List")
-			.doesNotContain("<java.lang.String>");
+				.contains("java.util.List")
+				.doesNotContain("<java.lang.String>");
 
 		// Ensure that our custom Proxy works as expected.
 		assertThat(proxy.getId()).isEqualTo("test");
@@ -341,40 +343,57 @@ class BeanUtilsTests {
 		assertThat(target.getLineItems()).containsExactly("foo", "bar");
 	}
 
+	@Test  // gh-32888
+	public void copyPropertiesWithGenericCglibClass() {
+		Enhancer enhancer = new Enhancer();
+		enhancer.setSuperclass(User.class);
+		enhancer.setCallback((MethodInterceptor) (obj, method, args, proxy) -> proxy.invokeSuper(obj, args));
+		User user = (User) enhancer.create();
+		user.setId(1);
+		user.setName("proxy");
+		user.setAddress("addr");
+
+		User target = new User();
+		BeanUtils.copyProperties(user, target);
+		assertThat(target.getId()).isEqualTo(user.getId());
+		assertThat(target.getName()).isEqualTo(user.getName());
+		assertThat(target.getAddress()).isEqualTo(user.getAddress());
+	}
+
 	@Test
 	void copyPropertiesWithEditable() throws Exception {
 		TestBean tb = new TestBean();
-		assertThat(tb.getName() == null).as("Name empty").isTrue();
+		assertThat(tb.getName()).as("Name empty").isNull();
 		tb.setAge(32);
 		tb.setTouchy("bla");
 		TestBean tb2 = new TestBean();
 		tb2.setName("rod");
-		assertThat(tb2.getAge() == 0).as("Age empty").isTrue();
-		assertThat(tb2.getTouchy() == null).as("Touchy empty").isTrue();
+		assertThat(tb2.getAge()).as("Age empty").isEqualTo(0);
+		assertThat(tb2.getTouchy()).as("Touchy empty").isNull();
 
 		// "touchy" should not be copied: it's not defined in ITestBean
 		BeanUtils.copyProperties(tb, tb2, ITestBean.class);
-		assertThat(tb2.getName() == null).as("Name copied").isTrue();
-		assertThat(tb2.getAge() == 32).as("Age copied").isTrue();
-		assertThat(tb2.getTouchy() == null).as("Touchy still empty").isTrue();
+		assertThat(tb2.getName()).as("Name copied").isNull();
+		assertThat(tb2.getAge()).as("Age copied").isEqualTo(32);
+		assertThat(tb2.getTouchy()).as("Touchy still empty").isNull();
 	}
 
 	@Test
 	void copyPropertiesWithIgnore() throws Exception {
 		TestBean tb = new TestBean();
-		assertThat(tb.getName() == null).as("Name empty").isTrue();
+		assertThat(tb.getName()).as("Name empty").isNull();
 		tb.setAge(32);
 		tb.setTouchy("bla");
 		TestBean tb2 = new TestBean();
 		tb2.setName("rod");
-		assertThat(tb2.getAge() == 0).as("Age empty").isTrue();
-		assertThat(tb2.getTouchy() == null).as("Touchy empty").isTrue();
+		assertThat(tb2.getAge()).as("Age empty").isEqualTo(0);
+		assertThat(tb2.getTouchy()).as("Touchy empty").isNull();
 
 		// "spouse", "touchy", "age" should not be copied
 		BeanUtils.copyProperties(tb, tb2, "spouse", "touchy", "age");
-		assertThat(tb2.getName() == null).as("Name copied").isTrue();
-		assertThat(tb2.getAge() == 0).as("Age still empty").isTrue();
-		assertThat(tb2.getTouchy() == null).as("Touchy still empty").isTrue();
+		assertThat(tb2.getName()).as("Name copied").isNull();
+		assertThat(tb2.getAge()).as("Age still empty").isEqualTo(0);
+		assertThat(tb2.getTouchy()).as("Touchy still empty").isNull();
 	}
 
 	@Test
@@ -383,7 +402,7 @@ class BeanUtilsTests {
 		source.setName("name");
 		TestBean target = new TestBean();
 		BeanUtils.copyProperties(source, target, "specialProperty");
-		assertThat("name").isEqualTo(target.getName());
+		assertThat(target.getName()).isEqualTo("name");
 	}
 
 	@Test
@@ -520,6 +539,7 @@ class BeanUtilsTests {
 		}
 	}
 
+
 	@SuppressWarnings("unused")
 	private static class IntegerHolder {
 
@@ -534,6 +554,7 @@ class BeanUtilsTests {
 		}
 	}
 
+
 	@SuppressWarnings("unused")
 	private static class WildcardListHolder1 {
 
@@ -548,6 +569,7 @@ class BeanUtilsTests {
 		}
 	}
 
+
 	@SuppressWarnings("unused")
 	private static class WildcardListHolder2 {
 
@@ -562,6 +584,7 @@ class BeanUtilsTests {
 		}
 	}
 
+
 	@SuppressWarnings("unused")
 	private static class NumberUpperBoundedWildcardListHolder {
 
@@ -576,6 +599,7 @@ class BeanUtilsTests {
 		}
 	}
 
+
 	@SuppressWarnings("unused")
 	private static class NumberListHolder {
 
@@ -590,6 +614,7 @@ class BeanUtilsTests {
 		}
 	}
 
+
 	@SuppressWarnings("unused")
 	private static class IntegerListHolder1 {
 
@@ -604,6 +629,7 @@ class BeanUtilsTests {
 		}
 	}
 
+
 	@SuppressWarnings("unused")
 	private static class IntegerListHolder2 {
 
@@ -618,6 +644,7 @@ class BeanUtilsTests {
 		}
 	}
 
+
 	@SuppressWarnings("unused")
 	private static class LongListHolder {
 
@@ -798,6 +825,7 @@ class BeanUtilsTests {
 		}
 	}
 
+
 	private static class BeanWithNullableTypes {
 
 		private Integer counter;
@@ -828,6 +856,7 @@ class BeanUtilsTests {
 		}
 	}
 
+
 	private static class BeanWithPrimitiveTypes {
 
 		private boolean flag;
@@ -840,7 +869,6 @@ class BeanUtilsTests {
 		private char character;
 		private String text;
 
-
 		@SuppressWarnings("unused")
 		public BeanWithPrimitiveTypes(boolean flag, byte byteCount, short shortCount, int intCount, long longCount,
 				float floatCount, double doubleCount, char character, String text) {
@@ -891,21 +919,22 @@ class BeanUtilsTests {
 		public String getText() {
 			return text;
 		}
-
 	}
 
+
 	private static class PrivateBeanWithPrivateConstructor {
 
 		private PrivateBeanWithPrivateConstructor() {
 		}
 	}
 
+
 	@SuppressWarnings("unused")
 	private static class Order {
 
 		private String id;
-		private List<String> lineItems;
 
+		private List<String> lineItems;
 
 		Order() {
 		}
@@ -937,6 +966,7 @@ class BeanUtilsTests {
 		}
 	}
 
+
 	private interface OrderSummary {
 
 		String getId();
@@ -945,17 +975,10 @@ class BeanUtilsTests {
 	}
 
 
-	private OrderSummary proxyOrder(Order order) {
-		return (OrderSummary) Proxy.newProxyInstance(getClass().getClassLoader(),
-			new Class<?>[] { OrderSummary.class }, new OrderInvocationHandler(order));
-	}
-
-
 	private static class OrderInvocationHandler implements InvocationHandler {
 
 		private final Order order;
 
-
 		OrderInvocationHandler(Order order) {
 			this.order = order;
 		}
@@ -973,4 +996,46 @@ class BeanUtilsTests {
 		}
 	}
 
+
+	private static class GenericBaseModel<T> {
+
+		private T id;
+
+		private String name;
+
+		public T getId() {
+			return id;
+		}
+
+		public void setId(T id) {
+			this.id = id;
+		}
+
+		public String getName() {
+			return name;
+		}
+
+		public void setName(String name) {
+			this.name = name;
+		}
+	}
+
+
+	private static class User extends GenericBaseModel<Integer> {
+
+		private String address;
+
+		public User() {
+			super();
+		}
+
+		public String getAddress() {
+			return address;
+		}
+
+		public void setAddress(String address) {
+			this.address = address;
+		}
+	}
+
 }

spring-core/src/main/java/org/springframework/core/BridgeMethodResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * Copyright 2002-2024 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.
@@ -57,11 +57,11 @@ public final class BridgeMethodResolver {
 
 
 	/**
-	 * Find the original method for the supplied {@link Method bridge Method}.
+	 * Find the local original method for the supplied {@link Method bridge Method}.
 	 * <p>It is safe to call this method passing in a non-bridge {@link Method} instance.
 	 * In such a case, the supplied {@link Method} instance is returned directly to the caller.
 	 * Callers are <strong>not</strong> required to check for bridging before calling this method.
-	 * @param bridgeMethod the method to introspect
+	 * @param bridgeMethod the method to introspect against its declaring class
 	 * @return the original method (either the bridged method or the passed-in method
 	 * if no more specific one could be found)
 	 */
@@ -73,8 +73,7 @@ public final class BridgeMethodResolver {
 		if (bridgedMethod == null) {
 			// Gather all methods with matching name and parameter size.
 			List<Method> candidateMethods = new ArrayList<>();
-			MethodFilter filter = candidateMethod ->
-					isBridgedCandidateFor(candidateMethod, bridgeMethod);
+			MethodFilter filter = (candidateMethod -> isBridgedCandidateFor(candidateMethod, bridgeMethod));
 			ReflectionUtils.doWithMethods(bridgeMethod.getDeclaringClass(), candidateMethods::add, filter);
 			if (!candidateMethods.isEmpty()) {
 				bridgedMethod = candidateMethods.size() == 1 ?
@@ -95,10 +94,10 @@ public final class BridgeMethodResolver {
 	 * Returns {@code true} if the supplied '{@code candidateMethod}' can be
 	 * considered a valid candidate for the {@link Method} that is {@link Method#isBridge() bridged}
 	 * by the supplied {@link Method bridge Method}. This method performs inexpensive
-	 * checks and can be used quickly to filter for a set of possible matches.
+	 * checks and can be used to quickly filter for a set of possible matches.
 	 */
 	private static boolean isBridgedCandidateFor(Method candidateMethod, Method bridgeMethod) {
-		return (!candidateMethod.isBridge() && !candidateMethod.equals(bridgeMethod) &&
+		return (!candidateMethod.isBridge() &&
 				candidateMethod.getName().equals(bridgeMethod.getName()) &&
 				candidateMethod.getParameterCount() == bridgeMethod.getParameterCount());
 	}
@@ -121,8 +120,8 @@ public final class BridgeMethodResolver {
 				return candidateMethod;
 			}
 			else if (previousMethod != null) {
-				sameSig = sameSig &&
-						Arrays.equals(candidateMethod.getGenericParameterTypes(), previousMethod.getGenericParameterTypes());
+				sameSig = sameSig && Arrays.equals(
+						candidateMethod.getGenericParameterTypes(), previousMethod.getGenericParameterTypes());
 			}
 			previousMethod = candidateMethod;
 		}
@@ -163,7 +162,8 @@ public final class BridgeMethodResolver {
 				}
 			}
 			// A non-array type: compare the type itself.
-			if (!ClassUtils.resolvePrimitiveIfNecessary(candidateParameter).equals(ClassUtils.resolvePrimitiveIfNecessary(genericParameter.toClass()))) {
+			if (!ClassUtils.resolvePrimitiveIfNecessary(candidateParameter).equals(
+					ClassUtils.resolvePrimitiveIfNecessary(genericParameter.toClass()))) {
 				return false;
 			}
 		}
@@ -226,8 +226,8 @@ public final class BridgeMethodResolver {
 	/**
 	 * Compare the signatures of the bridge method and the method which it bridges. If
 	 * the parameter and return types are the same, it is a 'visibility' bridge method
-	 * introduced in Java 6 to fix https://bugs.openjdk.org/browse/JDK-6342411.
-	 * See also https://stas-blogspot.blogspot.com/2010/03/java-bridge-methods-explained.html
+	 * introduced in Java 6 to fix <a href="https://bugs.openjdk.org/browse/JDK-6342411">
+	 * JDK-6342411</a>.
 	 * @return whether signatures match as described
 	 */
 	public static boolean isVisibilityBridgeMethodPair(Method bridgeMethod, Method bridgedMethod) {