Initial support for null representations in Binder

Change-Id: I1325c629da220317506306fe8f6fff5c0494d9d9

6 files changed, 159 insertions, 36 deletions

diff --git a/server/src/main/java/com/vaadin/data/BeanBinder.java b/server/src/main/java/com/vaadin/data/BeanBinder.java
index c73b89de81..4af38f7862 100644
--- a/server/src/main/java/com/vaadin/data/BeanBinder.java
+++ b/server/src/main/java/com/vaadin/data/BeanBinder.java
@@ -269,8 +269,11 @@ public class BeanBinder<BEAN> extends Binder<BEAN> {
     @Override
     public <FIELDVALUE> BeanBinding<BEAN, FIELDVALUE, FIELDVALUE> forField(
             HasValue<FIELDVALUE> field) {
-        return createBinding(field, Converter.identity(),
-                this::handleValidationStatus);
+        BeanBindingImpl<BEAN, FIELDVALUE, FIELDVALUE> binding = createBinding(
+                field, Converter.identity(), this::handleValidationStatus);
+        return binding.withConverter(fieldValue -> fieldValue,
+                modelValue -> modelValue != null ? modelValue
+                        : field.getEmptyValue());
     }
 
     @Override
diff --git a/server/src/main/java/com/vaadin/data/Binder.java b/server/src/main/java/com/vaadin/data/Binder.java
index df29f8d480..8603ed9946 100644
--- a/server/src/main/java/com/vaadin/data/Binder.java
+++ b/server/src/main/java/com/vaadin/data/Binder.java
@@ -282,6 +282,23 @@ public class Binder<BEAN> implements Serializable {
         }
 
         /**
+         * Maps binding value {@code null} to given null representation and back
+         * to {@code null} when converting back to model value.
+         *
+         * @param nullRepresentation
+         *            the value to use instead of {@code null}
+         * @return a new binding with null representation handling.
+         */
+        public default Binding<BEAN, FIELDVALUE, TARGET> withNullRepresentation(
+                TARGET nullRepresentation) {
+            return withConverter(
+                    fieldValue -> Objects.equals(fieldValue, nullRepresentation)
+                            ? null : fieldValue,
+                    modelValue -> Objects.equals(modelValue, null)
+                            ? nullRepresentation : modelValue);
+        }
+
+        /**
          * Gets the field the binding uses.
          *
          * @return the field for the binding
@@ -617,10 +634,6 @@ public class Binder<BEAN> implements Serializable {
             return validationStatus;
         }
 
-        private void setBeanValue(BEAN bean, TARGET value) {
-            setter.accept(bean, value);
-        }
-
         private void notifyStatusHandler(ValidationStatus<?> status) {
             statusHandler.accept(status);
         }
@@ -696,6 +709,13 @@ public class Binder<BEAN> implements Serializable {
      * {@link Binding#bind(Function, BiConsumer) Binding.bind} which completes
      * the binding. Until {@code Binding.bind} is called, the binding has no
      * effect.
+     * <p>
+     * <strong>Note:</strong> Not all {@link HasValue} implementations support
+     * passing {@code null} as the value. For these the Binder will
+     * automatically change {@code null} to a null representation provided by
+     * {@link HasValue#getEmptyValue()}. This conversion is one-way only, if you
+     * want to have a two-way mapping back to {@code null}, use
+     * {@link Binding#withNullRepresentation(Object))}.
      *
      * @param <FIELDVALUE>
      *            the value type of the field
@@ -712,8 +732,12 @@ public class Binder<BEAN> implements Serializable {
         clearError(field);
         getStatusLabel().ifPresent(label -> label.setValue(""));
 
-        return createBinding(field, Converter.identity(),
-                this::handleValidationStatus);
+        BindingImpl<BEAN, FIELDVALUE, FIELDVALUE> createBinding = createBinding(
+                field, Converter.identity(), this::handleValidationStatus);
+
+        return createBinding.withConverter(fieldValue -> fieldValue,
+                modelValue -> modelValue != null ? modelValue
+                        : field.getEmptyValue());
     }
 
     /**
@@ -802,6 +826,14 @@ public class Binder<BEAN> implements Serializable {
      * Use the {@link #forField(HasValue)} overload instead if you want to
      * further configure the new binding.
      * <p>
+     * <strong>Note:</strong> Not all {@link HasValue} implementations support
+     * passing {@code null} as the value. For these the Binder will
+     * automatically change {@code null} to a null representation provided by
+     * {@link HasValue#getEmptyValue()}. This conversion is one-way only, if you
+     * want to have a two-way mapping back to {@code null}, use
+     * {@link #forField(HasValue)} and
+     * {@link Binding#withNullRepresentation(Object))}.
+     * <p>
      * When a bean is bound with {@link Binder#bind(BEAN)}, the field value is
      * set to the return value of the given getter. The property value is then
      * updated via the given setter whenever the field value changes. The setter
@@ -922,7 +954,7 @@ public class Binder<BEAN> implements Serializable {
      * <pre>
      * class Feature {
      *     public enum Browser { CHROME, EDGE, FIREFOX, IE, OPERA, SAFARI }
-    
+
      *     public Set&lt;Browser> getSupportedBrowsers() { ... }
      *     public void setSupportedBrowsers(Set&lt;Browser> title) { ... }
      * }
@@ -1090,8 +1122,8 @@ public class Binder<BEAN> implements Serializable {
 
         // Save old bean values so we can restore them if validators fail
         Map<Binding<BEAN, ?, ?>, Object> oldValues = new HashMap<>();
-        bindings.forEach(binding -> oldValues.put(binding,
-                binding.convertDataToFieldType(bean)));
+        bindings.forEach(
+                binding -> oldValues.put(binding, binding.getter.apply(bean)));
 
         bindings.forEach(binding -> binding.storeFieldValue(bean));
         // Now run bean level validation against the updated bean
@@ -1100,8 +1132,8 @@ public class Binder<BEAN> implements Serializable {
                 .findAny().isPresent();
         if (hasErrors) {
             // Bean validator failed, revert values
-            bindings.forEach((BindingImpl binding) -> binding.setBeanValue(bean,
-                    oldValues.get(binding)));
+            bindings.forEach((BindingImpl binding) -> binding.setter
+                    .accept(bean, oldValues.get(binding)));
         } else {
             // Save successful, reset hasChanges to false
             setHasChanges(false);
diff --git a/server/src/main/java/com/vaadin/data/HasValue.java b/server/src/main/java/com/vaadin/data/HasValue.java
index 8333c8a02a..26fd345d91 100644
--- a/server/src/main/java/com/vaadin/data/HasValue.java
+++ b/server/src/main/java/com/vaadin/data/HasValue.java
@@ -17,6 +17,9 @@ package com.vaadin.data;
 
 import java.io.Serializable;
 import java.lang.reflect.Method;
+import java.util.Objects;
+import java.util.function.BiConsumer;
+import java.util.function.Function;
 
 import com.vaadin.event.ConnectorEvent;
 import com.vaadin.event.EventListener;
@@ -116,8 +119,8 @@ public interface HasValue<V> extends Serializable {
      * @see Registration
      */
     @FunctionalInterface
-    public interface ValueChangeListener<V> extends
-            EventListener<ValueChange<V>> {
+    public interface ValueChangeListener<V>
+            extends EventListener<ValueChange<V>> {
 
         @Deprecated
         public static final Method VALUE_CHANGE_METHOD = ReflectTools
@@ -170,4 +173,29 @@ public interface HasValue<V> extends Serializable {
      */
     public Registration addValueChangeListener(
             ValueChangeListener<? super V> listener);
+
+    /**
+     * Returns the value that represents an empty value.
+     * <p>
+     * By default {@link HasValue} is expected to support {@code null} as empty
+     * values. Specific implementations might not support this.
+     *
+     * @return empty value
+     * @see Binder#bind(HasValue, Function, BiConsumer)
+     */
+    public default V getEmptyValue() {
+        return null;
+    }
+
+    /**
+     * Returns whether this {@code HasValue} is considered to be empty.
+     * <p>
+     * By default this is an equality check between current value and empty
+     * value.
+     *
+     * @return {@code true} if considered empty; {@code false} if not
+     */
+    public default boolean isEmpty() {
+        return Objects.equals(getValue(), getEmptyValue());
+    }
 }
diff --git a/server/src/main/java/com/vaadin/ui/AbstractTextField.java b/server/src/main/java/com/vaadin/ui/AbstractTextField.java
index b5119ec92c..9c2720efd8 100644
--- a/server/src/main/java/com/vaadin/ui/AbstractTextField.java
+++ b/server/src/main/java/com/vaadin/ui/AbstractTextField.java
@@ -17,6 +17,7 @@
 package com.vaadin.ui;
 
 import java.util.Collection;
+import java.util.Objects;
 
 import org.jsoup.nodes.Attributes;
 import org.jsoup.nodes.Element;
@@ -80,11 +81,8 @@ public abstract class AbstractTextField extends AbstractField<String>
 
     @Override
     public void setValue(String value) {
-        if (value == null) {
-            setValue("", false);
-        } else {
-            setValue(value, false);
-        }
+        Objects.requireNonNull(value, "Null value not supported");
+        setValue(value, false);
     }
 
     /**
@@ -270,16 +268,6 @@ public abstract class AbstractTextField extends AbstractField<String>
         setValue("");
     }
 
-    /**
-     * Checks if the field is empty.
-     *
-     * @return <code>true</code> if the field value is an empty string,
-     *         <code>false</code> otherwise
-     */
-    public boolean isEmpty() {
-        return "".equals(getValue());
-    }
-
     @Override
     public void writeDesign(Element design, DesignContext designContext) {
         super.writeDesign(design, designContext);
@@ -299,4 +287,9 @@ public abstract class AbstractTextField extends AbstractField<String>
         customAttributes.add("cursor-position");
         return customAttributes;
     }
+
+    @Override
+    public String getEmptyValue() {
+        return "";
+    }
 }
diff --git a/server/src/test/java/com/vaadin/data/BinderConverterValidatorTest.java b/server/src/test/java/com/vaadin/data/BinderConverterValidatorTest.java
index d5959eb459..73443155c7 100644
--- a/server/src/test/java/com/vaadin/data/BinderConverterValidatorTest.java
+++ b/server/src/test/java/com/vaadin/data/BinderConverterValidatorTest.java
@@ -1,12 +1,12 @@
 /*
  * Copyright 2000-2016 Vaadin Ltd.
- * 
+ *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
  * use this file except in compliance with the License. You may obtain a copy of
  * the License at
- * 
+ *
  * http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
  * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
@@ -30,6 +30,7 @@ import org.junit.Before;
 import org.junit.Test;
 
 import com.vaadin.data.Binder.Binding;
+import com.vaadin.data.util.converter.StringToIntegerConverter;
 import com.vaadin.data.validator.NotEmptyValidator;
 import com.vaadin.server.AbstractErrorMessage;
 import com.vaadin.server.ErrorMessage;
@@ -38,8 +39,8 @@ import com.vaadin.tests.data.bean.Person;
 import com.vaadin.ui.Label;
 import com.vaadin.ui.TextField;
 
-public class BinderConverterValidatorTest extends
-        BinderTestBase<Binder<Person>, Person> {
+public class BinderConverterValidatorTest
+        extends BinderTestBase<Binder<Person>, Person> {
 
     private static class StatusBean {
         private String status;
@@ -684,4 +685,24 @@ public class BinderConverterValidatorTest extends
                 .bind(Person::getAge, Person::setAge);
         binder.bind(item);
     }
+
+    @Test(expected = ValidationException.class)
+    public void save_beanValidationErrorsWithConverter()
+            throws ValidationException {
+        Binder<Person> binder = new Binder<>();
+        binder.forField(ageField)
+                .withConverter(new StringToIntegerConverter("Can't convert"))
+                .bind(Person::getAge, Person::setAge);
+
+        binder.withValidator(Validator.from(person -> false, "b"));
+
+        Person person = new Person();
+        ageField.setValue("1");
+        try {
+            binder.save(person);
+        } finally {
+            // Bean should have been updated for item validation but reverted
+            Assert.assertEquals(0, person.getAge());
+        }
+    }
 }
diff --git a/server/src/test/java/com/vaadin/data/BinderTest.java b/server/src/test/java/com/vaadin/data/BinderTest.java
index b5e84cddd4..08af67ed32 100644
--- a/server/src/test/java/com/vaadin/data/BinderTest.java
+++ b/server/src/test/java/com/vaadin/data/BinderTest.java
@@ -9,6 +9,8 @@ import org.junit.Before;
 import org.junit.Test;
 
 import com.vaadin.tests.data.bean.Person;
+import com.vaadin.tests.data.bean.Sex;
+import com.vaadin.ui.TextField;
 
 public class BinderTest extends BinderTestBase<Binder<Person>, Person> {
 
@@ -174,4 +176,48 @@ public class BinderTest extends BinderTestBase<Binder<Person>, Person> {
         binder.bind(nameField, Person::getFirstName, Person::setFirstName);
         binder.bind(item);
     }
-}
+
+    @Test
+    public void binding_with_null_representation() {
+        String nullRepresentation = "Some arbitrary text";
+        String realName = "John";
+        Person namelessPerson = new Person(null, "Doe", "", 25, Sex.UNKNOWN,
+                null);
+
+        binder.forField(nameField).withNullRepresentation(nullRepresentation)
+                .bind(Person::getFirstName, Person::setFirstName);
+
+        binder.bind(namelessPerson);
+        Assert.assertEquals(nullRepresentation, nameField.getValue());
+
+        nameField.setValue(realName);
+        Assert.assertEquals(realName, namelessPerson.getFirstName());
+
+        nameField.setValue(nullRepresentation);
+        Assert.assertEquals(null, namelessPerson.getFirstName());
+    }
+
+    @Test
+    public void binding_with_default_null_representation() {
+        TextField nullTextField = new TextField() {
+            @Override
+            public String getEmptyValue() {
+                return "null";
+            }
+        };
+
+        Person namelessPerson = new Person(null, "Doe", "", 25, Sex.UNKNOWN,
+                null);
+        binder.bind(nullTextField, Person::getFirstName, Person::setFirstName);
+        binder.bind(namelessPerson);
+
+        Assert.assertTrue(nullTextField.isEmpty());
+        Assert.assertEquals(null, namelessPerson.getFirstName());
+
+        // Change to something and back to null representation
+        nullTextField.setValue("");
+        nullTextField.setValue("null");
+        Assert.assertTrue(nullTextField.isEmpty());
+        Assert.assertEquals("null", namelessPerson.getFirstName());
+    }
+}
\ No newline at end of file