Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit abf5a6d

Browse filesBrowse files
bitwisemanbitwiseman
authored
Merge pull request hub4j#1314 from bitwiseman/task/java11default
Make HttpClientGitHubConnector the default for Java 11+
2 parents 65d6de2 + 6c21554 commit abf5a6d
Copy full SHA for abf5a6d

File tree

Expand file treeCollapse file tree

7 files changed

+49
-28
lines changed
Filter options
Expand file treeCollapse file tree

7 files changed

+49
-28
lines changed

‎pom.xml

Copy file name to clipboardExpand all lines: pom.xml
+13Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -825,6 +825,19 @@
825825
<argLine>@{jacoco.surefire.argLine} ${surefire.argLine} -Dtest.github.connector=httpclient</argLine>
826826
</configuration>
827827
</execution>
828+
<execution>
829+
<id>java11-urlconnection-test</id>
830+
<phase>integration-test</phase>
831+
<goals>
832+
<goal>test</goal>
833+
</goals>
834+
<configuration>
835+
<classesDirectory>${project.basedir}/target/github-api-${project.version}.jar</classesDirectory>
836+
<useSystemClassLoader>false</useSystemClassLoader>
837+
<excludesFile>src/test/resources/slow-or-flaky-tests.txt</excludesFile>
838+
<argLine>@{jacoco.surefire.argLine} ${surefire.argLine} -Dtest.github.connector=urlconnection</argLine>
839+
</configuration>
840+
</execution>
828841
</executions>
829842
</plugin>
830843
</plugins>

‎src/main/java/org/kohsuke/github/GitHubClient.java

Copy file name to clipboardExpand all lines: src/main/java/org/kohsuke/github/GitHubClient.java
+16-3Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@
2525

2626
import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.ANY;
2727
import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.NONE;
28-
import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
29-
import static java.net.HttpURLConnection.HTTP_UNAUTHORIZED;
28+
import static java.net.HttpURLConnection.*;
3029
import static java.util.logging.Level.*;
3130
import static org.apache.commons.lang3.StringUtils.defaultString;
3231

