From af5acb6d34829de7b550bd40ffce601f43f142e3 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 30 Jan 2024 21:57:14 +0100 Subject: [PATCH] Avoid pre-conversion attempt in case of overloaded write methods Closes gh-32159 See gh-31872 --- .../org/springframework/beans/BeanUtils.java | 18 +++- .../GenericTypeAwarePropertyDescriptor.java | 6 +- .../AbstractAutowireCapableBeanFactory.java | 19 ++++- .../DefaultListableBeanFactoryTests.java | 83 +++++++++++++++---- 4 files changed, 105 insertions(+), 21 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java b/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java index 70081a97b64e..29d1bbd9094b 100644 --- a/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java +++ b/spring-beans/src/main/java/org/springframework/beans/BeanUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 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. @@ -607,6 +607,22 @@ public static Class findPropertyType(String propertyName, @Nullable Class. return Object.class; } + /** + * Determine whether the specified property has a unique write method, + * i.e. is writable but does not declare overloaded setter methods. + * @param pd the PropertyDescriptor for the property + * @return {@code true} if writable and unique, {@code false} otherwise + * @since 6.1.4 + */ + public static boolean hasUniqueWriteMethod(PropertyDescriptor pd) { + if (pd instanceof GenericTypeAwarePropertyDescriptor gpd) { + return gpd.hasUniqueWriteMethod(); + } + else { + return (pd.getWriteMethod() != null); + } + } + /** * Obtain a new MethodParameter object for the write method of the * specified property. diff --git a/spring-beans/src/main/java/org/springframework/beans/GenericTypeAwarePropertyDescriptor.java b/spring-beans/src/main/java/org/springframework/beans/GenericTypeAwarePropertyDescriptor.java index dd11eae7ec4f..eb3a15686914 100644 --- a/spring-beans/src/main/java/org/springframework/beans/GenericTypeAwarePropertyDescriptor.java +++ b/spring-beans/src/main/java/org/springframework/beans/GenericTypeAwarePropertyDescriptor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 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. @@ -171,6 +171,10 @@ public Method getWriteMethodFallback(@Nullable Class valueType) { return null; } + public boolean hasUniqueWriteMethod() { + return (this.writeMethod != null && this.ambiguousWriteMethods == null); + } + public MethodParameter getWriteMethodParameter() { Assert.state(this.writeMethodParameter != null, "No write method available"); return this.writeMethodParameter; diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java index 78edd84b2fd5..5883af47efd9 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 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. @@ -39,6 +39,7 @@ import org.springframework.beans.BeanWrapper; import org.springframework.beans.BeanWrapperImpl; import org.springframework.beans.BeansException; +import org.springframework.beans.InvalidPropertyException; import org.springframework.beans.MutablePropertyValues; import org.springframework.beans.PropertyAccessorUtils; import org.springframework.beans.PropertyValue; @@ -1683,8 +1684,7 @@ protected void applyPropertyValues(String beanName, BeanDefinition mbd, BeanWrap } Object resolvedValue = valueResolver.resolveValueIfNecessary(pv, originalValue); Object convertedValue = resolvedValue; - boolean convertible = bw.isWritableProperty(propertyName) && - !PropertyAccessorUtils.isNestedOrIndexedProperty(propertyName); + boolean convertible = isConvertibleProperty(propertyName, bw); if (convertible) { convertedValue = convertForProperty(resolvedValue, propertyName, bw, converter); } @@ -1721,6 +1721,19 @@ else if (convertible && originalValue instanceof TypedStringValue typedStringVal } } + /** + * Determine whether the factory should cache a converted value for the given property. + */ + private boolean isConvertibleProperty(String propertyName, BeanWrapper bw) { + try { + return !PropertyAccessorUtils.isNestedOrIndexedProperty(propertyName) && + BeanUtils.hasUniqueWriteMethod(bw.getPropertyDescriptor(propertyName)); + } + catch (InvalidPropertyException ex) { + return false; + } + } + /** * Convert the given value for the specified target property. */ diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java index 9b33ead73c08..501311bd2195 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java @@ -24,6 +24,7 @@ import java.net.MalformedURLException; import java.text.NumberFormat; import java.text.ParseException; +import java.time.Duration; import java.util.ArrayList; import java.util.HashSet; import java.util.Iterator; @@ -666,13 +667,13 @@ void possibleMatches() { bd.setPropertyValues(pvs); lbf.registerBeanDefinition("tb", bd); - assertThatExceptionOfType(BeanCreationException.class).as("invalid property").isThrownBy(() -> - lbf.getBean("tb")) - .withCauseInstanceOf(NotWritablePropertyException.class) - .satisfies(ex -> { - NotWritablePropertyException cause = (NotWritablePropertyException) ex.getCause(); - assertThat(cause.getPossibleMatches()).containsExactly("age"); - }); + assertThatExceptionOfType(BeanCreationException.class).as("invalid property") + .isThrownBy(() -> lbf.getBean("tb")) + .withCauseInstanceOf(NotWritablePropertyException.class) + .satisfies(ex -> { + NotWritablePropertyException cause = (NotWritablePropertyException) ex.getCause(); + assertThat(cause.getPossibleMatches()).containsExactly("age"); + }); } @Test @@ -720,9 +721,9 @@ void prototypeCircleLeadsToException() { p.setProperty("rod.spouse", "*kerry"); registerBeanDefinitions(p); - assertThatExceptionOfType(BeanCreationException.class).isThrownBy(() -> - lbf.getBean("kerry")) - .satisfies(ex -> assertThat(ex.contains(BeanCurrentlyInCreationException.class)).isTrue()); + assertThatExceptionOfType(BeanCreationException.class) + .isThrownBy(() -> lbf.getBean("kerry")) + .satisfies(ex -> assertThat(ex.contains(BeanCurrentlyInCreationException.class)).isTrue()); } @Test @@ -903,16 +904,16 @@ void beanDefinitionOverridingNotAllowed() { lbf.registerBeanDefinition("test", oldDef); lbf.registerAlias("test", "testX"); - assertThatExceptionOfType(BeanDefinitionOverrideException.class).isThrownBy(() -> - lbf.registerBeanDefinition("test", newDef)) + assertThatExceptionOfType(BeanDefinitionOverrideException.class) + .isThrownBy(() -> lbf.registerBeanDefinition("test", newDef)) .satisfies(ex -> { assertThat(ex.getBeanName()).isEqualTo("test"); assertThat(ex.getBeanDefinition()).isEqualTo(newDef); assertThat(ex.getExistingDefinition()).isEqualTo(oldDef); }); - assertThatExceptionOfType(BeanDefinitionOverrideException.class).isThrownBy(() -> - lbf.registerBeanDefinition("testX", newDef)) + assertThatExceptionOfType(BeanDefinitionOverrideException.class) + .isThrownBy(() -> lbf.registerBeanDefinition("testX", newDef)) .satisfies(ex -> { assertThat(ex.getBeanName()).isEqualTo("testX"); assertThat(ex.getBeanDefinition()).isEqualTo(newDef); @@ -1320,6 +1321,29 @@ void expressionInStringArray() { assertThat(properties.getProperty("foo")).isEqualTo("bar"); } + @Test + void withOverloadedSetters() { + RootBeanDefinition rbd = new RootBeanDefinition(SetterOverload.class); + rbd.getPropertyValues().add("object", "a String"); + lbf.registerBeanDefinition("overloaded", rbd); + assertThat(lbf.getBean(SetterOverload.class).getObject()).isEqualTo("a String"); + + rbd = new RootBeanDefinition(SetterOverload.class); + rbd.getPropertyValues().add("object", 1000); + lbf.registerBeanDefinition("overloaded", rbd); + assertThat(lbf.getBean(SetterOverload.class).getObject()).isEqualTo("1000"); + + rbd = new RootBeanDefinition(SetterOverload.class); + rbd.getPropertyValues().add("value", 1000); + lbf.registerBeanDefinition("overloaded", rbd); + assertThat(lbf.getBean(SetterOverload.class).getObject()).isEqualTo("1000i"); + + rbd = new RootBeanDefinition(SetterOverload.class); + rbd.getPropertyValues().add("value", Duration.ofSeconds(1000)); + lbf.registerBeanDefinition("overloaded", rbd); + assertThat(lbf.getBean(SetterOverload.class).getObject()).isEqualTo("1000s"); + } + @Test void autowireWithNoDependencies() { RootBeanDefinition bd = new RootBeanDefinition(TestBean.class); @@ -3219,6 +3243,7 @@ public Class getObjectType() { } } + public record City(String name) {} public static class CityRepository implements Repository {} @@ -3328,6 +3353,32 @@ public Resource[] getResourceArray() { } + public static class SetterOverload { + + public String value; + + public void setObject(Integer length) { + this.value = length + "i"; + } + + public void setObject(String object) { + this.value = object; + } + + public String getObject() { + return this.value; + } + + public void setValue(int length) { + this.value = length + "i"; + } + + public void setValue(Duration duration) { + this.value = duration.getSeconds() + "s"; + } + } + + /** * Bean with a dependency on a {@link FactoryBean}. */ @@ -3475,6 +3526,7 @@ public NonPublicEnum getNonPublicEnum() { } } + @Order private static class LowestPrecedenceTestBeanFactoryBean implements FactoryBean { @@ -3487,9 +3539,9 @@ public TestBean getObject() { public Class getObjectType() { return TestBean.class; } - } + @Order(Ordered.HIGHEST_PRECEDENCE) private static class HighestPrecedenceTestBeanFactoryBean implements FactoryBean { @@ -3502,7 +3554,6 @@ public TestBean getObject() { public Class getObjectType() { return TestBean.class; } - } }