From bf9bad8f21689a23555ef1806c93d019400a0a9b Mon Sep 17 00:00:00 2001 From: bbaker Date: Tue, 22 Apr 2025 20:47:01 +1000 Subject: [PATCH 1/7] This removes the FetchedValue wrapping by default --- .../graphql/execution/ExecutionStrategy.java | 79 +++++++++++-------- .../java/graphql/execution/FetchedValue.java | 35 +++++++- .../SubscriptionExecutionStrategy.java | 6 +- .../BreadthFirstExecutionTestStrategy.java | 12 +-- .../execution/BreadthFirstTestStrategy.java | 16 ++-- .../execution/ExecutionStrategyTest.groovy | 32 ++++++-- 6 files changed, 123 insertions(+), 57 deletions(-) diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index f9370d1e91..0a858f18cb 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -113,7 +113,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 @@ -370,7 +370,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)); @@ -379,10 +379,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); @@ -395,16 +394,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(); @@ -412,11 +411,11 @@ 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) { if (incrementAndCheckMaxNodesExceeded(executionContext)) { - return new FetchedValue(null, Collections.emptyList(), null); + return null; } MergedField field = parameters.getField(); @@ -487,17 +486,15 @@ private Object fetchField(GraphQLFieldDefinition fieldDef, ExecutionContext exec // because we added an artificial CF, we need to unwrap the exception fetchCtx.onCompleted(result, exception); if (exception != null) { - CompletableFuture handleFetchingExceptionResult = handleFetchingException(dataFetchingEnvironment.get(), parameters, exception); - return handleFetchingExceptionResult; + return handleFetchingException(dataFetchingEnvironment.get(), parameters, exception); } else { // we can simply return the fetched value CF and avoid a allocation return fetchedValue; } }); 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); @@ -529,9 +526,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; @@ -547,8 +556,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); } } @@ -586,36 +594,39 @@ 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) { Field field = parameters.getField().getSingleField(); GraphQLObjectType parentType = (GraphQLObjectType) parameters.getExecutionStepInfo().getUnwrappedNonNullType(); GraphQLFieldDefinition fieldDef = getFieldDef(executionContext.getGraphQLSchema(), parentType, field); 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); @@ -627,10 +638,13 @@ private FieldValueInfo completeField(GraphQLFieldDefinition fieldDef, ExecutionC NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext, executionStepInfo); + Object rawFetchedValue = FetchedValue.getFetchedValue(fetchedValue); + Object localContext = FetchedValue.getLocalContext(fetchedValue, parameters.getLocalContext()); + ExecutionStrategyParameters newParameters = parameters.transform(builder -> builder.executionStepInfo(executionStepInfo) - .source(fetchedValue.getFetchedValue()) - .localContext(fetchedValue.getLocalContext()) + .source(rawFetchedValue) + .localContext(localContext) .nonNullFieldValidator(nonNullableFieldValidator) ); @@ -788,14 +802,17 @@ protected FieldValueInfo completeValueForList(ExecutionContext executionContext, NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext, stepInfoForListElement); - FetchedValue value = unboxPossibleDataFetcherResult(executionContext, parameters, item); + Object fetchedValue = unboxPossibleDataFetcherResult(executionContext, parameters, item); + + Object rawFetchedValue = FetchedValue.getFetchedValue(fetchedValue); + Object localContext = FetchedValue.getLocalContext(fetchedValue, parameters.getLocalContext()); ExecutionStrategyParameters newParameters = parameters.transform(builder -> builder.executionStepInfo(stepInfoForListElement) .nonNullFieldValidator(nonNullableFieldValidator) - .localContext(value.getLocalContext()) + .localContext(localContext) .path(indexedPath) - .source(value.getFetchedValue()) + .source(rawFetchedValue) ); 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..ed674bb5cb 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#getFetchedValue()} * 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 464f725946..46fbddf0bb 100644 --- a/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java +++ b/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java @@ -108,9 +108,9 @@ private boolean keepOrdered(GraphQLContext graphQLContext) { private CompletableFuture> createSourceEventStream(ExecutionContext executionContext, ExecutionStrategyParameters parameters) { ExecutionStrategyParameters newParameters = firstFieldOfSubscriptionSelection(parameters); - 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); if (publisher != null) { assertTrue(publisher instanceof Publisher, () -> "Your data fetcher must return a Publisher of events when using graphql subscriptions"); } @@ -147,7 +147,7 @@ private CompletableFuture executeSubscriptionEvent(ExecutionCon i13nFieldParameters, executionContext.getInstrumentationState() )); - FetchedValue fetchedValue = unboxPossibleDataFetcherResult(newExecutionContext, parameters, eventPayload); + Object fetchedValue = unboxPossibleDataFetcherResult(newExecutionContext, parameters, eventPayload); FieldValueInfo fieldValueInfo = completeField(newExecutionContext, newParameters, fetchedValue); CompletableFuture overallResult = fieldValueInfo .getFieldValueFuture() 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 ca513d5ba1..ed2c047b1b 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 @@ -685,7 +686,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 @@ -857,25 +858,40 @@ 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) .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"() { From a8d574a03333f4315e245ee5c411f92a4c5a76a8 Mon Sep 17 00:00:00 2001 From: bbaker Date: Thu, 8 May 2025 14:31:32 +1000 Subject: [PATCH 2/7] Breakinf change on FetchedValue in instrumentation --- .../InstrumentationFieldCompleteParameters.java | 8 +++++++- .../groovy/graphql/execution/ExecutionStrategyTest.groovy | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldCompleteParameters.java b/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldCompleteParameters.java index bd89baa15a..8a9c1f3974 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 read + */ + public Object getFetchedObject() { return fetchedValue; } diff --git a/src/test/groovy/graphql/execution/ExecutionStrategyTest.groovy b/src/test/groovy/graphql/execution/ExecutionStrategyTest.groovy index ed2c047b1b..a4188d389b 100644 --- a/src/test/groovy/graphql/execution/ExecutionStrategyTest.groovy +++ b/src/test/groovy/graphql/execution/ExecutionStrategyTest.groovy @@ -666,8 +666,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) From 2406aa8f0e2d355da31b8c269bf3f69bc735387e Mon Sep 17 00:00:00 2001 From: bbaker Date: Thu, 8 May 2025 15:18:57 +1000 Subject: [PATCH 3/7] Javadoc fix up --- src/main/java/graphql/execution/FetchedValue.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/graphql/execution/FetchedValue.java b/src/main/java/graphql/execution/FetchedValue.java index ed674bb5cb..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 MAY be 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 From 6847d8edc0745361e26a62141758c8647c92d0ec Mon Sep 17 00:00:00 2001 From: bbaker Date: Mon, 7 Jul 2025 17:40:23 +1000 Subject: [PATCH 4/7] Merged in master --- .../graphql/execution/ExecutionStrategy.java | 30 ++++--------------- 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index 103b0765dd..dba87736c0 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -630,18 +630,13 @@ private FieldValueInfo completeField(GraphQLFieldDefinition fieldDef, ExecutionC instrumentationParams, executionContext.getInstrumentationState() )); - - NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext, executionStepInfo); - Object rawFetchedValue = FetchedValue.getFetchedValue(fetchedValue); Object localContext = FetchedValue.getLocalContext(fetchedValue, parameters.getLocalContext()); - ExecutionStrategyParameters newParameters = parameters.transform(builder -> - builder.executionStepInfo(executionStepInfo) - .source(rawFetchedValue) - .localContext(localContext) - .nonNullFieldValidator(nonNullableFieldValidator) - ); + ExecutionStrategyParameters newParameters = parameters.transform(executionStepInfo, + rawFetchedValue, + localContext); + FieldValueInfo fieldValueInfo = completeValue(executionContext, newParameters); ctxCompleteField.onDispatched(); if (fieldValueInfo.isFutureValue()) { @@ -799,26 +794,13 @@ protected FieldValueInfo completeValueForList(ExecutionContext executionContext, Object fetchedValue = unboxPossibleDataFetcherResult(executionContext, parameters, item); - ExecutionStrategyParameters newParameters = parameters.transform(stepInfoForListElement, - indexedPath, - value.getLocalContext(), - value.getFetchedValue()); - Object rawFetchedValue = FetchedValue.getFetchedValue(fetchedValue); Object localContext = FetchedValue.getLocalContext(fetchedValue, parameters.getLocalContext()); - //fix me ExecutionStrategyParameters newParameters = parameters.transform(stepInfoForListElement, indexedPath, - value.getLocalContext(), - value.getFetchedValue()); - - ExecutionStrategyParameters newParameters = parameters.transform(builder -> - builder.executionStepInfo(stepInfoForListElement) - .localContext(localContext) - .path(indexedPath) - .source(rawFetchedValue) - ); + localContext, + rawFetchedValue); fieldValueInfos.add(completeValue(executionContext, newParameters)); index++; From 9bf527a00d4a86973eed2983b15e01a32cb36a1c Mon Sep 17 00:00:00 2001 From: bbaker Date: Mon, 7 Jul 2025 17:47:43 +1000 Subject: [PATCH 5/7] updated javadoc --- .../parameters/InstrumentationFieldCompleteParameters.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldCompleteParameters.java b/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldCompleteParameters.java index 8a9c1f3974..ac7e1b27e5 100644 --- a/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldCompleteParameters.java +++ b/src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldCompleteParameters.java @@ -52,7 +52,7 @@ public ExecutionStepInfo getExecutionStepInfo() { * 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 read + * @return the object was fetched, ready to be completed as a value. */ public Object getFetchedObject() { return fetchedValue; From 2758735f3214ae254a9abdf49548bb8809ad4761 Mon Sep 17 00:00:00 2001 From: bbaker Date: Tue, 8 Jul 2025 11:49:25 +1000 Subject: [PATCH 6/7] Transposition error --- .../graphql/execution/ExecutionStrategy.java | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index dba87736c0..4a826bdcbc 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -630,12 +630,10 @@ private FieldValueInfo completeField(GraphQLFieldDefinition fieldDef, ExecutionC instrumentationParams, executionContext.getInstrumentationState() )); - Object rawFetchedValue = FetchedValue.getFetchedValue(fetchedValue); - Object localContext = FetchedValue.getLocalContext(fetchedValue, parameters.getLocalContext()); - - ExecutionStrategyParameters newParameters = parameters.transform(executionStepInfo, - rawFetchedValue, - localContext); + ExecutionStrategyParameters newParameters = parameters.transform( + executionStepInfo, + FetchedValue.getLocalContext(fetchedValue, parameters.getLocalContext()), + FetchedValue.getFetchedValue(fetchedValue)); FieldValueInfo fieldValueInfo = completeValue(executionContext, newParameters); ctxCompleteField.onDispatched(); @@ -794,13 +792,11 @@ protected FieldValueInfo completeValueForList(ExecutionContext executionContext, Object fetchedValue = unboxPossibleDataFetcherResult(executionContext, parameters, item); - Object rawFetchedValue = FetchedValue.getFetchedValue(fetchedValue); - Object localContext = FetchedValue.getLocalContext(fetchedValue, parameters.getLocalContext()); - - ExecutionStrategyParameters newParameters = parameters.transform(stepInfoForListElement, + ExecutionStrategyParameters newParameters = parameters.transform( + stepInfoForListElement, indexedPath, - localContext, - rawFetchedValue); + FetchedValue.getLocalContext(fetchedValue, parameters.getLocalContext()), + FetchedValue.getFetchedValue(fetchedValue)); fieldValueInfos.add(completeValue(executionContext, newParameters)); index++; From 095ba62f018e8c15307e28a6c214fd00a2006dd4 Mon Sep 17 00:00:00 2001 From: bbaker Date: Tue, 8 Jul 2025 11:53:08 +1000 Subject: [PATCH 7/7] Transposition error - code format --- src/main/java/graphql/execution/ExecutionStrategy.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index 4a826bdcbc..614b75e703 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -633,7 +633,8 @@ private FieldValueInfo completeField(GraphQLFieldDefinition fieldDef, ExecutionC ExecutionStrategyParameters newParameters = parameters.transform( executionStepInfo, FetchedValue.getLocalContext(fetchedValue, parameters.getLocalContext()), - FetchedValue.getFetchedValue(fetchedValue)); + FetchedValue.getFetchedValue(fetchedValue) + ); FieldValueInfo fieldValueInfo = completeValue(executionContext, newParameters); ctxCompleteField.onDispatched(); @@ -796,7 +797,8 @@ protected FieldValueInfo completeValueForList(ExecutionContext executionContext, stepInfoForListElement, indexedPath, FetchedValue.getLocalContext(fetchedValue, parameters.getLocalContext()), - FetchedValue.getFetchedValue(fetchedValue)); + FetchedValue.getFetchedValue(fetchedValue) + ); fieldValueInfos.add(completeValue(executionContext, newParameters)); index++;