Skip to content

Commit 9c5a52d

Browse files
czubinvelo
czubin
authored and
velo
committed
Add support for java 8's Optional type to represent a HTTP 404 response. (OpenFeign#822)
* Add support for java 8's Optional type to represent a HTTP 404 response. * Delete x.patch * Also dealing with Stream in case of empty responses * Add support for java 8's Optional type to represent a HTTP 404 response. * Delete x.patch
1 parent 2793a31 commit 9c5a52d

4 files changed

Lines changed: 61 additions & 25 deletions

File tree

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

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,11 @@
3838
import java.util.List;
3939
import java.util.Map;
4040
import java.util.NoSuchElementException;
41+
import java.util.Optional;
4142
import java.util.Set;
4243
import java.util.function.Predicate;
44+
import java.util.function.Supplier;
45+
import java.util.stream.Stream;
4346
import static java.lang.String.format;
4447

4548
/**
@@ -250,32 +253,22 @@ public static Type resolveLastTypeParameter(Type genericContext, Class<?> supert
250253
* the raw type (vs type hierarchy). Decorate for sophistication.
251254
*/
252255
public static Object emptyValueOf(Type type) {
253-
return EMPTIES.get(Types.getRawType(type));
256+
return EMPTIES.getOrDefault(Types.getRawType(type), () -> null).get();
254257
}
255258

