From e54fd896ad62ed2c5baedb55eba5e23c35b7dac0 Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Mon, 17 Jul 2023 13:26:10 +1200 Subject: [PATCH] Handle repeated query directives --- .../directives/DirectivesResolver.java | 9 ++++--- .../directives/QueryDirectivesImpl.java | 7 ++--- src/main/java/graphql/util/FpKit.java | 2 +- .../directives/QueryDirectivesImplTest.groovy | 27 ++++++++++++++++--- 4 files changed, 34 insertions(+), 11 deletions(-) diff --git a/src/main/java/graphql/execution/directives/DirectivesResolver.java b/src/main/java/graphql/execution/directives/DirectivesResolver.java index 84722c3e4..4163c268c 100644 --- a/src/main/java/graphql/execution/directives/DirectivesResolver.java +++ b/src/main/java/graphql/execution/directives/DirectivesResolver.java @@ -11,6 +11,7 @@ import graphql.schema.GraphQLDirective; import graphql.schema.GraphQLSchema; +import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; @@ -25,14 +26,14 @@ public class DirectivesResolver { public DirectivesResolver() { } - public Map resolveDirectives(List directives, GraphQLSchema schema, Map variables, GraphQLContext graphQLContext, Locale locale) { + public Map> resolveDirectives(List directives, GraphQLSchema schema, Map variables, GraphQLContext graphQLContext, Locale locale) { GraphQLCodeRegistry codeRegistry = schema.getCodeRegistry(); - Map directiveMap = new LinkedHashMap<>(); + Map> directiveMap = new LinkedHashMap<>(); directives.forEach(directive -> { GraphQLDirective protoType = schema.getDirective(directive.getName()); if (protoType != null) { GraphQLDirective newDirective = protoType.transform(builder -> buildArguments(builder, codeRegistry, protoType, directive, variables, graphQLContext, locale)); - directiveMap.put(newDirective.getName(), newDirective); + directiveMap.computeIfAbsent(newDirective.getName(), k -> new ArrayList<>()).add(newDirective); } }); return ImmutableMap.copyOf(directiveMap); @@ -60,4 +61,4 @@ private void buildArguments(GraphQLDirective.Builder directiveBuilder, } }); } -} \ No newline at end of file +} diff --git a/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java b/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java index 85468c347..6516e8b05 100644 --- a/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java +++ b/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java @@ -11,6 +11,7 @@ import graphql.schema.GraphQLArgument; import graphql.schema.GraphQLDirective; import graphql.schema.GraphQLSchema; +import graphql.util.FpKit; import java.util.ArrayList; import java.util.LinkedHashMap; @@ -22,7 +23,7 @@ /** * These objects are ALWAYS in the context of a single MergedField - * + *

* Also note we compute these values lazily */ @Internal @@ -57,11 +58,11 @@ private void computeValuesLazily() { final Map> byFieldApplied = new LinkedHashMap<>(); mergedField.getFields().forEach(field -> { List directives = field.getDirectives(); - ImmutableList resolvedDirectives = ImmutableList.copyOf( + ImmutableList resolvedDirectives = ImmutableList.copyOf(FpKit.flatList( directivesResolver .resolveDirectives(directives, schema, variables, graphQLContext, locale) .values() - ); + )); byField.put(field, resolvedDirectives); // at some point we will only use applied byFieldApplied.put(field, ImmutableKit.map(resolvedDirectives, this::toAppliedDirective)); diff --git a/src/main/java/graphql/util/FpKit.java b/src/main/java/graphql/util/FpKit.java index 86ea96374..a538450ad 100644 --- a/src/main/java/graphql/util/FpKit.java +++ b/src/main/java/graphql/util/FpKit.java @@ -265,7 +265,7 @@ public static CompletableFuture> flatList(CompletableFuture List flatList(List> listLists) { + public static List flatList(Collection> listLists) { return listLists.stream() .flatMap(List::stream) .collect(ImmutableList.toImmutableList()); diff --git a/src/test/groovy/graphql/execution/directives/QueryDirectivesImplTest.groovy b/src/test/groovy/graphql/execution/directives/QueryDirectivesImplTest.groovy index 80c2a861d..227681b55 100644 --- a/src/test/groovy/graphql/execution/directives/QueryDirectivesImplTest.groovy +++ b/src/test/groovy/graphql/execution/directives/QueryDirectivesImplTest.groovy @@ -16,6 +16,8 @@ class QueryDirectivesImplTest extends Specification { directive @cached(forMillis : Int = 99) on FIELD | QUERY directive @upper(place : String) on FIELD + + directive @rep(place : String) repeatable on FIELD type Query { f : String @@ -24,9 +26,7 @@ class QueryDirectivesImplTest extends Specification { def schema = TestUtil.schema(sdl) - def "can get immediate directives"() { - def f1 = TestUtil.parseField("f1 @cached @upper") def f2 = TestUtil.parseField("f2 @cached(forMillis : \$var) @timeout") @@ -68,7 +68,6 @@ class QueryDirectivesImplTest extends Specification { } def "builder works as expected"() { - def f1 = TestUtil.parseField("f1 @cached @upper") def f2 = TestUtil.parseField("f2 @cached(forMillis : \$var) @timeout") @@ -87,6 +86,28 @@ class QueryDirectivesImplTest extends Specification { then: appliedDirectivesByName.keySet().sort() == ["cached", "timeout", "upper"] + } + + def "gets repeated definitions"() { + def f1 = TestUtil.parseField("f1 @rep(place: \$var) @rep(place: \"HELLO\")") + def mergedField = MergedField.newMergedField([f1]).build() + + def queryDirectives = QueryDirectives.newQueryDirectives() + .mergedField(mergedField) + .schema(schema) + .coercedVariables(CoercedVariables.of([var: "ABC"])) + .graphQLContext(GraphQLContext.getDefault()) + .locale(Locale.getDefault()) + .build() + + when: + def appliedDirectivesByName = queryDirectives.getImmediateAppliedDirectivesByName() + + then: + appliedDirectivesByName.keySet() == ["rep"] as Set + appliedDirectivesByName["rep"].size() == 2 + // Groovy is a pathway to many abilities some consider to be unnatural + appliedDirectivesByName["rep"].arguments.value.flatten().sort() == ["ABC", "HELLO"] } }