Binder with Nested Properties (#8923)

8 files changed, 341 insertions, 25 deletions

diff --git a/server/src/main/java/com/vaadin/data/BeanPropertySet.java b/server/src/main/java/com/vaadin/data/BeanPropertySet.java
index 0b1bfa5bf1..f78945e9d1 100644
--- a/server/src/main/java/com/vaadin/data/BeanPropertySet.java
+++ b/server/src/main/java/com/vaadin/data/BeanPropertySet.java
@@ -21,6 +21,7 @@ import java.io.IOException;
 import java.io.Serializable;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.util.Arrays;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
@@ -103,15 +104,16 @@ public class BeanPropertySet<T> implements PropertySet<T> {
         }
     }
 
-    private static class BeanPropertyDefinition<T, V>
+    private abstract static class AbstractBeanPropertyDefinition<T, V>
             implements PropertyDefinition<T, V> {
-
         private final PropertyDescriptor descriptor;
         private final BeanPropertySet<T> propertySet;
+        private final Class<?> propertyHolderType;
 
-        public BeanPropertyDefinition(BeanPropertySet<T> propertySet,
-                PropertyDescriptor descriptor) {
+        public AbstractBeanPropertyDefinition(BeanPropertySet<T> propertySet,
+                Class<?> propertyHolderType, PropertyDescriptor descriptor) {
             this.propertySet = propertySet;
+            this.propertyHolderType = propertyHolderType;
             this.descriptor = descriptor;
 
             if (descriptor.getReadMethod() == null) {
@@ -120,13 +122,52 @@ public class BeanPropertySet<T> implements PropertySet<T> {
                                 + propertySet.beanType + "."
                                 + descriptor.getName());
             }
+        }
+
+        @SuppressWarnings("unchecked")
+        @Override
+        public Class<V> getType() {
+            return (Class<V>) ReflectTools
+                    .convertPrimitiveType(descriptor.getPropertyType());
+        }
+
+        @Override
+        public String getName() {
+            return descriptor.getName();
+        }
 
+        @Override
+        public String getCaption() {
+            return SharedUtil.propertyIdToHumanFriendly(getName());
+        }
+
+        @Override
+        public BeanPropertySet<T> getPropertySet() {
+            return propertySet;
+        }
+
+        protected PropertyDescriptor getDescriptor() {
+            return descriptor;
+        }
+
+        @Override
+        public Class<?> getPropertyHolderType() {
+            return propertyHolderType;
+        }
+    }
+
+    private static class BeanPropertyDefinition<T, V>
+            extends AbstractBeanPropertyDefinition<T, V> {
+
+        public BeanPropertyDefinition(BeanPropertySet<T> propertySet,
+                Class<T> propertyHolderType, PropertyDescriptor descriptor) {
+            super(propertySet, propertyHolderType, descriptor);
         }
 
         @Override
         public ValueProvider<T, V> getGetter() {
             return bean -> {
-                Method readMethod = descriptor.getReadMethod();
+                Method readMethod = getDescriptor().getReadMethod();
                 Object value = invokeWrapExceptions(readMethod, bean);
                 return getType().cast(value);
             };
@@ -134,40 +175,70 @@ public class BeanPropertySet<T> implements PropertySet<T> {
 
         @Override
         public Optional<Setter<T, V>> getSetter() {
-            if (descriptor.getWriteMethod() == null) {
+            if (getDescriptor().getWriteMethod() == null) {
                 return Optional.empty();
             }
 
             Setter<T, V> setter = (bean, value) -> {
                 // Do not "optimize" this getter call,
                 // if its done outside the code block, that will produce
-                // NotSerializableException because of some lambda compilation magic
-                Method innerSetter = descriptor.getWriteMethod();
+                // NotSerializableException because of some lambda compilation
+                // magic
+                Method innerSetter = getDescriptor().getWriteMethod();
                 invokeWrapExceptions(innerSetter, bean, value);
             };
             return Optional.of(setter);
         }
 
-        @SuppressWarnings("unchecked")
-        @Override
-        public Class<V> getType() {
-            return (Class<V>) ReflectTools
-                    .convertPrimitiveType(descriptor.getPropertyType());
+        private Object writeReplace() {
+            /*
+             * Instead of serializing this actual property definition, only
+             * serialize a DTO that when deserialized will get the corresponding
+             * property definition from the cache.
+             */
+            return new SerializedPropertyDefinition(getPropertySet().beanType,
+                    getName());
         }
+    }
 
-        @Override
-        public String getName() {
-            return descriptor.getName();
+    private static class NestedBeanPropertyDefinition<T, V>
+            extends AbstractBeanPropertyDefinition<T, V> {
+
+        private final PropertyDefinition<T, ?> parent;
+
+        public NestedBeanPropertyDefinition(BeanPropertySet<T> propertySet,
+                PropertyDefinition<T, ?> parent,
+                PropertyDescriptor descriptor) {
+            super(propertySet, parent.getType(), descriptor);
+            this.parent = parent;
         }
 
         @Override
-        public String getCaption() {
-            return SharedUtil.propertyIdToHumanFriendly(getName());
+        public ValueProvider<T, V> getGetter() {
+            return bean -> {
+                Method readMethod = getDescriptor().getReadMethod();
+                Object value = invokeWrapExceptions(readMethod,
+                        parent.getGetter().apply(bean));
+                return getType().cast(value);
+            };
         }
 
         @Override
-        public BeanPropertySet<T> getPropertySet() {
-            return propertySet;
+        public Optional<Setter<T, V>> getSetter() {
+            if (getDescriptor().getWriteMethod() == null) {
+                return Optional.empty();
+            }
+
+            Setter<T, V> setter = (bean, value) -> {
+                // Do not "optimize" this getter call,
+                // if its done outside the code block, that will produce
+                // NotSerializableException because of some lambda compilation
+                // magic
+                Method innerSetter = getDescriptor().getWriteMethod();
+                invokeWrapExceptions(innerSetter,
+                        parent.getGetter().apply(bean), value);
+            };
+            return Optional.of(setter);
         }
 
         private Object writeReplace() {
@@ -177,7 +248,7 @@ public class BeanPropertySet<T> implements PropertySet<T> {
              * property definition from the cache.
              */
             return new SerializedPropertyDefinition(getPropertySet().beanType,
-                    getName());
+                    parent.getName() + "." + getName());
         }
     }
 
@@ -194,7 +265,7 @@ public class BeanPropertySet<T> implements PropertySet<T> {
             definitions = BeanUtil.getBeanPropertyDescriptors(beanType).stream()
                     .filter(BeanPropertySet::hasNonObjectReadMethod)
                     .map(descriptor -> new BeanPropertyDefinition<>(this,
-                            descriptor))
+                            beanType, descriptor))
                     .collect(Collectors.toMap(PropertyDefinition::getName,
                             Function.identity()));
         } catch (IntrospectionException e) {
@@ -227,8 +298,42 @@ public class BeanPropertySet<T> implements PropertySet<T> {
     }
 
     @Override
-    public Optional<PropertyDefinition<T, ?>> getProperty(String name) {
-        return Optional.ofNullable(definitions.get(name));
+    public Optional<PropertyDefinition<T, ?>> getProperty(String name)
+            throws IllegalArgumentException {
+        Optional<PropertyDefinition<T, ?>> definition = Optional
+                .ofNullable(definitions.get(name));
+        if (!definition.isPresent() && name.contains(".")) {
+            try {
+                String parentName = name.substring(0, name.lastIndexOf('.'));
+                Optional<PropertyDefinition<T, ?>> parent = getProperty(
+                        parentName);
+                if (!parent.isPresent()) {
+                    throw new IllegalArgumentException(
+                            "Cannot find property descriptor [" + parentName
+                                    + "] for " + beanType.getName());
+                }
+
+                Optional<PropertyDescriptor> descriptor = Optional.ofNullable(
+                        BeanUtil.getPropertyDescriptor(beanType, name));
+                if (descriptor.isPresent()) {
+                    NestedBeanPropertyDefinition<T, ?> nestedDefinition = new NestedBeanPropertyDefinition<>(
+                            this, parent.get(), descriptor.get());
+                    definitions.put(name, nestedDefinition);
+                    return Optional.of(nestedDefinition);
+                } else {
+                    throw new IllegalArgumentException(
+                            "Cannot find property descriptor [" + name
+                                    + "] for " + beanType.getName());
+                }
+
+            } catch (IntrospectionException e) {
+                throw new IllegalArgumentException(
+                        "Cannot find property descriptors for "
+                                + beanType.getName(),
+                        e);
+            }
+        }
+        return definition;
     }
 
     private static boolean hasNonObjectReadMethod(
diff --git a/server/src/main/java/com/vaadin/data/BeanValidationBinder.java b/server/src/main/java/com/vaadin/data/BeanValidationBinder.java
index 101da5b1f7..c152a67b0c 100644
--- a/server/src/main/java/com/vaadin/data/BeanValidationBinder.java
+++ b/server/src/main/java/com/vaadin/data/BeanValidationBinder.java
@@ -104,8 +104,9 @@ public class BeanValidationBinder<BEAN> extends Binder<BEAN> {
     private void configureRequired(BindingBuilder<BEAN, ?> binding,
             PropertyDefinition<BEAN, ?> definition, BeanValidator validator) {
         assert requiredConfigurator != null;
+        Class<?> propertyHolderType = definition.getPropertyHolderType();
         BeanDescriptor descriptor = validator.getJavaxBeanValidator()
-                .getConstraintsForClass(beanType);
+                .getConstraintsForClass(propertyHolderType);
         PropertyDescriptor propertyDescriptor = descriptor
                 .getConstraintsForProperty(definition.getName());
         if (propertyDescriptor == null) {
diff --git a/server/src/main/java/com/vaadin/data/PropertyDefinition.java b/server/src/main/java/com/vaadin/data/PropertyDefinition.java
index 4d11e678a5..06252f8a74 100644
--- a/server/src/main/java/com/vaadin/data/PropertyDefinition.java
+++ b/server/src/main/java/com/vaadin/data/PropertyDefinition.java
@@ -55,6 +55,15 @@ public interface PropertyDefinition<T, V> extends Serializable {
     public Class<V> getType();
 
     /**
+     * Gets the type of the class containing this property.
+     *
+     * @since 8.1
+     *
+     * @return the property type. not <code>null</code>
+     */
+    public Class<?> getPropertyHolderType();
+
+    /**
      * Gets the name of this property.
      *
      * @return the property name, not <code>null</code>
diff --git a/server/src/test/java/com/vaadin/data/BeanBinderTest.java b/server/src/test/java/com/vaadin/data/BeanBinderTest.java
index 810f7e4e48..d9707c7496 100644
--- a/server/src/test/java/com/vaadin/data/BeanBinderTest.java
+++ b/server/src/test/java/com/vaadin/data/BeanBinderTest.java
@@ -66,6 +66,8 @@ public class BeanBinderTest
         @NotEmpty
         private String lastname;
 
+        private SubConstraint subfield;
+        
         public String getFirstname() {
             return firstname;
         }
@@ -89,6 +91,30 @@ public class BeanBinderTest
         public void setLastname(String lastname) {
             this.lastname = lastname;
         }
+        
+        
+        public SubConstraint getSubfield() {
+            return subfield;
+        }
+
+        public void setSubfield(SubConstraint subfield) {
+            this.subfield = subfield;
+        }
+
+
+        public static class SubConstraint implements Serializable{
+            
+            @NotNull
+            private String name;
+
+            public String getName() {
+                return name;
+            }
+
+            public void setName(String name) {
+                this.name = name;
+            }
+        }
     }
 
     @Before
@@ -337,6 +363,21 @@ public class BeanBinderTest
         Assert.assertTrue(field.isRequiredIndicatorVisible());
         testSerialization(binder);
     }
+    
+    @Test
+    public void subfield_name_fieldIsRequired() {
+        BeanValidationBinder<RequiredConstraints> binder = new BeanValidationBinder<>(
+                RequiredConstraints.class);
+        RequiredConstraints bean = new RequiredConstraints();
+        bean.setSubfield(new RequiredConstraints.SubConstraint());
+        
+        TextField field = new TextField();
+        binder.bind(field, "subfield.name");
+        binder.setBean(bean);
+
+        Assert.assertTrue(field.isRequiredIndicatorVisible());
+        testSerialization(binder);
+    }
 
     private void assertInvalid(HasValue<?> field, String message) {
         BinderValidationStatus<?> status = binder.validate();
diff --git a/server/src/test/java/com/vaadin/data/BeanPropertySetTest.java b/server/src/test/java/com/vaadin/data/BeanPropertySetTest.java
index 4e888846df..593479f24a 100644
--- a/server/src/test/java/com/vaadin/data/BeanPropertySetTest.java
+++ b/server/src/test/java/com/vaadin/data/BeanPropertySetTest.java
@@ -23,6 +23,7 @@ import java.lang.reflect.Field;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -30,6 +31,10 @@ import org.junit.Assert;
 import org.junit.Test;
 
 import com.vaadin.data.provider.bov.Person;
+import com.vaadin.tests.data.bean.Address;
+import com.vaadin.tests.data.bean.Country;
+import com.vaadin.tests.data.bean.FatherAndSon;
+import com.vaadin.tests.data.bean.Sex;
 import com.vaadin.tests.server.ClassesSerializableTest;
 
 public class BeanPropertySetTest {
@@ -102,6 +107,82 @@ public class BeanPropertySetTest {
     }
 
     @Test
+    public void testSerializeDeserialize_nestedPropertyDefinition()
+            throws Exception {
+        PropertyDefinition<com.vaadin.tests.data.bean.Person, ?> definition = BeanPropertySet
+                .get(com.vaadin.tests.data.bean.Person.class)
+                .getProperty("address.postalCode")
+                .orElseThrow(RuntimeException::new);
+
+        PropertyDefinition<com.vaadin.tests.data.bean.Person, ?> deserializedDefinition = ClassesSerializableTest
+                .serializeAndDeserialize(definition);
+
+        ValueProvider<com.vaadin.tests.data.bean.Person, ?> getter = deserializedDefinition
+                .getGetter();
+        Address address = new Address("Ruukinkatu 2-4", 20540, "Turku",
+                Country.FINLAND);
+        com.vaadin.tests.data.bean.Person person = new com.vaadin.tests.data.bean.Person(
+                "Jon", "Doe", "jon.doe@vaadin.com", 32, Sex.MALE, address);
+
+        Integer postalCode = (Integer) getter.apply(person);
+
+        Assert.assertEquals("Deserialized definition should be functional",
+                address.getPostalCode(), postalCode);
+
+        Assert.assertSame(
+                "Deserialized instance should be the same as in the cache",
+                BeanPropertySet.get(com.vaadin.tests.data.bean.Person.class)
+                        .getProperty("address.postalCode").orElseThrow(
+                                RuntimeException::new),
+                deserializedDefinition);
+    }
+
+    @Test
+    public void nestedPropertyDefinition_samePropertyNameOnMultipleLevels()
+            throws Exception {
+        PropertyDefinition<FatherAndSon, ?> definition = BeanPropertySet
+                .get(FatherAndSon.class).getProperty("father.father.firstName")
+                .orElseThrow(RuntimeException::new);
+
+        ValueProvider<FatherAndSon, ?> getter = definition.getGetter();
+
+        FatherAndSon grandFather = new FatherAndSon("Grand Old Jon", "Doe",
+                null, null);
+        FatherAndSon father = new FatherAndSon("Old Jon", "Doe", grandFather,
+                null);
+        FatherAndSon son = new FatherAndSon("Jon", "Doe", father, null);
+
+        String firstName = (String) getter.apply(son);
+
+        Assert.assertEquals(grandFather.getFirstName(), firstName);
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void nestedPropertyDefinition_propertyChainBroken()
+            throws Exception {
+        PropertyDefinition<FatherAndSon, ?> definition = BeanPropertySet
+                .get(FatherAndSon.class).getProperty("father.firstName")
+                .orElseThrow(RuntimeException::new);
+
+        ValueProvider<FatherAndSon, ?> getter = definition.getGetter();
+
+        getter.apply(new FatherAndSon("Jon", "Doe", null, null));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void nestedPropertyDefinition_invalidPropertyNameInChain()
+            throws Exception {
+        BeanPropertySet.get(FatherAndSon.class)
+                .getProperty("grandfather.firstName");
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void nestedPropertyDefinition_invalidPropertyNameAtChainEnd()
+            throws Exception {
+        BeanPropertySet.get(FatherAndSon.class).getProperty("father.age");
+    }
+
+    @Test
     public void properties() {
         PropertySet<Person> propertySet = BeanPropertySet.get(Person.class);
 
diff --git a/server/src/test/java/com/vaadin/data/BinderCustomPropertySetTest.java b/server/src/test/java/com/vaadin/data/BinderCustomPropertySetTest.java
index 65bfb36c95..f9609aa47a 100644
--- a/server/src/test/java/com/vaadin/data/BinderCustomPropertySetTest.java
+++ b/server/src/test/java/com/vaadin/data/BinderCustomPropertySetTest.java
@@ -59,6 +59,10 @@ public class BinderCustomPropertySetTest {
         public Class<String> getType() {
             return String.class;
         }
+        
+        public Class<?> getPropertyHolderType(){
+            return Map.class;
+        }
 
         @Override
         public String getName() {
diff --git a/server/src/test/java/com/vaadin/tests/data/bean/FatherAndSon.java b/server/src/test/java/com/vaadin/tests/data/bean/FatherAndSon.java
new file mode 100644
index 0000000000..5b1f4767a3
--- /dev/null
+++ b/server/src/test/java/com/vaadin/tests/data/bean/FatherAndSon.java
@@ -0,0 +1,65 @@
+package com.vaadin.tests.data.bean;
+
+import java.io.Serializable;
+
+public class FatherAndSon implements Serializable {
+    private String firstName;
+    private String lastName;
+    private FatherAndSon father;
+    private FatherAndSon son;
+
+    public FatherAndSon() {
+
+    }
+
+    @Override
+    public String toString() {
+        return "FatherAndSon [firstName=" + firstName + ", lastName=" + lastName
+                + ", father=" + father + ", son=" + son + "]";
+    }
+
+    public FatherAndSon(String firstName, String lastName, FatherAndSon father,
+            FatherAndSon son) {
+        super();
+        this.firstName = firstName;
+        this.lastName = lastName;
+        this.father = father;
+        if (this.father != null)
+            this.father.setSon(this);
+        else
+            this.son = son;
+    }
+
+    public String getFirstName() {
+        return firstName;
+    }
+
+    public void setFirstName(String firstName) {
+        this.firstName = firstName;
+    }
+
+    public String getLastName() {
+        return lastName;
+    }
+
+    public void setLastName(String lastName) {
+        this.lastName = lastName;
+    }
+
+    public FatherAndSon getFather() {
+        return father;
+    }
+
+    public void setFather(FatherAndSon father) {
+        this.father = father;
+    }
+
+    public FatherAndSon getSon() {
+        return son;
+    }
+
+    public void setSon(FatherAndSon son) {
+        this.son = son;
+    }
+
+}
diff --git a/server/src/test/java/com/vaadin/tests/server/component/grid/GridCustomPropertySetTest.java b/server/src/test/java/com/vaadin/tests/server/component/grid/GridCustomPropertySetTest.java
index 6277e85ae8..c1726d77ba 100644
--- a/server/src/test/java/com/vaadin/tests/server/component/grid/GridCustomPropertySetTest.java
+++ b/server/src/test/java/com/vaadin/tests/server/component/grid/GridCustomPropertySetTest.java
@@ -89,6 +89,11 @@ public class GridCustomPropertySetTest {
             }
 
             @Override
+            public Class<?> getPropertyHolderType() {
+                return MyBeanWithoutGetters.class;
+            }
+
+            @Override
             public String getName() {
                 return "string";
             }
@@ -130,6 +135,11 @@ public class GridCustomPropertySetTest {
             }
 
             @Override
+            public Class<?> getPropertyHolderType() {
+                return MyBeanWithoutGetters.class;
+            }
+
+            @Override
             public String getName() {
                 return "numbah";
             }