SLING-9053 reworking the solution
diff --git a/src/main/java/org/apache/sling/testing/clients/AbstractSlingClient.java b/src/main/java/org/apache/sling/testing/clients/AbstractSlingClient.java
index 1087158..bf573fd 100644
--- a/src/main/java/org/apache/sling/testing/clients/AbstractSlingClient.java
+++ b/src/main/java/org/apache/sling/testing/clients/AbstractSlingClient.java
@@ -453,12 +453,6 @@
return doStreamRequest(request, headers, expectedStatus);
}
- private boolean isServiceUnavailable(SlingHttpResponse response) {
- StatusLine statusLine = response.getStatusLine();
- return statusLine != null && statusLine.getStatusCode() == HttpStatus.SC_SERVICE_UNAVAILABLE;
- }
-
-
/**
* <p>Execute an HTTP request and consumes the entity in the response. The content is cached and can be retrieved using
* {@code response.getContent()}.
@@ -473,33 +467,10 @@
* @throws ClientException if the request could not be executed
*/
public SlingHttpResponse doRequest(HttpUriRequest request, List<Header> headers, int... expectedStatus) throws ClientException {
- int maxRetries = Constants.HTTP_RETRIES;
- boolean needRetry = false;
- SlingHttpResponse response = null;
- do {
- try {
- if (needRetry) {
- // add some pacing
- Thread.sleep(Constants.HTTP_RETRIES_DELAY);
- }
- response = doStreamRequest(request, headers, expectedStatus);
- needRetry = maxRetries > 0 && isServiceUnavailable(response);
- // Consume entity and cache the content so the connection is closed
- response.getContent();
- } catch (ClientException | RuntimeException ex ) {
- needRetry = ex.getMessage().contains("Could not read content from response")
- || ex.getMessage().contains("Instead 503 was returned!");
+ SlingHttpResponse response = doStreamRequest(request, headers, expectedStatus);
- if (needRetry) {
- log.warn("Retry needed due to " + ex.getMessage());
- }
- if (maxRetries == 0) {
- throw ex;
- }
- } catch (InterruptedException ex) {
- throw new ClientException("Interrupted while pacing request", ex);
- }
- } while (needRetry && maxRetries-- > 0);
+ // Consume entity and cache the content so the connection is closed
+ response.getContent();
return response;
}
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 9b8fb73..18eafc1 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 org.apache.http.client.ServiceUnavailableRetryStrategy;
+import org.apache.http.impl.client.DefaultServiceUnavailableRetryStrategy;
+import org.apache.sling.testing.clients.util.LoggedServiceUnavailableRetryStrategy;
+
public class Constants {
/**
@@ -33,16 +37,21 @@
private static int retries;
// Custom delay between retries in millisec
- private static long retriesDelay;
+ private static int retriesDelay;
+
+ // Custom log retries
+ private static boolean logRetries;
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 = Long.getLong(Constants.CONFIG_PROP_PREFIX + "http.retriesDelay", 1000);
+ Constants.retriesDelay = Integer.getInteger(Constants.CONFIG_PROP_PREFIX + "http.retriesDelay", 1000);
+ Constants.logRetries = Boolean.getBoolean(Constants.CONFIG_PROP_PREFIX + "http.logRetries");
} catch (NumberFormatException e) {
Constants.delay = 0;
Constants.retries = 5;
Constants.retriesDelay = 1000;
+ Constants.logRetries = false;
}
}
@@ -53,16 +62,13 @@
public static final long HTTP_DELAY = delay;
/**
- * Custom number of retries after an HTTP request failed due to 503 or any broken connection issues.
- * Used by {@link org.apache.sling.testing.clients.AbstractSlingClient}
+ * Custom ServiceUnavailableRetryStrategy.
+ * Used by {@link org.apache.sling.testing.clients.SlingClient}
+ * and {@link org.apache.sling.testing.clients.interceptors.FormBasedAuthInterceptor}
*/
- public static final int HTTP_RETRIES = retries;
-
- /**
- * Custom delay in milliseconds between each retries, default to 1000 milliseconds.
- * Used by {@link org.apache.sling.testing.clients.AbstractSlingClient}
- */
- public static final long HTTP_RETRIES_DELAY = retriesDelay;
+ public static final ServiceUnavailableRetryStrategy HTTP_RETRY_STRATEGY = logRetries
+ ? new LoggedServiceUnavailableRetryStrategy(retries, retriesDelay)
+ : new DefaultServiceUnavailableRetryStrategy(retries, retriesDelay);
/**
* Handle to OSGI console
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 b96e979..4ecf731 100644
--- a/src/main/java/org/apache/sling/testing/clients/SlingClient.java
+++ b/src/main/java/org/apache/sling/testing/clients/SlingClient.java
@@ -626,7 +626,8 @@
private final HttpClientBuilder httpClientBuilder;
protected InternalBuilder(URI url, String user, String password) {
- this.httpClientBuilder = HttpClientBuilder.create();
+ this.httpClientBuilder = HttpClientBuilder.create()
+ .setServiceUnavailableRetryStrategy(Constants.HTTP_RETRY_STRATEGY);
this.configBuilder = SlingClientConfig.Builder.create().setUrl(url).setUser(user).setPassword(password);
setDefaults();
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 ca461e6..9360f4b 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
@@ -27,8 +27,7 @@
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.ClientException;
-import org.apache.sling.testing.clients.SlingClient;
+import org.apache.sling.testing.clients.Constants;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -76,7 +75,9 @@
HttpPost loginPost = new HttpPost(URI.create(request.getRequestLine().getUri()).resolve(loginPath));
loginPost.setEntity(httpEntity);
- final CloseableHttpClient client = HttpClientBuilder.create().disableRedirectHandling().build();
+ final CloseableHttpClient client = HttpClientBuilder.create()
+ .setServiceUnavailableRetryStrategy(Constants.HTTP_RETRY_STRATEGY)
+ .disableRedirectHandling().build();
client.execute(host, loginPost, context);
diff --git a/src/main/java/org/apache/sling/testing/clients/util/LoggedServiceUnavailableRetryStrategy.java b/src/main/java/org/apache/sling/testing/clients/util/LoggedServiceUnavailableRetryStrategy.java
new file mode 100644
index 0000000..65bcf9f
--- /dev/null
+++ b/src/main/java/org/apache/sling/testing/clients/util/LoggedServiceUnavailableRetryStrategy.java
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You 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.apache.sling.testing.clients.util;
+
+import org.apache.http.HttpResponse;
+import org.apache.http.annotation.Immutable;
+import org.apache.http.impl.client.DefaultServiceUnavailableRetryStrategy;
+import org.apache.http.protocol.HttpContext;
+import org.apache.http.util.EntityUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.util.Arrays;
+
+@Immutable
+public class LoggedServiceUnavailableRetryStrategy extends DefaultServiceUnavailableRetryStrategy {
+
+ private static final Logger LOGGER = LoggerFactory.getLogger(LoggedServiceUnavailableRetryStrategy.class);
+
+ public LoggedServiceUnavailableRetryStrategy(final int maxRetries, final int retryInterval) {
+ super(maxRetries, retryInterval);
+ }
+
+ @Override
+ public boolean retryRequest(final HttpResponse response, final int executionCount, final HttpContext context) {
+ boolean needRetry = super.retryRequest(response, executionCount, context);
+ if (needRetry) {
+ LOGGER.warn("Request retry needed due to service unavailable response");
+ LOGGER.warn("Response headers contained:");
+ Arrays.stream(response.getAllHeaders()).forEach(h -> LOGGER.warn("Header {}:{}", h.getName(), h.getValue()));
+ try {
+ LOGGER.warn("Response content: {}", EntityUtils.toString(response.getEntity()));
+ } catch (IOException exc) {
+ LOGGER.warn("Response as no content");
+ }
+ }
+ return needRetry;
+ }
+
+}
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 7d4ac3b..d4c0bc1 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.0.0")
+@Version("1.1.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/SlingClientRetryServiceUnavailableTest.java b/src/test/java/org/apache/sling/testing/clients/SlingClientRetryServiceUnavailableTest.java
new file mode 100644
index 0000000..2670983
--- /dev/null
+++ b/src/test/java/org/apache/sling/testing/clients/SlingClientRetryServiceUnavailableTest.java
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You 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.apache.sling.testing.clients;
+
+import org.apache.http.HttpException;
+import org.apache.http.HttpRequest;
+import org.apache.http.HttpResponse;
+import org.apache.http.entity.StringEntity;
+import org.apache.http.protocol.HttpContext;
+import org.apache.http.protocol.HttpRequestHandler;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.concurrent.TimeoutException;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class SlingClientRetryServiceUnavailableTest {
+ private static final String GET_UNAVAILABLE_PATH = "/test/unavailable/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 int requestCount = 0;
+ private static int availableAtRequestCount = Integer.MAX_VALUE;
+
+ static {
+ System.setProperty(Constants.CONFIG_PROP_PREFIX + "http.logRetries", "true");
+ }
+
+ @ClassRule
+ public static HttpServerRule httpServer = new HttpServerRule() {
+ @Override
+ protected void registerHandlers() throws IOException {
+ serverBootstrap.registerHandler(GET_UNAVAILABLE_PATH, new HttpRequestHandler() {
+ @Override
+ public void handle(HttpRequest request, HttpResponse response, HttpContext context) throws HttpException, IOException {
+ requestCount++;
+ if (requestCount == availableAtRequestCount) {
+ response.setEntity(new StringEntity(OK_RESPONSE));
+ response.setStatusCode(200);
+ } else {
+ response.setEntity(new StringEntity(NOK_RESPONSE));
+ response.setStatusCode(503);
+ }
+ }
+ });
+ }
+ };
+
+ @Test
+ public void testRetryReallyUnavailable() throws Exception {
+ requestCount = 0;
+ availableAtRequestCount = Integer.MAX_VALUE; // never available
+ SlingClient c = new SlingClient(httpServer.getURI(), "user", "pass");
+ SlingHttpResponse slingHttpResponse = c.doGet(GET_UNAVAILABLE_PATH, 503, 10);
+ assertEquals(MAX_RETRIES + 1, requestCount);
+ assertEquals(NOK_RESPONSE, slingHttpResponse.getContent());
+ }
+
+ @Test
+ public void testRetryEventuallyAvailable() throws Exception {
+ requestCount = 0;
+ availableAtRequestCount = 3;
+ SlingClient c = new SlingClient(httpServer.getURI(), "user", "pass");
+ SlingHttpResponse slingHttpResponse = c.doGet(GET_UNAVAILABLE_PATH, 200, 10);
+ assertEquals(availableAtRequestCount, requestCount);
+ assertEquals(OK_RESPONSE, slingHttpResponse.getContent());
+
+ }
+
+}
