Skip to content

Commit

Permalink
Reorder date formatting converter in registrar
Browse files Browse the repository at this point in the history 
Prior to this commit, the `DateFormatterRegistrar` would register the
annotation-based formatter before the pattern-based formatter. This
would create an issue when an application tries to convert a `String` to
an annotated `@DateTimeFormat Date`: since the converters are considered
in reversed order of registration in
`GenericConversionServicei#ConvertersForPair`, the pattern-based variant
would always be considered before the annotation-based variant,
overriding the developer's opinion.

This commit aligns the `DateFormatterRegistrar` with the
`DateTimeFormatterRegistrar` and registers the annotation-based variant
last.
  • Loading branch information
@bclozel
bclozel committed Oct 30, 2019
1 parent 422c268 commit c4ef78a
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 22 deletions.
3 changes: 1 addition & 2 deletions spring-context/src/main/java/org/springframework/format/datetime/DateFormatterRegistrar.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,13 @@ public void setFormatter(DateFormatter dateFormatter) {
@Override
public void registerFormatters(FormatterRegistry registry) {
addDateConverters(registry);
registry.addFormatterForFieldAnnotation(new DateTimeFormatAnnotationFormatterFactory());

// In order to retain back compatibility we only register Date/Calendar
// types when a user defined formatter is specified (see SPR-10105)
if (this.dateFormatter != null) {
registry.addFormatter(this.dateFormatter);
registry.addFormatterForFieldType(Calendar.class, this.dateFormatter);
}
registry.addFormatterForFieldAnnotation(new DateTimeFormatAnnotationFormatterFactory());
}

/**
Expand Down
55 changes: 35 additions & 20 deletions spring-context/src/test/java/org/springframework/format/datetime/DateFormattingTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,19 @@
*/
public class DateFormattingTests {

private final FormattingConversionService conversionService = new FormattingConversionService();
private FormattingConversionService conversionService;

private DataBinder binder;


@BeforeEach
public void setup() {
void setup() {
DateFormatterRegistrar registrar = new DateFormatterRegistrar();
setup(registrar);
}

private void setup(DateFormatterRegistrar registrar) {
conversionService = new FormattingConversionService();
DefaultConversionService.addDefaultConverters(conversionService);
registrar.registerFormatters(conversionService);

Expand All @@ -69,13 +70,13 @@ private void setup(DateFormatterRegistrar registrar) {
}

@AfterEach
public void tearDown() {
void tearDown() {
LocaleContextHolder.setLocale(null);
}


@Test
public void testBindLong() {
void testBindLong() {
MutablePropertyValues propertyValues = new MutablePropertyValues();
propertyValues.add("millis", "1256961600");
binder.bind(propertyValues);
Expand All @@ -84,7 +85,7 @@ public void testBindLong() {
}

@Test
public void testBindLongAnnotated() {
void testBindLongAnnotated() {
MutablePropertyValues propertyValues = new MutablePropertyValues();
propertyValues.add("millisAnnotated", "10/31/09");
binder.bind(propertyValues);
Expand All @@ -93,7 +94,7 @@ public void testBindLongAnnotated() {
}

@Test
public void testBindCalendarAnnotated() {
void testBindCalendarAnnotated() {
MutablePropertyValues propertyValues = new MutablePropertyValues();
propertyValues.add("calendarAnnotated", "10/31/09");
binder.bind(propertyValues);
Expand All @@ -102,7 +103,7 @@ public void testBindCalendarAnnotated() {
}

@Test
public void testBindDateAnnotated() {
void testBindDateAnnotated() {
MutablePropertyValues propertyValues = new MutablePropertyValues();
propertyValues.add("dateAnnotated", "10/31/09");
binder.bind(propertyValues);
Expand All @@ -111,15 +112,15 @@ public void testBindDateAnnotated() {
}

@Test
public void testBindDateArray() {
void testBindDateArray() {
MutablePropertyValues propertyValues = new MutablePropertyValues();
propertyValues.add("dateAnnotated", new String[]{"10/31/09 12:00 PM"});
binder.bind(propertyValues);
assertThat(binder.getBindingResult().getErrorCount()).isEqualTo(0);
}

@Test
public void testBindDateAnnotatedWithError() {
void testBindDateAnnotatedWithError() {
MutablePropertyValues propertyValues = new MutablePropertyValues();
propertyValues.add("dateAnnotated", "Oct X31, 2009");
binder.bind(propertyValues);
Expand All @@ -129,7 +130,7 @@ public void testBindDateAnnotatedWithError() {

@Test
@Disabled
public void testBindDateAnnotatedWithFallbackError() {
void testBindDateAnnotatedWithFallbackError() {
// TODO This currently passes because of the Date(String) constructor fallback is used
MutablePropertyValues propertyValues = new MutablePropertyValues();
propertyValues.add("dateAnnotated", "Oct 031, 2009");
Expand All @@ -139,7 +140,7 @@ public void testBindDateAnnotatedWithFallbackError() {
}

@Test
public void testBindDateAnnotatedPattern() {
void testBindDateAnnotatedPattern() {
MutablePropertyValues propertyValues = new MutablePropertyValues();
propertyValues.add("dateAnnotatedPattern", "10/31/09 1:05");
binder.bind(propertyValues);
Expand All @@ -148,15 +149,29 @@ public void testBindDateAnnotatedPattern() {
}

@Test
public void testBindDateTimeOverflow() {
void testBindDateAnnotatedPatternWithGlobalFormat() {
DateFormatterRegistrar registrar = new DateFormatterRegistrar();
DateFormatter dateFormatter = new DateFormatter();
dateFormatter.setIso(ISO.DATE_TIME);
registrar.setFormatter(dateFormatter);
setup(registrar);
MutablePropertyValues propertyValues = new MutablePropertyValues();
propertyValues.add("dateAnnotatedPattern", "10/31/09 1:05");
binder.bind(propertyValues);
assertThat(binder.getBindingResult().getErrorCount()).isEqualTo(0);
assertThat(binder.getBindingResult().getFieldValue("dateAnnotatedPattern")).isEqualTo("10/31/09 1:05");
}

@Test
void testBindDateTimeOverflow() {
MutablePropertyValues propertyValues = new MutablePropertyValues();
propertyValues.add("dateAnnotatedPattern", "02/29/09 12:00 PM");
binder.bind(propertyValues);
assertThat(binder.getBindingResult().getErrorCount()).isEqualTo(1);
}

@Test
public void testBindISODate() {
void testBindISODate() {
MutablePropertyValues propertyValues = new MutablePropertyValues();
propertyValues.add("isoDate", "2009-10-31");
binder.bind(propertyValues);
Expand All @@ -165,7 +180,7 @@ public void testBindISODate() {
}

@Test
public void testBindISOTime() {
void testBindISOTime() {
MutablePropertyValues propertyValues = new MutablePropertyValues();
propertyValues.add("isoTime", "12:00:00.000-05:00");
binder.bind(propertyValues);
Expand All @@ -174,7 +189,7 @@ public void testBindISOTime() {
}

@Test
public void testBindISODateTime() {
void testBindISODateTime() {
MutablePropertyValues propertyValues = new MutablePropertyValues();
propertyValues.add("isoDateTime", "2009-10-31T12:00:00.000-08:00");
binder.bind(propertyValues);
Expand All @@ -183,7 +198,7 @@ public void testBindISODateTime() {
}

@Test
public void testBindNestedDateAnnotated() {
void testBindNestedDateAnnotated() {
MutablePropertyValues propertyValues = new MutablePropertyValues();
propertyValues.add("children[0].dateAnnotated", "10/31/09");
binder.bind(propertyValues);
Expand All @@ -192,15 +207,15 @@ public void testBindNestedDateAnnotated() {
}

@Test
public void dateToStringWithoutGlobalFormat() {
void dateToStringWithoutGlobalFormat() {
Date date = new Date();
Object actual = this.conversionService.convert(date, TypeDescriptor.valueOf(Date.class), TypeDescriptor.valueOf(String.class));
String expected = date.toString();
assertThat(actual).isEqualTo(expected);
}

@Test
public void dateToStringWithGlobalFormat() {
void dateToStringWithGlobalFormat() {
DateFormatterRegistrar registrar = new DateFormatterRegistrar();
registrar.setFormatter(new DateFormatter());
setup(registrar);
Expand All @@ -212,14 +227,14 @@ public void dateToStringWithGlobalFormat() {

@Test // SPR-10105
@SuppressWarnings("deprecation")
public void stringToDateWithoutGlobalFormat() {
void stringToDateWithoutGlobalFormat() {
String string = "Sat, 12 Aug 1995 13:30:00 GM";
Date date = this.conversionService.convert(string, Date.class);
assertThat(date).isEqualTo(new Date(string));
}

@Test // SPR-10105
public void stringToDateWithGlobalFormat() {
void stringToDateWithGlobalFormat() {
DateFormatterRegistrar registrar = new DateFormatterRegistrar();
DateFormatter dateFormatter = new DateFormatter();
dateFormatter.setIso(ISO.DATE_TIME);
Expand Down

0 comments on commit c4ef78a

Please sign in to comment.