Improve code coverage

hub4j · Feb 13, 2025 · af54d2c · af54d2c
1 parent 38ca010
commit af54d2c
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 63 deletions.
diff --git a/src/main/java/org/kohsuke/github/AbuseLimitHandler.java b/src/main/java/org/kohsuke/github/AbuseLimitHandler.java
@@ -3,11 +3,7 @@
 import org.kohsuke.github.connector.GitHubConnectorResponse;
 
 import java.io.IOException;
-import java.io.InterruptedIOException;
 import java.net.HttpURLConnection;
-import java.time.ZonedDateTime;
-import java.time.format.DateTimeFormatter;
-import java.time.temporal.ChronoUnit;
 
 import javax.annotation.Nonnull;
 
@@ -88,13 +84,8 @@ public void onError(@Nonnull GitHubConnectorResponse connectorResponse) throws I
     public static final AbuseLimitHandler WAIT = new AbuseLimitHandler() {
         @Override
         public void onError(IOException e, HttpURLConnection uc) throws IOException {
-            try {
-                Thread.sleep(parseWaitTime(uc));
-            } catch (InterruptedException ex) {
-                throw (InterruptedIOException) new InterruptedIOException().initCause(e);
-            }
+            sleep(parseWaitTime(uc));
         }
-
     };
 
     /**
@@ -116,19 +107,6 @@ public void onError(IOException e, HttpURLConnection uc) throws IOException {
      * number or a date (the spec allows both). If no header is found, wait for a reasonably amount of time.
      */
     long parseWaitTime(HttpURLConnection uc) {
-        String v = uc.getHeaderField("Retry-After");
-        if (v == null) {
-            // can't tell, wait for unambiguously over one minute per GitHub guidance
-            return DEFAULT_WAIT_MILLIS;
-        }
-
-        try {
-            return Math.max(1000, Long.parseLong(v) * 1000);
-        } catch (NumberFormatException nfe) {
-            // The retry-after header could be a number in seconds, or an http-date
-            ZonedDateTime zdt = ZonedDateTime.parse(v, DateTimeFormatter.RFC_1123_DATE_TIME);
-            return ChronoUnit.MILLIS.between(ZonedDateTime.now(), zdt);
-        }
+        return parseWaitTime(uc.getHeaderField("Retry-After"), null, DEFAULT_WAIT_MILLIS, 1000);
     }
-
 }
diff --git a/src/main/java/org/kohsuke/github/GitHubAbuseLimitHandler.java b/src/main/java/org/kohsuke/github/GitHubAbuseLimitHandler.java
@@ -3,12 +3,8 @@
 import org.kohsuke.github.connector.GitHubConnectorResponse;
 
 import java.io.IOException;
-import java.io.InterruptedIOException;
 import java.net.HttpURLConnection;
 import java.time.Duration;
-import java.time.ZonedDateTime;
-import java.time.format.DateTimeFormatter;
-import java.time.temporal.ChronoUnit;
 
 import javax.annotation.Nonnull;
 
@@ -125,11 +121,7 @@ private boolean hasHeader(GitHubConnectorResponse connectorResponse, String head
     public static final GitHubAbuseLimitHandler WAIT = new GitHubAbuseLimitHandler() {
         @Override
         public void onError(GitHubConnectorResponse connectorResponse) throws IOException {
-            try {
-                Thread.sleep(parseWaitTime(connectorResponse));
-            } catch (InterruptedException ex) {
-                throw (InterruptedIOException) new InterruptedIOException().initCause(ex);
-            }
+            sleep(parseWaitTime(connectorResponse));
         }
     };
 
@@ -155,30 +147,10 @@ public void onError(GitHubConnectorResponse connectorResponse) throws IOExceptio
      * number or a date (the spec allows both). If no header is found, wait for a reasonably amount of time.
      */
     static long parseWaitTime(GitHubConnectorResponse connectorResponse) {
-        String v = connectorResponse.header("Retry-After");
-        if (v == null) {
-            return DEFAULT_WAIT_MILLIS;
-        }
-
-        try {
-            return Math.max(MINIMUM_ABUSE_RETRY_MILLIS, Duration.ofSeconds(Long.parseLong(v)).toMillis());
-        } catch (NumberFormatException nfe) {
-            // The retry-after header could be a number in seconds, or an http-date
-            // We know it was a date if we got a number format exception :)
-
-            // Don't use ZonedDateTime.now(), because the local and remote server times may not be in sync
-            // Instead, we can take advantage of the Date field in the response to see what time the remote server
-            // thinks it is
-            String dateField = connectorResponse.header("Date");
-            ZonedDateTime now;
-            if (dateField != null) {
-                now = ZonedDateTime.parse(dateField, DateTimeFormatter.RFC_1123_DATE_TIME);
-            } else {
-                now = ZonedDateTime.now();
-            }
-            ZonedDateTime zdt = ZonedDateTime.parse(v, DateTimeFormatter.RFC_1123_DATE_TIME);
-            return Math.max(MINIMUM_ABUSE_RETRY_MILLIS, ChronoUnit.MILLIS.between(now, zdt));
-        }
+        return parseWaitTime(connectorResponse.header("Retry-After"),
+                connectorResponse.header("Date"),
+                DEFAULT_WAIT_MILLIS,
+                MINIMUM_ABUSE_RETRY_MILLIS);
     }
 
 }
