From 4d46872c3577d6e1aec969286050153425429880 Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Fri, 7 Aug 2020 15:34:29 -0700 Subject: [PATCH] Fixed and streamlined date parsing Fixes #917 --- .../java/org/kohsuke/github/GitHubClient.java | 37 ++++++++--------- .../org/kohsuke/github/GitHubStaticTest.java | 40 ++++++++++++++----- 2 files changed, 49 insertions(+), 28 deletions(-) diff --git a/src/main/java/org/kohsuke/github/GitHubClient.java b/src/main/java/org/kohsuke/github/GitHubClient.java index 2af68882d..b27932448 100644 --- a/src/main/java/org/kohsuke/github/GitHubClient.java +++ b/src/main/java/org/kohsuke/github/GitHubClient.java @@ -19,15 +19,15 @@ import java.net.SocketException; import java.net.SocketTimeoutException; import java.net.URL; import java.nio.charset.StandardCharsets; -import java.text.ParseException; -import java.text.SimpleDateFormat; +import java.time.Instant; +import java.time.format.DateTimeFormatter; +import java.time.temporal.ChronoUnit; import java.util.Base64; import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.TimeZone; import java.util.function.Consumer; import java.util.logging.Logger; @@ -80,9 +80,8 @@ abstract class GitHubClient { private static final ObjectMapper MAPPER = new ObjectMapper(); static final String GITHUB_URL = "https://api.github.com"; - private static final String[] TIME_FORMATS = { "yyyy/MM/dd HH:mm:ss ZZZZ", "yyyy-MM-dd'T'HH:mm:ss'Z'", - "yyyy-MM-dd'T'HH:mm:ss.S'Z'" // GitHub App endpoints return a different date format - }; + private static final DateTimeFormatter DATE_TIME_PARSER_SLASHES = DateTimeFormatter + .ofPattern("yyyy/MM/dd HH:mm:ss Z"); static { MAPPER.setVisibility(new VisibilityChecker.Std(NONE, NONE, NONE, NONE, ANY)); @@ -652,22 +651,24 @@ abstract class GitHubClient { static Date parseDate(String timestamp) { if (timestamp == null) return null; - for (String f : TIME_FORMATS) { - try { - SimpleDateFormat df = new SimpleDateFormat(f); - df.setTimeZone(TimeZone.getTimeZone("GMT")); - return df.parse(timestamp); - } catch (ParseException e) { - // try next - } + + return Date.from(parseInstant(timestamp)); + } + + static Instant parseInstant(String timestamp) { + if (timestamp == null) + return null; + + if (timestamp.charAt(4) == '/') { + // Unsure where this is used, but retained for compatibility. + return Instant.from(DATE_TIME_PARSER_SLASHES.parse(timestamp)); + } else { + return Instant.from(DateTimeFormatter.ISO_OFFSET_DATE_TIME.parse(timestamp)); } - throw new IllegalStateException("Unable to parse the timestamp: " + timestamp); } static String printDate(Date dt) { - SimpleDateFormat df = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss'Z'"); - df.setTimeZone(TimeZone.getTimeZone("GMT")); - return df.format(dt); + return DateTimeFormatter.ISO_INSTANT.format(Instant.ofEpochMilli(dt.getTime()).truncatedTo(ChronoUnit.SECONDS)); } /** diff --git a/src/test/java/org/kohsuke/github/GitHubStaticTest.java b/src/test/java/org/kohsuke/github/GitHubStaticTest.java index 4e3808998..2b5a4b430 100644 --- a/src/test/java/org/kohsuke/github/GitHubStaticTest.java +++ b/src/test/java/org/kohsuke/github/GitHubStaticTest.java @@ -4,6 +4,7 @@ import org.junit.Test; import java.text.SimpleDateFormat; import java.time.Instant; +import java.time.format.DateTimeParseException; import java.time.temporal.ChronoUnit; import java.util.Date; import java.util.TimeZone; @@ -20,25 +21,38 @@ public class GitHubStaticTest extends AbstractGitHubWireMockTest { @Test public void timeRoundTrip() throws Exception { - Instant instantNow = Instant.now(); + final long stableInstantEpochMilli = 1533721222255L; + Instant instantNow = Instant.ofEpochMilli(stableInstantEpochMilli); Date instantSeconds = Date.from(instantNow.truncatedTo(ChronoUnit.SECONDS)); Date instantMillis = Date.from(instantNow.truncatedTo(ChronoUnit.MILLIS)); - // if we happen to land exactly on zero milliseconds, add 1 milli - if (instantSeconds.equals(instantMillis)) { - instantMillis = Date.from(instantNow.plusMillis(1).truncatedTo(ChronoUnit.MILLIS)); - } + String instantFormatSlash = formatZonedDate(instantMillis, "yyyy/MM/dd HH:mm:ss ZZZZ", "PST"); + assertThat(instantFormatSlash, equalTo("2018/08/08 02:40:22 -0700")); - // TODO: other formats - String instantFormatSlash = formatDate(instantMillis, "yyyy/MM/dd HH:mm:ss ZZZZ"); String instantFormatDash = formatDate(instantMillis, "yyyy-MM-dd'T'HH:mm:ss'Z'"); + assertThat(instantFormatDash, equalTo("2018-08-08T09:40:22Z")); + String instantFormatMillis = formatDate(instantMillis, "yyyy-MM-dd'T'HH:mm:ss.S'Z'"); + assertThat(instantFormatMillis, equalTo("2018-08-08T09:40:22.255Z")); + + String instantFormatMillisZoned = formatZonedDate(instantMillis, "yyyy-MM-dd'T'HH:mm:ss.SXXX", "PST"); + assertThat(instantFormatMillisZoned, equalTo("2018-08-08T02:40:22.255-07:00")); + String instantSecondsFormatMillis = formatDate(instantSeconds, "yyyy-MM-dd'T'HH:mm:ss.S'Z'"); + assertThat(instantSecondsFormatMillis, equalTo("2018-08-08T09:40:22.0Z")); + + String instantSecondsFormatMillisZoned = formatZonedDate(instantSeconds, "yyyy-MM-dd'T'HH:mm:ss.SSSXXX", "PST"); + assertThat(instantSecondsFormatMillisZoned, equalTo("2018-08-08T02:40:22.000-07:00")); + String instantBadFormat = formatDate(instantMillis, "yy-MM-dd'T'HH:mm'Z'"); + assertThat(instantBadFormat, equalTo("18-08-08T09:40Z")); assertThat(GitHubClient.parseDate(GitHubClient.printDate(instantSeconds)), equalTo(GitHubClient.parseDate(GitHubClient.printDate(instantMillis)))); + assertThat(GitHubClient.printDate(instantSeconds), equalTo("2018-08-08T09:40:22Z")); + assertThat(GitHubClient.printDate(GitHubClient.parseDate(instantFormatMillisZoned)), + equalTo("2018-08-08T09:40:22Z")); assertThat(instantSeconds, equalTo(GitHubClient.parseDate(GitHubClient.printDate(instantSeconds)))); @@ -51,14 +65,16 @@ public class GitHubStaticTest extends AbstractGitHubWireMockTest { // This parser does not truncate to the nearest second, so it will be equal assertThat(instantMillis, equalTo(GitHubClient.parseDate(instantFormatMillis))); + assertThat(instantMillis, equalTo(GitHubClient.parseDate(instantFormatMillisZoned))); assertThat(instantSeconds, equalTo(GitHubClient.parseDate(instantSecondsFormatMillis))); + assertThat(instantSeconds, equalTo(GitHubClient.parseDate(instantSecondsFormatMillisZoned))); try { GitHubClient.parseDate(instantBadFormat); fail("Bad time format should throw."); - } catch (IllegalStateException e) { - assertThat(e.getMessage(), equalTo("Unable to parse the timestamp: " + instantBadFormat)); + } catch (DateTimeParseException e) { + assertThat(e.getMessage(), equalTo("Text '" + instantBadFormat + "' could not be parsed at index 0")); } } @@ -226,8 +242,12 @@ public class GitHubStaticTest extends AbstractGitHubWireMockTest { } static String formatDate(Date dt, String format) { + return formatZonedDate(dt, format, "GMT"); + } + + static String formatZonedDate(Date dt, String format, String timeZone) { SimpleDateFormat df = new SimpleDateFormat(format); - df.setTimeZone(TimeZone.getTimeZone("GMT")); + df.setTimeZone(TimeZone.getTimeZone(timeZone)); return df.format(dt); }