diff --git a/CHANGELOG.md b/CHANGELOG.md index e6276c3c..f2323dbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +### Version 8.6 +* Adds base api support via single-inheritance interfaces + ### Version 7.5/8.5 * Added possibility to leave slash encoded in path parameters diff --git a/README.md b/README.md index 2dab743d..11567fc0 100644 --- a/README.md +++ b/README.md @@ -238,6 +238,50 @@ client.json("denominator", "secret"); // {"user_name": "denominator", "password" ### Advanced usage +#### Base Apis +In many cases, apis for a service follow the same conventions. Feign supports this pattern via single-inheritance interfaces. + +Consider the example: +```java +interface BaseAPI { + @RequestLine("GET /health") + String health(); + + @RequestLine("GET /all") + List all(); +} +``` + +You can define and target a specific api, inheriting the base methods. +```java +interface CustomAPI extends BaseAPI { + @RequestLine("GET /custom") + String custom(); +} +``` + +In many cases, resource representations are also consistent. For this reason, type parameters are supported on the base api interface. + +```java +@Headers("Accept: application/json") +interface BaseApi { + + @RequestLine("GET /api/{key}") + V get(@Param("key") String); + + @RequestLine("GET /api") + List list(); + + @Headers("Content-Type: application/json") + @RequestLine("PUT /api/{key}") + void put(@Param("key") String, V value); +} + +interface FooApi extends BaseApi { } + +interface BarApi extends BaseApi { } +``` + #### Logging You can log the http messages going to and from the target by setting up a `Logger`. Here's the easiest way to do that: ```java diff --git a/core/src/main/java/feign/Contract.java b/core/src/main/java/feign/Contract.java index 6d7d8959..dbbc64b2 100644 --- a/core/src/main/java/feign/Contract.java +++ b/core/src/main/java/feign/Contract.java @@ -34,30 +34,53 @@ public interface Contract { /** * Called to parse the methods in the class that are linked to HTTP requests. + * + * @param targetType {@link feign.Target#type() type} of the Feign interface. */ - List parseAndValidatateMetadata(Class declaring); + // TODO: break this and correct spelling at some point + List parseAndValidatateMetadata(Class targetType); abstract class BaseContract implements Contract { @Override - public List parseAndValidatateMetadata(Class declaring) { - List metadata = new ArrayList(); - for (Method method : declaring.getDeclaredMethods()) { + public List parseAndValidatateMetadata(Class targetType) { + checkState(targetType.getTypeParameters().length == 0, "Parameterized types unsupported: %s", + 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()); + } + Map result = new LinkedHashMap(); + for (Method method : targetType.getMethods()) { if (method.getDeclaringClass() == Object.class) { continue; } - metadata.add(parseAndValidatateMetadata(method)); + MethodMetadata metadata = parseAndValidateMetadata(targetType, method); + checkState(!result.containsKey(metadata.configKey()), "Overrides unsupported: %s", + metadata.configKey()); + result.put(metadata.configKey(), metadata); } - return metadata; + return new ArrayList(result.values()); } /** - * Called indirectly by {@link #parseAndValidatateMetadata(Class)}. + * @deprecated use {@link #parseAndValidateMetadata(Class, Method)} instead. */ + @Deprecated public MethodMetadata parseAndValidatateMetadata(Method method) { + return parseAndValidateMetadata(method.getDeclaringClass(), method); + } + + /** + * Called indirectly by {@link #parseAndValidatateMetadata(Class)}. + */ + protected MethodMetadata parseAndValidateMetadata(Class targetType, Method method) { MethodMetadata data = new MethodMetadata(); - data.returnType(method.getGenericReturnType()); - data.configKey(Feign.configKey(method)); + data.returnType(Types.resolve(targetType, targetType, method.getGenericReturnType())); + data.configKey(Feign.configKey(targetType, method)); for (Annotation methodAnnotation : method.getAnnotations()) { processAnnotationOnMethod(data, methodAnnotation, method); @@ -81,7 +104,7 @@ public interface Contract { "Body parameters cannot be used with form parameters."); checkState(data.bodyIndex() == null, "Method has too many Body parameters: %s", method); data.bodyIndex(i); - data.bodyType(method.getGenericParameterTypes()[i]); + data.bodyType(Types.resolve(targetType, targetType, method.getGenericParameterTypes()[i])); } } return data; @@ -131,18 +154,25 @@ public interface Contract { class Default extends BaseContract { @Override - public MethodMetadata parseAndValidatateMetadata(Method method) { - MethodMetadata data = super.parseAndValidatateMetadata(method); - if (method.getDeclaringClass().isAnnotationPresent(Headers.class)) { - String[] headersOnType = method.getDeclaringClass().getAnnotation(Headers.class).value(); + protected MethodMetadata parseAndValidateMetadata(Class targetType, Method method) { + MethodMetadata data = super.parseAndValidateMetadata(targetType, method); + headersFromAnnotation(method.getDeclaringClass(), data); + if (method.getDeclaringClass() != targetType) { + headersFromAnnotation(targetType, data); + } + return data; + } + + private void headersFromAnnotation(Class targetType, MethodMetadata data) { + if (targetType.isAnnotationPresent(Headers.class)) { + String[] headersOnType = targetType.getAnnotation(Headers.class).value(); checkState(headersOnType.length > 0, "Headers annotation was empty on type %s.", - method.getDeclaringClass().getName()); + targetType.getName()); Map> headers = toMap(headersOnType); headers.putAll(data.template().headers()); data.template().headers(null); // to clear data.template().headers(headers); } - return data; } @Override diff --git a/core/src/main/java/feign/Feign.java b/core/src/main/java/feign/Feign.java index bf23c080..3d86a2b3 100644 --- a/core/src/main/java/feign/Feign.java +++ b/core/src/main/java/feign/Feign.java @@ -16,6 +16,7 @@ package feign; import java.lang.reflect.Method; +import java.lang.reflect.Type; import java.util.ArrayList; import java.util.List; @@ -47,13 +48,17 @@ public abstract class Feign { * Route53#listByNameAndType(String, String)}: would match a method such as {@code * denominator.route53.Route53#listAt(String, String)}
Note that there is no whitespace * expected in a key! + * + * @param targetType {@link feign.Target#type() type} of the Feign interface. + * @param method invoked method, present on {@code type} or its super. */ - public static String configKey(Method method) { + public static String configKey(Class targetType, Method method) { StringBuilder builder = new StringBuilder(); - builder.append(method.getDeclaringClass().getSimpleName()); + builder.append(targetType.getSimpleName()); builder.append('#').append(method.getName()).append('('); - for (Class param : method.getParameterTypes()) { - builder.append(param.getSimpleName()).append(','); + for (Type param : method.getGenericParameterTypes()) { + param = Types.resolve(targetType, targetType, param); + builder.append(Types.getRawType(param).getSimpleName()).append(','); } if (method.getParameterTypes().length > 0) { builder.deleteCharAt(builder.length() - 1); @@ -61,6 +66,14 @@ public abstract class Feign { return builder.append(')').toString(); } + /** + * @deprecated use {@link #configKey(Class, Method)} instead. + */ + @Deprecated + public static String configKey(Method method) { + return configKey(method.getDeclaringClass(), method); + } + /** * Returns a new instance of an HTTP API, defined by annotations in the {@link Feign Contract}, * for the specified {@code target}. You should cache this result. diff --git a/core/src/main/java/feign/Logger.java b/core/src/main/java/feign/Logger.java index 474786a3..bc24c009 100644 --- a/core/src/main/java/feign/Logger.java +++ b/core/src/main/java/feign/Logger.java @@ -40,7 +40,7 @@ public abstract class Logger { * Override to log requests and responses using your own implementation. Messages will be http * request and response text. * - * @param configKey value of {@link Feign#configKey(java.lang.reflect.Method)} + * @param configKey value of {@link Feign#configKey(Class, java.lang.reflect.Method)} * @param format {@link java.util.Formatter format string} * @param args arguments applied to {@code format} */ diff --git a/core/src/main/java/feign/MethodMetadata.java b/core/src/main/java/feign/MethodMetadata.java index ef2af470..91635bf0 100644 --- a/core/src/main/java/feign/MethodMetadata.java +++ b/core/src/main/java/feign/MethodMetadata.java @@ -35,8 +35,7 @@ public final class MethodMetadata implements Serializable { private transient Type bodyType; private RequestTemplate template = new RequestTemplate(); private List formParams = new ArrayList(); - private Map> - indexToName = + private Map> indexToName = new LinkedHashMap>(); private Map> indexToExpanderClass = new LinkedHashMap>(); @@ -45,7 +44,7 @@ public final class MethodMetadata implements Serializable { } /** - * @see Feign#configKey(java.lang.reflect.Method) + * @see Feign#configKey(Class, java.lang.reflect.Method) */ public String configKey() { return configKey; diff --git a/core/src/main/java/feign/ReflectiveFeign.java b/core/src/main/java/feign/ReflectiveFeign.java index b901d9c9..97cb735c 100644 --- a/core/src/main/java/feign/ReflectiveFeign.java +++ b/core/src/main/java/feign/ReflectiveFeign.java @@ -54,11 +54,11 @@ public class ReflectiveFeign extends Feign { public T newInstance(Target target) { Map nameToHandler = targetToHandlersByName.apply(target); Map methodToHandler = new LinkedHashMap(); - for (Method method : target.type().getDeclaredMethods()) { + for (Method method : target.type().getMethods()) { if (method.getDeclaringClass() == Object.class) { continue; } - methodToHandler.put(method, nameToHandler.get(Feign.configKey(method))); + methodToHandler.put(method, nameToHandler.get(Feign.configKey(target.type(), method))); } InvocationHandler handler = factory.create(target, methodToHandler); return (T) Proxy diff --git a/core/src/main/java/feign/Util.java b/core/src/main/java/feign/Util.java index 9cb07908..f37db367 100644 --- a/core/src/main/java/feign/Util.java +++ b/core/src/main/java/feign/Util.java @@ -168,8 +168,7 @@ public class Util { */ public static Type resolveLastTypeParameter(Type genericContext, Class supertype) throws IllegalStateException { - Type - resolvedSuperType = + Type resolvedSuperType = Types.getSupertype(genericContext, Types.getRawType(genericContext), supertype); checkState(resolvedSuperType instanceof ParameterizedType, "could not resolve %s into a parameterized type %s", diff --git a/core/src/test/java/feign/BaseApiTest.java b/core/src/test/java/feign/BaseApiTest.java new file mode 100644 index 00000000..ac85b6c3 --- /dev/null +++ b/core/src/test/java/feign/BaseApiTest.java @@ -0,0 +1,115 @@ +/* + * Copyright 2015 Netflix, Inc. + * + * 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 com.google.gson.reflect.TypeToken; + +import com.squareup.okhttp.mockwebserver.MockResponse; +import com.squareup.okhttp.mockwebserver.rule.MockWebServerRule; + +import org.junit.Rule; +import org.junit.Test; + +import java.lang.reflect.Type; +import java.util.List; + +import feign.codec.Decoder; +import feign.codec.Encoder; + +import static feign.assertj.MockWebServerAssertions.assertThat; + +public class BaseApiTest { + + @Rule + public final MockWebServerRule server = new MockWebServerRule(); + + interface BaseApi { + + @RequestLine("GET /api/{key}") + Entity get(@Param("key") K key); + + @RequestLine("POST /api") + Entities getAll(Keys keys); + } + + static class Keys { + + List keys; + } + + static class Entity { + + K key; + M model; + } + + static class Entities { + + List> entities; + } + + interface MyApi extends BaseApi { + + } + + @Test + public void resolvesParameterizedResult() throws InterruptedException { + server.enqueue(new MockResponse().setBody("foo")); + + String baseUrl = server.getUrl("/default").toString(); + + Feign.builder() + .decoder(new Decoder() { + @Override + public Object decode(Response response, Type type) { + assertThat(type) + .isEqualTo(new TypeToken>() { + }.getType()); + return null; + } + }) + .target(MyApi.class, baseUrl).get("foo"); + + assertThat(server.takeRequest()).hasPath("/default/api/foo"); + } + + @Test + public void resolvesBodyParameter() throws InterruptedException { + server.enqueue(new MockResponse().setBody("foo")); + + String baseUrl = server.getUrl("/default").toString(); + + Feign.builder() + .encoder(new Encoder() { + @Override + public void encode(Object object, Type bodyType, RequestTemplate template) { + assertThat(bodyType) + .isEqualTo(new TypeToken>() { + }.getType()); + } + }) + .decoder(new Decoder() { + @Override + public Object decode(Response response, Type type) { + assertThat(type) + .isEqualTo(new TypeToken>() { + }.getType()); + return null; + } + }) + .target(MyApi.class, baseUrl).getAll(new Keys()); + } +} diff --git a/core/src/test/java/feign/DefaultContractTest.java b/core/src/test/java/feign/DefaultContractTest.java index 2723b60a..31cac134 100644 --- a/core/src/test/java/feign/DefaultContractTest.java +++ b/core/src/test/java/feign/DefaultContractTest.java @@ -22,9 +22,10 @@ import org.junit.Test; import org.junit.rules.ExpectedException; import java.net.URI; -import java.util.Collections; import java.util.Date; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import static feign.assertj.FeignAssertions.assertThat; import static java.util.Arrays.asList; @@ -43,28 +44,22 @@ public class DefaultContractTest { @Test public void httpMethods() throws Exception { - assertThat( - contract.parseAndValidatateMetadata(Methods.class.getDeclaredMethod("post")).template()) + assertThat(parseAndValidateMetadata(Methods.class, "post").template()) .hasMethod("POST"); - assertThat( - contract.parseAndValidatateMetadata(Methods.class.getDeclaredMethod("put")).template()) + assertThat(parseAndValidateMetadata(Methods.class, "put").template()) .hasMethod("PUT"); - assertThat( - contract.parseAndValidatateMetadata(Methods.class.getDeclaredMethod("get")).template()) + assertThat(parseAndValidateMetadata(Methods.class, "get").template()) .hasMethod("GET"); - assertThat( - contract.parseAndValidatateMetadata(Methods.class.getDeclaredMethod("delete")).template()) + assertThat(parseAndValidateMetadata(Methods.class, "delete").template()) .hasMethod("DELETE"); } @Test public void bodyParamIsGeneric() throws Exception { - MethodMetadata - md = - contract.parseAndValidatateMetadata(BodyParams.class.getDeclaredMethod("post", List.class)); + MethodMetadata md = parseAndValidateMetadata(BodyParams.class, "post", List.class); assertThat(md.bodyIndex()) .isEqualTo(0); @@ -77,46 +72,36 @@ public class DefaultContractTest { public void tooManyBodies() throws Exception { thrown.expect(IllegalStateException.class); thrown.expectMessage("Method has too many Body"); - contract.parseAndValidatateMetadata( - BodyParams.class.getDeclaredMethod("tooMany", List.class, List.class)); + parseAndValidateMetadata(BodyParams.class, "tooMany", List.class, List.class); } @Test public void customMethodWithoutPath() throws Exception { - assertThat(contract.parseAndValidatateMetadata(CustomMethod.class.getDeclaredMethod("patch")) - .template()) + assertThat(parseAndValidateMetadata(CustomMethod.class, "patch").template()) .hasMethod("PATCH") .hasUrl(""); } @Test public void queryParamsInPathExtract() throws Exception { - assertThat( - contract.parseAndValidatateMetadata(WithQueryParamsInPath.class.getDeclaredMethod("none")) - .template()) + assertThat(parseAndValidateMetadata(WithQueryParamsInPath.class, "none").template()) .hasUrl("/") .hasQueries(); - assertThat( - contract.parseAndValidatateMetadata(WithQueryParamsInPath.class.getDeclaredMethod("one")) - .template()) + assertThat(parseAndValidateMetadata(WithQueryParamsInPath.class, "one").template()) .hasUrl("/") .hasQueries( entry("Action", asList("GetUser")) ); - assertThat( - contract.parseAndValidatateMetadata(WithQueryParamsInPath.class.getDeclaredMethod("two")) - .template()) + assertThat(parseAndValidateMetadata(WithQueryParamsInPath.class, "two").template()) .hasUrl("/") .hasQueries( entry("Action", asList("GetUser")), entry("Version", asList("2010-05-08")) ); - assertThat( - contract.parseAndValidatateMetadata(WithQueryParamsInPath.class.getDeclaredMethod("three")) - .template()) + assertThat(parseAndValidateMetadata(WithQueryParamsInPath.class, "three").template()) .hasUrl("/") .hasQueries( entry("Action", asList("GetUser")), @@ -124,9 +109,7 @@ public class DefaultContractTest { entry("limit", asList("1")) ); - assertThat( - contract.parseAndValidatateMetadata(WithQueryParamsInPath.class.getDeclaredMethod("twoAndOneEmpty")) - .template()) + assertThat(parseAndValidateMetadata(WithQueryParamsInPath.class, "twoAndOneEmpty").template()) .hasUrl("/") .hasQueries( entry("flag", asList(new String[]{null})), @@ -134,17 +117,13 @@ public class DefaultContractTest { entry("Version", asList("2010-05-08")) ); - assertThat( - contract.parseAndValidatateMetadata(WithQueryParamsInPath.class.getDeclaredMethod("oneEmpty")) - .template()) + assertThat(parseAndValidateMetadata(WithQueryParamsInPath.class, "oneEmpty").template()) .hasUrl("/") .hasQueries( entry("flag", asList(new String[]{null})) ); - assertThat( - contract.parseAndValidatateMetadata(WithQueryParamsInPath.class.getDeclaredMethod("twoEmpty")) - .template()) + assertThat(parseAndValidateMetadata(WithQueryParamsInPath.class, "twoEmpty").template()) .hasUrl("/") .hasQueries( entry("flag", asList(new String[]{null})), @@ -154,9 +133,7 @@ public class DefaultContractTest { @Test public void bodyWithoutParameters() throws Exception { - MethodMetadata - md = - contract.parseAndValidatateMetadata(BodyWithoutParameters.class.getDeclaredMethod("post")); + MethodMetadata md = parseAndValidateMetadata(BodyWithoutParameters.class, "post"); assertThat(md.template()) .hasBody(""); @@ -164,9 +141,7 @@ public class DefaultContractTest { @Test public void headersOnMethodAddsContentTypeHeader() throws Exception { - MethodMetadata - md = - contract.parseAndValidatateMetadata(BodyWithoutParameters.class.getDeclaredMethod("post")); + MethodMetadata md = parseAndValidateMetadata(BodyWithoutParameters.class, "post"); assertThat(md.template()) .hasHeaders( @@ -177,9 +152,7 @@ public class DefaultContractTest { @Test public void headersOnTypeAddsContentTypeHeader() throws Exception { - MethodMetadata - md = - contract.parseAndValidatateMetadata(HeadersOnType.class.getDeclaredMethod("post")); + MethodMetadata md = parseAndValidateMetadata(HeadersOnType.class, "post"); assertThat(md.template()) .hasHeaders( @@ -190,8 +163,8 @@ public class DefaultContractTest { @Test public void withPathAndURIParam() throws Exception { - MethodMetadata md = contract.parseAndValidatateMetadata( - WithURIParam.class.getDeclaredMethod("uriParam", String.class, URI.class, String.class)); + MethodMetadata md = parseAndValidateMetadata(WithURIParam.class, + "uriParam", String.class, URI.class, String.class); assertThat(md.indexToName()) .containsExactly( @@ -205,10 +178,9 @@ public class DefaultContractTest { @Test public void pathAndQueryParams() throws Exception { - MethodMetadata - md = - contract.parseAndValidatateMetadata(WithPathAndQueryParams.class.getDeclaredMethod - ("recordsByNameAndType", int.class, String.class, String.class)); + MethodMetadata md = parseAndValidateMetadata(WithPathAndQueryParams.class, + "recordsByNameAndType", int.class, String.class, + String.class); assertThat(md.template()) .hasQueries(entry("name", asList("{name}")), entry("type", asList("{type}"))); @@ -222,12 +194,8 @@ public class DefaultContractTest { @Test public void bodyWithTemplate() throws Exception { - MethodMetadata - md = - contract - .parseAndValidatateMetadata(FormParams.class.getDeclaredMethod("login", String.class, - String.class, - String.class)); + MethodMetadata md = parseAndValidateMetadata(FormParams.class, + "login", String.class, String.class, String.class); assertThat(md.template()) .hasBodyTemplate( @@ -236,12 +204,8 @@ public class DefaultContractTest { @Test public void formParamsParseIntoIndexToName() throws Exception { - MethodMetadata - md = - contract - .parseAndValidatateMetadata(FormParams.class.getDeclaredMethod("login", String.class, - String.class, - String.class)); + MethodMetadata md = parseAndValidateMetadata(FormParams.class, + "login", String.class, String.class, String.class); assertThat(md.formParams()) .containsExactly("customer_name", "user_name", "password"); @@ -258,22 +222,15 @@ public class DefaultContractTest { */ @Test public void formParamsDoesNotSetBodyType() throws Exception { - MethodMetadata - md = - contract - .parseAndValidatateMetadata(FormParams.class.getDeclaredMethod("login", String.class, - String.class, - String.class)); + MethodMetadata md = parseAndValidateMetadata(FormParams.class, + "login", String.class, String.class, String.class); assertThat(md.bodyType()).isNull(); } @Test public void headerParamsParseIntoIndexToName() throws Exception { - MethodMetadata - md = - contract.parseAndValidatateMetadata( - HeaderParams.class.getDeclaredMethod("logout", String.class)); + MethodMetadata md = parseAndValidateMetadata(HeaderParams.class, "logout", String.class); assertThat(md.template()) .hasHeaders(entry("Auth-Token", asList("{authToken}", "Foo"))); @@ -284,10 +241,7 @@ public class DefaultContractTest { @Test public void customExpander() throws Exception { - MethodMetadata - md = - contract - .parseAndValidatateMetadata(CustomExpander.class.getDeclaredMethod("date", Date.class)); + MethodMetadata md = parseAndValidateMetadata(CustomExpander.class, "date", Date.class); assertThat(md.indexToExpanderClass()) .containsExactly(entry(0, DateToMillis.class)); @@ -295,18 +249,14 @@ public class DefaultContractTest { @Test public void slashAreEncodedWhenNeeded() throws Exception { - MethodMetadata - md = - contract.parseAndValidatateMetadata( - SlashNeedToBeEncoded.class.getDeclaredMethod("getQueues", String.class)); + MethodMetadata md = parseAndValidateMetadata(SlashNeedToBeEncoded.class, + "getQueues", String.class); assertThat(md.template().decodeSlash()).isFalse(); - md = contract.parseAndValidatateMetadata( - SlashNeedToBeEncoded.class.getDeclaredMethod("getZone", String.class)); + md = parseAndValidateMetadata(SlashNeedToBeEncoded.class, "getZone", String.class); assertThat(md.template().decodeSlash()).isTrue(); - } interface Methods { @@ -429,4 +379,135 @@ public class DefaultContractTest { @RequestLine("GET /api/{zoneId}") String getZone(@Param("ZoneId") String vhost); } + + @Headers("Foo: Bar") + interface SimpleParameterizedBaseApi { + + @RequestLine("GET /api/{zoneId}") + M get(@Param("key") String key); + } + + interface SimpleParameterizedApi extends SimpleParameterizedBaseApi { + + } + + @Test + public void simpleParameterizedBaseApi() throws Exception { + List md = contract.parseAndValidatateMetadata(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.parseAndValidatateMetadata(SimpleParameterizedBaseApi.class); + } + + interface OverrideParameterizedApi extends SimpleParameterizedBaseApi { + + @Override + @RequestLine("GET /api/{zoneId}") + String get(@Param("key") String key); + } + + @Test + public void overrideBaseApiUnsupported() throws Exception { + thrown.expect(IllegalStateException.class); + thrown.expectMessage("Overrides unsupported: OverrideParameterizedApi#get(String)"); + contract.parseAndValidatateMetadata(OverrideParameterizedApi.class); + } + + interface Child extends SimpleParameterizedBaseApi> { + + } + + interface GrandChild extends Child { + + } + + @Test + public void onlySingleLevelInheritanceSupported() throws Exception { + thrown.expect(IllegalStateException.class); + thrown.expectMessage("Only single-level inheritance supported: GrandChild"); + contract.parseAndValidatateMetadata(GrandChild.class); + } + + @Headers("Foo: Bar") + interface ParameterizedBaseApi { + + @RequestLine("GET /api/{key}") + Entity get(@Param("key") K key); + + @RequestLine("POST /api") + Entities getAll(Keys keys); + } + + static class Keys { + + List keys; + } + + static class Entity { + + K key; + M model; + } + + static class Entities { + + private List> entities; + } + + @Headers("Version: 1") + interface ParameterizedApi extends ParameterizedBaseApi { + + } + + @Test + public void parameterizedBaseApi() throws Exception { + List md = contract.parseAndValidatateMetadata(ParameterizedApi.class); + + Map byConfigKey = new LinkedHashMap(); + for (MethodMetadata m : md) { + byConfigKey.put(m.configKey(), m); + } + + assertThat(byConfigKey) + .containsOnlyKeys("ParameterizedApi#get(String)", "ParameterizedApi#getAll(Keys)"); + + assertThat(byConfigKey.get("ParameterizedApi#get(String)").returnType()) + .isEqualTo(new TypeToken>() { + }.getType()); + assertThat(byConfigKey.get("ParameterizedApi#get(String)").template()).hasHeaders( + entry("Version", asList("1")), + entry("Foo", asList("Bar")) + ); + + assertThat(byConfigKey.get("ParameterizedApi#getAll(Keys)").returnType()) + .isEqualTo(new TypeToken>() { + }.getType()); + assertThat(byConfigKey.get("ParameterizedApi#getAll(Keys)").bodyType()) + .isEqualTo(new TypeToken>() { + }.getType()); + assertThat(byConfigKey.get("ParameterizedApi#getAll(Keys)").template()).hasHeaders( + entry("Version", asList("1")), + entry("Foo", asList("Bar")) + ); + } + + private MethodMetadata parseAndValidateMetadata(Class targetType, String method, + Class... parameterTypes) + throws NoSuchMethodException { + return contract.parseAndValidateMetadata(targetType, + targetType.getMethod(method, parameterTypes)); + } } diff --git a/core/src/test/java/feign/FeignTest.java b/core/src/test/java/feign/FeignTest.java index 053bedf7..7a14a364 100644 --- a/core/src/test/java/feign/FeignTest.java +++ b/core/src/test/java/feign/FeignTest.java @@ -211,7 +211,7 @@ public class FeignTest { } @Test - public void toKeyMethodFormatsAsExpected() throws Exception { + public void configKeyFormatsAsExpected() throws Exception { assertEquals("TestInterface#post()", Feign.configKey(TestInterface.class.getDeclaredMethod("post"))); assertEquals("TestInterface#uriParam(String,URI,String)", @@ -220,6 +220,12 @@ public class FeignTest { String.class))); } + @Test + public void configKeyUsesChildType() throws Exception { + assertEquals("List#iterator()", + Feign.configKey(List.class, Iterable.class.getDeclaredMethod("iterator"))); + } + @Test public void canOverrideErrorDecoder() throws Exception { server.enqueue(new MockResponse().setResponseCode(404).setBody("foo")); diff --git a/jaxrs/src/main/java/feign/jaxrs/JAXRSContract.java b/jaxrs/src/main/java/feign/jaxrs/JAXRSContract.java index 4c73af40..1e7759f8 100644 --- a/jaxrs/src/main/java/feign/jaxrs/JAXRSContract.java +++ b/jaxrs/src/main/java/feign/jaxrs/JAXRSContract.java @@ -44,13 +44,12 @@ public final class JAXRSContract extends Contract.BaseContract { static final String CONTENT_TYPE = "Content-Type"; @Override - public MethodMetadata parseAndValidatateMetadata(Method method) { - MethodMetadata md = super.parseAndValidatateMetadata(method); - Path path = method.getDeclaringClass().getAnnotation(Path.class); + protected MethodMetadata parseAndValidateMetadata(Class targetType, Method method) { + MethodMetadata md = super.parseAndValidateMetadata(targetType, method); + Path path = targetType.getAnnotation(Path.class); if (path != null) { String pathValue = emptyToNull(path.value()); - checkState(pathValue != null, "Path.value() was empty on type %s", - method.getDeclaringClass().getName()); + checkState(pathValue != null, "Path.value() was empty on type %s", targetType.getName()); if (!pathValue.startsWith("/")) { pathValue = "/" + pathValue; } diff --git a/jaxrs/src/test/java/feign/jaxrs/JAXRSContractTest.java b/jaxrs/src/test/java/feign/jaxrs/JAXRSContractTest.java index 75bb7e55..7b619817 100644 --- a/jaxrs/src/test/java/feign/jaxrs/JAXRSContractTest.java +++ b/jaxrs/src/test/java/feign/jaxrs/JAXRSContractTest.java @@ -59,59 +59,46 @@ public class JAXRSContractTest { @Test public void httpMethods() throws Exception { - assertThat( - contract.parseAndValidatateMetadata(Methods.class.getDeclaredMethod("post")).template()) + assertThat(parseAndValidateMetadata(Methods.class, "post").template()) .hasMethod("POST"); - assertThat( - contract.parseAndValidatateMetadata(Methods.class.getDeclaredMethod("put")).template()) + assertThat(parseAndValidateMetadata(Methods.class, "put").template()) .hasMethod("PUT"); - assertThat( - contract.parseAndValidatateMetadata(Methods.class.getDeclaredMethod("get")).template()) + assertThat(parseAndValidateMetadata(Methods.class, "get").template()) .hasMethod("GET"); - assertThat( - contract.parseAndValidatateMetadata(Methods.class.getDeclaredMethod("delete")).template()) + assertThat(parseAndValidateMetadata(Methods.class, "delete").template()) .hasMethod("DELETE"); } @Test public void customMethodWithoutPath() throws Exception { - assertThat(contract.parseAndValidatateMetadata(CustomMethod.class.getDeclaredMethod("patch")) - .template()) + assertThat(parseAndValidateMetadata(CustomMethod.class, "patch").template()) .hasMethod("PATCH") .hasUrl(""); } @Test public void queryParamsInPathExtract() throws Exception { - assertThat( - contract.parseAndValidatateMetadata(WithQueryParamsInPath.class.getDeclaredMethod("none")) - .template()) + assertThat(parseAndValidateMetadata(WithQueryParamsInPath.class, "none").template()) .hasUrl("/") .hasQueries(); - assertThat( - contract.parseAndValidatateMetadata(WithQueryParamsInPath.class.getDeclaredMethod("one")) - .template()) + assertThat(parseAndValidateMetadata(WithQueryParamsInPath.class, "one").template()) .hasUrl("/") .hasQueries( entry("Action", asList("GetUser")) ); - assertThat( - contract.parseAndValidatateMetadata(WithQueryParamsInPath.class.getDeclaredMethod("two")) - .template()) + assertThat(parseAndValidateMetadata(WithQueryParamsInPath.class, "two").template()) .hasUrl("/") .hasQueries( entry("Action", asList("GetUser")), entry("Version", asList("2010-05-08")) ); - assertThat( - contract.parseAndValidatateMetadata(WithQueryParamsInPath.class.getDeclaredMethod("three")) - .template()) + assertThat(parseAndValidateMetadata(WithQueryParamsInPath.class, "three").template()) .hasUrl("/") .hasQueries( entry("Action", asList("GetUser")), @@ -119,9 +106,7 @@ public class JAXRSContractTest { entry("limit", asList("1")) ); - assertThat( - contract.parseAndValidatateMetadata(WithQueryParamsInPath.class.getDeclaredMethod("empty")) - .template()) + assertThat(parseAndValidateMetadata(WithQueryParamsInPath.class, "empty").template()) .hasUrl("/") .hasQueries( entry("flag", asList(new String[]{null})), @@ -132,10 +117,7 @@ public class JAXRSContractTest { @Test public void producesAddsAcceptHeader() throws Exception { - MethodMetadata - md = - contract - .parseAndValidatateMetadata(ProducesAndConsumes.class.getDeclaredMethod("produces")); + MethodMetadata md = parseAndValidateMetadata(ProducesAndConsumes.class, "produces"); assertThat(md.template()) .hasHeaders(entry("Accept", asList("application/xml"))); @@ -146,8 +128,7 @@ public class JAXRSContractTest { thrown.expect(IllegalStateException.class); thrown.expectMessage("Produces.value() was empty on method producesNada"); - contract - .parseAndValidatateMetadata(ProducesAndConsumes.class.getDeclaredMethod("producesNada")); + parseAndValidateMetadata(ProducesAndConsumes.class, "producesNada"); } @Test @@ -155,16 +136,12 @@ public class JAXRSContractTest { thrown.expect(IllegalStateException.class); thrown.expectMessage("Produces.value() was empty on method producesEmpty"); - contract - .parseAndValidatateMetadata(ProducesAndConsumes.class.getDeclaredMethod("producesEmpty")); + parseAndValidateMetadata(ProducesAndConsumes.class, "producesEmpty"); } @Test public void consumesAddsContentTypeHeader() throws Exception { - MethodMetadata - md = - contract - .parseAndValidatateMetadata(ProducesAndConsumes.class.getDeclaredMethod("consumes")); + MethodMetadata md = parseAndValidateMetadata(ProducesAndConsumes.class, "consumes"); assertThat(md.template()) .hasHeaders(entry("Content-Type", asList("application/xml"))); @@ -175,8 +152,7 @@ public class JAXRSContractTest { thrown.expect(IllegalStateException.class); thrown.expectMessage("Consumes.value() was empty on method consumesNada"); - contract - .parseAndValidatateMetadata(ProducesAndConsumes.class.getDeclaredMethod("consumesNada")); + parseAndValidateMetadata(ProducesAndConsumes.class, "consumesNada"); } @Test @@ -184,16 +160,12 @@ public class JAXRSContractTest { thrown.expect(IllegalStateException.class); thrown.expectMessage("Consumes.value() was empty on method consumesEmpty"); - contract - .parseAndValidatateMetadata(ProducesAndConsumes.class.getDeclaredMethod("consumesEmpty")); + parseAndValidateMetadata(ProducesAndConsumes.class, "consumesEmpty"); } @Test public void bodyParamIsGeneric() throws Exception { - MethodMetadata - md = - contract.parseAndValidatateMetadata(BodyParams.class.getDeclaredMethod("post", - List.class)); + MethodMetadata md = parseAndValidateMetadata(BodyParams.class, "post", List.class); assertThat(md.bodyIndex()) .isEqualTo(0); @@ -206,8 +178,7 @@ public class JAXRSContractTest { thrown.expect(IllegalStateException.class); thrown.expectMessage("Method has too many Body"); - contract.parseAndValidatateMetadata( - BodyParams.class.getDeclaredMethod("tooMany", List.class, List.class)); + parseAndValidateMetadata(BodyParams.class, "tooMany", List.class, List.class); } @Test @@ -215,19 +186,15 @@ public class JAXRSContractTest { thrown.expect(IllegalStateException.class); thrown.expectMessage("Path.value() was empty on type "); - contract.parseAndValidatateMetadata(EmptyPathOnType.class.getDeclaredMethod("base")); - } - - private MethodMetadata parsePathOnTypeMethod(String name) throws NoSuchMethodException { - return contract.parseAndValidatateMetadata(PathOnType.class.getDeclaredMethod(name)); + parseAndValidateMetadata(EmptyPathOnType.class, "base"); } @Test public void parsePathMethod() throws Exception { - assertThat(parsePathOnTypeMethod("base").template()) + assertThat(parseAndValidateMetadata(PathOnType.class,"base").template()) .hasUrl("/base"); - assertThat(parsePathOnTypeMethod("get").template()) + assertThat(parseAndValidateMetadata(PathOnType.class,"get").template()) .hasUrl("/base/specific"); } @@ -236,7 +203,7 @@ public class JAXRSContractTest { thrown.expect(IllegalStateException.class); thrown.expectMessage("Path.value() was empty on method emptyPath"); - parsePathOnTypeMethod("emptyPath"); + parseAndValidateMetadata(PathOnType.class,"emptyPath"); } @Test @@ -244,32 +211,31 @@ public class JAXRSContractTest { thrown.expect(IllegalStateException.class); thrown.expectMessage("PathParam.value() was empty on parameter 0"); - contract.parseAndValidatateMetadata( - PathOnType.class.getDeclaredMethod("emptyPathParam", String.class)); + parseAndValidateMetadata(PathOnType.class, "emptyPathParam", String.class); } @Test public void pathParamWithSpaces() throws Exception { - assertThat(contract.parseAndValidatateMetadata( - PathOnType.class.getDeclaredMethod("pathParamWithSpaces", String.class)).template()) + assertThat(parseAndValidateMetadata( + PathOnType.class, "pathParamWithSpaces", String.class).template()) .hasUrl("/base/{param}"); } @Test public void regexPathOnMethod() throws Exception { - assertThat(contract.parseAndValidatateMetadata( - PathOnType.class.getDeclaredMethod("pathParamWithRegex", String.class)).template()) + assertThat(parseAndValidateMetadata( + PathOnType.class, "pathParamWithRegex", String.class).template()) .hasUrl("/base/regex/{param}"); - assertThat(contract.parseAndValidatateMetadata( - PathOnType.class.getDeclaredMethod("pathParamWithMultipleRegex", String.class, String.class)).template()) + assertThat(parseAndValidateMetadata( + PathOnType.class, "pathParamWithMultipleRegex", String.class, String.class).template()) .hasUrl("/base/regex/{param1}/{param2}"); } @Test public void withPathAndURIParams() throws Exception { - MethodMetadata md = contract.parseAndValidatateMetadata( - WithURIParam.class.getDeclaredMethod("uriParam", String.class, URI.class, String.class)); + MethodMetadata md = parseAndValidateMetadata(WithURIParam.class, + "uriParam", String.class, URI.class, String.class); assertThat(md.indexToName()).containsExactly( entry(0, asList("1")), @@ -281,10 +247,9 @@ public class JAXRSContractTest { @Test public void pathAndQueryParams() throws Exception { - MethodMetadata - md = - contract.parseAndValidatateMetadata(WithPathAndQueryParams.class.getDeclaredMethod - ("recordsByNameAndType", int.class, String.class, String.class)); + MethodMetadata md = + parseAndValidateMetadata(WithPathAndQueryParams.class, + "recordsByNameAndType", int.class, String.class, String.class); assertThat(md.template()) .hasQueries(entry("name", asList("{name}")), entry("type", asList("{type}"))); @@ -299,18 +264,13 @@ public class JAXRSContractTest { thrown.expect(IllegalStateException.class); thrown.expectMessage("QueryParam.value() was empty on parameter 0"); - contract.parseAndValidatateMetadata( - WithPathAndQueryParams.class.getDeclaredMethod("empty", String.class)); + parseAndValidateMetadata(WithPathAndQueryParams.class, "empty", String.class); } @Test public void formParamsParseIntoIndexToName() throws Exception { - MethodMetadata - md = - contract - .parseAndValidatateMetadata(FormParams.class.getDeclaredMethod("login", String.class, - String.class, - String.class)); + MethodMetadata md = parseAndValidateMetadata(FormParams.class, + "login", String.class, String.class, String.class); assertThat(md.formParams()) .containsExactly("customer_name", "user_name", "password"); @@ -327,12 +287,8 @@ public class JAXRSContractTest { */ @Test public void formParamsDoesNotSetBodyType() throws Exception { - MethodMetadata - md = - contract - .parseAndValidatateMetadata(FormParams.class.getDeclaredMethod("login", String.class, - String.class, - String.class)); + MethodMetadata md = parseAndValidateMetadata(FormParams.class, + "login", String.class, String.class, String.class); assertThat(md.bodyType()).isNull(); } @@ -342,15 +298,12 @@ public class JAXRSContractTest { thrown.expect(IllegalStateException.class); thrown.expectMessage("FormParam.value() was empty on parameter 0"); - contract.parseAndValidatateMetadata( - FormParams.class.getDeclaredMethod("emptyFormParam", String.class)); + parseAndValidateMetadata(FormParams.class, "emptyFormParam", String.class); } @Test public void headerParamsParseIntoIndexToName() throws Exception { - MethodMetadata md = - contract.parseAndValidatateMetadata( - HeaderParams.class.getDeclaredMethod("logout", String.class)); + MethodMetadata md = parseAndValidateMetadata(HeaderParams.class, "logout", String.class); assertThat(md.template()) .hasHeaders(entry("Auth-Token", asList("{Auth-Token}"))); @@ -364,47 +317,36 @@ public class JAXRSContractTest { thrown.expect(IllegalStateException.class); thrown.expectMessage("HeaderParam.value() was empty on parameter 0"); - contract.parseAndValidatateMetadata( - HeaderParams.class.getDeclaredMethod("emptyHeaderParam", String.class)); + parseAndValidateMetadata(HeaderParams.class, "emptyHeaderParam", String.class); } @Test public void pathsWithoutSlashesParseCorrectly() throws Exception { - assertThat( - contract.parseAndValidatateMetadata(PathsWithoutAnySlashes.class.getDeclaredMethod("get")) - .template()) + assertThat(parseAndValidateMetadata(PathsWithoutAnySlashes.class, "get").template()) .hasUrl("/base/specific"); } @Test public void pathsWithSomeSlashesParseCorrectly() throws Exception { - assertThat( - contract.parseAndValidatateMetadata(PathsWithSomeSlashes.class.getDeclaredMethod("get")) - .template()) + assertThat(parseAndValidateMetadata(PathsWithSomeSlashes.class, "get").template()) .hasUrl("/base/specific"); } @Test public void pathsWithSomeOtherSlashesParseCorrectly() throws Exception { - assertThat(contract.parseAndValidatateMetadata( - PathsWithSomeOtherSlashes.class.getDeclaredMethod("get")).template()) + assertThat(parseAndValidateMetadata(PathsWithSomeOtherSlashes.class, "get").template()) .hasUrl("/base/specific"); - } @Test public void classWithRootPathParsesCorrectly() throws Exception { - assertThat( - contract.parseAndValidatateMetadata(ClassRootPath.class.getDeclaredMethod("get")) - .template()) + assertThat(parseAndValidateMetadata(ClassRootPath.class, "get").template()) .hasUrl("/specific"); } @Test public void classPathWithTrailingSlashParsesCorrectly() throws Exception { - assertThat( - contract.parseAndValidatateMetadata(ClassPathWithTrailingSlash.class.getDeclaredMethod("get")) - .template()) + assertThat(parseAndValidateMetadata(ClassPathWithTrailingSlash.class, "get").template()) .hasUrl("/base/specific"); } @@ -609,4 +551,11 @@ public class JAXRSContractTest { @Path("/specific") Response get(); } + + private MethodMetadata parseAndValidateMetadata(Class targetType, String method, + Class... parameterTypes) + throws NoSuchMethodException { + return contract.parseAndValidateMetadata(targetType, + targetType.getMethod(method, parameterTypes)); + } }