Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion libraries/apollo-ast/api/apollo-ast.api
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 3 additions & 1 deletion libraries/apollo-ast/api/apollo-ast.klib.api
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" }) {
Expand Down Expand Up @@ -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 },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -59,4 +58,4 @@ internal fun Any.reflectiveCall(methodName: String, vararg args: Any?) {
*/
throw e.cause!!
}
}
}
Loading