Merge pull request #3535 from mapfish/gsmfp-39

Organize timeouts

build.gradle

@@ -52,10 +52,16 @@ configure(subprojects.findAll { ['core', 'examples'].contains(it.name) }) {
         reportLevel = "high"
     }
 
-    sourceCompatibility = JavaVersion.VERSION_17
-    targetCompatibility = JavaVersion.VERSION_17
+    java {
+        sourceCompatibility = JavaVersion.VERSION_17
+        targetCompatibility = JavaVersion.VERSION_17
+    }
 
-    compileJava.options.compilerArgs = ['-Xlint:deprecation', '-Xlint:unchecked']
+    compileJava {
+        options.encoding = 'utf-8'
+        options.deprecation = true
+        options.compilerArgs += ['-Xlint:unchecked']
+    }
 
     dependencies {
         testImplementation(
@@ -73,9 +79,6 @@ configure(subprojects.findAll { ['core', 'examples'].contains(it.name) }) {
         }
     }
 
-    tasks.withType(JavaCompile).configureEach {
-        options.encoding = 'utf-8'
-    }
     tasks.withType(Javadoc).tap {
         configureEach {
             options.encoding = 'utf-8'

core/build.gradle

@@ -38,7 +38,7 @@ distributions {
             }
             into('bin') {
                 from(startScripts)
-                fileMode = 0755
+                filePermissions { unix(0755) }
             }
         }
     }
@@ -205,8 +205,10 @@ static def gitRevision() {
     return gitRev != null ? gitRev : 'git rev-parse HEAD'.execute().text.trim()
 }
 
-compileJava.options.compilerArgs = ['-Xlint:deprecation', '-Xlint:unchecked']
-compileTestJava.options.compilerArgs = ['-Xlint:deprecation', '-Xlint:unchecked']
+compileTestJava {
+    options.deprecation = true
+    options.compilerArgs = ['-Xlint:unchecked']
+}
 
 // Add version to resources, used for User-Agent string:
 processResources {

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

@@ -12,7 +12,6 @@
 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;
@@ -32,7 +31,6 @@
 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;
@@ -53,54 +51,43 @@ public class MfClientHttpRequestFactoryImpl extends HttpComponentsClientHttpRequ
    *
    * @param maxConnTotal Maximum total connections.
    * @param maxConnPerRoute Maximum connections per route.
+   * @param connectionRequestTimeout Number of milliseconds used when requesting a connection from
+   *     the connection manager.
+   * @param connectTimeout Number of milliseconds until a connection is established.
+   * @param socketTimeout Maximum number of milliseconds during which a socket remains inactive
+   *     between two consecutive data packets.
    */
   public MfClientHttpRequestFactoryImpl(
       final int maxConnTotal,
       final int maxConnPerRoute,
-      final ThreadPoolJobManager threadPoolJobManager) {
-    super(createHttpClient(maxConnTotal, maxConnPerRoute, threadPoolJobManager));
+      final int connectionRequestTimeout,
+      final int connectTimeout,
+      final int socketTimeout) {
+    super(
+        createHttpClient(
+            maxConnTotal,
+            maxConnPerRoute,
+            connectionRequestTimeout,
+            connectTimeout,
+            socketTimeout));
   }
 
   @Nullable
   static Configuration getCurrentConfiguration() {
     return CURRENT_CONFIGURATION.get();
   }
 
-  /**
-   * 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) {
-      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 ThreadPoolJobManager threadPoolJobManager) {
+      final int connectionRequestTimeout,
+      final int connectTimeout,
+      final int socketTimeout) {
     final RequestConfig requestConfig =
         RequestConfig.custom()
-            .setConnectionRequestTimeout(
-                getTimeoutValue("http.connectionRequestTimeout", threadPoolJobManager))
-            .setConnectTimeout(getTimeoutValue("http.connectTimeout", threadPoolJobManager))
-            .setSocketTimeout(getTimeoutValue("http.socketTimeout", threadPoolJobManager))
+            .setConnectionRequestTimeout(connectionRequestTimeout)
+            .setConnectTimeout(connectTimeout)
+            .setSocketTimeout(socketTimeout)
             .build();
 
     final HttpClientBuilder httpClientBuilder =
@@ -118,9 +105,9 @@ private static CloseableHttpClient createHttpClient(
     LOGGER.debug(
         "Created CloseableHttpClient using connectionRequestTimeout: {} connectTimeout: {}"
             + " socketTimeout: {}",
-        getTimeoutValue("http.connectionRequestTimeout", threadPoolJobManager),
-        getTimeoutValue("http.connectTimeout", threadPoolJobManager),
-        getTimeoutValue("http.socketTimeout", threadPoolJobManager));
+        connectionRequestTimeout,
+        connectTimeout,
+        socketTimeout);
     return closeableHttpClient;
   }
 

core/src/main/java/org/mapfish/print/map/geotools/grid/LinearCoordinateSequence.java

@@ -192,6 +192,7 @@ private boolean ordinate0IsY() {
    *
    * @deprecated Recommend {@link #copy()}
    */
+  @Deprecated
   @Override
   public Object clone() {
     return copy();

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

@@ -43,13 +43,13 @@
 
 /** A JobManager backed by a {@link java.util.concurrent.ThreadPoolExecutor}. */
 public class ThreadPoolJobManager implements JobManager {
-  /** Default timeout for the duration of a print job. */
-  public static final long DEFAULT_TIMEOUT_IN_SECONDS = 600L;
+  /** Default timeout in seconds for the duration of a print job. */
+  public static final int DEFAULT_TIMEOUT_IN_SECONDS = 600;
 
   private static final Logger LOGGER = LoggerFactory.getLogger(ThreadPoolJobManager.class);
   private static final int DEFAULT_MAX_WAITING_JOBS = 5000;
   private static final long DEFAULT_THREAD_IDLE_TIME = 60L;
-  private static final long DEFAULT_ABANDONED_TIMEOUT_IN_SECONDS = 120L;
+  private static final int DEFAULT_ABANDONED_TIMEOUT_IN_SECONDS = 120;
   private static final boolean DEFAULT_OLD_FILES_CLEAN_UP = true;
   private static final long DEFAULT_CLEAN_UP_INTERVAL_IN_SECONDS = 86400;
 
@@ -73,17 +73,17 @@ public class ThreadPoolJobManager implements JobManager {
    */
   private int maxNumberOfWaitingJobs = DEFAULT_MAX_WAITING_JOBS;
 
-  /** The amount of time to let a thread wait before being shutdown. */
+  /** The number of seconds to let a thread wait before being shutdown. */
   private final long maxIdleTime = DEFAULT_THREAD_IDLE_TIME;
 
   /** A print job is canceled, if it is not completed after this amount of time (in seconds). */
-  private long timeout = DEFAULT_TIMEOUT_IN_SECONDS;
+  private int timeout = DEFAULT_TIMEOUT_IN_SECONDS;
 
   /**
    * A print job is canceled, if this amount of time (in seconds) has passed, without that the user
    * checked the status of the job.
    */
-  private long abandonedTimeout = DEFAULT_ABANDONED_TIMEOUT_IN_SECONDS;
+  private int abandonedTimeout = DEFAULT_ABANDONED_TIMEOUT_IN_SECONDS;
 
   /** Delete old report files? */
   private boolean oldFileCleanUp = DEFAULT_OLD_FILES_CLEAN_UP;
@@ -101,7 +101,7 @@ public class ThreadPoolJobManager implements JobManager {
    * A comparator for comparing {@link org.mapfish.print.servlet.job.impl.SubmittedPrintJob}s and
    * prioritizing them.
    *
-   * <p>For example it could be that requests from certain users (like executive officers) are
+   * <p>For example, it could be that requests from certain users (like executive officers) are
    * prioritized over requests from other users.
    */
   private Comparator<PrintJob> jobPriorityComparator =
@@ -132,15 +132,15 @@ public final void setMaxNumberOfWaitingJobs(final int maxNumberOfWaitingJobs) {
     this.maxNumberOfWaitingJobs = maxNumberOfWaitingJobs;
   }
 
-  public final void setTimeout(final long timeout) {
+  public final void setTimeout(final int timeout) {
     this.timeout = timeout;
   }
 
-  public final long getTimeout() {
+  public final int getTimeout() {
     return this.timeout;
   }
 
-  public final void setAbandonedTimeout(final long abandonedTimeout) {
+  public final void setAbandonedTimeout(final int abandonedTimeout) {
     this.abandonedTimeout = abandonedTimeout;
   }
 
@@ -426,8 +426,8 @@ public final PrintJobStatus getStatus(final String referenceId) throws NoSuchRef
   private void cancelOld() {
     // cancel old tasks
     this.jobQueue.cancelOld(
-        TimeUnit.MILLISECONDS.convert(this.timeout, TimeUnit.SECONDS),
-        TimeUnit.MILLISECONDS.convert(this.abandonedTimeout, TimeUnit.SECONDS),
+        TimeUnit.SECONDS.toMillis(this.timeout),
+        TimeUnit.SECONDS.toMillis(this.abandonedTimeout),
         "task canceled (timeout)");
   }
 

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

@@ -63,7 +63,9 @@
     <bean id="httpClientFactory" class="org.mapfish.print.http.MfClientHttpRequestFactoryImpl">
         <constructor-arg index="0" value="${maxConnectionsTotal}" />
         <constructor-arg index="1" value="${maxConnectionsPerRoute}"/>
-        <constructor-arg index="2" ref="jobManager"/>
+        <constructor-arg index="2" value="${http.connectionRequestTimeout}"/>
+        <constructor-arg index="3" value="${http.connectTimeout}"/>
+        <constructor-arg index="4" value="${http.socketTimeout}"/>
     </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/main/resources/mapfish-spring.properties

@@ -56,5 +56,19 @@ httpRequest.fetchRetry.intervalMillis=100
 # Amount of time in the past where we check if a print job was executed by this server
 healthStatus.expectedMaxTime.sinceLastPrint.InSeconds=300
 
-# Maximum number of Print Jobs queued before raising it i
+# Maximum number of Print Jobs queued before raising it is an issue
 healthStatus.unhealthyThreshold.maxNbrPrintJobQueued=4
+
+# Number of milliseconds used when requesting a connection from the connection manager.
+# Recommended 2s for interactive application
+# Recommended 10s for batch application
+http.connectionRequestTimeout=10000
+
+# Number of milliseconds until a connection is established.
+# Recommended 5s for applications in general
+# Recommended 10s for tolerant application with slow connection
+http.connectTimeout=10000
+
+# Maximum number of milliseconds during which a socket remains inactive between two consecutive data packets.
+# Using 5 minutes by default to support very slow and large downloads
+http.socketTimeout=300000

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

@@ -12,7 +12,6 @@
 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;
@@ -27,7 +26,7 @@ public class TestHttpClientFactory extends MfClientHttpRequestFactoryImpl
   private final Map<Predicate<URI>, Handler> handlers = new ConcurrentHashMap<>();
 
   public TestHttpClientFactory() {
-    super(20, 10, new ThreadPoolJobManager());
+    super(20, 10, 1000, 1000, 1000);
   }
 
   public void registerHandler(Predicate<URI> matcher, Handler handler) {

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

@@ -9,7 +9,6 @@
 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;
 
@@ -41,7 +40,7 @@ public void testGetHeaders() throws Exception {
         });
 
     MfClientHttpRequestFactoryImpl factory =
-        new MfClientHttpRequestFactoryImpl(20, 10, new ThreadPoolJobManager());
+        new MfClientHttpRequestFactoryImpl(20, 10, 1000, 1000, 1000);
     final ConfigurableRequest request =
         factory.createRequest(
             new URI("http://" + HttpProxyTest.LOCALHOST + ":" + TARGET_PORT + "/request"),

core/src/test/java/org/mapfish/print/servlet/MapPrinterServletTest.java

@@ -664,7 +664,7 @@ public void testCancelSpecificAppId() throws Exception {
   @Test(timeout = 60000)
   @DirtiesContext
   public void testCreateReport_Timeout() throws Exception {
-    jobManager.setTimeout(1L);
+    jobManager.setTimeout(1);
     setUpConfigFiles();
 
     final MockHttpServletRequest servletCreateRequest = new MockHttpServletRequest();
@@ -687,14 +687,14 @@ public void testCreateReport_Timeout() throws Exception {
 
     // now after canceling a task, check that the system is left in a state in which
     // it still can process jobs
-    jobManager.setTimeout(600L);
+    jobManager.setTimeout(600);
     testCreateReport_Success_explicitAppId();
   }
 
   @Test(timeout = 60000)
   @DirtiesContext
   public void testCreateReport_AbandonedTimeout() throws Exception {
-    jobManager.setAbandonedTimeout(1L);
+    jobManager.setAbandonedTimeout(1);
     setUpConfigFiles();
 
     final MockHttpServletRequest servletCreateRequest = new MockHttpServletRequest();
@@ -731,7 +731,7 @@ public void testCreateReport_AbandonedTimeout() throws Exception {
   @Test(timeout = 60000)
   @DirtiesContext
   public void testCreateReport_NotAbandonedTimeout() throws Exception {
-    jobManager.setAbandonedTimeout(1L);
+    jobManager.setAbandonedTimeout(1);
     setUpConfigFiles();
 
     final MockHttpServletRequest servletCreateRequest = new MockHttpServletRequest();
@@ -878,7 +878,7 @@ private String getFormEncodedRequestData() throws IOException {
   @Test(timeout = 60000)
   @DirtiesContext
   public void testCreateReportAndGet_Timeout() throws Exception {
-    jobManager.setTimeout(1L);
+    jobManager.setTimeout(1);
     setUpConfigFiles();
 
     final MockHttpServletRequest servletCreateRequest = new MockHttpServletRequest();

core/src/test/resources/org/mapfish/print/processor/http/map-uri/map-uri-228-bug-fix-processor-application-context.xml

@@ -1,13 +1,14 @@
 <?xml version="1.0" encoding="UTF-8"?>
 
 <beans xmlns="http://www.springframework.org/schema/beans" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-    xmlns:util="http://www.springframework.org/schema/util"
     xsi:schemaLocation="
-    http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.0.xsd
-    http://www.springframework.org/schema/util http://www.springframework.org/schema/util/spring-util-3.0.xsd">
+    http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.0.xsd">
 
   <bean id="httpClientFactory" class="org.mapfish.print.http.MfClientHttpRequestFactoryImpl">
     <constructor-arg index="0" value="20" />
     <constructor-arg index="1" value="10" />
+    <constructor-arg index="2" value="1000" />
+    <constructor-arg index="3" value="1000" />
+    <constructor-arg index="4" value="1000" />
   </bean>
 </beans>

examples/build.gradle

@@ -14,14 +14,14 @@ repositories {
 dependencies {
     testImplementation(
         project(':core'),
-        'commons-io:commons-io:2.11.0',
-        "org.springframework:spring-web:5.3.27",
-        'org.json:json:20230227',
+        'commons-io:commons-io:2.17.0',
+        "org.springframework:spring-web:5.3.39",
+        'org.json:json:20240303',
         'org.apache.commons:commons-lang3:3.5',
         "org.locationtech.jts:jts-core:1.18.2",
-        "org.slf4j:slf4j-api:2.0.7",
-        "org.springframework:spring-test:5.3.27",
-        "ch.qos.logback:logback-classic:1.4.7",
+        "org.slf4j:slf4j-api:2.0.16",
+        "org.springframework:spring-test:5.3.39",
+        "ch.qos.logback:logback-classic:1.5.15",
         "org.verapdf:validation-model:1.24.1"
     )
 }