Multi Level Multiple Inheritance and Override Support

core/src/main/java/feign/Contract.java

@@ -13,15 +13,23 @@
  */
 package feign;
 
-import static feign.Util.checkState;
-import static feign.Util.emptyToNull;
+import feign.Request.HttpMethod;
 import java.lang.annotation.Annotation;
-import java.lang.reflect.*;
+import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
+import java.lang.reflect.Parameter;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
 import java.net.URI;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
-import feign.Request.HttpMethod;
+import static feign.Util.checkState;
+import static feign.Util.emptyToNull;
 
 /**
  * Defines what annotations and values are valid on interfaces.
@@ -47,11 +55,6 @@ public List<MethodMetadata> parseAndValidateMetadata(Class<?> targetType) {
           targetType.getSimpleName());
       checkState(targetType.getInterfaces().length <= 1, "Only single inheritance supported: %s",
           targetType.getSimpleName());
-      if (targetType.getInterfaces().length == 1) {
-        checkState(targetType.getInterfaces()[0].getInterfaces().length == 0,
-            "Only single-level inheritance supported: %s",
-            targetType.getSimpleName());
-      }
       final Map<String, MethodMetadata> result = new LinkedHashMap<String, MethodMetadata>();
       for (final Method method : targetType.getMethods()) {
         if (method.getDeclaringClass() == Object.class ||
@@ -60,8 +63,16 @@ public List<MethodMetadata> parseAndValidateMetadata(Class<?> targetType) {
           continue;
         }
         final MethodMetadata metadata = parseAndValidateMetadata(targetType, method);
-        checkState(!result.containsKey(metadata.configKey()), "Overrides unsupported: %s",
-            metadata.configKey());
+        if (result.containsKey(metadata.configKey())) {
+          MethodMetadata existingMetadata = result.get(metadata.configKey());
+          Type existingReturnType = existingMetadata.returnType();
+          Type overridingReturnType = metadata.returnType();
+          Type resolvedType = Types.resolveReturnType(existingReturnType, overridingReturnType);
+          if (resolvedType.equals(overridingReturnType)) {
+            result.put(metadata.configKey(), metadata);
+          }
+          continue;
+        }
         result.put(metadata.configKey(), metadata);
       }
       return new ArrayList<>(result.values());
@@ -82,7 +93,8 @@ protected MethodMetadata parseAndValidateMetadata(Class<?> targetType, Method me
       final MethodMetadata data = new MethodMetadata();
       data.targetType(targetType);
       data.method(method);
-      data.returnType(Types.resolve(targetType, targetType, method.getGenericReturnType()));
+      data.returnType(
+          Types.resolve(targetType, targetType, method.getGenericReturnType()));
       data.configKey(Feign.configKey(targetType, method));
 
       if (targetType.getInterfaces().length == 1) {
@@ -127,7 +139,8 @@ protected MethodMetadata parseAndValidateMetadata(Class<?> targetType, Method me
             checkState(data.bodyIndex() == null,
                 "Method has too many Body parameters: %s%s", method, data.warnings());
             data.bodyIndex(i);
-            data.bodyType(Types.resolve(targetType, targetType, genericParameterTypes[i]));
+            data.bodyType(
+                Types.resolve(targetType, targetType, genericParameterTypes[i]));
           }
         }
       }
@@ -162,15 +175,13 @@ private static void checkMapKeys(String name, Type genericType) {
       } else if (genericType instanceof Class<?>) {
         // raw class, type parameters cannot be inferred directly, but we can scan any extended
         // interfaces looking for any explict types
-        final Type[] interfaces = ((Class) genericType).getGenericInterfaces();
-        if (interfaces != null) {
-          for (final Type extended : interfaces) {
-            if (ParameterizedType.class.isAssignableFrom(extended.getClass())) {
-              // use the first extended interface we find.
-              final Type[] parameterTypes = ((ParameterizedType) extended).getActualTypeArguments();
-              keyClass = (Class<?>) parameterTypes[0];
-              break;
-            }
+        final Type[] interfaces = ((Class<?>) genericType).getGenericInterfaces();
+        for (final Type extended : interfaces) {
+          if (ParameterizedType.class.isAssignableFrom(extended.getClass())) {
+            // use the first extended interface we find.
+            final Type[] parameterTypes = ((ParameterizedType) extended).getActualTypeArguments();
+            keyClass = (Class<?>) parameterTypes[0];
+            break;
           }
         }
       }
@@ -316,6 +327,5 @@ private static Map<String, Collection<String>> toMap(String[] input) {
       }
       return result;
     }
-
   }
 }

core/src/main/java/feign/DeclarativeContract.java

@@ -13,13 +13,13 @@
  */
 package feign;
 
