From 32640d57114e4826793bfe15825178bed4dc06f5 Mon Sep 17 00:00:00 2001 From: faizan-siddiqui Date: Thu, 24 Mar 2022 13:56:23 +1100 Subject: [PATCH 01/17] add check for hydration arg type --- .../validation/NadelHydrationValidation.kt | 37 ++++++++++--- .../validation/NadelSchemaValidationError.kt | 24 +++++++++ .../NadelHydrationValidationTest.kt | 54 +++++++++++++++++++ 3 files changed, 108 insertions(+), 7 deletions(-) diff --git a/engine-nextgen/src/main/kotlin/graphql/nadel/validation/NadelHydrationValidation.kt b/engine-nextgen/src/main/kotlin/graphql/nadel/validation/NadelHydrationValidation.kt index 64e42cf73..2ca54fd6f 100644 --- a/engine-nextgen/src/main/kotlin/graphql/nadel/validation/NadelHydrationValidation.kt +++ b/engine-nextgen/src/main/kotlin/graphql/nadel/validation/NadelHydrationValidation.kt @@ -1,19 +1,23 @@ package graphql.nadel.validation import graphql.nadel.Service -import graphql.nadel.dsl.RemoteArgumentSource +import graphql.nadel.dsl.RemoteArgumentDefinition import graphql.nadel.dsl.RemoteArgumentSource.SourceType.FieldArgument import graphql.nadel.dsl.RemoteArgumentSource.SourceType.ObjectField import graphql.nadel.dsl.UnderlyingServiceHydration import graphql.nadel.enginekt.util.getFieldAt import graphql.nadel.enginekt.util.isList import graphql.nadel.enginekt.util.isNonNull +import graphql.nadel.enginekt.util.isNotWrapped +import graphql.nadel.enginekt.util.isWrapped import graphql.nadel.enginekt.util.unwrapAll import graphql.nadel.enginekt.util.unwrapNonNull +import graphql.nadel.enginekt.util.unwrapOne import graphql.nadel.validation.NadelSchemaValidationError.CannotRenameHydratedField import graphql.nadel.validation.NadelSchemaValidationError.DuplicatedHydrationArgument import graphql.nadel.validation.NadelSchemaValidationError.FieldWithPolymorphicHydrationMustReturnAUnion import graphql.nadel.validation.NadelSchemaValidationError.HydrationFieldMustBeNullable +import graphql.nadel.validation.NadelSchemaValidationError.IncompatibleHydrationArgumentType import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationActorField import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationActorService import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationArgumentValueSource @@ -26,8 +30,11 @@ import graphql.nadel.validation.util.NadelSchemaUtil.getUnderlyingType import graphql.nadel.validation.util.NadelSchemaUtil.hasRename import graphql.schema.GraphQLFieldDefinition import graphql.schema.GraphQLFieldsContainer +import graphql.schema.GraphQLInputType import graphql.schema.GraphQLObjectType +import graphql.schema.GraphQLOutputType import graphql.schema.GraphQLSchema +import graphql.schema.GraphQLType import graphql.schema.GraphQLUnionType import graphql.schema.GraphQLUnmodifiedType @@ -179,8 +186,7 @@ internal class NadelHydrationValidation( argument = remoteArg.name, ) } else { - val remoteArgSource = remoteArg.remoteArgumentSource - getRemoteArgErrors(parent, overallField, remoteArgSource) + getRemoteArgErrors(parent, overallField, remoteArg, actorField) } } @@ -205,17 +211,25 @@ internal class NadelHydrationValidation( } private fun getRemoteArgErrors( - parent: NadelServiceSchemaElement, - overallField: GraphQLFieldDefinition, - remoteArgSource: RemoteArgumentSource, + parent: NadelServiceSchemaElement, + overallField: GraphQLFieldDefinition, + remoteArg: RemoteArgumentDefinition, + actorField: GraphQLFieldDefinition, ): NadelSchemaValidationError? { + val remoteArgSource = remoteArg.remoteArgumentSource return when (remoteArgSource.sourceType) { ObjectField -> { val field = (parent.underlying as GraphQLFieldsContainer).getFieldAt(remoteArgSource.pathToField!!) if (field == null) { MissingHydrationFieldValueSource(parent, overallField, remoteArgSource) } else { - // TODO: check argument type is correct + //check the input types match with hydration and actor fields + val actorArg = actorField.getArgument(remoteArg.name) + val fieldOutputType = field.type + val actorArgInputType = actorArg.type + if (!outputTypeMatchesInputType(fieldOutputType, actorArgInputType)) { + IncompatibleHydrationArgumentType(parent, overallField, remoteArg, fieldOutputType, actorArgInputType) + } null } } @@ -233,4 +247,13 @@ internal class NadelHydrationValidation( } } } + + /* + * Checks the type of a hydration argument against the type of an actor field argument to see if they match + * + */ + private fun hydrationArgTypesMatch(outputType: GraphQLOutputType, inputType: GraphQLInputType): Boolean { + //TODO + return true + } } diff --git a/engine-nextgen/src/main/kotlin/graphql/nadel/validation/NadelSchemaValidationError.kt b/engine-nextgen/src/main/kotlin/graphql/nadel/validation/NadelSchemaValidationError.kt index a8ffd1cfb..706fadc20 100644 --- a/engine-nextgen/src/main/kotlin/graphql/nadel/validation/NadelSchemaValidationError.kt +++ b/engine-nextgen/src/main/kotlin/graphql/nadel/validation/NadelSchemaValidationError.kt @@ -16,9 +16,11 @@ import graphql.schema.GraphQLDirective import graphql.schema.GraphQLEnumValueDefinition import graphql.schema.GraphQLFieldDefinition import graphql.schema.GraphQLInputObjectField +import graphql.schema.GraphQLInputType import graphql.schema.GraphQLNamedSchemaElement import graphql.schema.GraphQLNamedType import graphql.schema.GraphQLObjectType +import graphql.schema.GraphQLOutputType import graphql.schema.GraphQLType import graphql.schema.GraphQLTypeUtil @@ -321,6 +323,28 @@ sealed interface NadelSchemaValidationError { override val subject = overallField } + data class IncompatibleHydrationArgumentType( + val parentType: NadelServiceSchemaElement, + val overallField: GraphQLFieldDefinition, + val remoteArg: RemoteArgumentDefinition, + val outputFieldType: GraphQLOutputType, + val actorArgInputType: GraphQLInputType, + ) : NadelSchemaValidationError { + val service: Service get() = parentType.service + + override val message = run { + val hydrationArgName = remoteArg.name + val of = makeFieldCoordinates(parentType.overall.name, overallField.name) + val uf = "${parentType.underlying.name}.${remoteArg.remoteArgumentSource.pathToField?.joinToString(separator = ".")}" + val s = service.name + "Field $of tried to hydrate with argument $hydrationArgName using value from underlying field $uf from " + + "service $s with an argument of $outputFieldType whereas the actor field requires an argument of" + + "type $actorArgInputType" + } + + override val subject = overallField + } + data class MissingHydrationArgumentValueSource( val parentType: NadelServiceSchemaElement, val overallField: GraphQLFieldDefinition, diff --git a/engine-nextgen/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt b/engine-nextgen/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt index c0aba9f0f..8569a0a9f 100644 --- a/engine-nextgen/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt +++ b/engine-nextgen/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt @@ -920,5 +920,59 @@ class NadelHydrationValidationTest : DescribeSpec({ assert(error.overallField.name == "name") assert(error.subject == error.overallField) } + + it("fails if hydration argument types are mismatch with actor") { + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: JiraIssue + } + type JiraIssue @renamed(from: "Issue") { + id: ID! + creator: User @hydrated( + service: "users" + field: "user" + arguments: [ + {name: "id", value: "$source.creator"} + ] + ) + } + """.trimIndent(), + "users" to """ + type Query { + user(id: Int!): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + creator: ID! + } + """.trimIndent(), + "users" to """ + type Query { + user(id: Int!): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.map { it.message }.isEmpty()) + } } }) From 4a1d842b532c6fb2c68f33b3391297031eba8dbb Mon Sep 17 00:00:00 2001 From: faizan-siddiqui Date: Thu, 24 Mar 2022 22:12:49 +1100 Subject: [PATCH 02/17] add test case for args as well --- .../graphql/nadel/validation/NadelHydrationValidation.kt | 8 ++++---- .../nadel/validation/NadelHydrationValidationTest.kt | 7 ++++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/engine-nextgen/src/main/kotlin/graphql/nadel/validation/NadelHydrationValidation.kt b/engine-nextgen/src/main/kotlin/graphql/nadel/validation/NadelHydrationValidation.kt index 2ca54fd6f..971bd1230 100644 --- a/engine-nextgen/src/main/kotlin/graphql/nadel/validation/NadelHydrationValidation.kt +++ b/engine-nextgen/src/main/kotlin/graphql/nadel/validation/NadelHydrationValidation.kt @@ -217,6 +217,8 @@ internal class NadelHydrationValidation( actorField: GraphQLFieldDefinition, ): NadelSchemaValidationError? { val remoteArgSource = remoteArg.remoteArgumentSource + val actorArg = actorField.getArgument(remoteArg.name) + val actorArgInputType = actorArg.type return when (remoteArgSource.sourceType) { ObjectField -> { val field = (parent.underlying as GraphQLFieldsContainer).getFieldAt(remoteArgSource.pathToField!!) @@ -224,10 +226,8 @@ internal class NadelHydrationValidation( MissingHydrationFieldValueSource(parent, overallField, remoteArgSource) } else { //check the input types match with hydration and actor fields - val actorArg = actorField.getArgument(remoteArg.name) val fieldOutputType = field.type - val actorArgInputType = actorArg.type - if (!outputTypeMatchesInputType(fieldOutputType, actorArgInputType)) { + if (!hydrationArgTypesMatch(fieldOutputType, actorArgInputType)) { IncompatibleHydrationArgumentType(parent, overallField, remoteArg, fieldOutputType, actorArgInputType) } null @@ -238,7 +238,7 @@ internal class NadelHydrationValidation( if (argument == null) { MissingHydrationArgumentValueSource(parent, overallField, remoteArgSource) } else { - // TODO: check argument type is correct + //check the input types match with hydration and actor fields null } } diff --git a/engine-nextgen/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt b/engine-nextgen/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt index 8569a0a9f..84e0b6cc7 100644 --- a/engine-nextgen/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt +++ b/engine-nextgen/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt @@ -930,18 +930,19 @@ class NadelHydrationValidationTest : DescribeSpec({ } type JiraIssue @renamed(from: "Issue") { id: ID! - creator: User @hydrated( + creator(someArg: ID!): User @hydrated( service: "users" field: "user" arguments: [ {name: "id", value: "$source.creator"} + {name: "someArg", value: "$argument.someArg"} ] ) } """.trimIndent(), "users" to """ type Query { - user(id: Int!): User + user(id: Int!, someArg: Int!): User } type User { id: ID! @@ -961,7 +962,7 @@ class NadelHydrationValidationTest : DescribeSpec({ """.trimIndent(), "users" to """ type Query { - user(id: Int!): User + user(id: Int!, someArg: Int!): User } type User { id: ID! From 76feed3e2ff8e358352f04f946da5b5b4c379aaa Mon Sep 17 00:00:00 2001 From: faizan-siddiqui Date: Mon, 28 Mar 2022 20:14:33 +1100 Subject: [PATCH 03/17] add check for FieldArgument --- .../graphql/nadel/validation/NadelHydrationValidation.kt | 9 +++++++-- .../nadel/validation/NadelSchemaValidationError.kt | 6 +++--- .../java/graphql/nadel/validation/NadelTypeValidation.kt | 3 ++- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt index 198034d88..a28028884 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt @@ -30,6 +30,7 @@ import graphql.schema.GraphQLObjectType import graphql.schema.GraphQLOutputType import graphql.schema.GraphQLInterfaceType import graphql.schema.GraphQLNamedOutputType +import graphql.schema.GraphQLNamedType import graphql.schema.GraphQLSchema import graphql.schema.GraphQLType import graphql.schema.GraphQLUnionType @@ -209,8 +210,8 @@ internal class NadelHydrationValidation( } else { //check the input types match with hydration and actor fields val fieldOutputType = field.type - if (!hydrationArgTypesMatch(fieldOutputType, actorArgInputType)) { - IncompatibleHydrationArgumentType(parent, overallField, remoteArg, fieldOutputType, actorArgInputType) + if (!typeValidation.isAssignableTo(actorArgInputType as GraphQLNamedType, fieldOutputType as GraphQLNamedType)) { + IncompatibleHydrationArgumentType(parent, overallField, remoteArg, fieldOutputType.name, actorArgInputType.name) } null } @@ -221,6 +222,10 @@ internal class NadelHydrationValidation( MissingHydrationArgumentValueSource(parent, overallField, remoteArgSource) } else { //check the input types match with hydration and actor fields + val hydrationArgType = argument.type + if (!typeValidation.isAssignableTo(actorArgInputType as GraphQLNamedType, hydrationArgType as GraphQLNamedType)) { + IncompatibleHydrationArgumentType(parent, overallField, remoteArg, hydrationArgType.name, actorArgInputType.name) + } null } } diff --git a/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationError.kt b/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationError.kt index 3d8acd51d..0fb392dda 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationError.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationError.kt @@ -326,8 +326,8 @@ sealed interface NadelSchemaValidationError { val parentType: NadelServiceSchemaElement, val overallField: GraphQLFieldDefinition, val remoteArg: RemoteArgumentDefinition, - val outputFieldType: GraphQLOutputType, - val actorArgInputType: GraphQLInputType, + val hydrationType: String, + val actorArgInputType: String, ) : NadelSchemaValidationError { val service: Service get() = parentType.service @@ -337,7 +337,7 @@ sealed interface NadelSchemaValidationError { val uf = "${parentType.underlying.name}.${remoteArg.remoteArgumentSource.pathToField?.joinToString(separator = ".")}" val s = service.name "Field $of tried to hydrate with argument $hydrationArgName using value from underlying field $uf from " + - "service $s with an argument of $outputFieldType whereas the actor field requires an argument of" + + "service $s with an argument of $hydrationType whereas the actor field requires an argument of" + "type $actorArgInputType" } diff --git a/lib/src/main/java/graphql/nadel/validation/NadelTypeValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelTypeValidation.kt index e711e62df..8206c582a 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelTypeValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelTypeValidation.kt @@ -26,6 +26,7 @@ import graphql.nadel.validation.util.NadelSchemaUtil.getUnderlyingType import graphql.nadel.validation.util.getReachableTypeNames import graphql.schema.GraphQLFieldDefinition import graphql.schema.GraphQLImplementingType +import graphql.schema.GraphQLInputType import graphql.schema.GraphQLInterfaceType import graphql.schema.GraphQLNamedOutputType import graphql.schema.GraphQLNamedType @@ -118,7 +119,7 @@ internal class NadelTypeValidation( * Note: this assumes both types are from the same schema. This does NOT * deal with differences between overall and underlying schema. */ - fun isAssignableTo(lhs: GraphQLNamedOutputType, rhs: GraphQLNamedOutputType): Boolean { + fun isAssignableTo(lhs: GraphQLNamedType, rhs: GraphQLNamedType): Boolean { if (lhs.name == rhs.name) { return true } From a8476a695637b503e1fe1790a8d695ff73e076a1 Mon Sep 17 00:00:00 2001 From: faizan-siddiqui Date: Mon, 28 Mar 2022 21:19:51 +1100 Subject: [PATCH 04/17] don't use isAssignableTo --- .../nadel/validation/NadelHydrationValidation.kt | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt index a28028884..af92b9add 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt @@ -210,7 +210,7 @@ internal class NadelHydrationValidation( } else { //check the input types match with hydration and actor fields val fieldOutputType = field.type - if (!typeValidation.isAssignableTo(actorArgInputType as GraphQLNamedType, fieldOutputType as GraphQLNamedType)) { + if (!hydrationArgTypesMatch(fieldOutputType, actorArgInputType)) { IncompatibleHydrationArgumentType(parent, overallField, remoteArg, fieldOutputType.name, actorArgInputType.name) } null @@ -222,8 +222,8 @@ internal class NadelHydrationValidation( MissingHydrationArgumentValueSource(parent, overallField, remoteArgSource) } else { //check the input types match with hydration and actor fields - val hydrationArgType = argument.type - if (!typeValidation.isAssignableTo(actorArgInputType as GraphQLNamedType, hydrationArgType as GraphQLNamedType)) { + val hydrationArgType = argument.type + if (!hydrationArgTypesMatch(hydrationArgType, actorArgInputType)) { IncompatibleHydrationArgumentType(parent, overallField, remoteArg, hydrationArgType.name, actorArgInputType.name) } null @@ -243,4 +243,13 @@ internal class NadelHydrationValidation( //TODO return true } + + /* + * Checks the type of a hydration argument against the type of an actor field argument to see if they match + * + */ + private fun hydrationArgTypesMatch(argInputType: GraphQLInputType, inputType: GraphQLInputType): Boolean { + //TODO + return true + } } From 5996202a58279a95f7126386556fc271648346a4 Mon Sep 17 00:00:00 2001 From: faizan-siddiqui Date: Mon, 28 Mar 2022 21:30:17 +1100 Subject: [PATCH 05/17] change name reference to toString --- .../nadel/validation/NadelHydrationValidation.kt | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt index af92b9add..d96d21d6b 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt @@ -211,7 +211,7 @@ internal class NadelHydrationValidation( //check the input types match with hydration and actor fields val fieldOutputType = field.type if (!hydrationArgTypesMatch(fieldOutputType, actorArgInputType)) { - IncompatibleHydrationArgumentType(parent, overallField, remoteArg, fieldOutputType.name, actorArgInputType.name) + IncompatibleHydrationArgumentType(parent, overallField, remoteArg, fieldOutputType.toString(), actorArgInputType.toString()) } null } @@ -224,7 +224,7 @@ internal class NadelHydrationValidation( //check the input types match with hydration and actor fields val hydrationArgType = argument.type if (!hydrationArgTypesMatch(hydrationArgType, actorArgInputType)) { - IncompatibleHydrationArgumentType(parent, overallField, remoteArg, hydrationArgType.name, actorArgInputType.name) + IncompatibleHydrationArgumentType(parent, overallField, remoteArg, hydrationArgType.toString(), actorArgInputType.toString()) } null } @@ -236,16 +236,18 @@ internal class NadelHydrationValidation( } /* - * Checks the type of a hydration argument against the type of an actor field argument to see if they match + * Checks the type of a hydration argument derived from source field output against the type of an actor field + * argument to see if they match * */ - private fun hydrationArgTypesMatch(outputType: GraphQLOutputType, inputType: GraphQLInputType): Boolean { + private fun hydrationArgTypesMatch(fieldOutputType: GraphQLOutputType, inputType: GraphQLInputType): Boolean { //TODO return true } /* - * Checks the type of a hydration argument against the type of an actor field argument to see if they match + * Checks the type of a hydration argument derived from field input argument against the type of an actor field + * argument to see if they match * */ private fun hydrationArgTypesMatch(argInputType: GraphQLInputType, inputType: GraphQLInputType): Boolean { From 65c51566abed5a306be98ee704bc95f486611b2b Mon Sep 17 00:00:00 2001 From: faizan-siddiqui Date: Tue, 29 Mar 2022 00:21:03 +1100 Subject: [PATCH 06/17] add logic for hydrationArgsMatch function --- .../validation/NadelHydrationValidation.kt | 67 +++++++++++----- .../validation/NadelSchemaValidationError.kt | 4 +- .../NadelHydrationValidationTest.kt | 78 +++++++++++++++++-- 3 files changed, 123 insertions(+), 26 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt index d96d21d6b..1d86951d4 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt @@ -1,5 +1,6 @@ package graphql.nadel.validation +import graphql.Scalars import graphql.nadel.Service import graphql.nadel.dsl.RemoteArgumentDefinition import graphql.nadel.dsl.RemoteArgumentSource.SourceType.FieldArgument @@ -8,8 +9,11 @@ import graphql.nadel.dsl.UnderlyingServiceHydration import graphql.nadel.engine.util.getFieldAt import graphql.nadel.engine.util.isList import graphql.nadel.engine.util.isNonNull +import graphql.nadel.engine.util.isNotWrapped +import graphql.nadel.engine.util.isWrapped import graphql.nadel.engine.util.unwrapAll import graphql.nadel.engine.util.unwrapNonNull +import graphql.nadel.engine.util.unwrapOne import graphql.nadel.validation.NadelSchemaValidationError.CannotRenameHydratedField import graphql.nadel.validation.NadelSchemaValidationError.DuplicatedHydrationArgument import graphql.nadel.validation.NadelSchemaValidationError.FieldWithPolymorphicHydrationMustReturnAUnion @@ -21,19 +25,19 @@ import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationArgum import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationFieldValueSource import graphql.nadel.validation.NadelSchemaValidationError.MissingRequiredHydrationActorFieldArgument import graphql.nadel.validation.NadelSchemaValidationError.NonExistentHydrationActorFieldArgument +import graphql.nadel.validation.util.NadelSchemaUtil import graphql.nadel.validation.util.NadelSchemaUtil.getHydrations import graphql.nadel.validation.util.NadelSchemaUtil.hasRename import graphql.schema.GraphQLFieldDefinition import graphql.schema.GraphQLFieldsContainer +import graphql.schema.GraphQLImplementingType import graphql.schema.GraphQLInputType -import graphql.schema.GraphQLObjectType -import graphql.schema.GraphQLOutputType import graphql.schema.GraphQLInterfaceType import graphql.schema.GraphQLNamedOutputType -import graphql.schema.GraphQLNamedType import graphql.schema.GraphQLSchema import graphql.schema.GraphQLType import graphql.schema.GraphQLUnionType +import graphql.schema.GraphQLUnmodifiedType internal class NadelHydrationValidation( private val services: Map, @@ -211,7 +215,7 @@ internal class NadelHydrationValidation( //check the input types match with hydration and actor fields val fieldOutputType = field.type if (!hydrationArgTypesMatch(fieldOutputType, actorArgInputType)) { - IncompatibleHydrationArgumentType(parent, overallField, remoteArg, fieldOutputType.toString(), actorArgInputType.toString()) + return IncompatibleHydrationArgumentType(parent, overallField, remoteArg, fieldOutputType.toString(), actorArgInputType.toString()) } null } @@ -224,7 +228,7 @@ internal class NadelHydrationValidation( //check the input types match with hydration and actor fields val hydrationArgType = argument.type if (!hydrationArgTypesMatch(hydrationArgType, actorArgInputType)) { - IncompatibleHydrationArgumentType(parent, overallField, remoteArg, hydrationArgType.toString(), actorArgInputType.toString()) + return IncompatibleHydrationArgumentType(parent, overallField, remoteArg, hydrationArgType.toString(), actorArgInputType.toString()) } null } @@ -236,22 +240,49 @@ internal class NadelHydrationValidation( } /* - * Checks the type of a hydration argument derived from source field output against the type of an actor field - * argument to see if they match + * Checks the type of a hydration argument derived from either a $source field output type or $argument field input + * argument type against the type of an actor field argument to see if they match * */ - private fun hydrationArgTypesMatch(fieldOutputType: GraphQLOutputType, inputType: GraphQLInputType): Boolean { - //TODO - return true + private fun hydrationArgTypesMatch( + hydrationSourceType: GraphQLType, + actorArgumentInputType: GraphQLType, + ): Boolean { + var hydrationSource: GraphQLType = hydrationSourceType + var actorArgument: GraphQLType = actorArgumentInputType + + while (hydrationSource.isWrapped && actorArgument.isWrapped) { + if ((hydrationSource.isList && actorArgument.isList) || (hydrationSource.isNonNull && actorArgument.isNonNull)) { + hydrationSource = hydrationSource.unwrapOne() + actorArgument = actorArgument.unwrapOne() + } else { + return false + } + } + + if (hydrationSource.isNotWrapped && actorArgument.isNotWrapped) { + return isOutputTypeNameValid( + hydrationSourceType = hydrationSource as GraphQLUnmodifiedType, + actorArgumentInputType = actorArgument as GraphQLUnmodifiedType, + ) + } else { + return false + } } - /* - * Checks the type of a hydration argument derived from field input argument against the type of an actor field - * argument to see if they match - * - */ - private fun hydrationArgTypesMatch(argInputType: GraphQLInputType, inputType: GraphQLInputType): Boolean { - //TODO - return true + private fun isOutputTypeNameValid( + hydrationSourceType: GraphQLUnmodifiedType, + actorArgumentInputType: GraphQLUnmodifiedType, + ): Boolean { + if (NadelSchemaUtil.getUnderlyingName(hydrationSourceType) == NadelSchemaUtil.getUnderlyingName(actorArgumentInputType)) { + return true + } + if (actorArgumentInputType.name == Scalars.GraphQLID.name && hydrationSourceType.name == Scalars.GraphQLString.name) { + return true + } + if (actorArgumentInputType is GraphQLInterfaceType && hydrationSourceType is GraphQLImplementingType) { + return hydrationSourceType.interfaces.contains(actorArgumentInputType) + } + return false } } diff --git a/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationError.kt b/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationError.kt index 0fb392dda..3c67fdbb8 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationError.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationError.kt @@ -334,9 +334,9 @@ sealed interface NadelSchemaValidationError { override val message = run { val hydrationArgName = remoteArg.name val of = makeFieldCoordinates(parentType.overall.name, overallField.name) - val uf = "${parentType.underlying.name}.${remoteArg.remoteArgumentSource.pathToField?.joinToString(separator = ".")}" + val remoteArgSource = "${parentType.overall.name}.${remoteArg.remoteArgumentSource.pathToField?.joinToString(separator = ".")}" val s = service.name - "Field $of tried to hydrate with argument $hydrationArgName using value from underlying field $uf from " + + "Field $of tried to hydrate with argument $hydrationArgName using value from field $remoteArgSource from " + "service $s with an argument of $hydrationType whereas the actor field requires an argument of" + "type $actorArgInputType" } diff --git a/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt b/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt index 6ec492be8..69ffb2c21 100644 --- a/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt +++ b/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt @@ -5,6 +5,7 @@ import graphql.nadel.validation.NadelSchemaValidationError.CannotRenameHydratedF import graphql.nadel.validation.NadelSchemaValidationError.DuplicatedHydrationArgument import graphql.nadel.validation.NadelSchemaValidationError.HydrationFieldMustBeNullable import graphql.nadel.validation.NadelSchemaValidationError.HydrationIncompatibleOutputType +import graphql.nadel.validation.NadelSchemaValidationError.IncompatibleHydrationArgumentType import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationActorField import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationActorService import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationArgumentValueSource @@ -1205,7 +1206,7 @@ class NadelHydrationValidationTest : DescribeSpec({ assert(errors.map { it.message }.isEmpty()) } - it("fails if hydration argument types are mismatch with actor") { + it("fails if hydration argument source type is mismatch with actor field input arguments") { val fixture = NadelValidationTestFixture( overallSchema = mapOf( "issues" to """ @@ -1214,19 +1215,18 @@ class NadelHydrationValidationTest : DescribeSpec({ } type JiraIssue @renamed(from: "Issue") { id: ID! - creator(someArg: ID!): User @hydrated( + creator: User @hydrated( service: "users" field: "user" arguments: [ {name: "id", value: "$source.creator"} - {name: "someArg", value: "$argument.someArg"} ] ) } """.trimIndent(), "users" to """ type Query { - user(id: Int!, someArg: Int!): User + user(id: Int!): User } type User { id: ID! @@ -1246,7 +1246,7 @@ class NadelHydrationValidationTest : DescribeSpec({ """.trimIndent(), "users" to """ type Query { - user(id: Int!, someArg: Int!): User + user(id: Int!): User } type User { id: ID! @@ -1257,7 +1257,73 @@ class NadelHydrationValidationTest : DescribeSpec({ ) val errors = validate(fixture) - assert(errors.map { it.message }.isEmpty()) + assert(errors.isNotEmpty()) + val error = errors.singleOfType() + assert(error.parentType.overall.name == "JiraIssue") + assert(error.overallField.name == "creator") + assert(error.remoteArg.name == "id") + assert(error.hydrationType == "ID!") + assert(error.actorArgInputType == "Int!") + } + + it("fails if hydration argument types are mismatch with actor field input arguments") { + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: JiraIssue + } + type JiraIssue @renamed(from: "Issue") { + id: ID! + creator(someArg: ID!): User @hydrated( + service: "users" + field: "user" + arguments: [ + {name: "someArg", value: "$argument.someArg"} + ] + ) + } + """.trimIndent(), + "users" to """ + type Query { + user(someArg: Int!): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + creator: ID! + } + """.trimIndent(), + "users" to """ + type Query { + user(someArg: Int!): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.isNotEmpty()) + val error = errors.singleOfType() + assert(error.parentType.overall.name == "JiraIssue") + assert(error.overallField.name == "creator") + assert(error.remoteArg.name == "someArg") + assert(error.hydrationType == "ID!") + assert(error.actorArgInputType == "Int!") } } }) From 8c6e71119e355a5633ef1917f9e649d1b2dbf830 Mon Sep 17 00:00:00 2001 From: faizan-siddiqui Date: Tue, 29 Mar 2022 00:25:42 +1100 Subject: [PATCH 07/17] use isAssignableTo --- .../validation/NadelHydrationValidation.kt | 23 ++++--------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt index 1d86951d4..8d2b62f8e 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt @@ -34,6 +34,7 @@ import graphql.schema.GraphQLImplementingType import graphql.schema.GraphQLInputType import graphql.schema.GraphQLInterfaceType import graphql.schema.GraphQLNamedOutputType +import graphql.schema.GraphQLNamedType import graphql.schema.GraphQLSchema import graphql.schema.GraphQLType import graphql.schema.GraphQLUnionType @@ -261,28 +262,12 @@ internal class NadelHydrationValidation( } if (hydrationSource.isNotWrapped && actorArgument.isNotWrapped) { - return isOutputTypeNameValid( - hydrationSourceType = hydrationSource as GraphQLUnmodifiedType, - actorArgumentInputType = actorArgument as GraphQLUnmodifiedType, + return typeValidation.isAssignableTo( + lhs = hydrationSource as GraphQLNamedType, + rhs = actorArgument as GraphQLNamedType, ) } else { return false } } - - private fun isOutputTypeNameValid( - hydrationSourceType: GraphQLUnmodifiedType, - actorArgumentInputType: GraphQLUnmodifiedType, - ): Boolean { - if (NadelSchemaUtil.getUnderlyingName(hydrationSourceType) == NadelSchemaUtil.getUnderlyingName(actorArgumentInputType)) { - return true - } - if (actorArgumentInputType.name == Scalars.GraphQLID.name && hydrationSourceType.name == Scalars.GraphQLString.name) { - return true - } - if (actorArgumentInputType is GraphQLInterfaceType && hydrationSourceType is GraphQLImplementingType) { - return hydrationSourceType.interfaces.contains(actorArgumentInputType) - } - return false - } } From 2a267f040a04efa8f092af4f4f3b24e8a59c24e2 Mon Sep 17 00:00:00 2001 From: faizan-siddiqui Date: Tue, 29 Mar 2022 00:27:53 +1100 Subject: [PATCH 08/17] use isAssignableTo --- .../java/graphql/nadel/validation/NadelHydrationValidation.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt index 8d2b62f8e..62ef75807 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt @@ -263,8 +263,8 @@ internal class NadelHydrationValidation( if (hydrationSource.isNotWrapped && actorArgument.isNotWrapped) { return typeValidation.isAssignableTo( - lhs = hydrationSource as GraphQLNamedType, - rhs = actorArgument as GraphQLNamedType, + lhs = actorArgument as GraphQLNamedType, + rhs = hydrationSource as GraphQLNamedType, ) } else { return false From 33b5f240cca8f42ab8210f21952765205e39cf5a Mon Sep 17 00:00:00 2001 From: faizan-siddiqui Date: Fri, 1 Apr 2022 02:26:47 +1100 Subject: [PATCH 09/17] allow for batch hydration arguments --- .../validation/NadelHydrationValidation.kt | 153 +++++++++++++++--- .../NadelHydrationValidationTest.kt | 133 ++++++++++++++- 2 files changed, 259 insertions(+), 27 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt index 62ef75807..d4f190e03 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt @@ -31,10 +31,13 @@ import graphql.nadel.validation.util.NadelSchemaUtil.hasRename import graphql.schema.GraphQLFieldDefinition import graphql.schema.GraphQLFieldsContainer import graphql.schema.GraphQLImplementingType +import graphql.schema.GraphQLInputObjectType import graphql.schema.GraphQLInputType import graphql.schema.GraphQLInterfaceType import graphql.schema.GraphQLNamedOutputType import graphql.schema.GraphQLNamedType +import graphql.schema.GraphQLObjectType +import graphql.schema.GraphQLScalarType import graphql.schema.GraphQLSchema import graphql.schema.GraphQLType import graphql.schema.GraphQLUnionType @@ -203,7 +206,7 @@ internal class NadelHydrationValidation( overallField: GraphQLFieldDefinition, remoteArg: RemoteArgumentDefinition, actorField: GraphQLFieldDefinition, - ): NadelSchemaValidationError? { + ): List { val remoteArgSource = remoteArg.remoteArgumentSource val actorArg = actorField.getArgument(remoteArg.name) val actorArgInputType = actorArg.type @@ -211,31 +214,27 @@ internal class NadelHydrationValidation( ObjectField -> { val field = (parent.underlying as GraphQLFieldsContainer).getFieldAt(remoteArgSource.pathToField!!) if (field == null) { - MissingHydrationFieldValueSource(parent, overallField, remoteArgSource) + listOf( + MissingHydrationFieldValueSource(parent, overallField, remoteArgSource) + ) } else { //check the input types match with hydration and actor fields val fieldOutputType = field.type - if (!hydrationArgTypesMatch(fieldOutputType, actorArgInputType)) { - return IncompatibleHydrationArgumentType(parent, overallField, remoteArg, fieldOutputType.toString(), actorArgInputType.toString()) - } - null + return hydrationArgTypesMatch(parent, overallField, remoteArg, fieldOutputType, actorArgInputType) } } FieldArgument -> { val argument = overallField.getArgument(remoteArgSource.argumentName!!) if (argument == null) { - MissingHydrationArgumentValueSource(parent, overallField, remoteArgSource) + listOf(MissingHydrationArgumentValueSource(parent, overallField, remoteArgSource)) } else { //check the input types match with hydration and actor fields val hydrationArgType = argument.type - if (!hydrationArgTypesMatch(hydrationArgType, actorArgInputType)) { - return IncompatibleHydrationArgumentType(parent, overallField, remoteArg, hydrationArgType.toString(), actorArgInputType.toString()) - } - null + return hydrationArgTypesMatch(parent, overallField, remoteArg, hydrationArgType, actorArgInputType) } } else -> { - null + emptyList() } } } @@ -243,31 +242,147 @@ internal class NadelHydrationValidation( /* * Checks the type of a hydration argument derived from either a $source field output type or $argument field input * argument type against the type of an actor field argument to see if they match + * will also allow for cases of batched hydration eg ID! -> [ID] * + * use cases: ID! -> [ID] ok and done + * ID! -> ID ok and done + * ID -> [ID] ok and done + * ID! -> [ID!] ok and done + * object to input object ok and not done */ private fun hydrationArgTypesMatch( + parent: NadelServiceSchemaElement, + overallField: GraphQLFieldDefinition, + remoteArg: RemoteArgumentDefinition, hydrationSourceType: GraphQLType, actorArgumentInputType: GraphQLType, - ): Boolean { + ): List { + + val incompatibleHydrationArgError = listOf( + IncompatibleHydrationArgumentType( + parent, + overallField, + remoteArg, + hydrationSourceType.toString(), + actorArgumentInputType.toString() + ) + ) + var hydrationSource: GraphQLType = hydrationSourceType var actorArgument: GraphQLType = actorArgumentInputType while (hydrationSource.isWrapped && actorArgument.isWrapped) { - if ((hydrationSource.isList && actorArgument.isList) || (hydrationSource.isNonNull && actorArgument.isNonNull)) { + if (hydrationSource.isNonNull && !actorArgument.isNonNull) { + //the source can be stricter than the actor input argument + hydrationSource = hydrationSource.unwrapOne() + } else if ((hydrationSource.isList && actorArgument.isList) || (hydrationSource.isNonNull && actorArgument.isNonNull)) { hydrationSource = hydrationSource.unwrapOne() actorArgument = actorArgument.unwrapOne() + } else if (!hydrationSource.isList && actorArgument.isList) { + //could be a batch hydration eg ID -> [ID] + actorArgument = actorArgument.unwrapOne() + } else if (hydrationSource.isList && !actorArgument.isList) { + //could be a batch hydration with list input eg [ID] -> ID + hydrationSource = hydrationSource.unwrapOne() } else { - return false + return incompatibleHydrationArgError } } if (hydrationSource.isNotWrapped && actorArgument.isNotWrapped) { - return typeValidation.isAssignableTo( - lhs = actorArgument as GraphQLNamedType, - rhs = hydrationSource as GraphQLNamedType, + return checkScalarOrObjectTypeAssignability( + parent, + overallField, + remoteArg, + hydrationSource as GraphQLNamedType, + actorArgument as GraphQLNamedType, + ) + } else if (hydrationSource.isWrapped && actorArgument.isNotWrapped) { + //actor argument is more strict than the hydration source - this is ok + if (hydrationSource.isNonNull && hydrationSource.unwrapNonNull().isNotWrapped) { + return checkScalarOrObjectTypeAssignability( + parent, + overallField, + remoteArg, + hydrationSource.unwrapNonNull() as GraphQLNamedType, + actorArgument as GraphQLNamedType, + ) + } else if (hydrationSource.isList) { + // this is a case of batch hydration with list input + if (hydrationSource.unwrapOne().isNotWrapped) { + return checkScalarOrObjectTypeAssignability( + parent, + overallField, + remoteArg, + hydrationSource.unwrapOne() as GraphQLNamedType, + actorArgument as GraphQLNamedType, + ) + } else { + return incompatibleHydrationArgError + } + } else if (hydrationSource.isNotWrapped && actorArgument.isList) { + // this is a case of batch hydration + if (actorArgument.unwrapOne().isNotWrapped) { + return checkScalarOrObjectTypeAssignability( + parent, + overallField, + remoteArg, + hydrationSource as GraphQLNamedType, + actorArgument.unwrapOne() as GraphQLNamedType, + ) + } else { + return incompatibleHydrationArgError + } + } else { + return incompatibleHydrationArgError + } + } + } + + /* + * Checks assignability of a hydration type against actor input argument type to see if either the scalars types + * match or if the object type and input object type have the same fields + * + * use cases: maybe renames? + */ + private fun checkScalarOrObjectTypeAssignability( + parent: NadelServiceSchemaElement, + overallField: GraphQLFieldDefinition, + remoteArg: RemoteArgumentDefinition, + hydrationSourceType: GraphQLType, + actorArgumentInputType: GraphQLType, + ): List { + + val incompatibleHydrationArgError = listOf( + IncompatibleHydrationArgumentType( + parent, + overallField, + remoteArg, + hydrationSourceType.toString(), + actorArgumentInputType.toString() ) + ) + + if (hydrationSourceType is GraphQLScalarType && actorArgumentInputType is GraphQLScalarType) { + if (!typeValidation.isAssignableTo( + lhs = actorArgumentInputType, + rhs = hydrationSourceType, + )) { + return incompatibleHydrationArgError + } else { + return emptyList() + } + } else if (hydrationSourceType is GraphQLObjectType && actorArgumentInputType is GraphQLInputObjectType) { + //todo check fields + actorArgumentInputType.fields.map { field -> + } + return emptyList() + } else if (hydrationSourceType is GraphQLInputObjectType && actorArgumentInputType is GraphQLInputObjectType) { + //todo check fields + return emptyList() } else { - return false + return incompatibleHydrationArgError } } + } diff --git a/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt b/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt index 69ffb2c21..0051a0a4a 100644 --- a/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt +++ b/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt @@ -1208,8 +1208,8 @@ class NadelHydrationValidationTest : DescribeSpec({ it("fails if hydration argument source type is mismatch with actor field input arguments") { val fixture = NadelValidationTestFixture( - overallSchema = mapOf( - "issues" to """ + overallSchema = mapOf( + "issues" to """ type Query { issue: JiraIssue } @@ -1224,7 +1224,7 @@ class NadelHydrationValidationTest : DescribeSpec({ ) } """.trimIndent(), - "users" to """ + "users" to """ type Query { user(id: Int!): User } @@ -1233,9 +1233,9 @@ class NadelHydrationValidationTest : DescribeSpec({ name: String! } """.trimIndent(), - ), - underlyingSchema = mapOf( - "issues" to """ + ), + underlyingSchema = mapOf( + "issues" to """ type Query { issue: Issue } @@ -1244,7 +1244,7 @@ class NadelHydrationValidationTest : DescribeSpec({ creator: ID! } """.trimIndent(), - "users" to """ + "users" to """ type Query { user(id: Int!): User } @@ -1253,7 +1253,7 @@ class NadelHydrationValidationTest : DescribeSpec({ name: String! } """.trimIndent(), - ), + ), ) val errors = validate(fixture) @@ -1325,5 +1325,122 @@ class NadelHydrationValidationTest : DescribeSpec({ assert(error.hydrationType == "ID!") assert(error.actorArgInputType == "Int!") } + + + it("passes if hydration argument source types are matching with batch hydration") { + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: JiraIssue + } + type JiraIssue @renamed(from: "Issue") { + id: ID! + creators: [User] @hydrated( + service: "users" + field: "usersById" + arguments: [ + {name: "id", value: "$source.creators.id"} + ] + identifiedBy: "id" + ) + } + """.trimIndent(), + "users" to """ + type Query { + usersById(id: [ID]): [User] + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + creators: [CreatorRef] + } + type CreatorRef { + id: ID! + } + """.trimIndent(), + "users" to """ + type Query { + usersById(id: [ID]): [User] + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.isEmpty()) + + } + + it("passes if hydration argument source types are matching with batch hydration with list input") { + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: JiraIssue + } + type JiraIssue @renamed(from: "Issue") { + id: ID! + creators: [User] @hydrated( + service: "users" + field: "userById" + arguments: [ + {name: "id", value: "$source.creators"} + ] + identifiedBy: "id" + ) + } + """.trimIndent(), + "users" to """ + type Query { + userById(id: ID): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + creators: [ID] + } + """.trimIndent(), + "users" to """ + type Query { + userById(id: ID): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.isEmpty()) + + } } + }) From df16b5000f803776fe2d035045340e956417e3a6 Mon Sep 17 00:00:00 2001 From: faizan-siddiqui Date: Fri, 1 Apr 2022 02:27:39 +1100 Subject: [PATCH 10/17] allow for batch hydration arguments --- .../java/graphql/nadel/validation/NadelHydrationValidation.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt index d4f190e03..558f30bf8 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt @@ -374,8 +374,7 @@ internal class NadelHydrationValidation( } } else if (hydrationSourceType is GraphQLObjectType && actorArgumentInputType is GraphQLInputObjectType) { //todo check fields - actorArgumentInputType.fields.map { field -> - } + return emptyList() } else if (hydrationSourceType is GraphQLInputObjectType && actorArgumentInputType is GraphQLInputObjectType) { //todo check fields From 5ed70293042d25226d6afe0546022fcc36c55178 Mon Sep 17 00:00:00 2001 From: faizan-siddiqui Date: Fri, 1 Apr 2022 02:38:27 +1100 Subject: [PATCH 11/17] cleanup --- .../validation/NadelHydrationValidation.kt | 60 +++++++++++-------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt index 558f30bf8..5a3ef3c7f 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt @@ -258,15 +258,7 @@ internal class NadelHydrationValidation( actorArgumentInputType: GraphQLType, ): List { - val incompatibleHydrationArgError = listOf( - IncompatibleHydrationArgumentType( - parent, - overallField, - remoteArg, - hydrationSourceType.toString(), - actorArgumentInputType.toString() - ) - ) + var isIncompatible = false var hydrationSource: GraphQLType = hydrationSourceType var actorArgument: GraphQLType = actorArgumentInputType @@ -285,7 +277,7 @@ internal class NadelHydrationValidation( //could be a batch hydration with list input eg [ID] -> ID hydrationSource = hydrationSource.unwrapOne() } else { - return incompatibleHydrationArgError + isIncompatible = true } } @@ -318,7 +310,7 @@ internal class NadelHydrationValidation( actorArgument as GraphQLNamedType, ) } else { - return incompatibleHydrationArgError + isIncompatible = true } } else if (hydrationSource.isNotWrapped && actorArgument.isList) { // this is a case of batch hydration @@ -331,12 +323,25 @@ internal class NadelHydrationValidation( actorArgument.unwrapOne() as GraphQLNamedType, ) } else { - return incompatibleHydrationArgError + isIncompatible = true } } else { - return incompatibleHydrationArgError + isIncompatible = true } } + if (isIncompatible) { + return listOf( + IncompatibleHydrationArgumentType( + parent, + overallField, + remoteArg, + hydrationSourceType.toString(), + actorArgumentInputType.toString() + ) + ) + } else { + return emptyList() + } } /* @@ -353,34 +358,37 @@ internal class NadelHydrationValidation( actorArgumentInputType: GraphQLType, ): List { - val incompatibleHydrationArgError = listOf( - IncompatibleHydrationArgumentType( - parent, - overallField, - remoteArg, - hydrationSourceType.toString(), - actorArgumentInputType.toString() - ) - ) + var isIncompatible = false if (hydrationSourceType is GraphQLScalarType && actorArgumentInputType is GraphQLScalarType) { if (!typeValidation.isAssignableTo( lhs = actorArgumentInputType, rhs = hydrationSourceType, )) { - return incompatibleHydrationArgError + isIncompatible = true } else { return emptyList() } } else if (hydrationSourceType is GraphQLObjectType && actorArgumentInputType is GraphQLInputObjectType) { - //todo check fields - return emptyList() } else if (hydrationSourceType is GraphQLInputObjectType && actorArgumentInputType is GraphQLInputObjectType) { //todo check fields return emptyList() } else { - return incompatibleHydrationArgError + isIncompatible = true + } + + if (isIncompatible) { + return listOf( + IncompatibleHydrationArgumentType( + parent, + overallField, + remoteArg, + hydrationSourceType.toString(), + actorArgumentInputType.toString() + )) + } else { + return emptyList() } } From 1af9e674b13beb549d50b52b46d57a7b02e6c6e1 Mon Sep 17 00:00:00 2001 From: faizan-siddiqui Date: Fri, 1 Apr 2022 14:39:35 +1100 Subject: [PATCH 12/17] fix list bug --- .../graphql/nadel/validation/NadelHydrationValidation.kt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt index 5a3ef3c7f..b27f3b3c4 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt @@ -168,17 +168,18 @@ internal class NadelHydrationValidation( DuplicatedHydrationArgument(parent, overallField, it) } - val remoteArgErrors = hydration.arguments.mapNotNull { remoteArg -> + val remoteArgErrors = mutableListOf() + hydration.arguments.mapNotNull { remoteArg -> val actorFieldArgument = actorField.getArgument(remoteArg.name) if (actorFieldArgument == null) { - NonExistentHydrationActorFieldArgument( + remoteArgErrors.add(NonExistentHydrationActorFieldArgument( parent, overallField, hydration, argument = remoteArg.name, - ) + )) } else { - getRemoteArgErrors(parent, overallField, remoteArg, actorField) + remoteArgErrors.addAll(getRemoteArgErrors(parent, overallField, remoteArg, actorField)) } } From 146ed27de7228eb3a626abd5d8659e058b011b43 Mon Sep 17 00:00:00 2001 From: faizan-siddiqui Date: Mon, 4 Apr 2022 12:30:39 +1000 Subject: [PATCH 13/17] use boolean flag --- .../validation/NadelHydrationValidation.kt | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt index b27f3b3c4..3646e85c7 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt @@ -289,6 +289,8 @@ internal class NadelHydrationValidation( remoteArg, hydrationSource as GraphQLNamedType, actorArgument as GraphQLNamedType, + hydrationSourceType, + actorArgumentInputType, ) } else if (hydrationSource.isWrapped && actorArgument.isNotWrapped) { //actor argument is more strict than the hydration source - this is ok @@ -299,6 +301,8 @@ internal class NadelHydrationValidation( remoteArg, hydrationSource.unwrapNonNull() as GraphQLNamedType, actorArgument as GraphQLNamedType, + hydrationSourceType, + actorArgumentInputType, ) } else if (hydrationSource.isList) { // this is a case of batch hydration with list input @@ -309,6 +313,8 @@ internal class NadelHydrationValidation( remoteArg, hydrationSource.unwrapOne() as GraphQLNamedType, actorArgument as GraphQLNamedType, + hydrationSourceType, + actorArgumentInputType, ) } else { isIncompatible = true @@ -322,6 +328,8 @@ internal class NadelHydrationValidation( remoteArg, hydrationSource as GraphQLNamedType, actorArgument.unwrapOne() as GraphQLNamedType, + hydrationSourceType, + actorArgumentInputType, ) } else { isIncompatible = true @@ -357,20 +365,19 @@ internal class NadelHydrationValidation( remoteArg: RemoteArgumentDefinition, hydrationSourceType: GraphQLType, actorArgumentInputType: GraphQLType, + originalHydrationSourceType: GraphQLType, + originalActorArgumentInputType: GraphQLType, ): List { - var isIncompatible = false + val isIncompatible: Boolean if (hydrationSourceType is GraphQLScalarType && actorArgumentInputType is GraphQLScalarType) { - if (!typeValidation.isAssignableTo( + isIncompatible = !typeValidation.isAssignableTo( lhs = actorArgumentInputType, rhs = hydrationSourceType, - )) { - isIncompatible = true - } else { - return emptyList() - } + ) } else if (hydrationSourceType is GraphQLObjectType && actorArgumentInputType is GraphQLInputObjectType) { + //todo check fields return emptyList() } else if (hydrationSourceType is GraphQLInputObjectType && actorArgumentInputType is GraphQLInputObjectType) { //todo check fields @@ -385,8 +392,8 @@ internal class NadelHydrationValidation( parent, overallField, remoteArg, - hydrationSourceType.toString(), - actorArgumentInputType.toString() + originalHydrationSourceType.toString(), + originalActorArgumentInputType.toString() )) } else { return emptyList() From 01cd943eb944cb58a98ea984d0fd7da78c0a3aba Mon Sep 17 00:00:00 2001 From: faizan-siddiqui Date: Thu, 7 Apr 2022 06:20:02 +1000 Subject: [PATCH 14/17] add input object type check --- .../NadelHydrationArgumentValidation.kt | 346 ++++++ .../validation/NadelHydrationValidation.kt | 256 +---- .../validation/NadelSchemaValidationError.kt | 65 +- .../NadelHydrationArgumentValidationTest.kt | 1004 +++++++++++++++++ .../NadelHydrationValidationTest.kt | 610 ---------- 5 files changed, 1408 insertions(+), 873 deletions(-) create mode 100644 lib/src/main/java/graphql/nadel/validation/NadelHydrationArgumentValidation.kt create mode 100644 lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationArgumentValidationTest.kt diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationArgumentValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationArgumentValidation.kt new file mode 100644 index 000000000..081b6e8cf --- /dev/null +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationArgumentValidation.kt @@ -0,0 +1,346 @@ +package graphql.nadel.validation + +import graphql.nadel.dsl.RemoteArgumentDefinition +import graphql.nadel.dsl.RemoteArgumentSource.SourceType.FieldArgument +import graphql.nadel.dsl.RemoteArgumentSource.SourceType.ObjectField +import graphql.nadel.dsl.UnderlyingServiceHydration +import graphql.nadel.engine.util.getFieldAt +import graphql.nadel.engine.util.isList +import graphql.nadel.engine.util.isNonNull +import graphql.nadel.engine.util.isNotWrapped +import graphql.nadel.engine.util.isWrapped +import graphql.nadel.engine.util.unwrapNonNull +import graphql.nadel.engine.util.unwrapOne +import graphql.nadel.validation.NadelSchemaValidationError.DuplicatedHydrationArgument +import graphql.nadel.validation.NadelSchemaValidationError.IncompatibleHydrationArgumentType +import graphql.nadel.validation.NadelSchemaValidationError.IncompatibleHydrationInputObjectArgumentType +import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationArgumentValueSource +import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationFieldValueSource +import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationInputObjectArgumentValueSource +import graphql.nadel.validation.NadelSchemaValidationError.MissingRequiredHydrationActorFieldArgument +import graphql.nadel.validation.NadelSchemaValidationError.NonExistentHydrationActorFieldArgument +import graphql.schema.GraphQLFieldDefinition +import graphql.schema.GraphQLFieldsContainer +import graphql.schema.GraphQLInputObjectType +import graphql.schema.GraphQLNamedType +import graphql.schema.GraphQLObjectType +import graphql.schema.GraphQLScalarType +import graphql.schema.GraphQLType + +internal class NadelHydrationArgumentValidation( + private val typeValidation: NadelTypeValidation, +) { + fun validate( + parent: NadelServiceSchemaElement, + overallField: GraphQLFieldDefinition, + hydration: UnderlyingServiceHydration, + actorField: GraphQLFieldDefinition, + ): List { + // Can only provide one value for an argument + val duplicatedArgumentsErrors = hydration.arguments + .groupBy { it.name } + .filterValues { it.size > 1 } + .values + .map { + DuplicatedHydrationArgument(parent, overallField, it) + } + + val remoteArgErrors = mutableListOf() + hydration.arguments.mapNotNull { remoteArg -> + val actorFieldArgument = actorField.getArgument(remoteArg.name) + if (actorFieldArgument == null) { + remoteArgErrors.add(NonExistentHydrationActorFieldArgument( + parent, + overallField, + hydration, + argument = remoteArg.name, + )) + } else { + remoteArgErrors.addAll(getRemoteArgErrors(parent, overallField, remoteArg, actorField)) + } + } + + val missingActorArgErrors = actorField.arguments + .filter { it.type.isNonNull } + .mapNotNull { actorArg -> + val hydrationArg = hydration.arguments.find { it.name == actorArg.name } + if (hydrationArg == null) { + MissingRequiredHydrationActorFieldArgument( + parent, + overallField, + hydration, + argument = actorArg.name, + ) + } else { + null + } + } + + return duplicatedArgumentsErrors + remoteArgErrors + missingActorArgErrors + } + + private fun getRemoteArgErrors( + parent: NadelServiceSchemaElement, + overallField: GraphQLFieldDefinition, + remoteArg: RemoteArgumentDefinition, + actorField: GraphQLFieldDefinition, + ): List { + val remoteArgSource = remoteArg.remoteArgumentSource + val actorArg = actorField.getArgument(remoteArg.name) + val actorArgInputType = actorArg.type + return when (remoteArgSource.sourceType) { + ObjectField -> { + val field = (parent.underlying as GraphQLFieldsContainer).getFieldAt(remoteArgSource.pathToField!!) + if (field == null) { + listOf( + MissingHydrationFieldValueSource(parent, overallField, remoteArgSource) + ) + } else { + //check the input types match with hydration and actor fields + val fieldOutputType = field.type + return hydrationArgTypesMatch(parent, overallField, remoteArg, fieldOutputType, actorArgInputType) + } + } + FieldArgument -> { + val argument = overallField.getArgument(remoteArgSource.argumentName!!) + if (argument == null) { + listOf(MissingHydrationArgumentValueSource(parent, overallField, remoteArgSource)) + } else { + //check the input types match with hydration and actor fields + val hydrationArgType = argument.type + return hydrationArgTypesMatch(parent, overallField, remoteArg, hydrationArgType, actorArgInputType) + } + } + else -> { + emptyList() + } + } + } + + /* + * Checks the type of a hydration argument derived from either a $source field output type or $argument field input + * argument type against the type of an actor field argument to see if they match + * will also allow for cases of batched hydration eg ID! -> [ID] + * + * use cases: ID! -> [ID] ok and done + * ID! -> ID ok and done + * ID -> [ID] ok and done + * ID! -> [ID!] ok and done + * object to input object ok and not done + */ + private fun hydrationArgTypesMatch( + parent: NadelServiceSchemaElement, + overallField: GraphQLFieldDefinition, + remoteArg: RemoteArgumentDefinition, + hydrationSourceType: GraphQLType, + actorArgumentInputType: GraphQLType, + ): List { + + var isIncompatible = false + + var hydrationSource: GraphQLType = hydrationSourceType + var actorArgument: GraphQLType = actorArgumentInputType + + while (hydrationSource.isWrapped && actorArgument.isWrapped) { + if (hydrationSource.isNonNull && !actorArgument.isNonNull) { + //the source can be stricter than the actor input argument + hydrationSource = hydrationSource.unwrapOne() + } else if ((hydrationSource.isList && actorArgument.isList) || (hydrationSource.isNonNull && actorArgument.isNonNull)) { + hydrationSource = hydrationSource.unwrapOne() + actorArgument = actorArgument.unwrapOne() + } else if (!hydrationSource.isList && actorArgument.isList) { + //could be a batch hydration eg ID -> [ID] + actorArgument = actorArgument.unwrapOne() + } else if (hydrationSource.isList && !actorArgument.isList) { + //could be a batch hydration with list input eg [ID] -> ID + hydrationSource = hydrationSource.unwrapOne() + } else { + isIncompatible = true + } + } + + if (hydrationSource.isNotWrapped && actorArgument.isNotWrapped) { + return checkScalarOrObjectTypeAssignability( + parent, + overallField, + remoteArg, + hydrationSource as GraphQLNamedType, + actorArgument as GraphQLNamedType, + hydrationSourceType, + actorArgumentInputType, + ) + } else if (hydrationSource.isWrapped && actorArgument.isNotWrapped) { + //actor argument is more strict than the hydration source - this is ok + if (hydrationSource.isNonNull && hydrationSource.unwrapNonNull().isNotWrapped) { + return checkScalarOrObjectTypeAssignability( + parent, + overallField, + remoteArg, + hydrationSource.unwrapNonNull() as GraphQLNamedType, + actorArgument as GraphQLNamedType, + hydrationSourceType, + actorArgumentInputType, + ) + } else if (hydrationSource.isList) { + // this is a case of batch hydration with list input + if (hydrationSource.unwrapOne().isNotWrapped) { + return checkScalarOrObjectTypeAssignability( + parent, + overallField, + remoteArg, + hydrationSource.unwrapOne() as GraphQLNamedType, + actorArgument as GraphQLNamedType, + hydrationSourceType, + actorArgumentInputType, + ) + } else { + isIncompatible = true + } + } else if (hydrationSource.isNotWrapped && actorArgument.isList) { + // this is a case of batch hydration + if (actorArgument.unwrapOne().isNotWrapped) { + return checkScalarOrObjectTypeAssignability( + parent, + overallField, + remoteArg, + hydrationSource as GraphQLNamedType, + actorArgument.unwrapOne() as GraphQLNamedType, + hydrationSourceType, + actorArgumentInputType, + ) + } else { + isIncompatible = true + } + } else { + isIncompatible = true + } + } + if (isIncompatible) { + return listOf( + IncompatibleHydrationArgumentType( + parent, + overallField, + remoteArg, + hydrationSourceType, + actorArgumentInputType, + ) + ) + } else { + return emptyList() + } + } + + /* + * Checks assignability of a hydration type against actor input argument type to see if either the scalars types + * match or if the object type and input object type have the same fields + * + * use cases: maybe renames? + */ + private fun checkScalarOrObjectTypeAssignability( + parent: NadelServiceSchemaElement, + overallField: GraphQLFieldDefinition, + remoteArg: RemoteArgumentDefinition, + hydrationSourceType: GraphQLType, + actorArgumentInputType: GraphQLType, + originalHydrationSourceType: GraphQLType, + originalActorArgumentInputType: GraphQLType, + ): List { + + val isIncompatible: Boolean + + if (hydrationSourceType is GraphQLScalarType && actorArgumentInputType is GraphQLScalarType) { + isIncompatible = !typeValidation.isAssignableTo( + lhs = actorArgumentInputType, + rhs = hydrationSourceType, + ) + } else if (hydrationSourceType is GraphQLObjectType && actorArgumentInputType is GraphQLInputObjectType) { + val hydrationFields = hydrationSourceType.fields.associate { it.name to it.type as GraphQLType } + val actorFields = actorArgumentInputType.fields.associate { it.name to it.type as GraphQLType } + return checkInputObjectType( + parent, + overallField, + remoteArg, + hydrationFields, + actorFields, + ) + } else if (hydrationSourceType is GraphQLInputObjectType && actorArgumentInputType is GraphQLInputObjectType) { + val hydrationFields = hydrationSourceType.fields.associate { it.name to it.type as GraphQLType } + val actorFields = actorArgumentInputType.fields.associate { it.name to it.type as GraphQLType } + return checkInputObjectType( + parent, + overallField, + remoteArg, + hydrationFields, + actorFields, + ) + } else { + isIncompatible = true + } + + if (isIncompatible) { + return listOf( + IncompatibleHydrationArgumentType( + parent, + overallField, + remoteArg, + originalHydrationSourceType, + originalActorArgumentInputType, + )) + } else { + return emptyList() + } + } + + private fun checkInputObjectType( + parent: NadelServiceSchemaElement, + overallField: GraphQLFieldDefinition, + remoteArg: RemoteArgumentDefinition, + hydrationFields: Map, + actorInputFields: Map, + ): List { + val errors = mutableListOf() + actorInputFields.forEach { actorFieldName, actorFieldType -> + val hydrationType = hydrationFields.get(actorFieldName) + if (hydrationType != null) { + if (!typesAreSame(hydrationType, actorFieldType)) { + errors.add(IncompatibleHydrationInputObjectArgumentType( + parent, + overallField, + remoteArg, + actorFieldName, + actorFieldType, + hydrationType, + )) + } + } else { + errors.add(MissingHydrationInputObjectArgumentValueSource( + parent, + overallField, + remoteArg, + actorFieldName, + )) + } + } + return errors + } + + private fun typesAreSame(hydrationType: GraphQLType, actorFieldType: GraphQLType): Boolean { + var hydrationSource: GraphQLType = hydrationType + var actorArgument: GraphQLType = actorFieldType + + while (hydrationSource.isWrapped && actorArgument.isWrapped) { + if ((hydrationSource.isList && actorArgument.isList) || (hydrationSource.isNonNull && actorArgument.isNonNull)) { + hydrationSource = hydrationSource.unwrapOne() + actorArgument = actorArgument.unwrapOne() + } else { + return false + } + } + + if (hydrationSource.isNotWrapped && actorArgument.isNotWrapped) { + return typeValidation.isAssignableTo(lhs = actorArgument as GraphQLNamedType, + rhs = hydrationSource as GraphQLNamedType) + } + return false + } +} diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt index 3646e85c7..ded2ebde0 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt @@ -1,6 +1,5 @@ package graphql.nadel.validation -import graphql.Scalars import graphql.nadel.Service import graphql.nadel.dsl.RemoteArgumentDefinition import graphql.nadel.dsl.RemoteArgumentSource.SourceType.FieldArgument @@ -25,14 +24,11 @@ import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationArgum import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationFieldValueSource import graphql.nadel.validation.NadelSchemaValidationError.MissingRequiredHydrationActorFieldArgument import graphql.nadel.validation.NadelSchemaValidationError.NonExistentHydrationActorFieldArgument -import graphql.nadel.validation.util.NadelSchemaUtil import graphql.nadel.validation.util.NadelSchemaUtil.getHydrations import graphql.nadel.validation.util.NadelSchemaUtil.hasRename import graphql.schema.GraphQLFieldDefinition import graphql.schema.GraphQLFieldsContainer -import graphql.schema.GraphQLImplementingType import graphql.schema.GraphQLInputObjectType -import graphql.schema.GraphQLInputType import graphql.schema.GraphQLInterfaceType import graphql.schema.GraphQLNamedOutputType import graphql.schema.GraphQLNamedType @@ -41,13 +37,14 @@ import graphql.schema.GraphQLScalarType import graphql.schema.GraphQLSchema import graphql.schema.GraphQLType import graphql.schema.GraphQLUnionType -import graphql.schema.GraphQLUnmodifiedType internal class NadelHydrationValidation( private val services: Map, private val typeValidation: NadelTypeValidation, private val overallSchema: GraphQLSchema, ) { + private val hydrationArgumentValidation = NadelHydrationArgumentValidation(typeValidation) + fun validate( parent: NadelServiceSchemaElement, overallField: GraphQLFieldDefinition, @@ -78,7 +75,7 @@ internal class NadelHydrationValidation( continue } - val argumentIssues = getArgumentErrors(parent, overallField, hydration, actorField) + val argumentIssues = hydrationArgumentValidation.validate(parent, overallField, hydration, actorField) val outputTypeIssues = getOutputTypeIssues(parent, overallField, actorField, hasMoreThanOneHydration) errors.addAll(argumentIssues) errors.addAll(outputTypeIssues) @@ -153,251 +150,4 @@ internal class NadelHydrationValidation( return typeValidation + outputTypeMustBeNullable } - private fun getArgumentErrors( - parent: NadelServiceSchemaElement, - overallField: GraphQLFieldDefinition, - hydration: UnderlyingServiceHydration, - actorField: GraphQLFieldDefinition, - ): List { - // Can only provide one value for an argument - val duplicatedArgumentsErrors = hydration.arguments - .groupBy { it.name } - .filterValues { it.size > 1 } - .values - .map { - DuplicatedHydrationArgument(parent, overallField, it) - } - - val remoteArgErrors = mutableListOf() - hydration.arguments.mapNotNull { remoteArg -> - val actorFieldArgument = actorField.getArgument(remoteArg.name) - if (actorFieldArgument == null) { - remoteArgErrors.add(NonExistentHydrationActorFieldArgument( - parent, - overallField, - hydration, - argument = remoteArg.name, - )) - } else { - remoteArgErrors.addAll(getRemoteArgErrors(parent, overallField, remoteArg, actorField)) - } - } - - val missingActorArgErrors = actorField.arguments - .filter { it.type.isNonNull } - .mapNotNull { actorArg -> - val hydrationArg = hydration.arguments.find { it.name == actorArg.name } - if (hydrationArg == null) { - MissingRequiredHydrationActorFieldArgument( - parent, - overallField, - hydration, - argument = actorArg.name, - ) - } else { - null - } - } - - return duplicatedArgumentsErrors + remoteArgErrors + missingActorArgErrors - } - - private fun getRemoteArgErrors( - parent: NadelServiceSchemaElement, - overallField: GraphQLFieldDefinition, - remoteArg: RemoteArgumentDefinition, - actorField: GraphQLFieldDefinition, - ): List { - val remoteArgSource = remoteArg.remoteArgumentSource - val actorArg = actorField.getArgument(remoteArg.name) - val actorArgInputType = actorArg.type - return when (remoteArgSource.sourceType) { - ObjectField -> { - val field = (parent.underlying as GraphQLFieldsContainer).getFieldAt(remoteArgSource.pathToField!!) - if (field == null) { - listOf( - MissingHydrationFieldValueSource(parent, overallField, remoteArgSource) - ) - } else { - //check the input types match with hydration and actor fields - val fieldOutputType = field.type - return hydrationArgTypesMatch(parent, overallField, remoteArg, fieldOutputType, actorArgInputType) - } - } - FieldArgument -> { - val argument = overallField.getArgument(remoteArgSource.argumentName!!) - if (argument == null) { - listOf(MissingHydrationArgumentValueSource(parent, overallField, remoteArgSource)) - } else { - //check the input types match with hydration and actor fields - val hydrationArgType = argument.type - return hydrationArgTypesMatch(parent, overallField, remoteArg, hydrationArgType, actorArgInputType) - } - } - else -> { - emptyList() - } - } - } - - /* - * Checks the type of a hydration argument derived from either a $source field output type or $argument field input - * argument type against the type of an actor field argument to see if they match - * will also allow for cases of batched hydration eg ID! -> [ID] - * - * use cases: ID! -> [ID] ok and done - * ID! -> ID ok and done - * ID -> [ID] ok and done - * ID! -> [ID!] ok and done - * object to input object ok and not done - */ - private fun hydrationArgTypesMatch( - parent: NadelServiceSchemaElement, - overallField: GraphQLFieldDefinition, - remoteArg: RemoteArgumentDefinition, - hydrationSourceType: GraphQLType, - actorArgumentInputType: GraphQLType, - ): List { - - var isIncompatible = false - - var hydrationSource: GraphQLType = hydrationSourceType - var actorArgument: GraphQLType = actorArgumentInputType - - while (hydrationSource.isWrapped && actorArgument.isWrapped) { - if (hydrationSource.isNonNull && !actorArgument.isNonNull) { - //the source can be stricter than the actor input argument - hydrationSource = hydrationSource.unwrapOne() - } else if ((hydrationSource.isList && actorArgument.isList) || (hydrationSource.isNonNull && actorArgument.isNonNull)) { - hydrationSource = hydrationSource.unwrapOne() - actorArgument = actorArgument.unwrapOne() - } else if (!hydrationSource.isList && actorArgument.isList) { - //could be a batch hydration eg ID -> [ID] - actorArgument = actorArgument.unwrapOne() - } else if (hydrationSource.isList && !actorArgument.isList) { - //could be a batch hydration with list input eg [ID] -> ID - hydrationSource = hydrationSource.unwrapOne() - } else { - isIncompatible = true - } - } - - if (hydrationSource.isNotWrapped && actorArgument.isNotWrapped) { - return checkScalarOrObjectTypeAssignability( - parent, - overallField, - remoteArg, - hydrationSource as GraphQLNamedType, - actorArgument as GraphQLNamedType, - hydrationSourceType, - actorArgumentInputType, - ) - } else if (hydrationSource.isWrapped && actorArgument.isNotWrapped) { - //actor argument is more strict than the hydration source - this is ok - if (hydrationSource.isNonNull && hydrationSource.unwrapNonNull().isNotWrapped) { - return checkScalarOrObjectTypeAssignability( - parent, - overallField, - remoteArg, - hydrationSource.unwrapNonNull() as GraphQLNamedType, - actorArgument as GraphQLNamedType, - hydrationSourceType, - actorArgumentInputType, - ) - } else if (hydrationSource.isList) { - // this is a case of batch hydration with list input - if (hydrationSource.unwrapOne().isNotWrapped) { - return checkScalarOrObjectTypeAssignability( - parent, - overallField, - remoteArg, - hydrationSource.unwrapOne() as GraphQLNamedType, - actorArgument as GraphQLNamedType, - hydrationSourceType, - actorArgumentInputType, - ) - } else { - isIncompatible = true - } - } else if (hydrationSource.isNotWrapped && actorArgument.isList) { - // this is a case of batch hydration - if (actorArgument.unwrapOne().isNotWrapped) { - return checkScalarOrObjectTypeAssignability( - parent, - overallField, - remoteArg, - hydrationSource as GraphQLNamedType, - actorArgument.unwrapOne() as GraphQLNamedType, - hydrationSourceType, - actorArgumentInputType, - ) - } else { - isIncompatible = true - } - } else { - isIncompatible = true - } - } - if (isIncompatible) { - return listOf( - IncompatibleHydrationArgumentType( - parent, - overallField, - remoteArg, - hydrationSourceType.toString(), - actorArgumentInputType.toString() - ) - ) - } else { - return emptyList() - } - } - - /* - * Checks assignability of a hydration type against actor input argument type to see if either the scalars types - * match or if the object type and input object type have the same fields - * - * use cases: maybe renames? - */ - private fun checkScalarOrObjectTypeAssignability( - parent: NadelServiceSchemaElement, - overallField: GraphQLFieldDefinition, - remoteArg: RemoteArgumentDefinition, - hydrationSourceType: GraphQLType, - actorArgumentInputType: GraphQLType, - originalHydrationSourceType: GraphQLType, - originalActorArgumentInputType: GraphQLType, - ): List { - - val isIncompatible: Boolean - - if (hydrationSourceType is GraphQLScalarType && actorArgumentInputType is GraphQLScalarType) { - isIncompatible = !typeValidation.isAssignableTo( - lhs = actorArgumentInputType, - rhs = hydrationSourceType, - ) - } else if (hydrationSourceType is GraphQLObjectType && actorArgumentInputType is GraphQLInputObjectType) { - //todo check fields - return emptyList() - } else if (hydrationSourceType is GraphQLInputObjectType && actorArgumentInputType is GraphQLInputObjectType) { - //todo check fields - return emptyList() - } else { - isIncompatible = true - } - - if (isIncompatible) { - return listOf( - IncompatibleHydrationArgumentType( - parent, - overallField, - remoteArg, - originalHydrationSourceType.toString(), - originalActorArgumentInputType.toString() - )) - } else { - return emptyList() - } - } - } diff --git a/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationError.kt b/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationError.kt index 3c67fdbb8..9986a7c02 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationError.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationError.kt @@ -16,12 +16,10 @@ import graphql.schema.GraphQLDirective import graphql.schema.GraphQLEnumValueDefinition import graphql.schema.GraphQLFieldDefinition import graphql.schema.GraphQLInputObjectField -import graphql.schema.GraphQLInputType import graphql.schema.GraphQLNamedOutputType import graphql.schema.GraphQLNamedSchemaElement import graphql.schema.GraphQLNamedType import graphql.schema.GraphQLObjectType -import graphql.schema.GraphQLOutputType import graphql.schema.GraphQLType import graphql.schema.GraphQLTypeUtil import graphql.schema.GraphQLUnionType @@ -323,22 +321,52 @@ sealed interface NadelSchemaValidationError { } data class IncompatibleHydrationArgumentType( - val parentType: NadelServiceSchemaElement, - val overallField: GraphQLFieldDefinition, - val remoteArg: RemoteArgumentDefinition, - val hydrationType: String, - val actorArgInputType: String, + val parentType: NadelServiceSchemaElement, + val overallField: GraphQLFieldDefinition, + val remoteArg: RemoteArgumentDefinition, + val hydrationType: GraphQLType, + val actorArgInputType: GraphQLType, ) : NadelSchemaValidationError { val service: Service get() = parentType.service override val message = run { val hydrationArgName = remoteArg.name val of = makeFieldCoordinates(parentType.overall.name, overallField.name) - val remoteArgSource = "${parentType.overall.name}.${remoteArg.remoteArgumentSource.pathToField?.joinToString(separator = ".")}" + val remoteArgSource = + "${parentType.overall.name}.${remoteArg.remoteArgumentSource.pathToField?.joinToString(separator = ".")}" val s = service.name + val ht = GraphQLTypeUtil.simplePrint(hydrationType) + val at = GraphQLTypeUtil.simplePrint(actorArgInputType) "Field $of tried to hydrate with argument $hydrationArgName using value from field $remoteArgSource from " + - "service $s with an argument of $hydrationType whereas the actor field requires an argument of" + - "type $actorArgInputType" + "service $s with an argument of $ht whereas the actor field requires an argument of" + + "type $at" + } + + override val subject = overallField + } + + data class IncompatibleHydrationInputObjectArgumentType( + val parentType: NadelServiceSchemaElement, + val overallField: GraphQLFieldDefinition, + val remoteArg: RemoteArgumentDefinition, + val actorFieldName: String, + val actorFieldType: GraphQLType, + val hydrationType: GraphQLType, + ) : NadelSchemaValidationError { + val service: Service get() = parentType.service + + override val message = run { + val hydrationArgName = remoteArg.name + val of = makeFieldCoordinates(parentType.overall.name, overallField.name) + val remoteArgSource = + "${parentType.overall.name}.${remoteArg.remoteArgumentSource.pathToField?.joinToString(separator = ".")}" + val s = service.name + val ht = GraphQLTypeUtil.simplePrint(hydrationType) + val at = GraphQLTypeUtil.simplePrint(actorFieldType) + + "Field $of tried to hydrate with input object field argument $hydrationArgName.$actorFieldName using value from field $remoteArgSource from " + + "service $s with an argument of $ht whereas the actor field requires an argument of" + + "type $at" } override val subject = overallField @@ -360,6 +388,23 @@ sealed interface NadelSchemaValidationError { override val subject = overallField } + data class MissingHydrationInputObjectArgumentValueSource( + val parentType: NadelServiceSchemaElement, + val overallField: GraphQLFieldDefinition, + val remoteArg: RemoteArgumentDefinition, + val actorFieldName: String, + ) : NadelSchemaValidationError { + val service: Service get() = parentType.service + + override val message = run { + val of = makeFieldCoordinates(parentType.overall.name, overallField.name) + val a = remoteArg.name + "Field $of tried to hydrate using value of non-existent field within input type $a.$actorFieldName as an argument" + } + + override val subject = overallField + } + data class NonExistentHydrationActorFieldArgument( val parentType: NadelServiceSchemaElement, val overallField: GraphQLFieldDefinition, diff --git a/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationArgumentValidationTest.kt b/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationArgumentValidationTest.kt new file mode 100644 index 000000000..78387853a --- /dev/null +++ b/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationArgumentValidationTest.kt @@ -0,0 +1,1004 @@ +package graphql.nadel.validation + +import graphql.nadel.engine.util.singleOfType +import graphql.nadel.validation.NadelSchemaValidationError.DuplicatedHydrationArgument +import graphql.nadel.validation.NadelSchemaValidationError.IncompatibleHydrationArgumentType +import graphql.nadel.validation.NadelSchemaValidationError.IncompatibleHydrationInputObjectArgumentType +import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationArgumentValueSource +import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationFieldValueSource +import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationInputObjectArgumentValueSource +import graphql.nadel.validation.NadelSchemaValidationError.MissingRequiredHydrationActorFieldArgument +import graphql.nadel.validation.NadelSchemaValidationError.NonExistentHydrationActorFieldArgument +import graphql.nadel.validation.util.assertSingleOfType +import graphql.schema.GraphQLTypeUtil +import io.kotest.core.spec.style.DescribeSpec +import io.kotest.datatest.withData + +private const val source = "$" + "source" +private const val argument = "$" + "argument" + +class NadelHydrationArgumentValidationTest : DescribeSpec({ + describe("validate") { + it("fails if hydration argument references non existent field") { + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + creator: User @hydrated( + service: "users" + field: "user" + arguments: [ + {name: "id", value: "$source.creatorId"} + ] + ) + } + """.trimIndent(), + "users" to """ + type Query { + user(id: ID!): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + creator: ID! + } + """.trimIndent(), + "users" to """ + type Query { + user(id: ID!): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.map { it.message }.isNotEmpty()) + + val error = errors.assertSingleOfType() + assert(error.parentType.overall.name == "Issue") + assert(error.parentType.underlying.name == "Issue") + assert(error.overallField.name == "creator") + assert(error.remoteArgSource.pathToField == listOf("creatorId")) + assert(error.remoteArgSource.argumentName == null) + } + + it("fails if hydration argument references non existent argument") { + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + } + """.trimIndent(), + "users" to """ + type Query { + user(id: ID!, secrets: Boolean = false): User + } + type User { + id: ID! + name: String! + } + extend type Issue { + creator(someArg: Boolean): User @hydrated( + service: "users" + field: "user" + arguments: [ + {name: "id", value: "$source.creator"} + {name: "secrets", value: "$argument.secrets"} + ] + ) + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + creator: ID! + } + """.trimIndent(), + "users" to """ + type Query { + user(id: ID!, secrets: Boolean = false): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.map { it.message }.isNotEmpty()) + + val error = errors.assertSingleOfType() + assert(error.parentType.overall.name == "Issue") + assert(error.parentType.underlying.name == "Issue") + assert(error.overallField.name == "creator") + assert(error.remoteArgSource.argumentName == "secrets") + assert(error.remoteArgSource.pathToField == null) + } + + it("fails if hydration argument references non existent remote argument") { + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + } + """.trimIndent(), + "users" to """ + type Query { + user(id: ID!): User + } + type User { + id: ID! + name: String! + } + extend type Issue { + creator(someArg: Boolean): User @hydrated( + service: "users" + field: "user" + arguments: [ + {name: "id", value: "$source.creator"} + {name: "someArg", value: "$argument.someArg"} + ] + ) + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + creator: ID! + } + """.trimIndent(), + "users" to """ + type Query { + user(id: ID!): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.map { it.message }.isNotEmpty()) + + val error = errors.assertSingleOfType() + assert(error.parentType.overall.name == "Issue") + assert(error.parentType.underlying.name == "Issue") + assert(error.overallField.name == "creator") + assert(error.argument == "someArg") + } + + it("fails if hydration defines duplicated arguments") { + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + } + """.trimIndent(), + "users" to """ + type Query { + user(id: ID!, other: Boolean): User + } + type User { + id: ID! + name: String! + } + extend type Issue { + creator(someArg: ID!, other: Boolean): User @hydrated( + service: "users" + field: "user" + arguments: [ + {name: "id", value: "$source.creator"} + {name: "id", value: "$argument.someArg"} + {name: "other", value: "$argument.other"} + ] + ) + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + creator: ID! + } + """.trimIndent(), + "users" to """ + type Query { + user(id: ID!, other: Boolean): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.map { it.message }.isNotEmpty()) + + val error = errors.assertSingleOfType() + assert(error.parentType.overall.name == "Issue") + assert(error.parentType.underlying.name == "Issue") + assert(error.overallField.name == "creator") + assert(error.duplicates.map { it.name }.toSet() == setOf("id")) + } + + it("fails if hydration field has missing non-nullable arguments with underlying top level fields") { + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + } + """.trimIndent(), + "users" to """ + type Query { + user(id: ID!, other: Boolean!): User + } + type User { + id: ID! + name: String! + } + extend type Issue { + creator(someArg: ID!, other: Boolean): User @hydrated( + service: "users" + field: "user" + arguments: [ + {name: "id", value: "$argument.creator"} + ] + ) + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + creator: ID! + } + """.trimIndent(), + "users" to """ + type Query { + user(id: ID!, other: Boolean!): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.map { it.message }.isNotEmpty()) + + val error = errors.assertSingleOfType() + assert(error.parentType.overall.name == "Issue") + assert(error.parentType.underlying.name == "Issue") + assert(error.overallField.name == "creator") + assert(error.argument == "other") + } + + it("passes if hydration field has missing nullable arguments with underlying top level fields") { + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + } + """.trimIndent(), + "users" to """ + type Query { + user(id: ID!, other: Boolean): User + } + type User { + id: ID! + name: String! + } + extend type Issue { + creator(someArg: ID!, other: Boolean): User @hydrated( + service: "users" + field: "user" + arguments: [ + {name: "id", value: "$argument.someArg"} + ] + ) + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + creator: ID! + } + """.trimIndent(), + "users" to """ + type Query { + user(id: ID!, other: Boolean): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.map { it.message }.isEmpty()) + } + + it("fails if hydration argument source type is mismatch with actor field input arguments") { + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: JiraIssue + } + type JiraIssue @renamed(from: "Issue") { + id: ID! + creator: User @hydrated( + service: "users" + field: "user" + arguments: [ + {name: "id", value: "$source.creator"} + ] + ) + } + """.trimIndent(), + "users" to """ + type Query { + user(id: Int!): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + creator: ID! + } + """.trimIndent(), + "users" to """ + type Query { + user(id: Int!): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.isNotEmpty()) + val error = errors.singleOfType() + assert(error.parentType.overall.name == "JiraIssue") + assert(error.overallField.name == "creator") + assert(error.remoteArg.name == "id") + assert(GraphQLTypeUtil.simplePrint(error.hydrationType) == "ID!") + assert(GraphQLTypeUtil.simplePrint(error.actorArgInputType) == "Int!") + } + + it("fails if hydration argument types are mismatch with actor field input arguments") { + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: JiraIssue + } + type JiraIssue @renamed(from: "Issue") { + id: ID! + creator(someArg: ID!): User @hydrated( + service: "users" + field: "user" + arguments: [ + {name: "someArg", value: "$argument.someArg"} + ] + ) + } + """.trimIndent(), + "users" to """ + type Query { + user(someArg: Int!): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + creator: ID! + } + """.trimIndent(), + "users" to """ + type Query { + user(someArg: Int!): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.isNotEmpty()) + val error = errors.singleOfType() + assert(error.parentType.overall.name == "JiraIssue") + assert(error.overallField.name == "creator") + assert(error.remoteArg.name == "someArg") + assert(GraphQLTypeUtil.simplePrint(error.hydrationType) == "ID!") + assert(GraphQLTypeUtil.simplePrint(error.actorArgInputType) == "Int!") + } + + + it("passes if hydration argument source types are matching with batch hydration") { + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: JiraIssue + } + type JiraIssue @renamed(from: "Issue") { + id: ID! + creators: [User] @hydrated( + service: "users" + field: "usersById" + arguments: [ + {name: "id", value: "$source.creators.id"} + ] + identifiedBy: "id" + ) + } + """.trimIndent(), + "users" to """ + type Query { + usersById(id: [ID]): [User] + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + creators: [CreatorRef] + } + type CreatorRef { + id: ID! + } + """.trimIndent(), + "users" to """ + type Query { + usersById(id: [ID]): [User] + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.isEmpty()) + + } + + it("passes if hydration argument source types are matching with batch hydration with list input") { + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: JiraIssue + } + type JiraIssue @renamed(from: "Issue") { + id: ID! + creators: [User] @hydrated( + service: "users" + field: "userById" + arguments: [ + {name: "id", value: "$source.creators"} + ] + identifiedBy: "id" + ) + } + """.trimIndent(), + "users" to """ + type Query { + userById(id: ID): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + creators: [ID] + } + """.trimIndent(), + "users" to """ + type Query { + userById(id: ID): User + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.isEmpty()) + + } + + it("checks if missing hydration argument source type when it is object type") { + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: JiraIssue + } + type JiraIssue @renamed(from: "Issue") { + id: ID! + creators: [User] @hydrated( + service: "users" + field: "users" + arguments: [ + {name: "id", value: "$source.creators"} + ] + inputIdentifiedBy: [ + {sourceId: "creators.userId" resultId: "id"} + {sourceId: "creators.site" resultId: "siteId"} + ] + ) + } + """.trimIndent(), + "users" to """ + type Query { + users(id: [UserInput]): [User] + } + input UserInput { + userId: ID + site: String! + } + type User { + id: ID! + name: String + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + creators: [UserRef] + } + type UserRef { + userId: ID + } + """.trimIndent(), + "users" to """ + type Query { + users(id: [UserInput]): [User] + } + input UserInput { + userId: ID + site: String! + } + type User { + id: ID! + name: String! + siteId: ID + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.isNotEmpty()) + val error = errors.singleOfType() + assert(error.parentType.overall.name == "JiraIssue") + assert(error.overallField.name == "creators") + assert(error.remoteArg.name == "id") + assert(error.actorFieldName == "site") + } + + it("checks when hydration argument source type is object type") { + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: JiraIssue + } + type JiraIssue @renamed(from: "Issue") { + id: ID! + creators: [User] @hydrated( + service: "users" + field: "users" + arguments: [ + {name: "id", value: "$source.creators"} + ] + inputIdentifiedBy: [ + {sourceId: "creators.userId" resultId: "id"} + {sourceId: "creators.site" resultId: "siteId"} + ] + ) + } + """.trimIndent(), + "users" to """ + type Query { + users(id: [UserInput]): [User] + } + input UserInput { + userId: ID + site: String! + } + type User { + id: ID! + name: String + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + creators: [UserRef] + } + type UserRef { + userId: ID + site: Int! + } + """.trimIndent(), + "users" to """ + type Query { + users(id: [UserInput]): [User] + } + input UserInput { + userId: ID + site: String! + } + type User { + id: ID! + name: String! + siteId: ID + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.isNotEmpty()) + val error = errors.singleOfType() + assert(error.parentType.overall.name == "JiraIssue") + assert(error.overallField.name == "creators") + assert(error.remoteArg.name == "id") + assert(error.actorFieldName == "site") + assert(GraphQLTypeUtil.simplePrint(error.hydrationType) == "Int!") + assert(GraphQLTypeUtil.simplePrint(error.actorFieldType) == "String!") + } + + it("checks if missing hydration argument source type when it is input object type") { + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: JiraIssue + } + type JiraIssue @renamed(from: "Issue") { + id: ID! + creators(someArg: [CreatorInput]): [User] @hydrated( + service: "users" + field: "users" + arguments: [ + {name: "id", value: "$argument.someArg"} + ] + inputIdentifiedBy: [ + {sourceId: "creators.userId" resultId: "id"} + {sourceId: "creators.site" resultId: "siteId"} + ] + ) + } + input CreatorInput { + userId: ID + } + """.trimIndent(), + "users" to """ + type Query { + users(id: [UserInput]): [User] + } + input UserInput { + userId: ID + site: String! + } + type User { + id: ID! + name: String + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + } + """.trimIndent(), + "users" to """ + type Query { + users(id: [UserInput]): [User] + } + input UserInput { + userId: ID + site: String! + } + type User { + id: ID! + name: String! + siteId: ID + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.isNotEmpty()) + val error = errors.singleOfType() + assert(error.parentType.overall.name == "JiraIssue") + assert(error.overallField.name == "creators") + assert(error.remoteArg.name == "id") + assert(error.actorFieldName == "site") + } + + it("checks when hydration argument source type is input object type") { + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: JiraIssue + } + type JiraIssue @renamed(from: "Issue") { + id: ID! + creators(someArg: [CreatorInput]): [User] @hydrated( + service: "users" + field: "users" + arguments: [ + {name: "id", value: "$argument.someArg"} + ] + inputIdentifiedBy: [ + {sourceId: "creators.userId" resultId: "id"} + {sourceId: "creators.site" resultId: "siteId"} + ] + ) + } + input CreatorInput { + userId: ID + site: Int! + } + """.trimIndent(), + "users" to """ + type Query { + users(id: [UserInput]): [User] + } + input UserInput { + userId: ID + site: String! + } + type User { + id: ID! + name: String + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + } + """.trimIndent(), + "users" to """ + type Query { + users(id: [UserInput]): [User] + } + input UserInput { + userId: ID + site: String! + } + type User { + id: ID! + name: String! + siteId: ID + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.isNotEmpty()) + val error = errors.singleOfType() + assert(error.parentType.overall.name == "JiraIssue") + assert(error.overallField.name == "creators") + assert(error.remoteArg.name == "id") + assert(error.actorFieldName == "site") + assert(GraphQLTypeUtil.simplePrint(error.hydrationType) == "Int!") + assert(GraphQLTypeUtil.simplePrint(error.actorFieldType) == "String!") + } + + context("fails if hydration argument type list wrappings are not equal with actor") { + withData( + nameFn = { (hydration, actor) -> "Hydration=$hydration, actor=$actor" }, + // "ID!" to "[ID]", + // "ID!" to "[ID]", + // "ID!" to "[ID!]", + "ID" to "ID", + "[ID]" to "ID", + ) { (hydration, actor) -> + val fixture = NadelValidationTestFixture( + overallSchema = mapOf( + "issues" to """ + type Query { + issue: JiraIssue + } + type JiraIssue @renamed(from: "Issue") { + id: ID! + creators: [User] @hydrated( + service: "users" + field: "usersById" + arguments: [ + {name: "id", value: "$source.creators.id"} + ] + identifiedBy: "id" + ) + } + """.trimIndent(), + "users" to """ + type Query { + usersById(id: $actor): [User] + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + underlyingSchema = mapOf( + "issues" to """ + type Query { + issue: Issue + } + type Issue { + id: ID! + creators: [CreatorRef] + } + type CreatorRef { + id: $hydration + } + """.trimIndent(), + "users" to """ + type Query { + usersById(id: $actor): [User] + } + type User { + id: ID! + name: String! + } + """.trimIndent(), + ), + ) + + val errors = validate(fixture) + assert(errors.isEmpty()) + + // val error = errors.assertSingleOfType() + // assert(error.service.name == "test") + // assert(error.parentType.overall.name == "Role") + // assert(error.parentType.underlying.name == error.parentType.overall.name) + // assert(error.overallInputField.type.unwrapAll().name == error.underlyingInputField.type.unwrapAll().name) + // assert(error.overallInputField.name == "m") + } + } + } +}) diff --git a/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt b/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt index 0051a0a4a..03c987f80 100644 --- a/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt +++ b/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationValidationTest.kt @@ -2,16 +2,10 @@ package graphql.nadel.validation import graphql.nadel.engine.util.singleOfType import graphql.nadel.validation.NadelSchemaValidationError.CannotRenameHydratedField -import graphql.nadel.validation.NadelSchemaValidationError.DuplicatedHydrationArgument import graphql.nadel.validation.NadelSchemaValidationError.HydrationFieldMustBeNullable import graphql.nadel.validation.NadelSchemaValidationError.HydrationIncompatibleOutputType -import graphql.nadel.validation.NadelSchemaValidationError.IncompatibleHydrationArgumentType import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationActorField import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationActorService -import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationArgumentValueSource -import graphql.nadel.validation.NadelSchemaValidationError.MissingHydrationFieldValueSource -import graphql.nadel.validation.NadelSchemaValidationError.MissingRequiredHydrationActorFieldArgument -import graphql.nadel.validation.NadelSchemaValidationError.NonExistentHydrationActorFieldArgument import graphql.nadel.validation.util.assertSingleOfType import graphql.schema.GraphQLNamedType import io.kotest.core.spec.style.DescribeSpec @@ -477,374 +471,6 @@ class NadelHydrationValidationTest : DescribeSpec({ assert(error.overallField == error.subject) } - it("fails if hydration argument references non existent field") { - val fixture = NadelValidationTestFixture( - overallSchema = mapOf( - "issues" to """ - type Query { - issue: Issue - } - type Issue { - creator: User @hydrated( - service: "users" - field: "user" - arguments: [ - {name: "id", value: "$source.creatorId"} - ] - ) - } - """.trimIndent(), - "users" to """ - type Query { - user(id: ID!): User - } - type User { - id: ID! - name: String! - } - """.trimIndent(), - ), - underlyingSchema = mapOf( - "issues" to """ - type Query { - issue: Issue - } - type Issue { - creator: ID! - } - """.trimIndent(), - "users" to """ - type Query { - user(id: ID!): User - } - type User { - id: ID! - name: String! - } - """.trimIndent(), - ), - ) - - val errors = validate(fixture) - assert(errors.map { it.message }.isNotEmpty()) - - val error = errors.assertSingleOfType() - assert(error.parentType.overall.name == "Issue") - assert(error.parentType.underlying.name == "Issue") - assert(error.overallField.name == "creator") - assert(error.remoteArgSource.pathToField == listOf("creatorId")) - assert(error.remoteArgSource.argumentName == null) - } - - it("fails if hydration argument references non existent argument") { - val fixture = NadelValidationTestFixture( - overallSchema = mapOf( - "issues" to """ - type Query { - issue: Issue - } - type Issue { - id: ID! - } - """.trimIndent(), - "users" to """ - type Query { - user(id: ID!, secrets: Boolean = false): User - } - type User { - id: ID! - name: String! - } - extend type Issue { - creator(someArg: Boolean): User @hydrated( - service: "users" - field: "user" - arguments: [ - {name: "id", value: "$source.creator"} - {name: "secrets", value: "$argument.secrets"} - ] - ) - } - """.trimIndent(), - ), - underlyingSchema = mapOf( - "issues" to """ - type Query { - issue: Issue - } - type Issue { - id: ID! - creator: ID! - } - """.trimIndent(), - "users" to """ - type Query { - user(id: ID!, secrets: Boolean = false): User - } - type User { - id: ID! - name: String! - } - """.trimIndent(), - ), - ) - - val errors = validate(fixture) - assert(errors.map { it.message }.isNotEmpty()) - - val error = errors.assertSingleOfType() - assert(error.parentType.overall.name == "Issue") - assert(error.parentType.underlying.name == "Issue") - assert(error.overallField.name == "creator") - assert(error.remoteArgSource.argumentName == "secrets") - assert(error.remoteArgSource.pathToField == null) - } - - it("fails if hydration argument references non existent remote argument") { - val fixture = NadelValidationTestFixture( - overallSchema = mapOf( - "issues" to """ - type Query { - issue: Issue - } - type Issue { - id: ID! - } - """.trimIndent(), - "users" to """ - type Query { - user(id: ID!): User - } - type User { - id: ID! - name: String! - } - extend type Issue { - creator(someArg: Boolean): User @hydrated( - service: "users" - field: "user" - arguments: [ - {name: "id", value: "$source.creator"} - {name: "someArg", value: "$argument.someArg"} - ] - ) - } - """.trimIndent(), - ), - underlyingSchema = mapOf( - "issues" to """ - type Query { - issue: Issue - } - type Issue { - id: ID! - creator: ID! - } - """.trimIndent(), - "users" to """ - type Query { - user(id: ID!): User - } - type User { - id: ID! - name: String! - } - """.trimIndent(), - ), - ) - - val errors = validate(fixture) - assert(errors.map { it.message }.isNotEmpty()) - - val error = errors.assertSingleOfType() - assert(error.parentType.overall.name == "Issue") - assert(error.parentType.underlying.name == "Issue") - assert(error.overallField.name == "creator") - assert(error.argument == "someArg") - } - - it("fails if hydration defines duplicated arguments") { - val fixture = NadelValidationTestFixture( - overallSchema = mapOf( - "issues" to """ - type Query { - issue: Issue - } - type Issue { - id: ID! - } - """.trimIndent(), - "users" to """ - type Query { - user(id: ID!, other: Boolean): User - } - type User { - id: ID! - name: String! - } - extend type Issue { - creator(someArg: ID!, other: Boolean): User @hydrated( - service: "users" - field: "user" - arguments: [ - {name: "id", value: "$source.creator"} - {name: "id", value: "$argument.someArg"} - {name: "other", value: "$argument.other"} - ] - ) - } - """.trimIndent(), - ), - underlyingSchema = mapOf( - "issues" to """ - type Query { - issue: Issue - } - type Issue { - id: ID! - creator: ID! - } - """.trimIndent(), - "users" to """ - type Query { - user(id: ID!, other: Boolean): User - } - type User { - id: ID! - name: String! - } - """.trimIndent(), - ), - ) - - val errors = validate(fixture) - assert(errors.map { it.message }.isNotEmpty()) - - val error = errors.assertSingleOfType() - assert(error.parentType.overall.name == "Issue") - assert(error.parentType.underlying.name == "Issue") - assert(error.overallField.name == "creator") - assert(error.duplicates.map { it.name }.toSet() == setOf("id")) - } - - it("fails if hydration field has missing non-nullable arguments with underlying top level fields") { - val fixture = NadelValidationTestFixture( - overallSchema = mapOf( - "issues" to """ - type Query { - issue: Issue - } - type Issue { - id: ID! - } - """.trimIndent(), - "users" to """ - type Query { - user(id: ID!, other: Boolean!): User - } - type User { - id: ID! - name: String! - } - extend type Issue { - creator(someArg: ID!, other: Boolean): User @hydrated( - service: "users" - field: "user" - arguments: [ - {name: "id", value: "$argument.creator"} - ] - ) - } - """.trimIndent(), - ), - underlyingSchema = mapOf( - "issues" to """ - type Query { - issue: Issue - } - type Issue { - id: ID! - creator: ID! - } - """.trimIndent(), - "users" to """ - type Query { - user(id: ID!, other: Boolean!): User - } - type User { - id: ID! - name: String! - } - """.trimIndent(), - ), - ) - - val errors = validate(fixture) - assert(errors.map { it.message }.isNotEmpty()) - - val error = errors.assertSingleOfType() - assert(error.parentType.overall.name == "Issue") - assert(error.parentType.underlying.name == "Issue") - assert(error.overallField.name == "creator") - assert(error.argument == "other") - } - - it("passes if hydration field has missing nullable arguments with underlying top level fields") { - val fixture = NadelValidationTestFixture( - overallSchema = mapOf( - "issues" to """ - type Query { - issue: Issue - } - type Issue { - id: ID! - } - """.trimIndent(), - "users" to """ - type Query { - user(id: ID!, other: Boolean): User - } - type User { - id: ID! - name: String! - } - extend type Issue { - creator(someArg: ID!, other: Boolean): User @hydrated( - service: "users" - field: "user" - arguments: [ - {name: "id", value: "$argument.someArg"} - ] - ) - } - """.trimIndent(), - ), - underlyingSchema = mapOf( - "issues" to """ - type Query { - issue: Issue - } - type Issue { - id: ID! - creator: ID! - } - """.trimIndent(), - "users" to """ - type Query { - user(id: ID!, other: Boolean): User - } - type User { - id: ID! - name: String! - } - """.trimIndent(), - ), - ) - - val errors = validate(fixture) - assert(errors.map { it.message }.isEmpty()) - } - it("checks the output type of the actor field against the output type of the hydrated field") { val fixture = NadelValidationTestFixture( @@ -1205,242 +831,6 @@ class NadelHydrationValidationTest : DescribeSpec({ val errors = validate(fixture) assert(errors.map { it.message }.isEmpty()) } - - it("fails if hydration argument source type is mismatch with actor field input arguments") { - val fixture = NadelValidationTestFixture( - overallSchema = mapOf( - "issues" to """ - type Query { - issue: JiraIssue - } - type JiraIssue @renamed(from: "Issue") { - id: ID! - creator: User @hydrated( - service: "users" - field: "user" - arguments: [ - {name: "id", value: "$source.creator"} - ] - ) - } - """.trimIndent(), - "users" to """ - type Query { - user(id: Int!): User - } - type User { - id: ID! - name: String! - } - """.trimIndent(), - ), - underlyingSchema = mapOf( - "issues" to """ - type Query { - issue: Issue - } - type Issue { - id: ID! - creator: ID! - } - """.trimIndent(), - "users" to """ - type Query { - user(id: Int!): User - } - type User { - id: ID! - name: String! - } - """.trimIndent(), - ), - ) - - val errors = validate(fixture) - assert(errors.isNotEmpty()) - val error = errors.singleOfType() - assert(error.parentType.overall.name == "JiraIssue") - assert(error.overallField.name == "creator") - assert(error.remoteArg.name == "id") - assert(error.hydrationType == "ID!") - assert(error.actorArgInputType == "Int!") - } - - it("fails if hydration argument types are mismatch with actor field input arguments") { - val fixture = NadelValidationTestFixture( - overallSchema = mapOf( - "issues" to """ - type Query { - issue: JiraIssue - } - type JiraIssue @renamed(from: "Issue") { - id: ID! - creator(someArg: ID!): User @hydrated( - service: "users" - field: "user" - arguments: [ - {name: "someArg", value: "$argument.someArg"} - ] - ) - } - """.trimIndent(), - "users" to """ - type Query { - user(someArg: Int!): User - } - type User { - id: ID! - name: String! - } - """.trimIndent(), - ), - underlyingSchema = mapOf( - "issues" to """ - type Query { - issue: Issue - } - type Issue { - id: ID! - creator: ID! - } - """.trimIndent(), - "users" to """ - type Query { - user(someArg: Int!): User - } - type User { - id: ID! - name: String! - } - """.trimIndent(), - ), - ) - - val errors = validate(fixture) - assert(errors.isNotEmpty()) - val error = errors.singleOfType() - assert(error.parentType.overall.name == "JiraIssue") - assert(error.overallField.name == "creator") - assert(error.remoteArg.name == "someArg") - assert(error.hydrationType == "ID!") - assert(error.actorArgInputType == "Int!") - } - - - it("passes if hydration argument source types are matching with batch hydration") { - val fixture = NadelValidationTestFixture( - overallSchema = mapOf( - "issues" to """ - type Query { - issue: JiraIssue - } - type JiraIssue @renamed(from: "Issue") { - id: ID! - creators: [User] @hydrated( - service: "users" - field: "usersById" - arguments: [ - {name: "id", value: "$source.creators.id"} - ] - identifiedBy: "id" - ) - } - """.trimIndent(), - "users" to """ - type Query { - usersById(id: [ID]): [User] - } - type User { - id: ID! - name: String! - } - """.trimIndent(), - ), - underlyingSchema = mapOf( - "issues" to """ - type Query { - issue: Issue - } - type Issue { - id: ID! - creators: [CreatorRef] - } - type CreatorRef { - id: ID! - } - """.trimIndent(), - "users" to """ - type Query { - usersById(id: [ID]): [User] - } - type User { - id: ID! - name: String! - } - """.trimIndent(), - ), - ) - - val errors = validate(fixture) - assert(errors.isEmpty()) - - } - - it("passes if hydration argument source types are matching with batch hydration with list input") { - val fixture = NadelValidationTestFixture( - overallSchema = mapOf( - "issues" to """ - type Query { - issue: JiraIssue - } - type JiraIssue @renamed(from: "Issue") { - id: ID! - creators: [User] @hydrated( - service: "users" - field: "userById" - arguments: [ - {name: "id", value: "$source.creators"} - ] - identifiedBy: "id" - ) - } - """.trimIndent(), - "users" to """ - type Query { - userById(id: ID): User - } - type User { - id: ID! - name: String! - } - """.trimIndent(), - ), - underlyingSchema = mapOf( - "issues" to """ - type Query { - issue: Issue - } - type Issue { - id: ID! - creators: [ID] - } - """.trimIndent(), - "users" to """ - type Query { - userById(id: ID): User - } - type User { - id: ID! - name: String! - } - """.trimIndent(), - ), - ) - - val errors = validate(fixture) - assert(errors.isEmpty()) - - } } }) From 78f233eddbae7e713247154f9262d691fc2fd78e Mon Sep 17 00:00:00 2001 From: faizan-siddiqui Date: Thu, 7 Apr 2022 06:31:19 +1000 Subject: [PATCH 15/17] cleanup --- .../NadelHydrationArgumentValidation.kt | 7 ------- .../NadelHydrationArgumentValidationTest.kt | 19 +++++++------------ 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationArgumentValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationArgumentValidation.kt index 081b6e8cf..3c8ba8474 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationArgumentValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationArgumentValidation.kt @@ -121,12 +121,6 @@ internal class NadelHydrationArgumentValidation( * Checks the type of a hydration argument derived from either a $source field output type or $argument field input * argument type against the type of an actor field argument to see if they match * will also allow for cases of batched hydration eg ID! -> [ID] - * - * use cases: ID! -> [ID] ok and done - * ID! -> ID ok and done - * ID -> [ID] ok and done - * ID! -> [ID!] ok and done - * object to input object ok and not done */ private fun hydrationArgTypesMatch( parent: NadelServiceSchemaElement, @@ -234,7 +228,6 @@ internal class NadelHydrationArgumentValidation( * Checks assignability of a hydration type against actor input argument type to see if either the scalars types * match or if the object type and input object type have the same fields * - * use cases: maybe renames? */ private fun checkScalarOrObjectTypeAssignability( parent: NadelServiceSchemaElement, diff --git a/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationArgumentValidationTest.kt b/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationArgumentValidationTest.kt index 78387853a..c71a01ac0 100644 --- a/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationArgumentValidationTest.kt +++ b/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationArgumentValidationTest.kt @@ -927,15 +927,17 @@ class NadelHydrationArgumentValidationTest : DescribeSpec({ assert(GraphQLTypeUtil.simplePrint(error.actorFieldType) == "String!") } - context("fails if hydration argument type list wrappings are not equal with actor") { + context("passes for acceptable batch hydration cases") { withData( nameFn = { (hydration, actor) -> "Hydration=$hydration, actor=$actor" }, - // "ID!" to "[ID]", - // "ID!" to "[ID]", - // "ID!" to "[ID!]", "ID" to "ID", "[ID]" to "ID", - ) { (hydration, actor) -> + "ID" to "[ID]", + "ID!" to "[ID]", + "ID!" to "[ID]!", + "ID!" to "[ID!]", + + ) { (hydration, actor) -> val fixture = NadelValidationTestFixture( overallSchema = mapOf( "issues" to """ @@ -991,13 +993,6 @@ class NadelHydrationArgumentValidationTest : DescribeSpec({ val errors = validate(fixture) assert(errors.isEmpty()) - - // val error = errors.assertSingleOfType() - // assert(error.service.name == "test") - // assert(error.parentType.overall.name == "Role") - // assert(error.parentType.underlying.name == error.parentType.overall.name) - // assert(error.overallInputField.type.unwrapAll().name == error.underlyingInputField.type.unwrapAll().name) - // assert(error.overallInputField.name == "m") } } } From 0f89d86e27ea5e1ca7ae0f0dd4882c6a1977474e Mon Sep 17 00:00:00 2001 From: faizan-siddiqui Date: Wed, 25 May 2022 14:34:36 +1000 Subject: [PATCH 16/17] cleanup --- .../validation/NadelHydrationArgumentValidation.kt | 11 +++++------ .../NadelHydrationArgumentValidationTest.kt | 6 +++--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationArgumentValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationArgumentValidation.kt index 3c8ba8474..b796a94fb 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationArgumentValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationArgumentValidation.kt @@ -45,18 +45,17 @@ internal class NadelHydrationArgumentValidation( DuplicatedHydrationArgument(parent, overallField, it) } - val remoteArgErrors = mutableListOf() - hydration.arguments.mapNotNull { remoteArg -> + val remoteArgErrors = hydration.arguments.flatMap { remoteArg -> val actorFieldArgument = actorField.getArgument(remoteArg.name) if (actorFieldArgument == null) { - remoteArgErrors.add(NonExistentHydrationActorFieldArgument( + listOf(NonExistentHydrationActorFieldArgument( parent, overallField, hydration, argument = remoteArg.name, )) } else { - remoteArgErrors.addAll(getRemoteArgErrors(parent, overallField, remoteArg, actorField)) + getRemoteArgErrors(parent, overallField, remoteArg, actorField) } } @@ -98,7 +97,7 @@ internal class NadelHydrationArgumentValidation( } else { //check the input types match with hydration and actor fields val fieldOutputType = field.type - return hydrationArgTypesMatch(parent, overallField, remoteArg, fieldOutputType, actorArgInputType) + hydrationArgTypesMatch(parent, overallField, remoteArg, fieldOutputType, actorArgInputType) } } FieldArgument -> { @@ -108,7 +107,7 @@ internal class NadelHydrationArgumentValidation( } else { //check the input types match with hydration and actor fields val hydrationArgType = argument.type - return hydrationArgTypesMatch(parent, overallField, remoteArg, hydrationArgType, actorArgInputType) + hydrationArgTypesMatch(parent, overallField, remoteArg, hydrationArgType, actorArgInputType) } } else -> { diff --git a/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationArgumentValidationTest.kt b/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationArgumentValidationTest.kt index c71a01ac0..57f4e1041 100644 --- a/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationArgumentValidationTest.kt +++ b/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationArgumentValidationTest.kt @@ -623,7 +623,7 @@ class NadelHydrationArgumentValidationTest : DescribeSpec({ } - it("checks if missing hydration argument source type when it is object type") { + it("checks if missing hydration source type when it is object type") { val fixture = NadelValidationTestFixture( overallSchema = mapOf( "issues" to """ @@ -776,7 +776,7 @@ class NadelHydrationArgumentValidationTest : DescribeSpec({ assert(GraphQLTypeUtil.simplePrint(error.actorFieldType) == "String!") } - it("checks if missing hydration argument source type when it is input object type") { + it("checks if there is a missing hydration argument source type when the hydration input is an input object type") { val fixture = NadelValidationTestFixture( overallSchema = mapOf( "issues" to """ @@ -850,7 +850,7 @@ class NadelHydrationArgumentValidationTest : DescribeSpec({ assert(error.actorFieldName == "site") } - it("checks when hydration argument source type is input object type") { + it("checks if there is a mismatched hydration argument source type when the hydration input is an input object type") { val fixture = NadelValidationTestFixture( overallSchema = mapOf( "issues" to """ From 320c19d3df986abb35950549f12ece64f14bfc88 Mon Sep 17 00:00:00 2001 From: faizan-siddiqui Date: Wed, 25 May 2022 14:36:42 +1000 Subject: [PATCH 17/17] cleanup --- .../nadel/validation/NadelHydrationArgumentValidationTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationArgumentValidationTest.kt b/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationArgumentValidationTest.kt index 57f4e1041..0c086d84e 100644 --- a/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationArgumentValidationTest.kt +++ b/lib/src/test/kotlin/graphql/nadel/validation/NadelHydrationArgumentValidationTest.kt @@ -623,7 +623,7 @@ class NadelHydrationArgumentValidationTest : DescribeSpec({ } - it("checks if missing hydration source type when it is object type") { + it("checks if there is a missing hydration source type when the hydration input type is an object type") { val fixture = NadelValidationTestFixture( overallSchema = mapOf( "issues" to """