Set MfClientHttpRequestFactoryImpl timeouts to automatic cancel timeout.

core/src/main/java/org/mapfish/print/http/ConfigFileResolvingHttpRequestFactory.java

@@ -14,14 +14,15 @@
  * This request factory will attempt to load resources using {@link
  * org.mapfish.print.config.Configuration#loadFile(String)} and {@link
  * org.mapfish.print.config.Configuration#isAccessible(String)} to load the resources if the http
- * method is GET and will fallback to the normal/wrapped factory to make http requests.
+ * method is GET and will fall back to the normal/wrapped factory to make http requests.
  */
 public final class ConfigFileResolvingHttpRequestFactory implements MfClientHttpRequestFactory {
   private final Configuration config;
   @Nonnull private final Map<String, String> mdcContext;
   private final MfClientHttpRequestFactoryImpl httpRequestFactory;
   private final List<RequestConfigurator> callbacks = new CopyOnWriteArrayList<>();
 
+  /** Maximum number of attempts to try to fetch the same http request in case it is failing. */
   @Value("${httpRequest.fetchRetry.maxNumber}")
   private int httpRequestMaxNumberFetchRetry;
 

core/src/main/java/org/mapfish/print/http/ConfigFileResolvingRequest.java

@@ -205,7 +205,10 @@ private boolean sleepIfPossible(final Exception e, final AtomicInteger counter)
       LOGGER.debug("Retry fetching {} following exception", this.getURI(), e);
       return true;
     } else {
-      LOGGER.debug("Has reached maximum number of retry for {}", getURI());
+      LOGGER.debug(
+          "Reached maximum number of {} allowed requests attempts for {}",
+          getHttpRequestMaxNumberFetchRetry(),
+          getURI());
       return false;
     }
   }
