Addressing PR comments for non-empty validation and to improve tests.

Signed-off-by: David Venable <dlv@amazon.com>
opensearch-project · May 31, 2023 · 54ed709 · 54ed709
1 parent ac9a555
commit 54ed709
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 6 deletions.
diff --git a/...r-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/S3SinkConfig.java b/...r-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/...ugins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkConfigTest.java b/...ugins/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/...gins/s3-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/S3SinkServiceTest.java b/...gins/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();