Fix AbstractDateField parsing and errors handling, support locale (#9740)

Fixes #9518
Fixes #8991
Fixes #8687

8 files changed, 248 insertions, 189 deletions

diff --git a/documentation/components/components-datefield.asciidoc b/documentation/components/components-datefield.asciidoc
index 3cbba0588f..6137bd0c3d 100644
--- a/documentation/components/components-datefield.asciidoc
+++ b/documentation/components/components-datefield.asciidoc
@@ -121,10 +121,11 @@ in the following example.
 DateField date = new DateField("My Date") {
     @Override
     protected Result<LocalDate> handleUnparsableDateString(
-          String dateString) {        
+          String dateString) {
         try {
             // try to parse with alternative format
-            return Result.ok(LocalDate.of(dateString, myFormat));
+            LocalDate parsedAtServer = LocalDate.parse(dateString, DateTimeFormatter.ISO_DATE);
+            return Result.ok(parsedAtServer);
         } catch (DateTimeParseException e) {
            return Result.error("Bad date");
         }
diff --git a/server/src/main/java/com/vaadin/ui/AbstractDateField.java b/server/src/main/java/com/vaadin/ui/AbstractDateField.java
index 0b1f60c0e4..9d063af5ec 100644
--- a/server/src/main/java/com/vaadin/ui/AbstractDateField.java
+++ b/server/src/main/java/com/vaadin/ui/AbstractDateField.java
@@ -23,10 +23,10 @@ import java.time.temporal.Temporal;
 import java.time.temporal.TemporalAdjuster;
 import java.util.Calendar;
 import java.util.Date;
-import java.util.EventObject;
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.logging.Logger;
@@ -91,7 +91,7 @@ public abstract class AbstractDateField<T extends Temporal & TemporalAdjuster &
 
     private boolean lenient = false;
 
-    private String dateString = null;
+    private String dateString = "";
 
     private String currentParseErrorMessage;
 
@@ -110,13 +110,6 @@ public abstract class AbstractDateField<T extends Temporal & TemporalAdjuster &
 
     private String dateOutOfRangeMessage = "Date is out of allowed range";
 
-    /**
-     * Determines whether the ValueChangeEvent should be fired. Used to prevent
-     * firing the event when UI has invalid string until uiHasValidDateString
-     * flag is set
-     */
-    private boolean preventValueChangeEvent;
-
     /* Constructors */
 
     /**
@@ -185,11 +178,6 @@ public abstract class AbstractDateField<T extends Temporal & TemporalAdjuster &
         target.addAttribute(DateFieldConstants.ATTR_WEEK_NUMBERS,
                 isShowISOWeekNumbers());
         target.addAttribute("parsable", uiHasValidDateString);
-        /*
-         * TODO communicate back the invalid date string? E.g. returning back to
-         * app or refresh.
-         */
-
         final T currentDate = getValue();
 
         // Only paint variables for the resolution and up, e.g. Resolution DAY
@@ -218,116 +206,59 @@ public abstract class AbstractDateField<T extends Temporal & TemporalAdjuster &
 
             // Old and new dates
             final T oldDate = getValue();
-            T newDate = null;
 
             // this enables analyzing invalid input on the server
+            // this variable is null if the date was chosen with popup calendar
+            // or contains user-typed string
             final String newDateString = (String) variables.get("dateString");
-            dateString = newDateString;
 
-            // Gets the new date in parts
+            T newDate;
+
             boolean hasChanges = false;
-            Map<R, Integer> calendarFields = new HashMap<>();
-
-            for (R resolution : getResolutionsHigherOrEqualTo(
-                    getResolution())) {
-                // Only handle what the client is allowed to send. The same
-                // resolutions that are painted
-                String variableName = getResolutionVariable(resolution);
-
-                int value = getDatePart(oldDate, resolution);
-                if (variables.containsKey(variableName)) {
-                    Integer newValue = (Integer) variables.get(variableName);
-                    if (newValue >= 0) {
-                        hasChanges = true;
-                        value = newValue;
-                    }
-                }
-                calendarFields.put(resolution, value);
-            }
 
-            // If no new variable values were received, use the previous value
-            if (!hasChanges) {
+            if ("".equals(newDateString)) {
+
                 newDate = null;
+                // TODO check if the following 3 lines are necessary
+                hasChanges = !uiHasValidDateString;
+                uiHasValidDateString = true;
+                currentParseErrorMessage = null;
             } else {
-                newDate = buildDate(calendarFields);
+                newDate = reconstructDateFromFields(variables, oldDate);
             }
 
-            if (newDate == null && dateString != null
-                    && !dateString.isEmpty()) {
-                Result<T> parsedDate = handleUnparsableDateString(dateString);
-                if (parsedDate.isError()) {
-
-                    /*
-                     * Saves the localized message of parse error. This can be
-                     * overridden in handleUnparsableDateString. The message
-                     * will later be used to show a validation error.
-                     */
-                    currentParseErrorMessage = parsedDate.getMessage().get();
-
-                    /*
-                     * The value of the DateField should be null if an invalid
-                     * value has been given. Not using setValue() since we do
-                     * not want to cause the client side value to change.
-                     */
-                    uiHasValidDateString = false;
-
-                    /*
-                     * Datefield now contains some text that could't be parsed
-                     * into date. ValueChangeEvent is fired after the value is
-                     * changed and the flags are set
-                     */
-                    if (oldDate != null) {
-                        /*
-                         * Set the logic value to null without firing the
-                         * ValueChangeEvent
-                         */
-                        preventValueChangeEvent = true;
-                        try {
-                            setValue(null);
-                        } finally {
-                            preventValueChangeEvent = false;
-                        }
-
-                        /*
-                         * Reset the dateString (overridden to null by setValue)
-                         */
-                        dateString = newDateString;
-                    }
-
-                    /*
-                     * If value was changed fire the ValueChangeEvent
-                     */
-                    if (oldDate != null) {
-                        fireEvent(createValueChange(oldDate, true));
-                    }
+            hasChanges |= !Objects.equals(dateString, newDateString) ||
+                    !Objects.equals(oldDate, newDate);
 
-                    markAsDirty();
+            if (hasChanges) {
+                dateString = newDateString;
+                if (newDateString == null || newDateString.isEmpty()) {
+                    uiHasValidDateString = true;
+                    currentParseErrorMessage = null;
+                    setValue(newDate, true);
+                    setComponentError(null);
                 } else {
-                    parsedDate.ifOk(value -> {
+                    if (variables.get("lastInvalidDateString") != null) {
+                        Result<T> parsedDate = handleUnparsableDateString(dateString);
+                        parsedDate.ifOk(v-> {
+                            uiHasValidDateString = true;
+                            currentParseErrorMessage = null;
+                            setValue(v,true);
+                        });
+                        if (parsedDate.isError()) {
+                            dateString = null;
+                            uiHasValidDateString = false;
+                            currentParseErrorMessage = parsedDate.getMessage().orElse("Parsing error");
+                            setComponentError(new UserError(getParseErrorMessage()));
+                            setValue(null,true);
+                        }
+                    } else {
+                        uiHasValidDateString = true;
                         currentParseErrorMessage = null;
-                        setValue(value, true);
-                    });
-
-                    /*
-                     * Ensure the value is sent to the client if the value is
-                     * set to the same as the previous (#4304). Does not repaint
-                     * if handleUnparsableDateString throws an exception. In
-                     * this case the invalid text remains in the DateField.
-                     */
-                    markAsDirty();
+                        setValue(newDate, true);
+                    }
                 }
-
-            } else if (newDate != oldDate
-                    && (newDate == null || !newDate.equals(oldDate))) {
-                currentParseErrorMessage = null;
-                setValue(newDate, true); // Don't require a repaint, client
-                // updates itself
-            } else if (!uiHasValidDateString) {
-                // oldDate ==
-                // newDate == null
-                // Empty value set, previously contained unparsable date string,
-                // clear related internal fields
-                setValue(null);
+                markAsDirty();
             }
         }
 
@@ -341,6 +272,31 @@ public abstract class AbstractDateField<T extends Temporal & TemporalAdjuster &
     }
 
     /**
+     * Construct a date object from the individual field values received from the
+     * client.
+     *
+     * @since 8.1.1
+     */
+    protected T reconstructDateFromFields(Map<String, Object> variables, T oldDate) {
+        Map<R, Integer> calendarFields = new HashMap<>();
+
+        for (R resolution : getResolutionsHigherOrEqualTo(
+                getResolution())) {
+            // Only handle what the client is allowed to send. The same
+            // resolutions that are painted
+            String variableName = getResolutionVariable(resolution);
+
+            Integer newValue = (Integer) variables.get(variableName);
+            if (newValue != null && newValue >= 0) {
+                calendarFields.put(resolution, newValue);
+            } else {
+                calendarFields.put(resolution, getDatePart(oldDate, resolution));
+            }
+        }
+        return buildDate(calendarFields);
+    }
+
+    /**
      * Sets the start range for this component. If the value is set before this
      * date (taking the resolution into account), the component will not
      * validate. If <code>startDate</code> is set to <code>null</code>, any
@@ -504,34 +460,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.
@@ -617,11 +545,23 @@ public abstract class AbstractDateField<T extends Temporal & TemporalAdjuster &
             } else {
                 throw new RuntimeException("Cannot detect resoluton type "
                         + Optional.ofNullable(dateType).map(Type::getTypeName)
-                                .orElse(null));
+                        .orElse(null));
             }
         }
     }
 
+    /**
+     * 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.1
+     */
+    protected String formatDate(T value) {
+        return Objects.toString(value, "");
+    }
+
     @Override
     public void writeDesign(Element design, DesignContext designContext) {
         super.writeDesign(design, designContext);
@@ -631,17 +571,6 @@ public abstract class AbstractDateField<T extends Temporal & TemporalAdjuster &
         }
     }
 
-    @Override
-    protected void fireEvent(EventObject event) {
-        if (event instanceof ValueChangeEvent) {
-            if (!preventValueChangeEvent) {
-                super.fireEvent(event);
-            }
-        } else {
-            super.fireEvent(event);
-        }
-    }
-
     /**
      * This method is called to handle a non-empty date string from the client
      * if the client could not parse it as a Date.
@@ -673,26 +602,24 @@ public abstract class AbstractDateField<T extends Temporal & TemporalAdjuster &
 
     @Override
     protected void doSetValue(T value) {
+
+        this.value = value;
         // Also set the internal dateString
         if (value != null) {
-            dateString = value.toString();
+            dateString = formatDate(value);
         } else {
-            dateString = null;
+            dateString = formatDate(getEmptyValue());
         }
-
-        this.value = value;
-        setComponentError(null);
-        if (!uiHasValidDateString) {
-            // clear component error and parsing flag
-            uiHasValidDateString = true;
-            setComponentError(new UserError(currentParseErrorMessage));
+        RangeValidator<T> validator = getRangeValidator();// TODO move range check to internal validator?
+        ValidationResult result = validator.apply(value,
+                new ValueContext(this, this));
+        if (result.isError()) {
+            currentParseErrorMessage = getDateOutOfRangeMessage();
+        }
+        if (currentParseErrorMessage == null) {
+            setComponentError(null);
         } else {
-            RangeValidator<T> validator = getRangeValidator();
-            ValidationResult result = validator.apply(value,
-                    new ValueContext(this, this));
-            if (result.isError()) {
-                setComponentError(new UserError(getDateOutOfRangeMessage()));
-            }
+            setComponentError(new UserError(currentParseErrorMessage));
         }
     }
 
diff --git a/server/src/main/java/com/vaadin/ui/AbstractLocalDateField.java b/server/src/main/java/com/vaadin/ui/AbstractLocalDateField.java
index 7754757429..2fdf2bf382 100644
--- a/server/src/main/java/com/vaadin/ui/AbstractLocalDateField.java
+++ b/server/src/main/java/com/vaadin/ui/AbstractLocalDateField.java
@@ -18,7 +18,10 @@ package com.vaadin.ui;
 import java.time.Instant;
 import java.time.LocalDate;
 import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.time.format.FormatStyle;
 import java.util.Date;
+import java.util.Locale;
 import java.util.Map;
 
 import com.vaadin.data.validator.DateRangeValidator;
@@ -28,11 +31,9 @@ import com.vaadin.shared.ui.datefield.DateResolution;
 
 /**
  * Abstract DateField class for {@link LocalDate} type.
- * 
+ *
  * @author Vaadin Ltd
- * 
  * @since 8.0
- *
  */
 public abstract class AbstractLocalDateField
         extends AbstractDateField<LocalDate, DateResolution> {
@@ -47,8 +48,7 @@ public abstract class AbstractLocalDateField
     /**
      * Constructs an empty <code>AbstractLocalDateField</code> with caption.
      *
-     * @param caption
-     *            the caption of the datefield.
+     * @param caption the caption of the datefield.
      */
     public AbstractLocalDateField(String caption) {
         super(caption, DateResolution.DAY);
@@ -58,10 +58,8 @@ public abstract class AbstractLocalDateField
      * Constructs a new <code>AbstractLocalDateField</code> with the given
      * caption and initial text contents.
      *
-     * @param caption
-     *            the caption <code>String</code> for the editor.
-     * @param value
-     *            the LocalDate value.
+     * @param caption the caption <code>String</code> for the editor.
+     * @param value   the LocalDate value.
      */
     public AbstractLocalDateField(String caption, LocalDate value) {
         super(caption, value, DateResolution.DAY);
@@ -74,15 +72,15 @@ public abstract class AbstractLocalDateField
             value = LocalDate.of(1, 1, 1);
         }
         switch (resolution) {
-        case DAY:
-            return value.getDayOfMonth();
-        case MONTH:
-            return value.getMonthValue();
-        case YEAR:
-            return value.getYear();
-        default:
-            assert false : "Unexpected resolution argument " + resolution;
-            return -1;
+            case DAY:
+                return value.getDayOfMonth();
+            case MONTH:
+                return value.getMonthValue();
+            case YEAR:
+                return value.getYear();
+            default:
+                assert false : "Unexpected resolution argument " + resolution;
+                return -1;
         }
     }
 
@@ -140,4 +138,15 @@ public abstract class AbstractLocalDateField
             return date;
         }
     }
+
+    @Override
+    protected String formatDate(LocalDate value) {
+        if (value == null) return "";
+        DateTimeFormatter dateTimeFormatter = DateTimeFormatter.ofLocalizedDate(FormatStyle.SHORT);
+        Locale locale = getLocale();
+        if (locale != null){
+            dateTimeFormatter = dateTimeFormatter.withLocale(locale);
+        }
+        return value.format(dateTimeFormatter);
+    }
 }
diff --git a/server/src/main/java/com/vaadin/ui/AbstractLocalDateTimeField.java b/server/src/main/java/com/vaadin/ui/AbstractLocalDateTimeField.java
index 65da2a2b8b..1e036f371b 100644
--- a/server/src/main/java/com/vaadin/ui/AbstractLocalDateTimeField.java
+++ b/server/src/main/java/com/vaadin/ui/AbstractLocalDateTimeField.java
@@ -16,10 +16,14 @@
 package com.vaadin.ui;
 
 import java.time.Instant;
+import java.time.LocalDate;
 import java.time.LocalDateTime;
 import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.time.format.FormatStyle;
 import java.time.temporal.ChronoUnit;
 import java.util.Date;
+import java.util.Locale;
 import java.util.Map;
 
 import com.vaadin.data.validator.DateTimeRangeValidator;
@@ -163,4 +167,14 @@ public abstract class AbstractLocalDateTimeField
         }
     }
 
