Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adapt plugin for Java 21 #258

Merged
merged 5 commits into from Aug 13, 2023
Merged

Conversation

MarkEWaite
Copy link
Contributor

@MarkEWaite MarkEWaite commented Aug 12, 2023

JENKINS-71817 - Adapt plugin for Java 21

The Java 21 DateFormat class will not parse strings that contain only a time component without a date component. The plugin relies on the earlier Java behavior of parsing "10:00:00 PM" into a DateFormat for today. That's a bad assumption that the plugin was making, but the assumption exists and users may be relying on the assumption.

Java 21 date string output has changed from a space character to a Unicode short space character. That change required some test changes. jenkinsci/token-macro-plugin#191 shows a technique to handle that portably across multiple Java versions.

Checklist

  • I have read the CONTRIBUTING doc
  • I have referenced the Jira issue related to my changes in one or more commit messages
  • I have added tests that verify my changes
  • Unit tests pass locally with my changes
  • I have added documentation as necessary
  • No spotbugs warnings were introduced with my changes
  • I have interactively tested my changes

Types of changes

  • New feature (non-breaking change which adds functionality)

Further comments

More changes are required before this pull request can be considered. Automated tests are failing due to unparseable date.

@github-actions github-actions bot added the chore Reduces maintenance effort by changes not directly visible to users label Aug 12, 2023
Java 21 changed the date format to use a narrow no-break space, so the
regular expression used in the tests now fails to match. Changed the
test to use the '\h' character class, which matches.

jenkinsci/token-macro-plugin#191 uses the same
technique to handle test compatibility between Java 11, Java 17, and
Java 21.
Java 21 changed the behavior of the DateFormat parsing to no longer
accept strings that contain only a time.  Previous versions of Java
would default to January 1, 1970 if no date value was specified.

The Java 21 behavior is quite reasonable, since a Date specifies a number
of milliseconds since the epoch, not an offset since the start of the day.
The plugin misuses a Date object to represent what Java 8 declared as
LocalTime, a representation of time within a day.

This change continues with the previous behavior, checking a DateFormat
first.  If the DateFormat parse fails, then it iterates through a series
of DateTimeFormatter patterns to check if the passed string matches one
of those formats.

I've not detected any case in the typical use of the plugin where the
new Java 21 behavior affects plugin behavior.  It seems that there should
be cases where the change is relevant, but I was not able to detect any
of those cases.
Same parsers are needed in the action class, but there are not yet any
failing tests to show how they might help.
@MarkEWaite MarkEWaite merged commit 19e06ce into jenkinsci:master Aug 13, 2023
18 checks passed
@MarkEWaite MarkEWaite deleted the test-with-java-21 branch August 13, 2023 01:45
@MarkEWaite MarkEWaite added bugfix and removed chore Reduces maintenance effort by changes not directly visible to users labels Aug 13, 2023
@MarkEWaite MarkEWaite added bug Incorrect or flawed behavior and removed bugfix labels Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect or flawed behavior
Projects
None yet
1 participant