Improve schema validation factory (#638)

* Improve factory and dependency flow

* Replace context overloads with archunit test

* Fix stuff

* Update doco

lib/src/main/java/graphql/nadel/definition/hydration/NadelHydrationDefinition.kt

@@ -8,30 +8,17 @@ 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
 
 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<NadelHydrationDefinition> {
     return getAppliedDirectives(Keyword.hydrated)
         .map(::NadelHydrationDefinitionImpl)

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,25 +56,13 @@ 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
 
     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

lib/src/main/java/graphql/nadel/validation/NadelAssignableTypeValidation.kt

@@ -0,0 +1,59 @@
+package graphql.nadel.validation
+
+import graphql.nadel.engine.util.unwrapAll
+import graphql.schema.GraphQLType
+
+class NadelAssignableTypeValidation internal constructor(
+    private val typeWrappingValidation: NadelTypeWrappingValidation,
+) {
+    context(NadelValidationContext)
+    fun isOutputTypeAssignable(
+        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,
+            // Compare underlying type names
+            suppliedTypeName = underlyingType.unwrapAll().name,
+            requiredTypeName = getUnderlyingTypeName(overallType.unwrapAll()),
+        )
+    }
+
+    context(NadelValidationContext)
+    fun isInputTypeAssignable(
+        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,
+            // 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 {
+        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,
+        )
+    }
+}

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,
@@ -79,7 +82,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 {
@@ -108,14 +111,12 @@ 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
-                            )
+                    val isArgumentTypeAssignable = assignableTypeValidation.isInputTypeAssignable(
+                        overallType = overallArg.type,
+                        underlyingType = underlyingArg.type
+                    )
+                    if (isArgumentTypeAssignable) {
+                        ok()
                     } else {
                         IncompatibleArgumentInputType(
                             parentType = parent,
@@ -135,13 +136,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)
@@ -150,8 +199,12 @@ 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)) {
+        val isUnderlyingTypeAssignable = assignableTypeValidation.isOutputTypeAssignable(
+            overallType = overallField.type,
+            underlyingType = underlyingField.type,
+        )
+
+        return if (isUnderlyingTypeAssignable) {
             ok()
         } else {
             results(
@@ -160,25 +213,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

lib/src/main/java/graphql/nadel/validation/NadelInputValidation.kt → 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,15 @@ class NadelInputValidation internal constructor() {
         overallInputField: GraphQLInputObjectField,
         underlyingInputField: GraphQLInputObjectField,
     ): NadelSchemaValidationResult {
-        return if (!isInputTypeValid(overallInputField.type, underlyingInputField.type)) {
-            IncompatibleFieldInputType(parent, overallInputField, underlyingInputField)
-        } else {
-            ok()
-        }
-    }
+        val isTypeAssignable = assignableTypeValidation.isInputTypeAssignable(
+            overallType = overallInputField.type,
+            underlyingType = underlyingInputField.type
+        )
 
-    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 {
+        return if (isTypeAssignable) {
             ok()
+        } else {
+            IncompatibleFieldInputType(parent, overallInputField, underlyingInputField)
         }
     }
-
-    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
-    }
 }