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 ba7e5922..20b95827 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 @@ -33,12 +33,12 @@ import org.springframework.cloud.netflix.feign.annotation.RequestParamParameterP import org.springframework.util.Assert; import org.springframework.web.bind.annotation.RequestMapping; -import feign.Contract; -import feign.MethodMetadata; - import static feign.Util.checkState; import static feign.Util.emptyToNull; +import feign.Contract; +import feign.MethodMetadata; + /** * @author Spencer Gibb */ @@ -51,16 +51,19 @@ public class SpringMvcContract extends Contract.BaseContract { private final Map, AnnotatedParameterProcessor> annotatedArgumentProcessors; public SpringMvcContract() { - this(Collections.emptyList()); + this(Collections. emptyList()); } - public SpringMvcContract(List annotatedParameterProcessors) { - Assert.notNull(annotatedParameterProcessors, "Parameter processors can not be null."); + public SpringMvcContract( + List annotatedParameterProcessors) { + Assert.notNull(annotatedParameterProcessors, + "Parameter processors can not be null."); List processors; - if(!annotatedParameterProcessors.isEmpty()) { + if (!annotatedParameterProcessors.isEmpty()) { processors = new ArrayList<>(annotatedParameterProcessors); - } else { + } + else { processors = getDefaultAnnotatedArgumentsProcessors(); } this.annotatedArgumentProcessors = toAnnotatedArgumentProcessorMap(processors); @@ -75,7 +78,8 @@ public class SpringMvcContract extends Contract.BaseContract { // 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", + checkState(pathValue != null, + "RequestMapping.value() was empty on type %s", method.getDeclaringClass().getName()); if (!pathValue.startsWith("/")) { pathValue = "/" + pathValue; @@ -84,16 +88,17 @@ public class SpringMvcContract extends Contract.BaseContract { } // produces - use from class annotation only if method has not specified this - if(!md.template().headers().containsKey(ACCEPT)) { + 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)) { + if (!md.template().headers().containsKey(CONTENT_TYPE)) { parseConsumes(md, method, classAnnotation); } - // headers -- class annotation is inherited to methods, always write these if present + // headers -- class annotation is inherited to methods, always write these if + // present parseHeaders(md, method, classAnnotation); } return md; @@ -113,11 +118,12 @@ public class SpringMvcContract extends Contract.BaseContract { // path checkAtMostOne(method, methodMapping.value(), "value"); - if(methodMapping.value().length > 0) { + 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("/")) { + if (!pathValue.startsWith("/") + && !data.template().toString().endsWith("/")) { pathValue = "/" + pathValue; } data.template().append(pathValue); @@ -134,7 +140,6 @@ public class SpringMvcContract extends Contract.BaseContract { parseHeaders(data, method, methodMapping); } - private void checkAtMostOne(Method method, Object[] values, String fieldName) { checkState(values != null && (values.length == 0 || values.length == 1), "Method %s can only contain at most 1 %s field. Found: %s", @@ -149,20 +154,25 @@ public class SpringMvcContract extends Contract.BaseContract { } @Override - protected boolean processAnnotationsOnParameter(MethodMetadata data, Annotation[] annotations, int paramIndex) { + protected boolean processAnnotationsOnParameter(MethodMetadata data, + Annotation[] annotations, int paramIndex) { boolean isHttpAnnotation = false; - AnnotatedParameterProcessor.AnnotatedParameterContext context = - new SimpleAnnotatedParameterContext(data, paramIndex); + AnnotatedParameterProcessor.AnnotatedParameterContext context = new SimpleAnnotatedParameterContext( + data, paramIndex); for (Annotation parameterAnnotation : annotations) { - AnnotatedParameterProcessor processor = - annotatedArgumentProcessors.get(parameterAnnotation.annotationType()); - isHttpAnnotation |= processor.processArgument(context, parameterAnnotation); + AnnotatedParameterProcessor processor = this.annotatedArgumentProcessors + .get(parameterAnnotation.annotationType()); + if (processor != null) { + isHttpAnnotation |= processor.processArgument(context, + parameterAnnotation); + } } return isHttpAnnotation; } - private void parseProduces(MethodMetadata md, Method method, RequestMapping annotation) { + private void parseProduces(MethodMetadata md, Method method, + RequestMapping annotation) { checkAtMostOne(method, annotation.produces(), "produces"); String[] serverProduces = annotation.produces(); String clientAccepts = serverProduces.length == 0 ? null @@ -172,7 +182,8 @@ public class SpringMvcContract extends Contract.BaseContract { } } - private void parseConsumes(MethodMetadata md, Method method, RequestMapping annotation) { + private void parseConsumes(MethodMetadata md, Method method, + RequestMapping annotation) { checkAtMostOne(method, annotation.consumes(), "consumes"); String[] serverConsumes = annotation.consumes(); String clientProduces = serverConsumes.length == 0 ? null @@ -182,7 +193,8 @@ public class SpringMvcContract extends Contract.BaseContract { } } - private void parseHeaders(MethodMetadata md, Method method, RequestMapping annotation) { + 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()) { @@ -193,9 +205,10 @@ public class SpringMvcContract extends Contract.BaseContract { } } - private Map, AnnotatedParameterProcessor> toAnnotatedArgumentProcessorMap(List processors) { + private Map, AnnotatedParameterProcessor> toAnnotatedArgumentProcessorMap( + List processors) { Map, AnnotatedParameterProcessor> result = new HashMap<>(); - for(AnnotatedParameterProcessor processor : processors) { + for (AnnotatedParameterProcessor processor : processors) { result.put(processor.getAnnotationType(), processor); } return result; @@ -212,34 +225,37 @@ public class SpringMvcContract extends Contract.BaseContract { return annotatedArgumentResolvers; } - private class SimpleAnnotatedParameterContext implements AnnotatedParameterProcessor.AnnotatedParameterContext { + private class SimpleAnnotatedParameterContext + implements AnnotatedParameterProcessor.AnnotatedParameterContext { private final MethodMetadata methodMetadata; private final int parameterIndex; - public SimpleAnnotatedParameterContext(MethodMetadata methodMetadata, int parameterIndex) { + public SimpleAnnotatedParameterContext(MethodMetadata methodMetadata, + int parameterIndex) { this.methodMetadata = methodMetadata; this.parameterIndex = parameterIndex; } @Override public MethodMetadata getMethodMetadata() { - return methodMetadata; + return this.methodMetadata; } @Override public int getParameterIndex() { - return parameterIndex; + return this.parameterIndex; } @Override public void setParameterName(String name) { - nameParam(methodMetadata, name, parameterIndex); + nameParam(this.methodMetadata, name, this.parameterIndex); } @Override - public Collection setTemplateParameter(String name, Collection rest) { + public Collection setTemplateParameter(String name, + Collection rest) { return addTemplatedParam(rest, name); } } 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 index 4d0b80a5..2f87150d 100644 --- 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 @@ -8,6 +8,7 @@ import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestHeader; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; @@ -15,149 +16,197 @@ import org.springframework.web.bind.annotation.RequestParam; import com.fasterxml.jackson.annotation.JsonAutoDetect; +import static org.junit.Assert.assertEquals; + import feign.MethodMetadata; import lombok.AllArgsConstructor; import lombok.NoArgsConstructor; import lombok.ToString; -import static org.junit.Assert.assertEquals; - /** * @author chadjaros */ public class SpringMvcContractTest { - private SpringMvcContract contract; + private SpringMvcContract contract; - @Before - public void setup() { - contract = new SpringMvcContract(); - } + @Before + public void setup() { + this.contract = new SpringMvcContract(); + } - @Test - public void testProcessAnnotationOnMethod_Simple() throws Exception { - Method method = TestTemplate_Simple.class.getDeclaredMethod("getTest", String.class); - MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + @Test + public void testProcessAnnotationOnMethod_Simple() throws Exception { + Method method = TestTemplate_Simple.class.getDeclaredMethod("getTest", + String.class); + MethodMetadata data = this.contract + .parseAndValidateMetadata(method.getDeclaringClass(), 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("/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); - MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + @Test + public void testProcessAnnotations_Simple() throws Exception { + Method method = TestTemplate_Simple.class.getDeclaredMethod("getTest", + String.class); + MethodMetadata data = this.contract + .parseAndValidateMetadata(method.getDeclaringClass(), 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("/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()); - } + 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); - MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + @Test + public void testProcessAnnotations_SimplePost() throws Exception { + Method method = TestTemplate_Simple.class.getDeclaredMethod("postTest", + TestObject.class); + MethodMetadata data = this.contract + .parseAndValidateMetadata(method.getDeclaringClass(), 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("", data.template().url()); + assertEquals("POST", 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); - contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + } - // Don't throw an exception and this passes - } + @Test + public void testProcessAnnotationsOnMethod_Advanced() throws Exception { + Method method = TestTemplate_Advanced.class.getDeclaredMethod("getTest", + String.class, String.class, Integer.class); + MethodMetadata data = this.contract + .parseAndValidateMetadata(method.getDeclaringClass(), method); - @Test - public void testProcessAnnotations_Advanced() throws Exception { - Method method = TestTemplate_Advanced.class.getDeclaredMethod("getTest", String.class, String.class, Integer.class); - MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), 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("/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); + this.contract.parseAndValidateMetadata(method.getDeclaringClass(), method); - assertEquals("Authorization", data.indexToName().get(0).iterator().next()); - assertEquals("id", data.indexToName().get(1).iterator().next()); - assertEquals("amount", data.indexToName().get(2).iterator().next()); + // Don't throw an exception and this passes + } - assertEquals("{Authorization}", data.template().headers().get("Authorization").iterator().next()); - assertEquals("{amount}", data.template().queries().get("amount").iterator().next()); - } + @Test + public void testProcessAnnotations_Advanced() throws Exception { + Method method = TestTemplate_Advanced.class.getDeclaredMethod("getTest", + String.class, String.class, Integer.class); + MethodMetadata data = this.contract + .parseAndValidateMetadata(method.getDeclaringClass(), method); - @Test - public void testProcessAnnotations_Advanced2() throws Exception { - Method method = TestTemplate_Advanced.class.getDeclaredMethod("getTest"); - MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), 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("/advanced", data.template().url()); - assertEquals("GET", 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()); - @Test - public void testProcessAnnotations_Advanced3() throws Exception { - Method method = TestTemplate_Simple.class.getDeclaredMethod("getTest"); - MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + assertEquals("{Authorization}", + data.template().headers().get("Authorization").iterator().next()); + assertEquals("{amount}", + data.template().queries().get("amount").iterator().next()); + } - assertEquals("", data.template().url()); - assertEquals("GET", data.template().method()); - assertEquals(MediaType.APPLICATION_JSON_VALUE, data.template().headers().get("Accept").iterator().next()); - } + @Test + public void testProcessAnnotations_Advanced2() throws Exception { + Method method = TestTemplate_Advanced.class.getDeclaredMethod("getTest"); + MethodMetadata data = this.contract + .parseAndValidateMetadata(method.getDeclaringClass(), method); - public interface TestTemplate_Simple { - @RequestMapping(value = "/test/{id}", method = RequestMethod.GET, produces = MediaType.APPLICATION_JSON_VALUE) - ResponseEntity getTest(@PathVariable("id") String id); + assertEquals("/advanced", data.template().url()); + assertEquals("GET", data.template().method()); + assertEquals(MediaType.APPLICATION_JSON_VALUE, + data.template().headers().get("Accept").iterator().next()); + } - @RequestMapping(method = RequestMethod.GET, produces = MediaType.APPLICATION_JSON_VALUE) - TestObject getTest(); - } + @Test + public void testProcessAnnotations_Advanced3() throws Exception { + Method method = TestTemplate_Simple.class.getDeclaredMethod("getTest"); + MethodMetadata data = this.contract + .parseAndValidateMetadata(method.getDeclaringClass(), method); - @JsonAutoDetect - @RequestMapping("/advanced") - public interface TestTemplate_Advanced { + assertEquals("", data.template().url()); + assertEquals("GET", data.template().method()); + assertEquals(MediaType.APPLICATION_JSON_VALUE, + data.template().headers().get("Accept").iterator().next()); + } - @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 ); + public 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(); - } + @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 { + @RequestMapping(method = RequestMethod.POST, produces = MediaType.APPLICATION_JSON_VALUE) + TestObject postTest(@RequestBody TestObject object); + } - public String something; - public Double number; + @JsonAutoDetect + @RequestMapping("/advanced") + public interface TestTemplate_Advanced { - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; + @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); - TestObject that = (TestObject) o; + @RequestMapping(method = RequestMethod.GET, produces = MediaType.APPLICATION_JSON_VALUE) + TestObject getTest(); + } - if (number != null ? !number.equals(that.number) : that.number != null) return false; - if (something != null ? !something.equals(that.something) : that.something != null) return false; + @AllArgsConstructor + @NoArgsConstructor + @ToString + @JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY, getterVisibility = JsonAutoDetect.Visibility.NONE, setterVisibility = JsonAutoDetect.Visibility.NONE) + public class TestObject { - return true; - } + public String something; + public Double number; - @Override - public int hashCode() { - int result = (something != null ? something.hashCode() : 0); - result = 31 * result + (number != null ? number.hashCode() : 0); - return result; - } - } + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + TestObject that = (TestObject) o; + + if (this.number != null ? !this.number.equals(that.number) + : that.number != null) { + return false; + } + if (this.something != null ? !this.something.equals(that.something) + : that.something != null) { + return false; + } + + return true; + } + + @Override + public int hashCode() { + int result = (this.something != null ? this.something.hashCode() : 0); + result = 31 * result + (this.number != null ? this.number.hashCode() : 0); + return result; + } + } } \ No newline at end of file