From fb68283d9e5e1580bbaeb1f6d2f1fbe28b911aef Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Thu, 15 May 2025 17:02:33 +1000 Subject: [PATCH] Cherry pick 3931 imperative filters --- .../analysis/values/ValueTraverser.java | 13 ++-- .../java/graphql/collect/ImmutableKit.java | 70 ++++++++++++++++--- .../FieldValidationSupport.java | 3 +- .../java/graphql/language/AstSignature.java | 8 +-- src/main/java/graphql/language/Document.java | 7 +- .../java/graphql/language/NodeParentTree.java | 8 +-- .../java/graphql/language/SelectionSet.java | 7 +- .../schema/visibility/BlockedFields.java | 12 ++-- .../graphql/collect/ImmutableKitTest.groovy | 13 +++- src/test/groovy/graphql/util/FpKitTest.groovy | 12 ++-- 10 files changed, 106 insertions(+), 47 deletions(-) diff --git a/src/main/java/graphql/analysis/values/ValueTraverser.java b/src/main/java/graphql/analysis/values/ValueTraverser.java index 1cf7745aaa..be5c37326a 100644 --- a/src/main/java/graphql/analysis/values/ValueTraverser.java +++ b/src/main/java/graphql/analysis/values/ValueTraverser.java @@ -2,6 +2,7 @@ import com.google.common.collect.ImmutableList; import graphql.PublicApi; +import graphql.collect.ImmutableKit; import graphql.schema.DataFetchingEnvironment; import graphql.schema.DataFetchingEnvironmentImpl; import graphql.schema.GraphQLAppliedDirective; @@ -22,7 +23,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import static graphql.Assert.assertShouldNeverHappen; import static graphql.Assert.assertTrue; @@ -62,13 +62,12 @@ private InputElements(GraphQLInputSchemaElement startElement) { private InputElements(ImmutableList inputElements) { this.inputElements = inputElements; - this.unwrappedInputElements = inputElements.stream() - .filter(it -> !(it instanceof GraphQLNonNull || it instanceof GraphQLList)) - .collect(ImmutableList.toImmutableList()); + this.unwrappedInputElements = ImmutableKit.filter(inputElements, + it -> !(it instanceof GraphQLNonNull || it instanceof GraphQLList)); - List inputValDefs = unwrappedInputElements.stream() - .filter(it -> it instanceof GraphQLInputValueDefinition) - .map(GraphQLInputValueDefinition.class::cast).collect(Collectors.toList()); + List inputValDefs = ImmutableKit.filterAndMap(unwrappedInputElements, + it -> it instanceof GraphQLInputValueDefinition, + GraphQLInputValueDefinition.class::cast); this.lastElement = inputValDefs.isEmpty() ? null : inputValDefs.get(inputValDefs.size() - 1); } diff --git a/src/main/java/graphql/collect/ImmutableKit.java b/src/main/java/graphql/collect/ImmutableKit.java index 6fc66280c1..99ba867493 100644 --- a/src/main/java/graphql/collect/ImmutableKit.java +++ b/src/main/java/graphql/collect/ImmutableKit.java @@ -4,23 +4,26 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import graphql.Internal; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; import java.util.Collection; import java.util.List; import java.util.Map; import java.util.function.Function; +import java.util.function.Predicate; import static graphql.Assert.assertNotNull; @Internal -@SuppressWarnings({"UnstableApiUsage"}) +@NullMarked public final class ImmutableKit { public static ImmutableList emptyList() { return ImmutableList.of(); } - public static ImmutableList nonNullCopyOf(Collection collection) { + public static ImmutableList nonNullCopyOf(@Nullable Collection collection) { return collection == null ? emptyList() : ImmutableList.copyOf(collection); } @@ -41,9 +44,9 @@ public static ImmutableList concatLists(List l1, List l2) { * for the flexible style. Benchmarking has shown this to outperform `stream()`. * * @param collection the collection to map - * @param mapper the mapper function - * @param for two - * @param for result + * @param mapper the mapper function + * @param for two + * @param for result * * @return a map immutable list of results */ @@ -58,15 +61,66 @@ public static ImmutableList map(Collection collection, Fu return builder.build(); } + /** + * This is more efficient than `c.stream().filter().collect()` because it does not create the intermediate objects needed + * for the flexible style. Benchmarking has shown this to outperform `stream()`. + * + * @param collection the collection to map + * @param filter the filter predicate + * @param for two + * + * @return a map immutable list of results + */ + public static ImmutableList filter(Collection collection, Predicate filter) { + assertNotNull(collection); + assertNotNull(filter); + return filterAndMap(collection, filter, Function.identity()); + } + + /** + * This is more efficient than `c.stream().filter().map().collect()` because it does not create the intermediate objects needed + * for the flexible style. Benchmarking has shown this to outperform `stream()`. + * + * @param collection the collection to map + * @param filter the filter predicate + * @param mapper the mapper function + * @param for two + * @param for result + * + * @return a map immutable list of results + */ + public static ImmutableList filterAndMap(Collection collection, Predicate filter, Function mapper) { + assertNotNull(collection); + assertNotNull(mapper); + assertNotNull(filter); + ImmutableList.Builder builder = ImmutableList.builderWithExpectedSize(collection.size()); + for (T t : collection) { + if (filter.test(t)) { + R r = mapper.apply(t); + builder.add(r); + } + } + return builder.build(); + } + + public static ImmutableList flatMapList(Collection> listLists) { + ImmutableList.Builder builder = ImmutableList.builder(); + for (List t : listLists) { + builder.addAll(t); + } + return builder.build(); + } + + /** * This will map a collection of items but drop any that are null from the input. * This is more efficient than `c.stream().map().collect()` because it does not create the intermediate objects needed * for the flexible style. Benchmarking has shown this to outperform `stream()`. * * @param collection the collection to map - * @param mapper the mapper function - * @param for two - * @param for result + * @param mapper the mapper function + * @param for two + * @param for result * * @return a map immutable list of results */ diff --git a/src/main/java/graphql/execution/instrumentation/fieldvalidation/FieldValidationSupport.java b/src/main/java/graphql/execution/instrumentation/fieldvalidation/FieldValidationSupport.java index 888a012cc4..454fb30677 100644 --- a/src/main/java/graphql/execution/instrumentation/fieldvalidation/FieldValidationSupport.java +++ b/src/main/java/graphql/execution/instrumentation/fieldvalidation/FieldValidationSupport.java @@ -7,6 +7,7 @@ import graphql.analysis.QueryTraverser; import graphql.analysis.QueryVisitorFieldEnvironment; import graphql.analysis.QueryVisitorStub; +import graphql.collect.ImmutableKit; import graphql.execution.ExecutionContext; import graphql.execution.ResultPath; import graphql.language.Field; @@ -140,7 +141,7 @@ private static class FieldValidationEnvironmentImpl implements FieldValidationEn FieldValidationEnvironmentImpl(ExecutionContext executionContext, Map> fieldArgumentsMap) { this.executionContext = executionContext; this.fieldArgumentsMap = fieldArgumentsMap; - this.fieldArguments = fieldArgumentsMap.values().stream().flatMap(List::stream).collect(ImmutableList.toImmutableList()); + this.fieldArguments = ImmutableKit.flatMapList(fieldArgumentsMap.values()); } diff --git a/src/main/java/graphql/language/AstSignature.java b/src/main/java/graphql/language/AstSignature.java index 84b1d0e871..f6964305b2 100644 --- a/src/main/java/graphql/language/AstSignature.java +++ b/src/main/java/graphql/language/AstSignature.java @@ -1,6 +1,5 @@ package graphql.language; -import com.google.common.collect.ImmutableList; import graphql.PublicApi; import graphql.collect.ImmutableKit; import graphql.util.TraversalControl; @@ -149,16 +148,15 @@ private Document dropUnusedQueryDefinitions(Document document, final String oper NodeVisitorStub visitor = new NodeVisitorStub() { @Override public TraversalControl visitDocument(Document node, TraverserContext context) { - List wantedDefinitions = node.getDefinitions().stream() - .filter(d -> { + List wantedDefinitions = ImmutableKit.filter(node.getDefinitions(), + d -> { if (d instanceof OperationDefinition) { OperationDefinition operationDefinition = (OperationDefinition) d; return isThisOperation(operationDefinition, operationName); } return d instanceof FragmentDefinition; // SDL in a query makes no sense - its gone should it be present - }) - .collect(ImmutableList.toImmutableList()); + }); Document changedNode = node.transform(builder -> { builder.definitions(wantedDefinitions); diff --git a/src/main/java/graphql/language/Document.java b/src/main/java/graphql/language/Document.java index 3fdd459640..1f613686e4 100644 --- a/src/main/java/graphql/language/Document.java +++ b/src/main/java/graphql/language/Document.java @@ -55,10 +55,9 @@ public List getDefinitions() { * @return a list of definitions of that class or empty list */ public List getDefinitionsOfType(Class definitionClass) { - return definitions.stream() - .filter(d -> definitionClass.isAssignableFrom(d.getClass())) - .map(definitionClass::cast) - .collect(ImmutableList.toImmutableList()); + return ImmutableKit.filterAndMap(definitions, + d -> definitionClass.isAssignableFrom(d.getClass()), + definitionClass::cast); } /** diff --git a/src/main/java/graphql/language/NodeParentTree.java b/src/main/java/graphql/language/NodeParentTree.java index fc78ea093d..a5e51d89fc 100644 --- a/src/main/java/graphql/language/NodeParentTree.java +++ b/src/main/java/graphql/language/NodeParentTree.java @@ -3,6 +3,7 @@ import com.google.common.collect.ImmutableList; import graphql.Internal; import graphql.PublicApi; +import graphql.collect.ImmutableKit; import java.util.ArrayDeque; import java.util.ArrayList; @@ -42,10 +43,9 @@ public NodeParentTree(Deque nodeStack) { } private ImmutableList mkPath(Deque copy) { - return copy.stream() - .filter(node1 -> node1 instanceof NamedNode) - .map(node1 -> ((NamedNode) node1).getName()) - .collect(ImmutableList.toImmutableList()); + return ImmutableKit.filterAndMap(copy, + node1 -> node1 instanceof NamedNode, + node1 -> ((NamedNode) node1).getName()); } diff --git a/src/main/java/graphql/language/SelectionSet.java b/src/main/java/graphql/language/SelectionSet.java index 2ff152657c..8e85bdcdef 100644 --- a/src/main/java/graphql/language/SelectionSet.java +++ b/src/main/java/graphql/language/SelectionSet.java @@ -54,10 +54,9 @@ public List getSelections() { * @return a list of selections of that class or empty list */ public List getSelectionsOfType(Class selectionClass) { - return selections.stream() - .filter(d -> selectionClass.isAssignableFrom(d.getClass())) - .map(selectionClass::cast) - .collect(ImmutableList.toImmutableList()); + return ImmutableKit.filterAndMap(selections, + d -> selectionClass.isAssignableFrom(d.getClass()), + selectionClass::cast); } @Override diff --git a/src/main/java/graphql/schema/visibility/BlockedFields.java b/src/main/java/graphql/schema/visibility/BlockedFields.java index 57bd555bc5..937d029d83 100644 --- a/src/main/java/graphql/schema/visibility/BlockedFields.java +++ b/src/main/java/graphql/schema/visibility/BlockedFields.java @@ -1,8 +1,8 @@ package graphql.schema.visibility; -import com.google.common.collect.ImmutableList; import graphql.Internal; import graphql.PublicApi; +import graphql.collect.ImmutableKit; import graphql.schema.GraphQLFieldDefinition; import graphql.schema.GraphQLFieldsContainer; import graphql.schema.GraphQLInputFieldsContainer; @@ -35,9 +35,8 @@ private BlockedFields(List patterns) { @Override public List getFieldDefinitions(GraphQLFieldsContainer fieldsContainer) { - return fieldsContainer.getFieldDefinitions().stream() - .filter(fieldDefinition -> !block(mkFQN(fieldsContainer.getName(), fieldDefinition.getName()))) - .collect(ImmutableList.toImmutableList()); + return ImmutableKit.filter(fieldsContainer.getFieldDefinitions(), + fieldDefinition -> !block(mkFQN(fieldsContainer.getName(), fieldDefinition.getName()))); } @Override @@ -53,9 +52,8 @@ public GraphQLFieldDefinition getFieldDefinition(GraphQLFieldsContainer fieldsCo @Override public List getFieldDefinitions(GraphQLInputFieldsContainer fieldsContainer) { - return fieldsContainer.getFieldDefinitions().stream() - .filter(fieldDefinition -> !block(mkFQN(fieldsContainer.getName(), fieldDefinition.getName()))) - .collect(ImmutableList.toImmutableList()); + return ImmutableKit.filter(fieldsContainer.getFieldDefinitions(), + fieldDefinition -> !block(mkFQN(fieldsContainer.getName(), fieldDefinition.getName()))); } @Override diff --git a/src/test/groovy/graphql/collect/ImmutableKitTest.groovy b/src/test/groovy/graphql/collect/ImmutableKitTest.groovy index 82d76bae1e..f546147d7b 100644 --- a/src/test/groovy/graphql/collect/ImmutableKitTest.groovy +++ b/src/test/groovy/graphql/collect/ImmutableKitTest.groovy @@ -1,7 +1,6 @@ package graphql.collect import com.google.common.collect.ImmutableList -import com.google.common.collect.ImmutableMap import spock.lang.Specification class ImmutableKitTest extends Specification { @@ -63,4 +62,16 @@ class ImmutableKitTest extends Specification { then: set == ["a", "b", "c", "d", "e", "f"] as Set } + + def "flatMapList works"() { + def listOfLists = [ + ["A", "B"], + ["C"], + ["D", "E"], + ] + when: + def flatList = ImmutableKit.flatMapList(listOfLists) + then: + flatList == ["A", "B", "C", "D", "E",] + } } diff --git a/src/test/groovy/graphql/util/FpKitTest.groovy b/src/test/groovy/graphql/util/FpKitTest.groovy index 455e9b57a1..62350ff0d9 100644 --- a/src/test/groovy/graphql/util/FpKitTest.groovy +++ b/src/test/groovy/graphql/util/FpKitTest.groovy @@ -99,20 +99,20 @@ class FpKitTest extends Specification { } def "set intersection works"() { - def set1 = ["A","B","C"] as Set - def set2 = ["A","C","D"] as Set + def set1 = ["A", "B", "C"] as Set + def set2 = ["A", "C", "D"] as Set def singleSetA = ["A"] as Set - def disjointSet = ["X","Y"] as Set + def disjointSet = ["X", "Y"] as Set when: def intersection = FpKit.intersection(set1, set2) then: - intersection == ["A","C"] as Set + intersection == ["A", "C"] as Set when: // reversed parameters intersection = FpKit.intersection(set2, set1) then: - intersection == ["A","C"] as Set + intersection == ["A", "C"] as Set when: // singles intersection = FpKit.intersection(set1, singleSetA) @@ -130,7 +130,7 @@ class FpKitTest extends Specification { intersection.isEmpty() when: // disjoint reversed - intersection = FpKit.intersection(disjointSet,set1) + intersection = FpKit.intersection(disjointSet, set1) then: intersection.isEmpty() }