Simplified support for null intermediate properties (#11435)

Removed support for explicitly throwing NPE for intermediate properties as no use
case could be identified where this would be the expected or wanted behavior.

Change-Id: I10696467f792e234326075bbcdd5aad487905a7e

server/src/com/vaadin/data/util/AbstractBeanContainer.java

@@ -848,8 +848,9 @@ public abstract class AbstractBeanContainer<IDTYPE, BEANTYPE> extends
      * Adds a nested container property for the container, e.g.
      * "manager.address.street".
      * 
-     * All intermediate getters must exist and must return non-null values when
-     * the property value is accessed.
+     * All intermediate getters must exist and should return non-null values
+     * when the property value is accessed. If an intermediate getter returns
+     * null, a null value will be returned.
      * 
      * @see NestedMethodProperty
      * 
@@ -857,32 +858,8 @@ public abstract class AbstractBeanContainer<IDTYPE, BEANTYPE> extends
      * @return true if the property was added
      */
     public boolean addNestedContainerProperty(String propertyId) {
-        return addNestedContainerProperty(propertyId, false);
-    }
-
-    /**
-     * Adds a nested container property for the container, e.g.
-     * "manager.address.street".
-     * 
-     * All intermediate getters must exist and must return non-null values when
-     * the property value is accessed or the <code>nullBeansAllowed</code> must
-     * be set to true. If the <code>nullBeansAllowed</code> flag is set to true,
-     * calling getValue of the added property will return null if the property
-     * or any of its intermediate getters returns null. If set to false, null
-     * values returned by intermediate getters will cause NullPointerException.
-     * The default value is false to ensure backwards compatibility.
-     * 
-     * @see NestedMethodProperty
-     * 
-     * @param propertyId
-     * @param nullBeansAllowed
-     *            set true to allow null values from intermediate getters
-     * @return true if the property was added
-     */
-    public boolean addNestedContainerProperty(String propertyId,
-            boolean nullBeansAllowed) {
         return addContainerProperty(propertyId, new NestedPropertyDescriptor(
-                propertyId, type, nullBeansAllowed));
+                propertyId, type));
     }
 
     /**
@@ -890,8 +867,9 @@ public abstract class AbstractBeanContainer<IDTYPE, BEANTYPE> extends
      * property to the container. The named property itself is removed from the
      * model as its subproperties are added.
      * 
-     * All intermediate getters must exist and must return non-null values when
-     * the property value is accessed.
+     * All intermediate getters must exist and should return non-null values
+     * when the property value is accessed. If an intermediate getter returns
+     * null, a null value will be returned.
      * 
      * @see NestedMethodProperty
      * @see #addNestedContainerProperty(String)
@@ -900,42 +878,13 @@ public abstract class AbstractBeanContainer<IDTYPE, BEANTYPE> extends
      */
     @SuppressWarnings("unchecked")
     public void addNestedContainerBean(String propertyId) {
-        addNestedContainerBean(propertyId, false);
-    }
-
-    /**
-     * Adds a nested container properties for all sub-properties of a named
-     * property to the container. The named property itself is removed from the
-     * model as its subproperties are added.
-     * 
-     * Unless
-     * <code>nullBeansAllowed<code>  is set to true, all intermediate getters must
-     * exist and must return non-null values when the property values are
-     * accessed. If the <code>nullBeansAllowed</code> flag is set to true,
-     * calling getValue of the added subproperties will return null if the
-     * property or any of their intermediate getters returns null. If set to
-     * false, null values returned by intermediate getters will cause
-     * NullPointerException. The default value is false to ensure backwards
-     * compatibility.
-     * 
-     * @see NestedMethodProperty
-     * @see #addNestedContainerProperty(String)
-     * 
-     * @param propertyId
-     * @param nullBeansAllowed
-     *            set true to allow null values from intermediate getters
-     */
-    @SuppressWarnings("unchecked")
-    public void addNestedContainerBean(String propertyId,
-            boolean nullBeansAllowed) {
         Class<?> propertyType = getType(propertyId);
         LinkedHashMap<String, VaadinPropertyDescriptor<Object>> pds = BeanItem
                 .getPropertyDescriptors((Class<Object>) propertyType);
         for (String subPropertyId : pds.keySet()) {
             String qualifiedPropertyId = propertyId + "." + subPropertyId;
             NestedPropertyDescriptor<BEANTYPE> pd = new NestedPropertyDescriptor<BEANTYPE>(
-                    qualifiedPropertyId, (Class<BEANTYPE>) type,
-                    nullBeansAllowed);
+                    qualifiedPropertyId, (Class<BEANTYPE>) type);
             model.put(qualifiedPropertyId, pd);
             model.remove(propertyId);
             for (BeanItem<BEANTYPE> item : itemIdToItem.values()) {

server/src/com/vaadin/data/util/BeanItem.java

@@ -255,39 +255,19 @@ public class BeanItem<BT> extends PropertysetItem {
     }
 
     /**
-     * Adds a nested property to the item.
+     * Adds a nested property to the item. The property must not exist in the
+     * item already and must of form "field1.field2" where field2 is a field in
+     * the object referenced to by field1. If an intermediate property returns
+     * null, the property will return a null value
      * 
      * @param nestedPropertyId
-     *            property id to add. This property must not exist in the item
-     *            already and must of of form "field1.field2" where field2 is a
-     *            field in the object referenced to by field1
+     *            property id to add.
      */
     public void addNestedProperty(String nestedPropertyId) {
         addItemProperty(nestedPropertyId, new NestedMethodProperty<Object>(
                 getBean(), nestedPropertyId));
     }
 
-    /**
-     * Adds a nested property to the item. If the <code>nullBeansAllowed</code>
-     * flag is set to true, calling getValue of the added property will return
-     * null if the property or any of its intermediate getters returns null. If
-     * set to false, null values returned by intermediate getters will cause
-     * NullPointerException. The default value is false to ensure backwards
-     * compatibility.
-     * 
-     * @param nestedPropertyId
-     *            property id to add. This property must not exist in the item
-     *            already and must of of form "field1.field2" where field2 is a
-     *            field in the object referenced to by field1
-     * @param nullBeansAllowed
-     *            set true to allow null values from intermediate getters
-     */
-    public void addNestedProperty(String nestedPropertyId,
-            boolean nullBeansAllowed) {
-        addItemProperty(nestedPropertyId, new NestedMethodProperty<Object>(
-                getBean(), nestedPropertyId, nullBeansAllowed));
-    }
-
     /**
      * Gets the underlying JavaBean object.
      * 

server/src/com/vaadin/data/util/NestedMethodProperty.java

@@ -31,8 +31,9 @@ import com.vaadin.data.util.MethodProperty.MethodException;
  * The property is specified in the dotted notation, e.g. "address.street", and
  * can contain multiple levels of nesting.
  * 
- * When accessing the property value, all intermediate getters must return
- * non-null values or the <code>nullBeansAllowed</code> must be set to true.
+ * When accessing the property value, all intermediate getters must exist and
+ * should return non-null values when the property value is accessed. If an
+ * intermediate getter returns null, a null value will be returned.
  * 
  * @see MethodProperty
  * 
@@ -55,15 +56,6 @@ public class NestedMethodProperty<T> extends AbstractProperty<T> {
      */
     private Object instance;
 
-    /**
-     * a boolean flag indicating whether intermediate getters may return null
-     * values. If the flag is set to true, calling getValue will return null if
-     * the property or any of its intermediate getters returns null. If set to
-     * false, intermediate getters returning null value will throw Exception.
-     * The default value is false to ensure backwards compatibility.
-     */
-    private boolean nullBeansAllowed = false;
-
     private Class<? extends T> type;
 
     /* Special serialization to handle method references */
@@ -85,6 +77,8 @@ public class NestedMethodProperty<T> extends AbstractProperty<T> {
      * Constructs a nested method property for a given object instance. The
      * property name is a dot separated string pointing to a nested property,
      * e.g. "manager.address.street".
+     * <p>
+     * Calling getValue will return null if any intermediate getter returns null
      * 
      * @param instance
      *            top-level bean to which the property applies
@@ -94,33 +88,7 @@ public class NestedMethodProperty<T> extends AbstractProperty<T> {
      *             if the property name is invalid
      */
     public NestedMethodProperty(Object instance, String propertyName) {
-        this(instance, propertyName, false);
-    }
-
-    /**
-     * Constructs a nested method property for a given object instance. The
-     * property name is a dot separated string pointing to a nested property,
-     * e.g. "manager.address.street". The <code>nullBeansAllowed</code> controls
-     * the behavior in cases where the intermediate getters may return null
-     * values. If the flag is set to true, calling getValue will return null if
-     * the property or any of its intermediate getters returns null. If set to
-     * false, null values returned by intermediate getters will cause
-     * NullPointerException. The default value is false to ensure backwards
-     * compatibility.
-     * 
-     * @param instance
-     *            top-level bean to which the property applies
-     * @param propertyName
-     *            dot separated nested property name
-     * @param nullBeansAllowed
-     *            set true to allow null values from intermediate getters
-     * @throws IllegalArgumentException
-     *             if the property name is invalid
-     */
-    public NestedMethodProperty(Object instance, String propertyName,
-            boolean nullBeansAllowed) {
         this.instance = instance;
-        this.nullBeansAllowed = nullBeansAllowed;
         initialize(instance.getClass(), propertyName);
     }
 
@@ -138,25 +106,6 @@ public class NestedMethodProperty<T> extends AbstractProperty<T> {
         initialize(instanceClass, propertyName);
     }
 
-    /**
-     * For internal use to deduce property type etc. without a bean instance.
-     * Calling {@link #setValue(Object)} or {@link #getValue()} on properties
-     * constructed this way is not supported.
-     * 
-     * @param instanceClass
-     *            class of the top-level bean
-     * @param propertyName
-     *            dot separated nested property name
-     * @param nullBeansAllowed
-     *            set true to allow null values from intermediate getters
-     */
-    NestedMethodProperty(Class<?> instanceClass, String propertyName,
-            boolean nullBeansAllowed) {
-        instance = null;
-        this.nullBeansAllowed = nullBeansAllowed;
-        initialize(instanceClass, propertyName);
-    }
-
     /**
      * Initializes most of the internal fields based on the top-level bean
      * instance and property name (dot-separated string).
@@ -253,7 +202,7 @@ public class NestedMethodProperty<T> extends AbstractProperty<T> {
             Object object = instance;
             for (Method m : getMethods) {
                 object = m.invoke(object);
-                if (object == null && nullBeansAllowed) {
+                if (object == null) {
                     return null;
                 }
             }

server/src/com/vaadin/data/util/NestedPropertyDescriptor.java

@@ -34,7 +34,6 @@ public class NestedPropertyDescriptor<BT> implements
 
     private final String name;
     private final Class<?> propertyType;
-    private final boolean nullBeansAllowed;
 
     /**
      * Creates a property descriptor that can create MethodProperty instances to
@@ -49,29 +48,10 @@ public class NestedPropertyDescriptor<BT> implements
      */
     public NestedPropertyDescriptor(String name, Class<BT> beanType)
             throws IllegalArgumentException {
-        this(name, beanType, false);
-    }
-
-    /**
-     * Creates a property descriptor that can create MethodProperty instances to
-     * access the underlying bean property.
-     * 
-     * @param name
-     *            of the property in a dotted path format, e.g. "address.street"
-     * @param beanType
-     *            type (class) of the top-level bean
-     * @param nullBeansAllowed
-     *            set true to allow null values from intermediate getters
-     * @throws IllegalArgumentException
-     *             if the property name is invalid
-     */
-    public NestedPropertyDescriptor(String name, Class<BT> beanType,
-            boolean nullBeansAllowed) throws IllegalArgumentException {
         this.name = name;
         NestedMethodProperty<?> property = new NestedMethodProperty<Object>(
-                beanType, name, nullBeansAllowed);
+                beanType, name);
         this.propertyType = property.getType();
-        this.nullBeansAllowed = nullBeansAllowed;
     }
 
     @Override
@@ -86,7 +66,7 @@ public class NestedPropertyDescriptor<BT> implements
 
     @Override
     public Property<?> createProperty(BT bean) {
-        return new NestedMethodProperty<Object>(bean, name, nullBeansAllowed);
+        return new NestedMethodProperty<Object>(bean, name);
     }
 
 }

server/tests/src/com/vaadin/data/util/BeanContainerTest.java

@@ -465,20 +465,11 @@ public class BeanContainerTest extends AbstractBeanContainerTest {
         container.addBean(new NestedMethodPropertyTest.Person("John", null));
         assertTrue(container
                 .addNestedContainerProperty("address.postalCodeObject"));
-        assertTrue(container.addNestedContainerProperty("address.street", true));
+        assertTrue(container.addNestedContainerProperty("address.street"));
         // the nested properties added with allowNullBean setting should return
         // null
         assertNull(container.getContainerProperty("John", "address.street")
                 .getValue());
-        // nested properties added without allowNullBean setting should throw
-        // exception
-        try {
-            container.getContainerProperty("John", "address.postalCodeObject")
-                    .getValue();
-            fail();
-        } catch (Exception e) {
-            // should throw exception
-        }
     }
 
 }

server/tests/src/com/vaadin/data/util/BeanItemContainerTest.java

@@ -729,20 +729,10 @@ public class BeanItemContainerTest extends AbstractBeanContainerTest {
         assertNotNull(container.addBean(john));
         assertTrue(container
                 .addNestedContainerProperty("address.postalCodeObject"));
-        assertTrue(container.addNestedContainerProperty("address.street", true));
-        // the nested properties added with allowNullBean setting should return
-        // null
+        assertTrue(container.addNestedContainerProperty("address.street"));
+        // the nested properties should return null
         assertNull(container.getContainerProperty(john, "address.street")
                 .getValue());
-        // nested properties added without allowNullBean setting should throw
-        // exception
-        try {
-            container.getContainerProperty(john, "address.postalCodeObject")
-                    .getValue();
-            fail();
-        } catch (Exception e) {
-            // should throw exception
-        }
     }
 
     public void testItemAddedEvent() {

server/tests/src/com/vaadin/data/util/NestedMethodPropertyTest.java

@@ -7,9 +7,10 @@ import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
 import java.io.Serializable;
 
-import junit.framework.Assert;
 import junit.framework.TestCase;
 
+import org.junit.Assert;
+
 public class NestedMethodPropertyTest extends TestCase {
 
     public static class Address implements Serializable {
@@ -248,46 +249,16 @@ public class NestedMethodPropertyTest extends TestCase {
                 vaadin, "manager.address.street");
 
         joonas.setAddress(null);
-        try {
-            streetProperty.getValue();
-            fail();
-        } catch (Exception e) {
-            // should get exception
-        }
+        assertNull(streetProperty.getValue());
 
         vaadin.setManager(null);
-        try {
-            managerNameProperty.getValue();
-            fail();
-        } catch (Exception e) {
-            // should get exception
-        }
-        try {
-            streetProperty.getValue();
-            fail();
-        } catch (Exception e) {
-            // should get exception
-        }
+        assertNull(managerNameProperty.getValue());
+        assertNull(streetProperty.getValue());
 
         vaadin.setManager(joonas);
         Assert.assertEquals("Joonas", managerNameProperty.getValue());
-    }
-
-    public void testNullNestedPropertyWithAllowNullBeans() {
-        NestedMethodProperty<String> managerNameProperty = new NestedMethodProperty<String>(
-                vaadin, "manager.name", true);
-        NestedMethodProperty<String> streetProperty = new NestedMethodProperty<String>(
-                vaadin, "manager.address.street", true);
-
-        joonas.setAddress(null);
-        // should return null
         Assert.assertNull(streetProperty.getValue());
 
-        vaadin.setManager(null);
-        Assert.assertNull(managerNameProperty.getValue());
-        vaadin.setManager(joonas);
-        Assert.assertEquals("Joonas", managerNameProperty.getValue());
-        Assert.assertNull(streetProperty.getValue());
     }
 
     public void testMultiLevelNestedPropertySetValue() {
@@ -331,11 +302,11 @@ public class NestedMethodPropertyTest extends TestCase {
         Assert.assertEquals("Ruukinkatu 2-4", property2.getValue());
     }
 
-    public void testSerializationWithNullBeansAllowed() throws IOException,
+    public void testSerializationWithIntermediateNull() throws IOException,
             ClassNotFoundException {
         vaadin.setManager(null);
         NestedMethodProperty<String> streetProperty = new NestedMethodProperty<String>(
-                vaadin, "manager.address.street", true);
+                vaadin, "manager.address.street");
         ByteArrayOutputStream baos = new ByteArrayOutputStream();
         new ObjectOutputStream(baos).writeObject(streetProperty);
         @SuppressWarnings("unchecked")

server/tests/src/com/vaadin/data/util/PropertyDescriptorTest.java

@@ -39,7 +39,8 @@ public class PropertyDescriptorTest extends TestCase {
         Assert.assertEquals("John", property.getValue());
     }
 
-    public void testNestedPropertyDescriptorSerialization() throws Exception {
+    public void testSimpleNestedPropertyDescriptorSerialization()
+            throws Exception {
         NestedPropertyDescriptor<Person> pd = new NestedPropertyDescriptor<Person>(
                 "name", Person.class);
 
@@ -53,10 +54,9 @@ public class PropertyDescriptorTest extends TestCase {
         Assert.assertEquals("John", property.getValue());
     }
 
-    public void testNestedPropertyDescriptorWithNullBeansAllowedSerialization()
-            throws Exception {
+    public void testNestedPropertyDescriptorSerialization() throws Exception {
         NestedPropertyDescriptor<Person> pd = new NestedPropertyDescriptor<Person>(
-                "address.street", Person.class, true);
+                "address.street", Person.class);
 
         ByteArrayOutputStream baos = new ByteArrayOutputStream();
         new ObjectOutputStream(baos).writeObject(pd);