Fix AbstractDateField parsing and errors handling, support locale

Fixes #9518
Fixes #8991
Fixes #8687

2 files changed, 116 insertions, 141 deletions

diff --git a/server/src/main/java/com/vaadin/ui/AbstractDateField.java b/server/src/main/java/com/vaadin/ui/AbstractDateField.java
index 0b1f60c0e4..82c0baf139 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,42 @@ 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
             final String newDateString = (String) variables.get("dateString");
-            dateString = newDateString;
-
-            // Gets the new date in parts
-            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) {
+            T newDate;
+
+            if("".equals(newDateString)) {
                 newDate = null;
+                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));
+            boolean hasChanges = !Objects.equals(dateString, newDateString) ||
+                    !Objects.equals(oldDate, newDate);
+
+            if (hasChanges) {
+                dateString = newDateString;
+
+                if (newDateString != null && !newDateString.isEmpty()) {
+                    String invalidDateString = (String) variables.get("lastInvalidDateString");
+                    if (invalidDateString != null) {
+                        Result<T> parsedDate = handleUnparsableDateString(dateString);
+                        uiHasValidDateString = false;
+                        currentParseErrorMessage = parsedDate.getMessage().orElse("Parsing error");
+                        setComponentError(new UserError(getParseErrorMessage()));
+                    } else {
+                        uiHasValidDateString = true;
+                        currentParseErrorMessage = null;
+                        setValue(newDate,true);
                     }
-
-                    markAsDirty();
                 } else {
-                    parsedDate.ifOk(value -> {
-                        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();
             }
         }
 
@@ -340,6 +254,25 @@ public abstract class AbstractDateField<T extends Temporal & TemporalAdjuster &
         }
     }
 
+    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
@@ -613,6 +546,7 @@ public abstract class AbstractDateField<T extends Temporal & TemporalAdjuster &
                             .info("cannot parse " + design.attr("value")
                                     + " as date");
                 }
+                dateString = formatDate(value);
                 doSetValue(date);
             } else {
                 throw new RuntimeException("Cannot detect resoluton type "
@@ -622,6 +556,14 @@ public abstract class AbstractDateField<T extends Temporal & TemporalAdjuster &
         }
     }
 
+    /**
+     * Formats date according to the components locale.
+     *
+     * @param value the date or {@code null}
+     * @return textual representation of the date or empty string for {@code null}
+     */
+    protected abstract String formatDate(T value);
+
     @Override
     public void writeDesign(Element design, DesignContext designContext) {
         super.writeDesign(design, designContext);
@@ -631,17 +573,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.
@@ -675,25 +606,25 @@ public abstract class AbstractDateField<T extends Temporal & TemporalAdjuster &
     protected void doSetValue(T 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));
-        } else {
+        if (uiHasValidDateString) {
             RangeValidator<T> validator = getRangeValidator();
             ValidationResult result = validator.apply(value,
                     new ValueContext(this, this));
             if (result.isError()) {
-                setComponentError(new UserError(getDateOutOfRangeMessage()));
+                currentParseErrorMessage = getDateOutOfRangeMessage();
             }
         }
+        if (currentParseErrorMessage == null) {
+            setComponentError(null);
+        } else {
+            setComponentError(new UserError(currentParseErrorMessage));
+        }
     }
 
     /**
@@ -774,15 +705,12 @@ public abstract class AbstractDateField<T extends Temporal & TemporalAdjuster &
 
     @Override
     public Validator<T> getDefaultValidator() {
-        return new Validator<T>() {
-            @Override
-            public ValidationResult apply(T value, ValueContext context) {
-                if (currentParseErrorMessage != null) {
-                    return ValidationResult.error(currentParseErrorMessage);
-                }
-                // Pass to range validator.
-                return getRangeValidator().apply(value, context);
+        return (Validator<T>) (value, context) -> {
+            if (currentParseErrorMessage != null) {
+                return ValidationResult.error(currentParseErrorMessage);
             }
+            // Pass to range validator.
+            return getRangeValidator().apply(value, context);
         };
     }
 }
diff --git a/uitest/src/main/java/com/vaadin/tests/data/DateValidationUI.java b/uitest/src/main/java/com/vaadin/tests/data/DateValidationUI.java
new file mode 100644
index 0000000000..c41fa198c1
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/data/DateValidationUI.java
@@ -0,0 +1,47 @@
+package com.vaadin.tests.data;
+
+import com.vaadin.annotations.Widgetset;
+import com.vaadin.data.Binder;
+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.UI;
+import com.vaadin.ui.VerticalLayout;
+
+/**
+ * Created by elmot on 7/11/2017.
+ */
+@Widgetset("com.vaadin.DefaultWidgetSet")
+public class DateValidationUI extends AbstractTestUI {
+    @Override
+    protected void setup(VaadinRequest request) {
+        final VerticalLayout layout = new VerticalLayout();
+
+        DateField dateField = new DateField("Date");
+        dateField.setParseErrorMessage("Parse error");
+        dateField.setDateOutOfRangeMessage("Out of range");
+        layout.addComponent(dateField);
+        dateField.addValueChangeListener(event -> {
+            System.out.println(dateField.getValue());
+        });
+        Label errorLabel = new Label();
+        layout.addComponent(errorLabel);
+
+        Binder<Void> binder = new Binder<>();
+        binder.forField(dateField).withStatusLabel(errorLabel).bind(o -> dateField.getEmptyValue(), null);
+
+        Button button = new Button("Validate!");
+        button.addClickListener(event1 -> {
+            if (binder.validate().isOk()) {
+                System.out.println("Correct");
+            } else {
+                System.out.println(dateField.isEmpty() + "Error!");
+            }
+        });
+        layout.addComponent(button);
+
+        addComponent(layout);
+    }
+}