Skip to content

Commit 55a2b24

Browse files
bitwisemanbitwiseman
committed
Further clean up and coverage
This is why we test.
1 parent ac9c0ad commit 55a2b24

17 files changed

+588
-132
lines changed

‎pom.xml‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,15 +162,13 @@
162162

163163

164164
<!-- Deprecated -->
165-
<exclude>org.kohsuke.github.extras.okhttp3.OkHttpConnector</exclude>
166-
<exclude>org.kohsuke.github.extras.OkHttp3Connector</exclude>
165+
<exclude>org.kohsuke.github.extras.OkHttpConnector</exclude>
167166
<exclude>org.kohsuke.github.extras.OkHttp3Connector</exclude>
168167
<exclude>org.kohsuke.github.EnforcementLevel</exclude>
169168
<exclude>org.kohsuke.github.GHPerson.1</exclude>
170169
<exclude>org.kohsuke.github.GHCompare.User</exclude>
171170

172171
<!-- TODO: Some coverage, but more needed -->
173-
<exclude>org.kohsuke.github.GitHubConnectorResponseHttpUrlConnectionAdapter</exclude>
174172
<exclude>org.kohsuke.github.GHPullRequestReviewBuilder.DraftReviewComment</exclude>
175173
<exclude>org.kohsuke.github.GHIssue.PullRequest</exclude>
176174
<exclude>org.kohsuke.github.GHCommitSearchBuilder</exclude>
@@ -587,6 +585,17 @@
587585
<argLine>@{jacoco.surefire.argLine} ${surefire.argLine} -Dtest.github.connector=okhttp</argLine>
588586
</configuration>
589587
</execution>
588+
<execution>
589+
<id>okhttpconnector-test</id>
590+
<phase>test</phase>
591+
<goals>
592+
<goal>test</goal>
593+
</goals>
594+
<configuration>
595+
<excludesFile>src/test/resources/slow-or-flaky-tests.txt</excludesFile>
596+
<argLine>@{jacoco.surefire.argLine} ${surefire.argLine} -Dtest.github.connector=okhttpconnector</argLine>
597+
</configuration>
598+
</execution>
590599
<execution>
591600
<id>slow-or-flaky-test</id>
592601
<phase>test</phase>

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package org.kohsuke.github;
22

33
import org.kohsuke.github.connector.GitHubConnectorResponse;
4+
import org.kohsuke.github.internal.GitHubConnectorHttpConnectorAdapter;
45

56
import java.io.IOException;
67
import java.io.InterruptedIOException;
@@ -41,7 +42,14 @@ public void onError(@Nonnull GitHubConnectorResponse connectorResponse) throws I
4142
connectorResponse.statusCode(),
4243
connectorResponse.header("Status"),
4344
connectorResponse.request().url().toString()).withResponseHeaderFields(connectorResponse.allHeaders());
44-
onError(e, new GitHubConnectorResponseHttpUrlConnectionAdapter(connectorResponse));
45+
HttpURLConnection connection;
46+
if (connectorResponse instanceof GitHubConnectorHttpConnectorAdapter.HttpURLConnectionGitHubConnectorResponse) {
47+
connection = ((GitHubConnectorHttpConnectorAdapter.HttpURLConnectionGitHubConnectorResponse) connectorResponse)
48+
.getConnection();
49+
} else {
50+
connection = new GitHubConnectorResponseHttpUrlConnectionAdapter(connectorResponse);
51+
}
52+
onError(e, connection);
4553
}
4654

