diff --git a/src/main/java/graphql/GraphQL.java b/src/main/java/graphql/GraphQL.java index c43e6dec29..f3d678f1e5 100644 --- a/src/main/java/graphql/GraphQL.java +++ b/src/main/java/graphql/GraphQL.java @@ -420,17 +420,16 @@ public CompletableFuture executeAsync(ExecutionInput executionI InstrumentationExecutionParameters inputInstrumentationParameters = new InstrumentationExecutionParameters(executionInputWithId, this.graphQLSchema, instrumentationState); ExecutionInput instrumentedExecutionInput = instrumentation.instrumentExecutionInput(executionInputWithId, inputInstrumentationParameters, instrumentationState); - CompletableFuture beginExecutionCF = new CompletableFuture<>(); InstrumentationExecutionParameters instrumentationParameters = new InstrumentationExecutionParameters(instrumentedExecutionInput, this.graphQLSchema, instrumentationState); InstrumentationContext executionInstrumentation = nonNullCtx(instrumentation.beginExecution(instrumentationParameters, instrumentationState)); - executionInstrumentation.onDispatched(beginExecutionCF); + executionInstrumentation.onDispatched(); GraphQLSchema graphQLSchema = instrumentation.instrumentSchema(this.graphQLSchema, instrumentationParameters, instrumentationState); CompletableFuture executionResult = parseValidateAndExecute(instrumentedExecutionInput, graphQLSchema, instrumentationState); // // finish up instrumentation - executionResult = executionResult.whenComplete(completeInstrumentationCtxCF(executionInstrumentation, beginExecutionCF)); + executionResult = executionResult.whenComplete(completeInstrumentationCtxCF(executionInstrumentation)); // // allow instrumentation to tweak the result executionResult = executionResult.thenCompose(result -> instrumentation.instrumentExecutionResult(result, instrumentationParameters, instrumentationState)); @@ -507,15 +506,13 @@ private PreparsedDocumentEntry parseAndValidate(AtomicReference private ParseAndValidateResult parse(ExecutionInput executionInput, GraphQLSchema graphQLSchema, InstrumentationState instrumentationState) { InstrumentationExecutionParameters parameters = new InstrumentationExecutionParameters(executionInput, graphQLSchema, instrumentationState); InstrumentationContext parseInstrumentationCtx = nonNullCtx(instrumentation.beginParse(parameters, instrumentationState)); - CompletableFuture documentCF = new CompletableFuture<>(); - parseInstrumentationCtx.onDispatched(documentCF); + parseInstrumentationCtx.onDispatched(); ParseAndValidateResult parseResult = ParseAndValidate.parse(executionInput); if (parseResult.isFailure()) { parseInstrumentationCtx.onCompleted(null, parseResult.getSyntaxException()); return parseResult; } else { - documentCF.complete(parseResult.getDocument()); parseInstrumentationCtx.onCompleted(parseResult.getDocument(), null); DocumentAndVariables documentAndVariables = parseResult.getDocumentAndVariables(); @@ -527,15 +524,13 @@ private ParseAndValidateResult parse(ExecutionInput executionInput, GraphQLSchem private List validate(ExecutionInput executionInput, Document document, GraphQLSchema graphQLSchema, InstrumentationState instrumentationState) { InstrumentationContext> validationCtx = nonNullCtx(instrumentation.beginValidation(new InstrumentationValidationParameters(executionInput, document, graphQLSchema, instrumentationState), instrumentationState)); - CompletableFuture> cf = new CompletableFuture<>(); - validationCtx.onDispatched(cf); + validationCtx.onDispatched(); Predicate> validationRulePredicate = executionInput.getGraphQLContext().getOrDefault(ParseAndValidate.INTERNAL_VALIDATION_PREDICATE_HINT, r -> true); Locale locale = executionInput.getLocale() != null ? executionInput.getLocale() : Locale.getDefault(); List validationErrors = ParseAndValidate.validate(graphQLSchema, document, validationRulePredicate, locale); validationCtx.onCompleted(validationErrors, null); - cf.complete(validationErrors); return validationErrors; } diff --git a/src/main/java/graphql/execution/AsyncExecutionStrategy.java b/src/main/java/graphql/execution/AsyncExecutionStrategy.java index eee639bb09..3e640c9ace 100644 --- a/src/main/java/graphql/execution/AsyncExecutionStrategy.java +++ b/src/main/java/graphql/execution/AsyncExecutionStrategy.java @@ -50,7 +50,7 @@ public CompletableFuture execute(ExecutionContext executionCont Async.CombinedBuilder futures = getAsyncFieldValueInfo(executionContext, parameters, deferredExecutionSupport); CompletableFuture overallResult = new CompletableFuture<>(); - executionStrategyCtx.onDispatched(overallResult); + executionStrategyCtx.onDispatched(); futures.await().whenComplete((completeValueInfos, throwable) -> { List fieldsExecutedOnInitialResult = deferredExecutionSupport.getNonDeferredFieldNames(fieldNames); diff --git a/src/main/java/graphql/execution/AsyncSerialExecutionStrategy.java b/src/main/java/graphql/execution/AsyncSerialExecutionStrategy.java index 0a6c7c51db..e5772f052d 100644 --- a/src/main/java/graphql/execution/AsyncSerialExecutionStrategy.java +++ b/src/main/java/graphql/execution/AsyncSerialExecutionStrategy.java @@ -49,7 +49,7 @@ public CompletableFuture execute(ExecutionContext executionCont }); CompletableFuture overallResult = new CompletableFuture<>(); - executionStrategyCtx.onDispatched(overallResult); + executionStrategyCtx.onDispatched(); resultsFuture.whenComplete(handleResults(executionContext, fieldNames, overallResult)); overallResult.whenComplete(executionStrategyCtx::onCompleted); diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 3343b254d1..3a30bc1062 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -134,7 +134,7 @@ private CompletableFuture executeOperation(ExecutionContext exe ExecutionResult executionResult = new ExecutionResultImpl(Collections.singletonList((GraphQLError) rte)); CompletableFuture resultCompletableFuture = completedFuture(executionResult); - executeOperationCtx.onDispatched(resultCompletableFuture); + executeOperationCtx.onDispatched(); executeOperationCtx.onCompleted(executionResult, rte); return resultCompletableFuture; } @@ -189,7 +189,7 @@ private CompletableFuture executeOperation(ExecutionContext exe } // note this happens NOW - not when the result completes - executeOperationCtx.onDispatched(result); + executeOperationCtx.onDispatched(); // fill out extensions if we have them result = result.thenApply(er -> mergeExtensionsBuilderIfPresent(er, graphQLContext)); diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index 0d79d326b8..615699583c 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -211,7 +211,7 @@ protected CompletableFuture> executeObject(ExecutionContext Async.CombinedBuilder resolvedFieldFutures = getAsyncFieldValueInfo(executionContext, parameters, deferredExecutionSupport); CompletableFuture> overallResult = new CompletableFuture<>(); - resolveObjectCtx.onDispatched(overallResult); + resolveObjectCtx.onDispatched(); resolvedFieldFutures.await().whenComplete((completeValueInfos, throwable) -> { List fieldsExecutedOnInitialResult = deferredExecutionSupport.getNonDeferredFieldNames(fieldNames); @@ -354,7 +354,7 @@ protected CompletableFuture resolveFieldWithInfo(ExecutionContex CompletableFuture fieldValueFuture = result.thenCompose(FieldValueInfo::getFieldValueFuture); - fieldCtx.onDispatched(fieldValueFuture); + fieldCtx.onDispatched(); fieldValueFuture.whenComplete(fieldCtx::onCompleted); return result; } @@ -425,7 +425,7 @@ protected CompletableFuture fetchField(ExecutionContext executionC dataFetcher = executionContext.getDataLoaderDispatcherStrategy().modifyDataFetcher(dataFetcher); CompletableFuture fetchedValue = invokeDataFetcher(executionContext, parameters, fieldDef, dataFetchingEnvironment, dataFetcher); executionContext.getDataLoaderDispatcherStrategy().fieldFetched(executionContext, parameters, dataFetcher, fetchedValue); - fetchCtx.onDispatched(fetchedValue); + fetchCtx.onDispatched(); return fetchedValue .handle((result, exception) -> { fetchCtx.onCompleted(result, exception); @@ -575,7 +575,7 @@ protected FieldValueInfo completeField(ExecutionContext executionContext, Execut FieldValueInfo fieldValueInfo = completeValue(executionContext, newParameters); CompletableFuture executionResultFuture = fieldValueInfo.getFieldValueFuture(); - ctxCompleteField.onDispatched(executionResultFuture); + ctxCompleteField.onDispatched(); executionResultFuture.whenComplete(ctxCompleteField::onCompleted); return fieldValueInfo; } @@ -729,7 +729,7 @@ protected FieldValueInfo completeValueForList(ExecutionContext executionContext, CompletableFuture> resultsFuture = Async.each(fieldValueInfos, FieldValueInfo::getFieldValueFuture); CompletableFuture overallResult = new CompletableFuture<>(); - completeListCtx.onDispatched(overallResult); + completeListCtx.onDispatched(); overallResult.whenComplete(completeListCtx::onCompleted); resultsFuture.whenComplete((results, exception) -> { diff --git a/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java b/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java index ca0c7f98ae..f735b0dc6a 100644 --- a/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java +++ b/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java @@ -69,7 +69,7 @@ public CompletableFuture execute(ExecutionContext executionCont }); // dispatched the subscription query - executionStrategyCtx.onDispatched(overallResult); + executionStrategyCtx.onDispatched(); overallResult.whenComplete(executionStrategyCtx::onCompleted); return overallResult; @@ -140,7 +140,7 @@ private CompletableFuture executeSubscriptionEvent(ExecutionCon .thenApply(executionResult -> wrapWithRootFieldName(newParameters, executionResult)); // dispatch instrumentation so they can know about each subscription event - subscribedFieldCtx.onDispatched(overallResult); + subscribedFieldCtx.onDispatched(); overallResult.whenComplete(subscribedFieldCtx::onCompleted); // allow them to instrument each ER should they want to diff --git a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java index d0a59dcebe..81581a7522 100644 --- a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java @@ -397,8 +397,8 @@ private static class ChainedInstrumentationContext implements Instrumentation } @Override - public void onDispatched(CompletableFuture result) { - contexts.forEach(context -> context.onDispatched(result)); + public void onDispatched() { + contexts.forEach(InstrumentationContext::onDispatched); } @Override @@ -416,8 +416,8 @@ private static class ChainedExecutionStrategyInstrumentationContext implements E } @Override - public void onDispatched(CompletableFuture result) { - contexts.forEach(context -> context.onDispatched(result)); + public void onDispatched() { + contexts.forEach(InstrumentationContext::onDispatched); } @Override @@ -445,8 +445,8 @@ private static class ChainedExecuteObjectInstrumentationContext implements Execu } @Override - public void onDispatched(CompletableFuture> result) { - contexts.forEach(context -> context.onDispatched(result)); + public void onDispatched() { + contexts.forEach(InstrumentationContext::onDispatched); } @Override @@ -474,8 +474,8 @@ private static class ChainedDeferredExecutionStrategyInstrumentationContext impl } @Override - public void onDispatched(CompletableFuture result) { - contexts.forEach(context -> context.onDispatched(result)); + public void onDispatched() { + contexts.forEach(InstrumentationContext::onDispatched); } @Override diff --git a/src/main/java/graphql/execution/instrumentation/ExecuteObjectInstrumentationContext.java b/src/main/java/graphql/execution/instrumentation/ExecuteObjectInstrumentationContext.java index f78cbdc0b0..4e100238df 100644 --- a/src/main/java/graphql/execution/instrumentation/ExecuteObjectInstrumentationContext.java +++ b/src/main/java/graphql/execution/instrumentation/ExecuteObjectInstrumentationContext.java @@ -15,7 +15,7 @@ public interface ExecuteObjectInstrumentationContext extends InstrumentationCont @Internal ExecuteObjectInstrumentationContext NOOP = new ExecuteObjectInstrumentationContext() { @Override - public void onDispatched(CompletableFuture> result) { + public void onDispatched() { } @Override diff --git a/src/main/java/graphql/execution/instrumentation/ExecutionStrategyInstrumentationContext.java b/src/main/java/graphql/execution/instrumentation/ExecutionStrategyInstrumentationContext.java index 04fbceab81..7fc0a3e0d3 100644 --- a/src/main/java/graphql/execution/instrumentation/ExecutionStrategyInstrumentationContext.java +++ b/src/main/java/graphql/execution/instrumentation/ExecutionStrategyInstrumentationContext.java @@ -36,7 +36,7 @@ static ExecutionStrategyInstrumentationContext nonNullCtx(ExecutionStrategyInstr @Internal ExecutionStrategyInstrumentationContext NOOP = new ExecutionStrategyInstrumentationContext() { @Override - public void onDispatched(CompletableFuture result) { + public void onDispatched() { } @Override diff --git a/src/main/java/graphql/execution/instrumentation/InstrumentationContext.java b/src/main/java/graphql/execution/instrumentation/InstrumentationContext.java index 2d9626a113..2bb52272f8 100644 --- a/src/main/java/graphql/execution/instrumentation/InstrumentationContext.java +++ b/src/main/java/graphql/execution/instrumentation/InstrumentationContext.java @@ -17,10 +17,8 @@ public interface InstrumentationContext { /** * This is invoked when the instrumentation step is initially dispatched - * - * @param result the result of the step as a completable future */ - void onDispatched(CompletableFuture result); + void onDispatched(); /** * This is invoked when the instrumentation step is fully completed diff --git a/src/main/java/graphql/execution/instrumentation/SimpleInstrumentationContext.java b/src/main/java/graphql/execution/instrumentation/SimpleInstrumentationContext.java index 2621314a56..68c70b214e 100644 --- a/src/main/java/graphql/execution/instrumentation/SimpleInstrumentationContext.java +++ b/src/main/java/graphql/execution/instrumentation/SimpleInstrumentationContext.java @@ -15,7 +15,7 @@ public class SimpleInstrumentationContext implements InstrumentationContext NO_OP = new InstrumentationContext() { @Override - public void onDispatched(CompletableFuture result) { + public void onDispatched() { } @Override @@ -49,21 +49,21 @@ public static InstrumentationContext nonNullCtx(InstrumentationContext } private final BiConsumer codeToRunOnComplete; - private final Consumer> codeToRunOnDispatch; + private final Runnable codeToRunOnDispatch; public SimpleInstrumentationContext() { this(null, null); } - private SimpleInstrumentationContext(Consumer> codeToRunOnDispatch, BiConsumer codeToRunOnComplete) { + private SimpleInstrumentationContext(Runnable codeToRunOnDispatch, BiConsumer codeToRunOnComplete) { this.codeToRunOnComplete = codeToRunOnComplete; this.codeToRunOnDispatch = codeToRunOnDispatch; } @Override - public void onDispatched(CompletableFuture result) { + public void onDispatched() { if (codeToRunOnDispatch != null) { - codeToRunOnDispatch.accept(result); + codeToRunOnDispatch.run(); } } @@ -83,7 +83,7 @@ public void onCompleted(T result, Throwable t) { * * @return an instrumentation context */ - public static SimpleInstrumentationContext whenDispatched(Consumer> codeToRun) { + public static SimpleInstrumentationContext whenDispatched(Runnable codeToRun) { return new SimpleInstrumentationContext<>(codeToRun, null); } @@ -101,13 +101,8 @@ public static SimpleInstrumentationContext whenCompleted(BiConsumer BiConsumer completeInstrumentationCtxCF( - InstrumentationContext instrumentationContext, CompletableFuture targetCF) { + InstrumentationContext instrumentationContext) { return (result, throwable) -> { - if (throwable != null) { - targetCF.completeExceptionally(throwable); - } else { - targetCF.complete(result); - } nonNullCtx(instrumentationContext).onCompleted(result, throwable); }; } diff --git a/src/main/java/graphql/execution/instrumentation/adapters/ExecuteObjectInstrumentationContextAdapter.java b/src/main/java/graphql/execution/instrumentation/adapters/ExecuteObjectInstrumentationContextAdapter.java index 2c42d55906..2fd133eb89 100644 --- a/src/main/java/graphql/execution/instrumentation/adapters/ExecuteObjectInstrumentationContextAdapter.java +++ b/src/main/java/graphql/execution/instrumentation/adapters/ExecuteObjectInstrumentationContextAdapter.java @@ -4,12 +4,11 @@ import graphql.ExecutionResultImpl; import graphql.Internal; import graphql.execution.FieldValueInfo; -import graphql.execution.instrumentation.ExecutionStrategyInstrumentationContext; import graphql.execution.instrumentation.ExecuteObjectInstrumentationContext; +import graphql.execution.instrumentation.ExecutionStrategyInstrumentationContext; import java.util.List; import java.util.Map; -import java.util.concurrent.CompletableFuture; /** * A class to help adapt old {@link ExecutionResult} based ExecutionStrategyInstrumentationContext @@ -25,16 +24,17 @@ public ExecuteObjectInstrumentationContextAdapter(ExecutionStrategyInstrumentati } @Override - public void onDispatched(CompletableFuture> result) { - CompletableFuture future = result.thenApply(r -> ExecutionResultImpl.newExecutionResult().data(r).build()); - delegate.onDispatched(future); - // - // when the mapped future is completed, then call onCompleted on the delegate - future.whenComplete(delegate::onCompleted); + public void onDispatched() { + delegate.onDispatched(); } @Override public void onCompleted(Map result, Throwable t) { + if (t != null) { + delegate.onCompleted(null, t); + } else { + delegate.onCompleted(ExecutionResultImpl.newExecutionResult().data(result).build(), null); + } } @Override diff --git a/src/main/java/graphql/execution/instrumentation/adapters/ExecutionResultInstrumentationContextAdapter.java b/src/main/java/graphql/execution/instrumentation/adapters/ExecutionResultInstrumentationContextAdapter.java index d67eddb8d0..063bc75909 100644 --- a/src/main/java/graphql/execution/instrumentation/adapters/ExecutionResultInstrumentationContextAdapter.java +++ b/src/main/java/graphql/execution/instrumentation/adapters/ExecutionResultInstrumentationContextAdapter.java @@ -5,8 +5,6 @@ import graphql.Internal; import graphql.execution.instrumentation.InstrumentationContext; -import java.util.concurrent.CompletableFuture; - /** * A class to help adapt old {@link ExecutionResult} based InstrumentationContext * from the newer {@link Object} based ones. @@ -21,15 +19,16 @@ public ExecutionResultInstrumentationContextAdapter(InstrumentationContext result) { - CompletableFuture future = result.thenApply(obj -> ExecutionResultImpl.newExecutionResult().data(obj).build()); - delegate.onDispatched(future); - // - // when the mapped future is completed, then call onCompleted on the delegate - future.whenComplete(delegate::onCompleted); + public void onDispatched() { + delegate.onDispatched(); } @Override public void onCompleted(Object result, Throwable t) { + if (t != null) { + delegate.onCompleted(null, t); + } else { + delegate.onCompleted(ExecutionResultImpl.newExecutionResult().data(result).build(), null); + } } } diff --git a/src/test/groovy/graphql/execution/AsyncExecutionStrategyTest.groovy b/src/test/groovy/graphql/execution/AsyncExecutionStrategyTest.groovy index 2c9bbf5263..8769bb79a4 100644 --- a/src/test/groovy/graphql/execution/AsyncExecutionStrategyTest.groovy +++ b/src/test/groovy/graphql/execution/AsyncExecutionStrategyTest.groovy @@ -287,7 +287,7 @@ abstract class AsyncExecutionStrategyTest extends Specification { } @Override - void onDispatched(CompletableFuture result) { + void onDispatched() { } @Override diff --git a/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy b/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy index a0247abbd5..d99d3205aa 100644 --- a/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/InstrumentationTest.groovy @@ -171,7 +171,7 @@ class InstrumentationTest extends Specification { return new ExecutionStrategyInstrumentationContext() { @Override - void onDispatched(CompletableFuture result) { + void onDispatched() { System.out.println(String.format("t%s setting go signal on", Thread.currentThread().getId())) goSignal.set(true) } diff --git a/src/test/groovy/graphql/execution/instrumentation/TestingInstrumentContext.groovy b/src/test/groovy/graphql/execution/instrumentation/TestingInstrumentContext.groovy index cdfaa84513..402fd2aee0 100644 --- a/src/test/groovy/graphql/execution/instrumentation/TestingInstrumentContext.groovy +++ b/src/test/groovy/graphql/execution/instrumentation/TestingInstrumentContext.groovy @@ -25,7 +25,7 @@ class TestingInstrumentContext implements InstrumentationContext { } @Override - void onDispatched(CompletableFuture result) { + void onDispatched() { if (useOnDispatch) { this.executionList << "onDispatched:$op" }