From 271ac859bca905191907ad46d8ede78d86c1ec65 Mon Sep 17 00:00:00 2001 From: Adam Saghy Date: Mon, 28 Jul 2025 16:02:35 +0200 Subject: [PATCH] FINERACT-2118: Make region as optional and remove hardcoded default value --- .../core/config/ContentS3Config.java | 6 ++- .../src/main/resources/application.properties | 2 +- .../core/config/ContentS3ConfigTest.java | 40 +++++++------------ .../resources/application-test.properties | 3 ++ 4 files changed, 23 insertions(+), 28 deletions(-) diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/config/ContentS3Config.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/config/ContentS3Config.java index e2fa7514c..f9640afb0 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/config/ContentS3Config.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/config/ContentS3Config.java @@ -40,9 +40,11 @@ public class ContentS3Config { @Bean @ConditionalOnProperty("fineract.content.s3.enabled") public S3Client contentS3Client(FineractProperties fineractProperties) { - S3ClientBuilder builder = S3Client.builder().credentialsProvider(getCredentialProvider(fineractProperties.getContent().getS3())) - .region(Region.of(fineractProperties.getContent().getS3().getRegion())); + S3ClientBuilder builder = S3Client.builder().credentialsProvider(getCredentialProvider(fineractProperties.getContent().getS3())); + if (!Strings.isNullOrEmpty(fineractProperties.getContent().getS3().getRegion())) { + builder.region(Region.of(fineractProperties.getContent().getS3().getRegion())); + } if (!Strings.isNullOrEmpty(fineractProperties.getContent().getS3().getEndpoint())) { builder.endpointOverride(URI.create(fineractProperties.getContent().getS3().getEndpoint())) .forcePathStyle(fineractProperties.getContent().getS3().getPathStyleAddressingEnabled()); diff --git a/fineract-provider/src/main/resources/application.properties b/fineract-provider/src/main/resources/application.properties index be00ca5c9..955645f54 100644 --- a/fineract-provider/src/main/resources/application.properties +++ b/fineract-provider/src/main/resources/application.properties @@ -165,7 +165,7 @@ fineract.content.s3.enabled=${FINERACT_CONTENT_S3_ENABLED:false} fineract.content.s3.bucketName=${FINERACT_CONTENT_S3_BUCKET_NAME:} fineract.content.s3.accessKey=${FINERACT_CONTENT_S3_ACCESS_KEY:} fineract.content.s3.secretKey=${FINERACT_CONTENT_S3_SECRET_KEY:} -fineract.content.s3.region=${FINERACT_CONTENT_S3_REGION:us-east-1} +fineract.content.s3.region=${FINERACT_CONTENT_S3_REGION:} fineract.content.s3.endpoint=${FINERACT_CONTENT_S3_ENDPOINT:} fineract.content.s3.path-style-addressing-enabled=${FINERACT_CONTENT_S3_PATH_STYLE_ADDRESSING_ENABLED:false} diff --git a/fineract-provider/src/test/java/org/apache/fineract/infrastructure/core/config/ContentS3ConfigTest.java b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/core/config/ContentS3ConfigTest.java index 4880699f6..881a474f2 100644 --- a/fineract-provider/src/test/java/org/apache/fineract/infrastructure/core/config/ContentS3ConfigTest.java +++ b/fineract-provider/src/test/java/org/apache/fineract/infrastructure/core/config/ContentS3ConfigTest.java @@ -21,10 +21,12 @@ package org.apache.fineract.infrastructure.core.config; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -54,6 +56,13 @@ class ContentS3ConfigTest { when(contentProperties.getS3()).thenReturn(s3Properties); } + @AfterEach + void tearDown() { + reset(fineractProperties); + reset(contentProperties); + reset(s3Properties); + } + @Test void testContentS3Client_WithCredentialsAndNoEndpoint() { @@ -72,7 +81,7 @@ class ContentS3ConfigTest { verify(s3Properties, times(2)).getAccessKey(); verify(s3Properties, times(2)).getSecretKey(); - verify(s3Properties).getRegion(); + verify(s3Properties, times(2)).getRegion(); verify(s3Properties).getEndpoint(); } @@ -91,7 +100,7 @@ class ContentS3ConfigTest { verify(s3Properties, times(1)).getAccessKey(); verify(s3Properties, times(0)).getSecretKey(); - verify(s3Properties).getRegion(); + verify(s3Properties, times(2)).getRegion(); verify(s3Properties).getEndpoint(); } @@ -116,7 +125,7 @@ class ContentS3ConfigTest { verify(s3Properties, times(2)).getAccessKey(); verify(s3Properties, times(2)).getSecretKey(); - verify(s3Properties).getRegion(); + verify(s3Properties, times(2)).getRegion(); verify(s3Properties, times(2)).getEndpoint(); verify(s3Properties).getPathStyleAddressingEnabled(); } @@ -137,7 +146,7 @@ class ContentS3ConfigTest { verify(s3Properties, times(1)).getAccessKey(); verify(s3Properties, times(0)).getSecretKey(); - verify(s3Properties).getRegion(); + verify(s3Properties, times(2)).getRegion(); verify(s3Properties).getEndpoint(); } @@ -160,7 +169,7 @@ class ContentS3ConfigTest { verify(s3Properties, times(2)).getAccessKey(); verify(s3Properties, times(2)).getSecretKey(); - verify(s3Properties).getRegion(); + verify(s3Properties, times(2)).getRegion(); verify(s3Properties).getEndpoint(); verify(s3Properties, never()).getPathStyleAddressingEnabled(); @@ -182,26 +191,7 @@ class ContentS3ConfigTest { verify(s3Properties, times(1)).getAccessKey(); verify(s3Properties, times(1)).getSecretKey(); - verify(s3Properties).getRegion(); - verify(s3Properties).getEndpoint(); - } - - @Test - void testContentS3Client_WithOnlySecretKey() { - - String region = "eu-central-1"; - - when(s3Properties.getAccessKey()).thenReturn(null); - when(s3Properties.getEndpoint()).thenReturn(null); - when(s3Properties.getRegion()).thenReturn(region); - - S3Client s3Client = contentS3Config.contentS3Client(fineractProperties); - - assertNotNull(s3Client); - - verify(s3Properties, times(1)).getAccessKey(); - verify(s3Properties, times(0)).getSecretKey(); - verify(s3Properties).getRegion(); + verify(s3Properties, times(2)).getRegion(); verify(s3Properties).getEndpoint(); } } diff --git a/fineract-provider/src/test/resources/application-test.properties b/fineract-provider/src/test/resources/application-test.properties index 83ac38534..b2caf5dd7 100644 --- a/fineract-provider/src/test/resources/application-test.properties +++ b/fineract-provider/src/test/resources/application-test.properties @@ -93,6 +93,9 @@ fineract.content.s3.enabled=false fineract.content.s3.bucketName= fineract.content.s3.accessKey= fineract.content.s3.secretKey= +fineract.content.s3.region= +fineract.content.s3.endpoint= +fineract.content.s3.path-style-addressing-enabled=false fineract.report.export.s3.bucket=${FINERACT_REPORT_EXPORT_S3_BUCKET_NAME:} fineract.report.export.s3.enabled=${FINERACT_REPORT_EXPORT_S3_ENABLED:false}