[Scala][Java][okhttp] Fix Gson parsing of Joda DateTime without millis#4473
Merged
wing328 merged 2 commits intoswagger-api:masterfrom Jan 7, 2017
Merged
[Scala][Java][okhttp] Fix Gson parsing of Joda DateTime without millis#4473wing328 merged 2 commits intoswagger-api:masterfrom
wing328 merged 2 commits intoswagger-api:masterfrom
Conversation
Contributor
|
Yes but dateOptionalTimeParser should be used instead. |
Contributor
Author
|
@cbornet Why |
Contributor
Author
|
@cbornet I took a closer look at the grammars accepted by |
Contributor
Author
|
@cbornet I noticed that |
The DateTimeFormatter returned by ISODateTimeFormat.dateTime() only parses dates with millisecond values, and throws IllegalArgumentException when milliseconds are not present. The date-time construct from RFC 3339 Section 5.6 referenced by the Swagger/OpenAPI spec allows fractional second values to be omitted. This results in valid date-time values being rejected by the generated code. This commit fixes the problem by using .dateOptionalTimeParser() for parsing, which correctly handles date-time values without fractional seconds. A previous version of this commit used .dateTimeParser(), which accepted a time without a date and was considered too liberal. Note that .dateTime() must still be used for printing, which is not supported by .dateTimeParser(). Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
As in the previous commit, which fixed Java generators, ISOISODateTimeFormat.dateOptionalTimeParser() should be used for date-time parsing and ISOISODateTimeFormat.dateTime() for printing. Apply the same change to akka-scala. Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
kevinoid
force-pushed
the
fix-gson-joda-datetime
branch
from
e26b85b to
5765cd5
Compare
Contributor
Author
|
PR updated and CI passed. I included the analogous akka-scala change as a separate commit that can easily be dropped if it should be handled separately. |
Contributor
|
👍 |
Contributor
|
@kevinoid thanks for the contribution, especially to the |
Contributor
Author
|
@wing328 Thanks for reviewing and merging it! |
wing328
changed the title
wing328
changed the title
davidgri
pushed a commit
to davidgri/swagger-codegen
that referenced
this pull request
May 11, 2017
* Fix Gson parsing of Joda DateTime without millis The DateTimeFormatter returned by ISODateTimeFormat.dateTime() only parses dates with millisecond values, and throws IllegalArgumentException when milliseconds are not present. The date-time construct from RFC 3339 Section 5.6 referenced by the Swagger/OpenAPI spec allows fractional second values to be omitted. This results in valid date-time values being rejected by the generated code. This commit fixes the problem by using .dateOptionalTimeParser() for parsing, which correctly handles date-time values without fractional seconds. A previous version of this commit used .dateTimeParser(), which accepted a time without a date and was considered too liberal. Note that .dateTime() must still be used for printing, which is not supported by .dateTimeParser(). Signed-off-by: Kevin Locke <kevin@kevinlocke.name> * Fix akka-scala date-time parser with Joda As in the previous commit, which fixed Java generators, ISOISODateTimeFormat.dateOptionalTimeParser() should be used for date-time parsing and ISOISODateTimeFormat.dateTime() for printing. Apply the same change to akka-scala. Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.shand./bin/security/{LANG}-petstore.shif updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates)2.3.0branch for breaking (non-backward compatible) changes.Description of the PR
The
DateTimeFormatterreturned byISODateTimeFormat.dateTime()only parses dates with millisecond values, and throwsIllegalArgumentExceptionwhen milliseconds are not present. Thedate-time construct from RFC 3339 Section 5.6 referenced by the Swagger/OpenAPI spec allows fractional second values to be omitted. This results in valid date-time values being rejected by the generated code.
This PR fixes the problem by using
ISODateTimeFormat.dateTimeParser()for parsing, which correctly handles date-time values without fractional seconds. Note that.dateTime()muststill be used for printing, which is not supported by
.dateTimeParser().