Skip to content

Commit

Permalink
Avoid pre-conversion attempt in case of overloaded write methods
Browse files Browse the repository at this point in the history
Closes gh-32159
See gh-31872
  • Loading branch information
jhoeller committed Jan 30, 2024
1 parent 067638a commit af5acb6
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 21 deletions.
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -3219,6 +3243,7 @@ public Class<?> getObjectType() {
}
}


public record City(String name) {}

public static class CityRepository implements Repository<City, Long> {}
Expand Down Expand Up @@ -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}.
*/
Expand Down Expand Up @@ -3475,6 +3526,7 @@ public NonPublicEnum getNonPublicEnum() {
}
}


@Order
private static class LowestPrecedenceTestBeanFactoryBean implements FactoryBean<TestBean> {

Expand All @@ -3487,9 +3539,9 @@ public TestBean getObject() {
public Class<?> getObjectType() {
return TestBean.class;
}

}


@Order(Ordered.HIGHEST_PRECEDENCE)
private static class HighestPrecedenceTestBeanFactoryBean implements FactoryBean<TestBean> {

Expand All @@ -3502,7 +3554,6 @@ public TestBean getObject() {
public Class<?> getObjectType() {
return TestBean.class;
}

}

}

0 comments on commit af5acb6

Please sign in to comment.