Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reorder date formatting converter in registrar #23893

Merged
merged 1 commit into from
Oct 30, 2019

Conversation

bclozel
Copy link
Member

@bclozel bclozel commented Oct 30, 2019

This is an attempt at fixing #23890

In that issue, a developer configures the spring.mvc.date-format application property in Spring Boot and sees that the @DateTimeFormat annotation is not honored anymore when binding to a controller method argument. This property is merely registering a DateFormatterRegistrar with a specific pattern.

It seems that the ordering of registration of formatters in that registrar is off and registers the annotation-based formatter before the pattern-based one. Because the formatters are considered in reversed order when multiple candidates match the same source/target type pairs, this needs to be reversed. This PR aligns this registration order in DateFormatterRegistrar with what's already done in DateTimeFormatterRegistrar.

@bclozel bclozel added the type: bug A general bug label Oct 30, 2019
@bclozel bclozel self-assigned this Oct 30, 2019
@sbrannen sbrannen added this to the 5.2.1 milestone Oct 30, 2019
@sbrannen sbrannen added in: core Issues in core modules (aop, beans, core, context, expression) for: backport-to-5.1.x Marks an issue as a candidate for backport to 5.1.x labels Oct 30, 2019
@spring-projects-issues spring-projects-issues added status: backported An issue that has been backported to maintenance branches and removed for: backport-to-5.1.x Marks an issue as a candidate for backport to 5.1.x labels Oct 30, 2019
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.

Closes spring-projectsgh-23893
@bclozel bclozel merged commit 4beb25b into spring-projects:master Oct 30, 2019
@bclozel bclozel deleted the dateformatter branch October 30, 2019 16:22
pull bot pushed a commit to scope-demo/spring-framework that referenced this pull request Oct 30, 2019
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.

Closes spring-projectsgh-23893
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants