From d5c176c5f4af474633fbd9865c3f596df707a4c2 Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Mon, 16 Dec 2019 16:02:00 +0100 Subject: [PATCH] Use delegate if absolute url provided. (#264) Fixes gh-259 Fixes gh-257 --- .../openfeign/FeignClientFactoryBean.java | 6 ++ .../FeignBlockingLoadBalancerClient.java | 5 +- .../FeignLoadBalancerAutoConfiguration.java | 2 +- .../openfeign/FeignClientFactoryTests.java | 80 +++++++++++++++++++ 4 files changed, 89 insertions(+), 4 deletions(-) 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 3ea64a91..345a6bba 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 @@ -37,6 +37,7 @@ import org.springframework.beans.BeansException; import org.springframework.beans.factory.FactoryBean; import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.NoSuchBeanDefinitionException; +import org.springframework.cloud.openfeign.loadbalancer.FeignBlockingLoadBalancerClient; import org.springframework.cloud.openfeign.ribbon.LoadBalancerFeignClient; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; @@ -282,6 +283,11 @@ class FeignClientFactoryBean // but ribbon is on the classpath, so unwrap client = ((LoadBalancerFeignClient) client).getDelegate(); } + if (client instanceof FeignBlockingLoadBalancerClient) { + // not load balancing because we have a url, + // but Spring Cloud LoadBalancer is on the classpath, so unwrap + client = ((FeignBlockingLoadBalancerClient) client).getDelegate(); + } builder.client(client); } Targeter targeter = get(context, Targeter.class); diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClient.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClient.java index ef61e1e2..82dd8401 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClient.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClient.java @@ -38,7 +38,7 @@ import org.springframework.util.Assert; * @author Olga Maciaszek-Sharma * @since 2.2.0 */ -class FeignBlockingLoadBalancerClient implements Client { +public class FeignBlockingLoadBalancerClient implements Client { private static final Log LOG = LogFactory .getLog(FeignBlockingLoadBalancerClient.class); @@ -78,8 +78,7 @@ class FeignBlockingLoadBalancerClient implements Client { return delegate.execute(newRequest, options); } - // Visible for tests - Client getDelegate() { + public Client getDelegate() { return delegate; } diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/FeignLoadBalancerAutoConfiguration.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/FeignLoadBalancerAutoConfiguration.java index 16f46f8b..4eafc444 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/FeignLoadBalancerAutoConfiguration.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/FeignLoadBalancerAutoConfiguration.java @@ -52,6 +52,6 @@ import org.springframework.context.annotation.Import; @Import({ HttpClientFeignLoadBalancerConfiguration.class, OkHttpFeignLoadBalancerConfiguration.class, DefaultFeignLoadBalancerConfiguration.class }) -class FeignLoadBalancerAutoConfiguration { +public class FeignLoadBalancerAutoConfiguration { } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientFactoryTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientFactoryTests.java index 07670b13..b1b41ad6 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientFactoryTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/FeignClientFactoryTests.java @@ -16,14 +16,30 @@ package org.springframework.cloud.openfeign; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; +import java.util.Map; +import feign.Client; +import feign.InvocationHandlerFactory; import org.junit.Test; +import org.springframework.boot.test.context.assertj.AssertableApplicationContext; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient; +import org.springframework.cloud.loadbalancer.config.LoadBalancerAutoConfiguration; +import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.web.bind.annotation.RequestMapping; import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.web.bind.annotation.RequestMethod.GET; /** * @author Spencer Gibb @@ -49,10 +65,74 @@ public class FeignClientFactoryTests { assertThat(foobar).as("bar was not null").isNull(); } + @Test + public void shouldRedirectToDelegateWhenUrlSet() { + new ApplicationContextRunner().withUserConfiguration(TestConfig.class) + .run(this::defaultClientUsed); + } + + @SuppressWarnings({ "unchecked", "ConstantConditions" }) + private void defaultClientUsed(AssertableApplicationContext context) { + Proxy target = context.getBean(FeignClientFactoryBean.class).getTarget(); + Object invocationHandler = ReflectionTestUtils.getField(target, "h"); + Map dispatch = (Map) ReflectionTestUtils + .getField(invocationHandler, "dispatch"); + Method key = new ArrayList<>(dispatch.keySet()).get(0); + Object client = ReflectionTestUtils.getField(dispatch.get(key), "client"); + assertThat(client).isInstanceOf(Client.Default.class); + } + private FeignClientSpecification getSpec(String name, Class configClass) { return new FeignClientSpecification(name, new Class[] { configClass }); } + interface TestType { + + @RequestMapping(value = "/", method = GET) + String hello(); + + } + + @Configuration + static class TestConfig { + + @Bean + BlockingLoadBalancerClient loadBalancerClient() { + return new BlockingLoadBalancerClient(new LoadBalancerClientFactory()); + } + + @Bean + FeignContext feignContext() { + FeignContext feignContext = new FeignContext(); + feignContext.setConfigurations( + Collections.singletonList(new FeignClientSpecification("test", + new Class[] { LoadBalancerAutoConfiguration.class }))); + return feignContext; + } + + @Bean + FeignClientProperties feignClientProperties() { + return new FeignClientProperties(); + } + + @Bean + Targeter targeter() { + return new DefaultTargeter(); + } + + @Bean + FeignClientFactoryBean feignClientFactoryBean() { + FeignClientFactoryBean feignClientFactoryBean = new FeignClientFactoryBean(); + feignClientFactoryBean.setContextId("test"); + feignClientFactoryBean.setName("test"); + feignClientFactoryBean.setType(TestType.class); + feignClientFactoryBean.setPath(""); + feignClientFactoryBean.setUrl("http://some.absolute.url"); + return feignClientFactoryBean; + } + + } + static class FooConfig { @Bean