From 1f026e9a497ed4e51414a7b5f17d2a07da9b1216 Mon Sep 17 00:00:00 2001 From: Tim Ysewyn Date: Mon, 11 Dec 2017 17:04:40 +0100 Subject: [PATCH 1/3] The LoadBalancerClient is now responsible for reconstructing the new URI --- .../gateway/filter/LoadBalancerClientFilter.java | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/LoadBalancerClientFilter.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/LoadBalancerClientFilter.java index c8adb4db..f1f49909 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/LoadBalancerClientFilter.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/LoadBalancerClientFilter.java @@ -26,8 +26,6 @@ import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; import org.springframework.cloud.gateway.support.NotFoundException; import org.springframework.core.Ordered; import org.springframework.web.server.ServerWebExchange; -import org.springframework.cloud.gateway.filter.GatewayFilterChain; -import org.springframework.web.util.UriComponentsBuilder; import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.GATEWAY_REQUEST_URL_ATTR; import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.addOriginalRequestUrl; @@ -36,6 +34,7 @@ import reactor.core.publisher.Mono; /** * @author Spencer Gibb + * @author Tim Ysewyn */ public class LoadBalancerClientFilter implements GlobalFilter, Ordered { @@ -70,14 +69,9 @@ public class LoadBalancerClientFilter implements GlobalFilter, Ordered { throw new NotFoundException("Unable to find instance for " + url.getHost()); } - /*URI uri = exchange.getRequest().getURI(); - URI requestUrl = loadBalancer.reconstructURI(instance, uri);*/ - URI requestUrl = UriComponentsBuilder.fromUri(url) - .scheme(instance.isSecure()? "https" : "http") //TODO: support websockets - .host(instance.getHost()) - .port(instance.getPort()) - .build(true) - .toUri(); + URI uri = exchange.getRequest().getURI(); + URI requestUrl = loadBalancer.reconstructURI(instance, uri); + log.trace("LoadBalancerClientFilter url chosen: " + requestUrl); exchange.getAttributes().put(GATEWAY_REQUEST_URL_ATTR, requestUrl); return chain.filter(exchange); From 09ba3efc6ef8829f7bac692e37102552c21666d5 Mon Sep 17 00:00:00 2001 From: Tim Ysewyn Date: Mon, 11 Dec 2017 17:05:01 +0100 Subject: [PATCH 2/3] Added unit tests for the LoadBalancerClientFilter --- .../filter/LoadBalancerClientFilterTests.java | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/LoadBalancerClientFilterTests.java diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/LoadBalancerClientFilterTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/LoadBalancerClientFilterTests.java new file mode 100644 index 00000000..6ffd3a95 --- /dev/null +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/LoadBalancerClientFilterTests.java @@ -0,0 +1,114 @@ +package org.springframework.cloud.gateway.filter; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.cloud.client.DefaultServiceInstance; +import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; +import org.springframework.cloud.gateway.support.NotFoundException; +import org.springframework.mock.http.server.reactive.MockServerHttpRequest; +import org.springframework.mock.web.server.MockServerWebExchange; +import org.springframework.web.server.ServerWebExchange; +import org.springframework.web.util.UriComponentsBuilder; + +import java.net.URI; +import java.util.LinkedHashSet; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; +import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.GATEWAY_ORIGINAL_REQUEST_URL_ATTR; +import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.GATEWAY_REQUEST_URL_ATTR; + +/** + * @author Tim Ysewyn + */ +@RunWith(MockitoJUnitRunner.class) +public class LoadBalancerClientFilterTests { + + private ServerWebExchange exchange; + + @Mock + private GatewayFilterChain chain; + + @Mock + private LoadBalancerClient loadBalancerClient; + + @InjectMocks + private LoadBalancerClientFilter loadBalancerClientFilter; + + @Before + public void setup() { + exchange = MockServerWebExchange.from(MockServerHttpRequest.get("loadbalancerclient.org").build()); + } + + @Test + public void shouldNotFilterWhenGatewayRequestUrlIsMissing() { + loadBalancerClientFilter.filter(exchange, chain); + + verify(chain).filter(exchange); + verifyNoMoreInteractions(chain); + verifyZeroInteractions(loadBalancerClient); + } + + @Test + public void shouldNotFilterWhenGatewayRequestUrlSchemeIsNotLb() { + URI uri = UriComponentsBuilder.fromUriString("http://myservice").build().toUri(); + exchange.getAttributes().put(GATEWAY_REQUEST_URL_ATTR, uri); + + loadBalancerClientFilter.filter(exchange, chain); + + verify(chain).filter(exchange); + verifyNoMoreInteractions(chain); + verifyZeroInteractions(loadBalancerClient); + } + + @Test(expected = NotFoundException.class) + public void shouldThrowExceptionWhenNoServiceInstanceIsFound() { + URI uri = UriComponentsBuilder.fromUriString("lb://myservice").build().toUri(); + exchange.getAttributes().put(GATEWAY_REQUEST_URL_ATTR, uri); + + loadBalancerClientFilter.filter(exchange, chain); + } + + @Test + public void shouldFilter() { + URI url = UriComponentsBuilder.fromUriString("lb://myservice").build().toUri(); + exchange.getAttributes().put(GATEWAY_REQUEST_URL_ATTR, url); + + ServiceInstance serviceInstance = new DefaultServiceInstance("myservice", "localhost", 8080, true); + when(loadBalancerClient.choose("myservice")).thenReturn(serviceInstance); + + URI requestUrl = UriComponentsBuilder.fromUriString("https://localhost:8080").build().toUri(); + when(loadBalancerClient.reconstructURI(any(ServiceInstance.class), any(URI.class))).thenReturn(requestUrl); + + loadBalancerClientFilter.filter(exchange, chain); + + assertThat((LinkedHashSet)exchange.getAttribute(GATEWAY_ORIGINAL_REQUEST_URL_ATTR)).contains(url); + + verify(loadBalancerClient).choose("myservice"); + + ArgumentCaptor urlArgumentCaptor = ArgumentCaptor.forClass(URI.class); + verify(loadBalancerClient).reconstructURI(eq(serviceInstance), urlArgumentCaptor.capture()); + + URI uri = urlArgumentCaptor.getValue(); + assertThat(uri).isNotNull(); + assertThat(uri.toString()).isEqualTo("loadbalancerclient.org"); + + verifyNoMoreInteractions(loadBalancerClient); + + assertThat((URI)exchange.getAttribute(GATEWAY_REQUEST_URL_ATTR)).isEqualTo(requestUrl); + + verify(chain).filter(exchange); + verifyNoMoreInteractions(chain); + } +} From 84c00a4c75a52ecac2f4cd50e8a2a2fa30148a3c Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Tue, 16 Jan 2018 14:44:35 -0500 Subject: [PATCH 3/3] merged tests --- .../filter/LoadBalancerClientFilterTests.java | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/LoadBalancerClientFilterTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/LoadBalancerClientFilterTests.java index 2ac88c3a..ba80c8a6 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/LoadBalancerClientFilterTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/LoadBalancerClientFilterTests.java @@ -4,6 +4,8 @@ import java.net.URI; import java.util.Collections; import java.util.LinkedHashSet; +import com.netflix.loadbalancer.ILoadBalancer; +import com.netflix.loadbalancer.Server; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -15,6 +17,9 @@ import org.springframework.cloud.client.DefaultServiceInstance; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; import org.springframework.cloud.gateway.support.NotFoundException; +import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerClient; +import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerContext; +import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.http.HttpMethod; import org.springframework.mock.http.server.reactive.MockServerHttpRequest; import org.springframework.mock.web.server.MockServerWebExchange; @@ -196,18 +201,20 @@ public class LoadBalancerClientFilterTests { ServerWebExchange exchange = MockServerWebExchange.from(request); exchange.getAttributes().put(GATEWAY_REQUEST_URL_ATTR, uri); - GatewayFilterChain filterChain = mock(GatewayFilterChain.class); - ArgumentCaptor captor = ArgumentCaptor.forClass(ServerWebExchange.class); - when(filterChain.filter(captor.capture())).thenReturn(Mono.empty()); + when(chain.filter(captor.capture())).thenReturn(Mono.empty()); - LoadBalancerClient loadBalancerClient = mock(LoadBalancerClient.class); - when(loadBalancerClient.choose("service1")). - thenReturn(new DefaultServiceInstance("service1", "service1-host1", 8081, - false, Collections.emptyMap())); + SpringClientFactory clientFactory = mock(SpringClientFactory.class); + ILoadBalancer loadBalancer = mock(ILoadBalancer.class); - LoadBalancerClientFilter filter = new LoadBalancerClientFilter(loadBalancerClient); - filter.filter(exchange, filterChain); + when(clientFactory.getLoadBalancerContext("service1")).thenReturn(new RibbonLoadBalancerContext(loadBalancer)); + when(clientFactory.getLoadBalancer("service1")).thenReturn(loadBalancer); + when(loadBalancer.chooseServer(any())).thenReturn(new Server("service1-host1", 8081)); + + RibbonLoadBalancerClient client = new RibbonLoadBalancerClient(clientFactory); + + LoadBalancerClientFilter filter = new LoadBalancerClientFilter(client); + filter.filter(exchange, chain); return captor.getValue(); }