From 38ccf1be95c239c18c86a01bfb0943543318270a Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Tue, 2 Nov 2021 17:24:43 +0100 Subject: [PATCH] Use LB per-client properties. (#622) --- .../DefaultFeignLoadBalancerConfiguration.java | 10 ++++------ .../FeignBlockingLoadBalancerClient.java | 16 +++++++++++++--- ...tpClient5FeignLoadBalancerConfiguration.java | 10 ++++------ ...ttpClientFeignLoadBalancerConfiguration.java | 10 ++++------ .../OkHttpFeignLoadBalancerConfiguration.java | 10 ++++------ ...etryableFeignBlockingLoadBalancerClient.java | 17 ++++++++++++++--- .../FeignBlockingLoadBalancerClientTests.java | 14 ++++++++++---- ...bleFeignBlockingLoadBalancerClientTests.java | 13 +++++++------ 8 files changed, 60 insertions(+), 40 deletions(-) diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/DefaultFeignLoadBalancerConfiguration.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/DefaultFeignLoadBalancerConfiguration.java index f910cada..856e4e56 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/DefaultFeignLoadBalancerConfiguration.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/DefaultFeignLoadBalancerConfiguration.java @@ -26,7 +26,6 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; import org.springframework.cloud.client.loadbalancer.LoadBalancerClientsProperties; -import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties; import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Conditional; @@ -46,9 +45,9 @@ class DefaultFeignLoadBalancerConfiguration { @Bean @ConditionalOnMissingBean @Conditional(OnRetryNotEnabledCondition.class) - public Client feignClient(LoadBalancerClient loadBalancerClient, LoadBalancerProperties properties, + public Client feignClient(LoadBalancerClient loadBalancerClient, LoadBalancerClientFactory loadBalancerClientFactory) { - return new FeignBlockingLoadBalancerClient(new Client.Default(null, null), loadBalancerClient, properties, + return new FeignBlockingLoadBalancerClient(new Client.Default(null, null), loadBalancerClient, loadBalancerClientFactory); } @@ -59,10 +58,9 @@ class DefaultFeignLoadBalancerConfiguration { @ConditionalOnProperty(value = "spring.cloud.loadbalancer.retry.enabled", havingValue = "true", matchIfMissing = true) public Client feignRetryClient(LoadBalancerClient loadBalancerClient, - LoadBalancedRetryFactory loadBalancedRetryFactory, LoadBalancerProperties properties, - LoadBalancerClientFactory loadBalancerClientFactory) { + LoadBalancedRetryFactory loadBalancedRetryFactory, LoadBalancerClientFactory loadBalancerClientFactory) { return new RetryableFeignBlockingLoadBalancerClient(new Client.Default(null, null), loadBalancerClient, - loadBalancedRetryFactory, properties, loadBalancerClientFactory); + loadBalancedRetryFactory, loadBalancerClientFactory); } } 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 9c3751d2..9fd5429b 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 @@ -60,15 +60,24 @@ public class FeignBlockingLoadBalancerClient implements Client { private final LoadBalancerClient loadBalancerClient; - private final LoadBalancerProperties properties; - private final LoadBalancerClientFactory loadBalancerClientFactory; + /** + * @deprecated in favour of + * {@link FeignBlockingLoadBalancerClient#FeignBlockingLoadBalancerClient(Client, LoadBalancerClient, LoadBalancerClientFactory)} + */ + @Deprecated public FeignBlockingLoadBalancerClient(Client delegate, LoadBalancerClient loadBalancerClient, LoadBalancerProperties properties, LoadBalancerClientFactory loadBalancerClientFactory) { this.delegate = delegate; this.loadBalancerClient = loadBalancerClient; - this.properties = properties; + this.loadBalancerClientFactory = loadBalancerClientFactory; + } + + public FeignBlockingLoadBalancerClient(Client delegate, LoadBalancerClient loadBalancerClient, + LoadBalancerClientFactory loadBalancerClientFactory) { + this.delegate = delegate; + this.loadBalancerClient = loadBalancerClient; this.loadBalancerClientFactory = loadBalancerClientFactory; } @@ -116,6 +125,7 @@ public class FeignBlockingLoadBalancerClient implements Client { } private String getHint(String serviceId) { + LoadBalancerProperties properties = loadBalancerClientFactory.getProperties(serviceId); String defaultHint = properties.getHint().getOrDefault("default", "default"); String hintPropertyValue = properties.getHint().get(serviceId); return hintPropertyValue != null ? hintPropertyValue : defaultHint; diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/HttpClient5FeignLoadBalancerConfiguration.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/HttpClient5FeignLoadBalancerConfiguration.java index d3000207..6c3e8d09 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/HttpClient5FeignLoadBalancerConfiguration.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/HttpClient5FeignLoadBalancerConfiguration.java @@ -28,7 +28,6 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; import org.springframework.cloud.client.loadbalancer.LoadBalancerClientsProperties; -import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties; import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; import org.springframework.cloud.openfeign.clientconfig.HttpClient5FeignConfiguration; import org.springframework.context.annotation.Bean; @@ -54,9 +53,9 @@ class HttpClient5FeignLoadBalancerConfiguration { @ConditionalOnMissingBean @Conditional(OnRetryNotEnabledCondition.class) public Client feignClient(LoadBalancerClient loadBalancerClient, HttpClient httpClient5, - LoadBalancerProperties properties, LoadBalancerClientFactory loadBalancerClientFactory) { + LoadBalancerClientFactory loadBalancerClientFactory) { Client delegate = new ApacheHttp5Client(httpClient5); - return new FeignBlockingLoadBalancerClient(delegate, loadBalancerClient, properties, loadBalancerClientFactory); + return new FeignBlockingLoadBalancerClient(delegate, loadBalancerClient, loadBalancerClientFactory); } @Bean @@ -66,11 +65,10 @@ class HttpClient5FeignLoadBalancerConfiguration { @ConditionalOnProperty(value = "spring.cloud.loadbalancer.retry.enabled", havingValue = "true", matchIfMissing = true) public Client feignRetryClient(LoadBalancerClient loadBalancerClient, HttpClient httpClient5, - LoadBalancedRetryFactory loadBalancedRetryFactory, LoadBalancerProperties properties, - LoadBalancerClientFactory loadBalancerClientFactory) { + LoadBalancedRetryFactory loadBalancedRetryFactory, LoadBalancerClientFactory loadBalancerClientFactory) { Client delegate = new ApacheHttp5Client(httpClient5); return new RetryableFeignBlockingLoadBalancerClient(delegate, loadBalancerClient, loadBalancedRetryFactory, - properties, loadBalancerClientFactory); + loadBalancerClientFactory); } } diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/HttpClientFeignLoadBalancerConfiguration.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/HttpClientFeignLoadBalancerConfiguration.java index 56d4fb2a..57400c01 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/HttpClientFeignLoadBalancerConfiguration.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/HttpClientFeignLoadBalancerConfiguration.java @@ -28,7 +28,6 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; import org.springframework.cloud.client.loadbalancer.LoadBalancerClientsProperties; -import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties; import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; import org.springframework.cloud.openfeign.HttpClient5DisabledConditions; import org.springframework.cloud.openfeign.clientconfig.HttpClientFeignConfiguration; @@ -58,9 +57,9 @@ class HttpClientFeignLoadBalancerConfiguration { @ConditionalOnMissingBean @Conditional(OnRetryNotEnabledCondition.class) public Client feignClient(LoadBalancerClient loadBalancerClient, HttpClient httpClient, - LoadBalancerProperties properties, LoadBalancerClientFactory loadBalancerClientFactory) { + LoadBalancerClientFactory loadBalancerClientFactory) { ApacheHttpClient delegate = new ApacheHttpClient(httpClient); - return new FeignBlockingLoadBalancerClient(delegate, loadBalancerClient, properties, loadBalancerClientFactory); + return new FeignBlockingLoadBalancerClient(delegate, loadBalancerClient, loadBalancerClientFactory); } @Bean @@ -70,11 +69,10 @@ class HttpClientFeignLoadBalancerConfiguration { @ConditionalOnProperty(value = "spring.cloud.loadbalancer.retry.enabled", havingValue = "true", matchIfMissing = true) public Client feignRetryClient(LoadBalancerClient loadBalancerClient, HttpClient httpClient, - LoadBalancedRetryFactory loadBalancedRetryFactory, LoadBalancerProperties properties, - LoadBalancerClientFactory loadBalancerClientFactory) { + LoadBalancedRetryFactory loadBalancedRetryFactory, LoadBalancerClientFactory loadBalancerClientFactory) { ApacheHttpClient delegate = new ApacheHttpClient(httpClient); return new RetryableFeignBlockingLoadBalancerClient(delegate, loadBalancerClient, loadBalancedRetryFactory, - properties, loadBalancerClientFactory); + loadBalancerClientFactory); } } diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/OkHttpFeignLoadBalancerConfiguration.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/OkHttpFeignLoadBalancerConfiguration.java index 54be57cd..db24319b 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/OkHttpFeignLoadBalancerConfiguration.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/OkHttpFeignLoadBalancerConfiguration.java @@ -27,7 +27,6 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; import org.springframework.cloud.client.loadbalancer.LoadBalancerClientsProperties; -import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties; import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; import org.springframework.cloud.openfeign.clientconfig.OkHttpFeignConfiguration; import org.springframework.context.annotation.Bean; @@ -54,9 +53,9 @@ class OkHttpFeignLoadBalancerConfiguration { @ConditionalOnMissingBean @Conditional(OnRetryNotEnabledCondition.class) public Client feignClient(okhttp3.OkHttpClient okHttpClient, LoadBalancerClient loadBalancerClient, - LoadBalancerProperties properties, LoadBalancerClientFactory loadBalancerClientFactory) { + LoadBalancerClientFactory loadBalancerClientFactory) { OkHttpClient delegate = new OkHttpClient(okHttpClient); - return new FeignBlockingLoadBalancerClient(delegate, loadBalancerClient, properties, loadBalancerClientFactory); + return new FeignBlockingLoadBalancerClient(delegate, loadBalancerClient, loadBalancerClientFactory); } @Bean @@ -66,11 +65,10 @@ class OkHttpFeignLoadBalancerConfiguration { @ConditionalOnProperty(value = "spring.cloud.loadbalancer.retry.enabled", havingValue = "true", matchIfMissing = true) public Client feignRetryClient(LoadBalancerClient loadBalancerClient, okhttp3.OkHttpClient okHttpClient, - LoadBalancedRetryFactory loadBalancedRetryFactory, LoadBalancerProperties properties, - LoadBalancerClientFactory loadBalancerClientFactory) { + LoadBalancedRetryFactory loadBalancedRetryFactory, LoadBalancerClientFactory loadBalancerClientFactory) { OkHttpClient delegate = new OkHttpClient(okHttpClient); return new RetryableFeignBlockingLoadBalancerClient(delegate, loadBalancerClient, loadBalancedRetryFactory, - properties, loadBalancerClientFactory); + loadBalancerClientFactory); } } diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClient.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClient.java index 6883a179..0c2ef203 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClient.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClient.java @@ -78,17 +78,27 @@ public class RetryableFeignBlockingLoadBalancerClient implements Client { private final LoadBalancedRetryFactory loadBalancedRetryFactory; - private final LoadBalancerProperties properties; - private final LoadBalancerClientFactory loadBalancerClientFactory; + /** + * @deprecated in favour of + * {@link RetryableFeignBlockingLoadBalancerClient#RetryableFeignBlockingLoadBalancerClient(Client, LoadBalancerClient, LoadBalancedRetryFactory, LoadBalancerClientFactory)} + */ + @Deprecated public RetryableFeignBlockingLoadBalancerClient(Client delegate, LoadBalancerClient loadBalancerClient, LoadBalancedRetryFactory loadBalancedRetryFactory, LoadBalancerProperties properties, LoadBalancerClientFactory loadBalancerClientFactory) { this.delegate = delegate; this.loadBalancerClient = loadBalancerClient; this.loadBalancedRetryFactory = loadBalancedRetryFactory; - this.properties = properties; + this.loadBalancerClientFactory = loadBalancerClientFactory; + } + + public RetryableFeignBlockingLoadBalancerClient(Client delegate, LoadBalancerClient loadBalancerClient, + LoadBalancedRetryFactory loadBalancedRetryFactory, LoadBalancerClientFactory loadBalancerClientFactory) { + this.delegate = delegate; + this.loadBalancerClient = loadBalancerClient; + this.loadBalancedRetryFactory = loadBalancedRetryFactory; this.loadBalancerClientFactory = loadBalancerClientFactory; } @@ -232,6 +242,7 @@ public class RetryableFeignBlockingLoadBalancerClient implements Client { } private String getHint(String serviceId) { + LoadBalancerProperties properties = loadBalancerClientFactory.getProperties(serviceId); String defaultHint = properties.getHint().getOrDefault("default", "default"); String hintPropertyValue = properties.getHint().get(serviceId); return hintPropertyValue != null ? hintPropertyValue : defaultHint; diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClientTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClientTests.java index 468df6a1..ed9fb9d7 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClientTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClientTests.java @@ -31,6 +31,7 @@ import java.util.concurrent.ConcurrentHashMap; import feign.Client; import feign.Request; import feign.Response; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentCaptor; @@ -71,16 +72,21 @@ import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class FeignBlockingLoadBalancerClientTests { - private Client delegate = mock(Client.class); + private final Client delegate = mock(Client.class); - private BlockingLoadBalancerClient loadBalancerClient = mock(BlockingLoadBalancerClient.class); + private final BlockingLoadBalancerClient loadBalancerClient = mock(BlockingLoadBalancerClient.class); private final LoadBalancerClientFactory loadBalancerClientFactory = mock(LoadBalancerClientFactory.class); private final LoadBalancerProperties loadBalancerProperties = new LoadBalancerProperties(); - private FeignBlockingLoadBalancerClient feignBlockingLoadBalancerClient = new FeignBlockingLoadBalancerClient( - delegate, loadBalancerClient, loadBalancerProperties, loadBalancerClientFactory); + private final FeignBlockingLoadBalancerClient feignBlockingLoadBalancerClient = new FeignBlockingLoadBalancerClient( + delegate, loadBalancerClient, loadBalancerClientFactory); + + @BeforeEach + void setUp() { + when(loadBalancerClientFactory.getProperties(any(String.class))).thenReturn(loadBalancerProperties); + } @Test void shouldExtractServiceIdFromRequestUrl() throws IOException { diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClientTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClientTests.java index c8c7de61..76e8d193 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClientTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClientTests.java @@ -76,23 +76,24 @@ import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class RetryableFeignBlockingLoadBalancerClientTests { - private Client delegate = mock(Client.class); + private final Client delegate = mock(Client.class); - private LoadBalancedRetryFactory retryFactory = mock(LoadBalancedRetryFactory.class); + private final LoadBalancedRetryFactory retryFactory = mock(LoadBalancedRetryFactory.class); - private BlockingLoadBalancerClient loadBalancerClient = mock(BlockingLoadBalancerClient.class); + private final BlockingLoadBalancerClient loadBalancerClient = mock(BlockingLoadBalancerClient.class); private final LoadBalancerClientFactory loadBalancerClientFactory = mock(LoadBalancerClientFactory.class); - private LoadBalancerProperties properties = new LoadBalancerProperties(); + private final LoadBalancerProperties properties = new LoadBalancerProperties(); - private RetryableFeignBlockingLoadBalancerClient feignBlockingLoadBalancerClient = new RetryableFeignBlockingLoadBalancerClient( + private final RetryableFeignBlockingLoadBalancerClient feignBlockingLoadBalancerClient = new RetryableFeignBlockingLoadBalancerClient( delegate, loadBalancerClient, retryFactory, properties, loadBalancerClientFactory); - private ServiceInstance serviceInstance = new DefaultServiceInstance("test-a", "test", "testhost", 80, false); + private final ServiceInstance serviceInstance = new DefaultServiceInstance("test-a", "test", "testhost", 80, false); @BeforeEach void setUp() { + when(loadBalancerClientFactory.getProperties(any(String.class))).thenReturn(properties); when(retryFactory.createRetryPolicy(any(), eq(loadBalancerClient))) .thenReturn(new BlockingLoadBalancedRetryPolicy(properties)); when(loadBalancerClient.choose(eq("test"), any())).thenReturn(serviceInstance);