-
-
Notifications
You must be signed in to change notification settings - Fork 963
Use consistent ISO-8601 formatting for rendering JSON Date, Calendar, LocalDateTime, and Instant properties #15121
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
Conversation
@codeconsole Please see why the Code style fails |
I forgot to commit the newly added Calendar marshaller. |
This is a breaking change and we did not plan to do another RC. We need to discuss this PR at the weekly to merge it |
.../main/groovy/org/grails/web/converters/configuration/ConvertersConfigurationInitializer.java
Show resolved
Hide resolved
Per weekly meeting, we'll merge after @matrei reviews. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks solid! Nice work Scott 👍
I identified a possibility to drop commons-lang3
from grails-converters
in Grails 8 by replacing FastDateFormat
with stock Java DateTimeFormatter
.
@Override | ||
Object convert(Object value, String key) { | ||
DateTimeFormatter.ISO_ZONED_DATE_TIME.format((ZonedDateTime) value) | ||
DateTimeFormatter.ISO_OFFSET_DATE_TIME.format((ZonedDateTime) value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add comment why we are using ISO_OFFSET_DATE_TIME
instead of ISO_ZONED_DATE_TIME
formatter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matrei in order to match Spring Boot Jackson output:
ZonedDateTime value = ZonedDateTime.now()
println DateTimeFormatter.ISO_ZONED_DATE_TIME.format((ZonedDateTime) value)
println DateTimeFormatter.ISO_OFFSET_DATE_TIME.format((ZonedDateTime) value)
2025-10-09T17:27:04.791062156Z[UTC]
2025-10-09T17:27:04.791062156Z
Spring Boot produces "zonedDateTime": "2025-10-08T00:48:46.407254-07:00",
public void marshalObject(Object object, JSON converter) throws ConverterException { | ||
try { | ||
ZonedDateTime zonedDateTime = (ZonedDateTime) object; | ||
converter.getWriter().value(DateTimeFormatter.ISO_OFFSET_DATE_TIME.format(zonedDateTime)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add comment why we are using ISO_OFFSET_DATE_TIME
instead of ISO_ZONED_DATE_TIME
formatter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matrei see comment above
Fixes #15119
This makes consisteny across all formats and aligns with how Spring Boot 4 Jackson currently renders:
Z
"2025-10-08T07:24:09.696Z"
"2025-10-08T07:24:09.696-07:00"
"2025-10-08T00:24:09.696-07:00[America/Los_Angeles]"
"2025-10-08T00:24:09.696"
Z
"2025-10-08T07:24:09.696Z"
Spring Boot 4:
Grails After this PR: