From 1c11016e7a5b3528001b9ddab513a9d2a94012bc Mon Sep 17 00:00:00 2001 From: Denis Anisimov Date: Thu, 20 Oct 2016 08:27:11 +0000 Subject: [PATCH] Revert "Initial support for null representations in Binder" This reverts commit 9f672890c143098b266ede6397e89379a38cc098. Change-Id: I0952a7f9c7efc8a5d7de9987277b835d92d52b39 --- .../main/java/com/vaadin/data/BeanBinder.java | 7 +-- .../src/main/java/com/vaadin/data/Binder.java | 54 ++++--------------- .../main/java/com/vaadin/data/HasValue.java | 32 +---------- .../java/com/vaadin/ui/AbstractTextField.java | 23 +++++--- .../data/BinderConverterValidatorTest.java | 31 ++--------- .../test/java/com/vaadin/data/BinderTest.java | 48 +---------------- 6 files changed, 36 insertions(+), 159 deletions(-) diff --git a/server/src/main/java/com/vaadin/data/BeanBinder.java b/server/src/main/java/com/vaadin/data/BeanBinder.java index 4af38f7862..c73b89de81 100644 --- a/server/src/main/java/com/vaadin/data/BeanBinder.java +++ b/server/src/main/java/com/vaadin/data/BeanBinder.java @@ -269,11 +269,8 @@ public class BeanBinder extends Binder { @Override public BeanBinding forField( HasValue field) { - BeanBindingImpl 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 diff --git a/server/src/main/java/com/vaadin/data/Binder.java b/server/src/main/java/com/vaadin/data/Binder.java index 8603ed9946..df29f8d480 100644 --- a/server/src/main/java/com/vaadin/data/Binder.java +++ b/server/src/main/java/com/vaadin/data/Binder.java @@ -281,23 +281,6 @@ public class Binder 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 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 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 implements Serializable { * {@link Binding#bind(Function, BiConsumer) Binding.bind} which completes * the binding. Until {@code Binding.bind} is called, the binding has no * effect. - *

- * Note: 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 * the value type of the field @@ -732,12 +712,8 @@ public class Binder implements Serializable { clearError(field); getStatusLabel().ifPresent(label -> label.setValue("")); - BindingImpl 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 implements Serializable { * Use the {@link #forField(HasValue)} overload instead if you want to * further configure the new binding. *

- * Note: 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))}. - *

* 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 implements Serializable { * 

      * class Feature {
      *     public enum Browser { CHROME, EDGE, FIREFOX, IE, OPERA, SAFARI }
-
+    
      *     public Set<Browser> getSupportedBrowsers() { ... }
      *     public void setSupportedBrowsers(Set<Browser> title) { ... }
      * }
@@ -1122,8 +1090,8 @@ public class Binder implements Serializable {
 
         // Save old bean values so we can restore them if validators fail
         Map, 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 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);
diff --git a/server/src/main/java/com/vaadin/data/HasValue.java b/server/src/main/java/com/vaadin/data/HasValue.java
index 26fd345d91..8333c8a02a 100644
--- a/server/src/main/java/com/vaadin/data/HasValue.java
+++ b/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 extends Serializable {
      * @see Registration
      */
     @FunctionalInterface
-    public interface ValueChangeListener
-            extends EventListener> {
+    public interface ValueChangeListener extends
+            EventListener> {
 
         @Deprecated
         public static final Method VALUE_CHANGE_METHOD = ReflectTools
@@ -173,29 +170,4 @@ public interface HasValue extends Serializable {
      */
     public Registration addValueChangeListener(
             ValueChangeListener listener);
-
-    /**
-     * Returns the value that represents an empty value.
-     * 

-     * 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.
-     * 

-     * 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 9c2720efd8..b5119ec92c 100644
--- a/server/src/main/java/com/vaadin/ui/AbstractTextField.java
+++ b/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
 
     @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
         setValue("");
     }
 
+    /**
+     * Checks if the field is empty.
+     *
+     * @return true if the field value is an empty string,
+     *         false 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
         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 73443155c7..d5959eb459 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,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, Person> {
+public class BinderConverterValidatorTest extends
+        BinderTestBase, 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 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 08af67ed32..b5e84cddd4 100644
--- a/server/src/test/java/com/vaadin/data/BinderTest.java
+++ b/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, Person> {
 
@@ -176,48 +174,4 @@ public class BinderTest extends BinderTestBase, 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
+}
-- 
2.39.5