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

Swtich from HttpConnector to GitHubConnector #1290

Merged
merged 12 commits into from
Nov 10, 2021
Merged
23 changes: 22 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,6 @@
</annotationProcessorPaths>
</configuration>
</plugin>

<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<executions>
Expand Down Expand Up @@ -575,6 +574,28 @@
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<executions>
<execution>
<id>okhttp-test</id>
<phase>test</phase>
<goals>
<goal>test</goal>
</goals>
<configuration>
<excludesFile>src/test/resources/slow-or-flaky-tests.txt</excludesFile>
<argLine>@{jacoco.surefire.argLine} ${surefire.argLine} -Dtest.github.connector=okhttp</argLine>
</configuration>
</execution>
<execution>
<id>okhttpconnector-test</id>
<phase>test</phase>
<goals>
<goal>test</goal>
</goals>
<configuration>
<excludesFile>src/test/resources/slow-or-flaky-tests.txt</excludesFile>
<argLine>@{jacoco.surefire.argLine} ${surefire.argLine} -Dtest.github.connector=okhttpconnector</argLine>
</configuration>
</execution>
<execution>
<id>slow-or-flaky-test</id>
<phase>test</phase>
Expand Down
35 changes: 34 additions & 1 deletion src/main/java/org/kohsuke/github/AbuseLimitHandler.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package org.kohsuke.github;

import org.kohsuke.github.connector.GitHubConnectorResponse;

import java.io.IOException;
import java.io.InterruptedIOException;
import java.net.HttpURLConnection;

import javax.annotation.Nonnull;