@@ -222,7 +225,7 @@ private void sleepWithExceptionHandling() {
   }
 
   private boolean canRetry(final AtomicInteger counter) {
-    return counter.incrementAndGet() <= getHttpRequestMaxNumberFetchRetry();
+    return counter.incrementAndGet() < getHttpRequestMaxNumberFetchRetry();
   }
 
   private int getHttpRequestMaxNumberFetchRetry() {

core/src/main/java/org/mapfish/print/http/MfClientHttpRequestFactoryImpl.java

@@ -12,6 +12,7 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -31,6 +32,7 @@
 import org.apache.http.protocol.HTTP;
 import org.apache.http.protocol.HttpContext;
 import org.mapfish.print.config.Configuration;
+import org.mapfish.print.servlet.job.impl.ThreadPoolJobManager;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.http.HttpHeaders;
@@ -52,30 +54,53 @@ public class MfClientHttpRequestFactoryImpl extends HttpComponentsClientHttpRequ
    * @param maxConnTotal Maximum total connections.
    * @param maxConnPerRoute Maximum connections per route.
    */
-  public MfClientHttpRequestFactoryImpl(final int maxConnTotal, final int maxConnPerRoute) {
-    super(createHttpClient(maxConnTotal, maxConnPerRoute));
+  public MfClientHttpRequestFactoryImpl(
+      final int maxConnTotal,
+      final int maxConnPerRoute,
+      final ThreadPoolJobManager threadPoolJobManager) {
+    super(createHttpClient(maxConnTotal, maxConnPerRoute, threadPoolJobManager));
   }
 
   @Nullable
   static Configuration getCurrentConfiguration() {
     return CURRENT_CONFIGURATION.get();
   }
 
-  private static int getIntProperty(final String name) {
+  /**
+   * Return the number of milliseconds until the timeout Use the Automatic cancellation timeout if
+   * not defined.
+   *
+   * @param name timeout idemtifier
+   * @return the number of milliseconds until the timeout
+   */
+  private static int getTimeoutValue(
+      final String name, final ThreadPoolJobManager threadPoolJobManager) {
     final String value = System.getProperty(name);
     if (value == null) {
-      return -1;
+      long millis = TimeUnit.SECONDS.toMillis(threadPoolJobManager.getTimeout());
+      if (millis > Integer.MAX_VALUE) {
+        LOGGER.warn(
+            "The value of {} is too large.  The timeout will be set to the maximum value of {}",
+            name,
+            Integer.MAX_VALUE);
+        return Integer.MAX_VALUE;
+      } else {
+        return Integer.parseInt(Long.toString(millis));
+      }
     }
     return Integer.parseInt(value);
   }
 
   private static CloseableHttpClient createHttpClient(
-      final int maxConnTotal, final int maxConnPerRoute) {
+      final int maxConnTotal,
+      final int maxConnPerRoute,
+      final ThreadPoolJobManager threadPoolJobManager) {
     final RequestConfig requestConfig =
         RequestConfig.custom()
-            .setConnectionRequestTimeout(getIntProperty("http.connectionRequestTimeout"))
-            .setConnectTimeout(getIntProperty("http.connectTimeout"))
-            .setSocketTimeout(getIntProperty("http.socketTimeout"))
+            .setConnectionRequestTimeout(
+                getTimeoutValue("http.connectionRequestTimeout", threadPoolJobManager))
+            .setConnectTimeout(getTimeoutValue("http.connectTimeout", threadPoolJobManager))
+            .setSocketTimeout(getTimeoutValue("http.socketTimeout", threadPoolJobManager))
             .build();
 
     final HttpClientBuilder httpClientBuilder =
@@ -93,9 +118,9 @@ private static CloseableHttpClient createHttpClient(
     LOGGER.debug(
         "Created CloseableHttpClient using connectionRequestTimeout: {} connectTimeout: {}"
             + " socketTimeout: {}",
-        getIntProperty("http.connectionRequestTimeout"),
-        getIntProperty("http.connectTimeout"),
-        getIntProperty("http.socketTimeout"));
+        getTimeoutValue("http.connectionRequestTimeout", threadPoolJobManager),
+        getTimeoutValue("http.connectTimeout", threadPoolJobManager),
+        getTimeoutValue("http.socketTimeout", threadPoolJobManager));
     return closeableHttpClient;
   }
 

core/src/main/java/org/mapfish/print/servlet/job/impl/ThreadPoolJobManager.java

@@ -136,6 +136,10 @@ public final void setTimeout(final long timeout) {
     this.timeout = timeout;
   }
 
+  public final long getTimeout() {
+    return this.timeout;
+  }
+
   public final void setAbandonedTimeout(final long abandonedTimeout) {
     this.abandonedTimeout = abandonedTimeout;
   }

core/src/main/resources/mapfish-spring-application-context.xml

@@ -62,7 +62,8 @@
     <bean id="healthCheckRegistry" class="org.mapfish.print.metrics.HealthCheckingRegistry"/>
     <bean id="httpClientFactory" class="org.mapfish.print.http.MfClientHttpRequestFactoryImpl">
         <constructor-arg index="0" value="${maxConnectionsTotal}" />
-        <constructor-arg index="1" value="${maxConnectionsPerRoute}" />
+        <constructor-arg index="1" value="${maxConnectionsPerRoute}"/>
+        <constructor-arg index="2" ref="jobManager"/>
     </bean>
     <bean id="metricNameStrategy" class="org.mapfish.print.metrics.MetricsNameStrategyFactory" factory-method="hostAndMethod" />
     <bean id="loggingMetricsConfigurator" class="org.mapfish.print.metrics.LoggingMetricsConfigurator" lazy-init="false"/>

core/src/test/java/org/mapfish/print/TestHttpClientFactory.java

@@ -12,6 +12,7 @@
 import org.mapfish.print.http.ConfigurableRequest;
 import org.mapfish.print.http.MfClientHttpRequestFactory;
 import org.mapfish.print.http.MfClientHttpRequestFactoryImpl;
+import org.mapfish.print.servlet.job.impl.ThreadPoolJobManager;
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
@@ -26,7 +27,7 @@ public class TestHttpClientFactory extends MfClientHttpRequestFactoryImpl
   private final Map<Predicate<URI>, Handler> handlers = new ConcurrentHashMap<>();
 
   public TestHttpClientFactory() {
-    super(20, 10);
+    super(20, 10, new ThreadPoolJobManager());
   }
 
   public void registerHandler(Predicate<URI> matcher, Handler handler) {

core/src/test/java/org/mapfish/print/http/MfClientHttpRequestFactoryImpl_ResponseTest.java → core/src/test/java/org/mapfish/print/http/MfClientHttpRequestFactoryImplTest.java

@@ -3,19 +3,17 @@
 import static org.junit.Assert.assertEquals;
 
 import com.sun.net.httpserver.Headers;
-import com.sun.net.httpserver.HttpExchange;
-import com.sun.net.httpserver.HttpHandler;
 import com.sun.net.httpserver.HttpServer;
-import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.net.URI;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.mapfish.print.servlet.job.impl.ThreadPoolJobManager;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.client.ClientHttpResponse;
 
-public class MfClientHttpRequestFactoryImpl_ResponseTest {
+public class MfClientHttpRequestFactoryImplTest {
   private static final int TARGET_PORT = 33212;
   private static HttpServer targetServer;
 
@@ -35,23 +33,23 @@ public static void tearDown() {
   public void testGetHeaders() throws Exception {
     targetServer.createContext(
         "/request",
-        new HttpHandler() {
-          @Override
-          public void handle(HttpExchange httpExchange) throws IOException {
-            final Headers responseHeaders = httpExchange.getResponseHeaders();
-            responseHeaders.add("Content-Type", "application/json; charset=utf8");
-            httpExchange.sendResponseHeaders(200, 0);
-            httpExchange.close();
-          }
+        httpExchange -> {
+          final Headers responseHeaders = httpExchange.getResponseHeaders();
+          responseHeaders.add("Content-Type", "application/json; charset=utf8");
+          httpExchange.sendResponseHeaders(200, 0);
+          httpExchange.close();
         });
 
-    MfClientHttpRequestFactoryImpl factory = new MfClientHttpRequestFactoryImpl(20, 10);
+    MfClientHttpRequestFactoryImpl factory =
+        new MfClientHttpRequestFactoryImpl(20, 10, new ThreadPoolJobManager());
     final ConfigurableRequest request =
         factory.createRequest(
             new URI("http://" + HttpProxyTest.LOCALHOST + ":" + TARGET_PORT + "/request"),
             HttpMethod.GET);
 
-    final ClientHttpResponse response = request.execute();
-    assertEquals("application/json; charset=utf8", response.getHeaders().getFirst("Content-Type"));
+    try (ClientHttpResponse response = request.execute()) {
+      assertEquals(
+          "application/json; charset=utf8", response.getHeaders().getFirst("Content-Type"));
+    }
   }
 }