Merge branch 'master' into localstack-cleanup

Author: julianladisch

NEWS.md

@@ -1,5 +1,9 @@
 ## v3.1.0 - In Progress
 
+### Stories
+* [MSEARCH-1245](https://issues.folio.org/browse/MSEARCH-1245) Provide http client configuration for MinioS3Client/AwsS3Client
+* [MSEARCH-1245](https://issues.folio.org/browse/MSEARCH-1245) Fix bucket creation
+
 ## v3.0.0 - Released
 This release contains improvement client configuration and new features
 

pom.xml

@@ -20,18 +20,16 @@
     <maven.compiler.source>21</maven.compiler.source>
     <maven.compiler.target>21</maven.compiler.target>
     <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
-    <lombok.version>1.18.36</lombok.version>
-    <apache.commons.lang3.version>3.17.0</apache.commons.lang3.version>
+    <lombok.version>1.18.44</lombok.version>
+    <apache.commons.lang3.version>3.20.0</apache.commons.lang3.version>
     <minio.version>8.6.0</minio.version>
-    <okhttp.version>5.1.0</okhttp.version>
-    <log4j.version>2.23.1</log4j.version>
-    <aws.sdk.version>2.40.17</aws.sdk.version>
-    <junit.version>5.12.0</junit.version>
-    <testcontainers.version>1.20.5</testcontainers.version>
-    <commons-io.version>2.18.0</commons-io.version>
-    <sonar.exclusions>
-      **/src/main/java/org/folio/s3/client/S3ClientProperties.java
-    </sonar.exclusions>
+    <okhttp.version>5.3.2</okhttp.version>
+    <log4j.version>2.25.4</log4j.version>
+    <aws.sdk.version>2.42.34</aws.sdk.version>
+    <junit.version>6.0.3</junit.version>
+    <testcontainers.version>2.0.4</testcontainers.version>
+    <mockito.version>5.18.0</mockito.version>
+    <commons-io.version>2.21.0</commons-io.version>
   </properties>
 
   <dependencies>
@@ -72,6 +70,11 @@
       <artifactId>s3-transfer-manager</artifactId>
       <version>${aws.sdk.version}</version>
     </dependency>
+    <dependency>
+      <groupId>software.amazon.awssdk</groupId>
+      <artifactId>netty-nio-client</artifactId>
+      <version>${aws.sdk.version}</version>
+    </dependency>
     <dependency>
       <groupId>org.junit.jupiter</groupId>
       <artifactId>junit-jupiter-engine</artifactId>
@@ -84,6 +87,12 @@
       <version>${junit.version}</version>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-junit-jupiter</artifactId>
+      <version>${mockito.version}</version>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.testcontainers</groupId>
       <artifactId>testcontainers</artifactId>
@@ -92,7 +101,7 @@
     </dependency>
     <dependency>
       <groupId>org.testcontainers</groupId>
-      <artifactId>localstack</artifactId>
+      <artifactId>testcontainers-localstack</artifactId>
       <version>${testcontainers.version}</version>
       <scope>test</scope>
     </dependency>
@@ -104,7 +113,7 @@
       <plugin>
         <groupId>org.codehaus.mojo</groupId>
         <artifactId>versions-maven-plugin</artifactId>
-        <version>2.16.1</version>
+        <version>2.21.0</version>
         <executions>
           <execution>
             <id>display-dependency</id>
@@ -122,7 +131,7 @@
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-enforcer-plugin</artifactId>
-        <version>3.4.1</version>
+        <version>3.6.2</version>
         <executions>
           <execution>
             <id>enforce-maven</id>
@@ -143,7 +152,7 @@
       <plugin>
         <groupId>org.codehaus.mojo</groupId>
         <artifactId>build-helper-maven-plugin</artifactId>
-        <version>3.4.0</version>
+        <version>3.6.1</version>
         <executions>
           <execution>
             <phase>generate-sources</phase>
@@ -161,7 +170,7 @@
 
       <plugin>
         <artifactId>maven-source-plugin</artifactId>
-        <version>3.3.0</version>
+        <version>3.4.0</version>
         <executions>
           <execution>
             <id>attach-sources</id>
@@ -175,7 +184,7 @@
 
       <plugin>
         <artifactId>maven-javadoc-plugin</artifactId>
-        <version>3.6.0</version>
+        <version>3.12.0</version>
         <executions>
           <execution>
             <id>attach-javadocs</id>
@@ -189,7 +198,7 @@
 
       <plugin>
         <artifactId>maven-deploy-plugin</artifactId>
-        <version>3.1.1</version>
+        <version>3.1.4</version>
         <executions>
           <execution>
             <id>deploy</id>
@@ -204,7 +213,7 @@
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-compiler-plugin</artifactId>
-        <version>3.14.0</version>
+        <version>3.15.0</version>
         <configuration>
           <compilerVersion>21</compilerVersion>
         </configuration>
@@ -213,7 +222,7 @@
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-release-plugin</artifactId>
-        <version>3.0.1</version>
+        <version>3.3.1</version>
         <configuration>
           <preparationGoals>clean verify</preparationGoals>
           <tagNameFormat>v@{project.version}</tagNameFormat>
@@ -225,7 +234,7 @@
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-surefire-plugin</artifactId>
-        <version>3.2.1</version>
+        <version>3.5.5</version>
       </plugin>
 
     </plugins>

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

@@ -6,6 +6,7 @@
 import java.io.InputStream;
 import java.io.SequenceInputStream;
 import java.net.URI;
+import java.time.Duration;
 import java.util.concurrent.Executors;
 
 import org.folio.s3.exception.S3ClientException;
@@ -26,6 +27,7 @@
 import software.amazon.awssdk.services.s3.model.PutObjectRequest;
 import software.amazon.awssdk.services.s3.model.UploadPartCopyRequest;
 import software.amazon.awssdk.services.s3.model.UploadPartRequest;
+import software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClient;
 import software.amazon.awssdk.transfer.s3.S3TransferManager;
 import software.amazon.awssdk.transfer.s3.model.UploadRequest;
 
@@ -61,13 +63,29 @@ static S3AsyncClient createS3Client(S3ClientProperties s3ClientProperties) {
 
     credentialsProvider = getCredentialsProvider(accessKey, secretKey);
 
-    return S3AsyncClient.builder()
-            .endpointOverride(URI.create(endpoint))
-            .forcePathStyle(s3ClientProperties.isForcePathStyle())
-            .region(Region.of(region))
-            .credentialsProvider(credentialsProvider)
-            .multipartEnabled(true)
-            .build();
+    var s3Builder = S3AsyncClient.builder()
+        .endpointOverride(URI.create(endpoint))
+        .forcePathStyle(s3ClientProperties.isForcePathStyle())
+        .region(Region.of(region))
+        .credentialsProvider(credentialsProvider)
+        .multipartEnabled(true);
+
+    var idleKeepAlive = s3ClientProperties.getIdleKeepAliveSeconds();
+    var maxConcurrency = s3ClientProperties.getMaxRequestsPerHost();
+    if (idleKeepAlive != null || maxConcurrency != null) {
+      var httpBuilder = NettyNioAsyncHttpClient.builder();
+      if (idleKeepAlive != null) {
+        log.info("Configuring Netty HTTP client: connectionMaxIdleTime={}s", idleKeepAlive);
+        httpBuilder.connectionMaxIdleTime(Duration.ofSeconds(idleKeepAlive));
+      }
+      if (maxConcurrency != null) {
+        log.info("Configuring Netty HTTP client: maxConcurrency={}", maxConcurrency);
+        httpBuilder.maxConcurrency(maxConcurrency);
+      }
+      s3Builder.httpClient(httpBuilder.build());
+    }
+
+    return s3Builder.build();
   }
 
   @Override

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

@@ -47,6 +47,9 @@
 import io.minio.messages.DeleteObject;
 import io.minio.messages.Part;
 import lombok.extern.log4j.Log4j2;
+import okhttp3.ConnectionPool;
+import okhttp3.Dispatcher;
+import okhttp3.OkHttpClient;
 
 @Log4j2
 // 2142: we wrap and rethrow InterruptedException as S3ClientException
@@ -90,6 +93,23 @@ static ExtendedMinioAsyncClient createClient(S3ClientProperties properties) {
       builder.region(region);
     }
 
+    var idleKeepAliveSeconds = properties.getIdleKeepAliveSeconds();
+    if (idleKeepAliveSeconds != null) {
+      int maxIdle = properties.getMaxIdleConnections() != null ? properties.getMaxIdleConnections() : 5;
+      log.info("Configuring OkHttp connection pool: idleKeepAlive={}s maxIdleConnections={}", idleKeepAliveSeconds, maxIdle);
+      var pool = new ConnectionPool(maxIdle, idleKeepAliveSeconds, TimeUnit.SECONDS);
+      var dispatcher = new Dispatcher();
+      if (properties.getMaxRequestsPerHost() != null) {
+        log.info("Configuring OkHttp Dispatcher: maxRequestsPerHost={}", properties.getMaxRequestsPerHost());
+        dispatcher.setMaxRequestsPerHost(properties.getMaxRequestsPerHost());
+      }
+      var httpClient = new OkHttpClient.Builder()
+        .connectionPool(pool)
+        .dispatcher(dispatcher)
+        .build();
+      builder.httpClient(httpClient);
+    }
+
     Provider provider;
     if (StringUtils.isNotBlank(accessKey) && StringUtils.isNotBlank(secretKey)) {
       provider = new StaticProvider(accessKey, secretKey, null);
@@ -115,6 +135,7 @@ public void createBucketIfNotExists() {
         .get();
       if (Boolean.TRUE.equals(exists)) {
         log.debug("Bucket already exists.");
+        return;
       }
       client.makeBucket(MakeBucketArgs.builder()
         .bucket(bucket)

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

@@ -46,4 +46,36 @@ 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;
+
+  /**
+   * Maximum number of idle connections the OkHttp connection pool retains.
+   *
+   * <p>If {@code null}, defaults to 5 (OkHttp built-in default). Only takes effect when
+   * {@link #idleKeepAliveSeconds} is also set. Increase this when a single shared
+   * {@link FolioS3Client} instance services many concurrent callers so that connections
+   * from finished requests are available for immediate reuse.
+   */
+  private Integer maxIdleConnections;
+
+  /**
+   * Maximum number of concurrent HTTP requests OkHttp will dispatch to the same host
+   * (maps to {@code OkHttp Dispatcher.maxRequestsPerHost}).
+   *
+   * <p>If {@code null}, defaults to 5 (OkHttp built-in default). Only takes effect when
+   * {@link #idleKeepAliveSeconds} is also set. When a shared client handles more than 5
+   * concurrent callers, OkHttp queues the excess requests, which blocks the calling threads.
+   * Set this to at least the expected concurrency to avoid thread starvation.
+   */
+  private Integer maxRequestsPerHost;
 }

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

@@ -0,0 +1,74 @@
+package org.folio.s3.client;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Verifies that {@link AwsS3Client#createS3Client} correctly handles the
+ * connection-pool properties from {@link S3ClientProperties}.
+ *
+ * <p>The AWS SDK's {@link software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClient}
+ * does not expose its configuration after construction, so these tests focus on
+ * verifying that the client is created successfully for each combination rather
+ * than inspecting the internal Netty settings via reflection.
+ */
+class AwsS3ClientHttpClientTest {
+
+  private static final String ENDPOINT = "http://localhost:9000";
+
+  @Test
+  @DisplayName("S3AsyncClient is created when no pool properties are set (default Netty config)")
+  void defaultNettyConfigWhenNoPropertiesSet() {
+    var props = baseProps().build();
+
+    var client = AwsS3Client.createS3Client(props);
+
+    assertNotNull(client, "S3AsyncClient must be created with default Netty configuration");
+    client.close();
+  }
+
+  @Test
+  @DisplayName("S3AsyncClient is created when only idleKeepAliveSeconds is set")
+  void nettyConfigWithIdleKeepAliveOnly() {
+    var props = baseProps().idleKeepAliveSeconds(10).build();
+
+    var client = AwsS3Client.createS3Client(props);
+
+    assertNotNull(client);
+    client.close();
+  }
+
+  @Test
+  @DisplayName("S3AsyncClient is created when only maxRequestsPerHost is set")
+  void nettyConfigWithMaxConcurrencyOnly() {
+    var props = baseProps().maxRequestsPerHost(50).build();
+
+    var client = AwsS3Client.createS3Client(props);
+
+    assertNotNull(client);
+    client.close();
+  }
+
+  @Test
+  @DisplayName("S3AsyncClient is created when both idleKeepAliveSeconds and maxRequestsPerHost are set")
+  void nettyConfigWithBothPropertiesSet() {
+    var props = baseProps().idleKeepAliveSeconds(10).maxRequestsPerHost(50).build();
+
+    var client = AwsS3Client.createS3Client(props);
+
+    assertNotNull(client);
+    client.close();
+  }
+
+  private static S3ClientProperties.S3ClientPropertiesBuilder baseProps() {
+    return S3ClientProperties.builder()
+        .endpoint(ENDPOINT)
+        .region("us-east-1")
+        .bucket("test-bucket")
+        .accessKey("ak")
+        .secretKey("sk")
+        .forcePathStyle(true);
+  }
+}