4755
/**

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import com.fasterxml.jackson.annotation.JacksonInject;
44
import com.infradna.tool.bridge_method_injector.WithBridgeMethods;
55
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
6-
import org.jetbrains.annotations.NotNull;
76

87
import java.io.IOException;
98
import java.net.URL;
@@ -174,7 +173,7 @@ public PagedIterable<Commit> listCommits() {
174173
} else {
175174
// if not using paginated commits, adapt the returned commits array
176175
return new PagedIterable<Commit>() {
177-
@NotNull
176+
@Nonnull
178177
@Override
179178
public PagedIterator<Commit> _iterator(int pageSize) {
180179
return new PagedIterator<>(Collections.singleton(commits).iterator(), null);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@ public HttpConnector getConnector() {
488488
* @deprecated HttpConnector should not be changed. If you find yourself needing to do this, file an issue.
489489
*/
490490
@Deprecated
491-
public void setConnector(HttpConnector connector) {
491+
public void setConnector(@Nonnull HttpConnector connector) {
492492
client.setConnector(GitHubConnectorHttpConnectorAdapter.adapt(connector));
493493
}
494494

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ public GitHubBuilder withJwtToken(String jwtToken) {
334334
* the connector
335335
* @return the git hub builder
336336
*/
337-
public GitHubBuilder withConnector(HttpConnector connector) {
337+
public GitHubBuilder withConnector(@Nonnull HttpConnector connector) {
338338
return withConnector(GitHubConnectorHttpConnectorAdapter.adapt(connector));
339339
}
340340

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ public String getRequestProperty(String key) {
280280

281281
@Override
282282
public Map<String, List<String>> getRequestProperties() {
283-
return connectorResponse.request().allHeaders();
283+
throw new IllegalStateException("Already connected");
284284
}
285285

286286
@Override

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package org.kohsuke.github;
22

33
import org.kohsuke.github.connector.GitHubConnectorResponse;
4+
import org.kohsuke.github.internal.GitHubConnectorHttpConnectorAdapter;
45

56
import java.io.IOException;
67
import java.io.InterruptedIOException;
@@ -37,7 +38,14 @@ public void onError(@Nonnull GitHubConnectorResponse connectorResponse) throws I
3738
connectorResponse.statusCode(),
3839
connectorResponse.header("Status"),
3940
connectorResponse.request().url().toString()).withResponseHeaderFields(connectorResponse.allHeaders());
40-
onError(e, new GitHubConnectorResponseHttpUrlConnectionAdapter(connectorResponse));
41+
HttpURLConnection connection;
42+
if (connectorResponse instanceof GitHubConnectorHttpConnectorAdapter.HttpURLConnectionGitHubConnectorResponse) {
43+
connection = ((GitHubConnectorHttpConnectorAdapter.HttpURLConnectionGitHubConnectorResponse) connectorResponse)
44+
.getConnection();
45+
} else {
46+
connection = new GitHubConnectorResponseHttpUrlConnectionAdapter(connectorResponse);
47+
}
48+
onError(e, connection);
4149
}
4250

4351
/**

‎src/main/java/org/kohsuke/github/extras/okhttp3/OkHttpGitHubConnector.java‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@
77
import org.kohsuke.github.connector.GitHubConnectorRequest;
88
import org.kohsuke.github.connector.GitHubConnectorResponse;
99

10+
import java.io.ByteArrayInputStream;
1011
import java.io.FileNotFoundException;
1112
import java.io.IOException;
1213
import java.io.InputStream;
14+
import java.nio.charset.StandardCharsets;
1315
import java.util.Arrays;
1416
import java.util.List;
1517
import java.util.Map;
@@ -112,6 +114,9 @@ private List<ConnectionSpec> TlsConnectionSpecs() {
112114
*/
113115
static class OkHttpGitHubConnectorResponse extends GitHubConnectorResponse {
114116

117+
private boolean bodyBytesRead = false;
118+
private byte[] bodyBytes = null;
119+
115120
@Nonnull
116121
private final Response response;
117122

@@ -135,9 +140,27 @@ public InputStream bodyStream() throws IOException {
135140
}
136141
}
137142

138-
ResponseBody body = response.body();
139-
InputStream bytes = body != null ? body.byteStream() : null;
140-
return wrapStream(bytes);
143+
readBodyBytes();
144+
InputStream stream = bodyBytes == null ? null : new ByteArrayInputStream(bodyBytes);
145+
return stream;
146+
}
147+
148+
private void readBodyBytes() throws IOException {
149+
synchronized (this) {
150+
if (!bodyBytesRead) {
151+
try (ResponseBody body = response.body()) {
152+
if (body != null) {
153+
try (InputStream stream = wrapStream(body.byteStream())) {
154+
if (stream != null) {
155+
bodyBytes = IOUtils.toByteArray(stream);
156+
}
157+
}
158+
}
159+
}
160+
bodyBytesRead = true;
161+
}
162+
163+
}
141164
}
142165

143166
/**
@@ -147,8 +170,8 @@ public String errorMessage() {
147170
String result = null;
148171
try {
149172
if (!response.isSuccessful()) {
150-
ResponseBody body = response.body();
151-
result = body != null ? body.string() : null;
173+
readBodyBytes();
174+
result = bodyBytes == null ? null : new String(bodyBytes, StandardCharsets.UTF_8);
152175
}
153176
} catch (Exception e) {
154177
LOGGER.log(FINER, "Ignored exception get error message", e);

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import okhttp3.OkHttpClient;
44
import org.kohsuke.github.HttpConnector;
55
import org.kohsuke.github.connector.GitHubConnector;
6+
import org.kohsuke.github.extras.okhttp3.OkHttpConnector;
67
import org.kohsuke.github.extras.okhttp3.OkHttpGitHubConnector;
78

89
/**
@@ -29,15 +30,21 @@ public class DefaultGitHubConnector {
2930
*/
3031
public static GitHubConnector create() {
3132
String defaultConnectorProperty = System.getProperty("test.github.connector", "default");
33+
return create(defaultConnectorProperty);
34+
}
35+
36+
static GitHubConnector create(String defaultConnectorProperty) {
37+
3238
if (defaultConnectorProperty.equalsIgnoreCase("okhttp")) {
3339
return new OkHttpGitHubConnector(new OkHttpClient.Builder().build());
34-
} else if (defaultConnectorProperty.equalsIgnoreCase("httpconnector")) {
35-
return new GitHubConnectorHttpConnectorAdapter(HttpConnector.DEFAULT);
36-
} else if (defaultConnectorProperty.equalsIgnoreCase("default")) {
40+
} else if (defaultConnectorProperty.equalsIgnoreCase("okhttpconnector")) {
41+
return new GitHubConnectorHttpConnectorAdapter(new OkHttpConnector(new OkHttpClient.Builder().build()));
42+
} else if (defaultConnectorProperty.equalsIgnoreCase("urlconnection")
43+
|| defaultConnectorProperty.equalsIgnoreCase("default")) {
3744
return new GitHubConnectorHttpConnectorAdapter(HttpConnector.DEFAULT);
3845
} else {
3946
throw new IllegalStateException(
40-
"Property 'test.github.connector' must reference a valid built-in connector - okhttp, httpconnector, or default.");
47+
"Property 'test.github.connector' must reference a valid built-in connector - okhttp, okhttpconnector, urlconnection, or default.");
4148
}
4249
}
4350
}

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

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
package org.kohsuke.github.internal;
22

