Add allowAddingDirectivesToExistingFieldDefinitions (#6470)

* Add allowAddingDirectivesToExistingFieldDefinitions

* update apiDump

* fix builD

Author: martinbonnin

libraries/apollo-ast/api/apollo-ast.api

@@ -953,7 +953,8 @@ public final class com/apollographql/apollo/ast/IssueKt {
 
 public final class com/apollographql/apollo/ast/MergeOptions {
 	public static final field Companion Lcom/apollographql/apollo/ast/MergeOptions$Companion;
-	public fun <init> (Z)V
+	public fun <init> (ZZ)V
+	public final fun getAllowAddingDirectivesToExistingFieldDefinitions ()Z
 	public final fun getAllowFieldNullabilityModification ()Z
 }
 

libraries/apollo-ast/api/apollo-ast.klib.api

@@ -1093,8 +1093,10 @@ final class com.apollographql.apollo.ast/InvalidDeferLabel : com.apollographql.a
 }
 
 final class com.apollographql.apollo.ast/MergeOptions { // com.apollographql.apollo.ast/MergeOptions|null[0]
-    constructor <init>(kotlin/Boolean) // com.apollographql.apollo.ast/MergeOptions.<init>|<init>(kotlin.Boolean){}[0]
+    constructor <init>(kotlin/Boolean, kotlin/Boolean) // com.apollographql.apollo.ast/MergeOptions.<init>|<init>(kotlin.Boolean;kotlin.Boolean){}[0]
 
+    final val allowAddingDirectivesToExistingFieldDefinitions // com.apollographql.apollo.ast/MergeOptions.allowAddingDirectivesToExistingFieldDefinitions|{}allowAddingDirectivesToExistingFieldDefinitions[0]
+        final fun <get-allowAddingDirectivesToExistingFieldDefinitions>(): kotlin/Boolean // com.apollographql.apollo.ast/MergeOptions.allowAddingDirectivesToExistingFieldDefinitions.<get-allowAddingDirectivesToExistingFieldDefinitions>|<get-allowAddingDirectivesToExistingFieldDefinitions>(){}[0]
     final val allowFieldNullabilityModification // com.apollographql.apollo.ast/MergeOptions.allowFieldNullabilityModification|{}allowFieldNullabilityModification[0]
         final fun <get-allowFieldNullabilityModification>(): kotlin/Boolean // com.apollographql.apollo.ast/MergeOptions.allowFieldNullabilityModification.<get-allowFieldNullabilityModification>|<get-allowFieldNullabilityModification>(){}[0]
 

libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo/ast/gqldocument.kt

@@ -6,7 +6,6 @@ import com.apollographql.apollo.annotations.ApolloInternal
 import com.apollographql.apollo.ast.internal.ExtensionsMerger
 import com.apollographql.apollo.ast.internal.builtinsDefinitionsStr
 import com.apollographql.apollo.ast.internal.compilerOptions_0_0
-import com.apollographql.apollo.ast.internal.disableErrorPropagationStr
 import com.apollographql.apollo.ast.internal.kotlinLabsDefinitions_0_3
 import com.apollographql.apollo.ast.internal.kotlinLabsDefinitions_0_4
 import com.apollographql.apollo.ast.internal.kotlinLabsDefinitions_0_5
@@ -64,10 +63,11 @@ fun GQLDocument.toSchema(): Schema = validateAsSchema().getOrThrow()
 
 @ApolloExperimental
 class MergeOptions(
+    val allowAddingDirectivesToExistingFieldDefinitions: Boolean,
     val allowFieldNullabilityModification: Boolean
 ) {
   companion object {
-    val Default: MergeOptions = MergeOptions(false)
+    val Default: MergeOptions = MergeOptions(false, false)
   }
 }
 

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

@@ -316,28 +316,40 @@ private fun ExtensionsMerger.mergeFields(
       // field doesn't exist, add it
       result.add(newFieldDefinition)
     } else {
-      val existingFieldDefinition = result[index]
-      if (!mergeOptions.allowFieldNullabilityModification) {
+      if (!mergeOptions.allowFieldNullabilityModification && !mergeOptions.allowAddingDirectivesToExistingFieldDefinitions) {
         issues.add(OtherValidationIssue("There is already a field definition named `${newFieldDefinition.name}` for this type", newFieldDefinition.sourceLocation))
         return@forEach
       }
 
+      val existingFieldDefinition = result[index]
       if (!areEqual(newFieldDefinition.arguments, existingFieldDefinition.arguments)) {
         issues.add(OtherValidationIssue("Cannot merge field definition `${newFieldDefinition.name}`: its arguments do not match the arguments of the original field definition", newFieldDefinition.sourceLocation))
         return@forEach
       }
 
-      if (newFieldDefinition.directives.isNotEmpty()) {
-        issues.add(OtherValidationIssue("Cannot add directives to existing field definition `${newFieldDefinition.name}`", newFieldDefinition.sourceLocation))
-        return@forEach
+      if (mergeOptions.allowFieldNullabilityModification) {
+        if (!newFieldDefinition.type.isCompatibleWith(existingFieldDefinition.type)) {
+          issues.add(OtherValidationIssue("Cannot merge field definition `${newFieldDefinition.name}`: its type is not compatible with the original type.", newFieldDefinition.sourceLocation))
+          return@forEach
+        }
+      } else {
+        if (newFieldDefinition.type.toUtf8() != existingFieldDefinition.type.toUtf8()) {
+          issues.add(OtherValidationIssue("Cannot merge field definition`${newFieldDefinition.name}`: they have different types.", newFieldDefinition.sourceLocation))
+          return@forEach
+        }
       }
-
-      if (!newFieldDefinition.type.isCompatibleWith(existingFieldDefinition.type)) {
-        issues.add(OtherValidationIssue("Cannot merge field directives`${newFieldDefinition.name}`: its type is not compatible with the original type`", newFieldDefinition.sourceLocation))
-        return@forEach
+      if (!mergeOptions.allowAddingDirectivesToExistingFieldDefinitions) {
+        if (newFieldDefinition.directives.isNotEmpty()) {
+          issues.add(OtherValidationIssue("Cannot add directives to existing field definition `${newFieldDefinition.name}`", newFieldDefinition.sourceLocation))
+          return@forEach
+        }
       }
 
-      result[index] = existingFieldDefinition.copy(type = newFieldDefinition.type)
+      /*
+       * No need to validate repeated directives, this is done later on by schema validation.
+       */
+      val newDirectives = existingFieldDefinition.directives + newFieldDefinition.directives
+      result[index] = existingFieldDefinition.copy(type = newFieldDefinition.type, directives = newDirectives)
     }
   }
 

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

@@ -78,6 +78,9 @@ internal fun validateSchema(definitions: List<GQLDefinition>, options: SchemaVal
    */
   val fullDefinitions = definitions.withBuiltinDefinitions()
 
+  /**
+   * TODO: this should probably be done after merging so that we can handle @link on the schema definition itself.
+   */
   val linkedSchemas = definitions.filterIsInstance<GQLSchemaExtension>().getLinkedSchemas(issues, options.foreignSchemas).toMutableList()
 
   if (options.addKotlinLabsDefinitions && linkedSchemas.none { it.foreignSchema.name == "kotlin_labs" }) {
@@ -287,15 +290,17 @@ internal fun validateSchema(definitions: List<GQLDefinition>, options: SchemaVal
       }
 
   /**
-   * I'm not 100% clear on the order of validations, here I'm merging the extensions first thing
+   * I'm not 100% clear on the order of validations, here I'm merging the extensions first thing.
+   *
+   * It seems more natural. Two examples:
+   * - If we were one day to validate that objects implement all interfaces fields for an example, this would have to be
+   * done post merging (because extensions may add fields to interfaces).
+   * - Same for validated repeated directives.
    *
-   * Most of the validations that we do later on do not require merging the definitions though.
-   * If we were one day to validate that objects implement all interfaces fields for an example, this would have to be
-   * done post merging (because extensions may add fields to interfaces). As it is now, we could probably
-   * move validation of the directives before merging.
+   * Moving forward, extensions merging should probably be done first thing as a separate step, before any validation and/or linking of foreign schemas.
    */
   val dedupedDefinitions = listOfNotNull(schemaDefinition) + directiveDefinitions.values + typeDefinitions.values
-  val mergedDefinitions = ExtensionsMerger(dedupedDefinitions + typeSystemExtensions, MergeOptions(true)).merge().getOrThrow()
+  val mergedDefinitions = ExtensionsMerger(dedupedDefinitions + typeSystemExtensions, MergeOptions(false, true)).merge().getOrThrow()
 
   val mergedScope = DefaultValidationScope(
       typeDefinitions = mergedDefinitions.filterIsInstance<GQLTypeDefinition>().associateBy { it.name },

libraries/apollo-ast/src/jvmTest/kotlin/com/apollographql/apollo/graphql/ast/test/TypeExtensionsMergeTest.kt

@@ -30,7 +30,7 @@ class TypeExtensionsMergeTest {
       }
     """.trimIndent()
 
-    val result = sdl.toGQLDocument().toMergedGQLDocument(MergeOptions(true))
+    val result = sdl.toGQLDocument().toMergedGQLDocument(MergeOptions(false, true))
         .definitions
         .single() as GQLObjectTypeDefinition
 
@@ -60,7 +60,7 @@ class TypeExtensionsMergeTest {
       }
     """.trimIndent()
 
-    val issues = sdl.toGQLDocument().mergeExtensions(MergeOptions(true)).issues
+    val issues = sdl.toGQLDocument().mergeExtensions(MergeOptions(false, true)).issues
 
     assertEquals(3, issues.size)
     assertTrue(issues[0].message.contains("its type is not compatible with the original type"))

libraries/apollo-gradle-plugin-external/src/main/kotlin/com/apollographql/apollo/gradle/internal/utils.kt

@@ -3,7 +3,6 @@ package com.apollographql.apollo.gradle.internal
 import com.apollographql.apollo.compiler.InputFile
 import org.gradle.api.artifacts.ProjectDependency
 import org.gradle.api.file.FileCollection
-import java.io.File
 import java.lang.reflect.InvocationTargetException
 
 
@@ -59,4 +58,4 @@ internal fun Any.reflectiveCall(methodName: String, vararg args: Any?) {
      */
     throw e.cause!!
   }
-}
+}