This is an automated email from the ASF dual-hosted git repository.
dulvac pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-testing-clients.git
The following commit(s) were added to refs/heads/master by this push:
new a7c9b49 SLING-9329 Implement configurable retries mechanism for http
5XX response codes
a7c9b49 is described below
commit a7c9b4987a85576c0017e385cfa4f62a5736b2c7
Author: Andrei Dulvac <[email protected]>
AuthorDate: Fri Apr 3 18:10:09 2020 +0200
SLING-9329 Implement configurable retries mechanism for http 5XX response
codes
---
.../apache/sling/testing/clients/Constants.java | 53 ++++++++++++++++++++--
.../apache/sling/testing/clients/SlingClient.java | 6 ++-
.../interceptors/FormBasedAuthInterceptor.java | 27 ++++++++---
.../apache/sling/testing/clients/package-info.java | 2 +-
.../clients/util/ServerErrorRetryStrategy.java | 39 ++++++++++++++--
.../sling/testing/clients/util/package-info.java | 2 +-
.../clients/SlingClientRetryStrategyTest.java | 31 +++++++++++--
7 files changed, 138 insertions(+), 22 deletions(-)
diff --git a/src/main/java/org/apache/sling/testing/clients/Constants.java
b/src/main/java/org/apache/sling/testing/clients/Constants.java
index 43a2cf6..f8524d6 100644
--- a/src/main/java/org/apache/sling/testing/clients/Constants.java
+++ b/src/main/java/org/apache/sling/testing/clients/Constants.java
@@ -16,6 +16,10 @@
*/
package org.apache.sling.testing.clients;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
public class Constants {
/**
@@ -37,12 +41,47 @@ public class Constants {
// Custom log retries
private static boolean logRetries;
+
+ // CSV list of error codes to retry requests for
+ private static String retryCodes = "";
+
+ /**
+ * System property for {@see HTTP_DELAY}
+ * Prefixed by {@see CONFIG_PROP_PREFIX}
+ */
+ public static final String HTTP_DELAY_PROP = "http.delay";
+
+ /**
+ * System property for {@see HTTP_RETRIES}
+ * Prefixed by {@see CONFIG_PROP_PREFIX}
+ */
+ public static final String HTTP_RETRIES_PROP = "http.retries";
+
+ /**
+ * System property for {@see HTTP_RETRIES_DELAY}
+ * Prefixed by {@see CONFIG_PROP_PREFIX}
+ */
+ public static final String HTTP_RETRIES_DELAY_PROP = "http.retriesDelay";
+
+ /**
+ * System property for {@see HTTP_LOG_RETRIES}
+ * Prefixed by {@see CONFIG_PROP_PREFIX}
+ */
+ public static final String HTTP_LOG_RETRIES_PROP = "http.logRetries";
+
+ /**
+ * System property for {@see HTTP_RETRIES_ERROR_CODES}
+ * Prefixed by {@see CONFIG_PROP_PREFIX}
+ */
+ public static final String HTTP_RETRIES_ERROR_CODES_PROP =
"http.retriesErrorCodes";
+
static {
try {
- Constants.delay = Long.getLong(Constants.CONFIG_PROP_PREFIX +
"http.delay", 0);
- Constants.retries =
Integer.getInteger(Constants.CONFIG_PROP_PREFIX + "http.retries", 5);
- Constants.retriesDelay =
Integer.getInteger(Constants.CONFIG_PROP_PREFIX + "http.retriesDelay", 1000);
- Constants.logRetries =
Boolean.getBoolean(Constants.CONFIG_PROP_PREFIX + "http.logRetries");
+ Constants.delay = Long.getLong(Constants.CONFIG_PROP_PREFIX +
HTTP_DELAY_PROP, 0);
+ Constants.retries =
Integer.getInteger(Constants.CONFIG_PROP_PREFIX + HTTP_RETRIES_PROP, 10);
+ Constants.retriesDelay =
Integer.getInteger(Constants.CONFIG_PROP_PREFIX + HTTP_RETRIES_DELAY_PROP,
1000);
+ Constants.logRetries =
Boolean.getBoolean(Constants.CONFIG_PROP_PREFIX + HTTP_LOG_RETRIES_PROP);
+ Constants.retryCodes =
System.getProperty(Constants.CONFIG_PROP_PREFIX +
HTTP_RETRIES_ERROR_CODES_PROP, "");
} catch (NumberFormatException e) {
Constants.delay = 0;
Constants.retries = 5;
@@ -72,6 +111,12 @@ public class Constants {
*/
public static final boolean HTTP_LOG_RETRIES = logRetries;
+ /**
+ * Comma-separated list of http response codes for which to retry the
request
+ * If empty, all 5XX error codes will be retried
+ */
+ public static final String HTTP_RETRIES_ERROR_CODES = retryCodes;
+
/**
diff --git a/src/main/java/org/apache/sling/testing/clients/SlingClient.java
b/src/main/java/org/apache/sling/testing/clients/SlingClient.java
index 15a20db..fd5015c 100644
--- a/src/main/java/org/apache/sling/testing/clients/SlingClient.java
+++ b/src/main/java/org/apache/sling/testing/clients/SlingClient.java
@@ -693,7 +693,11 @@ public class SlingClient extends AbstractSlingClient {
// HTTP request strategy
httpClientBuilder.setServiceUnavailableRetryStrategy(new
ServerErrorRetryStrategy(
- Constants.HTTP_RETRIES, Constants.HTTP_RETRIES_DELAY,
Constants.HTTP_LOG_RETRIES));
+ Constants.HTTP_RETRIES,
+ Constants.HTTP_RETRIES_DELAY,
+ Constants.HTTP_LOG_RETRIES,
+ Constants.HTTP_RETRIES_ERROR_CODES)
+ );
return this;
}
diff --git
a/src/main/java/org/apache/sling/testing/clients/interceptors/FormBasedAuthInterceptor.java
b/src/main/java/org/apache/sling/testing/clients/interceptors/FormBasedAuthInterceptor.java
index e7e251f..a8bba17 100644
---
a/src/main/java/org/apache/sling/testing/clients/interceptors/FormBasedAuthInterceptor.java
+++
b/src/main/java/org/apache/sling/testing/clients/interceptors/FormBasedAuthInterceptor.java
@@ -16,9 +16,15 @@
*/
package org.apache.sling.testing.clients.interceptors;
-import org.apache.http.*;
+import org.apache.http.HttpEntity;
+import org.apache.http.HttpException;
+import org.apache.http.HttpHost;
+import org.apache.http.HttpRequest;
+import org.apache.http.HttpRequestInterceptor;
+import org.apache.http.NameValuePair;
import org.apache.http.auth.AuthScope;
import org.apache.http.client.CredentialsProvider;
+import org.apache.http.client.ServiceUnavailableRetryStrategy;
import org.apache.http.client.entity.UrlEncodedFormEntity;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.protocol.HttpClientContext;
@@ -27,15 +33,18 @@ import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.message.BasicNameValuePair;
import org.apache.http.protocol.HttpContext;
-import org.apache.sling.testing.clients.Constants;
import org.apache.sling.testing.clients.util.ServerErrorRetryStrategy;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.net.URI;
+import java.util.Arrays;
import java.util.LinkedList;
import java.util.List;
+import java.util.stream.Collectors;
+
+import static org.apache.sling.testing.clients.Constants.*;
public class FormBasedAuthInterceptor implements HttpRequestInterceptor {
static final Logger LOG =
LoggerFactory.getLogger(FormBasedAuthInterceptor.class);
@@ -56,7 +65,8 @@ public class FormBasedAuthInterceptor implements
HttpRequestInterceptor {
Cookie loginCookie = getLoginCookie(context, loginTokenName);
if (loginCookie != null) {
- LOG.debug("Request has cookie {}={} so I'm not intercepting the
request", loginCookie.getName(), loginCookie.getValue());
+ LOG.debug("Request has cookie {}={} so I'm not intercepting the
request",
+ loginCookie.getName(), loginCookie.getValue());
return;
}
@@ -76,10 +86,13 @@ public class FormBasedAuthInterceptor implements
HttpRequestInterceptor {
HttpPost loginPost = new
HttpPost(URI.create(request.getRequestLine().getUri()).resolve(loginPath));
loginPost.setEntity(httpEntity);
- final CloseableHttpClient client =
HttpClientBuilder.create().setServiceUnavailableRetryStrategy(
- new ServerErrorRetryStrategy(
- Constants.HTTP_RETRIES, Constants.HTTP_RETRIES_DELAY,
Constants.HTTP_LOG_RETRIES))
- .disableRedirectHandling().build();
+
+ ServiceUnavailableRetryStrategy retryStrategy = new
ServerErrorRetryStrategy(
+ HTTP_RETRIES, HTTP_RETRIES_DELAY, HTTP_LOG_RETRIES,
HTTP_RETRIES_ERROR_CODES);
+ final CloseableHttpClient client = HttpClientBuilder.create()
+ .setServiceUnavailableRetryStrategy(retryStrategy)
+ .disableRedirectHandling()
+ .build();
client.execute(host, loginPost, context);
diff --git a/src/main/java/org/apache/sling/testing/clients/package-info.java
b/src/main/java/org/apache/sling/testing/clients/package-info.java
index df6de91..8ace300 100644
--- a/src/main/java/org/apache/sling/testing/clients/package-info.java
+++ b/src/main/java/org/apache/sling/testing/clients/package-info.java
@@ -17,7 +17,7 @@
* under the License.
*/
-@Version("1.6.0")
+@Version("1.7.0")
package org.apache.sling.testing.clients;
import org.osgi.annotation.versioning.Version;
diff --git
a/src/main/java/org/apache/sling/testing/clients/util/ServerErrorRetryStrategy.java
b/src/main/java/org/apache/sling/testing/clients/util/ServerErrorRetryStrategy.java
index bcce9e4..0e3d770 100644
---
a/src/main/java/org/apache/sling/testing/clients/util/ServerErrorRetryStrategy.java
+++
b/src/main/java/org/apache/sling/testing/clients/util/ServerErrorRetryStrategy.java
@@ -27,6 +27,13 @@ import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Objects;
+import java.util.stream.Collectors;
+
+import static org.apache.http.HttpStatus.*;
+import static
org.apache.sling.testing.clients.Constants.HTTP_RETRIES_ERROR_CODES;
/**
* {code ServiceUnavailableRetryStrategy} strategy for retrying request in
case of a 5XX response code
@@ -38,14 +45,31 @@ public class ServerErrorRetryStrategy implements
ServiceUnavailableRetryStrategy
private final int maxRetries;
private final int retryInterval;
private final boolean logRetries;
+ private Collection<Integer> retryErrorCodes;
- public ServerErrorRetryStrategy(final int maxRetries, final int
retryInterval, final boolean logRetries) {
+ public ServerErrorRetryStrategy(final int maxRetries, final int
retryInterval, final boolean logRetries,
+ String retryErrorCodes) {
super();
Args.positive(maxRetries, "Max retries");
Args.positive(retryInterval, "Retry interval");
this.maxRetries = maxRetries;
this.retryInterval = retryInterval;
this.logRetries = logRetries;
+ if (retryErrorCodes == null) {
+ retryErrorCodes = "";
+ }
+ this.retryErrorCodes = Arrays.asList(retryErrorCodes.split(","))
+ .stream().map(s -> {
+ try {
+ return Integer.valueOf(s);
+ } catch (NumberFormatException e) {
+ return null;
+ }
+ }).filter(Objects::nonNull).collect(Collectors.toList());
+ }
+
+ public ServerErrorRetryStrategy(final int maxRetries, final int
retryInterval, final boolean logRetries) {
+ this(maxRetries, retryInterval, logRetries, "");
}
public ServerErrorRetryStrategy(int maxRetries, int retryInterval) {
@@ -58,9 +82,7 @@ public class ServerErrorRetryStrategy implements
ServiceUnavailableRetryStrategy
@Override
public boolean retryRequest(final HttpResponse response, final int
executionCount, final HttpContext context) {
- boolean needsRetry = executionCount <= this.maxRetries &&
- response.getStatusLine().getStatusCode() >=
HttpStatus.SC_INTERNAL_SERVER_ERROR &&
- response.getStatusLine().getStatusCode() <
HttpStatus.SC_INTERNAL_SERVER_ERROR + 100;
+ boolean needsRetry = executionCount <= this.maxRetries &&
responseRetryCondition(response);
if (this.logRetries && needsRetry && LOG.isWarnEnabled()) {
LOG.warn("Request retry needed due to service unavailable
response");
@@ -81,4 +103,13 @@ public class ServerErrorRetryStrategy implements
ServiceUnavailableRetryStrategy
return retryInterval;
}
+ private boolean responseRetryCondition(final HttpResponse response) {
+ final Integer statusCode = response.getStatusLine().getStatusCode();
+ if (retryErrorCodes != null && !retryErrorCodes.isEmpty()) {
+ return retryErrorCodes.contains(statusCode);
+ } else {
+ return statusCode >= SC_INTERNAL_SERVER_ERROR &&
+ statusCode < SC_INTERNAL_SERVER_ERROR + 100;
+ }
+ }
}
diff --git
a/src/main/java/org/apache/sling/testing/clients/util/package-info.java
b/src/main/java/org/apache/sling/testing/clients/util/package-info.java
index d4c0bc1..d497a5a 100644
--- a/src/main/java/org/apache/sling/testing/clients/util/package-info.java
+++ b/src/main/java/org/apache/sling/testing/clients/util/package-info.java
@@ -17,7 +17,7 @@
* under the License.
*/
-@Version("1.1.0")
+@Version("1.2.0")
package org.apache.sling.testing.clients.util;
import org.osgi.annotation.versioning.Version;
diff --git
a/src/test/java/org/apache/sling/testing/clients/SlingClientRetryStrategyTest.java
b/src/test/java/org/apache/sling/testing/clients/SlingClientRetryStrategyTest.java
index 23f3b63..60d782e 100644
---
a/src/test/java/org/apache/sling/testing/clients/SlingClientRetryStrategyTest.java
+++
b/src/test/java/org/apache/sling/testing/clients/SlingClientRetryStrategyTest.java
@@ -28,18 +28,20 @@ public class SlingClientRetryStrategyTest {
private static final String GET_UNAVAILABLE_PATH =
"/test/unavailable/resource";
private static final String GET_INEXISTENT_PATH =
"/test/inexistent/resource";
private static final String GET_INTERNAL_ERROR_PATH =
"/test/internalerror/resource";
+ private static final String GET_505_PATH =
"/test/unsupportedversion/resource";
private static final String NOK_RESPONSE = "TEST_NOK";
private static final String OK_RESPONSE = "TEST_OK";
- private static final int MAX_RETRIES = 5;
+ private static final int MAX_RETRIES = 4;
private static int requestCount = 0;
private static int availableAtRequestCount = Integer.MAX_VALUE;
static {
- System.setProperty(Constants.CONFIG_PROP_PREFIX +
Constants.HTTP_LOG_RETRIES, "true");
- System.setProperty(Constants.CONFIG_PROP_PREFIX +
Constants.HTTP_DELAY, "50");
- System.setProperty(Constants.CONFIG_PROP_PREFIX +
Constants.HTTP_RETRIES, "5");
+ System.setProperty(Constants.CONFIG_PROP_PREFIX +
Constants.HTTP_LOG_RETRIES_PROP, "true");
+ System.setProperty(Constants.CONFIG_PROP_PREFIX +
Constants.HTTP_DELAY_PROP, "50");
+ System.setProperty(Constants.CONFIG_PROP_PREFIX +
Constants.HTTP_RETRIES_PROP, "4");
+ System.setProperty(Constants.CONFIG_PROP_PREFIX +
Constants.HTTP_RETRIES_ERROR_CODES_PROP, "500,503");
}
@ClassRule
@@ -73,6 +75,17 @@ public class SlingClientRetryStrategyTest {
response.setEntity(new StringEntity(NOK_RESPONSE));
response.setStatusCode(404);
});
+
+ serverBootstrap.registerHandler(GET_505_PATH, (request, response,
context) -> {
+ requestCount++;
+ if (requestCount == availableAtRequestCount) {
+ response.setEntity(new StringEntity(OK_RESPONSE));
+ response.setStatusCode(200);
+ } else {
+ response.setEntity(new StringEntity(NOK_RESPONSE));
+ response.setStatusCode(505);
+ }
+ });
}
};
@@ -97,6 +110,16 @@ public class SlingClientRetryStrategyTest {
}
@Test
+ public void test505ShouldNotRetry() throws Exception {
+ requestCount = 0;
+ availableAtRequestCount = Integer.MAX_VALUE; // never 200
+ SlingClient c = new SlingClient(httpServer.getURI(), "user", "pass");
+ SlingHttpResponse slingHttpResponse = c.doGet(GET_505_PATH, 505);
+ assertEquals(1, requestCount);
+ assertEquals(NOK_RESPONSE, slingHttpResponse.getContent());
+ }
+
+ @Test
public void testRetryInexistent() throws Exception {
requestCount = 0;
availableAtRequestCount = Integer.MAX_VALUE; // never available
