diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index 563d4d5279..614b75e703 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -105,7 +105,7 @@ *

* The first phase (data fetching) is handled by the method {@link #fetchField(ExecutionContext, ExecutionStrategyParameters)} *

- * The second phase (value completion) is handled by the methods {@link #completeField(ExecutionContext, ExecutionStrategyParameters, FetchedValue)} + * The second phase (value completion) is handled by the methods {@link #completeField(ExecutionContext, ExecutionStrategyParameters, Object)} * and the other "completeXXX" methods. *

* The order of fields fetching and completion is up to the execution strategy. As the graphql specification @@ -358,7 +358,7 @@ protected Object resolveFieldWithInfo(ExecutionContext executionContext, Executi Object fetchedValueObj = fetchField(executionContext, parameters); if (fetchedValueObj instanceof CompletableFuture) { - CompletableFuture fetchFieldFuture = (CompletableFuture) fetchedValueObj; + CompletableFuture fetchFieldFuture = (CompletableFuture) fetchedValueObj; CompletableFuture result = fetchFieldFuture.thenApply((fetchedValue) -> completeField(fieldDef, executionContext, parameters, fetchedValue)); @@ -367,10 +367,9 @@ protected Object resolveFieldWithInfo(ExecutionContext executionContext, Executi return result; } else { try { - FetchedValue fetchedValue = (FetchedValue) fetchedValueObj; - FieldValueInfo fieldValueInfo = completeField(fieldDef, executionContext, parameters, fetchedValue); + FieldValueInfo fieldValueInfo = completeField(fieldDef, executionContext, parameters, fetchedValueObj); fieldCtx.onDispatched(); - fieldCtx.onCompleted(fetchedValue.getFetchedValue(), null); + fieldCtx.onCompleted(FetchedValue.getFetchedValue(fetchedValueObj), null); return fieldValueInfo; } catch (Exception e) { return Async.exceptionallyCompletedFuture(e); @@ -383,16 +382,16 @@ protected Object resolveFieldWithInfo(ExecutionContext executionContext, Executi * {@link GraphQLFieldDefinition}. *

* Graphql fragments mean that for any give logical field can have one or more {@link Field} values associated with it - * in the query, hence the fieldList. However the first entry is representative of the field for most purposes. + * in the query, hence the fieldList. However, the first entry is representative of the field for most purposes. * * @param executionContext contains the top level execution parameters * @param parameters contains the parameters holding the fields to be executed and source object * - * @return a promise to a {@link FetchedValue} object or the {@link FetchedValue} itself + * @return a promise to a value object or the value itself. The value maybe a raw object OR a {@link FetchedValue} * - * @throws NonNullableFieldWasNullException in the future if a non null field resolves to a null value + * @throws NonNullableFieldWasNullException in the future if a non-null field resolves to a null value */ - @DuckTyped(shape = "CompletableFuture | FetchedValue") + @DuckTyped(shape = "CompletableFuture | ") protected Object fetchField(ExecutionContext executionContext, ExecutionStrategyParameters parameters) { MergedField field = parameters.getField(); GraphQLObjectType parentType = (GraphQLObjectType) parameters.getExecutionStepInfo().getUnwrappedNonNullType(); @@ -400,12 +399,12 @@ protected Object fetchField(ExecutionContext executionContext, ExecutionStrategy return fetchField(fieldDef, executionContext, parameters); } - @DuckTyped(shape = "CompletableFuture | FetchedValue") + @DuckTyped(shape = "CompletableFuture | ") private Object fetchField(GraphQLFieldDefinition fieldDef, ExecutionContext executionContext, ExecutionStrategyParameters parameters) { executionContext.throwIfCancelled(); if (incrementAndCheckMaxNodesExceeded(executionContext)) { - return new FetchedValue(null, Collections.emptyList(), null); + return null; } MergedField field = parameters.getField(); @@ -486,9 +485,8 @@ private Object fetchField(GraphQLFieldDefinition fieldDef, ExecutionContext exec } }); CompletableFuture rawResultCF = engineRunningState.compose(handleCF, Function.identity()); - CompletableFuture fetchedValueCF = rawResultCF + return rawResultCF .thenApply(result -> unboxPossibleDataFetcherResult(executionContext, parameters, result)); - return fetchedValueCF; } else { fetchCtx.onCompleted(fetchedObject, null); return unboxPossibleDataFetcherResult(executionContext, parameters, fetchedObject); @@ -520,9 +518,21 @@ protected Supplier getNormalizedField(ExecutionContex return () -> normalizedQuery.get().getNormalizedField(parameters.getField(), executionStepInfo.get().getObjectType(), executionStepInfo.get().getPath()); } - protected FetchedValue unboxPossibleDataFetcherResult(ExecutionContext executionContext, - ExecutionStrategyParameters parameters, - Object result) { + /** + * If the data fetching returned a {@link DataFetcherResult} then it can contain errors and new local context + * and hence it gets turned into a {@link FetchedValue} but otherwise this method returns the unboxed + * value without the wrapper. This means its more efficient overall by default. + * + * @param executionContext the execution context in play + * @param parameters the parameters in play + * @param result the fetched raw object + * + * @return an unboxed value which can be a FetchedValue or an Object + */ + @DuckTyped(shape = "FetchedValue | Object") + protected Object unboxPossibleDataFetcherResult(ExecutionContext executionContext, + ExecutionStrategyParameters parameters, + Object result) { if (result instanceof DataFetcherResult) { DataFetcherResult dataFetcherResult = (DataFetcherResult) result; @@ -538,8 +548,7 @@ protected FetchedValue unboxPossibleDataFetcherResult(ExecutionContext execution Object unBoxedValue = executionContext.getValueUnboxer().unbox(dataFetcherResult.getData()); return new FetchedValue(unBoxedValue, dataFetcherResult.getErrors(), localContext); } else { - Object unBoxedValue = executionContext.getValueUnboxer().unbox(result); - return new FetchedValue(unBoxedValue, ImmutableList.of(), parameters.getLocalContext()); + return executionContext.getValueUnboxer().unbox(result); } } @@ -577,29 +586,32 @@ protected CompletableFuture handleFetchingException( private CompletableFuture asyncHandleException(DataFetcherExceptionHandler handler, DataFetcherExceptionHandlerParameters handlerParameters) { //noinspection unchecked return handler.handleException(handlerParameters).thenApply( - handlerResult -> (T) DataFetcherResult.newResult().errors(handlerResult.getErrors()).build() + handlerResult -> (T) DataFetcherResult.newResult().errors(handlerResult.getErrors()).build() ); } /** * Called to complete a field based on the type of the field. *

- * If the field is a scalar type, then it will be coerced and returned. However if the field type is an complex object type, then + * If the field is a scalar type, then it will be coerced and returned. However, if the field type is an complex object type, then * the execution strategy will be called recursively again to execute the fields of that type before returning. *

* Graphql fragments mean that for any give logical field can have one or more {@link Field} values associated with it - * in the query, hence the fieldList. However the first entry is representative of the field for most purposes. + * in the query, hence the fieldList. However, the first entry is representative of the field for most purposes. * * @param executionContext contains the top level execution parameters * @param parameters contains the parameters holding the fields to be executed and source object - * @param fetchedValue the fetched raw value + * @param fetchedValue the fetched raw value or perhaps a {@link FetchedValue} wrapper of that value * * @return a {@link FieldValueInfo} * * @throws NonNullableFieldWasNullException in the {@link FieldValueInfo#getFieldValueFuture()} future * if a nonnull field resolves to a null value */ - protected FieldValueInfo completeField(ExecutionContext executionContext, ExecutionStrategyParameters parameters, FetchedValue fetchedValue) { + protected FieldValueInfo completeField(ExecutionContext executionContext, + ExecutionStrategyParameters parameters, + @DuckTyped(shape = "Object | FetchedValue") + Object fetchedValue) { executionContext.throwIfCancelled(); Field field = parameters.getField().getSingleField(); @@ -608,7 +620,7 @@ protected FieldValueInfo completeField(ExecutionContext executionContext, Execut return completeField(fieldDef, executionContext, parameters, fetchedValue); } - private FieldValueInfo completeField(GraphQLFieldDefinition fieldDef, ExecutionContext executionContext, ExecutionStrategyParameters parameters, FetchedValue fetchedValue) { + private FieldValueInfo completeField(GraphQLFieldDefinition fieldDef, ExecutionContext executionContext, ExecutionStrategyParameters parameters, Object fetchedValue) { GraphQLObjectType parentType = (GraphQLObjectType) parameters.getExecutionStepInfo().getUnwrappedNonNullType(); ExecutionStepInfo executionStepInfo = createExecutionStepInfo(executionContext, parameters, fieldDef, parentType); @@ -618,9 +630,11 @@ private FieldValueInfo completeField(GraphQLFieldDefinition fieldDef, ExecutionC instrumentationParams, executionContext.getInstrumentationState() )); - ExecutionStrategyParameters newParameters = parameters.transform(executionStepInfo, - fetchedValue.getLocalContext(), - fetchedValue.getFetchedValue()); + ExecutionStrategyParameters newParameters = parameters.transform( + executionStepInfo, + FetchedValue.getLocalContext(fetchedValue, parameters.getLocalContext()), + FetchedValue.getFetchedValue(fetchedValue) + ); FieldValueInfo fieldValueInfo = completeValue(executionContext, newParameters); ctxCompleteField.onDispatched(); @@ -777,12 +791,14 @@ protected FieldValueInfo completeValueForList(ExecutionContext executionContext, ExecutionStepInfo stepInfoForListElement = executionStepInfoFactory.newExecutionStepInfoForListElement(executionStepInfo, indexedPath); - FetchedValue value = unboxPossibleDataFetcherResult(executionContext, parameters, item); + Object fetchedValue = unboxPossibleDataFetcherResult(executionContext, parameters, item); - ExecutionStrategyParameters newParameters = parameters.transform(stepInfoForListElement, + ExecutionStrategyParameters newParameters = parameters.transform( + stepInfoForListElement, indexedPath, - value.getLocalContext(), - value.getFetchedValue()); + FetchedValue.getLocalContext(fetchedValue, parameters.getLocalContext()), + FetchedValue.getFetchedValue(fetchedValue) + ); fieldValueInfos.add(completeValue(executionContext, newParameters)); index++; diff --git a/src/main/java/graphql/execution/FetchedValue.java b/src/main/java/graphql/execution/FetchedValue.java index 8ebac38ced..fe56fa0a10 100644 --- a/src/main/java/graphql/execution/FetchedValue.java +++ b/src/main/java/graphql/execution/FetchedValue.java @@ -8,7 +8,7 @@ import java.util.List; /** - * Note: This is returned by {@link InstrumentationFieldCompleteParameters#getFetchedValue()} + * Note: This MAY be returned by {@link InstrumentationFieldCompleteParameters#getFetchedObject()} * and therefore part of the public despite never used in a method signature. */ @PublicApi @@ -17,6 +17,39 @@ public class FetchedValue { private final Object localContext; private final ImmutableList errors; + /** + * This allows you to get to the underlying fetched value depending on whether the source + * value is a {@link FetchedValue} or not + * + * @param sourceValue the source value in play + * + * @return the {@link FetchedValue#getFetchedValue()} if its wrapped otherwise the source value itself + */ + public static Object getFetchedValue(Object sourceValue) { + if (sourceValue instanceof FetchedValue) { + return ((FetchedValue) sourceValue).fetchedValue; + } else { + return sourceValue; + } + } + + /** + * This allows you to get to the local context depending on whether the source + * value is a {@link FetchedValue} or not + * + * @param sourceValue the source value in play + * @param defaultLocalContext the default local context to use + * + * @return the {@link FetchedValue#getFetchedValue()} if its wrapped otherwise the default local context + */ + public static Object getLocalContext(Object sourceValue, Object defaultLocalContext) { + if (sourceValue instanceof FetchedValue) { + return ((FetchedValue) sourceValue).localContext; + } else { + return defaultLocalContext; + } + } + public FetchedValue(Object fetchedValue, List errors, Object localContext) { this.fetchedValue = fetchedValue; this.errors = ImmutableList.copyOf(errors); diff --git a/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java b/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java index dea9172fc0..45e2192248 100644 --- a/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java +++ b/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java @@ -109,9 +109,9 @@ private boolean keepOrdered(GraphQLContext graphQLContext) { private CompletableFuture> createSourceEventStream(ExecutionContext executionContext, ExecutionStrategyParameters parameters) { ExecutionStrategyParameters newParameters = firstFieldOfSubscriptionSelection(executionContext, parameters, false); - CompletableFuture fieldFetched = Async.toCompletableFuture(fetchField(executionContext, newParameters)); + CompletableFuture fieldFetched = Async.toCompletableFuture(fetchField(executionContext, newParameters)); return fieldFetched.thenApply(fetchedValue -> { - Object publisher = fetchedValue.getFetchedValue(); + Object publisher = FetchedValue.getFetchedValue(fetchedValue); return mkReactivePublisher(publisher); }); } @@ -168,7 +168,7 @@ private CompletableFuture executeSubscriptionEvent(ExecutionCon i13nFieldParameters, executionContext.getInstrumentationState() )); - FetchedValue fetchedValue = unboxPossibleDataFetcherResult(newExecutionContext, newParameters, eventPayload); + Object fetchedValue = unboxPossibleDataFetcherResult(newExecutionContext, newParameters, eventPayload); FieldValueInfo fieldValueInfo = completeField(newExecutionContext, newParameters, fetchedValue); executionContext.getDataLoaderDispatcherStrategy().newSubscriptionExecution(fieldValueInfo, newParameters.getDeferredCallContext()); CompletableFuture overallResult = fieldValueInfo diff --git a/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldCompleteParameters.java b/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldCompleteParameters.java index bd89baa15a..ac7e1b27e5 100644 --- a/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldCompleteParameters.java +++ b/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldCompleteParameters.java @@ -48,7 +48,13 @@ public ExecutionStepInfo getExecutionStepInfo() { return executionStepInfo.get(); } - public Object getFetchedValue() { + /** + * This returns the object that was fetched, ready to be completed as a value. This can sometimes be a {@link graphql.execution.FetchedValue} object + * but most often it's a simple POJO. + * + * @return the object was fetched, ready to be completed as a value. + */ + public Object getFetchedObject() { return fetchedValue; } diff --git a/src/test/groovy/graphql/execution/BreadthFirstExecutionTestStrategy.java b/src/test/groovy/graphql/execution/BreadthFirstExecutionTestStrategy.java index 167e58a3b9..c110f04947 100644 --- a/src/test/groovy/graphql/execution/BreadthFirstExecutionTestStrategy.java +++ b/src/test/groovy/graphql/execution/BreadthFirstExecutionTestStrategy.java @@ -22,11 +22,11 @@ public BreadthFirstExecutionTestStrategy() { public CompletableFuture execute(ExecutionContext executionContext, ExecutionStrategyParameters parameters) throws NonNullableFieldWasNullException { MergedSelectionSet fields = parameters.getFields(); - Map fetchedValues = new LinkedHashMap<>(); + Map fetchedValues = new LinkedHashMap<>(); // first fetch every value for (String fieldName : fields.keySet()) { - FetchedValue fetchedValue = fetchField(executionContext, parameters, fields, fieldName); + Object fetchedValue = fetchField(executionContext, parameters, fields, fieldName); fetchedValues.put(fieldName, fetchedValue); } @@ -34,7 +34,7 @@ public CompletableFuture execute(ExecutionContext executionCont Map results = new LinkedHashMap<>(); for (String fieldName : fetchedValues.keySet()) { MergedField currentField = fields.getSubField(fieldName); - FetchedValue fetchedValue = fetchedValues.get(fieldName); + Object fetchedValue = fetchedValues.get(fieldName); ResultPath fieldPath = parameters.getPath().segment(fieldName); ExecutionStrategyParameters newParameters = parameters @@ -51,17 +51,17 @@ public CompletableFuture execute(ExecutionContext executionCont return CompletableFuture.completedFuture(new ExecutionResultImpl(results, executionContext.getErrors())); } - private FetchedValue fetchField(ExecutionContext executionContext, ExecutionStrategyParameters parameters, MergedSelectionSet fields, String fieldName) { + private Object fetchField(ExecutionContext executionContext, ExecutionStrategyParameters parameters, MergedSelectionSet fields, String fieldName) { MergedField currentField = fields.getSubField(fieldName); ResultPath fieldPath = parameters.getPath().segment(fieldName); ExecutionStrategyParameters newParameters = parameters .transform(builder -> builder.field(currentField).path(fieldPath)); - return Async.toCompletableFuture(fetchField(executionContext, newParameters)).join(); + return Async.toCompletableFuture(fetchField(executionContext, newParameters)).join(); } - private void completeValue(ExecutionContext executionContext, Map results, String fieldName, FetchedValue fetchedValue, ExecutionStrategyParameters newParameters) { + private void completeValue(ExecutionContext executionContext, Map results, String fieldName, Object fetchedValue, ExecutionStrategyParameters newParameters) { Object resolvedResult = completeField(executionContext, newParameters, fetchedValue).getFieldValueFuture().join(); results.put(fieldName, resolvedResult); } diff --git a/src/test/groovy/graphql/execution/BreadthFirstTestStrategy.java b/src/test/groovy/graphql/execution/BreadthFirstTestStrategy.java index 653e035df1..54b5e57898 100644 --- a/src/test/groovy/graphql/execution/BreadthFirstTestStrategy.java +++ b/src/test/groovy/graphql/execution/BreadthFirstTestStrategy.java @@ -27,33 +27,33 @@ public BreadthFirstTestStrategy() { @Override public CompletableFuture execute(ExecutionContext executionContext, ExecutionStrategyParameters parameters) throws NonNullableFieldWasNullException { - Map fetchedValues = fetchFields(executionContext, parameters); + Map fetchedValues = fetchFields(executionContext, parameters); return completeFields(executionContext, parameters, fetchedValues); } - private Map fetchFields(ExecutionContext executionContext, ExecutionStrategyParameters parameters) { + private Map fetchFields(ExecutionContext executionContext, ExecutionStrategyParameters parameters) { MergedSelectionSet fields = parameters.getFields(); - Map> fetchFutures = new LinkedHashMap<>(); + Map> fetchFutures = new LinkedHashMap<>(); // first fetch every value for (String fieldName : fields.keySet()) { ExecutionStrategyParameters newParameters = newParameters(parameters, fields, fieldName); - CompletableFuture fetchFuture = Async.toCompletableFuture(fetchField(executionContext, newParameters)); + CompletableFuture fetchFuture = Async.toCompletableFuture(fetchField(executionContext, newParameters)); fetchFutures.put(fieldName, fetchFuture); } // now wait for all fetches to finish together via this join allOf(fetchFutures.values()).join(); - Map fetchedValues = new LinkedHashMap<>(); + Map fetchedValues = new LinkedHashMap<>(); fetchFutures.forEach((k, v) -> fetchedValues.put(k, v.join())); return fetchedValues; } - private CompletableFuture completeFields(ExecutionContext executionContext, ExecutionStrategyParameters parameters, Map fetchedValues) { + private CompletableFuture completeFields(ExecutionContext executionContext, ExecutionStrategyParameters parameters, Map fetchedValues) { MergedSelectionSet fields = parameters.getFields(); // then for every fetched value, complete it, breath first @@ -61,7 +61,7 @@ private CompletableFuture completeFields(ExecutionContext execu for (String fieldName : fetchedValues.keySet()) { ExecutionStrategyParameters newParameters = newParameters(parameters, fields, fieldName); - FetchedValue fetchedValue = fetchedValues.get(fieldName); + Object fetchedValue = fetchedValues.get(fieldName); try { Object resolvedResult = completeField(executionContext, newParameters, fetchedValue).getFieldValueFuture().join(); results.put(fieldName, resolvedResult); @@ -83,7 +83,7 @@ private ExecutionStrategyParameters newParameters(ExecutionStrategyParameters pa public static CompletableFuture> allOf(final Collection> futures) { - CompletableFuture[] cfs = futures.toArray(new CompletableFuture[futures.size()]); + CompletableFuture[] cfs = futures.toArray(new CompletableFuture[0]); return CompletableFuture.allOf(cfs) .thenApply(vd -> futures.stream() diff --git a/src/test/groovy/graphql/execution/ExecutionStrategyTest.groovy b/src/test/groovy/graphql/execution/ExecutionStrategyTest.groovy index bb6db0bd0e..81df0165af 100644 --- a/src/test/groovy/graphql/execution/ExecutionStrategyTest.groovy +++ b/src/test/groovy/graphql/execution/ExecutionStrategyTest.groovy @@ -28,6 +28,7 @@ import graphql.schema.FieldCoordinates import graphql.schema.GraphQLCodeRegistry import graphql.schema.GraphQLEnumType import graphql.schema.GraphQLFieldDefinition +import graphql.schema.GraphQLObjectType import graphql.schema.GraphQLScalarType import graphql.schema.GraphQLSchema import graphql.schema.LightDataFetcher @@ -667,8 +668,8 @@ class ExecutionStrategyTest extends Specification { @Override @Override InstrumentationContext beginFieldCompletion(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) { - if (parameters.fetchedValue instanceof FetchedValue) { - FetchedValue value = (FetchedValue) parameters.fetchedValue + if (parameters.getFetchedObject() instanceof FetchedValue) { + FetchedValue value = (FetchedValue) parameters.getFetchedObject() fetchedValues.put(parameters.field.name, value) } return super.beginFieldCompletion(parameters, state) @@ -687,7 +688,7 @@ class ExecutionStrategyTest extends Specification { overridingStrategy.resolveFieldWithInfo(instrumentedExecutionContext, params) then: - FetchedValue fetchedValue = instrumentation.fetchedValues.get("someField") + Object fetchedValue = instrumentation.fetchedValues.get("someField") fetchedValue != null fetchedValue.errors.size() == 1 def exceptionWhileDataFetching = fetchedValue.errors[0] as ExceptionWhileDataFetching @@ -860,26 +861,41 @@ class ExecutionStrategyTest extends Specification { def "#1558 forward localContext on nonBoxed return from DataFetcher"() { given: + def capturedLocalContext = "startingValue" + executionStrategy = new ExecutionStrategy(dataFetcherExceptionHandler) { + @Override + CompletableFuture execute(ExecutionContext executionContext, ExecutionStrategyParameters parameters) throws NonNullableFieldWasNullException { + return null + } + + @Override + protected FieldValueInfo completeValue(ExecutionContext executionContext, ExecutionStrategyParameters parameters) throws NonNullableFieldWasNullException { + // shows we set the local context if the value is non boxed + capturedLocalContext = parameters.getLocalContext() + return super.completeValue(executionContext, parameters) + } + } + ExecutionContext executionContext = buildContext() - def fieldType = list(Scalars.GraphQLInt) - def fldDef = newFieldDefinition().name("test").type(fieldType).build() + def fieldType = StarWarsSchema.droidType + def fldDef = StarWarsSchema.droidType.getFieldDefinition("name") def executionStepInfo = ExecutionStepInfo.newExecutionStepInfo().type(fieldType).fieldDefinition(fldDef).build() - def field = Field.newField("parent").sourceLocation(new SourceLocation(5, 10)).build() + def field = Field.newField("name").sourceLocation(new SourceLocation(5, 10)).build() def localContext = "localContext" def parameters = newParameters() - .path(ResultPath.fromList(["parent"])) + .path(ResultPath.fromList(["name"])) .localContext(localContext) .field(mergedField(field)) - .fields(mergedSelectionSet(["parent": [mergedField(field)]])) + .fields(mergedSelectionSet(["name": [mergedField(field)]])) .executionStepInfo(executionStepInfo) .nonNullFieldValidator(new NonNullableFieldValidator(executionContext)) .build() when: - def fetchedValue = executionStrategy.unboxPossibleDataFetcherResult(executionContext, parameters, new Object()) + executionStrategy.completeField(executionContext, parameters, new Object()) then: - fetchedValue.localContext == localContext + capturedLocalContext == localContext } def "#820 processes DataFetcherResult just message"() {