+import feign.Contract.BaseContract;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import java.lang.reflect.Parameter;
 import java.util.*;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
-import feign.Contract.BaseContract;
 
 /**
  * {@link Contract} base implementation that works by declaring witch annotations should be

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

@@ -13,13 +13,6 @@
  */
 package feign;
 
-import java.io.IOException;
-import java.lang.reflect.Method;
-import java.lang.reflect.Type;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.stream.Collectors;
-import feign.Logger.Level;
 import feign.Logger.NoOpLogger;
 import feign.ReflectiveFeign.ParseHandlersByName;
 import feign.Request.Options;
@@ -28,6 +21,12 @@
 import feign.codec.Encoder;
 import feign.codec.ErrorDecoder;
 import feign.querymap.FieldQueryMapEncoder;
+import java.io.IOException;
+import java.lang.reflect.Method;
+import java.lang.reflect.Type;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.stream.Collectors;
 import static feign.ExceptionPropagationPolicy.NONE;
 
 /**

core/src/main/java/feign/Types.java

@@ -306,6 +306,25 @@ private static void checkNotPrimitive(Type type) {
     }
   }
 
+  public static Type resolveReturnType(Type baseType, Type overridingType) {
+    if (baseType instanceof Class && overridingType instanceof Class &&
+        ((Class<?>) baseType).isAssignableFrom((Class<?>) overridingType)) {
+      // NOTE: javac generates multiple same methods for multiple inherited generic interfaces
+      return overridingType;
+    }
+    if (baseType instanceof Class && overridingType instanceof ParameterizedType) {
+      // NOTE: javac will generate multiple methods with different return types
+      // base interface declares generic method, override declares parameterized generic method
+      return overridingType;
+    }
+    if (baseType instanceof Class && overridingType instanceof TypeVariable) {
+      // NOTE: javac will generate multiple methods with different return types
+      // base interface declares non generic method, override declares generic method
+      return overridingType;
+    }
+    return baseType;
+  }
+
   static final class ParameterizedTypeImpl implements ParameterizedType {
 
     private final Type ownerType;

core/src/test/java/feign/DefaultContractInheritanceTest.java

@@ -0,0 +1,181 @@
+/**
+ * Copyright 2012-2020 The Feign Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package feign;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import java.util.List;
+import static feign.assertj.FeignAssertions.assertThat;
+import static java.util.Arrays.asList;
+import static org.assertj.core.data.MapEntry.entry;
+
+/**
+ * Tests interface inheritance defined per {@link Contract.Default} are interpreted into expected
+ * {@link feign .RequestTemplate template} instances.
+ */
+public class DefaultContractInheritanceTest {
+
+  @Rule
+  public final ExpectedException thrown = ExpectedException.none();
+
+  Contract.Default contract = new Contract.Default();
+
+  @Headers("Foo: Bar")
+  interface SimpleParameterizedBaseApi<M> {
+
+    @RequestLine("GET /api/{zoneId}")
+    M get(@Param("key") String key);
+  }
+
+  interface SimpleParameterizedApi extends SimpleParameterizedBaseApi<String> {
+
+  }
+
+  @Test
+  public void simpleParameterizedBaseApi() throws Exception {
+    final List<MethodMetadata> md = contract.parseAndValidateMetadata(SimpleParameterizedApi.class);
+
+    assertThat(md).hasSize(1);
+
+    assertThat(md.get(0).configKey())
+        .isEqualTo("SimpleParameterizedApi#get(String)");
+    assertThat(md.get(0).returnType())
+        .isEqualTo(String.class);
+    assertThat(md.get(0).template())
+        .hasHeaders(entry("Foo", asList("Bar")));
+  }
+
+  @Test
+  public void parameterizedApiUnsupported() throws Exception {
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("Parameterized types unsupported: SimpleParameterizedBaseApi");
+    contract.parseAndValidateMetadata(SimpleParameterizedBaseApi.class);
+  }
+
+  interface OverrideParameterizedApi extends SimpleParameterizedBaseApi<String> {
+
+    @Override
+    @RequestLine("GET /api/{zoneId}")
+    String get(@Param("key") String key);
+  }
+
+  @Headers("Foo: Bar")
+  interface SimpleBaseApi {
+
+    @RequestLine("GET /api/{zoneId}")
+    String get(@Param("key") String key);
+  }
+
+  interface OverrideSimpleApi extends SimpleBaseApi {
+
+    @RequestLine("GET /api/v2/{zoneId}")
+    String get(@Param("key") String key);
+  }
+
+  @Test
+  public void overrideParameterizedApiSupported() {
+    contract.parseAndValidateMetadata(OverrideParameterizedApi.class);
+  }
+
+  @Test
+  public void overrideSimpleApiSupported() {
+    contract.parseAndValidateMetadata(OverrideSimpleApi.class);
+  }
+
+  interface Child<T> extends SimpleParameterizedBaseApi<List<T>> {
+
+  }
+
+  interface GrandChild extends Child<String> {
+
+  }
+
+  interface SingleInheritanceChild extends SimpleParameterizedBaseApi<String> {
+  }
+
+  interface FirstServiceBaseApi<T> {
+    T get(String key);
+  }
+
+  interface SecondServiceBaseApi<T> {
+    void update(String key, T value);
+  }
+
+  interface MultipleInheritanceDoneWrong
+      extends SimpleParameterizedBaseApi<String>, FirstServiceBaseApi<String>,
+      SecondServiceBaseApi<String> {
+  }
+
+  @Headers("Foo: Bar")
+  interface MultipleInheritanceParameterizedBaseApi<T>
+      extends FirstServiceBaseApi<T>, SecondServiceBaseApi<T> {
+
+    @Override
+    @RequestLine("GET /api/{zoneId}")
+    T get(@Param("key") String key);
+
+    @Override
+    @RequestLine("POST /api/{zoneId}")
+    @Body("%7B\"value\": \"{value}\"%7D")
+    void update(@Param("key") String key, @Param("value") T value);
+  }
+
+  interface MultipleInheritanceDoneCorrectly
+      extends MultipleInheritanceParameterizedBaseApi<String> {
+  }
+
+  interface ServiceApi<T> extends FirstServiceBaseApi<T>, SecondServiceBaseApi<T> {
+  }
+
+  @Headers("Foo: Bar")
+  interface MultipleInheritanceApi extends ServiceApi<String> {
+
+    @Override
+    @RequestLine("GET /api/{zoneId}")
+    String get(@Param("key") String key);
+
+    @Override
+    @RequestLine("POST /api/{zoneId}")
+    @Body("%7B\"value\": \"{value}\"%7D")
+    void update(@Param("key") String key, @Param("value") String value);
+  }
+
+  @Test
+  public void singleInheritance() {
+    contract.parseAndValidateMetadata(SingleInheritanceChild.class);
+  }
+
+  @Test
+  public void multipleInheritanceDoneWrong() {
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("Only single inheritance supported: MultipleInheritanceDoneWrong");
+    contract.parseAndValidateMetadata(MultipleInheritanceDoneWrong.class);
+  }
+
+  @Test
+  public void multipleInheritanceDoneCorrectly() {
+    contract.parseAndValidateMetadata(MultipleInheritanceDoneCorrectly.class);
+  }
+
+  @Test
+  public void multipleInheritanceDoneCorrectly2() {
+    contract.parseAndValidateMetadata(MultipleInheritanceApi.class);
+  }
+
+  @Test
+  public void multipleInheritanceSupported() {
+    contract.parseAndValidateMetadata(GrandChild.class);
+  }
+}