From 4b0bf16389bf978e1febc473363bd041e856f728 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 18 Oct 2022 16:17:03 +0200 Subject: [PATCH 1/2] Select ambiguous write method based on read method (matching its return type) Also avoids unnecessary checks in name-based PropertyDescriptor constructor. See gh-29320 --- .../beans/PropertyDescriptorUtils.java | 65 ++++++++++++------- .../beans/SimpleBeanInfoFactory.java | 4 +- .../beans/BeanWrapperTests.java | 28 ++++++++ 3 files changed, 72 insertions(+), 25 deletions(-) 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 6b7c49a2bf..e22aa07733 100644 --- a/spring-beans/src/main/java/org/springframework/beans/PropertyDescriptorUtils.java +++ b/spring-beans/src/main/java/org/springframework/beans/PropertyDescriptorUtils.java @@ -20,8 +20,10 @@ import java.beans.IntrospectionException; import java.beans.Introspector; import java.beans.PropertyDescriptor; import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.Collection; import java.util.Enumeration; +import java.util.List; import java.util.Map; import java.util.TreeMap; @@ -52,8 +54,10 @@ abstract class PropertyDescriptorUtils { * @see SimpleBeanInfoFactory * @see java.beans.Introspector#getBeanInfo(Class) */ - public static Collection determineBasicProperties(Class beanClass) throws IntrospectionException { - Map pdMap = new TreeMap<>(); + public static Collection determineBasicProperties(Class beanClass) + throws IntrospectionException { + + Map pdMap = new TreeMap<>(); for (Method method : beanClass.getMethods()) { String methodName = method.getName(); @@ -81,39 +85,26 @@ abstract class PropertyDescriptorUtils { continue; } - PropertyDescriptor pd = pdMap.get(propertyName); + BasicPropertyDescriptor pd = pdMap.get(propertyName); if (pd != null) { if (setter) { if (pd.getWriteMethod() == null || pd.getWriteMethod().getParameterTypes()[0].isAssignableFrom(method.getParameterTypes()[0])) { - try { - pd.setWriteMethod(method); - } - catch (IntrospectionException ex) { - // typically a type mismatch -> ignore - } + pd.setWriteMethod(method); + } + else { + pd.addWriteMethod(method); } } else { if (pd.getReadMethod() == null || (pd.getReadMethod().getReturnType() == method.getReturnType() && method.getName().startsWith("is"))) { - try { - pd.setReadMethod(method); - } - catch (IntrospectionException ex) { - // typically a type mismatch -> ignore - } + pd.setReadMethod(method); } } } else { - pd = new BasicPropertyDescriptor(propertyName, beanClass); - if (setter) { - pd.setWriteMethod(method); - } - else { - pd.setReadMethod(method); - } + pd = new BasicPropertyDescriptor(propertyName, (!setter ? method : null), (setter ? method : null)); pdMap.put(propertyName, pd); } } @@ -277,8 +268,12 @@ abstract class PropertyDescriptorUtils { @Nullable private Method writeMethod; - public BasicPropertyDescriptor(String propertyName, Class beanClass) throws IntrospectionException { - super(propertyName, beanClass, null, null); + private final List alternativeWriteMethods = new ArrayList<>(); + + public BasicPropertyDescriptor(String propertyName, @Nullable Method readMethod, @Nullable Method writeMethod) + throws IntrospectionException { + + super(propertyName, readMethod, writeMethod); } @Override @@ -297,11 +292,33 @@ abstract class PropertyDescriptorUtils { this.writeMethod = writeMethod; } + public void addWriteMethod(Method writeMethod) { + if (this.writeMethod != null) { + this.alternativeWriteMethods.add(this.writeMethod); + this.writeMethod = null; + } + this.alternativeWriteMethods.add(writeMethod); + } + @Override @Nullable public Method getWriteMethod() { + if (this.writeMethod == null && !this.alternativeWriteMethods.isEmpty()) { + if (this.readMethod == null) { + return this.alternativeWriteMethods.get(0); + } + else { + for (Method method : this.alternativeWriteMethods) { + if (this.readMethod.getReturnType().isAssignableFrom(method.getParameterTypes()[0])) { + this.writeMethod = method; + break; + } + } + } + } return this.writeMethod; } } + } diff --git a/spring-beans/src/main/java/org/springframework/beans/SimpleBeanInfoFactory.java b/spring-beans/src/main/java/org/springframework/beans/SimpleBeanInfoFactory.java index 2e4705d1bd..1313867007 100644 --- a/spring-beans/src/main/java/org/springframework/beans/SimpleBeanInfoFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/SimpleBeanInfoFactory.java @@ -47,7 +47,9 @@ public class SimpleBeanInfoFactory implements BeanInfoFactory, Ordered { @Override @NonNull public BeanInfo getBeanInfo(Class beanClass) throws IntrospectionException { - Collection pds = PropertyDescriptorUtils.determineBasicProperties(beanClass); + Collection pds = + PropertyDescriptorUtils.determineBasicProperties(beanClass); + return new SimpleBeanInfo() { @Override public PropertyDescriptor[] getPropertyDescriptors() { diff --git a/spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java b/spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java index dd79e71cb1..30ab8b1716 100644 --- a/spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/BeanWrapperTests.java @@ -153,6 +153,16 @@ class BeanWrapperTests extends AbstractPropertyAccessorTests { assertThat(accessor.getPropertyValue("object")).isEqualTo(8); } + @Test + void setterOverload() { + SetterOverload target = new SetterOverload(); + BeanWrapper accessor = createAccessor(target); + accessor.setPropertyValue("object", "a String"); + assertThat(target.value).isEqualTo("a String"); + assertThat(target.getObject()).isEqualTo("a String"); + assertThat(accessor.getPropertyValue("object")).isEqualTo("a String"); + } + @Test void propertyDescriptors() throws Exception { TestBean target = new TestBean(); @@ -348,6 +358,24 @@ class BeanWrapperTests extends AbstractPropertyAccessorTests { } + public static class SetterOverload { + + public String value; + + public void setObject(Integer length) { + this.value = length.toString(); + } + + public void setObject(String object) { + this.value = object; + } + + public String getObject() { + return this.value; + } + } + + public static class GetterWithOptional { public TestBean value; From 8e25e32eb806f36659703f915e2efd5bc8c085c6 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 18 Oct 2022 16:17:30 +0200 Subject: [PATCH 2/2] Polishing --- .../Jsr310DateTimeFormatAnnotationFormatterFactory.java | 5 +++-- .../format/datetime/standard/TemporalAccessorParser.java | 9 +++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/format/datetime/standard/Jsr310DateTimeFormatAnnotationFormatterFactory.java b/spring-context/src/main/java/org/springframework/format/datetime/standard/Jsr310DateTimeFormatAnnotationFormatterFactory.java index a1fd17fe21..72679a427b 100644 --- a/spring-context/src/main/java/org/springframework/format/datetime/standard/Jsr310DateTimeFormatAnnotationFormatterFactory.java +++ b/spring-context/src/main/java/org/springframework/format/datetime/standard/Jsr310DateTimeFormatAnnotationFormatterFactory.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. @@ -96,6 +96,8 @@ public class Jsr310DateTimeFormatAnnotationFormatterFactory extends EmbeddedValu @Override @SuppressWarnings("unchecked") public Parser getParser(DateTimeFormat annotation, Class fieldType) { + DateTimeFormatter formatter = getFormatter(annotation, fieldType); + List resolvedFallbackPatterns = new ArrayList<>(); for (String fallbackPattern : annotation.fallbackPatterns()) { String resolvedFallbackPattern = resolveEmbeddedValue(fallbackPattern); @@ -104,7 +106,6 @@ public class Jsr310DateTimeFormatAnnotationFormatterFactory extends EmbeddedValu } } - DateTimeFormatter formatter = getFormatter(annotation, fieldType); return new TemporalAccessorParser((Class) fieldType, formatter, resolvedFallbackPatterns.toArray(new String[0]), annotation); } diff --git a/spring-context/src/main/java/org/springframework/format/datetime/standard/TemporalAccessorParser.java b/spring-context/src/main/java/org/springframework/format/datetime/standard/TemporalAccessorParser.java index b8dd20fe01..59c35f1ad3 100644 --- a/spring-context/src/main/java/org/springframework/format/datetime/standard/TemporalAccessorParser.java +++ b/spring-context/src/main/java/org/springframework/format/datetime/standard/TemporalAccessorParser.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. @@ -71,7 +71,8 @@ public final class TemporalAccessorParser implements Parser { } TemporalAccessorParser(Class temporalAccessorType, DateTimeFormatter formatter, - @Nullable String[] fallbackPatterns, @Nullable Object source) { + @Nullable String[] fallbackPatterns, @Nullable Object source) { + this.temporalAccessorType = temporalAccessorType; this.formatter = formatter; this.fallbackPatterns = fallbackPatterns; @@ -100,8 +101,8 @@ public final class TemporalAccessorParser implements Parser { } if (this.source != null) { throw new DateTimeParseException( - String.format("Unable to parse date time value \"%s\" using configuration from %s", text, this.source), - text, ex.getErrorIndex(), ex); + String.format("Unable to parse date time value \"%s\" using configuration from %s", text, this.source), + text, ex.getErrorIndex(), ex); } // else rethrow original exception throw ex;