Merge remote-tracking branch 'origin/2.2.x'

# Conflicts: # docs/src/main/asciidoc/_configprops.adoc # spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientProperties.java # spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/support/SpringMvcContract.java # spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractIntegrationTests.java # spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractTests.java
2020-12-01 17:22:59 +01:00
parent b231783e84 1a7917e2bb
commit 15054fa37e
9 changed files with 300 additions and 104 deletions
--- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/AbstractSpringMvcContractIntegrationTests.java
+++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/AbstractSpringMvcContractIntegrationTests.java
@@ -0,0 +1,112 @@
+/*
+ * Copyright 2013-2020 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.cloud.openfeign.support;
+
+import java.nio.charset.Charset;
+
+import feign.Response;
+import feign.codec.Decoder;
+import feign.codec.Encoder;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+
+import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
+import org.springframework.cloud.openfeign.EnableFeignClients;
+import org.springframework.cloud.openfeign.FeignClient;
+import org.springframework.cloud.openfeign.test.NoSecurityConfiguration;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Import;
+import org.springframework.http.HttpHeaders;
+import org.springframework.util.SocketUtils;
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.PostMapping;
+import org.springframework.web.bind.annotation.RequestBody;
+import org.springframework.web.bind.annotation.RequestHeader;
+import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.RestController;
+
+/**
+ * Abstract class for the integration tests for {@link SpringMvcContract}.
+ *
+ * @author Ram Anaswara
+ */
+public class AbstractSpringMvcContractIntegrationTests {
+
+	@BeforeAll
+	public static void beforeClass() {
+		System.setProperty("server.port", String.valueOf(SocketUtils.findAvailableTcpPort()));
+	}
+
+	@AfterAll
+	public static void afterClass() {
+		System.clearProperty("server.port");
+	}
+
+	protected String getUrlQueryParam(Response response) {
+		return response.request().requestTemplate().queries().get("url").stream().findFirst()
+				.orElseThrow(IllegalStateException::new);
+	}
+
+	@FeignClient(name = "test", url = "http://localhost:${server.port}/",
+			configuration = NoCodecsFeignConfiguration.class)
+	interface TestClient {
+
+		@PostMapping("/test")
+		Object sendMessage(@RequestBody String message, @RequestHeader(HttpHeaders.CONTENT_TYPE) String acceptHeader);
+
+		@GetMapping("/get")
+		Object getMessage(@RequestParam String url);
+
+	}
+
+	@Configuration(proxyBeanMethods = false)
+	@EnableFeignClients(clients = TestClient.class)
+	@EnableAutoConfiguration
+	@RestController
+	@Import(NoSecurityConfiguration.class)
+	protected static class Config {
+
+		@PostMapping("/test")
+		Object sendMessage(@RequestBody String message, @RequestHeader(HttpHeaders.CONTENT_TYPE) String acceptHeader) {
+			return message;
+		}
+
+		@GetMapping("/get")
+		Object getMessage(@RequestParam String url) {
+			return url;
+		}
+
+	}
+
+	// Avoid feign.codec.EncodeException - this feature works for users that override
+	// Encoder
+	protected static class NoCodecsFeignConfiguration {
+
+		@Bean
+		public Decoder decoder() {
+			return (response, type) -> response;
+		}
+
+		@Bean
+		public Encoder encoder() {
+			return (object, bodyType, request) -> request.body(object.toString().getBytes(), Charset.defaultCharset());
+		}
+
+	}
+
+}
--- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractIntegrationTests.java
+++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractIntegrationTests.java
@@ -16,96 +16,39 @@

 package org.springframework.cloud.openfeign.support;

-import java.nio.charset.Charset;
-
-import feign.codec.Decoder;
-import feign.codec.Encoder;
-import org.junit.jupiter.api.AfterAll;
-import org.junit.jupiter.api.BeforeAll;
+import feign.Response;
 import org.junit.jupiter.api.Test;

 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
 import org.springframework.boot.test.context.SpringBootTest;
