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

[compiler] validate operation directives & enforce presence of the nullability directive definitions #5443

Merged
merged 6 commits into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/source/advanced/nullability.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,15 @@ This is used for backward compatibility for clients where this was the default b
directive @ignoreErrors on QUERY | MUTATION | SUBSCRIPTION
```

To enable it, import it like `@catch`, `CatchTo` and `@semanticNonNull`:

```graphql
extend schema @link(
url: "https://specs.apollo.dev/nullability/v0.1",
import: ["@catch", "CatchTo", "@semanticNonNull", "@ignoreErrors"]
)
```

Enabling error aware parsing and adding `@ignoreErrors` to all your operations is a no-op. You can then remove `@ignoreErrors` from your operations progressively.

## Decision flowchart
Expand Down
1 change: 1 addition & 0 deletions libraries/apollo-ast/api/apollo-ast.api
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,7 @@ public final class com/apollographql/apollo3/ast/Schema {
public static final field FIELD_POLICY_PAGINATION_ARGS Ljava/lang/String;
public static final field IGNORE_ERRORS Ljava/lang/String;
public static final field NONNULL Ljava/lang/String;
public static final field ONE_OF Ljava/lang/String;
public static final field OPTIONAL Ljava/lang/String;
public static final field REQUIRES_OPT_IN Ljava/lang/String;
public static final field SEMANTIC_NON_NULL Ljava/lang/String;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,8 @@ class Schema internal constructor(
const val OPTIONAL = "optional"
const val REQUIRES_OPT_IN = "requiresOptIn"

@ApolloExperimental
const val ONE_OF = "oneOf"
@ApolloExperimental
const val CATCH = "catch"
@ApolloExperimental
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ fun List<GQLDirective>.findSpecifiedBy() = firstOrNull { it.name == "specifiedBy
}
}

fun List<GQLDirective>.findOneOf() = any { it.name == "oneOf" }
fun List<GQLDirective>.findOneOf() = any { it.name == Schema.ONE_OF }

@ApolloInternal
fun List<GQLDirective>.findOptInFeature(schema: Schema): String? = filter { schema.originalDirectiveName(it.name) == Schema.REQUIRES_OPT_IN }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,16 @@ internal class ExecutableValidationScope(
))
}
}
directives.forEach {
validateDirective(it, this) {
issues.add(
OtherValidationIssue(
"Operation directive arguments cannot contain variables",
it.variable.sourceLocation
)
)
}
}
}

private fun List<GQLSelection>.collectFragmentSpreads(): Set<String> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,9 @@ internal fun validateSchema(definitions: List<GQLDefinition>, requiresApolloDefi
}
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,20 @@ internal fun ValidationScope.validateDirective(
val directiveDefinition = directiveDefinitions[directive.name]

if (directiveDefinition == null) {
if (directive.name == "oneOf") {
// We require full schemas to allow the usage of @oneOf
issues.add(UnknownDirective("'@${directive.name}' directive must be defined in the schema to be used", directive.sourceLocation, requireDefinition = true))
} else {
issues.add(UnknownDirective("Unknown directive '@${directive.name}'", directive.sourceLocation, requireDefinition = false))
when (val originalName = originalDirectiveName(directive.name)) {
Schema.ONE_OF,
Schema.CATCH,
Schema.SEMANTIC_NON_NULL,
Schema.IGNORE_ERRORS,
-> {
// Require full schemas to allow the usage of newest directives
// See https://github.com/apollographql/apollo-kotlin/issues/2673
issues.add(UnknownDirective("No directive definition found for '@${originalName}'", directive.sourceLocation, requireDefinition = true))
}

else -> {
issues.add(UnknownDirective("Unknown directive '@${directive.name}'", directive.sourceLocation, requireDefinition = false))
}
}

return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,5 +334,5 @@ Never throw on field errors.

This is used for backward compatibility for clients where this was the default behaviour.
""${'"'}
directive @ignoreFieldErrors on QUERY | MUTATION | SUBSCRIPTION
directive @ignoreErrors on QUERY | MUTATION | SUBSCRIPTION
""".trimIndent()
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import com.apollographql.apollo3.ast.GQLType
import com.apollographql.apollo3.ast.GQLUnionTypeDefinition
import com.apollographql.apollo3.ast.GQLValue
import com.apollographql.apollo3.ast.HOST_FILESYSTEM
import com.apollographql.apollo3.ast.Schema
import com.apollographql.apollo3.ast.SourceLocation
import com.apollographql.apollo3.ast.parseAsGQLValue
import kotlinx.serialization.KSerializer
Expand Down Expand Up @@ -488,7 +489,7 @@ private class GQLDocumentBuilder(private val introspectionSchema: IntrospectionS
directives = if (isOneOf == true) {
listOf(
GQLDirective(
name = "oneOf",
name = Schema.ONE_OF,
arguments = emptyList()
)
)
Expand Down
2 changes: 1 addition & 1 deletion tests/catch/src/main/graphql/shared/schema.graphqls
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
extend schema @link(url: "https://specs.apollo.dev/nullability/v0.1", import: ["@semanticNonNull", "@catch", "CatchTo"])
extend schema @link(url: "https://specs.apollo.dev/nullability/v0.1", import: ["@semanticNonNull", "@catch", "CatchTo", "@ignoreErrors"])

type Query {
nullable: Int
Expand Down