From 035258988a4708a18769a90f59e331eae07e6093 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Wed, 14 Feb 2018 17:27:03 -0500 Subject: [PATCH 1/4] Creates a retry filter using reactor-extra fixes gh-94 --- spring-cloud-gateway-core/pom.xml | 4 + .../config/GatewayAutoConfiguration.java | 6 ++ .../factory/RetryGatewayFilterFactory.java | 46 ++++++++++ .../route/builder/GatewayFilterSpec.java | 5 ++ ...yGatewayFilterFactoryIntegrationTests.java | 88 +++++++++++++++++++ .../src/test/resources/application.yml | 8 ++ 6 files changed, 157 insertions(+) create mode 100644 spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java create mode 100644 spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactoryIntegrationTests.java diff --git a/spring-cloud-gateway-core/pom.xml b/spring-cloud-gateway-core/pom.xml index 956738f4..7c5d04cf 100644 --- a/spring-cloud-gateway-core/pom.xml +++ b/spring-cloud-gateway-core/pom.xml @@ -78,6 +78,10 @@ ${kotlin.version} true + + io.projectreactor.addons + reactor-extra + org.springframework.cloud spring-cloud-starter-netflix-eureka-client diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/GatewayAutoConfiguration.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/GatewayAutoConfiguration.java index e68aea07..2c954567 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/GatewayAutoConfiguration.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/config/GatewayAutoConfiguration.java @@ -53,6 +53,7 @@ import org.springframework.cloud.gateway.filter.factory.RemoveNonProxyHeadersGat import org.springframework.cloud.gateway.filter.factory.RemoveRequestHeaderGatewayFilterFactory; import org.springframework.cloud.gateway.filter.factory.RemoveResponseHeaderGatewayFilterFactory; import org.springframework.cloud.gateway.filter.factory.RequestRateLimiterGatewayFilterFactory; +import org.springframework.cloud.gateway.filter.factory.RetryGatewayFilterFactory; import org.springframework.cloud.gateway.filter.factory.RewritePathGatewayFilterFactory; import org.springframework.cloud.gateway.filter.factory.SaveSessionGatewayFilterFactory; import org.springframework.cloud.gateway.filter.factory.SecureHeadersGatewayFilterFactory; @@ -380,6 +381,11 @@ public class GatewayAutoConfiguration { return new RewritePathGatewayFilterFactory(); } + @Bean + public RetryGatewayFilterFactory retryGatewayFilterFactory() { + return new RetryGatewayFilterFactory(); + } + @Bean public SetPathGatewayFilterFactory setPathGatewayFilterFactory() { return new SetPathGatewayFilterFactory(); diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java new file mode 100644 index 00000000..82a2710c --- /dev/null +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java @@ -0,0 +1,46 @@ +/* + * Copyright 2013-2018 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.gateway.filter.factory; + +import java.util.function.Predicate; + +import org.springframework.http.HttpMethod; +import reactor.retry.DefaultRepeat; +import reactor.retry.Repeat; +import reactor.retry.RepeatContext; + +import org.springframework.cloud.gateway.filter.GatewayFilter; +import org.springframework.tuple.Tuple; +import org.springframework.web.server.ServerWebExchange; + +public class RetryGatewayFilterFactory implements GatewayFilterFactory { + @Override + public GatewayFilter apply(Tuple args) { + return (exchange, chain) -> { + Predicate> predicate = context -> { + ServerWebExchange ex = (ServerWebExchange) context.applicationContext(); + boolean retryableStatusCode = ex.getResponse().getStatusCode().is5xxServerError(); + boolean retryableMethod = ex.getRequest().getMethod().equals(HttpMethod.GET); + return retryableMethod && retryableStatusCode; + }; + Repeat repeat = DefaultRepeat.create(predicate, 4) + .withApplicationContext(exchange); + return chain.filter(exchange).repeatWhen(repeat).next(); + }; + } +} diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpec.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpec.java index bdc1732f..74dafbd7 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpec.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpec.java @@ -38,6 +38,7 @@ import org.springframework.cloud.gateway.filter.factory.RemoveNonProxyHeadersGat import org.springframework.cloud.gateway.filter.factory.RemoveRequestHeaderGatewayFilterFactory; import org.springframework.cloud.gateway.filter.factory.RemoveResponseHeaderGatewayFilterFactory; import org.springframework.cloud.gateway.filter.factory.RequestRateLimiterGatewayFilterFactory; +import org.springframework.cloud.gateway.filter.factory.RetryGatewayFilterFactory; import org.springframework.cloud.gateway.filter.factory.RewritePathGatewayFilterFactory; import org.springframework.cloud.gateway.filter.factory.SaveSessionGatewayFilterFactory; import org.springframework.cloud.gateway.filter.factory.SecureHeadersGatewayFilterFactory; @@ -202,6 +203,10 @@ public class GatewayFilterSpec extends UriSpec { return filter(getBean(RewritePathGatewayFilterFactory.class).apply(regex, replacement)); } + public GatewayFilterSpec retry() { + return filter(getBean(RetryGatewayFilterFactory.class).apply(EMPTY_TUPLE)); + } + public GatewayFilterSpec secureHeaders() { return filter(getBean(SecureHeadersGatewayFilterFactory.class).apply(EMPTY_TUPLE)); } diff --git a/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactoryIntegrationTests.java b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactoryIntegrationTests.java new file mode 100644 index 00000000..772a6d42 --- /dev/null +++ b/spring-cloud-gateway-core/src/test/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactoryIntegrationTests.java @@ -0,0 +1,88 @@ +/* + * Copyright 2013-2018 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.gateway.filter.factory; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicInteger; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.springframework.boot.SpringBootConfiguration; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.gateway.test.BaseWebClientTests; +import org.springframework.context.annotation.Import; +import org.springframework.http.server.reactive.ServerHttpRequest; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RestController; + +import static org.springframework.boot.test.context.SpringBootTest.WebEnvironment.RANDOM_PORT; + +@RunWith(SpringRunner.class) +@SpringBootTest(webEnvironment = RANDOM_PORT) +@DirtiesContext +public class RetryGatewayFilterFactoryIntegrationTests extends BaseWebClientTests { + + @Test + public void retryFilterGet() { + testClient.get() + .uri("/retry?key=get") + .exchange() + .expectStatus().isOk() + .expectBody(String.class).isEqualTo("3"); + } + + @Test + //TODO: support post + public void retryFilterPost() { + testClient.post() + .uri("/retry?key=post") + .exchange() + .expectStatus().is5xxServerError(); + // .expectBody(String.class).isEqualTo("3"); + } + + @RestController + @EnableAutoConfiguration + @SpringBootConfiguration + @Import(DefaultTestConfig.class) + public static class TestConfig { + Log log = LogFactory.getLog(getClass()); + + ConcurrentHashMap map = new ConcurrentHashMap<>(); + + @RequestMapping("/httpbin/retry") + public String retry(@RequestParam("key") String key) { + AtomicInteger count = map.computeIfAbsent(key, s -> new AtomicInteger()); + int i = count.incrementAndGet(); + log.warn("Retry count: "+i); + if (i < 3) { + throw new RuntimeException("temporarily broken"); + } + return String.valueOf(i); + } + } + +} diff --git a/spring-cloud-gateway-core/src/test/resources/application.yml b/spring-cloud-gateway-core/src/test/resources/application.yml index ef5b8be1..73a063ff 100644 --- a/spring-cloud-gateway-core/src/test/resources/application.yml +++ b/spring-cloud-gateway-core/src/test/resources/application.yml @@ -131,6 +131,14 @@ spring: - AddResponseHeader=X-Request-Foo, Bar - RemoveResponseHeader=X-Request-Foo + # ===================================== + - id: retry_test + uri: ${test.uri} + predicates: + - Path=/retry + filters: + - Retry + # ===================================== - id: secure_headers_test uri: ${test.uri} From 3bc54ab82cf2272d2ca4149a4494f37ef04ebceb Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Wed, 14 Feb 2018 23:30:13 -0500 Subject: [PATCH 2/4] Configuration for retry filter. --- .../factory/RetryGatewayFilterFactory.java | 130 ++++++++++++++++-- .../route/builder/GatewayFilterSpec.java | 37 ++++- 2 files changed, 152 insertions(+), 15 deletions(-) diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java index 82a2710c..c468bc51 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java @@ -17,30 +17,138 @@ package org.springframework.cloud.gateway.filter.factory; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; import java.util.function.Predicate; -import org.springframework.http.HttpMethod; import reactor.retry.DefaultRepeat; import reactor.retry.Repeat; import reactor.retry.RepeatContext; import org.springframework.cloud.gateway.filter.GatewayFilter; +import org.springframework.cloud.gateway.support.ServerWebExchangeUtils; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpStatus; +import org.springframework.http.HttpStatus.Series; import org.springframework.tuple.Tuple; +import org.springframework.util.Assert; import org.springframework.web.server.ServerWebExchange; public class RetryGatewayFilterFactory implements GatewayFilterFactory { @Override public GatewayFilter apply(Tuple args) { - return (exchange, chain) -> { - Predicate> predicate = context -> { - ServerWebExchange ex = (ServerWebExchange) context.applicationContext(); - boolean retryableStatusCode = ex.getResponse().getStatusCode().is5xxServerError(); - boolean retryableMethod = ex.getRequest().getMethod().equals(HttpMethod.GET); - return retryableMethod && retryableStatusCode; - }; - Repeat repeat = DefaultRepeat.create(predicate, 4) - .withApplicationContext(exchange); - return chain.filter(exchange).repeatWhen(repeat).next(); + Retry retry = new Retry(); + + if (args.hasFieldName("retries")) { + retry.retries(args.getInt("retries")); + } + + // TODO: list of statusSeries + if (args.hasFieldName("statusSeries")) { + int statusSeries = args.getInt("statusSeries"); + retry.series(Series.valueOf(statusSeries)); + } + + // TODO: list of status + if (args.hasFieldName("status")) { + retry.statuses(ServerWebExchangeUtils.parse(args.getRawString("status"))); + } + + // TODO: list of methods + if (args.hasFieldName("method")) { + retry.methods(HttpMethod.resolve(args.getString("method").toUpperCase())); + } + + return apply(retry); + } + + public GatewayFilter apply(Retry retry) { + retry.validate(); + + Predicate> predicate = context -> { + ServerWebExchange exchange = context.applicationContext(); + HttpStatus statusCode = exchange.getResponse().getStatusCode(); + HttpMethod httpMethod = exchange.getRequest().getMethod(); + + boolean retryableStatusCode = retry.getStatuses().contains(statusCode); + + if (!retryableStatusCode) { + // try the series + retryableStatusCode = retry.getSeries().stream() + .anyMatch(series -> statusCode.series().equals(series)); + } + + boolean retryableMethod = retry.getMethods().contains(httpMethod); + return retryableMethod && retryableStatusCode; }; + + //TODO: use Repeat statics once updated with a create() like method + Repeat repeat = DefaultRepeat.create(predicate, retry.getRetries()); + + //TODO: support timeout, backoff, jitter, etc... in Builder + return apply(repeat); + } + + public GatewayFilter apply(Repeat repeat) { + return (exchange, chain) -> chain.filter(exchange).repeatWhen( + repeat.withApplicationContext(exchange)).next(); + } + + public static class Retry { + private int retries = 3; + + private List series = Collections.singletonList(Series.SERVER_ERROR); + + private List statuses = Collections.emptyList(); + + private List methods = Collections.singletonList(HttpMethod.GET); + + public Retry retries(int retries) { + this.retries = retries; + return this; + } + + public Retry series(Series... series) { + this.series = Arrays.asList(series); + return this; + } + + public Retry statuses(HttpStatus... statuses) { + this.statuses = Arrays.asList(statuses); + return this; + } + + public Retry methods(HttpMethod... methods) { + this.methods = Arrays.asList(methods); + return this; + } + + public Retry allMethods() { + return methods(HttpMethod.values()); + } + + public void validate() { + Assert.isTrue(this.retries > 0, "retries must be greater than 0"); + Assert.isTrue(!this.series.isEmpty() || !this.statuses.isEmpty(), + "series and status may not both be empty"); + Assert.notEmpty(this.methods, "methods may not be empty"); + } + + public int getRetries() { + return retries; + } + + public List getSeries() { + return series; + } + + public List getStatuses() { + return statuses; + } + + public List getMethods() { + return methods; + } } } diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpec.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpec.java index 74dafbd7..9d89186f 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpec.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/route/builder/GatewayFilterSpec.java @@ -22,8 +22,11 @@ import java.util.Arrays; import java.util.Collection; import java.util.List; +import com.netflix.hystrix.HystrixObservableCommand; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import reactor.retry.Repeat; + import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.cloud.gateway.filter.GatewayFilter; import org.springframework.cloud.gateway.filter.OrderedGatewayFilter; @@ -50,10 +53,10 @@ import org.springframework.cloud.gateway.filter.factory.StripPrefixGatewayFilter import org.springframework.cloud.gateway.filter.ratelimit.KeyResolver; import org.springframework.cloud.gateway.route.Route; import org.springframework.core.Ordered; +import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.tuple.Tuple; - -import com.netflix.hystrix.HystrixObservableCommand; +import org.springframework.web.server.ServerWebExchange; import static org.springframework.tuple.TupleBuilder.tuple; @@ -203,8 +206,34 @@ public class GatewayFilterSpec extends UriSpec { return filter(getBean(RewritePathGatewayFilterFactory.class).apply(regex, replacement)); } - public GatewayFilterSpec retry() { - return filter(getBean(RetryGatewayFilterFactory.class).apply(EMPTY_TUPLE)); + /** + * 5xx errors and GET are retryable + * @param retries max number of retries + */ + public GatewayFilterSpec retry(int retries) { + return filter(getBean(RetryGatewayFilterFactory.class) + .apply(new RetryGatewayFilterFactory.Retry() + .retries(retries))); + } + + /** + * @param retries max number of retries + * @param httpStatusSeries the http status series that is retryable + * @param httpMethod the http method that is retryable + */ + public GatewayFilterSpec retry(int retries, HttpStatus.Series httpStatusSeries, HttpMethod httpMethod) { + return retry(new RetryGatewayFilterFactory.Retry() + .retries(retries) + .series(httpStatusSeries) + .methods(httpMethod)); + } + + public GatewayFilterSpec retry(RetryGatewayFilterFactory.Retry retry) { + return filter(getBean(RetryGatewayFilterFactory.class).apply(retry)); + } + + public GatewayFilterSpec retry(Repeat repeat) { + return filter(getBean(RetryGatewayFilterFactory.class).apply(repeat)); } public GatewayFilterSpec secureHeaders() { From 9ba1e67fd6415b03517574dda299e368b7023ce1 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Thu, 15 Feb 2018 14:16:38 -0500 Subject: [PATCH 3/4] Removes use of DefaultRepeat, moves iteration count to predicate. --- .../gateway/filter/factory/RetryGatewayFilterFactory.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java index c468bc51..788b2f84 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java @@ -67,6 +67,12 @@ public class RetryGatewayFilterFactory implements GatewayFilterFactory { retry.validate(); Predicate> predicate = context -> { + boolean retryableAttempt = context.iteration() < retry.getRetries(); + + if (!retryableAttempt) { + return false; + } + ServerWebExchange exchange = context.applicationContext(); HttpStatus statusCode = exchange.getResponse().getStatusCode(); HttpMethod httpMethod = exchange.getRequest().getMethod(); @@ -84,7 +90,7 @@ public class RetryGatewayFilterFactory implements GatewayFilterFactory { }; //TODO: use Repeat statics once updated with a create() like method - Repeat repeat = DefaultRepeat.create(predicate, retry.getRetries()); + Repeat repeat = Repeat.onlyIf(predicate); //TODO: support timeout, backoff, jitter, etc... in Builder return apply(repeat); From e051eecbca378458be964133160cb707f94e8db2 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Thu, 15 Feb 2018 14:17:28 -0500 Subject: [PATCH 4/4] polish --- .../cloud/gateway/filter/factory/RetryGatewayFilterFactory.java | 1 - 1 file changed, 1 deletion(-) diff --git a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java index 788b2f84..2de8b526 100644 --- a/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java +++ b/spring-cloud-gateway-core/src/main/java/org/springframework/cloud/gateway/filter/factory/RetryGatewayFilterFactory.java @@ -22,7 +22,6 @@ import java.util.Collections; import java.util.List; import java.util.function.Predicate; -import reactor.retry.DefaultRepeat; import reactor.retry.Repeat; import reactor.retry.RepeatContext;