SpringCloud/spring-cloud-openfeign
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Block clas-level request mapping on Feign clients.

Browse Source
This commit is contained in:
Olga MaciaszekSharma
2021-10-11 15:14:46 +02:00
parent 1275875ddb
commit d6783a6f1e
4 changed files with 40 additions and 116 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
README.adoc
View File

@@ -66,23 +66,9 @@ the `.mvn` configuration, so if you find you have to do it to make a
build succeed, please raise a ticket to get the settings added to
source control.
For hints on how to build the project look in `.travis.yml` if there
is one. There should be a "script" and maybe "install" command. Also
look at the "services" section to see if any services need to be
running locally (e.g. mongo or rabbit). Ignore the git-related bits
that you might find in "before_install" since they're related to setting git
credentials and you already have those.
The projects that require middleware (i.e. Redis) for testing generally
require that a local instance of [Docker](https://www.docker.com/get-started) is installed and running.
The projects that require middleware generally include a
`docker-compose.yml`, so consider using
https://docs.docker.com/compose/[Docker Compose] to run the middeware servers
in Docker containers. See the README in the
https://github.com/spring-cloud-samples/scripts[scripts demo
repository] for specific instructions about the common cases of mongo,
rabbit and redis.
NOTE: If all else fails, build with the command from `.travis.yml` (usually
`./mvnw install`).
=== Documentation

5
docs/src/main/asciidoc/spring-cloud-openfeign.adoc
View File

@@ -523,10 +523,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 also actually
doesn't work with Spring MVC in its current form (method parameter
mapping is not inherited).
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

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

@@ -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;
@@ -83,6 +85,8 @@ import static org.springframework.core.annotation.AnnotatedElementUtils.findMerg
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";
@@ -181,49 +185,20 @@ public class SpringMvcContract extends Contract.BaseContract
@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);
}
}
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");
}
}
}
@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

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

@@ -62,6 +62,7 @@ import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.ANY;
import static com.fasterxml.jackson.annotation.JsonAutoDetect.Visibility.NONE;
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;
/**
@@ -182,28 +183,23 @@ 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
public void testProcessAnnotations_Class_AnnotationsGetAllTests() throws Exception {
Method method = TestTemplate_Class_Annotations.class
.getDeclaredMethod("getAllTests", String.class);
.getDeclaredMethod("getAllTests", String.class);
MethodMetadata data = contract
.parseAndValidateMetadata(method.getDeclaringClass(), method);
.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");
@@ -211,22 +207,6 @@ public class SpringMvcContractTests {
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",
@@ -249,27 +229,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",
@@ -306,7 +265,7 @@ public class SpringMvcContractTests {
.parseAndValidateMetadata(method.getDeclaringClass(), method);
assertThat(data.template().url())
.isEqualTo("/advanced/test/{id}?amount=" + "{amount}");
.isEqualTo("/test/{id}?amount=" + "{amount}");
assertThat(data.template().method()).isEqualTo("PUT");
assertThat(data.template().headers().get("Accept").iterator().next())
.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
@@ -342,7 +301,7 @@ public class SpringMvcContractTests {
.parseAndValidateMetadata(method.getDeclaringClass(), method);
assertThat(data.template().url())
.isEqualTo("/advanced/test/{id}?amount=" + "{amount}");
.isEqualTo("/test/{id}?amount=" + "{amount}");
assertThat(data.template().method()).isEqualTo("PUT");
assertThat(data.template().headers().get("Accept").iterator().next())
.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
@@ -367,7 +326,7 @@ public class SpringMvcContractTests {
.parseAndValidateMetadata(method.getDeclaringClass(), method);
assertThat(data.template().url())
.isEqualTo("/advanced/test2?amount=" + "{amount}");
.isEqualTo("/test2?amount=" + "{amount}");
assertThat(data.template().method()).isEqualTo("PUT");
assertThat(data.template().headers().get("Accept").iterator().next())
.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
@@ -427,12 +386,12 @@ public class SpringMvcContractTests {
public void testProcessAnnotations_Advanced2() throws Exception {
Method method = TestTemplate_Advanced.class.getDeclaredMethod("getTest");
MethodMetadata data = contract
.parseAndValidateMetadata(method.getDeclaringClass(), method);
.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);
.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
}
@Test
@@ -539,7 +498,7 @@ public class SpringMvcContractTests {
.parseAndValidateMetadata(method.getDeclaringClass(), method);
assertThat(data.template().url())
.isEqualTo("/advanced/testfallback/{id}?amount=" + "{amount}");
.isEqualTo("/testfallback/{id}?amount=" + "{amount}");
assertThat(data.template().method()).isEqualTo("PUT");
assertThat(data.template().headers().get("Accept").iterator().next())
.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
@@ -699,7 +658,7 @@ public class SpringMvcContractTests {
ResponseEntity<TestObject> getMappingTest(@PathVariable("id") String id);
@RequestMapping(method = RequestMethod.POST,
produces = MediaType.APPLICATION_JSON_VALUE)
produces = MediaType.APPLICATION_JSON_VALUE)
TestObject postTest(@RequestBody TestObject object);
@PostMapping(produces = MediaType.APPLICATION_JSON_VALUE)
@@ -708,11 +667,19 @@ 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 {
@RequestMapping(value = "/test/{testId}", method = RequestMethod.GET)
TestObject getSpecificTest(@PathVariable("classId") String classId,
@PathVariable("testId") String testId);
@PathVariable("testId") String testId);
@RequestMapping(method = RequestMethod.GET)
TestObject getAllTests(@PathVariable("classId") String classId);
@@ -812,7 +779,6 @@ public class SpringMvcContractTests {
}
@JsonAutoDetect
@RequestMapping("/advanced")
public interface TestTemplate_Advanced {
@CollectionFormat(SSV)