diff --git a/src/main/java/org/kohsuke/github/GitHub.java b/src/main/java/org/kohsuke/github/GitHub.java index 5d613fe985..df9353e032 100644 --- a/src/main/java/org/kohsuke/github/GitHub.java +++ b/src/main/java/org/kohsuke/github/GitHub.java @@ -23,12 +23,10 @@ */ package org.kohsuke.github; -import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.ANY; -import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.NONE; -import static java.util.logging.Level.FINE; -import static java.net.HttpURLConnection.HTTP_UNAUTHORIZED; -import static org.kohsuke.github.Previews.DRAX; - +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.introspect.VisibilityChecker.Std; +import com.infradna.tool.bridge_method_injector.WithBridgeMethods; import java.io.ByteArrayInputStream; import java.io.FileNotFoundException; import java.io.IOException; @@ -49,17 +47,19 @@ import java.util.Map; import java.util.Set; import java.util.TimeZone; - +import java.util.concurrent.TimeUnit; +import java.util.logging.Level; +import java.util.logging.Logger; +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; import org.apache.commons.codec.Charsets; import org.apache.commons.codec.binary.Base64; -import com.fasterxml.jackson.databind.DeserializationFeature; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.introspect.VisibilityChecker.Std; -import com.infradna.tool.bridge_method_injector.WithBridgeMethods; - -import javax.annotation.Nonnull; -import java.util.logging.Logger; +import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.ANY; +import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.NONE; +import static java.net.HttpURLConnection.HTTP_UNAUTHORIZED; +import static java.util.logging.Level.FINE; +import static org.kohsuke.github.Previews.DRAX; /** * Root of the GitHub API. @@ -90,6 +90,10 @@ public class GitHub { private HttpConnector connector = HttpConnector.DEFAULT; + private final Object headerRateLimitLock = new Object(); + private GHRateLimit headerRateLimit = null; + private volatile GHRateLimit rateLimit = null; + /** * Creates a client API root object. * @@ -254,6 +258,10 @@ public HttpConnector getConnector() { return connector; } + public String getApiUrl() { + return apiUrl; + } + /** * Sets the custom connector used to make requests to GitHub. */ @@ -287,17 +295,61 @@ public void setConnector(HttpConnector connector) { */ public GHRateLimit getRateLimit() throws IOException { try { - return retrieve().to("/rate_limit", JsonRateLimit.class).rate; + return rateLimit = retrieve().to("/rate_limit", JsonRateLimit.class).rate; } catch (FileNotFoundException e) { // GitHub Enterprise doesn't have the rate limit, so in that case // return some big number that's not too big. // see issue #78 GHRateLimit r = new GHRateLimit(); r.limit = r.remaining = 1000000; - long hours = 1000L * 60 * 60; - r.reset = new Date(System.currentTimeMillis() + 1 * hours ); - return r; + long hour = 60L * 60L; // this is madness, storing the date as seconds in a Date object + r.reset = new Date((System.currentTimeMillis() + hour) / 1000L ); + return rateLimit = r; + } + } + + /*package*/ void updateRateLimit(@Nonnull GHRateLimit observed) { + synchronized (headerRateLimitLock) { + if (headerRateLimit == null + || headerRateLimit.getResetDate().getTime() < observed.getResetDate().getTime() + || headerRateLimit.remaining > observed.remaining) { + headerRateLimit = observed; + LOGGER.log(Level.INFO, "Rate limit now: {0}", headerRateLimit); + } + } + } + + /** + * Returns the most recently observed rate limit data or {@code null} if either there is no rate limit + * (for example GitHub Enterprise) or if no requests have been made. + * + * @return the most recently observed rate limit data or {@code null}. + */ + @CheckForNull + public GHRateLimit lastRateLimit() { + synchronized (headerRateLimitLock) { + return headerRateLimit; + } + } + + /** + * Gets the current rate limit while trying not to actually make any remote requests unless absolutely necessary. + * + * @return the current rate limit data. + * @throws IOException if we couldn't get the current rate limit data. + */ + @Nonnull + public GHRateLimit rateLimit() throws IOException { + synchronized (headerRateLimitLock) { + if (headerRateLimit != null) { + return headerRateLimit; + } + } + GHRateLimit rateLimit = this.rateLimit; + if (rateLimit == null || rateLimit.getResetDate().getTime() < System.currentTimeMillis()) { + rateLimit = getRateLimit(); } + return rateLimit; } /** diff --git a/src/main/java/org/kohsuke/github/Requester.java b/src/main/java/org/kohsuke/github/Requester.java index 70c48940c0..21b3afd150 100644 --- a/src/main/java/org/kohsuke/github/Requester.java +++ b/src/main/java/org/kohsuke/github/Requester.java @@ -25,8 +25,6 @@ import com.fasterxml.jackson.databind.JsonMappingException; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -import org.apache.commons.io.IOUtils; - import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; @@ -42,6 +40,7 @@ import java.net.URLEncoder; import java.util.ArrayList; import java.util.Collection; +import java.util.Date; import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; @@ -49,16 +48,18 @@ import java.util.Locale; import java.util.Map; import java.util.NoSuchElementException; +import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.zip.GZIPInputStream; - import javax.annotation.WillClose; +import org.apache.commons.io.IOUtils; +import org.apache.commons.lang.StringUtils; import static java.util.Arrays.asList; import static java.util.logging.Level.FINE; -import static org.kohsuke.github.GitHub.*; +import static org.kohsuke.github.GitHub.MAPPER; /** * A builder pattern for making HTTP call and parsing its output. @@ -281,6 +282,8 @@ private T _to(String tailApiUrl, Class type, T instance) throws IOExcepti return result; } catch (IOException e) { handleApiError(e); + } finally { + noteRateLimit(tailApiUrl); } } } @@ -299,6 +302,8 @@ public int asHttpStatusCode(String tailApiUrl) throws IOException { return uc.getResponseCode(); } catch (IOException e) { handleApiError(e); + } finally { + noteRateLimit(tailApiUrl); } } } @@ -313,6 +318,59 @@ public InputStream asStream(String tailApiUrl) throws IOException { return wrapStream(uc.getInputStream()); } catch (IOException e) { handleApiError(e); + } finally { + noteRateLimit(tailApiUrl); + } + } + } + + private void noteRateLimit(String tailApiUrl) { + if ("/rate_limit".equals(tailApiUrl)) { + // the rate_limit API is "free" + return; + } + if (tailApiUrl.startsWith("/search")) { + // the search API uses a different rate limit + return; + } + String limit = uc.getHeaderField("X-RateLimit-Limit"); + if (StringUtils.isBlank(limit)) { + // if we are missing a header, return fast + return; + } + String remaining = uc.getHeaderField("X-RateLimit-Remaining"); + if (StringUtils.isBlank(remaining)) { + // if we are missing a header, return fast + return; + } + String reset = uc.getHeaderField("X-RateLimit-Reset"); + if (StringUtils.isBlank(reset)) { + // if we are missing a header, return fast + return; + } + GHRateLimit observed = new GHRateLimit(); + try { + observed.limit = Integer.parseInt(limit); + } catch (NumberFormatException e) { + if (LOGGER.isLoggable(Level.FINEST)) { + LOGGER.log(Level.FINEST, "Malformed X-RateLimit-Limit header value " + limit, e); + } + return; + } + try { + observed.remaining = Integer.parseInt(remaining); + } catch (NumberFormatException e) { + if (LOGGER.isLoggable(Level.FINEST)) { + LOGGER.log(Level.FINEST, "Malformed X-RateLimit-Remaining header value " + remaining, e); + } + return; + } + try { + observed.reset = new Date(Long.parseLong(reset)); // this is madness, storing the date as seconds + root.updateRateLimit(observed); + } catch (NumberFormatException e) { + if (LOGGER.isLoggable(Level.FINEST)) { + LOGGER.log(Level.FINEST, "Malformed X-RateLimit-Reset header value " + reset, e); } } } @@ -382,7 +440,7 @@ private boolean isMethodWithBody() { } try { - return new PagingIterator(type, root.getApiURL(s.toString())); + return new PagingIterator(type, tailApiUrl, root.getApiURL(s.toString())); } catch (IOException e) { throw new Error(e); } @@ -391,6 +449,7 @@ private boolean isMethodWithBody() { class PagingIterator implements Iterator { private final Class type; + private final String tailApiUrl; /** * The next batch to be returned from {@link #next()}. @@ -402,9 +461,10 @@ class PagingIterator implements Iterator { */ private URL url; - PagingIterator(Class type, URL url) { - this.url = url; + PagingIterator(Class type, String tailApiUrl, URL url) { this.type = type; + this.tailApiUrl = tailApiUrl; + this.url = url; } public boolean hasNext() { @@ -438,6 +498,8 @@ private void fetch() { return; } catch (IOException e) { handleApiError(e); + } finally { + noteRateLimit(tailApiUrl); } } } catch (IOException e) {