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

Simplify handling of LocalDate value with invalid trailing time-portion #211

Merged
merged 4 commits into from
Mar 26, 2021

Conversation

cowtowncoder
Copy link
Member

As per title: test change just cleaning up, but for LocalDate, avoid unnecessary handling of timezone.

@@ -155,7 +155,8 @@ protected LocalDate _fromString(JsonParser p, DeserializationContext ctxt,
// JavaScript by default includes time in JSON serialized Dates (UTC/ISO instant format).
if (string.length() > 10 && string.charAt(10) == 'T') {
if (string.endsWith("Z")) {
return LocalDateTime.ofInstant(Instant.parse(string), ZoneOffset.UTC).toLocalDate();
return LocalDate.parse(string.substring(0, string.length() - 1),
DateTimeFormatter.ISO_LOCAL_DATE_TIME);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to me that there is no need to go back and forth to zoned instant when simply trying to retain local value.
Tests all pass as well.

@cowtowncoder cowtowncoder requested a review from kupci March 26, 2021 02:32
@cowtowncoder
Copy link
Member Author

@kupci Just noticed this when looking at LocalDate/LocalDateTime deserialization; wanted to sanity check my thinking here.

Copy link
Member

@kupci kupci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not contain time component when 'strict' mode set for property or type (enable 'lenient' handling to allow)

Nice solution! I see this question coming up every so often, so this looks like a good improvement.

@cowtowncoder
Copy link
Member Author

I copied that from LocalDateTime deserializer where it already existed. I hope logic makes sense; it should as lenient is the default.

I actually should file an issue for that change, and simplification can just be part of implementation for that issue.

@cowtowncoder cowtowncoder added this to the 2.13.0 milestone Mar 26, 2021
@cowtowncoder cowtowncoder merged commit 8b30c9b into 2.13 Mar 26, 2021
@cowtowncoder cowtowncoder deleted the tatu/simplify-local-dt-with-z-deser branch March 26, 2021 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants