MAINT: ref and fix test coverage

Signed-off-by: George Chen <[email protected]>
opensearch-project · Sep 18, 2023 · 73a265d · 73a265d
1 parent 8f90fe0
commit 73a265d
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 12 deletions.
diff --git a/data-prepper-plugins/aws-plugin/build.gradle b/data-prepper-plugins/aws-plugin/build.gradle
@@ -20,7 +20,7 @@ jacocoTestCoverageVerification {
     violationRules {
         rule {
             limit {
-                minimum = 0.98
+                minimum = 1.0
             }
         }
     }

diff --git a/...gins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretPlugin.java b/...gins/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretPlugin.java
@@ -1,5 +1,6 @@
 package org.opensearch.dataprepper.plugins.aws;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
 import org.opensearch.dataprepper.model.annotations.DataPrepperExtensionPlugin;
 import org.opensearch.dataprepper.model.annotations.DataPrepperPluginConstructor;
 import org.opensearch.dataprepper.model.plugin.ExtensionPlugin;
@@ -9,12 +10,13 @@
 @DataPrepperExtensionPlugin(modelType = AwsSecretPluginConfig.class, rootKeyJsonPath = "/aws/secrets",
         allowInPipelineConfigurations = true)
 public class AwsSecretPlugin implements ExtensionPlugin {
+    private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
     private final PluginConfigValueTranslator pluginConfigValueTranslator;
 
     @DataPrepperPluginConstructor
     public AwsSecretPlugin(final AwsSecretPluginConfig awsSecretPluginConfig) {
         if (awsSecretPluginConfig != null) {
-            final SecretsSupplier secretsSupplier = new AwsSecretsSupplier(awsSecretPluginConfig);
+            final SecretsSupplier secretsSupplier = new AwsSecretsSupplier(awsSecretPluginConfig, OBJECT_MAPPER);
             pluginConfigValueTranslator = new AwsSecretsPluginConfigValueTranslator(secretsSupplier);
         } else {
             pluginConfigValueTranslator = null;

diff --git a/...s/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplier.java b/...s/aws-plugin/src/main/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplier.java
@@ -11,13 +11,14 @@
 import java.util.stream.Collectors;
 
 public class AwsSecretsSupplier implements SecretsSupplier {
-    private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
-    private static final TypeReference<Map<String, String>> MAP_TYPE_REFERENCE = new TypeReference<>() {
+    static final TypeReference<Map<String, String>> MAP_TYPE_REFERENCE = new TypeReference<>() {
     };
 
+    private final ObjectMapper objectMapper;
     private final Map<String, Object> secretIdToValue;
 
-    public AwsSecretsSupplier(final AwsSecretPluginConfig awsSecretPluginConfig) {
+    public AwsSecretsSupplier(final AwsSecretPluginConfig awsSecretPluginConfig, final ObjectMapper objectMapper) {
+        this.objectMapper = objectMapper;
         secretIdToValue = toSecretMap(awsSecretPluginConfig);
     }
 
@@ -44,7 +45,7 @@ private Map<String, Object> toSecretMap(final AwsSecretPluginConfig awsSecretPlu
                     }
 
                     try {
-                        return OBJECT_MAPPER.readValue(getSecretValueResponse.secretString(), MAP_TYPE_REFERENCE);
+                        return objectMapper.readValue(getSecretValueResponse.secretString(), MAP_TYPE_REFERENCE);
                     } catch (JsonProcessingException e) {
                         return getSecretValueResponse.secretString();
                     }
@@ -85,10 +86,10 @@ public Object retrieveValue(String secretId) {
         }
         try {
             final Object secretValue = secretIdToValue.get(secretId);
-            return secretValue instanceof Map ? OBJECT_MAPPER.writeValueAsString(secretIdToValue.get(secretId)) :
+            return secretValue instanceof Map ? objectMapper.writeValueAsString(secretValue) :
                     secretValue;
         } catch (JsonProcessingException e) {
-            throw new IllegalArgumentException(String.format("Unable to read the value under secretId: %s as string",
+            throw new IllegalArgumentException(String.format("Unable to read the value under secretId: %s as string.",
                     secretId));
         }
     }

diff --git a/...ava/org/opensearch/dataprepper/plugins/aws/AwsSecretsPluginConfigValueTranslatorTest.java b/...ava/org/opensearch/dataprepper/plugins/aws/AwsSecretsPluginConfigValueTranslatorTest.java
@@ -15,6 +15,7 @@
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.when;
+import static org.opensearch.dataprepper.plugins.aws.AwsSecretsPluginConfigValueTranslator.AWS_SECRETS_PREFIX;
 
 @ExtendWith(MockitoExtension.class)
 class AwsSecretsPluginConfigValueTranslatorTest {
@@ -28,6 +29,11 @@ void setUp() {
         objectUnderTest = new AwsSecretsPluginConfigValueTranslator(secretsSupplier);
     }
 
+    @Test
+    void testGetPrefix() {
+        assertThat(objectUnderTest.getPrefix(), equalTo(AWS_SECRETS_PREFIX));
+    }
+
     @ParameterizedTest
     @ValueSource(strings = {
             "",

diff --git a/...s-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierTest.java b/...s-plugin/src/test/java/org/opensearch/dataprepper/plugins/aws/AwsSecretsSupplierTest.java
@@ -7,6 +7,7 @@
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
+import org.mockito.ArgumentMatchers;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import software.amazon.awssdk.services.secretsmanager.SecretsManagerClient;
@@ -20,7 +21,9 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
+import static org.opensearch.dataprepper.plugins.aws.AwsSecretsSupplier.MAP_TYPE_REFERENCE;
 
 @ExtendWith(MockitoExtension.class)
 class AwsSecretsSupplierTest {
@@ -60,7 +63,7 @@ void setUp() throws JsonProcessingException {
                 Map.of(TEST_KEY, TEST_VALUE)
         ));
         when(secretsManagerClient.getSecretValue(eq(getSecretValueRequest))).thenReturn(getSecretValueResponse);
-        objectUnderTest = new AwsSecretsSupplier(awsSecretPluginConfig);
+        objectUnderTest = new AwsSecretsSupplier(awsSecretPluginConfig, OBJECT_MAPPER);
     }
 
     @Test
@@ -83,7 +86,7 @@ void testRetrieveValueMissingKey() {
     @Test
     void testRetrieveValueInvalidKeyValuePair() {
         when(getSecretValueResponse.secretString()).thenReturn(TEST_VALUE);
-        objectUnderTest = new AwsSecretsSupplier(awsSecretPluginConfig);
+        objectUnderTest = new AwsSecretsSupplier(awsSecretPluginConfig, OBJECT_MAPPER);
         final Exception exception = assertThrows(IllegalArgumentException.class,
                 () -> objectUnderTest.retrieveValue(TEST_AWS_SECRET_CONFIGURATION_NAME, TEST_KEY));
         assertThat(exception.getMessage(), equalTo(String.format("The value under secretId: %s is not a valid json.",
@@ -96,17 +99,32 @@ void testRetrieveValueBySecretIdOnlyDoesNotExist() {
                 () -> objectUnderTest.retrieveValue("missing-config-id"));
     }
 
+    @Test
+    void testRetrieveValueBySecretIdOnlyNotSerializable() throws JsonProcessingException {
+        final ObjectMapper mockedObjectMapper = mock(ObjectMapper.class);
+        final JsonProcessingException mockedJsonProcessingException = mock(JsonProcessingException.class);
+        final String testValue = "{\"a\":\"b\"}";
+        when(mockedObjectMapper.readValue(eq(testValue), eq(MAP_TYPE_REFERENCE))).thenReturn(Map.of("a", "b"));
+        when(mockedObjectMapper.writeValueAsString(ArgumentMatchers.any())).thenThrow(mockedJsonProcessingException);
+        when(getSecretValueResponse.secretString()).thenReturn(testValue);
+        objectUnderTest = new AwsSecretsSupplier(awsSecretPluginConfig, mockedObjectMapper);
+        final Exception exception = assertThrows(IllegalArgumentException.class,
+                () -> objectUnderTest.retrieveValue(TEST_AWS_SECRET_CONFIGURATION_NAME));
+        assertThat(exception.getMessage(), equalTo(String.format("Unable to read the value under secretId: %s as string.",
+                TEST_AWS_SECRET_CONFIGURATION_NAME)));
+    }
+
     @ParameterizedTest
     @ValueSource(strings = {TEST_VALUE, "{\"a\":\"b\"}"})
     void testRetrieveValueWithoutKey(String testValue) {
         when(getSecretValueResponse.secretString()).thenReturn(testValue);
-        objectUnderTest = new AwsSecretsSupplier(awsSecretPluginConfig);
+        objectUnderTest = new AwsSecretsSupplier(awsSecretPluginConfig, OBJECT_MAPPER);
         assertThat(objectUnderTest.retrieveValue(TEST_AWS_SECRET_CONFIGURATION_NAME), equalTo(testValue));
     }
 
     @Test
     void testConstructorWithGetSecretValueFailure() {
         when(secretsManagerClient.getSecretValue(eq(getSecretValueRequest))).thenThrow(secretsManagerException);
-        assertThrows(RuntimeException.class, () -> new AwsSecretsSupplier(awsSecretPluginConfig));
+        assertThrows(RuntimeException.class, () -> new AwsSecretsSupplier(awsSecretPluginConfig, OBJECT_MAPPER));
     }
 }