Make binder set null values correctly

Change-Id: I5edf6136f155a9ee5dc5fd6286004f4e8cd8c3db

4 files changed, 41 insertions, 29 deletions

diff --git a/server/src/main/java/com/vaadin/data/Binder.java b/server/src/main/java/com/vaadin/data/Binder.java
index a208ad841f..658160a930 100644
--- a/server/src/main/java/com/vaadin/data/Binder.java
+++ b/server/src/main/java/com/vaadin/data/Binder.java
@@ -433,8 +433,9 @@ public class Binder<BEAN> implements Serializable {
             checkUnbound();
             Objects.requireNonNull(converter, "converter cannot be null");
 
-            return getBinder().createBinding(getField(), converterValidatorChain
-                    .chain(converter), statusChangeHandler);
+            return getBinder().createBinding(getField(),
+                    converterValidatorChain.chain(converter),
+                    statusChangeHandler);
         }
 
         @Override
@@ -482,8 +483,8 @@ public class Binder<BEAN> implements Serializable {
 
         private void bind(BEAN bean) {
             setFieldValue(bean);
-            onValueChange = getField().addValueChangeListener(
-                    e -> storeFieldValue(bean));
+            onValueChange = getField()
+                    .addValueChangeListener(e -> storeFieldValue(bean));
         }
 
         @Override
@@ -498,11 +499,11 @@ public class Binder<BEAN> implements Serializable {
         /**
          * Returns the field value run through all converters and validators.
          *
-         * @return an optional containing the validated and converted value or
-         *         an empty optional if a validator or converter failed
+         * @return a result containing the validated and converted value or
+         *         describing an error
          */
-        private Optional<TARGET> getTargetValue() {
-            return validate().getValue();
+        private Result<TARGET> getTargetValue() {
+            return validate();
         }
 
         private void unbind() {
@@ -523,8 +524,8 @@ public class Binder<BEAN> implements Serializable {
 
         private FIELDVALUE convertDataToFieldType(BEAN bean) {
             return converterValidatorChain.convertToPresentation(
-                    getter.apply(bean), ((AbstractComponent) getField())
-                            .getLocale());
+                    getter.apply(bean),
+                    ((AbstractComponent) getField()).getLocale());
         }
 
         /**
@@ -537,7 +538,7 @@ public class Binder<BEAN> implements Serializable {
         private void storeFieldValue(BEAN bean) {
             assert bean != null;
             if (setter != null) {
-                getTargetValue().ifPresent(value -> setter.accept(bean, value));
+                getTargetValue().ifOk(value -> setter.accept(bean, value));
             }
         }
 
@@ -705,9 +706,8 @@ public class Binder<BEAN> implements Serializable {
 
         List<ValidationError<?>> resultErrors = new ArrayList<>();
         for (BindingImpl<?, ?, ?> binding : bindings) {
-            binding.validate().ifError(errorMessage -> resultErrors
-                    .add(new ValidationError<>(binding.getField(),
-                            errorMessage)));
+            binding.validate().ifError(errorMessage -> resultErrors.add(
+                    new ValidationError<>(binding.getField(), errorMessage)));
         }
         return resultErrors;
     }
@@ -769,8 +769,7 @@ public class Binder<BEAN> implements Serializable {
      * @return the new incomplete binding
      */
     protected <FIELDVALUE, TARGET> BindingImpl<BEAN, FIELDVALUE, TARGET> createBinding(
-            HasValue<FIELDVALUE> field,
-            Converter<FIELDVALUE, TARGET> converter,
+            HasValue<FIELDVALUE> field, Converter<FIELDVALUE, TARGET> converter,
             StatusChangeHandler handler) {
         return new BindingImpl<>(this, field, converter, handler);
     }
diff --git a/server/src/main/java/com/vaadin/data/Result.java b/server/src/main/java/com/vaadin/data/Result.java
index fec5bdd719..07179851e4 100644
--- a/server/src/main/java/com/vaadin/data/Result.java
+++ b/server/src/main/java/com/vaadin/data/Result.java
@@ -155,9 +155,10 @@ public interface Result<R> extends Serializable {
     }
 
     /**
-     * Returns {@code true} if result is an error.
+     * Checks if the result denotes an error.
      *
-     * @return whether the result is an error
+     * @return <code>true</code> if the result denotes an error,
+     *         <code>false</code> otherwise
      */
     public boolean isError();
 
@@ -168,10 +169,4 @@ public interface Result<R> extends Serializable {
      */
     public Optional<String> getMessage();
 
-    /**
-     * Returns an Optional of the value, or an empty Optional if none.
-     *
-     * @return the optional value
-     */
-    public Optional<R> getValue();
 }
diff --git a/server/src/main/java/com/vaadin/data/SimpleResult.java b/server/src/main/java/com/vaadin/data/SimpleResult.java
index c324722baf..56c786942d 100644
--- a/server/src/main/java/com/vaadin/data/SimpleResult.java
+++ b/server/src/main/java/com/vaadin/data/SimpleResult.java
@@ -81,11 +81,6 @@ class SimpleResult<R> implements Result<R> {
     }
 
     @Override
-    public Optional<R> getValue() {
-        return Optional.ofNullable(value);
-    }
-
-    @Override
     public boolean isError() {
         return message != null;
     }
diff --git a/server/src/test/java/com/vaadin/data/BinderTest.java b/server/src/test/java/com/vaadin/data/BinderTest.java
index a93916f729..331d4d30de 100644
--- a/server/src/test/java/com/vaadin/data/BinderTest.java
+++ b/server/src/test/java/com/vaadin/data/BinderTest.java
@@ -180,6 +180,29 @@ public class BinderTest {
     }
 
     @Test
+    public void save_null_beanIsUpdated() {
+        Binder<Person> binder = new Binder<>();
+        binder.forField(nameField).withConverter(fieldValue -> {
+            if ("null".equals(fieldValue)) {
+                return null;
+            } else {
+                return fieldValue;
+            }
+        }, model -> {
+            return model;
+        }).bind(Person::getFirstName, Person::setFirstName);
+
+        Person person = new Person();
+        person.setFirstName("foo");
+
+        nameField.setValue("null");
+
+        binder.save(person);
+
+        Assert.assertNull(person.getFirstName());
+    }
+
+    @Test
     public void load_bound_fieldValueIsUpdated() {
         Binder<Person> binder = new Binder<>();
         binder.bind(nameField, Person::getFirstName, Person::setFirstName);