From 24885fe9620ed620af43f4d2d6ffcfc82980e097 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Sat, 31 Oct 2015 09:10:21 -0700 Subject: [PATCH] Adds Feign.Builder.decode404() to reduce boilerplate for empty semantics This adds the `Feign.Builder.decode404()` flag which indicates decoders should process responses with 404 status. It also changes all first-party decoders (like gson) to return well-known empty values by default. Further customization is possible by wrapping or creating a custom decoder. Prior to this change, we used custom invocation handlers as the way to add fallback values based on exception or return status. `feign-hystrix` uses this to return `HystrixCommand`, but the general pattern applies to anything that has a type representing both success and failure, such as `Try` or `Observable`. As we define it here, 404 status is not a retry or fallback policy, it is just empty semantics. By limiting Feign's special processing to 404, we gain a lot with very little supporting code. If instead we opened all codes, Feign could easily turn bad request, redirect, or server errors silently to null. This sort of configuration issue is hard to troubleshoot. 404 -> empty is a very safe policy vs all codes. Moreover, we don't create a cliff, where folks seeking fallback policy eventually realize they can't if only given a response code. Fallback systems like Hystrix address exceptions that occur before or in lieu of a response. By special-casing 404, we avoid a slippery slope of half- implementing Hystrix. Finally, 404 handling has been commonly requested: it has a clear use- case, and through that value. This design supports that without breaking compatibility, or impacting existing integrations such as Hystrix or Ribbon. See #238 #287 --- CHANGELOG.md | 3 ++ core/src/main/java/feign/Feign.java | 32 ++++++++--- .../java/feign/SynchronousMethodHandler.java | 15 ++++-- core/src/main/java/feign/Util.java | 53 +++++++++++++++++++ core/src/main/java/feign/codec/Decoder.java | 12 ++--- .../main/java/feign/codec/ErrorDecoder.java | 18 +++++-- .../src/test/java/feign/FeignBuilderTest.java | 35 +++++++++--- core/src/test/java/feign/FeignTest.java | 12 ++--- core/src/test/java/feign/UtilTest.java | 49 +++++++++++++---- .../src/main/java/feign/gson/GsonDecoder.java | 6 +-- .../test/java/feign/gson/GsonCodecTest.java | 9 ++++ .../jackson/jaxb/JacksonJaxbJsonDecoder.java | 3 ++ .../jackson/jaxb/JacksonJaxbCodecTest.java | 9 ++++ .../java/feign/jackson/JacksonDecoder.java | 6 +-- .../java/feign/jackson/JacksonCodecTest.java | 9 ++++ .../src/main/java/feign/jaxb/JAXBDecoder.java | 3 ++ .../test/java/feign/jaxb/JAXBCodecTest.java | 10 ++++ sax/src/main/java/feign/sax/SAXDecoder.java | 6 +-- .../test/java/feign/sax/SAXDecoderTest.java | 13 ++++- 19 files changed, 244 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 43ac3d014f..9f37c3b3c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +### Version 8.12 +* Adds `Feign.Builder.decode404()` to reduce boilerplate for empty semantics. + ### Version 8.11 * Adds support for Hystrix via a `HystrixFeign` builder. diff --git a/core/src/main/java/feign/Feign.java b/core/src/main/java/feign/Feign.java index 3d86a2b374..4b19c78131 100644 --- a/core/src/main/java/feign/Feign.java +++ b/core/src/main/java/feign/Feign.java @@ -82,8 +82,7 @@ public static String configKey(Method method) { public static class Builder { - private final List - requestInterceptors = + private final List requestInterceptors = new ArrayList(); private Logger.Level logLevel = Logger.Level.NONE; private Contract contract = new Contract.Default(); @@ -94,9 +93,9 @@ public static class Builder { private Decoder decoder = new Decoder.Default(); private ErrorDecoder errorDecoder = new ErrorDecoder.Default(); private Options options = new Options(); - private InvocationHandlerFactory - invocationHandlerFactory = + private InvocationHandlerFactory invocationHandlerFactory = new InvocationHandlerFactory.Default(); + private boolean decode404; public Builder logLevel(Logger.Level logLevel) { this.logLevel = logLevel; @@ -133,6 +132,26 @@ public Builder decoder(Decoder decoder) { return this; } + /** + * This flag indicates that the {@link #decoder(Decoder) decoder} should process responses with + * 404 status, specifically returning null or empty instead of throwing {@link FeignException}. + * + *

