diff --git a/src/main/java/graphql/execution/DataFetcherResult.java b/src/main/java/graphql/execution/DataFetcherResult.java index 9b78497ed..43214f35d 100644 --- a/src/main/java/graphql/execution/DataFetcherResult.java +++ b/src/main/java/graphql/execution/DataFetcherResult.java @@ -2,6 +2,7 @@ import com.google.common.collect.ImmutableList; import graphql.DeprecatedAt; +import graphql.ExecutionResult; import graphql.GraphQLError; import graphql.Internal; import graphql.PublicApi; @@ -9,20 +10,25 @@ import java.util.ArrayList; import java.util.List; +import java.util.Map; import java.util.function.Consumer; import static graphql.Assert.assertNotNull; /** - * An object that can be returned from a {@link DataFetcher} that contains both data, local context and errors to be relativized and - * added to the final result. This is a useful when your ``DataFetcher`` retrieves data from multiple sources - * or from another GraphQL resource or you want to pass extra context to lower levels. - * + * An object that can be returned from a {@link DataFetcher} that contains both data, local context and errors to be added to the final result. + * This is a useful when your ``DataFetcher`` retrieves data from multiple sources + * or from another GraphQL resource, or you want to pass extra context to lower levels. + *

* This also allows you to pass down new local context objects between parent and child fields. If you return a * {@link #getLocalContext()} value then it will be passed down into any child fields via * {@link graphql.schema.DataFetchingEnvironment#getLocalContext()} * + * You can also have {@link DataFetcher}s contribute to the {@link ExecutionResult#getExtensions()} by returning + * extensions maps that will be merged together via the {@link graphql.extensions.ExtensionsBuilder} and its {@link graphql.extensions.ExtensionsMerger} + * in place. + * * @param The type of the data fetched */ @PublicApi @@ -31,6 +37,7 @@ public class DataFetcherResult { private final T data; private final List errors; private final Object localContext; + private final Map extensions; /** * Creates a data fetcher result @@ -44,13 +51,14 @@ public class DataFetcherResult { @Deprecated @DeprecatedAt("2019-01-11") public DataFetcherResult(T data, List errors) { - this(data, errors, null); + this(data, errors, null, null); } - private DataFetcherResult(T data, List errors, Object localContext) { + private DataFetcherResult(T data, List errors, Object localContext, Map extensions) { this.data = data; this.errors = ImmutableList.copyOf(assertNotNull(errors)); this.localContext = localContext; + this.extensions = extensions; } /** @@ -83,6 +91,22 @@ public Object getLocalContext() { return localContext; } + /** + * A data fetcher result can supply extension values that will be merged into the result + * via the {@link graphql.extensions.ExtensionsBuilder} at the end of the operation. + *

+ * The {@link graphql.extensions.ExtensionsMerger} in place inside the {@link graphql.extensions.ExtensionsBuilder} + * will control how these extension values get merged. + * + * @return a map of extension values to be merged + * + * @see graphql.extensions.ExtensionsBuilder + * @see graphql.extensions.ExtensionsMerger + */ + public Map getExtensions() { + return extensions; + } + /** * This helps you transform the current DataFetcherResult into another one by starting a builder with all * the current values and allows you to transform it how you want. @@ -112,11 +136,13 @@ public static class Builder { private T data; private Object localContext; private final List errors = new ArrayList<>(); + private Map extensions; public Builder(DataFetcherResult existing) { data = existing.getData(); localContext = existing.getLocalContext(); errors.addAll(existing.getErrors()); + extensions = existing.extensions; } public Builder(T data) { @@ -158,8 +184,13 @@ public Builder localContext(Object localContext) { return this; } + public Builder extensions(Map extensions) { + this.extensions = extensions; + return this; + } + public DataFetcherResult build() { - return new DataFetcherResult<>(data, errors, localContext); + return new DataFetcherResult<>(data, errors, localContext, extensions); } } } diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index 9e38a5837..6ddd5bc19 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -18,6 +18,7 @@ import graphql.execution.instrumentation.parameters.InstrumentationFieldCompleteParameters; import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters; import graphql.execution.instrumentation.parameters.InstrumentationFieldParameters; +import graphql.extensions.ExtensionsBuilder; import graphql.introspection.Introspection; import graphql.language.Argument; import graphql.language.Field; @@ -330,6 +331,7 @@ protected FetchedValue unboxPossibleDataFetcherResult(ExecutionContext execution if (result instanceof DataFetcherResult) { DataFetcherResult dataFetcherResult = (DataFetcherResult) result; executionContext.addErrors(dataFetcherResult.getErrors()); + addExtensionsIfPresent(executionContext,dataFetcherResult); Object localContext = dataFetcherResult.getLocalContext(); if (localContext == null) { @@ -351,6 +353,16 @@ protected FetchedValue unboxPossibleDataFetcherResult(ExecutionContext execution } } + private void addExtensionsIfPresent(ExecutionContext executionContext, DataFetcherResult dataFetcherResult) { + Map extensions = dataFetcherResult.getExtensions(); + if (extensions != null) { + ExtensionsBuilder extensionsBuilder = executionContext.getGraphQLContext().get(ExtensionsBuilder.class); + if (extensionsBuilder != null) { + extensionsBuilder.addValues(extensions); + } + } + } + protected CompletableFuture handleFetchingException(ExecutionContext executionContext, DataFetchingEnvironment environment, Throwable e) { diff --git a/src/main/java/graphql/extensions/ExtensionsBuilder.java b/src/main/java/graphql/extensions/ExtensionsBuilder.java index 6c3a982e0..fe37c6474 100644 --- a/src/main/java/graphql/extensions/ExtensionsBuilder.java +++ b/src/main/java/graphql/extensions/ExtensionsBuilder.java @@ -55,6 +55,12 @@ public static ExtensionsBuilder newExtensionsBuilder(ExtensionsMerger extensions return new ExtensionsBuilder(extensionsMerger); } + /** + * @return how many extension changes have been made so far + */ + public int getChangeCount() { + return changes.size(); + } /** * Adds new values into the extension builder @@ -65,7 +71,9 @@ public static ExtensionsBuilder newExtensionsBuilder(ExtensionsMerger extensions */ public ExtensionsBuilder addValues(@NotNull Map newValues) { assertNotNull(newValues); - changes.add(newValues); + if (!newValues.isEmpty()) { + changes.add(newValues); + } return this; } diff --git a/src/test/groovy/graphql/execution/DataFetcherResultTest.groovy b/src/test/groovy/graphql/execution/DataFetcherResultTest.groovy index 3b91a9d62..ca0f1d99e 100644 --- a/src/test/groovy/graphql/execution/DataFetcherResultTest.groovy +++ b/src/test/groovy/graphql/execution/DataFetcherResultTest.groovy @@ -55,14 +55,43 @@ class DataFetcherResultTest extends Specification { !result.hasErrors() } - def "transforming"() { + def "can set extensions"() { + + when: + def dfr = DataFetcherResult.newResult() + .extensions([x: "y"]).build() + + then: + dfr.getExtensions() == [x : "y"] + + when: + dfr = DataFetcherResult.newResult() + .data("x") + .build() + + then: + dfr.getExtensions() == null + + } + + def "transforming works"() { when: def original = DataFetcherResult.newResult().data("hello") - .errors([error1]).localContext("world").build() + .errors([error1]).localContext("world") + .extensions([x: "y"]).build() def result = original.transform({ builder -> builder.error(error2) }) then: result.getData() == "hello" result.getLocalContext() == "world" + result.getExtensions() == [x : "y"] + result.getErrors() == [error1, error2] + + when: + result = result.transform({ builder -> builder.extensions(a : "b") }) + then: + result.getData() == "hello" + result.getLocalContext() == "world" + result.getExtensions() == [a : "b"] result.getErrors() == [error1, error2] } } diff --git a/src/test/groovy/graphql/extensions/ExtensionsBuilderTest.groovy b/src/test/groovy/graphql/extensions/ExtensionsBuilderTest.groovy index 326248cf7..12be824cf 100644 --- a/src/test/groovy/graphql/extensions/ExtensionsBuilderTest.groovy +++ b/src/test/groovy/graphql/extensions/ExtensionsBuilderTest.groovy @@ -1,14 +1,15 @@ package graphql.extensions -import graphql.ExecutionInput import graphql.ExecutionResult import graphql.TestUtil +import graphql.execution.DataFetcherResult import graphql.schema.DataFetcher import graphql.schema.DataFetchingEnvironment import graphql.schema.GraphQLTypeUtil import org.jetbrains.annotations.NotNull import spock.lang.Specification +import static graphql.ExecutionInput.newExecutionInput import static graphql.extensions.ExtensionsBuilder.newExtensionsBuilder import static graphql.schema.idl.RuntimeWiring.newRuntimeWiring import static graphql.schema.idl.TypeRuntimeWiring.newTypeWiring @@ -39,6 +40,27 @@ class ExtensionsBuilderTest extends Specification { extensions == [x: "overwrite3", y: "25", z: "overwriteZ", a: "1"] } + def "wont add empty changes"() { + def builder = newExtensionsBuilder() + when: + builder.addValues([:]) + + then: + builder.getChangeCount() == 0 + + when: + builder.addValues([:]) + + then: + builder.getChangeCount() == 0 + + when: + def extensions = builder.buildExtensions() + then: + extensions.isEmpty() + + } + def "can handle no changes"() { when: def extensions = newExtensionsBuilder() @@ -122,11 +144,12 @@ class ExtensionsBuilderTest extends Specification { """ def extensionsBuilder = newExtensionsBuilder() - extensionsBuilder.addValue("added","explicitly") + extensionsBuilder.addValue("added", "explicitly") - def ei = ExecutionInput.newExecutionInput("query q { name street id }") + def ei = newExecutionInput("query q { name street id }") .graphQLContext({ ctx -> - ctx.put(ExtensionsBuilder.class, extensionsBuilder) }) + ctx.put(ExtensionsBuilder.class, extensionsBuilder) + }) .build() @@ -144,12 +167,53 @@ class ExtensionsBuilderTest extends Specification { er.errors.isEmpty() er.extensions == [ "added": "explicitly", - common: [ + common : [ name : "String!", street: "String", id : "ID!", ], // we break them out so we have common and not common entries + name : "String!", + street : "String", + id : "ID!", + ] + } + + + def "integration test that shows it working when they use DataFetcherResult and defaulted values"() { + def sdl = """ + type Query { + name : String! + street : String + id : ID! + } + """ + + DataFetcher dfrDF = new DataFetcher() { + @Override + Object get(DataFetchingEnvironment env) throws Exception { + def fieldMap = [:] + fieldMap.put(env.getFieldDefinition().name, GraphQLTypeUtil.simplePrint(env.getFieldDefinition().type)) + return DataFetcherResult.newResult().data("ignored").extensions(fieldMap).build() + } + } + + def graphQL = TestUtil.graphQL(sdl, newRuntimeWiring() + .type(newTypeWiring("Query").dataFetchers([ + name : dfrDF, + street: dfrDF, + id : dfrDF, + ]))) + .build() + + when: + def ei = newExecutionInput("query q { name street id }") + .build() + + def er = graphQL.execute(ei) + then: + er.errors.isEmpty() + er.extensions == [ name : "String!", street: "String", id : "ID!", @@ -165,7 +229,7 @@ class ExtensionsBuilderTest extends Specification { } """ - def ei = ExecutionInput.newExecutionInput("query q { name street id }") + def ei = newExecutionInput("query q { name street id }") .build() @@ -203,8 +267,8 @@ class ExtensionsBuilderTest extends Specification { } """ - def ei = ExecutionInput.newExecutionInput("query q { name street id }") - .root(["name" : "Brad", "id" :1234]) + def ei = newExecutionInput("query q { name street id }") + .root(["name": "Brad", "id": 1234]) .build()