Directive validation is now enforced by default (#5758)

apollographql · Mar 22, 2024 · 3481de4 · 3481de4
1 parent b54afc5
commit 3481de4
Show file tree

Hide file tree

Showing 7 changed files with 21 additions and 11 deletions.
diff --git 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
@@ -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
@@ -51,7 +51,7 @@ fun List<GQLDirective>.findOptInFeature(schema: Schema): String? = filter { sche
     }.firstOrNull()
 
 @ApolloInternal
-fun List<GQLDirective>.findTargetName(schema: Schema): String? = firstOrNull { schema.originalDirectiveName(it.name) == "targetName" }
+fun List<GQLDirective>.findTargetName(schema: Schema): String? = firstOrNull { schema.originalDirectiveName(it.name) == Schema.TARGET_NAME }
     ?.let {
       it.arguments
           .firstOrNull { it.name == "name" }

diff --git a/...ollo-ast/src/commonMain/kotlin/com/apollographql/apollo3/ast/internal/ValidationCommon.kt b/...ollo-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
diff --git 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
@@ -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"])
 }