Skip to content

Commit

Permalink
Merge pull request hub4j#1290 from bitwiseman/task/response-to-urlcon…
Browse files Browse the repository at this point in the history
…nection

Swtich from HttpConnector to GitHubConnector
  • Loading branch information
bitwiseman authored Nov 10, 2021
2 parents 0601578 + 056d1f0 commit 1535817
Show file tree
Hide file tree
Showing 51 changed files with 2,025 additions and 658 deletions.
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

0 comments on commit 1535817

Please sign in to comment.