From 00433699e456f1350df74e0c6e85d894e8a07372 Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Tue, 26 Nov 2024 23:20:00 +1300 Subject: [PATCH 1/4] Improve factory and dependency flow --- .../NadelAssignableTypeValidation.kt | 31 +++++ .../nadel/validation/NadelFieldValidation.kt | 129 +++++++++++------- ...ation.kt => NadelInputObjectValidation.kt} | 50 +------ .../nadel/validation/NadelRenameValidation.kt | 75 ---------- .../nadel/validation/NadelSchemaValidation.kt | 2 +- .../NadelSchemaValidationFactory.kt | 77 +++++++---- .../nadel/validation/NadelTypeValidation.kt | 15 +- .../validation/NadelVirtualTypeValidation.kt | 65 ++++----- .../NadelHydrationArgumentTypeValidation.kt | 6 +- .../NadelHydrationArgumentValidation.kt | 6 +- .../hydration/NadelHydrationValidation.kt | 11 +- 11 files changed, 214 insertions(+), 253 deletions(-) create mode 100644 lib/src/main/java/graphql/nadel/validation/NadelAssignableTypeValidation.kt rename lib/src/main/java/graphql/nadel/validation/{NadelInputValidation.kt => NadelInputObjectValidation.kt} (54%) delete mode 100644 lib/src/main/java/graphql/nadel/validation/NadelRenameValidation.kt diff --git a/lib/src/main/java/graphql/nadel/validation/NadelAssignableTypeValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelAssignableTypeValidation.kt new file mode 100644 index 000000000..4b70ea0c1 --- /dev/null +++ b/lib/src/main/java/graphql/nadel/validation/NadelAssignableTypeValidation.kt @@ -0,0 +1,31 @@ +package graphql.nadel.validation + +import graphql.nadel.engine.util.unwrapAll +import graphql.schema.GraphQLType +import graphql.schema.GraphQLUnmodifiedType + +class NadelAssignableTypeValidation internal constructor( + private val typeWrappingValidation: NadelTypeWrappingValidation, +) { + context(NadelValidationContext) + fun isTypeAssignable( + suppliedType: GraphQLType, + requiredType: GraphQLType, + ): Boolean { + val typeWrappingValid = typeWrappingValidation.isTypeWrappingValid( + lhs = suppliedType, + rhs = requiredType, + rule = NadelTypeWrappingValidation.Rule.LHS_MUST_BE_STRICTER_OR_SAME, + ) + + return typeWrappingValid && isTypeNameValid(suppliedType.unwrapAll(), requiredType.unwrapAll()) + } + + context(NadelValidationContext) + private fun isTypeNameValid( + overallType: GraphQLUnmodifiedType, + underlyingType: GraphQLUnmodifiedType, + ): Boolean { + return getUnderlyingTypeName(overallType) == underlyingType.name + } +} diff --git a/lib/src/main/java/graphql/nadel/validation/NadelFieldValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelFieldValidation.kt index 8464e1536..f291d792b 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelFieldValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelFieldValidation.kt @@ -1,27 +1,30 @@ package graphql.nadel.validation -import graphql.nadel.engine.util.unwrapAll +import graphql.nadel.definition.renamed.NadelRenamedDefinition +import graphql.nadel.engine.blueprint.NadelDeepRenameFieldInstruction +import graphql.nadel.engine.blueprint.NadelFieldInstruction +import graphql.nadel.engine.blueprint.NadelRenameFieldInstruction +import graphql.nadel.engine.transform.query.NadelQueryPath +import graphql.nadel.engine.util.getFieldAt +import graphql.nadel.engine.util.makeFieldCoordinates import graphql.nadel.schema.NadelDirectives +import graphql.nadel.validation.NadelSchemaValidationError.CannotRenameHydratedField +import graphql.nadel.validation.NadelSchemaValidationError.CannotRenamePartitionedField import graphql.nadel.validation.NadelSchemaValidationError.IncompatibleArgumentInputType import graphql.nadel.validation.NadelSchemaValidationError.IncompatibleFieldOutputType import graphql.nadel.validation.NadelSchemaValidationError.MissingArgumentOnUnderlying +import graphql.nadel.validation.NadelSchemaValidationError.MissingRename import graphql.nadel.validation.NadelSchemaValidationError.MissingUnderlyingField -import graphql.nadel.validation.NadelTypeWrappingValidation.Rule.LHS_MUST_BE_LOOSER_OR_SAME import graphql.nadel.validation.hydration.NadelHydrationValidation import graphql.nadel.validation.util.NadelCombinedTypeUtil.getFieldsThatServiceContributed -import graphql.schema.GraphQLArgument import graphql.schema.GraphQLFieldDefinition import graphql.schema.GraphQLNamedSchemaElement -import graphql.schema.GraphQLOutputType class NadelFieldValidation internal constructor( private val hydrationValidation: NadelHydrationValidation, + private val partitionValidation: NadelPartitionValidation, + private val assignableTypeValidation: NadelAssignableTypeValidation, ) { - private val renameValidation = NadelRenameValidation(this) - private val inputValidation = NadelInputValidation() - private val partitionValidation = NadelPartitionValidation() - private val typeWrappingValidation = NadelTypeWrappingValidation() - context(NadelValidationContext) fun validate( schemaElement: NadelServiceSchemaElement.FieldsContainer, @@ -67,7 +70,7 @@ class NadelFieldValidation internal constructor( overallField: GraphQLFieldDefinition, ): NadelSchemaValidationResult { return if (isRenamed(parent, overallField)) { - renameValidation.validate(parent, overallField) + validateRename(parent, overallField) } else if (isHydrated(parent, overallField)) { hydrationValidation.validate(parent, overallField) } else { @@ -96,14 +99,14 @@ class NadelFieldValidation internal constructor( if (underlyingArg == null) { MissingArgumentOnUnderlying(parent, overallField, underlyingField, overallArg) } else { - if (isUnwrappedArgTypeSame(overallArg, underlyingArg)) { - inputValidation - .validate( - parent = parent, - overallField = overallField, - overallInputArgument = overallArg, - underlyingInputArgument = underlyingArg - ) + // Note: the value comes from the user (overall schema) + // So we are supplying the overall argument to the underlying argument + val isArgumentTypeAssignable = assignableTypeValidation.isTypeAssignable( + suppliedType = overallArg.type, + requiredType = underlyingArg.type + ) + if (isArgumentTypeAssignable) { + ok() } else { IncompatibleArgumentInputType( parentType = parent, @@ -123,13 +126,61 @@ class NadelFieldValidation internal constructor( } context(NadelValidationContext) - private fun isUnwrappedArgTypeSame( - overallArg: GraphQLArgument, - underlyingArg: GraphQLArgument, - ): Boolean { - val overallArgTypeUnwrapped = overallArg.type.unwrapAll() - val underlyingArgTypeUnwrapped = underlyingArg.type.unwrapAll() - return getUnderlyingTypeName(overallArgTypeUnwrapped) == underlyingArgTypeUnwrapped.name + private fun validateRename( + parent: NadelServiceSchemaElement.FieldsContainer, + overallField: GraphQLFieldDefinition, + ): NadelSchemaValidationResult { + if (isHydrated(parent, overallField)) { + return CannotRenameHydratedField(parent, overallField) + } + + if (isPartitioned(parent, overallField)) { + return CannotRenamePartitionedField(parent, overallField) + } + + val rename = getRenamedOrNull(parent, overallField) + ?: return ok() + + val underlyingField = parent.underlying.getFieldAt(rename.from) + ?: return MissingRename(parent, overallField, rename) + + // In theory, this should have .onError { return it } but the schema has too many violations… + val result = validate(parent, overallField, underlyingField) + + return if (parent is NadelServiceSchemaElement.Object) { + results( + result, + NadelValidatedFieldResult( + service = parent.service, + fieldInstruction = makeRenameFieldInstruction(parent, overallField, rename) + ), + ) + } else { + result + } + } + + context(NadelValidationContext) + private fun makeRenameFieldInstruction( + parent: NadelServiceSchemaElement.FieldsContainer, + overallField: GraphQLFieldDefinition, + rename: NadelRenamedDefinition.Field, + ): NadelFieldInstruction { + val location = makeFieldCoordinates( + parentType = parent.overall, + field = overallField, + ) + + val underlyingName = rename.from.singleOrNull() + ?: return NadelDeepRenameFieldInstruction( + location = location, + queryPathToField = NadelQueryPath(rename.from), + ) + + return NadelRenameFieldInstruction( + location = location, + underlyingName = underlyingName, + ) } context(NadelValidationContext) @@ -138,8 +189,13 @@ class NadelFieldValidation internal constructor( overallField: GraphQLFieldDefinition, underlyingField: GraphQLFieldDefinition, ): NadelSchemaValidationResult { - // This checks whether the output type e.g. name or List or NonNull wrappings are valid - return if (isOutputTypeValid(overallType = overallField.type, underlyingType = underlyingField.type)) { + // Note: the value comes from the underlying schema, so we are supplying the underlying field to the overall field + val isUnderlyingTypeAssignable = assignableTypeValidation.isTypeAssignable( + suppliedType = underlyingField.type, + requiredType = overallField.type, + ) + + return if (isUnderlyingTypeAssignable) { ok() } else { results( @@ -148,25 +204,6 @@ class NadelFieldValidation internal constructor( } } - /** - * It checks whether the type name and type wrappings e.g. [graphql.schema.GraphQLNonNull] make sense. - */ - context(NadelValidationContext) - private fun isOutputTypeValid( - overallType: GraphQLOutputType, - underlyingType: GraphQLOutputType, - ): Boolean { - val isTypeWrappingValid = typeWrappingValidation - .isTypeWrappingValid( - lhs = overallType, - rhs = underlyingType, - rule = LHS_MUST_BE_LOOSER_OR_SAME, - ) - - return isTypeWrappingValid - && getUnderlyingTypeName(overallType.unwrapAll()) == underlyingType.unwrapAll().name - } - context(NadelValidationContext) private fun isCombinedType(type: GraphQLNamedSchemaElement): Boolean { return type.name in combinedTypeNames diff --git a/lib/src/main/java/graphql/nadel/validation/NadelInputValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelInputObjectValidation.kt similarity index 54% rename from lib/src/main/java/graphql/nadel/validation/NadelInputValidation.kt rename to lib/src/main/java/graphql/nadel/validation/NadelInputObjectValidation.kt index fe9345366..2bf057ae1 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelInputValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelInputObjectValidation.kt @@ -1,19 +1,13 @@ package graphql.nadel.validation import graphql.nadel.engine.util.strictAssociateBy -import graphql.nadel.engine.util.unwrapAll -import graphql.nadel.validation.NadelSchemaValidationError.IncompatibleArgumentInputType import graphql.nadel.validation.NadelSchemaValidationError.IncompatibleFieldInputType import graphql.nadel.validation.NadelSchemaValidationError.MissingUnderlyingInputField -import graphql.schema.GraphQLArgument -import graphql.schema.GraphQLFieldDefinition import graphql.schema.GraphQLInputObjectField -import graphql.schema.GraphQLInputType -import graphql.schema.GraphQLUnmodifiedType - -class NadelInputValidation internal constructor() { - private val typeWrappingValidation = NadelTypeWrappingValidation() +class NadelInputObjectValidation internal constructor( + private val assignableTypeValidation: NadelAssignableTypeValidation, +) { context(NadelValidationContext) fun validate( schemaElement: NadelServiceSchemaElement.InputObject, @@ -61,46 +55,10 @@ class NadelInputValidation internal constructor() { overallInputField: GraphQLInputObjectField, underlyingInputField: GraphQLInputObjectField, ): NadelSchemaValidationResult { - return if (!isInputTypeValid(overallInputField.type, underlyingInputField.type)) { + return if (!assignableTypeValidation.isTypeAssignable(overallInputField.type, underlyingInputField.type)) { IncompatibleFieldInputType(parent, overallInputField, underlyingInputField) } else { ok() } } - - context(NadelValidationContext) - fun validate( - parent: NadelServiceSchemaElement.FieldsContainer, - overallField: GraphQLFieldDefinition, - overallInputArgument: GraphQLArgument, - underlyingInputArgument: GraphQLArgument, - ): NadelSchemaValidationResult { - return if (!isInputTypeValid(overallInputArgument.type, underlyingInputArgument.type)) { - IncompatibleArgumentInputType(parent, overallField, overallInputArgument, underlyingInputArgument) - } else { - ok() - } - } - - context(NadelValidationContext) - private fun isInputTypeValid( - overallType: GraphQLInputType, - underlyingType: GraphQLInputType, - ): Boolean { - val typeWrappingValid = typeWrappingValidation.isTypeWrappingValid( - lhs = overallType, - rhs = underlyingType, - rule = NadelTypeWrappingValidation.Rule.LHS_MUST_BE_STRICTER_OR_SAME, - ) - - return typeWrappingValid && isInputTypeNameValid(overallType.unwrapAll(), underlyingType.unwrapAll()) - } - - context(NadelValidationContext) - private fun isInputTypeNameValid( - overallType: GraphQLUnmodifiedType, - underlyingType: GraphQLUnmodifiedType, - ): Boolean { - return getUnderlyingTypeName(overallType) == underlyingType.name - } } diff --git a/lib/src/main/java/graphql/nadel/validation/NadelRenameValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelRenameValidation.kt deleted file mode 100644 index 72178da46..000000000 --- a/lib/src/main/java/graphql/nadel/validation/NadelRenameValidation.kt +++ /dev/null @@ -1,75 +0,0 @@ -package graphql.nadel.validation - -import graphql.nadel.definition.renamed.NadelRenamedDefinition -import graphql.nadel.engine.blueprint.NadelDeepRenameFieldInstruction -import graphql.nadel.engine.blueprint.NadelFieldInstruction -import graphql.nadel.engine.blueprint.NadelRenameFieldInstruction -import graphql.nadel.engine.transform.query.NadelQueryPath -import graphql.nadel.engine.util.getFieldAt -import graphql.nadel.engine.util.makeFieldCoordinates -import graphql.nadel.validation.NadelSchemaValidationError.CannotRenameHydratedField -import graphql.nadel.validation.NadelSchemaValidationError.CannotRenamePartitionedField -import graphql.nadel.validation.NadelSchemaValidationError.MissingRename -import graphql.schema.GraphQLFieldDefinition -import graphql.schema.GraphQLObjectType - -internal class NadelRenameValidation( - private val fieldValidation: NadelFieldValidation, -) { - context(NadelValidationContext) - fun validate( - parent: NadelServiceSchemaElement.FieldsContainer, - overallField: GraphQLFieldDefinition, - ): NadelSchemaValidationResult { - if (isHydrated(parent, overallField)) { - return CannotRenameHydratedField(parent, overallField) - } - - if (isPartitioned(parent, overallField)) { - return CannotRenamePartitionedField(parent, overallField) - } - - val rename = getRenamedOrNull(parent, overallField) - ?: return ok() - - val underlyingField = parent.underlying.getFieldAt(rename.from) - ?: return MissingRename(parent, overallField, rename) - - val result = fieldValidation.validate(parent, overallField, underlyingField) - - return if (parent.overall is GraphQLObjectType) { - results( - result, - NadelValidatedFieldResult( - service = parent.service, - fieldInstruction = makeRenameFieldInstruction(parent, overallField, rename) - ), - ) - } else { - result - } - } - - context(NadelValidationContext) - private fun makeRenameFieldInstruction( - parent: NadelServiceSchemaElement.FieldsContainer, - overallField: GraphQLFieldDefinition, - rename: NadelRenamedDefinition.Field, - ): NadelFieldInstruction { - val location = makeFieldCoordinates( - parentType = parent.overall, - field = overallField, - ) - - val underlyingName = rename.from.singleOrNull() - ?: return NadelDeepRenameFieldInstruction( - location = location, - queryPathToField = NadelQueryPath(rename.from), - ) - - return NadelRenameFieldInstruction( - location = location, - underlyingName = underlyingName, - ) - } -} diff --git a/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidation.kt index 156f6c163..117d70401 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidation.kt @@ -23,7 +23,7 @@ import graphql.schema.GraphQLUnionType class NadelSchemaValidation internal constructor( private val fieldValidation: NadelFieldValidation, - private val inputValidation: NadelInputValidation, + private val inputValidation: NadelInputObjectValidation, private val unionValidation: NadelUnionValidation, private val enumValidation: NadelEnumValidation, private val interfaceValidation: NadelInterfaceValidation, diff --git a/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationFactory.kt b/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationFactory.kt index 63deb7820..15ee46f5e 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationFactory.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelSchemaValidationFactory.kt @@ -1,58 +1,75 @@ package graphql.nadel.validation +import graphql.nadel.validation.hydration.NadelHydrationArgumentTypeValidation +import graphql.nadel.validation.hydration.NadelHydrationArgumentValidation +import graphql.nadel.validation.hydration.NadelHydrationConditionValidation +import graphql.nadel.validation.hydration.NadelHydrationSourceFieldValidation import graphql.nadel.validation.hydration.NadelHydrationValidation +import graphql.nadel.validation.hydration.NadelHydrationVirtualTypeValidation abstract class NadelSchemaValidationFactory { fun create(): NadelSchemaValidation { + val definitionParser = NadelDefinitionParser( + hook = hook, + idHydrationDefinitionParser = idHydrationDefinitionParser, + ) + return NadelSchemaValidation( fieldValidation = fieldValidation, - inputValidation = inputValidation, + inputValidation = inputObjectValidation, unionValidation = unionValidation, enumValidation = enumValidation, interfaceValidation = interfaceValidation, namespaceValidation = namespaceValidation, virtualTypeValidation = virtualTypeValidation, - definitionParser = NadelDefinitionParser( - hook = hook, - idHydrationDefinitionParser = NadelIdHydrationDefinitionParser(), - ), + definitionParser = definitionParser, hook = hook, ) } - protected val hydrationValidation: NadelHydrationValidation by lazy { - NadelHydrationValidation() - } + private val idHydrationDefinitionParser = NadelIdHydrationDefinitionParser() - protected val fieldValidation: NadelFieldValidation by lazy { - NadelFieldValidation(hydrationValidation) - } + private val partitionValidation = NadelPartitionValidation() - protected val virtualTypeValidation: NadelVirtualTypeValidation by lazy { - NadelVirtualTypeValidation(hydrationValidation) - } + private val typeWrappingValidation = NadelTypeWrappingValidation() - protected val inputValidation: NadelInputValidation by lazy { - NadelInputValidation() - } + private val assignableTypeValidation = NadelAssignableTypeValidation(typeWrappingValidation) - protected val unionValidation: NadelUnionValidation by lazy { - NadelUnionValidation() - } + private val inputObjectValidation = NadelInputObjectValidation(assignableTypeValidation) - protected val enumValidation: NadelEnumValidation by lazy { - NadelEnumValidation() - } + private val hydrationArgumentTypeValidation = NadelHydrationArgumentTypeValidation(typeWrappingValidation) + private val hydrationArgumentValidation = NadelHydrationArgumentValidation(hydrationArgumentTypeValidation) + private val hydrationConditionValidation = NadelHydrationConditionValidation() + private val hydrationSourceFieldValidation = NadelHydrationSourceFieldValidation() + private val hydrationVirtualTypeValidation = NadelHydrationVirtualTypeValidation() - protected val interfaceValidation: NadelInterfaceValidation by lazy { - NadelInterfaceValidation() - } + private val hydrationValidation = NadelHydrationValidation( + argumentValidation = hydrationArgumentValidation, + conditionValidation = hydrationConditionValidation, + sourceFieldValidation = hydrationSourceFieldValidation, + virtualTypeValidation = hydrationVirtualTypeValidation, + ) - protected val namespaceValidation: NadelNamespaceValidation by lazy { - NadelNamespaceValidation() - } + private val fieldValidation = NadelFieldValidation( + hydrationValidation = hydrationValidation, + partitionValidation = partitionValidation, + assignableTypeValidation = assignableTypeValidation, + ) + + private val virtualTypeValidation = NadelVirtualTypeValidation( + hydrationValidation = hydrationValidation, + assignableTypeValidation = assignableTypeValidation, + ) + + private val unionValidation = NadelUnionValidation() + + private val enumValidation = NadelEnumValidation() + + private val interfaceValidation = NadelInterfaceValidation() + + private val namespaceValidation = NadelNamespaceValidation() - protected open val hook: NadelSchemaValidationHook + open val hook: NadelSchemaValidationHook get() = object : NadelSchemaValidationHook() { } diff --git a/lib/src/main/java/graphql/nadel/validation/NadelTypeValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelTypeValidation.kt index d2f4ac963..2fee3060e 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelTypeValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelTypeValidation.kt @@ -18,7 +18,7 @@ import graphql.schema.GraphQLUnionType internal class NadelTypeValidation( private val fieldValidation: NadelFieldValidation, - private val inputValidation: NadelInputValidation, + private val inputValidation: NadelInputObjectValidation, private val unionValidation: NadelUnionValidation, private val enumValidation: NadelEnumValidation, private val interfaceValidation: NadelInterfaceValidation, @@ -262,15 +262,12 @@ internal class NadelTypeValidation( return engineSchema.operationTypes.any { it.name == typeName } } + /** + * @return true to visit + */ context(NadelValidationContext) private fun visitElement(schemaElement: NadelServiceSchemaElement): Boolean { - val schemaElementRef = schemaElement.toRef() - - return if (schemaElementRef in visitedTypes) { - false // Already visited - } else { - visitedTypes.add(schemaElementRef) - true - } + // Returns true if the element was added i.e. haven't visited before + return visitedTypes.add(schemaElement.toRef()) } } diff --git a/lib/src/main/java/graphql/nadel/validation/NadelVirtualTypeValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelVirtualTypeValidation.kt index c99aa8ac1..a878fbca4 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelVirtualTypeValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelVirtualTypeValidation.kt @@ -3,8 +3,6 @@ package graphql.nadel.validation import graphql.nadel.Service import graphql.nadel.definition.virtualType.hasVirtualTypeDefinition import graphql.nadel.engine.util.unwrapAll -import graphql.nadel.validation.NadelTypeWrappingValidation.Rule.LHS_MUST_BE_LOOSER_OR_SAME -import graphql.nadel.validation.NadelTypeWrappingValidation.Rule.LHS_MUST_BE_STRICTER_OR_SAME import graphql.nadel.validation.NadelVirtualTypeValidationContext.TraversalOutcome.DUPLICATED_BACKING_TYPE import graphql.nadel.validation.NadelVirtualTypeValidationContext.TraversalOutcome.SKIP import graphql.nadel.validation.NadelVirtualTypeValidationContext.TraversalOutcome.VISIT @@ -43,9 +41,8 @@ private class NadelVirtualTypeValidationContext { class NadelVirtualTypeValidation internal constructor( private val hydrationValidation: NadelHydrationValidation, + private val assignableTypeValidation: NadelAssignableTypeValidation, ) { - private val typeWrappingValidation = NadelTypeWrappingValidation() - context(NadelValidationContext) fun validate( schemaElement: NadelServiceSchemaElement.VirtualType, @@ -186,23 +183,21 @@ class NadelVirtualTypeValidation internal constructor( ) } - if (virtualFieldUnwrappedOutputType.name == backingFieldUnwrappedOutputType.name) { - val isTypeWrappingValid = typeWrappingValidation.isTypeWrappingValid( - lhs = virtualField.type, - rhs = backingField.type, - rule = LHS_MUST_BE_LOOSER_OR_SAME, - ) + // Note: the value comes from the backing field, and that value needs to fit the virtual field + val isOutputTypeAssignable = assignableTypeValidation.isTypeAssignable( + suppliedType = backingField.type, + requiredType = virtualField.type, + ) - if (isTypeWrappingValid) { - return ok() - } + return if (isOutputTypeAssignable) { + NadelVirtualTypeIncompatibleFieldOutputTypeError( + parent = parent, + virtualField = virtualField, + backingField = backingField, + ) + } else { + ok() } - - return NadelVirtualTypeIncompatibleFieldOutputTypeError( - parent = parent, - virtualField = virtualField, - backingField = backingField, - ) } /** @@ -249,25 +244,23 @@ class NadelVirtualTypeValidation internal constructor( virtualFieldArgument: GraphQLArgument, backingFieldArgument: GraphQLArgument, ): NadelSchemaValidationResult { - if (virtualFieldArgument.type.unwrapAll().name == backingFieldArgument.type.unwrapAll().name) { - val isTypeWrappingValid = typeWrappingValidation.isTypeWrappingValid( - lhs = virtualFieldArgument.type, - rhs = backingFieldArgument.type, - rule = LHS_MUST_BE_STRICTER_OR_SAME, - ) + // Note: the value comes from the virtual field's arg and needs to be assigned to the backing arg + val isInputTypeAssignable = assignableTypeValidation.isTypeAssignable( + suppliedType = virtualFieldArgument.type, + requiredType = backingFieldArgument.type, + ) - if (isTypeWrappingValid) { - return ok() - } + return if (isInputTypeAssignable) { + ok() + } else { + NadelVirtualTypeIncompatibleFieldArgumentError( + type = parent, + virtualField = virtualField, + backingField = backingField, + virtualFieldArgument = virtualFieldArgument, + backingFieldArgument = backingFieldArgument, + ) } - - return NadelVirtualTypeIncompatibleFieldArgumentError( - type = parent, - virtualField = virtualField, - backingField = backingField, - virtualFieldArgument = virtualFieldArgument, - backingFieldArgument = backingFieldArgument, - ) } /** diff --git a/lib/src/main/java/graphql/nadel/validation/hydration/NadelHydrationArgumentTypeValidation.kt b/lib/src/main/java/graphql/nadel/validation/hydration/NadelHydrationArgumentTypeValidation.kt index c1fc4a2d5..412520945 100644 --- a/lib/src/main/java/graphql/nadel/validation/hydration/NadelHydrationArgumentTypeValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/hydration/NadelHydrationArgumentTypeValidation.kt @@ -59,9 +59,9 @@ internal sealed class NadelHydrationArgumentTypeValidationResult { ) : Error() } -internal class NadelHydrationArgumentTypeValidation { - private val typeWrappingValidation = NadelTypeWrappingValidation() - +internal class NadelHydrationArgumentTypeValidation( + private val typeWrappingValidation: NadelTypeWrappingValidation, +) { fun isAssignable( isBatchHydration: Boolean, hydrationArgumentDefinition: NadelHydrationArgumentDefinition, diff --git a/lib/src/main/java/graphql/nadel/validation/hydration/NadelHydrationArgumentValidation.kt b/lib/src/main/java/graphql/nadel/validation/hydration/NadelHydrationArgumentValidation.kt index 187fe03f3..78bdfab54 100644 --- a/lib/src/main/java/graphql/nadel/validation/hydration/NadelHydrationArgumentValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/hydration/NadelHydrationArgumentValidation.kt @@ -48,7 +48,9 @@ private data class NadelHydrationArgumentValidationContext( val isBatchHydration: Boolean, ) -internal class NadelHydrationArgumentValidation { +internal class NadelHydrationArgumentValidation( + private val hydrationArgumentTypeValidation: NadelHydrationArgumentTypeValidation, +) { context(NadelValidationContext, NadelHydrationValidationContext) fun validateArguments( isBatchHydration: Boolean, @@ -296,7 +298,7 @@ internal class NadelHydrationArgumentValidation { ): NadelSchemaValidationResult { val backingFieldArg = backingField.getArgument(hydrationArgumentDefinition.name) - NadelHydrationArgumentTypeValidation() + hydrationArgumentTypeValidation .isAssignable( isBatchHydration = isBatchHydration, hydrationArgumentDefinition = hydrationArgumentDefinition, diff --git a/lib/src/main/java/graphql/nadel/validation/hydration/NadelHydrationValidation.kt b/lib/src/main/java/graphql/nadel/validation/hydration/NadelHydrationValidation.kt index e3bc5ac1a..59c50dfb0 100644 --- a/lib/src/main/java/graphql/nadel/validation/hydration/NadelHydrationValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/hydration/NadelHydrationValidation.kt @@ -71,11 +71,12 @@ internal data class NadelHydrationValidationContext( val backingField: GraphQLFieldDefinition, ) -class NadelHydrationValidation internal constructor() { - private val argumentValidation = NadelHydrationArgumentValidation() - private val conditionValidation = NadelHydrationConditionValidation() - private val sourceFieldValidation = NadelHydrationSourceFieldValidation() - private val virtualTypeValidation = NadelHydrationVirtualTypeValidation() +class NadelHydrationValidation internal constructor( + private val argumentValidation: NadelHydrationArgumentValidation, + private val conditionValidation: NadelHydrationConditionValidation, + private val sourceFieldValidation: NadelHydrationSourceFieldValidation, + private val virtualTypeValidation: NadelHydrationVirtualTypeValidation, +) { context(NadelValidationContext) fun validate( From 37df4f78c7bcc31704a0e7b1ae02c42d98a9e485 Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Wed, 27 Nov 2024 12:38:42 +1300 Subject: [PATCH 2/4] Replace context overloads with archunit test --- .../hydration/NadelHydrationDefinition.kt | 13 ------ .../renamed/NadelRenamedDefinition.kt | 18 -------- .../NadelValidationDefinitionsTest.kt | 44 +++++++++++++++++++ 3 files changed, 44 insertions(+), 31 deletions(-) create mode 100644 lib/src/test/kotlin/graphql/nadel/validation/NadelValidationDefinitionsTest.kt diff --git a/lib/src/main/java/graphql/nadel/definition/hydration/NadelHydrationDefinition.kt b/lib/src/main/java/graphql/nadel/definition/hydration/NadelHydrationDefinition.kt index db186a6be..c223a8545 100644 --- a/lib/src/main/java/graphql/nadel/definition/hydration/NadelHydrationDefinition.kt +++ b/lib/src/main/java/graphql/nadel/definition/hydration/NadelHydrationDefinition.kt @@ -8,7 +8,6 @@ import graphql.nadel.definition.NadelDefinition import graphql.nadel.definition.hydration.NadelHydrationDefinition.Keyword import graphql.nadel.engine.util.JsonMap import graphql.nadel.engine.util.parseDefinition -import graphql.nadel.validation.NadelValidationContext import graphql.schema.GraphQLAppliedDirective import graphql.schema.GraphQLFieldDefinition @@ -16,22 +15,10 @@ fun FieldDefinition.hasHydratedDefinition(): Boolean { return hasDirective(Keyword.hydrated) } -context(NadelValidationContext) -@Deprecated("Mistake to use this inside validation", level = DeprecationLevel.ERROR) -fun GraphQLFieldDefinition.hasHydratedDefinition(): Nothing { - throw UnsupportedOperationException() -} - fun GraphQLFieldDefinition.hasHydratedDefinition(): Boolean { return hasAppliedDirective(Keyword.hydrated) } -context(NadelValidationContext) -@Deprecated("Mistake to use this inside validation", level = DeprecationLevel.ERROR) -fun GraphQLFieldDefinition.parseHydrationDefinitions(): Nothing { - throw UnsupportedOperationException() -} - fun GraphQLFieldDefinition.parseHydrationDefinitions(): List { return getAppliedDirectives(Keyword.hydrated) .map(::NadelHydrationDefinitionImpl) diff --git a/lib/src/main/java/graphql/nadel/definition/renamed/NadelRenamedDefinition.kt b/lib/src/main/java/graphql/nadel/definition/renamed/NadelRenamedDefinition.kt index c7a0478d0..058ebd5a8 100644 --- a/lib/src/main/java/graphql/nadel/definition/renamed/NadelRenamedDefinition.kt +++ b/lib/src/main/java/graphql/nadel/definition/renamed/NadelRenamedDefinition.kt @@ -48,12 +48,6 @@ sealed class NadelRenamedDefinition : NadelDefinition { } } -context(NadelValidationContext) -@Deprecated("Mistake to use this inside validation", level = DeprecationLevel.ERROR) -fun GraphQLDirectiveContainer.hasRenameDefinition(): Nothing { - throw UnsupportedOperationException() -} - fun GraphQLDirectiveContainer.hasRenameDefinition(): Boolean { return hasAppliedDirective(Keyword.renamed) } @@ -62,12 +56,6 @@ fun DirectivesContainer<*>.hasRenameDefinition(): Boolean { return hasDirective(Keyword.renamed) } -context(NadelValidationContext) -@Deprecated("Mistake to use this inside validation", level = DeprecationLevel.ERROR) -fun GraphQLFieldDefinition.parseRenamedOrNull(): Nothing { - throw UnsupportedOperationException() -} - fun GraphQLFieldDefinition.parseRenamedOrNull(): NadelRenamedDefinition.Field? { val directive = getAppliedDirective(Keyword.renamed) ?: return null @@ -75,12 +63,6 @@ fun GraphQLFieldDefinition.parseRenamedOrNull(): NadelRenamedDefinition.Field? { return NadelRenamedDefinition.Field(directive) } -context(NadelValidationContext) -@Deprecated("Mistake to use this inside validation", level = DeprecationLevel.ERROR) -fun GraphQLNamedType.parseRenamedOrNull(): Nothing { - throw UnsupportedOperationException() -} - fun GraphQLNamedType.parseRenamedOrNull(): NadelRenamedDefinition.Type? { val directive = (this as GraphQLDirectiveContainer).getAppliedDirective(Keyword.renamed) ?: return null diff --git a/lib/src/test/kotlin/graphql/nadel/validation/NadelValidationDefinitionsTest.kt b/lib/src/test/kotlin/graphql/nadel/validation/NadelValidationDefinitionsTest.kt new file mode 100644 index 000000000..210565c25 --- /dev/null +++ b/lib/src/test/kotlin/graphql/nadel/validation/NadelValidationDefinitionsTest.kt @@ -0,0 +1,44 @@ +package graphql.nadel.validation + +import com.tngtech.archunit.base.DescribedPredicate +import com.tngtech.archunit.base.DescribedPredicate.not +import com.tngtech.archunit.core.domain.JavaClass +import com.tngtech.archunit.core.domain.JavaMethodCall +import com.tngtech.archunit.core.importer.ClassFileImporter +import com.tngtech.archunit.core.importer.ImportOption +import com.tngtech.archunit.lang.conditions.ArchConditions.callMethodWhere +import com.tngtech.archunit.lang.conditions.ArchConditions.not +import com.tngtech.archunit.lang.syntax.ArchRuleDefinition.classes +import kotlin.test.Test + +class NadelValidationDefinitionsTest { + @Test + fun `do not use parse definitions functions in validation`() { + val importedClasses = ClassFileImporter() + .withImportOption(ImportOption.DoNotIncludeTests()) + .importPackages("graphql.nadel.validation") + + val rule = classes() + .that( + not( + JavaClass.Predicates.belongTo( + JavaClass.Predicates.simpleNameEndingWith("DefinitionParser") + ), + ), + ) + .should( + not( + callMethodWhere( + object : DescribedPredicate("definition parse methods") { + override fun test(invocation: JavaMethodCall): Boolean { + return invocation.targetOwner.getPackage().name.startsWith("graphql.nadel.definition") + && invocation.target.name.startsWith("parse") + } + } + ), + ), + ) + + rule.check(importedClasses) + } +} From 196867ae789595387068d98eb115e6df09000fd6 Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Thu, 28 Nov 2024 12:36:56 +1300 Subject: [PATCH 3/4] Fix stuff --- .../NadelAssignableTypeValidation.kt | 50 ++++++++++++----- .../nadel/validation/NadelFieldValidation.kt | 12 ++--- .../validation/NadelInputObjectValidation.kt | 11 ++-- .../validation/NadelValidationContext.kt | 7 +++ .../validation/NadelVirtualTypeValidation.kt | 54 +++++++++++++++---- .../NadelVirtualTypeValidationError.kt | 9 ++++ .../NadelVirtualTypeValidationTest.kt | 2 +- 7 files changed, 113 insertions(+), 32 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/validation/NadelAssignableTypeValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelAssignableTypeValidation.kt index 4b70ea0c1..6a3c01e1a 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelAssignableTypeValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelAssignableTypeValidation.kt @@ -2,30 +2,56 @@ package graphql.nadel.validation import graphql.nadel.engine.util.unwrapAll import graphql.schema.GraphQLType -import graphql.schema.GraphQLUnmodifiedType class NadelAssignableTypeValidation internal constructor( private val typeWrappingValidation: NadelTypeWrappingValidation, ) { + context(NadelValidationContext) + fun isOutputTypeAssignable( + overallType: GraphQLType, + underlyingType: GraphQLType, + ): Boolean { + return isTypeAssignable( + suppliedType = underlyingType, + requiredType = overallType, + // Compare underlying type names + suppliedTypeName = underlyingType.unwrapAll().name, + requiredTypeName = getUnderlyingTypeName(overallType.unwrapAll()), + ) + } + + context(NadelValidationContext) + fun isInputTypeAssignable( + overallType: GraphQLType, + underlyingType: GraphQLType, + ): Boolean { + return isTypeAssignable( + suppliedType = overallType, + requiredType = underlyingType, + // Compare underlying type names + suppliedTypeName = getUnderlyingTypeName(overallType.unwrapAll()), + requiredTypeName = underlyingType.unwrapAll().name, + ) + } + context(NadelValidationContext) fun isTypeAssignable( suppliedType: GraphQLType, requiredType: GraphQLType, + suppliedTypeName: String, + requiredTypeName: String, ): Boolean { - val typeWrappingValid = typeWrappingValidation.isTypeWrappingValid( + return suppliedTypeName == requiredTypeName && isTypeWrappingValid(suppliedType, requiredType) + } + + private fun isTypeWrappingValid( + suppliedType: GraphQLType, + requiredType: GraphQLType, + ): Boolean { + return typeWrappingValidation.isTypeWrappingValid( lhs = suppliedType, rhs = requiredType, rule = NadelTypeWrappingValidation.Rule.LHS_MUST_BE_STRICTER_OR_SAME, ) - - return typeWrappingValid && isTypeNameValid(suppliedType.unwrapAll(), requiredType.unwrapAll()) - } - - context(NadelValidationContext) - private fun isTypeNameValid( - overallType: GraphQLUnmodifiedType, - underlyingType: GraphQLUnmodifiedType, - ): Boolean { - return getUnderlyingTypeName(overallType) == underlyingType.name } } diff --git a/lib/src/main/java/graphql/nadel/validation/NadelFieldValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelFieldValidation.kt index f291d792b..41b217012 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelFieldValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelFieldValidation.kt @@ -101,9 +101,9 @@ class NadelFieldValidation internal constructor( } else { // Note: the value comes from the user (overall schema) // So we are supplying the overall argument to the underlying argument - val isArgumentTypeAssignable = assignableTypeValidation.isTypeAssignable( - suppliedType = overallArg.type, - requiredType = underlyingArg.type + val isArgumentTypeAssignable = assignableTypeValidation.isInputTypeAssignable( + overallType = overallArg.type, + underlyingType = underlyingArg.type ) if (isArgumentTypeAssignable) { ok() @@ -190,9 +190,9 @@ class NadelFieldValidation internal constructor( underlyingField: GraphQLFieldDefinition, ): NadelSchemaValidationResult { // Note: the value comes from the underlying schema, so we are supplying the underlying field to the overall field - val isUnderlyingTypeAssignable = assignableTypeValidation.isTypeAssignable( - suppliedType = underlyingField.type, - requiredType = overallField.type, + val isUnderlyingTypeAssignable = assignableTypeValidation.isOutputTypeAssignable( + overallType = overallField.type, + underlyingType = underlyingField.type, ) return if (isUnderlyingTypeAssignable) { diff --git a/lib/src/main/java/graphql/nadel/validation/NadelInputObjectValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelInputObjectValidation.kt index 2bf057ae1..01febeba9 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelInputObjectValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelInputObjectValidation.kt @@ -55,10 +55,15 @@ class NadelInputObjectValidation internal constructor( overallInputField: GraphQLInputObjectField, underlyingInputField: GraphQLInputObjectField, ): NadelSchemaValidationResult { - return if (!assignableTypeValidation.isTypeAssignable(overallInputField.type, underlyingInputField.type)) { - IncompatibleFieldInputType(parent, overallInputField, underlyingInputField) - } else { + val isTypeAssignable = assignableTypeValidation.isInputTypeAssignable( + overallType = overallInputField.type, + underlyingType = underlyingInputField.type + ) + + return if (isTypeAssignable) { ok() + } else { + IncompatibleFieldInputType(parent, overallInputField, underlyingInputField) } } } diff --git a/lib/src/main/java/graphql/nadel/validation/NadelValidationContext.kt b/lib/src/main/java/graphql/nadel/validation/NadelValidationContext.kt index f179c41fe..f189dac1e 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelValidationContext.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelValidationContext.kt @@ -69,6 +69,13 @@ fun getHydrationDefinitions( .filterIsInstance() } +context(NadelValidationContext) +fun isRenamed( + container: NadelServiceSchemaElement.Type, +): Boolean { + return getRenamedOrNull(container.overall) != null +} + context(NadelValidationContext) fun isRenamed( container: GraphQLFieldsContainer, diff --git a/lib/src/main/java/graphql/nadel/validation/NadelVirtualTypeValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelVirtualTypeValidation.kt index a878fbca4..4bab261f4 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelVirtualTypeValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelVirtualTypeValidation.kt @@ -62,6 +62,10 @@ class NadelVirtualTypeValidation internal constructor( SKIP -> return ok() } + if (isRenamed(schemaElement)) { + return NadelVirtualTypeIllegalRenameError(schemaElement) + } + if (schemaElement.overall is GraphQLObjectType && schemaElement.underlying is GraphQLObjectType) { return validateType( service = schemaElement.service, @@ -184,19 +188,19 @@ class NadelVirtualTypeValidation internal constructor( } // Note: the value comes from the backing field, and that value needs to fit the virtual field - val isOutputTypeAssignable = assignableTypeValidation.isTypeAssignable( - suppliedType = backingField.type, - requiredType = virtualField.type, + val isOutputTypeAssignable = isOutputTypeAssignable( + backingField = backingField, + virtualField = virtualField, ) return if (isOutputTypeAssignable) { + ok() + } else { NadelVirtualTypeIncompatibleFieldOutputTypeError( parent = parent, virtualField = virtualField, backingField = backingField, ) - } else { - ok() } } @@ -244,11 +248,7 @@ class NadelVirtualTypeValidation internal constructor( virtualFieldArgument: GraphQLArgument, backingFieldArgument: GraphQLArgument, ): NadelSchemaValidationResult { - // Note: the value comes from the virtual field's arg and needs to be assigned to the backing arg - val isInputTypeAssignable = assignableTypeValidation.isTypeAssignable( - suppliedType = virtualFieldArgument.type, - requiredType = backingFieldArgument.type, - ) + val isInputTypeAssignable = isInputTypeAssignable(virtualFieldArgument, backingFieldArgument) return if (isInputTypeAssignable) { ok() @@ -287,4 +287,38 @@ class NadelVirtualTypeValidation internal constructor( } }.toResult() } + + context(NadelValidationContext, NadelVirtualTypeValidationContext) + private fun isInputTypeAssignable( + virtualFieldArgument: GraphQLArgument, + backingFieldArgument: GraphQLArgument, + ): Boolean { + val suppliedType = virtualFieldArgument.type + val requiredType = backingFieldArgument.type + + return assignableTypeValidation.isTypeAssignable( + suppliedType = suppliedType, + requiredType = requiredType, + // Note: we do not check for renames here, types must be used 1-1 + suppliedTypeName = suppliedType.unwrapAll().name, + requiredTypeName = requiredType.unwrapAll().name, + ) + } + + context(NadelValidationContext, NadelVirtualTypeValidationContext) + private fun isOutputTypeAssignable( + backingField: GraphQLFieldDefinition, + virtualField: GraphQLFieldDefinition, + ): Boolean { + val suppliedType = backingField.type + val requiredType = virtualField.type + + return assignableTypeValidation.isTypeAssignable( + suppliedType = suppliedType, + requiredType = requiredType, + // Note: we do not check for renames here, types must be used 1-1 + suppliedTypeName = suppliedType.unwrapAll().name, + requiredTypeName = requiredType.unwrapAll().name, + ) + } } diff --git a/lib/src/main/java/graphql/nadel/validation/NadelVirtualTypeValidationError.kt b/lib/src/main/java/graphql/nadel/validation/NadelVirtualTypeValidationError.kt index c5b54e81f..2b65e9b1c 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelVirtualTypeValidationError.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelVirtualTypeValidationError.kt @@ -23,6 +23,15 @@ data class NadelVirtualTypeDuplicationError( get() = type.overall } +data class NadelVirtualTypeIllegalRenameError( + val type: NadelServiceSchemaElement.VirtualType, +) : NadelSchemaValidationError { + override val message: String = "Virtual types cannot be renamed" + + override val subject: GraphQLNamedSchemaElement + get() = type.overall +} + data class NadelVirtualTypeMissingBackingFieldError( val type: NadelServiceSchemaElement.VirtualType, val virtualField: GraphQLFieldDefinition, diff --git a/lib/src/test/kotlin/graphql/nadel/validation/NadelVirtualTypeValidationTest.kt b/lib/src/test/kotlin/graphql/nadel/validation/NadelVirtualTypeValidationTest.kt index 71b8c266b..642950ccd 100644 --- a/lib/src/test/kotlin/graphql/nadel/validation/NadelVirtualTypeValidationTest.kt +++ b/lib/src/test/kotlin/graphql/nadel/validation/NadelVirtualTypeValidationTest.kt @@ -376,7 +376,7 @@ class NadelVirtualTypeValidationTest { } @Test - fun `can reference `() { + fun `can use original field output type in virtual type`() { // Given val fixture = makeFixture( overallSchema = mapOf( From 48566fae1af7a9f12971d4c7c98ee649cd8d8e9d Mon Sep 17 00:00:00 2001 From: Franklin Wang Date: Mon, 2 Dec 2024 14:42:43 +1100 Subject: [PATCH 4/4] Update doco --- .../graphql/nadel/validation/NadelAssignableTypeValidation.kt | 2 ++ .../main/java/graphql/nadel/validation/NadelFieldValidation.kt | 3 --- .../graphql/nadel/validation/NadelVirtualTypeValidation.kt | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/src/main/java/graphql/nadel/validation/NadelAssignableTypeValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelAssignableTypeValidation.kt index 6a3c01e1a..7b7d6cc43 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelAssignableTypeValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelAssignableTypeValidation.kt @@ -11,6 +11,7 @@ class NadelAssignableTypeValidation internal constructor( overallType: GraphQLType, underlyingType: GraphQLType, ): Boolean { + // Note: the (supplied) value comes from the underlying service, and that value needs to fit the (required) overall field's type return isTypeAssignable( suppliedType = underlyingType, requiredType = overallType, @@ -25,6 +26,7 @@ class NadelAssignableTypeValidation internal constructor( overallType: GraphQLType, underlyingType: GraphQLType, ): Boolean { + // Note: the (supplied) value comes from the user (overall schema) and needs to be assigned to the (required) underlying type return isTypeAssignable( suppliedType = overallType, requiredType = underlyingType, diff --git a/lib/src/main/java/graphql/nadel/validation/NadelFieldValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelFieldValidation.kt index 41b217012..33fe0fbc2 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelFieldValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelFieldValidation.kt @@ -99,8 +99,6 @@ class NadelFieldValidation internal constructor( if (underlyingArg == null) { MissingArgumentOnUnderlying(parent, overallField, underlyingField, overallArg) } else { - // Note: the value comes from the user (overall schema) - // So we are supplying the overall argument to the underlying argument val isArgumentTypeAssignable = assignableTypeValidation.isInputTypeAssignable( overallType = overallArg.type, underlyingType = underlyingArg.type @@ -189,7 +187,6 @@ class NadelFieldValidation internal constructor( overallField: GraphQLFieldDefinition, underlyingField: GraphQLFieldDefinition, ): NadelSchemaValidationResult { - // Note: the value comes from the underlying schema, so we are supplying the underlying field to the overall field val isUnderlyingTypeAssignable = assignableTypeValidation.isOutputTypeAssignable( overallType = overallField.type, underlyingType = underlyingField.type, diff --git a/lib/src/main/java/graphql/nadel/validation/NadelVirtualTypeValidation.kt b/lib/src/main/java/graphql/nadel/validation/NadelVirtualTypeValidation.kt index 4bab261f4..09eb1de56 100644 --- a/lib/src/main/java/graphql/nadel/validation/NadelVirtualTypeValidation.kt +++ b/lib/src/main/java/graphql/nadel/validation/NadelVirtualTypeValidation.kt @@ -187,7 +187,6 @@ class NadelVirtualTypeValidation internal constructor( ) } - // Note: the value comes from the backing field, and that value needs to fit the virtual field val isOutputTypeAssignable = isOutputTypeAssignable( backingField = backingField, virtualField = virtualField, @@ -310,6 +309,7 @@ class NadelVirtualTypeValidation internal constructor( backingField: GraphQLFieldDefinition, virtualField: GraphQLFieldDefinition, ): Boolean { + // Note: the (supplied) value comes from the backing service, and that value needs to fit the (required) virtual field's type val suppliedType = backingField.type val requiredType = virtualField.type