Removes blocking call from CachingRouteLocator.

Uses CacheFlux for caching and Flux.sort for sorting the incoming Routes.

fixes gh-272

spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/CachingRouteDefinitionLocator.java

@@ -17,9 +17,11 @@
 
 package org.springframework.cloud.gateway.route;
 
+import java.util.HashMap;
 import java.util.List;
-import java.util.concurrent.atomic.AtomicReference;
+import java.util.Map;
 
+import reactor.cache.CacheFlux;
 import reactor.core.publisher.Flux;
 
 import org.springframework.cloud.gateway.event.RefreshRoutesEvent;
@@ -31,29 +33,28 @@ import org.springframework.context.event.EventListener;
 public class CachingRouteDefinitionLocator implements RouteDefinitionLocator {
 
 	private final RouteDefinitionLocator delegate;
-	private final AtomicReference<List<RouteDefinition>> cachedRoutes = new AtomicReference<>();
+	private final Flux<RouteDefinition> routeDefinitions;
+	private final Map<String, List> cache = new HashMap<>();
 
 	public CachingRouteDefinitionLocator(RouteDefinitionLocator delegate) {
 		this.delegate = delegate;
-		this.cachedRoutes.compareAndSet(null, collectRoutes());
+		routeDefinitions = CacheFlux.lookup(cache, "routeDefs", RouteDefinition.class)
+				.onCacheMissResume(() -> this.delegate.getRouteDefinitions());
+
 	}
 
 	@Override
 	public Flux<RouteDefinition> getRouteDefinitions() {
-		return Flux.fromIterable(this.cachedRoutes.get());
+		return this.routeDefinitions;
 	}
 
 	/**
-	 * Sets the new routes
-	 * @return old routes
+	 * Clears the cache of routeDefinisions
+	 * @return routeDefinitions flux
 	 */
 	public Flux<RouteDefinition> refresh() {
-		return Flux.fromIterable(this.cachedRoutes.getAndUpdate(
-				routes -> CachingRouteDefinitionLocator.this.collectRoutes()));
-	}
-
-	private List<RouteDefinition> collectRoutes() {
-		return this.delegate.getRouteDefinitions().collectList().block();
+		this.cache.clear();
+		return this.routeDefinitions;
 	}
 
 	@EventListener(RefreshRoutesEvent.class)

spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/CachingRouteLocator.java

@@ -17,9 +17,11 @@
 
 package org.springframework.cloud.gateway.route;
 
+import java.util.HashMap;
 import java.util.List;
-import java.util.concurrent.atomic.AtomicReference;
+import java.util.Map;
 
+import reactor.cache.CacheFlux;
 import reactor.core.publisher.Flux;
 
 import org.springframework.cloud.gateway.event.RefreshRoutesEvent;
@@ -32,35 +34,31 @@ import org.springframework.core.annotation.AnnotationAwareOrderComparator;
 public class CachingRouteLocator implements RouteLocator {
 
 	private final RouteLocator delegate;
-	private final AtomicReference<List<Route>> cachedRoutes = new AtomicReference<>();
+	private final Flux<Route> routes;
+	private final Map<String, List> cache = new HashMap<>();
 
 	public CachingRouteLocator(RouteLocator delegate) {
 		this.delegate = delegate;
-		this.cachedRoutes.compareAndSet(null, collectRoutes());
+		routes = CacheFlux.lookup(cache, "routes", Route.class)
+				.onCacheMissResume(() -> this.delegate.getRoutes().sort(AnnotationAwareOrderComparator.INSTANCE));
 	}
 
 	@Override
 	public Flux<Route> getRoutes() {
-		return Flux.fromIterable(this.cachedRoutes.get());
+		return this.routes;
 	}
 
 	/**
-	 * Sets the new routes
-	 * @return old routes
+	 * Clears the routes cache
+	 * @return routes flux
 	 */
 	public Flux<Route> refresh() {
-		return Flux.fromIterable(this.cachedRoutes.getAndUpdate(
-				routes -> CachingRouteLocator.this.collectRoutes()));
-	}
-
-	private List<Route> collectRoutes() {
-		List<Route> routes = this.delegate.getRoutes().collectList().block();
-		AnnotationAwareOrderComparator.sort(routes);
-		return routes;
+		this.cache.clear();
+		return this.routes;
 	}
 
 	@EventListener(RefreshRoutesEvent.class)
-    /* for testing */ void handleRefresh() {
-        refresh();
-    }
+		/* for testing */ void handleRefresh() {
+		refresh();
+	}
 }

spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/actuate/GatewayControllerEndpointTests.java

@@ -0,0 +1,57 @@
+/*
+ * Copyright 2013-2018 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.springframework.cloud.gateway.actuate;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.SpringBootConfiguration;
+import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.boot.web.server.LocalServerPort;
+import org.springframework.cloud.gateway.test.PermitAllSecurityConfiguration;
+import org.springframework.context.annotation.Import;
+import org.springframework.test.context.junit4.SpringRunner;
+import org.springframework.test.web.reactive.server.WebTestClient;
+
+import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT;
+
+@RunWith(SpringRunner.class)
+@SpringBootTest(properties = "management.endpoints.web.exposure.include=*", webEnvironment = RANDOM_PORT)
+public class GatewayControllerEndpointTests {
+
+	@Autowired
+	WebTestClient testClient;
+
+	@LocalServerPort
+	int port;
+
+	@Test
+	public void testRefresh() {
+		testClient.post()
+				.uri("http://localhost:"+port+"/actuator/gateway/refresh")
+				.exchange()
+				.expectStatus().isOk();
+	}
+
+	@SpringBootConfiguration
+	@EnableAutoConfiguration
+	@Import(PermitAllSecurityConfiguration.class)
+	static class TestConfig{}
+}

spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/ratelimit/RedisRateLimiterConfigTests.java

@@ -51,7 +51,7 @@ public class RedisRateLimiterConfigTests {
 
 	@Before
 	public void init() {
-		System.out.println();
+		routeLocator.getRoutes().collectList().block(); // prime routes since getRoutes() no longer blocks
 	}
 
 	@Test

spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/CachingRouteDefinitionLocatorTests.java

@@ -0,0 +1,72 @@
+/*
+ * Copyright 2013-2018 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.springframework.cloud.gateway.route;
+
+import java.net.URI;
+import java.util.List;
+
+import org.junit.Test;
+import reactor.core.publisher.Flux;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class CachingRouteDefinitionLocatorTests {
+
+	@Test
+	public void getRouteDefinitionsWorks() {
+		RouteDefinition routeDef1 = routeDef(1);
+		RouteDefinition routeDef2 = routeDef(2);
+		CachingRouteDefinitionLocator locator = new CachingRouteDefinitionLocator(() -> Flux.just(routeDef2, routeDef1));
+
+		List<RouteDefinition> routes = locator.getRouteDefinitions().collectList().block();
+
+		assertThat(routes).containsExactlyInAnyOrder(routeDef1, routeDef2);
+	}
+
+
+	@Test
+	public void refreshWorks() {
+		RouteDefinition routeDef1 = routeDef(1);
+		RouteDefinition routeDef2 = routeDef(2);
+		CachingRouteDefinitionLocator locator = new CachingRouteDefinitionLocator(new RouteDefinitionLocator() {
+			int i = 0;
+
+			@Override
+			public Flux<RouteDefinition> getRouteDefinitions() {
+				if (i++ == 0) {
+					return Flux.just(routeDef2);
+				}
+				return Flux.just(routeDef2, routeDef1);
+			}
+		});
+
+		List<RouteDefinition> routes = locator.getRouteDefinitions().collectList().block();
+		assertThat(routes).containsExactlyInAnyOrder(routeDef2);
+
+		routes = locator.refresh().collectList().block();
+		assertThat(routes).containsExactlyInAnyOrder(routeDef1, routeDef2);
+	}
+
+	RouteDefinition routeDef(int id) {
+		RouteDefinition def = new RouteDefinition();
+		def.setId(String.valueOf(id));
+		def.setUri(URI.create("http://localhost/"+id));
+		def.setOrder(id);
+		return def;
+	}
+}

spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/route/CachingRouteLocatorTests.java

@@ -0,0 +1,70 @@
+/*
+ * Copyright 2013-2018 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.springframework.cloud.gateway.route;
+
+import java.util.List;
+
+import org.junit.Test;
+import reactor.core.publisher.Flux;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class CachingRouteLocatorTests {
+
+	@Test
+	public void getRoutesWorks() {
+		Route route1 = route(1);
+		Route route2 = route(2);
+		CachingRouteLocator locator = new CachingRouteLocator(() -> Flux.just(route2, route1));
+
+		List<Route> routes = locator.getRoutes().collectList().block();
+
+		assertThat(routes).containsExactly(route1, route2);
+	}
+
+
+	@Test
+	public void refreshWorks() {
+		Route route1 = route(1);
+		Route route2 = route(2);
+		CachingRouteLocator locator = new CachingRouteLocator(new RouteLocator() {
+			int i = 0;
+
+			@Override
+			public Flux<Route> getRoutes() {
+				if (i++ == 0) {
+					return Flux.just(route2);
+				}
+				return Flux.just(route2, route1);
+			}
+		});
+
+		List<Route> routes = locator.getRoutes().collectList().block();
+		assertThat(routes).containsExactly(route2);
+
+		routes = locator.refresh().collectList().block();
+		assertThat(routes).containsExactly(route1, route2);
+	}
+
+	Route route(int id) {
+		return Route.builder().id(String.valueOf(id))
+				.uri("http://localhost/"+id)
+				.order(id)
+				.predicate(exchange -> true).build();
+	}
+}