Merge pull request #76 from folio-org/FOLS3CL-52

FOLS3CL-52: Expose idle connection timeout http client property

Author: viacheslavkol

src/main/java/org/folio/s3/client/MinioS3Client.java

@@ -47,6 +47,8 @@
 import io.minio.messages.DeleteObject;
 import io.minio.messages.Part;
 import lombok.extern.log4j.Log4j2;
+import okhttp3.ConnectionPool;
+import okhttp3.OkHttpClient;
 
 @Log4j2
 // 2142: we wrap and rethrow InterruptedException as S3ClientException
@@ -90,6 +92,16 @@ static ExtendedMinioAsyncClient createClient(S3ClientProperties properties) {
       builder.region(region);
     }
 
+    var idleKeepAliveSeconds = properties.getIdleKeepAliveSeconds();
+    if (idleKeepAliveSeconds != null) {
+      log.info("Configuring OkHttp connection pool with idle keep-alive of {}s", idleKeepAliveSeconds);
+      // 5 = OkHttp default maxIdleConnections; only the keep-alive duration is customised
+      var httpClient = new OkHttpClient.Builder()
+        .connectionPool(new ConnectionPool(5, idleKeepAliveSeconds, TimeUnit.SECONDS))
+        .build();
+      builder.httpClient(httpClient);
+    }
+
     Provider provider;
     if (StringUtils.isNotBlank(accessKey) && StringUtils.isNotBlank(secretKey)) {
       provider = new StaticProvider(accessKey, secretKey, null);

src/main/java/org/folio/s3/client/S3ClientProperties.java

@@ -46,4 +46,15 @@ public class S3ClientProperties {
    * True for bucket name in the path, false for bucket name in the virtual host name.
    */
   private boolean forcePathStyle;
+
+  /**
+   * Maximum time, in seconds, that an idle HTTP connection in the OkHttp connection pool
+   * (used by the underlying Minio client) is kept alive before being evicted.
+   *
+   * <p>If {@code null}, the OkHttp default of 5 minutes is used. Set this to a value smaller
+   * than the server-side idle timeout (AWS S3 closes idle connections after ~20s) to avoid
+   * "unexpected end of stream" / connection-reset errors, especially during multipart uploads
+   * — which always go through the Minio client, even when {@link AwsS3Client} is used.
+   */
+  private Integer idleKeepAliveSeconds;
 }

src/test/java/org/folio/s3/client/MinioS3ClientHttpClientTest.java

@@ -0,0 +1,92 @@
+package org.folio.s3.client;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.lang.reflect.Field;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+import io.minio.MinioAsyncClient;
+import okhttp3.OkHttpClient;
+
+/**
+ * Unit tests for the OkHttp connection-pool wiring driven by
+ * {@link S3ClientProperties#getIdleKeepAliveSeconds()} in {@link MinioS3Client#createClient}.
+ *
+ * <p>Uses reflection to inspect the {@link OkHttpClient} instance held by the underlying
+ * {@link MinioAsyncClient}, since neither minio-java nor OkHttp expose it publicly.
+ */
+class MinioS3ClientHttpClientTest {
+
+  private static final String ENDPOINT = "http://localhost:9000";
+
+  @Test
+  @DisplayName("Default OkHttp keep-alive (5 minutes) is used when idleKeepAliveSeconds is not set")
+  void defaultKeepAliveWhenPropertyNotSet() throws Exception {
+    var props = baseProps().build();
+
+    var client = MinioS3Client.createClient(props);
+    var okHttp = extractOkHttpClient(client);
+
+    assertNotNull(okHttp, "Minio client must always have an OkHttpClient");
+    // OkHttp default ConnectionPool keep-alive is 5 minutes
+    assertEquals(TimeUnit.MINUTES.toNanos(5), keepAliveNanos(okHttp),
+        "Default OkHttp keep-alive should be 5 minutes when property is null");
+  }
+
+  @Test
+  @DisplayName("Custom OkHttp keep-alive is applied when idleKeepAliveSeconds is set")
+  void customKeepAliveWhenPropertySet() throws Exception {
+    var props = baseProps().idleKeepAliveSeconds(15).build();
+
+    var client = MinioS3Client.createClient(props);
+    var okHttp = extractOkHttpClient(client);
+
+    assertNotNull(okHttp);
+    assertEquals(TimeUnit.SECONDS.toNanos(15), keepAliveNanos(okHttp),
+        "OkHttp keep-alive must match the configured idleKeepAliveSeconds");
+  }
+
+  /**
+   * OkHttp 5 doesn't expose the keep-alive duration on the public {@link okhttp3.ConnectionPool}
+   * API anymore, but {@code getDelegate$okhttp()} returns the {@code RealConnectionPool} which
+   * does expose {@code getKeepAliveDurationNs$okhttp()} (both are JVM-public despite the suffix).
+   */
+  private static long keepAliveNanos(OkHttpClient okHttp) throws Exception {
+    var pool = okHttp.connectionPool();
+    var delegate = pool.getClass().getMethod("getDelegate$okhttp").invoke(pool);
+    return (long) delegate.getClass().getMethod("getKeepAliveDurationNs$okhttp").invoke(delegate);
+  }
+
+  private static S3ClientProperties.S3ClientPropertiesBuilder baseProps() {
+    return S3ClientProperties.builder()
+        .endpoint(ENDPOINT)
+        .region("us-east-1")
+        .bucket("test-bucket")
+        .accessKey("ak")
+        .secretKey("sk")
+        .forcePathStyle(true);
+  }
+
+  /**
+   * Pulls the private {@code httpClient} field from {@link MinioAsyncClient}'s class hierarchy.
+   * The field is declared on {@code io.minio.S3Base} (parent of MinioAsyncClient).
+   */
+  private static OkHttpClient extractOkHttpClient(MinioAsyncClient client) throws IllegalAccessException {
+    Class<?> c = client.getClass();
+    while (c != null) {
+      for (Field f : c.getDeclaredFields()) {
+        if (OkHttpClient.class.isAssignableFrom(f.getType())) {
+          f.setAccessible(true);
+          return (OkHttpClient) f.get(client);
+        }
+      }
+      c = c.getSuperclass();
+    }
+    throw new IllegalStateException("OkHttpClient field not found on MinioAsyncClient hierarchy");
+  }
+}
+