Stable port of #2940

bbakerman · bbakerman · commit 0a76c7675222 · 2022-09-07T13:46:54.000+10:00
Fix bug where parsing a sdl with @Skip definition would result in 2 directive definitions on the GraphQLSchema (#2940)
diff --git a/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java b/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java
@@ -80,8 +80,10 @@
 
 import static graphql.Assert.assertNotNull;
 import static graphql.Directives.DEPRECATED_DIRECTIVE_DEFINITION;
+import static graphql.Directives.IncludeDirective;
 import static graphql.Directives.NO_LONGER_SUPPORTED;
 import static graphql.Directives.SPECIFIED_BY_DIRECTIVE_DEFINITION;
+import static graphql.Directives.SkipDirective;
 import static graphql.Directives.SpecifiedByDirective;
 import static graphql.collect.ImmutableKit.emptyList;
 import static graphql.introspection.Introspection.DirectiveLocation.ARGUMENT_DEFINITION;
@@ -1055,6 +1057,11 @@ Set<GraphQLDirective> buildAdditionalDirectiveDefinitions(BuildContext buildCtx)
         TypeDefinitionRegistry typeRegistry = buildCtx.getTypeRegistry();
 
         for (DirectiveDefinition directiveDefinition : typeRegistry.getDirectiveDefinitions().values()) {
+            if (IncludeDirective.getName().equals(directiveDefinition.getName())
+                    || SkipDirective.getName().equals(directiveDefinition.getName())) {
+                // skip and include directives are added by default to the GraphQLSchema via the GraphQLSchema builder.
+                continue;
+            }
             GraphQLDirective directive = buildDirectiveDefinitionFromAst(buildCtx, directiveDefinition, inputTypeFactory(buildCtx));
             buildCtx.addDirectiveDefinition(directive);
             additionalDirectives.add(directive);
diff --git a/src/test/groovy/graphql/schema/idl/SchemaGeneratorTest.groovy b/src/test/groovy/graphql/schema/idl/SchemaGeneratorTest.groovy
@@ -2533,4 +2533,35 @@ class SchemaGeneratorTest extends Specification {
         then:
         noExceptionThrown()
     }
+
+    def "skip and include should be added to the schema only if not already defined"() {
+        def sdl = '''
+            "Directs the executor to skip this field or fragment when the `if`'argument is true."
+            directive @skip(
+                "Skipped when true."
+                if: Boolean!
+              ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
+              
+            "Directs the executor to include this field or fragment only when the `if` argument is true"
+            directive @include(
+                "Included when true."
+                if: Boolean!
+              ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
+              
+            type Query {
+                hello: String
+            }
+        '''
+        when:
+        def schema = TestUtil.schema(sdl)
+        then:
+        schema.getDirectives().findAll { it.name == "skip" }.size() == 1
+        schema.getDirectives().findAll { it.name == "include" }.size() == 1
+
+        and:
+        def newSchema = GraphQLSchema.newSchema(schema).build()
+        then:
+        newSchema.getDirectives().findAll { it.name == "skip" }.size() == 1
+        newSchema.getDirectives().findAll { it.name == "include" }.size() == 1
+    }
 }
