Skip to content

Commit b430230

Browse files
author
adriancole
committed
address findbugs
1 parent 9938808 commit b430230

File tree

13 files changed

+41
-30
lines changed

13 files changed

+41
-30
lines changed

‎CHANGES.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
### Version 5.0.1
22
* `Decoder.decode()` is no longer called for `Response` or `void` types.
3+
* Miscellaneous findbugs fixes.
34

45
### Version 5.0
56
* Remove support for Observable methods.

‎core/src/main/java/feign/Client.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ Response convertResponse(HttpURLConnection connection) throws IOException {
144144
} else {
145145
stream = connection.getInputStream();
146146
}
147-
Reader body = stream != null ? new InputStreamReader(stream) : null;
147+
Reader body = stream != null ? new InputStreamReader(stream, UTF_8) : null;
148148
return Response.create(status, reason, headers, body, length);
149149
}
150150
}

‎core/src/main/java/feign/FeignException.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@
2323
* Origin exception type for all Http Apis.
2424
*/
2525
public class FeignException extends RuntimeException {
26-
static FeignException errorReading(Request request, Response response, IOException cause) {
27-
return new FeignException(format("%s %s %s", cause.getMessage(), request.method(), request.url(), 0), cause);
26+
static FeignException errorReading(Request request, Response ignored, IOException cause) {
27+
return new FeignException(format("%s %s %s", cause.getMessage(), request.method(), request.url()), cause);
2828
}
2929

3030
public static FeignException errorStatus(String methodKey, Response response) {

‎core/src/main/java/feign/Logger.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,10 +176,9 @@ Response logAndRebufferResponse(String configKey, Level logLevel, Response respo
176176
log(configKey, ""); // CRLF
177177
}
178178

179-
Reader body = response.body().asReader();
179+
BufferedReader reader = new BufferedReader(response.body().asReader());
180180
try {
181181
StringBuilder buffered = new StringBuilder();
182-
BufferedReader reader = new BufferedReader(body);
183182
String line;
184183
while ((line = reader.readLine()) != null) {
185184
buffered.append(line);
@@ -191,7 +190,7 @@ Response logAndRebufferResponse(String configKey, Level logLevel, Response respo
191190
log(configKey, "<--- END HTTP (%s-byte body)", bodyAsString.getBytes(UTF_8).length);
192191
return Response.create(response.status(), response.reason(), response.headers(), bodyAsString);
193192
} finally {
194-
ensureClosed(body);
193+
ensureClosed(reader);
195194
}
196195
}
197196
}

‎core/src/main/java/feign/RetryableException.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,15 @@ public class RetryableException extends FeignException {
2626

2727
private static final long serialVersionUID = 1L;
2828

29-
private final Date retryAfter;
29+
private final Long retryAfter;
3030

3131
/**
3232
* @param retryAfter usually corresponds to the {@link feign.Util#RETRY_AFTER}
3333
* header.
3434
*/
3535
public RetryableException(String message, Throwable cause, Date retryAfter) {
3636
super(message, cause);
37-
this.retryAfter = retryAfter;
37+
this.retryAfter = retryAfter != null ? retryAfter.getTime() : null;
3838
}
3939

4040
/**
@@ -43,7 +43,7 @@ public RetryableException(String message, Throwable cause, Date retryAfter) {
4343
*/
4444
public RetryableException(String message, Date retryAfter) {
4545
super(message);
46-
this.retryAfter = retryAfter;
46+
this.retryAfter = retryAfter != null ? retryAfter.getTime() : null;
4747
}
4848

4949
/**
@@ -52,6 +52,6 @@ public RetryableException(String message, Date retryAfter) {
5252
* application-specific response. Null if unknown.
5353
*/
5454
public Date retryAfter() {
55-
return retryAfter;
55+
return retryAfter != null ? new Date(retryAfter) : null;
5656
}
5757
}

‎core/src/main/java/feign/Target.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ public HardCodedTarget(Class<T> type, String name, String url) {
100100
}
101101

102102
@Override public boolean equals(Object obj) {
103+
if (obj == null)
104+
return false;
103105
if (this == obj)
104106
return true;
105107
if (HardCodedTarget.class != obj.getClass())

‎core/src/test/java/feign/FeignTest.java‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ public void postTemplateParamsResolve() throws IOException, InterruptedException
136136
TestInterface api = Feign.create(TestInterface.class, "http://localhost:" + server.getPort(), new TestInterface.Module());
137137

138138
api.login("netflix", "denominator", "password");
139-
assertEquals(new String(server.takeRequest().getBody()),
139+
assertEquals(new String(server.takeRequest().getBody(), UTF_8),
140140
"{\"customer_name\": \"netflix\", \"user_name\": \"denominator\", \"password\": \"password\"}");
141141
} finally {
142142
server.shutdown();
@@ -171,7 +171,7 @@ public void postFormParams() throws IOException, InterruptedException {
171171
TestInterface api = Feign.create(TestInterface.class, "http://localhost:" + server.getPort(), new TestInterface.Module());
172172

173173
api.form("netflix", "denominator", "password");
174-
assertEquals(new String(server.takeRequest().getBody()),
174+
assertEquals(new String(server.takeRequest().getBody(), UTF_8),
175175
"customer_name=netflix,user_name=denominator,password=password");
176176
} finally {
177177
server.shutdown();
@@ -190,7 +190,7 @@ public void postBodyParam() throws IOException, InterruptedException {
190190
api.body(Arrays.asList("netflix", "denominator", "password"));
191191
RecordedRequest request = server.takeRequest();
192192
assertEquals(request.getHeader("Content-Length"), "32");
193-
assertEquals(new String(request.getBody()), "[netflix, denominator, password]");
193+
assertEquals(new String(request.getBody(), UTF_8), "[netflix, denominator, password]");
194194
} finally {
195195
server.shutdown();
196196
}
@@ -317,7 +317,7 @@ public void canOverrideErrorDecoder() throws IOException, InterruptedException {
317317
@Test public void retriesLostConnectionBeforeRead() throws IOException, InterruptedException {
318318
MockWebServer server = new MockWebServer();
319319
server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.DISCONNECT_AT_START));
320-
server.enqueue(new MockResponse().setBody("success!".getBytes()));
320+
server.enqueue(new MockResponse().setBody("success!".getBytes(UTF_8)));
321321
server.play();
322322

323323
try {
@@ -346,7 +346,7 @@ public Object decode(Response response, Type type) {
346346

347347
public void overrideTypeSpecificDecoder() throws IOException, InterruptedException {
348348
MockWebServer server = new MockWebServer();
349-
server.enqueue(new MockResponse().setBody("success!".getBytes()));
349+
server.enqueue(new MockResponse().setBody("success!".getBytes(UTF_8)));
350350
server.play();
351351

352352
try {
@@ -380,8 +380,8 @@ public Object decode(Response response, Type type) throws IOException, FeignExce
380380
*/
381381
public void retryableExceptionInDecoder() throws IOException, InterruptedException {
382382
MockWebServer server = new MockWebServer();
383-
server.enqueue(new MockResponse().setBody("retry!".getBytes()));
384-
server.enqueue(new MockResponse().setBody("success!".getBytes()));
383+
server.enqueue(new MockResponse().setBody("retry!".getBytes(UTF_8)));
384+
server.enqueue(new MockResponse().setBody("success!".getBytes(UTF_8)));
385385
server.play();
386386

387387
try {
@@ -410,7 +410,7 @@ public Object decode(Response response, Type type) throws IOException {
410410
@Test(expectedExceptions = FeignException.class, expectedExceptionsMessageRegExp = "error reading response POST http://.*")
411411
public void doesntRetryAfterResponseIsSent() throws IOException, InterruptedException {
412412
MockWebServer server = new MockWebServer();
413-
server.enqueue(new MockResponse().setBody("success!".getBytes()));
413+
server.enqueue(new MockResponse().setBody("success!".getBytes(UTF_8)));
414414
server.play();
415415

416416
try {
@@ -434,7 +434,7 @@ static class TrustSSLSockets {
434434
@Test public void canOverrideSSLSocketFactory() throws IOException, InterruptedException {
435435
MockWebServer server = new MockWebServer();
436436
server.useHttps(TrustingSSLSocketFactory.get("localhost"), false);
437-
server.enqueue(new MockResponse().setBody("success!".getBytes()));
437+
server.enqueue(new MockResponse().setBody("success!".getBytes(UTF_8)));
438438
server.play();
439439

440440
try {
@@ -456,7 +456,7 @@ static class DisableHostnameVerification {
456456
@Test public void canOverrideHostnameVerifier() throws IOException, InterruptedException {
457457
MockWebServer server = new MockWebServer();
458458
server.useHttps(TrustingSSLSocketFactory.get("bad.example.com"), false);
459-
server.enqueue(new MockResponse().setBody("success!".getBytes()));
459+
server.enqueue(new MockResponse().setBody("success!".getBytes(UTF_8)));
460460
server.play();
461461

462462
try {
@@ -472,7 +472,7 @@ static class DisableHostnameVerification {
472472
MockWebServer server = new MockWebServer();
473473
server.useHttps(TrustingSSLSocketFactory.get("localhost"), false);
474474
server.enqueue(new MockResponse().setSocketPolicy(SocketPolicy.FAIL_HANDSHAKE));
475-
server.enqueue(new MockResponse().setBody("success!".getBytes()));
475+
server.enqueue(new MockResponse().setBody("success!".getBytes(UTF_8)));
476476
server.play();
477477

478478
try {

‎core/src/test/java/feign/LoggerTest.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.List;
3434
import java.util.regex.Pattern;
3535

36+
import static feign.Util.UTF_8;
3637
import static org.testng.Assert.assertEquals;
3738
import static org.testng.Assert.assertTrue;
3839
import static org.testng.Assert.fail;
@@ -116,7 +117,7 @@ public void levelEmits(final Logger.Level logLevel, List<String> expectedMessage
116117
assertTrue(messages.get(i).matches(expectedMessages.get(i)), messages.get(i));
117118
}
118119

119-
assertEquals(new String(server.takeRequest().getBody()),
120+
assertEquals(new String(server.takeRequest().getBody(), UTF_8),
120121
"{\"customer_name\": \"netflix\", \"user_name\": \"denominator\", \"password\": \"password\"}");
121122
} finally {
122123
server.shutdown();
@@ -213,7 +214,7 @@ public void readTimeoutEmits(final Logger.Level logLevel, List<String> expectedM
213214

214215
assertMessagesMatch(expectedMessages);
215216

216-
assertEquals(new String(server.takeRequest().getBody()),
217+
assertEquals(new String(server.takeRequest().getBody(), UTF_8),
217218
"{\"customer_name\": \"netflix\", \"user_name\": \"denominator\", \"password\": \"password\"}");
218219
} finally {
219220
server.shutdown();

‎core/src/test/java/feign/UtilTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ interface ParameterizedDecoder<T extends List<String>> extends Decoder {
4242
interface Parameterized<T> {
4343
}
4444

45-
class ParameterizedSubtype implements Parameterized<String> {
45+
static class ParameterizedSubtype implements Parameterized<String> {
4646
}
4747

4848
@Test public void resolveLastTypeParameterWhenNotSubtype() throws Exception {

‎ribbon/src/main/java/feign/ribbon/LoadBalancingTarget.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,8 @@ public AbstractLoadBalancer lb() {
104104
}
105105

106106
@Override public boolean equals(Object obj) {
107+
if (obj == null)
108+
return false;
107109
if (this == obj)
108110
return true;
109111
if (LoadBalancingTarget.class != obj.getClass())

0 commit comments

Comments
 (0)