Fix java date parsing to be compatible with joda #36155
Conversation
ZonedDateTime toString format yyyy-MM-ddTHH:mmZ is failing parsing by our java-time DateFormatters whereas it passes Joda parsing. The pattern is strict_date_optional_time||epoch_millis
| @@ -85,13 +85,13 @@ | |||
| .optionalStart() | |||
| .appendFraction(MILLI_OF_SECOND, 3, 3, true) | |||
| .optionalEnd() | |||
| .optionalEnd() | |||
There was a problem hiding this comment.
this is just moving the zoneId below the optional section of seconds/milliseconds. Hard to see with this formatting
| //failing below | ||
| assertSameDate("2015-01-04T00:00Z", "strict_date_optional_time||epoch_millis"); | ||
| } | ||
| //Failing |
There was a problem hiding this comment.
will delete these if the PR is ok, but it helps investigating
There was a problem hiding this comment.
for sure I'm fine with the PR in general! :-)
| @@ -390,6 +391,28 @@ public void testDuelingStrictParsing() { | |||
|
|
|||
| assertSameDate("2012-W31-5", "strict_weekyear_week_day"); | |||
| assertParseException("2012-W1-1", "strict_weekyear_week_day"); | |||
| //failing below | |||
| assertSameDate("2015-01-04T00:00Z", "strict_date_optional_time||epoch_millis"); | |||
There was a problem hiding this comment.
can you just use strict_date_optional_time and remove epoch_millis? Also, can yo move this to the other strict_date_optional_time checks?
Can you also move this strict_date_optional_time checks to the others in this method?
There was a problem hiding this comment.
absolutely - will clean this up
| TemporalAccessor parse = formatter.parse("2015-01-04T00:00Z"); | ||
| } | ||
| //Passing | ||
| public void testParsingJoda(){ |
There was a problem hiding this comment.
these two methods are not needed anymore, correct?
There was a problem hiding this comment.
these are not needed. I only left them if you wanted to verify that quickly on your own
| @@ -33,6 +33,7 @@ | |||
| import java.util.Locale; | |||
|
|
|||
| import static org.hamcrest.Matchers.containsString; | |||
| import static org.hamcrest.Matchers.equalTo; | |||
|
Pinging @elastic/es-core-infra |
|
Please remember to add all relevant labels (area label, version label(s) and change type label) on all PRs and please look for this as part of reviews. The release note generation process is made much harder when PRs are not labelled correctly. |
|
sorry about that @colings86 . totally slipped my mind. Thanks for fixing this |
|
@pgomulka note that I didn't add the change type and version labels so could you please add those? |
|
@colings86 I do not think that this is necessary, as this PR is merged into a feature branch? |
|
Sorry I didn't see that this was for a feature branch. Area label only is fine then :) |
ZonedDateTime toString format yyyy-MM-ddTHH:mmZ is failing parsing by
our java-time DateFormatters whereas it passes Joda parsing. The pattern
is strict_date_optional_time||epoch_millis