From 25a4d09c076a14e14bf47a418ac11a30ffbb13df Mon Sep 17 00:00:00 2001 From: sebr72 Date: Thu, 19 Dec 2024 16:58:52 +0100 Subject: [PATCH] Set MfClientHttpRequestFactoryImpl timeouts to automatic cancel timeout. --- ...ConfigFileResolvingHttpRequestFactory.java | 3 +- .../http/ConfigFileResolvingRequest.java | 7 ++- .../http/MfClientHttpRequestFactoryImpl.java | 47 ++++++++++++++----- .../job/impl/ThreadPoolJobManager.java | 4 ++ .../mapfish-spring-application-context.xml | 3 +- .../mapfish/print/TestHttpClientFactory.java | 3 +- ...> MfClientHttpRequestFactoryImplTest.java} | 28 +++++------ .../org/mapfish/print/AbstractApiTest.java | 3 +- 8 files changed, 66 insertions(+), 32 deletions(-) rename core/src/test/java/org/mapfish/print/http/{MfClientHttpRequestFactoryImpl_ResponseTest.java => MfClientHttpRequestFactoryImplTest.java} (57%) diff --git a/core/src/main/java/org/mapfish/print/http/ConfigFileResolvingHttpRequestFactory.java b/core/src/main/java/org/mapfish/print/http/ConfigFileResolvingHttpRequestFactory.java index 099ad2e8c4..737633d620 100644 --- a/core/src/main/java/org/mapfish/print/http/ConfigFileResolvingHttpRequestFactory.java +++ b/core/src/main/java/org/mapfish/print/http/ConfigFileResolvingHttpRequestFactory.java @@ -14,7 +14,7 @@ * 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; @@ -22,6 +22,7 @@ public final class ConfigFileResolvingHttpRequestFactory implements MfClientHttp private final MfClientHttpRequestFactoryImpl httpRequestFactory; private final List 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; diff --git a/core/src/main/java/org/mapfish/print/http/ConfigFileResolvingRequest.java b/core/src/main/java/org/mapfish/print/http/ConfigFileResolvingRequest.java index 9754e009bc..a1ae107db2 100644 --- a/core/src/main/java/org/mapfish/print/http/ConfigFileResolvingRequest.java +++ b/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() { diff --git a/core/src/main/java/org/mapfish/print/http/MfClientHttpRequestFactoryImpl.java b/core/src/main/java/org/mapfish/print/http/MfClientHttpRequestFactoryImpl.java index bb865d8160..0cbafd288f 100644 --- a/core/src/main/java/org/mapfish/print/http/MfClientHttpRequestFactoryImpl.java +++ b/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,8 +54,11 @@ 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 @@ -61,21 +66,41 @@ 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; } diff --git a/core/src/main/java/org/mapfish/print/servlet/job/impl/ThreadPoolJobManager.java b/core/src/main/java/org/mapfish/print/servlet/job/impl/ThreadPoolJobManager.java index b5863fdffb..e91d531478 100644 --- a/core/src/main/java/org/mapfish/print/servlet/job/impl/ThreadPoolJobManager.java +++ b/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; } diff --git a/core/src/main/resources/mapfish-spring-application-context.xml b/core/src/main/resources/mapfish-spring-application-context.xml index aaf2d532dd..2a42ddc332 100644 --- a/core/src/main/resources/mapfish-spring-application-context.xml +++ b/core/src/main/resources/mapfish-spring-application-context.xml @@ -62,7 +62,8 @@ - + + diff --git a/core/src/test/java/org/mapfish/print/TestHttpClientFactory.java b/core/src/test/java/org/mapfish/print/TestHttpClientFactory.java index 7576293382..db8cf6e245 100644 --- a/core/src/test/java/org/mapfish/print/TestHttpClientFactory.java +++ b/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, Handler> handlers = new ConcurrentHashMap<>(); public TestHttpClientFactory() { - super(20, 10); + super(20, 10, new ThreadPoolJobManager()); } public void registerHandler(Predicate matcher, Handler handler) { diff --git a/core/src/test/java/org/mapfish/print/http/MfClientHttpRequestFactoryImpl_ResponseTest.java b/core/src/test/java/org/mapfish/print/http/MfClientHttpRequestFactoryImplTest.java similarity index 57% rename from core/src/test/java/org/mapfish/print/http/MfClientHttpRequestFactoryImpl_ResponseTest.java rename to core/src/test/java/org/mapfish/print/http/MfClientHttpRequestFactoryImplTest.java index eeb312eede..bfadcb69d7 100644 --- a/core/src/test/java/org/mapfish/print/http/MfClientHttpRequestFactoryImpl_ResponseTest.java +++ b/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")); + } } } diff --git a/examples/src/test/java/org/mapfish/print/AbstractApiTest.java b/examples/src/test/java/org/mapfish/print/AbstractApiTest.java index 7601a2b5ef..558ee4b5d0 100644 --- a/examples/src/test/java/org/mapfish/print/AbstractApiTest.java +++ b/examples/src/test/java/org/mapfish/print/AbstractApiTest.java @@ -10,6 +10,7 @@ import java.util.Map; import org.apache.commons.io.IOUtils; import org.mapfish.print.http.MfClientHttpRequestFactoryImpl; +import org.mapfish.print.servlet.job.impl.ThreadPoolJobManager; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; import org.springframework.http.client.ClientHttpRequest; @@ -21,7 +22,7 @@ public abstract class AbstractApiTest { protected static final String PRINT_SERVER = "http://print:8080/"; protected ClientHttpRequestFactory httpRequestFactory = - new MfClientHttpRequestFactoryImpl(10, 10); + new MfClientHttpRequestFactoryImpl(10, 10, new ThreadPoolJobManager()); protected ClientHttpRequest getRequest(String path, HttpMethod method) throws IOException, URISyntaxException {