Gather bean validation violations in a single field error

Fix BeanValidationError path according to the spec
smallrye · Oct 2, 2024 · 855de66 · 855de66
1 parent 1f30a07
commit 855de66
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 21 deletions.
diff --git a/server/implementation/src/main/java/io/smallrye/graphql/validation/BeanValidationError.java b/server/implementation/src/main/java/io/smallrye/graphql/validation/BeanValidationError.java
@@ -6,6 +6,8 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 
@@ -14,16 +16,21 @@
 
 import graphql.ErrorClassification;
 import graphql.GraphQLError;
-import graphql.language.NamedNode;
+import graphql.execution.ResultPath;
 import graphql.language.SourceLocation;
 
 public class BeanValidationError implements GraphQLError {
-    private final ConstraintViolation<?> violation;
-    private final List<NamedNode<?>> requestedPath;
+    private final Set<ConstraintViolation<?>> violations;
+    private final ResultPath resultPath;
+    private final List<SourceLocation> sourceLocations;
 
-    public BeanValidationError(ConstraintViolation<?> violation, List<NamedNode<?>> requestedPath) {
-        this.violation = violation;
-        this.requestedPath = requestedPath;
+    public BeanValidationError(
+            Set<ConstraintViolation<?>> violations,
+            ResultPath resultPath,
+            List<SourceLocation> sourceLocations) {
+        this.violations = violations;
+        this.resultPath = resultPath;
+        this.sourceLocations = sourceLocations;
     }
 
     @Override
@@ -33,28 +40,33 @@ public ErrorClassification getErrorType() {
 
     @Override
     public String getMessage() {
-        return "validation failed: " + violation.getPropertyPath() + " " + violation.getMessage();
+        String joinedMessage = violations.stream()
+                .map(violation -> violation.getPropertyPath() + " " + violation.getMessage())
+                .collect(Collectors.joining(", "));
+        return "validation failed: " + joinedMessage;
     }
 
     @Override
     public List<SourceLocation> getLocations() {
-        return requestedPath.stream().map(NamedNode::getSourceLocation).collect(toList());
+        return sourceLocations;
     }
 
     @Override
     public List<Object> getPath() {
-        return requestedPath.stream().map(argument -> (Object) argument.getName()).collect(toList());
+        return resultPath.toList();
     }
 
     @Override
     public Map<String, Object> getExtensions() {
-        Map<String, Object> extensions = new HashMap<>();
-        extensions.put("violation.message", violation.getMessage());
-        extensions.put("violation.propertyPath",
-                toStream(violation.getPropertyPath()).flatMap(this::items).collect(toList()));
-        extensions.put("violation.invalidValue", violation.getInvalidValue());
-        extensions.put("violation.constraint", getConstraintAttributes());
-        return extensions;
+        return Map.of("violations", violations.stream().map(this::getViolationAttributes).collect(toList()));
+    }
+
+    private Map<String, Object> getViolationAttributes(ConstraintViolation<?> violation) {
+        return Map.of(
+                "message", violation.getMessage(),
+                "propertyPath", toStream(violation.getPropertyPath()).flatMap(this::items).collect(toList()),
+                "invalidValue", violation.getInvalidValue(),
+                "constraint", getConstraintAttributes(violation));
     }
 
     private Stream<String> items(Path.Node node) {
@@ -63,7 +75,7 @@ private Stream<String> items(Path.Node node) {
         return Stream.of(node.getIndex().toString(), node.getName());
     }
 
-    private Map<String, Object> getConstraintAttributes() {
+    private Map<String, Object> getConstraintAttributes(ConstraintViolation<?> violation) {
         Map<String, Object> attributes = new HashMap<>(violation.getConstraintDescriptor().getAttributes());
         attributes.computeIfPresent("groups", BeanValidationError::classNames);
         attributes.computeIfPresent("payload", BeanValidationError::classNames);

diff --git a/server/implementation/src/main/java/io/smallrye/graphql/validation/BeanValidationUtil.java b/server/implementation/src/main/java/io/smallrye/graphql/validation/BeanValidationUtil.java
@@ -1,6 +1,7 @@
 package io.smallrye.graphql.validation;
 
 import static java.util.Arrays.asList;
+import static java.util.stream.Collectors.toList;
 
 import java.lang.reflect.Method;
 import java.lang.reflect.Parameter;
@@ -14,9 +15,11 @@
 import org.eclipse.microprofile.graphql.Source;
 
 import graphql.execution.DataFetcherResult;
+import graphql.execution.ResultPath;
 import graphql.language.Argument;
 import graphql.language.Field;
 import graphql.language.NamedNode;
+import graphql.language.SourceLocation;
 import graphql.schema.DataFetchingEnvironment;
 import io.smallrye.graphql.api.Context;
 
@@ -27,11 +30,15 @@ public static DataFetcherResult.Builder<Object> addConstraintViolationsToDataFet
             Method method,
             DataFetcherResult.Builder<Object> builder,
             DataFetchingEnvironment dfe) {
+        ResultPath resultPath = dfe.getExecutionStepInfo().getPath();
         RequestNodeBuilder requestNodeBuilder = new RequestNodeBuilder(method, dfe);
-        violations.stream()
-                .map(violation -> new BeanValidationError(violation, requestNodeBuilder.build(violation)))
-                .forEach(builder::error);
-        return builder;
+        List<SourceLocation> sourceLocations = violations.stream()
+                .map(requestNodeBuilder::build)
+                .flatMap(List::stream)
+                .distinct()
+                .map(NamedNode::getSourceLocation)
+                .collect(toList());
+        return builder.error(new BeanValidationError(violations, resultPath, sourceLocations));
     }
 
     static class RequestNodeBuilder {