From e80a3e71badc12aecc916ddd0ce945ecf92494ee Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Sat, 2 Dec 2023 17:30:46 +1100 Subject: [PATCH 1/2] Not quite finished - but has directive filtering --- .../graphql/schema/idl/SchemaPrinter.java | 16 ++++--- .../schema/idl/SchemaPrinterTest.groovy | 46 ++++++++++++++++--- 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/main/java/graphql/schema/idl/SchemaPrinter.java b/src/main/java/graphql/schema/idl/SchemaPrinter.java index 84c5c7e6e2..891eec5014 100644 --- a/src/main/java/graphql/schema/idl/SchemaPrinter.java +++ b/src/main/java/graphql/schema/idl/SchemaPrinter.java @@ -876,7 +876,7 @@ String directivesString(Class parentType, boolea private String directivesString(Class parentType, List directives) { directives = directives.stream() // @deprecated is special - we always print it if something is deprecated - .filter(directive -> options.getIncludeDirective().test(directive.getName()) || isDeprecatedDirective(directive)) + .filter(directive -> options.getIncludeDirective().test(directive.getName())) .filter(options.getIncludeSchemaElement()) .collect(toList()); @@ -909,10 +909,7 @@ private String directiveString(GraphQLAppliedDirective directive) { return ""; } if (!options.getIncludeDirective().test(directive.getName())) { - // @deprecated is special - we always print it if something is deprecated - if (!isDeprecatedDirective(directive)) { - return ""; - } + return ""; } StringBuilder sb = new StringBuilder(); @@ -948,6 +945,13 @@ private String directiveString(GraphQLAppliedDirective directive) { return sb.toString(); } + private boolean isDeprecatedDirectiveAllowed() { + // we ask if the special deprecated directive, + // which can be programmatically on a type without an applied directive, + // should be printed or not + return options.getIncludeDirective().test(DeprecatedDirective.getName()); + } + private boolean isDeprecatedDirective(GraphQLAppliedDirective directive) { return directive.getName().equals(DeprecatedDirective.getName()); } @@ -960,7 +964,7 @@ private boolean hasDeprecatedDirective(List directives) private List addDeprecatedDirectiveIfNeeded(GraphQLDirectiveContainer directiveContainer) { List directives = DirectivesUtil.toAppliedDirectives(directiveContainer); - if (!hasDeprecatedDirective(directives)) { + if (!hasDeprecatedDirective(directives) && isDeprecatedDirectiveAllowed()) { directives = new ArrayList<>(directives); String reason = getDeprecationReason(directiveContainer); GraphQLAppliedDirectiveArgument arg = GraphQLAppliedDirectiveArgument.newArgument() diff --git a/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy b/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy index c0c83ddca8..13e0319233 100644 --- a/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy +++ b/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy @@ -1556,7 +1556,7 @@ extend type Query { ''' } - def "@deprecated directives are always printed"() { + def "@deprecated directives are NOT always printed - they used to be"() { given: def idl = """ @@ -1588,7 +1588,7 @@ extend type Query { then: result == '''type Field { - deprecated: Enum @deprecated(reason : "No longer supported") + deprecated: Enum } type Query { @@ -1596,11 +1596,11 @@ type Query { } enum Enum { - enumVal @deprecated(reason : "No longer supported") + enumVal } input Input { - deprecated: String @deprecated(reason : "custom reason") + deprecated: String } ''' } @@ -1641,7 +1641,7 @@ type Query { ''' } - def "@deprecated directive are always printed regardless of options"() { + def "@deprecated directive are NOT always printed regardless of options"() { given: def idl = ''' @@ -1660,6 +1660,37 @@ type Query { then: result == '''type Query { + fieldX: String +} +''' + } + + def "@deprecated directive are printed respecting options"() { + given: + def idl = ''' + + type Query { + fieldX : String @deprecated + } + + ''' + def registry = new SchemaParser().parse(idl) + def runtimeWiring = newRuntimeWiring().build() + def options = SchemaGenerator.Options.defaultOptions() + def schema = new SchemaGenerator().makeExecutableSchema(options, registry, runtimeWiring) + + when: + def printOptions = defaultOptions().includeDirectives({ dName -> (dName == "deprecated") }) + def result = new SchemaPrinter(printOptions).print(schema) + + then: + result == '''"Marks the field, argument, input field or enum value as deprecated" +directive @deprecated( + "The reason for the deprecation" + reason: String = "No longer supported" + ) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION + +type Query { fieldX: String @deprecated(reason : "No longer supported") } ''' @@ -2678,7 +2709,10 @@ input Gun { .query(queryType) .build() when: - def result = "\n" + new SchemaPrinter(noDirectivesOption).print(schema) + + def printOptions = defaultOptions().includeDirectives({d -> true}) + + def result = "\n" + new SchemaPrinter(printOptions).print(schema) println(result) then: From 6c75035851c4c4de0c07b8a194cc0e11dd6e5a7b Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Fri, 16 Feb 2024 15:16:09 +1100 Subject: [PATCH 2/2] Directive definitions now have a predicate and fixed up tests --- .../graphql/schema/idl/SchemaPrinter.java | 72 +++++++++++++++---- .../schema/idl/SchemaPrinterTest.groovy | 47 +++++++++++- .../groovy/graphql/util/AnonymizerTest.groovy | 4 +- 3 files changed, 107 insertions(+), 16 deletions(-) diff --git a/src/main/java/graphql/schema/idl/SchemaPrinter.java b/src/main/java/graphql/schema/idl/SchemaPrinter.java index 891eec5014..9ac97f6a2e 100644 --- a/src/main/java/graphql/schema/idl/SchemaPrinter.java +++ b/src/main/java/graphql/schema/idl/SchemaPrinter.java @@ -98,6 +98,8 @@ public static class Options { private final boolean descriptionsAsHashComments; + private final Predicate includeDirectiveDefinition; + private final Predicate includeDirective; private final Predicate includeSchemaElement; @@ -110,6 +112,7 @@ private Options(boolean includeIntrospectionTypes, boolean includeScalars, boolean includeSchemaDefinition, boolean includeDirectiveDefinitions, + Predicate includeDirectiveDefinition, boolean useAstDefinitions, boolean descriptionsAsHashComments, Predicate includeDirective, @@ -120,6 +123,7 @@ private Options(boolean includeIntrospectionTypes, this.includeScalars = includeScalars; this.includeSchemaDefinition = includeSchemaDefinition; this.includeDirectiveDefinitions = includeDirectiveDefinitions; + this.includeDirectiveDefinition = includeDirectiveDefinition; this.includeDirective = includeDirective; this.useAstDefinitions = useAstDefinitions; this.descriptionsAsHashComments = descriptionsAsHashComments; @@ -144,6 +148,10 @@ public boolean isIncludeDirectiveDefinitions() { return includeDirectiveDefinitions; } + public Predicate getIncludeDirectiveDefinition() { + return includeDirectiveDefinition; + } + public Predicate getIncludeDirective() { return includeDirective; } @@ -164,14 +172,16 @@ public boolean isUseAstDefinitions() { return useAstDefinitions; } - public boolean isIncludeAstDefinitionComments() { return includeAstDefinitionComments; } + public boolean isIncludeAstDefinitionComments() { + return includeAstDefinitionComments; + } public static Options defaultOptions() { return new Options(false, true, false, true, - false, + directive -> true, false, false, directive -> true, element -> true, @@ -191,7 +201,7 @@ public Options includeIntrospectionTypes(boolean flag) { this.includeScalars, this.includeSchemaDefinition, this.includeDirectiveDefinitions, - this.useAstDefinitions, + this.includeDirectiveDefinition, this.useAstDefinitions, this.descriptionsAsHashComments, this.includeDirective, this.includeSchemaElement, @@ -211,7 +221,7 @@ public Options includeScalarTypes(boolean flag) { flag, this.includeSchemaDefinition, this.includeDirectiveDefinitions, - this.useAstDefinitions, + this.includeDirectiveDefinition, this.useAstDefinitions, this.descriptionsAsHashComments, this.includeDirective, this.includeSchemaElement, @@ -234,6 +244,7 @@ public Options includeSchemaDefinition(boolean flag) { this.includeScalars, flag, this.includeDirectiveDefinitions, + this.includeDirectiveDefinition, this.useAstDefinitions, this.descriptionsAsHashComments, this.includeDirective, @@ -259,6 +270,29 @@ public Options includeDirectiveDefinitions(boolean flag) { this.includeScalars, this.includeSchemaDefinition, flag, + directive -> flag, + this.useAstDefinitions, + this.descriptionsAsHashComments, + this.includeDirective, + this.includeSchemaElement, + this.comparatorRegistry, + this.includeAstDefinitionComments); + } + + + /** + * This is a Predicate that decides whether a directive definition is printed. + * + * @param includeDirectiveDefinition the predicate to decide of a directive defintion is printed + * + * @return new instance of options + */ + public Options includeDirectiveDefinition(Predicate includeDirectiveDefinition) { + return new Options(this.includeIntrospectionTypes, + this.includeScalars, + this.includeSchemaDefinition, + this.includeDirectiveDefinitions, + includeDirectiveDefinition, this.useAstDefinitions, this.descriptionsAsHashComments, this.includeDirective, @@ -280,6 +314,7 @@ public Options includeDirectives(boolean flag) { this.includeScalars, this.includeSchemaDefinition, this.includeDirectiveDefinitions, + this.includeDirectiveDefinition, this.useAstDefinitions, this.descriptionsAsHashComments, directive -> flag, @@ -300,6 +335,7 @@ public Options includeDirectives(Predicate includeDirective) { this.includeScalars, this.includeSchemaDefinition, this.includeDirectiveDefinitions, + this.includeDirectiveDefinition, this.useAstDefinitions, this.descriptionsAsHashComments, includeDirective, @@ -308,6 +344,7 @@ public Options includeDirectives(Predicate includeDirective) { this.includeAstDefinitionComments); } + /** * This is a general purpose Predicate that decides whether a schema element is printed ever. * @@ -321,6 +358,7 @@ public Options includeSchemaElement(Predicate includeSchem this.includeScalars, this.includeSchemaDefinition, this.includeDirectiveDefinitions, + this.includeDirectiveDefinition, this.useAstDefinitions, this.descriptionsAsHashComments, this.includeDirective, @@ -342,6 +380,7 @@ public Options useAstDefinitions(boolean flag) { this.includeScalars, this.includeSchemaDefinition, this.includeDirectiveDefinitions, + this.includeDirectiveDefinition, flag, this.descriptionsAsHashComments, this.includeDirective, @@ -365,6 +404,7 @@ public Options descriptionsAsHashComments(boolean flag) { this.includeScalars, this.includeSchemaDefinition, this.includeDirectiveDefinitions, + this.includeDirectiveDefinition, this.useAstDefinitions, flag, this.includeDirective, @@ -387,6 +427,7 @@ public Options setComparators(GraphqlTypeComparatorRegistry comparatorRegistry) this.includeScalars, this.includeSchemaDefinition, this.includeDirectiveDefinitions, + this.includeDirectiveDefinition, this.useAstDefinitions, this.descriptionsAsHashComments, this.includeDirective, @@ -409,6 +450,7 @@ public Options includeAstDefinitionComments(boolean flag) { this.includeScalars, this.includeSchemaDefinition, this.includeDirectiveDefinitions, + this.includeDirectiveDefinition, this.useAstDefinitions, this.descriptionsAsHashComments, this.includeDirective, @@ -638,7 +680,9 @@ private SchemaElementPrinter unionPrinter() { private SchemaElementPrinter directivePrinter() { return (out, directive, visibility) -> { - if (options.isIncludeDirectiveDefinitions()) { + boolean isOnEver = options.isIncludeDirectiveDefinitions(); + boolean specificTest = options.getIncludeDirectiveDefinition().test(directive.getName()); + if (isOnEver && specificTest) { String s = directiveDefinition(directive); out.format("%s", s); out.print("\n\n"); @@ -964,14 +1008,14 @@ private boolean hasDeprecatedDirective(List directives) private List addDeprecatedDirectiveIfNeeded(GraphQLDirectiveContainer directiveContainer) { List directives = DirectivesUtil.toAppliedDirectives(directiveContainer); - if (!hasDeprecatedDirective(directives) && isDeprecatedDirectiveAllowed()) { + if (!hasDeprecatedDirective(directives) && isDeprecatedDirectiveAllowed()) { directives = new ArrayList<>(directives); - String reason = getDeprecationReason(directiveContainer); - GraphQLAppliedDirectiveArgument arg = GraphQLAppliedDirectiveArgument.newArgument() - .name("reason") - .valueProgrammatic(reason) - .type(GraphQLString) - .build(); + String reason = getDeprecationReason(directiveContainer); + GraphQLAppliedDirectiveArgument arg = GraphQLAppliedDirectiveArgument.newArgument() + .name("reason") + .valueProgrammatic(reason) + .type(GraphQLString) + .build(); GraphQLAppliedDirective directive = GraphQLAppliedDirective.newDirective() .name("deprecated") .argument(arg) @@ -1108,7 +1152,7 @@ private void printComments(PrintWriter out, Object graphQLType, String prefix) { if (options.isIncludeAstDefinitionComments()) { String commentsText = getAstDefinitionComments(graphQLType); if (!isNullOrEmpty(commentsText)) { - List lines = Arrays.asList(commentsText.split("\n") ); + List lines = Arrays.asList(commentsText.split("\n")); if (!lines.isEmpty()) { printMultiLineHashDescription(out, prefix, lines); } @@ -1183,7 +1227,7 @@ private String getAstDefinitionComments(Object commentHolder) { } private String comments(List comments) { - if ( comments == null || comments.isEmpty() ) { + if (comments == null || comments.isEmpty()) { return null; } String s = comments.stream().map(c -> c.getContent()).collect(joining("\n", "", "\n")); diff --git a/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy b/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy index 13e0319233..1cff3fffec 100644 --- a/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy +++ b/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy @@ -2710,7 +2710,7 @@ input Gun { .build() when: - def printOptions = defaultOptions().includeDirectives({d -> true}) + def printOptions = defaultOptions().includeDirectiveDefinitions(false).includeDirectives({ d -> true }) def result = "\n" + new SchemaPrinter(printOptions).print(schema) println(result) @@ -2734,4 +2734,49 @@ input Input { } """ } + + def "can use predicate for directive definitions"() { + + def schema = TestUtil.schema(""" + type Query { + field: String @deprecated + } + """) + + + def options = defaultOptions() + .includeDirectiveDefinitions(true) + .includeDirectiveDefinition({ it != "skip" }) + def result = new SchemaPrinter(options).print(schema) + + expect: "has no skip definition" + + result == """"Marks the field, argument, input field or enum value as deprecated" +directive @deprecated( + "The reason for the deprecation" + reason: String = "No longer supported" + ) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION + +"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 + +"Indicates an Input Object is a OneOf Input Object." +directive @oneOf on INPUT_OBJECT + +"Exposes a URL that specifies the behaviour of this scalar." +directive @specifiedBy( + "The URL that specifies the behaviour of this scalar." + url: String! + ) on SCALAR + +type Query { + field: String @deprecated(reason : "No longer supported") } +""" + } +} + + diff --git a/src/test/groovy/graphql/util/AnonymizerTest.groovy b/src/test/groovy/graphql/util/AnonymizerTest.groovy index d1bf02e20a..439132eda3 100644 --- a/src/test/groovy/graphql/util/AnonymizerTest.groovy +++ b/src/test/groovy/graphql/util/AnonymizerTest.groovy @@ -718,7 +718,7 @@ type Object1 { when: def result = Anonymizer.anonymizeSchema(schema) def newSchema = new SchemaPrinter(SchemaPrinter.Options.defaultOptions() - .includeDirectives({!DirectiveInfo.isGraphqlSpecifiedDirective(it)})) + .includeDirectives({!DirectiveInfo.isGraphqlSpecifiedDirective(it) || it == "deprecated"})) .print(result) then: @@ -729,6 +729,8 @@ type Object1 { directive @Directive1(argument1: String! = "stringValue4") repeatable on SCHEMA | SCALAR | OBJECT | FIELD_DEFINITION | ARGUMENT_DEFINITION | INTERFACE | UNION | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION + directive @deprecated(reason: String) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION + interface Interface1 @Directive1(argument1 : "stringValue12") { field2: String field3: Enum1