`SimpleBeanInfoFactory` fails to reliably resolve read/write methods in type hierarchies with unresolved generics

[zhanhcs]

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()) {

[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"

[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. Not only does this issue occur occasionally in the above test environment, but it also occurs sporadically on the Linux cloud service. The versions of JDK, Spring, and other components are consistent with those in the test environment.

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 write methods and read methods, having an addReadMethod() method and trying to find a writeMethod that matches the type in the list of read methods within BasicPropertyDescriptor.getWriteMethod() would help resolve this problem.

spring-framework/spring-beans/src/main/java/org/springframework/beans/PropertyDescriptorUtils.java

Lines 91 to 107 in a4c72c8

	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);
	}
	}

spring-framework/spring-beans/src/main/java/org/springframework/beans/PropertyDescriptorUtils.java

Lines 302 to 317 in a4c72c8

	public @Nullable 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;
	}