From 5248f3fc326efed72d5bd48e6b4cab37b95b1e5b Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Wed, 17 Oct 2018 20:11:56 -0400 Subject: [PATCH] Return a client response from filter function instead of throwing an exception. Fixes #386 (#430) --- .../LoadBalancerExchangeFilterFunction.java | 20 +++++++++++--- ...adBalancerExchangeFilterFunctionTests.java | 26 +++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerExchangeFilterFunction.java b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerExchangeFilterFunction.java index b7c30c63..db8f239b 100644 --- a/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerExchangeFilterFunction.java +++ b/spring-cloud-commons/src/main/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerExchangeFilterFunction.java @@ -2,9 +2,11 @@ package org.springframework.cloud.client.loadbalancer.reactive; import java.net.URI; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; -import org.springframework.util.Assert; +import org.springframework.http.HttpStatus; import org.springframework.web.reactive.function.client.ClientRequest; import org.springframework.web.reactive.function.client.ClientResponse; import org.springframework.web.reactive.function.client.ExchangeFilterFunction; @@ -14,9 +16,13 @@ import reactor.core.publisher.Mono; /** * @author Spencer Gibb + * @author Ryan Baxter */ public class LoadBalancerExchangeFilterFunction implements ExchangeFilterFunction { + private static Log logger = LogFactory + .getLog(LoadBalancerExchangeFilterFunction.class); + private final LoadBalancerClient loadBalancerClient; public LoadBalancerExchangeFilterFunction(LoadBalancerClient loadBalancerClient) { @@ -27,10 +33,18 @@ public class LoadBalancerExchangeFilterFunction implements ExchangeFilterFunctio public Mono filter(ClientRequest request, ExchangeFunction next) { URI originalUrl = request.url(); String serviceId = originalUrl.getHost(); - Assert.state(serviceId != null, "Request URI does not contain a valid hostname: " + originalUrl); + if(serviceId == null) { + String msg = String.format("Request URI does not contain a valid hostname: %s", originalUrl.toString()); + logger.warn(msg); + return Mono.just(ClientResponse.create(HttpStatus.BAD_REQUEST).body(msg).build()); + } //TODO: reactive lb client - ServiceInstance instance = this.loadBalancerClient.choose(serviceId); + if(instance == null) { + String msg = String.format("Load balancer does not contain an instance for the service %s", serviceId); + logger.warn(msg); + return Mono.just(ClientResponse.create(HttpStatus.SERVICE_UNAVAILABLE).body(msg).build()); + } URI uri = this.loadBalancerClient.reconstructURI(instance, originalUrl); ClientRequest newRequest = ClientRequest.method(request.method(), uri) .headers(headers -> headers.addAll(request.headers())) diff --git a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerExchangeFilterFunctionTests.java b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerExchangeFilterFunctionTests.java index 2f22f8e9..b45072fa 100644 --- a/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerExchangeFilterFunctionTests.java +++ b/spring-cloud-commons/src/test/java/org/springframework/cloud/client/loadbalancer/reactive/LoadBalancerExchangeFilterFunctionTests.java @@ -22,9 +22,11 @@ import org.springframework.cloud.client.discovery.simple.SimpleDiscoveryProperti import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; import org.springframework.cloud.client.loadbalancer.LoadBalancerRequest; import org.springframework.context.annotation.Bean; +import org.springframework.http.HttpStatus; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.reactive.function.client.ClientResponse; import org.springframework.web.reactive.function.client.WebClient; import org.springframework.web.util.UriComponentsBuilder; @@ -33,6 +35,7 @@ import static org.springframework.boot.test.context.SpringBootTest.WebEnvironmen /** * @author Spencer Gibb + * @author Ryan Baxter */ @RunWith(SpringRunner.class) @SpringBootTest(webEnvironment = RANDOM_PORT) @@ -68,6 +71,26 @@ public class LoadBalancerExchangeFilterFunctionTests { assertThat(value).isEqualTo("Hello World"); } + @Test + public void testNoInstance() { + ClientResponse clientResponse = WebClient.builder() + .baseUrl("http://foobar") + .filter(lbFunction) + .build() + .get().exchange().block(); + assertThat(clientResponse.statusCode()).isEqualTo(HttpStatus.SERVICE_UNAVAILABLE); + } + + @Test + public void testNoHostName() { + ClientResponse clientResponse = WebClient.builder() + .baseUrl("http:///foobar") + .filter(lbFunction) + .build() + .get().exchange().block(); + assertThat(clientResponse.statusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + } + @EnableDiscoveryClient @EnableAutoConfiguration @SpringBootConfiguration @@ -106,6 +129,9 @@ public class LoadBalancerExchangeFilterFunctionTests { @Override public ServiceInstance choose(String serviceId) { List instances = discoveryClient.getInstances(serviceId); + if(instances.size() == 0) { + return null; + } int instanceIdx = random.nextInt(instances.size()); return instances.get(instanceIdx); }