-import org.springframework.cloud.openfeign.EnableFeignClients;
-import org.springframework.cloud.openfeign.FeignClient;
-import org.springframework.cloud.openfeign.test.NoSecurityConfiguration;
-import org.springframework.context.annotation.Bean;
-import org.springframework.context.annotation.Configuration;
-import org.springframework.context.annotation.Import;
-import org.springframework.http.HttpHeaders;
-import org.springframework.util.SocketUtils;
-import org.springframework.web.bind.annotation.PostMapping;
-import org.springframework.web.bind.annotation.RequestBody;
-import org.springframework.web.bind.annotation.RequestHeader;
-import org.springframework.web.bind.annotation.RestController;

+import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatCode;

 /**
- * Integration tests for {@link SpringMvcContract}
+ * Integration tests for {@link SpringMvcContract}.
 *
 * @author Olga Maciaszek-Sharma
+ * @author Ram Anaswara
 */
-@SpringBootTest(classes = SpringMvcContractIntegrationTests.Config.class,
+@SpringBootTest(classes = AbstractSpringMvcContractIntegrationTests.Config.class,
 		webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT)
-public class SpringMvcContractIntegrationTests {
+public class SpringMvcContractIntegrationTests extends AbstractSpringMvcContractIntegrationTests {

 	@Autowired
 	private TestClient client;

-	@BeforeAll
-	public static void beforeClass() {
-		System.setProperty("server.port", String.valueOf(SocketUtils.findAvailableTcpPort()));
-	}
-
-	@AfterAll
-	public static void afterClass() {
-		System.clearProperty("server.port");
-	}
-
 	@Test
 	public void shouldNotThrowInvalidMediaTypeExceptionWhenContentTypeTemplateUsed() {
 		assertThatCode(() -> client.sendMessage("test", "text/markdown")).doesNotThrowAnyException();
 	}

-	@FeignClient(name = "test", url = "http://localhost:${server.port}/",
-			configuration = NoCodecsFeignConfiguration.class)
-	interface TestClient {
-
-		@PostMapping("/test")
-		Object sendMessage(@RequestBody String message, @RequestHeader(HttpHeaders.CONTENT_TYPE) String acceptHeader);
-
-	}
-
-	@Configuration(proxyBeanMethods = false)
-	@EnableFeignClients(clients = TestClient.class)
-	@EnableAutoConfiguration
-	@RestController
-	@Import(NoSecurityConfiguration.class)
-	protected static class Config {
-
-		@PostMapping("/test")
-		Object sendMessage(@RequestBody String message, @RequestHeader(HttpHeaders.CONTENT_TYPE) String acceptHeader) {
-			return message;
-		}
-
-	}
-
-	// avoid feign.codec.EncodeException - this feature works for users that override
-	// Encoder
-	protected static class NoCodecsFeignConfiguration {
-
-		@Bean
-		public Decoder decoder() {
-			return (response, type) -> response;
-		}
-
-		@Bean
-		public Encoder encoder() {
-			return (object, bodyType, request) -> request.body(object.toString().getBytes(), Charset.defaultCharset());
-		}
+	@Test
+	public void feignClientShouldPreserveSlash() {
+		Response response = (Response) client.getMessage("https://www.google.com");

+		String urlQueryParam = getUrlQueryParam(response);
+		assertThat(urlQueryParam).isEqualTo("https%3A//www.google.com");
 	}

 }
--- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractSlashEncodingIntegrationTests.java
+++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/support/SpringMvcContractSlashEncodingIntegrationTests.java
@@ -0,0 +1,47 @@
+/*
+ * Copyright 2013-2020 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.cloud.openfeign.support;
+
+import feign.Response;
+import org.junit.jupiter.api.Test;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Integration tests for {@link SpringMvcContract}.
+ *
+ * @author Ram Anaswara
+ */
+@SpringBootTest(classes = SpringMvcContractSlashEncodingIntegrationTests.Config.class,
+		webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT, properties = { "feign.client.decodeSlash=false" })
+public class SpringMvcContractSlashEncodingIntegrationTests extends AbstractSpringMvcContractIntegrationTests {
+
+	@Autowired
+	private TestClient client;
+
+	@Test
+	public void feignClientShouldNotDecodeEncodedSlash() {
+		Response response = (Response) client.getMessage("https://www.google.com");
+
+		String urlQueryParam = getUrlQueryParam(response);
+		assertThat(urlQueryParam).isEqualTo("https%3A%2F%2Fwww.google.com");
+	}
+
+}
--- 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
@@ -114,11 +114,7 @@ public class SpringMvcContractTests {

 	@Before
 	public void setup() {
-		FormattingConversionServiceFactoryBean conversionServiceFactoryBean = new FormattingConversionServiceFactoryBean();
-		conversionServiceFactoryBean.afterPropertiesSet();
-		ConversionService conversionService = conversionServiceFactoryBean.getObject();
-
-		contract = new SpringMvcContract(Collections.emptyList(), conversionService);
+		contract = new SpringMvcContract(Collections.emptyList(), getConversionService());
 	}

 	@Test
@@ -130,6 +126,19 @@ public class SpringMvcContractTests {
 		assertThat(data.template().method()).isEqualTo("GET");
 		assertThat(data.template().headers().get("Accept").iterator().next())
 				.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
+		assertThat(data.template().decodeSlash()).isTrue();
+	}
+
+	@Test
+	public void testProcessAnnotationOnMethod_Simple_SlashEncoded() throws Exception {
+		contract = new SpringMvcContract(Collections.emptyList(), getConversionService(), false);
+
+		Method method = TestTemplate_Simple.class.getDeclaredMethod("getTest", String.class);
+		MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
+
+		assertThat(data.template().url()).isEqualTo("/test/{id}");
+
+		assertThat(data.template().decodeSlash()).isFalse();
 	}

 	@Test
@@ -180,6 +189,19 @@ public class SpringMvcContractTests {
 		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
@@ -193,8 +215,25 @@ public class SpringMvcContractTests {

 		assertThat(data.template().url()).isEqualTo(extendedData.template().url());
 		assertThat(data.template().method()).isEqualTo(extendedData.template().method());
-
 		assertThat(data.indexToName().get(0).iterator().next()).isEqualTo(data.indexToName().get(0).iterator().next());
+		assertThat(data.indexToName().get(0).iterator().next()).isEqualTo(data.indexToName().get(0).iterator().next());
+		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
@@ -343,6 +382,19 @@ public class SpringMvcContractTests {
 		assertThat(data.template().method()).isEqualTo("GET");
 		assertThat(data.template().headers().get("Accept").iterator().next())
 				.isEqualTo(MediaType.APPLICATION_JSON_VALUE);
+		assertThat(data.template().decodeSlash()).isTrue();
+	}
+
+	@Test
+	public void testProcessAnnotations_Advanced3_DecodeSlashFlagNotModified() throws Exception {
+		contract = new SpringMvcContract(Collections.emptyList(), getConversionService(), false);
+
+		Method method = TestTemplate_Simple.class.getDeclaredMethod("getTest");
+		MethodMetadata data = contract.parseAndValidateMetadata(method.getDeclaringClass(), method);
+
+		assertThat(data.template().url()).isEqualTo("/");
+
+		assertThat(data.template().decodeSlash()).isTrue();
 	}

 	@Test
@@ -524,6 +576,12 @@ public class SpringMvcContractTests {
 		assertThat(data.formParams()).contains("file", "id");
 	}

+	private ConversionService getConversionService() {
+		FormattingConversionServiceFactoryBean conversionServiceFactoryBean = new FormattingConversionServiceFactoryBean();
+		conversionServiceFactoryBean.afterPropertiesSet();
+		return conversionServiceFactoryBean.getObject();
+	}
+
 	public interface TestTemplate_Simple {

 		@RequestMapping(value = "/test/{id}", method = RequestMethod.GET, produces = MediaType.APPLICATION_JSON_VALUE)