Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add validation to check schema definitions are compatible with the bundled ones #5444

Merged
merged 12 commits into from
Dec 12, 2023
Merged
2 changes: 1 addition & 1 deletion libraries/apollo-ast/api/apollo-ast.api
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,7 @@ public abstract interface class com/apollographql/apollo3/ast/GraphQLIssue : com
public abstract interface class com/apollographql/apollo3/ast/GraphQLValidationIssue : com/apollographql/apollo3/ast/GraphQLIssue {
}

public final class com/apollographql/apollo3/ast/IncompatibleDirectiveDefinition : com/apollographql/apollo3/ast/GraphQLValidationIssue {
public final class com/apollographql/apollo3/ast/IncompatibleDefinition : com/apollographql/apollo3/ast/GraphQLValidationIssue {
public fun <init> (Ljava/lang/String;Ljava/lang/String;Lcom/apollographql/apollo3/ast/SourceLocation;)V
public fun getMessage ()Ljava/lang/String;
public fun getSourceLocation ()Lcom/apollographql/apollo3/ast/SourceLocation;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ class UnknownDirective @ApolloInternal constructor(
}

/**
* The directive definition is inconsistent with the expected one.
* The definition is inconsistent with the expected one.
*/
class IncompatibleDirectiveDefinition(
directiveName: String,
class IncompatibleDefinition(
name: String,
expectedDefinition: String,
override val sourceLocation: SourceLocation?,
) : GraphQLValidationIssue {
override val message = "Unexpected '@$directiveName' directive definition. Expecting '$expectedDefinition'."
override val message = "Unexpected '$name' definition. Expecting '$expectedDefinition'."
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,18 @@ private fun GQLValue?.toCatchTo(): CatchTo {
}
}

private fun GQLDirective.isDefinedAndMatchesOriginalName(schema: Schema, originalName: String): Boolean {
return schema.directiveDefinitions.get(name) != null && schema.originalDirectiveName(name) == originalName
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for nitpicking this but I'd rather have the codegen crash than silently ignore missing directive definitions.
All directive usage of @catch must be validated at that point so if we don't have a directive definition, it's a programming error that I'd rather catch (no pun intended) early

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why I had the impression it would still NPE without this, but it's not the case - just removed it 👍

}

@ApolloInternal
fun List<GQLDirective>.findCatches(schema: Schema): List<Catch> {
return filter {
schema.originalDirectiveName(it.name) == Schema.CATCH
it.isDefinedAndMatchesOriginalName(schema, Schema.CATCH)
}.map {
val to = it.getArgument("to", schema).toCatchTo()
Catch(
to = it.getArgument("to", schema).toCatchTo(),
to = to,
level = it.getArgument("level", schema)?.toIntOrNull(),
)
}
Expand All @@ -150,7 +155,7 @@ fun List<GQLDirective>.findCatches(schema: Schema): List<Catch> {
@ApolloInternal
fun GQLFieldDefinition.findSemanticNonNulls(schema: Schema): List<Int?> {
return directives.filter {
schema.originalDirectiveName(it.name) == Schema.SEMANTIC_NON_NULL
it.isDefinedAndMatchesOriginalName(schema, Schema.SEMANTIC_NON_NULL)
}.map {
it.getArgument("level", schema)?.toIntOrNull()
}
Expand All @@ -159,7 +164,7 @@ fun GQLFieldDefinition.findSemanticNonNulls(schema: Schema): List<Int?> {
@ApolloInternal
fun GQLTypeDefinition.findSemanticNonNulls(fieldName: String, schema: Schema): List<Int?> {
return directives.filter {
schema.originalDirectiveName(it.name) == Schema.SEMANTIC_NON_NULL
it.isDefinedAndMatchesOriginalName(schema, Schema.SEMANTIC_NON_NULL)
&& it.getArgument("field", schema)?.toStringOrNull() == fieldName
}.map {
it.getArgument("level", schema)?.toIntOrNull()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.apollographql.apollo3.ast

import com.apollographql.apollo3.annotations.ApolloDeprecatedSince
import com.apollographql.apollo3.annotations.ApolloExperimental
import com.apollographql.apollo3.annotations.ApolloInternal
import com.apollographql.apollo3.ast.internal.ExtensionsMerger
import com.apollographql.apollo3.ast.internal.builtinsDefinitionsStr
import com.apollographql.apollo3.ast.internal.ensureSchemaDefinition
Expand Down Expand Up @@ -79,23 +80,27 @@ fun builtinDefinitions() = definitionsFromString(builtinsDefinitionsStr)
*/
fun linkDefinitions() = definitionsFromString(linkDefinitionsStr)

@ApolloInternal const val KOTLIN_LABS_VERSION = "v0.2"

/**
* Extra apollo Kotlin specific definitions from https://specs.apollo.dev/kotlin_labs/<[version]>
*/
fun kotlinLabsDefinitions(version: String): List<GQLDefinition> {
return definitionsFromString(when (version) {
"v0.2" -> kotlinLabsDefinitions
else -> error("kotlin_labs/$version definitions are not supported, please use v0.2")
KOTLIN_LABS_VERSION -> kotlinLabsDefinitions
else -> error("kotlin_labs/$version definitions are not supported, please use $KOTLIN_LABS_VERSION")
})
}

@ApolloInternal const val NULLABILITY_VERSION = "v0.1"

/**
* Extra nullability definitions from https://specs.apollo.dev/nullability/<[version]>
*/
fun nullabilityDefinitions(version: String): List<GQLDefinition> {
return definitionsFromString(when (version) {
"v0.1" -> nullabilityDefinitionsStr
else -> error("nullability/$version definitions are not supported, please use v0.1")
NULLABILITY_VERSION -> nullabilityDefinitionsStr
else -> error("nullability/$version definitions are not supported, please use $NULLABILITY_VERSION")
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@ import com.apollographql.apollo3.ast.GQLTypeDefinition
import com.apollographql.apollo3.ast.GQLTypeDefinition.Companion.builtInTypes
import com.apollographql.apollo3.ast.GQLTypeSystemExtension
import com.apollographql.apollo3.ast.GQLUnionTypeDefinition
import com.apollographql.apollo3.ast.IncompatibleDirectiveDefinition
import com.apollographql.apollo3.ast.IncompatibleDefinition
import com.apollographql.apollo3.ast.Issue
import com.apollographql.apollo3.ast.KOTLIN_LABS_VERSION
import com.apollographql.apollo3.ast.MergeOptions
import com.apollographql.apollo3.ast.NULLABILITY_VERSION
import com.apollographql.apollo3.ast.NoQueryType
import com.apollographql.apollo3.ast.OtherValidationIssue
import com.apollographql.apollo3.ast.Schema
Expand Down Expand Up @@ -63,7 +65,7 @@ internal fun validateSchema(definitions: List<GQLDefinition>, requiresApolloDefi

var directivesToStrip = foreignSchemas.flatMap { it.directivesToStrip }

val kotlinLabsDefinitions = kotlinLabsDefinitions("v0.2")
val kotlinLabsDefinitions = kotlinLabsDefinitions(KOTLIN_LABS_VERSION)

if (requiresApolloDefinitions && foreignSchemas.none { it.name == "kotlin_labs" }) {
/**
Expand Down Expand Up @@ -132,9 +134,33 @@ internal fun validateSchema(definitions: List<GQLDefinition>, requiresApolloDefi
}
}

nullabilityDefinitions(NULLABILITY_VERSION).forEach { definition ->
when (definition) {
is GQLDirectiveDefinition -> {
val existing = directiveDefinitions[definition.name]
if (existing != null) {
if (!existing.semanticEquals(definition)) {
issues.add(IncompatibleDefinition(definition.name, definition.toSemanticSdl(), definition.sourceLocation))
}
}
}

is GQLEnumTypeDefinition -> {
val existing = typeDefinitions[definition.name]
if (existing != null) {
if (existing !is GQLEnumTypeDefinition || !existing.semanticEquals(definition)) {
BoD marked this conversation as resolved.
Show resolved Hide resolved
issues.add(IncompatibleDefinition(definition.name, definition.toSemanticSdl(), definition.sourceLocation))
}
}
}

else -> {}
}
}

directiveDefinitions[Schema.ONE_OF]?.let {
if (it.locations != listOf(GQLDirectiveLocation.INPUT_OBJECT) || it.arguments.isNotEmpty() || it.repeatable) {
issues.add(IncompatibleDirectiveDefinition(Schema.ONE_OF, "directive @oneOf on INPUT_OBJECT", it.sourceLocation))
issues.add(IncompatibleDefinition(Schema.ONE_OF, "directive @oneOf on INPUT_OBJECT", it.sourceLocation))
}
}

Expand Down Expand Up @@ -491,6 +517,7 @@ private fun ValidationScope.validateCatch(schemaDefinition: GQLSchemaDefinition?
}

}

private fun ValidationScope.validateInputObjects() {
typeDefinitions.values.filterIsInstance<GQLInputObjectTypeDefinition>().forEach { o ->
if (o.inputFields.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
package com.apollographql.apollo3.ast.internal

import com.apollographql.apollo3.ast.GQLArgument
import com.apollographql.apollo3.ast.GQLBooleanValue
import com.apollographql.apollo3.ast.GQLDirective
import com.apollographql.apollo3.ast.GQLDirectiveDefinition
import com.apollographql.apollo3.ast.GQLEnumTypeDefinition
import com.apollographql.apollo3.ast.GQLEnumValue
import com.apollographql.apollo3.ast.GQLFloatValue
import com.apollographql.apollo3.ast.GQLInputValueDefinition
import com.apollographql.apollo3.ast.GQLIntValue
import com.apollographql.apollo3.ast.GQLListType
import com.apollographql.apollo3.ast.GQLListValue
import com.apollographql.apollo3.ast.GQLNamed
import com.apollographql.apollo3.ast.GQLNamedType
import com.apollographql.apollo3.ast.GQLNode
import com.apollographql.apollo3.ast.GQLNonNullType
import com.apollographql.apollo3.ast.GQLNullValue
import com.apollographql.apollo3.ast.GQLObjectValue
import com.apollographql.apollo3.ast.GQLStringValue
import com.apollographql.apollo3.ast.GQLVariableValue
import com.apollographql.apollo3.ast.toUtf8

internal fun GQLNode.semanticEquals(other: GQLNode): Boolean {
martinbonnin marked this conversation as resolved.
Show resolved Hide resolved
when (this) {
is GQLDirectiveDefinition -> {
if (other !is GQLDirectiveDefinition) {
return false
martinbonnin marked this conversation as resolved.
Show resolved Hide resolved
}

if (locations != other.locations) {
return false
}

if (repeatable != other.repeatable) {
return false
}
}

is GQLInputValueDefinition -> {
if (other !is GQLInputValueDefinition) {
return false
}

if (!type.semanticEquals(other.type)) {
return false
}

if (defaultValue != null && other.defaultValue != null) {
if (!defaultValue.semanticEquals(other.defaultValue)) {
return false
}
} else if (defaultValue != null || other.defaultValue != null) {
return false
}
}

is GQLNonNullType -> {
if (other !is GQLNonNullType) {
return false
}
}

is GQLListType -> {
if (other !is GQLListType) {
return false
}
}

is GQLNamedType -> {
if (other !is GQLNamedType) {
return false
}
}

is GQLNullValue -> {
if (other !is GQLNullValue) {
return false
}
}

is GQLListValue -> {
if (other !is GQLListValue) {
return false
}
}

is GQLObjectValue -> {
if (other !is GQLObjectValue) {
return false
}
}

is GQLStringValue -> {
if (other !is GQLStringValue) {
return false
}
if (value != other.value) {
return false
}
}

is GQLBooleanValue -> {
if (other !is GQLBooleanValue) {
return false
}
if (value != other.value) {
return false
}
}

is GQLIntValue -> {
if (other !is GQLIntValue) {
return false
}
if (value != other.value) {
return false
}
}

is GQLFloatValue -> {
if (other !is GQLFloatValue) {
return false
}
if (value != other.value) {
return false
}
}

is GQLEnumValue -> {
if (other !is GQLEnumValue) {
return false
}
if (value != other.value) {
return false
}
}

is GQLVariableValue -> {
if (other !is GQLVariableValue) {
return false
}
if (name != other.name) {
return false
}
}

is GQLEnumTypeDefinition -> {
if (other !is GQLEnumTypeDefinition) {
return false
}
if (name != other.name) {
return false
}
}

is GQLDirective -> {
if (other !is GQLDirective) {
return false
}
if (name != other.name) {
return false
}
}

is GQLArgument -> {
if (other !is GQLArgument) {
return false
}
if (name != other.name) {
return false
}
}

else -> {}
martinbonnin marked this conversation as resolved.
Show resolved Hide resolved
}

if (this is GQLNamed) {
if (other !is GQLNamed) {
return false
}
if (name != other.name) {
return false
}
}

if (children.size != other.children.size) {
return false
}
for (i in children.indices) {
if (!children[i].semanticEquals(other.children[i])) {
return false
}
}
return true
}

internal fun GQLDirectiveDefinition.toSemanticSdl(): String {
return copy(description = null, arguments = arguments.map { it.copy(description = null) }).toUtf8().trim()
}

internal fun GQLEnumTypeDefinition.toSemanticSdl(): String {
return copy(description = null, enumValues = enumValues.map { it.copy(description = null) }).toUtf8().replace(Regex("[\\n ]+"), " ").trim()
}
Loading
Loading