Improvements to the schema for the key_value processor.

data-prepper-plugin-schema-cli/src/main/java/org/opensearch/dataprepper/schemas/DataPrepperPluginSchemaExecute.java

@@ -1,13 +1,10 @@
 package org.opensearch.dataprepper.schemas;
 
-import com.github.victools.jsonschema.generator.Module;
 import com.github.victools.jsonschema.generator.OptionPreset;
 import com.github.victools.jsonschema.generator.SchemaVersion;
-import com.github.victools.jsonschema.module.jakarta.validation.JakartaValidationModule;
-import com.github.victools.jsonschema.module.jakarta.validation.JakartaValidationOption;
 import org.opensearch.dataprepper.plugin.ClasspathPluginProvider;
 import org.opensearch.dataprepper.plugin.PluginProvider;
-import org.opensearch.dataprepper.schemas.module.CustomJacksonModule;
+import org.opensearch.dataprepper.schemas.module.DataPrepperModules;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import picocli.CommandLine;
@@ -17,16 +14,11 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Collection;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
-import static com.github.victools.jsonschema.module.jackson.JacksonOption.FLATTENED_ENUMS_FROM_JSONVALUE;
-import static com.github.victools.jsonschema.module.jackson.JacksonOption.RESPECT_JSONPROPERTY_ORDER;
-import static com.github.victools.jsonschema.module.jackson.JacksonOption.RESPECT_JSONPROPERTY_REQUIRED;
-
 public class DataPrepperPluginSchemaExecute implements Runnable {
     private static final Logger LOG = LoggerFactory.getLogger(DataPrepperPluginSchemaExecute.class);
     static final String DEFAULT_PLUGINS_CLASSPATH = "org.opensearch.dataprepper.plugins";
@@ -52,14 +44,9 @@ public static void main(String[] args) {
 
     @Override
     public void run() {
-        final List<Module> modules = List.of(
-                new CustomJacksonModule(RESPECT_JSONPROPERTY_REQUIRED, RESPECT_JSONPROPERTY_ORDER, FLATTENED_ENUMS_FROM_JSONVALUE),
-                new JakartaValidationModule(JakartaValidationOption.NOT_NULLABLE_FIELD_IS_REQUIRED,
-                        JakartaValidationOption.INCLUDE_PATTERN_EXPRESSIONS)
-        );
         final PluginProvider pluginProvider = new ClasspathPluginProvider();
         final PluginConfigsJsonSchemaConverter pluginConfigsJsonSchemaConverter = new PluginConfigsJsonSchemaConverter(
-                pluginProvider, new JsonSchemaConverter(modules, pluginProvider), siteUrl, siteBaseUrl);
+                pluginProvider, new JsonSchemaConverter(DataPrepperModules.dataPrepperModules(), pluginProvider), siteUrl, siteBaseUrl);
         final Class<?> pluginType = pluginConfigsJsonSchemaConverter.pluginTypeNameToPluginType(pluginTypeName);
         final Map<String, String> pluginNameToJsonSchemaMap = pluginConfigsJsonSchemaConverter.convertPluginConfigsIntoJsonSchemas(
                 SchemaVersion.DRAFT_2020_12, OptionPreset.PLAIN_JSON, pluginType);

data-prepper-plugin-schema/src/main/java/org/opensearch/dataprepper/schemas/module/DataPrepperJakartaValidationModule.java

@@ -0,0 +1,34 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.opensearch.dataprepper.schemas.module;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.github.victools.jsonschema.generator.MemberScope;
+import com.github.victools.jsonschema.module.jakarta.validation.JakartaValidationModule;
+import com.github.victools.jsonschema.module.jakarta.validation.JakartaValidationOption;
+
+/**
+ * Custom {@link JakartaValidationModule} which overrides the default behavior of {@link JakartaValidationModule}
+ * for Data Prepper.
+ * It considers the {@link JsonProperty} annotation as well as the Jakarta validations.
+ */
+class DataPrepperJakartaValidationModule extends JakartaValidationModule {
+    public DataPrepperJakartaValidationModule(final JakartaValidationOption... options) {
+        super(options);
+    }
+
+    @Override
+    protected boolean isRequired(final MemberScope<?, ?> member) {
+        final JsonProperty jsonPropertyAnnotation = member.getAnnotationConsideringFieldAndGetter(JsonProperty.class);
+        if (jsonPropertyAnnotation != null) {
+            if (jsonPropertyAnnotation.defaultValue() != null && !jsonPropertyAnnotation.defaultValue().isEmpty()) {
+                return false;
+            }
+        }
+
+        return super.isRequired(member);
+    }
+}

data-prepper-plugin-schema/src/main/java/org/opensearch/dataprepper/schemas/module/DataPrepperModules.java

@@ -0,0 +1,25 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.opensearch.dataprepper.schemas.module;
+
+import com.github.victools.jsonschema.generator.Module;
+import com.github.victools.jsonschema.module.jakarta.validation.JakartaValidationOption;
+
+import java.util.List;
+
+import static com.github.victools.jsonschema.module.jackson.JacksonOption.FLATTENED_ENUMS_FROM_JSONVALUE;
+import static com.github.victools.jsonschema.module.jackson.JacksonOption.RESPECT_JSONPROPERTY_ORDER;
+import static com.github.victools.jsonschema.module.jackson.JacksonOption.RESPECT_JSONPROPERTY_REQUIRED;
+
+public class DataPrepperModules {
+    public static List<Module> dataPrepperModules() {
+        return List.of(
+                new DataPrepperJakartaValidationModule(JakartaValidationOption.NOT_NULLABLE_FIELD_IS_REQUIRED,
+                        JakartaValidationOption.INCLUDE_PATTERN_EXPRESSIONS),
+                new CustomJacksonModule(RESPECT_JSONPROPERTY_REQUIRED, RESPECT_JSONPROPERTY_ORDER, FLATTENED_ENUMS_FROM_JSONVALUE)
+        );
+    }
+}

data-prepper-plugin-schema/src/test/java/org/opensearch/dataprepper/schemas/module/DataPrepperJakartaValidationModuleTest.java

@@ -0,0 +1,113 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.opensearch.dataprepper.schemas.module;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.github.victools.jsonschema.generator.MemberScope;
+import com.github.victools.jsonschema.module.jakarta.validation.JakartaValidationModule;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.junit.platform.commons.support.ReflectionSupport;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import java.util.UUID;
+
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+class DataPrepperJakartaValidationModuleTest {
+
+    @Mock
+    private MemberScope memberScope;
+
+    DataPrepperJakartaValidationModule createObjectUnderTest() {
+        return spy(new DataPrepperJakartaValidationModule());
+    }
+
+    @ParameterizedTest
+    @ValueSource(booleans = {true, false})
+    void isRequired_returns_inner_if_no_JsonProperty_annotation(final boolean innerIsRequired) throws NoSuchMethodException {
+        final DataPrepperJakartaValidationModule objectUnderTest = createObjectUnderTest();
+
+        final boolean isNullable = !innerIsRequired;
+        ReflectionSupport.invokeMethod(
+                JakartaValidationModule.class.getDeclaredMethod("isNullable", MemberScope.class),
+                doReturn(isNullable).when((JakartaValidationModule) objectUnderTest),
+                memberScope);
+
+        assertThat(objectUnderTest.isRequired(memberScope), equalTo(innerIsRequired));
+    }
+
+    @Nested
+    class WithJsonProperty {
+        private JsonProperty jsonPropertyAnnotation;
+
+        @BeforeEach
+        void setUp() {
+            jsonPropertyAnnotation = mock(JsonProperty.class);
+            when(memberScope.getAnnotationConsideringFieldAndGetter(JsonProperty.class))
+                    .thenReturn(jsonPropertyAnnotation);
+        }
+
+        @ParameterizedTest
+        @ValueSource(booleans = {true, false})
+        void isRequired_returns_inner_if_JsonProperty_annotation_has_null_defaultValue(final boolean innerIsRequired) throws NoSuchMethodException {
+            final DataPrepperJakartaValidationModule objectUnderTest = createObjectUnderTest();
+
+            final boolean isNullable = !innerIsRequired;
+            ReflectionSupport.invokeMethod(
+                    JakartaValidationModule.class.getDeclaredMethod("isNullable", MemberScope.class),
+                    doReturn(isNullable).when((JakartaValidationModule) objectUnderTest),
+                    memberScope);
+
+            when(jsonPropertyAnnotation.defaultValue()).thenReturn(null);
+
+            assertThat(objectUnderTest.isRequired(memberScope), equalTo(innerIsRequired));
+        }
+
+        @ParameterizedTest
+        @ValueSource(booleans = {true, false})
+        void isRequired_returns_inner_if_JsonProperty_annotation_has_empty_defaultValue(final boolean innerIsRequired) throws NoSuchMethodException {
+            final DataPrepperJakartaValidationModule objectUnderTest = createObjectUnderTest();
+
+            final boolean isNullable = !innerIsRequired;
+            ReflectionSupport.invokeMethod(
+                    JakartaValidationModule.class.getDeclaredMethod("isNullable", MemberScope.class),
+                    doReturn(isNullable).when((JakartaValidationModule) objectUnderTest),
+                    memberScope);
+
+            when(jsonPropertyAnnotation.defaultValue()).thenReturn("");
+
+            assertThat(objectUnderTest.isRequired(memberScope), equalTo(innerIsRequired));
+        }
+
+        @Test
+        void isRequired_returns_false_if_JsonProperty_has_default_value() throws NoSuchMethodException {
+            final DataPrepperJakartaValidationModule objectUnderTest = createObjectUnderTest();
+
+            when(jsonPropertyAnnotation.defaultValue()).thenReturn(UUID.randomUUID().toString());
+
+            assertThat(createObjectUnderTest().isRequired(memberScope), equalTo(false));
+
+            ReflectionSupport.invokeMethod(
+                    JakartaValidationModule.class.getDeclaredMethod("isNullable", MemberScope.class),
+                    verify(objectUnderTest, never()),
+                    memberScope);
+        }
+    }
+}

data-prepper-plugins/key-value-processor/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessor.java

@@ -62,6 +62,7 @@ public class KeyValueProcessor extends AbstractProcessor<Record<Event>, Record<E
     private final Set<Character> bracketSet = Set.of('[', ']', '(', ')', '<', '>');
     private final List<String> tagsOnFailure;
     private final Character stringLiteralCharacter;
+    private final String keyPrefix;
 
     @DataPrepperPluginConstructor
     public KeyValueProcessor(final PluginMetrics pluginMetrics,
@@ -190,6 +191,8 @@ public KeyValueProcessor(final PluginMetrics pluginMetrics,
             throw new InvalidPluginConfigurationException(
                     String.format("key_value_when %s is not a valid expression statement", keyValueProcessorConfig.getKeyValueWhen()));
         }
+
+        keyPrefix = keyValueProcessorConfig.getPrefix() != null ? keyValueProcessorConfig.getPrefix() : "";
     }
 
     private String buildRegexFromCharacters(String s) {
@@ -575,7 +578,7 @@ private Map<String, Object> executeConfigs(Map<String, Object> map) {
             if (keyValueProcessorConfig.getDeleteKeyRegex() != null && !Objects.equals(keyValueProcessorConfig.getDeleteKeyRegex(), "")) {
                 key = key.replaceAll(keyValueProcessorConfig.getDeleteKeyRegex(), "");
             }
-            key = keyValueProcessorConfig.getPrefix() + key;
+            key = keyPrefix + key;
 
             if (value != null
                     && value instanceof String

data-prepper-plugins/key-value-processor/src/main/java/org/opensearch/dataprepper/plugins/processor/keyvalue/KeyValueProcessorConfig.java

@@ -29,16 +29,9 @@ public class KeyValueProcessorConfig {
     static final Map<String, Object> DEFAULT_DEFAULT_VALUES = Map.of();
     public static final String DEFAULT_VALUE_SPLIT_CHARACTERS = "=";
     static final Object DEFAULT_NON_MATCH_VALUE = null;
-    static final String DEFAULT_PREFIX = "";
-    static final String DEFAULT_DELETE_KEY_REGEX = "";
-    static final String DEFAULT_DELETE_VALUE_REGEX = "";
-    static final WhitespaceOption DEFAULT_WHITESPACE = WhitespaceOption.LENIENT;
-    static final boolean DEFAULT_SKIP_DUPLICATE_VALUES = false;
-    static final boolean DEFAULT_REMOVE_BRACKETS = false;
-    static final boolean DEFAULT_VALUE_GROUPING = false;
-    static final boolean DEFAULT_RECURSIVE = false;
 
     @NotEmpty
+    @JsonProperty(defaultValue = DEFAULT_SOURCE)
     @JsonPropertyDescription("The source field to parse for key-value pairs. The default value is <code>message</code>.")
     private String source = DEFAULT_SOURCE;
 
@@ -56,6 +49,7 @@ public class KeyValueProcessorConfig {
 
     @JsonProperty("field_delimiter_regex")
     @JsonPropertyDescription("A regular expression specifying the delimiter that separates key-value pairs. " +
+            "For example, to split on multiple <code>&amp;</code> characters use <code>&amp;+</code>. " +
             "Special regular expression characters such as <code>[</code> and <code>]</code> must be escaped with <code>\\\\</code>. " +
             "This field cannot be defined along with <code>field_split_characters</code>. " +
             "If this option is not defined, the <code>key_value</code> processor will parse the source using <code>field_split_characters</code>.")
@@ -70,12 +64,13 @@ public class KeyValueProcessorConfig {
 
     @JsonProperty("key_value_delimiter_regex")
     @JsonPropertyDescription("A regular expression specifying the delimiter that separates keys from their values within a key-value pair. " +
+            "For example, to split on multiple <code>=</code> characters use <code>=+</code>. " +
             "Special regular expression characters such as <code>[</code> and <code>]</code> must be escaped with <code>\\\\</code>. " +
             "This field cannot be defined along with <code>value_split_characters</code>. " +
             "If this option is not defined, the <code>key_value</code> processor will parse the source using <code>value_split_characters</code>.")
     private String keyValueDelimiterRegex;
 
-    @JsonProperty("default_values")
+    @JsonProperty(value = "default_values", defaultValue = "{}")
     @JsonPropertyDescription("A map specifying the default keys and their values that should be added " +
             "to the event in case these keys do not exist in the source field being parsed. " +
             "If the key was parsed from the source field that value will remain and the default value is not used. " +
@@ -89,63 +84,58 @@ public class KeyValueProcessorConfig {
             "The default behavior is to drop the key-value pair.")
     private Object nonMatchValue = DEFAULT_NON_MATCH_VALUE;
 
-    @JsonProperty("include_keys")
+    @JsonProperty(value = "include_keys", defaultValue = "[]")
     @JsonPropertyDescription("An array specifying the keys that should be included in the destination field. " +
             "By default, all keys will be added.")
     @NotNull
     private List<String> includeKeys = DEFAULT_INCLUDE_KEYS;
 
-    @JsonProperty("exclude_keys")
+    @JsonProperty(value = "exclude_keys", defaultValue = "[]")
     @JsonPropertyDescription("An array specifying the parsed keys that should be excluded from the destination field. " +
             "By default, no keys will be excluded.")
     @NotNull
     private List<String> excludeKeys = DEFAULT_EXCLUDE_KEYS;
 
     @JsonPropertyDescription("A prefix to append before all keys. By default no prefix is added.")
-    @NotNull
-    private String prefix = DEFAULT_PREFIX;
+    private String prefix = null;
 
     @JsonProperty("delete_key_regex")
     @JsonPropertyDescription("A regular expression specifying characters to delete from the key. " +
             "Special regular expression characters such as <code>[</code> and <code>]</code> must be escaped with <code>\\\\</code>. " +
             "Cannot be an empty string. " +
             "By default, no characters are deleted from the key.")
-    @NotNull
-    private String deleteKeyRegex = DEFAULT_DELETE_KEY_REGEX;
+    private String deleteKeyRegex;
 
     @JsonProperty("delete_value_regex")
     @JsonPropertyDescription("A regular expression specifying characters to delete from the value. " +
             "Special regular expression characters such as <code>[</code> and <code>]</code> must be escaped with <code>\\\\</code>. " +
             "Cannot be an empty string. " +
             "By default, no characters are deleted from the value.")
-    @NotNull
-    private String deleteValueRegex = DEFAULT_DELETE_VALUE_REGEX;
+    private String deleteValueRegex;
 
-    @JsonProperty("transform_key")
+    @JsonProperty(value = "transform_key", defaultValue = "none")
     @JsonPropertyDescription("Allows transforming the key's name such as making the name all lowercase.")
-    @NotNull
     private TransformOption transformKey = TransformOption.NONE;
 
-    @JsonProperty("whitespace")
+    @JsonProperty(value = "whitespace", defaultValue = "lenient")
     @JsonPropertyDescription("Specifies whether to be lenient or strict with the acceptance of " +
             "unnecessary white space surrounding the configured value-split sequence. " +
             "In this case, strict means that whitespace is trimmed and lenient means it is retained in the key name and in the value." +
             "Default is <code>lenient</code>.")
     @NotNull
-    private WhitespaceOption whitespace = DEFAULT_WHITESPACE;
+    private WhitespaceOption whitespace = WhitespaceOption.LENIENT;
 
-    @JsonProperty("skip_duplicate_values")
+    @JsonProperty(value = "skip_duplicate_values", defaultValue = "false")
     @JsonPropertyDescription("A Boolean option for removing duplicate key-value pairs. When set to <code>true</code>, " +
             "only one unique key-value pair will be preserved. Default is <code>false</code>.")
     @NotNull
-    private boolean skipDuplicateValues = DEFAULT_SKIP_DUPLICATE_VALUES;
+    private boolean skipDuplicateValues = false;
 
-    @JsonProperty("remove_brackets")
+    @JsonProperty(value = "remove_brackets", defaultValue = "false")
     @JsonPropertyDescription("Specifies whether to treat certain grouping characters as wrapping text that should be removed from values." +
             "When set to <code>true</code>, the following grouping characters will be removed: square brackets, angle brackets, and parentheses. " +
             "The default configuration is <code>false</code> which retains those grouping characters.")
-    @NotNull
-    private boolean removeBrackets = DEFAULT_REMOVE_BRACKETS;
+    private boolean removeBrackets;
 
     @JsonProperty("value_grouping")
     @JsonPropertyDescription("Specifies whether to group values using predefined grouping delimiters. " +
@@ -154,9 +144,9 @@ public class KeyValueProcessorConfig {
             "<code>{...}</code>, <code>[...]</code>, <code>&lt;...&gt;</code>, <code>(...)</code>, <code>\"...\"</code>, <code>'...'</code>, <code>http://... (space)</code>, and <code>https:// (space)</code>. " +
             "Default is <code>false</code>. For example, if <code>value_grouping</code> is <code>true</code>, then " +
             "<code>{\"key1=[a=b,c=d]&amp;key2=value2\"}</code> parses to <code>{\"key1\": \"[a=b,c=d]\", \"key2\": \"value2\"}</code>.")
-    private boolean valueGrouping = DEFAULT_VALUE_GROUPING;
+    private boolean valueGrouping = false;
 
-    @JsonProperty("recursive")
+    @JsonProperty(value = "recursive", defaultValue = "false")
     @JsonPropertyDescription("Specifies whether to recursively obtain additional key-value pairs from values. " +
             "The extra key-value pairs will be stored as nested objects within the destination object. Default is <code>false</code>. " +
             "The levels of recursive parsing must be defined by different brackets for each level: " +
@@ -166,8 +156,7 @@ public class KeyValueProcessorConfig {
             "<code>remove_brackets</code> cannot also be <code>true</code>;\n" +
             "<code>skip_duplicate_values</code> will always be <code>true</code>;\n" +
             "<code>whitespace</code> will always be <code>\"strict\"</code>.")
-    @NotNull
-    private boolean recursive = DEFAULT_RECURSIVE;
+    private boolean recursive = false;
 
     @JsonProperty("overwrite_if_destination_exists")
     @JsonPropertyDescription("Specifies whether to overwrite existing fields if there are key conflicts " +