From 9161c54c43bf615dd95f17691cb8203cf27c5bba Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Thu, 14 Jan 2016 12:50:28 +0000 Subject: [PATCH] Factor out common interface from ProxyRouteLocator The RouteLocator interface wasn't very useful to anyone in its 1.0.x form. If ProxyRouteLocator features are consolidated into the RouteLocator interface then users can supply their own strategies more easily. Also added RefreshableRouteLocator for implementations that need to recompute routes when something changes. SimpleRouteLocator doesn't need to do that because all the routes are in the configuration. --- .../netflix/zuul/RefreshableRouteLocator.java | 30 +++++++++++ .../cloud/netflix/zuul/RoutesEndpoint.java | 6 +-- .../cloud/netflix/zuul/ZuulConfiguration.java | 11 ++-- .../netflix/zuul/ZuulProxyConfiguration.java | 43 +++++++--------- .../zuul/filters/ProxyRouteLocator.java | 51 +++++++------------ .../cloud/netflix/zuul/filters/Route.java | 36 +++++++++++++ .../netflix/zuul/filters/RouteLocator.java | 16 +++++- .../zuul/filters/ServiceRouteMapper.java | 4 +- .../zuul/filters/SimpleRouteLocator.java | 22 ++++++-- .../netflix/zuul/filters/ZuulProperties.java | 9 +++- .../zuul/filters/pre/PreDecorationFilter.java | 10 ++-- .../netflix/zuul/web/ZuulHandlerMapping.java | 24 +++++++-- ...SampleZuulProxyAppTestsWithHttpClient.java | 16 +++--- .../SimpleZuulServerApplicationTests.java | 2 +- .../zuul/filters/ProxyRouteLocatorTests.java | 37 +++++++------- .../zuul/web/ZuulHandlerMappingTests.java | 24 +++++---- 16 files changed, 218 insertions(+), 123 deletions(-) create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RefreshableRouteLocator.java create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/Route.java diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RefreshableRouteLocator.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RefreshableRouteLocator.java new file mode 100644 index 00000000..5ae7de0c --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RefreshableRouteLocator.java @@ -0,0 +1,30 @@ +/* + * Copyright 2013-2015 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.netflix.zuul; + +import org.springframework.cloud.netflix.zuul.filters.RouteLocator; + +/** + * Interface for a route locator that can be refreshed if routes change. + * + * @author Dave Syer + */ +public interface RefreshableRouteLocator extends RouteLocator { + + void refresh(); + +} diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RoutesEndpoint.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RoutesEndpoint.java index 3744764f..0174b743 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RoutesEndpoint.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RoutesEndpoint.java @@ -21,7 +21,7 @@ import java.util.Map; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.boot.actuate.endpoint.mvc.MvcEndpoint; -import org.springframework.cloud.netflix.zuul.filters.ProxyRouteLocator; +import org.springframework.cloud.netflix.zuul.filters.RouteLocator; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.ApplicationEventPublisherAware; import org.springframework.jmx.export.annotation.ManagedAttribute; @@ -40,7 +40,7 @@ import org.springframework.web.bind.annotation.ResponseBody; @ManagedResource(description = "Can be used to list and reset the reverse proxy routes") public class RoutesEndpoint implements MvcEndpoint, ApplicationEventPublisherAware { - private ProxyRouteLocator routes; + private RouteLocator routes; private ApplicationEventPublisher publisher; @@ -50,7 +50,7 @@ public class RoutesEndpoint implements MvcEndpoint, ApplicationEventPublisherAwa } @Autowired - public RoutesEndpoint(ProxyRouteLocator routes) { + public RoutesEndpoint(RouteLocator routes) { this.routes = routes; } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulConfiguration.java index 71245206..c64da6e7 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulConfiguration.java @@ -20,6 +20,7 @@ import java.util.Map; 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.context.embedded.ServletRegistrationBean; import org.springframework.boot.context.properties.EnableConfigurationProperties; @@ -66,6 +67,7 @@ public class ZuulConfiguration { } @Bean + @ConditionalOnMissingBean public RouteLocator routeLocator() { return new SimpleRouteLocator(this.zuulProperties); } @@ -140,8 +142,8 @@ public class ZuulConfiguration { } - private static class ZuulRefreshListener implements - ApplicationListener { + private static class ZuulRefreshListener + implements ApplicationListener { @Autowired private ZuulHandlerMapping zuulHandlerMapping; @@ -149,8 +151,9 @@ public class ZuulConfiguration { @Override public void onApplicationEvent(ApplicationEvent event) { if (event instanceof ContextRefreshedEvent - || event instanceof RefreshScopeRefreshedEvent) { - this.zuulHandlerMapping.registerHandlers(); + || event instanceof RefreshScopeRefreshedEvent + || event instanceof RoutesRefreshedEvent) { + this.zuulHandlerMapping.setDirty(true); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java index 83eb924c..024c5205 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java @@ -29,10 +29,10 @@ import org.springframework.cloud.client.discovery.event.HeartbeatEvent; import org.springframework.cloud.client.discovery.event.HeartbeatMonitor; import org.springframework.cloud.client.discovery.event.InstanceRegisteredEvent; import org.springframework.cloud.client.discovery.event.ParentHeartbeatEvent; -import org.springframework.cloud.context.scope.refresh.RefreshScopeRefreshedEvent; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelper; import org.springframework.cloud.netflix.zuul.filters.ProxyRouteLocator; +import org.springframework.cloud.netflix.zuul.filters.RouteLocator; import org.springframework.cloud.netflix.zuul.filters.ServiceRouteMapper; import org.springframework.cloud.netflix.zuul.filters.SimpleServiceRouteMapper; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; @@ -80,32 +80,35 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { @Bean @Override + @ConditionalOnMissingBean(RouteLocator.class) public ProxyRouteLocator routeLocator() { return new ProxyRouteLocator(this.server.getServletPrefix(), this.discovery, - this.zuulProperties, serviceRouteMapper); + this.zuulProperties, this.serviceRouteMapper); } @Bean @ConditionalOnMissingBean - public RibbonCommandFactory ribbonCommandFactory() { + public RibbonCommandFactory ribbonCommandFactory() { return new RestClientRibbonCommandFactory(this.clientFactory); } // pre filters @Bean - public PreDecorationFilter preDecorationFilter(ProxyRouteLocator routeLocator) { + public PreDecorationFilter preDecorationFilter(RouteLocator routeLocator) { return new PreDecorationFilter(routeLocator, this.zuulProperties.isAddProxyHeaders()); } // route filters @Bean - public RibbonRoutingFilter ribbonRoutingFilter(RibbonCommandFactory ribbonCommandFactory) { + public RibbonRoutingFilter ribbonRoutingFilter( + RibbonCommandFactory ribbonCommandFactory) { ProxyRequestHelper helper = new ProxyRequestHelper(); if (this.traces != null) { helper.setTraces(this.traces); } - RibbonRoutingFilter filter = new RibbonRoutingFilter(helper, ribbonCommandFactory); + RibbonRoutingFilter filter = new RibbonRoutingFilter(helper, + ribbonCommandFactory); return filter; } @@ -119,9 +122,8 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { } @Bean - @Override - public ApplicationListener zuulRefreshRoutesListener() { - return new ZuulRefreshListener(); + public ApplicationListener zuulDiscoveryRefreshRoutesListener() { + return new ZuulDiscoveryRefreshListener(); } @Configuration @@ -144,38 +146,28 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { } } - @Configuration @ConditionalOnClass(Endpoint.class) protected static class RoutesEndpointConfiguration { - @Autowired - private ProxyRouteLocator routeLocator; - @Bean - // @RefreshScope - public RoutesEndpoint zuulEndpoint() { - return new RoutesEndpoint(this.routeLocator); + public RoutesEndpoint zuulEndpoint(ProxyRouteLocator routeLocator) { + return new RoutesEndpoint(routeLocator); } } - private static class ZuulRefreshListener implements - ApplicationListener { + private static class ZuulDiscoveryRefreshListener + implements ApplicationListener { private HeartbeatMonitor monitor = new HeartbeatMonitor(); - @Autowired - private ProxyRouteLocator routeLocator; - @Autowired private ZuulHandlerMapping zuulHandlerMapping; @Override public void onApplicationEvent(ApplicationEvent event) { - if (event instanceof InstanceRegisteredEvent - || event instanceof RefreshScopeRefreshedEvent - || event instanceof RoutesRefreshedEvent) { + if (event instanceof InstanceRegisteredEvent) { reset(); } else if (event instanceof ParentHeartbeatEvent) { @@ -196,8 +188,7 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { } private void reset() { - this.routeLocator.resetRoutes(); - this.zuulHandlerMapping.registerHandlers(); + this.zuulHandlerMapping.setDirty(true); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRouteLocator.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRouteLocator.java index c059bba6..62fc6e55 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRouteLocator.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRouteLocator.java @@ -25,21 +25,20 @@ 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.RefreshableRouteLocator; 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; -import lombok.AllArgsConstructor; -import lombok.Data; import lombok.extern.apachecommons.CommonsLog; /** * @author Spencer Gibb */ @CommonsLog -public class ProxyRouteLocator implements RouteLocator { +public class ProxyRouteLocator implements RefreshableRouteLocator { public static final String DEFAULT_ROUTE = "/**"; @@ -83,7 +82,7 @@ public class ProxyRouteLocator implements RouteLocator { } public ProxyRouteLocator(String servletPath, DiscoveryClient discovery, - ZuulProperties properties, ServiceRouteMapper serviceRouteMapper) { + ZuulProperties properties, ServiceRouteMapper serviceRouteMapper) { this(servletPath, discovery, properties); this.serviceRouteMapper = serviceRouteMapper; } @@ -98,16 +97,12 @@ public class ProxyRouteLocator implements RouteLocator { resetRoutes(); } - @Override - public Collection getRoutePaths() { - return getRoutes().keySet(); - } - @Override public Collection getIgnoredPaths() { return this.properties.getIgnoredPatterns(); } + @Override public Map getRoutes() { if (this.routes.get() == null) { this.routes.set(locateRoutes()); @@ -120,11 +115,16 @@ public class ProxyRouteLocator implements RouteLocator { return values; } - public ProxyRouteSpec getMatchingRoute(String path) { + @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; @@ -164,7 +164,12 @@ public class ProxyRouteLocator implements RouteLocator { } } return (location == null ? null - : new ProxyRouteSpec(id, targetPath, location, prefix, retryable)); + : new Route(id, targetPath, location, prefix, retryable)); + } + + @Override + public void refresh() { + resetRoutes(); } protected boolean matchesIgnoredPatterns(String path) { @@ -178,7 +183,7 @@ public class ProxyRouteLocator implements RouteLocator { return false; } - public void resetRoutes() { + private void resetRoutes() { this.routes.set(locateRoutes()); } @@ -262,26 +267,4 @@ public class ProxyRouteLocator implements RouteLocator { } } - public String getTargetPath(String matchingRoute, String requestURI) { - String path = getRoutes().get(matchingRoute); - return (path != null ? path : requestURI); - - } - - @Data - @AllArgsConstructor - public static class ProxyRouteSpec { - - private String id; - - private String path; - - private String location; - - private String prefix; - - private Boolean retryable; - - } - } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/Route.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/Route.java new file mode 100644 index 00000000..488fe280 --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/Route.java @@ -0,0 +1,36 @@ +/* + * Copyright 2013-2015 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.netflix.zuul.filters; + +import lombok.AllArgsConstructor; +import lombok.Data; + +@Data +@AllArgsConstructor +public class Route { + + private String id; + + private String path; + + private String location; + + private String prefix; + + private Boolean retryable; + +} \ No newline at end of file diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/RouteLocator.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/RouteLocator.java index e5967cd7..2ae7f211 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/RouteLocator.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/RouteLocator.java @@ -17,14 +17,26 @@ package org.springframework.cloud.netflix.zuul.filters; import java.util.Collection; +import java.util.Map; /** * @author Dave Syer */ public interface RouteLocator { - Collection getRoutePaths(); - + /** + * Ignored route paths (or patterns), if any. + */ Collection getIgnoredPaths(); + /** + * A map of route path (pattern) to location (e.g. service id or URL). + */ + Map getRoutes(); + + /** + * Maps a path to an actual route with full metadata. + */ + Route getMatchingRoute(String path); + } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ServiceRouteMapper.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ServiceRouteMapper.java index f05bc6a2..cccce6c7 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ServiceRouteMapper.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ServiceRouteMapper.java @@ -1,10 +1,10 @@ package org.springframework.cloud.netflix.zuul.filters; /** - * @author Stéphane LEROY - * * Provide a way to apply convention between routes and discovered services name. * + * @author Stéphane LEROY + * */ public interface ServiceRouteMapper { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/SimpleRouteLocator.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/SimpleRouteLocator.java index c21c53ca..57a6ad7f 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/SimpleRouteLocator.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/SimpleRouteLocator.java @@ -17,9 +17,12 @@ package org.springframework.cloud.netflix.zuul.filters; import java.util.Collection; -import java.util.LinkedHashSet; +import java.util.LinkedHashMap; +import java.util.Map; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.ZuulRoute; +import org.springframework.util.AntPathMatcher; +import org.springframework.util.PathMatcher; /** * @author Dave Syer @@ -28,15 +31,17 @@ public class SimpleRouteLocator implements RouteLocator { private ZuulProperties properties; + private PathMatcher pathMatcher = new AntPathMatcher(); + public SimpleRouteLocator(ZuulProperties properties) { this.properties = properties; } @Override - public Collection getRoutePaths() { - Collection paths = new LinkedHashSet(); + public Map getRoutes() { + Map paths = new LinkedHashMap(); for (ZuulRoute route : this.properties.getRoutes().values()) { - paths.add(route.getPath()); + paths.put(route.getPath(), route.getId()); } return paths; } @@ -46,4 +51,13 @@ public class SimpleRouteLocator implements RouteLocator { return this.properties.getIgnoredPatterns(); } + @Override + public Route getMatchingRoute(String path) { + for (ZuulRoute route : this.properties.getRoutes().values()) { + if (this.pathMatcher.match(route.getPath(), path)) { + return route.getRoute(this.properties.getPrefix()); + } + } + return null; + } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java index 4aec27a7..db49a56b 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java @@ -21,6 +21,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Map.Entry; + import javax.annotation.PostConstruct; import org.springframework.boot.context.properties.ConfigurationProperties; @@ -106,8 +107,8 @@ public class ZuulProperties { String location = null; String path = text; if (text.contains("=")) { - String[] values = StringUtils.trimArrayElements(StringUtils.split(text, - "=")); + String[] values = StringUtils + .trimArrayElements(StringUtils.split(text, "=")); location = values[1]; path = values[0]; } @@ -148,6 +149,10 @@ public class ZuulProperties { return path; } + public Route getRoute(String prefix) { + return new Route(this.id, this.path, getLocation(), prefix, this.retryable); + } + } public String getServletPattern() { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java index 338438af..d5ce6fef 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java @@ -19,8 +19,8 @@ package org.springframework.cloud.netflix.zuul.filters.pre; import java.net.MalformedURLException; import java.net.URL; -import org.springframework.cloud.netflix.zuul.filters.ProxyRouteLocator; -import org.springframework.cloud.netflix.zuul.filters.ProxyRouteLocator.ProxyRouteSpec; +import org.springframework.cloud.netflix.zuul.filters.Route; +import org.springframework.cloud.netflix.zuul.filters.RouteLocator; import org.springframework.util.StringUtils; import org.springframework.web.util.UrlPathHelper; @@ -33,13 +33,13 @@ import lombok.extern.apachecommons.CommonsLog; @CommonsLog public class PreDecorationFilter extends ZuulFilter { - private ProxyRouteLocator routeLocator; + private RouteLocator routeLocator; private boolean addProxyHeaders; private UrlPathHelper urlPathHelper = new UrlPathHelper(); - public PreDecorationFilter(ProxyRouteLocator routeLocator, boolean addProxyHeaders) { + public PreDecorationFilter(RouteLocator routeLocator, boolean addProxyHeaders) { this.routeLocator = routeLocator; this.addProxyHeaders = addProxyHeaders; } @@ -65,7 +65,7 @@ public class PreDecorationFilter extends ZuulFilter { RequestContext ctx = RequestContext.getCurrentContext(); final String requestURI = this.urlPathHelper .getPathWithinApplication(ctx.getRequest()); - ProxyRouteSpec route = this.routeLocator.getMatchingRoute(requestURI); + Route route = this.routeLocator.getMatchingRoute(requestURI); if (route != null) { String location = route.getLocation(); if (location != null) { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMapping.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMapping.java index 5e99bae7..0b5e40f9 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMapping.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMapping.java @@ -21,6 +21,7 @@ import java.util.Collection; import javax.servlet.http.HttpServletRequest; import org.springframework.boot.autoconfigure.web.ErrorController; +import org.springframework.cloud.netflix.zuul.RefreshableRouteLocator; import org.springframework.cloud.netflix.zuul.filters.RouteLocator; import org.springframework.util.PatternMatchUtils; import org.springframework.web.servlet.handler.AbstractUrlHandlerMapping; @@ -41,6 +42,8 @@ public class ZuulHandlerMapping extends AbstractUrlHandlerMapping { private ErrorController errorController; + private volatile boolean dirty = true; + public ZuulHandlerMapping(RouteLocator routeLocator, ZuulController zuul) { this.routeLocator = routeLocator; this.zuul = zuul; @@ -51,6 +54,13 @@ public class ZuulHandlerMapping extends AbstractUrlHandlerMapping { this.errorController = errorController; } + public void setDirty(boolean dirty) { + this.dirty = dirty; + if (this.routeLocator instanceof RefreshableRouteLocator) { + ((RefreshableRouteLocator) this.routeLocator).refresh(); + } + } + @Override protected Object lookupHandler(String urlPath, HttpServletRequest request) throws Exception { @@ -66,13 +76,21 @@ public class ZuulHandlerMapping extends AbstractUrlHandlerMapping { if (ctx.containsKey("forward.to")) { return null; } + if (this.dirty) { + synchronized (this) { + if (this.dirty) { + registerHandlers(); + this.dirty = false; + } + } + } return super.lookupHandler(urlPath, request); } - public void registerHandlers() { - Collection routes = this.routeLocator.getRoutePaths(); + private void registerHandlers() { + Collection routes = this.routeLocator.getRoutes().keySet(); if (routes.isEmpty()) { - this.logger.warn("No routes found from ProxyRouteLocator"); + this.logger.warn("No routes found from RouteLocator"); } else { for (String url : routes) { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SampleZuulProxyAppTestsWithHttpClient.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SampleZuulProxyAppTestsWithHttpClient.java index ac975178..74742818 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SampleZuulProxyAppTestsWithHttpClient.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SampleZuulProxyAppTestsWithHttpClient.java @@ -79,7 +79,7 @@ public class SampleZuulProxyAppTestsWithHttpClient { private RoutesEndpoint endpoint; @Autowired - private RibbonCommandFactory ribbonCommandFactory; + private RibbonCommandFactory ribbonCommandFactory; @Test public void bindRouteUsingPhysicalRoute() { @@ -195,7 +195,7 @@ public class SampleZuulProxyAppTestsWithHttpClient { @Test public void simpleHostRouteWithSpace() { - routes.addRoute("/self/**", "http://localhost:" + this.port); + this.routes.addRoute("/self/**", "http://localhost:" + this.port); this.endpoint.reset(); ResponseEntity result = new TestRestTemplate().exchange( @@ -207,7 +207,7 @@ public class SampleZuulProxyAppTestsWithHttpClient { @Test public void simpleHostRouteWithOriginalQString() { - routes.addRoute("/self/**", "http://localhost:" + this.port); + this.routes.addRoute("/self/**", "http://localhost:" + this.port); this.endpoint.reset(); ResponseEntity result = new TestRestTemplate().exchange( @@ -220,13 +220,13 @@ public class SampleZuulProxyAppTestsWithHttpClient { @Test public void simpleHostRouteWithOverriddenQString() { - routes.addRoute("/self/**", "http://localhost:" + this.port); + this.routes.addRoute("/self/**", "http://localhost:" + this.port); this.endpoint.reset(); ResponseEntity result = new TestRestTemplate().exchange( "http://localhost:" + this.port - + "/self/qstring?override=true&different=key", HttpMethod.GET, - new HttpEntity<>((Void) null), String.class); + + "/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()); } @@ -234,7 +234,7 @@ public class SampleZuulProxyAppTestsWithHttpClient { @Test public void ribbonCommandFactoryOverridden() { assertTrue("ribbonCommandFactory not a MyRibbonCommandFactory", - ribbonCommandFactory instanceof HttpClientRibbonCommandFactory); + this.ribbonCommandFactory instanceof HttpClientRibbonCommandFactory); } } @@ -299,7 +299,7 @@ class SampleHttpClientZuulProxyApplication { } @Bean - public RibbonCommandFactory ribbonCommandFactory( + public RibbonCommandFactory ribbonCommandFactory( final SpringClientFactory clientFactory) { return new HttpClientRibbonCommandFactory(clientFactory); } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SimpleZuulServerApplicationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SimpleZuulServerApplicationTests.java index 58f57bee..a50a7224 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SimpleZuulServerApplicationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/SimpleZuulServerApplicationTests.java @@ -58,7 +58,7 @@ public class SimpleZuulServerApplicationTests { @Test public void bindRoute() { - assertTrue(this.routes.getRoutePaths().contains("/testing123/**")); + assertTrue(this.routes.getRoutes().keySet().contains("/testing123/**")); } @Test diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRouteLocatorTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRouteLocatorTests.java index d287366e..09c4cf82 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRouteLocatorTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRouteLocatorTests.java @@ -25,7 +25,6 @@ import org.junit.Test; import org.mockito.Mock; import org.springframework.cloud.client.DefaultServiceInstance; import org.springframework.cloud.client.discovery.DiscoveryClient; -import org.springframework.cloud.netflix.zuul.filters.ProxyRouteLocator.ProxyRouteSpec; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.ZuulRoute; import org.springframework.cloud.netflix.zuul.filters.regex.RegExServiceRouteMapper; import org.springframework.core.env.ConfigurableEnvironment; @@ -72,7 +71,7 @@ public class ProxyRouteLocatorTests { this.properties.getRoutes().put("foo", new ZuulRoute("/foo/**")); this.properties.init(); routeLocator.getRoutes(); // force refresh - ProxyRouteSpec route = routeLocator.getMatchingRoute("/foo/1"); + Route route = routeLocator.getMatchingRoute("/foo/1"); assertEquals("foo", route.getLocation()); assertEquals("foo", route.getId()); } @@ -85,7 +84,7 @@ public class ProxyRouteLocatorTests { this.properties.setPrefix("/proxy"); this.properties.init(); routeLocator.getRoutes(); // force refresh - ProxyRouteSpec route = routeLocator.getMatchingRoute("/proxy/foo/1"); + Route route = routeLocator.getMatchingRoute("/proxy/foo/1"); assertEquals("foo", route.getLocation()); assertEquals("/1", route.getPath()); } @@ -97,7 +96,7 @@ public class ProxyRouteLocatorTests { this.properties.getRoutes().put("foo", new ZuulRoute("/foo/**")); this.properties.init(); routeLocator.getRoutes(); // force refresh - ProxyRouteSpec route = routeLocator.getMatchingRoute("/app/foo/1"); + Route route = routeLocator.getMatchingRoute("/app/foo/1"); assertEquals("foo", route.getLocation()); assertEquals("/1", route.getPath()); } @@ -111,7 +110,7 @@ public class ProxyRouteLocatorTests { this.properties.setStripPrefix(false); this.properties.setPrefix("/proxy"); routeLocator.getRoutes(); // force refresh - ProxyRouteSpec route = routeLocator.getMatchingRoute("/proxy/foo/1"); + Route route = routeLocator.getMatchingRoute("/proxy/foo/1"); assertEquals("foo", route.getLocation()); assertEquals("/proxy/foo/1", route.getPath()); } @@ -124,7 +123,7 @@ public class ProxyRouteLocatorTests { this.properties.setStripPrefix(false); this.properties.setPrefix("/proxy"); routeLocator.getRoutes(); // force refresh - ProxyRouteSpec route = routeLocator.getMatchingRoute("/proxy/foo/1"); + Route route = routeLocator.getMatchingRoute("/proxy/foo/1"); assertEquals("foo", route.getLocation()); assertEquals("/proxy/1", route.getPath()); } @@ -137,7 +136,7 @@ public class ProxyRouteLocatorTests { new ZuulRoute("foo", "/foo/**", "foo", null, false, null)); this.properties.setPrefix("/proxy"); routeLocator.getRoutes(); // force refresh - ProxyRouteSpec route = routeLocator.getMatchingRoute("/proxy/foo/1"); + Route route = routeLocator.getMatchingRoute("/proxy/foo/1"); assertEquals("foo", route.getLocation()); assertEquals("/foo/1", route.getPath()); } @@ -151,7 +150,7 @@ public class ProxyRouteLocatorTests { this.properties.getRoutes().put("foo", zuulRoute); this.properties.init(); routeLocator.getRoutes(); // force refresh - ProxyRouteSpec route = routeLocator.getMatchingRoute("/foo/1"); + Route route = routeLocator.getMatchingRoute("/foo/1"); assertEquals("foo", route.getLocation()); assertEquals("/1", route.getPath()); } @@ -164,7 +163,7 @@ public class ProxyRouteLocatorTests { this.properties.getRoutes().put("bar", new ZuulRoute("/bar/**")); this.properties.init(); routeLocator.getRoutes(); // force refresh - ProxyRouteSpec route = routeLocator.getMatchingRoute("/bar/1"); + Route route = routeLocator.getMatchingRoute("/bar/1"); assertEquals("bar", route.getLocation()); assertEquals("bar", route.getId()); } @@ -177,7 +176,7 @@ public class ProxyRouteLocatorTests { this.properties.getRoutes().put("foo", new ZuulRoute("/foo/**")); this.properties.init(); routeLocator.getRoutes(); // force refresh - ProxyRouteSpec route = routeLocator.getMatchingRoute("/foo/1"); + Route route = routeLocator.getMatchingRoute("/foo/1"); assertNull("routes did not ignore " + IGNOREDPATTERN, route); } @@ -190,7 +189,7 @@ public class ProxyRouteLocatorTests { this.properties.setPrefix("/proxy"); this.properties.init(); routeLocator.getRoutes(); // force refresh - ProxyRouteSpec route = routeLocator.getMatchingRoute("/proxy/foo/1"); + Route route = routeLocator.getMatchingRoute("/proxy/foo/1"); assertEquals("foo", route.getLocation()); assertEquals("/1", route.getPath()); } @@ -203,7 +202,7 @@ public class ProxyRouteLocatorTests { this.properties.getRoutes().put("foo", new ZuulRoute("/foo/**")); this.properties.init(); routeLocator.getRoutes(); // force refresh - ProxyRouteSpec route = routeLocator.getMatchingRoute("/app/foo/1"); + Route route = routeLocator.getMatchingRoute("/app/foo/1"); assertNull("routes did not ignore " + IGNOREDPATTERN, route); } @@ -217,7 +216,7 @@ public class ProxyRouteLocatorTests { this.properties.setStripPrefix(false); this.properties.setPrefix("/proxy"); routeLocator.getRoutes(); // force refresh - ProxyRouteSpec route = routeLocator.getMatchingRoute("/proxy/foo/1"); + Route route = routeLocator.getMatchingRoute("/proxy/foo/1"); assertEquals("foo", route.getLocation()); assertEquals("/proxy/foo/1", route.getPath()); } @@ -232,7 +231,7 @@ public class ProxyRouteLocatorTests { this.properties.setStripPrefix(false); this.properties.setPrefix("/proxy"); routeLocator.getRoutes(); // force refresh - ProxyRouteSpec route = routeLocator.getMatchingRoute("/proxy/foo/1"); + Route route = routeLocator.getMatchingRoute("/proxy/foo/1"); assertNull("routes did not ignore " + "/proxy" + IGNOREDPATTERN, route); } @@ -245,7 +244,7 @@ public class ProxyRouteLocatorTests { this.properties.setStripPrefix(false); this.properties.setPrefix("/proxy"); routeLocator.getRoutes(); // force refresh - ProxyRouteSpec route = routeLocator.getMatchingRoute("/proxy/foo/1"); + Route route = routeLocator.getMatchingRoute("/proxy/foo/1"); assertEquals("foo", route.getLocation()); assertEquals("/proxy/1", route.getPath()); } @@ -259,7 +258,7 @@ public class ProxyRouteLocatorTests { this.properties.setStripPrefix(false); this.properties.setPrefix("/proxy"); routeLocator.getRoutes(); // force refresh - ProxyRouteSpec route = routeLocator.getMatchingRoute("/proxy/foo/1"); + Route route = routeLocator.getMatchingRoute("/proxy/foo/1"); assertNull("routes did not ignore " + "/proxy" + IGNOREDPATTERN, route); } @@ -272,7 +271,7 @@ public class ProxyRouteLocatorTests { new ZuulRoute("foo", "/foo/**", "foo", null, false, null)); this.properties.setPrefix("/proxy"); routeLocator.getRoutes(); // force refresh - ProxyRouteSpec route = routeLocator.getMatchingRoute("/proxy/foo/1"); + Route route = routeLocator.getMatchingRoute("/proxy/foo/1"); assertEquals("foo", route.getLocation()); assertEquals("/foo/1", route.getPath()); } @@ -286,7 +285,7 @@ public class ProxyRouteLocatorTests { new ZuulRoute("foo", "/foo/**", "foo", null, false, null)); this.properties.setPrefix("/proxy"); routeLocator.getRoutes(); // force refresh - ProxyRouteSpec route = routeLocator.getMatchingRoute("/proxy/foo/1"); + Route route = routeLocator.getMatchingRoute("/proxy/foo/1"); assertNull("routes did not ignore " + "/proxy" + IGNOREDPATTERN, route); } @@ -300,7 +299,7 @@ public class ProxyRouteLocatorTests { this.properties.getRoutes().put("foo", zuulRoute); this.properties.init(); routeLocator.getRoutes(); // force refresh - ProxyRouteSpec route = routeLocator.getMatchingRoute("/foo/1"); + Route route = routeLocator.getMatchingRoute("/foo/1"); assertNull("routes did not ignore " + IGNOREDPATTERN, route); } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMappingTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMappingTests.java index 417e33e9..9ecf9bd5 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMappingTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMappingTests.java @@ -16,10 +16,7 @@ package org.springframework.cloud.netflix.zuul.web; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; - -import java.util.Arrays; +import java.util.Collections; import org.junit.Before; import org.junit.Test; @@ -30,6 +27,9 @@ import org.springframework.mock.web.MockHttpServletRequest; import com.netflix.zuul.context.RequestContext; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + /** * @author Dave Syer */ @@ -53,25 +53,29 @@ public class ZuulHandlerMappingTests { @Test public void mappedPath() throws Exception { - Mockito.when(this.locator.getRoutePaths()).thenReturn(Arrays.asList("/foo/**")); + Mockito.when(this.locator.getRoutes()) + .thenReturn(Collections.singletonMap("/foo/**", "foo")); this.request.setServletPath("/foo/"); - this.mapping.registerHandlers(); + this.mapping.setDirty(true); assertNotNull(this.mapping.getHandler(this.request)); } @Test public void defaultPath() throws Exception { - Mockito.when(this.locator.getRoutePaths()).thenReturn(Arrays.asList("/**")); + Mockito.when(this.locator.getRoutes()) + .thenReturn(Collections.singletonMap("/**", "default")); + ; this.request.setServletPath("/"); - this.mapping.registerHandlers(); + this.mapping.setDirty(true); assertNotNull(this.mapping.getHandler(this.request)); } @Test public void errorPath() throws Exception { - Mockito.when(this.locator.getRoutePaths()).thenReturn(Arrays.asList("/**")); + Mockito.when(this.locator.getRoutes()) + .thenReturn(Collections.singletonMap("/**", "default")); this.request.setServletPath("/error"); - this.mapping.registerHandlers(); + this.mapping.setDirty(true); assertNull(this.mapping.getHandler(this.request)); }