Merge pull request #1489 from ryanjbaxter/forward-fallbacks-not-working

Remove routed attribute when executing fallbacks in circuit breakers.  Fixes #1421

spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/HystrixGatewayFilterFactory.java

@@ -53,6 +53,7 @@ import static org.springframework.cloud.gateway.support.GatewayToStringStyler.fi
 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;
+import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.removeAlreadyRouted;
 
 /**
  * Depends on `spring-cloud-starter-netflix-hystrix`,
@@ -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
+			removeAlreadyRouted(mutated);
 			return RxReactiveStreams.toObservable(getDispatcherHandler().handle(mutated));
 		}
 

spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java

@@ -49,7 +49,7 @@ import org.springframework.web.server.ServerWebExchange;
 
 import static org.springframework.cloud.gateway.support.GatewayToStringStyler.filterToStringCreator;
 import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.CLIENT_RESPONSE_HEADER_NAMES;
-import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.GATEWAY_ALREADY_ROUTED_ATTR;
+import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.removeAlreadyRouted;
 
 public class RetryGatewayFilterFactory
 		extends AbstractGatewayFilterFactory<RetryGatewayFilterFactory.RetryConfig> {
@@ -207,7 +207,7 @@ public class RetryGatewayFilterFactory
 				CLIENT_RESPONSE_HEADER_NAMES, Collections.emptySet());
 		addedHeaders
 				.forEach(header -> exchange.getResponse().getHeaders().remove(header));
-		exchange.getAttributes().remove(GATEWAY_ALREADY_ROUTED_ATTR);
+		removeAlreadyRouted(exchange);
 	}
 
 	@Deprecated

spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/SpringCloudCircuitBreakerFilterFactory.java

@@ -39,6 +39,7 @@ import static org.springframework.cloud.gateway.support.GatewayToStringStyler.fi
 import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.CIRCUITBREAKER_EXECUTION_EXCEPTION_ATTR;
 import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.GATEWAY_REQUEST_URL_ATTR;
 import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.containsEncodedParts;
+import static org.springframework.cloud.gateway.support.ServerWebExchangeUtils.removeAlreadyRouted;
 
 /**
  * @author Ryan Baxter
@@ -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
+					removeAlreadyRouted(exchange);
+
 					ServerHttpRequest request = exchange.getRequest().mutate()
 							.uri(requestUrl).build();
 					return getDispatcherHandler()

spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/support/ServerWebExchangeUtils.java

@@ -172,6 +172,10 @@ public final class ServerWebExchangeUtils {
 		exchange.getAttributes().put(GATEWAY_ALREADY_ROUTED_ATTR, true);
 	}
 
+	public static void removeAlreadyRouted(ServerWebExchange exchange) {
+		exchange.getAttributes().remove(GATEWAY_ALREADY_ROUTED_ATTR);
+	}
+
 	public static boolean isAlreadyRouted(ServerWebExchange exchange) {
 		return exchange.getAttributeOrDefault(GATEWAY_ALREADY_ROUTED_ATTR, false);
 	}

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\"}");
+	}
+
 }

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<String, String> 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 -> {

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\"}");
+	}
+
 }

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<String, String> 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(