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 16b92c0a44e..7fb8ac07181 100644 --- a/spring-beans/src/main/java/org/springframework/beans/PropertyDescriptorUtils.java +++ b/spring-beans/src/main/java/org/springframework/beans/PropertyDescriptorUtils.java @@ -91,14 +91,7 @@ abstract class PropertyDescriptorUtils { BasicPropertyDescriptor pd = pdMap.get(propertyName); if (pd != null) { if (setter) { - Method writeMethod = pd.getWriteMethod(); - if (writeMethod == null || - writeMethod.getParameterTypes()[0].isAssignableFrom(method.getParameterTypes()[0])) { - pd.setWriteMethod(method); - } - else { - pd.addWriteMethod(method); - } + pd.addWriteMethod(method); } else { Method readMethod = pd.getReadMethod(); @@ -270,7 +263,7 @@ abstract class PropertyDescriptorUtils { private @Nullable Method writeMethod; - private final List alternativeWriteMethods = new ArrayList<>(); + private final List candidateWriteMethods = new ArrayList<>(); public BasicPropertyDescriptor(String propertyName, Class beanClass, @Nullable Method readMethod, @Nullable Method writeMethod) throws IntrospectionException { @@ -294,34 +287,46 @@ abstract class PropertyDescriptorUtils { this.writeMethod = writeMethod; } - public void addWriteMethod(Method writeMethod) { + void addWriteMethod(Method writeMethod) { + // Since setWriteMethod() is invoked from the PropertyDescriptor(String, Method, Method) + // constructor, this.writeMethod may be non-null. if (this.writeMethod != null) { - this.alternativeWriteMethods.add(this.writeMethod); + this.candidateWriteMethods.add(this.writeMethod); this.writeMethod = null; } - this.alternativeWriteMethods.add(writeMethod); + this.candidateWriteMethods.add(writeMethod); } @Override public @Nullable Method getWriteMethod() { - if (this.writeMethod == null && !this.alternativeWriteMethods.isEmpty()) { - if (this.readMethod == null) { - this.writeMethod = this.alternativeWriteMethods.get(0); + if (this.writeMethod == null && !this.candidateWriteMethods.isEmpty()) { + if (this.readMethod == null || this.candidateWriteMethods.size() == 1) { + this.writeMethod = this.candidateWriteMethods.get(0); } else { - for (Method method : this.alternativeWriteMethods) { - // Check subtype match first. - if (this.readMethod.getReturnType().isAssignableFrom(method.getParameterTypes()[0])) { + Class resolvedReadType = + ResolvableType.forMethodReturnType(this.readMethod, this.beanClass).toClass(); + for (Method method : this.candidateWriteMethods) { + // 1) Check for an exact match against the resolved types. + Class resolvedWriteType = + ResolvableType.forMethodParameter(method, 0, this.beanClass).toClass(); + if (resolvedReadType.equals(resolvedWriteType)) { 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)) { + + // 2) Check if the candidate write method's parameter type is compatible with + // the read method's return type. + Class parameterType = method.getParameterTypes()[0]; + if (this.readMethod.getReturnType().isAssignableFrom(parameterType)) { + // If we haven't yet found a compatible write method, or if the current + // candidate's parameter type is a subtype of the previous candidate's + // parameter type, track the current candidate as the write method. + if (this.writeMethod == null || + this.writeMethod.getParameterTypes()[0].isAssignableFrom(parameterType)) { this.writeMethod = method; - break; + // We do not "break" here, since we need to compare the current candidate + // with all remaining candidates. } } } diff --git a/spring-beans/src/test/java/org/springframework/beans/PropertyDescriptorUtilsPropertyResolutionTests.java b/spring-beans/src/test/java/org/springframework/beans/PropertyDescriptorUtilsPropertyResolutionTests.java index dbfc08e3a40..11d772dedb1 100644 --- a/spring-beans/src/test/java/org/springframework/beans/PropertyDescriptorUtilsPropertyResolutionTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/PropertyDescriptorUtilsPropertyResolutionTests.java @@ -387,15 +387,7 @@ class PropertyDescriptorUtilsPropertyResolutionTests { void resolvePropertiesWithUnresolvedGenericsInSubclassWithOverloadedSetter() { var pdMap = resolver.resolve(PersonWithOverloadedSetter.class); - Class writeType = Number.class; - if (resolver instanceof BasicPropertiesResolver) { - // TODO: PropertyDescriptorUtils currently incorrectly resolves setId(Integer) - // as the write method instead of setId(Number) (where Number is the - // unresolved generic for Long). - writeType = Integer.class; - } - - assertReadAndWriteMethodsForClassAndId(pdMap, Number.class, writeType); + assertReadAndWriteMethodsForClassAndId(pdMap, Number.class, Number.class); }