Null handling, review fixesdatefield-set-invalid-value

6 files changed, 24 insertions, 45 deletions

diff --git a/server/src/main/java/com/vaadin/ui/AbstractDateField.java b/server/src/main/java/com/vaadin/ui/AbstractDateField.java
index e35840b62f..12e94a691c 100644
--- a/server/src/main/java/com/vaadin/ui/AbstractDateField.java
+++ b/server/src/main/java/com/vaadin/ui/AbstractDateField.java
@@ -229,22 +229,28 @@ public abstract class AbstractDateField<T extends Temporal & TemporalAdjuster &
             if (hasChanges) {
                 dateString = newDateString;
                 if (newDateString == null || newDateString.isEmpty()) {
-                    setValue(newDate, true);
                     uiHasValidDateString = true;
                     currentParseErrorMessage = null;
+                    setValue(newDate, true);
                     setComponentError(null);
                 } else {
                     if (variables.get("lastInvalidDateString") != null) {
                         Result<T> parsedDate = handleUnparsableDateString(dateString);
-                        parsedDate.ifOk(this::setValue);
+                        parsedDate.ifOk(v-> {
+                            uiHasValidDateString = true;
+                            currentParseErrorMessage = null;
+                            setValue(v,true);
+                        });
                         if (parsedDate.isError()) {
-                            doSetValue(null);
                             dateString = null;
                             uiHasValidDateString = false;
                             currentParseErrorMessage = parsedDate.getMessage().orElse("Parsing error");
                             setComponentError(new UserError(getParseErrorMessage()));
+                            setValue(null,true);
                         }
                     } else {
+                        uiHasValidDateString = true;
+                        currentParseErrorMessage = null;
                         setValue(newDate, true);
                     }
                 }
@@ -444,34 +450,6 @@ public abstract class AbstractDateField<T extends Temporal & TemporalAdjuster &
     }
 
     /**
-     * Sets the value of this object. If the new value is not equal to
-     * {@code getValue()}, fires a {@link ValueChangeEvent} .
-     *
-     * @param value
-     *            the new value, may be {@code null}
-     */
-    @Override
-    public void setValue(T value) {
-        /*
-         * First handle special case when the client side component have a date
-         * string but value is null (e.g. unparsable date string typed in by the
-         * user). No value changes should happen, but we need to do some
-         * internal housekeeping.
-         */
-        if (value == null && !uiHasValidDateString) {
-            /*
-             * Side-effects of doSetValue clears possible previous strings and
-             * flags about invalid input.
-             */
-            doSetValue(null);
-
-            markAsDirty();
-            return;
-        }
-        super.setValue(value);
-    }
-
-    /**
      * Checks whether ISO 8601 week numbers are shown in the date selector.
      *
      * @return true if week numbers are shown, false otherwise.
@@ -564,11 +542,15 @@ public abstract class AbstractDateField<T extends Temporal & TemporalAdjuster &
 
     /**
      * Formats date according to the components locale.
+     * To be reimplemented in subclasses.
      *
      * @param value the date or {@code null}
      * @return textual representation of the date or empty string for {@code null}
+     * @since 8.1
      */
