Skip to content

Commit

Permalink
Enhancements to map_to_list processor (#4033)
Browse files Browse the repository at this point in the history 
* Add convert-field-to-list option

---------

Signed-off-by: Hai Yan <oeyh@amazon.com>
  • Loading branch information
@oeyh
oeyh committed Feb 14, 2024
1 parent a8024e9 commit ab58c96
Show file tree
Hide file tree
Showing 6 changed files with 407 additions and 56 deletions.
26 changes: 20 additions & 6 deletions data-prepper-api/src/main/java/org/opensearch/dataprepper/model/event/JacksonEvent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,11 @@ public JsonNode getJsonNode() {
*/
@Override
public void put(final String key, final Object value) {
checkArgument(!key.isEmpty(), "key cannot be an empty string for put method");

final String trimmedKey = checkAndTrimKey(key);

final LinkedList<String> keys = new LinkedList<>(Arrays.asList(trimmedKey.split(SEPARATOR)));
final LinkedList<String> keys = new LinkedList<>(Arrays.asList(trimmedKey.split(SEPARATOR, -1)));

JsonNode parentNode = jsonNode;

Expand Down Expand Up @@ -247,7 +248,12 @@ private <T> List<T> mapNodeToList(final String key, final JsonNode node, final C
}

private JsonPointer toJsonPointer(final String key) {
String jsonPointerExpression = SEPARATOR + key;
final String jsonPointerExpression;
if (key.isEmpty() || key.startsWith("/")) {
jsonPointerExpression = key;
} else {
jsonPointerExpression = SEPARATOR + key;
}
return JsonPointer.compile(jsonPointerExpression);
}

Expand All @@ -259,6 +265,7 @@ private JsonPointer toJsonPointer(final String key) {
@Override
public void delete(final String key) {

checkArgument(!key.isEmpty(), "key cannot be an empty string for delete method");
final String trimmedKey = checkAndTrimKey(key);
final int index = trimmedKey.lastIndexOf(SEPARATOR);

Expand Down Expand Up @@ -399,24 +406,31 @@ public static boolean isValidEventKey(final String key) {
}
private String checkAndTrimKey(final String key) {
checkKey(key);
return trimKey(key);
return trimTrailingSlashInKey(key);
}

private static void checkKey(final String key) {
checkNotNull(key, "key cannot be null");
checkArgument(!key.isEmpty(), "key cannot be an empty string");
if (key.isEmpty()) {
// Empty string key is valid
return;
}
if (key.length() > MAX_KEY_LENGTH) {
throw new IllegalArgumentException("key cannot be longer than " + MAX_KEY_LENGTH + " characters");
}
if (!isValidKey(key)) {
throw new IllegalArgumentException("key " + key + " must contain only alphanumeric chars with .-_ and must follow JsonPointer (ie. 'field/to/key')");
throw new IllegalArgumentException("key " + key + " must contain only alphanumeric chars with .-_@/ and must follow JsonPointer (ie. 'field/to/key')");
}
}

private String trimKey(final String key) {

final String trimmedLeadingSlash = key.startsWith(SEPARATOR) ? key.substring(1) : key;
return trimmedLeadingSlash.endsWith(SEPARATOR) ? trimmedLeadingSlash.substring(0, trimmedLeadingSlash.length() - 2) : trimmedLeadingSlash;
return trimTrailingSlashInKey(trimmedLeadingSlash);
}

private String trimTrailingSlashInKey(final String key) {
return key.length() > 1 && key.endsWith(SEPARATOR) ? key.substring(0, key.length() - 1) : key;
}

private static boolean isValidKey(final String key) {
Expand Down
81 changes: 53 additions & 28 deletions data-prepper-api/src/test/java/org/opensearch/dataprepper/model/event/JacksonEventTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.Random;
import java.util.UUID;

import static org.hamcrest.CoreMatchers.containsStringIgnoringCase;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.not;
Expand Down Expand Up @@ -70,7 +71,7 @@ public void testPutAndGet_withRandomString() {
}

@ParameterizedTest
@ValueSource(strings = {"foo", "foo-bar", "foo_bar", "foo.bar", "/foo", "/foo/", "a1K.k3-01_02"})
@ValueSource(strings = {"/", "foo", "foo-bar", "foo_bar", "foo.bar", "/foo", "/foo/", "a1K.k3-01_02"})
void testPutAndGet_withStrings(final String key) {
final UUID value = UUID.randomUUID();

Expand All @@ -81,6 +82,12 @@ void testPutAndGet_withStrings(final String key) {
assertThat(result, is(equalTo(value)));
}

@Test
public void testPutKeyCannotBeEmptyString() {
Throwable exception = assertThrows(IllegalArgumentException.class, () -> event.put("", "value"));
assertThat(exception.getMessage(), containsStringIgnoringCase("key cannot be an empty string"));
}

@Test
public void testPutAndGet_withMultLevelKey() {
final String key = "foo/bar";
Expand Down Expand Up @@ -126,6 +133,28 @@ public void testPutAndGet_withMultiLevelKeyWithADash() {
assertThat(result, is(equalTo(value)));
}

@ParameterizedTest
@ValueSource(strings = {"foo", "/foo", "/foo/", "foo/"})
void testGetAtRootLevel(final String key) {
final String value = UUID.randomUUID().toString();

event.put(key, value);
final Map<String, String> result = event.get("", Map.class);

assertThat(result, is(Map.of("foo", value)));
}

@ParameterizedTest
@ValueSource(strings = {"/foo/bar", "foo/bar", "foo/bar/"})
void testGetAtRootLevelWithMultiLevelKey(final String key) {
final String value = UUID.randomUUID().toString();

event.put(key, value);
final Map<String, String> result = event.get("", Map.class);

assertThat(result, is(Map.of("foo", Map.of("bar", value))));
}

@Test
public void testPutUpdateAndGet_withPojo() {
final String key = "foo/bar";
Expand Down Expand Up @@ -250,31 +279,19 @@ public void testOverwritingExistingKey() {
assertThat(result, is(equalTo(value)));
}

@Test
public void testDeletingKey() {
final String key = "foo";

event.put(key, UUID.randomUUID());
event.delete(key);
final UUID result = event.get(key, UUID.class);

assertThat(result, is(nullValue()));
}

@Test
public void testDelete_withNestedKey() {
final String key = "foo/bar";

@ParameterizedTest
@ValueSource(strings = {"/", "foo", "/foo", "/foo/bar", "foo/bar", "foo/bar/", "/foo/bar/leaf/key"})
public void testDeleteKey(final String key) {
event.put(key, UUID.randomUUID());
event.delete(key);
final UUID result = event.get(key, UUID.class);

assertThat(result, is(nullValue()));
}

@Test
public void testDelete_withNonexistentKey() {
final String key = "foo/bar";
@ParameterizedTest
@ValueSource(strings = {"/", "foo", "/foo", "/foo/bar", "foo/bar", "foo/bar/", "/foo/bar/leaf/key"})
public void testDelete_withNonexistentKey(final String key) {
UUID result = event.get(key, UUID.class);
assertThat(result, is(nullValue()));

Expand All @@ -285,19 +302,27 @@ public void testDelete_withNonexistentKey() {
}

@Test
public void testContainsKey_withKey() {
final String key = "foo";

event.put(key, UUID.randomUUID());
assertThat(event.containsKey(key), is(true));
public void testDeleteKeyCannotBeEmptyString() {
Throwable exception = assertThrows(IllegalArgumentException.class, () -> event.delete(""));
assertThat(exception.getMessage(), containsStringIgnoringCase("key cannot be an empty string"));
}

@Test
public void testContainsKey_withouthKey() {
final String key = "foo";
public void testContainsKeyReturnsTrueForEmptyStringKey() {
assertThat(event.containsKey(""), is(true));
}

@ParameterizedTest
@ValueSource(strings = {"/", "foo", "/foo", "/foo/bar", "foo/bar", "foo/bar/", "/foo/bar/leaf/key"})
public void testContainsKey_withKey(final String key) {
event.put(key, UUID.randomUUID());
assertThat(event.containsKey("bar"), is(false));
assertThat(event.containsKey(key), is(true));
}

@ParameterizedTest
@ValueSource(strings = {"/", "foo", "/foo", "/foo/bar", "foo/bar", "foo/bar/", "/foo/bar/leaf/key"})
public void testContainsKey_withouthKey(final String key) {
assertThat(event.containsKey(key), is(false));
}

@Test
Expand All @@ -324,7 +349,7 @@ public void testIsValueAList_withNull() {
}

@ParameterizedTest
@ValueSource(strings = {"", "withSpecialChars*$%", "\\-withEscapeChars", "\\\\/withMultipleEscapeChars",
@ValueSource(strings = {"withSpecialChars*$%", "\\-withEscapeChars", "\\\\/withMultipleEscapeChars",
"with,Comma", "with:Colon", "with[Bracket", "with|Brace"})
void testKey_withInvalidKey_throwsIllegalArgumentException(final String invalidKey) {
assertThrowsForKeyCheck(IllegalArgumentException.class, invalidKey);
Expand Down
62 changes: 61 additions & 1 deletion data-prepper-plugins/mutate-event-processors/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -577,14 +577,74 @@ The processed event will have the following data:
}
```

If we enable `convert_field_to_list` option:
```yaml
...
processor:
- map_to_list:
source: "my-map"
target: "my-list"
convert_field_to_list: true
...
```
the processed event will have the following data:
```json
{
"my-list": [
["key1", "value1"],
["key2", "value2"],
["key3", "value3"]
],
"my-map": {
"key1": "value1",
"key2": "value2",
"key3": "value3"
}
}
```

If source is set to empty string (""), it will use the event root as source.
```yaml
...
processor:
- map_to_list:
source: ""
target: "my-list"
convert_field_to_list: true
...
```
Input data like this:
```json
{
"key1": "value1",
"key2": "value2",
"key3": "value3"
}
```
will end up with this after processing:
```json
{
"my-list": [
["key1", "value1"],
["key2", "value2"],
["key3", "value3"]
],
"key1": "value1",
"key2": "value2",
"key3": "value3"
}
```

### Configuration
* `source` - (required): the source map to perform the operation
* `source` - (required): the source map to perform the operation; If set to empty string (""), it will use the event root as source.
* `target` - (required): the target list to put the converted list
* `key_name` - (optional): the key name of the field to hold the original key, default is "key"
* `value_name` - (optional): the key name of the field to hold the original value, default is "value"
* `exclude_keys` - (optional): the keys in source map that will be excluded from processing, default is empty list
* `remove_processed_fields` - (optional): default is false; if true, will remove processed fields from source map
* `map_to_list_when` - (optional): used to configure a condition for event processing based on certain property of the incoming event. Default is null (all events will be processed).
* `convert_field_to_list` - (optional): default to false; if true, will convert fields to lists instead of objects
* `tags_on_failure` - (optional): a list of tags to add to event metadata when the event fails to process


## Developer Guide
Expand Down
Loading

0 comments on commit ab58c96

Please sign in to comment.