All first-party (ex gson) decoders return well-known empty values defined by + * {@link Util#emptyValueOf}. To customize further, wrap an existing + * {@link #decoder(Decoder) decoder} or make your own. + * + *

This flag only works with 404, as opposed to all or arbitrary status codes. This was an + * explicit decision: 404 -> empty is safe, common and doesn't complicate redirection, retry or + * fallback policy. If your server returns a different status for not-found, correct via a + * custom {@link #client(Client) client}. + * + * @since 8.12 + */ + public Builder decode404() { + this.decode404 = true; + return this; + } + public Builder errorDecoder(ErrorDecoder errorDecoder) { this.errorDecoder = errorDecoder; return this; @@ -182,9 +201,8 @@ public T target(Target target) { public Feign build() { SynchronousMethodHandler.Factory synchronousMethodHandlerFactory = new SynchronousMethodHandler.Factory(client, retryer, requestInterceptors, logger, - logLevel); - ParseHandlersByName - handlersByName = + logLevel, decode404); + ParseHandlersByName handlersByName = new ParseHandlersByName(contract, options, encoder, decoder, errorDecoder, synchronousMethodHandlerFactory); return new ReflectiveFeign(handlersByName, invocationHandlerFactory); diff --git a/core/src/main/java/feign/SynchronousMethodHandler.java b/core/src/main/java/feign/SynchronousMethodHandler.java index fa6ab83ded..49c5d570b9 100644 --- a/core/src/main/java/feign/SynchronousMethodHandler.java +++ b/core/src/main/java/feign/SynchronousMethodHandler.java @@ -43,11 +43,13 @@ final class SynchronousMethodHandler implements MethodHandler { private final Options options; private final Decoder decoder; private final ErrorDecoder errorDecoder; + private final boolean decode404; + private SynchronousMethodHandler(Target target, Client client, Retryer retryer, List requestInterceptors, Logger logger, Logger.Level logLevel, MethodMetadata metadata, RequestTemplate.Factory buildTemplateFromArgs, Options options, - Decoder decoder, ErrorDecoder errorDecoder) { + Decoder decoder, ErrorDecoder errorDecoder, boolean decode404) { this.target = checkNotNull(target, "target"); this.client = checkNotNull(client, "client for %s", target); this.retryer = checkNotNull(retryer, "retryer for %s", target); @@ -60,6 +62,7 @@ private SynchronousMethodHandler(Target target, Client client, Retryer retrye this.options = checkNotNull(options, "options for %s", target); this.errorDecoder = checkNotNull(errorDecoder, "errorDecoder for %s", target); this.decoder = checkNotNull(decoder, "decoder for %s", target); + this.decode404 = decode404; } @Override @@ -117,6 +120,8 @@ Object executeAndDecode(RequestTemplate template) throws Throwable { } else { return decode(response); } + } else if (decode404 && response.status() == 404) { + return decode(response); } else { throw errorDecoder.decode(metadata.configKey(), response); } @@ -158,22 +163,24 @@ static class Factory { private final List requestInterceptors; private final Logger logger; private final Logger.Level logLevel; + private final boolean decode404; Factory(Client client, Retryer retryer, List requestInterceptors, - Logger logger, Logger.Level logLevel) { + Logger logger, Logger.Level logLevel, boolean decode404) { this.client = checkNotNull(client, "client"); this.retryer = checkNotNull(retryer, "retryer"); this.requestInterceptors = checkNotNull(requestInterceptors, "requestInterceptors"); this.logger = checkNotNull(logger, "logger"); this.logLevel = checkNotNull(logLevel, "logLevel"); + this.decode404 = decode404; } public MethodHandler create(Target target, MethodMetadata md, RequestTemplate.Factory buildTemplateFromArgs, Options options, Decoder decoder, ErrorDecoder errorDecoder) { return new SynchronousMethodHandler(target, client, retryer, requestInterceptors, logger, - logLevel, md, - buildTemplateFromArgs, options, decoder, errorDecoder); + logLevel, md, buildTemplateFromArgs, options, decoder, + errorDecoder, decode404); } } } diff --git a/core/src/main/java/feign/Util.java b/core/src/main/java/feign/Util.java index f37db36788..5550e6bb2b 100644 --- a/core/src/main/java/feign/Util.java +++ b/core/src/main/java/feign/Util.java @@ -32,7 +32,12 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; +import java.util.NoSuchElementException; +import java.util.Set; import static java.lang.String.format; @@ -183,6 +188,54 @@ public static Type resolveLastTypeParameter(Type genericContext, Class supert return types[types.length - 1]; } + /** + * This returns well known empty values for well-known java types. This returns null for types not + * in the following list. + * + *

    + *
  • {@code [Bb]oolean}
    • + *
  • {@code byte[]}
    • + *
  • {@code Collection}
    • + *
  • {@code Iterator}
    • + *
  • {@code List}
    • + *
  • {@code Map}
    • + *
  • {@code Set}
    • + *
+ * + *

When {@link Feign.Builder#decode404() decoding HTTP 404 status}, you'll need to teach + * decoders a default empty value for a type. This method cheaply supports typical types by only + * looking at the raw type (vs type hierarchy). Decorate for sophistication. + */ + public static Object emptyValueOf(Type type) { + return EMPTIES.get(Types.getRawType(type)); + } + + private static final Map, Object> EMPTIES; + static { + Map, Object> empties = new LinkedHashMap, Object>(); + empties.put(boolean.class, false); + empties.put(Boolean.class, false); + empties.put(byte[].class, new byte[0]); + empties.put(Collection.class, Collections.emptyList()); + empties.put(Iterator.class, new Iterator() { // Collections.emptyIterator is a 1.7 api + public boolean hasNext() { + return false; + } + + public Object next() { + throw new NoSuchElementException(); + } + + public void remove() { + throw new IllegalStateException(); + } + }); + empties.put(List.class, Collections.emptyList()); + empties.put(Map.class, Collections.emptyMap()); + empties.put(Set.class, Collections.emptySet()); + EMPTIES = Collections.unmodifiableMap(empties); + } + /** * Adapted from {@code com.google.common.io.CharStreams.toString()}. */ diff --git a/core/src/main/java/feign/codec/Decoder.java b/core/src/main/java/feign/codec/Decoder.java index 58502afb6e..5941bf7056 100644 --- a/core/src/main/java/feign/codec/Decoder.java +++ b/core/src/main/java/feign/codec/Decoder.java @@ -67,19 +67,15 @@ public interface Decoder { */ Object decode(Response response, Type type) throws IOException, DecodeException, FeignException; - /** - * Default implementation of {@code Decoder}. - */ + /** Default implementation of {@code Decoder}. */ public class Default extends StringDecoder { @Override public Object decode(Response response, Type type) throws IOException { - Response.Body body = response.body(); - if (body == null) { - return null; - } + if (response.status() == 404) return Util.emptyValueOf(type); + if (response.body() == null) return null; if (byte[].class.equals(type)) { - return Util.toByteArray(body.asInputStream()); + return Util.toByteArray(response.body().asInputStream()); } return super.decode(response, type); } diff --git a/core/src/main/java/feign/codec/ErrorDecoder.java b/core/src/main/java/feign/codec/ErrorDecoder.java index cd4d09834f..ec9e3b0641 100644 --- a/core/src/main/java/feign/codec/ErrorDecoder.java +++ b/core/src/main/java/feign/codec/ErrorDecoder.java @@ -35,25 +35,35 @@ /** * Allows you to massage an exception into a application-specific one. Converting out to a throttle - * exception are examples of this in use.
Ex.
+ * exception are examples of this in use. + * + *

Ex: * 

  * class IllegalArgumentExceptionOn404Decoder extends ErrorDecoder {
  *
  *   @Override
  *   public Exception decode(String methodKey, Response response) {
- *    if (response.status() == 404)
- *        throw new IllegalArgumentException("zone not found");
+ *    if (response.status() == 400)
+ *        throw new IllegalArgumentException("bad zone name");
  *    return ErrorDecoder.DEFAULT.decode(methodKey, request, response);
  *   }
  *
  * }
  *
- *
Error handling

Responses where {@link Response#status()} is not in the 2xx + * + *

Error handling + * + *

Responses where {@link Response#status()} is not in the 2xx * range are classified as errors, addressed by the {@link ErrorDecoder}. That said, certain RPC * apis return errors defined in the {@link Response#body()} even on a 200 status. For example, in * the DynECT api, a job still running condition is returned with a 200 status, encoded in json. * When scenarios like this occur, you should raise an application-specific exception (which may be * {@link feign.RetryableException retryable}). + * + *

Not Found Semantics + *

It is commonly the case that 404 (Not Found) status has semantic value in HTTP apis. While + * the default behavior is to raise exeception, users can alternatively enable 404 processing via + * {@link feign.Feign.Builder#decode404()}. */ public interface ErrorDecoder { diff --git a/core/src/test/java/feign/FeignBuilderTest.java b/core/src/test/java/feign/FeignBuilderTest.java index 44b47513d2..a172487131 100644 --- a/core/src/test/java/feign/FeignBuilderTest.java +++ b/core/src/test/java/feign/FeignBuilderTest.java @@ -33,7 +33,9 @@ import feign.codec.Encoder; import static feign.assertj.MockWebServerAssertions.assertThat; +import static org.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; public class FeignBuilderTest { @@ -54,6 +56,27 @@ public void testDefaults() throws Exception { .hasBody("request data"); } + /** Shows exception handling isn't required to coerce 404 to null or empty */ + @Test + public void testDecode404() throws Exception { + server.enqueue(new MockResponse().setResponseCode(404)); + server.enqueue(new MockResponse().setResponseCode(404)); + server.enqueue(new MockResponse().setResponseCode(400)); + + String url = "http://localhost:" + server.getPort(); + TestInterface api = Feign.builder().decode404().target(TestInterface.class, url); + + assertThat(api.getQueues("/")).isEmpty(); // empty, not null! + assertThat(api.decodedPost()).isNull(); // null, not empty! + + try { // ensure other 400 codes are not impacted. + api.decodedPost(); + failBecauseExceptionWasNotThrown(FeignException.class); + } catch (FeignException e) { + assertThat(e.status()).isEqualTo(400); + } + } + @Test public void testUrlPathConcatUrlTrailingSlash() throws Exception { server.enqueue(new MockResponse().setBody("response data")); @@ -147,8 +170,7 @@ public void apply(RequestTemplate template) { } }; - TestInterface - api = + TestInterface api = Feign.builder().requestInterceptor(requestInterceptor).target(TestInterface.class, url); Response response = api.codecPost("request data"); assertEquals(Util.toString(response.body().asReader()), "response data"); @@ -175,8 +197,7 @@ public InvocationHandler create(Target target, Map dispat } }; - TestInterface - api = + TestInterface api = Feign.builder().invocationHandlerFactory(factory).target(TestInterface.class, url); Response response = api.codecPost("request data"); assertEquals("response data", Util.toString(response.body().asReader())); @@ -192,9 +213,7 @@ public void testSlashIsEncodedInPathParams() throws Exception { String url = "http://localhost:" + server.getPort(); - TestInterface - api = - Feign.builder().target(TestInterface.class, url); + TestInterface api = Feign.builder().target(TestInterface.class, url); api.getQueues("/"); assertThat(server.takeRequest()) @@ -218,6 +237,6 @@ interface TestInterface { String decodedPost(); @RequestLine(value = "GET /api/queues/{vhost}", decodeSlash = false) - String getQueues(@Param("vhost") String vhost); + byte[] getQueues(@Param("vhost") String vhost); } } diff --git a/core/src/test/java/feign/FeignTest.java b/core/src/test/java/feign/FeignTest.java index 3cbfadcadb..bf1e53e91f 100644 --- a/core/src/test/java/feign/FeignTest.java +++ b/core/src/test/java/feign/FeignTest.java @@ -231,12 +231,12 @@ public void configKeyUsesChildType() throws Exception { @Test public void canOverrideErrorDecoder() throws Exception { - server.enqueue(new MockResponse().setResponseCode(404).setBody("foo")); + server.enqueue(new MockResponse().setResponseCode(400).setBody("foo")); thrown.expect(IllegalArgumentException.class); - thrown.expectMessage("zone not found"); + thrown.expectMessage("bad zone name"); TestInterface api = new TestInterfaceBuilder() - .errorDecoder(new IllegalArgumentExceptionOn404()) + .errorDecoder(new IllegalArgumentExceptionOn400()) .target("http://localhost:" + server.getPort()); api.post(); @@ -548,12 +548,12 @@ public void apply(RequestTemplate template) { } } - static class IllegalArgumentExceptionOn404 extends ErrorDecoder.Default { + static class IllegalArgumentExceptionOn400 extends ErrorDecoder.Default { @Override public Exception decode(String methodKey, Response response) { - if (response.status() == 404) { - return new IllegalArgumentException("zone not found"); + if (response.status() == 400) { + return new IllegalArgumentException("bad zone name"); } return super.decode(methodKey, response); } diff --git a/core/src/test/java/feign/UtilTest.java b/core/src/test/java/feign/UtilTest.java index 4998cc0ac2..ed6720f24a 100644 --- a/core/src/test/java/feign/UtilTest.java +++ b/core/src/test/java/feign/UtilTest.java @@ -19,19 +19,50 @@ import java.io.Reader; import java.lang.reflect.Type; +import java.util.Collection; +import java.util.Collections; +import java.util.Iterator; import java.util.List; +import java.util.Map; +import java.util.Set; import feign.codec.Decoder; import static feign.Util.resolveLastTypeParameter; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; public class UtilTest { + @Test + public void emptyValueOf() throws Exception { + assertEquals(false, Util.emptyValueOf(boolean.class)); + assertEquals(false, Util.emptyValueOf(Boolean.class)); + assertThat((byte[]) Util.emptyValueOf(byte[].class)).isEmpty(); + assertEquals(Collections.emptyList(), Util.emptyValueOf(Collection.class)); + assertThat((Iterator) Util.emptyValueOf(Iterator.class)).isEmpty(); + assertEquals(Collections.emptyList(), Util.emptyValueOf(List.class)); + assertEquals(Collections.emptyMap(), Util.emptyValueOf(Map.class)); + assertEquals(Collections.emptySet(), Util.emptyValueOf(Set.class)); + } + + /** In other words, {@code List} is as empty as {@code List}. */ + @Test + public void emptyValueOf_considersRawType() throws Exception { + Type listStringType = LastTypeParameter.class.getDeclaredField("LIST_STRING").getGenericType(); + assertThat((List) Util.emptyValueOf(listStringType)).isEmpty(); + } + + /** Ex. your {@code Foo} object would be null, but so would things like Number. */ + @Test + public void emptyValueOf_nullForUndefined() throws Exception { + assertThat(Util.emptyValueOf(Number.class)).isNull(); + assertThat(Util.emptyValueOf(Parameterized.class)).isNull(); + } + @Test public void resolveLastTypeParameterWhenNotSubtype() throws Exception { - Type - context = + Type context = LastTypeParameter.class.getDeclaredField("PARAMETERIZED_LIST_STRING").getGenericType(); Type listStringType = LastTypeParameter.class.getDeclaredField("LIST_STRING").getGenericType(); Type last = resolveLastTypeParameter(context, Parameterized.class); @@ -40,14 +71,14 @@ public void resolveLastTypeParameterWhenNotSubtype() throws Exception { @Test public void lastTypeFromInstance() throws Exception { - Parameterized instance = new ParameterizedSubtype(); + Parameterized instance = new ParameterizedSubtype(); Type last = resolveLastTypeParameter(instance.getClass(), Parameterized.class); assertEquals(String.class, last); } @Test public void lastTypeFromAnonymous() throws Exception { - Parameterized instance = new Parameterized() { + Parameterized instance = new Parameterized() { }; Type last = resolveLastTypeParameter(instance.getClass(), Parameterized.class); assertEquals(Reader.class, last); @@ -55,8 +86,7 @@ public void lastTypeFromAnonymous() throws Exception { @Test public void resolveLastTypeParameterWhenWildcard() throws Exception { - Type - context = + Type context = LastTypeParameter.class.getDeclaredField("PARAMETERIZED_WILDCARD_LIST_STRING") .getGenericType(); Type listStringType = LastTypeParameter.class.getDeclaredField("LIST_STRING").getGenericType(); @@ -66,8 +96,7 @@ public void resolveLastTypeParameterWhenWildcard() throws Exception { @Test public void resolveLastTypeParameterWhenParameterizedSubtype() throws Exception { - Type - context = + Type context = LastTypeParameter.class.getDeclaredField("PARAMETERIZED_DECODER_LIST_STRING") .getGenericType(); Type listStringType = LastTypeParameter.class.getDeclaredField("LIST_STRING").getGenericType(); @@ -77,15 +106,13 @@ public void resolveLastTypeParameterWhenParameterizedSubtype() throws Exception @Test public void unboundWildcardIsObject() throws Exception { - Type - context = + Type context = LastTypeParameter.class.getDeclaredField("PARAMETERIZED_DECODER_UNBOUND").getGenericType(); Type last = resolveLastTypeParameter(context, ParameterizedDecoder.class); assertEquals(Object.class, last); } interface LastTypeParameter { - final List LIST_STRING = null; final Parameterized> PARAMETERIZED_LIST_STRING = null; final Parameterized> PARAMETERIZED_WILDCARD_LIST_STRING = null; diff --git a/gson/src/main/java/feign/gson/GsonDecoder.java b/gson/src/main/java/feign/gson/GsonDecoder.java index cd5fcf4b2f..c56c73f5fe 100644 --- a/gson/src/main/java/feign/gson/GsonDecoder.java +++ b/gson/src/main/java/feign/gson/GsonDecoder.java @@ -25,6 +25,7 @@ import java.util.Collections; import feign.Response; +import feign.Util; import feign.codec.Decoder; import static feign.Util.ensureClosed; @@ -47,9 +48,8 @@ public GsonDecoder(Gson gson) { @Override public Object decode(Response response, Type type) throws IOException { - if (response.body() == null) { - return null; - } + if (response.status() == 404) return Util.emptyValueOf(type); + if (response.body() == null) return null; Reader reader = response.body().asReader(); try { return gson.fromJson(reader, type); diff --git a/gson/src/test/java/feign/gson/GsonCodecTest.java b/gson/src/test/java/feign/gson/GsonCodecTest.java index 5ed5a2b177..751a5b268c 100644 --- a/gson/src/test/java/feign/gson/GsonCodecTest.java +++ b/gson/src/test/java/feign/gson/GsonCodecTest.java @@ -210,4 +210,13 @@ public void customEncoder() throws Exception { + " }\n" // + "]"); } + + /** Enabled via {@link feign.Feign.Builder#decode404()} */ + @Test + public void notFoundDecodesToEmpty() throws Exception { + Response response = Response.create(404, "NOT FOUND", + Collections.>emptyMap(), + (byte[]) null); + assertThat((byte[]) new GsonDecoder().decode(response, byte[].class)).isEmpty(); + } } diff --git a/jackson-jaxb/src/main/java/feign/jackson/jaxb/JacksonJaxbJsonDecoder.java b/jackson-jaxb/src/main/java/feign/jackson/jaxb/JacksonJaxbJsonDecoder.java index 52bdf39dc9..1c9f772406 100644 --- a/jackson-jaxb/src/main/java/feign/jackson/jaxb/JacksonJaxbJsonDecoder.java +++ b/jackson-jaxb/src/main/java/feign/jackson/jaxb/JacksonJaxbJsonDecoder.java @@ -8,6 +8,7 @@ import feign.FeignException; import feign.Response; +import feign.Util; import feign.codec.Decoder; import static com.fasterxml.jackson.jaxrs.json.JacksonJaxbJsonProvider.DEFAULT_ANNOTATIONS; @@ -26,6 +27,8 @@ public JacksonJaxbJsonDecoder(ObjectMapper objectMapper) { @Override public Object decode(Response response, Type type) throws IOException, FeignException { + if (response.status() == 404) return Util.emptyValueOf(type); + if (response.body() == null) return null; return jacksonJaxbJsonProvider.readFrom(Object.class, type, null, APPLICATION_JSON_TYPE, null, response.body().asInputStream()); } } diff --git a/jackson-jaxb/src/test/java/feign/jackson/jaxb/JacksonJaxbCodecTest.java b/jackson-jaxb/src/test/java/feign/jackson/jaxb/JacksonJaxbCodecTest.java index 282f638a60..1a030869dc 100644 --- a/jackson-jaxb/src/test/java/feign/jackson/jaxb/JacksonJaxbCodecTest.java +++ b/jackson-jaxb/src/test/java/feign/jackson/jaxb/JacksonJaxbCodecTest.java @@ -38,6 +38,15 @@ public void decodeTest() throws Exception { .isEqualTo(new MockObject("Test")); } + /** Enabled via {@link feign.Feign.Builder#decode404()} */ + @Test + public void notFoundDecodesToEmpty() throws Exception { + Response response = Response.create(404, "NOT FOUND", + Collections.>emptyMap(), + (byte[]) null); + assertThat((byte[]) new JacksonJaxbJsonDecoder().decode(response, byte[].class)).isEmpty(); + } + @XmlRootElement @XmlAccessorType(XmlAccessType.FIELD) static class MockObject { diff --git a/jackson/src/main/java/feign/jackson/JacksonDecoder.java b/jackson/src/main/java/feign/jackson/JacksonDecoder.java index 1a2cb7821c..a907c9cf3d 100644 --- a/jackson/src/main/java/feign/jackson/JacksonDecoder.java +++ b/jackson/src/main/java/feign/jackson/JacksonDecoder.java @@ -27,6 +27,7 @@ import java.util.Collections; import feign.Response; +import feign.Util; import feign.codec.Decoder; public class JacksonDecoder implements Decoder { @@ -48,9 +49,8 @@ public JacksonDecoder(ObjectMapper mapper) { @Override public Object decode(Response response, Type type) throws IOException { - if (response.body() == null) { - return null; - } + if (response.status() == 404) return Util.emptyValueOf(type); + if (response.body() == null) return null; Reader reader = response.body().asReader(); if (!reader.markSupported()) { reader = new BufferedReader(reader, 1); diff --git a/jackson/src/test/java/feign/jackson/JacksonCodecTest.java b/jackson/src/test/java/feign/jackson/JacksonCodecTest.java index bcb098cba2..c4c3f798c1 100644 --- a/jackson/src/test/java/feign/jackson/JacksonCodecTest.java +++ b/jackson/src/test/java/feign/jackson/JacksonCodecTest.java @@ -201,4 +201,13 @@ public void serialize(Zone value, JsonGenerator jgen, SerializerProvider provide jgen.writeEndObject(); } } + + /** Enabled via {@link feign.Feign.Builder#decode404()} */ + @Test + public void notFoundDecodesToEmpty() throws Exception { + Response response = Response.create(404, "NOT FOUND", + Collections.>emptyMap(), + (byte[]) null); + assertThat((byte[]) new JacksonDecoder().decode(response, byte[].class)).isEmpty(); + } } diff --git a/jaxb/src/main/java/feign/jaxb/JAXBDecoder.java b/jaxb/src/main/java/feign/jaxb/JAXBDecoder.java index 3e593ff695..3a97db27e0 100644 --- a/jaxb/src/main/java/feign/jaxb/JAXBDecoder.java +++ b/jaxb/src/main/java/feign/jaxb/JAXBDecoder.java @@ -22,6 +22,7 @@ import javax.xml.bind.Unmarshaller; import feign.Response; +import feign.Util; import feign.codec.DecodeException; import feign.codec.Decoder; @@ -50,6 +51,8 @@ public JAXBDecoder(JAXBContextFactory jaxbContextFactory) { @Override public Object decode(Response response, Type type) throws IOException { + if (response.status() == 404) return Util.emptyValueOf(type); + if (response.body() == null) return null; if (!(type instanceof Class)) { throw new UnsupportedOperationException( "JAXB only supports decoding raw types. Found " + type); diff --git a/jaxb/src/test/java/feign/jaxb/JAXBCodecTest.java b/jaxb/src/test/java/feign/jaxb/JAXBCodecTest.java index bf8f395492..7070c023de 100644 --- a/jaxb/src/test/java/feign/jaxb/JAXBCodecTest.java +++ b/jaxb/src/test/java/feign/jaxb/JAXBCodecTest.java @@ -198,6 +198,16 @@ class ParameterizedHolder { new JAXBDecoder(new JAXBContextFactory.Builder().build()).decode(response, parameterized); } + /** Enabled via {@link feign.Feign.Builder#decode404()} */ + @Test + public void notFoundDecodesToEmpty() throws Exception { + Response response = Response.create(404, "NOT FOUND", + Collections.>emptyMap(), + (byte[]) null); + assertThat((byte[]) new JAXBDecoder(new JAXBContextFactory.Builder().build()) + .decode(response, byte[].class)).isEmpty(); + } + @XmlRootElement @XmlAccessorType(XmlAccessType.FIELD) static class MockObject { diff --git a/sax/src/main/java/feign/sax/SAXDecoder.java b/sax/src/main/java/feign/sax/SAXDecoder.java index a0af0fd2ad..0da7d40f08 100644 --- a/sax/src/main/java/feign/sax/SAXDecoder.java +++ b/sax/src/main/java/feign/sax/SAXDecoder.java @@ -29,6 +29,7 @@ import java.util.Map; import feign.Response; +import feign.Util; import feign.codec.DecodeException; import feign.codec.Decoder; @@ -63,9 +64,8 @@ public static Builder builder() { @Override public Object decode(Response response, Type type) throws IOException, DecodeException { - if (response.body() == null) { - return null; - } + if (response.status() == 404) return Util.emptyValueOf(type); + if (response.body() == null) return null; ContentHandlerWithResult.Factory handlerFactory = handlerFactories.get(type); checkState(handlerFactory != null, "type %s not in configured handlers %s", type, handlerFactories.keySet()); diff --git a/sax/src/test/java/feign/sax/SAXDecoderTest.java b/sax/src/test/java/feign/sax/SAXDecoderTest.java index 063018ab7e..5973a2e95c 100644 --- a/sax/src/test/java/feign/sax/SAXDecoderTest.java +++ b/sax/src/test/java/feign/sax/SAXDecoderTest.java @@ -29,6 +29,7 @@ import feign.codec.Decoder; import static feign.Util.UTF_8; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; @@ -79,13 +80,21 @@ private Response statusFailedResponse() { @Test public void nullBodyDecodesToNull() throws Exception { - Response - response = + Response response = Response .create(204, "OK", Collections.>emptyMap(), (byte[]) null); assertNull(decoder.decode(response, String.class)); } + /** Enabled via {@link feign.Feign.Builder#decode404()} */ + @Test + public void notFoundDecodesToEmpty() throws Exception { + Response response = Response.create(404, "NOT FOUND", + Collections.>emptyMap(), + (byte[]) null); + assertThat((byte[]) decoder.decode(response, byte[].class)).isEmpty(); + } + static enum NetworkStatus { GOOD, FAILED; }