From fedb6fce77ed7171bd5b5d1060d0487750525ecd Mon Sep 17 00:00:00 2001 From: Felipe Adorno Date: Thu, 18 Feb 2021 21:35:46 -0300 Subject: [PATCH 1/3] 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"); From 91e475e65fa2eba190ba06c89c555ed39e414ec8 Mon Sep 17 00:00:00 2001 From: spencergibb Date: Fri, 12 Mar 2021 16:05:37 -0500 Subject: [PATCH 2/3] Adds routeId to toString() and formatting --- .../filter/factory/RetryGatewayFilterFactory.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 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 097d4c00..6e79e358 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,10 +168,13 @@ 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("routeId", retryConfig.getRouteId()) + .append("retries", retryConfig.getRetries()) + .append("series", retryConfig.getSeries()) + .append("statuses", retryConfig.getStatuses()) + .append("methods", retryConfig.getMethods()) + .append("exceptions", retryConfig.getExceptions()).toString(); } }; } From 79adb1b3f08a759c0940c1faf48dd5cdb2c45916 Mon Sep 17 00:00:00 2001 From: spencergibb Date: Fri, 12 Mar 2021 16:06:13 -0500 Subject: [PATCH 3/3] Formatting and updated Constructor for 2.2.x --- .../route/RouteDefinitionRouteLocatorTests.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) 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 42825b88..0eb400fd 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 @@ -121,8 +121,10 @@ public class RouteDefinitionRouteLocatorTests { @Test public void contextLoadsAndApplyRouteIdToRetryFilter() { - List predicates = Arrays.asList(new HostRoutePredicateFactory()); - List gatewayFilterFactories = Arrays.asList(new RetryGatewayFilterFactory(), + 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"))); @@ -130,8 +132,10 @@ public class RouteDefinitionRouteLocatorTests { { 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"))); + setPredicates( + Arrays.asList(new PredicateDefinition("Host=*.example.com"))); + setFilters(Arrays.asList( + new FilterDefinition("AddResponseHeader=X-Response-Foo, Bar"))); } })); @@ -139,8 +143,9 @@ public class RouteDefinitionRouteLocatorTests { gatewayProperties); @SuppressWarnings("deprecation") RouteDefinitionRouteLocator routeDefinitionRouteLocator = new RouteDefinitionRouteLocator( - new CompositeRouteDefinitionLocator(Flux.just(routeDefinitionLocator)), predicates, - gatewayFilterFactories, gatewayProperties, new ConfigurationService(null, () -> null, () -> null)); + new CompositeRouteDefinitionLocator(Flux.just(routeDefinitionLocator)), + predicates, gatewayFilterFactories, gatewayProperties, + new ConfigurationService()); StepVerifier.create(routeDefinitionRouteLocator.getRoutes()).assertNext(route -> { List filters = route.getFilters();