From 31ee7c6c3b8086cc35a527148b478015a5ea8c50 Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Wed, 28 Jan 2026 12:43:44 +1000 Subject: [PATCH 1/5] Remove clearDirectives and use first-class properties for @specifiedBy in SchemaPrinter Remove GraphQLSchema.Builder.clearDirectives() as it allows removing spec-required built-in directives, which arguably violates the spec. All 7 built-in directives are now always present. Change SchemaPrinter to synthesize @specifiedBy from the first-class specifiedByUrl property on GraphQLScalarType, matching how @deprecated already uses the first-class deprecationReason property. This ensures both directives rely on their first-class properties as the source of truth rather than applied directives. Co-Authored-By: Claude Opus 4.5 --- .../java/graphql/schema/GraphQLSchema.java | 7 --- .../graphql/schema/idl/SchemaPrinter.java | 63 ++++++++++++++++++- .../graphql/schema/GraphQLSchemaTest.groovy | 27 ++++---- 3 files changed, 74 insertions(+), 23 deletions(-) diff --git a/src/main/java/graphql/schema/GraphQLSchema.java b/src/main/java/graphql/schema/GraphQLSchema.java index da7a2ccde6..daaa896050 100644 --- a/src/main/java/graphql/schema/GraphQLSchema.java +++ b/src/main/java/graphql/schema/GraphQLSchema.java @@ -692,7 +692,6 @@ public static Builder newSchema(GraphQLSchema existingSchema) { .introspectionSchemaType(existingSchema.getIntrospectionSchemaType()) .codeRegistry(existingSchema.getCodeRegistry()) .clearAdditionalTypes() - .clearDirectives() .additionalDirectives(new LinkedHashSet<>(existingSchema.getDirectives())) .clearSchemaDirectives() .withSchemaDirectives(schemaDirectivesArray(existingSchema)) @@ -862,12 +861,6 @@ public Builder additionalDirective(GraphQLDirective additionalDirective) { return this; } - public Builder clearDirectives() { - this.additionalDirectives.clear(); - return this; - } - - public Builder withSchemaDirectives(GraphQLDirective... directives) { for (GraphQLDirective directive : directives) { withSchemaDirective(directive); diff --git a/src/main/java/graphql/schema/idl/SchemaPrinter.java b/src/main/java/graphql/schema/idl/SchemaPrinter.java index cafa142e0c..a1782b3943 100644 --- a/src/main/java/graphql/schema/idl/SchemaPrinter.java +++ b/src/main/java/graphql/schema/idl/SchemaPrinter.java @@ -64,6 +64,7 @@ import java.util.stream.Stream; import static graphql.Directives.DeprecatedDirective; +import static graphql.Directives.SpecifiedByDirective; import static graphql.Scalars.GraphQLString; import static graphql.schema.visibility.DefaultGraphqlFieldVisibility.DEFAULT_FIELD_VISIBILITY; import static graphql.util.EscapeUtil.escapeJsonString; @@ -559,7 +560,8 @@ private SchemaElementPrinter scalarPrinter() { printAsAst(out, type.getDefinition(), type.getExtensionDefinitions()); } else { printComments(out, type, ""); - out.format("scalar %s%s\n\n", type.getName(), directivesString(GraphQLScalarType.class, type)); + List directives = addOrUpdateSpecifiedByDirectiveIfNeeded(type); + out.format("scalar %s%s\n\n", type.getName(), directivesString(GraphQLScalarType.class, directives)); } } }; @@ -1103,6 +1105,65 @@ private String getDeprecationReason(GraphQLDirectiveContainer directiveContainer } } + private boolean isSpecifiedByDirectiveAllowed() { + return options.getIncludeDirective().test(SpecifiedByDirective.getName()); + } + + private boolean isSpecifiedByDirective(GraphQLAppliedDirective directive) { + return directive.getName().equals(SpecifiedByDirective.getName()); + } + + private boolean hasSpecifiedByDirective(List directives) { + return directives.stream().anyMatch(this::isSpecifiedByDirective); + } + + private List addOrUpdateSpecifiedByDirectiveIfNeeded(GraphQLScalarType scalarType) { + List directives = DirectivesUtil.toAppliedDirectives(scalarType); + String url = scalarType.getSpecifiedByUrl(); + + if (url == null) { + // first-class property is not set - remove any @specifiedBy applied directive + return directives.stream() + .filter(d -> !isSpecifiedByDirective(d)) + .collect(toList()); + } + if (!hasSpecifiedByDirective(directives) && isSpecifiedByDirectiveAllowed()) { + directives = new ArrayList<>(directives); + directives.add(createSpecifiedByDirective(url)); + } else if (hasSpecifiedByDirective(directives) && isSpecifiedByDirectiveAllowed()) { + // Update URL in case modified by schema transform + directives = updateSpecifiedByDirective(directives, url); + } + return directives; + } + + private GraphQLAppliedDirective createSpecifiedByDirective(String url) { + GraphQLAppliedDirectiveArgument arg = GraphQLAppliedDirectiveArgument.newArgument() + .name("url") + .valueProgrammatic(url) + .type(GraphQLString) + .build(); + return GraphQLAppliedDirective.newDirective() + .name("specifiedBy") + .argument(arg) + .build(); + } + + private List updateSpecifiedByDirective(List directives, String url) { + GraphQLAppliedDirectiveArgument newArg = GraphQLAppliedDirectiveArgument.newArgument() + .name("url") + .valueProgrammatic(url) + .type(GraphQLString) + .build(); + + return directives.stream().map(d -> { + if (isSpecifiedByDirective(d)) { + return d.transform(builder -> builder.argument(newArg)); + } + return d; + }).collect(toList()); + } + private String directiveDefinition(GraphQLDirective directive) { StringBuilder sb = new StringBuilder(); diff --git a/src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy b/src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy index a3e5d624e2..f331d1d201 100644 --- a/src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy +++ b/src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy @@ -148,28 +148,25 @@ class GraphQLSchemaTest extends Specification { ((Directive) newSchema.extensionDefinitions.first().getDirectives().first()).name == "pizza" } - def "clear directives works as expected"() { + def "all built-in directives are always present"() { setup: def schemaBuilder = basicSchemaBuilder() - when: "no additional directives have been specified" + when: "a schema is built" def schema = schemaBuilder.build() - then: + then: "all 7 built-in directives are present" schema.directives.size() == 7 - - when: "clear directives is called" - schema = schemaBuilder.clearDirectives().build() - then: - schema.directives.size() == 5 // @deprecated and @specifiedBy and @oneOf et al is ALWAYS added if missing - - when: "clear directives is called with more directives" - schema = schemaBuilder.clearDirectives().additionalDirective(Directives.SkipDirective).build() - then: - schema.directives.size() == 6 + schema.getDirective("include") != null + schema.getDirective("skip") != null + schema.getDirective("deprecated") != null + schema.getDirective("specifiedBy") != null + schema.getDirective("oneOf") != null + schema.getDirective("defer") != null + schema.getDirective("experimental_disableErrorPropagation") != null when: "the schema is transformed, things are copied" - schema = schema.transform({ builder -> builder.additionalDirective(Directives.IncludeDirective) }) - then: + schema = schema.transform({ builder -> builder }) + then: "all 7 built-in directives are still present" schema.directives.size() == 7 } From 6ece42b598ba57237ad279069d3091765d447b12 Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Wed, 28 Jan 2026 13:19:43 +1000 Subject: [PATCH 2/5] Consolidate built-in directive handling and move helpers to Directives class All 7 built-in directives are now handled uniformly in buildImpl() via ensureBuiltInDirectives(), fixing the inconsistency where @include/@skip were pre-populated in the Builder field initializer while the other 5 were force-added in buildImpl(). This eliminates a potential duplicate directive bug when SchemaTransformer replaces a built-in directive. Add BUILT_IN_DIRECTIVES set, BUILT_IN_DIRECTIVES_MAP, and isBuiltInDirective() methods to the Directives class. Deprecate DirectiveInfo in favor of these new Directives methods. Update all internal callers to use Directives directly. Co-Authored-By: Claude Opus 4.5 --- src/main/java/graphql/Directives.java | 56 +++++++++++++++++++ .../IntrospectionResultToSchema.java | 4 +- .../java/graphql/schema/GraphQLSchema.java | 33 ++++++----- .../java/graphql/schema/GraphQLTypeUtil.java | 4 +- .../schema/diffing/SchemaGraphFactory.java | 4 +- .../graphql/schema/idl/DirectiveInfo.java | 46 +++++++-------- .../graphql/schema/idl/SchemaPrinter.java | 3 +- .../graphql/schema/usage/SchemaUsage.java | 4 +- src/main/java/graphql/util/Anonymizer.java | 9 ++- ...rospectionWithDirectivesSupportTest.groovy | 10 ++-- .../groovy/graphql/util/AnonymizerTest.groovy | 4 +- 11 files changed, 119 insertions(+), 58 deletions(-) diff --git a/src/main/java/graphql/Directives.java b/src/main/java/graphql/Directives.java index 37f2b28550..17d57f8443 100644 --- a/src/main/java/graphql/Directives.java +++ b/src/main/java/graphql/Directives.java @@ -7,6 +7,11 @@ import graphql.language.StringValue; import graphql.schema.GraphQLDirective; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import static graphql.Scalars.GraphQLBoolean; @@ -249,6 +254,57 @@ public class Directives { .definition(EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION) .build(); + /** + * The set of all built-in directives that are always present in a graphql schema. + * The iteration order is stable and meaningful. + */ + public static final Set BUILT_IN_DIRECTIVES; + + /** + * A map from directive name to directive for all built-in directives. + */ + public static final Map BUILT_IN_DIRECTIVES_MAP; + + static { + LinkedHashSet directives = new LinkedHashSet<>(); + directives.add(IncludeDirective); + directives.add(SkipDirective); + directives.add(DeprecatedDirective); + directives.add(SpecifiedByDirective); + directives.add(OneOfDirective); + directives.add(DeferDirective); + directives.add(ExperimentalDisableErrorPropagationDirective); + BUILT_IN_DIRECTIVES = Collections.unmodifiableSet(directives); + + LinkedHashMap map = new LinkedHashMap<>(); + for (GraphQLDirective d : BUILT_IN_DIRECTIVES) { + map.put(d.getName(), d); + } + BUILT_IN_DIRECTIVES_MAP = Collections.unmodifiableMap(map); + } + + /** + * Returns true if a directive with the provided name is a built-in directive. + * + * @param directiveName the name of the directive in question + * + * @return true if the directive is built-in, false otherwise + */ + public static boolean isBuiltInDirective(String directiveName) { + return BUILT_IN_DIRECTIVES_MAP.containsKey(directiveName); + } + + /** + * Returns true if the provided directive is a built-in directive. + * + * @param directive the directive in question + * + * @return true if the directive is built-in, false otherwise + */ + public static boolean isBuiltInDirective(GraphQLDirective directive) { + return isBuiltInDirective(directive.getName()); + } + private static Description createDescription(String s) { return new Description(s, null, false); } diff --git a/src/main/java/graphql/introspection/IntrospectionResultToSchema.java b/src/main/java/graphql/introspection/IntrospectionResultToSchema.java index 608c570909..81ae6ce928 100644 --- a/src/main/java/graphql/introspection/IntrospectionResultToSchema.java +++ b/src/main/java/graphql/introspection/IntrospectionResultToSchema.java @@ -41,7 +41,7 @@ import static graphql.Assert.assertShouldNeverHappen; import static graphql.Assert.assertTrue; import static graphql.collect.ImmutableKit.map; -import static graphql.schema.idl.DirectiveInfo.isGraphqlSpecifiedDirective; +import static graphql.Directives.isBuiltInDirective; @SuppressWarnings("unchecked") @PublicApi @@ -131,7 +131,7 @@ public Document createSchemaDefinition(Map introspectionResult) private DirectiveDefinition createDirective(Map input) { String directiveName = (String) input.get("name"); - if (isGraphqlSpecifiedDirective(directiveName)) { + if (isBuiltInDirective(directiveName)) { return null; } diff --git a/src/main/java/graphql/schema/GraphQLSchema.java b/src/main/java/graphql/schema/GraphQLSchema.java index daaa896050..d8e166cc0d 100644 --- a/src/main/java/graphql/schema/GraphQLSchema.java +++ b/src/main/java/graphql/schema/GraphQLSchema.java @@ -37,7 +37,7 @@ import static graphql.collect.ImmutableKit.nonNullCopyOf; import static graphql.schema.GraphqlTypeComparators.byNameAsc; import static graphql.schema.GraphqlTypeComparators.sortTypes; -import static java.util.Arrays.asList; + /** * The schema represents the combined type system of the graphql engine. This is how the engine knows @@ -740,10 +740,7 @@ public static class Builder { private List extensionDefinitions; private String description; - // we default these in - private final Set additionalDirectives = new LinkedHashSet<>( - asList(Directives.IncludeDirective, Directives.SkipDirective) - ); + private final Set additionalDirectives = new LinkedHashSet<>(); private final Set additionalTypes = new LinkedHashSet<>(); private final List schemaDirectives = new ArrayList<>(); private final List schemaAppliedDirectives = new ArrayList<>(); @@ -953,13 +950,8 @@ private GraphQLSchema buildImpl() { assertNotNull(additionalTypes, "additionalTypes can't be null"); assertNotNull(additionalDirectives, "additionalDirectives can't be null"); - // schemas built via the schema generator have the deprecated directive BUT we want it present for hand built - // schemas - it's inherently part of the spec! - addBuiltInDirective(Directives.DeprecatedDirective, additionalDirectives); - addBuiltInDirective(Directives.SpecifiedByDirective, additionalDirectives); - addBuiltInDirective(Directives.OneOfDirective, additionalDirectives); - addBuiltInDirective(Directives.DeferDirective, additionalDirectives); - addBuiltInDirective(Directives.ExperimentalDisableErrorPropagationDirective, additionalDirectives); + // built-in directives are always present in a schema and come first + ensureBuiltInDirectives(); // quick build - no traversing final GraphQLSchema partiallyBuiltSchema = new GraphQLSchema(this); @@ -982,10 +974,21 @@ private GraphQLSchema buildImpl() { return validateSchema(finalSchema); } - private void addBuiltInDirective(GraphQLDirective qlDirective, Set additionalDirectives1) { - if (additionalDirectives1.stream().noneMatch(d -> d.getName().equals(qlDirective.getName()))) { - additionalDirectives1.add(qlDirective); + private void ensureBuiltInDirectives() { + // put built-in directives first, preserving user-supplied overrides by name + Set userDirectiveNames = new LinkedHashSet<>(); + for (GraphQLDirective d : additionalDirectives) { + userDirectiveNames.add(d.getName()); + } + LinkedHashSet ordered = new LinkedHashSet<>(); + for (GraphQLDirective builtIn : Directives.BUILT_IN_DIRECTIVES) { + if (!userDirectiveNames.contains(builtIn.getName())) { + ordered.add(builtIn); + } } + ordered.addAll(additionalDirectives); + additionalDirectives.clear(); + additionalDirectives.addAll(ordered); } private GraphQLSchema validateSchema(GraphQLSchema graphQLSchema) { diff --git a/src/main/java/graphql/schema/GraphQLTypeUtil.java b/src/main/java/graphql/schema/GraphQLTypeUtil.java index e3a6e58d89..2c318c8ae3 100644 --- a/src/main/java/graphql/schema/GraphQLTypeUtil.java +++ b/src/main/java/graphql/schema/GraphQLTypeUtil.java @@ -3,7 +3,7 @@ import graphql.Assert; import graphql.PublicApi; import graphql.introspection.Introspection; -import graphql.schema.idl.DirectiveInfo; +import graphql.Directives; import graphql.schema.idl.ScalarInfo; import java.util.Stack; @@ -293,7 +293,7 @@ public static Predicate isSystemElement() { return ScalarInfo.isGraphqlSpecifiedScalar((GraphQLScalarType) schemaElement); } if (schemaElement instanceof GraphQLDirective) { - return DirectiveInfo.isGraphqlSpecifiedDirective((GraphQLDirective) schemaElement); + return Directives.isBuiltInDirective((GraphQLDirective) schemaElement); } if (schemaElement instanceof GraphQLNamedType) { return Introspection.isIntrospectionTypes((GraphQLNamedType) schemaElement); diff --git a/src/main/java/graphql/schema/diffing/SchemaGraphFactory.java b/src/main/java/graphql/schema/diffing/SchemaGraphFactory.java index 37ac8d5e31..926d1a4e3f 100644 --- a/src/main/java/graphql/schema/diffing/SchemaGraphFactory.java +++ b/src/main/java/graphql/schema/diffing/SchemaGraphFactory.java @@ -6,7 +6,7 @@ import graphql.introspection.Introspection; import graphql.language.AstPrinter; import graphql.schema.*; -import graphql.schema.idl.DirectiveInfo; +import graphql.Directives; import graphql.schema.idl.ScalarInfo; import graphql.util.TraversalControl; import graphql.util.Traverser; @@ -390,7 +390,7 @@ private void newDirective(GraphQLDirective directive, SchemaGraph schemaGraph) { directiveVertex.add("name", directive.getName()); directiveVertex.add("repeatable", directive.isRepeatable()); directiveVertex.add("locations", directive.validLocations()); - boolean graphqlSpecified = DirectiveInfo.isGraphqlSpecifiedDirective(directive.getName()); + boolean graphqlSpecified = Directives.isBuiltInDirective(directive.getName()); directiveVertex.setBuiltInType(graphqlSpecified); directiveVertex.add("description", desc(directive.getDescription())); for (GraphQLArgument argument : directive.getArguments()) { diff --git a/src/main/java/graphql/schema/idl/DirectiveInfo.java b/src/main/java/graphql/schema/idl/DirectiveInfo.java index 9b8317f97e..199e493fd5 100644 --- a/src/main/java/graphql/schema/idl/DirectiveInfo.java +++ b/src/main/java/graphql/schema/idl/DirectiveInfo.java @@ -1,7 +1,5 @@ package graphql.schema.idl; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; import graphql.Directives; import graphql.PublicApi; import graphql.schema.GraphQLDirective; @@ -10,30 +8,30 @@ import java.util.Set; /** - * Info on all the directives provided by graphql specification + * Info on all the directives provided by graphql specification. + * + * @deprecated Use {@link Directives} instead, specifically {@link Directives#BUILT_IN_DIRECTIVES}, + * {@link Directives#BUILT_IN_DIRECTIVES_MAP}, and {@link Directives#isBuiltInDirective(String)}. */ +@Deprecated @PublicApi public class DirectiveInfo { - /** - * A map from directive name to directive which provided by specification + * A set of directives which provided by graphql specification + * + * @deprecated Use {@link Directives#BUILT_IN_DIRECTIVES} instead. */ - public static final Map GRAPHQL_SPECIFICATION_DIRECTIVE_MAP = ImmutableMap.of( - Directives.IncludeDirective.getName(), Directives.IncludeDirective, - Directives.SkipDirective.getName(), Directives.SkipDirective, - Directives.DeprecatedDirective.getName(), Directives.DeprecatedDirective, - Directives.SpecifiedByDirective.getName(), Directives.SpecifiedByDirective, - Directives.OneOfDirective.getName(), Directives.OneOfDirective, - // technically this is NOT yet in spec - but it is added by default by graphql-java so we include it - // we should also do @defer at some future time soon - Directives.DeferDirective.getName(), Directives.DeferDirective, - Directives.ExperimentalDisableErrorPropagationDirective.getName(), Directives.ExperimentalDisableErrorPropagationDirective - ); + @Deprecated + public static final Set GRAPHQL_SPECIFICATION_DIRECTIVES = Directives.BUILT_IN_DIRECTIVES; + /** - * A set of directives which provided by graphql specification + * A map from directive name to directive which provided by specification + * + * @deprecated Use {@link Directives#BUILT_IN_DIRECTIVES_MAP} instead. */ - public static final Set GRAPHQL_SPECIFICATION_DIRECTIVES =ImmutableSet.copyOf(GRAPHQL_SPECIFICATION_DIRECTIVE_MAP.values()); + @Deprecated + public static final Map GRAPHQL_SPECIFICATION_DIRECTIVE_MAP = Directives.BUILT_IN_DIRECTIVES_MAP; /** * Returns true if a directive with provided directiveName has been defined in graphql specification @@ -41,9 +39,12 @@ public class DirectiveInfo { * @param directiveName the name of directive in question * * @return true if the directive provided by graphql specification, and false otherwise + * + * @deprecated Use {@link Directives#isBuiltInDirective(String)} instead. */ + @Deprecated public static boolean isGraphqlSpecifiedDirective(String directiveName) { - return GRAPHQL_SPECIFICATION_DIRECTIVE_MAP.containsKey(directiveName); + return Directives.isBuiltInDirective(directiveName); } /** @@ -52,10 +53,11 @@ public static boolean isGraphqlSpecifiedDirective(String directiveName) { * @param graphQLDirective the directive in question * * @return true if the directive provided by graphql specification, and false otherwise + * + * @deprecated Use {@link Directives#isBuiltInDirective(GraphQLDirective)} instead. */ + @Deprecated public static boolean isGraphqlSpecifiedDirective(GraphQLDirective graphQLDirective) { - return isGraphqlSpecifiedDirective(graphQLDirective.getName()); + return Directives.isBuiltInDirective(graphQLDirective); } - - } diff --git a/src/main/java/graphql/schema/idl/SchemaPrinter.java b/src/main/java/graphql/schema/idl/SchemaPrinter.java index a1782b3943..d1e3220983 100644 --- a/src/main/java/graphql/schema/idl/SchemaPrinter.java +++ b/src/main/java/graphql/schema/idl/SchemaPrinter.java @@ -1,6 +1,7 @@ package graphql.schema.idl; import graphql.Assert; +import graphql.Directives; import graphql.DirectivesUtil; import graphql.GraphQLContext; import graphql.PublicApi; @@ -81,7 +82,7 @@ public class SchemaPrinter { * This predicate excludes all directives which are specified by the GraphQL Specification. * Printing these directives is optional. */ - public static final Predicate ExcludeGraphQLSpecifiedDirectivesPredicate = d -> !DirectiveInfo.isGraphqlSpecifiedDirective(d); + public static final Predicate ExcludeGraphQLSpecifiedDirectivesPredicate = d -> !Directives.isBuiltInDirective(d); /** * Options to use when printing a schema diff --git a/src/main/java/graphql/schema/usage/SchemaUsage.java b/src/main/java/graphql/schema/usage/SchemaUsage.java index 25ed7fb24c..32ac6c42c6 100644 --- a/src/main/java/graphql/schema/usage/SchemaUsage.java +++ b/src/main/java/graphql/schema/usage/SchemaUsage.java @@ -11,7 +11,7 @@ import graphql.schema.GraphQLNamedType; import graphql.schema.GraphQLObjectType; import graphql.schema.GraphQLSchema; -import graphql.schema.idl.DirectiveInfo; +import graphql.Directives; import graphql.schema.idl.ScalarInfo; import java.util.HashSet; @@ -180,7 +180,7 @@ private boolean isReferencedImpl(GraphQLSchema schema, String elementName, Set context) { - if (DirectiveInfo.isGraphqlSpecifiedDirective(graphQLDirective.getName())) { + if (Directives.isBuiltInDirective(graphQLDirective.getName())) { return TraversalControl.ABORT; } String newName = assertNotNull(newNameMap.get(graphQLDirective)); @@ -282,7 +281,7 @@ public TraversalControl visitGraphQLAppliedDirective(GraphQLAppliedDirective gra changeNode(context, newElement); return TraversalControl.ABORT; } - if (DirectiveInfo.isGraphqlSpecifiedDirective(graphQLDirective.getName())) { + if (Directives.isBuiltInDirective(graphQLDirective.getName())) { return TraversalControl.ABORT; } String newName = assertNotNull(newNameMap.get(graphQLDirective)); @@ -516,7 +515,7 @@ public TraversalControl visitGraphQLAppliedDirectiveArgument(GraphQLAppliedDirec @Override public TraversalControl visitGraphQLDirective(GraphQLDirective graphQLDirective, TraverserContext context) { - if (DirectiveInfo.isGraphqlSpecifiedDirective(graphQLDirective)) { + if (Directives.isBuiltInDirective(graphQLDirective)) { return TraversalControl.ABORT; } recordDirectiveName.accept(graphQLDirective); @@ -525,7 +524,7 @@ public TraversalControl visitGraphQLDirective(GraphQLDirective graphQLDirective, @Override public TraversalControl visitGraphQLAppliedDirective(GraphQLAppliedDirective graphQLAppliedDirective, TraverserContext context) { - if (DirectiveInfo.isGraphqlSpecifiedDirective(graphQLAppliedDirective.getName())) { + if (Directives.isBuiltInDirective(graphQLAppliedDirective.getName())) { return TraversalControl.ABORT; } recordDirectiveName.accept(graphQLAppliedDirective); diff --git a/src/test/groovy/graphql/introspection/IntrospectionWithDirectivesSupportTest.groovy b/src/test/groovy/graphql/introspection/IntrospectionWithDirectivesSupportTest.groovy index 591a44146e..d78bb30952 100644 --- a/src/test/groovy/graphql/introspection/IntrospectionWithDirectivesSupportTest.groovy +++ b/src/test/groovy/graphql/introspection/IntrospectionWithDirectivesSupportTest.groovy @@ -90,9 +90,9 @@ class IntrospectionWithDirectivesSupportTest extends Specification { def schemaType = er.data["__schema"] schemaType["directives"] == [ - [name: "include"], [name: "skip"], [name: "example"], [name: "secret"], - [name: "noDefault"], [name: "deprecated"], [name: "specifiedBy"], [name: "oneOf"], - [name: "defer"], [name: "experimental_disableErrorPropagation"] + [name: "include"], [name: "skip"], [name: "defer"], [name: "experimental_disableErrorPropagation"], + [name: "example"], [name: "secret"], [name: "noDefault"], + [name: "deprecated"], [name: "specifiedBy"], [name: "oneOf"] ] schemaType["appliedDirectives"] == [[name: "example", args: [[name: "argName", value: '"onSchema"']]]] @@ -174,8 +174,8 @@ class IntrospectionWithDirectivesSupportTest extends Specification { def definedDirectives = er.data["__schema"]["directives"] // secret is filter out - definedDirectives == [[name: "include"], [name: "skip"], [name: "example"], [name: "deprecated"], [name: "specifiedBy"], [name: "oneOf"], - [name: "defer"], [name: "experimental_disableErrorPropagation"] + definedDirectives == [[name: "include"], [name: "skip"], [name: "defer"], [name: "experimental_disableErrorPropagation"], + [name: "example"], [name: "deprecated"], [name: "specifiedBy"], [name: "oneOf"] ] } diff --git a/src/test/groovy/graphql/util/AnonymizerTest.groovy b/src/test/groovy/graphql/util/AnonymizerTest.groovy index 6865879f55..ae6ed614a4 100644 --- a/src/test/groovy/graphql/util/AnonymizerTest.groovy +++ b/src/test/groovy/graphql/util/AnonymizerTest.groovy @@ -2,7 +2,7 @@ package graphql.util import graphql.AssertException import graphql.TestUtil -import graphql.schema.idl.DirectiveInfo +import graphql.Directives import graphql.schema.idl.SchemaPrinter import spock.lang.Specification @@ -718,7 +718,7 @@ type Object1 { when: def result = Anonymizer.anonymizeSchema(schema) def newSchema = new SchemaPrinter(SchemaPrinter.Options.defaultOptions() - .includeDirectives({!DirectiveInfo.isGraphqlSpecifiedDirective(it) || it == "deprecated"})) + .includeDirectives({!Directives.isBuiltInDirective(it) || it == "deprecated"})) .print(result) then: From f27faf8b3746781f17df37673f110524f4a943d6 Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Wed, 28 Jan 2026 13:31:10 +1000 Subject: [PATCH 3/5] Remove DirectiveInfo class DirectiveInfo is fully replaced by methods in Directives: BUILT_IN_DIRECTIVES, BUILT_IN_DIRECTIVES_MAP, isBuiltInDirective(). Co-Authored-By: Claude Opus 4.5 --- .../graphql/schema/idl/DirectiveInfo.java | 63 ------------------- .../archunit/JSpecifyAnnotationsCheck.groovy | 1 - 2 files changed, 64 deletions(-) delete mode 100644 src/main/java/graphql/schema/idl/DirectiveInfo.java diff --git a/src/main/java/graphql/schema/idl/DirectiveInfo.java b/src/main/java/graphql/schema/idl/DirectiveInfo.java deleted file mode 100644 index 199e493fd5..0000000000 --- a/src/main/java/graphql/schema/idl/DirectiveInfo.java +++ /dev/null @@ -1,63 +0,0 @@ -package graphql.schema.idl; - -import graphql.Directives; -import graphql.PublicApi; -import graphql.schema.GraphQLDirective; - -import java.util.Map; -import java.util.Set; - -/** - * Info on all the directives provided by graphql specification. - * - * @deprecated Use {@link Directives} instead, specifically {@link Directives#BUILT_IN_DIRECTIVES}, - * {@link Directives#BUILT_IN_DIRECTIVES_MAP}, and {@link Directives#isBuiltInDirective(String)}. - */ -@Deprecated -@PublicApi -public class DirectiveInfo { - - /** - * A set of directives which provided by graphql specification - * - * @deprecated Use {@link Directives#BUILT_IN_DIRECTIVES} instead. - */ - @Deprecated - public static final Set GRAPHQL_SPECIFICATION_DIRECTIVES = Directives.BUILT_IN_DIRECTIVES; - - /** - * A map from directive name to directive which provided by specification - * - * @deprecated Use {@link Directives#BUILT_IN_DIRECTIVES_MAP} instead. - */ - @Deprecated - public static final Map GRAPHQL_SPECIFICATION_DIRECTIVE_MAP = Directives.BUILT_IN_DIRECTIVES_MAP; - - /** - * Returns true if a directive with provided directiveName has been defined in graphql specification - * - * @param directiveName the name of directive in question - * - * @return true if the directive provided by graphql specification, and false otherwise - * - * @deprecated Use {@link Directives#isBuiltInDirective(String)} instead. - */ - @Deprecated - public static boolean isGraphqlSpecifiedDirective(String directiveName) { - return Directives.isBuiltInDirective(directiveName); - } - - /** - * Returns true if the provided directive has been defined in graphql specification - * - * @param graphQLDirective the directive in question - * - * @return true if the directive provided by graphql specification, and false otherwise - * - * @deprecated Use {@link Directives#isBuiltInDirective(GraphQLDirective)} instead. - */ - @Deprecated - public static boolean isGraphqlSpecifiedDirective(GraphQLDirective graphQLDirective) { - return Directives.isBuiltInDirective(graphQLDirective); - } -} diff --git a/src/test/groovy/graphql/archunit/JSpecifyAnnotationsCheck.groovy b/src/test/groovy/graphql/archunit/JSpecifyAnnotationsCheck.groovy index 95dc0cc435..8d09336874 100644 --- a/src/test/groovy/graphql/archunit/JSpecifyAnnotationsCheck.groovy +++ b/src/test/groovy/graphql/archunit/JSpecifyAnnotationsCheck.groovy @@ -286,7 +286,6 @@ class JSpecifyAnnotationsCheck extends Specification { "graphql.schema.diff.reporting.PrintStreamReporter", "graphql.schema.diffing.SchemaGraph", "graphql.schema.idl.CombinedWiringFactory", - "graphql.schema.idl.DirectiveInfo", "graphql.schema.idl.MapEnumValuesProvider", "graphql.schema.idl.NaturalEnumValuesProvider", "graphql.schema.idl.RuntimeWiring", From e1b5c3cdf7f7b2a19505b44caa32d4d589d9e703 Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Wed, 28 Jan 2026 13:38:30 +1000 Subject: [PATCH 4/5] Simplify @specifiedBy printing to use specifiedByUrl directly Instead of creating a synthetic applied directive object just to feed it into the directive printing pipeline, print @specifiedBy directly from the first-class specifiedByUrl property as a simple string. Co-Authored-By: Claude Opus 4.5 --- .../graphql/schema/idl/SchemaPrinter.java | 67 +++---------------- 1 file changed, 10 insertions(+), 57 deletions(-) diff --git a/src/main/java/graphql/schema/idl/SchemaPrinter.java b/src/main/java/graphql/schema/idl/SchemaPrinter.java index d1e3220983..b199926588 100644 --- a/src/main/java/graphql/schema/idl/SchemaPrinter.java +++ b/src/main/java/graphql/schema/idl/SchemaPrinter.java @@ -561,8 +561,12 @@ private SchemaElementPrinter scalarPrinter() { printAsAst(out, type.getDefinition(), type.getExtensionDefinitions()); } else { printComments(out, type, ""); - List directives = addOrUpdateSpecifiedByDirectiveIfNeeded(type); - out.format("scalar %s%s\n\n", type.getName(), directivesString(GraphQLScalarType.class, directives)); + List directives = DirectivesUtil.toAppliedDirectives(type).stream() + .filter(d -> !d.getName().equals(SpecifiedByDirective.getName())) + .collect(toList()); + out.format("scalar %s%s%s\n\n", type.getName(), + directivesString(GraphQLScalarType.class, directives), + specifiedByUrlString(type)); } } }; @@ -1106,63 +1110,12 @@ private String getDeprecationReason(GraphQLDirectiveContainer directiveContainer } } - private boolean isSpecifiedByDirectiveAllowed() { - return options.getIncludeDirective().test(SpecifiedByDirective.getName()); - } - - private boolean isSpecifiedByDirective(GraphQLAppliedDirective directive) { - return directive.getName().equals(SpecifiedByDirective.getName()); - } - - private boolean hasSpecifiedByDirective(List directives) { - return directives.stream().anyMatch(this::isSpecifiedByDirective); - } - - private List addOrUpdateSpecifiedByDirectiveIfNeeded(GraphQLScalarType scalarType) { - List directives = DirectivesUtil.toAppliedDirectives(scalarType); + private String specifiedByUrlString(GraphQLScalarType scalarType) { String url = scalarType.getSpecifiedByUrl(); - - if (url == null) { - // first-class property is not set - remove any @specifiedBy applied directive - return directives.stream() - .filter(d -> !isSpecifiedByDirective(d)) - .collect(toList()); - } - if (!hasSpecifiedByDirective(directives) && isSpecifiedByDirectiveAllowed()) { - directives = new ArrayList<>(directives); - directives.add(createSpecifiedByDirective(url)); - } else if (hasSpecifiedByDirective(directives) && isSpecifiedByDirectiveAllowed()) { - // Update URL in case modified by schema transform - directives = updateSpecifiedByDirective(directives, url); + if (url == null || !options.getIncludeDirective().test(SpecifiedByDirective.getName())) { + return ""; } - return directives; - } - - private GraphQLAppliedDirective createSpecifiedByDirective(String url) { - GraphQLAppliedDirectiveArgument arg = GraphQLAppliedDirectiveArgument.newArgument() - .name("url") - .valueProgrammatic(url) - .type(GraphQLString) - .build(); - return GraphQLAppliedDirective.newDirective() - .name("specifiedBy") - .argument(arg) - .build(); - } - - private List updateSpecifiedByDirective(List directives, String url) { - GraphQLAppliedDirectiveArgument newArg = GraphQLAppliedDirectiveArgument.newArgument() - .name("url") - .valueProgrammatic(url) - .type(GraphQLString) - .build(); - - return directives.stream().map(d -> { - if (isSpecifiedByDirective(d)) { - return d.transform(builder -> builder.argument(newArg)); - } - return d; - }).collect(toList()); + return " @specifiedBy(url : \"" + escapeJsonString(url) + "\")"; } private String directiveDefinition(GraphQLDirective directive) { From f361f4fe43ff5cf55ecc6d7d9d010f2a72a00b18 Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Wed, 28 Jan 2026 15:50:18 +1000 Subject: [PATCH 5/5] Add test for modifying a built-in directive via SchemaTransformer Verifies that a built-in directive (e.g. @deprecated) can be modified by adding a new argument through SchemaTransformer, and that all other built-in directives remain unchanged after the transformation. Co-Authored-By: Claude Opus 4.5 --- .../schema/SchemaTransformerTest.groovy | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/test/groovy/graphql/schema/SchemaTransformerTest.groovy b/src/test/groovy/graphql/schema/SchemaTransformerTest.groovy index ebe47ad2c4..bf0279755c 100644 --- a/src/test/groovy/graphql/schema/SchemaTransformerTest.groovy +++ b/src/test/groovy/graphql/schema/SchemaTransformerTest.groovy @@ -1464,4 +1464,53 @@ type Rental { e.getMessage().contains("All types within a GraphQL schema must have unique names") e.getMessage().contains("ExistingType") } + + def "can modify a built-in directive via schema transformation"() { + given: + GraphQLSchema schema = TestUtil.schema(""" + type Query { + hello: String @deprecated(reason: "use goodbye") + goodbye: String + } + """) + + when: + GraphQLSchema newSchema = SchemaTransformer.transformSchema(schema, new GraphQLTypeVisitorStub() { + @Override + TraversalControl visitGraphQLDirective(GraphQLDirective node, TraverserContext context) { + if (node.getName() == "deprecated") { + def changedNode = node.transform({ builder -> + builder.argument(GraphQLArgument.newArgument() + .name("deletionDate") + .type(Scalars.GraphQLString) + .description("The date when this field will be removed")) + }) + return changeNode(context, changedNode) + } + return TraversalControl.CONTINUE + } + }) + + then: "the modified built-in directive has the new argument" + def deprecatedDirective = newSchema.getDirective("deprecated") + deprecatedDirective != null + deprecatedDirective.getArguments().size() == 2 + deprecatedDirective.getArgument("reason") != null + deprecatedDirective.getArgument("deletionDate") != null + deprecatedDirective.getArgument("deletionDate").getType() == Scalars.GraphQLString + + and: "other built-in directives remain unchanged" + newSchema.getDirective("include").getArguments().size() == 1 + newSchema.getDirective("skip").getArguments().size() == 1 + + and: "all built-in directives are still present" + newSchema.getDirective("include") != null + newSchema.getDirective("skip") != null + newSchema.getDirective("deprecated") != null + newSchema.getDirective("specifiedBy") != null + newSchema.getDirective("oneOf") != null + newSchema.getDirective("defer") != null + newSchema.getDirective("experimental_disableErrorPropagation") != null + newSchema.getDirectives().size() == schema.getDirectives().size() + } }