Binding a null bean should clear the bound field values (#8288)

* Add clear field values #8287

Added functionality to make it possible to clear bound field values manually
or when setting a null bean to binder.

Binder will now clear bound fields for setBean(null) or if using readBean() functionality
when calling the clear() method. Binder.clear() will throw a IllegalStateException if
we have a bound bean.

* MultiSelectionModel MultiSelect should implement emptyValue

* Removing public clear() method and clearing field values only through
setBean(null) and readBean(null) to be consistent and not have
confusing functionality.

* Simplified clearFields() as we don't need to check if field is readOnly

* Document in removeBean JavaDoc that it will also clear the bound fields
if we have a bean the is removed.

* Binder should not have changes after clearing fields.

* Add getEmptyValue as default to MultiSelect.
Multiselect default for empty value should be an empty set.

* Revert "Add getEmptyValue as default to MultiSelect."

This reverts commit 45de8f45c4a1e3e45ab8a0353ab2a48bb4f1be6f.

* Merge branch 'master' into issue/8287_Clear_bound_fields

* Remove override for default implementation.

* Fire status change event on clearFields if we had changes before clearing.

* Formatting.

2 files changed, 139 insertions, 35 deletions

diff --git a/server/src/main/java/com/vaadin/data/Binder.java b/server/src/main/java/com/vaadin/data/Binder.java
index 8c5bd09938..50bb182920 100644
--- a/server/src/main/java/com/vaadin/data/Binder.java
+++ b/server/src/main/java/com/vaadin/data/Binder.java
@@ -1324,13 +1324,14 @@ public class Binder<BEAN> implements Serializable {
      *
      * @param bean
      *            the bean to edit, or {@code null} to remove a currently bound
-     *            bean
+     *            bean and clear bound fields
      */
     public void setBean(BEAN bean) {
         checkBindingsCompleted("setBean");
         if (bean == null) {
             if (this.bean != null) {
                 doRemoveBean(true);
+                clearFields();
             }
         } else {
             doRemoveBean(false);
@@ -1345,8 +1346,8 @@ public class Binder<BEAN> implements Serializable {
     }
 
     /**
-     * Removes the currently set bean, if any. If there is no bound bean, does
-     * nothing.
+     * Removes the currently set bean and clears bound fields. If there is no
+     * bound bean, does nothing.
      * <p>
      * This is a shorthand for {@link #setBean(Object)} with {@code null} bean.
      */
@@ -1367,17 +1368,20 @@ public class Binder<BEAN> implements Serializable {
      * @see #writeBean(Object)
      *
      * @param bean
-     *            the bean whose property values to read, not null
+     *            the bean whose property values to read or {@code null} to
+     *            clear bound fields
      */
     public void readBean(BEAN bean) {
-        Objects.requireNonNull(bean, "bean cannot be null");
         checkBindingsCompleted("readBean");
-        setHasChanges(false);
-        bindings.forEach(binding -> binding.initFieldValue(bean));
-
-        getValidationStatusHandler().statusChange(
-                BinderValidationStatus.createUnresolvedStatus(this));
-        fireStatusChangeEvent(false);
+        if (bean == null) {
+            clearFields();
+        } else {
+            setHasChanges(false);
+            bindings.forEach(binding -> binding.initFieldValue(bean));
+            getValidationStatusHandler().statusChange(
+                    BinderValidationStatus.createUnresolvedStatus(this));
+            fireStatusChangeEvent(false);
+        }
     }
 
     /**
@@ -1552,6 +1556,17 @@ public class Binder<BEAN> implements Serializable {
     }
 
     /**
+     * Clear all the bound fields for this binder.
+     */
+    private void clearFields() {
+        bindings.forEach(binding -> binding.getField().clear());
+        if (hasChanges()) {
+            fireStatusChangeEvent(false);
+        }
+        setHasChanges(false);
+    }
+
+    /**
      * Validates the values of all bound fields and returns the validation
      * status.
      * <p>
diff --git a/server/src/test/java/com/vaadin/data/BinderTest.java b/server/src/test/java/com/vaadin/data/BinderTest.java
index 8dc4c76c3b..ee92c86f58 100644
--- a/server/src/test/java/com/vaadin/data/BinderTest.java
+++ b/server/src/test/java/com/vaadin/data/BinderTest.java
@@ -1,9 +1,11 @@
 package com.vaadin.data;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
 
 import java.util.Locale;
 import java.util.Objects;
@@ -40,6 +42,93 @@ public class BinderTest extends BinderTestBase<Binder<Person>, Person> {
         assertNull(binder.getBean());
     }
 
+    @Test
+    public void bindNullBean_FieldsAreCleared() {
+        binder.forField(nameField).bind(Person::getFirstName,
+                Person::setFirstName);
+        binder.forField(ageField)
+                .withConverter(new StringToIntegerConverter(""))
+                .bind(Person::getAge, Person::setAge);
+        binder.setBean(item);
+        assertEquals("No name field value","Johannes", nameField.getValue());
+        assertEquals("No age field value", "32", ageField.getValue());
+
+        binder.setBean(null);
+        assertEquals("Name field not empty", "", nameField.getValue());
+        assertEquals("Age field not empty", "", ageField.getValue());
+    }
+
+    @Test
+    public void clearForReadBean_boundFieldsAreCleared() {
+        binder.forField(nameField).bind(Person::getFirstName,
+                Person::setFirstName);
+        binder.forField(ageField)
+                .withConverter(new StringToIntegerConverter(""))
+                .bind(Person::getAge, Person::setAge);
+        binder.readBean(item);
+
+        assertEquals("No name field value","Johannes", nameField.getValue());
+        assertEquals("No age field value", "32", ageField.getValue());
+
+        binder.readBean(null);
+        assertEquals("Name field not empty", "", nameField.getValue());
+        assertEquals("Age field not empty", "", ageField.getValue());
+    }
+
+    @Test
+    public void clearReadOnlyField_shouldClearField() {
+        binder.forField(nameField).bind(Person::getFirstName,
+                Person::setFirstName);
+
+        // Make name field read only
+        nameField.setReadOnly(true);
+
+        binder.setBean(item);
+        assertEquals("No name field value", "Johannes", nameField.getValue());
+
+        binder.setBean(null);
+
+        assertEquals("ReadOnly field not empty","", nameField.getValue());
+    }
+
+    @Test
+    public void clearBean_setsHasChangesToFalse() {
+        binder.forField(nameField).bind(Person::getFirstName,
+                Person::setFirstName);
+
+        // Make name field read only
+        nameField.setReadOnly(true);
+
+        binder.readBean(item);
+        assertEquals("No name field value", "Johannes", nameField.getValue());
+        nameField.setValue("James");
+
+        assertTrue("Binder did not have value changes", binder.hasChanges());
+
+        binder.readBean(null);
+
+        assertFalse("Binder has changes after clearing all fields",
+                binder.hasChanges());
+
+    }
+
+    @Test
+    public void clearReadOnlyBinder_shouldClearFields() {
+        binder.forField(nameField).bind(Person::getFirstName,
+                Person::setFirstName);
+        binder.forField(ageField)
+                .withConverter(new StringToIntegerConverter(""))
+                .bind(Person::getAge, Person::setAge);
+
+        binder.setReadOnly(true);
+
+        binder.setBean(item);
+
+        binder.setBean(null);
+        assertEquals("ReadOnly name field not empty", "", nameField.getValue());
+        assertEquals("ReadOnly age field not empty", "", ageField.getValue());
+    }
+
     @Test(expected = NullPointerException.class)
     public void bindNullField_throws() {
         binder.forField(null);
@@ -234,18 +323,18 @@ public class BinderTest extends BinderTestBase<Binder<Person>, Person> {
         binder.bind(nullTextField, Person::getFirstName, Person::setFirstName);
         binder.setBean(namelessPerson);
 
-        Assert.assertTrue(nullTextField.isEmpty());
+        assertTrue(nullTextField.isEmpty());
         Assert.assertEquals(null, namelessPerson.getFirstName());
 
         // Change value, see that textfield is not empty and bean is updated.
         nullTextField.setValue("");
-        Assert.assertFalse(nullTextField.isEmpty());
+        assertFalse(nullTextField.isEmpty());
         Assert.assertEquals("First name of person was not properly updated", "",
                 namelessPerson.getFirstName());
 
         // Verify that default null representation does not map back to null
         nullTextField.setValue("null");
-        Assert.assertTrue(nullTextField.isEmpty());
+        assertTrue(nullTextField.isEmpty());
         Assert.assertEquals("Default one-way null representation failed.",
                 "null", namelessPerson.getFirstName());
     }
@@ -257,7 +346,7 @@ public class BinderTest extends BinderTestBase<Binder<Person>, Person> {
         binder.forField(nameField).withNullRepresentation(nullRepresentation)
                 .bind(Person::getFirstName, Person::setFirstName);
 
-        Assert.assertFalse("First name in item should not be null",
+        assertFalse("First name in item should not be null",
                 Objects.isNull(item.getFirstName()));
         binder.setBean(item);
 
@@ -329,13 +418,13 @@ public class BinderTest extends BinderTestBase<Binder<Person>, Person> {
     @Test
     public void setRequired_withErrorMessage_fieldGetsRequiredIndicatorAndValidator() {
         TextField textField = new TextField();
-        Assert.assertFalse(textField.isRequiredIndicatorVisible());
+        assertFalse(textField.isRequiredIndicatorVisible());
 
         BindingBuilder<Person, String> binding = binder.forField(textField);
-        Assert.assertFalse(textField.isRequiredIndicatorVisible());
+        assertFalse(textField.isRequiredIndicatorVisible());
 
         binding.asRequired("foobar");
-        Assert.assertTrue(textField.isRequiredIndicatorVisible());
+        assertTrue(textField.isRequiredIndicatorVisible());
 
         binding.bind(Person::getFirstName, Person::setFirstName);
         binder.setBean(item);
@@ -348,17 +437,17 @@ public class BinderTest extends BinderTestBase<Binder<Person>, Person> {
 
         textField.setValue("value");
         Assert.assertNull(textField.getErrorMessage());
-        Assert.assertTrue(textField.isRequiredIndicatorVisible());
+        assertTrue(textField.isRequiredIndicatorVisible());
     }
 
     @Test
     public void setRequired_withErrorMessageProvider_fieldGetsRequiredIndicatorAndValidator() {
         TextField textField = new TextField();
         textField.setLocale(Locale.CANADA);
-        Assert.assertFalse(textField.isRequiredIndicatorVisible());
+        assertFalse(textField.isRequiredIndicatorVisible());
 
         BindingBuilder<Person, String> binding = binder.forField(textField);
-        Assert.assertFalse(textField.isRequiredIndicatorVisible());
+        assertFalse(textField.isRequiredIndicatorVisible());
         AtomicInteger invokes = new AtomicInteger();
 
         binding.asRequired(context -> {
@@ -366,7 +455,7 @@ public class BinderTest extends BinderTestBase<Binder<Person>, Person> {
             Assert.assertSame(Locale.CANADA, context.getLocale().get());
             return "foobar";
         });
-        Assert.assertTrue(textField.isRequiredIndicatorVisible());
+        assertTrue(textField.isRequiredIndicatorVisible());
 
         binding.bind(Person::getFirstName, Person::setFirstName);
         binder.setBean(item);
@@ -383,7 +472,7 @@ public class BinderTest extends BinderTestBase<Binder<Person>, Person> {
 
         textField.setValue("value");
         Assert.assertNull(textField.getErrorMessage());
-        Assert.assertTrue(textField.isRequiredIndicatorVisible());
+        assertTrue(textField.isRequiredIndicatorVisible());
     }
 
     @Test
@@ -441,13 +530,13 @@ public class BinderTest extends BinderTestBase<Binder<Person>, Person> {
 
         binder.setReadOnly(true);
 
-        Assert.assertTrue(nameField.isReadOnly());
-        Assert.assertFalse(ageField.isReadOnly());
+        assertTrue(nameField.isReadOnly());
+        assertFalse(ageField.isReadOnly());
 
         binder.setReadOnly(false);
 
-        Assert.assertFalse(nameField.isReadOnly());
-        Assert.assertFalse(ageField.isReadOnly());
+        assertFalse(nameField.isReadOnly());
+        assertFalse(ageField.isReadOnly());
     }
 
     @Test
@@ -463,13 +552,13 @@ public class BinderTest extends BinderTestBase<Binder<Person>, Person> {
 
         binder.setReadOnly(true);
 
-        Assert.assertTrue(nameField.isReadOnly());
-        Assert.assertTrue(ageField.isReadOnly());
+        assertTrue(nameField.isReadOnly());
+        assertTrue(ageField.isReadOnly());
 
         binder.setReadOnly(false);
 
-        Assert.assertFalse(nameField.isReadOnly());
-        Assert.assertFalse(ageField.isReadOnly());
+        assertFalse(nameField.isReadOnly());
+        assertFalse(ageField.isReadOnly());
     }
 
     @Test
@@ -485,13 +574,13 @@ public class BinderTest extends BinderTestBase<Binder<Person>, Person> {
 
         binder.setReadOnly(true);
 
-        Assert.assertTrue(nameField.isReadOnly());
-        Assert.assertTrue(ageField.isReadOnly());
+        assertTrue(nameField.isReadOnly());
+        assertTrue(ageField.isReadOnly());
 
         binder.setReadOnly(false);
 
-        Assert.assertFalse(nameField.isReadOnly());
-        Assert.assertFalse(ageField.isReadOnly());
+        assertFalse(nameField.isReadOnly());
+        assertFalse(ageField.isReadOnly());
     }
 
     @Test