Skip to content

Commit 056d1f0

Browse files
bitwisemanbitwiseman
committed
Clean up GitHubConnectorResponseHttpUrlConnectionAdapter
1 parent 55a2b24 commit 056d1f0

File tree

7 files changed

+65
-113
lines changed

7 files changed

+65
-113
lines changed

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

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

33
import org.kohsuke.github.connector.GitHubConnectorResponse;
4-
import org.kohsuke.github.internal.GitHubConnectorHttpConnectorAdapter;
54

65
import java.io.IOException;
76
import java.io.InterruptedIOException;
@@ -42,14 +41,7 @@ public void onError(@Nonnull GitHubConnectorResponse connectorResponse) throws I
4241
connectorResponse.statusCode(),
4342
connectorResponse.header("Status"),
4443
connectorResponse.request().url().toString()).withResponseHeaderFields(connectorResponse.allHeaders());
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);
44+
onError(e, connectorResponse.toHttpURLConnection());
5345
}
5446

5547
/**

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

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

33
import org.kohsuke.github.connector.GitHubConnectorResponse;
4-
import org.kohsuke.github.internal.GitHubConnectorHttpConnectorAdapter;
54

65
import java.io.IOException;
76
import java.io.InterruptedIOException;
@@ -38,14 +37,7 @@ public void onError(@Nonnull GitHubConnectorResponse connectorResponse) throws I
3837
connectorResponse.statusCode(),
3938
connectorResponse.header("Status"),
4039
connectorResponse.request().url().toString()).withResponseHeaderFields(connectorResponse.allHeaders());
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);
40+
onError(e, connectorResponse.toHttpURLConnection());
4941
}
5042

5143
/**

‎src/main/java/org/kohsuke/github/connector/GitHubConnectorResponse.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import java.io.Closeable;
66
import java.io.IOException;
77
import java.io.InputStream;
8+
import java.net.HttpURLConnection;
89
import java.util.*;
910

1011
import javax.annotation.CheckForNull;
@@ -43,6 +44,19 @@ protected GitHubConnectorResponse(@Nonnull GitHubConnectorRequest request,
4344
this.headers = Collections.unmodifiableMap(caseInsensitiveMap);
4445
}
4546

47+
/**
48+
* Get this response as a {@link HttpURLConnection}.
49+
*
50+
* @return an object that implements at least the response related methods of {@link HttpURLConnection}.
51+
*/
52+
@Deprecated
53+
@Nonnull
54+
public HttpURLConnection toHttpURLConnection() {
55+
HttpURLConnection connection;
56+
connection = new GitHubConnectorResponseHttpUrlConnectionAdapter(this);
57+
return connection;
58+
}
59+
4660
/**
4761
* Gets the value of a header field for this response.
4862
*

‎src/main/java/org/kohsuke/github/GitHubConnectorResponseHttpUrlConnectionAdapter.java renamed to ‎src/main/java/org/kohsuke/github/connector/GitHubConnectorResponseHttpUrlConnectionAdapter.java

Lines changed: 4 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,21 @@
1-
package org.kohsuke.github;
2-
3-
import org.kohsuke.github.connector.GitHubConnectorResponse;
1+
package org.kohsuke.github.connector;
42

53
import java.io.ByteArrayInputStream;
64
import java.io.IOException;
75
import java.io.InputStream;
86
import java.io.OutputStream;
97
import java.net.HttpURLConnection;
10-
import java.net.ProtocolException;
118
import java.nio.charset.StandardCharsets;
129
import java.security.Permission;
1310
import java.util.*;
1411

1512
class GitHubConnectorResponseHttpUrlConnectionAdapter extends HttpURLConnection {
1613

17-
private static final Comparator<String> nullableCaseInsensitiveComparator = Comparator
18-
.nullsFirst(String.CASE_INSENSITIVE_ORDER);
19-
2014
private final GitHubConnectorResponse connectorResponse;
2115

2216
public GitHubConnectorResponseHttpUrlConnectionAdapter(GitHubConnectorResponse connectorResponse) {
2317
super(connectorResponse.request().url());
18+
this.connected = true;
2419
this.connectorResponse = connectorResponse;
2520
}
2621

@@ -30,21 +25,6 @@ public String getHeaderFieldKey(int n) {
3025
return keys.get(n);
3126
}
3227

33-
@Override
34-
public void setFixedLengthStreamingMode(int contentLength) {
35-
throw new UnsupportedOperationException();
36-
}
37-
38-
@Override
39-
public void setFixedLengthStreamingMode(long contentLength) {
40-
throw new UnsupportedOperationException();
41-
}
42-
43-
@Override
44-
public void setChunkedStreamingMode(int chunklen) {
45-
throw new UnsupportedOperationException();
46-
}
47-
4828
@Override
4929
public String getHeaderField(int n) {
5030
return connectorResponse.header(getHeaderFieldKey(n));
@@ -60,11 +40,6 @@ public boolean getInstanceFollowRedirects() {
6040
throw new UnsupportedOperationException();
6141
}
6242

63-
@Override
64-
public void setRequestMethod(String method) throws ProtocolException {
65-
throw new UnsupportedOperationException();
66-
}
67-
6843
@Override
6944
public String getRequestMethod() {
7045
return connectorResponse.request().method();
@@ -210,49 +185,24 @@ public OutputStream getOutputStream() throws IOException {
210185

211186
@Override
212187
public String toString() {
213-
return connectorResponse.toString();
214-
}
215-
216-
@Override
217-
public void setDoInput(boolean doinput) {
218-
throw new UnsupportedOperationException();
188+
return this.getClass().getName() + ": " + connectorResponse.toString();
219189
}
220190

221191
@Override
222192
public boolean getDoInput() {
223193
throw new UnsupportedOperationException();
224194
}
225195

226-
@Override
227-
public void setDoOutput(boolean dooutput) {
228-
throw new UnsupportedOperationException();
229-
}
230-
231196
@Override
232197
public boolean getDoOutput() {
233198
throw new UnsupportedOperationException();
234199
}
235200

236-
@Override
237-
public void setAllowUserInteraction(boolean allowuserinteraction) {
238-
throw new UnsupportedOperationException();
239-
}
240-
241-
@Override
242-
public void setUseCaches(boolean usecaches) {
243-
throw new UnsupportedOperationException();
244-
}
245-
246201
@Override
247202
public boolean getUseCaches() {
248203
throw new UnsupportedOperationException();
249204
}
250205

251-
@Override
252-
public void setIfModifiedSince(long ifmodifiedsince) {
253-
throw new UnsupportedOperationException();
254-
}
255-
256206
@Override
257207
public long getIfModifiedSince() {
258208
return getHeaderFieldDate("If-Modified-Since", 0);
@@ -263,26 +213,11 @@ public void setDefaultUseCaches(boolean defaultusecaches) {
263213
throw new UnsupportedOperationException();
264214
}
265215

266-
@Override
267-
public void setRequestProperty(String key, String value) {
268-
throw new UnsupportedOperationException();
269-
}
270-
271-
@Override
272-
public void addRequestProperty(String key, String value) {
273-
throw new UnsupportedOperationException();
274-
}
275-
276216
@Override
277217
public String getRequestProperty(String key) {
278218
return connectorResponse.request().header(key);
279219
}
280220

281-
@Override
282-
public Map<String, List<String>> getRequestProperties() {
283-
throw new IllegalStateException("Already connected");
284-
}
285-
286221
@Override
287222
public boolean getAllowUserInteraction() {
288223
throw new UnsupportedOperationException();
@@ -305,6 +240,6 @@ public boolean usingProxy() {
305240

306241
@Override
307242
public void connect() throws IOException {
308-
throw new UnsupportedOperationException();
243+
// no op
309244
}
310245
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@
1111
*
1212
* Allow behavior to be changed for different version of Java, such as supporting Java 11 HttpClient.
1313
*/
14-
public class DefaultGitHubConnector {
14+
public final class DefaultGitHubConnector {
15+
16+
private DefaultGitHubConnector() {
17+
}
1518

1619
/**
1720
* Creates a {@link GitHubConnector} that will be used as the default connector.

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,9 @@ public String errorMessage() {
234234
@SuppressFBWarnings(value = { "EI_EXPOSE_REP" },
235235
justification = "Internal implementation class. Should not be used externally.")
236236
@Nonnull
237-
public HttpURLConnection getConnection() {
237+
@Override
238+
@Deprecated
239+
public HttpURLConnection toHttpURLConnection() {
238240
return connection;
239241
}
240242

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

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import java.io.IOException;
1010
import java.io.InputStream;
1111
import java.net.HttpURLConnection;
12+
import java.net.ProtocolException;
1213
import java.nio.charset.StandardCharsets;
1314
import java.util.Date;
1415
import java.util.Map;
@@ -102,13 +103,25 @@ public void onError(IOException e, HttpURLConnection uc) throws IOException {
102103

103104
Assert.assertThrows(IllegalStateException.class, () -> uc.getRequestProperties());
104105

105-
// disconnect does nothing, never throws
106-
uc.disconnect();
107-
uc.disconnect();
108-
109-
if (uc instanceof GitHubConnectorResponseHttpUrlConnectionAdapter) {
110-
Assert.assertThrows(UnsupportedOperationException.class, () -> uc.connect());
106+
// Actions that are not allowed because connection already opened.
107+
Assert.assertThrows(IllegalStateException.class, () -> uc.addRequestProperty("bogus", "item"));
108+
109+
Assert.assertThrows(IllegalStateException.class, () -> uc.setAllowUserInteraction(true));
110+
Assert.assertThrows(IllegalStateException.class, () -> uc.setChunkedStreamingMode(1));
111+
Assert.assertThrows(IllegalStateException.class, () -> uc.setDoInput(true));
112+
Assert.assertThrows(IllegalStateException.class, () -> uc.setDoOutput(true));
113+
Assert.assertThrows(IllegalStateException.class, () -> uc.setFixedLengthStreamingMode(1));
114+
Assert.assertThrows(IllegalStateException.class, () -> uc.setFixedLengthStreamingMode(1L));
115+
Assert.assertThrows(IllegalStateException.class, () -> uc.setIfModifiedSince(1L));
116+
Assert.assertThrows(IllegalStateException.class, () -> uc.setRequestProperty("bogus", "thing"));
117+
Assert.assertThrows(IllegalStateException.class, () -> uc.setUseCaches(true));
118+
119+
boolean isAdapter = false;
120+
if (uc.toString().contains("GitHubConnectorResponseHttpUrlConnectionAdapter")) {
121+
isAdapter = true;
122+
}
111123

124+
if (isAdapter) {
112125
Assert.assertThrows(UnsupportedOperationException.class,
113126
() -> uc.getAllowUserInteraction());
114127
Assert.assertThrows(UnsupportedOperationException.class, () -> uc.getConnectTimeout());
@@ -125,32 +138,33 @@ public void onError(IOException e, HttpURLConnection uc) throws IOException {
125138
Assert.assertThrows(UnsupportedOperationException.class, () -> uc.getUseCaches());
126139
Assert.assertThrows(UnsupportedOperationException.class, () -> uc.usingProxy());
127140

128-
Assert.assertThrows(UnsupportedOperationException.class,
129-
() -> uc.addRequestProperty("bogus", "item"));
130-
Assert.assertThrows(UnsupportedOperationException.class,
131-
() -> uc.setAllowUserInteraction(true));
132-
Assert.assertThrows(UnsupportedOperationException.class,
133-
() -> uc.setChunkedStreamingMode(1));
134141
Assert.assertThrows(UnsupportedOperationException.class, () -> uc.setConnectTimeout(10));
135142
Assert.assertThrows(UnsupportedOperationException.class,
136143
() -> uc.setDefaultUseCaches(true));
137-
Assert.assertThrows(UnsupportedOperationException.class, () -> uc.setDoInput(true));
138-
Assert.assertThrows(UnsupportedOperationException.class, () -> uc.setDoOutput(true));
139-
Assert.assertThrows(UnsupportedOperationException.class,
140-
() -> uc.setFixedLengthStreamingMode(1));
141-
Assert.assertThrows(UnsupportedOperationException.class,
142-
() -> uc.setFixedLengthStreamingMode(1L));
143-
Assert.assertThrows(UnsupportedOperationException.class, () -> uc.setIfModifiedSince(1L));
144+
144145
Assert.assertThrows(UnsupportedOperationException.class,
145146
() -> uc.setInstanceFollowRedirects(true));
146147
Assert.assertThrows(UnsupportedOperationException.class, () -> uc.setReadTimeout(10));
147-
Assert.assertThrows(UnsupportedOperationException.class, () -> uc.setRequestMethod("GET"));
148-
Assert.assertThrows(UnsupportedOperationException.class,
149-
() -> uc.setRequestProperty("bogus", "thing"));
150-
Assert.assertThrows(UnsupportedOperationException.class, () -> uc.setUseCaches(true));
148+
Assert.assertThrows(ProtocolException.class, () -> uc.setRequestMethod("GET"));
149+
} else {
150+
uc.getDefaultUseCaches();
151+
assertThat(uc.getDoInput(), is(true));
152+
153+
// Depending on the underlying implementation, this may throw or not
154+
// Assert.assertThrows(IllegalStateException.class, () -> uc.setRequestMethod("GET"));
151155
}
152156

153-
RateLimitHandler.FAIL.onError(e, uc);
157+
// ignored
158+
uc.connect();
159+
160+
// disconnect does nothing, never throws
161+
uc.disconnect();
162+
uc.disconnect();
163+
164+
// ignored
165+
uc.connect();
166+
167+
AbuseLimitHandler.FAIL.onError(e, uc);
154168
}
155169
})
156170
.build();

0 commit comments

Comments
 (0)