Consolidate RouteLocator features in base class

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulConfiguration.java

@@ -22,6 +22,7 @@ import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
 import org.springframework.boot.autoconfigure.web.ErrorController;
+import org.springframework.boot.autoconfigure.web.ServerProperties;
 import org.springframework.boot.context.embedded.ServletRegistrationBean;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.cloud.client.actuator.HasFeatures;
@@ -58,6 +59,9 @@ public class ZuulConfiguration {
 	@Autowired
 	private ZuulProperties zuulProperties;
 
+	@Autowired
+	private ServerProperties server;
+
 	@Autowired(required = false)
 	private ErrorController errorController;
 
@@ -69,7 +73,8 @@ public class ZuulConfiguration {
 	@Bean
 	@ConditionalOnMissingBean
 	public RouteLocator routeLocator() {
-		return new SimpleRouteLocator(this.zuulProperties);
+		return new SimpleRouteLocator(this.server.getServletPrefix(),
+				this.zuulProperties);
 	}
 
 	@Bean

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/SimpleRouteLocator.java

@@ -19,31 +19,54 @@ package org.springframework.cloud.netflix.zuul.filters;
 import java.util.Collection;
 import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.Map.Entry;
+import java.util.concurrent.atomic.AtomicReference;
 
 import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.ZuulRoute;
 import org.springframework.util.AntPathMatcher;
 import org.springframework.util.PathMatcher;
+import org.springframework.util.StringUtils;
+
+import lombok.extern.apachecommons.CommonsLog;
 
 /**
+ * Simple {@link RouteLocator} based on configuration data held in {@link ZuulProperties}.
+ *
  * @author Dave Syer
  */
+@CommonsLog
 public class SimpleRouteLocator implements RouteLocator {
 
 	private ZuulProperties properties;
 
 	private PathMatcher pathMatcher = new AntPathMatcher();
 
-	public SimpleRouteLocator(ZuulProperties properties) {
+	private String servletPath;
+
+	private AtomicReference<Map<String, ZuulRoute>> routes = new AtomicReference<>();
+
+	public SimpleRouteLocator(String servletPath, ZuulProperties properties) {
 		this.properties = properties;
+		if (StringUtils.hasText(servletPath)) { // a servletPath is passed explicitly
+			this.servletPath = servletPath;
+		}
+		else {
+			// set Zuul servlet path
+			this.servletPath = properties.getServletPath() != null
+					? properties.getServletPath() : "";
+		}
 	}
 
 	@Override
 	public Map<String, String> getRoutes() {
-		Map<String, String> paths = new LinkedHashMap<String, String>();
-		for (ZuulRoute route : this.properties.getRoutes().values()) {
-			paths.put(route.getPath(), route.getId());
+		if (this.routes.get() == null) {
+			this.routes.set(locateRoutes());
 		}
-		return paths;
+		Map<String, String> values = new LinkedHashMap<>();
+		for (String url : this.routes.get().keySet()) {
+			values.put(url, this.routes.get().get(url).getLocation());
+		}
+		return values;
 	}
 
 	@Override
@@ -53,11 +76,91 @@ public class SimpleRouteLocator implements RouteLocator {
 
 	@Override
 	public Route getMatchingRoute(String path) {
+
+		if (log.isDebugEnabled()) {
+			log.debug("Finding route for path: " + path);
+		}
+
+		if (this.routes.get() == null) {
+			this.routes.set(locateRoutes());
+		}
+
+		log.debug("servletPath=" + this.servletPath);
+		if (StringUtils.hasText(this.servletPath) && !this.servletPath.equals("/")
+				&& path.startsWith(this.servletPath)) {
+			path = path.substring(this.servletPath.length());
+		}
+		log.debug("path=" + path);
+		ZuulRoute route = null;
+		if (!matchesIgnoredPatterns(path)) {
+			for (Entry<String, ZuulRoute> entry : this.routes.get().entrySet()) {
+				String pattern = entry.getKey();
+				log.debug("Matching pattern:" + pattern);
+				if (this.pathMatcher.match(pattern, path)) {
+					route = entry.getValue();
+					break;
+				}
+			}
+		}
+
+		return getRoute(route, path);
+
+	}
+
+	private Route getRoute(ZuulRoute route, String path) {
+		if (route == null) {
+			return null;
+		}
+		String targetPath = path;
+		String prefix = this.properties.getPrefix();
+		if (path.startsWith(prefix) && this.properties.isStripPrefix()) {
+			targetPath = path.substring(prefix.length());
+		}
+		if (route.isStripPrefix()) {
+			int index = route.getPath().indexOf("*") - 1;
+			if (index > 0) {
+				String routePrefix = route.getPath().substring(0, index);
+				targetPath = targetPath.replaceFirst(routePrefix, "");
+				prefix = prefix + routePrefix;
+			}
+		}
+		Boolean retryable = this.properties.getRetryable();
+		if (route.getRetryable() != null) {
+			retryable = route.getRetryable();
+		}
+		return new Route(route.getId(), targetPath, route.getLocation(), prefix,
+				retryable);
+	}
+
+	/**
+	 * Calculate all the routes and set up a cache for the values. Subclasses can call
+	 * this method if they need to implement {@link RefreshableRouteLocator}.
+	 */
+	protected void doRefresh() {
+		this.routes.set(locateRoutes());
+	}
+
+	/**
+	 * Compute a map of path pattern to route. The default is just a static map from the
+	 * {@link ZuulProperties}, but subclasses can add dynamic calculations.
+	 */
+	protected Map<String, ZuulRoute> locateRoutes() {
+		LinkedHashMap<String, ZuulRoute> routesMap = new LinkedHashMap<String, ZuulRoute>();
 		for (ZuulRoute route : this.properties.getRoutes().values()) {
-			if (this.pathMatcher.match(route.getPath(), path)) {
-				return route.getRoute(this.properties.getPrefix());
+			routesMap.put(route.getPath(), route);
+		}
+		return routesMap;
+	}
+
+	protected boolean matchesIgnoredPatterns(String path) {
+		for (String pattern : this.properties.getIgnoredPatterns()) {
+			log.debug("Matching ignored pattern:" + pattern);
+			if (this.pathMatcher.match(pattern, path)) {
+				log.debug("Path " + path + " matches ignored pattern " + pattern);
+				return true;
 			}
 		}
-		return null;
+		return false;
 	}
+
 }

spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/discovery/DiscoveryClientRouteLocator.java

@@ -16,22 +16,18 @@
 
 package org.springframework.cloud.netflix.zuul.filters.discovery;
 
-import java.util.Collection;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
-import java.util.concurrent.atomic.AtomicReference;
 
 import org.springframework.cloud.client.ServiceInstance;
 import org.springframework.cloud.client.discovery.DiscoveryClient;
 import org.springframework.cloud.netflix.zuul.filters.RefreshableRouteLocator;
-import org.springframework.cloud.netflix.zuul.filters.Route;
 import org.springframework.cloud.netflix.zuul.filters.RouteLocator;
+import org.springframework.cloud.netflix.zuul.filters.SimpleRouteLocator;
 import org.springframework.cloud.netflix.zuul.filters.ZuulProperties;
 import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.ZuulRoute;
-import org.springframework.util.AntPathMatcher;
-import org.springframework.util.PathMatcher;
 import org.springframework.util.PatternMatchUtils;
 import org.springframework.util.StringUtils;
 
@@ -45,7 +41,8 @@ import lombok.extern.apachecommons.CommonsLog;
  * @author Dave Syer
  */
 @CommonsLog
-public class DiscoveryClientRouteLocator implements RefreshableRouteLocator {
+public class DiscoveryClientRouteLocator extends SimpleRouteLocator
+		implements RefreshableRouteLocator {
 
 	public static final String DEFAULT_ROUTE = "/**";
 
@@ -53,26 +50,11 @@ public class DiscoveryClientRouteLocator implements RefreshableRouteLocator {
 
 	private ZuulProperties properties;
 
-	private PathMatcher pathMatcher = new AntPathMatcher();
-
-	private AtomicReference<Map<String, ZuulRoute>> routes = new AtomicReference<>();
-
-	private Map<String, ZuulRoute> staticRoutes = new LinkedHashMap<>();
-
-	private String servletPath;
-
 	private ServiceRouteMapper serviceRouteMapper;
 
 	public DiscoveryClientRouteLocator(String servletPath, DiscoveryClient discovery,
 			ZuulProperties properties) {
-		if (StringUtils.hasText(servletPath)) { // a servletPath is passed explicitly
-			this.servletPath = servletPath;
-		}
-		else {
-			// set Zuul servlet path
-			this.servletPath = properties.getServletPath() != null
-					? properties.getServletPath() : "";
-		}
+		super(servletPath, properties);
 
 		if (properties.isIgnoreLocalService()) {
 			ServiceInstance instance = discovery.getLocalServiceInstance();
@@ -95,109 +77,19 @@ public class DiscoveryClientRouteLocator implements RefreshableRouteLocator {
 	}
 
 	public void addRoute(String path, String location) {
-		this.staticRoutes.put(path, new ZuulRoute(path, location));
-		resetRoutes();
+		this.properties.getRoutes().put(path, new ZuulRoute(path, location));
+		refresh();
 	}
 
 	public void addRoute(ZuulRoute route) {
-		this.staticRoutes.put(route.getPath(), route);
-		resetRoutes();
-	}
-
-	@Override
-	public Collection<String> getIgnoredPaths() {
-		return this.properties.getIgnoredPatterns();
-	}
-
-	@Override
-	public Map<String, String> getRoutes() {
-		if (this.routes.get() == null) {
-			this.routes.set(locateRoutes());
-		}
-		Map<String, String> values = new LinkedHashMap<>();
-		for (String key : this.routes.get().keySet()) {
-			String url = key;
-			values.put(url, this.routes.get().get(key).getLocation());
-		}
-		return values;
-	}
-
-	@Override
-	public Route getMatchingRoute(String path) {
-		if (log.isDebugEnabled()) {
-			log.debug("Finding route for path: " + path);
-		}
-
-		if (this.routes.get() == null) {
-			this.routes.set(locateRoutes());
-		}
-
-		String location = null;
-		String targetPath = null;
-		String id = null;
-		String prefix = this.properties.getPrefix();
-		log.debug("servletPath=" + this.servletPath);
-		if (StringUtils.hasText(this.servletPath) && !this.servletPath.equals("/")
-				&& path.startsWith(this.servletPath)) {
-			path = path.substring(this.servletPath.length());
-		}
-		log.debug("path=" + path);
-		Boolean retryable = this.properties.getRetryable();
-		if (!matchesIgnoredPatterns(path)) {
-			for (Entry<String, ZuulRoute> entry : this.routes.get().entrySet()) {
-				String pattern = entry.getKey();
-				log.debug("Matching pattern:" + pattern);
-				if (this.pathMatcher.match(pattern, path)) {
-					ZuulRoute route = entry.getValue();
-					id = route.getId();
-					location = route.getLocation();
-					targetPath = path;
-					if (path.startsWith(prefix) && this.properties.isStripPrefix()) {
-						targetPath = path.substring(prefix.length());
-					}
-					if (route.isStripPrefix()) {
-						int index = route.getPath().indexOf("*") - 1;
-						if (index > 0) {
-							String routePrefix = route.getPath().substring(0, index);
-							targetPath = targetPath.replaceFirst(routePrefix, "");
-							prefix = prefix + routePrefix;
-						}
-					}
-					if (route.getRetryable() != null) {
-						retryable = route.getRetryable();
-					}
-					break;
-				}
-			}
-		}
-		return (location == null ? null
-				: new Route(id, targetPath, location, prefix, retryable));
+		this.properties.getRoutes().put(route.getPath(), route);
+		refresh();
 	}
 
 	@Override
-	public void refresh() {
-		resetRoutes();
-	}
-
-	protected boolean matchesIgnoredPatterns(String path) {
-		for (String pattern : this.properties.getIgnoredPatterns()) {
-			log.debug("Matching ignored pattern:" + pattern);
-			if (this.pathMatcher.match(pattern, path)) {
-				log.debug("Path " + path + " matches ignored pattern " + pattern);
-				return true;
-			}
-		}
-		return false;
-	}
-
-	private void resetRoutes() {
-		this.routes.set(locateRoutes());
-	}
-
 	protected LinkedHashMap<String, ZuulRoute> locateRoutes() {
 		LinkedHashMap<String, ZuulRoute> routesMap = new LinkedHashMap<String, ZuulRoute>();
-		addConfiguredRoutes(routesMap);
-		routesMap.putAll(this.staticRoutes);
+		routesMap.putAll(super.locateRoutes());
 		if (this.discovery != null) {
 			Map<String, ZuulRoute> staticServices = new LinkedHashMap<String, ZuulRoute>();
 			for (ZuulRoute route : routesMap.values()) {
@@ -258,6 +150,11 @@ public class DiscoveryClientRouteLocator implements RefreshableRouteLocator {
 		return values;
 	}
 
+	@Override
+	public void refresh() {
+		doRefresh();
+	}
+
 	protected String mapRouteToService(String serviceId) {
 		return this.serviceRouteMapper.apply(serviceId);
 	}

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SampleZuulProxyAppTestsWithHttpClient.java

@@ -81,15 +81,18 @@ public class SampleZuulProxyAppTestsWithHttpClient {
 	@Autowired
 	private RibbonCommandFactory<?> ribbonCommandFactory;
 
+	private String getRoute(String path) {
+		return this.routes.getRoutes().get(path);
+	}
+
 	@Test
 	public void bindRouteUsingPhysicalRoute() {
-		assertEquals("http://localhost:7777/local",
-				this.routes.getRoutes().get("/test/**"));
+		assertEquals("http://localhost:7777/local", getRoute("/test/**"));
 	}
 
 	@Test
 	public void bindRouteUsingOnlyPath() {
-		assertEquals("simple", this.routes.getRoutes().get("/simple/**"));
+		assertEquals("simple", getRoute("/simple/**"));
 	}
 
 	@Test

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SampleZuulProxyApplicationTests.java

@@ -20,6 +20,7 @@ import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+
 import javax.servlet.http.HttpServletRequest;
 
 import org.junit.Test;
@@ -62,6 +63,7 @@ import com.netflix.zuul.ZuulFilter;
 import com.netflix.zuul.context.RequestContext;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 @RunWith(SpringJUnit4ClassRunner.class)
@@ -83,17 +85,20 @@ public class SampleZuulProxyApplicationTests {
 	private RoutesEndpoint endpoint;
 
 	@Autowired
-	private RibbonCommandFactory ribbonCommandFactory;
+	private RibbonCommandFactory<?> ribbonCommandFactory;
+
+	private String getRoute(String path) {
+		return this.routes.getRoutes().get(path);
+	}
 
 	@Test
 	public void bindRouteUsingPhysicalRoute() {
-		assertEquals("http://localhost:7777/local",
-				this.routes.getRoutes().get("/test/**"));
+		assertEquals("http://localhost:7777/local", getRoute("/test/**"));
 	}
 
 	@Test
 	public void bindRouteUsingOnlyPath() {
-		assertEquals("simple", this.routes.getRoutes().get("/simple/**"));
+		assertNotNull(getRoute("/simple/**"));
 	}
 
 	@Test
@@ -149,52 +154,54 @@ public class SampleZuulProxyApplicationTests {
 	@Test
 	public void ribbonRouteWithSpace() {
 		ResponseEntity<String> result = new TestRestTemplate().exchange(
-				"http://localhost:" + this.port + "/simple/spa ce",
-				HttpMethod.GET, new HttpEntity<>((Void) null), String.class);
+				"http://localhost:" + this.port + "/simple/spa ce", HttpMethod.GET,
+				new HttpEntity<>((Void) null), String.class);
 		assertEquals(HttpStatus.OK, result.getStatusCode());
 		assertEquals("Hello space", result.getBody());
 	}
 
 	@Test
 	public void simpleHostRouteWithSpace() {
-		routes.addRoute("/self/**", "http://localhost:" + this.port);
+		this.routes.addRoute("/self/**", "http://localhost:" + this.port);
 		this.endpoint.reset();
 		ResponseEntity<String> result = new TestRestTemplate().exchange(
-				"http://localhost:" + this.port + "/self/spa ce",
-				HttpMethod.GET, new HttpEntity<>((Void) null), String.class);
+				"http://localhost:" + this.port + "/self/spa ce", HttpMethod.GET,
+				new HttpEntity<>((Void) null), String.class);
 		assertEquals(HttpStatus.OK, result.getStatusCode());
 		assertEquals("Hello space", result.getBody());
 	}
 
 	@Test
 	public void simpleHostRouteWithOriginalQString() {
-		routes.addRoute("/self/**", "http://localhost:" + this.port);
+		this.routes.addRoute("/self/**", "http://localhost:" + this.port);
 		this.endpoint.reset();
 		ResponseEntity<String> result = new TestRestTemplate().exchange(
-				"http://localhost:" + this.port + "/self/qstring?original=value1&original=value2", HttpMethod.GET,
-				new HttpEntity<>((Void) null), String.class);
+				"http://localhost:" + this.port
+						+ "/self/qstring?original=value1&original=value2",
+				HttpMethod.GET, new HttpEntity<>((Void) null), String.class);
 		assertEquals(HttpStatus.OK, result.getStatusCode());
 		assertEquals("Received {original=[value1, value2]}", result.getBody());
 	}
 
 	@Test
 	public void simpleHostRouteWithOverriddenQString() {
-		routes.addRoute("/self/**", "http://localhost:" + this.port);
+		this.routes.addRoute("/self/**", "http://localhost:" + this.port);
 		this.endpoint.reset();
 		ResponseEntity<String> result = new TestRestTemplate().exchange(
-				"http://localhost:" + this.port + "/self/qstring?override=true&different=key", HttpMethod.GET,
-				new HttpEntity<>((Void) null), String.class);
+				"http://localhost:" + this.port
+						+ "/self/qstring?override=true&different=key",
+				HttpMethod.GET, new HttpEntity<>((Void) null), String.class);
 		assertEquals(HttpStatus.OK, result.getStatusCode());
 		assertEquals("Received {key=[overridden]}", result.getBody());
 	}
 
 	@Test
 	public void simpleHostRouteWithTrailingSlash() {
-		routes.addRoute("/self/**", "http://localhost:" + this.port + "/");
+		this.routes.addRoute("/self/**", "http://localhost:" + this.port + "/");
 		this.endpoint.reset();
 		ResponseEntity<String> result = new TestRestTemplate().exchange(
-				"http://localhost:" + this.port + "/self/trailing-slash",
-				HttpMethod.GET, new HttpEntity<>((Void) null), String.class);
+				"http://localhost:" + this.port + "/self/trailing-slash", HttpMethod.GET,
+				new HttpEntity<>((Void) null), String.class);
 		assertEquals(HttpStatus.OK, result.getStatusCode());
 		assertEquals("/trailing-slash", result.getBody());
 	}
@@ -202,7 +209,7 @@ public class SampleZuulProxyApplicationTests {
 	@Test
 	public void ribbonCommandFactoryOverridden() {
 		assertTrue("ribbonCommandFactory not a MyRibbonCommandFactory",
-				ribbonCommandFactory instanceof SampleZuulProxyApplication.MyRibbonCommandFactory);
+				this.ribbonCommandFactory instanceof SampleZuulProxyApplication.MyRibbonCommandFactory);
 	}
 
 }
@@ -243,8 +250,8 @@ class SampleZuulProxyApplication {
 	}
 
 	@RequestMapping(value = "/qstring")
-	public String qstring(@RequestParam MultiValueMap<String,String> params) {
-		return "Received "+params.toString();	
+	public String qstring(@RequestParam MultiValueMap<String, String> params) {
+		return "Received " + params.toString();
 	}
 
 	@RequestMapping("/")
@@ -263,7 +270,8 @@ class SampleZuulProxyApplication {
 	}
 
 	@Bean
-	public RibbonCommandFactory ribbonCommandFactory(SpringClientFactory clientFactory) {
+	public RibbonCommandFactory<?> ribbonCommandFactory(
+			SpringClientFactory clientFactory) {
 		return new MyRibbonCommandFactory(clientFactory);
 	}
 
@@ -282,8 +290,9 @@ class SampleZuulProxyApplication {
 
 			@Override
 			public Object run() {
-				if (RequestContext.getCurrentContext().getRequest().getParameterMap().containsKey("override")) {
-					Map<String,List<String>> overridden=new HashMap<>();
+				if (RequestContext.getCurrentContext().getRequest().getParameterMap()
+						.containsKey("override")) {
+					Map<String, List<String>> overridden = new HashMap<>();
 					overridden.put("key", Arrays.asList("overridden"));
 					RequestContext.getCurrentContext().setRequestQueryParams(overridden);
 				}
@@ -319,7 +328,7 @@ class SimpleRibbonClientConfiguration {
 
 	@Bean
 	public ServerList<Server> ribbonServerList() {
-		return new StaticServerList<>(new Server("localhost", port));
+		return new StaticServerList<>(new Server("localhost", this.port));
 	}
 
 }
@@ -332,7 +341,7 @@ class AnotherRibbonClientConfiguration {
 
 	@Bean
 	public ServerList<Server> ribbonServerList() {
-		return new StaticServerList<>(new Server("localhost", port));
+		return new StaticServerList<>(new Server("localhost", this.port));
 	}
 
 }

spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SimpleZuulServerApplicationTests.java

@@ -41,7 +41,7 @@ import org.springframework.web.bind.annotation.RestController;
 import com.netflix.zuul.ZuulFilter;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertNotNull;
 
 @RunWith(SpringJUnit4ClassRunner.class)
 @SpringApplicationConfiguration(classes = SimpleZuulServerApplication.class)
@@ -56,9 +56,13 @@ public class SimpleZuulServerApplicationTests {
 	@Autowired
 	private RouteLocator routes;
 
+	private String getRoute(String path) {
+		return this.routes.getRoutes().get(path);
+	}
+
 	@Test
 	public void bindRoute() {
-		assertTrue(this.routes.getRoutes().keySet().contains("/testing123/**"));
+		assertNotNull(getRoute("/testing123/**"));
 	}
 
 	@Test