Supported parsing date time text with valid zero zone offsets #19
Conversation
Offset format with `+00`, `+0000` and `+00:00` are all supported now.
| * @since 2.9.0 | ||
| */ | ||
| private static final String ISO8601_UTC_ZERO_OFFSET_SUFFIX_REGEX = "\\+00:?(00)?$"; | ||
| private static final String ISO8601_UTC_ZERO_OFFSET_REGEX = "[^\\+]+" + ISO8601_UTC_ZERO_OFFSET_SUFFIX_REGEX; |
There was a problem hiding this comment.
This should be pre-compiled as Pattern for performance reasons.
| @@ -285,6 +288,20 @@ private ZoneId getZone(DeserializationContext context) | |||
| return (_valueClass == Instant.class) ? null : context.getTimeZone().toZoneId(); | |||
| } | |||
|
|
|||
| private String replaceZeroOffsetAsZIfNecessary(String text) | |||
| { | |||
| if (replaceZeroOffsetAsZ && endsWithZeroOffset(text)) { | |||
There was a problem hiding this comment.
This seems to evaluate regexp twice, first to match, then to replace: would it be possible to make that just one operation? Regexp evaluation has non-trivial cost, even considering that textual date handling itself is rather expensive operation.
There was a problem hiding this comment.
Thanks for the comment @cowtowncoder, I just checked the doc, your point makes perfect sense, I was wrong, I remember if no match found that would throw an exception. will fix
|
Thank you for contributing this, yes, makes sense. The only suggestion I have is that since regexp handling has some overhead, it'd be good to do the usual optimization: pre-compile regexp String as |
|
One other thing: I'm happy to merge this (ideally with changes although I can probably make them myself too if need be), but unless I've asked before we need a filled Contributor License Agreement (CLA) before the first contribution (but just once, it's good for any other patches). https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf and usually it's easiest to print, fill & sign, scan, email to Thank you again for the contribution! |
|
@cowtowncoder I am really grateful for the comments and excited to contribute to this project, I will sign that CLA and send email later (well, before I fixed all the issues you pointed) |
check if the input matches the pattern.
|
@cowtowncoder comments fixed and CLA mail sent. Please have a look. |
|
@kevinjom Thank you! |
Refer to #18