Skip to content
This repository has been archived by the owner on Nov 7, 2019. It is now read-only.

Handle JSON serialized Dates from JavaScript in LocalDateDeserializer #56

Merged
merged 1 commit into from
Jan 18, 2016

Conversation

sandermak
Copy link
Contributor

I've been at this before (#28), however, during the review phase of the PR I accidentally borked my own use-case. Unfortunately I only found out after we switched from our internally patched version to the current 2.7 release.

It's now possible to parse a date and time ("2016-01-18T15:41:32.232") as LocalDate. However, what I needed was to be able to parse this, coming from JavaScript:

new Date().toJSON()
"2016-01-18T14:09:30.163Z"

(notice the trailing Z, which is currently not accepted)

JavaScript generates an ISO instant string when serialising a JavaScript Date [1]. It seems I am not the only one running into this issue [2]. I think it would be great to revise my original PR to allow for this. That way, it works for many people doing Java/JS interop without having to override the pattern on each usage of a LocalDate field. The check for custom formatter (https://github.com/FasterXML/jackson-datatype-jsr310/issues/37) is obviously still in place. I also made the check for the presence of the time component a bit more robust.

Thanks for considering this PR. My CLA is already in place, so that should not be an issue.

[1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toJSON & https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toISOString
[2] http://stackoverflow.com/questions/26233882/javascript-date-to-java-time-localdate & http://stackoverflow.com/questions/5591967/how-to-deserialize-js-date-using-jackson

@cowtowncoder
Copy link
Member

Thank you for following up on this. This will be in 2.7.1 then.

cowtowncoder added a commit that referenced this pull request Jan 18, 2016
Handle JSON serialized Dates from JavaScript in LocalDateDeserializer
@cowtowncoder cowtowncoder merged commit e74927b into FasterXML:master Jan 18, 2016
cowtowncoder added a commit that referenced this pull request Jan 18, 2016
@sandermak
Copy link
Contributor Author

Awesome, thanks for being so responsive and running this great project!

@cowtowncoder
Copy link
Member

@sandermak No problem; good to have active contributors to catch remaining issues!

@cbornet
Copy link

cbornet commented Jul 13, 2016

I think using a formatter

ISO_DATE_OPTIONAL_TIME = new DateTimeFormatterBuilder()
            .append(DateTimeFormatter.ISO_LOCAL_DATE)
            .optionalStart()
            .appendLiteral('T')
            .append(DateTimeFormatter.ISO_TIME)
            .toFormatter();

would be cleaner

@cowtowncoder
Copy link
Member

@cbornet we are always open for further improvement PRs!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants