Allow removing trailing slashes

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

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.

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2023 the original author or authors.
+ * Copyright 2013-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -61,6 +61,11 @@ public class FeignClientProperties {
 	 */
 	private boolean decodeSlash = true;
 
+	/**
+	 * If {@code true}, trailing slashes at the end of request urls will be removed.
+	 */
+	private boolean removeTrailingSlash;
+
 	public boolean isDefaultToProperties() {
 		return defaultToProperties;
 	}
@@ -93,6 +98,14 @@ public void setDecodeSlash(boolean decodeSlash) {
 		this.decodeSlash = decodeSlash;
 	}
 
+	public boolean isRemoveTrailingSlash() {
+		return removeTrailingSlash;
+	}
+
+	public void setRemoveTrailingSlash(boolean removeTrailingSlash) {
+		this.removeTrailingSlash = removeTrailingSlash;
+	}
+
 	@Override
 	public boolean equals(Object o) {
 		if (this == o) {
@@ -103,12 +116,13 @@ public boolean equals(Object o) {
 		}
 		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(config, that.config) && Objects.equals(decodeSlash, that.decodeSlash)
+				&& Objects.equals(removeTrailingSlash, that.removeTrailingSlash);
 	}
 
 	@Override
 	public int hashCode() {
-		return Objects.hash(defaultToProperties, defaultConfig, config, decodeSlash);
+		return Objects.hash(defaultToProperties, defaultConfig, config, decodeSlash, removeTrailingSlash);
 	}
 
 	/**

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2022 the original author or authors.
+ * Copyright 2013-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -145,8 +145,7 @@ public QueryMapEncoder feignQueryMapEncoderPageable() {
 	@Bean
 	@ConditionalOnMissingBean
 	public Contract feignContract(ConversionService feignConversionService) {
-		boolean decodeSlash = feignClientProperties == null || feignClientProperties.isDecodeSlash();
-		return new SpringMvcContract(parameterProcessors, feignConversionService, decodeSlash);
+		return new SpringMvcContract(parameterProcessors, feignConversionService, feignClientProperties);
 	}
 
 	@Bean

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

@@ -118,6 +118,9 @@ static String getUrl(String url) {
 			if (!url.contains("://")) {
 				url = "http://" + url;
 			}
+			if (url.endsWith("/")) {
+				url = url.substring(0, url.length() - 1);
+			}
 			try {
 				new URL(url);
 			}

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

@@ -41,6 +41,7 @@
 
 import org.springframework.cloud.openfeign.AnnotatedParameterProcessor;
 import org.springframework.cloud.openfeign.CollectionFormat;
+import org.springframework.cloud.openfeign.FeignClientProperties;
 import org.springframework.cloud.openfeign.SpringQueryMap;
 import org.springframework.cloud.openfeign.annotation.CookieValueParameterProcessor;
 import org.springframework.cloud.openfeign.annotation.MatrixVariableParameterProcessor;
@@ -115,6 +116,8 @@ public class SpringMvcContract extends Contract.BaseContract implements Resource
 
 	private final boolean decodeSlash;
 
+	private final boolean removeTrailingSlash;
+
 	public SpringMvcContract() {
 		this(Collections.emptyList());
 	}
@@ -128,8 +131,36 @@ public SpringMvcContract(List<AnnotatedParameterProcessor> annotatedParameterPro
 		this(annotatedParameterProcessors, conversionService, true);
 	}
 
+	/**
+	 * Creates a {@link SpringMvcContract} based on annotatedParameterProcessors,
+	 * conversionService and decodeSlash value.
+	 * @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
 	public SpringMvcContract(List<AnnotatedParameterProcessor> annotatedParameterProcessors,
 			ConversionService conversionService, boolean decodeSlash) {
+		this(annotatedParameterProcessors, conversionService, decodeSlash, false);
+	}
+
+	/**
+	 * Creates a {@link SpringMvcContract} based on annotatedParameterProcessors,
+	 * conversionService and decodeSlash value.
+	 * @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
+	public SpringMvcContract(List<AnnotatedParameterProcessor> annotatedParameterProcessors,
+			ConversionService conversionService, boolean decodeSlash, boolean removeTrailingSlash) {
 		Assert.notNull(annotatedParameterProcessors, "Parameter processors can not be null.");
 		Assert.notNull(conversionService, "ConversionService can not be null.");
 
@@ -140,6 +171,14 @@ public SpringMvcContract(List<AnnotatedParameterProcessor> annotatedParameterPro
 		this.conversionService = conversionService;
 		convertingExpanderFactory = new ConvertingExpanderFactory(conversionService);
 		this.decodeSlash = decodeSlash;
+		this.removeTrailingSlash = removeTrailingSlash;
+	}
+
+	public SpringMvcContract(List<AnnotatedParameterProcessor> annotatedParameterProcessors,
+			ConversionService conversionService, FeignClientProperties feignClientProperties) {
+		this(annotatedParameterProcessors, conversionService,
+				feignClientProperties == null || feignClientProperties.isDecodeSlash(),
+				feignClientProperties != null && feignClientProperties.isRemoveTrailingSlash());
 	}
 
 	private static TypeDescriptor createTypeDescriptor(Method method, int paramIndex) {
@@ -229,6 +268,9 @@ protected void processAnnotationOnMethod(MethodMetadata data, Annotation methodA
 				if (!pathValue.startsWith("/") && !data.template().path().endsWith("/")) {
 					pathValue = "/" + pathValue;
 				}
+				if (removeTrailingSlash && pathValue.endsWith("/")) {
+					pathValue = pathValue.substring(0, pathValue.length() - 1);
+				}
 				data.template().uri(pathValue, true);
 				if (data.template().decodeSlash() != decodeSlash) {
 					data.template().decodeSlash(decodeSlash);

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

@@ -131,7 +131,7 @@ void forType_allFieldsSetOnBuilder() {
 		assertFactoryBeanField(builder, "contextId", "TestContext");
 
 		// and:
-		assertFactoryBeanField(builder, "url", "http://Url/");
+		assertFactoryBeanField(builder, "url", "http://Url");
 		assertFactoryBeanField(builder, "path", "/Path");
 		assertFactoryBeanField(builder, "dismiss404", true);
 
@@ -155,7 +155,7 @@ void forType_clientFactoryBeanProvided() {
 		assertFactoryBeanField(builder, "contextId", "TestContext");
 
 		// and:
-		assertFactoryBeanField(builder, "url", "http://Url/");
+		assertFactoryBeanField(builder, "url", "http://Url");
 		assertFactoryBeanField(builder, "path", "/Path");
 		assertFactoryBeanField(builder, "dismiss404", true);
 		List<FeignBuilderCustomizer> additionalCustomizers = getFactoryBeanField(builder, "additionalCustomizers");

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013-2022 the original author or authors.
+ * Copyright 2013-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -89,6 +89,12 @@ private String testGetName(String name) {
 		return registrar.getName(Collections.singletonMap("name", name));
 	}
 
+	@Test
+	void testRemoveTrailingSlashFromUrl() {
+		String url = FeignClientsRegistrar.getUrl("http://localhost/");
+		assertThat(url).isEqualTo("http://localhost");
+	}
+
 	@Test
 	void testFallback() {
 		assertThatExceptionOfType(IllegalArgumentException.class)

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

@@ -36,6 +36,7 @@
 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 @@ void testProcessAnnotations_SimpleNoPath() throws Exception {
 	}
 
 	@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 @@ void testProcessAnnotations_SimplePostMapping() throws Exception {
 
 	}
 
+	@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 @@ public interface TestTemplate_Simple {
 		TestObject postMappingTest(@RequestBody TestObject object);
 
 		@GetMapping(value = "/", produces = MediaType.APPLICATION_JSON_VALUE)
-		ResponseEntity<TestObject> getSlashPath(@RequestParam("id") String id);
+		ResponseEntity<TestObject> getSlashPathWithParam(@RequestParam("id") String id);
+
+		@GetMapping(value = "/", produces = MediaType.APPLICATION_JSON_VALUE)
+		ResponseEntity<TestObject> getSlashPath();
+
+		@GetMapping(value = "test1/test2/", produces = MediaType.APPLICATION_JSON_VALUE)
+		ResponseEntity<TestObject> getTrailingSlash();
 
 		@GetMapping(path = "test", produces = MediaType.APPLICATION_JSON_VALUE)
 		ResponseEntity<TestObject> getTestNoLeadingSlash(@RequestParam("name") String name);