From 38e25f390f93d133aab2d21f43769ee57d1b1778 Mon Sep 17 00:00:00 2001 From: Chad Jaros Date: Tue, 31 Mar 2015 08:35:55 -0500 Subject: [PATCH] Support for @RequestMapping on class and ResponseEntity return types --- .../feign/FeignClientsConfiguration.java | 17 +- .../feign/support/ResponseEntityDecoder.java | 64 +++++++ .../netflix/feign/support/SpringDecoder.java | 5 +- .../netflix/feign/support/SpringEncoder.java | 6 +- .../feign/support/SpringMvcContract.java | 143 +++++++++----- .../netflix/feign/SpringDecoderTests.java | 20 ++ .../feign/support/SpringMvcContractTest.java | 179 ++++++++++++++++++ 7 files changed, 375 insertions(+), 59 deletions(-) create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/ResponseEntityDecoder.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringMvcContractTest.java diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsConfiguration.java index 2be24148..033cbed6 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignClientsConfiguration.java @@ -16,6 +16,10 @@ package org.springframework.cloud.netflix.feign; +import org.springframework.beans.factory.ObjectFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.web.HttpMessageConverters; +import org.springframework.cloud.netflix.feign.support.ResponseEntityDecoder; import org.springframework.cloud.netflix.feign.support.SpringDecoder; import org.springframework.cloud.netflix.feign.support.SpringEncoder; import org.springframework.cloud.netflix.feign.support.SpringMvcContract; @@ -24,6 +28,8 @@ import org.springframework.context.annotation.Configuration; import feign.Contract; import feign.Logger; +import feign.codec.Decoder; +import feign.codec.Encoder; import feign.slf4j.Slf4jLogger; /** @@ -32,14 +38,17 @@ import feign.slf4j.Slf4jLogger; @Configuration public class FeignClientsConfiguration { + @Autowired + private ObjectFactory messageConverters; + @Bean - public SpringDecoder feignDecoder() { - return new SpringDecoder(); + public Decoder feignDecoder() { + return new ResponseEntityDecoder(new SpringDecoder(messageConverters)); } @Bean - public SpringEncoder feignEncoder() { - return new SpringEncoder(); + public Encoder feignEncoder() { + return new SpringEncoder(messageConverters); } @Bean diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/ResponseEntityDecoder.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/ResponseEntityDecoder.java new file mode 100644 index 00000000..bb99b2bc --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/ResponseEntityDecoder.java @@ -0,0 +1,64 @@ + +package org.springframework.cloud.netflix.feign.support; + +import feign.FeignException; +import feign.Response; +import feign.codec.Decoder; +import lombok.extern.slf4j.Slf4j; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.MultiValueMap; + +import java.io.IOException; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; +import java.util.LinkedList; + +/** + * Decoder adds compatibility for Spring MVC's ResponseEntity to any + * other decoder via composition. + * @author chadjaros + */ +@Slf4j +public class ResponseEntityDecoder implements Decoder { + + private Decoder decoder; + + public ResponseEntityDecoder(Decoder decoder) { + this.decoder = decoder; + } + + @Override + public Object decode(final Response response, Type type) throws IOException, + FeignException { + + if(type instanceof ParameterizedType && + ((ParameterizedType) type).getRawType().equals(ResponseEntity.class)) { + + type = ((ParameterizedType) type).getActualTypeArguments()[0]; + Object decodedObject = decoder.decode(response, type); + + return createResponse( + decodedObject.getClass(), + decodedObject, + response); + } + else { + return decoder.decode(response, type); + } + } + + private ResponseEntity createResponse(Class clazz, Object instance, Response response) { + + MultiValueMap headers = new LinkedMultiValueMap<>(); + for(String key: response.headers().keySet()) { + headers.put(key, new LinkedList<>(response.headers().get(key))); + } + + return new ResponseEntity( + clazz.cast(instance), + headers, + HttpStatus.valueOf(response.status())); + } +} \ No newline at end of file diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringDecoder.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringDecoder.java index 28f573c9..6651283c 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringDecoder.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringDecoder.java @@ -24,7 +24,6 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import org.springframework.beans.factory.ObjectFactory; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.web.HttpMessageConverters; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; @@ -41,10 +40,10 @@ import feign.codec.Decoder; */ public class SpringDecoder implements Decoder { - @Autowired private ObjectFactory messageConverters; - public SpringDecoder() { + public SpringDecoder(ObjectFactory messageConverters) { + this.messageConverters = messageConverters; } @Override diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringEncoder.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringEncoder.java index e423478d..939995c8 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringEncoder.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringEncoder.java @@ -28,7 +28,6 @@ import java.util.Collection; import lombok.extern.apachecommons.CommonsLog; import org.springframework.beans.factory.ObjectFactory; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.web.HttpMessageConverters; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpOutputMessage; @@ -45,9 +44,12 @@ import feign.codec.Encoder; @CommonsLog public class SpringEncoder implements Encoder { - @Autowired private ObjectFactory messageConverters; + public SpringEncoder(ObjectFactory messageConverters) { + this.messageConverters = messageConverters; + } + @Override public void encode(Object requestBody, Type bodyType, RequestTemplate request) throws EncodeException { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringMvcContract.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringMvcContract.java index a2dd09e0..c7f4a631 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringMvcContract.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/SpringMvcContract.java @@ -16,19 +16,18 @@ package org.springframework.cloud.netflix.feign.support; -import java.lang.annotation.Annotation; -import java.lang.reflect.Method; -import java.util.Arrays; -import java.util.Collection; -import java.util.Map; - +import feign.Contract; +import feign.MethodMetadata; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestHeader; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestParam; -import feign.Contract; -import feign.MethodMetadata; +import java.lang.annotation.Annotation; +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.Collection; +import java.util.Map; import static feign.Util.checkState; import static feign.Util.emptyToNull; @@ -42,59 +41,72 @@ public class SpringMvcContract extends Contract.BaseContract { private static final String CONTENT_TYPE = "Content-Type"; + @Override + public MethodMetadata parseAndValidatateMetadata(Method method) { + MethodMetadata md = super.parseAndValidatateMetadata(method); + + RequestMapping classAnnotation = method.getDeclaringClass().getAnnotation(RequestMapping.class); + if (classAnnotation != null) { + // Prepend path from class annotation if specified + if (classAnnotation.value().length > 0) { + String pathValue = emptyToNull(classAnnotation.value()[0]); + checkState(pathValue != null, "RequestMapping.value() was empty on type %s", + method.getDeclaringClass().getName()); + if (!pathValue.startsWith("/")) { + pathValue = "/" + pathValue; + } + md.template().insert(0, pathValue); + } + + // produces - use from class annotation only if method has not specified this + if(!md.template().headers().containsKey(ACCEPT)) { + parseProduces(md, method, classAnnotation); + } + + // consumes -- use from class annotation only if method has not specified this + if(!md.template().headers().containsKey(CONTENT_TYPE)) { + parseConsumes(md, method, classAnnotation); + } + + // headers -- class annotation is inherited to methods, always write these if present + parseHeaders(md, method, classAnnotation); + } + return md; + } + @Override protected void processAnnotationOnMethod(MethodMetadata data, Annotation methodAnnotation, Method method) { if (!(methodAnnotation instanceof RequestMapping)) { return; } - RequestMapping mapping = RequestMapping.class.cast(methodAnnotation); - if (mapping != null) { - // HTTP Method - checkOne(method, mapping.method(), "method"); - data.template().method(mapping.method()[0].name()); - // path - checkOne(method, mapping.value(), "value"); + RequestMapping methodMapping = RequestMapping.class.cast(methodAnnotation); + // HTTP Method + checkOne(method, methodMapping.method(), "method"); + data.template().method(methodMapping.method()[0].name()); - String methodAnnotationValue = mapping.value()[0]; - String pathValue = emptyToNull(methodAnnotationValue); - checkState(pathValue != null, "value was empty on method %s", - method.getName()); - if (!methodAnnotationValue.startsWith("/") - && !data.template().toString().endsWith("/")) { - methodAnnotationValue = "/" + methodAnnotationValue; - } - data.template().append(methodAnnotationValue); - - // produces - checkAtMostOne(method, mapping.produces(), "produces"); - String[] serverProduces = mapping.produces(); - String clientAccepts = serverProduces.length == 0 ? null - : emptyToNull(serverProduces[0]); - if (clientAccepts != null) { - data.template().header(ACCEPT, clientAccepts); - } - - // consumes - checkAtMostOne(method, mapping.consumes(), "consumes"); - String[] serverConsumes = mapping.consumes(); - String clientProduces = serverConsumes.length == 0 ? null - : emptyToNull(serverConsumes[0]); - if (clientProduces != null) { - data.template().header(CONTENT_TYPE, clientProduces); - } - - // headers - // TODO: only supports one header value per key - if (mapping.headers() != null && mapping.headers().length > 0) { - for (String header : mapping.headers()) { - int colon = header.indexOf(':'); - data.template().header(header.substring(0, colon), - header.substring(colon + 2)); + // path + checkAtMostOne(method, methodMapping.value(), "value"); + if(methodMapping.value().length > 0) { + String pathValue = emptyToNull(methodMapping.value()[0]); + if (pathValue != null) { + // Append path from @RequestMapping if value is present on method + if (!pathValue.startsWith("/") && !data.template().toString().endsWith("/")) { + pathValue = "/" + pathValue; } + data.template().append(pathValue); } } + + // produces + parseProduces(data, method, methodMapping); + + // consumes + parseConsumes(data, method, methodMapping); + + // headers + parseHeaders(data, method, methodMapping); } private void checkAtMostOne(Method method, Object[] values, String fieldName) { @@ -179,4 +191,35 @@ public class SpringMvcContract extends Contract.BaseContract { return false; } + private void parseProduces(MethodMetadata md, Method method, RequestMapping annotation) { + checkAtMostOne(method, annotation.produces(), "produces"); + String[] serverProduces = annotation.produces(); + String clientAccepts = serverProduces.length == 0 ? null + : emptyToNull(serverProduces[0]); + if (clientAccepts != null) { + md.template().header(ACCEPT, clientAccepts); + } + } + + private void parseConsumes(MethodMetadata md, Method method, RequestMapping annotation) { + checkAtMostOne(method, annotation.consumes(), "consumes"); + String[] serverConsumes = annotation.consumes(); + String clientProduces = serverConsumes.length == 0 ? null + : emptyToNull(serverConsumes[0]); + if (clientProduces != null) { + md.template().header(CONTENT_TYPE, clientProduces); + } + } + + private void parseHeaders(MethodMetadata md, Method method, RequestMapping annotation) { + // TODO: only supports one header value per key + if (annotation.headers() != null && annotation.headers().length > 0) { + for (String header : annotation.headers()) { + int colon = header.indexOf(':'); + md.template().header(header.substring(0, colon), + header.substring(colon + 2)); + } + } + } + } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/SpringDecoderTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/SpringDecoderTests.java index 650117a5..5c09e8e2 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/SpringDecoderTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/SpringDecoderTests.java @@ -31,6 +31,8 @@ import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.boot.test.IntegrationTest; import org.springframework.boot.test.SpringApplicationConfiguration; import org.springframework.context.annotation.Configuration; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.test.context.web.WebAppConfiguration; @@ -59,6 +61,16 @@ public class SpringDecoderTests extends FeignClientFactoryBean { return feign().target(TestClient.class, "http://localhost:" + this.port); } + @Test + public void testResponseEntity() { + ResponseEntity response = testClient().getHelloResponse(); + assertNotNull("response was null", response); + assertEquals("wrong status code", HttpStatus.OK, response.getStatusCode()); + Hello hello = response.getBody(); + assertNotNull("hello was null", hello); + assertEquals("first hello didn't match", new Hello("hello world via response"), hello); + } + @Test public void testSimpleType() { Hello hello = testClient().getHello(); @@ -91,6 +103,9 @@ public class SpringDecoderTests extends FeignClientFactoryBean { } protected static interface TestClient { + @RequestMapping(method = RequestMethod.GET, value = "/helloresponse") + public ResponseEntity getHelloResponse(); + @RequestMapping(method = RequestMethod.GET, value = "/hello") public Hello getHello(); @@ -106,6 +121,11 @@ public class SpringDecoderTests extends FeignClientFactoryBean { @RestController protected static class Application implements TestClient { + @Override + public ResponseEntity getHelloResponse() { + return ResponseEntity.ok(new Hello("hello world via response")); + } + @Override public Hello getHello() { return new Hello("hello world 1"); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringMvcContractTest.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringMvcContractTest.java new file mode 100644 index 00000000..3b6f7b78 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringMvcContractTest.java @@ -0,0 +1,179 @@ +package org.springframework.cloud.netflix.feign.support; + +import com.fasterxml.jackson.annotation.JsonAutoDetect; +import feign.MethodMetadata; +import lombok.AllArgsConstructor; +import lombok.NoArgsConstructor; +import lombok.ToString; +import org.junit.Before; +import org.junit.Test; +import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.*; + +import java.lang.annotation.Annotation; +import java.lang.reflect.Constructor; +import java.lang.reflect.Method; + +import static org.junit.Assert.assertEquals; + +/** + * @author chadjaros + */ +public class SpringMvcContractTest { + + private SpringMvcContract contract; + + @Before + public void setup() { + contract = new SpringMvcContract(); + } + + @Test + public void testProcessAnnotationOnMethod_Simple() throws Exception { + Method method = TestTemplate_Simple.class.getDeclaredMethod("getTest", String.class); + Annotation annotation = method.getAnnotation(RequestMapping.class); + + MethodMetadata data = contract.parseAndValidatateMetadata(method); + + assertEquals("/test/{id}", data.template().url()); + assertEquals("GET", data.template().method()); + assertEquals(MediaType.APPLICATION_JSON_VALUE, data.template().headers().get("Accept").iterator().next()); + } + + @Test + public void testProcessAnnotations_Simple() throws Exception { + Method method = TestTemplate_Simple.class.getDeclaredMethod("getTest", String.class); + Annotation annotation = method.getAnnotation(RequestMapping.class); + + MethodMetadata data = contract.parseAndValidatateMetadata(method); + + assertEquals("/test/{id}", data.template().url()); + assertEquals("GET", data.template().method()); + assertEquals(MediaType.APPLICATION_JSON_VALUE, data.template().headers().get("Accept").iterator().next()); + + assertEquals("id", data.indexToName().get(0).iterator().next()); + } + + @Test + public void testProcessAnnotationsOnMethod_Advanced() throws Exception { + Method method = TestTemplate_Advanced.class.getDeclaredMethod("getTest", String.class, String.class, Integer.class); + Annotation annotation = method.getAnnotation(RequestMapping.class); + + MethodMetadata data = contract.parseAndValidatateMetadata(method); + + assertEquals("/advanced/test/{id}", data.template().url()); + assertEquals("PUT", data.template().method()); + assertEquals(MediaType.APPLICATION_JSON_VALUE, data.template().headers().get("Accept").iterator().next()); + } + + @Test + public void testProcessAnnotationsOnMethod_Advanced_UnknownAnnotation() throws Exception { + Method method = TestTemplate_Advanced.class.getDeclaredMethod("getTest", String.class, String.class, Integer.class); + Annotation annotation = method.getAnnotation(ExceptionHandler.class); + + MethodMetadata data = contract.parseAndValidatateMetadata(method); + + // Don't throw an exception and this passes + } + + @Test + public void testProcessAnnotations_Advanced() throws Exception { + Method method = TestTemplate_Advanced.class.getDeclaredMethod("getTest", String.class, String.class, Integer.class); + Annotation annotation = method.getAnnotation(RequestMapping.class); + + MethodMetadata data = contract.parseAndValidatateMetadata(method); + + assertEquals("/advanced/test/{id}", data.template().url()); + assertEquals("PUT", data.template().method()); + assertEquals(MediaType.APPLICATION_JSON_VALUE, data.template().headers().get("Accept").iterator().next()); + + assertEquals("Authorization", data.indexToName().get(0).iterator().next()); + assertEquals("id", data.indexToName().get(1).iterator().next()); + assertEquals("amount", data.indexToName().get(2).iterator().next()); + + assertEquals("{Authorization}", data.template().headers().get("Authorization").iterator().next()); + assertEquals("{amount}", data.template().queries().get("amount").iterator().next()); + } + + @Test + public void testProcessAnnotations_Advanced2() throws Exception { + Method method = TestTemplate_Advanced.class.getDeclaredMethod("getTest"); + Annotation annotation = method.getAnnotation(RequestMapping.class); + + MethodMetadata data = contract.parseAndValidatateMetadata(method); + + assertEquals("/advanced", data.template().url()); + assertEquals("GET", data.template().method()); + assertEquals(MediaType.APPLICATION_JSON_VALUE, data.template().headers().get("Accept").iterator().next()); + } + + @Test + public void testProcessAnnotations_Advanced3() throws Exception { + Method method = TestTemplate_Simple.class.getDeclaredMethod("getTest"); + Annotation annotation = method.getAnnotation(RequestMapping.class); + + MethodMetadata data = contract.parseAndValidatateMetadata(method); + + assertEquals("", data.template().url()); + assertEquals("GET", data.template().method()); + assertEquals(MediaType.APPLICATION_JSON_VALUE, data.template().headers().get("Accept").iterator().next()); + } + + private MethodMetadata newMethodMetadata() throws Exception { + // Reflect because constructor is package private :( + Constructor constructor = MethodMetadata.class.getDeclaredConstructor(); + constructor.setAccessible(true); + return (MethodMetadata)constructor.newInstance(); + } + + public static interface TestTemplate_Simple { + @RequestMapping(value = "/test/{id}", method = RequestMethod.GET, produces = MediaType.APPLICATION_JSON_VALUE) + ResponseEntity getTest(@PathVariable("id") String id); + + @RequestMapping(method = RequestMethod.GET, produces = MediaType.APPLICATION_JSON_VALUE) + TestObject getTest(); + } + + @JsonAutoDetect + @RequestMapping("/advanced") + public static interface TestTemplate_Advanced { + + @ExceptionHandler + @RequestMapping(value = "/test/{id}", method = RequestMethod.PUT, produces = MediaType.APPLICATION_JSON_VALUE) + ResponseEntity getTest(@RequestHeader("Authorization") String auth, @PathVariable("id") String id, @RequestParam("amount") Integer amount ); + + @RequestMapping(method = RequestMethod.GET, produces = MediaType.APPLICATION_JSON_VALUE) + TestObject getTest(); + } + + @AllArgsConstructor + @NoArgsConstructor + @ToString + @JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY, getterVisibility = JsonAutoDetect.Visibility.NONE, setterVisibility = JsonAutoDetect.Visibility.NONE) + public class TestObject { + + public String something; + public Double number; + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + TestObject that = (TestObject) o; + + if (number != null ? !number.equals(that.number) : that.number != null) return false; + if (something != null ? !something.equals(that.something) : that.something != null) return false; + + return true; + } + + @Override + public int hashCode() { + int result = (something != null ? something.hashCode() : 0); + result = 31 * result + (number != null ? number.hashCode() : 0); + return result; + } + } +} \ No newline at end of file