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<X>`, but the general pattern applies
to anything that has a type representing both success and failure, such
as `Try<X>` or `Observable<X>`.

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

@@ -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.
 

core/src/main/java/feign/Feign.java

@@ -82,8 +82,7 @@ public static String configKey(Method method) {
 
   public static class Builder {
 
-    private final List<RequestInterceptor>
-        requestInterceptors =
+    private final List<RequestInterceptor> requestInterceptors =
         new ArrayList<RequestInterceptor>();
     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}.
+     *
+     * <p/> 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.
+     *
+     * <p/> 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> T target(Target<T> 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);

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<RequestInterceptor> 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<RequestInterceptor> requestInterceptors;
     private final Logger logger;
     private final Logger.Level logLevel;
+    private final boolean decode404;
 
     Factory(Client client, Retryer retryer, List<RequestInterceptor> 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);
     }
   }
 }

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.
+   *
+   * <ul>
+   *   <li>{@code [Bb]oolean}</li>
+   *   <li>{@code byte[]}</li>
+   *   <li>{@code Collection}</li>
+   *   <li>{@code Iterator}</li>
+   *   <li>{@code List}</li>
+   *   <li>{@code Map}</li>
+   *   <li>{@code Set}</li>
+   * </ul>
+   *
+   * <p/> 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<Class<?>, Object> EMPTIES;
+  static {
+    Map<Class<?>, Object> empties = new LinkedHashMap<Class<?>, 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<Object>() { // 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()}.
    */

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);
     }

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. <br> Ex. <br>
+ * exception are examples of this in use.
+ *
+ * <p/>Ex:
  * <pre>
  * class IllegalArgumentExceptionOn404Decoder extends ErrorDecoder {
  *
  *   &#064;Override
  *   public Exception decode(String methodKey, Response response) {
- *    if (response.status() == 404)
- *        throw new IllegalArgumentException(&quot;zone not found&quot;);
+ *    if (response.status() == 400)
+ *        throw new IllegalArgumentException(&quot;bad zone name&quot;);
  *    return ErrorDecoder.DEFAULT.decode(methodKey, request, response);
  *   }
  *
  * }
  * </pre>
- * <br> <b>Error handling</b><br> <br> Responses where {@link Response#status()} is not in the 2xx
+ *
+ * <p/><b>Error handling</b>
+ *
+ * <p/>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}).
+ *
+ * <p/><b>Not Found Semantics</b>
+ * <p/> 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 {
 

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<Method, MethodHandler> 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);
   }
 }

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);
     }