Adds Feign.Builder.decode404() to reduce boilerplate for empty semantics

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