From c29f002653ee36b73136e9eacbfe43f89b5c84fe Mon Sep 17 00:00:00 2001 From: Taylor Gray Date: Fri, 25 Oct 2024 16:48:31 -0500 Subject: [PATCH 1/2] Revert "Add jsonValue check on Enum deserializer, default to checking name() function of enum (#5103)" This reverts commit 9065acbdbd72c89772a4a7679355626c3cd732a2. --- .../integration/PipelinesWithAcksIT.java | 2 - .../core/parser/PipelineTransformer.java | 4 +- .../core/parser/PipelineTransformerTests.java | 7 +- .../pipeline/parser/EnumDeserializer.java | 12 ++-- .../pipeline/parser/EnumDeserializerTest.java | 64 ++----------------- .../plugin/ObjectMapperConfiguration.java | 1 - 6 files changed, 16 insertions(+), 74 deletions(-) diff --git a/data-prepper-core/src/integrationTest/java/org/opensearch/dataprepper/integration/PipelinesWithAcksIT.java b/data-prepper-core/src/integrationTest/java/org/opensearch/dataprepper/integration/PipelinesWithAcksIT.java index e1893dffa7..4b5c4106d8 100644 --- a/data-prepper-core/src/integrationTest/java/org/opensearch/dataprepper/integration/PipelinesWithAcksIT.java +++ b/data-prepper-core/src/integrationTest/java/org/opensearch/dataprepper/integration/PipelinesWithAcksIT.java @@ -5,7 +5,6 @@ package org.opensearch.dataprepper.integration; -import org.junit.FixMethodOrder; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.opensearch.dataprepper.model.event.Event; @@ -29,7 +28,6 @@ import static org.junit.Assert.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; -@FixMethodOrder() class PipelinesWithAcksIT { private static final Logger LOG = LoggerFactory.getLogger(PipelinesWithAcksIT.class); private static final String IN_MEMORY_IDENTIFIER = "PipelinesWithAcksIT"; diff --git a/data-prepper-core/src/main/java/org/opensearch/dataprepper/core/parser/PipelineTransformer.java b/data-prepper-core/src/main/java/org/opensearch/dataprepper/core/parser/PipelineTransformer.java index 374b9629c9..c8c7f2ed5e 100644 --- a/data-prepper-core/src/main/java/org/opensearch/dataprepper/core/parser/PipelineTransformer.java +++ b/data-prepper-core/src/main/java/org/opensearch/dataprepper/core/parser/PipelineTransformer.java @@ -151,9 +151,7 @@ private void buildPipelineFromConfiguration( Buffer pipelineDefinedBuffer = null; final PluginSetting bufferPluginSetting = pipelineConfiguration.getBufferPluginSetting(); try { - if (source != null) { - pipelineDefinedBuffer = pluginFactory.loadPlugin(Buffer.class, bufferPluginSetting, source.getDecoder()); - } + pipelineDefinedBuffer = pluginFactory.loadPlugin(Buffer.class, bufferPluginSetting, source.getDecoder()); } catch (Exception e) { final PluginError pluginError = PluginError.builder() .componentType(PipelineModel.BUFFER_PLUGIN_TYPE) diff --git a/data-prepper-core/src/test/java/org/opensearch/dataprepper/core/parser/PipelineTransformerTests.java b/data-prepper-core/src/test/java/org/opensearch/dataprepper/core/parser/PipelineTransformerTests.java index 9caa18820f..2725bcbd26 100644 --- a/data-prepper-core/src/test/java/org/opensearch/dataprepper/core/parser/PipelineTransformerTests.java +++ b/data-prepper-core/src/test/java/org/opensearch/dataprepper/core/parser/PipelineTransformerTests.java @@ -222,11 +222,16 @@ void parseConfiguration_with_invalid_root_source_pipeline_creates_empty_pipeline final Map connectedPipelines = pipelineTransformer.transformConfiguration(); assertThat(connectedPipelines.size(), equalTo(0)); verify(dataPrepperConfiguration).getPipelineExtensions(); - assertThat(pluginErrorCollector.getPluginErrors().size(), equalTo(1)); + assertThat(pluginErrorCollector.getPluginErrors().size(), equalTo(2)); final PluginError sourcePluginError = pluginErrorCollector.getPluginErrors().get(0); assertThat(sourcePluginError.getPipelineName(), equalTo("test-pipeline-1")); assertThat(sourcePluginError.getPluginName(), equalTo("file")); assertThat(sourcePluginError.getException(), notNullValue()); + // Buffer plugin gets error due to instantiated source is null + final PluginError bufferPluginError = pluginErrorCollector.getPluginErrors().get(1); + assertThat(bufferPluginError.getPipelineName(), equalTo("test-pipeline-1")); + assertThat(bufferPluginError.getPluginName(), equalTo("bounded_blocking")); + assertThat(bufferPluginError.getException(), notNullValue()); } @ParameterizedTest diff --git a/data-prepper-pipeline-parser/src/main/java/org/opensearch/dataprepper/pipeline/parser/EnumDeserializer.java b/data-prepper-pipeline-parser/src/main/java/org/opensearch/dataprepper/pipeline/parser/EnumDeserializer.java index e31fe6314f..e95f10b962 100644 --- a/data-prepper-pipeline-parser/src/main/java/org/opensearch/dataprepper/pipeline/parser/EnumDeserializer.java +++ b/data-prepper-pipeline-parser/src/main/java/org/opensearch/dataprepper/pipeline/parser/EnumDeserializer.java @@ -44,7 +44,6 @@ public Enum deserialize(final JsonParser p, final DeserializationContext ctxt final String enumValue = node.asText(); final Optional jsonCreator = findJsonCreatorMethod(); - final Optional jsonValueMethod = findJsonValueMethodForClass(); try { jsonCreator.ifPresent(method -> method.setAccessible(true)); @@ -53,11 +52,7 @@ public Enum deserialize(final JsonParser p, final DeserializationContext ctxt try { if (jsonCreator.isPresent() && enumConstant.equals(jsonCreator.get().invoke(null, enumValue))) { return (Enum) enumConstant; - } else if (jsonCreator.isEmpty() && jsonValueMethod.isPresent() - && jsonValueMethod.get().invoke(enumConstant).equals(enumValue)) { - return (Enum) enumConstant; - } else if (jsonCreator.isEmpty() && jsonValueMethod.isEmpty() && - ((Enum) enumConstant).name().equals(enumValue)) { + } else if (jsonCreator.isEmpty() && enumConstant.toString().toLowerCase().equals(enumValue)) { return (Enum) enumConstant; } } catch (IllegalAccessException | InvocationTargetException e) { @@ -68,6 +63,9 @@ public Enum deserialize(final JsonParser p, final DeserializationContext ctxt jsonCreator.ifPresent(method -> method.setAccessible(false)); } + + + final Optional jsonValueMethod = findJsonValueMethodForClass(); final List listOfEnums = jsonValueMethod.map(method -> Arrays.stream(enumClass.getEnumConstants()) .map(valueEnum -> { try { @@ -77,7 +75,7 @@ public Enum deserialize(final JsonParser p, final DeserializationContext ctxt } }) .collect(Collectors.toList())).orElseGet(() -> Arrays.stream(enumClass.getEnumConstants()) - .map(valueEnum -> ((Enum) valueEnum).name()) + .map(valueEnum -> valueEnum.toString().toLowerCase()) .collect(Collectors.toList())); throw new IllegalArgumentException(String.format(INVALID_ENUM_VALUE_ERROR_FORMAT, enumValue, listOfEnums)); diff --git a/data-prepper-pipeline-parser/src/test/java/org/opensearch/dataprepper/pipeline/parser/EnumDeserializerTest.java b/data-prepper-pipeline-parser/src/test/java/org/opensearch/dataprepper/pipeline/parser/EnumDeserializerTest.java index 1276ccad7a..990fd60e7d 100644 --- a/data-prepper-pipeline-parser/src/test/java/org/opensearch/dataprepper/pipeline/parser/EnumDeserializerTest.java +++ b/data-prepper-pipeline-parser/src/test/java/org/opensearch/dataprepper/pipeline/parser/EnumDeserializerTest.java @@ -66,45 +66,15 @@ void enum_class_with_json_creator_annotation_returns_expected_enum_constant(fina assertThat(result, equalTo(testEnumOption)); } - @ParameterizedTest - @EnumSource(TestEnumWithJsonValue.class) - void enum_class_with_no_json_creator_and_a_json_value_annotation_returns_expected_enum_constant(final TestEnumWithJsonValue testEnumOption) throws IOException { - final EnumDeserializer objectUnderTest = createObjectUnderTest(TestEnumWithJsonValue.class); - final JsonParser jsonParser = mock(JsonParser.class); - final DeserializationContext deserializationContext = mock(DeserializationContext.class); - when(jsonParser.getCodec()).thenReturn(objectMapper); - - when(objectMapper.readTree(jsonParser)).thenReturn(new TextNode(testEnumOption.toString())); - - Enum result = objectUnderTest.deserialize(jsonParser, deserializationContext); - - assertThat(result, equalTo(testEnumOption)); - } - - @ParameterizedTest - @EnumSource(TestEnumOnlyUppercase.class) - void enum_class_with_just_enum_values_returns_expected_enum_constant(final TestEnumOnlyUppercase testEnumOption) throws IOException { - final EnumDeserializer objectUnderTest = createObjectUnderTest(TestEnumOnlyUppercase.class); - final JsonParser jsonParser = mock(JsonParser.class); - final DeserializationContext deserializationContext = mock(DeserializationContext.class); - when(jsonParser.getCodec()).thenReturn(objectMapper); - - when(objectMapper.readTree(jsonParser)).thenReturn(new TextNode(testEnumOption.name())); - - Enum result = objectUnderTest.deserialize(jsonParser, deserializationContext); - - assertThat(result, equalTo(testEnumOption)); - } - @ParameterizedTest @EnumSource(TestEnumWithoutJsonCreator.class) - void enum_class_without_json_creator_or_json_value_annotation_returns_expected_enum_constant(final TestEnumWithoutJsonCreator enumWithoutJsonCreator) throws IOException { + void enum_class_without_json_creator_annotation_returns_expected_enum_constant(final TestEnumWithoutJsonCreator enumWithoutJsonCreator) throws IOException { final EnumDeserializer objectUnderTest = createObjectUnderTest(TestEnumWithoutJsonCreator.class); final JsonParser jsonParser = mock(JsonParser.class); final DeserializationContext deserializationContext = mock(DeserializationContext.class); when(jsonParser.getCodec()).thenReturn(objectMapper); - when(objectMapper.readTree(jsonParser)).thenReturn(new TextNode(enumWithoutJsonCreator.name())); + when(objectMapper.readTree(jsonParser)).thenReturn(new TextNode(enumWithoutJsonCreator.toString())); Enum result = objectUnderTest.deserialize(jsonParser, deserializationContext); @@ -146,7 +116,7 @@ void enum_class_with_invalid_value_and_no_jsonValue_annotation_throws_IllegalArg assertThat(exception, notNullValue()); final String expectedErrorMessage = "Invalid value \"" + invalidValue + "\". Valid options include"; assertThat(exception.getMessage(), Matchers.startsWith(expectedErrorMessage)); - assertThat(exception.getMessage(), containsString("[TEST]")); + } @Test @@ -185,27 +155,6 @@ static TestEnum fromOptionValue(final String option) { } } - private enum TestEnumWithJsonValue { - TEST_ONE("test_json_value_one"), - TEST_TWO("test_json_value_two"), - TEST_THREE("test_json_value_three"); - private static final Map NAMES_MAP = Arrays.stream(TestEnum.values()) - .collect(Collectors.toMap(TestEnum::toString, Function.identity())); - private final String name; - TestEnumWithJsonValue(final String name) { - this.name = name; - } - - @JsonValue - public String toString() { - return this.name; - } - - static TestEnum fromOptionValue(final String option) { - return NAMES_MAP.get(option); - } - } - private enum TestEnumWithoutJsonCreator { TEST("test"); private static final Map NAMES_MAP = Arrays.stream(TestEnumWithoutJsonCreator.values()) @@ -215,16 +164,11 @@ private enum TestEnumWithoutJsonCreator { this.name = name; } public String toString() { - return UUID.randomUUID().toString(); + return this.name; } static TestEnumWithoutJsonCreator fromOptionValue(final String option) { return NAMES_MAP.get(option); } } - - private enum TestEnumOnlyUppercase { - VALUE_ONE, - VALUE_TWO; - } } diff --git a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ObjectMapperConfiguration.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ObjectMapperConfiguration.java index 513ff3d2fa..0ec428397b 100644 --- a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ObjectMapperConfiguration.java +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ObjectMapperConfiguration.java @@ -35,7 +35,6 @@ ObjectMapper extensionPluginConfigObjectMapper() { final SimpleModule simpleModule = new SimpleModule(); simpleModule.addDeserializer(Duration.class, new DataPrepperDurationDeserializer()); simpleModule.addDeserializer(Enum.class, new EnumDeserializer()); - simpleModule.addDeserializer(ByteCount.class, new ByteCountDeserializer()); return new ObjectMapper() From 0d03ff89cac6c6a6600fd074e8f75a7628ffa866 Mon Sep 17 00:00:00 2001 From: Taylor Gray Date: Fri, 25 Oct 2024 16:48:49 -0500 Subject: [PATCH 2/2] Revert "Add custom enum deserializer to improve error messaging, improve byte count error mesages (#5076)" This reverts commit e9bffee81e49151f37012b511f8d60ecc27bdbfb. --- .../dataprepper/model/types/ByteCount.java | 6 +- .../parser/ByteCountDeserializer.java | 2 +- .../pipeline/parser/EnumDeserializer.java | 111 ----------- .../parser/ByteCountDeserializerTest.java | 26 +-- .../pipeline/parser/EnumDeserializerTest.java | 174 ------------------ .../plugin/ObjectMapperConfiguration.java | 3 - .../plugin/ObjectMapperConfigurationTest.java | 43 +---- .../processor/drop/DropEventsProcessor.java | 3 +- .../s3/configuration/ThresholdOptions.java | 4 +- 9 files changed, 17 insertions(+), 355 deletions(-) delete mode 100644 data-prepper-pipeline-parser/src/main/java/org/opensearch/dataprepper/pipeline/parser/EnumDeserializer.java delete mode 100644 data-prepper-pipeline-parser/src/test/java/org/opensearch/dataprepper/pipeline/parser/EnumDeserializerTest.java diff --git a/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/types/ByteCount.java b/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/types/ByteCount.java index 3ca0f2e40b..e3037427f5 100644 --- a/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/types/ByteCount.java +++ b/data-prepper-api/src/main/java/org/opensearch/dataprepper/model/types/ByteCount.java @@ -79,13 +79,11 @@ public static ByteCount parse(final String string) { final String unitString = matcher.group("unit"); if(unitString == null) { - throw new ByteCountInvalidInputException("Byte counts must have a unit. Valid byte units include: " + - Arrays.stream(Unit.values()).map(unitValue -> unitValue.unitString).collect(Collectors.toList())); + throw new ByteCountInvalidInputException("Byte counts must have a unit."); } final Unit unit = Unit.fromString(unitString) - .orElseThrow(() -> new ByteCountInvalidInputException("Invalid byte unit: '" + unitString + "'. Valid byte units include: " - + Arrays.stream(Unit.values()).map(unitValue -> unitValue.unitString).collect(Collectors.toList()))); + .orElseThrow(() -> new ByteCountInvalidInputException("Invalid byte unit: '" + unitString + "'")); final BigDecimal valueBigDecimal = new BigDecimal(valueString); diff --git a/data-prepper-pipeline-parser/src/main/java/org/opensearch/dataprepper/pipeline/parser/ByteCountDeserializer.java b/data-prepper-pipeline-parser/src/main/java/org/opensearch/dataprepper/pipeline/parser/ByteCountDeserializer.java index d1fd2106a3..223821b7e0 100644 --- a/data-prepper-pipeline-parser/src/main/java/org/opensearch/dataprepper/pipeline/parser/ByteCountDeserializer.java +++ b/data-prepper-pipeline-parser/src/main/java/org/opensearch/dataprepper/pipeline/parser/ByteCountDeserializer.java @@ -34,7 +34,7 @@ public ByteCount deserialize(final JsonParser parser, final DeserializationConte try { return ByteCount.parse(byteString); } catch (final Exception ex) { - throw new IllegalArgumentException(ex.getMessage()); + throw new IllegalArgumentException(ex); } } } diff --git a/data-prepper-pipeline-parser/src/main/java/org/opensearch/dataprepper/pipeline/parser/EnumDeserializer.java b/data-prepper-pipeline-parser/src/main/java/org/opensearch/dataprepper/pipeline/parser/EnumDeserializer.java deleted file mode 100644 index e95f10b962..0000000000 --- a/data-prepper-pipeline-parser/src/main/java/org/opensearch/dataprepper/pipeline/parser/EnumDeserializer.java +++ /dev/null @@ -1,111 +0,0 @@ -package org.opensearch.dataprepper.pipeline.parser; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonValue; -import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.databind.BeanProperty; -import com.fasterxml.jackson.databind.DeserializationContext; -import com.fasterxml.jackson.databind.JavaType; -import com.fasterxml.jackson.databind.JsonDeserializer; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.deser.ContextualDeserializer; - -import java.io.IOException; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; -import java.util.Arrays; -import java.util.List; -import java.util.Optional; -import java.util.stream.Collectors; - - -/** - * This deserializer is used for any Enum classes when converting the pipeline configuration file into the plugin model classes - * @since 2.11 - */ -public class EnumDeserializer extends JsonDeserializer> implements ContextualDeserializer { - - static final String INVALID_ENUM_VALUE_ERROR_FORMAT = "Invalid value \"%s\". Valid options include %s."; - - private Class enumClass; - - public EnumDeserializer() {} - - public EnumDeserializer(final Class enumClass) { - if (!enumClass.isEnum()) { - throw new IllegalArgumentException("The provided class is not an enum: " + enumClass.getName()); - } - - this.enumClass = enumClass; - } - @Override - public Enum deserialize(final JsonParser p, final DeserializationContext ctxt) throws IOException { - final JsonNode node = p.getCodec().readTree(p); - final String enumValue = node.asText(); - - final Optional jsonCreator = findJsonCreatorMethod(); - - try { - jsonCreator.ifPresent(method -> method.setAccessible(true)); - - for (Object enumConstant : enumClass.getEnumConstants()) { - try { - if (jsonCreator.isPresent() && enumConstant.equals(jsonCreator.get().invoke(null, enumValue))) { - return (Enum) enumConstant; - } else if (jsonCreator.isEmpty() && enumConstant.toString().toLowerCase().equals(enumValue)) { - return (Enum) enumConstant; - } - } catch (IllegalAccessException | InvocationTargetException e) { - throw new RuntimeException(e); - } - } - } finally { - jsonCreator.ifPresent(method -> method.setAccessible(false)); - } - - - - final Optional jsonValueMethod = findJsonValueMethodForClass(); - final List listOfEnums = jsonValueMethod.map(method -> Arrays.stream(enumClass.getEnumConstants()) - .map(valueEnum -> { - try { - return method.invoke(valueEnum); - } catch (IllegalAccessException | InvocationTargetException e) { - throw new RuntimeException(e); - } - }) - .collect(Collectors.toList())).orElseGet(() -> Arrays.stream(enumClass.getEnumConstants()) - .map(valueEnum -> valueEnum.toString().toLowerCase()) - .collect(Collectors.toList())); - - throw new IllegalArgumentException(String.format(INVALID_ENUM_VALUE_ERROR_FORMAT, enumValue, listOfEnums)); - } - - @Override - public JsonDeserializer createContextual(final DeserializationContext ctxt, final BeanProperty property) { - final JavaType javaType = property.getType(); - final Class rawClass = javaType.getRawClass(); - - return new EnumDeserializer(rawClass); - } - - private Optional findJsonValueMethodForClass() { - for (final Method method : enumClass.getDeclaredMethods()) { - if (method.isAnnotationPresent(JsonValue.class)) { - return Optional.of(method); - } - } - - return Optional.empty(); - } - - private Optional findJsonCreatorMethod() { - for (final Method method : enumClass.getDeclaredMethods()) { - if (method.isAnnotationPresent(JsonCreator.class)) { - return Optional.of(method); - } - } - - return Optional.empty(); - } -} diff --git a/data-prepper-pipeline-parser/src/test/java/org/opensearch/dataprepper/pipeline/parser/ByteCountDeserializerTest.java b/data-prepper-pipeline-parser/src/test/java/org/opensearch/dataprepper/pipeline/parser/ByteCountDeserializerTest.java index 6d4575bf4e..21f0b000f3 100644 --- a/data-prepper-pipeline-parser/src/test/java/org/opensearch/dataprepper/pipeline/parser/ByteCountDeserializerTest.java +++ b/data-prepper-pipeline-parser/src/test/java/org/opensearch/dataprepper/pipeline/parser/ByteCountDeserializerTest.java @@ -13,7 +13,6 @@ import org.junit.jupiter.params.provider.ValueSource; import org.opensearch.dataprepper.model.types.ByteCount; -import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.notNullValue; import static org.hamcrest.MatcherAssert.assertThat; @@ -32,28 +31,9 @@ void setUp() { } @ParameterizedTest - @ValueSource(strings = {"1", "10"}) - void convert_with_no_byte_unit_throws_expected_exception(final String invalidByteString) { - final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> objectMapper.convertValue(invalidByteString, ByteCount.class)); - assertThat(exception.getMessage(), containsString("Byte counts must have a unit. Valid byte units include: [b, kb, mb, gb]")); - } - - @ParameterizedTest - @ValueSource(strings = {"10 2b", "bad"}) - void convert_with_non_parseable_values_throws(final String invalidByteString) { - final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> objectMapper.convertValue(invalidByteString, ByteCount.class)); - assertThat(exception.getMessage(), containsString("Unable to parse bytes")); - } - - @ParameterizedTest - @CsvSource({ - "10f, f", - "1vb, vb", - "3g, g" - }) - void convert_with_invalid_byte_units_throws(final String invalidByteString, final String invalidUnit) { - final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> objectMapper.convertValue(invalidByteString, ByteCount.class)); - assertThat(exception.getMessage(), containsString("Invalid byte unit: '" + invalidUnit + "'. Valid byte units include: [b, kb, mb, gb]")); + @ValueSource(strings = {"1", "1b 2b", "1vb", "bad"}) + void convert_with_invalid_values_throws(final String invalidByteString) { + assertThrows(IllegalArgumentException.class, () -> objectMapper.convertValue(invalidByteString, ByteCount.class)); } @ParameterizedTest diff --git a/data-prepper-pipeline-parser/src/test/java/org/opensearch/dataprepper/pipeline/parser/EnumDeserializerTest.java b/data-prepper-pipeline-parser/src/test/java/org/opensearch/dataprepper/pipeline/parser/EnumDeserializerTest.java deleted file mode 100644 index 990fd60e7d..0000000000 --- a/data-prepper-pipeline-parser/src/test/java/org/opensearch/dataprepper/pipeline/parser/EnumDeserializerTest.java +++ /dev/null @@ -1,174 +0,0 @@ -package org.opensearch.dataprepper.pipeline.parser; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonValue; -import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.databind.BeanProperty; -import com.fasterxml.jackson.databind.DeserializationContext; -import com.fasterxml.jackson.databind.JavaType; -import com.fasterxml.jackson.databind.JsonDeserializer; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.node.TextNode; -import org.hamcrest.Matchers; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.EnumSource; -import org.opensearch.dataprepper.model.event.HandleFailedEventsOption; - -import java.io.IOException; -import java.time.Duration; -import java.util.Arrays; -import java.util.Map; -import java.util.UUID; -import java.util.function.Function; -import java.util.stream.Collectors; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.notNullValue; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -public class EnumDeserializerTest { - - private ObjectMapper objectMapper; - - @BeforeEach - void setup() { - objectMapper = mock(ObjectMapper.class); - } - - private EnumDeserializer createObjectUnderTest(final Class enumClass) { - return new EnumDeserializer(enumClass); - } - - @Test - void non_enum_class_throws_IllegalArgumentException() { - assertThrows(IllegalArgumentException.class, () -> new EnumDeserializer(Duration.class)); - } - - @ParameterizedTest - @EnumSource(TestEnum.class) - void enum_class_with_json_creator_annotation_returns_expected_enum_constant(final TestEnum testEnumOption) throws IOException { - final EnumDeserializer objectUnderTest = createObjectUnderTest(TestEnum.class); - final JsonParser jsonParser = mock(JsonParser.class); - final DeserializationContext deserializationContext = mock(DeserializationContext.class); - when(jsonParser.getCodec()).thenReturn(objectMapper); - - when(objectMapper.readTree(jsonParser)).thenReturn(new TextNode(testEnumOption.toString())); - - Enum result = objectUnderTest.deserialize(jsonParser, deserializationContext); - - assertThat(result, equalTo(testEnumOption)); - } - - @ParameterizedTest - @EnumSource(TestEnumWithoutJsonCreator.class) - void enum_class_without_json_creator_annotation_returns_expected_enum_constant(final TestEnumWithoutJsonCreator enumWithoutJsonCreator) throws IOException { - final EnumDeserializer objectUnderTest = createObjectUnderTest(TestEnumWithoutJsonCreator.class); - final JsonParser jsonParser = mock(JsonParser.class); - final DeserializationContext deserializationContext = mock(DeserializationContext.class); - when(jsonParser.getCodec()).thenReturn(objectMapper); - - when(objectMapper.readTree(jsonParser)).thenReturn(new TextNode(enumWithoutJsonCreator.toString())); - - Enum result = objectUnderTest.deserialize(jsonParser, deserializationContext); - - assertThat(result, equalTo(enumWithoutJsonCreator)); - } - - @Test - void enum_class_with_invalid_value_and_jsonValue_annotation_throws_IllegalArgumentException() throws IOException { - final EnumDeserializer objectUnderTest = createObjectUnderTest(TestEnum.class); - final JsonParser jsonParser = mock(JsonParser.class); - final DeserializationContext deserializationContext = mock(DeserializationContext.class); - when(jsonParser.getCodec()).thenReturn(objectMapper); - - final String invalidValue = UUID.randomUUID().toString(); - when(objectMapper.readTree(jsonParser)).thenReturn(new TextNode(invalidValue)); - - final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> - objectUnderTest.deserialize(jsonParser, deserializationContext)); - - assertThat(exception, notNullValue()); - final String expectedErrorMessage = "Invalid value \"" + invalidValue + "\". Valid options include"; - assertThat(exception.getMessage(), Matchers.startsWith(expectedErrorMessage)); - assertThat(exception.getMessage(), containsString("[test_display_one, test_display_two, test_display_three]")); - } - - @Test - void enum_class_with_invalid_value_and_no_jsonValue_annotation_throws_IllegalArgumentException() throws IOException { - final EnumDeserializer objectUnderTest = createObjectUnderTest(TestEnumWithoutJsonCreator.class); - final JsonParser jsonParser = mock(JsonParser.class); - final DeserializationContext deserializationContext = mock(DeserializationContext.class); - when(jsonParser.getCodec()).thenReturn(objectMapper); - - final String invalidValue = UUID.randomUUID().toString(); - when(objectMapper.readTree(jsonParser)).thenReturn(new TextNode(invalidValue)); - - final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> - objectUnderTest.deserialize(jsonParser, deserializationContext)); - - assertThat(exception, notNullValue()); - final String expectedErrorMessage = "Invalid value \"" + invalidValue + "\". Valid options include"; - assertThat(exception.getMessage(), Matchers.startsWith(expectedErrorMessage)); - - } - - @Test - void create_contextual_returns_expected_enum_deserializer() { - final DeserializationContext context = mock(DeserializationContext.class); - final BeanProperty property = mock(BeanProperty.class); - - final ObjectMapper mapper = new ObjectMapper(); - final JavaType javaType = mapper.constructType(HandleFailedEventsOption.class); - when(property.getType()).thenReturn(javaType); - - final EnumDeserializer objectUnderTest = new EnumDeserializer(); - JsonDeserializer result = objectUnderTest.createContextual(context, property); - - assertThat(result, instanceOf(EnumDeserializer.class)); - } - - private enum TestEnum { - TEST_ONE("test_display_one"), - TEST_TWO("test_display_two"), - TEST_THREE("test_display_three"); - private static final Map NAMES_MAP = Arrays.stream(TestEnum.values()) - .collect(Collectors.toMap(TestEnum::toString, Function.identity())); - private final String name; - TestEnum(final String name) { - this.name = name; - } - - @JsonValue - public String toString() { - return this.name; - } - @JsonCreator - static TestEnum fromOptionValue(final String option) { - return NAMES_MAP.get(option); - } - } - - private enum TestEnumWithoutJsonCreator { - TEST("test"); - private static final Map NAMES_MAP = Arrays.stream(TestEnumWithoutJsonCreator.values()) - .collect(Collectors.toMap(TestEnumWithoutJsonCreator::toString, Function.identity())); - private final String name; - TestEnumWithoutJsonCreator(final String name) { - this.name = name; - } - public String toString() { - return this.name; - } - - static TestEnumWithoutJsonCreator fromOptionValue(final String option) { - return NAMES_MAP.get(option); - } - } -} diff --git a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ObjectMapperConfiguration.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ObjectMapperConfiguration.java index 0ec428397b..ca2cea4ee8 100644 --- a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ObjectMapperConfiguration.java +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ObjectMapperConfiguration.java @@ -13,7 +13,6 @@ import org.opensearch.dataprepper.model.types.ByteCount; import org.opensearch.dataprepper.pipeline.parser.ByteCountDeserializer; import org.opensearch.dataprepper.pipeline.parser.DataPrepperDurationDeserializer; -import org.opensearch.dataprepper.pipeline.parser.EnumDeserializer; import org.opensearch.dataprepper.pipeline.parser.EventKeyDeserializer; import org.springframework.context.annotation.Bean; @@ -34,7 +33,6 @@ public class ObjectMapperConfiguration { ObjectMapper extensionPluginConfigObjectMapper() { final SimpleModule simpleModule = new SimpleModule(); simpleModule.addDeserializer(Duration.class, new DataPrepperDurationDeserializer()); - simpleModule.addDeserializer(Enum.class, new EnumDeserializer()); simpleModule.addDeserializer(ByteCount.class, new ByteCountDeserializer()); return new ObjectMapper() @@ -49,7 +47,6 @@ ObjectMapper pluginConfigObjectMapper( final SimpleModule simpleModule = new SimpleModule(); simpleModule.addDeserializer(Duration.class, new DataPrepperDurationDeserializer()); simpleModule.addDeserializer(ByteCount.class, new ByteCountDeserializer()); - simpleModule.addDeserializer(Enum.class, new EnumDeserializer()); simpleModule.addDeserializer(EventKey.class, new EventKeyDeserializer(eventKeyFactory)); TRANSLATE_VALUE_SUPPORTED_JAVA_TYPES.stream().forEach(clazz -> simpleModule.addDeserializer( clazz, new DataPrepperScalarTypeDeserializer<>(variableExpander, clazz))); diff --git a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ObjectMapperConfigurationTest.java b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ObjectMapperConfigurationTest.java index b2cdea61e0..594d3a47c2 100644 --- a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ObjectMapperConfigurationTest.java +++ b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/ObjectMapperConfigurationTest.java @@ -6,8 +6,6 @@ package org.opensearch.dataprepper.plugin; import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -24,7 +22,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; -import static org.hamcrest.Matchers.notNullValue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -47,12 +44,11 @@ void test_duration_with_pluginConfigObjectMapper() { } @Test - void test_enum_with_pluginConfigObjectMapper() throws JsonProcessingException { - final String testModelAsString = "{ \"name\": \"my-name\", \"test_type\": \"test\" }"; + void test_enum_with_pluginConfigObjectMapper() { + final String testString = "test"; final ObjectMapper objectMapper = objectMapperConfiguration.pluginConfigObjectMapper(variableExpander, eventKeyFactory); - final TestModel testModel = objectMapper.readValue(testModelAsString, TestModel.class); - assertThat(testModel, notNullValue()); - assertThat(testModel.getTestType(), equalTo(TestType.TEST)); + final TestType duration = objectMapper.convertValue(testString, TestType.class); + assertThat(duration, equalTo(TestType.fromOptionValue(testString))); } @Test @@ -64,12 +60,11 @@ void test_duration_with_extensionPluginConfigObjectMapper() { } @Test - void test_enum_with_extensionPluginConfigObjectMapper() throws JsonProcessingException { - final String testModelAsString = "{ \"name\": \"my-name\", \"test_type\": \"test\" }"; + void test_enum_with_extensionPluginConfigObjectMapper() { + final String testString = "test"; final ObjectMapper objectMapper = objectMapperConfiguration.extensionPluginConfigObjectMapper(); - final TestModel testModel = objectMapper.readValue(testModelAsString, TestModel.class); - assertThat(testModel, notNullValue()); - assertThat(testModel.getTestType(), equalTo(TestType.TEST)); + final TestType duration = objectMapper.convertValue(testString, TestType.class); + assertThat(duration, equalTo(TestType.fromOptionValue(testString))); } @Test @@ -105,26 +100,4 @@ static TestType fromOptionValue(final String option) { } } - private static class TestModel { - @JsonProperty("name") - private final String name; - - @JsonProperty("test_type") - private final TestType testType; - - public TestModel(@JsonProperty("name") final String name, - @JsonProperty("test_type") final TestType testType) { - this.name = name; - this.testType = testType; - } - - public String getName() { - return name; - } - - public TestType getTestType() { - return testType; - } - } - } \ No newline at end of file diff --git a/data-prepper-plugins/drop-events-processor/src/main/java/org/opensearch/dataprepper/plugins/processor/drop/DropEventsProcessor.java b/data-prepper-plugins/drop-events-processor/src/main/java/org/opensearch/dataprepper/plugins/processor/drop/DropEventsProcessor.java index 3355e15fde..6196894beb 100644 --- a/data-prepper-plugins/drop-events-processor/src/main/java/org/opensearch/dataprepper/plugins/processor/drop/DropEventsProcessor.java +++ b/data-prepper-plugins/drop-events-processor/src/main/java/org/opensearch/dataprepper/plugins/processor/drop/DropEventsProcessor.java @@ -36,8 +36,7 @@ public DropEventsProcessor( if (dropEventProcessorConfig.getDropWhen() != null && (!expressionEvaluator.isValidExpressionStatement(dropEventProcessorConfig.getDropWhen()))) { - throw new InvalidPluginConfigurationException(String.format("drop_when \"%s\" is not a valid expression statement. " + - "See https://opensearch.org/docs/latest/data-prepper/pipelines/expression-syntax/ for valid expression syntax", dropEventProcessorConfig.getDropWhen())); + throw new InvalidPluginConfigurationException("drop_when {} is not a valid expression statement. See https://opensearch.org/docs/latest/data-prepper/pipelines/expression-syntax/ for valid expression syntax"); } whenCondition = new DropEventsWhenCondition.Builder() diff --git a/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/s3/configuration/ThresholdOptions.java b/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/s3/configuration/ThresholdOptions.java index 0ba43c2c02..d6dbf6877f 100644 --- a/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/s3/configuration/ThresholdOptions.java +++ b/data-prepper-plugins/s3-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/s3/configuration/ThresholdOptions.java @@ -28,7 +28,7 @@ public class ThresholdOptions { private int eventCount; @JsonProperty("maximum_size") - private ByteCount maximumSize = ByteCount.parse(DEFAULT_BYTE_CAPACITY); + private String maximumSize = DEFAULT_BYTE_CAPACITY; @JsonProperty("event_collect_timeout") @DurationMin(seconds = 1) @@ -49,7 +49,7 @@ public Duration getEventCollectTimeOut() { * @return maximum byte count. */ public ByteCount getMaximumSize() { - return maximumSize; + return ByteCount.parse(maximumSize); } /**