From 1dc971eb498b1ad10423f3d542018cf402bcb73e Mon Sep 17 00:00:00 2001 From: pelletier197 Date: Thu, 1 Oct 2020 21:31:14 -0400 Subject: [PATCH 1/2] fix schema wiring (#30) (cherry picked from commit d70dbe62873b3189d82c547d365e0bf9c90dc9c6) --- .../FieldValidatorDataFetcher.java | 79 +++++++++++++++++++ .../schemawiring/ValidationSchemaWiring.java | 44 ++++------- 2 files changed, 94 insertions(+), 29 deletions(-) create mode 100644 src/main/java/graphql/validation/schemawiring/FieldValidatorDataFetcher.java diff --git a/src/main/java/graphql/validation/schemawiring/FieldValidatorDataFetcher.java b/src/main/java/graphql/validation/schemawiring/FieldValidatorDataFetcher.java new file mode 100644 index 0000000..a02836a --- /dev/null +++ b/src/main/java/graphql/validation/schemawiring/FieldValidatorDataFetcher.java @@ -0,0 +1,79 @@ +package graphql.validation.schemawiring; + +import graphql.GraphQLError; +import graphql.schema.*; +import graphql.validation.interpolation.MessageInterpolator; +import graphql.validation.rules.OnValidationErrorStrategy; +import graphql.validation.rules.TargetedValidationRules; +import graphql.validation.rules.ValidationRule; +import graphql.validation.rules.ValidationRules; +import graphql.validation.util.Util; + +import java.util.List; +import java.util.Locale; + +public class FieldValidatorDataFetcher implements DataFetcher { + private final OnValidationErrorStrategy errorStrategy; + private final MessageInterpolator messageInterpolator; + private final DataFetcher defaultDataFetcher; + private final Locale defaultLocale; + private final ValidationRules validationRules; + private TargetedValidationRules applicableRules; + + public FieldValidatorDataFetcher(OnValidationErrorStrategy errorStrategy, + MessageInterpolator messageInterpolator, + DataFetcher defaultDataFetcher, + Locale defaultLocale, + ValidationRules validationRules) { + this.errorStrategy = errorStrategy; + this.messageInterpolator = messageInterpolator; + this.defaultDataFetcher = defaultDataFetcher; + this.defaultLocale = defaultLocale; + this.validationRules = validationRules; + this.applicableRules = null; + } + + @Override + public Object get(DataFetchingEnvironment environment) throws Exception { + if (!wereApplicableRulesFetched()) { + fetchApplicableRules(environment); + } + + // When no validation is performed, this data fetcher is a pass-through + if (applicableRules.isEmpty()) { + return defaultDataFetcher.get(environment); + } + + List errors = applicableRules.runValidationRules(environment, messageInterpolator, defaultLocale); + if (!errors.isEmpty()) { + if (!errorStrategy.shouldContinue(errors, environment)) { + return errorStrategy.onErrorValue(errors, environment); + } + } + + Object returnValue = defaultDataFetcher.get(environment); + if (errors.isEmpty()) { + return returnValue; + } + return Util.mkDFRFromFetchedResult(errors, returnValue); + } + + private void fetchApplicableRules(DataFetchingEnvironment environment) { + final GraphQLFieldDefinition field = environment.getFieldDefinition(); + final GraphQLFieldsContainer container = asContainer(environment); + + applicableRules = validationRules.buildRulesFor(field, container); + } + + private GraphQLFieldsContainer asContainer(DataFetchingEnvironment environment) { + final GraphQLType parent = environment.getParentType(); + if (parent == null) { + return null; + } + return (GraphQLFieldsContainer) environment.getParentType(); + } + + private boolean wereApplicableRulesFetched() { + return applicableRules != null; + } +} diff --git a/src/main/java/graphql/validation/schemawiring/ValidationSchemaWiring.java b/src/main/java/graphql/validation/schemawiring/ValidationSchemaWiring.java index 26d9c97..36cf6ba 100644 --- a/src/main/java/graphql/validation/schemawiring/ValidationSchemaWiring.java +++ b/src/main/java/graphql/validation/schemawiring/ValidationSchemaWiring.java @@ -1,6 +1,5 @@ package graphql.validation.schemawiring; -import graphql.GraphQLError; import graphql.PublicApi; import graphql.schema.DataFetcher; import graphql.schema.GraphQLFieldDefinition; @@ -11,17 +10,15 @@ import graphql.validation.rules.OnValidationErrorStrategy; import graphql.validation.rules.TargetedValidationRules; import graphql.validation.rules.ValidationRules; -import graphql.validation.util.Util; -import java.util.List; import java.util.Locale; /** - * A {@link graphql.schema.idl.SchemaDirectiveWiring} that can be used to inject validation rules into the data fetchers + * A {@link SchemaDirectiveWiring} that can be used to inject validation rules into the data fetchers * when the graphql schema is being built. It will use the validation rules and ask each one of they apply to the field and or its * arguments. *

- * If there are rules that apply then it will it will change the {@link graphql.schema.DataFetcher} of that field so that rules get run + * If there are rules that apply then it will it will change the {@link DataFetcher} of that field so that rules get run * BEFORE the original field fetch is run. */ @PublicApi @@ -38,40 +35,29 @@ public GraphQLFieldDefinition onField(SchemaDirectiveWiringEnvironment currentDF = env.getCodeRegistry().getDataFetcher(fieldsContainer, fieldDefinition); + final DataFetcher newDF = buildValidatingDataFetcher(errorStrategy, messageInterpolator, currentDF, locale); env.getCodeRegistry().dataFetcher(fieldsContainer, fieldDefinition, newDF); return fieldDefinition; } - private DataFetcher buildValidatingDataFetcher(TargetedValidationRules rules, OnValidationErrorStrategy errorStrategy, MessageInterpolator messageInterpolator, DataFetcher currentDF, final Locale defaultLocale) { - // ok we have some rules that need to be applied to this field and its arguments - return environment -> { - List errors = rules.runValidationRules(environment, messageInterpolator, defaultLocale); - if (!errors.isEmpty()) { - // should we continue? - if (!errorStrategy.shouldContinue(errors, environment)) { - return errorStrategy.onErrorValue(errors, environment); - } - } - // we have no validation errors or they said continue so call the underlying data fetcher - Object returnValue = currentDF.get(environment); - if (errors.isEmpty()) { - return returnValue; - } - return Util.mkDFRFromFetchedResult(errors, returnValue); - }; + private DataFetcher buildValidatingDataFetcher(OnValidationErrorStrategy errorStrategy, + MessageInterpolator messageInterpolator, + DataFetcher currentDF, + final Locale defaultLocale) { + return new FieldValidatorDataFetcher( + errorStrategy, + messageInterpolator, + currentDF, + defaultLocale, + ruleCandidates + ); } } From 60d176dbffaafbf13d5060c128511bf18963d385 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Sun, 4 Oct 2020 12:48:36 +1100 Subject: [PATCH 2/2] This allows circular types to be applied to. It fixes a StackOverflow bug (#34) (#35) --- .../AbstractDirectiveConstraint.java | 2 +- .../util/DirectivesAndTypeWalker.java | 28 ++++++-- .../rules/ValidationRulesTest.groovy | 72 +++++++++++++++++++ .../util/DirectivesAndTypeWalkerTest.groovy | 32 +++++++++ 4 files changed, 127 insertions(+), 7 deletions(-) create mode 100644 src/test/groovy/graphql/validation/util/DirectivesAndTypeWalkerTest.groovy diff --git a/src/main/java/graphql/validation/constraints/AbstractDirectiveConstraint.java b/src/main/java/graphql/validation/constraints/AbstractDirectiveConstraint.java index 288b318..f1ccf6a 100644 --- a/src/main/java/graphql/validation/constraints/AbstractDirectiveConstraint.java +++ b/src/main/java/graphql/validation/constraints/AbstractDirectiveConstraint.java @@ -64,7 +64,7 @@ public boolean appliesTo(GraphQLFieldDefinition fieldDefinition, GraphQLFieldsCo @Override public boolean appliesTo(GraphQLArgument argument, GraphQLFieldDefinition fieldDefinition, GraphQLFieldsContainer fieldsContainer) { - boolean suitable = DirectivesAndTypeWalker.isSuitable(argument, (inputType, directive) -> { + boolean suitable = new DirectivesAndTypeWalker().isSuitable(argument, (inputType, directive) -> { boolean hasNamedDirective = directive.getName().equals(this.getName()); if (hasNamedDirective) { inputType = Util.unwrapNonNull(inputType); diff --git a/src/main/java/graphql/validation/util/DirectivesAndTypeWalker.java b/src/main/java/graphql/validation/util/DirectivesAndTypeWalker.java index 6d32590..53d7e07 100644 --- a/src/main/java/graphql/validation/util/DirectivesAndTypeWalker.java +++ b/src/main/java/graphql/validation/util/DirectivesAndTypeWalker.java @@ -8,34 +8,45 @@ import graphql.schema.GraphQLInputObjectType; import graphql.schema.GraphQLInputType; import graphql.schema.GraphQLList; +import graphql.schema.GraphQLTypeUtil; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.function.BiFunction; @Internal public class DirectivesAndTypeWalker { - public static boolean isSuitable(GraphQLArgument argument, BiFunction isSuitable) { + private final Map seenTypes = new HashMap<>(); + + public boolean isSuitable(GraphQLArgument argument, BiFunction isSuitable) { GraphQLInputType inputType = argument.getType(); List directives = argument.getDirectives(); return walkInputType(inputType, directives, isSuitable); } - private static boolean walkInputType(GraphQLInputType inputType, List directives, BiFunction isSuitable) { + private boolean walkInputType(GraphQLInputType inputType, List directives, BiFunction isSuitable) { + String typeName = GraphQLTypeUtil.unwrapAll(inputType).getName(); GraphQLInputType unwrappedInputType = Util.unwrapNonNull(inputType); for (GraphQLDirective directive : directives) { if (isSuitable.apply(unwrappedInputType, directive)) { - return true; + return seen(typeName,true); } } if (unwrappedInputType instanceof GraphQLInputObjectType) { GraphQLInputObjectType inputObjType = (GraphQLInputObjectType) unwrappedInputType; + if (seenTypes.containsKey(typeName)) { + return seenTypes.get(typeName); + } + seen(typeName,false); + for (GraphQLInputObjectField inputField : inputObjType.getFieldDefinitions()) { inputType = inputField.getType(); directives = inputField.getDirectives(); if (walkInputType(inputType, directives, isSuitable)) { - return true; + return seen(typeName,true); } } } @@ -44,11 +55,16 @@ private static boolean walkInputType(GraphQLInputType inputType, List + return "OK" + } + + def runtime = RuntimeWiring.newRuntimeWiring() + .type(newTypeWiring("Query").dataFetcher("request", df)) + .directiveWiring(validationWiring) + .build() + def graphQLSchema = TestUtil.schema(sdl, runtime) + def graphQL = GraphQL.newGraphQL(graphQLSchema).build() + + when: + + def er = graphQL.execute(''' + { + request ( + arg : { + title : "Mr BRAD", givenName : "BRADLEY" , surName : "BAKER" + inner : { title : "Mr BRAD", givenName : "BRADLEY" , surName : "BAKER" } + innerList : [ { title : "Mr BRAD", givenName : "BRADLEY" , surName : "BAKER" } ] + } + ) + } + ''') + then: + er != null + er.data["request"] == null + er.errors.size() == 6 + + er.errors[0].getMessage() == "/request/arg/givenName size must be between 1 and 1" + er.errors[1].getMessage() == "/request/arg/inner/givenName size must be between 1 and 1" + er.errors[2].getMessage() == "/request/arg/inner/title size must be between 1 and 1" + er.errors[3].getMessage() == "/request/arg/innerList[0]/givenName size must be between 1 and 1" + er.errors[4].getMessage() == "/request/arg/innerList[0]/title size must be between 1 and 1" + er.errors[5].getMessage() == "/request/arg/title size must be between 1 and 1" + } } diff --git a/src/test/groovy/graphql/validation/util/DirectivesAndTypeWalkerTest.groovy b/src/test/groovy/graphql/validation/util/DirectivesAndTypeWalkerTest.groovy new file mode 100644 index 0000000..30d157e --- /dev/null +++ b/src/test/groovy/graphql/validation/util/DirectivesAndTypeWalkerTest.groovy @@ -0,0 +1,32 @@ +package graphql.validation.util + + +import graphql.schema.GraphQLObjectType +import graphql.validation.TestUtil +import spock.lang.Specification + +class DirectivesAndTypeWalkerTest extends Specification { + + def "can walk self referencing types"() { + def sdl = """ + input TestInput @deprecated { + name : String + list : [TestInput] + self : TestInput + } + + type Query { + f(arg : TestInput) : String + } + """ + + def schema = TestUtil.schema(sdl) + def arg = (schema.getType("Query") as GraphQLObjectType).getFieldDefinition("f").getArgument("arg") + def callback = { t, d -> true } + when: + def suitable = new DirectivesAndTypeWalker().isSuitable(arg, callback) + + then: + suitable + } +}