Add null check to FieldGroup.bind. (#14729)

Change-Id: I56ee44f34307d76c8c98ca3346feed8e7ee2f72e

2 files changed, 62 insertions, 7 deletions

diff --git a/server/src/com/vaadin/data/fieldgroup/FieldGroup.java b/server/src/com/vaadin/data/fieldgroup/FieldGroup.java
index 3a60b6912c..d84a882d80 100644
--- a/server/src/com/vaadin/data/fieldgroup/FieldGroup.java
+++ b/server/src/com/vaadin/data/fieldgroup/FieldGroup.java
@@ -243,15 +243,13 @@ public class FieldGroup implements Serializable {
      * @param propertyId
      *            The propertyId to bind to the field
      * @throws BindException
-     *             If the property id is already bound to another field by this
-     *             field binder
+     *             If the field is null or the property id is already bound to
+     *             another field by this field binder
      */
     public void bind(Field<?> field, Object propertyId) throws BindException {
-        if (propertyIdToField.containsKey(propertyId)
-                && propertyIdToField.get(propertyId) != field) {
-            throw new BindException("Property id " + propertyId
-                    + " is already bound to another field");
-        }
+        throwIfFieldIsNull(field, propertyId);
+        throwIfPropertyIdAlreadyBound(field, propertyId);
+
         fieldToPropertyId.put(field, propertyId);
         propertyIdToField.put(propertyId, field);
         if (itemDataSource == null) {
@@ -263,6 +261,23 @@ public class FieldGroup implements Serializable {
         configureField(field);
     }
 
+    private void throwIfFieldIsNull(Field<?> field, Object propertyId) {
+        if (field == null) {
+            throw new BindException(
+                    String.format(
+                            "Cannot bind property id '%s' to a null field.",
+                            propertyId));
+        }
+    }
+
+    private void throwIfPropertyIdAlreadyBound(Field<?> field, Object propertyId) {
+        if (propertyIdToField.containsKey(propertyId)
+                && propertyIdToField.get(propertyId) != field) {
+            throw new BindException("Property id " + propertyId
+                    + " is already bound to another field");
+        }
+    }
+
     private <T> Property.Transactional<T> wrapInTransactionalProperty(
             Property<T> itemProperty) {
         return new TransactionalPropertyWrapper<T>(itemProperty);
diff --git a/server/tests/src/com/vaadin/data/fieldgroup/FieldGroupTests.java b/server/tests/src/com/vaadin/data/fieldgroup/FieldGroupTests.java
new file mode 100644
index 0000000000..eb8f21c839
--- /dev/null
+++ b/server/tests/src/com/vaadin/data/fieldgroup/FieldGroupTests.java
@@ -0,0 +1,40 @@
+package com.vaadin.data.fieldgroup;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.Is.is;
+import static org.mockito.Mockito.mock;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import com.vaadin.ui.Field;
+
+public class FieldGroupTests {
+
+    private FieldGroup sut;
+    private Field field;
+
+    @Before
+    public void setup() {
+        sut = new FieldGroup();
+        field = mock(Field.class);
+    }
+
+    @Test
+    public void fieldIsBound() {
+        sut.bind(field, "foobar");
+
+        assertThat(sut.getField("foobar"), is(field));
+    }
+
+    @Test(expected = FieldGroup.BindException.class)
+    public void cannotBindToAlreadyBoundProperty() {
+        sut.bind(field, "foobar");
+        sut.bind(mock(Field.class), "foobar");
+    }
+
+    @Test(expected = FieldGroup.BindException.class)
+    public void cannotBindNullField() {
+        sut.bind(null, "foobar");
+    }
+}