3+
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
34
import org.apache.commons.io.IOUtils;
4-
import org.jetbrains.annotations.NotNull;
55
import org.kohsuke.github.*;
66
import org.kohsuke.github.connector.GitHubConnector;
77
import org.kohsuke.github.connector.GitHubConnectorRequest;
88
import org.kohsuke.github.connector.GitHubConnectorResponse;
99

1010
import java.io.ByteArrayInputStream;
11+
import java.io.FileNotFoundException;
1112
import java.io.IOException;
1213
import java.io.InputStream;
1314
import java.lang.reflect.Field;
@@ -22,6 +23,8 @@
2223

2324
import javax.annotation.Nonnull;
2425

26+
import static java.net.HttpURLConnection.HTTP_BAD_REQUEST;
27+
import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
2528
import static java.util.logging.Level.*;
2629

2730
/**
@@ -31,6 +34,9 @@
3134
*/
3235
public final class GitHubConnectorHttpConnectorAdapter implements GitHubConnector, HttpConnector {
3336

37+
/**
38+
* Internal for testing.
39+
*/
3440
final HttpConnector httpConnector;
3541

3642
/**
@@ -52,8 +58,8 @@ public GitHubConnectorHttpConnectorAdapter(HttpConnector httpConnector) {
5258
* the HttpConnector to be adapted.
5359
* @return a GitHubConnector that calls into the provided HttpConnector.
5460
*/
55-
@NotNull
56-
public static GitHubConnector adapt(HttpConnector connector) {
61+
@Nonnull
62+
public static GitHubConnector adapt(@Nonnull HttpConnector connector) {
5763
GitHubConnector gitHubConnector;
5864
if (connector == HttpConnector.DEFAULT) {
5965
gitHubConnector = GitHubConnector.DEFAULT;
@@ -152,8 +158,6 @@ private static void setRequestMethod(String method, HttpURLConnection connection
152158
* initially received and before the body is processed.
153159
*
154160
* Implementation specific to {@link HttpURLConnection}. For internal use only.
155-
*
156-
*
157161
*/
158162
public final static class HttpURLConnectionGitHubConnectorResponse extends GitHubConnectorResponse {
159163

@@ -177,14 +181,25 @@ public final static class HttpURLConnectionGitHubConnectorResponse extends GitHu
177181
* {@inheritDoc}
178182
*/
179183
public InputStream bodyStream() throws IOException {
184+
if (connection.getResponseCode() >= HTTP_BAD_REQUEST) {
185+
if (connection.getResponseCode() == HTTP_NOT_FOUND) {
186+
throw new FileNotFoundException(request().url().toString());
187+
} else {
188+
throw new HttpException(errorMessage(),
189+
connection.getResponseCode(),
190+
connection.getResponseMessage(),
191+
connection.getURL().toString());
192+
}
193+
}
194+
180195
synchronized (this) {
181196
if (!inputStreamRead) {
182197
try (InputStream stream = wrapStream(connection.getInputStream())) {
183198
if (stream != null) {
184199
inputBytes = IOUtils.toByteArray(stream);
185-
inputStreamRead = true;
186200
}
187201
}
202+
inputStreamRead = true;
188203
}
189204
}
190205

@@ -202,9 +217,9 @@ public String errorMessage() {
202217
try (InputStream stream = wrapStream(connection.getErrorStream())) {
203218
if (stream != null) {
204219
errorString = new String(IOUtils.toByteArray(stream), StandardCharsets.UTF_8);
205-
errorStreamRead = true;
206220
}
207221
}
222+
errorStreamRead = true;
208223
}
209224
}
210225
if (errorString != null) {
@@ -216,6 +231,13 @@ public String errorMessage() {
216231
return result;
217232
}
218233

234+
@SuppressFBWarnings(value = { "EI_EXPOSE_REP" },
235+
justification = "Internal implementation class. Should not be used externally.")
236+
@Nonnull
237+
public HttpURLConnection getConnection() {
238+
return connection;
239+
}
240+
219241
/**
220242
* Handles the "Content-Encoding" header.
221243
*

0 commit comments

Comments
 (0)