diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java index cff06615..9ad6833b 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/FeignClientFactoryBean.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2020 the original author or authors. + * Copyright 2013-2021 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. @@ -45,6 +45,7 @@ import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.cloud.openfeign.clientconfig.FeignClientConfigurer; import org.springframework.cloud.openfeign.loadbalancer.FeignBlockingLoadBalancerClient; +import org.springframework.cloud.openfeign.loadbalancer.RetryableFeignBlockingLoadBalancerClient; import org.springframework.cloud.openfeign.ribbon.LoadBalancerFeignClient; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; @@ -385,6 +386,12 @@ public class FeignClientFactoryBean implements FactoryBean, Initializing // but Spring Cloud LoadBalancer is on the classpath, so unwrap client = ((FeignBlockingLoadBalancerClient) client).getDelegate(); } + if (client instanceof RetryableFeignBlockingLoadBalancerClient) { + // not load balancing because we have a url, + // but Spring Cloud LoadBalancer is on the classpath, so unwrap + client = ((RetryableFeignBlockingLoadBalancerClient) client) + .getDelegate(); + } builder.client(client); } Targeter targeter = get(context, Targeter.class); diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignHttpClientUrlTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignHttpClientUrlTests.java index 8316dca6..822d0c32 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignHttpClientUrlTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignHttpClientUrlTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2020 the original author or authors. + * Copyright 2013-2021 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. @@ -23,10 +23,9 @@ import feign.Client; import feign.Feign; import feign.Target; import feign.httpclient.ApacheHttpClient; -import org.junit.AfterClass; -import org.junit.BeforeClass; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; @@ -36,7 +35,6 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.test.annotation.DirtiesContext; -import org.springframework.test.context.junit4.SpringRunner; import org.springframework.util.ReflectionUtils; import org.springframework.util.SocketUtils; import org.springframework.web.bind.annotation.RequestMapping; @@ -48,14 +46,14 @@ import static org.springframework.boot.test.context.SpringBootTest.WebEnvironmen /** * @author Spencer Gibb + * @author Olga Maciaszek-Sharma */ -@RunWith(SpringRunner.class) @SpringBootTest(classes = FeignHttpClientUrlTests.TestConfig.class, webEnvironment = DEFINED_PORT, value = { "spring.application.name=feignclienturltest", "feign.hystrix.enabled=false", "feign.okhttp.enabled=false" }) @DirtiesContext -public class FeignHttpClientUrlTests { +class FeignHttpClientUrlTests { static int port; @@ -68,19 +66,19 @@ public class FeignHttpClientUrlTests { @Autowired private BeanUrlClient beanClient; - @BeforeClass - public static void beforeClass() { + @BeforeAll + static void beforeClass() { port = SocketUtils.findAvailableTcpPort(); System.setProperty("server.port", String.valueOf(port)); } - @AfterClass - public static void afterClass() { + @AfterAll + static void afterClass() { System.clearProperty("server.port"); } @Test - public void testUrlHttpClient() { + void testUrlHttpClient() { assertThat(this.urlClient).as("UrlClient was null").isNotNull(); Hello hello = this.urlClient.getHello(); assertThat(hello).as("hello was null").isNotNull(); @@ -89,7 +87,7 @@ public class FeignHttpClientUrlTests { } @Test - public void testBeanUrl() { + void testBeanUrl() { Hello hello = this.beanClient.getHello(); assertThat(hello).as("hello was null").isNotNull(); assertThat(hello).as("first hello didn't match") @@ -97,7 +95,7 @@ public class FeignHttpClientUrlTests { } @Test - public void testBeanUrlNoProtocol() { + void testBeanUrlNoProtocol() { Hello hello = this.beanClientNoProtocol.getHello(); assertThat(hello).as("hello was null").isNotNull(); assertThat(hello).as("first hello didn't match") @@ -183,6 +181,7 @@ public class FeignHttpClientUrlTests { private String message; public Hello() { + } public Hello(String message) { diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignHttpClientUrlTestsWithLoadBalancer.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignHttpClientUrlTestsWithLoadBalancer.java new file mode 100644 index 00000000..03527fa5 --- /dev/null +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignHttpClientUrlTestsWithLoadBalancer.java @@ -0,0 +1,221 @@ +/* + * Copyright 2013-2021 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; + +import java.lang.reflect.Field; +import java.util.Objects; + +import feign.Client; +import feign.Feign; +import feign.Target; +import feign.httpclient.ApacheHttpClient; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +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.test.NoSecurityConfiguration; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.util.ReflectionUtils; +import org.springframework.util.SocketUtils; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.RestController; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.DEFINED_PORT; + +/** + * @author Spencer Gibb + * @author Olga Maciaszek-Sharma + */ +@SpringBootTest(classes = FeignHttpClientUrlTestsWithLoadBalancer.TestConfig.class, + webEnvironment = DEFINED_PORT, + value = { "spring.application.name=feignclienturlwithloadbalancertest", + "feign.hystrix.enabled=false", "feign.okhttp.enabled=false", + "spring.cloud.loadbalancer.ribbon.enabled=false", + "spring.cloud.loadbalancer.retry.enabled=false" }) +@DirtiesContext +class FeignHttpClientUrlTestsWithLoadBalancer { + + static int port; + + @Autowired + BeanUrlClientNoProtocol beanClientNoProtocol; + + @Autowired + private UrlClient urlClient; + + @Autowired + private BeanUrlClient beanClient; + + @BeforeAll + static void beforeClass() { + port = SocketUtils.findAvailableTcpPort(); + System.setProperty("server.port", String.valueOf(port)); + } + + @AfterAll + static void afterClass() { + System.clearProperty("server.port"); + } + + @Test + void testUrlHttpClient() { + assertThat(this.urlClient).as("UrlClient was null").isNotNull(); + Hello hello = this.urlClient.getHello(); + assertThat(hello).as("hello was null").isNotNull(); + assertThat(hello).as("first hello didn't match") + .isEqualTo(new Hello("hello world 1")); + } + + @Test + void testBeanUrl() { + Hello hello = this.beanClient.getHello(); + assertThat(hello).as("hello was null").isNotNull(); + assertThat(hello).as("first hello didn't match") + .isEqualTo(new Hello("hello world 1")); + } + + @Test + void testBeanUrlNoProtocol() { + Hello hello = this.beanClientNoProtocol.getHello(); + assertThat(hello).as("hello was null").isNotNull(); + assertThat(hello).as("first hello didn't match") + .isEqualTo(new Hello("hello world 1")); + } + + // this tests that + @FeignClient(name = "localappurl", url = "http://localhost:${server.port}/") + protected interface UrlClient { + + @RequestMapping(method = RequestMethod.GET, value = "/hello") + Hello getHello(); + + } + + @FeignClient(name = "beanappurl", url = "#{SERVER_URL}path") + protected interface BeanUrlClient { + + @RequestMapping(method = RequestMethod.GET, value = "/hello") + Hello getHello(); + + } + + @FeignClient(name = "beanappurlnoprotocol", url = "#{SERVER_URL_NO_PROTOCOL}path") + protected interface BeanUrlClientNoProtocol { + + @RequestMapping(method = RequestMethod.GET, value = "/hello") + Hello getHello(); + + } + + @Configuration(proxyBeanMethods = false) + @EnableAutoConfiguration + @RestController + @EnableFeignClients(clients = { UrlClient.class, BeanUrlClient.class, + BeanUrlClientNoProtocol.class }) + @Import(NoSecurityConfiguration.class) + protected static class TestConfig { + + @GetMapping("/hello") + public Hello getHello() { + return new Hello("hello world 1"); + } + + @GetMapping("/path/hello") + public Hello getHelloWithPath() { + return getHello(); + } + + @Bean(name = "SERVER_URL") + public String serverUrl() { + return "http://localhost:" + port + "/"; + } + + @Bean(name = "SERVER_URL_NO_PROTOCOL") + public String serverUrlNoProtocol() { + return "localhost:" + port + "/"; + } + + @Bean + public Targeter feignTargeter() { + return new Targeter() { + @Override + public T target(FeignClientFactoryBean factory, Feign.Builder feign, + FeignContext context, Target.HardCodedTarget target) { + Field field = ReflectionUtils.findField(Feign.Builder.class, + "client"); + ReflectionUtils.makeAccessible(field); + Client client = (Client) ReflectionUtils.getField(field, feign); + if (target.name().equals("localappurl")) { + assertThat(client).isInstanceOf(ApacheHttpClient.class) + .as("client was wrong type"); + } + return feign.target(target); + } + }; + } + + } + + public static class Hello { + + private String message; + + public Hello() { + + } + + public Hello(String message) { + this.message = message; + } + + public String getMessage() { + return this.message; + } + + public void setMessage(String message) { + this.message = message; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + Hello that = (Hello) o; + return Objects.equals(this.message, that.message); + } + + @Override + public int hashCode() { + return Objects.hash(this.message); + } + + } + +} diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignHttpClientUrlTestsWithRetryableLoadBalancer.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignHttpClientUrlTestsWithRetryableLoadBalancer.java new file mode 100644 index 00000000..50f4372c --- /dev/null +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignHttpClientUrlTestsWithRetryableLoadBalancer.java @@ -0,0 +1,221 @@ +/* + * Copyright 2013-2021 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; + +import java.lang.reflect.Field; +import java.util.Objects; + +import feign.Client; +import feign.Feign; +import feign.Target; +import feign.httpclient.ApacheHttpClient; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +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.test.NoSecurityConfiguration; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.util.ReflectionUtils; +import org.springframework.util.SocketUtils; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.RestController; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.DEFINED_PORT; + +/** + * @author Spencer Gibb + * @author Olga Maciaszek-Sharma + */ +@SpringBootTest( + classes = FeignHttpClientUrlTestsWithRetryableLoadBalancer.TestConfig.class, + webEnvironment = DEFINED_PORT, + value = { "spring.application.name=feignclienturlwithretryableloadbalancertest", + "feign.hystrix.enabled=false", "feign.okhttp.enabled=false", + "spring.cloud.loadbalancer.ribbon.enabled=false" }) +@DirtiesContext +class FeignHttpClientUrlTestsWithRetryableLoadBalancer { + + static int port; + + @Autowired + BeanUrlClientNoProtocol beanClientNoProtocol; + + @Autowired + private UrlClient urlClient; + + @Autowired + private BeanUrlClient beanClient; + + @BeforeAll + static void beforeClass() { + port = SocketUtils.findAvailableTcpPort(); + System.setProperty("server.port", String.valueOf(port)); + } + + @AfterAll + static void afterClass() { + System.clearProperty("server.port"); + } + + @Test + void testUrlHttpClient() { + assertThat(this.urlClient).as("UrlClient was null").isNotNull(); + Hello hello = this.urlClient.getHello(); + assertThat(hello).as("hello was null").isNotNull(); + assertThat(hello).as("first hello didn't match") + .isEqualTo(new Hello("hello world 1")); + } + + @Test + void testBeanUrl() { + Hello hello = this.beanClient.getHello(); + assertThat(hello).as("hello was null").isNotNull(); + assertThat(hello).as("first hello didn't match") + .isEqualTo(new Hello("hello world 1")); + } + + @Test + void testBeanUrlNoProtocol() { + Hello hello = this.beanClientNoProtocol.getHello(); + assertThat(hello).as("hello was null").isNotNull(); + assertThat(hello).as("first hello didn't match") + .isEqualTo(new Hello("hello world 1")); + } + + // this tests that + @FeignClient(name = "localappurl", url = "http://localhost:${server.port}/") + protected interface UrlClient { + + @RequestMapping(method = RequestMethod.GET, value = "/hello") + Hello getHello(); + + } + + @FeignClient(name = "beanappurl", url = "#{SERVER_URL}path") + protected interface BeanUrlClient { + + @RequestMapping(method = RequestMethod.GET, value = "/hello") + Hello getHello(); + + } + + @FeignClient(name = "beanappurlnoprotocol", url = "#{SERVER_URL_NO_PROTOCOL}path") + protected interface BeanUrlClientNoProtocol { + + @RequestMapping(method = RequestMethod.GET, value = "/hello") + Hello getHello(); + + } + + @Configuration(proxyBeanMethods = false) + @EnableAutoConfiguration + @RestController + @EnableFeignClients(clients = { UrlClient.class, BeanUrlClient.class, + BeanUrlClientNoProtocol.class }) + @Import(NoSecurityConfiguration.class) + protected static class TestConfig { + + @GetMapping("/hello") + public Hello getHello() { + return new Hello("hello world 1"); + } + + @GetMapping("/path/hello") + public Hello getHelloWithPath() { + return getHello(); + } + + @Bean(name = "SERVER_URL") + public String serverUrl() { + return "http://localhost:" + port + "/"; + } + + @Bean(name = "SERVER_URL_NO_PROTOCOL") + public String serverUrlNoProtocol() { + return "localhost:" + port + "/"; + } + + @Bean + public Targeter feignTargeter() { + return new Targeter() { + @Override + public T target(FeignClientFactoryBean factory, Feign.Builder feign, + FeignContext context, Target.HardCodedTarget target) { + Field field = ReflectionUtils.findField(Feign.Builder.class, + "client"); + ReflectionUtils.makeAccessible(field); + Client client = (Client) ReflectionUtils.getField(field, feign); + if (target.name().equals("localappurl")) { + assertThat(client).isInstanceOf(ApacheHttpClient.class) + .as("client was wrong type"); + } + return feign.target(target); + } + }; + } + + } + + public static class Hello { + + private String message; + + public Hello() { + + } + + public Hello(String message) { + this.message = message; + } + + public String getMessage() { + return this.message; + } + + public void setMessage(String message) { + this.message = message; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + Hello that = (Hello) o; + return Objects.equals(this.message, that.message); + } + + @Override + public int hashCode() { + return Objects.hash(this.message); + } + + } + +} diff --git a/src/checkstyle/checkstyle-suppressions.xml b/src/checkstyle/checkstyle-suppressions.xml index 39915018..f96986bf 100644 --- a/src/checkstyle/checkstyle-suppressions.xml +++ b/src/checkstyle/checkstyle-suppressions.xml @@ -16,6 +16,7 @@ +