Do not error on interfaces with default methods.

Dan Jasek · Adrian Cole · commit 70f0d97007ea · 2016-04-02T09:07:17.000+08:00
Pass calls to default methods on proxy through to implementation on interface.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,6 @@
 ### Version 8.16
 * Adds `@HeaderMap` annotation to support dynamic header fields and values
+* Add support for default and static methods on interfaces
 
 ### Version 8.15
 * Adds `@QueryMap` annotation to support dynamic query parameters
diff --git a/README.md b/README.md
@@ -428,3 +428,39 @@ A Map parameter can be annotated with `QueryMap` to construct a query that uses
 @RequestLine("GET /find")
 V find(@QueryMap Map<String, Object> queryMap);
 ```
+
+#### Static and Default Methods
+Interfaces targeted by Feign may have static or default methods (if using Java 8+).
+These allows Feign clients to contain logic that is not expressly defined by the underlying API.
+For example, static methods make it easy to specify common client build configurations; default methods can be used to compose queries or define default parameters.
+
+```java
+interface GitHub {
+  @RequestLine("GET /repos/{owner}/{repo}/contributors")
+  List<Contributor> contributors(@Param("owner") String owner, @Param("repo") String repo);
+
+  @RequestLine("GET /users/{username}/repos?sort={sort}")
+  List<Repo> repos(@Param("username") String owner, @Param("sort") String sort);
+
+  default List<Repo> repos(String owner) {
+    return repos(owner, "full_name");
+  }
+
+  /**
+   * Lists all contributors for all repos owned by a user.
+   */
+  default List<Contributor> contributors(String user) {
+    MergingContributorList contributors = new MergingContributorList();
+    for(Repo repo : this.repos(owner)) {
+      contributors.addAll(this.contributors(user, repo.getName()));
+    }
+    return contributors.mergeResult();
+  }
+
+  static GitHub connect() {
+    return Feign.builder()
+                .decoder(new GsonDecoder())
+                .target(GitHub.class, "https://api.github.com");
+  }
+}
+```
diff --git a/core/build.gradle b/core/build.gradle
@@ -4,6 +4,7 @@ apply plugin: 'osgi'
 sourceCompatibility = 1.6
 
 dependencies {
+    compile 'org.jvnet:animal-sniffer-annotation:1.0'
     testCompile 'junit:junit:4.12'
     testCompile 'org.assertj:assertj-core:1.7.1' // last version supporting JDK 7
     testCompile 'com.squareup.okhttp:mockwebserver:2.7.5'
diff --git a/core/src/main/java/feign/Contract.java b/core/src/main/java/feign/Contract.java
@@ -57,7 +57,8 @@ public List<MethodMetadata> parseAndValidatateMetadata(Class<?> targetType) {
       Map<String, MethodMetadata> result = new LinkedHashMap<String, MethodMetadata>();
       for (Method method : targetType.getMethods()) {
         if (method.getDeclaringClass() == Object.class ||
-            (method.getModifiers() & Modifier.STATIC) != 0) {
+            (method.getModifiers() & Modifier.STATIC) != 0 ||
+            Util.isDefault(method)) {
           continue;
         }
         MethodMetadata metadata = parseAndValidateMetadata(targetType, method);
diff --git a/core/src/main/java/feign/DefaultMethodHandler.java b/core/src/main/java/feign/DefaultMethodHandler.java
@@ -0,0 +1,62 @@
+package feign;
+
+import feign.InvocationHandlerFactory.MethodHandler;
+import org.jvnet.animal_sniffer.IgnoreJRERequirement;
+
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles.Lookup;
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+
+/**
+ * Handles default methods by directly invoking the default method code on the interface.
+ * The bindTo method must be called on the result before invoke is called.
+ */
+@IgnoreJRERequirement
+final class DefaultMethodHandler implements MethodHandler {
+  // Uses Java 7 MethodHandle based reflection.  As default methods will only exist when
+  // run on a Java 8 JVM this will not affect use on legacy JVMs.
+  // When Feign upgrades to Java 7, remove the @IgnoreJRERequirement annotation.
+  private final MethodHandle unboundHandle;
+
+  // handle is effectively final after bindTo has been called.
+  private MethodHandle handle;
+
+  public DefaultMethodHandler(Method defaultMethod) {
+    try {
+      Class<?> declaringClass = defaultMethod.getDeclaringClass();
+      Field field = Lookup.class.getDeclaredField("IMPL_LOOKUP");
+      field.setAccessible(true);
+      Lookup lookup = (Lookup) field.get(null);
+
+      this.unboundHandle = lookup.unreflectSpecial(defaultMethod, declaringClass);
+    } catch (NoSuchFieldException ex) {
+      throw new IllegalStateException(ex);
+    } catch (IllegalAccessException ex) {
+      throw new IllegalStateException(ex);
+    }
+  }
+
+  /**
+   * Bind this handler to a proxy object.  After bound, DefaultMethodHandler#invoke will act as if it was called
+   * on the proxy object.  Must be called once and only once for a given instance of DefaultMethodHandler
+   */
+  public void bindTo(Object proxy) {
+    if(handle != null) {
+      throw new IllegalStateException("Attempted to rebind a default method handler that was already bound");
+    }
+    handle = unboundHandle.bindTo(proxy);
+  }
+
+  /**
+   * Invoke this method.  DefaultMethodHandler#bindTo must be called before the first
+   * time invoke is called.
+   */
+  @Override
+  public Object invoke(Object[] argv) throws Throwable {
+    if(handle == null) {
+      throw new IllegalStateException("Default method handler invoked before proxy has been bound.");
+    }
+    return handle.invokeWithArguments(argv);
+  }
+}
diff --git a/core/src/main/java/feign/ReflectiveFeign.java b/core/src/main/java/feign/ReflectiveFeign.java
@@ -18,12 +18,7 @@
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
 import java.lang.reflect.Proxy;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Iterator;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 import java.util.Map.Entry;
 
 import feign.InvocationHandlerFactory.MethodHandler;
@@ -57,15 +52,26 @@ public class ReflectiveFeign extends Feign {
   public <T> T newInstance(Target<T> target) {
     Map<String, MethodHandler> nameToHandler = targetToHandlersByName.apply(target);
     Map<Method, MethodHandler> methodToHandler = new LinkedHashMap<Method, MethodHandler>();
+    List<DefaultMethodHandler> defaultMethodHandlers = new LinkedList<DefaultMethodHandler>();
+
     for (Method method : target.type().getMethods()) {
       if (method.getDeclaringClass() == Object.class) {
         continue;
+      } else if(Util.isDefault(method)) {
+        DefaultMethodHandler handler = new DefaultMethodHandler(method);
+        defaultMethodHandlers.add(handler);
+        methodToHandler.put(method, handler);
+      } else {
+        methodToHandler.put(method, nameToHandler.get(Feign.configKey(target.type(), method)));
       }
-      methodToHandler.put(method, nameToHandler.get(Feign.configKey(target.type(), method)));
     }
     InvocationHandler handler = factory.create(target, methodToHandler);
-    return (T) Proxy
-        .newProxyInstance(target.type().getClassLoader(), new Class<?>[]{target.type()}, handler);
+    T proxy = (T) Proxy.newProxyInstance(target.type().getClassLoader(), new Class<?>[]{target.type()}, handler);
+
+    for(DefaultMethodHandler defaultMethodHandler : defaultMethodHandlers) {
+      defaultMethodHandler.bindTo(proxy);
+    }
+    return proxy;
   }
 
   static class FeignInvocationHandler implements InvocationHandler {
diff --git a/core/src/main/java/feign/Util.java b/core/src/main/java/feign/Util.java
@@ -22,6 +22,8 @@
 import java.io.OutputStream;
 import java.io.Reader;
 import java.lang.reflect.Array;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.lang.reflect.WildcardType;
@@ -127,6 +129,19 @@ public static void checkState(boolean expression,
     }
   }
 
+  /**
+   * Identifies a method as a default instance method.
+   */
+  public static boolean isDefault(Method method) {
+    // Default methods are public non-abstract, non-synthetic, and non-static instance methods
+    // declared in an interface.
+    // method.isDefault() is not sufficient for our usage as it does not check
+    // for synthetic methods.  As a result, it picks up overridden methods as well as actual default methods.
+    final int SYNTHETIC = 0x00001000;
+    return ((method.getModifiers() & (Modifier.ABSTRACT | Modifier.PUBLIC | Modifier.STATIC | SYNTHETIC)) ==
+            Modifier.PUBLIC) && method.getDeclaringClass().isInterface();
+  }
+
   /**
    * Adapted from {@code com.google.common.base.Strings#emptyToNull}.
    */
diff --git a/core/src/test/java/feign/DefaultContractTest.java b/core/src/test/java/feign/DefaultContractTest.java
@@ -710,4 +710,21 @@ public void staticMethodsOnInterfaceIgnored() throws Exception {
     MethodMetadata md = mds.get(0);
     assertThat(md.configKey()).isEqualTo("StaticMethodOnInterface#get(String)");
   }
+
+  interface DefaultMethodOnInterface {
+    @RequestLine("GET /api/{key}")
+    String get(@Param("key") String key);
+
+    default String defaultGet(String key) {
+      return get(key);
+    }
+  }
+
+  @Test
+  public void defaultMethodsOnInterfaceIgnored() throws Exception {
+    List<MethodMetadata> mds = contract.parseAndValidatateMetadata(DefaultMethodOnInterface.class);
+    assertThat(mds).hasSize(1);
+    MethodMetadata md = mds.get(0);
+    assertThat(md.configKey()).isEqualTo("DefaultMethodOnInterface#get(String)");
+  }
 }
diff --git a/core/src/test/java/feign/FeignBuilderTest.java b/core/src/test/java/feign/FeignBuilderTest.java
@@ -220,6 +220,28 @@ public void testSlashIsEncodedInPathParams() throws Exception {
         .hasPath("/api/queues/%2F");
   }
 
+  @Test
+  public void testBasicDefaultMethod() throws Exception {
+    String url = "http://localhost:" + server.getPort();
+
+    TestInterface api = Feign.builder().target(TestInterface.class, url);
+    String result = api.independentDefaultMethod();
+
+    assertThat(result.equals("default result"));
+  }
+
+  @Test
+  public void testDefaultCallingProxiedMethod() throws Exception {
+    server.enqueue(new MockResponse().setBody("response data"));
+
+    String url = "http://localhost:" + server.getPort();
+    TestInterface api = Feign.builder().target(TestInterface.class, url);
+
+    Response response = api.defaultMethodPassthrough();
+    assertEquals("response data", Util.toString(response.body().asReader()));
+    assertThat(server.takeRequest()).hasPath("/");
+  }
+
   interface TestInterface {
     @RequestLine("GET")
     Response getNoPath();
@@ -238,5 +260,13 @@ interface TestInterface {
     
     @RequestLine(value = "GET /api/queues/{vhost}", decodeSlash = false)
     byte[] getQueues(@Param("vhost") String vhost);
+
+    default String independentDefaultMethod() {
+      return "default result";
+    }
+
+    default Response defaultMethodPassthrough() {
+      return getNoPath();
+    }
   }
 }
