From 8d6256e98a63222f4b4969612d8d777a31974613 Mon Sep 17 00:00:00 2001 From: Artur Signell Date: Fri, 29 Nov 2013 11:53:20 +0200 Subject: [PATCH] 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 --- .../data/util/AbstractBeanContainer.java | 67 +++---------------- server/src/com/vaadin/data/util/BeanItem.java | 30 ++------- .../data/util/NestedMethodProperty.java | 63 ++--------------- .../data/util/NestedPropertyDescriptor.java | 24 +------ .../vaadin/data/util/BeanContainerTest.java | 11 +-- .../data/util/BeanItemContainerTest.java | 14 +--- .../data/util/NestedMethodPropertyTest.java | 43 ++---------- .../data/util/PropertyDescriptorTest.java | 8 +-- 8 files changed, 35 insertions(+), 225 deletions(-) diff --git a/server/src/com/vaadin/data/util/AbstractBeanContainer.java b/server/src/com/vaadin/data/util/AbstractBeanContainer.java index 67239996a2..d94588bdc9 100644 --- a/server/src/com/vaadin/data/util/AbstractBeanContainer.java +++ b/server/src/com/vaadin/data/util/AbstractBeanContainer.java @@ -848,8 +848,9 @@ public abstract class AbstractBeanContainer 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 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 nullBeansAllowed must - * be set to true. If the nullBeansAllowed 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 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 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 - * nullBeansAllowed is set to true, all intermediate getters must - * exist and must return non-null values when the property values are - * accessed. If the nullBeansAllowed 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> pds = BeanItem .getPropertyDescriptors((Class) propertyType); for (String subPropertyId : pds.keySet()) { String qualifiedPropertyId = propertyId + "." + subPropertyId; NestedPropertyDescriptor pd = new NestedPropertyDescriptor( - qualifiedPropertyId, (Class) type, - nullBeansAllowed); + qualifiedPropertyId, (Class) type); model.put(qualifiedPropertyId, pd); model.remove(propertyId); for (BeanItem item : itemIdToItem.values()) { diff --git a/server/src/com/vaadin/data/util/BeanItem.java b/server/src/com/vaadin/data/util/BeanItem.java index 4834fe4f89..49f5f95898 100644 --- a/server/src/com/vaadin/data/util/BeanItem.java +++ b/server/src/com/vaadin/data/util/BeanItem.java @@ -255,39 +255,19 @@ public class BeanItem 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( getBean(), nestedPropertyId)); } - /** - * Adds a nested property to the item. If the nullBeansAllowed - * 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( - getBean(), nestedPropertyId, nullBeansAllowed)); - } - /** * Gets the underlying JavaBean object. * diff --git a/server/src/com/vaadin/data/util/NestedMethodProperty.java b/server/src/com/vaadin/data/util/NestedMethodProperty.java index 7a3963c17e..8fe3b9d4c5 100644 --- a/server/src/com/vaadin/data/util/NestedMethodProperty.java +++ b/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 nullBeansAllowed 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 extends AbstractProperty { */ 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 type; /* Special serialization to handle method references */ @@ -85,6 +77,8 @@ public class NestedMethodProperty extends AbstractProperty { * 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". + *

+ * 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 extends AbstractProperty { * 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 nullBeansAllowed 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 extends AbstractProperty { 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 extends AbstractProperty { Object object = instance; for (Method m : getMethods) { object = m.invoke(object); - if (object == null && nullBeansAllowed) { + if (object == null) { return null; } } diff --git a/server/src/com/vaadin/data/util/NestedPropertyDescriptor.java b/server/src/com/vaadin/data/util/NestedPropertyDescriptor.java index 67eb30fae5..b2055fe776 100644 --- a/server/src/com/vaadin/data/util/NestedPropertyDescriptor.java +++ b/server/src/com/vaadin/data/util/NestedPropertyDescriptor.java @@ -34,7 +34,6 @@ public class NestedPropertyDescriptor 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 implements */ public NestedPropertyDescriptor(String name, Class 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 beanType, - boolean nullBeansAllowed) throws IllegalArgumentException { this.name = name; NestedMethodProperty property = new NestedMethodProperty( - beanType, name, nullBeansAllowed); + beanType, name); this.propertyType = property.getType(); - this.nullBeansAllowed = nullBeansAllowed; } @Override @@ -86,7 +66,7 @@ public class NestedPropertyDescriptor implements @Override public Property createProperty(BT bean) { - return new NestedMethodProperty(bean, name, nullBeansAllowed); + return new NestedMethodProperty(bean, name); } } diff --git a/server/tests/src/com/vaadin/data/util/BeanContainerTest.java b/server/tests/src/com/vaadin/data/util/BeanContainerTest.java index 2dcbb4aed8..a6c3bb0b8b 100644 --- a/server/tests/src/com/vaadin/data/util/BeanContainerTest.java +++ b/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 - } } } diff --git a/server/tests/src/com/vaadin/data/util/BeanItemContainerTest.java b/server/tests/src/com/vaadin/data/util/BeanItemContainerTest.java index 35f09fc8f3..b9633753b4 100644 --- a/server/tests/src/com/vaadin/data/util/BeanItemContainerTest.java +++ b/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() { diff --git a/server/tests/src/com/vaadin/data/util/NestedMethodPropertyTest.java b/server/tests/src/com/vaadin/data/util/NestedMethodPropertyTest.java index d517322010..1133626df9 100644 --- a/server/tests/src/com/vaadin/data/util/NestedMethodPropertyTest.java +++ b/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 managerNameProperty = new NestedMethodProperty( - vaadin, "manager.name", true); - NestedMethodProperty streetProperty = new NestedMethodProperty( - 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 streetProperty = new NestedMethodProperty( - vaadin, "manager.address.street", true); + vaadin, "manager.address.street"); ByteArrayOutputStream baos = new ByteArrayOutputStream(); new ObjectOutputStream(baos).writeObject(streetProperty); @SuppressWarnings("unchecked") diff --git a/server/tests/src/com/vaadin/data/util/PropertyDescriptorTest.java b/server/tests/src/com/vaadin/data/util/PropertyDescriptorTest.java index 0ae76430f6..12ded84fe2 100644 --- a/server/tests/src/com/vaadin/data/util/PropertyDescriptorTest.java +++ b/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 pd = new NestedPropertyDescriptor( "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 pd = new NestedPropertyDescriptor( - "address.street", Person.class, true); + "address.street", Person.class); ByteArrayOutputStream baos = new ByteArrayOutputStream(); new ObjectOutputStream(baos).writeObject(pd); -- 2.39.5