Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BeanUtils.copyProperties no longer copies generic type properties from a base class that has been enhanced #32888

Closed
namwonmkw opened this issue May 23, 2024 · 3 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Milestone

Comments

@namwonmkw
Copy link

namwonmkw commented May 23, 2024

With a recent release the BeanUtils.copyProperties method stopped working for generic properties of enhanced classes. The breaking change was introduced with this commit. 09aa59f

Below is a Unit Test which fail when using the latest version BeanUtils implementation and the former version.

import static org.junit.Assert.assertEquals;

import java.beans.PropertyDescriptor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;

import org.junit.Test;
import org.springframework.beans.BeanUtils;
import org.springframework.beans.BeansException;
import org.springframework.beans.FatalBeanException;
import org.springframework.cglib.proxy.Enhancer;
import org.springframework.cglib.proxy.MethodInterceptor;
import org.springframework.core.ResolvableType;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
import org.springframework.util.ReflectionUtils;

public class TestCopyProperties {

    public static class BaseModel<T> {

        public BaseModel() {
        }

        private T id;
        private String name;

        /**
         * @return the id
         */
        public T getId() {
            return id;
        }

        /**
         * @param id the id to set
         */
        public void setId(T id) {
            this.id = id;
        }

        /**
         * @return the name
         */
        public String getName() {
            return name;
        }

        /**
         * @param name the name to set
         */
        public void setName(String name) {
            this.name = name;
        }
    }

    public static class User extends BaseModel<Integer> {
        private String address;

        public User() {
            super();
        }

        /**
         * @return the address
         */
        public String getAddress() {
            return address;
        }

        /**
         * @param address the address to set
         */
        public void setAddress(String address) {
            this.address = address;
        }

    }

    @Test
    public void testCopyFailed() throws Exception {
        User f = createCglibProxy(User.class);
        f.setId(1);
        f.setName("proxy");
        f.setAddress("addr");

        User copy = new User();

        // copyProperties(f, copy, null, (String[]) null);

        BeanUtils.copyProperties(f, copy);

        assertEquals(f.getName(), copy.getName());
        assertEquals(f.getAddress(), copy.getAddress());
        assertEquals(f.getId(), copy.getId());
    }

    @Test
    public void testCopyPrevious() throws Exception {
        User f = createCglibProxy(User.class);
        f.setId(1);
        f.setName("proxy");
        f.setAddress("addr");

        User copy = new User();

        copyProperties(f, copy, null, (String[]) null);

        assertEquals(f.getName(), copy.getName());
        assertEquals(f.getAddress(), copy.getAddress());
        assertEquals(f.getId(), copy.getId());
    }

    @SuppressWarnings("unchecked")
    private <T> T createCglibProxy(Class<T> clazz)
            throws NoSuchMethodException, InstantiationException, IllegalAccessException, InvocationTargetException {
        Enhancer enhancer = new Enhancer();
        enhancer.setSuperclass(clazz);
        enhancer.setCallback((MethodInterceptor) (obj, method, args, proxy) -> {

            return proxy.invokeSuper(obj, args);
        });

        return (T) enhancer.create();

    }

    private static void copyProperties(Object source, Object target, @Nullable Class<?> editable,
            @Nullable String... ignoreProperties) throws BeansException {

        Assert.notNull(source, "Source must not be null");
        Assert.notNull(target, "Target must not be null");

        Class<?> actualEditable = target.getClass();
        if (editable != null) {
            if (!editable.isInstance(target)) {
                throw new IllegalArgumentException("Target class [" + target.getClass().getName() +
                        "] not assignable to editable class [" + editable.getName() + "]");
            }
            actualEditable = editable;
        }
        PropertyDescriptor[] targetPds = BeanUtils.getPropertyDescriptors(actualEditable);
        Set<String> ignoredProps = (ignoreProperties != null ? new HashSet<>(Arrays.asList(ignoreProperties)) : null);

        for (PropertyDescriptor targetPd : targetPds) {
            Method writeMethod = targetPd.getWriteMethod();
            if (writeMethod != null && (ignoredProps == null || !ignoredProps.contains(targetPd.getName()))) {
                PropertyDescriptor sourcePd = BeanUtils.getPropertyDescriptor(source.getClass(), targetPd.getName());

                if (sourcePd != null) {
                    Method readMethod = sourcePd.getReadMethod();
                    if (readMethod != null) {

                        ResolvableType sourceResolvableType = ResolvableType.forMethodReturnType(readMethod);
                        ResolvableType targetResolvableType = ResolvableType.forMethodParameter(writeMethod, 0);

                        boolean isAssignable = (sourceResolvableType.hasUnresolvableGenerics()
                                || targetResolvableType.hasUnresolvableGenerics()
                                        ? ClassUtils.isAssignable(writeMethod.getParameterTypes()[0],
                                                readMethod.getReturnType())
                                        : targetResolvableType.isAssignableFrom(sourceResolvableType));

                        if (isAssignable) {
                            try {
                                ReflectionUtils.makeAccessible(readMethod);
                                Object value = readMethod.invoke(source);
                                ReflectionUtils.makeAccessible(writeMethod);
                                writeMethod.invoke(target, value);
                            } catch (Throwable ex) {
                                throw new FatalBeanException(
                                        "Could not copy property '" + targetPd.getName() + "' from source to target",
                                        ex);
                            }
                        }
                    }
                }
            }
        }
    }

}
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label May 23, 2024
@jhoeller jhoeller added type: regression A bug that is also a regression in: core Issues in core modules (aop, beans, core, context, expression) and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels May 24, 2024
@jhoeller jhoeller self-assigned this May 24, 2024
@jhoeller jhoeller added this to the 6.1.9 milestone May 24, 2024
@jhoeller jhoeller added the for: backport-to-6.0.x Marks an issue as a candidate for backport to 6.0.x label May 24, 2024
@github-actions github-actions bot added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-6.0.x Marks an issue as a candidate for backport to 6.0.x labels May 24, 2024
@jhoeller jhoeller added the for: backport-to-5.3.x Marks an issue as a candidate for backport to 5.3.x label May 24, 2024
@github-actions github-actions bot removed the for: backport-to-5.3.x Marks an issue as a candidate for backport to 5.3.x label May 24, 2024
@jhoeller jhoeller removed the status: backported An issue that has been backported to maintenance branches label May 24, 2024
jhoeller added a commit that referenced this issue May 24, 2024
Includes isBridgedCandidateFor optimization (aligned with 6.0.x)

See gh-32888
@jhoeller
Copy link
Contributor

jhoeller commented Jun 4, 2024

This is available in current 6.1.9 snapshots already. Feel free to give it an early try!

@namwonmkw
Copy link
Author

namwonmkw commented Jun 5, 2024 via email

@jhoeller
Copy link
Contributor

jhoeller commented Jun 5, 2024

It's scheduled for release next week: June 13. You can always see the current target dates on https://github.com/spring-projects/spring-framework/milestones

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

3 participants