From 323aed7eb8a8b0518f9afe88252732b33b3f10b0 Mon Sep 17 00:00:00 2001 From: Jonathan Miller Date: Tue, 23 Jun 2026 15:18:28 -0400 Subject: [PATCH] MLE-30245 read/write timeout support to DatabaseClient Expose withReadTimeout() and withWriteTimeout() on DatabaseClientBuilder so callers can configure per-client OkHttp read and write timeouts without relying on global defaults. --- .../client/DatabaseClientBuilder.java | 52 +++++++- .../client/DatabaseClientFactory.java | 98 +++++++++++++-- .../impl/DatabaseClientPropertySource.java | 21 +++- .../marklogic/client/impl/OkHttpServices.java | 10 +- .../DatabaseClientPropertySourceTest.java | 32 ++++- .../client/impl/okhttp/OkHttpTimeoutTest.java | 118 ++++++++++++++++++ .../test/DatabaseClientBuilderTest.java | 25 +++- 7 files changed, 339 insertions(+), 17 deletions(-) create mode 100644 marklogic-client-api/src/test/java/com/marklogic/client/impl/okhttp/OkHttpTimeoutTest.java diff --git a/marklogic-client-api/src/main/java/com/marklogic/client/DatabaseClientBuilder.java b/marklogic-client-api/src/main/java/com/marklogic/client/DatabaseClientBuilder.java index a39d00802..dd2834664 100644 --- a/marklogic-client-api/src/main/java/com/marklogic/client/DatabaseClientBuilder.java +++ b/marklogic-client-api/src/main/java/com/marklogic/client/DatabaseClientBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2025 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved. + * Copyright (c) 2010-2026 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved. */ package com.marklogic.client; @@ -10,6 +10,8 @@ import javax.net.ssl.X509TrustManager; import java.util.HashMap; import java.util.Map; +import java.util.Objects; +import java.util.concurrent.TimeUnit; import java.util.function.Function; /** @@ -379,4 +381,52 @@ public DatabaseClientBuilder withTrustStoreAlgorithm(String algorithm) { props.put(PREFIX + "ssl.truststore.algorithm", algorithm); return this; } + + /** + * Sets the read timeout for HTTP requests made by the underlying OkHttp client. + * + *

By default, the read timeout is zero (no timeout), which is intentional for use cases + * that transfer large amounts of data (e.g., bulk exports or large document reads). Setting + * a non-zero timeout is recommended for applications that do not expect large responses, as + * an indefinitely blocked read thread can exhaust thread pools if the MarkLogic server + * becomes unresponsive.

+ * + * @param value the timeout duration; use 0 to disable the timeout + * @param unit the time unit of the duration + * @return this builder + * @since 8.2.0 + */ + public DatabaseClientBuilder withReadTimeout(long value, TimeUnit unit) { + Objects.requireNonNull(unit, "unit must not be null"); + if (value < 0) { + throw new IllegalArgumentException( + "Timeout value must be zero (no timeout) or a positive duration, but was: " + value); + } + props.put(PREFIX + "readTimeoutMillis", unit.toMillis(value)); + return this; + } + + /** + * Sets the write timeout for HTTP requests made by the underlying OkHttp client. + * + *

By default, the write timeout is zero (no timeout), which is intentional for use cases + * that transfer large amounts of data (e.g., bulk imports or large document writes). Setting + * a non-zero timeout is recommended for applications that do not expect large request bodies, + * as an indefinitely blocked write thread can exhaust thread pools if the MarkLogic server + * becomes unresponsive.

+ * + * @param value the timeout duration; use 0 to disable the timeout + * @param unit the time unit of the duration + * @return this builder + * @since 8.2.0 + */ + public DatabaseClientBuilder withWriteTimeout(long value, TimeUnit unit) { + Objects.requireNonNull(unit, "unit must not be null"); + if (value < 0) { + throw new IllegalArgumentException( + "Timeout value must be zero (no timeout) or a positive duration, but was: " + value); + } + props.put(PREFIX + "writeTimeoutMillis", unit.toMillis(value)); + return this; + } } diff --git a/marklogic-client-api/src/main/java/com/marklogic/client/DatabaseClientFactory.java b/marklogic-client-api/src/main/java/com/marklogic/client/DatabaseClientFactory.java index 63e4cd212..1e4df5029 100644 --- a/marklogic-client-api/src/main/java/com/marklogic/client/DatabaseClientFactory.java +++ b/marklogic-client-api/src/main/java/com/marklogic/client/DatabaseClientFactory.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2025 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved. + * Copyright (c) 2010-2026 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved. */ package com.marklogic.client; @@ -1265,21 +1265,70 @@ static public DatabaseClient newClient(String host, int port, String database, static public DatabaseClient newClient(String host, int port, String basePath, String database, SecurityContext securityContext, DatabaseClient.ConnectionType connectionType) { - // As of 6.1.0, the following optimization is made as it's guaranteed that if the user is connecting to a - // Progress Data Cloud instance, then port 443 will be used. Every path for constructing a DatabaseClient goes through - // this method, ensuring that this optimization will always be applied, and thus freeing the user from having to - // worry about what port to configure when using Progress Data Cloud. + return newClient(host, port, basePath, database, securityContext, connectionType, 0L, 0L); + } + + /** + * Creates a client with configurable read and write timeouts for HTTP requests. + * + *

As of 6.1.0, the Progress Data Cloud port optimization is applied here: port 443 is used automatically + * when connecting to a Progress Data Cloud instance.

+ * + * @param host the MarkLogic host + * @param port the REST server port + * @param basePath optional base path + * @param database optional database name + * @param securityContext the security context + * @param connectionType whether the client connects directly or via a gateway + * @param readTimeoutMillis read timeout in milliseconds; 0 means no timeout + * @param writeTimeoutMillis write timeout in milliseconds; 0 means no timeout + * @return a new client for making database requests + * @since 8.2.0 + */ + static public DatabaseClient newClient(String host, int port, String basePath, String database, + SecurityContext securityContext, + DatabaseClient.ConnectionType connectionType, + long readTimeoutMillis, long writeTimeoutMillis) { + // As of 6.1.0, port 443 is forced for Progress Data Cloud connections, freeing the caller + // from having to specify the port when using Progress Data Cloud. if (securityContext instanceof ProgressDataCloudAuthContext) { port = 443; } - - OkHttpServices.ConnectionConfig config = new OkHttpServices.ConnectionConfig(host, port, basePath, database, securityContext, clientConfigurators); + OkHttpServices.ConnectionConfig config = new OkHttpServices.ConnectionConfig(host, port, basePath, database, securityContext, clientConfigurators, readTimeoutMillis, writeTimeoutMillis); RESTServices services = new OkHttpServices(config); DatabaseClientImpl client = new DatabaseClientImpl(services, host, port, basePath, database, securityContext, connectionType); client.setHandleRegistry(getHandleRegistry().copy()); return client; } + /** + * Creates a client based on the given bean's properties, including any configured read and write timeouts. + * Unlike the other {@code newClient} overloads, this method uses the bean's own handle registry rather than + * the global {@link #getHandleRegistry()} registry. + * + * @param bean the bean describing the connection and timeout configuration + * @return a new client for making database requests + * @since 8.2.0 + */ + static public DatabaseClient newClient(Bean bean) { + String host = bean.getHost(); + int port = bean.getPort(); + SecurityContext securityContext = bean.getSecurityContext(); + // Port 443 is forced for Progress Data Cloud connections, freeing the caller + // from having to specify the port when using Progress Data Cloud. + if (securityContext instanceof ProgressDataCloudAuthContext) { + port = 443; + } + OkHttpServices.ConnectionConfig config = new OkHttpServices.ConnectionConfig( + host, port, bean.getBasePath(), bean.getDatabase(), securityContext, clientConfigurators, + bean.getReadTimeoutMillis(), bean.getWriteTimeoutMillis()); + RESTServices services = new OkHttpServices(config); + DatabaseClientImpl client = new DatabaseClientImpl(services, host, port, bean.getBasePath(), + bean.getDatabase(), securityContext, bean.getConnectionType()); + client.setHandleRegistry(bean.getHandleRegistry().copy()); + return client; + } + /** * Returns the default registry with factories for creating handles * as adapters for IO representations. To create custom registries, @@ -1350,6 +1399,8 @@ static public class Bean implements Serializable { transient private SecurityContext securityContext; transient private HandleFactoryRegistry handleRegistry = HandleFactoryRegistryImpl.newDefault(); + private long readTimeoutMillis = 0; + private long writeTimeoutMillis = 0; /** * Zero-argument constructor for bean applications. Other @@ -1502,10 +1553,35 @@ public void registerDefaultHandles() { * @return a new client for making database requests */ public DatabaseClient newClient() { - DatabaseClientImpl client = (DatabaseClientImpl) DatabaseClientFactory.newClient( - host, port, basePath, database, securityContext, connectionType); - client.setHandleRegistry(getHandleRegistry().copy()); - return client; + return DatabaseClientFactory.newClient(this); } + + /** + * Returns the read timeout in milliseconds applied to HTTP requests. Zero means no timeout (the default). + * @return the read timeout in milliseconds + * @since 8.2.0 + */ + public long getReadTimeoutMillis() { return readTimeoutMillis; } + + /** + * Sets the read timeout for HTTP requests in milliseconds. Zero means no timeout (the default). + * @param readTimeoutMillis the read timeout in milliseconds + * @since 8.2.0 + */ + public void setReadTimeoutMillis(long readTimeoutMillis) { this.readTimeoutMillis = readTimeoutMillis; } + + /** + * Returns the write timeout in milliseconds applied to HTTP requests. Zero means no timeout (the default). + * @return the write timeout in milliseconds + * @since 8.2.0 + */ + public long getWriteTimeoutMillis() { return writeTimeoutMillis; } + + /** + * Sets the write timeout for HTTP requests in milliseconds. Zero means no timeout (the default). + * @param writeTimeoutMillis the write timeout in milliseconds + * @since 8.2.0 + */ + public void setWriteTimeoutMillis(long writeTimeoutMillis) { this.writeTimeoutMillis = writeTimeoutMillis; } } } diff --git a/marklogic-client-api/src/main/java/com/marklogic/client/impl/DatabaseClientPropertySource.java b/marklogic-client-api/src/main/java/com/marklogic/client/impl/DatabaseClientPropertySource.java index ceffa379a..d16ba1e37 100644 --- a/marklogic-client-api/src/main/java/com/marklogic/client/impl/DatabaseClientPropertySource.java +++ b/marklogic-client-api/src/main/java/com/marklogic/client/impl/DatabaseClientPropertySource.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2025 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved. + * Copyright (c) 2010-2026 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved. */ package com.marklogic.client.impl; @@ -105,6 +105,10 @@ public class DatabaseClientPropertySource { DatabaseClientFactory.addConfigurator(new RemoveAcceptEncodingConfigurator()); } }); + connectionPropertyHandlers.put(PREFIX + "readTimeoutMillis", (bean, value) -> + bean.setReadTimeoutMillis(toLong(value))); + connectionPropertyHandlers.put(PREFIX + "writeTimeoutMillis", (bean, value) -> + bean.setWriteTimeoutMillis(toLong(value))); } public DatabaseClientPropertySource(Function propertySource) { @@ -122,7 +126,7 @@ public DatabaseClient newClient() { // DatabaseClientFactory.getHandleRegistry() will still impact the DatabaseClient returned by this method // (and this behavior is expected by some existing tests). return DatabaseClientFactory.newClient(bean.getHost(), bean.getPort(), bean.getBasePath(), bean.getDatabase(), - bean.getSecurityContext(), bean.getConnectionType()); + bean.getSecurityContext(), bean.getConnectionType(), bean.getReadTimeoutMillis(), bean.getWriteTimeoutMillis()); } /** @@ -455,4 +459,17 @@ private SSLUtil.SSLInputs useNewSSLContext(String sslProtocol, X509TrustManager } return new SSLUtil.SSLInputs(sslContext, userTrustManager); } + + private static long toLong(Object value) { + long millis; + if (value instanceof Long) millis = (Long) value; + else if (value instanceof Integer) millis = ((Integer) value).longValue(); + else if (value instanceof String) millis = Long.parseLong((String) value); + else throw new IllegalArgumentException("Timeout value must be a Long, Integer, or String; got: " + value.getClass()); + if (millis < 0) { + throw new IllegalArgumentException( + "Timeout value must be zero (no timeout) or a positive duration, but was: " + millis); + } + return millis; + } } diff --git a/marklogic-client-api/src/main/java/com/marklogic/client/impl/OkHttpServices.java b/marklogic-client-api/src/main/java/com/marklogic/client/impl/OkHttpServices.java index f3cf56ba9..8ded86c9f 100644 --- a/marklogic-client-api/src/main/java/com/marklogic/client/impl/OkHttpServices.java +++ b/marklogic-client-api/src/main/java/com/marklogic/client/impl/OkHttpServices.java @@ -151,7 +151,8 @@ static protected class ThreadState { ThreadLocal.withInitial(() -> new ThreadState(useDigestAuthPing)); public record ConnectionConfig(String host, int port, String basePath, String database, - SecurityContext securityContext, List clientConfigurators) { + SecurityContext securityContext, List clientConfigurators, + long readTimeoutMillis, long writeTimeoutMillis) { } public OkHttpServices(ConnectionConfig connectionConfig) { @@ -220,6 +221,13 @@ private OkHttpClient connect(ConnectionConfig config) { OkHttpClient.Builder clientBuilder = OkHttpUtil.newOkHttpClientBuilder(config.host, config.securityContext, config.clientConfigurators); + if (config.readTimeoutMillis() > 0) { + clientBuilder.readTimeout(config.readTimeoutMillis(), TimeUnit.MILLISECONDS); + } + if (config.writeTimeoutMillis() > 0) { + clientBuilder.writeTimeout(config.writeTimeoutMillis(), TimeUnit.MILLISECONDS); + } + Properties props = System.getProperties(); if (props.containsKey(OKHTTP_LOGGINGINTERCEPTOR_LEVEL)) { configureOkHttpLogging(clientBuilder, props); diff --git a/marklogic-client-api/src/test/java/com/marklogic/client/impl/DatabaseClientPropertySourceTest.java b/marklogic-client-api/src/test/java/com/marklogic/client/impl/DatabaseClientPropertySourceTest.java index 2c74ed783..ba0d52e42 100644 --- a/marklogic-client-api/src/test/java/com/marklogic/client/impl/DatabaseClientPropertySourceTest.java +++ b/marklogic-client-api/src/test/java/com/marklogic/client/impl/DatabaseClientPropertySourceTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2025 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved. + * Copyright (c) 2010-2026 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved. */ package com.marklogic.client.impl; @@ -216,4 +216,34 @@ private DatabaseClientFactory.Bean buildBean() { DatabaseClientPropertySource source = new DatabaseClientPropertySource(propertyName -> props.get(propertyName)); return source.newClientBean(); } + + @Test + void readAndWriteTimeoutsFromPropertySource() { + props.put(PREFIX + "readTimeoutMillis", 5000L); + props.put(PREFIX + "writeTimeoutMillis", 10000L); + DatabaseClientFactory.Bean bean = buildBean(); + + assertEquals(5000L, bean.getReadTimeoutMillis()); + assertEquals(10000L, bean.getWriteTimeoutMillis()); + } + + @Test + void readAndWriteTimeoutsFromPropertySourceAsStrings() { + props.put(PREFIX + "readTimeoutMillis", "15000"); + props.put(PREFIX + "writeTimeoutMillis", "30000"); + DatabaseClientFactory.Bean bean = buildBean(); + + assertEquals(15000L, bean.getReadTimeoutMillis()); + assertEquals(30000L, bean.getWriteTimeoutMillis()); + } + + @Test + void readAndWriteTimeoutsFromPropertySourceAsIntegers() { + props.put(PREFIX + "readTimeoutMillis", 5000); // Integer, not Long + props.put(PREFIX + "writeTimeoutMillis", 10000); + DatabaseClientFactory.Bean bean = buildBean(); + + assertEquals(5000L, bean.getReadTimeoutMillis()); + assertEquals(10000L, bean.getWriteTimeoutMillis()); + } } diff --git a/marklogic-client-api/src/test/java/com/marklogic/client/impl/okhttp/OkHttpTimeoutTest.java b/marklogic-client-api/src/test/java/com/marklogic/client/impl/okhttp/OkHttpTimeoutTest.java new file mode 100644 index 000000000..1d6fa0415 --- /dev/null +++ b/marklogic-client-api/src/test/java/com/marklogic/client/impl/okhttp/OkHttpTimeoutTest.java @@ -0,0 +1,118 @@ +/* + * Copyright (c) 2010-2026 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved. + */ +package com.marklogic.client.impl.okhttp; + +import com.marklogic.client.DatabaseClientFactory; +import com.marklogic.client.impl.OkHttpServices; +import mockwebserver3.MockResponse; +import mockwebserver3.MockWebServer; +import okhttp3.OkHttpClient; +import okhttp3.Request; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.net.SocketTimeoutException; +import java.util.Collections; +import java.util.concurrent.TimeUnit; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * Verifies that read and write timeouts configured via {@link OkHttpServices.ConnectionConfig} are applied to the + * underlying {@link OkHttpClient}. + */ +class OkHttpTimeoutTest { + + private MockWebServer mockWebServer; + + @BeforeEach + void setUp() throws IOException { + mockWebServer = new MockWebServer(); + mockWebServer.start(); + } + + @AfterEach + void tearDown() { + mockWebServer.close(); + } + + /** + * Verifies that a configured read timeout actually fires when the server stalls before sending + * any response. This is an end-to-end behavioral test: if the timeout were not wired to the + * OkHttpClient, the call would block indefinitely rather than throwing a SocketTimeoutException. + */ + @Test + void readTimeoutFiresWhenServerStalls() throws Exception { + // Use a very short read timeout (100 ms) to keep the test fast. + DatabaseClientFactory.DigestAuthContext securityContext = + new DatabaseClientFactory.DigestAuthContext("user", "password"); + OkHttpServices.ConnectionConfig config = new OkHttpServices.ConnectionConfig( + mockWebServer.getHostName(), mockWebServer.getPort(), null, null, + securityContext, Collections.emptyList(), 100L, 0L); + + OkHttpServices services = new OkHttpServices(config); + + OkHttpClient okHttpClient = services.getClientImplementation(); + + // Server stalls for 5 seconds before sending headers. The read timeout is 100 ms, so the + // test completes in ~100 ms when passing. The 50× margin eliminates the risk of a GC pause + // causing MockWebServer to respond before the timeout fires. + mockWebServer.enqueue(new MockResponse.Builder() + .headersDelay(5_000, TimeUnit.MILLISECONDS) + .code(200) + .body("ok") + .build()); + + Request request = new Request.Builder() + .url(mockWebServer.url("/")) + .build(); + + // The read timeout must fire, producing a SocketTimeoutException (subtype of IOException). + assertThrows(SocketTimeoutException.class, + () -> okHttpClient.newCall(request).execute().close(), + "Expected a SocketTimeoutException when the server does not respond within the configured timeout"); + } + + @Test + void readAndWriteTimeoutsAreApplied() throws Exception { + DatabaseClientFactory.DigestAuthContext securityContext = + new DatabaseClientFactory.DigestAuthContext("user", "password"); + + OkHttpServices.ConnectionConfig config = new OkHttpServices.ConnectionConfig( + mockWebServer.getHostName(), mockWebServer.getPort(), null, null, + securityContext, Collections.emptyList(), + TimeUnit.SECONDS.toMillis(30), TimeUnit.MINUTES.toMillis(2)); + + OkHttpServices services = new OkHttpServices(config); + + OkHttpClient okHttpClient = services.getClientImplementation(); + + assertEquals(30_000, okHttpClient.readTimeoutMillis(), + "Read timeout should be 30 seconds expressed in milliseconds"); + assertEquals(120_000, okHttpClient.writeTimeoutMillis(), + "Write timeout should be 2 minutes expressed in milliseconds"); + } + + @Test + void defaultTimeoutsRemainZero() throws Exception { + DatabaseClientFactory.DigestAuthContext securityContext = + new DatabaseClientFactory.DigestAuthContext("user", "password"); + + OkHttpServices.ConnectionConfig config = new OkHttpServices.ConnectionConfig( + mockWebServer.getHostName(), mockWebServer.getPort(), null, null, + securityContext, Collections.emptyList(), 0L, 0L); + + OkHttpServices services = new OkHttpServices(config); + + OkHttpClient okHttpClient = services.getClientImplementation(); + + assertEquals(0, okHttpClient.readTimeoutMillis(), + "Default read timeout should be 0 (no timeout), preserving existing behaviour"); + assertEquals(0, okHttpClient.writeTimeoutMillis(), + "Default write timeout should be 0 (no timeout), preserving existing behaviour"); + } +} diff --git a/marklogic-client-api/src/test/java/com/marklogic/client/test/DatabaseClientBuilderTest.java b/marklogic-client-api/src/test/java/com/marklogic/client/test/DatabaseClientBuilderTest.java index 44d77c3cf..94a5f707c 100644 --- a/marklogic-client-api/src/test/java/com/marklogic/client/test/DatabaseClientBuilderTest.java +++ b/marklogic-client-api/src/test/java/com/marklogic/client/test/DatabaseClientBuilderTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2025 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved. + * Copyright (c) 2010-2026 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved. */ package com.marklogic.client.test; @@ -15,6 +15,7 @@ import javax.net.ssl.SSLContext; import javax.net.ssl.X509TrustManager; import java.security.NoSuchAlgorithmException; +import java.util.concurrent.TimeUnit; import static org.junit.jupiter.api.Assertions.*; @@ -424,4 +425,26 @@ void sslProtocolAndTrustManagerAndHostnameVerifier() { assertEquals(Common.TRUST_ALL_MANAGER, context.getTrustManager()); assertEquals(DatabaseClientFactory.SSLHostnameVerifier.COMMON, context.getSSLHostnameVerifier()); } + + @Test + void defaultTimeoutsAreZero() { + bean = Common.newClientBuilder() + .withDigestAuth("user", "password") + .buildBean(); + + assertEquals(0L, bean.getReadTimeoutMillis(), "Read timeout should default to 0 (no timeout)"); + assertEquals(0L, bean.getWriteTimeoutMillis(), "Write timeout should default to 0 (no timeout)"); + } + + @Test + void readAndWriteTimeouts() { + bean = Common.newClientBuilder() + .withDigestAuth("user", "password") + .withReadTimeout(30, TimeUnit.SECONDS) + .withWriteTimeout(2, TimeUnit.MINUTES) + .buildBean(); + + assertEquals(30_000L, bean.getReadTimeoutMillis()); + assertEquals(120_000L, bean.getWriteTimeoutMillis()); + } }