/**
* Pluggable strategy to determine what to do when the API abuse limit is hit.
*
Expand All @@ -13,6 +17,33 @@
* @see RateLimitHandler
*/
public abstract class AbuseLimitHandler {

/**
* 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 void onError(@Nonnull GitHubConnectorResponse connectorResponse) throws IOException {
GHIOException e = new HttpException("Abuse limit violation",
connectorResponse.statusCode(),
connectorResponse.header("Status"),
connectorResponse.request().url().toString()).withResponseHeaderFields(connectorResponse.allHeaders());
onError(e, connectorResponse.toHttpURLConnection());
}

/**
* Called when the library encounters HTTP error indicating that the API abuse limit is reached.
*
Expand All @@ -34,7 +65,9 @@ public abstract class AbuseLimitHandler {
* with abuse rate limits</a>
*
*/
public abstract void onError(IOException e, HttpURLConnection uc) throws IOException;
@Deprecated
public void onError(IOException e, HttpURLConnection uc) throws IOException {
}

/**
* Wait until the API abuse "wait time" is passed.
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/org/kohsuke/github/GHCompare.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import com.fasterxml.jackson.annotation.JacksonInject;
import com.infradna.tool.bridge_method_injector.WithBridgeMethods;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import org.jetbrains.annotations.NotNull;

import java.io.IOException;
import java.net.URL;
Expand Down Expand Up @@ -174,7 +173,7 @@ public PagedIterable<Commit> listCommits() {
} else {
// if not using paginated commits, adapt the returned commits array
return new PagedIterable<Commit>() {
@NotNull
@Nonnull
@Override
public PagedIterator<Commit> _iterator(int pageSize) {
return new PagedIterator<>(Collections.singleton(commits).iterator(), null);
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/kohsuke/github/GHNotificationStream.java
Original file line number Diff line number Diff line change
Expand Up @@ -192,15 +192,15 @@ GHThread fetch() {
idx = threads.length - 1;

nextCheckTime = calcNextCheckTime(response);
lastModified = response.headerField("Last-Modified");
lastModified = response.header("Last-Modified");
}
} catch (IOException | InterruptedException e) {
throw new RuntimeException(e);
}
}

private long calcNextCheckTime(GitHubResponse<GHThread[]> response) {
String v = response.headerField("X-Poll-Interval");
String v = response.header("X-Poll-Interval");
if (v == null)
v = "60";
long seconds = Integer.parseInt(v);
Expand Down
11 changes: 6 additions & 5 deletions src/main/java/org/kohsuke/github/GHObject.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import org.apache.commons.lang3.builder.ReflectionToStringBuilder;
import org.apache.commons.lang3.builder.ToStringStyle;
import org.kohsuke.github.connector.GitHubConnectorResponse;

import java.io.IOException;
import java.lang.reflect.Field;
Expand Down Expand Up @@ -39,13 +40,13 @@ public abstract class GHObject extends GitHubInteractiveObject {
/**
* Called by Jackson
*
* @param responseInfo
* the {@link GitHubResponse.ResponseInfo} to get headers from.
* @param connectorResponse
* the {@link GitHubConnectorResponse} to get headers from.
*/
@JacksonInject
protected void setResponseHeaderFields(@CheckForNull GitHubResponse.ResponseInfo responseInfo) {
if (responseInfo != null) {
responseHeaderFields = responseInfo.headers();
protected void setResponseHeaderFields(@CheckForNull GitHubConnectorResponse connectorResponse) {
if (connectorResponse != null) {
responseHeaderFields = connectorResponse.allHeaders();
}
}

Expand Down
9 changes: 5 additions & 4 deletions src/main/java/org/kohsuke/github/GHRateLimit.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.fasterxml.jackson.annotation.JsonProperty;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import org.apache.commons.lang3.StringUtils;
import org.kohsuke.github.connector.GitHubConnectorResponse;

import java.time.Duration;
import java.time.ZonedDateTime;
Expand Down Expand Up @@ -436,20 +437,20 @@ public Record(@JsonProperty(value = "limit", required = true) int limit,
* the remaining
* @param resetEpochSeconds
* the reset epoch seconds
* @param responseInfo
* @param connectorResponse
* the response info
*/
@JsonCreator
Record(@JsonProperty(value = "limit", required = true) int limit,
@JsonProperty(value = "remaining", required = true) int remaining,
@JsonProperty(value = "reset", required = true) long resetEpochSeconds,
@JacksonInject @CheckForNull GitHubResponse.ResponseInfo responseInfo) {
@JacksonInject @CheckForNull GitHubConnectorResponse connectorResponse) {
this.limit = limit;
this.remaining = remaining;
this.resetEpochSeconds = resetEpochSeconds;
String updatedAt = null;
if (responseInfo != null) {
updatedAt = responseInfo.headerField("Date");
if (connectorResponse != null) {
updatedAt = connectorResponse.header("Date");
}
this.resetDate = calculateResetDate(updatedAt);
}
Expand Down
15 changes: 10 additions & 5 deletions src/main/java/org/kohsuke/github/GitHub.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import org.kohsuke.github.authorization.AuthorizationProvider;
import org.kohsuke.github.authorization.ImmutableAuthorizationProvider;
import org.kohsuke.github.authorization.UserAuthorizationProvider;
import org.kohsuke.github.connector.GitHubConnector;
import org.kohsuke.github.internal.GitHubConnectorHttpConnectorAdapter;
import org.kohsuke.github.internal.Previews;

import java.io.*;
Expand Down Expand Up @@ -110,7 +112,7 @@ public class GitHub {
* a authorization provider
*/
GitHub(String apiUrl,
HttpConnector connector,
GitHubConnector connector,
RateLimitHandler rateLimitHandler,
AbuseLimitHandler abuseLimitHandler,
GitHubRateLimitChecker rateLimitChecker,
Expand All @@ -129,7 +131,7 @@ public class GitHub {
users = new ConcurrentHashMap<>();
orgs = new ConcurrentHashMap<>();

this.client = new GitHubHttpUrlConnectionClient(apiUrl,
this.client = new GitHubClient(apiUrl,
connector,
rateLimitHandler,
abuseLimitHandler,
Expand Down Expand Up @@ -441,7 +443,7 @@ public static GitHub connectToEnterpriseAnonymously(String apiUrl) throws IOExce
public static GitHub offline() {
try {
return new GitHubBuilder().withEndpoint("https://api.github.invalid")
.withConnector(HttpConnector.OFFLINE)
.withConnector(GitHubConnector.OFFLINE)
.build();
} catch (IOException e) {
throw new IllegalStateException("The offline implementation constructor should not connect", e);
Expand Down Expand Up @@ -470,7 +472,10 @@ public boolean isOffline() {
* Gets connector.
*
* @return the connector
* @deprecated HttpConnector has been replaced by GitHubConnector which is generally not useful outside of this
* library. If you are using this method, file an issue describing your use case.
*/
@Deprecated
public HttpConnector getConnector() {
return client.getConnector();
}
Expand All @@ -483,8 +488,8 @@ public HttpConnector getConnector() {
* @deprecated HttpConnector should not be changed. If you find yourself needing to do this, file an issue.
*/
@Deprecated
public void setConnector(HttpConnector connector) {
client.setConnector(connector);
public void setConnector(@Nonnull HttpConnector connector) {
client.setConnector(GitHubConnectorHttpConnectorAdapter.adapt(connector));
}

/**
Expand Down
17 changes: 15 additions & 2 deletions src/main/java/org/kohsuke/github/GitHubBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
import org.apache.commons.io.IOUtils;
import org.kohsuke.github.authorization.AuthorizationProvider;
import org.kohsuke.github.authorization.ImmutableAuthorizationProvider;
import org.kohsuke.github.connector.GitHubConnector;
import org.kohsuke.github.extras.ImpatientHttpConnector;
import org.kohsuke.github.internal.GitHubConnectorHttpConnectorAdapter;

import java.io.File;
import java.io.FileInputStream;
Expand All @@ -30,7 +32,7 @@ public class GitHubBuilder implements Cloneable {
// default scoped so unit tests can read them.
/* private */ String endpoint = GitHubClient.GITHUB_URL;

private HttpConnector connector;
private GitHubConnector connector;

private RateLimitHandler rateLimitHandler = RateLimitHandler.WAIT;
private AbuseLimitHandler abuseLimitHandler = AbuseLimitHandler.WAIT;
Expand Down Expand Up @@ -332,7 +334,18 @@ public GitHubBuilder withJwtToken(String jwtToken) {
* the connector
* @return the git hub builder
*/
public GitHubBuilder withConnector(HttpConnector connector) {
public GitHubBuilder withConnector(@Nonnull HttpConnector connector) {
return withConnector(GitHubConnectorHttpConnectorAdapter.adapt(connector));
}

/**
* With connector git hub builder.
*
* @param connector
* the connector
* @return the git hub builder
*/
public GitHubBuilder withConnector(GitHubConnector connector) {
this.connector = connector;
return this;
}
Expand Down
Loading