From 188fab7b323af41e9072989788d845fd5482b4a0 Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Fri, 11 Dec 2020 17:41:29 +0100 Subject: [PATCH] Fix after merge. --- .../core/NoopServiceInstanceListSupplier.java | 6 ++ .../loadbalancer/core/RandomLoadBalancer.java | 61 +++++-------------- .../core/RoundRobinLoadBalancer.java | 2 +- .../core/RandomLoadBalancerTests.java | 33 ++++++---- 4 files changed, 44 insertions(+), 58 deletions(-) diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/NoopServiceInstanceListSupplier.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/NoopServiceInstanceListSupplier.java index bfd0a952..fd69bede 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/NoopServiceInstanceListSupplier.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/NoopServiceInstanceListSupplier.java @@ -22,6 +22,7 @@ import java.util.List; import reactor.core.publisher.Flux; import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.Request; /** * A no-op implementation of {@link ServiceInstanceListSupplier}. @@ -40,4 +41,9 @@ public class NoopServiceInstanceListSupplier implements ServiceInstanceListSuppl return Flux.defer(() -> Flux.just(Collections.emptyList())); } + @Override + public Flux> get(Request request) { + return Flux.defer(() -> Flux.just(Collections.emptyList())); + } + } diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RandomLoadBalancer.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RandomLoadBalancer.java index 597c813d..325fb17b 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RandomLoadBalancer.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RandomLoadBalancer.java @@ -25,10 +25,10 @@ import reactor.core.publisher.Mono; import org.springframework.beans.factory.ObjectProvider; import org.springframework.cloud.client.ServiceInstance; -import org.springframework.cloud.client.loadbalancer.reactive.DefaultResponse; -import org.springframework.cloud.client.loadbalancer.reactive.EmptyResponse; -import org.springframework.cloud.client.loadbalancer.reactive.Request; -import org.springframework.cloud.client.loadbalancer.reactive.Response; +import org.springframework.cloud.client.loadbalancer.DefaultResponse; +import org.springframework.cloud.client.loadbalancer.EmptyResponse; +import org.springframework.cloud.client.loadbalancer.Request; +import org.springframework.cloud.client.loadbalancer.Response; /** * A random-based implementation of {@link ReactorServiceInstanceLoadBalancer}. @@ -42,9 +42,6 @@ public class RandomLoadBalancer implements ReactorServiceInstanceLoadBalancer { private final String serviceId; - @Deprecated - private ObjectProvider serviceInstanceSupplier; - private ObjectProvider serviceInstanceListSupplierProvider; /** @@ -52,59 +49,31 @@ public class RandomLoadBalancer implements ReactorServiceInstanceLoadBalancer { * {@link ServiceInstanceListSupplier} that will be used to get available instances * @param serviceId id of the service for which to choose an instance */ - public RandomLoadBalancer( - ObjectProvider serviceInstanceListSupplierProvider, + public RandomLoadBalancer(ObjectProvider serviceInstanceListSupplierProvider, String serviceId) { this.serviceId = serviceId; this.serviceInstanceListSupplierProvider = serviceInstanceListSupplierProvider; } - /** - * @param serviceId id of the service for which to choose an instance - * @param serviceInstanceSupplier a provider of {@link ServiceInstanceSupplier} that - * will be used to get available instances - * @deprecated Use {@link #RandomLoadBalancer(ObjectProvider, String)}} instead. - */ - @Deprecated - public RandomLoadBalancer(String serviceId, - ObjectProvider serviceInstanceSupplier) { - this.serviceId = serviceId; - this.serviceInstanceSupplier = serviceInstanceSupplier; - } - @SuppressWarnings("rawtypes") @Override - public Mono> choose( - Request request) { - // TODO: move supplier to Request? - // Temporary conditional logic till deprecated members are removed. - if (serviceInstanceListSupplierProvider != null) { - ServiceInstanceListSupplier supplier = serviceInstanceListSupplierProvider - .getIfAvailable(NoopServiceInstanceListSupplier::new); - return supplier.get().next() - .map(serviceInstances -> processInstanceResponse(supplier, - serviceInstances)); - } - ServiceInstanceSupplier supplier = this.serviceInstanceSupplier - .getIfAvailable(NoopServiceInstanceSupplier::new); - return supplier.get().collectList().map(this::getInstanceResponse); + public Mono> choose(Request request) { + ServiceInstanceListSupplier supplier = serviceInstanceListSupplierProvider + .getIfAvailable(NoopServiceInstanceListSupplier::new); + return supplier.get(request).next() + .map(serviceInstances -> processInstanceResponse(supplier, serviceInstances)); } - private org.springframework.cloud.client.loadbalancer.reactive.Response processInstanceResponse( - ServiceInstanceListSupplier supplier, + private Response processInstanceResponse(ServiceInstanceListSupplier supplier, List serviceInstances) { - org.springframework.cloud.client.loadbalancer.reactive.Response serviceInstanceResponse = getInstanceResponse( - serviceInstances); - if (supplier instanceof SelectedInstanceCallback - && serviceInstanceResponse.hasServer()) { - ((SelectedInstanceCallback) supplier) - .selectedServiceInstance(serviceInstanceResponse.getServer()); + Response serviceInstanceResponse = getInstanceResponse(serviceInstances); + if (supplier instanceof SelectedInstanceCallback && serviceInstanceResponse.hasServer()) { + ((SelectedInstanceCallback) supplier).selectedServiceInstance(serviceInstanceResponse.getServer()); } return serviceInstanceResponse; } - private Response getInstanceResponse( - List instances) { + private Response getInstanceResponse(List instances) { if (instances.isEmpty()) { if (log.isWarnEnabled()) { log.warn("No servers available for service: " + serviceId); diff --git a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RoundRobinLoadBalancer.java b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RoundRobinLoadBalancer.java index 4ed2f005..dc15f340 100644 --- a/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RoundRobinLoadBalancer.java +++ b/spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/core/RoundRobinLoadBalancer.java @@ -91,7 +91,7 @@ public class RoundRobinLoadBalancer implements ReactorServiceInstanceLoadBalance return serviceInstanceResponse; } - Response getInstanceResponse(List instances) { + private Response getInstanceResponse(List instances) { if (instances.isEmpty()) { if (log.isWarnEnabled()) { log.warn("No servers available for service: " + serviceId); diff --git a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/RandomLoadBalancerTests.java b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/RandomLoadBalancerTests.java index 35ce612d..b57f4f51 100644 --- a/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/RandomLoadBalancerTests.java +++ b/spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/core/RandomLoadBalancerTests.java @@ -28,7 +28,9 @@ import org.springframework.cloud.client.loadbalancer.Response; import org.springframework.cloud.loadbalancer.support.SimpleObjectProvider; 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.verify; import static org.mockito.Mockito.when; /** @@ -44,12 +46,9 @@ class RandomLoadBalancerTests { @Test void shouldReturnOneServiceInstance() { - DiscoveryClientServiceInstanceListSupplier supplier = mock( - DiscoveryClientServiceInstanceListSupplier.class); - when(supplier.get()).thenReturn( - Flux.just(Arrays.asList(serviceInstance, new DefaultServiceInstance()))); - loadBalancer = new RandomLoadBalancer(new SimpleObjectProvider<>(supplier), - "test"); + DiscoveryClientServiceInstanceListSupplier supplier = mock(DiscoveryClientServiceInstanceListSupplier.class); + when(supplier.get(any())).thenReturn(Flux.just(Arrays.asList(serviceInstance, new DefaultServiceInstance()))); + loadBalancer = new RandomLoadBalancer(new SimpleObjectProvider<>(supplier), "test"); Response response = loadBalancer.choose().block(); @@ -67,15 +66,27 @@ class RandomLoadBalancerTests { @Test void shouldReturnEmptyResponseWhenNoInstancesAvailable() { - DiscoveryClientServiceInstanceListSupplier supplier = mock( - DiscoveryClientServiceInstanceListSupplier.class); - when(supplier.get()).thenReturn(Flux.just(Collections.emptyList())); - loadBalancer = new RandomLoadBalancer(new SimpleObjectProvider<>(supplier), - "test"); + DiscoveryClientServiceInstanceListSupplier supplier = mock(DiscoveryClientServiceInstanceListSupplier.class); + when(supplier.get(any())).thenReturn(Flux.just(Collections.emptyList())); + loadBalancer = new RandomLoadBalancer(new SimpleObjectProvider<>(supplier), "test"); Response response = loadBalancer.choose().block(); assertThat(response.hasServer()).isFalse(); } + @Test + void shouldTriggerSelectedInstanceCallback() { + SameInstancePreferenceServiceInstanceListSupplier supplier = mock( + SameInstancePreferenceServiceInstanceListSupplier.class); + when(supplier.get(any())).thenReturn(Flux.just(Collections.singletonList(serviceInstance))); + loadBalancer = new RandomLoadBalancer(new SimpleObjectProvider<>(supplier), "test"); + + Response response = loadBalancer.choose().block(); + + assertThat(response.hasServer()).isTrue(); + assertThat(response.getServer()).isEqualTo(serviceInstance); + verify((SelectedInstanceCallback) supplier).selectedServiceInstance(serviceInstance); + } + }