Revert "Initial support for null representations in Binder"

This reverts commit 9f672890c1.

Change-Id: I0952a7f9c7efc8a5d7de9987277b835d92d52b39

server/src/main/java/com/vaadin/data/BeanBinder.java

@@ -269,11 +269,8 @@ public class BeanBinder<BEAN> extends Binder<BEAN> {
     @Override
     public <FIELDVALUE> BeanBinding<BEAN, FIELDVALUE, FIELDVALUE> forField(
             HasValue<FIELDVALUE> field) {
-        BeanBindingImpl<BEAN, FIELDVALUE, FIELDVALUE> binding = createBinding(
-                field, Converter.identity(), this::handleValidationStatus);
-        return binding.withConverter(fieldValue -> fieldValue,
-                modelValue -> modelValue != null ? modelValue
-                        : field.getEmptyValue());
+        return createBinding(field, Converter.identity(),
+                this::handleValidationStatus);
     }
 
     @Override

server/src/main/java/com/vaadin/data/Binder.java

@@ -281,23 +281,6 @@ public class Binder<BEAN> implements Serializable {
                     exception -> errorMessage));
         }
 
-        /**
-         * 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.
          *
@@ -634,6 +617,10 @@ 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);
         }
@@ -709,13 +696,6 @@ 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
@@ -732,12 +712,8 @@ public class Binder<BEAN> implements Serializable {
         clearError(field);
         getStatusLabel().ifPresent(label -> label.setValue(""));
 
-        BindingImpl<BEAN, FIELDVALUE, FIELDVALUE> createBinding = createBinding(
-                field, Converter.identity(), this::handleValidationStatus);
-
-        return createBinding.withConverter(fieldValue -> fieldValue,
-                modelValue -> modelValue != null ? modelValue
-                        : field.getEmptyValue());
+        return createBinding(field, Converter.identity(),
+                this::handleValidationStatus);
     }
 
     /**
@@ -826,14 +802,6 @@ 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
@@ -954,7 +922,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) { ... }
      * }
@@ -1122,8 +1090,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.getter.apply(bean)));
+        bindings.forEach(binding -> oldValues.put(binding,
+                binding.convertDataToFieldType(bean)));
 
         bindings.forEach(binding -> binding.storeFieldValue(bean));
         // Now run bean level validation against the updated bean
@@ -1132,8 +1100,8 @@ public class Binder<BEAN> implements Serializable {
                 .findAny().isPresent();
         if (hasErrors) {
             // Bean validator failed, revert values
-            bindings.forEach((BindingImpl binding) -> binding.setter
-                    .accept(bean, oldValues.get(binding)));
+            bindings.forEach((BindingImpl binding) -> binding.setBeanValue(bean,
+                    oldValues.get(binding)));
         } else {
             // Save successful, reset hasChanges to false
             setHasChanges(false);

server/src/main/java/com/vaadin/data/HasValue.java

@@ -17,9 +17,6 @@ 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;
@@ -119,8 +116,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
@@ -173,29 +170,4 @@ 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());
-    }
 }

server/src/main/java/com/vaadin/ui/AbstractTextField.java

@@ -17,7 +17,6 @@
 package com.vaadin.ui;
 
 import java.util.Collection;
-import java.util.Objects;
 
 import org.jsoup.nodes.Attributes;
 import org.jsoup.nodes.Element;
@@ -81,8 +80,11 @@ public abstract class AbstractTextField extends AbstractField<String>
 
     @Override
     public void setValue(String value) {
-        Objects.requireNonNull(value, "Null value not supported");
-        setValue(value, false);
+        if (value == null) {
+            setValue("", false);
+        } else {
+            setValue(value, false);
+        }
     }
 
     /**
@@ -268,6 +270,16 @@ 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);
@@ -287,9 +299,4 @@ public abstract class AbstractTextField extends AbstractField<String>
         customAttributes.add("cursor-position");
         return customAttributes;
     }
-
-    @Override
-    public String getEmptyValue() {
-        return "";
-    }
 }

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,7 +30,6 @@ 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;
@@ -39,8 +38,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;
@@ -685,24 +684,4 @@ public class BinderConverterValidatorTest
                 .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());
-        }
-    }
 }

server/src/test/java/com/vaadin/data/BinderTest.java

@@ -9,8 +9,6 @@ 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> {
 
@@ -176,48 +174,4 @@ 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());
-    }
-}
+}