Improve validation error messages for unknown property and json mappi…

…ng exceptions

Signed-off-by: Taylor Gray <[email protected]>

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();
         }
 

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'
 }

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<String, Object> 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");
     }
+
 }

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<String> closestRecommendation = getClosestField(e);
+
+        if (closestRecommendation.isPresent()) {
+            errorMessage += " Did you mean \"" + closestRecommendation.get() + "\"?";
+        }
+
+        throw new InvalidPluginConfigurationException(errorMessage);
+    }
+
+    private Optional<String> 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<JsonMappingException.Reference> path) {
+        return path.stream()
+                .map(JsonMappingException.Reference::getFieldName)
+                .collect(Collectors.joining("."));
+    }
+}

data-prepper-plugin-framework/src/main/java/org/opensearch/dataprepper/plugin/PluginCreator.java

@@ -54,10 +54,10 @@ <T> T newPluginInstance(final Class<T> 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());
         }
     }
 

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();
+    }
 }

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));
+    }
 }