diff --git a/data-prepper-pipeline-parser/src/main/java/org/opensearch/dataprepper/validation/PluginError.java b/data-prepper-pipeline-parser/src/main/java/org/opensearch/dataprepper/validation/PluginError.java index 50a130c4f4..84a17966ef 100644 --- a/data-prepper-pipeline-parser/src/main/java/org/opensearch/dataprepper/validation/PluginError.java +++ b/data-prepper-pipeline-parser/src/main/java/org/opensearch/dataprepper/validation/PluginError.java @@ -37,8 +37,10 @@ private String getFlattenedExceptionMessage(final String delimiter) { Throwable throwable = exception; while (throwable != null) { - message.append(delimiter); - message.append(throwable.getMessage()); + if (throwable.getMessage() != null) { + message.append(delimiter); + message.append(throwable.getMessage()); + } throwable = throwable.getCause(); } diff --git a/data-prepper-plugin-framework/build.gradle b/data-prepper-plugin-framework/build.gradle index 14f03fe15d..8eff39b1dc 100644 --- a/data-prepper-plugin-framework/build.gradle +++ b/data-prepper-plugin-framework/build.gradle @@ -24,4 +24,5 @@ dependencies { } implementation libs.reflections.core implementation 'com.fasterxml.jackson.core:jackson-databind' + implementation 'org.apache.commons:commons-text:1.10.0' } \ No newline at end of file diff --git a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/PluginConfigurationConverter.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/PluginConfigurationConverter.java index 27822e06e4..ddefb4d614 100644 --- a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/PluginConfigurationConverter.java +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/PluginConfigurationConverter.java @@ -32,11 +32,15 @@ class PluginConfigurationConverter { private final ObjectMapper objectMapper; private final Validator validator; + private final PluginConfigurationErrorHandler pluginConfigurationErrorHandler; + PluginConfigurationConverter(final Validator validator, @Named("pluginConfigObjectMapper") - final ObjectMapper objectMapper) { + final ObjectMapper objectMapper, + final PluginConfigurationErrorHandler pluginConfigurationErrorHandler) { this.objectMapper = objectMapper; this.validator = validator; + this.pluginConfigurationErrorHandler = pluginConfigurationErrorHandler; } /** @@ -80,6 +84,14 @@ private Object convertSettings(final Class pluginConfigurationType, final Plu Map settingsMap = pluginSetting.getSettings(); if (settingsMap == null) settingsMap = Collections.emptyMap(); - return objectMapper.convertValue(settingsMap, pluginConfigurationType); + + try { + return objectMapper.convertValue(settingsMap, pluginConfigurationType); + } catch (final Exception e) { + pluginConfigurationErrorHandler.handleException(pluginSetting, e); + } + + throw new InvalidPluginConfigurationException("Unhandled exception in when parsing the pipeline configuration"); } + } diff --git a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/PluginConfigurationErrorHandler.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/PluginConfigurationErrorHandler.java new file mode 100644 index 0000000000..818bee854f --- /dev/null +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/PluginConfigurationErrorHandler.java @@ -0,0 +1,91 @@ +package org.opensearch.dataprepper.plugin; + +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException; +import org.apache.commons.text.similarity.LevenshteinDistance; +import org.opensearch.dataprepper.model.configuration.PluginSetting; +import org.opensearch.dataprepper.model.plugin.InvalidPluginConfigurationException; + +import javax.inject.Inject; +import javax.inject.Named; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; + +@Named +public class PluginConfigurationErrorHandler { + + static final String UNRECOGNIZED_PROPERTY_EXCEPTION_FORMAT = "Parameter \"%s\" for plugin \"%s\" does not exist. Available options include %s."; + static final String JSON_MAPPING_EXCEPTION_FORMAT = "Parameter \"%s\" for plugin \"%s\" is invalid: %s"; + static final String GENERIC_PLUGIN_EXCEPTION_FORMAT = "Plugin \"%s\" is invalid: %s"; + + static final Integer MIN_DISTANCE_TO_RECOMMEND_PROPERTY = 3; + + private final LevenshteinDistance levenshteinDistance; + + @Inject + public PluginConfigurationErrorHandler(final LevenshteinDistance levenshteinDistance) { + this.levenshteinDistance = levenshteinDistance; + } + + public void handleException(final PluginSetting pluginSetting, final Exception e) { + if (e.getCause() instanceof UnrecognizedPropertyException) { + handleUnrecognizedPropertyException((UnrecognizedPropertyException) e.getCause(), pluginSetting); + } else if (e.getCause() instanceof JsonMappingException) { + handleJsonMappingException((JsonMappingException) e.getCause(), pluginSetting); + } + + throw new InvalidPluginConfigurationException( + String.format(GENERIC_PLUGIN_EXCEPTION_FORMAT, pluginSetting.getName(), e.getMessage())); + } + + private void handleJsonMappingException(final JsonMappingException e, final PluginSetting pluginSetting) { + final String parameterPath = getParameterPath(e.getPath()); + + final String errorMessage = String.format(JSON_MAPPING_EXCEPTION_FORMAT, + parameterPath, pluginSetting.getName(), e.getOriginalMessage()); + + throw new InvalidPluginConfigurationException(errorMessage); + } + + private void handleUnrecognizedPropertyException(final UnrecognizedPropertyException e, final PluginSetting pluginSetting) { + String errorMessage = String.format(UNRECOGNIZED_PROPERTY_EXCEPTION_FORMAT, + getParameterPath(e.getPath()), + pluginSetting.getName(), + e.getKnownPropertyIds()); + + final Optional closestRecommendation = getClosestField(e); + + if (closestRecommendation.isPresent()) { + errorMessage += " Did you mean \"" + closestRecommendation.get() + "\"?"; + } + + throw new InvalidPluginConfigurationException(errorMessage); + } + + private Optional getClosestField(final UnrecognizedPropertyException e) { + String closestMatch = null; + int smallestDistance = Integer.MAX_VALUE; + + for (final String field : e.getKnownPropertyIds().stream().map(Object::toString).collect(Collectors.toList())) { + int distance = levenshteinDistance.apply(e.getPropertyName(), field); + + if (distance < smallestDistance) { + smallestDistance = distance; + closestMatch = field; + } + } + + if (smallestDistance <= MIN_DISTANCE_TO_RECOMMEND_PROPERTY) { + return Optional.ofNullable(closestMatch); + } + + return Optional.empty(); + } + + private String getParameterPath(final List path) { + return path.stream() + .map(JsonMappingException.Reference::getFieldName) + .collect(Collectors.joining(".")); + } +} diff --git a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/PluginCreator.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/PluginCreator.java index 857f519b31..d718bf6bc4 100644 --- a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/PluginCreator.java +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/PluginCreator.java @@ -54,10 +54,10 @@ T newPluginInstance(final Class pluginClass, } catch (final IllegalAccessException | InstantiationException ex) { LOG.error("Encountered exception while instantiating the plugin {}", pluginClass.getSimpleName(), ex); throw new InvalidPluginDefinitionException( - "Unable to access or instantiate the plugin '" + pluginClass.getSimpleName() + ".'", ex); + "Unable to access or instantiate the plugin \"" + pluginName + "\".", ex); } catch (final InvocationTargetException ex) { LOG.error("Encountered exception while instantiating the plugin {}", pluginClass.getSimpleName(), ex); - throw new PluginInvocationException("Exception throw from the plugin'" + pluginClass.getSimpleName() + "'." , ex); + throw new PluginInvocationException("Exception thrown from plugin \"" + pluginName + "\".", ex.getTargetException()); } } diff --git a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ValidatorConfiguration.java b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ValidatorConfiguration.java index 31c6a7079a..4a3dc0893a 100644 --- a/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ValidatorConfiguration.java +++ b/data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/ValidatorConfiguration.java @@ -8,6 +8,7 @@ import jakarta.validation.Validation; import jakarta.validation.Validator; import jakarta.validation.ValidatorFactory; +import org.apache.commons.text.similarity.LevenshteinDistance; import org.hibernate.validator.messageinterpolation.ParameterMessageInterpolator; import org.springframework.context.annotation.Bean; @@ -26,4 +27,9 @@ Validator validator() { .buildValidatorFactory(); return validationFactory.getValidator(); } + + @Bean + LevenshteinDistance levenshteinDistance() { + return new LevenshteinDistance(); + } } diff --git a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/PluginConfigurationConverterTest.java b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/PluginConfigurationConverterTest.java index 91df976a76..27e3937bd7 100644 --- a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/PluginConfigurationConverterTest.java +++ b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/PluginConfigurationConverterTest.java @@ -7,6 +7,9 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; import org.opensearch.dataprepper.model.configuration.PluginSetting; import org.opensearch.dataprepper.model.plugin.InvalidPluginConfigurationException; import jakarta.validation.ConstraintViolation; @@ -30,12 +33,19 @@ import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.then; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +@ExtendWith(MockitoExtension.class) class PluginConfigurationConverterTest { private PluginSetting pluginSetting; private Validator validator; - private final ObjectMapper objectMapper = new ObjectMapper(); + + @Mock + private PluginConfigurationErrorHandler pluginConfigurationErrorHandler; + private ObjectMapper objectMapper = new ObjectMapper(); static class TestConfiguration { @SuppressWarnings("unused") @@ -55,7 +65,7 @@ void setUp() { } private PluginConfigurationConverter createObjectUnderTest() { - return new PluginConfigurationConverter(validator, objectMapper); + return new PluginConfigurationConverter(validator, objectMapper, pluginConfigurationErrorHandler); } @Test @@ -165,4 +175,44 @@ void convert_with_other_target_should_throw_exception_when_there_are_constraint_ assertThat(actualException.getMessage(), containsString(propertyPathString)); assertThat(actualException.getMessage(), containsString(errorMessage)); } + + @Test + void convert_with_error_when_converting_with_object_mapper_calls_plugin_configuration_error_handler() { + objectMapper = mock(ObjectMapper.class); + + final String value = UUID.randomUUID().toString(); + given(pluginSetting.getSettings()) + .willReturn(Collections.singletonMap("my_value", value)); + + final RuntimeException e = mock(RuntimeException.class); + + when(objectMapper.convertValue(pluginSetting.getSettings(), TestConfiguration.class)) + .thenThrow(e); + + doThrow(IllegalArgumentException.class).when(pluginConfigurationErrorHandler).handleException(pluginSetting, e); + + final PluginConfigurationConverter objectUnderTest = createObjectUnderTest(); + + assertThrows(IllegalArgumentException.class, () -> objectUnderTest.convert(TestConfiguration.class, pluginSetting)); + } + + @Test + void convert_with_error_when_throws_InvalidPluginConfiguration_when_plugin_configuration_error_handler_does_not_throw() { + objectMapper = mock(ObjectMapper.class); + + final String value = UUID.randomUUID().toString(); + given(pluginSetting.getSettings()) + .willReturn(Collections.singletonMap("my_value", value)); + + final RuntimeException e = mock(RuntimeException.class); + + when(objectMapper.convertValue(pluginSetting.getSettings(), TestConfiguration.class)) + .thenThrow(e); + + doNothing().when(pluginConfigurationErrorHandler).handleException(pluginSetting, e); + + final PluginConfigurationConverter objectUnderTest = createObjectUnderTest(); + + assertThrows(InvalidPluginConfigurationException.class, () -> objectUnderTest.convert(TestConfiguration.class, pluginSetting)); + } } \ No newline at end of file diff --git a/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/PluginConfigurationErrorHandlerTest.java b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/PluginConfigurationErrorHandlerTest.java new file mode 100644 index 0000000000..62405b99cb --- /dev/null +++ b/data-prepper-plugin-framework/src/test/java/org/opensearch/dataprepper/plugin/PluginConfigurationErrorHandlerTest.java @@ -0,0 +1,190 @@ +package org.opensearch.dataprepper.plugin; + +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException; +import org.apache.commons.text.similarity.LevenshteinDistance; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.opensearch.dataprepper.model.configuration.PluginSetting; +import org.opensearch.dataprepper.model.plugin.InvalidPluginConfigurationException; +import org.opensearch.dataprepper.model.plugin.InvalidPluginDefinitionException; + +import java.util.List; +import java.util.UUID; + +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.contains; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.opensearch.dataprepper.plugin.PluginConfigurationErrorHandler.GENERIC_PLUGIN_EXCEPTION_FORMAT; +import static org.opensearch.dataprepper.plugin.PluginConfigurationErrorHandler.JSON_MAPPING_EXCEPTION_FORMAT; +import static org.opensearch.dataprepper.plugin.PluginConfigurationErrorHandler.MIN_DISTANCE_TO_RECOMMEND_PROPERTY; +import static org.opensearch.dataprepper.plugin.PluginConfigurationErrorHandler.UNRECOGNIZED_PROPERTY_EXCEPTION_FORMAT; + +@ExtendWith(MockitoExtension.class) +public class PluginConfigurationErrorHandlerTest { + + @Mock + private LevenshteinDistance levenshteinDistance; + + private PluginConfigurationErrorHandler createObjectUnderTest() { + return new PluginConfigurationErrorHandler(levenshteinDistance); + } + + @Test + void handle_exception_with_unrecognized_property_exception_throws_expected_exception_without_parameter_recommendation() { + final List knownPropertyIds = List.of(UUID.randomUUID().toString(), UUID.randomUUID().toString()); + final String pluginName = UUID.randomUUID().toString(); + final String propertyName = UUID.randomUUID().toString(); + + final JsonMappingException.Reference firstPathReference = mock(JsonMappingException.Reference.class); + when(firstPathReference.getFieldName()).thenReturn(UUID.randomUUID().toString()); + + final JsonMappingException.Reference secondPathReference = mock(JsonMappingException.Reference.class); + when(secondPathReference.getFieldName()).thenReturn(UUID.randomUUID().toString()); + + final List path = List.of(firstPathReference, secondPathReference); + + final PluginSetting pluginSetting = mock(PluginSetting.class); + when(pluginSetting.getName()).thenReturn(pluginName); + + final RuntimeException exception = mock(RuntimeException.class); + + final UnrecognizedPropertyException unrecognizedPropertyException = mock(UnrecognizedPropertyException.class); + when(unrecognizedPropertyException.getKnownPropertyIds()).thenReturn(knownPropertyIds); + when(unrecognizedPropertyException.getPropertyName()).thenReturn(propertyName); + when(unrecognizedPropertyException.getPath()).thenReturn(path); + + when(exception.getCause()).thenReturn(unrecognizedPropertyException); + + when(levenshteinDistance.apply(eq(propertyName), anyString())).thenReturn(10).thenReturn(MIN_DISTANCE_TO_RECOMMEND_PROPERTY + 1); + + final PluginConfigurationErrorHandler objectUnderTest = createObjectUnderTest(); + + final InvalidPluginConfigurationException resultingException = + assertThrows(InvalidPluginConfigurationException.class, + () -> objectUnderTest.handleException(pluginSetting, exception)); + + final String expectedErrorMessage = String.format(UNRECOGNIZED_PROPERTY_EXCEPTION_FORMAT, + firstPathReference.getFieldName() + "." + secondPathReference.getFieldName(), + pluginName, + knownPropertyIds); + + assertThat(resultingException.getMessage(), equalTo(expectedErrorMessage)); + } + + @Test + void handle_exception_with_unrecognized_property_exception_throws_expected_exception_with_parameter_recommendation() { + final List knownPropertyIds = List.of(UUID.randomUUID().toString(), UUID.randomUUID().toString()); + final String pluginName = UUID.randomUUID().toString(); + final String propertyName = UUID.randomUUID().toString(); + + final JsonMappingException.Reference firstPathReference = mock(JsonMappingException.Reference.class); + when(firstPathReference.getFieldName()).thenReturn(UUID.randomUUID().toString()); + + final JsonMappingException.Reference secondPathReference = mock(JsonMappingException.Reference.class); + when(secondPathReference.getFieldName()).thenReturn(UUID.randomUUID().toString()); + + final List path = List.of(firstPathReference, secondPathReference); + + final PluginSetting pluginSetting = mock(PluginSetting.class); + when(pluginSetting.getName()).thenReturn(pluginName); + + final RuntimeException exception = mock(RuntimeException.class); + + final UnrecognizedPropertyException unrecognizedPropertyException = mock(UnrecognizedPropertyException.class); + when(unrecognizedPropertyException.getKnownPropertyIds()).thenReturn(knownPropertyIds); + when(unrecognizedPropertyException.getPropertyName()).thenReturn(propertyName); + when(unrecognizedPropertyException.getPath()).thenReturn(path); + + when(exception.getCause()).thenReturn(unrecognizedPropertyException); + + when(levenshteinDistance.apply(eq(propertyName), anyString())).thenReturn(10).thenReturn(MIN_DISTANCE_TO_RECOMMEND_PROPERTY - 1); + + final PluginConfigurationErrorHandler objectUnderTest = createObjectUnderTest(); + + final InvalidPluginConfigurationException resultingException = + assertThrows(InvalidPluginConfigurationException.class, + () -> objectUnderTest.handleException(pluginSetting, exception)); + + String expectedErrorMessage = String.format(UNRECOGNIZED_PROPERTY_EXCEPTION_FORMAT, + firstPathReference.getFieldName() + "." + secondPathReference.getFieldName(), + pluginName, + knownPropertyIds); + + expectedErrorMessage += " Did you mean \"" + knownPropertyIds.get(1).toString() + "\"?"; + + + + assertThat(resultingException.getMessage(), equalTo(expectedErrorMessage)); + } + + @Test + void handle_exception_with_json_mapping_exception_returns_expected_error_message() { + final String pluginName = UUID.randomUUID().toString(); + + final JsonMappingException.Reference firstPathReference = mock(JsonMappingException.Reference.class); + when(firstPathReference.getFieldName()).thenReturn(UUID.randomUUID().toString()); + + final JsonMappingException.Reference secondPathReference = mock(JsonMappingException.Reference.class); + when(secondPathReference.getFieldName()).thenReturn(UUID.randomUUID().toString()); + + final List path = List.of(firstPathReference, secondPathReference); + + final PluginSetting pluginSetting = mock(PluginSetting.class); + when(pluginSetting.getName()).thenReturn(pluginName); + + final RuntimeException exception = mock(RuntimeException.class); + + final JsonMappingException jsonMappingException = mock(JsonMappingException.class); + when(jsonMappingException.getPath()).thenReturn(path); + when(jsonMappingException.getOriginalMessage()).thenReturn(UUID.randomUUID().toString()); + + when(exception.getCause()).thenReturn(jsonMappingException); + + final PluginConfigurationErrorHandler objectUnderTest = createObjectUnderTest(); + + final InvalidPluginConfigurationException resultingException = + assertThrows(InvalidPluginConfigurationException.class, + () -> objectUnderTest.handleException(pluginSetting, exception)); + + final String expectedErrorMessage = String.format(JSON_MAPPING_EXCEPTION_FORMAT, + firstPathReference.getFieldName() + "." + secondPathReference.getFieldName(), + pluginName, + jsonMappingException.getOriginalMessage()); + + assertThat(resultingException.getMessage(), equalTo(expectedErrorMessage)); + } + + @Test + void non_handled_exception_throws_generic_invalid_plugin_configuration_exception() { + final String pluginName = UUID.randomUUID().toString(); + + final PluginSetting pluginSetting = mock(PluginSetting.class); + when(pluginSetting.getName()).thenReturn(pluginName); + + final RuntimeException exception = mock(RuntimeException.class); + when(exception.getMessage()).thenReturn(UUID.randomUUID().toString()); + + final RuntimeException cause = mock(RuntimeException.class); + when(exception.getCause()).thenReturn(cause); + + final PluginConfigurationErrorHandler objectUnderTest = createObjectUnderTest(); + + final InvalidPluginConfigurationException resultingException = + assertThrows(InvalidPluginConfigurationException.class, + () -> objectUnderTest.handleException(pluginSetting, exception)); + + final String expectedErrorMessage = String.format(GENERIC_PLUGIN_EXCEPTION_FORMAT, + pluginName, + exception.getMessage()); + + assertThat(resultingException.getMessage(), equalTo(expectedErrorMessage)); + } +}