From c31aa9e1ab9f7ae9c20089c4d9918298ab70ed25 Mon Sep 17 00:00:00 2001 From: Steven Barker Date: Fri, 13 Oct 2023 16:58:52 +1300 Subject: [PATCH] add actor field name to error message --- .../NadelHydrationArgumentValidation.kt | 50 +++++++++++++------ .../validation/NadelHydrationValidation.kt | 7 ++- .../validation/NadelSchemaValidationError.kt | 25 ++++++---- 3 files changed, 55 insertions(+), 27 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationArgumentValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationArgumentValidation.kt index 28fd8348d..47f93196f 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationArgumentValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationArgumentValidation.kt @@ -20,7 +20,8 @@ internal class NadelHydrationArgumentValidation private constructor() { overallField: GraphQLFieldDefinition, remoteArg: RemoteArgumentDefinition, hydration: UnderlyingServiceHydration, - isBatchHydration: Boolean + isBatchHydration: Boolean, + actorFieldName: String ): NadelSchemaValidationError? { val unwrappedHydrationSourceFieldType = hydrationSourceFieldType.unwrapNonNull() val unwrappedActorFieldArgType = actorFieldArgType.unwrapNonNull() @@ -34,7 +35,8 @@ internal class NadelHydrationArgumentValidation private constructor() { parent, overallField, remoteArg, - hydration + hydration, + actorFieldName ) if (error != null) { return NadelSchemaValidationError.IncompatibleHydrationArgumentType( @@ -43,6 +45,7 @@ internal class NadelHydrationArgumentValidation private constructor() { remoteArg, hydrationSourceFieldType, actorFieldArgType, + actorFieldName ) } } @@ -55,7 +58,8 @@ internal class NadelHydrationArgumentValidation private constructor() { parent, overallField, remoteArg, - hydration + hydration, + actorFieldName ) if (error != null) { return NadelSchemaValidationError.IncompatibleHydrationArgumentType( @@ -64,6 +68,7 @@ internal class NadelHydrationArgumentValidation private constructor() { remoteArg, hydrationSourceFieldType, actorFieldArgType, + actorFieldName ) } @@ -76,7 +81,8 @@ internal class NadelHydrationArgumentValidation private constructor() { parent, overallField, remoteArg, - hydration + hydration, + actorFieldName ) } return null @@ -88,7 +94,8 @@ internal class NadelHydrationArgumentValidation private constructor() { parent: NadelServiceSchemaElement, overallField: GraphQLFieldDefinition, remoteArg: RemoteArgumentDefinition, - hydration: UnderlyingServiceHydration + hydration: UnderlyingServiceHydration, + actorFieldName: String ): NadelSchemaValidationError? { //need to check null compatibility @@ -101,6 +108,7 @@ internal class NadelHydrationArgumentValidation private constructor() { remoteArg, hydrationSourceFieldType, actorFieldArgType, + actorFieldName ) } val unwrappedHydrationSourceFieldType = hydrationSourceFieldType.unwrapNonNull() @@ -108,15 +116,15 @@ internal class NadelHydrationArgumentValidation private constructor() { // scalar feed into scalar if (unwrappedHydrationSourceFieldType is GraphQLScalarType && unwrappedActorFieldArgType is GraphQLScalarType) { - return validateScalarArg(hydrationSourceFieldType, actorFieldArgType, parent, overallField, remoteArg) + return validateScalarArg(hydrationSourceFieldType, actorFieldArgType, parent, overallField, remoteArg, actorFieldName) } // list feed into list else if (unwrappedHydrationSourceFieldType.isList && unwrappedActorFieldArgType.isList) { - return validateListArg(hydrationSourceFieldType, actorFieldArgType, parent, overallField, remoteArg, hydration) + return validateListArg(hydrationSourceFieldType, actorFieldArgType, parent, overallField, remoteArg, hydration, actorFieldName) } // object feed into inputObject (i.e. hydrating with a $source object) else if (sourceType == ObjectField && unwrappedHydrationSourceFieldType is GraphQLObjectType && unwrappedActorFieldArgType is GraphQLInputObjectType) { - return validateInputObjectArg(unwrappedHydrationSourceFieldType, unwrappedActorFieldArgType, parent, overallField, remoteArg, hydration) + return validateInputObjectArg(unwrappedHydrationSourceFieldType, unwrappedActorFieldArgType, parent, overallField, remoteArg, hydration, actorFieldName) } // inputObject feed into inputObject (i.e. hydrating with a $argument object) else if (sourceType == FieldArgument && unwrappedHydrationSourceFieldType is GraphQLInputObjectType && unwrappedActorFieldArgType is GraphQLInputObjectType) { @@ -127,6 +135,7 @@ internal class NadelHydrationArgumentValidation private constructor() { remoteArg, hydrationSourceFieldType, actorFieldArgType, + actorFieldName ) } return null @@ -138,6 +147,7 @@ internal class NadelHydrationArgumentValidation private constructor() { remoteArg, hydrationSourceFieldType, actorFieldArgType, + actorFieldName ) } @@ -146,7 +156,8 @@ internal class NadelHydrationArgumentValidation private constructor() { actorFieldArgType: GraphQLType, parent: NadelServiceSchemaElement, overallField: GraphQLFieldDefinition, - remoteArg: RemoteArgumentDefinition + remoteArg: RemoteArgumentDefinition, + actorFieldName: String ): NadelSchemaValidationError? { if (hydrationSourceFieldType.unwrapNonNull() is GraphQLScalarType && actorFieldArgType.unwrapNonNull() is GraphQLScalarType) { if (isScalarAssignable(hydrationSourceFieldType.unwrapNonNull() as GraphQLScalarType, actorFieldArgType.unwrapNonNull() as GraphQLScalarType)) { @@ -158,6 +169,7 @@ internal class NadelHydrationArgumentValidation private constructor() { remoteArg, hydrationSourceFieldType, actorFieldArgType, + actorFieldName ) } } @@ -181,7 +193,8 @@ internal class NadelHydrationArgumentValidation private constructor() { parent: NadelServiceSchemaElement, overallField: GraphQLFieldDefinition, remoteArg: RemoteArgumentDefinition, - hydration: UnderlyingServiceHydration + hydration: UnderlyingServiceHydration, + actorFieldName: String ): NadelSchemaValidationError? { var hydrationSourceFieldInnerType: GraphQLType = hydrationSourceFieldType.unwrapNonNull().unwrapOne() var actorFieldArgInnerType: GraphQLType = actorFieldArgType.unwrapNonNull().unwrapOne() @@ -191,7 +204,8 @@ internal class NadelHydrationArgumentValidation private constructor() { parent, overallField, remoteArg, - hydration + hydration, + actorFieldName ) != null if (errorExists) { return NadelSchemaValidationError.IncompatibleHydrationArgumentType( @@ -200,6 +214,7 @@ internal class NadelHydrationArgumentValidation private constructor() { remoteArg, hydrationSourceFieldType, actorFieldArgType, + actorFieldName ) } return null @@ -211,7 +226,8 @@ internal class NadelHydrationArgumentValidation private constructor() { parent: NadelServiceSchemaElement, overallField: GraphQLFieldDefinition, remoteArg: RemoteArgumentDefinition, - hydration: UnderlyingServiceHydration + hydration: UnderlyingServiceHydration, + actorFieldName: String ): NadelSchemaValidationError? { for (actorInnerField in actorFieldArgType.fields) { val actorInnerFieldName = actorInnerField.name @@ -223,7 +239,8 @@ internal class NadelHydrationArgumentValidation private constructor() { parent, overallField, remoteArg, - actorInnerFieldName + actorInnerFieldName, + actorFieldName ) } } else { @@ -233,16 +250,17 @@ internal class NadelHydrationArgumentValidation private constructor() { parent, overallField, remoteArg, - hydration + hydration, + actorFieldName ) != null - if (thisFieldHasError) { // want to return top level type to user + if (thisFieldHasError) { return NadelSchemaValidationError.IncompatibleFieldInHydratedInputObject( parent, overallField, remoteArg, hydrationSourceFieldType, actorFieldArgType, - actorInnerFieldName + actorInnerFieldName, ) } } diff --git a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt index 85b526389..61b095f77 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelHydrationValidation.kt @@ -229,7 +229,8 @@ internal class NadelHydrationValidation( overallField, remoteArgDef, hydration, - isBatchHydration + isBatchHydration, + actorField.name ) } } @@ -248,7 +249,8 @@ internal class NadelHydrationValidation( overallField, remoteArgDef, hydration, - isBatchHydration + isBatchHydration, + actorField.name ) } } @@ -267,6 +269,7 @@ internal class NadelHydrationValidation( overallField, remoteArgDef, actorFieldArg.type, + actorField.name ) } return null diff --git a/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationError.kt b/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationError.kt index b13bb85a7..733bbc584 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationError.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationError.kt @@ -360,6 +360,7 @@ sealed interface NadelSchemaValidationError { val remoteArg: RemoteArgumentDefinition, val hydrationType: GraphQLType, val actorArgInputType: GraphQLType, + val actorFieldName: String ) : NadelSchemaValidationError { val service: Service get() = parentType.service @@ -374,10 +375,11 @@ sealed interface NadelSchemaValidationError { val argumentSuppliedFromSubString = if (remoteArg.remoteArgumentSource.sourceType == ObjectField) "the value from field \"$remoteArgSource\" from service \"$s\"" - else "the supplied argument called \"${remoteArg.remoteArgumentSource.argumentName}\"" + else "a supplied argument called \"${remoteArg.remoteArgumentSource.argumentName}\"" - "Field \"$of\" tried to hydrate with argument \"$hydrationArgName\" using $argumentSuppliedFromSubString" + - " of type $ht but it was not assignable to the expected type $at" + "Field \"$of\" tried to hydrate using the actor field \"$actorFieldName\" and argument \"$hydrationArgName\"." + + "However, you are supplying actor field argument with $argumentSuppliedFromSubString " + + "of type $ht which is not assignable to the expected type $at" } override val subject = overallField @@ -388,6 +390,7 @@ sealed interface NadelSchemaValidationError { val overallField: GraphQLFieldDefinition, val remoteArg: RemoteArgumentDefinition, val actorArgInputType: GraphQLType, + val actorFieldName: String ) : NadelSchemaValidationError { val service: Service get() = parentType.service @@ -396,8 +399,9 @@ sealed interface NadelSchemaValidationError { val of = makeFieldCoordinates(parentType.overall.name, overallField.name) val at = GraphQLTypeUtil.simplePrint(actorArgInputType) - "Field $of tried to hydrate with argument $hydrationArgName of type $at using a statically supplied argument, " + - "but the type of the supplied static argument is incompatible" + "Field $of tried to hydrate using actor field \"$actorFieldName\". " + + "However, the type of the static argument you are supplying actor field arg \"$hydrationArgName\" with " + + "is not assignable to the expected type $at" } override val subject = overallField @@ -419,8 +423,10 @@ sealed interface NadelSchemaValidationError { val of = makeFieldCoordinates(parentType.overall.name, overallField.name) val remoteArgSource = "${parentType.underlying.name}.${remoteArg.remoteArgumentSource.pathToField?.joinToString(separator = ".")}" - "Field $of tried to hydrate with argument \"$hydrationArgName\" using value from $remoteArgSource " + - "but the types are not compatible" + + "Field \"$of\" tried to hydrate using the actor field \"$actorFieldName\" and argument \"$hydrationArgName\"." + + "However, you are supplying actor field argument with the value from $remoteArgSource " + + "and the types are incompatible" } override val subject = overallField @@ -431,6 +437,7 @@ sealed interface NadelSchemaValidationError { val overallField: GraphQLFieldDefinition, val remoteArg: RemoteArgumentDefinition, val missingFieldName: String, + val actorFieldName: String, ) : NadelSchemaValidationError { val service: Service get() = parentType.service @@ -440,8 +447,8 @@ sealed interface NadelSchemaValidationError { val remoteArgSource = "${parentType.underlying.name}.${remoteArg.remoteArgumentSource.pathToField?.joinToString(separator = ".")}" val s = service.name - "Field $of tried to hydrate with argument \"$hydrationArgName\" using value from $remoteArgSource in service $s" + - " but it was missing the field $missingFieldName" + "Field $of tried to hydrate using field \"$actorFieldName\" with argument \"$hydrationArgName\" using value from $remoteArgSource in service $s" + + " but it was missing the required field $missingFieldName" } override val subject = overallField