Overview
- jdk21
- Spring 6.2.11
In the following case, the setId(int id) method is inconsistent with the generic type String, and since it implements two interfaces, random copying failures may occur.
The methods of the IModel class cannot change their order; doing so may prevent this issue from being reproduced.
public class BeanUtilsCopyPropertiesTests {
public static class BaseModel<T> {
public BaseModel() {
}
private T id;
/**
* @return the id
*/
public T getId() {
return id;
}
/**
* @param id the id to set
*/
public void setId(T id) {
this.id = id;
}
}
interface IModel<T>{
void setId(T id);
T getId();
}
interface A<T> extends IModel<T>{
}
interface B<T> extends IModel<T>{
}
public static class User extends BaseModel<String> implements A<String>,B<String> {
public User() {
super();
}
@Override
public String getId() {
return super.getId();
}
public void setId(int id) {
setId(String.valueOf(id));
}
}
@org.junit.jupiter.api.Test
public void testCopyFailed(){
User source = new User();
source.setId(1);
User target = new User();
BeanUtils.copyProperties(source, target);
assertEquals(source.getId(), target.getId());
}
}
Analysis
Root cause : PropertyDescriptorUtils
This is caused by the inconsistent order returned by the Class#getMethods.
public static Collection<? extends PropertyDescriptor> determineBasicProperties(Class<?> beanClass)
throws IntrospectionException {
Map<String, BasicPropertyDescriptor> pdMap = new TreeMap<>();
for (Method method : beanClass.getMethods()) {
Comment From: sbrannen
Hi @zhanhcs,
Congratulations on submitting your first issue for the Spring Framework! 👍
Can you please tell me the exact version of the JDK you are using as well as what distribution of the JDK you are using to compile and run your tests?
The reason I ask is that I am only able to reproduce the error if I modify PropertyDescriptorUtils.determineBasicProperties() so that it either randomly shuffles the methods or sorts them in reverse order based on their toString().
In other words, if I don't modify determineBasicProperties() it doesn't seem that I can get it to fail as you've claimed.
I am using Eclipse IDE with JDK 21.0.8 support. So, I am wondering if perhaps your IDE or JDK results in a different ordering for those methods more frequently.
Update
If I store the methods in a List and reverse the order (via Collections.reverse(list)), the test then fails every time as follows (note that I switched to AssertJ).
org.opentest4j.AssertionFailedError:
expected: null
but was: "1"
Comment From: zhanhcs
ok, Thanks jdk 21.0.7 Spring6.2.11 IntelliJ IDEA 2024.3.2.2 (Ultimate Edition)
In beanClass.getMethods(), if there are two getId() methods, and the one with return type String comes before the one with return type Object, it will cause a failure; otherwise, it will succeed. The return order of Class.getMethods() is non-fixed or unspecified, which causes this issue.
Of course, the order of getMethods is not the key issue; it is merely a trigger for this occasional failure. In the determineBasicProperties method, if there are multiple writeMethods and readMethods, having an addReadMethod method and trying to find a writeMethod that matches the type in the list of readMethods within BasicPropertyDescriptor.getWriteMethod would help resolve this problem.
`
PropertyDescriptorUtils.determineBasicProperties
determineBasicProperties(){ if (setter) { Method writeMethod = pd.getWriteMethod(); if (writeMethod == null || writeMethod.getParameterTypes()[0].isAssignableFrom(method.getParameterTypes()[0])) { pd.setWriteMethod(method); } else { pd.addWriteMethod(method); } } else { Method readMethod = pd.getReadMethod(); if (readMethod == null || (readMethod.getReturnType() == method.getReturnType() && method.getName().startsWith("is"))) { pd.setReadMethod(method); } } `
`
BasicPropertyDescriptor.getWriteMethod
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;
}
}
`