fix: Include a given property only once in getBeanPropertyDescriptors (#21836) (#21863)

* fix: Include a given property only once in getBeanPropertyDescriptors

* format

* Skip testing as the interface read method is returned for the class

---------

Co-authored-by: Artur <artur@vaadin.com>
Co-authored-by: Mikhail Shabarov <61410877+mshabarov@users.noreply.github.com>

Author: 3 people

flow-server/src/main/java/com/vaadin/flow/internal/BeanUtil.java

@@ -24,6 +24,8 @@
 import java.lang.reflect.Method;
 import java.lang.reflect.RecordComponent;
 import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 
 import org.slf4j.LoggerFactory;
@@ -67,6 +69,33 @@ private BeanUtil() {
      */
     public static List<PropertyDescriptor> getBeanPropertyDescriptors(
             final Class<?> beanType) throws IntrospectionException {
+        List<PropertyDescriptor> descriptorsWithDuplicates = internalGetBeanPropertyDescriptors(
+                beanType);
+
+        // As we scan for default methods, we might get duplicates
+        // for the same property, so we need to remove them.
+        // We prefer to keep a class property over an interface
+        // property.
+        LinkedHashMap<String, PropertyDescriptor> descriptors = new LinkedHashMap<>();
+        for (PropertyDescriptor descriptor : descriptorsWithDuplicates) {
+            String name = descriptor.getName();
+            if (descriptors.containsKey(name)) {
+                PropertyDescriptor existing = descriptors.get(name);
+                // If the existing descriptor is from a class, keep it
+                // otherwise replace it with the new one.
+                if (existing.getReadMethod() != null && !existing
+                        .getReadMethod().getDeclaringClass().isInterface()) {
+                    continue;
+                }
+            }
+            descriptors.put(name, descriptor);
+        }
+
+        return new ArrayList<>(descriptors.values());
+    }
+
+    private static List<PropertyDescriptor> internalGetBeanPropertyDescriptors(
+            Class<?> beanType) throws IntrospectionException {
 
         if (beanType.isRecord()) {
             List<PropertyDescriptor> propertyDescriptors = new ArrayList<>();
@@ -75,15 +104,14 @@ public static List<PropertyDescriptor> getBeanPropertyDescriptors(
                 propertyDescriptors.add(new PropertyDescriptor(
                         component.getName(), component.getAccessor(), null));
             }
-
             return propertyDescriptors;
         }
         // Introspector does not consider superinterfaces of
         // an interface nor does it consider default methods of interfaces.
         List<PropertyDescriptor> propertyDescriptors = new ArrayList<>();
 
         for (Class<?> cls : beanType.getInterfaces()) {
-            propertyDescriptors.addAll(getBeanPropertyDescriptors(cls));
+            propertyDescriptors.addAll(internalGetBeanPropertyDescriptors(cls));
         }
 
         BeanInfo info = Introspector.getBeanInfo(beanType);

flow-server/src/test/java/com/vaadin/flow/internal/BeanUtilTest.java

@@ -0,0 +1,76 @@
+package com.vaadin.flow.internal;
+
+import java.beans.PropertyDescriptor;
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class BeanUtilTest {
+
+    public interface FirstInterface {
+        default void setOneInterface(boolean bothInterfaces) {
+        }
+
+        default boolean isOneInterface() {
+            return true;
+        }
+
+        default void setExistsInAllPlaces(boolean visible) {
+        }
+
+        default boolean isExistsInAllPlaces() {
+            return true;
+        }
+    }
+
+    public interface SecondInterface {
+        default void setExistsInAllPlaces(boolean visible) {
+        }
+
+        default boolean isExistsInAllPlaces() {
+            return true;
+        }
+    }
+
+    public class TestSomething implements FirstInterface, SecondInterface {
+        public void setExistsInAllPlaces(boolean visible) {
+        }
+
+        public boolean isExistsInAllPlaces() {
+            return true;
+        }
+    }
+
+    @Test
+    public void duplicatesAreRemoved() throws Exception {
+        List<PropertyDescriptor> descriptors = BeanUtil
+                .getBeanPropertyDescriptors(TestSomething.class);
+        List<PropertyDescriptor> existsInAllPlacesProperties = descriptors
+                .stream()
+                .filter(desc -> desc.getName().equals("existsInAllPlaces"))
+                .toList();
+        Assert.assertEquals(
+                "There should be only one 'existsInAllPlaces' property descriptor",
+                1, existsInAllPlacesProperties.size());
+
+        // The property from the class should be retained
+        // but we cannot test this as some introspector implementations
+        // return the read method from the interface when introspecting the
+        // class
+        // Assert.assertEquals(TestSomething.class,
+        // existsInAllPlacesProperties.get(0).getReadMethod().getDeclaringClass());
+
+        List<PropertyDescriptor> oneInterfaceProperties = descriptors.stream()
+                .filter(desc -> desc.getName().equals("oneInterface")).toList();
+        Assert.assertEquals(
+                "There should be only one 'oneInterface' property descriptor",
+                1, oneInterfaceProperties.size());
+
+        PropertyDescriptor oneInterfaceProperty = oneInterfaceProperties.get(0);
+        // The property from thedefault method
+        Assert.assertEquals(FirstInterface.class,
+                oneInterfaceProperty.getReadMethod().getDeclaringClass());
+
+    }
+}