From 98aa03c0c9c1dc02a97d79404f3567b4f2bac310 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 24 May 2024 12:29:31 +0200 Subject: [PATCH] Test detection of original generic method for CGLIB bridge method Includes isBridgedCandidateFor optimization (aligned with 6.0.x) See gh-32888 --- .../springframework/beans/BeanUtilsTests.java | 179 ++++++++++++------ .../core/BridgeMethodResolver.java | 24 +-- 2 files changed, 134 insertions(+), 69 deletions(-) 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 7510c8bc2765..1a03af2bce9d 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-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.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.SoftAssertions.assertSoftly; /** - * Unit tests for {@link BeanUtils}. + * Tests for {@link BeanUtils}. * * @author Juergen Hoeller * @author Rob Harrop @@ -136,7 +138,7 @@ void getPropertyDescriptors() throws Exception { 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 @@ void copyProperties() throws Exception { 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 @@ void copyPropertiesWithDifferentTypes1() throws Exception { 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 @@ void copyPropertiesWithDifferentTypes2() throws Exception { 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 @@ void copyPropertiesHonorsGenericTypeMatchesFromIntegerToInteger() { 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 @@ void copyPropertiesHonorsGenericTypeMatchesFromIntegerToWildcard() { 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 @@ void copyPropertiesHonorsGenericTypeMatchesForUpperBoundedWildcard() { 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 @@ void copyPropertiesHonorsGenericTypeMatchesForUpperBoundedWildcard() { @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 @@ void copyPropertiesDoesNotHonorGenericTypeMismatches() { 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 @@ void copyPropertiesDoesNotHonorGenericTypeMismatchesFromSubTypeToSuperType() { 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 @@ void copyPropertiesIgnoresGenericsIfSourceOrTargetHasUnresolvableGenerics() thro 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"); + .contains("java.util.List"); assertThat(proxy.getClass().getDeclaredMethod("getLineItems").toGenericString()) - .contains("java.util.List") - .doesNotContain(""); + .contains("java.util.List") + .doesNotContain(""); // Ensure that our custom Proxy works as expected. assertThat(proxy.getId()).isEqualTo("test"); @@ -341,40 +343,57 @@ void copyPropertiesIgnoresGenericsIfSourceOrTargetHasUnresolvableGenerics() thro 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 @@ void copyPropertiesWithIgnoredNonExistingProperty() { 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 @@ public void setNumber(Number number) { } } + @SuppressWarnings("unused") private static class IntegerHolder { @@ -534,6 +554,7 @@ public void setNumber(Integer number) { } } + @SuppressWarnings("unused") private static class WildcardListHolder1 { @@ -548,6 +569,7 @@ public void setList(List list) { } } + @SuppressWarnings("unused") private static class WildcardListHolder2 { @@ -562,6 +584,7 @@ public void setList(List list) { } } + @SuppressWarnings("unused") private static class NumberUpperBoundedWildcardListHolder { @@ -576,6 +599,7 @@ public void setList(List list) { } } + @SuppressWarnings("unused") private static class NumberListHolder { @@ -590,6 +614,7 @@ public void setList(List list) { } } + @SuppressWarnings("unused") private static class IntegerListHolder1 { @@ -604,6 +629,7 @@ public void setList(List list) { } } + @SuppressWarnings("unused") private static class IntegerListHolder2 { @@ -618,6 +644,7 @@ public void setList(List list) { } } + @SuppressWarnings("unused") private static class LongListHolder { @@ -798,6 +825,7 @@ public void setValue(String aValue) { } } + private static class BeanWithNullableTypes { private Integer counter; @@ -828,6 +856,7 @@ public String getValue() { } } + private static class BeanWithPrimitiveTypes { private boolean flag; @@ -840,7 +869,6 @@ private static class BeanWithPrimitiveTypes { 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 @@ public char getCharacter() { public String getText() { return text; } - } + private static class PrivateBeanWithPrivateConstructor { private PrivateBeanWithPrivateConstructor() { } } + @SuppressWarnings("unused") private static class Order { private String id; - private List lineItems; + private List lineItems; Order() { } @@ -937,6 +966,7 @@ public String toString() { } } + private interface OrderSummary { String getId(); @@ -945,17 +975,10 @@ private interface OrderSummary { } - 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 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl } } + + private static class GenericBaseModel { + + 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 { + + private String address; + + public User() { + super(); + } + + public String getAddress() { + return address; + } + + public void setAddress(String address) { + this.address = address; + } + } + } diff --git a/spring-core/src/main/java/org/springframework/core/BridgeMethodResolver.java b/spring-core/src/main/java/org/springframework/core/BridgeMethodResolver.java index 016f0587ed77..811816478dd6 100644 --- a/spring-core/src/main/java/org/springframework/core/BridgeMethodResolver.java +++ b/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 @@ private BridgeMethodResolver() { /** - * Find the original method for the supplied {@link Method bridge Method}. + * Find the local original method for the supplied {@link Method bridge Method}. *

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 not 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 static Method findBridgedMethod(Method bridgeMethod) { if (bridgedMethod == null) { // Gather all methods with matching name and parameter size. List 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 static Method findBridgedMethod(Method bridgeMethod) { * 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 @@ private static Method searchCandidates(List candidateMethods, Method bri 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 @@ private static boolean isResolvedTypeMatch(Method genericMethod, Method candidat } } // 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 @@ private static Method searchForMatch(Class type, Method bridgeMethod) { /** * 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 + * JDK-6342411. * @return whether signatures match as described */ public static boolean isVisibilityBridgeMethodPair(Method bridgeMethod, Method bridgedMethod) {