From 7b9a09edbf27269eaade3898ea49aa4d81efd927 Mon Sep 17 00:00:00 2001 From: Ilia Motornyi Date: Wed, 2 Aug 2017 13:30:55 +0300 Subject: Fix AbstractDateField parsing and errors handling, support locale (#9740) Fixes #9518 Fixes #8991 Fixes #8687 --- .../components/components-datefield.asciidoc | 5 +- .../main/java/com/vaadin/ui/AbstractDateField.java | 257 ++++++++------------- .../java/com/vaadin/ui/AbstractLocalDateField.java | 45 ++-- .../com/vaadin/ui/AbstractLocalDateTimeField.java | 14 ++ .../components/datefield/DateTextHandling.java | 67 ++++++ .../components/datefield/DateTextHandlingTest.java | 41 ++++ .../datefield/DisabledParentLayoutTest.java | 3 +- .../AbstractTextElementSetValueTest.java | 5 +- 8 files changed, 248 insertions(+), 189 deletions(-) create mode 100644 uitest/src/main/java/com/vaadin/tests/components/datefield/DateTextHandling.java create mode 100644 uitest/src/test/java/com/vaadin/tests/components/datefield/DateTextHandlingTest.java 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 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 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 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 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(); } } @@ -340,6 +271,31 @@ public abstract class AbstractDateField variables, T oldDate) { + Map 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 @@ -503,34 +459,6 @@ public abstract class AbstractDateField 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 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 { @@ -47,8 +48,7 @@ public abstract class AbstractLocalDateField /** * Constructs an empty AbstractLocalDateField 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 AbstractLocalDateField with the given * caption and initial text contents. * - * @param caption - * the caption String for the editor. - * @param value - * the LocalDate value. + * @param caption the caption String 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 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 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 -- cgit v1.2.3