Fix stuff

atlassian-labs · Nov 27, 2024 · 9fbd4d7 · 9fbd4d7
1 parent eb7bc7d
commit 9fbd4d7
Show file tree

Hide file tree

Showing 7 changed files with 113 additions and 32 deletions.
diff --git 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
@@ -93,9 +93,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()
@@ -182,9 +182,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
@@ -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
@@ -69,6 +69,13 @@ fun getHydrationDefinitions(
         .filterIsInstance<NadelHydrationDefinition>()
 }
 
+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
@@ -40,6 +40,10 @@ class NadelVirtualTypeValidation internal constructor(
             return ok()
         }
 
+        if (isRenamed(schemaElement)) {
+            return NadelVirtualTypeIllegalRenameError(schemaElement)
+        }
+
         if (schemaElement.overall is GraphQLObjectType && schemaElement.underlying is GraphQLObjectType) {
             return validateType(
                 service = schemaElement.service,
@@ -162,19 +166,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()
         }
     }
 
@@ -222,11 +226,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()
@@ -265,4 +265,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
@@ -14,6 +14,15 @@ data class NadelVirtualTypeIllegalTypeError(
         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
@@ -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(