Update SNAPSHOT to 3.1.0-M4

This reverts commit f7e490c466.

docs/pom.xml

@@ -6,7 +6,7 @@
 	<parent>
 		<groupId>org.springframework.cloud</groupId>
 		<artifactId>spring-cloud-openfeign</artifactId>
-		<version>3.1.0-M3</version>
+		<version>3.1.0-M4</version>
 	</parent>
 	<artifactId>spring-cloud-openfeign-docs</artifactId>
 	<packaging>jar</packaging>

docs/src/main/asciidoc/spring-cloud-openfeign.adoc

@@ -494,10 +494,7 @@ public interface UserClient extends UserService {
 }
 ----
 
-NOTE: It is generally not advisable to share an interface between a
-server and a client. It introduces tight coupling, and is also not supported by
-all the maintained Spring MVC versions (method parameter
-mapping is not inherited in some versions).
+WARNING: `@FeignClient` interfaces should not be shared between server and client and annotating `@FeignClient` interfaces with `@RequestMapping` on class level is no longer supported.
 
 === Feign request/response compression
 
@@ -692,20 +689,21 @@ public interface DemoTemplate {
 ----
 
 === Feign `CollectionFormat` support
-We support `feign.CollectionFormat` by providing the `@CollectionFormat` annotation.You can annotate a Feign client method with it by passing the desired `feign.CollectionFormat` as annotation value.
+We support `feign.CollectionFormat` by providing the `@CollectionFormat` annotation.
+You can annotate a Feign client method (or the whole class to affect all methods) with it by passing the desired `feign.CollectionFormat` as annotation value.
 
 In the following example, the `CSV` format is used instead of the default `EXPLODED` to process the method.
 
 [source,java,indent=0]
 ----
 @FeignClient(name = "demo")
-	protected interface PageableFeignClient {
+protected interface PageableFeignClient {
 
-		@CollectionFormat(feign.CollectionFormat.CSV)
-		@GetMapping(path = "/page")
-		ResponseEntity performRequest(Pageable page);
+    @CollectionFormat(feign.CollectionFormat.CSV)
+    @GetMapping(path = "/page")
+    ResponseEntity performRequest(Pageable page);
 
-	}
+}
 ----
 
 TIP: Set the `CSV` format while sending `Pageable` as a query parameter in order for it to be encoded correctly.

pom.xml

@@ -4,7 +4,7 @@
 		 xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
 	<modelVersion>4.0.0</modelVersion>
 	<artifactId>spring-cloud-openfeign</artifactId>
-	<version>3.1.0-M3</version>
+	<version>3.1.0-M4</version>
 	<packaging>pom</packaging>
 	<name>Spring Cloud OpenFeign</name>
 	<description>Spring Cloud OpenFeign</description>

spring-cloud-openfeign-core/pom.xml

@@ -6,7 +6,7 @@
 	<parent>
 		<groupId>org.springframework.cloud</groupId>
 		<artifactId>spring-cloud-openfeign</artifactId>
-		<version>3.1.0-M3</version>
+		<version>3.1.0-M4</version>
 		<relativePath>..</relativePath> <!-- lookup parent from repository -->
 	</parent>
 	<artifactId>spring-cloud-openfeign-core</artifactId>

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

@@ -25,9 +25,10 @@ import java.lang.annotation.Target;
  * Indicates which collection format should be used while processing the annotated method.
  *
  * @author Olga Maciaszek-Sharma
+ * @author Sam Kruglov
  * @see feign.CollectionFormat
  */
-@Target(ElementType.METHOD)
+@Target({ ElementType.METHOD, ElementType.TYPE })
 @Retention(RetentionPolicy.RUNTIME)
 public @interface CollectionFormat {
 

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

@@ -35,6 +35,8 @@ import feign.Feign;
 import feign.MethodMetadata;
 import feign.Param;
 import feign.Request;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 
 import org.springframework.cloud.openfeign.AnnotatedParameterProcessor;
 import org.springframework.cloud.openfeign.CollectionFormat;
@@ -80,9 +82,12 @@ import static org.springframework.core.annotation.AnnotatedElementUtils.findMerg
  * @author Artyom Romanenko
  * @author Darren Foong
  * @author Ram Anaswara
+ * @author Sam Kruglov
  */
 public class SpringMvcContract extends Contract.BaseContract implements ResourceLoaderAware {
 
+	private static final Log LOG = LogFactory.getLog(SpringMvcContract.class);
+
 	private static final String ACCEPT = "Accept";
 
 	private static final String CONTENT_TYPE = "Content-Type";
@@ -170,47 +175,22 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource
 
 	@Override
 	protected void processAnnotationOnClass(MethodMetadata data, Class<?> clz) {
-		if (clz.getInterfaces().length == 0) {
-			RequestMapping classAnnotation = findMergedAnnotation(clz, RequestMapping.class);
-			if (classAnnotation != null) {
-				// Prepend path from class annotation if specified
-				if (classAnnotation.value().length > 0) {
-					String pathValue = emptyToNull(classAnnotation.value()[0]);
-					pathValue = resolve(pathValue);
-					if (!pathValue.startsWith("/")) {
-						pathValue = "/" + pathValue;
-					}
-					data.template().uri(pathValue);
-					if (data.template().decodeSlash() != decodeSlash) {
-						data.template().decodeSlash(decodeSlash);
-					}
-				}
-			}
+		RequestMapping classAnnotation = findMergedAnnotation(clz, RequestMapping.class);
+		if (classAnnotation != null) {
+			LOG.error("Cannot process class: " + clz.getName()
+					+ ". @RequestMapping annotation is not allowed on @FeignClient interfaces.");
+			throw new IllegalArgumentException("@RequestMapping annotation not allowed on @FeignClient interfaces");
+		}
+		CollectionFormat collectionFormat = findMergedAnnotation(clz, CollectionFormat.class);
+		if (collectionFormat != null) {
+			data.template().collectionFormat(collectionFormat.value());
 		}
 	}
 
 	@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);
-		if (classAnnotation != null) {
-			// 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;
+		return super.parseAndValidateMetadata(targetType, method);
 	}
 
 	@Override

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

@@ -62,8 +62,10 @@ import org.springframework.web.multipart.MultipartFile;
 
 import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.ANY;
 import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.NONE;
+import static feign.CollectionFormat.CSV;
 import static feign.CollectionFormat.SSV;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.junit.Assume.assumeTrue;
 
 /**
@@ -73,6 +75,7 @@ import static org.junit.Assume.assumeTrue;
  * @author Aaron Whiteside
  * @author Artyom Romanenko
  * @author Olga Maciaszek-Sharma
+ * @author Sam Kruglov
  */
 public class SpringMvcContractTests {
 
@@ -180,16 +183,12 @@ public class SpringMvcContractTests {
 	}
 
 	@Test
-	public void testProcessAnnotations_Class_AnnotationsGetSpecificTest() throws Exception {
-		Method method = TestTemplate_Class_Annotations.class.getDeclaredMethod("getSpecificTest", String.class,
-				String.class);
-		MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
-
-		assertThat(data.template().url()).isEqualTo("/prepend/{classId}/test/{testId}");
-		assertThat(data.template().method()).isEqualTo("GET");
-
-		assertThat(data.indexToName().get(0).iterator().next()).isEqualTo("classId");
-		assertThat(data.indexToName().get(1).iterator().next()).isEqualTo("testId");
+	public void testProcessAnnotations_Class_Annotations_RequestMapping() {
+		assertThatIllegalArgumentException().isThrownBy(() -> {
+			Method method = TestTemplate_Class_RequestMapping.class.getDeclaredMethod("getSpecificTest", String.class,
+					String.class);
+			contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
+		});
 	}
 
 	@Test
@@ -197,25 +196,13 @@ public class SpringMvcContractTests {
 		Method method = TestTemplate_Class_Annotations.class.getDeclaredMethod("getAllTests", String.class);
 		MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
 
-		assertThat(data.template().url()).isEqualTo("/prepend/{classId}");
+		assertThat(data.template().url()).isEqualTo("/");
 		assertThat(data.template().method()).isEqualTo("GET");
 
 		assertThat(data.indexToName().get(0).iterator().next()).isEqualTo("classId");
 		assertThat(data.template().decodeSlash()).isTrue();
 	}
 
-	@Test
-	public void testProcessAnnotations_Class_AnnotationsGetAllTests_EncodeSlash() throws Exception {
-		contract = new SpringMvcContract(Collections.emptyList(), getConversionService(), false);
-
-		Method method = TestTemplate_Class_Annotations.class.getDeclaredMethod("getAllTests", String.class);
-		MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
-
-		assertThat(data.template().url()).isEqualTo("/prepend/{classId}");
-
-		assertThat(data.template().decodeSlash()).isFalse();
-	}
-
 	@Test
 	public void testProcessAnnotations_ExtendedInterface() throws Exception {
 		Method extendedMethod = TestTemplate_Extended.class.getMethod("getAllTests", String.class);
@@ -232,22 +219,6 @@ public class SpringMvcContractTests {
 		assertThat(data.template().decodeSlash()).isTrue();
 	}
 
-	@Test
-	public void testProcessAnnotations_ExtendedInterface_EncodeSlash() throws Exception {
-		contract = new SpringMvcContract(Collections.emptyList(), getConversionService(), false);
-
-		Method extendedMethod = TestTemplate_Extended.class.getMethod("getAllTests", String.class);
-		MethodMetadata extendedData = contract.parseAndValidateMetadata(extendedMethod.getDeclaringClass(),
-				extendedMethod);
-
-		Method method = TestTemplate_Class_Annotations.class.getDeclaredMethod("getAllTests", String.class);
-		MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
-
-		assertThat(data.template().url()).isEqualTo(extendedData.template().url());
-		assertThat(data.template().method()).isEqualTo(extendedData.template().method());
-		assertThat(data.template().decodeSlash()).isFalse();
-	}
-
 	@Test
 	public void testProcessAnnotations_SimplePost() throws Exception {
 		Method method = TestTemplate_Simple.class.getDeclaredMethod("postTest", TestObject.class);
@@ -278,7 +249,7 @@ public class SpringMvcContractTests {
 				Integer.class);
 		MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
 
-		assertThat(data.template().url()).isEqualTo("/advanced/test/{id}?amount=" + "{amount}");
+		assertThat(data.template().url()).isEqualTo("/test/{id}?amount=" + "{amount}");
 		assertThat(data.template().method()).isEqualTo("PUT");
 		assertThat(data.template().headers().get("Accept").iterator().next())
 				.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
@@ -302,13 +273,22 @@ public class SpringMvcContractTests {
 		assertThat(data.template().collectionFormat()).isEqualTo(SSV);
 	}
 
+	@Test
+	public void processAnnotationOnClass_CollectionFormat() throws NoSuchMethodException {
+		Method method = TestTemplate_Advanced.class.getDeclaredMethod("getWithoutCollectionFormat");
+
+		MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
+
+		assertThat(data.template().collectionFormat()).isEqualTo(CSV);
+	}
+
 	@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);
 
-		assertThat(data.template().url()).isEqualTo("/advanced/test/{id}?amount=" + "{amount}");
+		assertThat(data.template().url()).isEqualTo("/test/{id}?amount=" + "{amount}");
 		assertThat(data.template().method()).isEqualTo("PUT");
 		assertThat(data.template().headers().get("Accept").iterator().next())
 				.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
@@ -327,7 +307,7 @@ public class SpringMvcContractTests {
 		Method method = TestTemplate_Advanced.class.getDeclaredMethod("getTest2", String.class, Integer.class);
 		MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
 
-		assertThat(data.template().url()).isEqualTo("/advanced/test2?amount=" + "{amount}");
+		assertThat(data.template().url()).isEqualTo("/test2?amount=" + "{amount}");
 		assertThat(data.template().method()).isEqualTo("PUT");
 		assertThat(data.template().headers().get("Accept").iterator().next())
 				.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
@@ -379,7 +359,7 @@ public class SpringMvcContractTests {
 		Method method = TestTemplate_Advanced.class.getDeclaredMethod("getTest");
 		MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
 
-		assertThat(data.template().url()).isEqualTo("/advanced");
+		assertThat(data.template().url()).isEqualTo("/");
 		assertThat(data.template().method()).isEqualTo("GET");
 		assertThat(data.template().headers().get("Accept").iterator().next())
 				.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
@@ -471,7 +451,7 @@ public class SpringMvcContractTests {
 
 		MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
 
-		assertThat(data.template().url()).isEqualTo("/advanced/testfallback/{id}?amount=" + "{amount}");
+		assertThat(data.template().url()).isEqualTo("/testfallback/{id}?amount=" + "{amount}");
 		assertThat(data.template().method()).isEqualTo("PUT");
 		assertThat(data.template().headers().get("Accept").iterator().next())
 				.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
@@ -635,6 +615,13 @@ public class SpringMvcContractTests {
 	}
 
 	@RequestMapping("/prepend/{classId}")
+	public interface TestTemplate_Class_RequestMapping {
+
+		@RequestMapping(value = "/test/{testId}", method = RequestMethod.GET)
+		TestObject getSpecificTest(@PathVariable("classId") String classId, @PathVariable("testId") String testId);
+
+	}
+
 	public interface TestTemplate_Class_Annotations {
 
 		@GetMapping("/test/{testId}")
@@ -743,13 +730,16 @@ public class SpringMvcContractTests {
 	}
 
 	@JsonAutoDetect
-	@RequestMapping("/advanced")
+	@CollectionFormat(CSV)
 	public interface TestTemplate_Advanced {
 
 		@CollectionFormat(SSV)
 		@GetMapping
 		ResponseEntity<TestObject> getWithCollectionFormat();
 
+		@GetMapping
+		ResponseEntity<TestObject> getWithoutCollectionFormat();
+
 		@ExceptionHandler
 		@PutMapping(path = "/test/{id}", produces = MediaType.APPLICATION_JSON_VALUE)
 		ResponseEntity<TestObject> getTest(@RequestHeader("Authorization") String auth, @PathVariable("id") String id,

spring-cloud-openfeign-dependencies/pom.xml

@@ -10,7 +10,7 @@
 		<relativePath/>
 	</parent>
 	<artifactId>spring-cloud-openfeign-dependencies</artifactId>
-	<version>3.1.0-M3</version>
+	<version>3.1.0-M4</version>
 	<packaging>pom</packaging>
 	<name>spring-cloud-openfeign-dependencies</name>
 	<description>Spring Cloud OpenFeign Dependencies</description>

spring-cloud-starter-openfeign/pom.xml

@@ -5,7 +5,7 @@
 	<parent>
 		<groupId>org.springframework.cloud</groupId>
 		<artifactId>spring-cloud-openfeign</artifactId>
-		<version>3.1.0-M3</version>
+		<version>3.1.0-M4</version>
 		<relativePath>..</relativePath>
 	</parent>
 	<artifactId>spring-cloud-starter-openfeign</artifactId>