@@ -392,7 +391,7 @@ public <T> GitHubResponse<T> sendRequest(GitHubRequest request, @CheckForNull Bo
392391
connectorRequest = e.connectorRequest;
393392
}
394393
} catch (SocketException | SocketTimeoutException | SSLHandshakeException e) {
395-
// These transient errors the
394+
// These transient errors thrown by HttpURLConnection
396395
if (retries > 0) {
397396
logRetryConnectionError(e, request.url(), retries);
398397
continue;
@@ -413,6 +412,7 @@ private void detectKnownErrors(GitHubConnectorResponse connectorResponse,
413412
boolean detectStatusCodeError) throws IOException {
414413
detectOTPRequired(connectorResponse);
415414
detectInvalidCached404Response(connectorResponse, request);
415+
detectRedirect(connectorResponse);
416416
if (rateLimitHandler.isError(connectorResponse)) {
417417
rateLimitHandler.onError(connectorResponse);
418418
throw new RetryRequestException();
@@ -425,6 +425,19 @@ private void detectKnownErrors(GitHubConnectorResponse connectorResponse,
425425
}
426426
}
427427

428+
private void detectRedirect(GitHubConnectorResponse connectorResponse) throws IOException {
429+
if (connectorResponse.statusCode() == HTTP_MOVED_PERM || connectorResponse.statusCode() == HTTP_MOVED_TEMP) {
430+
// GitHubClient depends on GitHubConnector implementations to follow any redirects automatically
431+
// If this is not done and a redirect is requested, throw in order to maintain security and consistency
432+
throw new HttpException(
433+
"GitHubConnnector did not automatically follow redirect.\n"
434+
+ "Change your http client configuration to automatically follow redirects as appropriate.",
435+
connectorResponse.statusCode(),
436+
"Redirect",
437+
connectorResponse.request().url().toString());
438+
}
439+
}
440+
428441
private GitHubConnectorRequest prepareConnectorRequest(GitHubRequest request) throws IOException {
429442
GitHubRequest.Builder<?> builder = request.toBuilder();
430443
// if the authentication is needed but no credential is given, try it anyway (so that some calls

‎src/main/java/org/kohsuke/github/internal/DefaultGitHubConnector.java

Copy file name to clipboardExpand all lines: src/main/java/org/kohsuke/github/internal/DefaultGitHubConnector.java
+5-5Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,11 @@ static GitHubConnector create(String defaultConnectorProperty) {
5050
} else if (defaultConnectorProperty.equalsIgnoreCase("httpclient")) {
5151
return new HttpClientGitHubConnector();
5252
} else if (defaultConnectorProperty.equalsIgnoreCase("default")) {
53-
// try {
54-
// return new HttpClientGitHubConnector();
55-
// } catch (UnsupportedOperationException | LinkageError e) {
56-
return new GitHubConnectorHttpConnectorAdapter(HttpConnector.DEFAULT);
57-
// }
53+
try {
54+
return new HttpClientGitHubConnector();
55+
} catch (UnsupportedOperationException | LinkageError e) {
56+
return new GitHubConnectorHttpConnectorAdapter(HttpConnector.DEFAULT);
57+
}
5858
} else {
5959
throw new IllegalStateException(
6060
"Property 'test.github.connector' must reference a valid built-in connector - okhttp, okhttpconnector, urlconnection, or default.");

‎src/main/java11/org/kohsuke/github/extras/HttpClientGitHubConnector.java

Copy file name to clipboardExpand all lines: src/main/java11/org/kohsuke/github/extras/HttpClientGitHubConnector.java
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ public class HttpClientGitHubConnector implements GitHubConnector {
2828
private final HttpClient client;
2929

3030
/**
31-
* Instantiates a new HttpClientGitHubConnector with a defaut HttpClient.
31+
* Instantiates a new HttpClientGitHubConnector with a default HttpClient.
3232
*/
3333
public HttpClientGitHubConnector() {
34-
this(HttpClient.newHttpClient());
34+
this(HttpClient.newBuilder().followRedirects(HttpClient.Redirect.NORMAL).build());
3535
}
3636

3737
/**

‎src/test/java/org/kohsuke/github/GHWorkflowRunTest.java

Copy file name to clipboardExpand all lines: src/test/java/org/kohsuke/github/GHWorkflowRunTest.java
-15Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
11
package org.kohsuke.github;
22

33
import org.awaitility.Awaitility;
4-
import org.junit.Assume;
54
import org.junit.Before;
65
import org.junit.Test;
76
import org.kohsuke.github.GHWorkflowJob.Step;
87
import org.kohsuke.github.GHWorkflowRun.Conclusion;
98
import org.kohsuke.github.GHWorkflowRun.Status;
10-
import org.kohsuke.github.extras.HttpClientGitHubConnector;
119
import org.kohsuke.github.function.InputStreamFunction;
12-
import org.kohsuke.github.internal.DefaultGitHubConnector;
1310

1411
import java.io.IOException;
1512
import java.time.Duration;
@@ -201,10 +198,6 @@ public void testSearchOnBranch() throws IOException {
201198

202199
@Test
203200
public void testLogs() throws IOException {
204-
// This test fails for HttpClientGitHubConnector.
205-
// Not sure why, but it is better to move forward and come back to it later
206-
Assume.assumeFalse(DefaultGitHubConnector.create() instanceof HttpClientGitHubConnector);
207-
208201
GHWorkflow workflow = repo.getWorkflow(FAST_WORKFLOW_PATH);
209202

210203
long latestPreexistingWorkflowRunId = getLatestPreexistingWorkflowRunId();
@@ -243,10 +236,6 @@ public void testLogs() throws IOException {
243236
@SuppressWarnings("resource")
244237
@Test
245238
public void testArtifacts() throws IOException {
246-
// This test fails for HttpClientGitHubConnector.
247-
// Not sure why, but it is better to move forward and come back to it later
248-
Assume.assumeFalse(DefaultGitHubConnector.create() instanceof HttpClientGitHubConnector);
249-
250239
GHWorkflow workflow = repo.getWorkflow(ARTIFACTS_WORKFLOW_PATH);
251240

252241
long latestPreexistingWorkflowRunId = getLatestPreexistingWorkflowRunId();
@@ -327,10 +316,6 @@ public void testArtifacts() throws IOException {
327316

328317
@Test
329318
public void testJobs() throws IOException {
330-
// This test fails for HttpClientGitHubConnector.
331-
// Not sure why, but it is better to move forward and come back to it later
332-
Assume.assumeFalse(DefaultGitHubConnector.create() instanceof HttpClientGitHubConnector);
333-
334319
GHWorkflow workflow = repo.getWorkflow(MULTI_JOBS_WORKFLOW_PATH);
335320

336321
long latestPreexistingWorkflowRunId = getLatestPreexistingWorkflowRunId();

‎src/test/java/org/kohsuke/github/RequesterRetryTest.java

Copy file name to clipboardExpand all lines: src/test/java/org/kohsuke/github/RequesterRetryTest.java
+13-1Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,14 @@
44
import com.github.tomakehurst.wiremock.stubbing.Scenario;
55
import okhttp3.ConnectionPool;
66
import okhttp3.OkHttpClient;
7+
import org.junit.Assume;
78
import org.junit.Before;
89
import org.junit.Ignore;
910
import org.junit.Test;
11+
import org.kohsuke.github.extras.HttpClientGitHubConnector;
1012
import org.kohsuke.github.extras.ImpatientHttpConnector;
1113
import org.kohsuke.github.extras.okhttp3.OkHttpGitHubConnector;
14+
import org.kohsuke.github.internal.DefaultGitHubConnector;
1215
import org.mockito.Mockito;
1316

1417
import java.io.ByteArrayOutputStream;
@@ -78,7 +81,7 @@ public void resetTestCapturedLog() throws IOException {
7881
attachLogCapturer();
7982
}
8083

81-
@Ignore("Used okhttp3 and this to verify connection closing. To variable for CI system.")
84+
@Ignore("Used okhttp3 and this to verify connection closing. Too flaky for CI system.")
8285
@Test
8386
public void testGitHubIsApiUrlValid() throws Exception {
8487

@@ -109,6 +112,9 @@ public void testGitHubIsApiUrlValid() throws Exception {
109112
// Issue #539
110113
@Test
111114
public void testSocketConnectionAndRetry() throws Exception {
115+
// Only implemented for HttpURLConnection connectors
116+
Assume.assumeThat(DefaultGitHubConnector.create(), not(instanceOf(HttpClientGitHubConnector.class)));
117+
112118
// CONNECTION_RESET_BY_PEER errors result in two requests each
113119
// to get this failure for "3" tries we have to do 6 queries.
114120
this.mockGitHub.apiServer()
@@ -136,6 +142,9 @@ public void testSocketConnectionAndRetry() throws Exception {
136142
// Issue #539
137143
@Test
138144
public void testSocketConnectionAndRetry_StatusCode() throws Exception {
145+
// Only implemented for HttpURLConnection connectors
146+
Assume.assumeThat(DefaultGitHubConnector.create(), not(instanceOf(HttpClientGitHubConnector.class)));
147+
139148
// CONNECTION_RESET_BY_PEER errors result in two requests each
140149
// to get this failure for "3" tries we have to do 6 queries.
141150
this.mockGitHub.apiServer()
@@ -164,6 +173,9 @@ public void testSocketConnectionAndRetry_StatusCode() throws Exception {
164173

165174
@Test
166175
public void testSocketConnectionAndRetry_Success() throws Exception {
176+
// Only implemented for HttpURLConnection connectors
177+
Assume.assumeThat(DefaultGitHubConnector.create(), not(instanceOf(HttpClientGitHubConnector.class)));
178+
167179
// CONNECTION_RESET_BY_PEER errors result in two requests each
168180
// to get this failure for "3" tries we have to do 6 queries.
169181
// If there are only 5 errors we succeed.

‎src/test/java/org/kohsuke/github/internal/DefaultGitHubConnectorTest.java

Copy file name to clipboardExpand all lines: src/test/java/org/kohsuke/github/internal/DefaultGitHubConnectorTest.java
-2Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,6 @@ public void testCreate() throws Exception {
3737

3838
connector = DefaultGitHubConnector.create("default");
3939

40-
// Current implementation never uses httpclient for default.
41-
usingHttpClient = false;
4240
if (usingHttpClient) {
4341
assertThat(connector, instanceOf(HttpClientGitHubConnector.class));
4442
} else {

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.