Merge pull request hub4j#1298 from bitwiseman/task/deprecation2

Deprecate limit handlers using HttpURLConnection

Author: bitwiseman

src/main/java/org/kohsuke/github/AbuseLimitHandler.java

@@ -12,11 +12,13 @@
  * Pluggable strategy to determine what to do when the API abuse limit is hit.
  *
  * @author Kohsuke Kawaguchi
- * @see GitHubBuilder#withAbuseLimitHandler(AbuseLimitHandler) GitHubBuilder#withAbuseLimitHandler(AbuseLimitHandler)
+ * @see GitHubBuilder#withAbuseLimitHandler(GitHubAbuseLimitHandler)
+ *      GitHubBuilder#withAbuseLimitHandler(GitHubAbuseLimitHandler)
  * @see <a href="https://developer.github.com/v3/#abuse-rate-limits">documentation</a>
  * @see RateLimitHandler
  */
-public abstract class AbuseLimitHandler {
+@Deprecated
+public abstract class AbuseLimitHandler extends GitHubAbuseLimitHandler {
 
     /**
      * Called when the library encounters HTTP error indicating that the API abuse limit is reached.
@@ -37,7 +39,7 @@ public abstract class AbuseLimitHandler {
      *
      */
     public void onError(@Nonnull GitHubConnectorResponse connectorResponse) throws IOException {
-        GHIOException e = new HttpException("Abuse limit violation",
+        GHIOException e = new HttpException("Abuse limit reached",
                 connectorResponse.statusCode(),
                 connectorResponse.header("Status"),
                 connectorResponse.request().url().toString()).withResponseHeaderFields(connectorResponse.allHeaders());
@@ -66,12 +68,12 @@ public void onError(@Nonnull GitHubConnectorResponse connectorResponse) throws I
      *
      */
     @Deprecated
-    public void onError(IOException e, HttpURLConnection uc) throws IOException {
-    }
+    public abstract void onError(IOException e, HttpURLConnection uc) throws IOException;
 
     /**
      * Wait until the API abuse "wait time" is passed.
      */
+    @Deprecated
     public static final AbuseLimitHandler WAIT = new AbuseLimitHandler() {
         @Override
         public void onError(IOException e, HttpURLConnection uc) throws IOException {
@@ -94,10 +96,11 @@ private long parseWaitTime(HttpURLConnection uc) {
     /**
      * Fail immediately.
      */
+    @Deprecated
     public static final AbuseLimitHandler FAIL = new AbuseLimitHandler() {
         @Override
         public void onError(IOException e, HttpURLConnection uc) throws IOException {
-            throw (IOException) new IOException("Abuse limit reached").initCause(e);
+            throw e;
         }
     };
 }

src/main/java/org/kohsuke/github/GitHub.java

@@ -113,8 +113,8 @@ public class GitHub {
      */
     GitHub(String apiUrl,
             GitHubConnector connector,
-            RateLimitHandler rateLimitHandler,
-            AbuseLimitHandler abuseLimitHandler,
+            GitHubRateLimitHandler rateLimitHandler,
+            GitHubAbuseLimitHandler abuseLimitHandler,
             GitHubRateLimitChecker rateLimitChecker,
             AuthorizationProvider authorizationProvider) throws IOException {
         if (authorizationProvider instanceof DependentAuthorizationProvider) {

src/main/java/org/kohsuke/github/GitHubAbuseLimitHandler.java

@@ -0,0 +1,44 @@
+package org.kohsuke.github;
+
+import org.kohsuke.github.connector.GitHubConnectorResponse;
+
+import java.io.IOException;
+import java.net.HttpURLConnection;
+
+import javax.annotation.Nonnull;
+
+/**
+ * Pluggable strategy to determine what to do when the API rate limit is reached.
+ *
+ * @author Kohsuke Kawaguchi
+ * @see GitHubBuilder#withAbuseLimitHandler(AbuseLimitHandler) GitHubBuilder#withRateLimitHandler(AbuseLimitHandler)
+ * @see GitHubRateLimitHandler
+ */
+public abstract class GitHubAbuseLimitHandler extends GitHubConnectorResponseErrorHandler {
+
+    @Override
+    boolean isError(@Nonnull GitHubConnectorResponse connectorResponse) throws IOException {
+        return connectorResponse.statusCode() == HttpURLConnection.HTTP_FORBIDDEN
+                && connectorResponse.header("Retry-After") != null;
+    }
+
+    /**
+     * Called when the library encounters HTTP error indicating that the API abuse limit is reached.
+     *
+     * <p>
+     * Any exception thrown from this method will cause the request to fail, and the caller of github-api will receive
+     * an exception. If this method returns normally, another request will be attempted. For that to make sense, the
+     * implementation needs to wait for some time.
+     *
+     * @param connectorResponse
+     *            Response information for this request.
+     * @throws IOException
+     *             on failure
+     * @see <a href="https://developer.github.com/v3/#abuse-rate-limits">API documentation from GitHub</a>
+     * @see <a href=
+     *      "https://developer.github.com/v3/guides/best-practices-for-integrators/#dealing-with-abuse-rate-limits">Dealing
+     *      with abuse rate limits</a>
+     *
+     */
+    public abstract void onError(@Nonnull GitHubConnectorResponse connectorResponse) throws IOException;
+}

src/main/java/org/kohsuke/github/GitHubBuilder.java

@@ -4,6 +4,7 @@
 import org.kohsuke.github.authorization.AuthorizationProvider;
 import org.kohsuke.github.authorization.ImmutableAuthorizationProvider;
 import org.kohsuke.github.connector.GitHubConnector;
+import org.kohsuke.github.connector.GitHubConnectorResponse;
 import org.kohsuke.github.extras.ImpatientHttpConnector;
 import org.kohsuke.github.internal.GitHubConnectorHttpConnectorAdapter;
 
@@ -34,8 +35,8 @@ public class GitHubBuilder implements Cloneable {
 
     private GitHubConnector connector;
 
-    private RateLimitHandler rateLimitHandler = RateLimitHandler.WAIT;
-    private AbuseLimitHandler abuseLimitHandler = AbuseLimitHandler.WAIT;
+    private GitHubRateLimitHandler rateLimitHandler = RateLimitHandler.WAIT;
+    private GitHubAbuseLimitHandler abuseLimitHandler = AbuseLimitHandler.WAIT;
     private GitHubRateLimitChecker rateLimitChecker = new GitHubRateLimitChecker();
     /* private */ AuthorizationProvider authorizationProvider = AuthorizationProvider.ANONYMOUS;
 
@@ -334,6 +335,7 @@ public GitHubBuilder withJwtToken(String jwtToken) {
      *            the connector
      * @return the git hub builder
      */
+    @Deprecated
     public GitHubBuilder withConnector(@Nonnull HttpConnector connector) {
         return withConnector(GitHubConnectorHttpConnectorAdapter.adapt(connector));
     }
@@ -373,6 +375,32 @@ public GitHubBuilder withConnector(GitHubConnector connector) {
      * @see #withRateLimitChecker(RateLimitChecker)
      */
     public GitHubBuilder withRateLimitHandler(RateLimitHandler handler) {
+        return withRateLimitHandler((GitHubRateLimitHandler) handler);
+    }
+
+    /**
+     * Adds a {@link GitHubRateLimitHandler} to this {@link GitHubBuilder}.
+     * <p>
+     * GitHub allots a certain number of requests to each user or application per period of time (usually per hour). The
+     * number of requests remaining is returned in the response header and can also be requested using
+     * {@link GitHub#getRateLimit()}. This requests per interval is referred to as the "rate limit".
+     * </p>
+     * <p>
+     * When the remaining number of requests reaches zero, the next request will return an error. If this happens,
+     * {@link GitHubRateLimitHandler#onError(GitHubConnectorResponse)} will be called.
+     * </p>
+     * <p>
+     * NOTE: GitHub treats clients that exceed their rate limit very harshly. If possible, clients should avoid
+     * exceeding their rate limit. Consider adding a {@link RateLimitChecker} to automatically check the rate limit for
+     * each request and wait if needed.
+     * </p>
+     *
+     * @param handler
+     *            the handler
+     * @return the git hub builder
+     * @see #withRateLimitChecker(RateLimitChecker)
+     */
+    public GitHubBuilder withRateLimitHandler(GitHubRateLimitHandler handler) {
         this.rateLimitHandler = handler;
         return this;
     }
@@ -389,7 +417,24 @@ public GitHubBuilder withRateLimitHandler(RateLimitHandler handler) {
      *            the handler
      * @return the git hub builder
      */
+    @Deprecated
     public GitHubBuilder withAbuseLimitHandler(AbuseLimitHandler handler) {
+        return withAbuseLimitHandler((GitHubAbuseLimitHandler) handler);
+    }
+
+    /**
+     * Adds a {@link GitHubAbuseLimitHandler} to this {@link GitHubBuilder}.
+     * <p>
+     * When a client sends too many requests in a short time span, GitHub may return an error and set a header telling
+     * the client to not make any more request for some period of time. If this happens,
+     * {@link GitHubAbuseLimitHandler#onError(GitHubConnectorResponse)} will be called.
+     * </p>
+     *
+     * @param handler
+     *            the handler
+     * @return the git hub builder
+     */
+    public GitHubBuilder withAbuseLimitHandler(GitHubAbuseLimitHandler handler) {
         this.abuseLimitHandler = handler;
         return this;
     }