256-
private static final Map<Class<?>, Object> EMPTIES;
259+
private static final Map<Class<?>, Supplier<Object>> EMPTIES;
257260
static {
258-
Map<Class<?>, Object> empties = new LinkedHashMap<Class<?>, Object>();
259-
empties.put(boolean.class, false);
260-
empties.put(Boolean.class, false);
261-
empties.put(byte[].class, new byte[0]);
262-
empties.put(Collection.class, Collections.emptyList());
263-
empties.put(Iterator.class, new Iterator<Object>() { // Collections.emptyIterator is a 1.7 api
264-
public boolean hasNext() {
265-
return false;
266-
}
267-
268-
public Object next() {
269-
throw new NoSuchElementException();
270-
}
271-
272-
public void remove() {
273-
throw new IllegalStateException();
274-
}
275-
});
276-
empties.put(List.class, Collections.emptyList());
277-
empties.put(Map.class, Collections.emptyMap());
278-
empties.put(Set.class, Collections.emptySet());
261+
final Map<Class<?>, Supplier<Object>> empties = new LinkedHashMap<Class<?>, Supplier<Object>>();
262+
empties.put(boolean.class, () -> false);
263+
empties.put(Boolean.class, () -> false);
264+
empties.put(byte[].class, () -> new byte[0]);
265+
empties.put(Collection.class, Collections::emptyList);
266+
empties.put(Iterator.class, Collections::emptyIterator);
267+
empties.put(List.class, Collections::emptyList);
268+
empties.put(Map.class, Collections::emptyMap);
269+
empties.put(Set.class, Collections::emptySet);
270+
empties.put(Optional.class, Optional::empty);
271+
empties.put(Stream.class, Stream::empty);
279272
EMPTIES = Collections.unmodifiableMap(empties);
280273
}
281274

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public class Default extends StringDecoder {
8282

8383
@Override
8484
public Object decode(Response response, Type type) throws IOException {
85-
if (response.status() == 404)
85+
if (response.status() == 404 || response.status() == 204)
8686
return Util.emptyValueOf(type);
8787
if (response.body() == null)
8888
return null;

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

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
*/
1414
package feign;
1515

16-
import java.util.HashMap;
1716
import okhttp3.mockwebserver.MockResponse;
1817
import okhttp3.mockwebserver.MockWebServer;
1918
import org.assertj.core.data.MapEntry;
@@ -28,11 +27,14 @@
2827
import java.lang.reflect.Type;
2928
import java.util.Arrays;
3029
import java.util.Collections;
30+
import java.util.HashMap;
3131
import java.util.Iterator;
3232
import java.util.List;
3333
import java.util.Map;
34+
import java.util.Optional;
3435
import java.util.concurrent.atomic.AtomicBoolean;
3536
import java.util.concurrent.atomic.AtomicInteger;
37+
import java.util.stream.Stream;
3638
import feign.codec.Decoder;
3739
import feign.codec.Encoder;
3840
import static feign.assertj.MockWebServerAssertions.assertThat;
@@ -64,6 +66,9 @@ public void testDefaults() throws Exception {
6466
/** Shows exception handling isn't required to coerce 404 to null or empty */
6567
@Test
6668
public void testDecode404() throws Exception {
69+
server.enqueue(new MockResponse().setResponseCode(404));
70+
server.enqueue(new MockResponse().setResponseCode(404));
71+
server.enqueue(new MockResponse().setResponseCode(404));
6772
server.enqueue(new MockResponse().setResponseCode(404));
6873
server.enqueue(new MockResponse().setResponseCode(404));
6974
server.enqueue(new MockResponse().setResponseCode(400));
@@ -72,6 +77,36 @@ public void testDecode404() throws Exception {
7277
TestInterface api = Feign.builder().decode404().target(TestInterface.class, url);
7378

7479
assertThat(api.getQueues("/")).isEmpty(); // empty, not null!
80+
assertThat(api.decodedLazyPost()).isEmpty(); // empty, not null!
81+
assertThat(api.optionalContent()).isEmpty(); // empty, not null!
82+
assertThat(api.streamPost()).isEmpty(); // empty, not null!
83+
assertThat(api.decodedPost()).isNull(); // null, not empty!
84+
85+
try { // ensure other 400 codes are not impacted.
86+
api.decodedPost();
87+
failBecauseExceptionWasNotThrown(FeignException.class);
88+
} catch (FeignException e) {
89+
assertThat(e.status()).isEqualTo(400);
90+
}
91+
}
92+
93+
/** Shows exception handling isn't required to coerce 204 to null or empty */
94+
@Test
95+
public void testDecode204() throws Exception {
96+
server.enqueue(new MockResponse().setResponseCode(204));
97+
server.enqueue(new MockResponse().setResponseCode(204));
98+
server.enqueue(new MockResponse().setResponseCode(204));
99+
server.enqueue(new MockResponse().setResponseCode(204));
100+
server.enqueue(new MockResponse().setResponseCode(204));
101+
server.enqueue(new MockResponse().setResponseCode(400));
102+
103+
String url = "http://localhost:" + server.getPort();
104+
TestInterface api = Feign.builder().target(TestInterface.class, url);
105+
106+
assertThat(api.getQueues("/")).isEmpty(); // empty, not null!
107+
assertThat(api.decodedLazyPost()).isEmpty(); // empty, not null!
108+
assertThat(api.optionalContent()).isEmpty(); // empty, not null!
109+
assertThat(api.streamPost()).isEmpty(); // empty, not null!
75110
assertThat(api.decodedPost()).isNull(); // null, not empty!
76111

77112
try { // ensure other 400 codes are not impacted.
@@ -467,6 +502,12 @@ interface TestInterface {
467502
@RequestLine("POST /")
468503
Iterator<String> decodedLazyPost();
469504

505+
@RequestLine("POST /")
506+
Optional<String> optionalContent();
507+
508+
@RequestLine("POST /")
509+
Stream<String> streamPost();
510+
470511
@RequestLine(value = "GET /api/queues/{vhost}", decodeSlash = false)
471512
byte[] getQueues(@Param("vhost") String vhost);
472513

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.Iterator;
2525
import java.util.List;
2626
import java.util.Map;
27+
import java.util.Optional;
2728
import java.util.Set;
2829
import feign.codec.Decoder;
2930
import static feign.Util.emptyToNull;
@@ -61,6 +62,7 @@ public void emptyValueOf() throws Exception {
6162
assertEquals(Collections.emptyList(), Util.emptyValueOf(List.class));
6263
assertEquals(Collections.emptyMap(), Util.emptyValueOf(Map.class));
6364
assertEquals(Collections.emptySet(), Util.emptyValueOf(Set.class));
65+
assertEquals(Optional.empty(), Util.emptyValueOf(Optional.class));
6466
}
6567

6668
/** In other words, {@code List<String>} is as empty as {@code List<?>}. */

0 commit comments

Comments
 (0)