From bf275ec6d93af2e3f3772c16d7360b5b587abf0f Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Thu, 10 Oct 2024 14:37:04 +0200 Subject: [PATCH] Add docs. Add more tests. Reformat code. --- .../ROOT/pages/spring-cloud-openfeign.adoc | 7 +- docs/modules/ROOT/partials/_configprops.adoc | 1 + .../openfeign/FeignClientProperties.java | 7 +- .../openfeign/support/SpringMvcContract.java | 20 +++--- .../support/SpringMvcContractTests.java | 70 ++++++++++++++++++- 5 files changed, 89 insertions(+), 16 deletions(-) diff --git a/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc b/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc index 60bc386e..f9996001 100644 --- a/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc +++ b/docs/modules/ROOT/pages/spring-cloud-openfeign.adoc @@ -288,7 +288,12 @@ public class CustomConfiguration { } ---- -TIP: By default, Feign clients do not encode slash `/` characters. You can change this behaviour, by setting the value of `spring.cloud.openfeign.client.decodeSlash` to `false`. +TIP: By default, Feign clients do not encode slash `/` characters. You can change this behaviour, by setting the value of `spring.cloud.openfeign.client.decode-slash` to `false`. + + +TIP: By default, Feign clients do not remove trailing slash `/` characters from the request path. +You can change this behaviour, by setting the value of `spring.cloud.openfeign.client.remove-trailing-slash` to `true`. +Trailing slash removal from the request path is going to be made the default behaviour in the next major release. [[springencoder-configuration]] ==== `SpringEncoder` configuration diff --git a/docs/modules/ROOT/partials/_configprops.adoc b/docs/modules/ROOT/partials/_configprops.adoc index 73e1462f..d5e19961 100644 --- a/docs/modules/ROOT/partials/_configprops.adoc +++ b/docs/modules/ROOT/partials/_configprops.adoc @@ -73,6 +73,7 @@ |spring.cloud.openfeign.client.default-config | `+++default+++` | |spring.cloud.openfeign.client.default-to-properties | `+++true+++` | |spring.cloud.openfeign.client.refresh-enabled | `+++false+++` | Enables options value refresh capability for Feign. +|spring.cloud.openfeign.client.remove-trailing-slash | `+++false+++` | If {@code true}, trailing slashes at the end of request urls will be removed. |spring.cloud.openfeign.compression.request.content-encoding-types | | The list of content encodings (applicable encodings depend on the used client). |spring.cloud.openfeign.compression.request.enabled | `+++false+++` | Enables the request sent by Feign to be compressed. |spring.cloud.openfeign.compression.request.mime-types | `+++[text/xml, application/xml, application/json]+++` | The list of supported mime types. diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientProperties.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientProperties.java index 02041e60..c6c26c01 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientProperties.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientProperties.java @@ -62,8 +62,7 @@ public class FeignClientProperties { private boolean decodeSlash = true; /** - * If {@code true}, trailing slashes at the end - * of request urls will be removed. + * If {@code true}, trailing slashes at the end of request urls will be removed. */ private boolean removeTrailingSlash; @@ -117,8 +116,8 @@ public class FeignClientProperties { } FeignClientProperties that = (FeignClientProperties) o; return defaultToProperties == that.defaultToProperties && Objects.equals(defaultConfig, that.defaultConfig) - && Objects.equals(config, that.config) && Objects.equals(decodeSlash, that.decodeSlash) - && Objects.equals(removeTrailingSlash, that.removeTrailingSlash); + && Objects.equals(config, that.config) && Objects.equals(decodeSlash, that.decodeSlash) + && Objects.equals(removeTrailingSlash, that.removeTrailingSlash); } @Override diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java index 2ab0c5d4..f5474e18 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java @@ -134,10 +134,12 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource /** * Creates a {@link SpringMvcContract} based on annotatedParameterProcessors, * conversionService and decodeSlash value. - * @param annotatedParameterProcessors list of {@link AnnotatedParameterProcessor} objects used to resolve parameters + * @param annotatedParameterProcessors list of {@link AnnotatedParameterProcessor} + * objects used to resolve parameters * @param conversionService {@link ConversionService} used for type conversion * @param decodeSlash indicates whether slashes should be decoded - * @deprecated in favour of {@link SpringMvcContract#SpringMvcContract(List, ConversionService, FeignClientProperties)} + * @deprecated in favour of + * {@link SpringMvcContract#SpringMvcContract(List, ConversionService, FeignClientProperties)} */ @Deprecated public SpringMvcContract(List annotatedParameterProcessors, @@ -148,15 +150,17 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource /** * Creates a {@link SpringMvcContract} based on annotatedParameterProcessors, * conversionService and decodeSlash value. - * @param annotatedParameterProcessors list of {@link AnnotatedParameterProcessor} objects used to resolve parameters + * @param annotatedParameterProcessors list of {@link AnnotatedParameterProcessor} + * objects used to resolve parameters * @param conversionService {@link ConversionService} used for type conversion * @param decodeSlash indicates whether slashes should be decoded * @param removeTrailingSlash indicates whether trailing slashes should be removed - * @deprecated in favour of {@link SpringMvcContract#SpringMvcContract(List, ConversionService, FeignClientProperties)} + * @deprecated in favour of + * {@link SpringMvcContract#SpringMvcContract(List, ConversionService, FeignClientProperties)} */ @Deprecated public SpringMvcContract(List annotatedParameterProcessors, - ConversionService conversionService, boolean decodeSlash, boolean removeTrailingSlash) { + ConversionService conversionService, boolean decodeSlash, boolean removeTrailingSlash) { Assert.notNull(annotatedParameterProcessors, "Parameter processors can not be null."); Assert.notNull(conversionService, "ConversionService can not be null."); @@ -171,10 +175,10 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource } public SpringMvcContract(List annotatedParameterProcessors, - ConversionService conversionService, FeignClientProperties feignClientProperties) { + ConversionService conversionService, FeignClientProperties feignClientProperties) { this(annotatedParameterProcessors, conversionService, - feignClientProperties == null || feignClientProperties.isDecodeSlash(), - feignClientProperties != null && feignClientProperties.isRemoveTrailingSlash()); + feignClientProperties == null || feignClientProperties.isDecodeSlash(), + feignClientProperties != null && feignClientProperties.isRemoveTrailingSlash()); } private static TypeDescriptor createTypeDescriptor(Method method, int paramIndex) { diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java index 02219e2c..55a3b97d 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java @@ -36,6 +36,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.cloud.openfeign.CollectionFormat; +import org.springframework.cloud.openfeign.FeignClientProperties; import org.springframework.cloud.openfeign.SpringQueryMap; import org.springframework.core.convert.ConversionService; import org.springframework.data.domain.Page; @@ -189,8 +190,23 @@ class SpringMvcContractTests { } @Test - void testProcessAnnotations_SimplePathIsOnlyASlash() throws Exception { - Method method = TestTemplate_Simple.class.getDeclaredMethod("getSlashPath", String.class); + void testProcessAnnotations_SimplePathIsOnlyASlashWithParam() throws Exception { + Method method = TestTemplate_Simple.class.getDeclaredMethod("getSlashPathWithParam", String.class); + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertThat(data.template().url()).isEqualTo("/?id=" + "{id}"); + assertThat(data.template().method()).isEqualTo("GET"); + assertThat(data.template().headers().get("Accept").iterator().next()) + .isEqualTo(MediaType.APPLICATION_JSON_VALUE); + } + + @Test + void testProcessAnnotations_SimplePathIsOnlyASlashWithParamWithTrailingSlashRemoval() throws Exception { + FeignClientProperties properties = new FeignClientProperties(); + properties.setRemoveTrailingSlash(true); + contract = new SpringMvcContract(Collections.emptyList(), getConversionService(), properties); + Method method = TestTemplate_Simple.class.getDeclaredMethod("getSlashPathWithParam", String.class); + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); assertThat(data.template().url()).isEqualTo("/?id=" + "{id}"); @@ -284,6 +300,48 @@ class SpringMvcContractTests { } + @Test + void testProcessAnnotations_SimplePathIsOnlyASlashWithTrailingSlashRemoval() throws Exception { + FeignClientProperties properties = new FeignClientProperties(); + properties.setRemoveTrailingSlash(true); + contract = new SpringMvcContract(Collections.emptyList(), getConversionService(), properties); + Method method = TestTemplate_Simple.class.getDeclaredMethod("getSlashPath"); + + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertThat(data.template().url()).isEqualTo("/"); + assertThat(data.template().method()).isEqualTo("GET"); + assertThat(data.template().headers().get("Accept").iterator().next()) + .isEqualTo(MediaType.APPLICATION_JSON_VALUE); + } + + @Test + void testProcessAnnotations_SimplePathHasTrailingSlash() throws Exception { + Method method = TestTemplate_Simple.class.getDeclaredMethod("getTrailingSlash"); + + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertThat(data.template().url()).isEqualTo("/test1/test2/"); + assertThat(data.template().method()).isEqualTo("GET"); + assertThat(data.template().headers().get("Accept").iterator().next()) + .isEqualTo(MediaType.APPLICATION_JSON_VALUE); + } + + @Test + void testProcessAnnotations_SimplePathHasTrailingSlashWithTrailingSlashRemoval() throws Exception { + FeignClientProperties properties = new FeignClientProperties(); + properties.setRemoveTrailingSlash(true); + contract = new SpringMvcContract(Collections.emptyList(), getConversionService(), properties); + Method method = TestTemplate_Simple.class.getDeclaredMethod("getTrailingSlash"); + + MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method); + + assertThat(data.template().url()).isEqualTo("/test1/test2"); + assertThat(data.template().method()).isEqualTo("GET"); + assertThat(data.template().headers().get("Accept").iterator().next()) + .isEqualTo(MediaType.APPLICATION_JSON_VALUE); + } + @Test void testProcessAnnotationsOnMethod_Advanced() throws Exception { Method method = TestTemplate_Advanced.class.getDeclaredMethod("getTest", String.class, String.class, @@ -738,7 +796,13 @@ class SpringMvcContractTests { TestObject postMappingTest(@RequestBody TestObject object); @GetMapping(value = "/", produces = MediaType.APPLICATION_JSON_VALUE) - ResponseEntity getSlashPath(@RequestParam("id") String id); + ResponseEntity getSlashPathWithParam(@RequestParam("id") String id); + + @GetMapping(value = "/", produces = MediaType.APPLICATION_JSON_VALUE) + ResponseEntity getSlashPath(); + + @GetMapping(value = "test1/test2/", produces = MediaType.APPLICATION_JSON_VALUE) + ResponseEntity getTrailingSlash(); @GetMapping(path = "test", produces = MediaType.APPLICATION_JSON_VALUE) ResponseEntity getTestNoLeadingSlash(@RequestParam("name") String name);