From 0d4fc6e2d87b39bc9930dcf35ed30672d59cf025 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Sat, 21 Oct 2023 12:29:06 +1100 Subject: [PATCH] This changes onDispatched so it no longer takes a CF --- src/main/java/graphql/GraphQL.java | 13 ++++-------- .../execution/AsyncExecutionStrategy.java | 2 +- .../AsyncSerialExecutionStrategy.java | 2 +- .../java/graphql/execution/Execution.java | 4 ++-- .../graphql/execution/ExecutionStrategy.java | 8 ++++---- .../SubscriptionExecutionStrategy.java | 4 ++-- .../ChainedInstrumentation.java | 8 ++++---- ...ecutionStrategyInstrumentationContext.java | 2 +- .../InstrumentationContext.java | 8 +++----- .../SimpleInstrumentationContext.java | 20 +++++++------------ .../FieldLevelTrackingApproach.java | 5 ++--- .../AsyncExecutionStrategyTest.groovy | 2 +- .../InstrumentationTest.groovy | 2 +- .../TestingInstrumentContext.groovy | 2 +- 14 files changed, 34 insertions(+), 48 deletions(-) diff --git a/src/main/java/graphql/GraphQL.java b/src/main/java/graphql/GraphQL.java index 2ccb54b955..7a626219f0 100644 --- a/src/main/java/graphql/GraphQL.java +++ b/src/main/java/graphql/GraphQL.java @@ -430,17 +430,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)); @@ -525,15 +524,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(); @@ -545,15 +542,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 6608fba0f3..9df1ffda7a 100644 --- a/src/main/java/graphql/execution/AsyncExecutionStrategy.java +++ b/src/main/java/graphql/execution/AsyncExecutionStrategy.java @@ -56,7 +56,7 @@ public CompletableFuture execute(ExecutionContext executionCont futures.add(future); } CompletableFuture overallResult = new CompletableFuture<>(); - executionStrategyCtx.onDispatched(overallResult); + executionStrategyCtx.onDispatched(); futures.await().whenComplete((completeValueInfos, throwable) -> { BiConsumer, Throwable> handleResultsConsumer = handleResults(executionContext, fieldNames, overallResult); diff --git a/src/main/java/graphql/execution/AsyncSerialExecutionStrategy.java b/src/main/java/graphql/execution/AsyncSerialExecutionStrategy.java index fc2dde0980..9538882280 100644 --- a/src/main/java/graphql/execution/AsyncSerialExecutionStrategy.java +++ b/src/main/java/graphql/execution/AsyncSerialExecutionStrategy.java @@ -48,7 +48,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 916bc64659..0c9e8036e4 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -122,7 +122,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; } @@ -171,7 +171,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 4ed0f1e644..8ae5e42213 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -216,7 +216,7 @@ protected CompletableFuture resolveFieldWithInfo(ExecutionContex CompletableFuture executionResultFuture = result.thenCompose(FieldValueInfo::getFieldValue); - fieldCtx.onDispatched(executionResultFuture); + fieldCtx.onDispatched(); executionResultFuture.whenComplete(fieldCtx::onCompleted); return result; } @@ -286,7 +286,7 @@ protected CompletableFuture fetchField(ExecutionContext executionC dataFetcher = instrumentation.instrumentDataFetcher(dataFetcher, instrumentationFieldFetchParams, executionContext.getInstrumentationState()); CompletableFuture fetchedValue = invokeDataFetcher(executionContext, parameters, fieldDef, dataFetchingEnvironment, dataFetcher); - fetchCtx.onDispatched(fetchedValue); + fetchCtx.onDispatched(); return fetchedValue .handle((result, exception) -> { fetchCtx.onCompleted(result, exception); @@ -435,7 +435,7 @@ protected FieldValueInfo completeField(ExecutionContext executionContext, Execut FieldValueInfo fieldValueInfo = completeValue(executionContext, newParameters); CompletableFuture executionResultFuture = fieldValueInfo.getFieldValue(); - ctxCompleteField.onDispatched(executionResultFuture); + ctxCompleteField.onDispatched(); executionResultFuture.whenComplete(ctxCompleteField::onCompleted); return fieldValueInfo; } @@ -590,7 +590,7 @@ protected FieldValueInfo completeValueForList(ExecutionContext executionContext, CompletableFuture> resultsFuture = Async.each(fieldValueInfos, FieldValueInfo::getFieldValue); CompletableFuture overallResult = new CompletableFuture<>(); - completeListCtx.onDispatched(overallResult); + completeListCtx.onDispatched(); resultsFuture.whenComplete((results, exception) -> { if (exception != null) { diff --git a/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java b/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java index be816e7add..8659eae975 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; @@ -139,7 +139,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 70e7bd063f..99d5769de6 100644 --- a/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java @@ -395,8 +395,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 @@ -414,8 +414,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 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..3477e6f8c0 100644 --- a/src/main/java/graphql/execution/instrumentation/InstrumentationContext.java +++ b/src/main/java/graphql/execution/instrumentation/InstrumentationContext.java @@ -7,8 +7,8 @@ /** * When a {@link Instrumentation}.'beginXXX()' method is called then it must return a non null InstrumentationContext * that will be invoked when the step is first dispatched and then when it completes. Sometimes this is effectively the same time - * whereas at other times its when an asynchronous {@link java.util.concurrent.CompletableFuture} completes. - * + * whereas at other times it's when an asynchronous {@link java.util.concurrent.CompletableFuture} completes. + *

* This pattern of construction of an object then call back is intended to allow "timers" to be created that can instrument what has * just happened or "loggers" to be called to record what has happened. */ @@ -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..9904dacef1 100644 --- a/src/main/java/graphql/execution/instrumentation/SimpleInstrumentationContext.java +++ b/src/main/java/graphql/execution/instrumentation/SimpleInstrumentationContext.java @@ -5,7 +5,6 @@ import java.util.concurrent.CompletableFuture; import java.util.function.BiConsumer; -import java.util.function.Consumer; /** * A simple implementation of {@link InstrumentationContext} @@ -15,7 +14,7 @@ public class SimpleInstrumentationContext implements InstrumentationContext NO_OP = new InstrumentationContext() { @Override - public void onDispatched(CompletableFuture result) { + public void onDispatched() { } @Override @@ -49,21 +48,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 +82,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 +100,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/dataloader/FieldLevelTrackingApproach.java b/src/main/java/graphql/execution/instrumentation/dataloader/FieldLevelTrackingApproach.java index 7da689db9a..1d4b73d209 100644 --- a/src/main/java/graphql/execution/instrumentation/dataloader/FieldLevelTrackingApproach.java +++ b/src/main/java/graphql/execution/instrumentation/dataloader/FieldLevelTrackingApproach.java @@ -134,8 +134,7 @@ ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationEx return new ExecutionStrategyInstrumentationContext() { @Override - public void onDispatched(CompletableFuture result) { - + public void onDispatched() { } @Override @@ -192,7 +191,7 @@ public InstrumentationContext beginFieldFetch(InstrumentationFieldFetchP return new InstrumentationContext<>() { @Override - public void onDispatched(CompletableFuture result) { + public void onDispatched() { boolean dispatchNeeded = callStack.lock.callLocked(() -> { callStack.increaseFetchCount(level); return dispatchIfNeeded(callStack, level); diff --git a/src/test/groovy/graphql/execution/AsyncExecutionStrategyTest.groovy b/src/test/groovy/graphql/execution/AsyncExecutionStrategyTest.groovy index 951246df0c..4676236c6c 100644 --- a/src/test/groovy/graphql/execution/AsyncExecutionStrategyTest.groovy +++ b/src/test/groovy/graphql/execution/AsyncExecutionStrategyTest.groovy @@ -278,7 +278,7 @@ 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 85b274089a..92aaf0d891 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" }