Skip to content

SPIKE - WIP - Another PR that shows how we could transition to the new idea of state per instrumentation method call #2747

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bbakerman wants to merge 2 commits into from
Closed

SPIKE - WIP - Another PR that shows how we could transition to the new idea of state per instrumentation method call #2747

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
003bfdc
Another WIP PR that shows how we could transition to the new idea of …
bbakerman Feb 27, 2022
3cb1003
Opps didnt write it right
bbakerman Feb 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/main/java/graphql/execution/ExecutionStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ protected CompletableFuture<FieldValueInfo> resolveFieldWithInfo(ExecutionContex

Instrumentation instrumentation = executionContext.getInstrumentation();
InstrumentationContext<ExecutionResult> fieldCtx = instrumentation.beginField(
new InstrumentationFieldParameters(executionContext, executionStepInfo)
new InstrumentationFieldParameters(executionContext, executionStepInfo), executionContext.getInstrumentationState()
);

CompletableFuture<FetchedValue> fetchFieldFuture = fetchField(executionContext, parameters);
Expand Down Expand Up @@ -270,7 +270,7 @@ protected CompletableFuture<FetchedValue> fetchField(ExecutionContext executionC
Instrumentation instrumentation = executionContext.getInstrumentation();

InstrumentationFieldFetchParameters instrumentationFieldFetchParams = new InstrumentationFieldFetchParameters(executionContext, environment, parameters, dataFetcher instanceof TrivialDataFetcher);
InstrumentationContext<Object> fetchCtx = instrumentation.beginFieldFetch(instrumentationFieldFetchParams);
InstrumentationContext<Object> fetchCtx = instrumentation.beginFieldFetch(instrumentationFieldFetchParams, executionContext.getInstrumentationState());

CompletableFuture<Object> fetchedValue;
dataFetcher = instrumentation.instrumentDataFetcher(dataFetcher, instrumentationFieldFetchParams);
Expand Down Expand Up @@ -354,9 +354,9 @@ protected <T> CompletableFuture<T> handleFetchingException(ExecutionContext exec

private <T> CompletableFuture<T> asyncHandleException(DataFetcherExceptionHandler handler, DataFetcherExceptionHandlerParameters handlerParameters, ExecutionContext executionContext) {
//noinspection unchecked
return handler.handleException(handlerParameters)
.thenApply(handlerResult -> (T) DataFetcherResult.<FetchedValue>newResult().errors(handlerResult.getErrors()).build()
);
return handler.handleException(handlerParameters)
.thenApply(handlerResult -> (T) DataFetcherResult.<FetchedValue>newResult().errors(handlerResult.getErrors()).build()
);
}

/**
Expand Down Expand Up @@ -385,7 +385,7 @@ protected FieldValueInfo completeField(ExecutionContext executionContext, Execut
Instrumentation instrumentation = executionContext.getInstrumentation();
InstrumentationFieldCompleteParameters instrumentationParams = new InstrumentationFieldCompleteParameters(executionContext, parameters, () -> executionStepInfo, fetchedValue);
InstrumentationContext<ExecutionResult> ctxCompleteField = instrumentation.beginFieldComplete(
instrumentationParams
instrumentationParams, executionContext.getInstrumentationState()
);

NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext, executionStepInfo);
Expand Down Expand Up @@ -519,7 +519,7 @@ protected FieldValueInfo completeValueForList(ExecutionContext executionContext,
Instrumentation instrumentation = executionContext.getInstrumentation();

InstrumentationContext<ExecutionResult> completeListCtx = instrumentation.beginFieldListComplete(
instrumentationParams
instrumentationParams, executionContext.getInstrumentationState()
);

List<FieldValueInfo> fieldValueInfos = new ArrayList<>(size.orElse(1));
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/graphql/execution/SubscriptionExecutionStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,9 @@ private CompletableFuture<ExecutionResult> executeSubscriptionEvent(ExecutionCon
ExecutionStepInfo subscribedFieldStepInfo = createSubscribedFieldStepInfo(executionContext, newParameters);

InstrumentationFieldParameters i13nFieldParameters = new InstrumentationFieldParameters(executionContext, () -> subscribedFieldStepInfo);
InstrumentationContext<ExecutionResult> subscribedFieldCtx = instrumentation.beginSubscribedFieldEvent(i13nFieldParameters);
InstrumentationContext<ExecutionResult> subscribedFieldCtx = instrumentation.beginSubscribedFieldEvent(
i13nFieldParameters, executionContext.getInstrumentationState()
);

FetchedValue fetchedValue = unboxPossibleDataFetcherResult(newExecutionContext, parameters, eventPayload);
FieldValueInfo fieldValueInfo = completeField(newExecutionContext, newParameters, fetchedValue);
Expand Down
25 changes: 25 additions & 0 deletions src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ public InstrumentationContext<ExecutionResult> beginSubscribedFieldEvent(Instrum
}));
}

@Override
public InstrumentationContext<ExecutionResult> beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, InstrumentationState state) {
return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginSubscribedFieldEvent(parameters, getState(it, state))));
}

@Override
public InstrumentationContext<ExecutionResult> beginField(InstrumentationFieldParameters parameters) {
return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> {
Expand All @@ -126,6 +131,11 @@ public InstrumentationContext<ExecutionResult> beginField(InstrumentationFieldPa
}));
}

@Override
public InstrumentationContext<ExecutionResult> beginField(InstrumentationFieldParameters parameters, InstrumentationState state) {
return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginField(parameters, getState(it, state))));
}

Copy link
Member Author

@bbakerman bbakerman Feb 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note I still need to create a wrapper ChainedInstrumentationContext object per call

The alternative is

        ImmutableList<InstrumentationContext<Object>> contexts = map(instrumentations, it -> it.beginFieldFetch(parameters, getState(it, state)));
        return new InstrumentationContext<Object>() {
            @Override
            public void onDispatched(CompletableFuture<Object> result) {
                contexts.forEach(ctx -> ctx.onDispatched(result));
            }

            @Override
            public void onCompleted(Object result, Throwable t) {
                contexts.forEach(ctx -> ctx.onCompleted(result,t));
            }
        };

and this is an object allocation anyway - since we MUST call back when the result is dispatched and most importantly when its finished

@Override
public InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters) {
return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> {
Expand All @@ -134,6 +144,11 @@ public InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchP
}));
}

@Override
public InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) {
return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginFieldFetch(parameters, getState(it, state))));
}

@Override
public InstrumentationContext<ExecutionResult> beginFieldComplete(InstrumentationFieldCompleteParameters parameters) {
return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> {
Expand All @@ -142,6 +157,11 @@ public InstrumentationContext<ExecutionResult> beginFieldComplete(Instrumentatio
}));
}

@Override
public InstrumentationContext<ExecutionResult> beginFieldComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) {
return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginFieldComplete(parameters, getState(it, state))));
}

@Override
public InstrumentationContext<ExecutionResult> beginFieldListComplete(InstrumentationFieldCompleteParameters parameters) {
return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> {
Expand All @@ -150,6 +170,11 @@ public InstrumentationContext<ExecutionResult> beginFieldListComplete(Instrument
}));
}

@Override
public InstrumentationContext<ExecutionResult> beginFieldListComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) {
return new ChainedInstrumentationContext<>(map(instrumentations, it -> it.beginFieldListComplete(parameters, getState(it, state))));
}

@Override
public ExecutionInput instrumentExecutionInput(ExecutionInput executionInput, InstrumentationExecutionParameters parameters) {
for (Instrumentation instrumentation : instrumentations) {
Expand Down
25 changes: 25 additions & 0 deletions src/main/java/graphql/execution/instrumentation/Instrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,28 +114,43 @@ default InstrumentationState createState(InstrumentationCreateStateParameters pa
*
* @return a non null {@link InstrumentationContext} object that will be called back when the step ends
*/
@Deprecated
default InstrumentationContext<ExecutionResult> beginSubscribedFieldEvent(InstrumentationFieldParameters parameters) {
return noOp();
}

default InstrumentationContext<ExecutionResult> beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, InstrumentationState state) {
return beginSubscribedFieldEvent(parameters.withNewState(state));
}

/**
* This is called just before a field is resolved into a value.
*
* @param parameters the parameters to this step
*
* @return a non null {@link InstrumentationContext} object that will be called back when the step ends
*/
@Deprecated
InstrumentationContext<ExecutionResult> beginField(InstrumentationFieldParameters parameters);

default InstrumentationContext<ExecutionResult> beginField(InstrumentationFieldParameters parameters, InstrumentationState state) {
return beginField(parameters.withNewState(state));
}

/**
* This is called just before a field {@link DataFetcher} is invoked.
*
* @param parameters the parameters to this step
*
* @return a non null {@link InstrumentationContext} object that will be called back when the step ends
*/
@Deprecated
InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters);

default InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) {
return beginFieldFetch(parameters.withNewState(state));
}


/**
* This is called just before the complete field is started.
Expand All @@ -144,21 +159,31 @@ default InstrumentationContext<ExecutionResult> beginSubscribedFieldEvent(Instru
*
* @return a non null {@link InstrumentationContext} object that will be called back when the step ends
*/
@Deprecated
default InstrumentationContext<ExecutionResult> beginFieldComplete(InstrumentationFieldCompleteParameters parameters) {
return noOp();
}

default InstrumentationContext<ExecutionResult> beginFieldComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) {
return beginFieldComplete(parameters.withNewState(state));
}

/**
* This is called just before the complete field list is started.
*
* @param parameters the parameters to this step
*
* @return a non null {@link InstrumentationContext} object that will be called back when the step ends
*/
@Deprecated
default InstrumentationContext<ExecutionResult> beginFieldListComplete(InstrumentationFieldCompleteParameters parameters) {
return noOp();
}

default InstrumentationContext<ExecutionResult> beginFieldListComplete(InstrumentationFieldCompleteParameters parameters, InstrumentationState state) {
return beginFieldListComplete(parameters.withNewState(state));
}

/**
* This is called to instrument a {@link graphql.ExecutionInput} before it is used to parse, validate
* and execute a query, allowing you to adjust what query input parameters are used
Expand Down