Add feature toggle to read numeric strings as numeric timestamps #269
Conversation
Please hold one moment while I print liquid gold on dead tree. |
|
Ok: as per my comments, I'd like to use a module configuration setting here. But looking at the way deserializers are implemented, this appears a bit tricky: not so much wrt configuring |
|
Ok, I found time to implement #281, and for that introduced |
|
Cool. I'll rework the PR to use that. |
There is an apparent inconsistency in the way Jackson de-serializes numbers that are shaped as a string into an instant. * When not providing a custom date format, quoted numbers are treated as epoch milis/nanos * When not providing a custom date, quoted numbers are assumed to be handled by the pattern. There is however no way to construct a pattern that handles both ISO dates and epoch milis/nanos. This feature toggle allow numeric strings to be read as numeric timestamps. Fixes FasterXML#263
mpkorstanje
force-pushed
the
feature/toggle-to-parse-qouted-numbers-with-custom-formatter
branch
from
e5b0adb
to
d93b065
Compare
datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/JavaTimeFeature.java
Outdated
Show resolved
Hide resolved
|
Looks like I got CLA too, great! |
|
@mpkorstanje I renamed the feature, made some other minor tweaks and so I think this is ready for merge from my POV. |
| * Note that when a pattern is not explicitly defined numeric strings are | ||
| * interpreted as a numeric timestamp. | ||
| */ | ||
| ALWAYS_ALLOW_STRINGIFIED_TIMESTAMPS(false) |
There was a problem hiding this comment.
To be consistent with READ_DATE_TIMESTAMPS_AS_NANOSECONDS I reckon ALWAYS_READ_STRINGIFIED_DATE_TIMESTAMPS would be more accurate. Especially when used together, because they then would appear to refer to the same thing.
There was a problem hiding this comment.
I'll add DATE_ in there, good suggestion.
The reason I think "ALLOW" is important is because this is used in addition to -- but not instead of -- custom pattern.
| * Flag set from | ||
| * {@link com.fasterxml.jackson.datatype.jsr310.JavaTimeFeature#ALWAYS_ALLOW_STRINGIFIED_TIMESTAMPS} | ||
| * to determine whether numeric strings are interpreted as numeric timestamps | ||
| * (enabled) nor not (disabled) in addition to an explicitly defined pattern. |
There was a problem hiding this comment.
Should we replace "pattern" with "custom DateTimeFormatter" as you added to the changelog?
datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/deser/InstantDeserializer.java
Outdated
Show resolved
Hide resolved
datetime/src/main/java/com/fasterxml/jackson/datatype/jsr310/deser/InstantDeserializer.java
Outdated
Show resolved
Hide resolved
|
Sorry for the delay. Yesterday was a bit full. Naming aside, it looks good to me. |
|
Thank you for all the work here & improvement suggestions @mpkorstanje ! |
mpkorstanje
deleted the
feature/toggle-to-parse-qouted-numbers-with-custom-formatter
branch
There is an apparent inconsistency in the way Jackson de-serializes numbers that are shaped as a string into an instant.
There is however no way to construct a pattern that handles both ISO dates and epoch milis/nanos. This feature toggle allow numeric strings to be read as numeric timestamps.
Fixes: #263.
This PR is paired with FasterXML/jackson-databind#3830.