Skip to content

Commit 99760f7

Browse files
davidmc24davidmc24
committed
Fix default decoder's support for Response decoding, clean up usage of StringDecoder
Decoder.Default's decoding to Response didn't actually work; the reader would always be closed when used from Feign, as it depended on the url connection, which would have been closed by the time the Response object was returned to the client. This wasn't noticed because the default decoder tests don't use the mock web server. There will be test coverage added for this shortly as part of the enhancements to support a Builder.
1 parent df405a6 commit 99760f7

5 files changed

Lines changed: 41 additions & 35 deletions

File tree

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

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919

2020
import java.io.IOException;
2121

22-
import feign.codec.StringDecoder;
23-
2422
/**
2523
* Origin exception type for all Http Apis.
2624
*/
@@ -29,14 +27,11 @@ static FeignException errorReading(Request request, Response response, IOExcepti
2927
return new FeignException(format("%s %s %s", cause.getMessage(), request.method(), request.url(), 0), cause);
3028
}
3129

32-
private static final StringDecoder toString = new StringDecoder();
33-
3430
public static FeignException errorStatus(String methodKey, Response response) {
3531
String message = format("status %s reading %s", response.status(), methodKey);
3632
try {
3733
if (response.body() != null) {
38-
String body = toString.decode(response, String.class).toString();
39-
response = Response.create(response.status(), response.reason(), response.headers(), body);
34+
String body = Util.toString(response.body().asReader());
4035
message += "; content:\n" + body;
4136
}
4237
} catch (IOException ignored) { // NOPMD

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@
1717

1818
import java.io.Closeable;
1919
import java.io.IOException;
20+
import java.io.Reader;
2021
import java.lang.reflect.Array;
2122
import java.lang.reflect.ParameterizedType;
2223
import java.lang.reflect.Type;
2324
import java.lang.reflect.WildcardType;
25+
import java.nio.CharBuffer;
2426
import java.nio.charset.Charset;
2527
import java.util.ArrayList;
2628
import java.util.Collection;
@@ -163,4 +165,24 @@ public static Type resolveLastTypeParameter(Type genericContext, Class<?> supert
163165
}
164166
return types[types.length - 1];
165167
}
168+
169+
private static final int BUF_SIZE = 0x800; // 2K chars (4K bytes)
170+
171+
public static String toString(Reader reader) throws IOException {
172+
if (reader == null) {
173+
return null;
174+
}
175+
try {
176+
StringBuilder to = new StringBuilder();
177+
CharBuffer buf = CharBuffer.allocate(BUF_SIZE);
178+
while (reader.read(buf) != -1) {
179+
buf.flip();
180+
to.append(buf);
181+
buf.clear();
182+
}
183+
return to.toString();
184+
} finally {
185+
ensureClosed(reader);
186+
}
187+
}
166188
}

‎core/src/main/java/feign/codec/Decoder.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import feign.FeignException;
1919
import feign.Response;
20+
import feign.Util;
2021

2122
import java.io.IOException;
2223
import java.lang.reflect.Type;
@@ -74,16 +75,18 @@ public interface Decoder {
7475
* signatures.
7576
*/
7677
public class Default implements Decoder {
77-
private final StringDecoder stringDecoder = new StringDecoder();
78-
7978
@Override
8079
public Object decode(Response response, Type type) throws IOException {
8180
if (Response.class.equals(type)) {
82-
return response;
83-
} else if (String.class.equals(type)) {
84-
return stringDecoder.decode(response, type);
81+
String bodyString = null;
82+
if (response.body() != null) {
83+
bodyString = Util.toString(response.body().asReader());
84+
}
85+
return Response.create(response.status(), response.reason(), response.headers(), bodyString);
8586
} else if (void.class.equals(type) || response.body() == null) {
8687
return null;
88+
} else if (String.class.equals(type)) {
89+
return Util.toString(response.body().asReader());
8790
}
8891
throw new DecodeException(format("%s is not a type supported by this decoder.", type));
8992
}

‎core/src/main/java/feign/codec/StringDecoder.java‎

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,38 +16,21 @@
1616
package feign.codec;
1717

1818
import feign.Response;
19+
import feign.Util;
1920

2021
import java.io.IOException;
21-
import java.io.Reader;
2222
import java.lang.reflect.Type;
23-
import java.nio.CharBuffer;
24-
25-
import static feign.Util.ensureClosed;
2623

2724
/**
2825
* Adapted from {@code com.google.common.io.CharStreams.toString()}.
2926
*/
3027
public class StringDecoder implements Decoder {
31-
private static final int BUF_SIZE = 0x800; // 2K chars (4K bytes)
32-
3328
@Override
3429
public Object decode(Response response, Type type) throws IOException {
3530
Response.Body body = response.body();
3631
if (body == null) {
3732
return null;
3833
}
39-
Reader from = body.asReader();
40-
try {
41-
StringBuilder to = new StringBuilder();
42-
CharBuffer buf = CharBuffer.allocate(BUF_SIZE);
43-
while (from.read(buf) != -1) {
44-
buf.flip();
45-
to.append(buf);
46-
buf.clear();
47-
}
48-
return to.toString();
49-
} finally {
50-
ensureClosed(from);
51-
}
34+
return Util.toString(body.asReader());
5235
}
5336
}

‎core/src/test/java/feign/codec/DefaultDecoderTest.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,12 @@
1515
*/
1616
package feign.codec;
1717

18-
import feign.FeignException;
1918
import feign.Response;
19+
import feign.Util;
2020
import org.testng.annotations.Test;
2121
import org.w3c.dom.Document;
2222

23+
import java.io.StringReader;
2324
import java.util.Collection;
2425
import java.util.Collections;
2526
import java.util.HashMap;
@@ -38,19 +39,19 @@ public class DefaultDecoderTest {
3839
@Test public void testDecodesToResponse() throws Exception {
3940
Response response = knownResponse();
4041
Object decodedObject = decoder.decode(response, Response.class);
41-
assertEquals(decodedObject.getClass(), Response.class, "");
42+
assertEquals(decodedObject.getClass(), Response.class);
4243
Response decodedResponse = (Response) decodedObject;
4344
assertEquals(decodedResponse.status(), response.status());
4445
assertEquals(decodedResponse.reason(), response.reason());
4546
assertEquals(decodedResponse.headers(), response.headers());
46-
assertEquals(decodedResponse.body().toString(), response.body().toString());
47+
assertEquals(Util.toString(decodedResponse.body().asReader()), "response body");
4748
}
4849

4950
@Test public void testDecodesToString() throws Exception {
5051
Response response = knownResponse();
5152
Object decodedObject = decoder.decode(response, String.class);
5253
assertEquals(decodedObject.getClass(), String.class);
53-
assertEquals(decodedObject.toString(), response.body().toString());
54+
assertEquals(decodedObject.toString(), "response body");
5455
}
5556

5657
@Test public void testDecodesNullBodyToNull() throws Exception {
@@ -63,9 +64,11 @@ public void testRefusesToDecodeOtherTypes() throws Exception {
6364
}
6465

6566
private Response knownResponse() {
67+
String content = "response body";
68+
StringReader reader = new StringReader(content);
6669
Map<String, Collection<String>> headers = new HashMap<String, Collection<String>>();
6770
headers.put("Content-Type", Collections.singleton("text/plain"));
68-
return Response.create(200, "OK", headers, "response body");
71+
return Response.create(200, "OK", headers, reader, content.length());
6972
}
7073

7174
private Response nullBodyResponse() {

0 commit comments

Comments
 (0)