fix: handle leap days correctly during timestamp extraction

[vinothchandar]

Description

fixes #4864, in a localized way, added a unit test case

Testing done

unit tests, also tested using an end-end example with some sample data containing leap days

Reviewer checklist

• Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
• Ensure relevant issues are linked (description should include text like "Fixes #")

[ghost]

@confluentinc It looks like @vinothchandar just signed our Contributor License Agreement. 👍

Always at your service,

clabot

[vinothchandar]

@agavra Would you be able to share your thoughts here..Made a localized fix to get us moving on a production issue.. but there has to be a nicer way to do this prioritization of chrono fields settng order? any other cases that may demand this sort of ordering .. wdyt> ?

[agavra]

Honestly I'm not totally sure what the implications of this are. I remember adding significant unit testing around this because the behavior was fragile around partial date formats (see #2499).

If those tests pass, then I don't see harm in having it done the way you suggest here but again anything with datetime parsing in Java is very brittle.

[vinothchandar]

@agavra tests seem happy.. We were depending on the ordinal value of these enum and that was making me bit uneasy. Hopefully, that does not get changed over time.

I confirmed the sorting holds true

.
May be a more explicit way is to pick YEAR as the first item and then append the remaining list as is (skipping YEAR)?

[agavra]

May be a more explicit way is to pick YEAR as the first item and then append the remaining list as is (skipping YEAR)?

That might work, but I feel like the most important thing here is to just have a comprehensive suite of tests and then if anything changes from underneath us we'll be able to at least detect and react

[vinothchandar]

private static final List<ChronoField> CHRONO_FIELDS = Stream.concat(
      Stream.of(ChronoField.YEAR),
      Arrays.stream(ChronoField.values()).filter(f -> !f.equals(ChronoField.YEAR)))
      .collect(Collectors.toList());

doing so, fails the tests when DayOfWeek is set.

java.time.DateTimeException: Invalid date 'DayOfYear 366' as '2011' is not a leap year

So there the order is not meaningless. Let me think more.

[vinothchandar]

@agavra if you are still curious, the ordering might have some trickiness in general..

So I reworked it into a very specific fix: eagerly overriding the year if it's a leap day.

[agavra]

Tricky! It still looks good to me. Thanks @vinothchandar