Don't automatically add key fields to union selections (#5562)

* Perform directive validation on interfaces and unions, and add validation for embeddedFields and connectionFields

* Refactor type policy validation, and simplify message

libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/internal/SchemaValidationScope.kt

@@ -209,6 +209,7 @@ internal fun validateSchema(definitions: List<GQLDefinition>, requiresApolloDefi
 
   mergedScope.validateInterfaces()
   mergedScope.validateObjects()
+  mergedScope.validateUnions()
   mergedScope.validateInputObjects()
   mergedScope.validateCatch(mergedSchemaDefinition)
 
@@ -433,9 +434,30 @@ internal fun ValidationScope.validateRootOperationTypes(schemaDefinition: GQLSch
 }
 
 private fun ValidationScope.validateInterfaces() {
-  typeDefinitions.values.filterIsInstance<GQLInterfaceTypeDefinition>().forEach {
-    if (it.fields.isEmpty()) {
-      registerIssue("Interfaces must specify one or more fields", it.sourceLocation)
+  typeDefinitions.values.filterIsInstance<GQLInterfaceTypeDefinition>().forEach { i ->
+    if (i.fields.isEmpty()) {
+      registerIssue("Interfaces must specify one or more fields", i.sourceLocation)
+    }
+
+    i.implementsInterfaces.forEach { implementsInterface ->
+      val iface = typeDefinitions[implementsInterface] as? GQLInterfaceTypeDefinition
+      if (iface == null) {
+        registerIssue("Interface '${i.name}' cannot implement non-interface '$implementsInterface'", i.sourceLocation)
+      }
+    }
+
+    i.directives.forEach { directive ->
+      validateDirective(directive, i) {
+        issues.add(it.constContextError())
+      }
+    }
+
+    i.fields.forEach { gqlFieldDefinition ->
+      gqlFieldDefinition.directives.forEach { gqlDirective ->
+        validateDirective(gqlDirective, gqlFieldDefinition) {
+          issues.add(it.constContextError())
+        }
+      }
     }
   }
 }
@@ -469,6 +491,16 @@ private fun ValidationScope.validateObjects() {
   }
 }
 
+private fun ValidationScope.validateUnions() {
+  typeDefinitions.values.filterIsInstance<GQLUnionTypeDefinition>().forEach { u ->
+    u.directives.forEach { directive ->
+      validateDirective(directive, u) {
+        issues.add(it.constContextError())
+      }
+    }
+  }
+}
+
 private fun ValidationScope.validateCatch(schemaDefinition: GQLSchemaDefinition?) {
   val hasCatchDefinition = directiveDefinitions.any {
     originalDirectiveName(it.key) == Schema.CATCH

libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/internal/ValidationCommon.kt

@@ -212,27 +212,22 @@ internal fun ValidationScope.extraValidateNonNullDirective(directive: GQLDirecti
  * Extra Apollo-specific validation for @typePolicy
  */
 internal fun ValidationScope.extraValidateTypePolicyDirective(directive: GQLDirective, directiveContext: GQLNode) {
-  val keyFieldsArg = directive.arguments.firstOrNull { it.name == "keyFields" }
-  if (keyFieldsArg == null) {
-    return
-  }
-
   val fieldDefinitions: List<GQLFieldDefinition>
-  val type: String
+  val typeName: String
   when (directiveContext) {
     is GQLInterfaceTypeDefinition -> {
       fieldDefinitions = directiveContext.fields
-      type = directiveContext.name
+      typeName = directiveContext.name
     }
 
     is GQLObjectTypeDefinition -> {
       fieldDefinitions = directiveContext.fields
-      type = directiveContext.name
+      typeName = directiveContext.name
     }
 
     is GQLUnionTypeDefinition -> {
       fieldDefinitions = emptyList()
-      type = directiveContext.name
+      typeName = directiveContext.name
     }
 
     else -> {
@@ -241,15 +236,29 @@ internal fun ValidationScope.extraValidateTypePolicyDirective(directive: GQLDire
     }
   }
 
-  (keyFieldsArg.value as GQLStringValue).value.parseAsGQLSelections().getOrThrow().forEach { selection ->
-    if (selection !is GQLField) {
-      registerIssue("Fragments are not supported in @$TYPE_POLICY directives", keyFieldsArg.sourceLocation)
-    } else if (selection.selections.isNotEmpty()) {
-      registerIssue("Composite fields are not supported in @$TYPE_POLICY directives", keyFieldsArg.sourceLocation)
-    } else {
-      val definition = fieldDefinitions.firstOrNull { it.name == selection.name }
-      if (definition == null) {
-        registerIssue("Field '${selection.name}' is not a valid key field for type '$type'", keyFieldsArg.sourceLocation)
+  validateTypePolicyArgument(directive, "keyFields", typeName, fieldDefinitions)
+  validateTypePolicyArgument(directive, "embeddedFields", typeName, fieldDefinitions)
+  validateTypePolicyArgument(directive, "connectionFields", typeName, fieldDefinitions)
+}
+
+private fun ValidationScope.validateTypePolicyArgument(
+    directive: GQLDirective,
+    argumentName: String,
+    typeName: String,
+    fieldDefinitions: List<GQLFieldDefinition>,
+) {
+  val keyFieldsArg = directive.arguments.firstOrNull { it.name == argumentName }
+  if (keyFieldsArg != null) {
+    (keyFieldsArg.value as GQLStringValue).value.parseAsGQLSelections().getOrThrow().forEach { selection ->
+      if (selection !is GQLField) {
+        registerIssue("Fragments are not supported in @$TYPE_POLICY directives", keyFieldsArg.sourceLocation)
+      } else if (selection.selections.isNotEmpty()) {
+        registerIssue("Composite fields are not supported in @$TYPE_POLICY directives", keyFieldsArg.sourceLocation)
+      } else {
+        val definition = fieldDefinitions.firstOrNull { it.name == selection.name }
+        if (definition == null) {
+          registerIssue("No such field: '$typeName.${selection.name}'", keyFieldsArg.sourceLocation)
+        }
       }
     }
   }

libraries/apollo-compiler/src/test/kotlin/com/apollographql/apollo3/compiler/keyfields/KeyFieldsTest.kt

@@ -4,7 +4,6 @@ import com.apollographql.apollo3.ast.GQLDocument
 import com.apollographql.apollo3.ast.GQLExecutableDefinition
 import com.apollographql.apollo3.ast.GQLFragmentDefinition
 import com.apollographql.apollo3.ast.GQLOperationDefinition
-import com.apollographql.apollo3.ast.SourceAwareException
 import com.apollographql.apollo3.ast.parseAsGQLDocument
 import com.apollographql.apollo3.ast.toGQLDocument
 import com.apollographql.apollo3.ast.toSchema
@@ -56,15 +55,6 @@ class KeyFieldsTest {
     assertEquals(setOf("id"), schema.keyFields("Node"))
   }
 
-  @Test
-  fun testExtendUnionTypePolicyDirective() {
-    val schema = "src/test/kotlin/com/apollographql/apollo3/compiler/keyfields/extendsSchema.graphqls"
-        .toPath()
-        .toGQLDocument()
-        .toSchema()
-    assertEquals(setOf("x"), schema.keyFields("Foo"))
-  }
-
   @Test
   fun testObjectWithTypePolicyAndInterfaceTypePolicyErrors() {
     "src/test/kotlin/com/apollographql/apollo3/compiler/keyfields/objectAndInterfaceTypePolicySchema.graphqls"
@@ -125,15 +115,9 @@ class KeyFieldsTest {
 
     val (operationDefinitions, schemaDefinitions) = definitions.partition { it is GQLExecutableDefinition }
 
-    try {
-      val schema = GQLDocument(schemaDefinitions, null).validateAsSchema().getOrThrow()
-      var operation = (operationDefinitions.single() as GQLOperationDefinition)
-      operation = addRequiredFields(operation, "always", schema, emptyMap())
-      checkKeyFields(operation, schema, emptyMap())
-      fail("An exception was expected")
-    } catch (e: SourceAwareException) {
-      check(e.message!!.contains("Field 'id' is not a valid key field for type 'Animal'"))
-      return
-    }
+    val issues = GQLDocument(schemaDefinitions, null).validateAsSchema().issues
+    check(issues.any { it.message.contains("No such field: 'Animal.id'") })
+    check(issues.any { it.message.contains("No such field: 'Node.version'") })
+    check(issues.any { it.message.contains("No such field: 'Foo.x'") })
   }
 }

libraries/apollo-compiler/src/test/kotlin/com/apollographql/apollo3/compiler/keyfields/extendsSchema.graphqls

@@ -15,13 +15,3 @@ type Bar {
 }
 
 extend type Bar implements Node
-
-union Foo = A | B
-type A {
-  x: String!
-}
-type B {
-  x: String!
-}
-
-extend union Foo @typePolicy(keyFields: "x")

libraries/apollo-compiler/src/test/kotlin/com/apollographql/apollo3/compiler/keyfields/nonexistentKeyField.graphqls

@@ -8,9 +8,23 @@ type Animal @typePolicy(keyFields: "id") {
   species: String!
 }
 
+interface Node @typePolicy(keyFields: "id version") {
+  id: ID!
+}
+
+union Foo = A | B
+type A {
+  x: String!
+}
+type B {
+  x: String!
+}
+
+extend union Foo @typePolicy(keyFields: "x")
+
 
 query GetAnimal {
   animal {
     species
   }
-}
+}

libraries/apollo-compiler/src/test/validation/schema/typepolicy-unknown-fields.graphql

@@ -0,0 +1,24 @@
+extend schema @link(url: "https://specs.apollo.dev/kotlin_labs/v0.2/", import: ["@typePolicy"])
+
+type Query {
+  x: Int
+}
+
+type Person {
+  id: ID!
+}
+
+extend type Person @typePolicy(keyFields: "id name" connectionFields: "id foo bar" embeddedFields: "id baz")
+
+interface Node {
+  id: ID!
+}
+
+extend interface Node @typePolicy(keyFields: "id name" connectionFields: "id foo bar" embeddedFields: "id baz")
+
+
+union Foo = A | B
+type A { a: String }
+type B { b: String }
+
+extend union Foo @typePolicy(keyFields: "id name" connectionFields: "id foo bar" embeddedFields: "id baz")