From fedb6fce77ed7171bd5b5d1060d0487750525ecd Mon Sep 17 00:00:00 2001 From: Felipe Adorno Date: Thu, 18 Feb 2021 21:35:46 -0300 Subject: [PATCH] Default filters now get route specific events. Filters like Retry with a POST body require an event to be sent from the filter factory to the caching filter higher up the filter chain. Default filters were created with a static id and therefor didn't receive the event. This updates default filters to use the route id. Fixes gh-1918 Fixes gh-2150 --- .../factory/RetryGatewayFilterFactory.java | 10 +++--- .../route/RouteDefinitionRouteLocator.java | 2 +- .../RouteDefinitionRouteLocatorTests.java | 33 +++++++++++++++++++ 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java index d3e998e4..097d4c00 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java @@ -168,12 +168,10 @@ public class RetryGatewayFilterFactory @Override public String toString() { - return filterToStringCreator(RetryGatewayFilterFactory.this) - .append("retries", retryConfig.getRetries()) - .append("series", retryConfig.getSeries()) - .append("statuses", retryConfig.getStatuses()) - .append("methods", retryConfig.getMethods()) - .append("exceptions", retryConfig.getExceptions()).toString(); + return filterToStringCreator(RetryGatewayFilterFactory.this).append("retries", retryConfig.getRetries()) + .append("series", retryConfig.getSeries()).append("statuses", retryConfig.getStatuses()) + .append("methods", retryConfig.getMethods()).append("exceptions", retryConfig.getExceptions()) + .toString(); } }; } diff --git a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocator.java b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocator.java index 2845aebf..7478bb0f 100644 --- a/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocator.java +++ b/spring-cloud-gateway-server/src/main/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocator.java @@ -225,7 +225,7 @@ public class RouteDefinitionRouteLocator // TODO: support option to apply defaults after route specific filters? if (!this.gatewayProperties.getDefaultFilters().isEmpty()) { - filters.addAll(loadGatewayFilters(DEFAULT_FILTERS, + filters.addAll(loadGatewayFilters(routeDefinition.getId(), new ArrayList<>(this.gatewayProperties.getDefaultFilters()))); } diff --git a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocatorTests.java b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocatorTests.java index ba777a4f..42825b88 100644 --- a/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocatorTests.java +++ b/spring-cloud-gateway-server/src/test/java/org/springframework/cloud/gateway/route/RouteDefinitionRouteLocatorTests.java @@ -33,6 +33,7 @@ import org.springframework.cloud.gateway.filter.factory.AbstractGatewayFilterFac import org.springframework.cloud.gateway.filter.factory.AddResponseHeaderGatewayFilterFactory; import org.springframework.cloud.gateway.filter.factory.GatewayFilterFactory; import org.springframework.cloud.gateway.filter.factory.RemoveResponseHeaderGatewayFilterFactory; +import org.springframework.cloud.gateway.filter.factory.RetryGatewayFilterFactory; import org.springframework.cloud.gateway.handler.predicate.HostRoutePredicateFactory; import org.springframework.cloud.gateway.handler.predicate.PredicateDefinition; import org.springframework.cloud.gateway.handler.predicate.RoutePredicateFactory; @@ -118,6 +119,38 @@ public class RouteDefinitionRouteLocatorTests { }).expectComplete().verify(); } + @Test + public void contextLoadsAndApplyRouteIdToRetryFilter() { + List predicates = Arrays.asList(new HostRoutePredicateFactory()); + List gatewayFilterFactories = Arrays.asList(new RetryGatewayFilterFactory(), + new AddResponseHeaderGatewayFilterFactory()); + GatewayProperties gatewayProperties = new GatewayProperties(); + gatewayProperties.setDefaultFilters(Arrays.asList(new FilterDefinition("Retry"))); + gatewayProperties.setRoutes(Arrays.asList(new RouteDefinition() { + { + setId("foo"); + setUri(URI.create("https://foo.example.com")); + setPredicates(Arrays.asList(new PredicateDefinition("Host=*.example.com"))); + setFilters(Arrays.asList(new FilterDefinition("AddResponseHeader=X-Response-Foo, Bar"))); + } + })); + + PropertiesRouteDefinitionLocator routeDefinitionLocator = new PropertiesRouteDefinitionLocator( + gatewayProperties); + @SuppressWarnings("deprecation") + RouteDefinitionRouteLocator routeDefinitionRouteLocator = new RouteDefinitionRouteLocator( + new CompositeRouteDefinitionLocator(Flux.just(routeDefinitionLocator)), predicates, + gatewayFilterFactories, gatewayProperties, new ConfigurationService(null, () -> null, () -> null)); + + StepVerifier.create(routeDefinitionRouteLocator.getRoutes()).assertNext(route -> { + List filters = route.getFilters(); + assertThat(filters).hasSize(2); + assertThat(filters.get(0).toString()).contains("routeId = 'foo'"); + assertThat(getFilterClassName(filters.get(0))).contains("Retry"); + assertThat(getFilterClassName(filters.get(1))).contains("AddResponseHeader"); + }).expectComplete().verify(); + } + private List containsInvalidRoutes() { RouteDefinition foo = new RouteDefinition(); foo.setId("foo");