From ac9a555074917f76f2d560108fb4f8c9cbd269f7 Mon Sep 17 00:00:00 2001 From: David Venable Date: Wed, 31 May 2023 09:54:13 -0500 Subject: [PATCH 1/2] Moves the S3 sink bucket configuration up a level to simplify the YAML. Signed-off-by: David Venable --- .../plugins/sink/S3SinkServiceIT.java | 36 ++++++++-------- .../plugins/sink/S3SinkConfig.java | 33 ++++++++++----- .../plugins/sink/S3SinkService.java | 2 +- .../plugins/sink/accumulator/ObjectKey.java | 4 +- .../sink/configuration/BucketOptions.java | 41 ------------------- .../plugins/sink/S3SinkConfigTest.java | 18 +++++--- .../plugins/sink/S3SinkServiceTest.java | 17 ++++---- .../dataprepper/plugins/sink/S3SinkTest.java | 29 ++++++------- .../sink/accumulator/ObjectKeyTest.java | 17 ++++---- .../sink/configuration/BucketOptionsTest.java | 24 ----------- 10 files changed, 83 insertions(+), 138 deletions(-) delete mode 100644 data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/configuration/BucketOptions.java delete mode 100644 data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/configuration/BucketOptionsTest.java diff --git a/data-prepper-plugins/s3-sink/src/integrationTest/java/org/opensearch/dataprepper/plugins/sink/S3SinkServiceIT.java b/data-prepper-plugins/s3-sink/src/integrationTest/java/org/opensearch/dataprepper/plugins/sink/S3SinkServiceIT.java index 581a69a4bb..897c1eae87 100644 --- a/data-prepper-plugins/s3-sink/src/integrationTest/java/org/opensearch/dataprepper/plugins/sink/S3SinkServiceIT.java +++ b/data-prepper-plugins/s3-sink/src/integrationTest/java/org/opensearch/dataprepper/plugins/sink/S3SinkServiceIT.java @@ -5,19 +5,6 @@ package org.opensearch.dataprepper.plugins.sink; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; -import static org.mockito.Mockito.lenient; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -import java.time.Duration; -import java.util.Arrays; -import java.util.Collection; -import java.util.LinkedHashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.UUID; import io.micrometer.core.instrument.Counter; import io.micrometer.core.instrument.DistributionSummary; import org.hamcrest.CoreMatchers; @@ -38,7 +25,6 @@ import org.opensearch.dataprepper.plugins.sink.accumulator.ObjectKey; import org.opensearch.dataprepper.plugins.sink.codec.JsonCodec; import org.opensearch.dataprepper.plugins.sink.configuration.AwsAuthenticationOptions; -import org.opensearch.dataprepper.plugins.sink.configuration.BucketOptions; import org.opensearch.dataprepper.plugins.sink.configuration.ObjectKeyOptions; import org.opensearch.dataprepper.plugins.sink.configuration.ThresholdOptions; import software.amazon.awssdk.core.ResponseBytes; @@ -50,6 +36,21 @@ import software.amazon.awssdk.services.s3.model.ListObjectsResponse; import software.amazon.awssdk.services.s3.model.S3Object; +import java.time.Duration; +import java.util.Arrays; +import java.util.Collection; +import java.util.LinkedHashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.UUID; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + @ExtendWith(MockitoExtension.class) class S3SinkServiceIT { @@ -67,8 +68,6 @@ class S3SinkServiceIT { @Mock private ObjectKeyOptions objectKeyOptions; @Mock - private BucketOptions bucketOptions; - @Mock private PluginMetrics pluginMetrics; @Mock private Counter snapshotSuccessCounter; @@ -94,13 +93,12 @@ public void setUp() { when(objectKeyOptions.getNamePattern()).thenReturn("elb-log-%{yyyy-MM-dd'T'hh-mm-ss}"); when(objectKeyOptions.getPathPrefix()).thenReturn(PATH_PREFIX); - when(bucketOptions.getBucketName()).thenReturn(bucketName); - when(bucketOptions.getObjectKeyOptions()).thenReturn(objectKeyOptions); + when(s3SinkConfig.getBucketName()).thenReturn(bucketName); + when(s3SinkConfig.getObjectKeyOptions()).thenReturn(objectKeyOptions); when(thresholdOptions.getEventCount()).thenReturn(2); when(thresholdOptions.getMaximumSize()).thenReturn(ByteCount.parse("2mb")); when(thresholdOptions.getEventCollectTimeOut()).thenReturn(Duration.parse("PT3M")); when(s3SinkConfig.getThresholdOptions()).thenReturn(thresholdOptions); - when(s3SinkConfig.getBucketOptions()).thenReturn(bucketOptions); when(s3SinkConfig.getAwsAuthenticationOptions()).thenReturn(awsAuthenticationOptions); when(awsAuthenticationOptions.getAwsRegion()).thenReturn(Region.of(s3region)); diff --git a/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/S3SinkConfig.java b/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/S3SinkConfig.java index 5c41a65c81..95cb9f9574 100644 --- a/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/S3SinkConfig.java +++ b/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/S3SinkConfig.java @@ -5,14 +5,14 @@ package org.opensearch.dataprepper.plugins.sink; +import com.fasterxml.jackson.annotation.JsonProperty; +import jakarta.validation.Valid; +import jakarta.validation.constraints.NotNull; import org.opensearch.dataprepper.model.configuration.PluginModel; import org.opensearch.dataprepper.plugins.sink.accumulator.BufferTypeOptions; import org.opensearch.dataprepper.plugins.sink.configuration.AwsAuthenticationOptions; -import org.opensearch.dataprepper.plugins.sink.configuration.BucketOptions; +import org.opensearch.dataprepper.plugins.sink.configuration.ObjectKeyOptions; import org.opensearch.dataprepper.plugins.sink.configuration.ThresholdOptions; -import com.fasterxml.jackson.annotation.JsonProperty; -import jakarta.validation.Valid; -import jakarta.validation.constraints.NotNull; /** * s3 sink configuration class contains properties, used to read yaml configuration. @@ -29,8 +29,10 @@ public class S3SinkConfig { @JsonProperty("bucket") @NotNull - @Valid - private BucketOptions bucketOptions; + private String bucketName; + + @JsonProperty("object_key") + private ObjectKeyOptions objectKeyOptions; @JsonProperty("threshold") @NotNull @@ -65,11 +67,22 @@ public ThresholdOptions getThresholdOptions() { } /** - * S3 bucket configuration Options. - * @return bucket option object. + * Read s3 bucket name configuration. + * @return bucket name. + */ + public String getBucketName() { + return bucketName; + } + + /** + * S3 {@link ObjectKeyOptions} configuration Options. + * @return object key options. */ - public BucketOptions getBucketOptions() { - return bucketOptions; + public ObjectKeyOptions getObjectKeyOptions() { + if (objectKeyOptions == null) { + objectKeyOptions = new ObjectKeyOptions(); + } + return objectKeyOptions; } /** diff --git a/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/S3SinkService.java b/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/S3SinkService.java index e7c80e9477..9c8e402060 100644 --- a/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/S3SinkService.java +++ b/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/S3SinkService.java @@ -78,7 +78,7 @@ public S3SinkService(final S3SinkConfig s3SinkConfig, final BufferFactory buffer maxBytes = s3SinkConfig.getThresholdOptions().getMaximumSize(); maxCollectionDuration = s3SinkConfig.getThresholdOptions().getEventCollectTimeOut().getSeconds(); - bucket = s3SinkConfig.getBucketOptions().getBucketName(); + bucket = s3SinkConfig.getBucketName(); maxRetries = s3SinkConfig.getMaxUploadRetries(); objectsSucceededCounter = pluginMetrics.counter(OBJECTS_SUCCEEDED); diff --git a/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/accumulator/ObjectKey.java b/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/accumulator/ObjectKey.java index 0d4ee6b420..b3e85d8381 100644 --- a/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/accumulator/ObjectKey.java +++ b/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/accumulator/ObjectKey.java @@ -31,7 +31,7 @@ private ObjectKey(){} * @return s3 object path */ public static String buildingPathPrefix(final S3SinkConfig s3SinkConfig) { - String pathPrefix = s3SinkConfig.getBucketOptions().getObjectKeyOptions().getPathPrefix(); + String pathPrefix = s3SinkConfig.getObjectKeyOptions().getPathPrefix(); StringBuilder s3ObjectPath = new StringBuilder(); if (pathPrefix != null && !pathPrefix.isEmpty()) { String[] pathPrefixList = pathPrefix.split("\\/"); @@ -53,7 +53,7 @@ public static String buildingPathPrefix(final S3SinkConfig s3SinkConfig) { * @return s3 object name with prefix */ public static String objectFileName(S3SinkConfig s3SinkConfig) { - String configNamePattern = s3SinkConfig.getBucketOptions().getObjectKeyOptions().getNamePattern(); + String configNamePattern = s3SinkConfig.getObjectKeyOptions().getNamePattern(); int extensionIndex = configNamePattern.lastIndexOf('.'); if (extensionIndex > 0) { return S3ObjectIndexUtility.getObjectNameWithDateTimeId(configNamePattern.substring(0, extensionIndex)) + "." diff --git a/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/configuration/BucketOptions.java b/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/configuration/BucketOptions.java deleted file mode 100644 index 833aaa8927..0000000000 --- a/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/configuration/BucketOptions.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.dataprepper.plugins.sink.configuration; - -import com.fasterxml.jackson.annotation.JsonProperty; -import jakarta.validation.constraints.NotNull; - -/** - * An implementation class of bucket name and {@link ObjectKeyOptions} configuration Options - */ -public class BucketOptions { - - @JsonProperty("name") - @NotNull - private String bucketName; - - @JsonProperty("object_key") - private ObjectKeyOptions objectKeyOptions; - - /** - * Read s3 bucket name configuration. - * @return bucket name. - */ - public String getBucketName() { - return bucketName; - } - - /** - * S3 {@link ObjectKeyOptions} configuration Options. - * @return object key options. - */ - public ObjectKeyOptions getObjectKeyOptions() { - if (objectKeyOptions == null) { - objectKeyOptions = new ObjectKeyOptions(); - } - return objectKeyOptions; - } -} \ No newline at end of file diff --git a/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkConfigTest.java b/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkConfigTest.java index e794d13568..cb10cabe50 100644 --- a/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkConfigTest.java +++ b/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkConfigTest.java @@ -5,13 +5,14 @@ package org.opensearch.dataprepper.plugins.sink; +import org.junit.jupiter.api.Test; +import org.opensearch.dataprepper.plugins.sink.accumulator.BufferTypeOptions; +import org.opensearch.dataprepper.plugins.sink.configuration.ObjectKeyOptions; + import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertNull; -import org.junit.jupiter.api.Test; -import org.opensearch.dataprepper.plugins.sink.accumulator.BufferTypeOptions; - class S3SinkConfigTest { private static final int MAX_CONNECTION_RETRIES = 5; @@ -33,10 +34,17 @@ void default_max_upload_retries_test() throws NoSuchFieldException, IllegalAcces } @Test - void get_bucket_option_test() { - assertThat(new S3SinkConfig().getBucketOptions(), equalTo(null)); + void get_bucket_name_test() { + assertThat(new S3SinkConfig().getBucketName(), equalTo(null)); } + @Test + void get_object_key_test() { + assertThat("Object key is not an instance of ObjectKeyOptions", + new S3SinkConfig().getObjectKeyOptions() instanceof ObjectKeyOptions); + } + + @Test void get_threshold_option_test() { assertThat(new S3SinkConfig().getThresholdOptions(), equalTo(null)); diff --git a/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkServiceTest.java b/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkServiceTest.java index 03ee6b2c1c..0db9604708 100644 --- a/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkServiceTest.java +++ b/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkServiceTest.java @@ -26,7 +26,6 @@ import org.opensearch.dataprepper.plugins.sink.codec.Codec; import org.opensearch.dataprepper.plugins.sink.codec.JsonCodec; import org.opensearch.dataprepper.plugins.sink.configuration.AwsAuthenticationOptions; -import org.opensearch.dataprepper.plugins.sink.configuration.BucketOptions; import org.opensearch.dataprepper.plugins.sink.configuration.ObjectKeyOptions; import org.opensearch.dataprepper.plugins.sink.configuration.ThresholdOptions; import software.amazon.awssdk.awscore.exception.AwsServiceException; @@ -89,7 +88,6 @@ void setUp() throws Exception { random = new Random(); s3SinkConfig = mock(S3SinkConfig.class); ThresholdOptions thresholdOptions = mock(ThresholdOptions.class); - BucketOptions bucketOptions = mock(BucketOptions.class); ObjectKeyOptions objectKeyOptions = mock(ObjectKeyOptions.class); AwsAuthenticationOptions awsAuthenticationOptions = mock(AwsAuthenticationOptions.class); PluginSetting pluginSetting = mock(PluginSetting.class); @@ -112,10 +110,9 @@ void setUp() throws Exception { when(s3SinkConfig.getThresholdOptions().getMaximumSize()).thenReturn(ByteCount.parse(MAXIMUM_SIZE)); when(s3SinkConfig.getThresholdOptions().getEventCollectTimeOut()).thenReturn(Duration.ofSeconds(5)); when(s3SinkConfig.getBufferType()).thenReturn(BufferTypeOptions.INMEMORY); - when(s3SinkConfig.getBucketOptions()).thenReturn(bucketOptions); - when(s3SinkConfig.getBucketOptions().getObjectKeyOptions()).thenReturn(objectKeyOptions); - when(s3SinkConfig.getBucketOptions().getBucketName()).thenReturn(BUCKET_NAME); - when(s3SinkConfig.getBucketOptions().getObjectKeyOptions().getPathPrefix()).thenReturn(PATH_PREFIX); + when(s3SinkConfig.getObjectKeyOptions()).thenReturn(objectKeyOptions); + when(s3SinkConfig.getBucketName()).thenReturn(BUCKET_NAME); + when(s3SinkConfig.getObjectKeyOptions().getPathPrefix()).thenReturn(PATH_PREFIX); when(s3SinkConfig.getAwsAuthenticationOptions()).thenReturn(awsAuthenticationOptions); when(awsAuthenticationOptions.getAwsRegion()).thenReturn(Region.of(S3_REGION)); when(s3SinkConfig.getCodec()).thenReturn(pluginModel); @@ -153,7 +150,7 @@ void test_s3Client_notNull() { @Test void test_generateKey_with_general_prefix() { String pathPrefix = "events/"; - when(s3SinkConfig.getBucketOptions().getObjectKeyOptions().getPathPrefix()).thenReturn(pathPrefix); + when(s3SinkConfig.getObjectKeyOptions().getPathPrefix()).thenReturn(pathPrefix); S3SinkService s3SinkService = createObjectUnderTest(); String key = s3SinkService.generateKey(); assertNotNull(key); @@ -171,7 +168,7 @@ void test_generateKey_with_date_prefix() { .withZoneSameInstant(ZoneId.of(TimeZone.getTimeZone("UTC").getID())); String dateString = fomatter.format(zdt); - when(s3SinkConfig.getBucketOptions().getObjectKeyOptions() + when(s3SinkConfig.getObjectKeyOptions() .getPathPrefix()).thenReturn(pathPrefix + datePattern); S3SinkService s3SinkService = createObjectUnderTest(); String key = s3SinkService.generateKey(); @@ -263,7 +260,7 @@ void test_output_with_uploadedToS3_success_records_byte_count() throws IOExcepti @Test void test_output_with_uploadedToS3_failed() throws IOException { - when(s3SinkConfig.getBucketOptions().getBucketName()).thenReturn(null); + when(s3SinkConfig.getBucketName()).thenReturn(null); when(s3SinkConfig.getMaxUploadRetries()).thenReturn(3); when(codec.parse(any())).thenReturn("{\"message\":\"31824252-adba-4c47-a2ac-05d16c5b8140\"}"); S3SinkService s3SinkService = createObjectUnderTest(); @@ -313,7 +310,7 @@ void test_retryFlushToS3_positive() throws InterruptedException, IOException { @Test void test_retryFlushToS3_negative() throws InterruptedException, IOException { - when(s3SinkConfig.getBucketOptions().getBucketName()).thenReturn(""); + when(s3SinkConfig.getBucketName()).thenReturn(""); S3SinkService s3SinkService = createObjectUnderTest(); assertNotNull(s3SinkService); Buffer buffer = bufferFactory.getBuffer(); diff --git a/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkTest.java b/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkTest.java index 530aa014e1..b7fc33d196 100644 --- a/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkTest.java +++ b/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkTest.java @@ -5,15 +5,6 @@ package org.opensearch.dataprepper.plugins.sink; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -import java.time.Duration; -import java.util.ArrayList; -import java.util.Collection; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -27,11 +18,21 @@ import org.opensearch.dataprepper.plugins.sink.codec.Codec; import org.opensearch.dataprepper.plugins.sink.codec.JsonCodec; import org.opensearch.dataprepper.plugins.sink.configuration.AwsAuthenticationOptions; -import org.opensearch.dataprepper.plugins.sink.configuration.BucketOptions; import org.opensearch.dataprepper.plugins.sink.configuration.ObjectKeyOptions; import org.opensearch.dataprepper.plugins.sink.configuration.ThresholdOptions; import software.amazon.awssdk.regions.Region; +import java.time.Duration; +import java.util.ArrayList; +import java.util.Collection; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + class S3SinkTest { public static final int MAX_EVENTS = 100; @@ -47,7 +48,6 @@ class S3SinkTest { private S3Sink s3Sink; private PluginSetting pluginSetting; private PluginFactory pluginFactory; - private BucketOptions bucketOptions; @BeforeEach void setUp() { @@ -57,7 +57,6 @@ void setUp() { AwsAuthenticationOptions awsAuthenticationOptions = mock(AwsAuthenticationOptions.class); Codec codec = mock(JsonCodec.class); ObjectKeyOptions objectKeyOptions = mock(ObjectKeyOptions.class); - bucketOptions = mock(BucketOptions.class); pluginSetting = mock(PluginSetting.class); PluginModel pluginModel = mock(PluginModel.class); pluginFactory = mock(PluginFactory.class); @@ -75,8 +74,7 @@ void setUp() { when(pluginFactory.loadPlugin(any(), any())).thenReturn(codec); when(pluginSetting.getName()).thenReturn(SINK_PLUGIN_NAME); when(pluginSetting.getPipelineName()).thenReturn(SINK_PIPELINE_NAME); - when(s3SinkConfig.getBucketOptions()).thenReturn(bucketOptions); - when(s3SinkConfig.getBucketOptions().getBucketName()).thenReturn(BUCKET_NAME); + when(s3SinkConfig.getBucketName()).thenReturn(BUCKET_NAME); } @Test @@ -105,8 +103,7 @@ void test_doInitialize_with_exception() { @Test void test_doOutput_with_empty_records() { - when(s3SinkConfig.getBucketOptions()).thenReturn(bucketOptions); - when(s3SinkConfig.getBucketOptions().getBucketName()).thenReturn(BUCKET_NAME); + when(s3SinkConfig.getBucketName()).thenReturn(BUCKET_NAME); s3Sink = new S3Sink(pluginSetting, s3SinkConfig, pluginFactory); Assertions.assertNotNull(s3Sink); s3Sink.doInitialize(); diff --git a/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/accumulator/ObjectKeyTest.java b/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/accumulator/ObjectKeyTest.java index 0043720574..c2ee676e5d 100644 --- a/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/accumulator/ObjectKeyTest.java +++ b/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/accumulator/ObjectKeyTest.java @@ -5,9 +5,6 @@ package org.opensearch.dataprepper.plugins.sink.accumulator; -import static org.hamcrest.CoreMatchers.startsWith; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.mockito.Mockito.when; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -19,9 +16,12 @@ import org.opensearch.dataprepper.model.plugin.PluginFactory; import org.opensearch.dataprepper.plugins.sink.S3SinkConfig; import org.opensearch.dataprepper.plugins.sink.codec.JsonCodec; -import org.opensearch.dataprepper.plugins.sink.configuration.BucketOptions; import org.opensearch.dataprepper.plugins.sink.configuration.ObjectKeyOptions; +import static org.hamcrest.CoreMatchers.startsWith; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.mockito.Mockito.when; + @ExtendWith(MockitoExtension.class) class ObjectKeyTest { @@ -38,14 +38,11 @@ class ObjectKeyTest { @Mock private JsonCodec codec; @Mock - private BucketOptions bucketOptions; - @Mock private ObjectKeyOptions objectKeyOptions; @BeforeEach void setUp() throws Exception { - when(s3SinkConfig.getBucketOptions()).thenReturn(bucketOptions); - when(s3SinkConfig.getBucketOptions().getObjectKeyOptions()).thenReturn(objectKeyOptions); + when(s3SinkConfig.getObjectKeyOptions()).thenReturn(objectKeyOptions); } @Test @@ -69,7 +66,7 @@ void test_objectFileName() { @Test void test_objectFileName_with_fileExtension() { - when(s3SinkConfig.getBucketOptions().getObjectKeyOptions().getNamePattern()) + when(s3SinkConfig.getObjectKeyOptions().getNamePattern()) .thenReturn("events-%{yyyy-MM-dd'T'hh-mm-ss}.pdf"); String objectFileName = ObjectKey.objectFileName(s3SinkConfig); Assertions.assertNotNull(objectFileName); @@ -79,7 +76,7 @@ void test_objectFileName_with_fileExtension() { @Test void test_objectFileName_default_fileExtension() { - when(s3SinkConfig.getBucketOptions().getObjectKeyOptions().getNamePattern()) + when(s3SinkConfig.getObjectKeyOptions().getNamePattern()) .thenReturn("events-%{yyyy-MM-dd'T'hh-mm-ss}"); String objectFileName = ObjectKey.objectFileName(s3SinkConfig); Assertions.assertNotNull(objectFileName); diff --git a/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/configuration/BucketOptionsTest.java b/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/configuration/BucketOptionsTest.java deleted file mode 100644 index eee4a7370c..0000000000 --- a/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/configuration/BucketOptionsTest.java +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.dataprepper.plugins.sink.configuration; - -import static org.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.MatcherAssert.assertThat; -import org.junit.jupiter.api.Test; - -class BucketOptionsTest { - - @Test - void get_bucket_name_test() { - assertThat(new BucketOptions().getBucketName(), equalTo(null)); - } - - @Test - void get_object_key_test() { - assertThat("Object key is not an instance of ObjectKeyOptions", - new BucketOptions().getObjectKeyOptions() instanceof ObjectKeyOptions); - } -} \ No newline at end of file From 54ed709fe161ea42ae611e36c4069dcdc28d8937 Mon Sep 17 00:00:00 2001 From: David Venable Date: Thu, 25 May 2023 19:28:08 -0500 Subject: [PATCH 2/2] Addressing PR comments for non-empty validation and to improve tests. Signed-off-by: David Venable --- .../dataprepper/plugins/sink/S3SinkConfig.java | 2 ++ .../plugins/sink/S3SinkConfigTest.java | 17 ++++++++++++----- .../plugins/sink/S3SinkServiceTest.java | 2 +- 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/S3SinkConfig.java b/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/S3SinkConfig.java index 95cb9f9574..588f4984f4 100644 --- a/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/S3SinkConfig.java +++ b/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/S3SinkConfig.java @@ -7,6 +7,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import jakarta.validation.Valid; +import jakarta.validation.constraints.NotEmpty; import jakarta.validation.constraints.NotNull; import org.opensearch.dataprepper.model.configuration.PluginModel; import org.opensearch.dataprepper.plugins.sink.accumulator.BufferTypeOptions; @@ -29,6 +30,7 @@ public class S3SinkConfig { @JsonProperty("bucket") @NotNull + @NotEmpty private String bucketName; @JsonProperty("object_key") diff --git a/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkConfigTest.java b/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkConfigTest.java index cb10cabe50..3d619aa601 100644 --- a/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkConfigTest.java +++ b/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkConfigTest.java @@ -8,8 +8,12 @@ import org.junit.jupiter.api.Test; import org.opensearch.dataprepper.plugins.sink.accumulator.BufferTypeOptions; import org.opensearch.dataprepper.plugins.sink.configuration.ObjectKeyOptions; +import org.opensearch.dataprepper.test.helper.ReflectivelySetField; + +import java.util.UUID; import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertNull; @@ -24,24 +28,27 @@ void default_buffer_type_option_test() { } @Test - void default_max_connection_retries_test() throws NoSuchFieldException, IllegalAccessException { + void default_max_connection_retries_test() { assertThat(new S3SinkConfig().getMaxConnectionRetries(), equalTo(MAX_CONNECTION_RETRIES)); } @Test - void default_max_upload_retries_test() throws NoSuchFieldException, IllegalAccessException { + void default_max_upload_retries_test() { assertThat(new S3SinkConfig().getMaxUploadRetries(), equalTo(MAX_UPLOAD_RETRIES)); } @Test - void get_bucket_name_test() { - assertThat(new S3SinkConfig().getBucketName(), equalTo(null)); + void get_bucket_name_test() throws NoSuchFieldException, IllegalAccessException { + final String bucketName = UUID.randomUUID().toString(); + final S3SinkConfig objectUnderTest = new S3SinkConfig(); + ReflectivelySetField.setField(S3SinkConfig.class, objectUnderTest, "bucketName", bucketName); + assertThat(objectUnderTest.getBucketName(), equalTo(bucketName)); } @Test void get_object_key_test() { assertThat("Object key is not an instance of ObjectKeyOptions", - new S3SinkConfig().getObjectKeyOptions() instanceof ObjectKeyOptions); + new S3SinkConfig().getObjectKeyOptions(), instanceOf(ObjectKeyOptions.class)); } diff --git a/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkServiceTest.java b/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkServiceTest.java index 0db9604708..ee7c4275a1 100644 --- a/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkServiceTest.java +++ b/data-prepper-plugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkServiceTest.java @@ -260,7 +260,7 @@ void test_output_with_uploadedToS3_success_records_byte_count() throws IOExcepti @Test void test_output_with_uploadedToS3_failed() throws IOException { - when(s3SinkConfig.getBucketName()).thenReturn(null); + when(s3SinkConfig.getBucketName()).thenReturn(UUID.randomUUID().toString()); when(s3SinkConfig.getMaxUploadRetries()).thenReturn(3); when(codec.parse(any())).thenReturn("{\"message\":\"31824252-adba-4c47-a2ac-05d16c5b8140\"}"); S3SinkService s3SinkService = createObjectUnderTest();