Skip to content

Commit

Permalink
allow multiple inheritance on the interface level with current restri…
Browse files Browse the repository at this point in the history
…ctions intact.
  • Loading branch information
silkentrance committed Feb 17, 2021
1 parent ee8d517 commit 7f0d9e1
Show file tree
Hide file tree
Showing 11 changed files with 843 additions and 117 deletions.
58 changes: 34 additions & 24 deletions core/src/main/java/feign/Contract.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 ||
Expand All @@ -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());
Expand All @@ -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) {
Expand Down Expand Up @@ -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]));
}
}
}
Expand Down Expand Up @@ -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;
}
}
}
Expand Down Expand Up @@ -316,6 +327,5 @@ private static Map<String, Collection<String>> toMap(String[] input) {
}
return result;
}

}
}
2 changes: 1 addition & 1 deletion core/src/main/java/feign/DeclarativeContract.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 6 additions & 7 deletions core/src/main/java/feign/Feign.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

/**
Expand Down
19 changes: 19 additions & 0 deletions core/src/main/java/feign/Types.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
181 changes: 181 additions & 0 deletions core/src/test/java/feign/DefaultContractInheritanceTest.java
Original file line number Diff line number Diff line change
@@ -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);
}
}

0 comments on commit 7f0d9e1

Please sign in to comment.