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 4b5c4106d8..e1893dffa7 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,6 +5,7 @@ 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; @@ -28,6 +29,7 @@ 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 c8c7f2ed5e..374b9629c9 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,7 +151,9 @@ private void buildPipelineFromConfiguration( Buffer pipelineDefinedBuffer = null; final PluginSetting bufferPluginSetting = pipelineConfiguration.getBufferPluginSetting(); try { - pipelineDefinedBuffer = pluginFactory.loadPlugin(Buffer.class, bufferPluginSetting, source.getDecoder()); + if (source != null) { + 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 2725bcbd26..9caa18820f 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,16 +222,11 @@ 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(2)); + assertThat(pluginErrorCollector.getPluginErrors().size(), equalTo(1)); 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 e95f10b962..e31fe6314f 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,6 +44,7 @@ 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)); @@ -52,7 +53,11 @@ 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() && enumConstant.toString().toLowerCase().equals(enumValue)) { + } 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)) { return (Enum) enumConstant; } } catch (IllegalAccessException | InvocationTargetException e) { @@ -63,9 +68,6 @@ 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 { @@ -75,7 +77,7 @@ public Enum deserialize(final JsonParser p, final DeserializationContext ctxt } }) .collect(Collectors.toList())).orElseGet(() -> Arrays.stream(enumClass.getEnumConstants()) - .map(valueEnum -> valueEnum.toString().toLowerCase()) + .map(valueEnum -> ((Enum) valueEnum).name()) .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 990fd60e7d..1276ccad7a 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,15 +66,45 @@ 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_annotation_returns_expected_enum_constant(final TestEnumWithoutJsonCreator enumWithoutJsonCreator) throws IOException { + void enum_class_without_json_creator_or_json_value_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())); + when(objectMapper.readTree(jsonParser)).thenReturn(new TextNode(enumWithoutJsonCreator.name())); Enum result = objectUnderTest.deserialize(jsonParser, deserializationContext); @@ -116,7 +146,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 @@ -155,6 +185,27 @@ 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()) @@ -164,11 +215,16 @@ private enum TestEnumWithoutJsonCreator { this.name = name; } public String toString() { - return this.name; + return UUID.randomUUID().toString(); } 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 0ec428397b..513ff3d2fa 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,6 +35,7 @@ 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()