fixes graphql-java#1524 (graphql-java#1525)

wasche · bbakerman · commit 961613261849 · 2019-05-14T14:56:23.000+10:00
* fixes graphql-java#1524 * fixes graphql-java#1524
diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java
@@ -15,8 +15,10 @@
 import org.dataloader.DataLoaderRegistry;
 
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.function.Consumer;
 
@@ -38,6 +40,7 @@ public class ExecutionContext {
     private final Object context;
     private final Instrumentation instrumentation;
     private final List<GraphQLError> errors = new CopyOnWriteArrayList<>();
+    private final Set<ExecutionPath> errorPaths = new HashSet<>();
     private final DataLoaderRegistry dataLoaderRegistry;
     private final CacheControl cacheControl;
     private final DeferSupport deferSupport = new DeferSupport();
@@ -128,13 +131,8 @@ public void addError(GraphQLError error, ExecutionPath fieldPath) {
         // field errors should be handled - ie only once per field if its already there for nullability
         // but unclear if its not that error path
         //
-        for (GraphQLError graphQLError : errors) {
-            List<Object> path = graphQLError.getPath();
-            if (path != null) {
-                if (fieldPath.equals(ExecutionPath.fromList(path))) {
-                    return;
-                }
-            }
+        if (!errorPaths.add(fieldPath)) {
+            return;
         }
         this.errors.add(error);
     }
@@ -149,6 +147,9 @@ public void addError(GraphQLError error) {
         // see https://github.com/graphql-java/graphql-java/issues/888 on how the spec is unclear
         // on how exactly multiple errors should be handled - ie only once per field or not outside the nullability
         // aspect.
+        if (error.getPath() != null) {
+            this.errorPaths.add(ExecutionPath.fromList(error.getPath()));
+        }
         this.errors.add(error);
     }
 
diff --git a/src/test/java/benchmark/AddError.java b/src/test/java/benchmark/AddError.java
@@ -0,0 +1,38 @@
+package benchmark;
+
+import graphql.execution.ExecutionContext;
+import graphql.execution.ExecutionContextBuilder;
+import graphql.execution.ExecutionId;
+import graphql.execution.ExecutionPath;
+import graphql.schema.idl.errors.SchemaMissingError;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Warmup;
+
+import java.util.Collections;
+
+@State(Scope.Benchmark)
+public class AddError {
+
+    private ExecutionContext context = new ExecutionContextBuilder()
+            .executionId(ExecutionId.generate())
+            .build();
+
+    private volatile int x = 0;
+
+    @Benchmark
+    @BenchmarkMode(Mode.SingleShotTime)
+    @Warmup(iterations = 1, batchSize = 50000)
+    @Measurement(iterations = 1, batchSize = 5000)
+    public ExecutionContext benchMarkAddError() {
+        context.addError(
+                new SchemaMissingError(),
+                ExecutionPath.fromList(Collections.singletonList(x++))
+        );
+        return context;
+    }
+}
