diff --git a/CHANGELOG.md b/CHANGELOG.md index e422cbbfb..35c78ea5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Unreleased +## Multi-query variable references + +`GraphQLMultiQueryRequest` now preserves variable references and declarations, including projection variables, fixing #860 and #669. +Shared variables with compatible types use the strictest nullability. +Single requests apply the same variable merging in regular and compact output. +Reusing a variable name with incompatible types or conflicting defaults now produces a server validation error instead of silently dropping root variable values. +Multi-query requests now preserve each request's operation field directives and explicit selection set. + ## Operation roots (breaking change) Operation discovery now follows explicit `schema` declarations and `extend schema` declarations. diff --git a/graphql-dgs-codegen-core/src/integTest/java/com/netflix/graphql/dgs/codegen/java/testcases/projections/projectionWithTypeAndArgs/test/ProjectionWithTypeAndArgsQueryTest.java b/graphql-dgs-codegen-core/src/integTest/java/com/netflix/graphql/dgs/codegen/java/testcases/projections/projectionWithTypeAndArgs/test/ProjectionWithTypeAndArgsQueryTest.java index c52d1a280..36bfdc961 100644 --- a/graphql-dgs-codegen-core/src/integTest/java/com/netflix/graphql/dgs/codegen/java/testcases/projections/projectionWithTypeAndArgs/test/ProjectionWithTypeAndArgsQueryTest.java +++ b/graphql-dgs-codegen-core/src/integTest/java/com/netflix/graphql/dgs/codegen/java/testcases/projections/projectionWithTypeAndArgs/test/ProjectionWithTypeAndArgsQueryTest.java @@ -22,9 +22,17 @@ import com.netflix.graphql.dgs.client.codegen.GraphQLQueryRequest; import com.netflix.graphql.dgs.codegen.java.testcases.projections.projectionWithTypeAndArgs.expected.client.PersonGraphQLQuery; import com.netflix.graphql.dgs.codegen.java.testcases.projections.projectionWithTypeAndArgs.expected.client.PersonProjectionRoot; +import graphql.parser.Parser; +import graphql.schema.idl.RuntimeWiring; +import graphql.schema.idl.SchemaGenerator; +import graphql.schema.idl.SchemaParser; +import graphql.validation.Validator; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import java.util.List; +import java.util.Locale; import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; @@ -75,6 +83,95 @@ void testQueryWithUnnamedArgs() { }"""); } + @Test + void testMultipleQueriesWithVariableReferences() { + var query1 = PersonGraphQLQuery.newRequest() + .queryName("Combined") + .a1("literal") + .a2Reference("firstName") + .build(); + query1.setQueryAlias("first"); + + var query2 = PersonGraphQLQuery.newRequest() + .a2Reference("secondName") + .build(); + query2.setQueryAlias("second"); + + var projection = new PersonProjectionRoot<>().firstname(); + var request = new GraphQLMultiQueryRequest(List.of( + new GraphQLQueryRequest(query1, projection), + new GraphQLQueryRequest(query2, projection) + )); + + assertThat(request.serialize()).isEqualTo(""" + query Combined($firstName: String!, $secondName: String!) { + first: person(a1: "literal", a2: $firstName) { + firstname + } + second: person(a2: $secondName) { + firstname + } + }"""); + } + + @Test + void testMultipleQueriesWithSharedVariableReferencesValidate() { + var query1 = PersonGraphQLQuery.newRequest() + .a1Reference("shared") + .a2("literal") + .build(); + query1.setQueryAlias("first"); + + var query2 = PersonGraphQLQuery.newRequest() + .a1Reference("shared") + .a2("literal") + .build(); + query2.setQueryAlias("second"); + + var projection = new PersonProjectionRoot<>().firstname(); + var request = new GraphQLMultiQueryRequest(List.of( + new GraphQLQueryRequest(query1, projection), + new GraphQLQueryRequest(query2, projection) + )); + var schema = new SchemaGenerator().makeExecutableSchema( + new SchemaParser().parse(""" + type Query { person(a1: String, a2: String!): Person } + type Person { firstname: String } + """), + RuntimeWiring.newRuntimeWiring().build() + ); + + assertThat(new Validator().validateDocument(schema, new Parser().parseDocument(request.serialize()), Locale.ROOT)) + .isEmpty(); + } + + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testSharedVariableWithNullableAndNonNullArguments(boolean reverseOrder) { + var nullable = PersonGraphQLQuery.newRequest().a1Reference("shared").a2("literal").build(); + nullable.setQueryAlias("nullable"); + var nonNull = PersonGraphQLQuery.newRequest().a2Reference("shared").build(); + nonNull.setQueryAlias("nonNull"); + var projection = new PersonProjectionRoot<>().firstname(); + var nullableRequest = new GraphQLQueryRequest(nullable, projection); + var nonNullRequest = new GraphQLQueryRequest(nonNull, projection); + var requests = reverseOrder + ? List.of(nonNullRequest, nullableRequest) + : List.of(nullableRequest, nonNullRequest); + var serialized = new GraphQLMultiQueryRequest(requests).serialize(); + var schema = new SchemaGenerator().makeExecutableSchema( + new SchemaParser().parse(""" + type Query { person(a1: String, a2: String!): Person } + type Person { firstname: String } + """), + RuntimeWiring.newRuntimeWiring().build() + ); + + assertThat(new Validator().validateDocument(schema, new Parser().parseDocument(serialized), Locale.ROOT)) + .isEmpty(); + assertThat(serialized).contains("query ($shared: String!)"); + } + @Test void testQueryWithAlias() { var query1 = PersonGraphQLQuery.newRequest() diff --git a/graphql-dgs-codegen-shared-core/src/main/kotlin/com/netflix/graphql/dgs/client/codegen/GraphQLMultiQueryRequest.kt b/graphql-dgs-codegen-shared-core/src/main/kotlin/com/netflix/graphql/dgs/client/codegen/GraphQLMultiQueryRequest.kt index e2262e71d..e251dd488 100644 --- a/graphql-dgs-codegen-shared-core/src/main/kotlin/com/netflix/graphql/dgs/client/codegen/GraphQLMultiQueryRequest.kt +++ b/graphql-dgs-codegen-shared-core/src/main/kotlin/com/netflix/graphql/dgs/client/codegen/GraphQLMultiQueryRequest.kt @@ -32,48 +32,23 @@ class GraphQLMultiQueryRequest( val queryType = requests[0].query.getOperationType().toString() val variableDefinitions = mutableListOf() - val selectionList: MutableList = mutableListOf() + val selectionList = mutableListOf() for (request in this.requests) { val query = request.query + variableDefinitions.addAll(query.variableDefinitions) // Graphql only supports multiple mutations or multiple queries. Not a combination of the two. // Graphql does not support multiple subscriptions in one request http://spec.graphql.org/June2018/#sec-Single-root-field if (!query.getOperationType().equals(queryType) || queryType == OperationDefinition.Operation.SUBSCRIPTION.name) { throw AssertionError("Request has to have exclusively queries or mutations in a multi operation request") } - if (request.query.variableDefinitions.isNotEmpty()) { - variableDefinitions.addAll(request.query.variableDefinitions) - } - - val selection = Field.newField(request.query.getOperationName()) - if (query.input.isNotEmpty()) { - selection.arguments( - query.input.map { (name, value) -> - Argument(name, request.inputValueSerializer.toValue(value)) - }, - ) - } - - if (request.projection != null) { - val selectionSet = - if (request.projection is BaseSubProjectionNode<*, *> && request.projection.root() != null) { - request.projectionSerializer.toSelectionSet(request.projection.root() as BaseProjectionNode) - } else { - request.projectionSerializer.toSelectionSet(request.projection) - } - if (selectionSet.selections.isNotEmpty()) { - selection.selectionSet(selectionSet) - } - } - if (query.queryAlias.isNotEmpty()) { - selection.alias(query.queryAlias) - } - - selectionList.add(selection) + val selection = request.toField(variableDefinitions) + selectionList.add(if (query.queryAlias.isNotEmpty()) selection.transform { it.alias(query.queryAlias) } else selection) } - operationDef.selectionSet(SelectionSet.newSelectionSet(selectionList.map(Field.Builder::build).toList()).build()) + operationDef.variableDefinitions(mergeVariableDefinitions(variableDefinitions)) + operationDef.selectionSet(SelectionSet.newSelectionSet(selectionList).build()) return AstPrinter.printAst(operationDef.build()) } diff --git a/graphql-dgs-codegen-shared-core/src/main/kotlin/com/netflix/graphql/dgs/client/codegen/GraphQLQueryRequest.kt b/graphql-dgs-codegen-shared-core/src/main/kotlin/com/netflix/graphql/dgs/client/codegen/GraphQLQueryRequest.kt index 438e46967..6cad71ba7 100644 --- a/graphql-dgs-codegen-shared-core/src/main/kotlin/com/netflix/graphql/dgs/client/codegen/GraphQLQueryRequest.kt +++ b/graphql-dgs-codegen-shared-core/src/main/kotlin/com/netflix/graphql/dgs/client/codegen/GraphQLQueryRequest.kt @@ -21,8 +21,13 @@ import graphql.language.Argument import graphql.language.AstPrinter import graphql.language.Directive import graphql.language.Field +import graphql.language.ListType +import graphql.language.NonNullType import graphql.language.OperationDefinition import graphql.language.SelectionSet +import graphql.language.Type +import graphql.language.TypeName +import graphql.language.VariableDefinition import graphql.language.VariableReference import graphql.schema.Coercing @@ -89,12 +94,28 @@ class GraphQLQueryRequest private fun serialize(compact: Boolean): String { val operationDef = OperationDefinition.newOperationDefinition() + val variableDefinitions = query.variableDefinitions.toMutableList() query.name?.let { operationDef.name(it) } query .getOperationType() ?.let { operationDef.operation(OperationDefinition.Operation.valueOf(it.uppercase())) } + val selection = toField(variableDefinitions) + if (variableDefinitions.isNotEmpty()) { + operationDef.variableDefinitions(mergeVariableDefinitions(variableDefinitions)) + } + + operationDef.selectionSet(SelectionSet.newSelectionSet().selection(selection).build()) + + return if (compact) { + AstPrinter.printAstCompact(operationDef.build()) + } else { + AstPrinter.printAst(operationDef.build()) + } + } + + internal fun toField(variableDefinitions: MutableList): Field { val selection = Field.newField(query.getOperationName()) if (query.input.isNotEmpty()) { selection.arguments( @@ -113,29 +134,62 @@ class GraphQLQueryRequest if (projection != null) { val selectionSetFromProjection = if (projection is BaseSubProjectionNode<*, *> && projection.root() != null) { - projectionSerializer.toSelectionSet(projection.root() as BaseProjectionNode) + projectionSerializer.toSelectionSet(projection.root() as BaseProjectionNode, variableDefinitions) } else { - projectionSerializer.toSelectionSet(projection) + projectionSerializer.toSelectionSet(projection, variableDefinitions) } if (selectionSetFromProjection.selections.isNotEmpty()) { selection.selectionSet(selectionSetFromProjection) } } - if (query.variableDefinitions.isNotEmpty()) { - operationDef.variableDefinitions(query.variableDefinitions) - } - if (selectionSet != null) { selection.selectionSet(selectionSet) } - operationDef.selectionSet(SelectionSet.newSelectionSet().selection(selection.build()).build()) + return selection.build() + } + } + +@JvmSynthetic +internal fun mergeVariableDefinitions(definitions: List): List = + definitions + .groupBy { definition -> + Triple( + definition.name, + definition.defaultValue?.let { AstPrinter.printAstCompact(it) }, + definition.directives.map { AstPrinter.printAstCompact(it) }, + ) + }.values + .flatMap { group -> + var type: Type<*> = group.first().type + for (definition in group.drop(1)) { + // Conflicting types or list shapes must remain visible to server validation. + type = mergeVariableTypes(type, definition.type) ?: return@flatMap group.distinctBy { AstPrinter.printAstCompact(it) } + } + listOf(group.first().transform { it.type(type) }) + } + +private fun mergeVariableTypes( + first: Type<*>, + second: Type<*>, +): Type<*>? { + val firstType = if (first is NonNullType) first.type else first + val secondType = if (second is NonNullType) second.type else second + val merged = + when { + firstType is TypeName && secondType is TypeName && firstType.name == secondType.name -> { + firstType + } - return if (compact) { - AstPrinter.printAstCompact(operationDef.build()) - } else { - AstPrinter.printAst(operationDef.build()) + firstType is ListType && secondType is ListType -> { + ListType(mergeVariableTypes(firstType.type, secondType.type) ?: return null) + } + + else -> { + return null } } - } + // A non-null variable can also satisfy a nullable argument, including within lists. + return if (first is NonNullType || second is NonNullType) NonNullType(merged) else merged +} diff --git a/graphql-dgs-codegen-shared-core/src/main/kotlin/com/netflix/graphql/dgs/client/codegen/ProjectionSerializer.kt b/graphql-dgs-codegen-shared-core/src/main/kotlin/com/netflix/graphql/dgs/client/codegen/ProjectionSerializer.kt index 932a2fe6b..7d15d825e 100644 --- a/graphql-dgs-codegen-shared-core/src/main/kotlin/com/netflix/graphql/dgs/client/codegen/ProjectionSerializer.kt +++ b/graphql-dgs-codegen-shared-core/src/main/kotlin/com/netflix/graphql/dgs/client/codegen/ProjectionSerializer.kt @@ -29,7 +29,12 @@ class ProjectionSerializer( private val inputValueSerializer: InputValueSerializerInterface, private val query: GraphQLQuery, ) { - fun toSelectionSet(projection: BaseProjectionNode): SelectionSet { + fun toSelectionSet(projection: BaseProjectionNode): SelectionSet = toSelectionSet(projection, query.variableDefinitions) + + internal fun toSelectionSet( + projection: BaseProjectionNode, + variableDefinitions: MutableList, + ): SelectionSet { val selectionSet = SelectionSet.newSelectionSet() for ((fieldName, value) in projection.fields) { @@ -40,7 +45,7 @@ class ProjectionSerializer( .arguments( projection.inputArguments[fieldName].orEmpty().map { (argName, values, isReference, type) -> if (isReference) { - query.variableDefinitions.add(VariableDefinition(values as String, type)) + variableDefinitions.add(VariableDefinition(values as String, type)) Argument(argName, VariableReference(values)) } else { Argument(argName, inputValueSerializer.toValue(values)) @@ -48,7 +53,7 @@ class ProjectionSerializer( }, ) if (value is BaseProjectionNode) { - val fieldSelectionSet = toSelectionSet(value) + val fieldSelectionSet = toSelectionSet(value, variableDefinitions) if (fieldSelectionSet.selections.isNotEmpty()) { fieldSelection.selectionSet(fieldSelectionSet) } @@ -82,7 +87,7 @@ class ProjectionSerializer( InlineFragment .newInlineFragment() .typeCondition(typeCondition) - .selectionSet(toSelectionSet(fragment)) + .selectionSet(toSelectionSet(fragment, variableDefinitions)) .build(), ) } diff --git a/graphql-dgs-codegen-shared-core/src/test/kotlin/com/netflix/graphql/dgs/client/codegen/GraphQLMultiQueryRequestTest.kt b/graphql-dgs-codegen-shared-core/src/test/kotlin/com/netflix/graphql/dgs/client/codegen/GraphQLMultiQueryRequestTest.kt index 182de5e99..aae92a08d 100644 --- a/graphql-dgs-codegen-shared-core/src/test/kotlin/com/netflix/graphql/dgs/client/codegen/GraphQLMultiQueryRequestTest.kt +++ b/graphql-dgs-codegen-shared-core/src/test/kotlin/com/netflix/graphql/dgs/client/codegen/GraphQLMultiQueryRequestTest.kt @@ -18,11 +18,361 @@ package com.netflix.graphql.dgs.client.codegen +import com.netflix.graphql.dgs.client.codegen.GraphQLQueryRequest.GraphQLQueryRequestOptions +import graphql.language.Argument +import graphql.language.BooleanValue +import graphql.language.Directive +import graphql.language.Field +import graphql.language.ListType +import graphql.language.NonNullType +import graphql.language.SelectionSet +import graphql.language.StringValue +import graphql.language.TypeName +import graphql.language.VariableDefinition +import graphql.language.VariableReference +import graphql.parser.Parser +import graphql.schema.idl.RuntimeWiring +import graphql.schema.idl.SchemaGenerator +import graphql.schema.idl.SchemaParser +import graphql.validation.Validator import org.assertj.core.api.Assertions import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.CsvSource +import org.junit.jupiter.params.provider.ValueSource +import java.util.Locale class GraphQLMultiQueryRequestTest { + @ParameterizedTest + @ValueSource(strings = ["query", "mutation"]) + fun testPreservesRootFieldDirectivesForEachRequest(operation: String) { + val directives = mutableListOf(Directive("include", listOf(Argument("if", VariableReference.of("enabled"))))) + val options = GraphQLQueryRequestOptions().apply { operationFieldDirectives = directives } + val firstQuery = + object : GraphQLQuery(operation) { + override fun getOperationName(): String = "test" + }.apply { + queryAlias = "first" + variableDefinitions.add(VariableDefinition("enabled", NonNullType(TypeName("Boolean")))) + } + val firstRequest = GraphQLQueryRequest(firstQuery, MovieProjection().name(), options) + directives.clear() + options.operationFieldDirectives = listOf(Directive("skip", listOf(Argument("if", BooleanValue(false))))) + val secondQuery = + object : GraphQLQuery(operation) { + override fun getOperationName(): String = "test" + }.apply { queryAlias = "second" } + val secondRequest = GraphQLQueryRequest(secondQuery, MovieProjection().name(), options) + val schema = + SchemaGenerator().makeExecutableSchema( + SchemaParser().parse("type Query { test: Movie } type Mutation { test: Movie } type Movie { name: String }"), + RuntimeWiring.newRuntimeWiring().build(), + ) + + val serialized = GraphQLMultiQueryRequest(listOf(firstRequest, secondRequest)).serialize() + + Assertions.assertThat(Validator().validateDocument(schema, Parser().parseDocument(serialized), Locale.ROOT)).isEmpty() + Assertions.assertThat(serialized).isEqualTo( + """$operation (${'$'}enabled: Boolean!) { + | first: test @include(if: ${'$'}enabled) { + | name + | } + | second: test @skip(if: false) { + | name + | } + |} + """.trimMargin(), + ) + } + + @ParameterizedTest + @ValueSource(strings = ["query", "mutation"]) + fun testPreservesExplicitSelectionSetsForEachRequest(operation: String) { + val requests = + listOf("name", "movieId").map { field -> + val query = + object : GraphQLQuery(operation, "Combined") { + override fun getOperationName(): String = "test" + }.apply { + queryAlias = field + input["actors"] = "actor" + } + val selectionSet = SelectionSet.newSelectionSet().selection(Field.newField(field).build()).build() + GraphQLQueryRequest(query, selectionSet) + } + val schema = + SchemaGenerator().makeExecutableSchema( + SchemaParser().parse( + """ + type Query { test(actors: String): Movie } + type Mutation { test(actors: String): Movie } + type Movie { name: String movieId: Int } + """.trimIndent(), + ), + RuntimeWiring.newRuntimeWiring().build(), + ) + + val serialized = GraphQLMultiQueryRequest(requests).serialize() + + Assertions.assertThat(Validator().validateDocument(schema, Parser().parseDocument(serialized), Locale.ROOT)).isEmpty() + Assertions.assertThat(serialized).isEqualTo( + """$operation Combined { + | name: test(actors: "actor") { + | name + | } + | movieId: test(actors: "actor") { + | movieId + | } + |} + """.trimMargin(), + ) + } + + @Test + fun testSerializeVariableReferencesWithMultipleQueries() { + val booksQuery = + object : GraphQLQuery() { + override fun getOperationName(): String = "books" + }.apply { + input["author"] = "" + variableReferences["author"] = "authorName" + variableDefinitions.add(VariableDefinition("authorName", TypeName("String"))) + } + val authorsQuery = + object : GraphQLQuery() { + override fun getOperationName(): String = "authors" + } + val booksRequest = GraphQLQueryRequest(booksQuery, MovieProjection().name()) + val authorsRequest = GraphQLQueryRequest(authorsQuery, MovieProjection().name()) + + Assertions.assertThat(booksRequest.serialize()).contains("author: \$authorName", "\$authorName: String") + Assertions.assertThat(GraphQLMultiQueryRequest(listOf(booksRequest, authorsRequest)).serialize()).isEqualTo( + """query (${'$'}authorName: String) { + | books(author: ${'$'}authorName) { + | name + | } + | authors { + | name + | } + |} + """.trimMargin(), + ) + } + + @ParameterizedTest + @ValueSource(strings = ["query", "mutation"]) + fun testSerializeVariablesFromEveryRequestAndProjection(operation: String) { + val firstQuery = + object : GraphQLQuery(operation, "Combined") { + override fun getOperationName(): String = "test" + }.apply { + queryAlias = "first" + input["actors"] = "" + input["movies"] = listOf("literal") + variableReferences["actors"] = "actor" + variableDefinitions.add(VariableDefinition("actor", NonNullType(TypeName("String")))) + } + val secondQuery = + object : GraphQLQuery(operation) { + override fun getOperationName(): String = "test" + }.apply { + queryAlias = "second" + input["movies"] = emptyList() + variableReferences["movies"] = "movieIds" + variableDefinitions.add(VariableDefinition("movieIds", NonNullType(ListType(NonNullType(TypeName("Int")))))) + } + val projection = + MovieProjection().name().apply { + inputArguments["name"] = listOf(BaseProjectionNode.InputArgument("locale", "locale", true, TypeName("String"))) + } + val request = + GraphQLMultiQueryRequest( + listOf(GraphQLQueryRequest(firstQuery, projection), GraphQLQueryRequest(secondQuery)), + ) + + Assertions.assertThat(request.serialize()).isEqualTo( + """$operation Combined(${'$'}actor: String!, ${'$'}locale: String, ${'$'}movieIds: [Int!]!) { + | first: test(actors: ${'$'}actor, movies: ["literal"]) { + | name(locale: ${'$'}locale) + | } + | second: test(movies: ${'$'}movieIds) + |} + """.trimMargin(), + ) + } + + @Test + fun testSharedVariableBetweenRootAndProjectionAcrossSerializations() { + val query = + TestGraphQLQuery().apply { + input["actors"] = "" + variableReferences["actors"] = "shared" + variableDefinitions.add(VariableDefinition("shared", NonNullType(TypeName("String")))) + } + val projection = + MovieProjection().name().apply { + inputArguments["name"] = + listOf(BaseProjectionNode.InputArgument("locale", "shared", true, NonNullType(TypeName("String")))) + } + val request = GraphQLMultiQueryRequest(listOf(GraphQLQueryRequest(query, projection))) + val expected = + """query (${'$'}shared: String!) { + | test(actors: ${'$'}shared) { + | name(locale: ${'$'}shared) + | } + |} + """.trimMargin() + + Assertions.assertThat(request.serialize()).isEqualTo(expected) + Assertions.assertThat(request.serialize()).isEqualTo(expected) + } + + @ParameterizedTest + @CsvSource("false, false", "false, true", "true, false", "true, true") + fun testReusedQueryDropsObsoleteProjectionVariables( + serializeMultiQueryFirst: Boolean, + reuseProjection: Boolean, + ) { + val query = + TestGraphQLQuery().apply { + input["actors"] = "" + variableReferences["actors"] = "actor" + variableDefinitions.add(VariableDefinition("actor", TypeName("String"))) + } + val projection = + MovieProjection().name().apply { + inputArguments["name"] = listOf(BaseProjectionNode.InputArgument("locale", "locale", true, TypeName("String"))) + } + val request = GraphQLQueryRequest(query, projection) + val multiRequest = GraphQLMultiQueryRequest(listOf(request)) + val schema = + SchemaGenerator().makeExecutableSchema( + SchemaParser().parse("type Query { test(actors: String): Movie } type Movie { name(locale: String): String }"), + RuntimeWiring.newRuntimeWiring().build(), + ) + val first = if (serializeMultiQueryFirst) multiRequest.serialize() else request.serialize() + Assertions.assertThat(Validator().validateDocument(schema, Parser().parseDocument(first), Locale.ROOT)).isEmpty() + Assertions.assertThat(first).contains("\$locale: String", "locale: \$locale") + + val literalProjection = if (reuseProjection) projection else MovieProjection().name() + literalProjection.inputArguments["name"] = listOf(BaseProjectionNode.InputArgument("locale", "en")) + val literalRequest = if (reuseProjection) request else GraphQLQueryRequest(query, literalProjection) + val literalMultiRequest = if (reuseProjection) multiRequest else GraphQLMultiQueryRequest(listOf(literalRequest)) + + for (serialized in listOf(literalMultiRequest.serialize(), literalRequest.serialize(), literalRequest.serializeCompact())) { + Assertions.assertThat(Validator().validateDocument(schema, Parser().parseDocument(serialized), Locale.ROOT)).isEmpty() + Assertions.assertThat(serialized).contains("\$actor").doesNotContain("\$locale") + } + } + + @ParameterizedTest + @CsvSource("false, false", "true, false", "false, true", "true, true") + fun testSharedVariableDefaultsRemainUnambiguous( + conflictingDefaults: Boolean, + differingNullability: Boolean, + ) { + val requests = + listOf("first", "second").map { alias -> + val query = + TestGraphQLQuery().apply { + queryAlias = alias + input["actors"] = "" + variableReferences["actors"] = "shared" + variableDefinitions.add( + VariableDefinition + .newVariableDefinition( + "shared", + if (differingNullability && alias == "second") NonNullType(TypeName("String")) else TypeName("String"), + ).defaultValue(StringValue(if (conflictingDefaults) alias else "default")) + .build(), + ) + } + GraphQLQueryRequest(query) + } + val schema = + SchemaGenerator().makeExecutableSchema( + SchemaParser().parse("type Query { test(actors: String): String }"), + RuntimeWiring.newRuntimeWiring().build(), + ) + val serialized = GraphQLMultiQueryRequest(requests).serialize() + val errors = Validator().validateDocument(schema, Parser().parseDocument(serialized), Locale.ROOT) + + if (conflictingDefaults) { + Assertions.assertThat(errors).isNotEmpty() + } else { + Assertions.assertThat(errors).isEmpty() + val expectedType = if (differingNullability) "String!" else "String" + Assertions.assertThat(serialized).contains("\$shared: $expectedType = \"default\"") + } + } + + @ParameterizedTest + @CsvSource( + "String, String!, String!", + "String!, String, String!", + "[String], [String!]!, [String!]!", + "[String!]!, [String], [String!]!", + "[String!], [String]!, [String!]!", + "[[String]], [[String!]!]!, [[String!]!]!", + ) + fun testSharedVariableUsesStrictestNullability( + firstType: String, + secondType: String, + expectedType: String, + ) { + val requests = + listOf("first" to firstType, "second" to secondType).map { (field, type) -> + val query = + object : GraphQLQuery() { + override fun getOperationName(): String = field + }.apply { + input["value"] = "" + variableReferences["value"] = "shared" + variableDefinitions.add(VariableDefinition("shared", Parser.parseType(type))) + } + GraphQLQueryRequest(query) + } + val schema = + SchemaGenerator().makeExecutableSchema( + SchemaParser().parse("type Query { first(value: $firstType): String second(value: $secondType): String }"), + RuntimeWiring.newRuntimeWiring().build(), + ) + val serialized = GraphQLMultiQueryRequest(requests).serialize() + + Assertions.assertThat(Validator().validateDocument(schema, Parser().parseDocument(serialized), Locale.ROOT)).isEmpty() + Assertions.assertThat(serialized).contains("query (\$shared: $expectedType)") + } + + @ParameterizedTest + @CsvSource("String, ID", "String, [String]", "[String], [[String]]") + fun testConflictingSharedVariableTypesRemainInvalid( + firstType: String, + secondType: String, + ) { + val requests = + listOf("first" to firstType, "second" to secondType).map { (field, type) -> + val query = + object : GraphQLQuery() { + override fun getOperationName(): String = field + }.apply { + input["value"] = "" + variableReferences["value"] = "shared" + variableDefinitions.add(VariableDefinition("shared", Parser.parseType(type))) + } + GraphQLQueryRequest(query) + } + val schema = + SchemaGenerator().makeExecutableSchema( + SchemaParser().parse("type Query { first(value: $firstType): String second(value: $secondType): String }"), + RuntimeWiring.newRuntimeWiring().build(), + ) + val serialized = GraphQLMultiQueryRequest(requests).serialize() + + Assertions.assertThat(Validator().validateDocument(schema, Parser().parseDocument(serialized), Locale.ROOT)).isNotEmpty() + Assertions.assertThat(serialized).contains("\$shared: $firstType", "\$shared: $secondType") + } + @Test fun testSerializeInputClassWithProjectionAndMultipleQueries() { val query = diff --git a/graphql-dgs-codegen-shared-core/src/test/kotlin/com/netflix/graphql/dgs/client/codegen/GraphQLQueryRequestTest.kt b/graphql-dgs-codegen-shared-core/src/test/kotlin/com/netflix/graphql/dgs/client/codegen/GraphQLQueryRequestTest.kt index a654ec760..831a948df 100644 --- a/graphql-dgs-codegen-shared-core/src/test/kotlin/com/netflix/graphql/dgs/client/codegen/GraphQLQueryRequestTest.kt +++ b/graphql-dgs-codegen-shared-core/src/test/kotlin/com/netflix/graphql/dgs/client/codegen/GraphQLQueryRequestTest.kt @@ -23,6 +23,7 @@ import com.netflix.graphql.dgs.client.codegen.exampleprojection.EntitiesProjecti import graphql.GraphQLContext import graphql.execution.CoercedVariables import graphql.language.Argument +import graphql.language.AstPrinter import graphql.language.BooleanValue import graphql.language.Directive import graphql.language.IntValue @@ -41,12 +42,88 @@ import graphql.schema.idl.SchemaParser import graphql.validation.Validator import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.CsvSource import java.time.LocalDate import java.time.ZoneId import java.time.format.DateTimeFormatter import java.util.* class GraphQLQueryRequestTest { + @ParameterizedTest + @CsvSource( + "String!, String!, String!", + "String, String!, String!", + "String!, String, String!", + "[String], [String!]!, [String!]!", + "[String!], [String]!, [String!]!", + ) + fun testSharedRootAndProjectionVariableUsesStrictestNullability( + rootType: String, + projectionType: String, + expectedType: String, + ) { + val query = + TestGraphQLQuery().apply { + input["actors"] = "" + variableReferences["actors"] = "shared" + variableDefinitions.add(VariableDefinition("shared", Parser.parseType(rootType))) + } + val projection = + MovieProjection().name().apply { + inputArguments["name"] = + listOf(BaseProjectionNode.InputArgument("locale", "shared", true, Parser.parseType(projectionType))) + } + val request = GraphQLQueryRequest(query, projection) + val schema = + SchemaGenerator().makeExecutableSchema( + SchemaParser().parse("type Query { test(actors: $rootType): Movie } type Movie { name(locale: $projectionType): String }"), + RuntimeWiring.newRuntimeWiring().build(), + ) + + repeat(2) { + for (serialized in listOf(request.serialize(), request.serializeCompact())) { + val document = Parser().parseDocument(serialized) + assertThat(Validator().validateDocument(schema, document, Locale.ROOT)).isEmpty() + val definition = document.definitions.single() as OperationDefinition + assertThat(definition.variableDefinitions.map { AstPrinter.printAstCompact(it) }).containsExactly("\$shared:$expectedType") + } + } + } + + @ParameterizedTest + @CsvSource("String, ID", "String, [String]", "[String], [[String]]") + fun testConflictingRootAndProjectionVariableTypesRemainInvalid( + rootType: String, + projectionType: String, + ) { + val query = + TestGraphQLQuery().apply { + input["actors"] = "" + variableReferences["actors"] = "shared" + variableDefinitions.add(VariableDefinition("shared", Parser.parseType(rootType))) + } + val projection = + MovieProjection().name().apply { + inputArguments["name"] = + listOf(BaseProjectionNode.InputArgument("locale", "shared", true, Parser.parseType(projectionType))) + } + val request = GraphQLQueryRequest(query, projection) + val schema = + SchemaGenerator().makeExecutableSchema( + SchemaParser().parse("type Query { test(actors: $rootType): Movie } type Movie { name(locale: $projectionType): String }"), + RuntimeWiring.newRuntimeWiring().build(), + ) + + for (serialized in listOf(request.serialize(), request.serializeCompact())) { + val document = Parser().parseDocument(serialized) + assertThat(Validator().validateDocument(schema, document, Locale.ROOT)).isNotEmpty() + val definition = document.definitions.single() as OperationDefinition + assertThat(definition.variableDefinitions.map { AstPrinter.printAstCompact(it) }) + .containsExactly("\$shared:$rootType", "\$shared:$projectionType") + } + } + @Test fun testSerializeCompactListOfStringsAsInput() { val query =