Support for @RequestMapping on class and ResponseEntity return types

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<HttpMessageConverters> 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

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/ResponseEntityDecoder.java

+
+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 <T> ResponseEntity<T> createResponse(Class<T> clazz, Object instance, Response response) {
+
+        MultiValueMap<String, String> headers = new LinkedMultiValueMap<>();
+        for(String key: response.headers().keySet()) {
+            headers.put(key, new LinkedList<>(response.headers().get(key)));
+        }
+
+        return new ResponseEntity<T>(
+                clazz.cast(instance),
+                headers,
+                HttpStatus.valueOf(response.status()));
+    }
+}
\ No newline at end of file

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<HttpMessageConverters> messageConverters;
 
-	public SpringDecoder() {
+	public SpringDecoder(ObjectFactory<HttpMessageConverters> messageConverters) {
+		this.messageConverters = messageConverters;
 	}
 
 	@Override

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<HttpMessageConverters> messageConverters;
 
+	public SpringEncoder(ObjectFactory<HttpMessageConverters> messageConverters) {
+		this.messageConverters = messageConverters;
+	}
+
 	@Override
 	public void encode(Object requestBody, Type bodyType, RequestTemplate request)
 			throws EncodeException {

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;
@@ -43,58 +42,71 @@ 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");
-
-			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));
+		RequestMapping methodMapping = RequestMapping.class.cast(methodAnnotation);
+		// HTTP Method
+		checkOne(method, methodMapping.method(), "method");
+		data.template().method(methodMapping.method()[0].name());
+
+		// 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));
+			}
+		}
+	}
+
 }

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;
@@ -60,6 +62,16 @@ public class SpringDecoderTests extends FeignClientFactoryBean {
 	}
 
 	@Test
+	public void testResponseEntity() {
+		ResponseEntity<Hello> 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();
 		assertNotNull("hello was null", hello);
@@ -91,6 +103,9 @@ public class SpringDecoderTests extends FeignClientFactoryBean {
 	}
 
 	protected static interface TestClient {
+		@RequestMapping(method = RequestMethod.GET, value = "/helloresponse")
+		public ResponseEntity<Hello> getHelloResponse();
+
 		@RequestMapping(method = RequestMethod.GET, value = "/hello")
 		public Hello getHello();
 
@@ -107,6 +122,11 @@ public class SpringDecoderTests extends FeignClientFactoryBean {
 	protected static class Application implements TestClient {
 
 		@Override
+		public ResponseEntity<Hello> getHelloResponse() {
+			return ResponseEntity.ok(new Hello("hello world via response"));
+		}
+
+		@Override
 		public Hello getHello() {
 			return new Hello("hello world 1");
 		}

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringMvcContractTest.java

+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<TestObject> 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<TestObject> 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