-    protected abstract String formatDate(T value);
+    protected String formatDate(T value) {
+        return Objects.toString(value, "");
+    }
 
     @Override
     public void writeDesign(Element design, DesignContext designContext) {
@@ -610,8 +592,6 @@ public abstract class AbstractDateField<T extends Temporal & TemporalAdjuster &
 
     @Override
     protected void doSetValue(T value) {
-        uiHasValidDateString = true;
-        currentParseErrorMessage = null;
 
         this.value = value;
         // Also set the internal dateString
diff --git a/server/src/test/java/com/vaadin/tests/server/component/datefield/DateFieldListenersTest.java b/server/src/test/java/com/vaadin/tests/server/component/datefield/DateFieldListenersTest.java
index 508243964d..cab1addf58 100644
--- a/server/src/test/java/com/vaadin/tests/server/component/datefield/DateFieldListenersTest.java
+++ b/server/src/test/java/com/vaadin/tests/server/component/datefield/DateFieldListenersTest.java
@@ -50,10 +50,6 @@ public class DateFieldListenersTest extends AbstractListenerMethodsTestBase {
             return null;
         }
 
-        @Override
-        protected String formatDate(T value) {
-            return value.toString();
-        }
     }
 
     @Test
diff --git a/uitest/src/main/java/com/vaadin/tests/components/datefield/DateTextHandling.java b/uitest/src/main/java/com/vaadin/tests/components/datefield/DateTextHandling.java
index 695063ae68..9b59370b32 100644
--- a/uitest/src/main/java/com/vaadin/tests/components/datefield/DateTextHandling.java
+++ b/uitest/src/main/java/com/vaadin/tests/components/datefield/DateTextHandling.java
@@ -45,7 +45,7 @@ public class DateTextHandling extends AbstractTestUI {
         layout.addComponent(errorLabel);
 
         Binder<Void> binder = new Binder<>();
-        binder.forField(dateField).withStatusLabel(errorLabel).bind(o -> dateField.getEmptyValue(), null);
+        binder.forField(dateField).withStatusLabel(errorLabel).bind(o -> dateField.getValue(), null);
 
         Button button = new Button("Validate!");
         button.addClickListener(event1 -> {
diff --git a/uitest/src/test/java/com/vaadin/tests/components/datefield/DateFieldIsValidTest.java b/uitest/src/test/java/com/vaadin/tests/components/datefield/DateFieldIsValidTest.java
index b7df7d7f20..dacbae3db1 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/datefield/DateFieldIsValidTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/datefield/DateFieldIsValidTest.java
@@ -46,14 +46,15 @@ public class DateFieldIsValidTest extends MultiBrowserTest {
         assertLogText("2. buttonClick: value: 01/01/01, is valid: true");
 
         dateTextbox.sendKeys("lala", Keys.TAB);
+        assertLogText("3. valueChange: value: null, is valid: false");
         button.click();
-        assertLogText("3. buttonClick: value: null, is valid: false");
+        assertLogText("4. buttonClick: value: null, is valid: false");
 
         dateTextbox.clear();
         dateTextbox.sendKeys("02/02/02", Keys.TAB);
-        assertLogText("4. valueChange: value: 02/02/02, is valid: true");
+        assertLogText("5. valueChange: value: 02/02/02, is valid: true");
         button.click();
-        assertLogText("5. buttonClick: value: 02/02/02, is valid: true");
+        assertLogText("6. buttonClick: value: 02/02/02, is valid: true");
     }
 
     private void assertLogText(String expected) throws Exception {
diff --git a/uitest/src/test/java/com/vaadin/tests/components/datefield/DateTextHandlingTest.java b/uitest/src/test/java/com/vaadin/tests/components/datefield/DateTextHandlingTest.java
index e347bdbf3b..6a264b1575 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/datefield/DateTextHandlingTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/datefield/DateTextHandlingTest.java
@@ -13,18 +13,20 @@ import org.openqa.selenium.WebElement;
 import static org.junit.Assert.assertEquals;
 
 public class DateTextHandlingTest extends SingleBrowserTest {
+
+    public static final String Y2K_GB_LOCALE = "01-Jan-2000";
+
     @Test
     public void testSpecialValue() throws InterruptedException {
         openTestURL();
         DateFieldElement dateFieldElement = $(DateFieldElement.class).first();
         ButtonElement validate = $(ButtonElement.class).first();
-        LabelElement validateResult = $(LabelElement.class).first();
         WebElement dateTextbox = dateFieldElement
                 .findElement(com.vaadin.testbench.By.className("v-textfield"));
 
         dateTextbox.sendKeys("Y2K",Keys.TAB);
         validate.click();
-        assertNotification("Y2K Sould be converted to 1-JAN-2000", "01-Jan-2000");
+        assertNotification("Y2K Should be converted to " + Y2K_GB_LOCALE, Y2K_GB_LOCALE);
 
         dateTextbox.clear();
         validate.click();
diff --git a/uitest/src/test/java/com/vaadin/tests/elements/abstracttextfield/AbstractTextElementSetValueTest.java b/uitest/src/test/java/com/vaadin/tests/elements/abstracttextfield/AbstractTextElementSetValueTest.java
index 60b66f05bf..5e6ab39c6b 100644
--- a/uitest/src/test/java/com/vaadin/tests/elements/abstracttextfield/AbstractTextElementSetValueTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/elements/abstracttextfield/AbstractTextElementSetValueTest.java
@@ -64,7 +64,7 @@ public class AbstractTextElementSetValueTest extends MultiBrowserTest {
         LabelElement eventCount = $(LabelElement.class).get(4);
         // we can type any string in date field element
         elem.setValue(TYPED_STRING);
-        // invalid values are cleared stays
+        // invalid values should stay unchanged
         Assert.assertEquals(TYPED_STRING, elem.getValue());
     }