Do not warn if first parameter of GetMapping is of type URL. Fixes #1326 (#1327)

Author: ryanjbaxter

spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java

@@ -20,6 +20,7 @@
 import java.lang.reflect.Method;
 import java.lang.reflect.Parameter;
 import java.lang.reflect.Type;
+import java.net.URI;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -239,7 +240,8 @@ public MethodMetadata parseAndValidateMetadata(Class<?> targetType, Method metho
 		processedMethods.put(Feign.configKey(targetType, method), method);
 		MethodMetadata metadata = super.parseAndValidateMetadata(targetType, method);
 
-		if (isGetMethod(metadata) && method.getParameterCount() > 0 && !hasHttpAnnotations(method)) {
+		if (isGetMethod(metadata) && method.getParameterCount() > 0
+				&& shouldWarnAboutUnannotatedGetParameters(method)) {
 			if (LOG.isWarnEnabled()) {
 				LOG.warn(String
 					.format("[OpenFeign Warning] Feign method '%s' is declared as GET with parameters, but none of the parameters are annotated "
@@ -255,8 +257,19 @@ private boolean isGetMethod(MethodMetadata metadata) {
 		return "GET".equalsIgnoreCase(metadata.template().method());
 	}
 
-	private boolean hasHttpAnnotations(Method method) {
-		for (Parameter parameter : method.getParameters()) {
+	private boolean shouldWarnAboutUnannotatedGetParameters(Method method) {
+		Parameter[] params = method.getParameters();
+		if (params.length == 1 && params[0].getType() == URI.class) {
+			return false;
+		}
+		int fromIndex = (params.length > 0 && params[0].getType() == URI.class) ? 1 : 0;
+		return !hasHttpAnnotations(method, fromIndex);
+	}
+
+	private boolean hasHttpAnnotations(Method method, int fromIndex) {
+		Parameter[] parameters = method.getParameters();
+		for (int i = fromIndex; i < parameters.length; i++) {
+			Parameter parameter = parameters[i];
 			for (Annotation annotation : parameter.getAnnotations()) {
 				Class<? extends Annotation> annotationType = annotation.annotationType();
 				if (annotationType == RequestParam.class || annotationType == RequestHeader.class

spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java

@@ -19,6 +19,7 @@
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.math.BigDecimal;
+import java.net.URI;
 import java.time.LocalDateTime;
 import java.time.format.DateTimeFormatter;
 import java.util.Collection;
@@ -34,7 +35,10 @@
 import feign.Param;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 
+import org.springframework.boot.test.system.CapturedOutput;
+import org.springframework.boot.test.system.OutputCaptureExtension;
 import org.springframework.cloud.openfeign.CollectionFormat;
 import org.springframework.cloud.openfeign.FeignClientProperties;
 import org.springframework.cloud.openfeign.SpringQueryMap;
@@ -87,6 +91,7 @@
  * @author Tang Xiong
  **/
 
+@ExtendWith(OutputCaptureExtension.class)
 class SpringMvcContractTests {
 
 	private static final Class<?> EXECUTABLE_TYPE;
@@ -216,6 +221,42 @@ void testProcessAnnotations_SimplePathIsOnlyASlashWithParamWithTrailingSlashRemo
 			.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
 	}
 
+	@Test
+	void getWithSingleUriParameterShouldNotWarn(CapturedOutput output) throws Exception {
+		Method method = GetWithUriParameter.class.getDeclaredMethod("getValue", URI.class);
+		MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
+
+		assertThat(data.template().method()).isEqualTo("GET");
+		assertThat(output).doesNotContain("OpenFeign Warning");
+	}
+
+	@Test
+	void getWithUnannotatedNonUriParameterShouldWarn(CapturedOutput output) throws Exception {
+		Method method = GetWithUnannotatedParam.class.getDeclaredMethod("getValue", String.class);
+		contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
+
+		assertThat(output).contains("OpenFeign Warning",
+				"is declared as GET with parameters, but none of the parameters are annotated");
+	}
+
+	@Test
+	void getWithUriAndUnannotatedParameterShouldWarn(CapturedOutput output) throws Exception {
+		Method method = GetWithUriAndUnannotatedParam.class.getDeclaredMethod("getValue", URI.class, String.class);
+		contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
+
+		assertThat(output).contains("OpenFeign Warning",
+				"is declared as GET with parameters, but none of the parameters are annotated");
+	}
+
+	@Test
+	void getWithUriAndAnnotatedParameterShouldNotWarn(CapturedOutput output) throws Exception {
+		Method method = GetWithUriAndAnnotatedParam.class.getDeclaredMethod("getValue", URI.class, String.class);
+		MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
+
+		assertThat(data.template().method()).isEqualTo("GET");
+		assertThat(output).doesNotContain("OpenFeign Warning");
+	}
+
 	@Test
 	void testProcessAnnotations_MissingLeadingSlashInPath() throws Exception {
 		Method method = TestTemplate_Simple.class.getDeclaredMethod("getTestNoLeadingSlash", String.class);
@@ -810,6 +851,34 @@ public interface TestTemplate_Simple {
 
 	}
 
+	interface GetWithUriParameter {
+
+		@GetMapping("/sample/receive")
+		String getValue(URI baseUri);
+
+	}
+
+	interface GetWithUnannotatedParam {
+
+		@GetMapping("/test")
+		String getValue(String id);
+
+	}
+
+	interface GetWithUriAndUnannotatedParam {
+
+		@GetMapping("/sample/receive")
+		String getValue(URI baseUri, String unannotated);
+
+	}
+
+	interface GetWithUriAndAnnotatedParam {
+
+		@GetMapping("/sample/receive")
+		String getValue(URI baseUri, @RequestParam("id") String id);
+
+	}
+
 	@RequestMapping("/prepend/{classId}")
 	public interface TestTemplate_Class_RequestMapping {