From 8f998e222fb53aa5b2532005d1a80113b5fc35b4 Mon Sep 17 00:00:00 2001 From: bbaker Date: Thu, 28 Nov 2024 10:33:09 +1100 Subject: [PATCH 01/13] Extra tests to show that the document compiler can inline everything --- ...ormalizedOperationToAstCompilerTest.groovy | 44 ++++++++++++++++++- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy index a135ac3f9b..a1f1ae3329 100644 --- a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy +++ b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy @@ -2068,7 +2068,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat } - def "test a combination of plain objects and interfaces will be all variables"() { + def "test a combination of plain objects and interfaces with all variables and no variables"() { def sdl = ''' type Query { listField1(arg: [Int]): String @@ -2153,6 +2153,46 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat v3: [arg1: "barFragArg"], v4: [arg1: "barArg"], v5: [arg1: "fooArg"]] + + // + // Test the opposite - when we use no variables predicate everything should be inlined + // + when: "it has no variables" + + fields = createNormalizedFields(schema, query, [v0: [1, 2, 3], + v1: [[arg1: "v1", arg2: [[arg1: "v1.1"]]], [arg1: "v2"], [arg1: "v3"]], + v2: [[arg1: "fooNonNullArg1"], [arg1: "fooNonNullArg2"]], + v3: [arg1: "barFragArg"], + v4: [arg1: "barArg"], + v5: [arg1: "fooArg"]]) + + result = localCompileToDocument(schema, QUERY, "named", fields, noVariables) + document = result.document + vars = result.variables + ast = AstPrinter.printAst(new AstSorter().sort(document)) + + then: "they should be inlined as values" + + // no vars created + vars == [:] + + // everything inlined + ast == '''query named { + foo(arg: {arg1 : "fooArg"}) { + bar(arg: {arg1 : "barArg"}) { + baz { + ... on ABaz { + a + boo(arg: {arg1 : "barFragArg"}) + } + } + } + } + fooNonNull(arg: [{arg1 : "fooNonNullArg1"}, {arg1 : "fooNonNullArg2"}]) + listField1(arg: [1, 2, 3]) + listField2(arg: [{arg1 : "v1", arg2 : [{arg1 : "v1.1"}]}, {arg1 : "v2"}, {arg1 : "v3"}]) +} +''' } private ExecutableNormalizedOperation createNormalizedTree(GraphQLSchema schema, String query, Map variables = [:]) { @@ -2186,7 +2226,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat List topLevelFields, VariablePredicate variablePredicate ) { - return localCompileToDocument(schema, operationKind, operationName, topLevelFields,Map.of(), variablePredicate); + return localCompileToDocument(schema, operationKind, operationName, topLevelFields, Map.of(), variablePredicate); } private static ExecutableNormalizedOperationToAstCompiler.CompilerResult localCompileToDocument( From 32802f267b83b608fd70fa35893a46517ba92e93 Mon Sep 17 00:00:00 2001 From: bbaker Date: Thu, 28 Nov 2024 11:24:07 +1100 Subject: [PATCH 02/13] Extra tests to show that the document compiler can inline everything --- ...ormalizedOperationToAstCompilerTest.groovy | 163 +++++++++++++----- 1 file changed, 117 insertions(+), 46 deletions(-) diff --git a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy index a1f1ae3329..f4a72f0090 100644 --- a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy +++ b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy @@ -134,7 +134,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat def fields = createNormalizedFields(schema, query) when: def result = localCompileToDocument(schema, QUERY, null, fields, noVariables) - def printed = AstPrinter.printAst(new AstSorter().sort(result.document)) + def printed = printDoc(result.document) then: printed == '''{ animal { @@ -204,7 +204,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, QUERY, null, tree.topLevelFields, noVariables) - def printed = AstPrinter.printAst(new AstSorter().sort(result.document)) + def printed = printDoc(result.document) then: printed == """{ @@ -255,7 +255,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, QUERY, null, tree.topLevelFields, noVariables) - def printed = AstPrinter.printAst(new AstSorter().sort(result.document)) + def printed = printDoc(result.document) then: printed == """{ @@ -336,7 +336,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, QUERY, null, tree.topLevelFields, noVariables) - def printed = AstPrinter.printAst(new AstSorter().sort(result.document)) + def printed = printDoc(result.document) then: printed == """{ @@ -428,7 +428,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, QUERY, null, tree.topLevelFields, noVariables) - def printed = AstPrinter.printAst(new AstSorter().sort(result.document)) + def printed = printDoc(result.document) then: printed == """{ @@ -523,7 +523,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, QUERY, null, tree.topLevelFields, noVariables) - def printed = AstPrinter.printAst(new AstSorter().sort(result.document)) + def printed = printDoc(result.document) then: printed == """{ @@ -590,7 +590,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, QUERY, null, tree.topLevelFields, noVariables) - def printed = AstPrinter.printAst(new AstSorter().sort(result.document)) + def printed = printDoc(result.document) then: printed == """{ @@ -647,7 +647,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, QUERY, null, tree.topLevelFields, noVariables) - def printed = AstPrinter.printAst(new AstSorter().sort(result.document)) + def printed = printDoc(result.document) then: printed == """{ @@ -705,7 +705,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, QUERY, null, tree.topLevelFields, noVariables) - def printed = AstPrinter.printAst(new AstSorter().sort(result.document)) + def printed = printDoc(result.document) then: // Perhaps the typename should be hoisted out of the fragments, but the impl currently generates @@ -772,7 +772,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, QUERY, null, tree.topLevelFields, noVariables) - def printed = AstPrinter.printAst(new AstSorter().sort(result.document)) + def printed = printDoc(result.document) then: // Note: the name field is spread across both fragments @@ -870,7 +870,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, QUERY, null, tree.topLevelFields, noVariables) - def printed = AstPrinter.printAst(new AstSorter().sort(result.document)) + def printed = printDoc(result.document) then: // Ensure that age location name etc are not surrounded by fragments unnecessarily @@ -968,7 +968,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, QUERY, null, tree.topLevelFields, noVariables) - def printed = AstPrinter.printAst(new AstSorter().sort(result.document)) + def printed = printDoc(result.document) then: // Ensure that __typename id fieldId fieldName etc. are not surrounded by fragments unnecessarily @@ -1035,7 +1035,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat def fields = createNormalizedFields(schema, query) when: def result = localCompileToDocument(schema, QUERY, null, fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: documentPrinted == '''{ @@ -1069,7 +1069,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat def fields = createNormalizedFields(schema, query) when: def result = localCompileToDocument(schema, QUERY, null, fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: documentPrinted == '''{ @@ -1095,7 +1095,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat def fields = createNormalizedFields(schema, query) when: def result = localCompileToDocument(schema, QUERY, "My_Op23", fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: documentPrinted == '''query My_Op23 { @@ -1140,7 +1140,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat def fields = createNormalizedFields(schema, query) when: def result = localCompileToDocument(schema, QUERY, null, fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: documentPrinted == '''{ @@ -1174,7 +1174,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat def fields = createNormalizedFields(schema, query, ["v": 123]) when: def result = localCompileToDocument(schema, QUERY, null, fields, allVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: fields[0].normalizedArguments["arg"].value["a"].value["b"].value["c"].value.isEqualTo(IntValue.of(123)) @@ -1206,7 +1206,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat def fields = createNormalizedFields(schema, query) when: def result = localCompileToDocument(schema, MUTATION, null, fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: documentPrinted == '''mutation { @@ -1237,7 +1237,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat def fields = createNormalizedFields(schema, query) when: def result = localCompileToDocument(schema, SUBSCRIPTION, null, fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: documentPrinted == '''subscription { @@ -1287,7 +1287,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat OperationDefinition operationDefinition = result.document.getDefinitionsOfType(OperationDefinition.class)[0] def fooField = (Field) operationDefinition.selectionSet.children[0] def nameField = (Field) fooField.selectionSet.children[0] - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: @@ -1332,7 +1332,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat def fields = createNormalizedFields(schema, query) when: def result = localCompileToDocument(schema, MUTATION, null, fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: documentPrinted == '''mutation { @@ -1377,7 +1377,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat def fields = createNormalizedFields(schema, query) when: def result = localCompileToDocument(schema, QUERY, null, fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: documentPrinted == '''{ foo1(arg: {arg1 : "Query"}) { @@ -1409,7 +1409,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat def fields = createNormalizedFields(schema, query) when: def result = localCompileToDocument(schema, QUERY, null, fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: documentPrinted == '''{ __schema { @@ -1444,7 +1444,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat def fields = createNormalizedFields(schema, query) when: def result = localCompileToDocument(schema, QUERY, null, fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: documentPrinted == '''{ __type(name: "World") { @@ -1488,7 +1488,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat def fields = createNormalizedFields(schema, query) when: def result = localCompileToDocument(schema, QUERY, null, fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: documentPrinted == '''{ foo1 { @@ -1527,7 +1527,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat def fields = createNormalizedFields(schema, query) when: def result = localCompileToDocument(schema, QUERY, null, fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: documentPrinted == '''{ foo1 { @@ -1578,7 +1578,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat def fields = createNormalizedFields(schema, query) when: def result = localCompileToDocument(schema, QUERY, null, fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: // Note: the typename field moves out of a fragment because AFoo is the only impl documentPrinted == '''{ @@ -1629,7 +1629,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat def fields = createNormalizedFields(schema, query) when: def result = localCompileToDocument(schema, QUERY, null, fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: // Note: the typename field moves out of a fragment because AFoo is the only impl documentPrinted == '''{ @@ -1679,7 +1679,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat def fields = createNormalizedFields(schema, query) when: def result = localCompileToDocument(schema, QUERY, null, fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: // Note: the typename field moves out of a fragment because AFoo is the only impl documentPrinted == '''{ @@ -1715,7 +1715,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, MUTATION, null, fields, jsonVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: result.variables == [v0: ["48x48": "hello"]] @@ -1747,7 +1747,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, MUTATION, null, fields, jsonVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: result.variables == [v0: "hello there"] @@ -1779,7 +1779,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, MUTATION, null, fields, jsonVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: result.variables == [v0: 1] @@ -1809,7 +1809,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, MUTATION, null, fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: result.variables == [:] @@ -1839,7 +1839,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, MUTATION, null, fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: result.variables == [:] @@ -1869,7 +1869,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, MUTATION, null, fields, jsonVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: result.variables == [v0: [one: "two", three: ["four", "five"]]] @@ -1899,7 +1899,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, MUTATION, null, fields, jsonVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: result.variables.size() == 2 @@ -1936,7 +1936,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, MUTATION, null, fields, jsonVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) def vars = result.variables then: @@ -1973,7 +1973,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, MUTATION, null, fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: result.variables == [:] @@ -2008,7 +2008,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, MUTATION, null, fields, noVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: result.variables == [:] @@ -2051,7 +2051,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat when: def result = localCompileToDocument(schema, MUTATION, null, fields, jsonVariables) - def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + def documentPrinted = printDoc(result.document) then: result.variables.size() == 1 @@ -2126,7 +2126,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat def result = localCompileToDocument(schema, QUERY, "named", fields, allVariables) def document = result.document def vars = result.variables - def ast = AstPrinter.printAst(new AstSorter().sort(document)) + def ast = printDoc(document) then: @@ -2169,7 +2169,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat result = localCompileToDocument(schema, QUERY, "named", fields, noVariables) document = result.document vars = result.variables - ast = AstPrinter.printAst(new AstSorter().sort(document)) + ast = printDoc(document) then: "they should be inlined as values" @@ -2195,7 +2195,70 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat ''' } - private ExecutableNormalizedOperation createNormalizedTree(GraphQLSchema schema, String query, Map variables = [:]) { + def "can extract variables or inline values for directives on the query"() { + def sdl = ''' + type Query { + foo(fooArg : String) : Foo + } + + type Foo { + bar(barArg : String) : String + } + + directive @optIn(to : [String!]!) repeatable on FIELD + ''' + + def query = ''' + query named($fooArgVar : String, $barArgVar : String, $skipVar : Boolean!) { + foo(fooArg : $fooArgVar) @skip(if : $skipVar) { + bar(barArg : $barArgVar) @optIn(to : ["optToX"]) + } + } + ''' + GraphQLSchema schema = mkSchema(sdl) + def fields = createNormalizedFields(schema, query, + [fooArgVar: "fooArgVar", barArgVar: "barArgVar", skipVar: false]) + + when: + def result = localCompileToDocument(schema, QUERY, "named", fields, allVariables) + def document = result.document + def vars = result.variables + def ast = printDoc(document) + + then: + vars == [v0:"barArgVar", v1:"fooArgVar"] + // + // the below is what ir currently produces but its WRONG + // fix up when the other tests starts to work + // + ast == '''query named($v0: String, $v1: String) { + foo(fooArg: $v1) { + bar(barArg: $v0) + } +} +''' + + + when: "it has no variables" + + + result = localCompileToDocument(schema, QUERY, "named", fields, noVariables) + document = result.document + vars = result.variables + ast = printDoc(document) + + then: + vars == [:] + ast == '''query named { + foo(fooArg: "fooArgVar") @skip(if: false) { + bar(barArg: "barArgVar") @optIn(to: ["optToX"]) + } +} +''' + + } + + private static ExecutableNormalizedOperation createNormalizedTree(GraphQLSchema schema, String query, Map variables = [:]) { assertValidQuery(schema, query, variables) Document originalDocument = TestUtil.parseQuery(query) @@ -2203,16 +2266,16 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat return ExecutableNormalizedOperationFactory.createExecutableNormalizedOperationWithRawVariables(schema, originalDocument, null, RawVariables.of(variables), options) } - private List createNormalizedFields(GraphQLSchema schema, String query, Map variables = [:]) { + private static List createNormalizedFields(GraphQLSchema schema, String query, Map variables = [:]) { return createNormalizedTree(schema, query, variables).getTopLevelFields() } - private void assertValidQuery(GraphQLSchema graphQLSchema, String query, Map variables = [:]) { + private static void assertValidQuery(GraphQLSchema graphQLSchema, String query, Map variables = [:]) { GraphQL graphQL = GraphQL.newGraphQL(graphQLSchema).build() assert graphQL.execute(newExecutionInput().query(query).variables(variables)).errors.isEmpty() } - GraphQLSchema mkSchema(String sdl) { + static GraphQLSchema mkSchema(String sdl) { def wiringFactory = new TestLiveMockedWiringFactory([JsonScalar.JSON_SCALAR]) def runtimeWiring = RuntimeWiring.newRuntimeWiring() .wiringFactory(wiringFactory).build() @@ -2229,6 +2292,14 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat return localCompileToDocument(schema, operationKind, operationName, topLevelFields, Map.of(), variablePredicate); } + private static Document sortDoc(Document doc) { + return new AstSorter().sort(doc) + } + + private static printDoc(Document doc) { + return AstPrinter.printAst(sortDoc(doc)) + } + private static ExecutableNormalizedOperationToAstCompiler.CompilerResult localCompileToDocument( GraphQLSchema schema, OperationDefinition.Operation operationKind, From 08c6bb833c18034c051725c41206260ab907805e Mon Sep 17 00:00:00 2001 From: bbaker Date: Thu, 28 Nov 2024 14:36:57 +1100 Subject: [PATCH 03/13] Extra tests to show that the document compiler can inline everything or not --- ...ormalizedOperationToAstCompilerTest.groovy | 45 ++++++++++--------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy index f4a72f0090..0c3d024a0c 100644 --- a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy +++ b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy @@ -2209,18 +2209,19 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat ''' def query = ''' - query named($fooArgVar : String, $barArgVar : String, $skipVar : Boolean!) { + query named($fooArgVar : String, $barArgVar : String, $skipVar : Boolean!, $optVar : [String!]!) { foo(fooArg : $fooArgVar) @skip(if : $skipVar) { - bar(barArg : $barArgVar) @optIn(to : ["optToX"]) + bar(barArg : $barArgVar) @optIn(to : ["optToX"]) @optIn(to : $optVar) } } ''' GraphQLSchema schema = mkSchema(sdl) - def fields = createNormalizedFields(schema, query, - [fooArgVar: "fooArgVar", barArgVar: "barArgVar", skipVar: false]) + def eno = createNormalizedTree(schema, query, + [fooArgVar: "fooArgVar", barArgVar: "barArgVar", skipVar: false, optVar : ["optToY"]]) when: - def result = localCompileToDocument(schema, QUERY, "named", fields, allVariables) + def result = localCompileToDocument(schema, QUERY, "named", + eno.getTopLevelFields(),eno.getNormalizedFieldToQueryDirectives(), allVariables) def document = result.document def vars = result.variables def ast = printDoc(document) @@ -2231,18 +2232,19 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat // the below is what ir currently produces but its WRONG // fix up when the other tests starts to work // - ast == '''query named($v0: String, $v1: String) { - foo(fooArg: $v1) { - bar(barArg: $v0) - } -} -''' +// ast == '''query named { +// foo(fooArg: "fooArgVar") @skip(if: false) { +// bar(barArg: "barArgVar") @optIn(to: ["optToX"]) @optIn(to: ["optToY"]) +// } +//} +//''' when: "it has no variables" - result = localCompileToDocument(schema, QUERY, "named", fields, noVariables) + result = localCompileToDocument(schema, QUERY, "named", + eno.getTopLevelFields(),eno.getNormalizedFieldToQueryDirectives(), noVariables) document = result.document vars = result.variables ast = printDoc(document) @@ -2251,7 +2253,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat vars == [:] ast == '''query named { foo(fooArg: "fooArgVar") @skip(if: false) { - bar(barArg: "barArgVar") @optIn(to: ["optToX"]) + bar(barArg: "barArgVar") @optIn(to: ["optToX"]) @optIn(to: ["optToY"]) } } ''' @@ -2292,14 +2294,6 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat return localCompileToDocument(schema, operationKind, operationName, topLevelFields, Map.of(), variablePredicate); } - private static Document sortDoc(Document doc) { - return new AstSorter().sort(doc) - } - - private static printDoc(Document doc) { - return AstPrinter.printAst(sortDoc(doc)) - } - private static ExecutableNormalizedOperationToAstCompiler.CompilerResult localCompileToDocument( GraphQLSchema schema, OperationDefinition.Operation operationKind, @@ -2313,6 +2307,15 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat } return compileToDocument(schema, operationKind, operationName, topLevelFields, normalizedFieldToQueryDirectives, variablePredicate) } + + private static Document sortDoc(Document doc) { + return new AstSorter().sort(doc) + } + + private static printDoc(Document doc) { + return AstPrinter.printAst(sortDoc(doc)) + } + } class ExecutableNormalizedOperationToAstCompilerTestWithDeferSupport extends ExecutableNormalizedOperationToAstCompilerTest { From 3e23fea41c3c6e1ba812ae27ed33b5e481936390 Mon Sep 17 00:00:00 2001 From: bbaker Date: Wed, 4 Dec 2024 14:34:23 +1100 Subject: [PATCH 04/13] This adds more support for Query directives in ENFs and the ability to print them back out - we need NormalisedValues for this --- .../java/graphql/execution/Execution.java | 7 ++ .../graphql/execution/ExecutionContext.java | 6 + .../execution/ExecutionContextBuilder.java | 6 + .../graphql/execution/ExecutionStrategy.java | 1 + .../execution/NormalizedVariables.java | 45 +++++++ .../graphql/execution/ValuesResolver.java | 6 +- .../execution/directives/QueryDirectives.java | 14 +++ .../directives/QueryDirectivesBuilder.java | 10 +- .../directives/QueryDirectivesImpl.java | 58 +++++++-- .../graphql/normalized/ArgumentMaker.java | 110 ++++++++++++++++++ .../ExecutableNormalizedOperationFactory.java | 18 ++- ...tableNormalizedOperationToAstCompiler.java | 90 ++++---------- .../normalized/VariableAccumulator.java | 9 +- .../graphql/normalized/VariablePredicate.java | 23 +++- .../directives/QueryDirectivesImplTest.groovy | 6 +- .../QueryDirectivesIntegrationTest.groovy | 23 ++++ .../values/InputInterceptorTest.groovy | 2 +- ...ormalizedOperationToAstCompilerTest.groovy | 4 +- 18 files changed, 347 insertions(+), 91 deletions(-) create mode 100644 src/main/java/graphql/execution/NormalizedVariables.java create mode 100644 src/main/java/graphql/normalized/ArgumentMaker.java diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index cf805a84fb..260d235fd1 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -77,8 +77,14 @@ public CompletableFuture execute(Document document, GraphQLSche List variableDefinitions = operationDefinition.getVariableDefinitions(); CoercedVariables coercedVariables; + NormalizedVariables normalizedVariableValues; try { coercedVariables = ValuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, inputVariables, executionInput.getGraphQLContext(), executionInput.getLocale()); + + normalizedVariableValues = ValuesResolver.getNormalizedVariableValues(graphQLSchema, + variableDefinitions, + inputVariables, + executionInput.getGraphQLContext(), executionInput.getLocale()); } catch (RuntimeException rte) { if (rte instanceof GraphQLError) { return completedFuture(new ExecutionResultImpl((GraphQLError) rte)); @@ -100,6 +106,7 @@ public CompletableFuture execute(Document document, GraphQLSche .root(executionInput.getRoot()) .fragmentsByName(fragmentsByName) .coercedVariables(coercedVariables) + .normalizedVariableValues(normalizedVariableValues) .document(document) .operationDefinition(operationDefinition) .dataLoaderRegistry(executionInput.getDataLoaderRegistry()) diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index c122f9d2c3..864446607e 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -45,6 +45,7 @@ public class ExecutionContext { private final OperationDefinition operationDefinition; private final Document document; private final CoercedVariables coercedVariables; + private final NormalizedVariables normalizedVariables; private final Object root; private final Object context; private final GraphQLContext graphQLContext; @@ -74,6 +75,7 @@ public class ExecutionContext { this.subscriptionStrategy = builder.subscriptionStrategy; this.fragmentsByName = builder.fragmentsByName; this.coercedVariables = builder.coercedVariables; + this.normalizedVariables = builder.normalizedVariables; this.document = builder.document; this.operationDefinition = builder.operationDefinition; this.context = builder.context; @@ -127,6 +129,10 @@ public CoercedVariables getCoercedVariables() { return coercedVariables; } + public NormalizedVariables getNormalizedVariables() { + return normalizedVariables; + } + /** * @param for two * diff --git a/src/main/java/graphql/execution/ExecutionContextBuilder.java b/src/main/java/graphql/execution/ExecutionContextBuilder.java index b60c793b20..5e1cdd7071 100644 --- a/src/main/java/graphql/execution/ExecutionContextBuilder.java +++ b/src/main/java/graphql/execution/ExecutionContextBuilder.java @@ -38,6 +38,7 @@ public class ExecutionContextBuilder { Document document; OperationDefinition operationDefinition; CoercedVariables coercedVariables = CoercedVariables.emptyVariables(); + NormalizedVariables normalizedVariables = NormalizedVariables.emptyVariables(); ImmutableMap fragmentsByName = ImmutableKit.emptyMap(); DataLoaderRegistry dataLoaderRegistry; Locale locale; @@ -170,6 +171,11 @@ public ExecutionContextBuilder coercedVariables(CoercedVariables coercedVariable return this; } + public ExecutionContextBuilder normalizedVariableValues(NormalizedVariables normalizedVariables) { + this.normalizedVariables = normalizedVariables; + return this; + } + public ExecutionContextBuilder fragmentsByName(Map fragmentsByName) { this.fragmentsByName = ImmutableMap.copyOf(fragmentsByName); return this; diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index f2cccd4072..f751f3ddbb 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -464,6 +464,7 @@ Async.CombinedBuilder getAsyncFieldValueInfo( QueryDirectives queryDirectives = new QueryDirectivesImpl(field, executionContext.getGraphQLSchema(), executionContext.getCoercedVariables().toMap(), + executionContext.getNormalizedVariables().toMap(), executionContext.getGraphQLContext(), executionContext.getLocale()); diff --git a/src/main/java/graphql/execution/NormalizedVariables.java b/src/main/java/graphql/execution/NormalizedVariables.java new file mode 100644 index 0000000000..ef16fec3cf --- /dev/null +++ b/src/main/java/graphql/execution/NormalizedVariables.java @@ -0,0 +1,45 @@ +package graphql.execution; + +import graphql.PublicApi; +import graphql.collect.ImmutableKit; +import graphql.collect.ImmutableMapWithNullValues; +import graphql.normalized.NormalizedInputValue; + +import java.util.Map; + +/** + * Holds coerced variables, that is their values are now in a normalized {@link graphql.normalized.NormalizedInputValue} form. + */ +@PublicApi +public class NormalizedVariables { + private final ImmutableMapWithNullValues normalisedVariables; + + public NormalizedVariables(Map normalisedVariables) { + this.normalisedVariables = ImmutableMapWithNullValues.copyOf(normalisedVariables); + } + + public Map toMap() { + return normalisedVariables; + } + + public boolean containsKey(String key) { + return normalisedVariables.containsKey(key); + } + + public Object get(String key) { + return normalisedVariables.get(key); + } + + public static NormalizedVariables emptyVariables() { + return new NormalizedVariables(ImmutableKit.emptyMap()); + } + + public static NormalizedVariables of(Map normalisedVariables) { + return new NormalizedVariables(normalisedVariables); + } + + @Override + public String toString() { + return normalisedVariables.toString(); + } +} diff --git a/src/main/java/graphql/execution/ValuesResolver.java b/src/main/java/graphql/execution/ValuesResolver.java index 94073cbe6d..e86bc18170 100644 --- a/src/main/java/graphql/execution/ValuesResolver.java +++ b/src/main/java/graphql/execution/ValuesResolver.java @@ -101,7 +101,7 @@ public static CoercedVariables coerceVariableValues(GraphQLSchema schema, * * @return a map of the normalised values */ - public static Map getNormalizedVariableValues( + public static NormalizedVariables getNormalizedVariableValues( GraphQLSchema schema, List variableDefinitions, RawVariables rawVariables, @@ -131,9 +131,7 @@ public static Map getNormalizedVariableValues( } } } - - return result; - + return NormalizedVariables.of(result); } diff --git a/src/main/java/graphql/execution/directives/QueryDirectives.java b/src/main/java/graphql/execution/directives/QueryDirectives.java index 162b6f3c53..7063f566c3 100644 --- a/src/main/java/graphql/execution/directives/QueryDirectives.java +++ b/src/main/java/graphql/execution/directives/QueryDirectives.java @@ -4,7 +4,9 @@ import graphql.PublicApi; import graphql.execution.CoercedVariables; import graphql.execution.MergedField; +import graphql.execution.NormalizedVariables; import graphql.language.Field; +import graphql.normalized.NormalizedInputValue; import graphql.schema.GraphQLDirective; import graphql.schema.GraphQLSchema; @@ -66,6 +68,16 @@ public interface QueryDirectives { */ Map> getImmediateAppliedDirectivesByField(); + /** + * This will return a map of {@link QueryAppliedDirective} to a map of their argument values in {@link NormalizedInputValue} form + *

+ * NOTE : This will only be available when {@link graphql.normalized.ExecutableNormalizedOperationFactory} is used + * to create the {@link QueryAppliedDirective} information + * + * @return a map of applied directive to named argument values + */ + Map> getNormalizedInputValueByImmediateAppliedDirectives(); + /** * This will return a list of the named directives that are immediately on this merged field. * @@ -108,6 +120,8 @@ interface Builder { Builder coercedVariables(CoercedVariables coercedVariables); + Builder normalizedVariables(NormalizedVariables normalizedVariables); + Builder graphQLContext(GraphQLContext graphQLContext); Builder locale(Locale locale); diff --git a/src/main/java/graphql/execution/directives/QueryDirectivesBuilder.java b/src/main/java/graphql/execution/directives/QueryDirectivesBuilder.java index 80f80d1a06..27c98c93dd 100644 --- a/src/main/java/graphql/execution/directives/QueryDirectivesBuilder.java +++ b/src/main/java/graphql/execution/directives/QueryDirectivesBuilder.java @@ -4,6 +4,7 @@ import graphql.Internal; import graphql.execution.CoercedVariables; import graphql.execution.MergedField; +import graphql.execution.NormalizedVariables; import graphql.language.Field; import graphql.schema.GraphQLSchema; @@ -15,6 +16,7 @@ public class QueryDirectivesBuilder implements QueryDirectives.Builder { private MergedField mergedField; private GraphQLSchema schema; private CoercedVariables coercedVariables = CoercedVariables.emptyVariables(); + private NormalizedVariables normalizedVariables = NormalizedVariables.emptyVariables(); private GraphQLContext graphQLContext = GraphQLContext.getDefault(); private Locale locale = Locale.getDefault(); @@ -42,6 +44,12 @@ public QueryDirectives.Builder coercedVariables(CoercedVariables coercedVariable return this; } + @Override + public QueryDirectives.Builder normalizedVariables(NormalizedVariables normalizedVariables) { + this.normalizedVariables = normalizedVariables; + return this; + } + @Override public QueryDirectives.Builder graphQLContext(GraphQLContext graphQLContext) { this.graphQLContext = graphQLContext; @@ -57,6 +65,6 @@ public QueryDirectives.Builder locale(Locale locale) { @Override public QueryDirectives build() { - return new QueryDirectivesImpl(mergedField, schema, coercedVariables.toMap(), graphQLContext, locale); + return new QueryDirectivesImpl(mergedField, schema, coercedVariables.toMap(), normalizedVariables.toMap(), graphQLContext, locale); } } diff --git a/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java b/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java index 68c9b46b57..7981677419 100644 --- a/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java +++ b/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java @@ -1,13 +1,16 @@ package graphql.execution.directives; +import com.google.common.collect.BiMap; +import com.google.common.collect.HashBiMap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import graphql.GraphQLContext; import graphql.Internal; -import graphql.collect.ImmutableKit; import graphql.execution.MergedField; +import graphql.execution.ValuesResolver; import graphql.language.Directive; import graphql.language.Field; +import graphql.normalized.NormalizedInputValue; import graphql.schema.GraphQLArgument; import graphql.schema.GraphQLDirective; import graphql.schema.GraphQLSchema; @@ -33,7 +36,8 @@ public class QueryDirectivesImpl implements QueryDirectives { private final DirectivesResolver directivesResolver = new DirectivesResolver(); private final MergedField mergedField; private final GraphQLSchema schema; - private final Map variables; + private final Map coercedVariables; + private final Map normalizedVariableValues; private final GraphQLContext graphQLContext; private final Locale locale; @@ -42,11 +46,13 @@ public class QueryDirectivesImpl implements QueryDirectives { private volatile ImmutableMap> fieldDirectivesByName; private volatile ImmutableMap> fieldAppliedDirectivesByField; private volatile ImmutableMap> fieldAppliedDirectivesByName; + private volatile ImmutableMap> normalizedValuesByAppliedDirective; - public QueryDirectivesImpl(MergedField mergedField, GraphQLSchema schema, Map variables, GraphQLContext graphQLContext, Locale locale) { + public QueryDirectivesImpl(MergedField mergedField, GraphQLSchema schema, Map coercedVariables, Map normalizedVariableValues, GraphQLContext graphQLContext, Locale locale) { this.mergedField = mergedField; this.schema = schema; - this.variables = variables; + this.coercedVariables = coercedVariables; + this.normalizedVariableValues = normalizedVariableValues; this.graphQLContext = graphQLContext; this.locale = locale; } @@ -56,16 +62,32 @@ private void computeValuesLazily() { final Map> byField = new LinkedHashMap<>(); final Map> byFieldApplied = new LinkedHashMap<>(); + + BiMap directiveCounterParts = HashBiMap.create(); + BiMap gqlDirectiveCounterParts = HashBiMap.create(); + BiMap gqlDirectiveCounterPartsInverse = gqlDirectiveCounterParts.inverse(); mergedField.getFields().forEach(field -> { List directives = field.getDirectives(); ImmutableList resolvedDirectives = ImmutableList.copyOf(FpKit.flatList( directivesResolver - .resolveDirectives(directives, schema, variables, graphQLContext, locale) + .resolveDirectives(directives, schema, coercedVariables, graphQLContext, locale) .values() )); + for (int i = 0; i < directives.size(); i++) { + Directive directive = directives.get(i); + GraphQLDirective graphQLDirective = resolvedDirectives.get(i); + directiveCounterParts.put(graphQLDirective, directive); + } + + ImmutableList.Builder appliedDirectiveBuilder = ImmutableList.builder(); + for (GraphQLDirective resolvedDirective : resolvedDirectives) { + QueryAppliedDirective appliedDirective = toAppliedDirective(resolvedDirective); + gqlDirectiveCounterParts.put(resolvedDirective, appliedDirective); + appliedDirectiveBuilder.add(appliedDirective); + } byField.put(field, resolvedDirectives); // at some point we will only use applied - byFieldApplied.put(field, ImmutableKit.map(resolvedDirectives, this::toAppliedDirective)); + byFieldApplied.put(field, appliedDirectiveBuilder.build()); }); Map> byName = new LinkedHashMap<>(); @@ -74,13 +96,29 @@ private void computeValuesLazily() { String name = directive.getName(); byName.computeIfAbsent(name, k -> new ArrayList<>()).add(directive); // at some point we will only use applied - byNameApplied.computeIfAbsent(name, k -> new ArrayList<>()).add(toAppliedDirective(directive)); + QueryAppliedDirective appliedDirective = gqlDirectiveCounterParts.get(directive); + byNameApplied.computeIfAbsent(name, k -> new ArrayList<>()).add(appliedDirective); })); + // create NormalizedInputValue values for directive arguments + Map> normalizedValuesByAppliedDirective = new LinkedHashMap<>(); + if (this.normalizedVariableValues != null) { + byNameApplied.values().forEach(directiveList -> { + for (QueryAppliedDirective queryAppliedDirective : directiveList) { + GraphQLDirective graphQLDirective = gqlDirectiveCounterPartsInverse.get(queryAppliedDirective); + Directive directive = directiveCounterParts.get(graphQLDirective); + + Map normalizedArgumentValues = ValuesResolver.getNormalizedArgumentValues(graphQLDirective.getArguments(), directive.getArguments(), this.normalizedVariableValues); + normalizedValuesByAppliedDirective.put(queryAppliedDirective, normalizedArgumentValues); + } + }); + } + this.fieldDirectivesByName = ImmutableMap.copyOf(byName); this.fieldDirectivesByField = ImmutableMap.copyOf(byField); this.fieldAppliedDirectivesByName = ImmutableMap.copyOf(byNameApplied); this.fieldAppliedDirectivesByField = ImmutableMap.copyOf(byFieldApplied); + this.normalizedValuesByAppliedDirective = ImmutableMap.copyOf(normalizedValuesByAppliedDirective); }); } @@ -114,6 +152,12 @@ public Map> getImmediateAppliedDirectivesByFi return fieldAppliedDirectivesByField; } + @Override + public Map> getNormalizedInputValueByImmediateAppliedDirectives() { + computeValuesLazily(); + return normalizedValuesByAppliedDirective; + } + @Override public Map> getImmediateDirectivesByName() { computeValuesLazily(); diff --git a/src/main/java/graphql/normalized/ArgumentMaker.java b/src/main/java/graphql/normalized/ArgumentMaker.java new file mode 100644 index 0000000000..29d6a0ae8b --- /dev/null +++ b/src/main/java/graphql/normalized/ArgumentMaker.java @@ -0,0 +1,110 @@ +package graphql.normalized; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import graphql.Internal; +import graphql.execution.directives.QueryAppliedDirective; +import graphql.execution.directives.QueryAppliedDirectiveArgument; +import graphql.execution.directives.QueryDirectives; +import graphql.language.Argument; +import graphql.language.ArrayValue; +import graphql.language.NullValue; +import graphql.language.ObjectField; +import graphql.language.ObjectValue; +import graphql.language.Value; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.List; +import java.util.Map; + +import static graphql.collect.ImmutableKit.emptyMap; +import static graphql.collect.ImmutableKit.map; +import static graphql.language.Argument.newArgument; + +/** + * This class is a peer class and broken out of {@link ExecutableNormalizedOperationToAstCompiler} to deal with + * argument value making. + */ +@Internal +class ArgumentMaker { + + static List createArguments(ExecutableNormalizedField executableNormalizedField, + VariableAccumulator variableAccumulator) { + ImmutableList.Builder result = ImmutableList.builder(); + ImmutableMap normalizedArguments = executableNormalizedField.getNormalizedArguments(); + for (String argName : normalizedArguments.keySet()) { + NormalizedInputValue normalizedInputValue = normalizedArguments.get(argName); + Value value = argValue(executableNormalizedField, null, argName, normalizedInputValue, variableAccumulator); + Argument argument = newArgument() + .name(argName) + .value(value) + .build(); + result.add(argument); + } + return result.build(); + } + + static List createDirectiveArguments(ExecutableNormalizedField executableNormalizedField, + QueryDirectives queryDirectives, + QueryAppliedDirective queryAppliedDirective, + VariableAccumulator variableAccumulator) { + + Map argValueMap = queryDirectives.getNormalizedInputValueByImmediateAppliedDirectives().getOrDefault(queryAppliedDirective, emptyMap()); + + ImmutableList.Builder result = ImmutableList.builder(); + for (QueryAppliedDirectiveArgument directiveArgument : queryAppliedDirective.getArguments()) { + String argName = directiveArgument.getName(); + if (argValueMap != null && argValueMap.containsKey(argName)) { + NormalizedInputValue normalizedInputValue = argValueMap.get(argName); + Value value = argValue(executableNormalizedField, queryAppliedDirective, argName, normalizedInputValue, variableAccumulator); + Argument argument = newArgument() + .name(argName) + .value(value) + .build(); + result.add(argument); + } + } + return result.build(); + } + + @SuppressWarnings("unchecked") + private static Value argValue(ExecutableNormalizedField executableNormalizedField, + QueryAppliedDirective queryAppliedDirective, + String argName, + @Nullable Object value, + VariableAccumulator variableAccumulator) { + if (value instanceof List) { + ArrayValue.Builder arrayValue = ArrayValue.newArrayValue(); + arrayValue.values(map((List) value, val -> argValue(executableNormalizedField, queryAppliedDirective, argName, val, variableAccumulator))); + return arrayValue.build(); + } + if (value instanceof Map) { + ObjectValue.Builder objectValue = ObjectValue.newObjectValue(); + Map map = (Map) value; + for (String fieldName : map.keySet()) { + Value fieldValue = argValue(executableNormalizedField, queryAppliedDirective, argName, (NormalizedInputValue) map.get(fieldName), variableAccumulator); + objectValue.objectField(ObjectField.newObjectField().name(fieldName).value(fieldValue).build()); + } + return objectValue.build(); + } + if (value == null) { + return NullValue.newNullValue().build(); + } + return (Value) value; + } + + @NotNull + private static Value argValue(ExecutableNormalizedField executableNormalizedField, + QueryAppliedDirective queryAppliedDirective, + String argName, + NormalizedInputValue normalizedInputValue, + VariableAccumulator variableAccumulator) { + if (variableAccumulator.shouldMakeVariable(executableNormalizedField, queryAppliedDirective, argName, normalizedInputValue)) { + VariableValueWithDefinition variableWithDefinition = variableAccumulator.accumulateVariable(normalizedInputValue); + return variableWithDefinition.getVariableReference(); + } else { + return argValue(executableNormalizedField, queryAppliedDirective, argName, normalizedInputValue.getValue(), variableAccumulator); + } + } +} diff --git a/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java b/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java index 085fad0afb..6292b46688 100644 --- a/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java +++ b/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java @@ -12,6 +12,7 @@ import graphql.execution.AbortExecutionException; import graphql.execution.CoercedVariables; import graphql.execution.MergedField; +import graphql.execution.NormalizedVariables; import graphql.execution.RawVariables; import graphql.execution.ValuesResolver; import graphql.execution.conditional.ConditionalNodes; @@ -52,6 +53,7 @@ import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.function.Function; import java.util.function.Predicate; @@ -423,7 +425,7 @@ public static ExecutableNormalizedOperation createExecutableNormalizedOperationW rawVariables, options.getGraphQLContext(), options.getLocale()); - Map normalizedVariableValues = ValuesResolver.getNormalizedVariableValues(graphQLSchema, + NormalizedVariables normalizedVariableValues = ValuesResolver.getNormalizedVariableValues(graphQLSchema, variableDefinitions, rawVariables, options.getGraphQLContext(), @@ -445,7 +447,7 @@ private static class ExecutableNormalizedOperationFactoryImpl { private final OperationDefinition operationDefinition; private final Map fragments; private final CoercedVariables coercedVariableValues; - private final @Nullable Map normalizedVariableValues; + private final @Nullable NormalizedVariables normalizedVariableValues; private final Options options; private final List possibleMergerList = new ArrayList<>(); @@ -462,7 +464,7 @@ private ExecutableNormalizedOperationFactoryImpl( OperationDefinition operationDefinition, Map fragments, CoercedVariables coercedVariableValues, - @Nullable Map normalizedVariableValues, + @Nullable NormalizedVariables normalizedVariableValues, Options options ) { this.graphQLSchema = graphQLSchema; @@ -516,7 +518,13 @@ private ExecutableNormalizedOperation createNormalizedQueryImpl() { private void captureMergedField(ExecutableNormalizedField enf, MergedField mergedFld) { // QueryDirectivesImpl is a lazy object and only computes itself when asked for - QueryDirectives queryDirectives = new QueryDirectivesImpl(mergedFld, graphQLSchema, coercedVariableValues.toMap(), options.getGraphQLContext(), options.getLocale()); + Map normalizedVariableValues = Optional.ofNullable(this.normalizedVariableValues).map(NormalizedVariables::toMap).orElse(null); + QueryDirectives queryDirectives = new QueryDirectivesImpl(mergedFld, + graphQLSchema, + coercedVariableValues.toMap(), + normalizedVariableValues, + options.getGraphQLContext(), + options.getLocale()); normalizedFieldToQueryDirectives.put(enf, queryDirectives); normalizedFieldToMergedField.put(enf, mergedFld); } @@ -674,7 +682,7 @@ private ExecutableNormalizedField createNF(CollectedFieldGroup collectedFieldGro Map argumentValues = ValuesResolver.getArgumentValues(fieldDefinition.getArguments(), field.getArguments(), CoercedVariables.of(this.coercedVariableValues.toMap()), this.options.graphQLContext, this.options.locale); Map normalizedArgumentValues = null; if (this.normalizedVariableValues != null) { - normalizedArgumentValues = ValuesResolver.getNormalizedArgumentValues(fieldDefinition.getArguments(), field.getArguments(), this.normalizedVariableValues); + normalizedArgumentValues = ValuesResolver.getNormalizedArgumentValues(fieldDefinition.getArguments(), field.getArguments(), this.normalizedVariableValues.toMap()); } ImmutableList objectTypeNames = map(objectTypes, GraphQLObjectType::getName); return ExecutableNormalizedField.newNormalizedField() diff --git a/src/main/java/graphql/normalized/ExecutableNormalizedOperationToAstCompiler.java b/src/main/java/graphql/normalized/ExecutableNormalizedOperationToAstCompiler.java index 877decb767..cb499eaab4 100644 --- a/src/main/java/graphql/normalized/ExecutableNormalizedOperationToAstCompiler.java +++ b/src/main/java/graphql/normalized/ExecutableNormalizedOperationToAstCompiler.java @@ -1,28 +1,23 @@ package graphql.normalized; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import graphql.Assert; import graphql.Directives; import graphql.ExperimentalApi; import graphql.PublicApi; +import graphql.execution.directives.QueryAppliedDirective; import graphql.execution.directives.QueryDirectives; import graphql.introspection.Introspection; import graphql.language.Argument; -import graphql.language.ArrayValue; import graphql.language.Directive; import graphql.language.Document; import graphql.language.Field; import graphql.language.InlineFragment; -import graphql.language.NullValue; -import graphql.language.ObjectField; -import graphql.language.ObjectValue; import graphql.language.OperationDefinition; import graphql.language.Selection; import graphql.language.SelectionSet; import graphql.language.StringValue; import graphql.language.TypeName; -import graphql.language.Value; import graphql.normalized.incremental.NormalizedDeferredExecution; import graphql.schema.GraphQLCompositeType; import graphql.schema.GraphQLFieldDefinition; @@ -41,12 +36,12 @@ import java.util.stream.Collectors; import static graphql.collect.ImmutableKit.emptyList; -import static graphql.collect.ImmutableKit.map; import static graphql.language.Argument.newArgument; import static graphql.language.Field.newField; import static graphql.language.InlineFragment.newInlineFragment; import static graphql.language.SelectionSet.newSelectionSet; import static graphql.language.TypeName.newTypeName; +import static graphql.normalized.ArgumentMaker.createArguments; import static graphql.schema.GraphQLTypeUtil.unwrapAll; /** @@ -382,20 +377,34 @@ private static Field selectionForNormalizedField(GraphQLSchema schema, QueryDirectives queryDirectives = normalizedFieldToQueryDirectives.get(executableNormalizedField); - Field.Builder builder = newField() .name(executableNormalizedField.getFieldName()) .alias(executableNormalizedField.getAlias()) .selectionSet(selectionSet) .arguments(arguments); + + List directives = buildDirectives(executableNormalizedField, queryDirectives, variableAccumulator); + return builder + .directives(directives) + .build(); + } + + private static @NotNull List buildDirectives(ExecutableNormalizedField executableNormalizedField, QueryDirectives queryDirectives, VariableAccumulator variableAccumulator) { if (queryDirectives == null || queryDirectives.getImmediateAppliedDirectivesByField().isEmpty()) { - return builder.build(); - } else { - List directives = queryDirectives.getImmediateAppliedDirectivesByField().keySet().stream().flatMap(field -> field.getDirectives().stream()).collect(Collectors.toList()); - return builder - .directives(directives) - .build(); + return emptyList(); } + return queryDirectives.getImmediateAppliedDirectivesByField().entrySet().stream() + .flatMap(entry -> entry.getValue().stream()) + .map(queryAppliedDirective -> buildDirective(executableNormalizedField, queryDirectives, queryAppliedDirective, variableAccumulator)) + .collect(Collectors.toList()); + } + + private static Directive buildDirective(ExecutableNormalizedField executableNormalizedField, QueryDirectives queryDirectives, QueryAppliedDirective queryAppliedDirective, VariableAccumulator variableAccumulator) { + + List arguments = ArgumentMaker.createDirectiveArguments(executableNormalizedField,queryDirectives,queryAppliedDirective, variableAccumulator); + return Directive.newDirective() + .name(queryAppliedDirective.getName()) + .arguments(arguments).build(); } @Nullable @@ -407,59 +416,6 @@ private static SelectionSet selectionSet(List fields) { return newSelectionSet().selections(fields).build(); } - private static List createArguments(ExecutableNormalizedField executableNormalizedField, - VariableAccumulator variableAccumulator) { - ImmutableList.Builder result = ImmutableList.builder(); - ImmutableMap normalizedArguments = executableNormalizedField.getNormalizedArguments(); - for (String argName : normalizedArguments.keySet()) { - NormalizedInputValue normalizedInputValue = normalizedArguments.get(argName); - Value value = argValue(executableNormalizedField, argName, normalizedInputValue, variableAccumulator); - Argument argument = newArgument() - .name(argName) - .value(value) - .build(); - result.add(argument); - } - return result.build(); - } - - @SuppressWarnings("unchecked") - private static Value argValue(ExecutableNormalizedField executableNormalizedField, - String argName, - @Nullable Object value, - VariableAccumulator variableAccumulator) { - if (value instanceof List) { - ArrayValue.Builder arrayValue = ArrayValue.newArrayValue(); - arrayValue.values(map((List) value, val -> argValue(executableNormalizedField, argName, val, variableAccumulator))); - return arrayValue.build(); - } - if (value instanceof Map) { - ObjectValue.Builder objectValue = ObjectValue.newObjectValue(); - Map map = (Map) value; - for (String fieldName : map.keySet()) { - Value fieldValue = argValue(executableNormalizedField, argName, (NormalizedInputValue) map.get(fieldName), variableAccumulator); - objectValue.objectField(ObjectField.newObjectField().name(fieldName).value(fieldValue).build()); - } - return objectValue.build(); - } - if (value == null) { - return NullValue.newNullValue().build(); - } - return (Value) value; - } - - @NotNull - private static Value argValue(ExecutableNormalizedField executableNormalizedField, - String argName, - NormalizedInputValue normalizedInputValue, - VariableAccumulator variableAccumulator) { - if (variableAccumulator.shouldMakeVariable(executableNormalizedField, argName, normalizedInputValue)) { - VariableValueWithDefinition variableWithDefinition = variableAccumulator.accumulateVariable(normalizedInputValue); - return variableWithDefinition.getVariableReference(); - } else { - return argValue(executableNormalizedField, argName, normalizedInputValue.getValue(), variableAccumulator); - } - } @NotNull private static GraphQLFieldDefinition getFieldDefinition(GraphQLSchema schema, diff --git a/src/main/java/graphql/normalized/VariableAccumulator.java b/src/main/java/graphql/normalized/VariableAccumulator.java index ccf1d43e12..d2a7fe2a37 100644 --- a/src/main/java/graphql/normalized/VariableAccumulator.java +++ b/src/main/java/graphql/normalized/VariableAccumulator.java @@ -1,6 +1,7 @@ package graphql.normalized; import graphql.Internal; +import graphql.execution.directives.QueryAppliedDirective; import graphql.language.VariableDefinition; import org.jetbrains.annotations.Nullable; @@ -28,8 +29,12 @@ public VariableAccumulator(@Nullable VariablePredicate variablePredicate) { valueWithDefinitions = new ArrayList<>(); } - public boolean shouldMakeVariable(ExecutableNormalizedField executableNormalizedField, String argName, NormalizedInputValue normalizedInputValue) { - return variablePredicate != null && variablePredicate.shouldMakeVariable(executableNormalizedField, argName, normalizedInputValue); + public boolean shouldMakeVariable(ExecutableNormalizedField executableNormalizedField, QueryAppliedDirective queryAppliedDirective, String argName, NormalizedInputValue normalizedInputValue) { + if (queryAppliedDirective != null) { + return variablePredicate != null && variablePredicate.shouldMakeVariable(executableNormalizedField, queryAppliedDirective, argName, normalizedInputValue); + } else { + return variablePredicate != null && variablePredicate.shouldMakeVariable(executableNormalizedField, argName, normalizedInputValue); + } } public VariableValueWithDefinition accumulateVariable(NormalizedInputValue normalizedInputValue) { diff --git a/src/main/java/graphql/normalized/VariablePredicate.java b/src/main/java/graphql/normalized/VariablePredicate.java index e4a0347050..a516c308ec 100644 --- a/src/main/java/graphql/normalized/VariablePredicate.java +++ b/src/main/java/graphql/normalized/VariablePredicate.java @@ -1,6 +1,7 @@ package graphql.normalized; import graphql.PublicSpi; +import graphql.execution.directives.QueryAppliedDirective; /** * This predicate indicates whether a variable should be made for this field argument OR whether it will be compiled @@ -17,5 +18,25 @@ public interface VariablePredicate { * * @return true if a variable should be made */ - boolean shouldMakeVariable(ExecutableNormalizedField executableNormalizedField, String argName, NormalizedInputValue normalizedInputValue); + boolean shouldMakeVariable(ExecutableNormalizedField executableNormalizedField, + String argName, + NormalizedInputValue normalizedInputValue); + + /** + * Return true if a variable should be made for this query directive argument + * on the specified field + * + * @param executableNormalizedField the field in question + * @param queryAppliedDirective the query directive in question + * @param argName the argument on the directive + * @param normalizedInputValue the input value for that argument + * + * @return true if a variable should be made + */ + default boolean shouldMakeVariable(ExecutableNormalizedField executableNormalizedField, + QueryAppliedDirective queryAppliedDirective, + String argName, + NormalizedInputValue normalizedInputValue) { + return false; + } } diff --git a/src/test/groovy/graphql/execution/directives/QueryDirectivesImplTest.groovy b/src/test/groovy/graphql/execution/directives/QueryDirectivesImplTest.groovy index 227681b555..1ddf0f62d3 100644 --- a/src/test/groovy/graphql/execution/directives/QueryDirectivesImplTest.groovy +++ b/src/test/groovy/graphql/execution/directives/QueryDirectivesImplTest.groovy @@ -4,6 +4,9 @@ import graphql.GraphQLContext import graphql.TestUtil import graphql.execution.CoercedVariables import graphql.execution.MergedField +import graphql.execution.NormalizedVariables +import graphql.language.IntValue +import graphql.normalized.NormalizedInputValue import spock.lang.Specification import static graphql.language.AstPrinter.printAst @@ -32,7 +35,7 @@ class QueryDirectivesImplTest extends Specification { def mergedField = MergedField.newMergedField([f1, f2]).build() - def impl = new QueryDirectivesImpl(mergedField, schema, [var: 10], GraphQLContext.getDefault(), Locale.getDefault()) + def impl = new QueryDirectivesImpl(mergedField, schema, [var: 10], [var : new NormalizedInputValue("type", IntValue.of(10))], GraphQLContext.getDefault(), Locale.getDefault()) when: def directives = impl.getImmediateDirectivesByName() @@ -77,6 +80,7 @@ class QueryDirectivesImplTest extends Specification { .mergedField(mergedField) .schema(schema) .coercedVariables(CoercedVariables.of([var: 10])) + .normalizedVariables(NormalizedVariables.of([var: new NormalizedInputValue("type", IntValue.of(10))])) .graphQLContext(GraphQLContext.getDefault()) .locale(Locale.getDefault()) .build() diff --git a/src/test/groovy/graphql/execution/directives/QueryDirectivesIntegrationTest.groovy b/src/test/groovy/graphql/execution/directives/QueryDirectivesIntegrationTest.groovy index 89a5d85a07..ab7c2dc206 100644 --- a/src/test/groovy/graphql/execution/directives/QueryDirectivesIntegrationTest.groovy +++ b/src/test/groovy/graphql/execution/directives/QueryDirectivesIntegrationTest.groovy @@ -1,8 +1,14 @@ package graphql.execution.directives +import com.google.common.collect.ImmutableList import graphql.GraphQL import graphql.TestUtil +import graphql.execution.CoercedVariables +import graphql.execution.RawVariables +import graphql.normalized.ExecutableNormalizedField +import graphql.normalized.ExecutableNormalizedOperationFactory import graphql.schema.DataFetcher +import graphql.schema.FieldCoordinates import spock.lang.Specification /** @@ -134,4 +140,21 @@ class QueryDirectivesIntegrationTest extends Specification { joinArgs(immediate) == "cached(forMillis:10)" } + def "can capture directive argument values inside ENO path"() { + def query = TestUtil.parseQuery(pathologicalQuery) + when: + def eno = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperationWithRawVariables( + schema, query, "Books", RawVariables.emptyVariables()) + + + then: + def booksENF = eno.getTopLevelFields()[0] + booksENF != null + def bookQueryDirectives = eno.getQueryDirectives(booksENF) + bookQueryDirectives.immediateAppliedDirectivesByName.isEmpty() + + def reviewField = eno.getCoordinatesToNormalizedFields().get(FieldCoordinates.coordinates("Book", "review")) + def reviewQueryDirectives = eno.getQueryDirectives(reviewField[0]) + !reviewQueryDirectives.immediateAppliedDirectivesByName.isEmpty() + } } diff --git a/src/test/groovy/graphql/execution/values/InputInterceptorTest.groovy b/src/test/groovy/graphql/execution/values/InputInterceptorTest.groovy index de6914a50d..46b15a3d9f 100644 --- a/src/test/groovy/graphql/execution/values/InputInterceptorTest.groovy +++ b/src/test/groovy/graphql/execution/values/InputInterceptorTest.groovy @@ -98,7 +98,7 @@ class InputInterceptorTest extends Specification { ''') .graphQLContext({ it.put(InputInterceptor.class, interceptor) }) .variables( - "booleanArg": "truthy", + "booleanArg": false, "stringArg": "sdrawkcab" ) diff --git a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy index 0c3d024a0c..f44cb23477 100644 --- a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy +++ b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy @@ -1294,8 +1294,8 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat fooField.directives.size() == 1 nameField.directives.size() == 1 documentPrinted == '''subscription { - foo1(arg: {arg1 : "Subscription"}) @optIn(to: "foo") { - name @optIn(to: "devOps") + foo1(arg: {arg1 : "Subscription"}) @optIn(to: ["foo"]) { + name @optIn(to: ["devOps"]) } } ''' From 641bea451bdaef63cf6184c51615a633f1c99431 Mon Sep 17 00:00:00 2001 From: bbaker Date: Wed, 4 Dec 2024 15:24:30 +1100 Subject: [PATCH 05/13] Tweaked code and fixed test --- .../directives/QueryDirectivesImpl.java | 32 ++++++++++++------- .../groovy/graphql/schema/CoercingTest.groovy | 7 ++++ 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java b/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java index 7981677419..652e43b6a2 100644 --- a/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java +++ b/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java @@ -16,6 +16,7 @@ import graphql.schema.GraphQLSchema; import graphql.util.FpKit; import graphql.util.LockKit; +import org.jetbrains.annotations.Nullable; import java.util.ArrayList; import java.util.LinkedHashMap; @@ -37,6 +38,7 @@ public class QueryDirectivesImpl implements QueryDirectives { private final MergedField mergedField; private final GraphQLSchema schema; private final Map coercedVariables; + @Nullable private final Map normalizedVariableValues; private final GraphQLContext graphQLContext; private final Locale locale; @@ -68,22 +70,26 @@ private void computeValuesLazily() { BiMap gqlDirectiveCounterPartsInverse = gqlDirectiveCounterParts.inverse(); mergedField.getFields().forEach(field -> { List directives = field.getDirectives(); - ImmutableList resolvedDirectives = ImmutableList.copyOf(FpKit.flatList( - directivesResolver - .resolveDirectives(directives, schema, coercedVariables, graphQLContext, locale) - .values() - )); - for (int i = 0; i < directives.size(); i++) { - Directive directive = directives.get(i); + Map astDirectivesByName = FpKit.getByName(directives, Directive::getName); + Map> directivesMap = directivesResolver + .resolveDirectives(directives, schema, coercedVariables, graphQLContext, locale); + ImmutableList resolvedDirectives = ImmutableList.copyOf( + FpKit.flatList(directivesMap.values())); + + // build a counterpart map of GraphQLDirective to AST directive + for (int i = 0; i < resolvedDirectives.size(); i++) { GraphQLDirective graphQLDirective = resolvedDirectives.get(i); - directiveCounterParts.put(graphQLDirective, directive); + Directive astDirective = astDirectivesByName.get(graphQLDirective.getName()); + if (astDirective != null) { + directiveCounterParts.put(graphQLDirective, astDirective); + } } ImmutableList.Builder appliedDirectiveBuilder = ImmutableList.builder(); for (GraphQLDirective resolvedDirective : resolvedDirectives) { QueryAppliedDirective appliedDirective = toAppliedDirective(resolvedDirective); - gqlDirectiveCounterParts.put(resolvedDirective, appliedDirective); appliedDirectiveBuilder.add(appliedDirective); + gqlDirectiveCounterParts.put(resolvedDirective, appliedDirective); } byField.put(field, resolvedDirectives); // at some point we will only use applied @@ -106,10 +112,12 @@ private void computeValuesLazily() { byNameApplied.values().forEach(directiveList -> { for (QueryAppliedDirective queryAppliedDirective : directiveList) { GraphQLDirective graphQLDirective = gqlDirectiveCounterPartsInverse.get(queryAppliedDirective); + // we need this counterpart because the ValuesResolver needs the runtime and AST element Directive directive = directiveCounterParts.get(graphQLDirective); - - Map normalizedArgumentValues = ValuesResolver.getNormalizedArgumentValues(graphQLDirective.getArguments(), directive.getArguments(), this.normalizedVariableValues); - normalizedValuesByAppliedDirective.put(queryAppliedDirective, normalizedArgumentValues); + if (directive != null) { + Map normalizedArgumentValues = ValuesResolver.getNormalizedArgumentValues(graphQLDirective.getArguments(), directive.getArguments(), this.normalizedVariableValues); + normalizedValuesByAppliedDirective.put(queryAppliedDirective, normalizedArgumentValues); + } } }); } diff --git a/src/test/groovy/graphql/schema/CoercingTest.groovy b/src/test/groovy/graphql/schema/CoercingTest.groovy index ad1d306603..5e96baece1 100644 --- a/src/test/groovy/graphql/schema/CoercingTest.groovy +++ b/src/test/groovy/graphql/schema/CoercingTest.groovy @@ -1,6 +1,7 @@ package graphql.schema import graphql.ExecutionInput +import graphql.GraphQLContext import graphql.TestUtil import graphql.analysis.MaxQueryDepthInstrumentation import graphql.language.ArrayValue @@ -13,6 +14,7 @@ import graphql.language.StringValue import graphql.language.VariableReference import graphql.schema.idl.RuntimeWiring import graphql.schema.idl.TypeRuntimeWiring +import org.jetbrains.annotations.NotNull import spock.lang.Specification import java.time.ZonedDateTime @@ -244,6 +246,11 @@ class CoercingTest extends Specification { Object parseLiteral(Object input) throws CoercingParseLiteralException { return input } + + @Override + StringValue valueToLiteral(@NotNull Object input, @NotNull GraphQLContext graphQLContext, @NotNull Locale locale) { + return new StringValue(input.toString()) + } }) .build() From f4939e39ffea67b5ddfbe247810fdd09a73d91b1 Mon Sep 17 00:00:00 2001 From: bbaker Date: Thu, 5 Dec 2024 13:30:39 +1100 Subject: [PATCH 06/13] Added more tests cases to QueryDirectivesIntegrationTest.groovy --- .../QueryDirectivesIntegrationTest.groovy | 55 +++++++++++++++++-- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/src/test/groovy/graphql/execution/directives/QueryDirectivesIntegrationTest.groovy b/src/test/groovy/graphql/execution/directives/QueryDirectivesIntegrationTest.groovy index ab7c2dc206..2b99d6e3be 100644 --- a/src/test/groovy/graphql/execution/directives/QueryDirectivesIntegrationTest.groovy +++ b/src/test/groovy/graphql/execution/directives/QueryDirectivesIntegrationTest.groovy @@ -1,12 +1,12 @@ package graphql.execution.directives -import com.google.common.collect.ImmutableList + import graphql.GraphQL import graphql.TestUtil -import graphql.execution.CoercedVariables import graphql.execution.RawVariables -import graphql.normalized.ExecutableNormalizedField +import graphql.language.IntValue import graphql.normalized.ExecutableNormalizedOperationFactory +import graphql.normalized.NormalizedInputValue import graphql.schema.DataFetcher import graphql.schema.FieldCoordinates import spock.lang.Specification @@ -155,6 +155,53 @@ class QueryDirectivesIntegrationTest extends Specification { def reviewField = eno.getCoordinatesToNormalizedFields().get(FieldCoordinates.coordinates("Book", "review")) def reviewQueryDirectives = eno.getQueryDirectives(reviewField[0]) - !reviewQueryDirectives.immediateAppliedDirectivesByName.isEmpty() + def reviewImmediateDirectivesMap = reviewQueryDirectives.immediateAppliedDirectivesByName + def argInputValues = simplifiedInputValuesWithState(reviewImmediateDirectivesMap) + argInputValues == [ + timeout: [ + [timeout: [[afterMillis: 5]]], [timeout: [[afterMillis: 28]]], [timeout: [[afterMillis: 10]]] + ], + cached : [ + [cached: [[forMillis: 5]]], [cached: [[forMillis: 10]]] + ] + ] + + // normalised values are AST values + def normalizedValues = simplifiedNormalizedValues(reviewQueryDirectives.getNormalizedInputValueByImmediateAppliedDirectives()) + normalizedValues == [ + timeout: [ + [afterMillis: 5], [afterMillis: 28], [afterMillis: 10]], + cached : [ + [forMillis: 5], [forMillis: 10]] + ] + + } + + def simplifiedInputValuesWithState(Map> mapOfDirectives) { + def simpleMap = [:] + mapOfDirectives.forEach { k, listOfDirectives -> + + def dirVals = listOfDirectives.collect { qd -> + def argVals = qd.getArguments().collect { arg -> + def argValue = arg.getArgumentValue() + return [(arg.name): argValue?.value] + } + return [(qd.name): argVals] + } + simpleMap[k] = dirVals + } + return simpleMap + } + + def simplifiedNormalizedValues(Map> mapOfDirectives) { + Map>> simpleMap = new LinkedHashMap<>() + mapOfDirectives.forEach { qd, argMap -> + def argVals = argMap.collect { entry -> + def argValueAst = entry.value?.value as IntValue // just assume INtValue for these tests + return [(entry.key): argValueAst?.value?.toInteger()] + } + simpleMap.computeIfAbsent(qd.name, { _ -> [] }).addAll(argVals) + } + return simpleMap } } From 6563ef9124f1dffc3b9275f51c155c6e329dfc26 Mon Sep 17 00:00:00 2001 From: bbaker Date: Thu, 5 Dec 2024 16:25:21 +1100 Subject: [PATCH 07/13] Fixed up tests and made a new test class --- .../directives/DirectivesResolver.java | 16 +- .../directives/QueryDirectivesImpl.java | 18 +- .../ENOToAstCompilerDirectivesTest.groovy | 169 +++++++++++++++ ...ormalizedOperationToAstCompilerTest.groovy | 205 +++++++----------- 4 files changed, 263 insertions(+), 145 deletions(-) create mode 100644 src/test/groovy/graphql/normalized/ENOToAstCompilerDirectivesTest.groovy diff --git a/src/main/java/graphql/execution/directives/DirectivesResolver.java b/src/main/java/graphql/execution/directives/DirectivesResolver.java index 4163c268cb..187f7ef5e6 100644 --- a/src/main/java/graphql/execution/directives/DirectivesResolver.java +++ b/src/main/java/graphql/execution/directives/DirectivesResolver.java @@ -1,6 +1,8 @@ package graphql.execution.directives; -import com.google.common.collect.ImmutableMap; +import com.google.common.collect.BiMap; +import com.google.common.collect.HashBiMap; +import com.google.common.collect.ImmutableBiMap; import graphql.GraphQLContext; import graphql.Internal; import graphql.execution.CoercedVariables; @@ -11,8 +13,6 @@ import graphql.schema.GraphQLDirective; import graphql.schema.GraphQLSchema; -import java.util.ArrayList; -import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -26,17 +26,17 @@ public class DirectivesResolver { public DirectivesResolver() { } - public Map> resolveDirectives(List directives, GraphQLSchema schema, Map variables, GraphQLContext graphQLContext, Locale locale) { + public BiMap resolveDirectives(List directives, GraphQLSchema schema, Map variables, GraphQLContext graphQLContext, Locale locale) { GraphQLCodeRegistry codeRegistry = schema.getCodeRegistry(); - Map> directiveMap = new LinkedHashMap<>(); + BiMap directiveMap = HashBiMap.create(); 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.computeIfAbsent(newDirective.getName(), k -> new ArrayList<>()).add(newDirective); + GraphQLDirective graphQLDirective = protoType.transform(builder -> buildArguments(builder, codeRegistry, protoType, directive, variables, graphQLContext, locale)); + directiveMap.put(graphQLDirective, directive); } }); - return ImmutableMap.copyOf(directiveMap); + return ImmutableBiMap.copyOf(directiveMap); } private void buildArguments(GraphQLDirective.Builder directiveBuilder, diff --git a/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java b/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java index 652e43b6a2..fe216a1341 100644 --- a/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java +++ b/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java @@ -70,20 +70,12 @@ private void computeValuesLazily() { BiMap gqlDirectiveCounterPartsInverse = gqlDirectiveCounterParts.inverse(); mergedField.getFields().forEach(field -> { List directives = field.getDirectives(); - Map astDirectivesByName = FpKit.getByName(directives, Directive::getName); - Map> directivesMap = directivesResolver + BiMap directivesMap = directivesResolver .resolveDirectives(directives, schema, coercedVariables, graphQLContext, locale); - ImmutableList resolvedDirectives = ImmutableList.copyOf( - FpKit.flatList(directivesMap.values())); - - // build a counterpart map of GraphQLDirective to AST directive - for (int i = 0; i < resolvedDirectives.size(); i++) { - GraphQLDirective graphQLDirective = resolvedDirectives.get(i); - Directive astDirective = astDirectivesByName.get(graphQLDirective.getName()); - if (astDirective != null) { - directiveCounterParts.put(graphQLDirective, astDirective); - } - } + + directiveCounterParts.putAll(directivesMap); + + ImmutableList resolvedDirectives = ImmutableList.copyOf(directivesMap.keySet()); ImmutableList.Builder appliedDirectiveBuilder = ImmutableList.builder(); for (GraphQLDirective resolvedDirective : resolvedDirectives) { diff --git a/src/test/groovy/graphql/normalized/ENOToAstCompilerDirectivesTest.groovy b/src/test/groovy/graphql/normalized/ENOToAstCompilerDirectivesTest.groovy new file mode 100644 index 0000000000..9d55d3dccc --- /dev/null +++ b/src/test/groovy/graphql/normalized/ENOToAstCompilerDirectivesTest.groovy @@ -0,0 +1,169 @@ +package graphql.normalized + +import graphql.schema.GraphQLSchema + +import static graphql.language.OperationDefinition.Operation.QUERY + +/** + * Test related to ENO and directives + */ +class ENOToAstCompilerDirectivesTest extends ENOToAstCompilerTestBase { + + def "can extract variables or inline values for directives on the query"() { + def sdl = ''' + type Query { + foo(fooArg : String) : Foo + } + + type Foo { + bar(barArg : String) : String + } + + directive @optIn(to : [String!]!) repeatable on FIELD + ''' + + def query = ''' + query named($fooArgVar : String, $barArgVar : String, $skipVar : Boolean!, $optVar : [String!]!) { + foo(fooArg : $fooArgVar) @skip(if : $skipVar) { + bar(barArg : $barArgVar) @optIn(to : ["optToX"]) @optIn(to : $optVar) + } + } + ''' + GraphQLSchema schema = mkSchema(sdl) + def eno = createNormalizedTree(schema, query, + [fooArgVar: "fooArgVar", barArgVar: "barArgVar", skipVar: false, optVar: ["optToY"]]) + + when: + def result = localCompileToDocument(schema, QUERY, "named", + eno.getTopLevelFields(), eno.getNormalizedFieldToQueryDirectives(), + allVariables) + def document = result.document + def vars = result.variables + def ast = printDoc(document) + + then: + vars == [v0: "barArgVar", v1: ["optToX"], v2: ["optToY"], v3: "fooArgVar", v4: false] + // + // the below is what ir currently produces but its WRONG + // fix up when the other tests starts to work + // + ast == '''query named($v0: String, $v1: [String!]!, $v2: [String!]!, $v3: String, $v4: Boolean!) { + foo(fooArg: $v3) @skip(if: $v4) { + bar(barArg: $v0) @optIn(to: $v1) @optIn(to: $v2) + } +} +''' + + + when: "it has no variables" + + + result = localCompileToDocument(schema, QUERY, "named", + eno.getTopLevelFields(), eno.getNormalizedFieldToQueryDirectives(), + noVariables) + document = result.document + vars = result.variables + ast = printDoc(document) + + then: + vars == [:] + ast == '''query named { + foo(fooArg: "fooArgVar") @skip(if: false) { + bar(barArg: "barArgVar") @optIn(to: ["optToX"]) @optIn(to: ["optToY"]) + } +} +''' + + } + + def "can handle quite a pathological fragment query as expected"() { + def sdl = ''' + directive @timeout(afterMillis : Int) on FIELD | FRAGMENT_DEFINITION | FRAGMENT_SPREAD | INLINE_FRAGMENT | QUERY + + directive @cached(forMillis : Int) on FIELD | FRAGMENT_DEFINITION | FRAGMENT_SPREAD | INLINE_FRAGMENT | QUERY + + directive @importance(place : String) on FIELD | FRAGMENT_DEFINITION | FRAGMENT_SPREAD | INLINE_FRAGMENT | QUERY + + type Query { + books(searchString : String) : [Book] + } + + type Book { + id : ID + title : String + review : String + } + ''' + + def pathologicalQuery = ''' + fragment Details on Book @timeout(afterMillis: 25) @cached(forMillis : 25) @importance(place:"FragDef") { + title + review @timeout(afterMillis: 5) @cached(forMillis : 5) + ...InnerDetails @timeout(afterMillis: 26) + } + + fragment InnerDetails on Book @timeout(afterMillis: 27) { + review @timeout(afterMillis: 28) + } + + query Books @timeout(afterMillis: 30) @importance(place:"Operation") { + books(searchString: "monkey") { + id + ...Details @timeout(afterMillis: 20) + ...on Book @timeout(afterMillis: 15) { + review @timeout(afterMillis: 10) @cached(forMillis : 10) + } + } + } + ''' + + GraphQLSchema schema = mkSchema(sdl) + def eno = createNormalizedTree(schema, pathologicalQuery, [:]) + + when: + def result = localCompileToDocument(schema, QUERY, "Books", + eno.getTopLevelFields(), eno.getNormalizedFieldToQueryDirectives(), + allVariables) + def document = result.document + def vars = result.variables + def ast = printDoc(document) + + then: + vars == [v0:5, v1:5, v2:28, v3:10, v4:10, v5:"monkey"] + // + // the below is what ir currently produces but its WRONG + // fix up when the other tests starts to work + // + ast == '''query Books($v0: Int, $v1: Int, $v2: Int, $v3: Int, $v4: Int, $v5: String) { + books(searchString: $v5) { + id + review @cached(forMillis: $v1) @cached(forMillis: $v4) @timeout(afterMillis: $v0) @timeout(afterMillis: $v2) @timeout(afterMillis: $v3) + title + } +} +''' + + + when: "it has no variables" + + + result = localCompileToDocument(schema, QUERY, "Books", + eno.getTopLevelFields(), eno.getNormalizedFieldToQueryDirectives(), + noVariables) + document = result.document + vars = result.variables + ast = printDoc(document) + + then: + vars == [:] + ast == '''query Books { + books(searchString: "monkey") { + id + review @cached(forMillis: 5) @cached(forMillis: 10) @timeout(afterMillis: 5) @timeout(afterMillis: 28) @timeout(afterMillis: 10) + title + } +} +''' + + } +} diff --git a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy index f44cb23477..8f4a0851ac 100644 --- a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy +++ b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy @@ -3,6 +3,7 @@ package graphql.normalized import graphql.GraphQL import graphql.TestUtil import graphql.execution.RawVariables +import graphql.execution.directives.QueryAppliedDirective import graphql.execution.directives.QueryDirectives import graphql.language.AstPrinter import graphql.language.AstSorter @@ -25,11 +26,16 @@ import static graphql.language.OperationDefinition.Operation.SUBSCRIPTION import static graphql.normalized.ExecutableNormalizedOperationToAstCompiler.compileToDocument import static graphql.normalized.ExecutableNormalizedOperationToAstCompiler.compileToDocumentWithDeferSupport -abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specification { +abstract class ENOToAstCompilerTestBase extends Specification { static boolean deferSupport - VariablePredicate noVariables = new VariablePredicate() { + + @Override + boolean shouldMakeVariable(ExecutableNormalizedField executableNormalizedField, QueryAppliedDirective queryAppliedDirective, String argName, NormalizedInputValue normalizedInputValue) { + return false + } + @Override boolean shouldMakeVariable(ExecutableNormalizedField executableNormalizedField, String argName, NormalizedInputValue normalizedInputValue) { return false @@ -41,6 +47,11 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat boolean shouldMakeVariable(ExecutableNormalizedField executableNormalizedField, String argName, NormalizedInputValue normalizedInputValue) { "JSON" == normalizedInputValue.unwrappedTypeName && normalizedInputValue.value != null } + + @Override + boolean shouldMakeVariable(ExecutableNormalizedField executableNormalizedField, QueryAppliedDirective queryAppliedDirective, String argName, NormalizedInputValue normalizedInputValue) { + "JSON" == normalizedInputValue.unwrappedTypeName && normalizedInputValue.value != null + } } VariablePredicate allVariables = new VariablePredicate() { @@ -48,8 +59,76 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat boolean shouldMakeVariable(ExecutableNormalizedField executableNormalizedField, String argName, NormalizedInputValue normalizedInputValue) { return true } + + @Override + boolean shouldMakeVariable(ExecutableNormalizedField executableNormalizedField, QueryAppliedDirective queryAppliedDirective, String argName, NormalizedInputValue normalizedInputValue) { + return true + } + } + + + static ExecutableNormalizedOperation createNormalizedTree(GraphQLSchema schema, String query, Map variables = [:]) { + assertValidQuery(schema, query, variables) + Document originalDocument = TestUtil.parseQuery(query) + + def options = ExecutableNormalizedOperationFactory.Options.defaultOptions().deferSupport(deferSupport) + return ExecutableNormalizedOperationFactory.createExecutableNormalizedOperationWithRawVariables(schema, originalDocument, null, RawVariables.of(variables), options) + } + + static List createNormalizedFields(GraphQLSchema schema, String query, Map variables = [:]) { + return createNormalizedTree(schema, query, variables).getTopLevelFields() + } + + static void assertValidQuery(GraphQLSchema graphQLSchema, String query, Map variables = [:]) { + GraphQL graphQL = GraphQL.newGraphQL(graphQLSchema).build() + assert graphQL.execute(newExecutionInput().query(query).variables(variables)).errors.isEmpty() + } + + static GraphQLSchema mkSchema(String sdl) { + def wiringFactory = new TestLiveMockedWiringFactory([JsonScalar.JSON_SCALAR]) + def runtimeWiring = RuntimeWiring.newRuntimeWiring() + .wiringFactory(wiringFactory).build() + TestUtil.schema(sdl, runtimeWiring) + } + + static ExecutableNormalizedOperationToAstCompiler.CompilerResult localCompileToDocument( + GraphQLSchema schema, + OperationDefinition.Operation operationKind, + String operationName, + List topLevelFields, + VariablePredicate variablePredicate + ) { + return localCompileToDocument(schema, operationKind, operationName, topLevelFields, Map.of(), variablePredicate); + } + + static ExecutableNormalizedOperationToAstCompiler.CompilerResult localCompileToDocument( + GraphQLSchema schema, + OperationDefinition.Operation operationKind, + String operationName, + List topLevelFields, + Map normalizedFieldToQueryDirectives, + VariablePredicate variablePredicate + ) { + if (deferSupport) { + return compileToDocumentWithDeferSupport(schema, operationKind, operationName, topLevelFields, normalizedFieldToQueryDirectives, variablePredicate) + } + return compileToDocument(schema, operationKind, operationName, topLevelFields, normalizedFieldToQueryDirectives, variablePredicate) } + static Document sortDoc(Document doc) { + return new AstSorter().sort(doc) + } + + static printDoc(Document doc) { + return AstPrinter.printAst(sortDoc(doc)) + } +} + +/** + * Test code in here - helps in the base class + */ +abstract class ExecutableNormalizedOperationToAstCompilerTest extends ENOToAstCompilerTestBase { + def "test pet interfaces"() { String sdl = """ type Query { @@ -2194,128 +2273,6 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat } ''' } - - def "can extract variables or inline values for directives on the query"() { - def sdl = ''' - type Query { - foo(fooArg : String) : Foo - } - - type Foo { - bar(barArg : String) : String - } - - directive @optIn(to : [String!]!) repeatable on FIELD - ''' - - def query = ''' - query named($fooArgVar : String, $barArgVar : String, $skipVar : Boolean!, $optVar : [String!]!) { - foo(fooArg : $fooArgVar) @skip(if : $skipVar) { - bar(barArg : $barArgVar) @optIn(to : ["optToX"]) @optIn(to : $optVar) - } - } - ''' - GraphQLSchema schema = mkSchema(sdl) - def eno = createNormalizedTree(schema, query, - [fooArgVar: "fooArgVar", barArgVar: "barArgVar", skipVar: false, optVar : ["optToY"]]) - - when: - def result = localCompileToDocument(schema, QUERY, "named", - eno.getTopLevelFields(),eno.getNormalizedFieldToQueryDirectives(), allVariables) - def document = result.document - def vars = result.variables - def ast = printDoc(document) - - then: - vars == [v0:"barArgVar", v1:"fooArgVar"] - // - // the below is what ir currently produces but its WRONG - // fix up when the other tests starts to work - // -// ast == '''query named { -// foo(fooArg: "fooArgVar") @skip(if: false) { -// bar(barArg: "barArgVar") @optIn(to: ["optToX"]) @optIn(to: ["optToY"]) -// } -//} -//''' - - - when: "it has no variables" - - - result = localCompileToDocument(schema, QUERY, "named", - eno.getTopLevelFields(),eno.getNormalizedFieldToQueryDirectives(), noVariables) - document = result.document - vars = result.variables - ast = printDoc(document) - - then: - vars == [:] - ast == '''query named { - foo(fooArg: "fooArgVar") @skip(if: false) { - bar(barArg: "barArgVar") @optIn(to: ["optToX"]) @optIn(to: ["optToY"]) - } -} -''' - - } - - private static ExecutableNormalizedOperation createNormalizedTree(GraphQLSchema schema, String query, Map variables = [:]) { - assertValidQuery(schema, query, variables) - Document originalDocument = TestUtil.parseQuery(query) - - def options = ExecutableNormalizedOperationFactory.Options.defaultOptions().deferSupport(deferSupport) - return ExecutableNormalizedOperationFactory.createExecutableNormalizedOperationWithRawVariables(schema, originalDocument, null, RawVariables.of(variables), options) - } - - private static List createNormalizedFields(GraphQLSchema schema, String query, Map variables = [:]) { - return createNormalizedTree(schema, query, variables).getTopLevelFields() - } - - private static void assertValidQuery(GraphQLSchema graphQLSchema, String query, Map variables = [:]) { - GraphQL graphQL = GraphQL.newGraphQL(graphQLSchema).build() - assert graphQL.execute(newExecutionInput().query(query).variables(variables)).errors.isEmpty() - } - - static GraphQLSchema mkSchema(String sdl) { - def wiringFactory = new TestLiveMockedWiringFactory([JsonScalar.JSON_SCALAR]) - def runtimeWiring = RuntimeWiring.newRuntimeWiring() - .wiringFactory(wiringFactory).build() - TestUtil.schema(sdl, runtimeWiring) - } - - private static ExecutableNormalizedOperationToAstCompiler.CompilerResult localCompileToDocument( - GraphQLSchema schema, - OperationDefinition.Operation operationKind, - String operationName, - List topLevelFields, - VariablePredicate variablePredicate - ) { - return localCompileToDocument(schema, operationKind, operationName, topLevelFields, Map.of(), variablePredicate); - } - - private static ExecutableNormalizedOperationToAstCompiler.CompilerResult localCompileToDocument( - GraphQLSchema schema, - OperationDefinition.Operation operationKind, - String operationName, - List topLevelFields, - Map normalizedFieldToQueryDirectives, - VariablePredicate variablePredicate - ) { - if (deferSupport) { - return compileToDocumentWithDeferSupport(schema, operationKind, operationName, topLevelFields, normalizedFieldToQueryDirectives, variablePredicate) - } - return compileToDocument(schema, operationKind, operationName, topLevelFields, normalizedFieldToQueryDirectives, variablePredicate) - } - - private static Document sortDoc(Document doc) { - return new AstSorter().sort(doc) - } - - private static printDoc(Document doc) { - return AstPrinter.printAst(sortDoc(doc)) - } - } class ExecutableNormalizedOperationToAstCompilerTestWithDeferSupport extends ExecutableNormalizedOperationToAstCompilerTest { From fe13e10396c34167e3722ae75ddd8c647d763ffc Mon Sep 17 00:00:00 2001 From: bbaker Date: Fri, 6 Dec 2024 13:51:08 +1100 Subject: [PATCH 08/13] made a supplier of normalised variables --- .../java/graphql/execution/Execution.java | 18 ++++++++--- .../graphql/execution/ExecutionContext.java | 7 +++-- .../execution/ExecutionContextBuilder.java | 5 +-- .../graphql/execution/ExecutionStrategy.java | 4 +-- .../directives/DirectivesResolver.java | 6 ++-- .../execution/directives/QueryDirectives.java | 3 +- .../directives/QueryDirectivesBuilder.java | 7 +++-- .../directives/QueryDirectivesImpl.java | 31 +++++++++++-------- .../ExecutableNormalizedOperationFactory.java | 5 ++- .../directives/QueryDirectivesImplTest.groovy | 7 +++-- 10 files changed, 57 insertions(+), 36 deletions(-) diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 260d235fd1..11a33b90ba 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -28,6 +28,7 @@ import graphql.schema.GraphQLObjectType; import graphql.schema.GraphQLSchema; import graphql.schema.impl.SchemaUtil; +import graphql.util.FpKit; import org.reactivestreams.Publisher; import java.util.Collections; @@ -35,6 +36,7 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.CompletableFuture; +import java.util.function.Supplier; import static graphql.execution.ExecutionContextBuilder.newExecutionContextBuilder; import static graphql.execution.ExecutionStepInfo.newExecutionStepInfo; @@ -77,14 +79,20 @@ public CompletableFuture execute(Document document, GraphQLSche List variableDefinitions = operationDefinition.getVariableDefinitions(); CoercedVariables coercedVariables; - NormalizedVariables normalizedVariableValues; + Supplier normalizedVariableValues; try { - coercedVariables = ValuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, inputVariables, executionInput.getGraphQLContext(), executionInput.getLocale()); - - normalizedVariableValues = ValuesResolver.getNormalizedVariableValues(graphQLSchema, + coercedVariables = ValuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, inputVariables, - executionInput.getGraphQLContext(), executionInput.getLocale()); + executionInput.getGraphQLContext(), + executionInput.getLocale()); + + normalizedVariableValues = FpKit.intraThreadMemoize(() -> + ValuesResolver.getNormalizedVariableValues(graphQLSchema, + variableDefinitions, + inputVariables, + executionInput.getGraphQLContext(), executionInput.getLocale()) + ); } catch (RuntimeException rte) { if (rte instanceof GraphQLError) { return completedFuture(new ExecutionResultImpl((GraphQLError) rte)); diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index 864446607e..a9c6a376e3 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -45,7 +45,7 @@ public class ExecutionContext { private final OperationDefinition operationDefinition; private final Document document; private final CoercedVariables coercedVariables; - private final NormalizedVariables normalizedVariables; + private final Supplier normalizedVariables; private final Object root; private final Object context; private final GraphQLContext graphQLContext; @@ -129,7 +129,10 @@ public CoercedVariables getCoercedVariables() { return coercedVariables; } - public NormalizedVariables getNormalizedVariables() { + /** + * @return a supplier that will give out the operations variables in normalized form + */ + public Supplier getNormalizedVariables() { return normalizedVariables; } diff --git a/src/main/java/graphql/execution/ExecutionContextBuilder.java b/src/main/java/graphql/execution/ExecutionContextBuilder.java index 5e1cdd7071..f45f4408b3 100644 --- a/src/main/java/graphql/execution/ExecutionContextBuilder.java +++ b/src/main/java/graphql/execution/ExecutionContextBuilder.java @@ -18,6 +18,7 @@ import java.util.Locale; import java.util.Map; +import java.util.function.Supplier; import static graphql.Assert.assertNotNull; import static graphql.collect.ImmutableKit.emptyList; @@ -38,7 +39,7 @@ public class ExecutionContextBuilder { Document document; OperationDefinition operationDefinition; CoercedVariables coercedVariables = CoercedVariables.emptyVariables(); - NormalizedVariables normalizedVariables = NormalizedVariables.emptyVariables(); + Supplier normalizedVariables = NormalizedVariables::emptyVariables; ImmutableMap fragmentsByName = ImmutableKit.emptyMap(); DataLoaderRegistry dataLoaderRegistry; Locale locale; @@ -171,7 +172,7 @@ public ExecutionContextBuilder coercedVariables(CoercedVariables coercedVariable return this; } - public ExecutionContextBuilder normalizedVariableValues(NormalizedVariables normalizedVariables) { + public ExecutionContextBuilder normalizedVariableValues(Supplier normalizedVariables) { this.normalizedVariables = normalizedVariables; return this; } diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index f751f3ddbb..0e1629a6a2 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -463,8 +463,8 @@ Async.CombinedBuilder getAsyncFieldValueInfo( DataFetchingFieldSelectionSet fieldCollector = DataFetchingFieldSelectionSetImpl.newCollector(executionContext.getGraphQLSchema(), fieldDef.getType(), normalizedFieldSupplier); QueryDirectives queryDirectives = new QueryDirectivesImpl(field, executionContext.getGraphQLSchema(), - executionContext.getCoercedVariables().toMap(), - executionContext.getNormalizedVariables().toMap(), + executionContext.getCoercedVariables(), + executionContext.getNormalizedVariables(), executionContext.getGraphQLContext(), executionContext.getLocale()); diff --git a/src/main/java/graphql/execution/directives/DirectivesResolver.java b/src/main/java/graphql/execution/directives/DirectivesResolver.java index 187f7ef5e6..4f177052e4 100644 --- a/src/main/java/graphql/execution/directives/DirectivesResolver.java +++ b/src/main/java/graphql/execution/directives/DirectivesResolver.java @@ -26,7 +26,7 @@ public class DirectivesResolver { public DirectivesResolver() { } - public BiMap resolveDirectives(List directives, GraphQLSchema schema, Map variables, GraphQLContext graphQLContext, Locale locale) { + public BiMap resolveDirectives(List directives, GraphQLSchema schema, CoercedVariables variables, GraphQLContext graphQLContext, Locale locale) { GraphQLCodeRegistry codeRegistry = schema.getCodeRegistry(); BiMap directiveMap = HashBiMap.create(); directives.forEach(directive -> { @@ -43,10 +43,10 @@ private void buildArguments(GraphQLDirective.Builder directiveBuilder, GraphQLCodeRegistry codeRegistry, GraphQLDirective protoType, Directive fieldDirective, - Map variables, + CoercedVariables variables, GraphQLContext graphQLContext, Locale locale) { - Map argumentValues = ValuesResolver.getArgumentValues(codeRegistry, protoType.getArguments(), fieldDirective.getArguments(), CoercedVariables.of(variables), graphQLContext, locale); + Map argumentValues = ValuesResolver.getArgumentValues(codeRegistry, protoType.getArguments(), fieldDirective.getArguments(), variables, graphQLContext, locale); directiveBuilder.clearArguments(); protoType.getArguments().forEach(protoArg -> { if (argumentValues.containsKey(protoArg.getName())) { diff --git a/src/main/java/graphql/execution/directives/QueryDirectives.java b/src/main/java/graphql/execution/directives/QueryDirectives.java index 7063f566c3..6eee9f9323 100644 --- a/src/main/java/graphql/execution/directives/QueryDirectives.java +++ b/src/main/java/graphql/execution/directives/QueryDirectives.java @@ -13,6 +13,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.function.Supplier; /** * This gives you access to the immediate directives on a {@link graphql.execution.MergedField}. This does not include directives on parent @@ -120,7 +121,7 @@ interface Builder { Builder coercedVariables(CoercedVariables coercedVariables); - Builder normalizedVariables(NormalizedVariables normalizedVariables); + Builder normalizedVariables(Supplier normalizedVariables); Builder graphQLContext(GraphQLContext graphQLContext); diff --git a/src/main/java/graphql/execution/directives/QueryDirectivesBuilder.java b/src/main/java/graphql/execution/directives/QueryDirectivesBuilder.java index 27c98c93dd..78b6998588 100644 --- a/src/main/java/graphql/execution/directives/QueryDirectivesBuilder.java +++ b/src/main/java/graphql/execution/directives/QueryDirectivesBuilder.java @@ -9,6 +9,7 @@ import graphql.schema.GraphQLSchema; import java.util.Locale; +import java.util.function.Supplier; @Internal public class QueryDirectivesBuilder implements QueryDirectives.Builder { @@ -16,7 +17,7 @@ public class QueryDirectivesBuilder implements QueryDirectives.Builder { private MergedField mergedField; private GraphQLSchema schema; private CoercedVariables coercedVariables = CoercedVariables.emptyVariables(); - private NormalizedVariables normalizedVariables = NormalizedVariables.emptyVariables(); + private Supplier normalizedVariables = NormalizedVariables::emptyVariables; private GraphQLContext graphQLContext = GraphQLContext.getDefault(); private Locale locale = Locale.getDefault(); @@ -45,7 +46,7 @@ public QueryDirectives.Builder coercedVariables(CoercedVariables coercedVariable } @Override - public QueryDirectives.Builder normalizedVariables(NormalizedVariables normalizedVariables) { + public QueryDirectives.Builder normalizedVariables(Supplier normalizedVariables) { this.normalizedVariables = normalizedVariables; return this; } @@ -65,6 +66,6 @@ public QueryDirectives.Builder locale(Locale locale) { @Override public QueryDirectives build() { - return new QueryDirectivesImpl(mergedField, schema, coercedVariables.toMap(), normalizedVariables.toMap(), graphQLContext, locale); + return new QueryDirectivesImpl(mergedField, schema, coercedVariables, normalizedVariables, graphQLContext, locale); } } diff --git a/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java b/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java index fe216a1341..5bb7bf6964 100644 --- a/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java +++ b/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java @@ -4,9 +4,12 @@ import com.google.common.collect.HashBiMap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import graphql.Assert; import graphql.GraphQLContext; import graphql.Internal; +import graphql.execution.CoercedVariables; import graphql.execution.MergedField; +import graphql.execution.NormalizedVariables; import graphql.execution.ValuesResolver; import graphql.language.Directive; import graphql.language.Field; @@ -14,7 +17,6 @@ import graphql.schema.GraphQLArgument; import graphql.schema.GraphQLDirective; import graphql.schema.GraphQLSchema; -import graphql.util.FpKit; import graphql.util.LockKit; import org.jetbrains.annotations.Nullable; @@ -23,7 +25,9 @@ import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.function.Supplier; +import static graphql.Assert.assertNotNull; import static graphql.collect.ImmutableKit.emptyList; /** @@ -37,9 +41,8 @@ public class QueryDirectivesImpl implements QueryDirectives { private final DirectivesResolver directivesResolver = new DirectivesResolver(); private final MergedField mergedField; private final GraphQLSchema schema; - private final Map coercedVariables; - @Nullable - private final Map normalizedVariableValues; + private final CoercedVariables coercedVariables; + private final Supplier normalizedVariableValues; private final GraphQLContext graphQLContext; private final Locale locale; @@ -50,13 +53,13 @@ public class QueryDirectivesImpl implements QueryDirectives { private volatile ImmutableMap> fieldAppliedDirectivesByName; private volatile ImmutableMap> normalizedValuesByAppliedDirective; - public QueryDirectivesImpl(MergedField mergedField, GraphQLSchema schema, Map coercedVariables, Map normalizedVariableValues, GraphQLContext graphQLContext, Locale locale) { - this.mergedField = mergedField; - this.schema = schema; - this.coercedVariables = coercedVariables; - this.normalizedVariableValues = normalizedVariableValues; - this.graphQLContext = graphQLContext; - this.locale = locale; + public QueryDirectivesImpl(MergedField mergedField, GraphQLSchema schema, CoercedVariables coercedVariables, Supplier normalizedVariableValues, GraphQLContext graphQLContext, Locale locale) { + this.mergedField = assertNotNull(mergedField); + this.schema = assertNotNull(schema); + this.coercedVariables = assertNotNull(coercedVariables); + this.normalizedVariableValues = assertNotNull(normalizedVariableValues); + this.graphQLContext = assertNotNull(graphQLContext); + this.locale = assertNotNull(locale); } private void computeValuesLazily() { @@ -100,14 +103,16 @@ private void computeValuesLazily() { // create NormalizedInputValue values for directive arguments Map> normalizedValuesByAppliedDirective = new LinkedHashMap<>(); - if (this.normalizedVariableValues != null) { + NormalizedVariables normalizedVariableValues = this.normalizedVariableValues.get(); + if (normalizedVariableValues != null) { byNameApplied.values().forEach(directiveList -> { for (QueryAppliedDirective queryAppliedDirective : directiveList) { GraphQLDirective graphQLDirective = gqlDirectiveCounterPartsInverse.get(queryAppliedDirective); // we need this counterpart because the ValuesResolver needs the runtime and AST element Directive directive = directiveCounterParts.get(graphQLDirective); if (directive != null) { - Map normalizedArgumentValues = ValuesResolver.getNormalizedArgumentValues(graphQLDirective.getArguments(), directive.getArguments(), this.normalizedVariableValues); + Map normalizedVariables = normalizedVariableValues.toMap(); + Map normalizedArgumentValues = ValuesResolver.getNormalizedArgumentValues(graphQLDirective.getArguments(), directive.getArguments(), normalizedVariables); normalizedValuesByAppliedDirective.put(queryAppliedDirective, normalizedArgumentValues); } } diff --git a/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java b/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java index 6292b46688..27d43122ed 100644 --- a/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java +++ b/src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java @@ -518,11 +518,10 @@ private ExecutableNormalizedOperation createNormalizedQueryImpl() { private void captureMergedField(ExecutableNormalizedField enf, MergedField mergedFld) { // QueryDirectivesImpl is a lazy object and only computes itself when asked for - Map normalizedVariableValues = Optional.ofNullable(this.normalizedVariableValues).map(NormalizedVariables::toMap).orElse(null); QueryDirectives queryDirectives = new QueryDirectivesImpl(mergedFld, graphQLSchema, - coercedVariableValues.toMap(), - normalizedVariableValues, + coercedVariableValues, + () -> normalizedVariableValues, options.getGraphQLContext(), options.getLocale()); normalizedFieldToQueryDirectives.put(enf, queryDirectives); diff --git a/src/test/groovy/graphql/execution/directives/QueryDirectivesImplTest.groovy b/src/test/groovy/graphql/execution/directives/QueryDirectivesImplTest.groovy index 1ddf0f62d3..80b1bbbd5c 100644 --- a/src/test/groovy/graphql/execution/directives/QueryDirectivesImplTest.groovy +++ b/src/test/groovy/graphql/execution/directives/QueryDirectivesImplTest.groovy @@ -35,7 +35,10 @@ class QueryDirectivesImplTest extends Specification { def mergedField = MergedField.newMergedField([f1, f2]).build() - def impl = new QueryDirectivesImpl(mergedField, schema, [var: 10], [var : new NormalizedInputValue("type", IntValue.of(10))], GraphQLContext.getDefault(), Locale.getDefault()) + def impl = new QueryDirectivesImpl(mergedField, schema, + CoercedVariables.of([var: 10]), + { -> NormalizedVariables.of([var: new NormalizedInputValue("type", IntValue.of(10))]) }, + GraphQLContext.getDefault(), Locale.getDefault()) when: def directives = impl.getImmediateDirectivesByName() @@ -80,7 +83,7 @@ class QueryDirectivesImplTest extends Specification { .mergedField(mergedField) .schema(schema) .coercedVariables(CoercedVariables.of([var: 10])) - .normalizedVariables(NormalizedVariables.of([var: new NormalizedInputValue("type", IntValue.of(10))])) + .normalizedVariables({ NormalizedVariables.of([var: new NormalizedInputValue("type", IntValue.of(10))]) }) .graphQLContext(GraphQLContext.getDefault()) .locale(Locale.getDefault()) .build() From 4760249617788740710a4851791e22178191fa9c Mon Sep 17 00:00:00 2001 From: bbaker Date: Mon, 9 Dec 2024 10:44:08 +1100 Subject: [PATCH 09/13] Added more tests --- .../QueryDirectivesIntegrationTest.groovy | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/src/test/groovy/graphql/execution/directives/QueryDirectivesIntegrationTest.groovy b/src/test/groovy/graphql/execution/directives/QueryDirectivesIntegrationTest.groovy index 2b99d6e3be..8c907862a7 100644 --- a/src/test/groovy/graphql/execution/directives/QueryDirectivesIntegrationTest.groovy +++ b/src/test/groovy/graphql/execution/directives/QueryDirectivesIntegrationTest.groovy @@ -94,7 +94,7 @@ class QueryDirectivesIntegrationTest extends Specification { capturedDirectives = [:] } - def "can collector directives as expected"() { + def "can collect directives as expected"() { when: def er = execute(pathologicalQuery) then: @@ -112,6 +112,36 @@ class QueryDirectivesIntegrationTest extends Specification { joinArgs(immediate) == "cached(forMillis:5),cached(forMillis:10)" } + def "can collect merged field directives as expected"() { + when: + def query = """ + query Books { + books(searchString: "monkey") { + review @timeout(afterMillis: 10) @cached(forMillis : 10) + review @timeout(afterMillis: 100) @cached(forMillis : 100) + } + } + + """ + def er = execute(query) + then: + er.errors.isEmpty() + + def immediateMap = capturedDirectives["review"].getImmediateAppliedDirectivesByName() + def entries = immediateMap.entrySet().collectEntries({ + [(it.getKey()): joinArgs(it.getValue())] + }) + entries == [cached : "cached(forMillis:10),cached(forMillis:100)", + timeout: "timeout(afterMillis:10),timeout(afterMillis:100)" + ] + + def immediate = capturedDirectives["review"].getImmediateAppliedDirective("cached") + joinArgs(immediate) == "cached(forMillis:10),cached(forMillis:100)" + + def immediate2 = capturedDirectives["review"].getImmediateAppliedDirective("timeout") + joinArgs(immediate2) == "timeout(afterMillis:10),timeout(afterMillis:100)" + } + def "wont create directives for peer fields accidentally"() { def query = '''query Books { books(searchString: "monkey") { From 883ba63969ce5995a4b85799d1b9d6635a7c8637 Mon Sep 17 00:00:00 2001 From: bbaker Date: Mon, 9 Dec 2024 13:20:28 +1100 Subject: [PATCH 10/13] Added compiler tst for merged fields --- ...erationToAstCompilerDirectivesTest.groovy} | 71 +++++++++++++------ 1 file changed, 48 insertions(+), 23 deletions(-) rename src/test/groovy/graphql/normalized/{ENOToAstCompilerDirectivesTest.groovy => ExecutableNormalizedOperationToAstCompilerDirectivesTest.groovy} (82%) diff --git a/src/test/groovy/graphql/normalized/ENOToAstCompilerDirectivesTest.groovy b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerDirectivesTest.groovy similarity index 82% rename from src/test/groovy/graphql/normalized/ENOToAstCompilerDirectivesTest.groovy rename to src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerDirectivesTest.groovy index 9d55d3dccc..b7928b4aed 100644 --- a/src/test/groovy/graphql/normalized/ENOToAstCompilerDirectivesTest.groovy +++ b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerDirectivesTest.groovy @@ -7,7 +7,24 @@ import static graphql.language.OperationDefinition.Operation.QUERY /** * Test related to ENO and directives */ -class ENOToAstCompilerDirectivesTest extends ENOToAstCompilerTestBase { +class ExecutableNormalizedOperationToAstCompilerDirectivesTest extends ENOToAstCompilerTestBase { + def bookSDL = ''' + directive @timeout(afterMillis : Int) on FIELD | FRAGMENT_DEFINITION | FRAGMENT_SPREAD | INLINE_FRAGMENT | QUERY + + directive @cached(forMillis : Int) on FIELD | FRAGMENT_DEFINITION | FRAGMENT_SPREAD | INLINE_FRAGMENT | QUERY + + directive @importance(place : String) on FIELD | FRAGMENT_DEFINITION | FRAGMENT_SPREAD | INLINE_FRAGMENT | QUERY + + type Query { + books(searchString : String) : [Book] + } + + type Book { + id : ID + title : String + review : String + } + ''' def "can extract variables or inline values for directives on the query"() { def sdl = ''' @@ -77,23 +94,6 @@ class ENOToAstCompilerDirectivesTest extends ENOToAstCompilerTestBase { } def "can handle quite a pathological fragment query as expected"() { - def sdl = ''' - directive @timeout(afterMillis : Int) on FIELD | FRAGMENT_DEFINITION | FRAGMENT_SPREAD | INLINE_FRAGMENT | QUERY - - directive @cached(forMillis : Int) on FIELD | FRAGMENT_DEFINITION | FRAGMENT_SPREAD | INLINE_FRAGMENT | QUERY - - directive @importance(place : String) on FIELD | FRAGMENT_DEFINITION | FRAGMENT_SPREAD | INLINE_FRAGMENT | QUERY - - type Query { - books(searchString : String) : [Book] - } - - type Book { - id : ID - title : String - review : String - } - ''' def pathologicalQuery = ''' fragment Details on Book @timeout(afterMillis: 25) @cached(forMillis : 25) @importance(place:"FragDef") { @@ -117,7 +117,7 @@ class ENOToAstCompilerDirectivesTest extends ENOToAstCompilerTestBase { } ''' - GraphQLSchema schema = mkSchema(sdl) + GraphQLSchema schema = mkSchema(bookSDL) def eno = createNormalizedTree(schema, pathologicalQuery, [:]) when: @@ -130,10 +130,6 @@ class ENOToAstCompilerDirectivesTest extends ENOToAstCompilerTestBase { then: vars == [v0:5, v1:5, v2:28, v3:10, v4:10, v5:"monkey"] - // - // the below is what ir currently produces but its WRONG - // fix up when the other tests starts to work - // ast == '''query Books($v0: Int, $v1: Int, $v2: Int, $v3: Int, $v4: Int, $v5: String) { books(searchString: $v5) { id @@ -166,4 +162,33 @@ class ENOToAstCompilerDirectivesTest extends ENOToAstCompilerTestBase { ''' } + + def "merged fields with the same field will capture all directives"() { + def query = ''' + query Books { + books(searchString: "monkey") { + review @timeout(afterMillis: 10) @cached(forMillis : 10) + review @timeout(afterMillis: 100) @cached(forMillis : 100) + } + } + ''' + + GraphQLSchema schema = mkSchema(bookSDL) + def eno = createNormalizedTree(schema, query, [:]) + + when: + def result = localCompileToDocument(schema, QUERY, "Books", + eno.getTopLevelFields(), eno.getNormalizedFieldToQueryDirectives(), + noVariables) + def document = result.document + def ast = printDoc(document) + + then: + ast == '''query Books { + books(searchString: "monkey") { + review @cached(forMillis: 10) @cached(forMillis: 100) @timeout(afterMillis: 10) @timeout(afterMillis: 100) + } +} +''' + } } From e09e6b698fd2980027fa4fe50f4de991bff84146 Mon Sep 17 00:00:00 2001 From: bbaker Date: Mon, 9 Dec 2024 21:37:00 +1100 Subject: [PATCH 11/13] Extra comment --- src/main/java/graphql/normalized/VariableAccumulator.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/graphql/normalized/VariableAccumulator.java b/src/main/java/graphql/normalized/VariableAccumulator.java index d2a7fe2a37..c555016476 100644 --- a/src/main/java/graphql/normalized/VariableAccumulator.java +++ b/src/main/java/graphql/normalized/VariableAccumulator.java @@ -30,6 +30,8 @@ public VariableAccumulator(@Nullable VariablePredicate variablePredicate) { } public boolean shouldMakeVariable(ExecutableNormalizedField executableNormalizedField, QueryAppliedDirective queryAppliedDirective, String argName, NormalizedInputValue normalizedInputValue) { + // when a variable is used on the argument to a query directive then the queryAppliedDirective will be nonnull. + // otherwise it must be a field argument if (queryAppliedDirective != null) { return variablePredicate != null && variablePredicate.shouldMakeVariable(executableNormalizedField, queryAppliedDirective, argName, normalizedInputValue); } else { From bfe691ab3116c8111f5be38097243b08baf3b946 Mon Sep 17 00:00:00 2001 From: bbaker Date: Thu, 27 Feb 2025 11:54:24 +1100 Subject: [PATCH 12/13] Merged in master and tweaked Execution.java --- .../java/graphql/execution/Execution.java | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 1444be2af3..dcdcf59407 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -70,24 +70,13 @@ public Execution(ExecutionStrategy queryStrategy, } public CompletableFuture execute(Document document, GraphQLSchema graphQLSchema, ExecutionId executionId, ExecutionInput executionInput, InstrumentationState instrumentationState) { - - NodeUtil.GetOperationResult getOperationResult; CoercedVariables coercedVariables; Supplier normalizedVariableValues; try { getOperationResult = NodeUtil.getOperation(document, executionInput.getOperationName()); coercedVariables = coerceVariableValues(graphQLSchema, executionInput, getOperationResult.operationDefinition); - - RawVariables inputVariables = executionInput.getRawVariables(); - List variableDefinitions = getOperationResult.operationDefinition.getVariableDefinitions(); - - normalizedVariableValues = FpKit.intraThreadMemoize(() -> - ValuesResolver.getNormalizedVariableValues(graphQLSchema, - variableDefinitions, - inputVariables, - executionInput.getGraphQLContext(), executionInput.getLocale()); - + normalizedVariableValues = normalizedVariableValues(graphQLSchema, executionInput, getOperationResult); } catch (RuntimeException rte) { if (rte instanceof GraphQLError) { return completedFuture(new ExecutionResultImpl((GraphQLError) rte)); @@ -133,6 +122,19 @@ public CompletableFuture execute(Document document, GraphQLSche return ValuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, inputVariables, executionInput.getGraphQLContext(), executionInput.getLocale()); } + private static @NotNull Supplier normalizedVariableValues(GraphQLSchema graphQLSchema, ExecutionInput executionInput, NodeUtil.GetOperationResult getOperationResult) { + Supplier normalizedVariableValues; + RawVariables inputVariables = executionInput.getRawVariables(); + List variableDefinitions = getOperationResult.operationDefinition.getVariableDefinitions(); + + normalizedVariableValues = FpKit.intraThreadMemoize(() -> + ValuesResolver.getNormalizedVariableValues(graphQLSchema, + variableDefinitions, + inputVariables, + executionInput.getGraphQLContext(), executionInput.getLocale())); + return normalizedVariableValues; + } + private CompletableFuture executeOperation(ExecutionContext executionContext, Object root, OperationDefinition operationDefinition) { @@ -244,7 +246,7 @@ private DataLoaderDispatchStrategy createDataLoaderDispatchStrategy(ExecutionCon if (executionContext.getDataLoaderRegistry() == EMPTY_DATALOADER_REGISTRY || doNotAutomaticallyDispatchDataLoader) { return DataLoaderDispatchStrategy.NO_OP; } - if (! executionContext.isSubscriptionOperation()) { + if (!executionContext.isSubscriptionOperation()) { boolean deferEnabled = Optional.ofNullable(executionContext.getGraphQLContext()) .map(graphqlContext -> graphqlContext.getBoolean(ExperimentalApi.ENABLE_INCREMENTAL_SUPPORT)) .orElse(false); From 053aac4a70e6b4aa000b6f08b58aebd8f006d52f Mon Sep 17 00:00:00 2001 From: bbaker Date: Thu, 20 Mar 2025 13:57:05 +1100 Subject: [PATCH 13/13] Merged in master --- .../java/graphql/execution/Execution.java | 3 +- .../directives/QueryDirectivesImpl.java | 1 - .../graphql/normalized/ArgumentMaker.java | 6 +- ...tableNormalizedOperationToAstCompiler.java | 57 +------------------ 4 files changed, 6 insertions(+), 61 deletions(-) diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 56cabbf29f..634837f987 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -31,7 +31,6 @@ import graphql.schema.impl.SchemaUtil; import org.jspecify.annotations.NonNull; import graphql.util.FpKit; -import org.jetbrains.annotations.NotNull; import org.reactivestreams.Publisher; import java.util.Collections; @@ -129,7 +128,7 @@ public CompletableFuture execute(Document document, GraphQLSche return ValuesResolver.coerceVariableValues(graphQLSchema, variableDefinitions, inputVariables, executionInput.getGraphQLContext(), executionInput.getLocale()); } - private static @NotNull Supplier normalizedVariableValues(GraphQLSchema graphQLSchema, ExecutionInput executionInput, NodeUtil.GetOperationResult getOperationResult) { + private static @NonNull Supplier normalizedVariableValues(GraphQLSchema graphQLSchema, ExecutionInput executionInput, NodeUtil.GetOperationResult getOperationResult) { Supplier normalizedVariableValues; RawVariables inputVariables = executionInput.getRawVariables(); List variableDefinitions = getOperationResult.operationDefinition.getVariableDefinitions(); diff --git a/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java b/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java index 5bb7bf6964..4b3fde5f4a 100644 --- a/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java +++ b/src/main/java/graphql/execution/directives/QueryDirectivesImpl.java @@ -18,7 +18,6 @@ import graphql.schema.GraphQLDirective; import graphql.schema.GraphQLSchema; import graphql.util.LockKit; -import org.jetbrains.annotations.Nullable; import java.util.ArrayList; import java.util.LinkedHashMap; diff --git a/src/main/java/graphql/normalized/ArgumentMaker.java b/src/main/java/graphql/normalized/ArgumentMaker.java index 29d6a0ae8b..c6ad4868ba 100644 --- a/src/main/java/graphql/normalized/ArgumentMaker.java +++ b/src/main/java/graphql/normalized/ArgumentMaker.java @@ -12,8 +12,8 @@ import graphql.language.ObjectField; import graphql.language.ObjectValue; import graphql.language.Value; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; +import org.jspecify.annotations.NonNull; +import org.jspecify.annotations.Nullable; import java.util.List; import java.util.Map; @@ -94,7 +94,7 @@ private static Value argValue(ExecutableNormalizedField executableNormalizedF return (Value) value; } - @NotNull + @NonNull private static Value argValue(ExecutableNormalizedField executableNormalizedField, QueryAppliedDirective queryAppliedDirective, String argName, diff --git a/src/main/java/graphql/normalized/ExecutableNormalizedOperationToAstCompiler.java b/src/main/java/graphql/normalized/ExecutableNormalizedOperationToAstCompiler.java index 1e53f24302..9509d9554b 100644 --- a/src/main/java/graphql/normalized/ExecutableNormalizedOperationToAstCompiler.java +++ b/src/main/java/graphql/normalized/ExecutableNormalizedOperationToAstCompiler.java @@ -389,7 +389,7 @@ private static Field selectionForNormalizedField(GraphQLSchema schema, .build(); } - private static @NotNull List buildDirectives(ExecutableNormalizedField executableNormalizedField, QueryDirectives queryDirectives, VariableAccumulator variableAccumulator) { + private static @NonNull List buildDirectives(ExecutableNormalizedField executableNormalizedField, QueryDirectives queryDirectives, VariableAccumulator variableAccumulator) { if (queryDirectives == null || queryDirectives.getImmediateAppliedDirectivesByField().isEmpty()) { return emptyList(); } @@ -401,7 +401,7 @@ private static Field selectionForNormalizedField(GraphQLSchema schema, private static Directive buildDirective(ExecutableNormalizedField executableNormalizedField, QueryDirectives queryDirectives, QueryAppliedDirective queryAppliedDirective, VariableAccumulator variableAccumulator) { - List arguments = ArgumentMaker.createDirectiveArguments(executableNormalizedField,queryDirectives,queryAppliedDirective, variableAccumulator); + List arguments = ArgumentMaker.createDirectiveArguments(executableNormalizedField, queryDirectives, queryAppliedDirective, variableAccumulator); return Directive.newDirective() .name(queryAppliedDirective.getName()) .arguments(arguments).build(); @@ -416,59 +416,6 @@ private static SelectionSet selectionSet(List fields) { return newSelectionSet().selections(fields).build(); } - private static List createArguments(ExecutableNormalizedField executableNormalizedField, - VariableAccumulator variableAccumulator) { - ImmutableList.Builder result = ImmutableList.builder(); - ImmutableMap normalizedArguments = executableNormalizedField.getNormalizedArguments(); - for (String argName : normalizedArguments.keySet()) { - NormalizedInputValue normalizedInputValue = normalizedArguments.get(argName); - Value value = argValue(executableNormalizedField, argName, normalizedInputValue, variableAccumulator); - Argument argument = newArgument() - .name(argName) - .value(value) - .build(); - result.add(argument); - } - return result.build(); - } - - @SuppressWarnings("unchecked") - private static Value argValue(ExecutableNormalizedField executableNormalizedField, - String argName, - @Nullable Object value, - VariableAccumulator variableAccumulator) { - if (value instanceof List) { - ArrayValue.Builder arrayValue = ArrayValue.newArrayValue(); - arrayValue.values(map((List) value, val -> argValue(executableNormalizedField, argName, val, variableAccumulator))); - return arrayValue.build(); - } - if (value instanceof Map) { - ObjectValue.Builder objectValue = ObjectValue.newObjectValue(); - Map map = (Map) value; - for (String fieldName : map.keySet()) { - Value fieldValue = argValue(executableNormalizedField, argName, (NormalizedInputValue) map.get(fieldName), variableAccumulator); - objectValue.objectField(ObjectField.newObjectField().name(fieldName).value(fieldValue).build()); - } - return objectValue.build(); - } - if (value == null) { - return NullValue.newNullValue().build(); - } - return (Value) value; - } - - @NonNull - private static Value argValue(ExecutableNormalizedField executableNormalizedField, - String argName, - NormalizedInputValue normalizedInputValue, - VariableAccumulator variableAccumulator) { - if (variableAccumulator.shouldMakeVariable(executableNormalizedField, argName, normalizedInputValue)) { - VariableValueWithDefinition variableWithDefinition = variableAccumulator.accumulateVariable(normalizedInputValue); - return variableWithDefinition.getVariableReference(); - } else { - return argValue(executableNormalizedField, argName, normalizedInputValue.getValue(), variableAccumulator); - } - } @NonNull private static GraphQLFieldDefinition getFieldDefinition(GraphQLSchema schema,