Skip to content

Commit 7075d38

Browse files
chenqi0805divbok
authored andcommitted
ENH: improve regex validaiton message (opensearch-project#5447)
* ENH: improve regex validaiton message Signed-off-by: George Chen <qchea@amazon.com>
1 parent aa18d0e commit 7075d38

14 files changed

Lines changed: 146 additions & 5 deletions

File tree

data-prepper-api/build.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ dependencies {
1414
implementation 'com.fasterxml.jackson.datatype:jackson-datatype-jdk8'
1515
implementation libs.parquet.common
1616
implementation libs.commons.lang3
17+
implementation 'jakarta.validation:jakarta.validation-api:3.0.2'
1718
testImplementation 'com.fasterxml.jackson.dataformat:jackson-dataformat-yaml'
1819
testImplementation project(':data-prepper-test-common')
1920
testImplementation 'org.skyscreamer:jsonassert:1.5.3'
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.dataprepper.model.annotations;
7+
8+
import jakarta.validation.Constraint;
9+
import jakarta.validation.Payload;
10+
import org.opensearch.dataprepper.model.validation.RegexValueValidator;
11+
12+
import java.lang.annotation.ElementType;
13+
import java.lang.annotation.Retention;
14+
import java.lang.annotation.RetentionPolicy;
15+
import java.lang.annotation.Target;
16+
17+
@Constraint(validatedBy = RegexValueValidator.class)
18+
@Target({ElementType.FIELD, ElementType.PARAMETER})
19+
@Retention(RetentionPolicy.RUNTIME)
20+
public @interface ValidRegex {
21+
String message() default "Invalid regular expression pattern";
22+
23+
Class<?>[] groups() default {};
24+
25+
Class<? extends Payload>[] payload() default {};
26+
}

data-prepper-api/src/main/java/org/opensearch/dataprepper/model/plugin/InvalidPluginConfigurationException.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,8 @@ public class InvalidPluginConfigurationException extends RuntimeException {
1515
public InvalidPluginConfigurationException(final String message) {
1616
super(message);
1717
}
18+
19+
public InvalidPluginConfigurationException(final String message, final Throwable cause) {
20+
super(message, cause);
21+
}
1822
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.dataprepper.model.validation;
7+
8+
import jakarta.validation.ConstraintValidator;
9+
import jakarta.validation.ConstraintValidatorContext;
10+
import org.opensearch.dataprepper.model.annotations.ValidRegex;
11+
12+
import java.util.regex.Pattern;
13+
import java.util.regex.PatternSyntaxException;
14+
15+
public class RegexValueValidator implements ConstraintValidator<ValidRegex, String> {
16+
@Override
17+
public boolean isValid(final String pattern, final ConstraintValidatorContext constraintValidatorContext) {
18+
if (pattern != null && !pattern.isEmpty()) {
19+
try {
20+
Pattern.compile(pattern);
21+
} catch (PatternSyntaxException e) {
22+
return false;
23+
}
24+
}
25+
26+
return true;
27+
}
28+
}

data-prepper-api/src/test/java/org/opensearch/dataprepper/model/plugin/InvalidPluginConfigurationExceptionTest.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,20 @@
77

88
import org.junit.jupiter.api.BeforeEach;
99
import org.junit.jupiter.api.Test;
10+
import org.junit.jupiter.api.extension.ExtendWith;
11+
import org.mockito.Mock;
12+
import org.mockito.junit.jupiter.MockitoExtension;
1013

1114
import java.util.UUID;
1215

1316
import static org.hamcrest.CoreMatchers.equalTo;
1417
import static org.hamcrest.MatcherAssert.assertThat;
1518

19+
@ExtendWith(MockitoExtension.class)
1620
class InvalidPluginConfigurationExceptionTest {
21+
@Mock
22+
private Throwable cause;
23+
1724
private String message;
1825

1926
@BeforeEach
@@ -30,4 +37,11 @@ void getMessage_returns_message() {
3037
assertThat(createObjectUnderTest().getMessage(),
3138
equalTo(message));
3239
}
40+
41+
@Test
42+
void getCause_returns_cause() {
43+
final InvalidPluginConfigurationException objectUnderTest = new InvalidPluginConfigurationException(
44+
message, cause);
45+
assertThat(objectUnderTest.getCause(), equalTo(cause));
46+
}
3347
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.dataprepper.model.validation;
7+
8+
import jakarta.validation.ConstraintValidatorContext;
9+
import org.junit.jupiter.api.extension.ExtendWith;
10+
import org.junit.jupiter.params.ParameterizedTest;
11+
import org.junit.jupiter.params.provider.Arguments;
12+
import org.junit.jupiter.params.provider.MethodSource;
13+
import org.mockito.Mock;
14+
import org.mockito.junit.jupiter.MockitoExtension;
15+
16+
import java.util.stream.Stream;
17+
18+
import static org.hamcrest.CoreMatchers.is;
19+
import static org.hamcrest.MatcherAssert.assertThat;
20+
import static org.junit.jupiter.params.provider.Arguments.arguments;
21+
22+
@ExtendWith(MockitoExtension.class)
23+
class RegexValueValidatorTest {
24+
@Mock
25+
private ConstraintValidatorContext constraintValidatorContext;
26+
27+
@ParameterizedTest
28+
@MethodSource("provideRegexAndExpectedResult")
29+
void testValidateRegex(final String delimiterRegex, final boolean isValid) {
30+
final RegexValueValidator objectUnderTest = new RegexValueValidator();
31+
assertThat(objectUnderTest.isValid(delimiterRegex, constraintValidatorContext), is(isValid));
32+
}
33+
34+
private static Stream<Arguments> provideRegexAndExpectedResult() {
35+
return Stream.of(
36+
arguments(null, true),
37+
arguments("", true),
38+
arguments("abc", true),
39+
arguments("(abc", false)
40+
);
41+
}
42+
}

data-prepper-plugins/grok-processor/src/main/java/org/opensearch/dataprepper/plugins/processor/grok/GrokProcessor.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,15 @@ private void compileMatchPatterns() {
267267
for (final Map.Entry<String, List<String>> entry : grokProcessorConfig.getMatch().entrySet()) {
268268
fieldToGrok.put(entry.getKey(), entry.getValue()
269269
.stream()
270-
.map(item -> grokCompiler.compile(item, grokProcessorConfig.isNamedCapturesOnly()))
270+
.map(item -> {
271+
try {
272+
return grokCompiler.compile(item, grokProcessorConfig.isNamedCapturesOnly());
273+
} catch (IllegalArgumentException e) {
274+
throw new InvalidPluginConfigurationException(
275+
String.format("Invalid regular expression pattern in match: %s",
276+
entry.getKey()), e);
277+
}
278+
})
271279
.collect(Collectors.toList()));
272280
}
273281
}

data-prepper-plugins/grok-processor/src/test/java/org/opensearch/dataprepper/plugins/processor/grok/GrokProcessorIT.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import org.opensearch.dataprepper.metrics.PluginMetrics;
2020
import org.opensearch.dataprepper.model.configuration.PluginSetting;
2121
import org.opensearch.dataprepper.model.event.Event;
22+
import org.opensearch.dataprepper.model.plugin.InvalidPluginConfigurationException;
2223
import org.opensearch.dataprepper.model.record.Record;
2324

2425
import java.util.ArrayList;
@@ -30,6 +31,7 @@
3031
import java.util.stream.Stream;
3132

3233
import static org.hamcrest.CoreMatchers.equalTo;
34+
import static org.hamcrest.CoreMatchers.instanceOf;
3335
import static org.hamcrest.CoreMatchers.notNullValue;
3436
import static org.hamcrest.MatcherAssert.assertThat;
3537
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -453,9 +455,11 @@ public void testPatternsDirWithCustomPatternsFilesGlob() throws JsonProcessingEx
453455
pluginSetting.getSettings().put(GrokProcessorConfig.MATCH, matchConfigWithPatterns2Pattern);
454456
grokProcessorConfig = OBJECT_MAPPER.convertValue(pluginSetting.getSettings(), GrokProcessorConfig.class);
455457

456-
Throwable throwable = assertThrows(IllegalArgumentException.class, () -> new GrokProcessor(
458+
Throwable throwable = assertThrows(InvalidPluginConfigurationException.class, () -> new GrokProcessor(
457459
pluginMetrics, grokProcessorConfig, expressionEvaluator));
458-
assertThat("No definition for key 'CUSTOMBIRTHDAYPATTERN' found, aborting", equalTo(throwable.getMessage()));
460+
assertThat(throwable.getCause(), instanceOf(IllegalArgumentException.class));
461+
assertThat("No definition for key 'CUSTOMBIRTHDAYPATTERN' found, aborting", equalTo(throwable
462+
.getCause().getMessage()));
459463
}
460464

461465
@Test
@@ -522,7 +526,7 @@ public void testCompileNonRegisteredPatternThrowsIllegalArgumentException() {
522526
pluginSetting.getSettings().put(GrokProcessorConfig.MATCH, matchConfig);
523527
grokProcessorConfig = OBJECT_MAPPER.convertValue(pluginSetting.getSettings(), GrokProcessorConfig.class);
524528

525-
assertThrows(IllegalArgumentException.class, () -> new GrokProcessor(
529+
assertThrows(InvalidPluginConfigurationException.class, () -> new GrokProcessor(
526530
pluginMetrics, grokProcessorConfig, expressionEvaluator));
527531
}
528532

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.opensearch.dataprepper.model.annotations.AlsoRequired;
1717
import org.opensearch.dataprepper.model.annotations.ExampleValues;
1818
import org.opensearch.dataprepper.model.annotations.ExampleValues.Example;
19+
import org.opensearch.dataprepper.model.annotations.ValidRegex;
1920

2021
import java.util.ArrayList;
2122
import java.util.List;
@@ -65,6 +66,7 @@ public class KeyValueProcessorConfig {
6566
})
6667
private String fieldSplitCharacters = DEFAULT_FIELD_SPLIT_CHARACTERS;
6768

69+
@ValidRegex(message = "The value of field_delimiter_regex is not a valid regex string")
6870
@JsonProperty(FIELD_DELIMITER_REGEX_KEY)
6971
@JsonPropertyDescription("A regular expression specifying the delimiter that separates key-value pairs. " +
7072
"For example, to split on multiple <code>&amp;</code> characters use <code>&amp;+</code>. " +
@@ -93,6 +95,7 @@ public class KeyValueProcessorConfig {
9395
})
9496
private String valueSplitCharacters = DEFAULT_VALUE_SPLIT_CHARACTERS;
9597

98+
@ValidRegex(message = "The value of key_value_delimiter_regex is not a valid regex string")
9699
@JsonProperty(KEY_VALUE_DELIMITER_REGEX_KEY)
97100
@JsonPropertyDescription("A regular expression specifying the delimiter that separates keys from their values within a key-value pair. " +
98101
"For example, to split on multiple <code>=</code> characters use <code>=+</code>. " +
@@ -142,6 +145,7 @@ public class KeyValueProcessorConfig {
142145
})
143146
private String prefix = null;
144147

148+
@ValidRegex(message = "The value of delete_key_regex is not a valid regex string")
145149
@JsonProperty("delete_key_regex")
146150
@JsonPropertyDescription("A regular expression specifying characters to delete from the key. " +
147151
"Special regular expression characters such as <code>[</code> and <code>]</code> must be escaped with <code>\\\\</code>. " +
@@ -152,6 +156,7 @@ public class KeyValueProcessorConfig {
152156
})
153157
private String deleteKeyRegex;
154158

159+
@ValidRegex(message = "The value of delete_value_regex is not a valid regex string")
155160
@JsonProperty("delete_value_regex")
156161
@JsonPropertyDescription("A regular expression specifying characters to delete from the value. " +
157162
"Special regular expression characters such as <code>[</code> and <code>]</code> must be escaped with <code>\\\\</code>. " +

data-prepper-plugins/mutate-event-processors/src/main/java/org/opensearch/dataprepper/plugins/processor/mutateevent/RenameKeyProcessorConfig.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.opensearch.dataprepper.model.annotations.AlsoRequired;
1717
import org.opensearch.dataprepper.model.annotations.ExampleValues;
1818
import org.opensearch.dataprepper.model.annotations.ExampleValues.Example;
19+
import org.opensearch.dataprepper.model.annotations.ValidRegex;
1920
import org.opensearch.dataprepper.model.event.EventKey;
2021
import org.opensearch.dataprepper.model.event.EventKeyConfiguration;
2122
import org.opensearch.dataprepper.model.event.EventKeyFactory;
@@ -40,6 +41,7 @@ public static class Entry {
4041
private EventKey fromKey;
4142

4243

44+
@ValidRegex(message = "The value of from_key_regex is not a valid regex string")
4345
@JsonProperty(defaultValue = FROM_KEY_REGEX)
4446
@JsonPropertyDescription("The regex pattern of the key of the entry to be renamed. " +
4547
"This field cannot be defined along with <code>from_key</code>.")

0 commit comments

Comments
 (0)