From 0732b51d104fe8a25ed6873ee2e434a1f52b5864 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Tue, 17 Dec 2019 19:50:10 -0500 Subject: [PATCH] Remove routed attribute when executing fallbacks in circuit breakers. Fixes #1421 --- .../factory/HystrixGatewayFilterFactory.java | 5 +++++ .../SpringCloudCircuitBreakerFilterFactory.java | 6 ++++++ .../factory/HystrixGatewayFilterFactoryTests.java | 7 +++++++ .../gateway/filter/factory/HystrixTestConfig.java | 11 +++++++++++ ...ringCloudCircuitBreakerFilterFactoryTests.java | 8 ++++++++ .../SpringCloudCircuitBreakerTestConfig.java | 15 ++++++++++++++- 6 files changed, 51 insertions(+), 1 deletion(-) diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/HystrixGatewayFilterFactory.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/HystrixGatewayFilterFactory.java index 4fff80f2..9a25cb62 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/HystrixGatewayFilterFactory.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/HystrixGatewayFilterFactory.java @@ -50,6 +50,7 @@ import org.springframework.web.util.UriComponentsBuilder; import static java.util.Collections.singletonList; import static java.util.Optional.ofNullable; import static org.springframework.cloud.gateway.support.GatewayToStringStyler.filterToStringCreator; +import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.GATEWAY_ALREADY_ROUTED_ATTR; import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.GATEWAY_REQUEST_URL_ATTR; import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.HYSTRIX_EXECUTION_EXCEPTION_ATTR; import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.containsEncodedParts; @@ -272,6 +273,10 @@ public class HystrixGatewayFilterFactory ServerHttpRequest request = this.exchange.getRequest().mutate() .uri(requestUrl).build(); ServerWebExchange mutated = exchange.mutate().request(request).build(); + // Before we continue on remove the already routed attribute since the + // fallback may go back through the route handler if the fallback + // is to another route in the Gateway + mutated.getAttributes().remove(GATEWAY_ALREADY_ROUTED_ATTR); return RxReactiveStreams.toObservable(getDispatcherHandler().handle(mutated)); } diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerFilterFactory.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerFilterFactory.java index b82e95f8..4a756ff9 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerFilterFactory.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerFilterFactory.java @@ -37,6 +37,7 @@ import static java.util.Collections.singletonList; import static java.util.Optional.ofNullable; import static org.springframework.cloud.gateway.support.GatewayToStringStyler.filterToStringCreator; import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.CIRCUITBREAKER_EXECUTION_EXCEPTION_ATTR; +import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.GATEWAY_ALREADY_ROUTED_ATTR; import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.GATEWAY_REQUEST_URL_ATTR; import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.containsEncodedParts; @@ -99,6 +100,11 @@ public abstract class SpringCloudCircuitBreakerFilterFactory extends exchange.getAttributes().put(GATEWAY_REQUEST_URL_ATTR, requestUrl); addExceptionDetails(t, exchange); + // Before we continue on remove the already routed attribute since the + // fallback may go back through the route handler if the fallback + // is to another route in the Gateway + exchange.getAttributes().remove(GATEWAY_ALREADY_ROUTED_ATTR); + ServerHttpRequest request = exchange.getRequest().mutate() .uri(requestUrl).build(); return getDispatcherHandler() diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/HystrixGatewayFilterFactoryTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/HystrixGatewayFilterFactoryTests.java index b6c1cd0b..fc9696da 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/HystrixGatewayFilterFactoryTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/HystrixGatewayFilterFactoryTests.java @@ -139,4 +139,11 @@ public class HystrixGatewayFilterFactoryTests extends BaseWebClientTests { assertThat(filter.toString()).contains("myname").contains("forward:/myfallback"); } + @Test + public void filterFallbackForward() { + testClient.get().uri("/delay/3?a=c").header("Host", "www.hystrixforward.org") + .exchange().expectStatus().isOk().expectBody() + .json("{\"from\":\"hystrixfallbackcontroller3\"}"); + } + } diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/HystrixTestConfig.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/HystrixTestConfig.java index 81d28bd2..3b804319 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/HystrixTestConfig.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/HystrixTestConfig.java @@ -66,12 +66,23 @@ public class HystrixTestConfig { return Collections.singletonMap("from", "fallbackcontroller2"); } + @RequestMapping("/hystrixFallbackController3") + public Map fallbackcontroller3() { + return Collections.singletonMap("from", "hystrixfallbackcontroller3"); + } + @Bean public RouteLocator hystrixRouteLocator(RouteLocatorBuilder builder) { return builder.routes().route("hystrix_java", r -> r.host("**.hystrixjava.org") .filters(f -> f.prefixPath("/httpbin").hystrix( config -> config.setFallbackUri("forward:/fallbackcontroller2"))) .uri(uri)) + .route("hystrix_fallback_forward", r -> r.host("**.hystrixforward.org") + .filters(f -> f.hystrix( + config -> config.setFallbackUri("forward:/fallback"))) + .uri(uri)) + .route("hystrix_fallback_controller_3", r -> r.path("/fallback") + .filters(f -> f.setPath("/hystrixFallbackController3")).uri(uri)) .route("hystrix_connection_failure", r -> r.host("**.hystrixconnectfail.org") .filters(f -> f.prefixPath("/httpbin").hystrix(config -> { diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerFilterFactoryTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerFilterFactoryTests.java index 13209e19..77fbff73 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerFilterFactoryTests.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerFilterFactoryTests.java @@ -96,4 +96,12 @@ public abstract class SpringCloudCircuitBreakerFilterFactoryTests .isNotEmpty().jsonPath("$.error").isEqualTo("Internal Server Error"); } + @Test + public void filterFallbackForward() { + testClient.get().uri("/delay/3?a=c") + .header("Host", "www.circuitbreakerforward.org").exchange().expectStatus() + .isOk().expectBody() + .json("{\"from\":\"circuitbreakerfallbackcontroller3\"}"); + } + } diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerTestConfig.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerTestConfig.java index 56c68f68..070ab1f7 100644 --- a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerTestConfig.java +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerTestConfig.java @@ -64,9 +64,22 @@ public class SpringCloudCircuitBreakerTestConfig { return Collections.singletonMap("from", "circuitbreakerfallbackcontroller2"); } + @RequestMapping("/circuitbreakerFallbackController3") + public Map fallbackcontroller3() { + return Collections.singletonMap("from", "circuitbreakerfallbackcontroller3"); + } + @Bean public RouteLocator circuitBreakerRouteLocator(RouteLocatorBuilder builder) { - return builder.routes() + return builder.routes().route("circuitbreaker_fallback_forward", + r -> r.host("**.circuitbreakerforward.org") + .filters(f -> f.circuitBreaker( + config -> config.setFallbackUri("forward:/fallback"))) + .uri(uri)) + .route("fallback_controller_3", + r -> r.path("/fallback").filters( + f -> f.setPath("/circuitbreakerFallbackController3")) + .uri(uri)) .route("circuitbreaker_java", r -> r.host("**.circuitbreakerjava.org") .filters(f -> f.prefixPath("/httpbin") .circuitBreaker(config -> config.setFallbackUri(