+    @Override
+    protected String formatDate(LocalDateTime value) {
+        if (value == null) return "";
+        DateTimeFormatter dateTimeFormatter = DateTimeFormatter.ofLocalizedDateTime(FormatStyle.SHORT);
+        Locale locale = getLocale();
+        if (locale != null) {
+            dateTimeFormatter = dateTimeFormatter.withLocale(locale);
+        }
+        return value.format(dateTimeFormatter);
+    }
 }
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
new file mode 100644
index 0000000000..1eb04e5331
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/datefield/DateTextHandling.java
@@ -0,0 +1,67 @@
+package com.vaadin.tests.components.datefield;
+
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.data.Binder;
+import com.vaadin.data.Result;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.DateField;
+import com.vaadin.ui.Label;
+import com.vaadin.ui.Notification;
+import com.vaadin.ui.VerticalLayout;
+
+import java.time.LocalDate;
+import java.time.format.DateTimeFormatter;
+import java.time.format.FormatStyle;
+import java.util.Locale;
+
+@Widgetset("com.vaadin.DefaultWidgetSet")
+public class DateTextHandling extends AbstractTestUI {
+
+    public static final DateTimeFormatter DATE_TIME_FORMATTER = DateTimeFormatter.ofLocalizedDate(FormatStyle.MEDIUM).withLocale(Locale.UK);
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        final VerticalLayout layout = new VerticalLayout();
+
+        DateField dateField = new DateField("Date") {
+            @Override
+            protected Result<LocalDate> handleUnparsableDateString(String dateString) {
+                if (dateString.equalsIgnoreCase("Y2K")) {
+                    return Result.ok(LocalDate.of(2000, 1, 1));
+                } else {
+                    return super.handleUnparsableDateString(dateString);
+                }
+            }
+
+            ;
+        };
+        dateField.setParseErrorMessage("Parse error");
+        dateField.setDateOutOfRangeMessage("Out of range");
+        layout.addComponent(dateField);
+        Label errorLabel = new Label();
+        errorLabel.setId("errorLabel");
+        layout.addComponent(errorLabel);
+
+        Binder<Void> binder = new Binder<>();
+        binder.forField(dateField).withStatusLabel(errorLabel).bind(o -> dateField.getValue(), null);
+
+        Button buttonValidate = new Button("Validate!");
+        buttonValidate.addClickListener(event1 -> {
+            binder.validate();
+            if (dateField.getValue() == null) {
+                Notification.show("NULL");
+            } else {
+                Notification.show(DATE_TIME_FORMATTER.format(dateField.getValue()));
+            }
+
+        });
+        layout.addComponent(buttonValidate);
+
+        Button setValueButton = new Button("Set 2011-12-13",
+                e -> dateField.setValue(LocalDate.of(2011, 12, 13)));
+        layout.addComponent(setValueButton);
+        addComponent(layout);
+    }
+}
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
new file mode 100644
index 0000000000..602d709261
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/datefield/DateTextHandlingTest.java
@@ -0,0 +1,41 @@
+package com.vaadin.tests.components.datefield;
+
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.DateFieldElement;
+import com.vaadin.testbench.elements.LabelElement;
+import com.vaadin.testbench.elements.NotificationElement;
+import com.vaadin.tests.tb3.SingleBrowserTest;
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.Keys;
+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();
+        WebElement dateTextbox = dateFieldElement.getInputElement();
+
+        dateTextbox.sendKeys("Y2K",Keys.TAB);
+        validate.click();
+        assertNotification("Y2K Should be converted to " + Y2K_GB_LOCALE, Y2K_GB_LOCALE);
+
+        dateTextbox.clear();
+        validate.click();
+        assertNotification("Null for empty string","NULL");
+    }
+
+    protected void assertNotification(String message, String expected) {
+        NotificationElement notification = $(NotificationElement.class).first();
+        assertEquals(message, expected, notification.getCaption());
+        notification.close();
+    }
+
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/datefield/DisabledParentLayoutTest.java b/uitest/src/test/java/com/vaadin/tests/components/datefield/DisabledParentLayoutTest.java
index 80ea1ca4b2..84cb6f3e05 100644
--- a/uitest/src/test/java/com/vaadin/tests/components/datefield/DisabledParentLayoutTest.java
+++ b/uitest/src/test/java/com/vaadin/tests/components/datefield/DisabledParentLayoutTest.java
@@ -54,7 +54,7 @@ public class DisabledParentLayoutTest extends MultiBrowserTest {
         button.click();
 
         Assert.assertTrue(
-                "Date input text field shoud be enabled for enabled DateField",
+                "Date input text field should be enabled for enabled DateField",
                 textField.isEnabled());
 
         textField.click();
@@ -65,6 +65,7 @@ public class DisabledParentLayoutTest extends MultiBrowserTest {
                 textField.getAttribute("value"));
 
         dataFieldButton.click();
+        dataFieldButton.click();//Requires two clicks because of error message. TODO fix
         Assert.assertFalse("Unexpected disabled element found",
                 isElementPresent(By.className("v-disabled")));
 
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 012b4605f2..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,9 +64,8 @@ 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 from the field
-        Assert.assertEquals("", elem.getValue());
-        Assert.assertEquals("1", eventCount.getText());
+        // invalid values should stay unchanged
+        Assert.assertEquals(TYPED_STRING, elem.getValue());
     }
 
     // helper methods