From 05305568ffe5f3dabd4c986751f2d34edad09840 Mon Sep 17 00:00:00 2001 From: Matt Benson Date: Mon, 15 Feb 2016 13:51:43 -0600 Subject: [PATCH] Parameter name discovery using compiler parameters Spring MVC Feign Contract did not support parameter name fallback for the #value() attributes of known parameter annotations. Makes this feature available when the interface has been compiled with the Java 8 -parameters compiler arg Fixes gh-835 --- spring-cloud-netflix-core/pom.xml | 21 ++++++ .../feign/support/SpringMvcContract.java | 32 ++++++++- .../feign/support/SpringMvcContractTests.java | 70 +++++++++++++++++++ 3 files changed, 122 insertions(+), 1 deletion(-) diff --git a/spring-cloud-netflix-core/pom.xml b/spring-cloud-netflix-core/pom.xml index 681f5400..65061fac 100644 --- a/spring-cloud-netflix-core/pom.xml +++ b/spring-cloud-netflix-core/pom.xml @@ -199,4 +199,25 @@ true + + + java8plus + + [1.8,2.0) + + + + + org.apache.maven.plugins + maven-compiler-plugin + + + -parameters + + + + + + + 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 327656b9..6c4b9138 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 @@ -30,11 +30,14 @@ import org.springframework.cloud.netflix.feign.AnnotatedParameterProcessor; import org.springframework.cloud.netflix.feign.annotation.PathVariableParameterProcessor; import org.springframework.cloud.netflix.feign.annotation.RequestHeaderParameterProcessor; import org.springframework.cloud.netflix.feign.annotation.RequestParamParameterProcessor; +import org.springframework.core.DefaultParameterNameDiscoverer; +import org.springframework.core.ParameterNameDiscoverer; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.util.Assert; import org.springframework.web.bind.annotation.RequestMapping; import feign.Contract; +import feign.Feign; import feign.MethodMetadata; import static feign.Util.checkState; @@ -50,7 +53,10 @@ public class SpringMvcContract extends Contract.BaseContract { private static final String CONTENT_TYPE = "Content-Type"; + private static final ParameterNameDiscoverer PARAMETER_NAME_DISCOVERER = new DefaultParameterNameDiscoverer(); + private final Map, AnnotatedParameterProcessor> annotatedArgumentProcessors; + private final Map processedMethods = new HashMap<>(); public SpringMvcContract() { this(Collections. emptyList()); @@ -73,6 +79,7 @@ public class SpringMvcContract extends Contract.BaseContract { @Override public MethodMetadata parseAndValidateMetadata(Class targetType, Method method) { + processedMethods.put(Feign.configKey(targetType, method), method); MethodMetadata md = super.parseAndValidateMetadata(targetType, method); RequestMapping classAnnotation = findMergedAnnotation(targetType, RequestMapping.class); @@ -162,12 +169,18 @@ public class SpringMvcContract extends Contract.BaseContract { AnnotatedParameterProcessor.AnnotatedParameterContext context = new SimpleAnnotatedParameterContext( data, paramIndex); + Method method = processedMethods.get(data.configKey()); for (Annotation parameterAnnotation : annotations) { AnnotatedParameterProcessor processor = this.annotatedArgumentProcessors .get(parameterAnnotation.annotationType()); if (processor != null) { + Annotation processParameterAnnotation; + // synthesize, handling @AliasFor, while falling back to parameter name on + // missing String #value(): + processParameterAnnotation = synthesizeWithMethodParameterNameAsFallbackValue( + parameterAnnotation, method, paramIndex); isHttpAnnotation |= processor.processArgument(context, - AnnotationUtils.synthesizeAnnotation(parameterAnnotation, null)); + processParameterAnnotation); } } return isHttpAnnotation; @@ -227,6 +240,23 @@ public class SpringMvcContract extends Contract.BaseContract { return annotatedArgumentResolvers; } + private Annotation synthesizeWithMethodParameterNameAsFallbackValue( + Annotation parameterAnnotation, Method method, int parameterIndex) { + Map annotationAttributes = AnnotationUtils + .getAnnotationAttributes(parameterAnnotation); + Object defaultValue = AnnotationUtils.getDefaultValue(parameterAnnotation); + if (defaultValue instanceof String + && defaultValue.equals(annotationAttributes.get(AnnotationUtils.VALUE))) { + String[] parameterNames = PARAMETER_NAME_DISCOVERER.getParameterNames(method); + if (parameterNames != null && parameterNames.length > parameterIndex) { + annotationAttributes.put(AnnotationUtils.VALUE, + parameterNames[parameterIndex]); + } + } + return AnnotationUtils.synthesizeAnnotation(annotationAttributes, + parameterAnnotation.annotationType(), null); + } + private class SimpleAnnotatedParameterContext implements AnnotatedParameterProcessor.AnnotatedParameterContext { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringMvcContractTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringMvcContractTests.java index 93936e7e..fde15e62 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringMvcContractTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/SpringMvcContractTests.java @@ -1,11 +1,13 @@ package org.springframework.cloud.netflix.feign.support; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import org.junit.Before; import org.junit.Test; import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; +import org.springframework.util.ReflectionUtils; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestBody; @@ -17,6 +19,7 @@ import org.springframework.web.bind.annotation.RequestParam; import com.fasterxml.jackson.annotation.JsonAutoDetect; import static org.junit.Assert.assertEquals; +import static org.junit.Assume.assumeTrue; import feign.MethodMetadata; import lombok.AllArgsConstructor; @@ -27,6 +30,18 @@ import lombok.ToString; * @author chadjaros */ public class SpringMvcContractTests { + private static final Class EXECUTABLE_TYPE; + + static { + Class executableType; + try { + executableType = Class.forName("java.lang.reflect.Executable"); + } + catch (ClassNotFoundException ex) { + executableType = null; + } + EXECUTABLE_TYPE = executableType; + } private SpringMvcContract contract; @@ -167,6 +182,56 @@ public class SpringMvcContractTests { data.template().headers().get("Accept").iterator().next()); } + @Test + public void testProcessAnnotations_Fallback() throws Exception { + Method method = TestTemplate_Advanced.class.getDeclaredMethod("getTestFallback", + String.class, String.class, Integer.class); + + assumeTrue(hasJava8ParameterNames(method)); + + MethodMetadata data = this.contract + .parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertEquals("/advanced/testfallback/{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()); + } + + /** + * For abstract (e.g. interface) methods, only Java 8 Parameter names (compiler arg + * -parameters) can supply parameter names; bytecode-based strategies use local + * variable declarations, of which there are none for abstract methods. + * @param m + * @return whether a parameter name was found + * @throws IllegalArgumentException if method has no parameters + */ + private static boolean hasJava8ParameterNames(Method m) { + org.springframework.util.Assert.isTrue(m.getParameterTypes().length > 0, + "method has no parameters"); + if (EXECUTABLE_TYPE != null) { + Method getParameters = ReflectionUtils.findMethod(EXECUTABLE_TYPE, "getParameters"); + try { + Object[] parameters = (Object[]) getParameters.invoke(m); + Method isNamePresent = ReflectionUtils.findMethod(parameters[0].getClass(), "isNamePresent"); + return Boolean.TRUE.equals(isNamePresent.invoke(parameters[0])); + } + catch (IllegalAccessException | IllegalArgumentException + | InvocationTargetException ex) { + } + } + return false; + } + public interface TestTemplate_Simple { @RequestMapping(value = "/test/{id}", method = RequestMethod.GET, produces = MediaType.APPLICATION_JSON_VALUE) ResponseEntity getTest(@PathVariable("id") String id); @@ -191,6 +256,11 @@ public class SpringMvcContractTests { ResponseEntity getTest2(@RequestHeader(name = "Authorization") String auth, @RequestParam(name = "amount") Integer amount); + @ExceptionHandler + @RequestMapping(path = "/testfallback/{id}", method = RequestMethod.PUT, produces = MediaType.APPLICATION_JSON_VALUE) + ResponseEntity getTestFallback(@RequestHeader String Authorization, + @PathVariable String id, @RequestParam Integer amount); + @RequestMapping(method = RequestMethod.GET, produces = MediaType.APPLICATION_JSON_VALUE) TestObject getTest(); }