Call get request on delegates (#1250)

This commit is contained in:
Olga Maciaszek-Sharma
2023-06-28 12:28:44 +02:00
committed by GitHub
parent 31c07aa2db
commit 354a894606
11 changed files with 200 additions and 47 deletions

View File

@@ -93,7 +93,7 @@ public class LoadBalancerClientConfiguration {
@Conditional(ZonePreferenceConfigurationCondition.class)
public ServiceInstanceListSupplier zonePreferenceDiscoveryClientServiceInstanceListSupplier(
ConfigurableApplicationContext context) {
return ServiceInstanceListSupplier.builder().withDiscoveryClient().withZonePreference().withCaching()
return ServiceInstanceListSupplier.builder().withDiscoveryClient().withCaching().withZonePreference()
.build(context);
}
@@ -119,8 +119,8 @@ public class LoadBalancerClientConfiguration {
@Conditional(RequestBasedStickySessionConfigurationCondition.class)
public ServiceInstanceListSupplier requestBasedStickySessionDiscoveryClientServiceInstanceListSupplier(
ConfigurableApplicationContext context) {
return ServiceInstanceListSupplier.builder().withDiscoveryClient().withRequestBasedStickySession()
.withCaching().build(context);
return ServiceInstanceListSupplier.builder().withDiscoveryClient().withCaching()
.withRequestBasedStickySession().build(context);
}
@Bean
@@ -129,8 +129,8 @@ public class LoadBalancerClientConfiguration {
@Conditional(SameInstancePreferenceConfigurationCondition.class)
public ServiceInstanceListSupplier sameInstancePreferenceServiceInstanceListSupplier(
ConfigurableApplicationContext context) {
return ServiceInstanceListSupplier.builder().withDiscoveryClient().withSameInstancePreference()
.withCaching().build(context);
return ServiceInstanceListSupplier.builder().withDiscoveryClient().withCaching()
.withSameInstancePreference().build(context);
}
}
@@ -155,8 +155,8 @@ public class LoadBalancerClientConfiguration {
@Conditional(ZonePreferenceConfigurationCondition.class)
public ServiceInstanceListSupplier zonePreferenceDiscoveryClientServiceInstanceListSupplier(
ConfigurableApplicationContext context) {
return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withZonePreference()
.withCaching().build(context);
return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withCaching()
.withZonePreference().build(context);
}
@Bean
@@ -175,8 +175,8 @@ public class LoadBalancerClientConfiguration {
@Conditional(RequestBasedStickySessionConfigurationCondition.class)
public ServiceInstanceListSupplier requestBasedStickySessionDiscoveryClientServiceInstanceListSupplier(
ConfigurableApplicationContext context) {
return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withRequestBasedStickySession()
.withCaching().build(context);
return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withCaching()
.withRequestBasedStickySession().build(context);
}
@Bean
@@ -185,8 +185,8 @@ public class LoadBalancerClientConfiguration {
@Conditional(SameInstancePreferenceConfigurationCondition.class)
public ServiceInstanceListSupplier sameInstancePreferenceServiceInstanceListSupplier(
ConfigurableApplicationContext context) {
return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withSameInstancePreference()
.withCaching().build(context);
return ServiceInstanceListSupplier.builder().withBlockingDiscoveryClient().withCaching()
.withSameInstancePreference().build(context);
}
}

View File

@@ -38,12 +38,12 @@ public abstract class DelegatingServiceInstanceListSupplier
}
public ServiceInstanceListSupplier getDelegate() {
return this.delegate;
return delegate;
}
@Override
public String getServiceId() {
return this.delegate.getServiceId();
return delegate.getServiceId();
}
@Override

View File

@@ -24,6 +24,8 @@ import org.apache.commons.logging.LogFactory;
import reactor.core.publisher.Flux;
import org.springframework.cloud.client.ServiceInstance;
import org.springframework.cloud.client.loadbalancer.Request;
import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer;
/**
* An implementation of {@link ServiceInstanceListSupplier} that selects the previously
@@ -39,10 +41,19 @@ public class SameInstancePreferenceServiceInstanceListSupplier extends Delegatin
private ServiceInstance previouslyReturnedInstance;
private boolean callGetWithRequestOnDelegates;
public SameInstancePreferenceServiceInstanceListSupplier(ServiceInstanceListSupplier delegate) {
super(delegate);
}
public SameInstancePreferenceServiceInstanceListSupplier(ServiceInstanceListSupplier delegate,
ReactiveLoadBalancer.Factory<ServiceInstance> loadBalancerClientFactory) {
super(delegate);
callGetWithRequestOnDelegates = loadBalancerClientFactory.getProperties(getServiceId())
.isCallGetWithRequestOnDelegates();
}
@Override
public String getServiceId() {
return delegate.getServiceId();
@@ -53,6 +64,14 @@ public class SameInstancePreferenceServiceInstanceListSupplier extends Delegatin
return delegate.get().map(this::filteredBySameInstancePreference);
}
@Override
public Flux<List<ServiceInstance>> get(Request request) {
if (callGetWithRequestOnDelegates) {
return delegate.get(request).map(this::filteredBySameInstancePreference);
}
return get();
}
private List<ServiceInstance> filteredBySameInstancePreference(List<ServiceInstance> serviceInstances) {
if (previouslyReturnedInstance != null && serviceInstances.contains(previouslyReturnedInstance)) {
if (LOG.isDebugEnabled()) {

View File

@@ -57,8 +57,6 @@ public final class ServiceInstanceListSupplierBuilder {
private Creator baseCreator;
private DelegateCreator cachingCreator;
private final List<DelegateCreator> creators = new ArrayList<>();
ServiceInstanceListSupplierBuilder() {
@@ -148,8 +146,10 @@ public final class ServiceInstanceListSupplierBuilder {
* @return the {@link ServiceInstanceListSupplierBuilder} object
*/
public ServiceInstanceListSupplierBuilder withSameInstancePreference() {
DelegateCreator creator = (context,
delegate) -> new SameInstancePreferenceServiceInstanceListSupplier(delegate);
DelegateCreator creator = (context, delegate) -> {
LoadBalancerClientFactory loadBalancerClientFactory = context.getBean(LoadBalancerClientFactory.class);
return new SameInstancePreferenceServiceInstanceListSupplier(delegate, loadBalancerClientFactory);
};
this.creators.add(creator);
return this;
}
@@ -191,8 +191,9 @@ public final class ServiceInstanceListSupplierBuilder {
*/
public ServiceInstanceListSupplierBuilder withZonePreference() {
DelegateCreator creator = (context, delegate) -> {
LoadBalancerClientFactory loadBalancerClientFactory = context.getBean(LoadBalancerClientFactory.class);
LoadBalancerZoneConfig zoneConfig = context.getBean(LoadBalancerZoneConfig.class);
return new ZonePreferenceServiceInstanceListSupplier(delegate, zoneConfig);
return new ZonePreferenceServiceInstanceListSupplier(delegate, zoneConfig, loadBalancerClientFactory);
};
this.creators.add(creator);
return this;
@@ -206,8 +207,9 @@ public final class ServiceInstanceListSupplierBuilder {
*/
public ServiceInstanceListSupplierBuilder withZonePreference(String zoneName) {
DelegateCreator creator = (context, delegate) -> {
LoadBalancerClientFactory loadBalancerClientFactory = context.getBean(LoadBalancerClientFactory.class);
LoadBalancerZoneConfig zoneConfig = new LoadBalancerZoneConfig(zoneName);
return new ZonePreferenceServiceInstanceListSupplier(delegate, zoneConfig);
return new ZonePreferenceServiceInstanceListSupplier(delegate, zoneConfig, loadBalancerClientFactory);
};
this.creators.add(creator);
return this;
@@ -228,19 +230,15 @@ public final class ServiceInstanceListSupplierBuilder {
}
/**
* If {@link LoadBalancerCacheManager} is available in the context, wraps created
* {@link ServiceInstanceListSupplier} hierarchy with a
* {@link CachingServiceInstanceListSupplier} instance to provide a caching mechanism
* for service instances. Uses {@link ObjectProvider} to lazily resolve
* If {@link LoadBalancerCacheManager} is available in the context, adds a
* {@link CachingServiceInstanceListSupplier} instance to the
* {@link ServiceInstanceListSupplier} hierarchy to provide a caching mechanism for
* service instances. Uses {@link ObjectProvider} to lazily resolve
* {@link LoadBalancerCacheManager}.
* @return the {@link ServiceInstanceListSupplierBuilder} object
*/
public ServiceInstanceListSupplierBuilder withCaching() {
if (cachingCreator != null && LOG.isWarnEnabled()) {
LOG.warn(
"Overriding a previously set cachingCreator with a CachingServiceInstanceListSupplier-based cachingCreator.");
}
this.cachingCreator = (context, delegate) -> {
DelegateCreator creator = (context, delegate) -> {
ObjectProvider<LoadBalancerCacheManager> cacheManagerProvider = context
.getBeanProvider(LoadBalancerCacheManager.class);
if (cacheManagerProvider.getIfAvailable() != null) {
@@ -251,6 +249,7 @@ public final class ServiceInstanceListSupplierBuilder {
}
return delegate;
};
creators.add(creator);
return this;
}
@@ -297,9 +296,6 @@ public final class ServiceInstanceListSupplierBuilder {
supplier = creator.apply(context, supplier);
}
if (this.cachingCreator != null) {
supplier = this.cachingCreator.apply(context, supplier);
}
return supplier;
}

View File

@@ -23,6 +23,8 @@ import java.util.Map;
import reactor.core.publisher.Flux;
import org.springframework.cloud.client.ServiceInstance;
import org.springframework.cloud.client.loadbalancer.Request;
import org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancer;
import org.springframework.cloud.loadbalancer.config.LoadBalancerZoneConfig;
/**
@@ -43,17 +45,36 @@ public class ZonePreferenceServiceInstanceListSupplier extends DelegatingService
private String zone;
private boolean callGetWithRequestOnDelegates;
public ZonePreferenceServiceInstanceListSupplier(ServiceInstanceListSupplier delegate,
LoadBalancerZoneConfig zoneConfig) {
super(delegate);
this.zoneConfig = zoneConfig;
}
public ZonePreferenceServiceInstanceListSupplier(ServiceInstanceListSupplier delegate,
LoadBalancerZoneConfig zoneConfig,
ReactiveLoadBalancer.Factory<ServiceInstance> loadBalancerClientFactory) {
super(delegate);
this.zoneConfig = zoneConfig;
callGetWithRequestOnDelegates = loadBalancerClientFactory.getProperties(getServiceId())
.isCallGetWithRequestOnDelegates();
}
@Override
public Flux<List<ServiceInstance>> get() {
return getDelegate().get().map(this::filteredByZone);
}
@Override
public Flux<List<ServiceInstance>> get(Request request) {
if (callGetWithRequestOnDelegates) {
return getDelegate().get(request).map(this::filteredByZone);
}
return get();
}
private List<ServiceInstance> filteredByZone(List<ServiceInstance> serviceInstances) {
if (zone == null) {
zone = zoneConfig.getZone();

View File

@@ -91,10 +91,10 @@ class LoadBalancerClientConfigurationTests {
reactiveDiscoveryClientRunner.withPropertyValues("spring.cloud.loadbalancer.configurations=zone-preference")
.run(context -> {
ServiceInstanceListSupplier supplier = context.getBean(ServiceInstanceListSupplier.class);
then(supplier).isInstanceOf(CachingServiceInstanceListSupplier.class);
then(supplier).isInstanceOf(ZonePreferenceServiceInstanceListSupplier.class);
ServiceInstanceListSupplier delegate = ((DelegatingServiceInstanceListSupplier) supplier)
.getDelegate();
then(delegate).isInstanceOf(ZonePreferenceServiceInstanceListSupplier.class);
then(delegate).isInstanceOf(CachingServiceInstanceListSupplier.class);
ServiceInstanceListSupplier secondDelegate = ((DelegatingServiceInstanceListSupplier) delegate)
.getDelegate();
then(secondDelegate).isInstanceOf(DiscoveryClientServiceInstanceListSupplier.class);
@@ -119,10 +119,10 @@ class LoadBalancerClientConfigurationTests {
.withPropertyValues("spring.cloud.loadbalancer.configurations=request-based-sticky-session")
.run(context -> {
ServiceInstanceListSupplier supplier = context.getBean(ServiceInstanceListSupplier.class);
then(supplier).isInstanceOf(CachingServiceInstanceListSupplier.class);
then(supplier).isInstanceOf(RequestBasedStickySessionServiceInstanceListSupplier.class);
ServiceInstanceListSupplier delegate = ((DelegatingServiceInstanceListSupplier) supplier)
.getDelegate();
then(delegate).isInstanceOf(RequestBasedStickySessionServiceInstanceListSupplier.class);
then(delegate).isInstanceOf(CachingServiceInstanceListSupplier.class);
ServiceInstanceListSupplier secondDelegate = ((DelegatingServiceInstanceListSupplier) delegate)
.getDelegate();
then(secondDelegate).isInstanceOf(DiscoveryClientServiceInstanceListSupplier.class);

View File

@@ -19,13 +19,20 @@ package org.springframework.cloud.loadbalancer.core;
import java.util.Arrays;
import java.util.List;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import reactor.core.publisher.Flux;
import org.springframework.cloud.client.DefaultServiceInstance;
import org.springframework.cloud.client.ServiceInstance;
import org.springframework.cloud.client.loadbalancer.DefaultRequest;
import org.springframework.cloud.client.loadbalancer.DefaultRequestContext;
import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties;
import org.springframework.cloud.client.loadbalancer.Request;
import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -39,8 +46,9 @@ class SameInstancePreferenceServiceInstanceListSupplierTests {
private final DiscoveryClientServiceInstanceListSupplier delegate = mock(
DiscoveryClientServiceInstanceListSupplier.class);
private final SameInstancePreferenceServiceInstanceListSupplier supplier = new SameInstancePreferenceServiceInstanceListSupplier(
delegate);
private final LoadBalancerClientFactory loadBalancerClientFactory = mock(LoadBalancerClientFactory.class);
private SameInstancePreferenceServiceInstanceListSupplier supplier;
private final ServiceInstance first = serviceInstance("test-1");
@@ -48,6 +56,14 @@ class SameInstancePreferenceServiceInstanceListSupplierTests {
private final ServiceInstance third = serviceInstance("test-3");
@BeforeEach
void setUp() {
LoadBalancerProperties properties = new LoadBalancerProperties();
properties.setCallGetWithRequestOnDelegates(true);
when(loadBalancerClientFactory.getProperties(any())).thenReturn(properties);
supplier = new SameInstancePreferenceServiceInstanceListSupplier(delegate, loadBalancerClientFactory);
}
@Test
void shouldReturnPreviouslySelectedInstanceIfAvailable() {
when(delegate.get()).thenReturn(Flux.just(Arrays.asList(first, second, third)));
@@ -69,7 +85,7 @@ class SameInstancePreferenceServiceInstanceListSupplierTests {
}
@Test
void shouldReturnAllInstancesFromDelegateIfPreviouslySelectedInstanceIfAvailable() {
void shouldReturnAllInstancesFromDelegateIfPreviouslySelectedInstanceIsNotAvailable() {
when(delegate.get()).thenReturn(Flux.just(Arrays.asList(second, third)));
supplier.selectedServiceInstance(first);
@@ -78,6 +94,17 @@ class SameInstancePreferenceServiceInstanceListSupplierTests {
assertThat(instances).hasSize(2);
}
@Test
void shouldCallGetRequestOnDelegate() {
Request<DefaultRequestContext> request = new DefaultRequest<>(new DefaultRequestContext());
when(delegate.get()).thenReturn(Flux.just(Arrays.asList(first, second, third)));
when(delegate.get(request)).thenReturn(Flux.just(Arrays.asList(first, second)));
List<ServiceInstance> instances = supplier.get(request).blockFirst();
assertThat(instances).hasSize(2);
}
private DefaultServiceInstance serviceInstance(String instanceId) {
return new DefaultServiceInstance(instanceId, "test", "http://test.test", 9080, false);
}

View File

@@ -37,11 +37,9 @@ public class ServiceInstanceListSupplierBuilderTests {
public void testBuilder() {
new ApplicationContextRunner().withUserConfiguration(CacheTestConfig.class).run(context -> {
ServiceInstanceListSupplier supplier = ServiceInstanceListSupplier.builder().withDiscoveryClient()
.withHealthChecks().withCaching().build(context);
assertThat(supplier).isInstanceOf(CachingServiceInstanceListSupplier.class);
.withHealthChecks().build(context);
assertThat(supplier).isInstanceOf(HealthCheckServiceInstanceListSupplier.class);
DelegatingServiceInstanceListSupplier delegating = (DelegatingServiceInstanceListSupplier) supplier;
assertThat(delegating.getDelegate()).isInstanceOf(HealthCheckServiceInstanceListSupplier.class);
delegating = (DelegatingServiceInstanceListSupplier) delegating.getDelegate();
assertThat(delegating.getDelegate()).isInstanceOf(DiscoveryClientServiceInstanceListSupplier.class);
});
}

View File

@@ -22,15 +22,22 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import reactor.core.publisher.Flux;
import org.springframework.cloud.client.DefaultServiceInstance;
import org.springframework.cloud.client.ServiceInstance;
import org.springframework.cloud.client.loadbalancer.DefaultRequest;
import org.springframework.cloud.client.loadbalancer.DefaultRequestContext;
import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties;
import org.springframework.cloud.client.loadbalancer.Request;
import org.springframework.cloud.loadbalancer.config.LoadBalancerZoneConfig;
import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
@@ -46,8 +53,9 @@ class ZonePreferenceServiceInstanceListSupplierTests {
private final LoadBalancerZoneConfig zoneConfig = new LoadBalancerZoneConfig(null);
private final ZonePreferenceServiceInstanceListSupplier supplier = new ZonePreferenceServiceInstanceListSupplier(
delegate, zoneConfig);
private ZonePreferenceServiceInstanceListSupplier supplier;
private final LoadBalancerClientFactory loadBalancerClientFactory = mock(LoadBalancerClientFactory.class);
private final ServiceInstance first = serviceInstance("test-1", buildZoneMetadata("zone1"));
@@ -59,6 +67,14 @@ class ZonePreferenceServiceInstanceListSupplierTests {
private final ServiceInstance fifth = serviceInstance("test-5", buildZoneMetadata(null));
@BeforeEach
void setUp() {
LoadBalancerProperties properties = new LoadBalancerProperties();
properties.setCallGetWithRequestOnDelegates(true);
when(loadBalancerClientFactory.getProperties(any())).thenReturn(properties);
supplier = new ZonePreferenceServiceInstanceListSupplier(delegate, zoneConfig, loadBalancerClientFactory);
}
@Test
void shouldFilterInstancesByZone() {
zoneConfig.setZone("zone1");
@@ -73,6 +89,19 @@ class ZonePreferenceServiceInstanceListSupplierTests {
assertThat(filtered).doesNotContain(fifth);
}
@Test
void shouldCallGetRequestOnDelegate() {
zoneConfig.setZone("zone1");
Request<DefaultRequestContext> request = new DefaultRequest<>(new DefaultRequestContext());
when(delegate.get()).thenReturn(Flux.just(Arrays.asList(first, second, third, fourth, fifth)));
when(delegate.get(request)).thenReturn(Flux.just(Arrays.asList(first, third, fourth, fifth)));
List<ServiceInstance> filtered = supplier.get(request).blockFirst();
assertThat(filtered).hasSize(1);
assertThat(filtered).containsOnly(first);
}
@Test
void shouldReturnAllInstancesIfNoZoneInstances() {
zoneConfig.setZone("zone1");