From 3481de48ac2612f51e856fa946c8d28d30c7faa1 Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Fri, 22 Mar 2024 20:58:16 +0100 Subject: [PATCH] Directive validation is now enforced by default (#5758) --- libraries/apollo-ast/api/apollo-ast.api | 1 + .../com/apollographql/apollo3/ast/Schema.kt | 1 + .../apollographql/apollo3/ast/gqldirective.kt | 2 +- .../apollo3/ast/internal/ValidationCommon.kt | 22 +++++++++++-------- .../operation/MissingDirective.expected | 2 +- .../src/main/graphql/appsync/schema.graphqls | 2 ++ .../graphql/appsync/schema.graphqls | 2 ++ 7 files changed, 21 insertions(+), 11 deletions(-) diff --git a/libraries/apollo-ast/api/apollo-ast.api b/libraries/apollo-ast/api/apollo-ast.api index ecfdcc8f0a2..28257d06fc1 100644 --- a/libraries/apollo-ast/api/apollo-ast.api +++ b/libraries/apollo-ast/api/apollo-ast.api @@ -944,6 +944,7 @@ public final class com/apollographql/apollo3/ast/Schema { public static final field REQUIRES_OPT_IN Ljava/lang/String; public static final field SEMANTIC_NON_NULL Ljava/lang/String; public static final field SEMANTIC_NON_NULL_FIELD Ljava/lang/String; + public static final field TARGET_NAME Ljava/lang/String; public static final field TYPE_POLICY Ljava/lang/String; public final fun getDirectiveDefinitions ()Ljava/util/Map; public final fun getErrorAware ()Z diff --git a/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/Schema.kt b/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/Schema.kt index be3689ed3be..927e50bb99b 100644 --- a/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/Schema.kt +++ b/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/Schema.kt @@ -204,6 +204,7 @@ class Schema internal constructor( const val NONNULL = "nonnull" const val OPTIONAL = "optional" const val REQUIRES_OPT_IN = "requiresOptIn" + const val TARGET_NAME = "targetName" @ApolloExperimental const val ONE_OF = "oneOf" diff --git a/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/gqldirective.kt b/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/gqldirective.kt index daa9fb02c68..59872348368 100644 --- a/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/gqldirective.kt +++ b/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/gqldirective.kt @@ -51,7 +51,7 @@ fun List.findOptInFeature(schema: Schema): String? = filter { sche }.firstOrNull() @ApolloInternal -fun List.findTargetName(schema: Schema): String? = firstOrNull { schema.originalDirectiveName(it.name) == "targetName" } +fun List.findTargetName(schema: Schema): String? = firstOrNull { schema.originalDirectiveName(it.name) == Schema.TARGET_NAME } ?.let { it.arguments .firstOrNull { it.name == "name" } diff --git a/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/internal/ValidationCommon.kt b/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/internal/ValidationCommon.kt index 876561fc2cf..433aa2b4344 100644 --- a/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/internal/ValidationCommon.kt +++ b/libraries/apollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/internal/ValidationCommon.kt @@ -153,18 +153,22 @@ internal fun ValidationScope.validateDirectives( val directiveDefinition = directiveDefinitions[directive.name] if (directiveDefinition == null) { when (val originalName = originalDirectiveName(directive.name)) { - Schema.ONE_OF, - Schema.CATCH, - Schema.SEMANTIC_NON_NULL, - Schema.IGNORE_ERRORS, + Schema.OPTIONAL, + Schema.NONNULL, + Schema.TYPE_POLICY, + Schema.FIELD_POLICY, + Schema.REQUIRES_OPT_IN, + Schema.TARGET_NAME, -> { - // 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)) + /** + * This validation is lenient for historical reasons. We don't want to break users relying on this. + * If you're reading this and there's a good reason to, you can move directives out of this branch and require user to + * specify the correct `@link` directive + */ + issues.add(UnknownDirective("Unknown directive '@${directive.name}'", directive.sourceLocation, requireDefinition = false)) } - else -> { - issues.add(UnknownDirective("Unknown directive '@${directive.name}'", directive.sourceLocation, requireDefinition = false)) + issues.add(UnknownDirective("No directive definition found for '@${originalName}'", directive.sourceLocation, requireDefinition = true)) } } diff --git a/libraries/apollo-compiler/src/test/validation/operation/MissingDirective.expected b/libraries/apollo-compiler/src/test/validation/operation/MissingDirective.expected index 31ff83abc13..7c4d1d63c63 100644 --- a/libraries/apollo-compiler/src/test/validation/operation/MissingDirective.expected +++ b/libraries/apollo-compiler/src/test/validation/operation/MissingDirective.expected @@ -1,2 +1,2 @@ UnknownDirective (4:10) -Unknown directive '@required' \ No newline at end of file +No directive definition found for '@required' \ No newline at end of file diff --git a/tests/java-client/src/main/graphql/appsync/schema.graphqls b/tests/java-client/src/main/graphql/appsync/schema.graphqls index fec7d68c5cf..ee20dfed627 100644 --- a/tests/java-client/src/main/graphql/appsync/schema.graphqls +++ b/tests/java-client/src/main/graphql/appsync/schema.graphqls @@ -51,6 +51,8 @@ type Query { listEvents(filter: TableEventFilterInput, limit: Int, nextToken: String): EventConnection } +directive @aws_subscribe(mutations: [String]!) on FIELD_DEFINITION + type Subscription { subscribeToEventComments(eventId: String!): Comment @aws_subscribe(mutations : ["commentOnEvent"]) } diff --git a/tests/websockets/src/commonMain/graphql/appsync/schema.graphqls b/tests/websockets/src/commonMain/graphql/appsync/schema.graphqls index fec7d68c5cf..ee20dfed627 100644 --- a/tests/websockets/src/commonMain/graphql/appsync/schema.graphqls +++ b/tests/websockets/src/commonMain/graphql/appsync/schema.graphqls @@ -51,6 +51,8 @@ type Query { listEvents(filter: TableEventFilterInput, limit: Int, nextToken: String): EventConnection } +directive @aws_subscribe(mutations: [String]!) on FIELD_DEFINITION + type Subscription { subscribeToEventComments(eventId: String!): Comment @aws_subscribe(mutations : ["commentOnEvent"]) }