From df8737f7ba7fd3c4706e5b7f7bbd2ca346aa6760 Mon Sep 17 00:00:00 2001 From: David Venable Date: Fri, 12 Nov 2021 17:38:12 -0600 Subject: [PATCH 1/2] Fix issues with extra quotes in Logstash converted YAML files. Signed-off-by: David Venable --- .../logstash/parser/LogstashVisitor.java | 11 +++-- .../logstash/parser/LogstashVisitorTest.java | 46 ++++++++++++++++++- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/data-prepper-logstash-configuration/src/main/java/org/opensearch/dataprepper/logstash/parser/LogstashVisitor.java b/data-prepper-logstash-configuration/src/main/java/org/opensearch/dataprepper/logstash/parser/LogstashVisitor.java index 698f25a3f5..08e72feeec 100644 --- a/data-prepper-logstash-configuration/src/main/java/org/opensearch/dataprepper/logstash/parser/LogstashVisitor.java +++ b/data-prepper-logstash-configuration/src/main/java/org/opensearch/dataprepper/logstash/parser/LogstashVisitor.java @@ -104,7 +104,7 @@ else if (attributeContext.value().BAREWORD() != null && attributeContext.value() } else if (attributeContext.value().STRING() != null && attributeContext.value().getText().equals(attributeContext.value().STRING().toString())) { logstashValueType = LogstashValueType.STRING; - value = attributeContext.value().getText().replaceAll("^\"|\"$|^'|'$", ""); + value = normalizeText(attributeContext.value().getText()); } final LogstashAttributeValue logstashAttributeValue = LogstashAttributeValue.builder() @@ -122,6 +122,7 @@ else if (attributeContext.value().STRING() != null && attributeContext.value().g public Object visitArray(final LogstashParser.ArrayContext arrayContext) { return arrayContext.value().stream() .map(RuleContext::getText) + .map(this::normalizeText) .collect(Collectors.toList()); } @@ -135,7 +136,7 @@ public Object visitHashentries(final LogstashParser.HashentriesContext hashentri final Map hashEntries = new LinkedHashMap<>(); hashentriesContext.hashentry().forEach(hashentryContext -> { - final String key = hashentryContext.hashname().getText(); + final String key = normalizeText(hashentryContext.hashname().getText()); final Object value = visitHashentry(hashentryContext); hashEntries.put(key, value); }); @@ -148,6 +149,10 @@ public Object visitHashentry(final LogstashParser.HashentryContext hashentryCont if (hashentryContext.value().getChild(0) instanceof LogstashParser.ArrayContext) return visitArray(hashentryContext.value().array()); - return hashentryContext.value().getText(); + return normalizeText(hashentryContext.value().getText()); + } + + private String normalizeText(final String unNormalizedLogstashText) { + return unNormalizedLogstashText.replaceAll("^\"|\"$|^'|'$", ""); } } \ No newline at end of file diff --git a/data-prepper-logstash-configuration/src/test/java/org/opensearch/dataprepper/logstash/parser/LogstashVisitorTest.java b/data-prepper-logstash-configuration/src/test/java/org/opensearch/dataprepper/logstash/parser/LogstashVisitorTest.java index ac4e925edf..f0c02a2236 100644 --- a/data-prepper-logstash-configuration/src/test/java/org/opensearch/dataprepper/logstash/parser/LogstashVisitorTest.java +++ b/data-prepper-logstash-configuration/src/test/java/org/opensearch/dataprepper/logstash/parser/LogstashVisitorTest.java @@ -73,6 +73,9 @@ void createObjectUnderTest() { @Mock private LogstashParser.HashnameContext hashnameContextMock = mock(LogstashParser.HashnameContext.class); + private static String wrapInQuotes(final String inputString) { + return "\"" + inputString + "\""; + } @Test void visit_config_return_logstash_configuration_object_test() { @@ -389,8 +392,8 @@ void visit_attribute_with_value_type_string_returns_correct_logstash_attribute_t given(nameContextMock.getText()).willReturn(TestDataProvider.RANDOM_STRING_1); given(attributeContextMock.value()).willReturn(valueContextMock); given(valueContextMock.STRING()).willReturn(terminalNodeMock); - given(valueContextMock.getText()).willReturn("\"" + TestDataProvider.RANDOM_STRING_2 + "\""); - given(valueContextMock.STRING().toString()).willReturn("\"" + TestDataProvider.RANDOM_STRING_2 + "\""); + given(valueContextMock.getText()).willReturn(wrapInQuotes(TestDataProvider.RANDOM_STRING_2)); + given(valueContextMock.STRING().toString()).willReturn(wrapInQuotes(TestDataProvider.RANDOM_STRING_2)); LogstashAttribute actualLogstashAttribute = (LogstashAttribute) logstashVisitor.visitAttribute(attributeContextMock); LogstashAttribute expectedLogstashAttribute = TestDataProvider.attributeWithStringTypeValueData(); @@ -435,6 +438,19 @@ void visit_array_with_array_of_size_more_than_one_returns_list_of_size_more_than assertThat(actualList, equalTo(TestDataProvider.arrayData())); } + @Test + void visit_array_with_array_of_with_quoted_strings_returns_them_unquoted() { + List valueContextList = new LinkedList<>(Arrays.asList(arrayValueContextMock1, arrayValueContextMock2)); + + given(arrayContextMock.value()).willReturn(valueContextList); + given(arrayValueContextMock1.getText()).willReturn(wrapInQuotes(TestDataProvider.RANDOM_STRING_1)); + given(arrayValueContextMock2.getText()).willReturn(wrapInQuotes(TestDataProvider.RANDOM_STRING_2)); + + Object actualList = logstashVisitor.visitArray(arrayContextMock); + + assertThat(actualList, equalTo(TestDataProvider.arrayData())); + } + @Test void visit_hash_entries_with_string_value_returns_map_of_hash_entries_test() { List hashentryContextList = new LinkedList<>( @@ -451,6 +467,22 @@ void visit_hash_entries_with_string_value_returns_map_of_hash_entries_test() { assertThat(actualMap, equalTo(TestDataProvider.hashEntriesStringData())); } + @Test + void visit_hash_entries_with_quoted_values_returns_unquoted() { + List hashentryContextList = new LinkedList<>( + Collections.singletonList(hashentryContextMock)); + + given(hashentriesContextMock.hashentry()).willReturn(hashentryContextList); + given(hashentryContextMock.hashname()).willReturn(hashnameContextMock); + given(hashnameContextMock.getText()).willReturn(wrapInQuotes(TestDataProvider.RANDOM_STRING_1)); + given(hashentryContextMock.value()).willReturn(valueContextMock); + given(valueContextMock.getText()).willReturn(wrapInQuotes(TestDataProvider.RANDOM_STRING_2)); + + Object actualMap = logstashVisitor.visitHashentries(hashentriesContextMock); + + assertThat(actualMap, equalTo(TestDataProvider.hashEntriesStringData())); + } + @Test void visit_hash_entries_with_array_value_returns_map_of_hash_entries_test() { List hashentryContextList = new LinkedList<>( @@ -482,6 +514,16 @@ void visit_hash_entry_with_value_type_string_returns_string_object_test() { assertThat(actualValue, equalTo(TestDataProvider.hashEntryStringData())); } + @Test + void visit_hash_entry_with_quoted_text_returns_it_unquoted() { + given(hashentryContextMock.value()).willReturn(valueContextMock); + given(valueContextMock.getText()).willReturn(wrapInQuotes(TestDataProvider.RANDOM_STRING_2)); + + Object actualValue = logstashVisitor.visitHashentry(hashentryContextMock); + + assertThat(actualValue, equalTo(TestDataProvider.hashEntryStringData())); + } + @Test void visit_hash_entry_with_array_value_type_returns_list_test() { final List linkedListValuesMock = new LinkedList<>( From 7670f3ef37399f283d530e414663da0176597b3f Mon Sep 17 00:00:00 2001 From: David Venable Date: Fri, 12 Nov 2021 18:41:33 -0600 Subject: [PATCH 2/2] Improvements from PR. Signed-off-by: David Venable --- .../logstash/parser/LogstashVisitor.java | 4 +-- .../logstash/parser/LogstashVisitorTest.java | 28 ++++++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/data-prepper-logstash-configuration/src/main/java/org/opensearch/dataprepper/logstash/parser/LogstashVisitor.java b/data-prepper-logstash-configuration/src/main/java/org/opensearch/dataprepper/logstash/parser/LogstashVisitor.java index 08e72feeec..2cda72b225 100644 --- a/data-prepper-logstash-configuration/src/main/java/org/opensearch/dataprepper/logstash/parser/LogstashVisitor.java +++ b/data-prepper-logstash-configuration/src/main/java/org/opensearch/dataprepper/logstash/parser/LogstashVisitor.java @@ -63,7 +63,7 @@ public Object visitBranch_or_plugin(final LogstashParser.Branch_or_pluginContext @Override public Object visitPlugin(final LogstashParser.PluginContext pluginContext) { - final String pluginName = pluginContext.name().getText(); + final String pluginName = normalizeText(pluginContext.name().getText()); final List logstashAttributeList = pluginContext.attributes().attribute().stream() .map(attribute -> (LogstashAttribute) visitAttribute(attribute)) .filter(Objects::nonNull) @@ -113,7 +113,7 @@ else if (attributeContext.value().STRING() != null && attributeContext.value().g .build(); return LogstashAttribute.builder() - .attributeName(attributeContext.name().getText()) + .attributeName(normalizeText(attributeContext.name().getText())) .attributeValue(logstashAttributeValue) .build(); } diff --git a/data-prepper-logstash-configuration/src/test/java/org/opensearch/dataprepper/logstash/parser/LogstashVisitorTest.java b/data-prepper-logstash-configuration/src/test/java/org/opensearch/dataprepper/logstash/parser/LogstashVisitorTest.java index f0c02a2236..444eabe93e 100644 --- a/data-prepper-logstash-configuration/src/test/java/org/opensearch/dataprepper/logstash/parser/LogstashVisitorTest.java +++ b/data-prepper-logstash-configuration/src/test/java/org/opensearch/dataprepper/logstash/parser/LogstashVisitorTest.java @@ -279,6 +279,17 @@ void visit_plugin_with_more_than_one_array_context_attribute_test() { } } + @Test + void visit_plugin_with_quoted_pluginName_returns_it_unquoted() { + given(pluginContextMock.name()).willReturn(nameContextMock); + given(nameContextMock.getText()).willReturn(wrapInQuotes(TestDataProvider.RANDOM_STRING_1)); + given(pluginContextMock.attributes()).willReturn(attributesContextMock); + + final LogstashPlugin actualLogstashPlugin = (LogstashPlugin) logstashVisitor.visitPlugin(pluginContextMock); + + assertThat(actualLogstashPlugin.getPluginName(), equalTo(TestDataProvider.RANDOM_STRING_1)); + } + @Test void visit_attribute_without_a_value_returns_null() { assertThat(logstashVisitor.visitAttribute(attributeContextMock), @@ -407,6 +418,21 @@ void visit_attribute_with_value_type_string_returns_correct_logstash_attribute_t } + @Test + void visitAttribute_with_quoted_attribute_name_returns_unquoted() { + given(attributeContextMock.name()).willReturn(nameContextMock); + given(nameContextMock.getText()).willReturn(wrapInQuotes(TestDataProvider.RANDOM_STRING_1)); + given(attributeContextMock.value()).willReturn(valueContextMock); + given(valueContextMock.STRING()).willReturn(terminalNodeMock); + given(valueContextMock.getText()).willReturn(TestDataProvider.RANDOM_STRING_2); + given(valueContextMock.STRING().toString()).willReturn(TestDataProvider.RANDOM_STRING_2); + + final LogstashAttribute actualLogstashAttribute = (LogstashAttribute) logstashVisitor.visitAttribute(attributeContextMock); + + assertThat(actualLogstashAttribute.getAttributeName(), + equalTo(TestDataProvider.RANDOM_STRING_1)); + } + @Test void visit_array_with_empty_array_returns_empty_list_test() { given(arrayContextMock.value()).willReturn(Collections.emptyList()); @@ -439,7 +465,7 @@ void visit_array_with_array_of_size_more_than_one_returns_list_of_size_more_than } @Test - void visit_array_with_array_of_with_quoted_strings_returns_them_unquoted() { + void visitArray_with_with_quoted_strings_returns_them_unquoted() { List valueContextList = new LinkedList<>(Arrays.asList(arrayValueContextMock1, arrayValueContextMock2)); given(arrayContextMock.value()).willReturn(valueContextList);