diff --git a/spring-beans/src/main/java/org/springframework/beans/PropertyDescriptorUtils.java b/spring-beans/src/main/java/org/springframework/beans/PropertyDescriptorUtils.java index 8dfc4e53ff7..5cb9b02a72b 100644 --- a/spring-beans/src/main/java/org/springframework/beans/PropertyDescriptorUtils.java +++ b/spring-beans/src/main/java/org/springframework/beans/PropertyDescriptorUtils.java @@ -26,6 +26,7 @@ import java.util.List; import java.util.Map; import java.util.TreeMap; +import org.springframework.core.ResolvableType; import org.springframework.lang.Nullable; import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; @@ -35,6 +36,7 @@ import org.springframework.util.StringUtils; * * @author Chris Beams * @author Juergen Hoeller + * @author Sam Brannen */ abstract class PropertyDescriptorUtils { @@ -99,14 +101,13 @@ abstract class PropertyDescriptorUtils { } else { Method readMethod = pd.getReadMethod(); - if (readMethod == null || - (readMethod.getReturnType() == method.getReturnType() && method.getName().startsWith("is"))) { + if (readMethod == null || readMethod.getReturnType().isAssignableFrom(method.getReturnType())) { pd.setReadMethod(method); } } } else { - pd = new BasicPropertyDescriptor(propertyName, (!setter ? method : null), (setter ? method : null)); + pd = new BasicPropertyDescriptor(propertyName, beanClass, (!setter ? method : null), (setter ? method : null)); pdMap.put(propertyName, pd); } } @@ -264,6 +265,8 @@ abstract class PropertyDescriptorUtils { */ private static class BasicPropertyDescriptor extends PropertyDescriptor { + private final Class beanClass; + @Nullable private Method readMethod; @@ -272,10 +275,11 @@ abstract class PropertyDescriptorUtils { private final List alternativeWriteMethods = new ArrayList<>(); - public BasicPropertyDescriptor(String propertyName, @Nullable Method readMethod, @Nullable Method writeMethod) + public BasicPropertyDescriptor(String propertyName, Class beanClass, @Nullable Method readMethod, @Nullable Method writeMethod) throws IntrospectionException { super(propertyName, readMethod, writeMethod); + this.beanClass = beanClass; } @Override @@ -307,14 +311,24 @@ abstract class PropertyDescriptorUtils { public Method getWriteMethod() { if (this.writeMethod == null && !this.alternativeWriteMethods.isEmpty()) { if (this.readMethod == null) { - return this.alternativeWriteMethods.get(0); + this.writeMethod = this.alternativeWriteMethods.get(0); } else { for (Method method : this.alternativeWriteMethods) { + // Check subtype match first. if (this.readMethod.getReturnType().isAssignableFrom(method.getParameterTypes()[0])) { this.writeMethod = method; break; } + // Check exact match against resolved generic parameter type as a fallback. + if (!(method.getGenericParameterTypes()[0] instanceof Class)) { + Class resolvedParameterType = + ResolvableType.forMethodParameter(method, 0, this.beanClass).toClass(); + if (this.readMethod.getReturnType().equals(resolvedParameterType)) { + this.writeMethod = method; + break; + } + } } } } @@ -322,5 +336,4 @@ abstract class PropertyDescriptorUtils { } } - } 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 c8ae06c4d80..34d5903a4aa 100644 --- a/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/BeanUtilsTests.java @@ -38,6 +38,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.beans.PropertyDescriptorUtilsPropertyResolutionTests.ServiceWithOverriddenGetterAndOverloadedSetter; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.propertyeditors.CustomDateEditor; import org.springframework.beans.testfixture.beans.DerivedTestBean; @@ -541,6 +542,17 @@ class BeanUtilsTests { assertThat((boolean) target.getFlag1()).isTrue(); assertThat(target.getFlag2()).isTrue(); } + + @Test // gh-36019 + void copyPropertiesHonorsGenericsInTypeHieararchyAndIgnoresOverloadedSetterMethod() { + var source = new ServiceWithOverriddenGetterAndOverloadedSetter(); + var target = new ServiceWithOverriddenGetterAndOverloadedSetter(); + + source.setId(1); + BeanUtils.copyProperties(source, target); + + assertThat(target.getId()).isEqualTo(source.getId()).isEqualTo("1"); + } } diff --git a/spring-beans/src/test/java/org/springframework/beans/PropertyDescriptorUtilsPropertyResolutionTests.java b/spring-beans/src/test/java/org/springframework/beans/PropertyDescriptorUtilsPropertyResolutionTests.java new file mode 100644 index 00000000000..66207423f1b --- /dev/null +++ b/spring-beans/src/test/java/org/springframework/beans/PropertyDescriptorUtilsPropertyResolutionTests.java @@ -0,0 +1,265 @@ +/* + * Copyright 2002-present 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.beans; + +import java.beans.Introspector; +import java.beans.PropertyDescriptor; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.stream.Stream; + +import org.junit.jupiter.api.Named; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.Parameter; +import org.junit.jupiter.params.ParameterizedClass; +import org.junit.jupiter.params.provider.FieldSource; + +import static java.util.stream.Collectors.groupingBy; +import static java.util.stream.Collectors.toList; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assumptions.assumeThat; +import static org.assertj.core.api.SoftAssertions.assertSoftly; +import static org.junit.jupiter.api.Named.named; + +/** + * Unit tests for property descriptor resolution via + * {@link PropertyDescriptorUtils#determineBasicProperties(Class)}. + * + *

Results are compared to the behavior of the standard {@link Introspector}. + * + * @author Sam Brannen + * @since 6.2.16 + */ +@ParameterizedClass(name = "{0}") +@FieldSource("resolvers") +class PropertyDescriptorUtilsPropertyResolutionTests { + + static final List> resolvers = List.of( + named("Basic Properties", new BasicPropertiesResolver()), + named("Standard Properties", new StandardPropertiesResolver())); + + + @Parameter + PropertiesResolver resolver; + + + @Test + void determineBasicPropertiesWithUnresolvedGenericsInInterface() { + var pdMap = resolver.resolve(GenericService.class); + + assertThat(pdMap).containsOnlyKeys("id"); + assertReadAndWriteMethodsForId(pdMap.get("id"), Object.class, Object.class); + } + + @Test + void determineBasicPropertiesWithUnresolvedGenericsInSubInterface() { + // FYI: java.beans.Introspector does not resolve properties for sub-interfaces. + assumeThat(resolver).isNotInstanceOf(StandardPropertiesResolver.class); + + var pdMap = resolver.resolve(SubGenericService.class); + + assertThat(pdMap).containsOnlyKeys("id"); + assertReadAndWriteMethodsForId(pdMap.get("id"), Object.class, Object.class); + } + + @Test + void resolvePropertiesWithUnresolvedGenericsInClass() { + var pdMap = resolver.resolve(BaseService.class); + + assertReadAndWriteMethodsForClassAndId(pdMap, Object.class, Object.class); + } + + @Test // gh-36019 + void resolvePropertiesInSubclassWithOverriddenGetterAndSetter() { + var pdMap = resolver.resolve(ServiceWithOverriddenGetterAndSetter.class); + + assertReadAndWriteMethodsForClassAndId(pdMap, String.class, String.class); + } + + @Test // gh-36019 + void resolvePropertiesWithUnresolvedGenericsInSubclassWithOverloadedSetter() { + var pdMap = resolver.resolve(ServiceWithOverloadedSetter.class); + + assertReadAndWriteMethodsForClassAndId(pdMap, Object.class, Object.class); + } + + @Test // gh-36019 + void resolvePropertiesWithPartiallyUnresolvedGenericsInSubclassWithOverriddenGetter() { + var pdMap = resolver.resolve(ServiceWithOverriddenGetter.class); + + assertReadAndWriteMethodsForClassAndId(pdMap, String.class, Object.class); + } + + @Test // gh-36019 + void resolvePropertiesWithPartiallyUnresolvedGenericsInSubclassWithOverriddenGetterAndOverloadedSetter() { + var pdMap = resolver.resolve(ServiceWithOverriddenGetterAndOverloadedSetter.class); + + assertReadAndWriteMethodsForClassAndId(pdMap, String.class, Object.class); + } + + + private static void assertReadAndWriteMethodsForClassAndId(Map> pdMap, + Class readType, Class writeType) { + + assertThat(pdMap).containsOnlyKeys("class", "id"); + assertReadAndWriteMethodsForClass(pdMap.get("class")); + assertReadAndWriteMethodsForId(pdMap.get("id"), readType, writeType); + } + + private static void assertReadAndWriteMethodsForClass(List pds) { + assertThat(pds).hasSize(1); + var pd = pds.get(0); + assertThat(pd.getName()).isEqualTo("class"); + + var readMethod = pd.getReadMethod(); + assertThat(readMethod.getName()).isEqualTo("getClass"); + assertThat(readMethod.getReturnType()).as("read type").isEqualTo(Class.class); + assertThat(readMethod.getParameterCount()).isZero(); + + assertThat(pd.getWriteMethod()).as("write method").isNull(); + } + + private static void assertReadAndWriteMethodsForId(List pds, Class readType, Class writeType) { + assertThat(pds).hasSize(1); + var pd = pds.get(0); + assertThat(pd.getName()).isEqualTo("id"); + + var readMethod = pd.getReadMethod(); + var writeMethod = pd.getWriteMethod(); + + assertSoftly(softly -> { + softly.assertThat(readMethod.getName()).isEqualTo("getId"); + softly.assertThat(readMethod.getReturnType()).as("read type").isEqualTo(readType); + softly.assertThat(readMethod.getParameterCount()).isZero(); + + softly.assertThat(writeMethod).as("write method").isNotNull(); + if (writeMethod != null) { + softly.assertThat(writeMethod.getName()).isEqualTo("setId"); + softly.assertThat(writeMethod.getReturnType()).isEqualTo(void.class); + softly.assertThat(writeMethod.getParameterCount()).isEqualTo(1); + softly.assertThat(writeMethod.getParameterTypes()[0]).as("write type").isEqualTo(writeType); + } + }); + } + + private static Map> toMap(Stream stream) { + return stream.collect(groupingBy(PropertyDescriptor::getName, toList())); + } + + + interface PropertiesResolver { + + Map> resolve(Class beanClass); + } + + static class BasicPropertiesResolver implements PropertiesResolver { + + @Override + public Map> resolve(Class beanClass) { + try { + var pds = PropertyDescriptorUtils.determineBasicProperties(beanClass); + return toMap(pds.stream()); + } + catch (Exception ex) { + throw new RuntimeException(ex); + } + } + } + + static class StandardPropertiesResolver implements PropertiesResolver { + + @Override + public Map> resolve(Class beanClass) { + try { + var beanInfo = Introspector.getBeanInfo(beanClass); + return toMap(Arrays.stream(beanInfo.getPropertyDescriptors())); + } + catch (Exception ex) { + throw new RuntimeException(ex); + } + } + } + + interface GenericService { + + void setId(T id); + + T getId(); + } + + interface SubGenericService extends GenericService { + } + + static class BaseService { + + private T id; + + public T getId() { + return id; + } + + public void setId(T id) { + this.id = id; + } + } + + static class ServiceWithOverriddenGetterAndSetter extends BaseService + implements SubGenericService { + + @Override + public String getId() { + return super.getId(); + } + + @Override + public void setId(String id) { + super.setId(id); + } + } + + static class ServiceWithOverloadedSetter extends BaseService + implements SubGenericService { + + public void setId(int id) { + setId(String.valueOf(id)); + } + } + + static class ServiceWithOverriddenGetter extends BaseService + implements SubGenericService { + + @Override + public String getId() { + return super.getId(); + } + } + + static class ServiceWithOverriddenGetterAndOverloadedSetter extends BaseService + implements SubGenericService { + + @Override + public String getId() { + return super.getId(); + } + + public void setId(int id) { + setId(String.valueOf(id)); + } + } + +}