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

`