diff --git a/libraries/apollo-ast/api/apollo-ast.api b/libraries/apollo-ast/api/apollo-ast.api index 7708984baca..83355521b50 100644 --- a/libraries/apollo-ast/api/apollo-ast.api +++ b/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 (Z)V + public fun (ZZ)V + public final fun getAllowAddingDirectivesToExistingFieldDefinitions ()Z public final fun getAllowFieldNullabilityModification ()Z } diff --git a/libraries/apollo-ast/api/apollo-ast.klib.api b/libraries/apollo-ast/api/apollo-ast.klib.api index bbd4a875016..f4a6f80b55f 100644 --- a/libraries/apollo-ast/api/apollo-ast.klib.api +++ b/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 (kotlin/Boolean) // com.apollographql.apollo.ast/MergeOptions.|(kotlin.Boolean){}[0] + constructor (kotlin/Boolean, kotlin/Boolean) // com.apollographql.apollo.ast/MergeOptions.|(kotlin.Boolean;kotlin.Boolean){}[0] + final val allowAddingDirectivesToExistingFieldDefinitions // com.apollographql.apollo.ast/MergeOptions.allowAddingDirectivesToExistingFieldDefinitions|{}allowAddingDirectivesToExistingFieldDefinitions[0] + final fun (): kotlin/Boolean // com.apollographql.apollo.ast/MergeOptions.allowAddingDirectivesToExistingFieldDefinitions.|(){}[0] final val allowFieldNullabilityModification // com.apollographql.apollo.ast/MergeOptions.allowFieldNullabilityModification|{}allowFieldNullabilityModification[0] final fun (): kotlin/Boolean // com.apollographql.apollo.ast/MergeOptions.allowFieldNullabilityModification.|(){}[0] diff --git a/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo/ast/gqldocument.kt b/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo/ast/gqldocument.kt index 708aa07229a..338ef11d075 100644 --- a/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo/ast/gqldocument.kt +++ b/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) } } diff --git a/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo/ast/internal/ExtensionsMerger.kt b/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo/ast/internal/ExtensionsMerger.kt index cd6b355f433..950d2310405 100644 --- a/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo/ast/internal/ExtensionsMerger.kt +++ b/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) } } diff --git a/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo/ast/internal/SchemaValidationScope.kt b/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo/ast/internal/SchemaValidationScope.kt index 1a0a354abdb..90a8ab0f400 100644 --- a/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo/ast/internal/SchemaValidationScope.kt +++ b/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo/ast/internal/SchemaValidationScope.kt @@ -78,6 +78,9 @@ internal fun validateSchema(definitions: List, 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().getLinkedSchemas(issues, options.foreignSchemas).toMutableList() if (options.addKotlinLabsDefinitions && linkedSchemas.none { it.foreignSchema.name == "kotlin_labs" }) { @@ -287,15 +290,17 @@ internal fun validateSchema(definitions: List, 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().associateBy { it.name }, diff --git a/libraries/apollo-ast/src/jvmTest/kotlin/com/apollographql/apollo/graphql/ast/test/TypeExtensionsMergeTest.kt b/libraries/apollo-ast/src/jvmTest/kotlin/com/apollographql/apollo/graphql/ast/test/TypeExtensionsMergeTest.kt index 2ba7a8103e2..12b267ab7bf 100644 --- a/libraries/apollo-ast/src/jvmTest/kotlin/com/apollographql/apollo/graphql/ast/test/TypeExtensionsMergeTest.kt +++ b/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")) diff --git a/libraries/apollo-gradle-plugin-external/src/main/kotlin/com/apollographql/apollo/gradle/internal/utils.kt b/libraries/apollo-gradle-plugin-external/src/main/kotlin/com/apollographql/apollo/gradle/internal/utils.kt index ad65e21dd9c..571ef9d2387 100644 --- a/libraries/apollo-gradle-plugin-external/src/main/kotlin/com/apollographql/apollo/gradle/internal/utils.kt +++ b/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!! } -} \ No newline at end of file +}