From 3557a713f46bdf0b0e99e5b89c1f191c8f211a49 Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Wed, 11 Dec 2024 00:19:24 +0100 Subject: [PATCH 01/27] Add @errorHandling --- src/main/java/graphql/Directives.java | 49 ++++++++++++++----- src/main/java/graphql/Enums.java | 35 +++++++++++++ .../java/graphql/execution/Execution.java | 32 ++++++------ .../graphql/execution/ExecutionContext.java | 11 +++-- .../execution/ExecutionContextBuilder.java | 15 ++++-- .../execution/NonNullableFieldValidator.java | 2 +- .../graphql/schema/idl/SchemaGenerator.java | 31 ++++++------ ...SchemaGeneratorAppliedDirectiveHelper.java | 2 +- .../schema/idl/SchemaGeneratorHelper.java | 19 ++++--- .../NonNullableFieldValidatorTest.groovy | 4 +- 10 files changed, 136 insertions(+), 64 deletions(-) create mode 100644 src/main/java/graphql/Enums.java diff --git a/src/main/java/graphql/Directives.java b/src/main/java/graphql/Directives.java index f0d9eb6745..ac78a9cb0e 100644 --- a/src/main/java/graphql/Directives.java +++ b/src/main/java/graphql/Directives.java @@ -1,23 +1,13 @@ package graphql; -import graphql.language.BooleanValue; -import graphql.language.Description; -import graphql.language.DirectiveDefinition; -import graphql.language.StringValue; +import graphql.language.*; import graphql.schema.GraphQLDirective; +import graphql.schema.GraphQLEnumType; import static graphql.Scalars.GraphQLBoolean; import static graphql.Scalars.GraphQLString; -import static graphql.introspection.Introspection.DirectiveLocation.ARGUMENT_DEFINITION; -import static graphql.introspection.Introspection.DirectiveLocation.ENUM_VALUE; -import static graphql.introspection.Introspection.DirectiveLocation.FIELD; -import static graphql.introspection.Introspection.DirectiveLocation.FIELD_DEFINITION; -import static graphql.introspection.Introspection.DirectiveLocation.FRAGMENT_SPREAD; -import static graphql.introspection.Introspection.DirectiveLocation.INLINE_FRAGMENT; -import static graphql.introspection.Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION; -import static graphql.introspection.Introspection.DirectiveLocation.INPUT_OBJECT; -import static graphql.introspection.Introspection.DirectiveLocation.SCALAR; +import static graphql.introspection.Introspection.DirectiveLocation.*; import static graphql.language.DirectiveLocation.newDirectiveLocation; import static graphql.language.InputValueDefinition.newInputValueDefinition; import static graphql.language.NonNullType.newNonNullType; @@ -37,6 +27,7 @@ public class Directives { private static final String SPECIFIED_BY = "specifiedBy"; private static final String ONE_OF = "oneOf"; private static final String DEFER = "defer"; + private static final String ERROR_HANDLING = "errorHandling"; public static final DirectiveDefinition DEPRECATED_DIRECTIVE_DEFINITION; public static final DirectiveDefinition INCLUDE_DIRECTIVE_DEFINITION; @@ -46,6 +37,8 @@ public class Directives { public static final DirectiveDefinition ONE_OF_DIRECTIVE_DEFINITION; @ExperimentalApi public static final DirectiveDefinition DEFER_DIRECTIVE_DEFINITION; + @ExperimentalApi + public static final DirectiveDefinition ERROR_HANDLING_DIRECTIVE_DEFINITION; public static final String BOOLEAN = "Boolean"; public static final String STRING = "String"; @@ -133,6 +126,19 @@ public class Directives { .type(newTypeName().name(STRING).build()) .build()) .build(); + ERROR_HANDLING_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition() + .name(ERROR_HANDLING) + .directiveLocation(newDirectiveLocation().name(QUERY.name()).build()) + .directiveLocation(newDirectiveLocation().name(MUTATION.name()).build()) + .directiveLocation(newDirectiveLocation().name(SUBSCRIPTION.name()).build()) + .description(createDescription("This directive controls how to handle errors")) + .inputValueDefinition( + newInputValueDefinition() + .name("onError") + .type(newNonNullType(newTypeName().name(Enums.ON_ERROR).build()).build()) + .defaultValue(EnumValue.newEnumValue(Enums.ON_ERROR_PROPAGATE).build()) + .build()) + .build(); } /** @@ -226,6 +232,23 @@ public class Directives { .definition(ONE_OF_DIRECTIVE_DEFINITION) .build(); + @ExperimentalApi + public static final GraphQLDirective ErrorHandlingDirective = GraphQLDirective.newDirective() + .name(ERROR_HANDLING) + .description("This directive controls how to handle errors.") + .argument(newArgument() + .name("onError") + .type(nonNull(GraphQLEnumType.newEnum() + .name(Enums.ON_ERROR) + .value(Enums.ON_ERROR_PROPAGATE) + .value(Enums.ON_ERROR_NULL) + .build())) + .defaultValueProgrammatic(Enums.ON_ERROR_PROPAGATE) + .description("The URL that specifies the behaviour of this scalar.")) + .validLocations(QUERY, MUTATION, SUBSCRIPTION) + .definition(ERROR_HANDLING_DIRECTIVE_DEFINITION) + .build(); + private static Description createDescription(String s) { return new Description(s, null, false); } diff --git a/src/main/java/graphql/Enums.java b/src/main/java/graphql/Enums.java new file mode 100644 index 0000000000..90df54f527 --- /dev/null +++ b/src/main/java/graphql/Enums.java @@ -0,0 +1,35 @@ +package graphql; + +import graphql.language.Description; +import graphql.language.EnumTypeDefinition; +import graphql.language.EnumValueDefinition; + +/** + * The enums that are understood by graphql-java + */ +public class Enums { + public static final String ON_ERROR = "OnError"; + public static final String ON_ERROR_PROPAGATE = "PROPAGATE"; + public static final String ON_ERROR_NULL = "NULL"; + + @ExperimentalApi + public static final EnumTypeDefinition ON_ERROR_TYPE_DEFINITION; + + static { + ON_ERROR_TYPE_DEFINITION = EnumTypeDefinition.newEnumTypeDefinition() + .name(ON_ERROR) + .enumValueDefinition( + EnumValueDefinition.newEnumValueDefinition() + .name(ON_ERROR_PROPAGATE) + .description(new Description("If the error happens in a non-nullable position, the error is propagated to the neareast nullable parent position", null, true)) + .build() + ) + .enumValueDefinition( + EnumValueDefinition.newEnumValueDefinition() + .name(ON_ERROR_NULL) + .description(new Description("Null is always returned regardless of whether the position is nullable or not", null, true)) + .build() + ) + .build(); + } +} diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 2403e5294d..6d3cc4574a 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -1,13 +1,7 @@ package graphql.execution; -import graphql.ExecutionInput; -import graphql.ExecutionResult; -import graphql.ExecutionResultImpl; -import graphql.ExperimentalApi; -import graphql.GraphQLContext; -import graphql.GraphQLError; -import graphql.Internal; +import graphql.*; import graphql.execution.incremental.IncrementalCallState; import graphql.execution.instrumentation.Instrumentation; import graphql.execution.instrumentation.InstrumentationContext; @@ -20,22 +14,16 @@ import graphql.extensions.ExtensionsBuilder; import graphql.incremental.DelayedIncrementalPartialResult; import graphql.incremental.IncrementalExecutionResultImpl; -import graphql.language.Document; -import graphql.language.FragmentDefinition; -import graphql.language.NodeUtil; -import graphql.language.OperationDefinition; -import graphql.language.VariableDefinition; +import graphql.language.*; import graphql.schema.GraphQLObjectType; import graphql.schema.GraphQLSchema; import graphql.schema.impl.SchemaUtil; import org.reactivestreams.Publisher; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.Optional; +import java.util.*; import java.util.concurrent.CompletableFuture; +import static graphql.Directives.*; import static graphql.execution.ExecutionContextBuilder.newExecutionContextBuilder; import static graphql.execution.ExecutionStepInfo.newExecutionStepInfo; import static graphql.execution.ExecutionStrategyParameters.newParameters; @@ -106,6 +94,7 @@ public CompletableFuture execute(Document document, GraphQLSche .locale(executionInput.getLocale()) .valueUnboxer(valueUnboxer) .executionInput(executionInput) + .propagateErrors(propagateErrors(coercedVariables, operationDefinition.getDirectives(), true)) .build(); executionContext.getGraphQLContext().put(ResultNodesInfo.RESULT_NODES_INFO, executionContext.getResultNodesInfo()); @@ -262,4 +251,15 @@ private ExecutionResult mergeExtensionsBuilderIfPresent(ExecutionResult executio } return executionResult; } + + private boolean propagateErrors(CoercedVariables variables, List directives, boolean defaultValue) { + Directive foundDirective = NodeUtil.findNodeByName(directives, ERROR_HANDLING_DIRECTIVE_DEFINITION.getName()); + if (foundDirective != null) { + Map argumentValues = ValuesResolver.getArgumentValues(ErrorHandlingDirective.getArguments(), foundDirective.getArguments(), variables, GraphQLContext.getDefault(), Locale.getDefault()); + Object flag = argumentValues.get("onError"); + Assert.assertTrue(flag instanceof String, "The '%s' directive MUST have a OnError value for the 'if' argument", ERROR_HANDLING_DIRECTIVE_DEFINITION.getName()); + return flag.equals(Enums.ON_ERROR_PROPAGATE); + } + return defaultValue; + } } diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index ddb1fd6db8..e34ae793e3 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -3,11 +3,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import graphql.ExecutionInput; -import graphql.GraphQLContext; -import graphql.GraphQLError; -import graphql.Internal; -import graphql.PublicApi; +import graphql.*; import graphql.collect.ImmutableKit; import graphql.execution.incremental.IncrementalCallState; import graphql.execution.instrumentation.Instrumentation; @@ -59,6 +55,7 @@ public class ExecutionContext { private final ValueUnboxer valueUnboxer; private final ExecutionInput executionInput; private final Supplier queryTree; + private final boolean propagateErrors; // this is modified after creation so it needs to be volatile to ensure visibility across Threads private volatile DataLoaderDispatchStrategy dataLoaderDispatcherStrategy = DataLoaderDispatchStrategy.NO_OP; @@ -88,6 +85,7 @@ public class ExecutionContext { this.executionInput = builder.executionInput; this.dataLoaderDispatcherStrategy = builder.dataLoaderDispatcherStrategy; this.queryTree = FpKit.interThreadMemoize(() -> ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(graphQLSchema, operationDefinition, fragmentsByName, coercedVariables)); + this.propagateErrors = builder.propagateErrors; } @@ -170,6 +168,9 @@ public ValueUnboxer getValueUnboxer() { return valueUnboxer; } + @ExperimentalApi + public boolean propagateErrors() { return propagateErrors; } + /** * @return true if the current operation is a Query */ diff --git a/src/main/java/graphql/execution/ExecutionContextBuilder.java b/src/main/java/graphql/execution/ExecutionContextBuilder.java index b60c793b20..d4b970d8d3 100644 --- a/src/main/java/graphql/execution/ExecutionContextBuilder.java +++ b/src/main/java/graphql/execution/ExecutionContextBuilder.java @@ -2,11 +2,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import graphql.ExecutionInput; -import graphql.GraphQLContext; -import graphql.GraphQLError; -import graphql.Internal; -import graphql.PublicApi; +import graphql.*; import graphql.collect.ImmutableKit; import graphql.execution.instrumentation.Instrumentation; import graphql.execution.instrumentation.InstrumentationState; @@ -46,6 +42,7 @@ public class ExecutionContextBuilder { Object localContext; ExecutionInput executionInput; DataLoaderDispatchStrategy dataLoaderDispatcherStrategy = DataLoaderDispatchStrategy.NO_OP; + boolean propagateErrors = true; /** * @return a new builder of {@link graphql.execution.ExecutionContext}s @@ -92,6 +89,7 @@ public ExecutionContextBuilder() { valueUnboxer = other.getValueUnboxer(); executionInput = other.getExecutionInput(); dataLoaderDispatcherStrategy = other.getDataLoaderDispatcherStrategy(); + propagateErrors = other.propagateErrors(); } public ExecutionContextBuilder instrumentation(Instrumentation instrumentation) { @@ -216,6 +214,13 @@ public ExecutionContextBuilder resetErrors() { return this; } + @ExperimentalApi + public ExecutionContextBuilder propagateErrors(boolean propagateErrors) { + this.propagateErrors = propagateErrors; + return this; + } + + public ExecutionContext build() { // preconditions assertNotNull(executionId, () -> "You must provide a query identifier"); diff --git a/src/main/java/graphql/execution/NonNullableFieldValidator.java b/src/main/java/graphql/execution/NonNullableFieldValidator.java index 3241fa247a..a4d763ae90 100644 --- a/src/main/java/graphql/execution/NonNullableFieldValidator.java +++ b/src/main/java/graphql/execution/NonNullableFieldValidator.java @@ -34,7 +34,7 @@ public NonNullableFieldValidator(ExecutionContext executionContext, ExecutionSte */ public T validate(ExecutionStrategyParameters parameters, T result) throws NonNullableFieldWasNullException { if (result == null) { - if (executionStepInfo.isNonNullType()) { + if (executionStepInfo.isNonNullType() && executionContext.propagateErrors()) { // see https://spec.graphql.org/October2021/#sec-Errors-and-Non-Nullability // // > If the field returns null because of an error which has already been added to the "errors" list in the response, diff --git a/src/main/java/graphql/schema/idl/SchemaGenerator.java b/src/main/java/graphql/schema/idl/SchemaGenerator.java index 868aec4b76..90201610c7 100644 --- a/src/main/java/graphql/schema/idl/SchemaGenerator.java +++ b/src/main/java/graphql/schema/idl/SchemaGenerator.java @@ -59,9 +59,7 @@ public SchemaGenerator() { * Created a schema from the SDL that is has a mocked runtime. * * @param sdl the SDL to be mocked - * * @return a schema with a mocked runtime - * * @see RuntimeWiring#MOCKED_WIRING */ public static GraphQLSchema createdMockedSchema(String sdl) { @@ -75,9 +73,7 @@ public static GraphQLSchema createdMockedSchema(String sdl) { * * @param typeRegistry this can be obtained via {@link SchemaParser#parse(String)} * @param wiring this can be built using {@link RuntimeWiring#newRuntimeWiring()} - * * @return an executable schema - * * @throws SchemaProblem if there are problems in assembling a schema such as missing type resolvers or no operations defined */ public GraphQLSchema makeExecutableSchema(TypeDefinitionRegistry typeRegistry, RuntimeWiring wiring) throws SchemaProblem { @@ -91,9 +87,7 @@ public GraphQLSchema makeExecutableSchema(TypeDefinitionRegistry typeRegistry, R * @param options the controlling options * @param typeRegistry this can be obtained via {@link SchemaParser#parse(String)} * @param wiring this can be built using {@link RuntimeWiring#newRuntimeWiring()} - * * @return an executable schema - * * @throws SchemaProblem if there are problems in assembling a schema such as missing type resolvers or no operations defined */ public GraphQLSchema makeExecutableSchema(Options options, TypeDefinitionRegistry typeRegistry, RuntimeWiring wiring) throws SchemaProblem { @@ -101,7 +95,7 @@ public GraphQLSchema makeExecutableSchema(Options options, TypeDefinitionRegistr TypeDefinitionRegistry typeRegistryCopy = new TypeDefinitionRegistry(); typeRegistryCopy.merge(typeRegistry); - schemaGeneratorHelper.addDirectivesIncludedByDefault(typeRegistryCopy); + schemaGeneratorHelper.addDirectivesIncludedByDefault(typeRegistryCopy, options.addOnErrorDirective); List errors = typeChecker.checkTypeRegistry(typeRegistryCopy, wiring); if (!errors.isEmpty()) { @@ -164,11 +158,13 @@ public static class Options { private final boolean useCommentsAsDescription; private final boolean captureAstDefinitions; private final boolean useAppliedDirectivesOnly; + private final boolean addOnErrorDirective; - Options(boolean useCommentsAsDescription, boolean captureAstDefinitions, boolean useAppliedDirectivesOnly) { + Options(boolean useCommentsAsDescription, boolean captureAstDefinitions, boolean useAppliedDirectivesOnly, boolean addOnErrorDirective) { this.useCommentsAsDescription = useCommentsAsDescription; this.captureAstDefinitions = captureAstDefinitions; this.useAppliedDirectivesOnly = useAppliedDirectivesOnly; + this.addOnErrorDirective = addOnErrorDirective; } public boolean isUseCommentsAsDescription() { @@ -183,8 +179,12 @@ public boolean isUseAppliedDirectivesOnly() { return useAppliedDirectivesOnly; } + public boolean getAddOnErrorDirective() { + return addOnErrorDirective; + } + public static Options defaultOptions() { - return new Options(true, true, false); + return new Options(true, true, false, false); } /** @@ -193,11 +193,10 @@ public static Options defaultOptions() { * descriptions are the sanctioned way to make scheme element descriptions. * * @param useCommentsAsDescription the flag to control whether comments can be used as schema element descriptions - * * @return a new Options object */ public Options useCommentsAsDescriptions(boolean useCommentsAsDescription) { - return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly); + return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly, addOnErrorDirective); } /** @@ -205,11 +204,10 @@ public Options useCommentsAsDescriptions(boolean useCommentsAsDescription) { * some tooling may require them. * * @param captureAstDefinitions the flag on whether to capture AST definitions - * * @return a new Options object */ public Options captureAstDefinitions(boolean captureAstDefinitions) { - return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly); + return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly, addOnErrorDirective); } /** @@ -218,11 +216,14 @@ public Options captureAstDefinitions(boolean captureAstDefinitions) { * elements. This flag allows you to only use {@link graphql.schema.GraphQLAppliedDirective} on schema elements. * * @param useAppliedDirectivesOnly the flag on whether to use {@link graphql.schema.GraphQLAppliedDirective}s only on schema elements - * * @return a new Options object */ public Options useAppliedDirectivesOnly(boolean useAppliedDirectivesOnly) { - return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly); + return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly, addOnErrorDirective); + } + + public Options addOnErrorDirective(boolean addOnErrorDirective) { + return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly, addOnErrorDirective); } } } \ No newline at end of file diff --git a/src/main/java/graphql/schema/idl/SchemaGeneratorAppliedDirectiveHelper.java b/src/main/java/graphql/schema/idl/SchemaGeneratorAppliedDirectiveHelper.java index 7a98b46078..e7d94dffdf 100644 --- a/src/main/java/graphql/schema/idl/SchemaGeneratorAppliedDirectiveHelper.java +++ b/src/main/java/graphql/schema/idl/SchemaGeneratorAppliedDirectiveHelper.java @@ -35,7 +35,7 @@ import static java.util.stream.Collectors.toMap; /** - * This contains helper code to build out appliedm directives on schema element + * This contains helper code to build out applied directives on schema element */ @Internal class SchemaGeneratorAppliedDirectiveHelper { diff --git a/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java b/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java index 9fa7ea286f..11f16781da 100644 --- a/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java +++ b/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java @@ -1,6 +1,7 @@ package graphql.schema.idl; import graphql.AssertException; +import graphql.Enums; import graphql.Internal; import graphql.introspection.Introspection.DirectiveLocation; import graphql.language.Argument; @@ -80,13 +81,8 @@ import java.util.stream.Collectors; 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.ONE_OF_DIRECTIVE_DEFINITION; -import static graphql.Directives.SPECIFIED_BY_DIRECTIVE_DEFINITION; -import static graphql.Directives.SkipDirective; -import static graphql.Directives.SpecifiedByDirective; +import static graphql.Directives.*; +import static graphql.Enums.ON_ERROR_TYPE_DEFINITION; import static graphql.collect.ImmutableKit.emptyList; import static graphql.introspection.Introspection.DirectiveLocation.ARGUMENT_DEFINITION; import static graphql.introspection.Introspection.DirectiveLocation.ENUM; @@ -1084,12 +1080,21 @@ Set buildAdditionalDirectiveDefinitions(BuildContext buildCtx) } void addDirectivesIncludedByDefault(TypeDefinitionRegistry typeRegistry) { + addDirectivesIncludedByDefault(typeRegistry, false); + } + + void addDirectivesIncludedByDefault(TypeDefinitionRegistry typeRegistry, boolean addOnErrorDirective) { // we synthesize this into the type registry - no need for them to add it typeRegistry.add(DEPRECATED_DIRECTIVE_DEFINITION); typeRegistry.add(SPECIFIED_BY_DIRECTIVE_DEFINITION); typeRegistry.add(ONE_OF_DIRECTIVE_DEFINITION); + if (addOnErrorDirective) { + typeRegistry.add(ERROR_HANDLING_DIRECTIVE_DEFINITION); + typeRegistry.add(ON_ERROR_TYPE_DEFINITION); + } } + private Optional getOperationNamed(String name, Map operationTypeDefs) { return Optional.ofNullable(operationTypeDefs.get(name)); } diff --git a/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy b/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy index b288faff07..c2d1ac3143 100644 --- a/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy +++ b/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy @@ -7,7 +7,9 @@ import static graphql.schema.GraphQLNonNull.nonNull class NonNullableFieldValidatorTest extends Specification { - ExecutionContext context = Mock(ExecutionContext) + ExecutionContext context = Mock(ExecutionContext) { + propagateErrors() >> true + } def parameters = Mock(ExecutionStrategyParameters) { getPath() >> ResultPath.rootPath() From 63dcac38b57caef9679d6806945b832549d13a1b Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Wed, 11 Dec 2024 00:23:41 +0100 Subject: [PATCH 02/27] cosmetics --- src/main/java/graphql/execution/Execution.java | 6 +++--- src/main/java/graphql/schema/idl/SchemaGenerator.java | 6 ++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 6d3cc4574a..650f43aadb 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -256,9 +256,9 @@ private boolean propagateErrors(CoercedVariables variables, List dire Directive foundDirective = NodeUtil.findNodeByName(directives, ERROR_HANDLING_DIRECTIVE_DEFINITION.getName()); if (foundDirective != null) { Map argumentValues = ValuesResolver.getArgumentValues(ErrorHandlingDirective.getArguments(), foundDirective.getArguments(), variables, GraphQLContext.getDefault(), Locale.getDefault()); - Object flag = argumentValues.get("onError"); - Assert.assertTrue(flag instanceof String, "The '%s' directive MUST have a OnError value for the 'if' argument", ERROR_HANDLING_DIRECTIVE_DEFINITION.getName()); - return flag.equals(Enums.ON_ERROR_PROPAGATE); + Object argumentValue = argumentValues.get("onError"); + Assert.assertTrue(argumentValue instanceof String, "The '%s' directive MUST have an OnError value for the 'onError' argument", ERROR_HANDLING_DIRECTIVE_DEFINITION.getName()); + return argumentValue.equals(Enums.ON_ERROR_PROPAGATE); } return defaultValue; } diff --git a/src/main/java/graphql/schema/idl/SchemaGenerator.java b/src/main/java/graphql/schema/idl/SchemaGenerator.java index 90201610c7..eb62c1ded2 100644 --- a/src/main/java/graphql/schema/idl/SchemaGenerator.java +++ b/src/main/java/graphql/schema/idl/SchemaGenerator.java @@ -59,7 +59,9 @@ public SchemaGenerator() { * Created a schema from the SDL that is has a mocked runtime. * * @param sdl the SDL to be mocked + * * @return a schema with a mocked runtime + * * @see RuntimeWiring#MOCKED_WIRING */ public static GraphQLSchema createdMockedSchema(String sdl) { @@ -73,7 +75,9 @@ public static GraphQLSchema createdMockedSchema(String sdl) { * * @param typeRegistry this can be obtained via {@link SchemaParser#parse(String)} * @param wiring this can be built using {@link RuntimeWiring#newRuntimeWiring()} + * * @return an executable schema + * * @throws SchemaProblem if there are problems in assembling a schema such as missing type resolvers or no operations defined */ public GraphQLSchema makeExecutableSchema(TypeDefinitionRegistry typeRegistry, RuntimeWiring wiring) throws SchemaProblem { @@ -87,7 +91,9 @@ public GraphQLSchema makeExecutableSchema(TypeDefinitionRegistry typeRegistry, R * @param options the controlling options * @param typeRegistry this can be obtained via {@link SchemaParser#parse(String)} * @param wiring this can be built using {@link RuntimeWiring#newRuntimeWiring()} + * * @return an executable schema + * * @throws SchemaProblem if there are problems in assembling a schema such as missing type resolvers or no operations defined */ public GraphQLSchema makeExecutableSchema(Options options, TypeDefinitionRegistry typeRegistry, RuntimeWiring wiring) throws SchemaProblem { From 2c6fd1c7668347e24cf683783d7f885bb68f9fdf Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Wed, 11 Dec 2024 14:19:15 +0100 Subject: [PATCH 03/27] add test for NonNullableFieldValidator --- .../NonNullableFieldValidatorTest.groovy | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy b/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy index c2d1ac3143..75f0cc79cd 100644 --- a/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy +++ b/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy @@ -7,15 +7,15 @@ import static graphql.schema.GraphQLNonNull.nonNull class NonNullableFieldValidatorTest extends Specification { - ExecutionContext context = Mock(ExecutionContext) { - propagateErrors() >> true - } - def parameters = Mock(ExecutionStrategyParameters) { getPath() >> ResultPath.rootPath() } def "non nullable field throws exception"() { + ExecutionContext context = Mock(ExecutionContext) { + propagateErrors() >> true + } + ExecutionStepInfo typeInfo = ExecutionStepInfo.newExecutionStepInfo().type(nonNull(GraphQLString)).build() NonNullableFieldValidator validator = new NonNullableFieldValidator(context, typeInfo) @@ -29,6 +29,10 @@ class NonNullableFieldValidatorTest extends Specification { } def "nullable field does not throw exception"() { + ExecutionContext context = Mock(ExecutionContext) { + propagateErrors() >> true + } + ExecutionStepInfo typeInfo = ExecutionStepInfo.newExecutionStepInfo().type(GraphQLString).build() NonNullableFieldValidator validator = new NonNullableFieldValidator(context, typeInfo) @@ -39,4 +43,20 @@ class NonNullableFieldValidatorTest extends Specification { then: result == null } + + def "non nullable field returns null if errors are not propagated"() { + ExecutionContext context = Mock(ExecutionContext) { + propagateErrors() >> false + } + + ExecutionStepInfo typeInfo = ExecutionStepInfo.newExecutionStepInfo().type(nonNull(GraphQLString)).build() + + NonNullableFieldValidator validator = new NonNullableFieldValidator(context, typeInfo) + + when: + def result = validator.validate(parameters, null) + + then: + result == null + } } From eb00f570ac12b8bf06eea43d0344d12b5305516d Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Wed, 11 Dec 2024 14:38:20 +0100 Subject: [PATCH 04/27] Add integration test --- .../execution/NonNullableFieldValidator.java | 6 ++- .../execution/NoErrorPropagationTest.groovy | 41 +++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy diff --git a/src/main/java/graphql/execution/NonNullableFieldValidator.java b/src/main/java/graphql/execution/NonNullableFieldValidator.java index a4d763ae90..2e1068b225 100644 --- a/src/main/java/graphql/execution/NonNullableFieldValidator.java +++ b/src/main/java/graphql/execution/NonNullableFieldValidator.java @@ -34,7 +34,7 @@ public NonNullableFieldValidator(ExecutionContext executionContext, ExecutionSte */ public T validate(ExecutionStrategyParameters parameters, T result) throws NonNullableFieldWasNullException { if (result == null) { - if (executionStepInfo.isNonNullType() && executionContext.propagateErrors()) { + if (executionStepInfo.isNonNullType()) { // see https://spec.graphql.org/October2021/#sec-Errors-and-Non-Nullability // // > If the field returns null because of an error which has already been added to the "errors" list in the response, @@ -56,7 +56,9 @@ public T validate(ExecutionStrategyParameters parameters, T result) throws N } else { executionContext.addError(error, path); } - throw nonNullException; + if (executionContext.propagateErrors()) { + throw nonNullException; + } } } return result; diff --git a/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy b/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy new file mode 100644 index 0000000000..be20bd599a --- /dev/null +++ b/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy @@ -0,0 +1,41 @@ +package graphql.execution + +import graphql.ExecutionInput +import graphql.TestUtil +import graphql.schema.idl.RuntimeWiring +import graphql.schema.idl.SchemaGenerator +import graphql.GraphQL +import spock.lang.Specification + +class NoErrorPropagationTest extends Specification { + + def "when error propagation is disabled null is returned"() { + + def sdl = ''' + type Query { + foo : Int! + } + ''' + + def options = SchemaGenerator.Options.defaultOptions().addOnErrorDirective(true) + + def schema = TestUtil.schema(options, sdl, RuntimeWiring.MOCKED_WIRING) + def graphql = GraphQL.newGraphQL(schema).build() + + def query = ''' + query GetFoo @errorHandling(onError: NULL) { foo } + ''' + when: + + ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( + [foo: null] + ).build() + + def er = graphql.execute(ei) + + then: + er.data != null + er.data.foo == null + er.errors[0].path.toList() == ["foo"] + } +} From 6bf581d2a6fde7d6a7723b1dbf99a0a10c3245ad Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Wed, 11 Dec 2024 14:38:46 +0100 Subject: [PATCH 05/27] Add @PublicApi --- src/main/java/graphql/Enums.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/graphql/Enums.java b/src/main/java/graphql/Enums.java index 90df54f527..80b1a6ceec 100644 --- a/src/main/java/graphql/Enums.java +++ b/src/main/java/graphql/Enums.java @@ -7,6 +7,7 @@ /** * The enums that are understood by graphql-java */ +@PublicApi public class Enums { public static final String ON_ERROR = "OnError"; public static final String ON_ERROR_PROPAGATE = "PROPAGATE"; From 1b66c6e7941ad422addb9688811d42dd47efc4ce Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Wed, 11 Dec 2024 14:41:48 +0100 Subject: [PATCH 06/27] make some enums package-private --- src/main/java/graphql/Enums.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/graphql/Enums.java b/src/main/java/graphql/Enums.java index 80b1a6ceec..c226339215 100644 --- a/src/main/java/graphql/Enums.java +++ b/src/main/java/graphql/Enums.java @@ -9,9 +9,9 @@ */ @PublicApi public class Enums { - public static final String ON_ERROR = "OnError"; + static final String ON_ERROR = "OnError"; public static final String ON_ERROR_PROPAGATE = "PROPAGATE"; - public static final String ON_ERROR_NULL = "NULL"; + static final String ON_ERROR_NULL = "NULL"; @ExperimentalApi public static final EnumTypeDefinition ON_ERROR_TYPE_DEFINITION; From 8f012ef825098798d6e14d7dc685f06ff46ff8bf Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Wed, 11 Dec 2024 14:53:55 +0100 Subject: [PATCH 07/27] NULL -> ALLOW_NULL --- src/main/java/graphql/Enums.java | 2 +- src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/graphql/Enums.java b/src/main/java/graphql/Enums.java index c226339215..6cea2edab4 100644 --- a/src/main/java/graphql/Enums.java +++ b/src/main/java/graphql/Enums.java @@ -11,7 +11,7 @@ public class Enums { static final String ON_ERROR = "OnError"; public static final String ON_ERROR_PROPAGATE = "PROPAGATE"; - static final String ON_ERROR_NULL = "NULL"; + static final String ON_ERROR_NULL = "ALLOW_NULL"; @ExperimentalApi public static final EnumTypeDefinition ON_ERROR_TYPE_DEFINITION; diff --git a/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy b/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy index be20bd599a..f0695e1dd6 100644 --- a/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy +++ b/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy @@ -23,7 +23,7 @@ class NoErrorPropagationTest extends Specification { def graphql = GraphQL.newGraphQL(schema).build() def query = ''' - query GetFoo @errorHandling(onError: NULL) { foo } + query GetFoo @errorHandling(onError: ALLOW_NULL) { foo } ''' when: From ed23db441b99f447ec8944bfdf81d8c3dbfca423 Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Wed, 11 Dec 2024 15:03:02 +0100 Subject: [PATCH 08/27] remove wildcard imports --- src/main/java/graphql/Directives.java | 19 +++++++++++++-- .../java/graphql/execution/Execution.java | 23 ++++++++++++++++--- .../graphql/execution/ExecutionContext.java | 7 +++++- 3 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/main/java/graphql/Directives.java b/src/main/java/graphql/Directives.java index ac78a9cb0e..0dd6d1a7e1 100644 --- a/src/main/java/graphql/Directives.java +++ b/src/main/java/graphql/Directives.java @@ -1,13 +1,28 @@ package graphql; -import graphql.language.*; +import graphql.language.BooleanValue; +import graphql.language.Description; +import graphql.language.DirectiveDefinition; +import graphql.language.EnumValue; +import graphql.language.StringValue; import graphql.schema.GraphQLDirective; import graphql.schema.GraphQLEnumType; import static graphql.Scalars.GraphQLBoolean; import static graphql.Scalars.GraphQLString; -import static graphql.introspection.Introspection.DirectiveLocation.*; +import static graphql.introspection.Introspection.DirectiveLocation.ARGUMENT_DEFINITION; +import static graphql.introspection.Introspection.DirectiveLocation.ENUM_VALUE; +import static graphql.introspection.Introspection.DirectiveLocation.FIELD; +import static graphql.introspection.Introspection.DirectiveLocation.FIELD_DEFINITION; +import static graphql.introspection.Introspection.DirectiveLocation.FRAGMENT_SPREAD; +import static graphql.introspection.Introspection.DirectiveLocation.INLINE_FRAGMENT; +import static graphql.introspection.Introspection.DirectiveLocation.INPUT_FIELD_DEFINITION; +import static graphql.introspection.Introspection.DirectiveLocation.INPUT_OBJECT; +import static graphql.introspection.Introspection.DirectiveLocation.MUTATION; +import static graphql.introspection.Introspection.DirectiveLocation.QUERY; +import static graphql.introspection.Introspection.DirectiveLocation.SCALAR; +import static graphql.introspection.Introspection.DirectiveLocation.SUBSCRIPTION; import static graphql.language.DirectiveLocation.newDirectiveLocation; import static graphql.language.InputValueDefinition.newInputValueDefinition; import static graphql.language.NonNullType.newNonNullType; diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 650f43aadb..43e902e5b8 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -1,7 +1,15 @@ package graphql.execution; -import graphql.*; +import graphql.Assert; +import graphql.Enums; +import graphql.ExecutionInput; +import graphql.ExecutionResult; +import graphql.ExecutionResultImpl; +import graphql.ExperimentalApi; +import graphql.GraphQLContext; +import graphql.GraphQLError; +import graphql.Internal; import graphql.execution.incremental.IncrementalCallState; import graphql.execution.instrumentation.Instrumentation; import graphql.execution.instrumentation.InstrumentationContext; @@ -14,13 +22,22 @@ import graphql.extensions.ExtensionsBuilder; import graphql.incremental.DelayedIncrementalPartialResult; import graphql.incremental.IncrementalExecutionResultImpl; -import graphql.language.*; +import graphql.language.Directive; +import graphql.language.Document; +import graphql.language.FragmentDefinition; +import graphql.language.NodeUtil; +import graphql.language.OperationDefinition; +import graphql.language.VariableDefinition; import graphql.schema.GraphQLObjectType; import graphql.schema.GraphQLSchema; import graphql.schema.impl.SchemaUtil; import org.reactivestreams.Publisher; -import java.util.*; +import java.util.Collections; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Optional; import java.util.concurrent.CompletableFuture; import static graphql.Directives.*; diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index e34ae793e3..f8cd289431 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -3,7 +3,12 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import graphql.*; +import graphql.ExecutionInput; +import graphql.ExperimentalApi; +import graphql.GraphQLContext; +import graphql.GraphQLError; +import graphql.Internal; +import graphql.PublicApi; import graphql.collect.ImmutableKit; import graphql.execution.incremental.IncrementalCallState; import graphql.execution.instrumentation.Instrumentation; From 4d794cbdd92d4b7f10f1a9549e2ba401c853efd4 Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Wed, 11 Dec 2024 15:07:18 +0100 Subject: [PATCH 09/27] Add Javadoc --- src/main/java/graphql/execution/ExecutionContext.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index f8cd289431..34c9309ff3 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -173,6 +173,11 @@ public ValueUnboxer getValueUnboxer() { return valueUnboxer; } + /** + * @return true if the current operation should propagate errors in non-null positions + * Propagating errors is the default. Error aware clients may opt in returning null in non-null positions + * by using the `@errorHandling` directive. + */ @ExperimentalApi public boolean propagateErrors() { return propagateErrors; } From 072276b29b0e00cf326fc8bf9e31d009b8c2084c Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Wed, 11 Dec 2024 15:11:40 +0100 Subject: [PATCH 10/27] rename boolean getter --- src/main/java/graphql/schema/idl/SchemaGenerator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/graphql/schema/idl/SchemaGenerator.java b/src/main/java/graphql/schema/idl/SchemaGenerator.java index eb62c1ded2..2d51d766f6 100644 --- a/src/main/java/graphql/schema/idl/SchemaGenerator.java +++ b/src/main/java/graphql/schema/idl/SchemaGenerator.java @@ -185,7 +185,7 @@ public boolean isUseAppliedDirectivesOnly() { return useAppliedDirectivesOnly; } - public boolean getAddOnErrorDirective() { + public boolean isAddOnErrorDirective() { return addOnErrorDirective; } From 7d3097ed1695cc1d52b15c637143a7edb0df5271 Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Wed, 11 Dec 2024 15:16:33 +0100 Subject: [PATCH 11/27] remove wildcard imports --- src/main/java/graphql/execution/Execution.java | 3 ++- .../graphql/execution/ExecutionContextBuilder.java | 7 ++++++- .../graphql/schema/idl/SchemaGeneratorHelper.java | 11 ++++++++--- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 43e902e5b8..3f9e7c9c14 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -40,7 +40,8 @@ import java.util.Optional; import java.util.concurrent.CompletableFuture; -import static graphql.Directives.*; +import static graphql.Directives.ERROR_HANDLING_DIRECTIVE_DEFINITION; +import static graphql.Directives.ErrorHandlingDirective; import static graphql.execution.ExecutionContextBuilder.newExecutionContextBuilder; import static graphql.execution.ExecutionStepInfo.newExecutionStepInfo; import static graphql.execution.ExecutionStrategyParameters.newParameters; diff --git a/src/main/java/graphql/execution/ExecutionContextBuilder.java b/src/main/java/graphql/execution/ExecutionContextBuilder.java index d4b970d8d3..09fb1ecd45 100644 --- a/src/main/java/graphql/execution/ExecutionContextBuilder.java +++ b/src/main/java/graphql/execution/ExecutionContextBuilder.java @@ -2,7 +2,12 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import graphql.*; +import graphql.ExecutionInput; +import graphql.ExperimentalApi; +import graphql.GraphQLContext; +import graphql.GraphQLError; +import graphql.Internal; +import graphql.PublicApi; import graphql.collect.ImmutableKit; import graphql.execution.instrumentation.Instrumentation; import graphql.execution.instrumentation.InstrumentationState; diff --git a/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java b/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java index 11f16781da..bdb1c6357e 100644 --- a/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java +++ b/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java @@ -1,7 +1,6 @@ package graphql.schema.idl; import graphql.AssertException; -import graphql.Enums; import graphql.Internal; import graphql.introspection.Introspection.DirectiveLocation; import graphql.language.Argument; @@ -57,7 +56,6 @@ import graphql.schema.GraphQLTypeReference; import graphql.schema.GraphQLUnionType; import graphql.schema.GraphqlTypeComparatorRegistry; -import graphql.schema.PropertyDataFetcher; import graphql.schema.SingletonPropertyDataFetcher; import graphql.schema.TypeResolver; import graphql.schema.TypeResolverProxy; @@ -81,7 +79,14 @@ import java.util.stream.Collectors; import static graphql.Assert.assertNotNull; -import static graphql.Directives.*; +import static graphql.Directives.DEPRECATED_DIRECTIVE_DEFINITION; +import static graphql.Directives.ERROR_HANDLING_DIRECTIVE_DEFINITION; +import static graphql.Directives.IncludeDirective; +import static graphql.Directives.NO_LONGER_SUPPORTED; +import static graphql.Directives.ONE_OF_DIRECTIVE_DEFINITION; +import static graphql.Directives.SPECIFIED_BY_DIRECTIVE_DEFINITION; +import static graphql.Directives.SkipDirective; +import static graphql.Directives.SpecifiedByDirective; import static graphql.Enums.ON_ERROR_TYPE_DEFINITION; import static graphql.collect.ImmutableKit.emptyList; import static graphql.introspection.Introspection.DirectiveLocation.ARGUMENT_DEFINITION; From dd9a01e20a887f6875686e319331546039442027 Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Wed, 11 Dec 2024 15:18:22 +0100 Subject: [PATCH 12/27] restore empty lines --- src/main/java/graphql/schema/idl/SchemaGenerator.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/graphql/schema/idl/SchemaGenerator.java b/src/main/java/graphql/schema/idl/SchemaGenerator.java index 2d51d766f6..38131b7f03 100644 --- a/src/main/java/graphql/schema/idl/SchemaGenerator.java +++ b/src/main/java/graphql/schema/idl/SchemaGenerator.java @@ -199,6 +199,7 @@ public static Options defaultOptions() { * descriptions are the sanctioned way to make scheme element descriptions. * * @param useCommentsAsDescription the flag to control whether comments can be used as schema element descriptions + * * @return a new Options object */ public Options useCommentsAsDescriptions(boolean useCommentsAsDescription) { @@ -210,6 +211,7 @@ public Options useCommentsAsDescriptions(boolean useCommentsAsDescription) { * some tooling may require them. * * @param captureAstDefinitions the flag on whether to capture AST definitions + * * @return a new Options object */ public Options captureAstDefinitions(boolean captureAstDefinitions) { @@ -222,6 +224,7 @@ public Options captureAstDefinitions(boolean captureAstDefinitions) { * elements. This flag allows you to only use {@link graphql.schema.GraphQLAppliedDirective} on schema elements. * * @param useAppliedDirectivesOnly the flag on whether to use {@link graphql.schema.GraphQLAppliedDirective}s only on schema elements + * * @return a new Options object */ public Options useAppliedDirectivesOnly(boolean useAppliedDirectivesOnly) { From 41863ea4ca974952b77e8d9b7a41c5b4f1703510 Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Fri, 13 Dec 2024 11:45:15 +0100 Subject: [PATCH 13/27] revert SchemaGenerator changes and add ExperimentalApi.ENABLE_CUSTOM_ERROR_PROPAGATION --- src/main/java/graphql/Directives.java | 3 +- src/main/java/graphql/ExperimentalApi.java | 4 + .../java/graphql/execution/Execution.java | 7 +- .../graphql/schema/idl/SchemaGenerator.java | 22 +--- .../schema/idl/SchemaGeneratorHelper.java | 9 -- .../execution/NoErrorPropagationTest.groovy | 111 ++++++++++++++++-- 6 files changed, 120 insertions(+), 36 deletions(-) diff --git a/src/main/java/graphql/Directives.java b/src/main/java/graphql/Directives.java index 0dd6d1a7e1..2b529bdd4b 100644 --- a/src/main/java/graphql/Directives.java +++ b/src/main/java/graphql/Directives.java @@ -152,6 +152,7 @@ public class Directives { .name("onError") .type(newNonNullType(newTypeName().name(Enums.ON_ERROR).build()).build()) .defaultValue(EnumValue.newEnumValue(Enums.ON_ERROR_PROPAGATE).build()) + .description(createDescription("How to handle errors.")) .build()) .build(); } @@ -259,7 +260,7 @@ public class Directives { .value(Enums.ON_ERROR_NULL) .build())) .defaultValueProgrammatic(Enums.ON_ERROR_PROPAGATE) - .description("The URL that specifies the behaviour of this scalar.")) + .description("How to handle errors.")) .validLocations(QUERY, MUTATION, SUBSCRIPTION) .definition(ERROR_HANDLING_DIRECTIVE_DEFINITION) .build(); diff --git a/src/main/java/graphql/ExperimentalApi.java b/src/main/java/graphql/ExperimentalApi.java index 80be253cd1..0be2f5a7a1 100644 --- a/src/main/java/graphql/ExperimentalApi.java +++ b/src/main/java/graphql/ExperimentalApi.java @@ -24,4 +24,8 @@ * The key that should be associated with a boolean value which indicates whether @defer and @stream behaviour is enabled for this execution. */ String ENABLE_INCREMENTAL_SUPPORT = "ENABLE_INCREMENTAL_SUPPORT"; + /** + * The key that should be associated with a boolean value which indicates whether @errorHandling behaviour is enabled for this execution. + */ + String ENABLE_CUSTOM_ERROR_HANDLING = "ENABLE_CUSTOM_ERROR_HANDLING"; } diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 3f9e7c9c14..a8d4d6e6f9 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -92,6 +92,11 @@ public CompletableFuture execute(Document document, GraphQLSche throw rte; } + boolean customErrorPropagationEnabled = Optional.ofNullable(executionInput.getGraphQLContext()) + .map(graphqlContext -> graphqlContext.getBoolean(ExperimentalApi.ENABLE_CUSTOM_ERROR_HANDLING)) + .orElse(false); + boolean propagateErrors = !customErrorPropagationEnabled || propagateErrors(coercedVariables, operationDefinition.getDirectives(), true); + ExecutionContext executionContext = newExecutionContextBuilder() .instrumentation(instrumentation) .instrumentationState(instrumentationState) @@ -112,7 +117,7 @@ public CompletableFuture execute(Document document, GraphQLSche .locale(executionInput.getLocale()) .valueUnboxer(valueUnboxer) .executionInput(executionInput) - .propagateErrors(propagateErrors(coercedVariables, operationDefinition.getDirectives(), true)) + .propagateErrors(propagateErrors) .build(); executionContext.getGraphQLContext().put(ResultNodesInfo.RESULT_NODES_INFO, executionContext.getResultNodesInfo()); diff --git a/src/main/java/graphql/schema/idl/SchemaGenerator.java b/src/main/java/graphql/schema/idl/SchemaGenerator.java index 38131b7f03..868aec4b76 100644 --- a/src/main/java/graphql/schema/idl/SchemaGenerator.java +++ b/src/main/java/graphql/schema/idl/SchemaGenerator.java @@ -101,7 +101,7 @@ public GraphQLSchema makeExecutableSchema(Options options, TypeDefinitionRegistr TypeDefinitionRegistry typeRegistryCopy = new TypeDefinitionRegistry(); typeRegistryCopy.merge(typeRegistry); - schemaGeneratorHelper.addDirectivesIncludedByDefault(typeRegistryCopy, options.addOnErrorDirective); + schemaGeneratorHelper.addDirectivesIncludedByDefault(typeRegistryCopy); List errors = typeChecker.checkTypeRegistry(typeRegistryCopy, wiring); if (!errors.isEmpty()) { @@ -164,13 +164,11 @@ public static class Options { private final boolean useCommentsAsDescription; private final boolean captureAstDefinitions; private final boolean useAppliedDirectivesOnly; - private final boolean addOnErrorDirective; - Options(boolean useCommentsAsDescription, boolean captureAstDefinitions, boolean useAppliedDirectivesOnly, boolean addOnErrorDirective) { + Options(boolean useCommentsAsDescription, boolean captureAstDefinitions, boolean useAppliedDirectivesOnly) { this.useCommentsAsDescription = useCommentsAsDescription; this.captureAstDefinitions = captureAstDefinitions; this.useAppliedDirectivesOnly = useAppliedDirectivesOnly; - this.addOnErrorDirective = addOnErrorDirective; } public boolean isUseCommentsAsDescription() { @@ -185,12 +183,8 @@ public boolean isUseAppliedDirectivesOnly() { return useAppliedDirectivesOnly; } - public boolean isAddOnErrorDirective() { - return addOnErrorDirective; - } - public static Options defaultOptions() { - return new Options(true, true, false, false); + return new Options(true, true, false); } /** @@ -203,7 +197,7 @@ public static Options defaultOptions() { * @return a new Options object */ public Options useCommentsAsDescriptions(boolean useCommentsAsDescription) { - return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly, addOnErrorDirective); + return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly); } /** @@ -215,7 +209,7 @@ public Options useCommentsAsDescriptions(boolean useCommentsAsDescription) { * @return a new Options object */ public Options captureAstDefinitions(boolean captureAstDefinitions) { - return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly, addOnErrorDirective); + return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly); } /** @@ -228,11 +222,7 @@ public Options captureAstDefinitions(boolean captureAstDefinitions) { * @return a new Options object */ public Options useAppliedDirectivesOnly(boolean useAppliedDirectivesOnly) { - return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly, addOnErrorDirective); - } - - public Options addOnErrorDirective(boolean addOnErrorDirective) { - return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly, addOnErrorDirective); + return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly); } } } \ No newline at end of file diff --git a/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java b/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java index bdb1c6357e..6d7dfe1c5a 100644 --- a/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java +++ b/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java @@ -1085,21 +1085,12 @@ Set buildAdditionalDirectiveDefinitions(BuildContext buildCtx) } void addDirectivesIncludedByDefault(TypeDefinitionRegistry typeRegistry) { - addDirectivesIncludedByDefault(typeRegistry, false); - } - - void addDirectivesIncludedByDefault(TypeDefinitionRegistry typeRegistry, boolean addOnErrorDirective) { // we synthesize this into the type registry - no need for them to add it typeRegistry.add(DEPRECATED_DIRECTIVE_DEFINITION); typeRegistry.add(SPECIFIED_BY_DIRECTIVE_DEFINITION); typeRegistry.add(ONE_OF_DIRECTIVE_DEFINITION); - if (addOnErrorDirective) { - typeRegistry.add(ERROR_HANDLING_DIRECTIVE_DEFINITION); - typeRegistry.add(ON_ERROR_TYPE_DEFINITION); - } } - private Optional getOperationNamed(String name, Map operationTypeDefs) { return Optional.ofNullable(operationTypeDefs.get(name)); } diff --git a/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy b/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy index f0695e1dd6..c05645f9fa 100644 --- a/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy +++ b/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy @@ -1,26 +1,26 @@ package graphql.execution import graphql.ExecutionInput +import graphql.ExperimentalApi import graphql.TestUtil -import graphql.schema.idl.RuntimeWiring -import graphql.schema.idl.SchemaGenerator -import graphql.GraphQL import spock.lang.Specification class NoErrorPropagationTest extends Specification { - def "when error propagation is disabled null is returned"() { + def "when onError is ALLOW_NULL null is returned"() { def sdl = ''' type Query { foo : Int! } + enum OnError { + ALLOW_NULL + PROPAGATE + } + directive @errorHandling(onError: OnError) on QUERY | MUTATION | SUBSCRIPTION ''' - def options = SchemaGenerator.Options.defaultOptions().addOnErrorDirective(true) - - def schema = TestUtil.schema(options, sdl, RuntimeWiring.MOCKED_WIRING) - def graphql = GraphQL.newGraphQL(schema).build() + def graphql = TestUtil.graphQL(sdl).build() def query = ''' query GetFoo @errorHandling(onError: ALLOW_NULL) { foo } @@ -29,7 +29,8 @@ class NoErrorPropagationTest extends Specification { ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( [foo: null] - ).build() + ).graphQLContext([(ExperimentalApi.ENABLE_CUSTOM_ERROR_HANDLING): true]) + .build() def er = graphql.execute(ei) @@ -38,4 +39,96 @@ class NoErrorPropagationTest extends Specification { er.data.foo == null er.errors[0].path.toList() == ["foo"] } + + def "when onError is PROPAGATE error is propagated"() { + + def sdl = ''' + type Query { + foo : Int! + } + enum OnError { + ALLOW_NULL + PROPAGATE + } + directive @errorHandling(onError: OnError) on QUERY | MUTATION | SUBSCRIPTION + ''' + + def graphql = TestUtil.graphQL(sdl).build() + + def query = ''' + query GetFoo @errorHandling(onError: PROPAGATE) { foo } + ''' + when: + + ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( + [foo: null] + ).graphQLContext([(ExperimentalApi.ENABLE_CUSTOM_ERROR_HANDLING): true]) + .build() + + def er = graphql.execute(ei) + + then: + er.data == null + er.errors[0].path.toList() == ["foo"] + } + + + def "when custom error propagation is disabled error is propagated"() { + + def sdl = ''' + type Query { + foo : Int! + } + enum OnError { + ALLOW_NULL + PROPAGATE + } + directive @errorHandling(onError: OnError) on QUERY | MUTATION | SUBSCRIPTION + ''' + + def graphql = TestUtil.graphQL(sdl).build() + + def query = ''' + query GetFoo @errorHandling(onError: ALLOW_NULL) { foo } + ''' + when: + + ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( + [foo: null] + ).build() + + def er = graphql.execute(ei) + + then: + er.data == null + er.errors[0].path.toList() == ["foo"] + } + + def "when @errorHandling is not added to the schema operation does not validate"() { + + def sdl = ''' + type Query { + foo : Int! + } + ''' + + def graphql = TestUtil.graphQL(sdl).build() + + def query = ''' + query GetFoo @errorHandling(onError: PROPAGATE) { foo } + ''' + when: + + ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( + [foo: null] + ).graphQLContext([(ExperimentalApi.ENABLE_CUSTOM_ERROR_HANDLING): true]) + .build() + + def er = graphql.execute(ei) + + then: + er.data == null + er.errors[0].message.equals("Validation error (UnknownDirective) : Unknown directive 'errorHandling'") + } + } From 44be290688ba2c34b4ec8b2ddaf81f6d711deca6 Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Fri, 13 Dec 2024 11:52:09 +0100 Subject: [PATCH 14/27] remove unused imports --- src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java b/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java index 6d7dfe1c5a..e658c85d17 100644 --- a/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java +++ b/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java @@ -80,14 +80,12 @@ import static graphql.Assert.assertNotNull; import static graphql.Directives.DEPRECATED_DIRECTIVE_DEFINITION; -import static graphql.Directives.ERROR_HANDLING_DIRECTIVE_DEFINITION; import static graphql.Directives.IncludeDirective; import static graphql.Directives.NO_LONGER_SUPPORTED; import static graphql.Directives.ONE_OF_DIRECTIVE_DEFINITION; import static graphql.Directives.SPECIFIED_BY_DIRECTIVE_DEFINITION; import static graphql.Directives.SkipDirective; import static graphql.Directives.SpecifiedByDirective; -import static graphql.Enums.ON_ERROR_TYPE_DEFINITION; import static graphql.collect.ImmutableKit.emptyList; import static graphql.introspection.Introspection.DirectiveLocation.ARGUMENT_DEFINITION; import static graphql.introspection.Introspection.DirectiveLocation.ENUM; From 160abcd10978530771f90be0d4084f097395f21b Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Mon, 16 Dec 2024 00:32:38 +0100 Subject: [PATCH 15/27] Also check error message --- src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy b/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy index c05645f9fa..367d9e82e6 100644 --- a/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy +++ b/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy @@ -69,6 +69,7 @@ class NoErrorPropagationTest extends Specification { then: er.data == null + er.errors[0].message == "The field at path '/foo' was declared as a non null type, but the code involved in retrieving data has wrongly returned a null value. The graphql specification requires that the parent field be set to null, or if that is non nullable that it bubble up null to its parent and so on. The non-nullable type is 'Int' within parent type 'Query'" er.errors[0].path.toList() == ["foo"] } From 2c5cac1d737a94d60b68d38a71aa232446f6d095 Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Tue, 17 Dec 2024 15:29:34 +0100 Subject: [PATCH 16/27] Rename to @nullOnError --- src/main/java/graphql/Directives.java | 34 ++++----------- src/main/java/graphql/Enums.java | 36 ---------------- src/main/java/graphql/ExperimentalApi.java | 4 +- .../java/graphql/execution/Execution.java | 19 +++------ .../graphql/execution/ExecutionContext.java | 2 +- .../execution/NoErrorPropagationTest.groovy | 42 +++++++------------ 6 files changed, 33 insertions(+), 104 deletions(-) delete mode 100644 src/main/java/graphql/Enums.java diff --git a/src/main/java/graphql/Directives.java b/src/main/java/graphql/Directives.java index 2b529bdd4b..4b48bc2118 100644 --- a/src/main/java/graphql/Directives.java +++ b/src/main/java/graphql/Directives.java @@ -42,7 +42,7 @@ public class Directives { private static final String SPECIFIED_BY = "specifiedBy"; private static final String ONE_OF = "oneOf"; private static final String DEFER = "defer"; - private static final String ERROR_HANDLING = "errorHandling"; + private static final String NULL_ON_ERROR = "nullOnError"; public static final DirectiveDefinition DEPRECATED_DIRECTIVE_DEFINITION; public static final DirectiveDefinition INCLUDE_DIRECTIVE_DEFINITION; @@ -53,7 +53,7 @@ public class Directives { @ExperimentalApi public static final DirectiveDefinition DEFER_DIRECTIVE_DEFINITION; @ExperimentalApi - public static final DirectiveDefinition ERROR_HANDLING_DIRECTIVE_DEFINITION; + public static final DirectiveDefinition NULL_ON_ERROR_DIRECTIVE_DEFINITION; public static final String BOOLEAN = "Boolean"; public static final String STRING = "String"; @@ -141,19 +141,12 @@ public class Directives { .type(newTypeName().name(STRING).build()) .build()) .build(); - ERROR_HANDLING_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition() - .name(ERROR_HANDLING) + NULL_ON_ERROR_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition() + .name(NULL_ON_ERROR) .directiveLocation(newDirectiveLocation().name(QUERY.name()).build()) .directiveLocation(newDirectiveLocation().name(MUTATION.name()).build()) .directiveLocation(newDirectiveLocation().name(SUBSCRIPTION.name()).build()) - .description(createDescription("This directive controls how to handle errors")) - .inputValueDefinition( - newInputValueDefinition() - .name("onError") - .type(newNonNullType(newTypeName().name(Enums.ON_ERROR).build()).build()) - .defaultValue(EnumValue.newEnumValue(Enums.ON_ERROR_PROPAGATE).build()) - .description(createDescription("How to handle errors.")) - .build()) + .description(createDescription("This directive allows returning null in non-null locations that have an associated error")) .build(); } @@ -249,20 +242,11 @@ public class Directives { .build(); @ExperimentalApi - public static final GraphQLDirective ErrorHandlingDirective = GraphQLDirective.newDirective() - .name(ERROR_HANDLING) - .description("This directive controls how to handle errors.") - .argument(newArgument() - .name("onError") - .type(nonNull(GraphQLEnumType.newEnum() - .name(Enums.ON_ERROR) - .value(Enums.ON_ERROR_PROPAGATE) - .value(Enums.ON_ERROR_NULL) - .build())) - .defaultValueProgrammatic(Enums.ON_ERROR_PROPAGATE) - .description("How to handle errors.")) + public static final GraphQLDirective NullOnErrorDirective = GraphQLDirective.newDirective() + .name(NULL_ON_ERROR) + .description("This directive allows returning null in non-null locations that have an associated error.") .validLocations(QUERY, MUTATION, SUBSCRIPTION) - .definition(ERROR_HANDLING_DIRECTIVE_DEFINITION) + .definition(NULL_ON_ERROR_DIRECTIVE_DEFINITION) .build(); private static Description createDescription(String s) { diff --git a/src/main/java/graphql/Enums.java b/src/main/java/graphql/Enums.java deleted file mode 100644 index 6cea2edab4..0000000000 --- a/src/main/java/graphql/Enums.java +++ /dev/null @@ -1,36 +0,0 @@ -package graphql; - -import graphql.language.Description; -import graphql.language.EnumTypeDefinition; -import graphql.language.EnumValueDefinition; - -/** - * The enums that are understood by graphql-java - */ -@PublicApi -public class Enums { - static final String ON_ERROR = "OnError"; - public static final String ON_ERROR_PROPAGATE = "PROPAGATE"; - static final String ON_ERROR_NULL = "ALLOW_NULL"; - - @ExperimentalApi - public static final EnumTypeDefinition ON_ERROR_TYPE_DEFINITION; - - static { - ON_ERROR_TYPE_DEFINITION = EnumTypeDefinition.newEnumTypeDefinition() - .name(ON_ERROR) - .enumValueDefinition( - EnumValueDefinition.newEnumValueDefinition() - .name(ON_ERROR_PROPAGATE) - .description(new Description("If the error happens in a non-nullable position, the error is propagated to the neareast nullable parent position", null, true)) - .build() - ) - .enumValueDefinition( - EnumValueDefinition.newEnumValueDefinition() - .name(ON_ERROR_NULL) - .description(new Description("Null is always returned regardless of whether the position is nullable or not", null, true)) - .build() - ) - .build(); - } -} diff --git a/src/main/java/graphql/ExperimentalApi.java b/src/main/java/graphql/ExperimentalApi.java index 0be2f5a7a1..65ebd9ddc0 100644 --- a/src/main/java/graphql/ExperimentalApi.java +++ b/src/main/java/graphql/ExperimentalApi.java @@ -25,7 +25,7 @@ */ String ENABLE_INCREMENTAL_SUPPORT = "ENABLE_INCREMENTAL_SUPPORT"; /** - * The key that should be associated with a boolean value which indicates whether @errorHandling behaviour is enabled for this execution. + * The key that should be associated with a boolean value which indicates whether @nullOnError behaviour is enabled for this execution. */ - String ENABLE_CUSTOM_ERROR_HANDLING = "ENABLE_CUSTOM_ERROR_HANDLING"; + String ENABLE_NULL_ON_ERROR = "ENABLE_NULL_ON_ERROR"; } diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index a8d4d6e6f9..e93617ee8c 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -1,8 +1,6 @@ package graphql.execution; -import graphql.Assert; -import graphql.Enums; import graphql.ExecutionInput; import graphql.ExecutionResult; import graphql.ExecutionResultImpl; @@ -35,13 +33,11 @@ import java.util.Collections; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.concurrent.CompletableFuture; -import static graphql.Directives.ERROR_HANDLING_DIRECTIVE_DEFINITION; -import static graphql.Directives.ErrorHandlingDirective; +import static graphql.Directives.NULL_ON_ERROR_DIRECTIVE_DEFINITION; import static graphql.execution.ExecutionContextBuilder.newExecutionContextBuilder; import static graphql.execution.ExecutionStepInfo.newExecutionStepInfo; import static graphql.execution.ExecutionStrategyParameters.newParameters; @@ -93,9 +89,9 @@ public CompletableFuture execute(Document document, GraphQLSche } boolean customErrorPropagationEnabled = Optional.ofNullable(executionInput.getGraphQLContext()) - .map(graphqlContext -> graphqlContext.getBoolean(ExperimentalApi.ENABLE_CUSTOM_ERROR_HANDLING)) + .map(graphqlContext -> graphqlContext.getBoolean(ExperimentalApi.ENABLE_NULL_ON_ERROR)) .orElse(false); - boolean propagateErrors = !customErrorPropagationEnabled || propagateErrors(coercedVariables, operationDefinition.getDirectives(), true); + boolean propagateErrors = !customErrorPropagationEnabled || propagateErrors(operationDefinition.getDirectives(), true); ExecutionContext executionContext = newExecutionContextBuilder() .instrumentation(instrumentation) @@ -275,13 +271,10 @@ private ExecutionResult mergeExtensionsBuilderIfPresent(ExecutionResult executio return executionResult; } - private boolean propagateErrors(CoercedVariables variables, List directives, boolean defaultValue) { - Directive foundDirective = NodeUtil.findNodeByName(directives, ERROR_HANDLING_DIRECTIVE_DEFINITION.getName()); + private boolean propagateErrors(List directives, boolean defaultValue) { + Directive foundDirective = NodeUtil.findNodeByName(directives, NULL_ON_ERROR_DIRECTIVE_DEFINITION.getName()); if (foundDirective != null) { - Map argumentValues = ValuesResolver.getArgumentValues(ErrorHandlingDirective.getArguments(), foundDirective.getArguments(), variables, GraphQLContext.getDefault(), Locale.getDefault()); - Object argumentValue = argumentValues.get("onError"); - Assert.assertTrue(argumentValue instanceof String, "The '%s' directive MUST have an OnError value for the 'onError' argument", ERROR_HANDLING_DIRECTIVE_DEFINITION.getName()); - return argumentValue.equals(Enums.ON_ERROR_PROPAGATE); + return false; } return defaultValue; } diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index 34c9309ff3..73b12a1088 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -176,7 +176,7 @@ public ValueUnboxer getValueUnboxer() { /** * @return true if the current operation should propagate errors in non-null positions * Propagating errors is the default. Error aware clients may opt in returning null in non-null positions - * by using the `@errorHandling` directive. + * by using the `@nullOnError` directive. */ @ExperimentalApi public boolean propagateErrors() { return propagateErrors; } diff --git a/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy b/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy index 367d9e82e6..fa9ddb8002 100644 --- a/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy +++ b/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy @@ -7,29 +7,25 @@ import spock.lang.Specification class NoErrorPropagationTest extends Specification { - def "when onError is ALLOW_NULL null is returned"() { + def "with nullOnError, null is returned"() { def sdl = ''' type Query { foo : Int! } - enum OnError { - ALLOW_NULL - PROPAGATE - } - directive @errorHandling(onError: OnError) on QUERY | MUTATION | SUBSCRIPTION + directive @nullOnError on QUERY | MUTATION | SUBSCRIPTION ''' def graphql = TestUtil.graphQL(sdl).build() def query = ''' - query GetFoo @errorHandling(onError: ALLOW_NULL) { foo } + query GetFoo @nullOnError { foo } ''' when: ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( [foo: null] - ).graphQLContext([(ExperimentalApi.ENABLE_CUSTOM_ERROR_HANDLING): true]) + ).graphQLContext([(ExperimentalApi.ENABLE_NULL_ON_ERROR): true]) .build() def er = graphql.execute(ei) @@ -40,29 +36,25 @@ class NoErrorPropagationTest extends Specification { er.errors[0].path.toList() == ["foo"] } - def "when onError is PROPAGATE error is propagated"() { + def "without nullOnError, error is propagated"() { def sdl = ''' type Query { foo : Int! } - enum OnError { - ALLOW_NULL - PROPAGATE - } - directive @errorHandling(onError: OnError) on QUERY | MUTATION | SUBSCRIPTION + directive @nullOnError on QUERY | MUTATION | SUBSCRIPTION ''' def graphql = TestUtil.graphQL(sdl).build() def query = ''' - query GetFoo @errorHandling(onError: PROPAGATE) { foo } + query GetFoo { foo } ''' when: ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( [foo: null] - ).graphQLContext([(ExperimentalApi.ENABLE_CUSTOM_ERROR_HANDLING): true]) + ).graphQLContext([(ExperimentalApi.ENABLE_NULL_ON_ERROR): true]) .build() def er = graphql.execute(ei) @@ -74,23 +66,19 @@ class NoErrorPropagationTest extends Specification { } - def "when custom error propagation is disabled error is propagated"() { + def "when ENABLE_NULL_ON_ERROR is false, error is propagated"() { def sdl = ''' type Query { foo : Int! } - enum OnError { - ALLOW_NULL - PROPAGATE - } - directive @errorHandling(onError: OnError) on QUERY | MUTATION | SUBSCRIPTION + directive @nullOnError on QUERY | MUTATION | SUBSCRIPTION ''' def graphql = TestUtil.graphQL(sdl).build() def query = ''' - query GetFoo @errorHandling(onError: ALLOW_NULL) { foo } + query GetFoo @nullOnError { foo } ''' when: @@ -105,7 +93,7 @@ class NoErrorPropagationTest extends Specification { er.errors[0].path.toList() == ["foo"] } - def "when @errorHandling is not added to the schema operation does not validate"() { + def "when @nullOnError is not added to the schema operation does not validate"() { def sdl = ''' type Query { @@ -116,20 +104,20 @@ class NoErrorPropagationTest extends Specification { def graphql = TestUtil.graphQL(sdl).build() def query = ''' - query GetFoo @errorHandling(onError: PROPAGATE) { foo } + query GetFoo @nullOnError { foo } ''' when: ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( [foo: null] - ).graphQLContext([(ExperimentalApi.ENABLE_CUSTOM_ERROR_HANDLING): true]) + ).graphQLContext([(ExperimentalApi.ENABLE_NULL_ON_ERROR): true]) .build() def er = graphql.execute(ei) then: er.data == null - er.errors[0].message.equals("Validation error (UnknownDirective) : Unknown directive 'errorHandling'") + er.errors[0].message.equals("Validation error (UnknownDirective) : Unknown directive 'nullOnError'") } } From 6542fe2b3acaa033afcd92c572c5916a65b7d3a7 Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Tue, 17 Dec 2024 15:30:52 +0100 Subject: [PATCH 17/27] add .idea/codeStyles --- .gitignore | 3 ++- .idea/codeStyles/Project.xml | 8 ++++++++ .idea/codeStyles/codeStyleConfig.xml | 5 +++++ 3 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 .idea/codeStyles/Project.xml create mode 100644 .idea/codeStyles/codeStyleConfig.xml diff --git a/.gitignore b/.gitignore index dad8d9885c..fe4fe0f9ef 100644 --- a/.gitignore +++ b/.gitignore @@ -1,7 +1,8 @@ *.iml *.ipr *.iws -.idea +.idea/* +!.idea/codeStyles .gradle build classes diff --git a/.idea/codeStyles/Project.xml b/.idea/codeStyles/Project.xml new file mode 100644 index 0000000000..1951b805d3 --- /dev/null +++ b/.idea/codeStyles/Project.xml @@ -0,0 +1,8 @@ + + + + + + \ No newline at end of file diff --git a/.idea/codeStyles/codeStyleConfig.xml b/.idea/codeStyles/codeStyleConfig.xml new file mode 100644 index 0000000000..79ee123c2b --- /dev/null +++ b/.idea/codeStyles/codeStyleConfig.xml @@ -0,0 +1,5 @@ + + + + \ No newline at end of file From e992d4dac87707a24df3a512a22d0c4976703855 Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Tue, 17 Dec 2024 15:35:04 +0100 Subject: [PATCH 18/27] location -> position --- src/main/java/graphql/Directives.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/main/java/graphql/Directives.java b/src/main/java/graphql/Directives.java index 4b48bc2118..5625889411 100644 --- a/src/main/java/graphql/Directives.java +++ b/src/main/java/graphql/Directives.java @@ -4,10 +4,8 @@ import graphql.language.BooleanValue; import graphql.language.Description; import graphql.language.DirectiveDefinition; -import graphql.language.EnumValue; import graphql.language.StringValue; import graphql.schema.GraphQLDirective; -import graphql.schema.GraphQLEnumType; import static graphql.Scalars.GraphQLBoolean; import static graphql.Scalars.GraphQLString; @@ -146,7 +144,7 @@ public class Directives { .directiveLocation(newDirectiveLocation().name(QUERY.name()).build()) .directiveLocation(newDirectiveLocation().name(MUTATION.name()).build()) .directiveLocation(newDirectiveLocation().name(SUBSCRIPTION.name()).build()) - .description(createDescription("This directive allows returning null in non-null locations that have an associated error")) + .description(createDescription("This directive allows returning null in non-null positions that have an associated error")) .build(); } @@ -244,7 +242,7 @@ public class Directives { @ExperimentalApi public static final GraphQLDirective NullOnErrorDirective = GraphQLDirective.newDirective() .name(NULL_ON_ERROR) - .description("This directive allows returning null in non-null locations that have an associated error.") + .description("This directive allows returning null in non-null positions that have an associated error.") .validLocations(QUERY, MUTATION, SUBSCRIPTION) .definition(NULL_ON_ERROR_DIRECTIVE_DEFINITION) .build(); From e744ccaa53014d43b9632ffc9bb3a7e0f0502943 Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Fri, 10 Jan 2025 00:09:03 +0100 Subject: [PATCH 19/27] Rename to @nullOnNonNullError --- src/main/java/graphql/Directives.java | 12 ++++---- src/main/java/graphql/ExperimentalApi.java | 4 +-- .../java/graphql/execution/Execution.java | 6 ++-- .../graphql/execution/ExecutionContext.java | 2 +- .../execution/NoErrorPropagationTest.groovy | 28 +++++++++---------- 5 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/main/java/graphql/Directives.java b/src/main/java/graphql/Directives.java index 5625889411..061de74129 100644 --- a/src/main/java/graphql/Directives.java +++ b/src/main/java/graphql/Directives.java @@ -40,7 +40,7 @@ public class Directives { private static final String SPECIFIED_BY = "specifiedBy"; private static final String ONE_OF = "oneOf"; private static final String DEFER = "defer"; - private static final String NULL_ON_ERROR = "nullOnError"; + private static final String NULL_ON_NON_NULL_ERROR = "nullOnNonNullError"; public static final DirectiveDefinition DEPRECATED_DIRECTIVE_DEFINITION; public static final DirectiveDefinition INCLUDE_DIRECTIVE_DEFINITION; @@ -51,7 +51,7 @@ public class Directives { @ExperimentalApi public static final DirectiveDefinition DEFER_DIRECTIVE_DEFINITION; @ExperimentalApi - public static final DirectiveDefinition NULL_ON_ERROR_DIRECTIVE_DEFINITION; + public static final DirectiveDefinition NULL_ON_NON_NULL_ERROR_DIRECTIVE_DEFINITION; public static final String BOOLEAN = "Boolean"; public static final String STRING = "String"; @@ -139,8 +139,8 @@ public class Directives { .type(newTypeName().name(STRING).build()) .build()) .build(); - NULL_ON_ERROR_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition() - .name(NULL_ON_ERROR) + NULL_ON_NON_NULL_ERROR_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition() + .name(NULL_ON_NON_NULL_ERROR) .directiveLocation(newDirectiveLocation().name(QUERY.name()).build()) .directiveLocation(newDirectiveLocation().name(MUTATION.name()).build()) .directiveLocation(newDirectiveLocation().name(SUBSCRIPTION.name()).build()) @@ -241,10 +241,10 @@ public class Directives { @ExperimentalApi public static final GraphQLDirective NullOnErrorDirective = GraphQLDirective.newDirective() - .name(NULL_ON_ERROR) + .name(NULL_ON_NON_NULL_ERROR) .description("This directive allows returning null in non-null positions that have an associated error.") .validLocations(QUERY, MUTATION, SUBSCRIPTION) - .definition(NULL_ON_ERROR_DIRECTIVE_DEFINITION) + .definition(NULL_ON_NON_NULL_ERROR_DIRECTIVE_DEFINITION) .build(); private static Description createDescription(String s) { diff --git a/src/main/java/graphql/ExperimentalApi.java b/src/main/java/graphql/ExperimentalApi.java index 65ebd9ddc0..333a177721 100644 --- a/src/main/java/graphql/ExperimentalApi.java +++ b/src/main/java/graphql/ExperimentalApi.java @@ -25,7 +25,7 @@ */ String ENABLE_INCREMENTAL_SUPPORT = "ENABLE_INCREMENTAL_SUPPORT"; /** - * The key that should be associated with a boolean value which indicates whether @nullOnError behaviour is enabled for this execution. + * The key that should be associated with a boolean value which indicates whether @nullOnNonNullError behaviour is enabled for this execution. */ - String ENABLE_NULL_ON_ERROR = "ENABLE_NULL_ON_ERROR"; + String ENABLE_NULL_ON_NON_NULL_ERROR = "ENABLE_NULL_ON_NON_NULL_ERROR"; } diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index e93617ee8c..2f4b6b0ba2 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -37,7 +37,7 @@ import java.util.Optional; import java.util.concurrent.CompletableFuture; -import static graphql.Directives.NULL_ON_ERROR_DIRECTIVE_DEFINITION; +import static graphql.Directives.NULL_ON_NON_NULL_ERROR_DIRECTIVE_DEFINITION; import static graphql.execution.ExecutionContextBuilder.newExecutionContextBuilder; import static graphql.execution.ExecutionStepInfo.newExecutionStepInfo; import static graphql.execution.ExecutionStrategyParameters.newParameters; @@ -89,7 +89,7 @@ public CompletableFuture execute(Document document, GraphQLSche } boolean customErrorPropagationEnabled = Optional.ofNullable(executionInput.getGraphQLContext()) - .map(graphqlContext -> graphqlContext.getBoolean(ExperimentalApi.ENABLE_NULL_ON_ERROR)) + .map(graphqlContext -> graphqlContext.getBoolean(ExperimentalApi.ENABLE_NULL_ON_NON_NULL_ERROR)) .orElse(false); boolean propagateErrors = !customErrorPropagationEnabled || propagateErrors(operationDefinition.getDirectives(), true); @@ -272,7 +272,7 @@ private ExecutionResult mergeExtensionsBuilderIfPresent(ExecutionResult executio } private boolean propagateErrors(List directives, boolean defaultValue) { - Directive foundDirective = NodeUtil.findNodeByName(directives, NULL_ON_ERROR_DIRECTIVE_DEFINITION.getName()); + Directive foundDirective = NodeUtil.findNodeByName(directives, NULL_ON_NON_NULL_ERROR_DIRECTIVE_DEFINITION.getName()); if (foundDirective != null) { return false; } diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index 73b12a1088..196aa31856 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -176,7 +176,7 @@ public ValueUnboxer getValueUnboxer() { /** * @return true if the current operation should propagate errors in non-null positions * Propagating errors is the default. Error aware clients may opt in returning null in non-null positions - * by using the `@nullOnError` directive. + * by using the `@nullOnNonNullError` directive. */ @ExperimentalApi public boolean propagateErrors() { return propagateErrors; } diff --git a/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy b/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy index fa9ddb8002..7c63193fff 100644 --- a/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy +++ b/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy @@ -7,25 +7,25 @@ import spock.lang.Specification class NoErrorPropagationTest extends Specification { - def "with nullOnError, null is returned"() { + def "with nullOnNonNullError, null is returned"() { def sdl = ''' type Query { foo : Int! } - directive @nullOnError on QUERY | MUTATION | SUBSCRIPTION + directive @nullOnNonNullError on QUERY | MUTATION | SUBSCRIPTION ''' def graphql = TestUtil.graphQL(sdl).build() def query = ''' - query GetFoo @nullOnError { foo } + query GetFoo @nullOnNonNullError { foo } ''' when: ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( [foo: null] - ).graphQLContext([(ExperimentalApi.ENABLE_NULL_ON_ERROR): true]) + ).graphQLContext([(ExperimentalApi.ENABLE_NULL_ON_NON_NULL_ERROR): true]) .build() def er = graphql.execute(ei) @@ -36,13 +36,13 @@ class NoErrorPropagationTest extends Specification { er.errors[0].path.toList() == ["foo"] } - def "without nullOnError, error is propagated"() { + def "without nullOnNonNullError, error is propagated"() { def sdl = ''' type Query { foo : Int! } - directive @nullOnError on QUERY | MUTATION | SUBSCRIPTION + directive @nullOnNonNullError on QUERY | MUTATION | SUBSCRIPTION ''' def graphql = TestUtil.graphQL(sdl).build() @@ -54,7 +54,7 @@ class NoErrorPropagationTest extends Specification { ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( [foo: null] - ).graphQLContext([(ExperimentalApi.ENABLE_NULL_ON_ERROR): true]) + ).graphQLContext([(ExperimentalApi.ENABLE_NULL_ON_NON_NULL_ERROR): true]) .build() def er = graphql.execute(ei) @@ -66,19 +66,19 @@ class NoErrorPropagationTest extends Specification { } - def "when ENABLE_NULL_ON_ERROR is false, error is propagated"() { + def "when ENABLE_NULL_ON_NON_NULL_ERROR is false, error is propagated"() { def sdl = ''' type Query { foo : Int! } - directive @nullOnError on QUERY | MUTATION | SUBSCRIPTION + directive @nullOnNonNullError on QUERY | MUTATION | SUBSCRIPTION ''' def graphql = TestUtil.graphQL(sdl).build() def query = ''' - query GetFoo @nullOnError { foo } + query GetFoo @nullOnNonNullError { foo } ''' when: @@ -93,7 +93,7 @@ class NoErrorPropagationTest extends Specification { er.errors[0].path.toList() == ["foo"] } - def "when @nullOnError is not added to the schema operation does not validate"() { + def "when @nullOnNonNullError is not added to the schema operation does not validate"() { def sdl = ''' type Query { @@ -104,20 +104,20 @@ class NoErrorPropagationTest extends Specification { def graphql = TestUtil.graphQL(sdl).build() def query = ''' - query GetFoo @nullOnError { foo } + query GetFoo @nullOnNonNullError { foo } ''' when: ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( [foo: null] - ).graphQLContext([(ExperimentalApi.ENABLE_NULL_ON_ERROR): true]) + ).graphQLContext([(ExperimentalApi.ENABLE_NULL_ON_NON_NULL_ERROR): true]) .build() def er = graphql.execute(ei) then: er.data == null - er.errors[0].message.equals("Validation error (UnknownDirective) : Unknown directive 'nullOnError'") + er.errors[0].message.equals("Validation error (UnknownDirective) : Unknown directive 'nullOnNonNullError'") } } From a5f0facb434e2aa90dfbf3c4c0da33ae2c7bf179 Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Tue, 4 Feb 2025 14:05:41 +0100 Subject: [PATCH 20/27] Revert "Rename to @nullOnNonNullError" This reverts commit e744ccaa53014d43b9632ffc9bb3a7e0f0502943. --- src/main/java/graphql/Directives.java | 12 ++++---- src/main/java/graphql/ExperimentalApi.java | 4 +-- .../java/graphql/execution/Execution.java | 6 ++-- .../graphql/execution/ExecutionContext.java | 2 +- .../execution/NoErrorPropagationTest.groovy | 28 +++++++++---------- 5 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/main/java/graphql/Directives.java b/src/main/java/graphql/Directives.java index 061de74129..5625889411 100644 --- a/src/main/java/graphql/Directives.java +++ b/src/main/java/graphql/Directives.java @@ -40,7 +40,7 @@ public class Directives { private static final String SPECIFIED_BY = "specifiedBy"; private static final String ONE_OF = "oneOf"; private static final String DEFER = "defer"; - private static final String NULL_ON_NON_NULL_ERROR = "nullOnNonNullError"; + private static final String NULL_ON_ERROR = "nullOnError"; public static final DirectiveDefinition DEPRECATED_DIRECTIVE_DEFINITION; public static final DirectiveDefinition INCLUDE_DIRECTIVE_DEFINITION; @@ -51,7 +51,7 @@ public class Directives { @ExperimentalApi public static final DirectiveDefinition DEFER_DIRECTIVE_DEFINITION; @ExperimentalApi - public static final DirectiveDefinition NULL_ON_NON_NULL_ERROR_DIRECTIVE_DEFINITION; + public static final DirectiveDefinition NULL_ON_ERROR_DIRECTIVE_DEFINITION; public static final String BOOLEAN = "Boolean"; public static final String STRING = "String"; @@ -139,8 +139,8 @@ public class Directives { .type(newTypeName().name(STRING).build()) .build()) .build(); - NULL_ON_NON_NULL_ERROR_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition() - .name(NULL_ON_NON_NULL_ERROR) + NULL_ON_ERROR_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition() + .name(NULL_ON_ERROR) .directiveLocation(newDirectiveLocation().name(QUERY.name()).build()) .directiveLocation(newDirectiveLocation().name(MUTATION.name()).build()) .directiveLocation(newDirectiveLocation().name(SUBSCRIPTION.name()).build()) @@ -241,10 +241,10 @@ public class Directives { @ExperimentalApi public static final GraphQLDirective NullOnErrorDirective = GraphQLDirective.newDirective() - .name(NULL_ON_NON_NULL_ERROR) + .name(NULL_ON_ERROR) .description("This directive allows returning null in non-null positions that have an associated error.") .validLocations(QUERY, MUTATION, SUBSCRIPTION) - .definition(NULL_ON_NON_NULL_ERROR_DIRECTIVE_DEFINITION) + .definition(NULL_ON_ERROR_DIRECTIVE_DEFINITION) .build(); private static Description createDescription(String s) { diff --git a/src/main/java/graphql/ExperimentalApi.java b/src/main/java/graphql/ExperimentalApi.java index 333a177721..65ebd9ddc0 100644 --- a/src/main/java/graphql/ExperimentalApi.java +++ b/src/main/java/graphql/ExperimentalApi.java @@ -25,7 +25,7 @@ */ String ENABLE_INCREMENTAL_SUPPORT = "ENABLE_INCREMENTAL_SUPPORT"; /** - * The key that should be associated with a boolean value which indicates whether @nullOnNonNullError behaviour is enabled for this execution. + * The key that should be associated with a boolean value which indicates whether @nullOnError behaviour is enabled for this execution. */ - String ENABLE_NULL_ON_NON_NULL_ERROR = "ENABLE_NULL_ON_NON_NULL_ERROR"; + String ENABLE_NULL_ON_ERROR = "ENABLE_NULL_ON_ERROR"; } diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 2f4b6b0ba2..e93617ee8c 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -37,7 +37,7 @@ import java.util.Optional; import java.util.concurrent.CompletableFuture; -import static graphql.Directives.NULL_ON_NON_NULL_ERROR_DIRECTIVE_DEFINITION; +import static graphql.Directives.NULL_ON_ERROR_DIRECTIVE_DEFINITION; import static graphql.execution.ExecutionContextBuilder.newExecutionContextBuilder; import static graphql.execution.ExecutionStepInfo.newExecutionStepInfo; import static graphql.execution.ExecutionStrategyParameters.newParameters; @@ -89,7 +89,7 @@ public CompletableFuture execute(Document document, GraphQLSche } boolean customErrorPropagationEnabled = Optional.ofNullable(executionInput.getGraphQLContext()) - .map(graphqlContext -> graphqlContext.getBoolean(ExperimentalApi.ENABLE_NULL_ON_NON_NULL_ERROR)) + .map(graphqlContext -> graphqlContext.getBoolean(ExperimentalApi.ENABLE_NULL_ON_ERROR)) .orElse(false); boolean propagateErrors = !customErrorPropagationEnabled || propagateErrors(operationDefinition.getDirectives(), true); @@ -272,7 +272,7 @@ private ExecutionResult mergeExtensionsBuilderIfPresent(ExecutionResult executio } private boolean propagateErrors(List directives, boolean defaultValue) { - Directive foundDirective = NodeUtil.findNodeByName(directives, NULL_ON_NON_NULL_ERROR_DIRECTIVE_DEFINITION.getName()); + Directive foundDirective = NodeUtil.findNodeByName(directives, NULL_ON_ERROR_DIRECTIVE_DEFINITION.getName()); if (foundDirective != null) { return false; } diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index 196aa31856..73b12a1088 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -176,7 +176,7 @@ public ValueUnboxer getValueUnboxer() { /** * @return true if the current operation should propagate errors in non-null positions * Propagating errors is the default. Error aware clients may opt in returning null in non-null positions - * by using the `@nullOnNonNullError` directive. + * by using the `@nullOnError` directive. */ @ExperimentalApi public boolean propagateErrors() { return propagateErrors; } diff --git a/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy b/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy index 7c63193fff..fa9ddb8002 100644 --- a/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy +++ b/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy @@ -7,25 +7,25 @@ import spock.lang.Specification class NoErrorPropagationTest extends Specification { - def "with nullOnNonNullError, null is returned"() { + def "with nullOnError, null is returned"() { def sdl = ''' type Query { foo : Int! } - directive @nullOnNonNullError on QUERY | MUTATION | SUBSCRIPTION + directive @nullOnError on QUERY | MUTATION | SUBSCRIPTION ''' def graphql = TestUtil.graphQL(sdl).build() def query = ''' - query GetFoo @nullOnNonNullError { foo } + query GetFoo @nullOnError { foo } ''' when: ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( [foo: null] - ).graphQLContext([(ExperimentalApi.ENABLE_NULL_ON_NON_NULL_ERROR): true]) + ).graphQLContext([(ExperimentalApi.ENABLE_NULL_ON_ERROR): true]) .build() def er = graphql.execute(ei) @@ -36,13 +36,13 @@ class NoErrorPropagationTest extends Specification { er.errors[0].path.toList() == ["foo"] } - def "without nullOnNonNullError, error is propagated"() { + def "without nullOnError, error is propagated"() { def sdl = ''' type Query { foo : Int! } - directive @nullOnNonNullError on QUERY | MUTATION | SUBSCRIPTION + directive @nullOnError on QUERY | MUTATION | SUBSCRIPTION ''' def graphql = TestUtil.graphQL(sdl).build() @@ -54,7 +54,7 @@ class NoErrorPropagationTest extends Specification { ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( [foo: null] - ).graphQLContext([(ExperimentalApi.ENABLE_NULL_ON_NON_NULL_ERROR): true]) + ).graphQLContext([(ExperimentalApi.ENABLE_NULL_ON_ERROR): true]) .build() def er = graphql.execute(ei) @@ -66,19 +66,19 @@ class NoErrorPropagationTest extends Specification { } - def "when ENABLE_NULL_ON_NON_NULL_ERROR is false, error is propagated"() { + def "when ENABLE_NULL_ON_ERROR is false, error is propagated"() { def sdl = ''' type Query { foo : Int! } - directive @nullOnNonNullError on QUERY | MUTATION | SUBSCRIPTION + directive @nullOnError on QUERY | MUTATION | SUBSCRIPTION ''' def graphql = TestUtil.graphQL(sdl).build() def query = ''' - query GetFoo @nullOnNonNullError { foo } + query GetFoo @nullOnError { foo } ''' when: @@ -93,7 +93,7 @@ class NoErrorPropagationTest extends Specification { er.errors[0].path.toList() == ["foo"] } - def "when @nullOnNonNullError is not added to the schema operation does not validate"() { + def "when @nullOnError is not added to the schema operation does not validate"() { def sdl = ''' type Query { @@ -104,20 +104,20 @@ class NoErrorPropagationTest extends Specification { def graphql = TestUtil.graphQL(sdl).build() def query = ''' - query GetFoo @nullOnNonNullError { foo } + query GetFoo @nullOnError { foo } ''' when: ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( [foo: null] - ).graphQLContext([(ExperimentalApi.ENABLE_NULL_ON_NON_NULL_ERROR): true]) + ).graphQLContext([(ExperimentalApi.ENABLE_NULL_ON_ERROR): true]) .build() def er = graphql.execute(ei) then: er.data == null - er.errors[0].message.equals("Validation error (UnknownDirective) : Unknown directive 'nullOnNonNullError'") + er.errors[0].message.equals("Validation error (UnknownDirective) : Unknown directive 'nullOnError'") } } From 36ebb402cd7fc72566885642c6e041ad996531a7 Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Mon, 24 Feb 2025 12:51:11 +0100 Subject: [PATCH 21/27] keep in sync with graphql-js https://github.com/graphql/graphql-js/pull/4348 --- src/main/java/graphql/Directives.java | 16 +++--- src/main/java/graphql/ExperimentalApi.java | 4 -- .../java/graphql/execution/Execution.java | 9 ++-- .../graphql/execution/ExecutionContext.java | 2 +- .../execution/NoErrorPropagationTest.groovy | 54 ++++--------------- 5 files changed, 23 insertions(+), 62 deletions(-) diff --git a/src/main/java/graphql/Directives.java b/src/main/java/graphql/Directives.java index 5625889411..5900bbc74c 100644 --- a/src/main/java/graphql/Directives.java +++ b/src/main/java/graphql/Directives.java @@ -40,7 +40,7 @@ public class Directives { private static final String SPECIFIED_BY = "specifiedBy"; private static final String ONE_OF = "oneOf"; private static final String DEFER = "defer"; - private static final String NULL_ON_ERROR = "nullOnError"; + private static final String DISABLE_ERROR_PROPAGATION = "experimental_disableErrorPropagation"; public static final DirectiveDefinition DEPRECATED_DIRECTIVE_DEFINITION; public static final DirectiveDefinition INCLUDE_DIRECTIVE_DEFINITION; @@ -51,7 +51,7 @@ public class Directives { @ExperimentalApi public static final DirectiveDefinition DEFER_DIRECTIVE_DEFINITION; @ExperimentalApi - public static final DirectiveDefinition NULL_ON_ERROR_DIRECTIVE_DEFINITION; + public static final DirectiveDefinition DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION; public static final String BOOLEAN = "Boolean"; public static final String STRING = "String"; @@ -139,8 +139,8 @@ public class Directives { .type(newTypeName().name(STRING).build()) .build()) .build(); - NULL_ON_ERROR_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition() - .name(NULL_ON_ERROR) + DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition() + .name(DISABLE_ERROR_PROPAGATION) .directiveLocation(newDirectiveLocation().name(QUERY.name()).build()) .directiveLocation(newDirectiveLocation().name(MUTATION.name()).build()) .directiveLocation(newDirectiveLocation().name(SUBSCRIPTION.name()).build()) @@ -240,11 +240,11 @@ public class Directives { .build(); @ExperimentalApi - public static final GraphQLDirective NullOnErrorDirective = GraphQLDirective.newDirective() - .name(NULL_ON_ERROR) - .description("This directive allows returning null in non-null positions that have an associated error.") + public static final GraphQLDirective DisableErrorPropagationDirective = GraphQLDirective.newDirective() + .name(DISABLE_ERROR_PROPAGATION) + .description("This directive disables error propagation for the given operation.") .validLocations(QUERY, MUTATION, SUBSCRIPTION) - .definition(NULL_ON_ERROR_DIRECTIVE_DEFINITION) + .definition(DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION) .build(); private static Description createDescription(String s) { diff --git a/src/main/java/graphql/ExperimentalApi.java b/src/main/java/graphql/ExperimentalApi.java index 65ebd9ddc0..80be253cd1 100644 --- a/src/main/java/graphql/ExperimentalApi.java +++ b/src/main/java/graphql/ExperimentalApi.java @@ -24,8 +24,4 @@ * The key that should be associated with a boolean value which indicates whether @defer and @stream behaviour is enabled for this execution. */ String ENABLE_INCREMENTAL_SUPPORT = "ENABLE_INCREMENTAL_SUPPORT"; - /** - * The key that should be associated with a boolean value which indicates whether @nullOnError behaviour is enabled for this execution. - */ - String ENABLE_NULL_ON_ERROR = "ENABLE_NULL_ON_ERROR"; } diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index e93617ee8c..c7870d0cfb 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -37,7 +37,7 @@ import java.util.Optional; import java.util.concurrent.CompletableFuture; -import static graphql.Directives.NULL_ON_ERROR_DIRECTIVE_DEFINITION; +import static graphql.Directives.DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION; import static graphql.execution.ExecutionContextBuilder.newExecutionContextBuilder; import static graphql.execution.ExecutionStepInfo.newExecutionStepInfo; import static graphql.execution.ExecutionStrategyParameters.newParameters; @@ -88,10 +88,7 @@ public CompletableFuture execute(Document document, GraphQLSche throw rte; } - boolean customErrorPropagationEnabled = Optional.ofNullable(executionInput.getGraphQLContext()) - .map(graphqlContext -> graphqlContext.getBoolean(ExperimentalApi.ENABLE_NULL_ON_ERROR)) - .orElse(false); - boolean propagateErrors = !customErrorPropagationEnabled || propagateErrors(operationDefinition.getDirectives(), true); + boolean propagateErrors = propagateErrors(operationDefinition.getDirectives(), true); ExecutionContext executionContext = newExecutionContextBuilder() .instrumentation(instrumentation) @@ -272,7 +269,7 @@ private ExecutionResult mergeExtensionsBuilderIfPresent(ExecutionResult executio } private boolean propagateErrors(List directives, boolean defaultValue) { - Directive foundDirective = NodeUtil.findNodeByName(directives, NULL_ON_ERROR_DIRECTIVE_DEFINITION.getName()); + Directive foundDirective = NodeUtil.findNodeByName(directives, DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION.getName()); if (foundDirective != null) { return false; } diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index 73b12a1088..287f8bb20d 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -176,7 +176,7 @@ public ValueUnboxer getValueUnboxer() { /** * @return true if the current operation should propagate errors in non-null positions * Propagating errors is the default. Error aware clients may opt in returning null in non-null positions - * by using the `@nullOnError` directive. + * by using the `@experimental_disableErrorPropagation` directive. */ @ExperimentalApi public boolean propagateErrors() { return propagateErrors; } diff --git a/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy b/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy index fa9ddb8002..839f3a1dcb 100644 --- a/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy +++ b/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy @@ -1,32 +1,30 @@ package graphql.execution import graphql.ExecutionInput -import graphql.ExperimentalApi import graphql.TestUtil import spock.lang.Specification class NoErrorPropagationTest extends Specification { - def "with nullOnError, null is returned"() { + def "with experimental_disableErrorPropagation, null is returned"() { def sdl = ''' type Query { foo : Int! } - directive @nullOnError on QUERY | MUTATION | SUBSCRIPTION + directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION ''' def graphql = TestUtil.graphQL(sdl).build() def query = ''' - query GetFoo @nullOnError { foo } + query GetFoo @experimental_disableErrorPropagation { foo } ''' when: ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( [foo: null] - ).graphQLContext([(ExperimentalApi.ENABLE_NULL_ON_ERROR): true]) - .build() + ).build() def er = graphql.execute(ei) @@ -36,13 +34,13 @@ class NoErrorPropagationTest extends Specification { er.errors[0].path.toList() == ["foo"] } - def "without nullOnError, error is propagated"() { + def "without experimental_disableErrorPropagation, error is propagated"() { def sdl = ''' type Query { foo : Int! } - directive @nullOnError on QUERY | MUTATION | SUBSCRIPTION + directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION ''' def graphql = TestUtil.graphQL(sdl).build() @@ -54,8 +52,7 @@ class NoErrorPropagationTest extends Specification { ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( [foo: null] - ).graphQLContext([(ExperimentalApi.ENABLE_NULL_ON_ERROR): true]) - .build() + ).build() def er = graphql.execute(ei) @@ -64,21 +61,19 @@ class NoErrorPropagationTest extends Specification { er.errors[0].message == "The field at path '/foo' was declared as a non null type, but the code involved in retrieving data has wrongly returned a null value. The graphql specification requires that the parent field be set to null, or if that is non nullable that it bubble up null to its parent and so on. The non-nullable type is 'Int' within parent type 'Query'" er.errors[0].path.toList() == ["foo"] } - - - def "when ENABLE_NULL_ON_ERROR is false, error is propagated"() { + + def "when @experimental_disableErrorPropagation is not added to the schema operation does not validate"() { def sdl = ''' type Query { foo : Int! } - directive @nullOnError on QUERY | MUTATION | SUBSCRIPTION ''' def graphql = TestUtil.graphQL(sdl).build() def query = ''' - query GetFoo @nullOnError { foo } + query GetFoo @experimental_disableErrorPropagation { foo } ''' when: @@ -90,34 +85,7 @@ class NoErrorPropagationTest extends Specification { then: er.data == null - er.errors[0].path.toList() == ["foo"] - } - - def "when @nullOnError is not added to the schema operation does not validate"() { - - def sdl = ''' - type Query { - foo : Int! - } - ''' - - def graphql = TestUtil.graphQL(sdl).build() - - def query = ''' - query GetFoo @nullOnError { foo } - ''' - when: - - ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( - [foo: null] - ).graphQLContext([(ExperimentalApi.ENABLE_NULL_ON_ERROR): true]) - .build() - - def er = graphql.execute(ei) - - then: - er.data == null - er.errors[0].message.equals("Validation error (UnknownDirective) : Unknown directive 'nullOnError'") + er.errors[0].message.equals("Validation error (UnknownDirective) : Unknown directive 'experimental_disableErrorPropagation'") } } From 01323743a863fb0611dbd203d3ea7695d0b52103 Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Mon, 24 Feb 2025 12:55:55 +0100 Subject: [PATCH 22/27] resolve merge --- src/main/java/graphql/execution/Execution.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 8b858fa368..e92f62a5e3 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -83,7 +83,7 @@ public CompletableFuture execute(Document document, GraphQLSche throw rte; } - boolean propagateErrors = propagateErrors(operationDefinition.getDirectives(), true); + boolean propagateErrors = propagateErrors(getOperationResult.operationDefinition.getDirectives(), true); ExecutionContext executionContext = newExecutionContextBuilder() .instrumentation(instrumentation) From e5a5342217f9b831fdbab2ff59ee54f3885ba522 Mon Sep 17 00:00:00 2001 From: bbaker Date: Thu, 27 Feb 2025 16:59:44 +1100 Subject: [PATCH 23/27] Brad Baker here Rather than ask for these changes - I decided to push to your branch as a maintainer and then get them into your PR --- src/main/java/graphql/Directives.java | 22 +++++++++++ .../java/graphql/execution/Execution.java | 16 ++++---- .../graphql/execution/ExecutionContext.java | 25 ++++-------- .../execution/ExecutionContextBuilder.java | 8 ++-- .../execution/NonNullableFieldValidator.java | 2 +- ...imentalDisableErrorPropagationTest.groovy} | 38 ++++++++++++++++++- .../NonNullableFieldValidatorTest.groovy | 6 +-- 7 files changed, 83 insertions(+), 34 deletions(-) rename src/test/groovy/graphql/execution/{NoErrorPropagationTest.groovy => ExperimentalDisableErrorPropagationTest.groovy} (65%) diff --git a/src/main/java/graphql/Directives.java b/src/main/java/graphql/Directives.java index 5900bbc74c..3b85837b64 100644 --- a/src/main/java/graphql/Directives.java +++ b/src/main/java/graphql/Directives.java @@ -7,6 +7,8 @@ import graphql.language.StringValue; import graphql.schema.GraphQLDirective; +import java.util.concurrent.atomic.AtomicBoolean; + import static graphql.Scalars.GraphQLBoolean; import static graphql.Scalars.GraphQLString; import static graphql.introspection.Introspection.DirectiveLocation.ARGUMENT_DEFINITION; @@ -250,4 +252,24 @@ public class Directives { private static Description createDescription(String s) { return new Description(s, null, false); } + + private static final AtomicBoolean EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_ENABLED = new AtomicBoolean(true); + + /** + * This can be used to get the state the `@experimental_disableErrorPropagation` directive support on a JVM wide basis . + * @return true if the `@experimental_disableErrorPropagation` directive will be respected + */ + public static boolean isExperimentalDisableErrorPropagationDirectiveEnabled() { + return EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_ENABLED.get(); + } + + /** + * This can be used to disable the `@experimental_disableErrorPropagation` directive support on a JVM wide basis in case your server + * implementation does NOT want to act on this directive ever. + * + * @param flag the desired state of the flag + */ + public static void setExperimentalDisableErrorPropagationEnabled(boolean flag) { + EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_ENABLED.set(flag); + } } diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index e92f62a5e3..6f84b91b28 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -1,6 +1,7 @@ package graphql.execution; +import graphql.Directives; import graphql.ExecutionInput; import graphql.ExecutionResult; import graphql.ExecutionResultImpl; @@ -83,7 +84,7 @@ public CompletableFuture execute(Document document, GraphQLSche throw rte; } - boolean propagateErrors = propagateErrors(getOperationResult.operationDefinition.getDirectives(), true); + boolean propagateErrorsOnNonNullContractFailure = propagateErrorsOnNonNullContractFailure(getOperationResult.operationDefinition.getDirectives()); ExecutionContext executionContext = newExecutionContextBuilder() .instrumentation(instrumentation) @@ -105,7 +106,7 @@ public CompletableFuture execute(Document document, GraphQLSche .locale(executionInput.getLocale()) .valueUnboxer(valueUnboxer) .executionInput(executionInput) - .propagateErrors(propagateErrors) + .propagapropagateErrorsOnNonNullContractFailureeErrors(propagateErrorsOnNonNullContractFailure) .build(); executionContext.getGraphQLContext().put(ResultNodesInfo.RESULT_NODES_INFO, executionContext.getResultNodesInfo()); @@ -269,11 +270,12 @@ private ExecutionResult mergeExtensionsBuilderIfPresent(ExecutionResult executio return executionResult; } - private boolean propagateErrors(List directives, boolean defaultValue) { - Directive foundDirective = NodeUtil.findNodeByName(directives, DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION.getName()); - if (foundDirective != null) { - return false; + private boolean propagateErrorsOnNonNullContractFailure(List directives) { + boolean jvmWideEnabled = Directives.isExperimentalDisableErrorPropagationDirectiveEnabled(); + if (! jvmWideEnabled) { + return true; } - return defaultValue; + Directive foundDirective = NodeUtil.findNodeByName(directives, DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION.getName()); + return foundDirective == null; } } diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index 287f8bb20d..331bba580f 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -3,12 +3,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import graphql.ExecutionInput; -import graphql.ExperimentalApi; -import graphql.GraphQLContext; -import graphql.GraphQLError; -import graphql.Internal; -import graphql.PublicApi; +import graphql.*; import graphql.collect.ImmutableKit; import graphql.execution.incremental.IncrementalCallState; import graphql.execution.instrumentation.Instrumentation; @@ -23,11 +18,7 @@ import graphql.util.LockKit; import org.dataloader.DataLoaderRegistry; -import java.util.HashSet; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.Supplier; @@ -60,7 +51,7 @@ public class ExecutionContext { private final ValueUnboxer valueUnboxer; private final ExecutionInput executionInput; private final Supplier queryTree; - private final boolean propagateErrors; + private final boolean propagateErrorsOnNonNullContractFailure; // this is modified after creation so it needs to be volatile to ensure visibility across Threads private volatile DataLoaderDispatchStrategy dataLoaderDispatcherStrategy = DataLoaderDispatchStrategy.NO_OP; @@ -90,7 +81,7 @@ public class ExecutionContext { this.executionInput = builder.executionInput; this.dataLoaderDispatcherStrategy = builder.dataLoaderDispatcherStrategy; this.queryTree = FpKit.interThreadMemoize(() -> ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(graphQLSchema, operationDefinition, fragmentsByName, coercedVariables)); - this.propagateErrors = builder.propagateErrors; + this.propagateErrorsOnNonNullContractFailure = builder.propagateErrorsOnNonNullContractFailure; } @@ -132,9 +123,7 @@ public CoercedVariables getCoercedVariables() { /** * @param for two - * * @return the legacy context - * * @deprecated use {@link #getGraphQLContext()} instead */ @Deprecated(since = "2021-07-05") @@ -177,9 +166,12 @@ public ValueUnboxer getValueUnboxer() { * @return true if the current operation should propagate errors in non-null positions * Propagating errors is the default. Error aware clients may opt in returning null in non-null positions * by using the `@experimental_disableErrorPropagation` directive. + * @see graphql.Directives#setExperimentalDisableErrorPropagationEnabled(boolean) to change the JVM wide default */ @ExperimentalApi - public boolean propagateErrors() { return propagateErrors; } + public boolean propagateErrorsOnNonNullContractFailure() { + return propagateErrorsOnNonNullContractFailure; + } /** * @return true if the current operation is a Query @@ -328,7 +320,6 @@ public DataLoaderDispatchStrategy getDataLoaderDispatcherStrategy() { * the current values and allows you to transform it how you want. * * @param builderConsumer the consumer code that will be given a builder to transform - * * @return a new ExecutionContext object based on calling build on that builder */ public ExecutionContext transform(Consumer builderConsumer) { diff --git a/src/main/java/graphql/execution/ExecutionContextBuilder.java b/src/main/java/graphql/execution/ExecutionContextBuilder.java index 09fb1ecd45..33a5d3b3d1 100644 --- a/src/main/java/graphql/execution/ExecutionContextBuilder.java +++ b/src/main/java/graphql/execution/ExecutionContextBuilder.java @@ -47,7 +47,7 @@ public class ExecutionContextBuilder { Object localContext; ExecutionInput executionInput; DataLoaderDispatchStrategy dataLoaderDispatcherStrategy = DataLoaderDispatchStrategy.NO_OP; - boolean propagateErrors = true; + boolean propagateErrorsOnNonNullContractFailure = true; /** * @return a new builder of {@link graphql.execution.ExecutionContext}s @@ -94,7 +94,7 @@ public ExecutionContextBuilder() { valueUnboxer = other.getValueUnboxer(); executionInput = other.getExecutionInput(); dataLoaderDispatcherStrategy = other.getDataLoaderDispatcherStrategy(); - propagateErrors = other.propagateErrors(); + propagateErrorsOnNonNullContractFailure = other.propagateErrorsOnNonNullContractFailure(); } public ExecutionContextBuilder instrumentation(Instrumentation instrumentation) { @@ -220,8 +220,8 @@ public ExecutionContextBuilder resetErrors() { } @ExperimentalApi - public ExecutionContextBuilder propagateErrors(boolean propagateErrors) { - this.propagateErrors = propagateErrors; + public ExecutionContextBuilder propagapropagateErrorsOnNonNullContractFailureeErrors(boolean propagateErrorsOnNonNullContractFailure) { + this.propagateErrorsOnNonNullContractFailure = propagateErrorsOnNonNullContractFailure; return this; } diff --git a/src/main/java/graphql/execution/NonNullableFieldValidator.java b/src/main/java/graphql/execution/NonNullableFieldValidator.java index 2e1068b225..d7e14900a4 100644 --- a/src/main/java/graphql/execution/NonNullableFieldValidator.java +++ b/src/main/java/graphql/execution/NonNullableFieldValidator.java @@ -56,7 +56,7 @@ public T validate(ExecutionStrategyParameters parameters, T result) throws N } else { executionContext.addError(error, path); } - if (executionContext.propagateErrors()) { + if (executionContext.propagateErrorsOnNonNullContractFailure()) { throw nonNullException; } } diff --git a/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy b/src/test/groovy/graphql/execution/ExperimentalDisableErrorPropagationTest.groovy similarity index 65% rename from src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy rename to src/test/groovy/graphql/execution/ExperimentalDisableErrorPropagationTest.groovy index 839f3a1dcb..a77d51d392 100644 --- a/src/test/groovy/graphql/execution/NoErrorPropagationTest.groovy +++ b/src/test/groovy/graphql/execution/ExperimentalDisableErrorPropagationTest.groovy @@ -1,10 +1,15 @@ package graphql.execution +import graphql.Directives import graphql.ExecutionInput import graphql.TestUtil import spock.lang.Specification -class NoErrorPropagationTest extends Specification { +class ExperimentalDisableErrorPropagationTest extends Specification { + + void setup() { + Directives.setExperimentalDisableErrorPropagationEnabled(true) + } def "with experimental_disableErrorPropagation, null is returned"() { @@ -61,7 +66,36 @@ class NoErrorPropagationTest extends Specification { er.errors[0].message == "The field at path '/foo' was declared as a non null type, but the code involved in retrieving data has wrongly returned a null value. The graphql specification requires that the parent field be set to null, or if that is non nullable that it bubble up null to its parent and so on. The non-nullable type is 'Int' within parent type 'Query'" er.errors[0].path.toList() == ["foo"] } - + + def "With experimental_disableErrorPropagation JVM disabled, error is propagated"() { + def sdl = ''' + type Query { + foo : Int! + } + directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION + ''' + + def graphql = TestUtil.graphQL(sdl).build() + + def query = ''' + query GetFoo @experimental_disableErrorPropagation { foo } + ''' + when: + + Directives.setExperimentalDisableErrorPropagationEnabled(false) // JVM wide + + ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( + [foo: null] + ).build() + + def er = graphql.execute(ei) + + then: + er.data == null + er.errors[0].message == "The field at path '/foo' was declared as a non null type, but the code involved in retrieving data has wrongly returned a null value. The graphql specification requires that the parent field be set to null, or if that is non nullable that it bubble up null to its parent and so on. The non-nullable type is 'Int' within parent type 'Query'" + er.errors[0].path.toList() == ["foo"] + } + def "when @experimental_disableErrorPropagation is not added to the schema operation does not validate"() { def sdl = ''' diff --git a/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy b/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy index 75f0cc79cd..34a48affe7 100644 --- a/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy +++ b/src/test/groovy/graphql/execution/NonNullableFieldValidatorTest.groovy @@ -13,7 +13,7 @@ class NonNullableFieldValidatorTest extends Specification { def "non nullable field throws exception"() { ExecutionContext context = Mock(ExecutionContext) { - propagateErrors() >> true + propagateErrorsOnNonNullContractFailure() >> true } ExecutionStepInfo typeInfo = ExecutionStepInfo.newExecutionStepInfo().type(nonNull(GraphQLString)).build() @@ -30,7 +30,7 @@ class NonNullableFieldValidatorTest extends Specification { def "nullable field does not throw exception"() { ExecutionContext context = Mock(ExecutionContext) { - propagateErrors() >> true + propagateErrorsOnNonNullContractFailure() >> true } ExecutionStepInfo typeInfo = ExecutionStepInfo.newExecutionStepInfo().type(GraphQLString).build() @@ -46,7 +46,7 @@ class NonNullableFieldValidatorTest extends Specification { def "non nullable field returns null if errors are not propagated"() { ExecutionContext context = Mock(ExecutionContext) { - propagateErrors() >> false + propagateErrorsOnNonNullContractFailure() >> false } ExecutionStepInfo typeInfo = ExecutionStepInfo.newExecutionStepInfo().type(nonNull(GraphQLString)).build() From 535a620505004c1b92563ec0f7a2fee4f0d00f3a Mon Sep 17 00:00:00 2001 From: Martin Bonnin Date: Thu, 27 Feb 2025 11:15:36 +0100 Subject: [PATCH 24/27] revert codeStyle changes --- .gitignore | 3 +-- .idea/codeStyles/Project.xml | 8 -------- .idea/codeStyles/codeStyleConfig.xml | 5 ----- 3 files changed, 1 insertion(+), 15 deletions(-) delete mode 100644 .idea/codeStyles/Project.xml delete mode 100644 .idea/codeStyles/codeStyleConfig.xml diff --git a/.gitignore b/.gitignore index fe4fe0f9ef..dad8d9885c 100644 --- a/.gitignore +++ b/.gitignore @@ -1,8 +1,7 @@ *.iml *.ipr *.iws -.idea/* -!.idea/codeStyles +.idea .gradle build classes diff --git a/.idea/codeStyles/Project.xml b/.idea/codeStyles/Project.xml deleted file mode 100644 index 1951b805d3..0000000000 --- a/.idea/codeStyles/Project.xml +++ /dev/null @@ -1,8 +0,0 @@ - - - - - - \ No newline at end of file diff --git a/.idea/codeStyles/codeStyleConfig.xml b/.idea/codeStyles/codeStyleConfig.xml deleted file mode 100644 index 79ee123c2b..0000000000 --- a/.idea/codeStyles/codeStyleConfig.xml +++ /dev/null @@ -1,5 +0,0 @@ - - - - \ No newline at end of file From a37f8bde4c0f991d3254ba9dba36f66bf2856f9a Mon Sep 17 00:00:00 2001 From: bbaker Date: Fri, 28 Feb 2025 09:29:59 +1100 Subject: [PATCH 25/27] Brad Baker here Rather than ask for these changes - I decided to push to your branch as a maintainer and then get them into your PR This now adds the directive to the schema if not present --- src/main/java/graphql/Directives.java | 16 +++++++-------- .../java/graphql/execution/Execution.java | 4 ++-- .../java/graphql/schema/GraphQLSchema.java | 20 +++++++++---------- ...rimentalDisableErrorPropagationTest.groovy | 17 +++------------- 4 files changed, 23 insertions(+), 34 deletions(-) diff --git a/src/main/java/graphql/Directives.java b/src/main/java/graphql/Directives.java index 3b85837b64..37f2b28550 100644 --- a/src/main/java/graphql/Directives.java +++ b/src/main/java/graphql/Directives.java @@ -42,7 +42,7 @@ public class Directives { private static final String SPECIFIED_BY = "specifiedBy"; private static final String ONE_OF = "oneOf"; private static final String DEFER = "defer"; - private static final String DISABLE_ERROR_PROPAGATION = "experimental_disableErrorPropagation"; + private static final String EXPERIMENTAL_DISABLE_ERROR_PROPAGATION = "experimental_disableErrorPropagation"; public static final DirectiveDefinition DEPRECATED_DIRECTIVE_DEFINITION; public static final DirectiveDefinition INCLUDE_DIRECTIVE_DEFINITION; @@ -53,7 +53,7 @@ public class Directives { @ExperimentalApi public static final DirectiveDefinition DEFER_DIRECTIVE_DEFINITION; @ExperimentalApi - public static final DirectiveDefinition DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION; + public static final DirectiveDefinition EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION; public static final String BOOLEAN = "Boolean"; public static final String STRING = "String"; @@ -141,8 +141,8 @@ public class Directives { .type(newTypeName().name(STRING).build()) .build()) .build(); - DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition() - .name(DISABLE_ERROR_PROPAGATION) + EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION = DirectiveDefinition.newDirectiveDefinition() + .name(EXPERIMENTAL_DISABLE_ERROR_PROPAGATION) .directiveLocation(newDirectiveLocation().name(QUERY.name()).build()) .directiveLocation(newDirectiveLocation().name(MUTATION.name()).build()) .directiveLocation(newDirectiveLocation().name(SUBSCRIPTION.name()).build()) @@ -242,11 +242,11 @@ public class Directives { .build(); @ExperimentalApi - public static final GraphQLDirective DisableErrorPropagationDirective = GraphQLDirective.newDirective() - .name(DISABLE_ERROR_PROPAGATION) - .description("This directive disables error propagation for the given operation.") + public static final GraphQLDirective ExperimentalDisableErrorPropagationDirective = GraphQLDirective.newDirective() + .name(EXPERIMENTAL_DISABLE_ERROR_PROPAGATION) + .description("This directive disables error propagation when a non nullable field returns null for the given operation.") .validLocations(QUERY, MUTATION, SUBSCRIPTION) - .definition(DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION) + .definition(EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION) .build(); private static Description createDescription(String s) { diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 6f84b91b28..ddc384d468 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -38,7 +38,7 @@ import java.util.Optional; import java.util.concurrent.CompletableFuture; -import static graphql.Directives.DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION; +import static graphql.Directives.EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION; import static graphql.execution.ExecutionContextBuilder.newExecutionContextBuilder; import static graphql.execution.ExecutionStepInfo.newExecutionStepInfo; import static graphql.execution.ExecutionStrategyParameters.newParameters; @@ -275,7 +275,7 @@ private boolean propagateErrorsOnNonNullContractFailure(List directiv if (! jvmWideEnabled) { return true; } - Directive foundDirective = NodeUtil.findNodeByName(directives, DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION.getName()); + Directive foundDirective = NodeUtil.findNodeByName(directives, EXPERIMENTAL_DISABLE_ERROR_PROPAGATION_DIRECTIVE_DEFINITION.getName()); return foundDirective == null; } } diff --git a/src/main/java/graphql/schema/GraphQLSchema.java b/src/main/java/graphql/schema/GraphQLSchema.java index a24928869c..021932c1b7 100644 --- a/src/main/java/graphql/schema/GraphQLSchema.java +++ b/src/main/java/graphql/schema/GraphQLSchema.java @@ -847,16 +847,10 @@ private GraphQLSchema buildImpl() { // 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! - if (additionalDirectives.stream().noneMatch(d -> d.getName().equals(Directives.DeprecatedDirective.getName()))) { - additionalDirectives.add(Directives.DeprecatedDirective); - } - - if (additionalDirectives.stream().noneMatch(d -> d.getName().equals(Directives.SpecifiedByDirective.getName()))) { - additionalDirectives.add(Directives.SpecifiedByDirective); - } - if (additionalDirectives.stream().noneMatch(d -> d.getName().equals(Directives.OneOfDirective.getName()))) { - additionalDirectives.add(Directives.OneOfDirective); - } + addBuiltInDirective(Directives.DeprecatedDirective, additionalDirectives); + addBuiltInDirective(Directives.SpecifiedByDirective, additionalDirectives); + addBuiltInDirective(Directives.OneOfDirective, additionalDirectives); + addBuiltInDirective(Directives.ExperimentalDisableErrorPropagationDirective, additionalDirectives); // quick build - no traversing final GraphQLSchema partiallyBuiltSchema = new GraphQLSchema(this); @@ -879,6 +873,12 @@ 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 GraphQLSchema validateSchema(GraphQLSchema graphQLSchema) { Collection errors = new SchemaValidator().validateSchema(graphQLSchema); if (!errors.isEmpty()) { diff --git a/src/test/groovy/graphql/execution/ExperimentalDisableErrorPropagationTest.groovy b/src/test/groovy/graphql/execution/ExperimentalDisableErrorPropagationTest.groovy index a77d51d392..366c95426e 100644 --- a/src/test/groovy/graphql/execution/ExperimentalDisableErrorPropagationTest.groovy +++ b/src/test/groovy/graphql/execution/ExperimentalDisableErrorPropagationTest.groovy @@ -96,7 +96,7 @@ class ExperimentalDisableErrorPropagationTest extends Specification { er.errors[0].path.toList() == ["foo"] } - def "when @experimental_disableErrorPropagation is not added to the schema operation does not validate"() { + def "when @experimental_disableErrorPropagation is not added to the schema operation is gets added by schema code"() { def sdl = ''' type Query { @@ -104,22 +104,11 @@ class ExperimentalDisableErrorPropagationTest extends Specification { } ''' - def graphql = TestUtil.graphQL(sdl).build() - - def query = ''' - query GetFoo @experimental_disableErrorPropagation { foo } - ''' when: - - ExecutionInput ei = ExecutionInput.newExecutionInput(query).root( - [foo: null] - ).build() - - def er = graphql.execute(ei) + def graphql = TestUtil.graphQL(sdl).build() then: - er.data == null - er.errors[0].message.equals("Validation error (UnknownDirective) : Unknown directive 'experimental_disableErrorPropagation'") + graphql.getGraphQLSchema().getDirective(Directives.ExperimentalDisableErrorPropagationDirective.getName()) === Directives.ExperimentalDisableErrorPropagationDirective } } From bcffe72642aa27e8b257e6a799805ed76cd089ad Mon Sep 17 00:00:00 2001 From: bbaker Date: Fri, 28 Feb 2025 09:33:51 +1100 Subject: [PATCH 26/27] Brad Baker here Rather than ask for these changes - I decided to push to your branch as a maintainer and then get them into your PR * import --- .../java/graphql/execution/ExecutionContext.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index 331bba580f..f7727371a5 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -3,7 +3,12 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import graphql.*; +import graphql.ExecutionInput; +import graphql.ExperimentalApi; +import graphql.GraphQLContext; +import graphql.GraphQLError; +import graphql.Internal; +import graphql.PublicApi; import graphql.collect.ImmutableKit; import graphql.execution.incremental.IncrementalCallState; import graphql.execution.instrumentation.Instrumentation; @@ -18,7 +23,11 @@ import graphql.util.LockKit; import org.dataloader.DataLoaderRegistry; -import java.util.*; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.Supplier; From 8eb9fb5c8c657f0577c5fed78ad43a21c14095c8 Mon Sep 17 00:00:00 2001 From: bbaker Date: Fri, 28 Feb 2025 11:35:26 +1100 Subject: [PATCH 27/27] Brad Baker here Rather than ask for these changes - I decided to push to your branch as a maintainer and then get them into your PR Added the directive to common list --- .../graphql/schema/idl/DirectiveInfo.java | 5 ++- src/test/groovy/graphql/Issue2141.groovy | 3 ++ .../graphql/StarWarsIntrospectionTests.groovy | 2 +- ...rospectionWithDirectivesSupportTest.groovy | 7 ++-- .../graphql/schema/GraphQLSchemaTest.groovy | 8 ++--- .../schema/diffing/SchemaDiffingTest.groovy | 4 +-- ...GeneratorAppliedDirectiveHelperTest.groovy | 2 ++ .../schema/idl/SchemaGeneratorTest.groovy | 4 +-- .../schema/idl/SchemaPrinterTest.groovy | 36 +++++++++++++++++++ 9 files changed, 59 insertions(+), 12 deletions(-) diff --git a/src/main/java/graphql/schema/idl/DirectiveInfo.java b/src/main/java/graphql/schema/idl/DirectiveInfo.java index 428a97aceb..d71e52cd40 100644 --- a/src/main/java/graphql/schema/idl/DirectiveInfo.java +++ b/src/main/java/graphql/schema/idl/DirectiveInfo.java @@ -34,7 +34,10 @@ public class DirectiveInfo { Directives.SkipDirective.getName(), Directives.SkipDirective, Directives.DeprecatedDirective.getName(), Directives.DeprecatedDirective, Directives.SpecifiedByDirective.getName(), Directives.SpecifiedByDirective, - Directives.OneOfDirective.getName(), Directives.OneOfDirective + 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.ExperimentalDisableErrorPropagationDirective.getName(), Directives.ExperimentalDisableErrorPropagationDirective ); /** diff --git a/src/test/groovy/graphql/Issue2141.groovy b/src/test/groovy/graphql/Issue2141.groovy index 4bcd6b6359..efa74969e3 100644 --- a/src/test/groovy/graphql/Issue2141.groovy +++ b/src/test/groovy/graphql/Issue2141.groovy @@ -30,6 +30,9 @@ directive @deprecated( reason: String! = "No longer supported" ) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION +"This directive disables error propagation when a non nullable field returns null for the given operation." +directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION + "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." diff --git a/src/test/groovy/graphql/StarWarsIntrospectionTests.groovy b/src/test/groovy/graphql/StarWarsIntrospectionTests.groovy index 9b374bfbd3..89fc781138 100644 --- a/src/test/groovy/graphql/StarWarsIntrospectionTests.groovy +++ b/src/test/groovy/graphql/StarWarsIntrospectionTests.groovy @@ -430,6 +430,6 @@ class StarWarsIntrospectionTests extends Specification { schemaParts.get('mutationType').size() == 1 schemaParts.get('subscriptionType') == null schemaParts.get('types').size() == 17 - schemaParts.get('directives').size() == 5 + schemaParts.get('directives').size() == 6 } } diff --git a/src/test/groovy/graphql/introspection/IntrospectionWithDirectivesSupportTest.groovy b/src/test/groovy/graphql/introspection/IntrospectionWithDirectivesSupportTest.groovy index 262770eb31..3f38717984 100644 --- a/src/test/groovy/graphql/introspection/IntrospectionWithDirectivesSupportTest.groovy +++ b/src/test/groovy/graphql/introspection/IntrospectionWithDirectivesSupportTest.groovy @@ -91,7 +91,8 @@ class IntrospectionWithDirectivesSupportTest extends Specification { schemaType["directives"] == [ [name: "include"], [name: "skip"], [name: "example"], [name: "secret"], - [name: "noDefault"], [name: "deprecated"], [name: "specifiedBy"], [name: "oneOf"] + [name: "noDefault"], [name: "deprecated"], [name: "specifiedBy"], [name: "oneOf"], + [name: "experimental_disableErrorPropagation"] ] schemaType["appliedDirectives"] == [[name: "example", args: [[name: "argName", value: '"onSchema"']]]] @@ -173,7 +174,9 @@ 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"]] + definedDirectives == [[name: "include"], [name: "skip"], [name: "example"], [name: "deprecated"], [name: "specifiedBy"], [name: "oneOf"], + [name: "experimental_disableErrorPropagation"] + ] } def "can set prefixes onto the Applied types"() { diff --git a/src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy b/src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy index f3eabc1eeb..0d75a9af46 100644 --- a/src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy +++ b/src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy @@ -155,22 +155,22 @@ class GraphQLSchemaTest extends Specification { when: "no additional directives have been specified" def schema = schemaBuilder.build() then: - schema.directives.size() == 5 + schema.directives.size() == 6 when: "clear directives is called" schema = schemaBuilder.clearDirectives().build() then: - schema.directives.size() == 3 // @deprecated and @specifiedBy and @oneOf is ALWAYS added if missing + schema.directives.size() == 4 // @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() == 4 + schema.directives.size() == 5 when: "the schema is transformed, things are copied" schema = schema.transform({ builder -> builder.additionalDirective(Directives.IncludeDirective) }) then: - schema.directives.size() == 5 + schema.directives.size() == 6 } def "clear additional types works as expected"() { diff --git a/src/test/groovy/graphql/schema/diffing/SchemaDiffingTest.groovy b/src/test/groovy/graphql/schema/diffing/SchemaDiffingTest.groovy index efa708dcf4..629d351661 100644 --- a/src/test/groovy/graphql/schema/diffing/SchemaDiffingTest.groovy +++ b/src/test/groovy/graphql/schema/diffing/SchemaDiffingTest.groovy @@ -38,10 +38,10 @@ class SchemaDiffingTest extends Specification { schemaGraph.getVerticesByType(SchemaGraph.ARGUMENT).size() == 9 schemaGraph.getVerticesByType(SchemaGraph.INPUT_FIELD).size() == 0 schemaGraph.getVerticesByType(SchemaGraph.INPUT_OBJECT).size() == 0 - schemaGraph.getVerticesByType(SchemaGraph.DIRECTIVE).size() == 5 + schemaGraph.getVerticesByType(SchemaGraph.DIRECTIVE).size() == 6 schemaGraph.getVerticesByType(SchemaGraph.APPLIED_ARGUMENT).size() == 0 schemaGraph.getVerticesByType(SchemaGraph.APPLIED_DIRECTIVE).size() == 0 - schemaGraph.size() == 93 + schemaGraph.size() == 94 } diff --git a/src/test/groovy/graphql/schema/idl/SchemaGeneratorAppliedDirectiveHelperTest.groovy b/src/test/groovy/graphql/schema/idl/SchemaGeneratorAppliedDirectiveHelperTest.groovy index 824db92e15..944de75893 100644 --- a/src/test/groovy/graphql/schema/idl/SchemaGeneratorAppliedDirectiveHelperTest.groovy +++ b/src/test/groovy/graphql/schema/idl/SchemaGeneratorAppliedDirectiveHelperTest.groovy @@ -56,6 +56,7 @@ class SchemaGeneratorAppliedDirectiveHelperTest extends Specification { "bar", "complex", "deprecated", + "experimental_disableErrorPropagation", "foo", "include", "oneOf", @@ -106,6 +107,7 @@ class SchemaGeneratorAppliedDirectiveHelperTest extends Specification { "bar", "complex", "deprecated", + "experimental_disableErrorPropagation", "foo", "include", "oneOf", diff --git a/src/test/groovy/graphql/schema/idl/SchemaGeneratorTest.groovy b/src/test/groovy/graphql/schema/idl/SchemaGeneratorTest.groovy index fd5d118f42..1eb98cc82e 100644 --- a/src/test/groovy/graphql/schema/idl/SchemaGeneratorTest.groovy +++ b/src/test/groovy/graphql/schema/idl/SchemaGeneratorTest.groovy @@ -2025,7 +2025,7 @@ class SchemaGeneratorTest extends Specification { directives = schema.getDirectives() then: - directives.size() == 8 // built in ones : include / skip and deprecated + directives.size() == 9 // built in ones : include / skip and deprecated def directiveNames = directives.collect { it.name } directiveNames.contains("include") directiveNames.contains("skip") @@ -2040,7 +2040,7 @@ class SchemaGeneratorTest extends Specification { directivesMap = schema.getDirectivesByName() then: - directivesMap.size() == 8 // built in ones + directivesMap.size() == 9 // built in ones directivesMap.containsKey("include") directivesMap.containsKey("skip") directivesMap.containsKey("deprecated") diff --git a/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy b/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy index 0d82101be4..bb311d696a 100644 --- a/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy +++ b/src/test/groovy/graphql/schema/idl/SchemaPrinterTest.groovy @@ -975,6 +975,9 @@ directive @enumTypeDirective on ENUM directive @enumValueDirective on ENUM_VALUE +"This directive disables error propagation when a non nullable field returns null for the given operation." +directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION + directive @fieldDirective1 on FIELD_DEFINITION directive @fieldDirective2(argBool: Boolean, argFloat: Float, argInt: Int, argStr: String) on FIELD_DEFINITION @@ -1147,6 +1150,9 @@ directive @deprecated( reason: String! = "No longer supported" ) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION +"This directive disables error propagation when a non nullable field returns null for the given operation." +directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION + "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." @@ -1245,6 +1251,9 @@ directive @deprecated( directive @example on FIELD_DEFINITION +"This directive disables error propagation when a non nullable field returns null for the given operation." +directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION + "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." @@ -1313,6 +1322,9 @@ directive @deprecated( directive @example on FIELD_DEFINITION +"This directive disables error propagation when a non nullable field returns null for the given operation." +directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION + "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." @@ -1408,6 +1420,9 @@ directive @deprecated( reason: String! = "No longer supported" ) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION +"This directive disables error propagation when a non nullable field returns null for the given operation." +directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION + "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." @@ -1494,6 +1509,9 @@ directive @deprecated( reason: String! = "No longer supported" ) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION +"This directive disables error propagation when a non nullable field returns null for the given operation." +directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION + "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." @@ -1636,6 +1654,9 @@ directive @deprecated( directive @directive1 on SCALAR +"This directive disables error propagation when a non nullable field returns null for the given operation." +directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION + "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." @@ -2170,6 +2191,9 @@ directive @deprecated( reason: String! = "No longer supported" ) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION +"This directive disables error propagation when a non nullable field returns null for the given operation." +directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION + directive @foo on SCHEMA "Directs the executor to include this field or fragment only when the `if` argument is true" @@ -2396,6 +2420,9 @@ directive @include( if: Boolean! ) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT +"This directive disables error propagation when a non nullable field returns null for the given operation." +directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION + "Marks the field, argument, input field or enum value as deprecated" directive @deprecated( "The reason for the deprecation" @@ -2522,6 +2549,9 @@ directive @deprecated( # custom directive 'example' comment 1 directive @example on ENUM_VALUE +"This directive disables error propagation when a non nullable field returns null for the given operation." +directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION + "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." @@ -2758,6 +2788,9 @@ directive @deprecated( " custom directive 'example' description 1" directive @example on ENUM_VALUE +"This directive disables error propagation when a non nullable field returns null for the given operation." +directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION + "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true." @@ -2943,6 +2976,9 @@ directive @deprecated( reason: String! = "No longer supported" ) on FIELD_DEFINITION | ARGUMENT_DEFINITION | ENUM_VALUE | INPUT_FIELD_DEFINITION +"This directive disables error propagation when a non nullable field returns null for the given operation." +directive @experimental_disableErrorPropagation on QUERY | MUTATION | SUBSCRIPTION + "Directs the executor to include this field or fragment only when the `if` argument is true" directive @include( "Included when true."