diff --git a/src/main/java/org/kohsuke/github/GitHubConnectorResponseErrorHandler.java b/src/main/java/org/kohsuke/github/GitHubConnectorResponseErrorHandler.java
@@ -7,7 +7,12 @@
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.io.InterruptedIOException;
 import java.nio.charset.StandardCharsets;
+import java.time.Duration;
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.time.temporal.ChronoUnit;
 
 import javax.annotation.Nonnull;
 
@@ -111,4 +116,38 @@ private boolean isServiceDown(GitHubConnectorResponse connectorResponse) throws
             return false;
         }
     };
+
+    static void sleep(long waitMillis) throws IOException {
+        try {
+            Thread.sleep(waitMillis);
+        } catch (InterruptedException ex) {
+            throw (InterruptedIOException) new InterruptedIOException().initCause(ex);
+        }
+    }
+
+    static long parseWaitTime(String waitHeader, String dateHeader, long defaultMillis, long minimumMillis) {
+        if (waitHeader == null) {
+            return defaultMillis;
+        }
+
+        try {
+            return Math.max(minimumMillis, Duration.ofSeconds(Long.parseLong(waitHeader)).toMillis());
+        } catch (NumberFormatException nfe) {
+            // The wait header could be a number in seconds, or an http-date
+            // We know it was a date if we got a number format exception :)
+
+            // Try not to use ZonedDateTime.now(), because the local and remote server times may not be in sync
+            // Instead, we can take advantage of the Date field in the response to see what time the remote server
+            // thinks it is
+            String dateField = dateHeader;
+            ZonedDateTime now;
+            if (dateField != null) {
+                now = ZonedDateTime.parse(dateField, DateTimeFormatter.RFC_1123_DATE_TIME);
+            } else {
+                now = ZonedDateTime.now();
+            }
+            ZonedDateTime zdt = ZonedDateTime.parse(waitHeader, DateTimeFormatter.RFC_1123_DATE_TIME);
+            return Math.max(minimumMillis, ChronoUnit.MILLIS.between(now, zdt));
+        }
+    }
 }
diff --git a/src/main/java/org/kohsuke/github/GitHubRateLimitHandler.java b/src/main/java/org/kohsuke/github/GitHubRateLimitHandler.java
@@ -4,7 +4,6 @@
 import org.kohsuke.github.connector.GitHubConnectorResponse;
 
 import java.io.IOException;
-import java.io.InterruptedIOException;
 import java.net.HttpURLConnection;
 import java.time.Duration;
 import java.time.ZonedDateTime;
@@ -72,11 +71,7 @@ boolean isError(@NotNull GitHubConnectorResponse connectorResponse) throws IOExc
     public static final GitHubRateLimitHandler WAIT = new GitHubRateLimitHandler() {
         @Override
         public void onError(GitHubConnectorResponse connectorResponse) throws IOException {
-            try {
-                Thread.sleep(parseWaitTime(connectorResponse));
-            } catch (InterruptedException ex) {
-                throw (InterruptedIOException) new InterruptedIOException().initCause(ex);
-            }
+            sleep(parseWaitTime(connectorResponse));
         }
     };
 
@@ -100,6 +95,10 @@ long parseWaitTime(GitHubConnectorResponse connectorResponse) {
             now = ZonedDateTime.now();
         }
         return Math.max(MINIMUM_RATE_LIMIT_RETRY_MILLIS, (Long.parseLong(v) - now.toInstant().getEpochSecond()) * 1000);
+        // return parseWaitTime(connectorResponse.header("X-RateLimit-Reset"),
+        // connectorResponse.header("Date"),
+        // Duration.ofMinutes(1).toMillis(),
+        // MINIMUM_RATE_LIMIT_RETRY_MILLIS);
     }
 
     /**