Skip to content

Commit ecd84a1

Browse files
dondonzdondonz
authored
Merge pull request #3924 from graphql-java/remove-fetch-value-wrapping
This removes the FetchedValue wrapping by default
2 parents 1a4c778 + 095ba62 commit ecd84a1

File tree

7 files changed

+131
-60
lines changed

7 files changed

+131
-60
lines changed

‎src/main/java/graphql/execution/ExecutionStrategy.java

Lines changed: 47 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@
105105
* <p>
106106
* The first phase (data fetching) is handled by the method {@link #fetchField(ExecutionContext, ExecutionStrategyParameters)}
107107
* <p>
108-
* The second phase (value completion) is handled by the methods {@link #completeField(ExecutionContext, ExecutionStrategyParameters, FetchedValue)}
108+
* The second phase (value completion) is handled by the methods {@link #completeField(ExecutionContext, ExecutionStrategyParameters, Object)}
109109
* and the other "completeXXX" methods.
110110
* <p>
111111
* 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
358358

359359
Object fetchedValueObj = fetchField(executionContext, parameters);
360360
if (fetchedValueObj instanceof CompletableFuture) {
361-
CompletableFuture<FetchedValue> fetchFieldFuture = (CompletableFuture<FetchedValue>) fetchedValueObj;
361+
CompletableFuture<Object> fetchFieldFuture = (CompletableFuture<Object>) fetchedValueObj;
362362
CompletableFuture<FieldValueInfo> result = fetchFieldFuture.thenApply((fetchedValue) ->
363363
completeField(fieldDef, executionContext, parameters, fetchedValue));
364364

@@ -367,10 +367,9 @@ protected Object resolveFieldWithInfo(ExecutionContext executionContext, Executi
367367
return result;
368368
} else {
369369
try {
370-
FetchedValue fetchedValue = (FetchedValue) fetchedValueObj;
371-
FieldValueInfo fieldValueInfo = completeField(fieldDef, executionContext, parameters, fetchedValue);
370+
FieldValueInfo fieldValueInfo = completeField(fieldDef, executionContext, parameters, fetchedValueObj);
372371
fieldCtx.onDispatched();
373-
fieldCtx.onCompleted(fetchedValue.getFetchedValue(), null);
372+
fieldCtx.onCompleted(FetchedValue.getFetchedValue(fetchedValueObj), null);
374373
return fieldValueInfo;
375374
} catch (Exception e) {
376375
return Async.exceptionallyCompletedFuture(e);
@@ -383,29 +382,29 @@ protected Object resolveFieldWithInfo(ExecutionContext executionContext, Executi
383382
* {@link GraphQLFieldDefinition}.
384383
* <p>
385384
* Graphql fragments mean that for any give logical field can have one or more {@link Field} values associated with it
386-
* in the query, hence the fieldList. However the first entry is representative of the field for most purposes.
385+
* in the query, hence the fieldList. However, the first entry is representative of the field for most purposes.
387386
*
388387
* @param executionContext contains the top level execution parameters
389388
* @param parameters contains the parameters holding the fields to be executed and source object
390389
*
391-
* @return a promise to a {@link FetchedValue} object or the {@link FetchedValue} itself
390+
* @return a promise to a value object or the value itself. The value maybe a raw object OR a {@link FetchedValue}
392391
*
393-
* @throws NonNullableFieldWasNullException in the future if a non null field resolves to a null value
392+
* @throws NonNullableFieldWasNullException in the future if a non-null field resolves to a null value
394393
*/
395-
@DuckTyped(shape = "CompletableFuture<FetchedValue> | FetchedValue")
394+
@DuckTyped(shape = "CompletableFuture<FetchedValue|Object> | <FetchedValue|Object>")
396395
protected Object fetchField(ExecutionContext executionContext, ExecutionStrategyParameters parameters) {
397396
MergedField field = parameters.getField();
398397
GraphQLObjectType parentType = (GraphQLObjectType) parameters.getExecutionStepInfo().getUnwrappedNonNullType();
399398
GraphQLFieldDefinition fieldDef = getFieldDef(executionContext.getGraphQLSchema(), parentType, field.getSingleField());
400399
return fetchField(fieldDef, executionContext, parameters);
401400
}
402401

403-
@DuckTyped(shape = "CompletableFuture<FetchedValue> | FetchedValue")
402+
@DuckTyped(shape = "CompletableFuture<FetchedValue|Object> | <FetchedValue|Object>")
404403
private Object fetchField(GraphQLFieldDefinition fieldDef, ExecutionContext executionContext, ExecutionStrategyParameters parameters) {
405404
executionContext.throwIfCancelled();
406405

407406
if (incrementAndCheckMaxNodesExceeded(executionContext)) {
408-
return new FetchedValue(null, Collections.emptyList(), null);
407+
return null;
409408
}
410409

411410
MergedField field = parameters.getField();
@@ -486,9 +485,8 @@ private Object fetchField(GraphQLFieldDefinition fieldDef, ExecutionContext exec
486485
}
487486
});
488487
CompletableFuture<Object> rawResultCF = engineRunningState.compose(handleCF, Function.identity());
489-
CompletableFuture<FetchedValue> fetchedValueCF = rawResultCF
488+
return rawResultCF
490489
.thenApply(result -> unboxPossibleDataFetcherResult(executionContext, parameters, result));
491-
return fetchedValueCF;
492490
} else {
493491
fetchCtx.onCompleted(fetchedObject, null);
494492
return unboxPossibleDataFetcherResult(executionContext, parameters, fetchedObject);
@@ -520,9 +518,21 @@ protected Supplier<ExecutableNormalizedField> getNormalizedField(ExecutionContex
520518
return () -> normalizedQuery.get().getNormalizedField(parameters.getField(), executionStepInfo.get().getObjectType(), executionStepInfo.get().getPath());
521519
}
522520

523-
protected FetchedValue unboxPossibleDataFetcherResult(ExecutionContext executionContext,
524-
ExecutionStrategyParameters parameters,
525-
Object result) {
521+
/**
522+
* If the data fetching returned a {@link DataFetcherResult} then it can contain errors and new local context
523+
* and hence it gets turned into a {@link FetchedValue} but otherwise this method returns the unboxed
524+
* value without the wrapper. This means its more efficient overall by default.
525+
*
526+
* @param executionContext the execution context in play
527+
* @param parameters the parameters in play
528+
* @param result the fetched raw object
529+
*
530+
* @return an unboxed value which can be a FetchedValue or an Object
531+
*/
532+
@DuckTyped(shape = "FetchedValue | Object")
533+
protected Object unboxPossibleDataFetcherResult(ExecutionContext executionContext,
534+
ExecutionStrategyParameters parameters,
535+
Object result) {
526536
if (result instanceof DataFetcherResult) {
527537
DataFetcherResult<?> dataFetcherResult = (DataFetcherResult<?>) result;
528538

@@ -538,8 +548,7 @@ protected FetchedValue unboxPossibleDataFetcherResult(ExecutionContext execution
538548
Object unBoxedValue = executionContext.getValueUnboxer().unbox(dataFetcherResult.getData());
539549
return new FetchedValue(unBoxedValue, dataFetcherResult.getErrors(), localContext);
540550
} else {
541-
Object unBoxedValue = executionContext.getValueUnboxer().unbox(result);
542-
return new FetchedValue(unBoxedValue, ImmutableList.of(), parameters.getLocalContext());
551+
return executionContext.getValueUnboxer().unbox(result);
543552
}
544553
}
545554

@@ -577,29 +586,32 @@ protected <T> CompletableFuture<T> handleFetchingException(
577586
private <T> CompletableFuture<T> asyncHandleException(DataFetcherExceptionHandler handler, DataFetcherExceptionHandlerParameters handlerParameters) {
578587
//noinspection unchecked
579588
return handler.handleException(handlerParameters).thenApply(
580-
handlerResult -> (T) DataFetcherResult.<FetchedValue>newResult().errors(handlerResult.getErrors()).build()
589+
handlerResult -> (T) DataFetcherResult.newResult().errors(handlerResult.getErrors()).build()
581590
);
582591
}
583592

584593
/**
585594
* Called to complete a field based on the type of the field.
586595
* <p>
587-
* 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
596+
* 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
588597
* the execution strategy will be called recursively again to execute the fields of that type before returning.
589598
* <p>
590599
* Graphql fragments mean that for any give logical field can have one or more {@link Field} values associated with it
591-
* in the query, hence the fieldList. However the first entry is representative of the field for most purposes.
600+
* in the query, hence the fieldList. However, the first entry is representative of the field for most purposes.
592601
*
593602
* @param executionContext contains the top level execution parameters
594603
* @param parameters contains the parameters holding the fields to be executed and source object
595-
* @param fetchedValue the fetched raw value
604+
* @param fetchedValue the fetched raw value or perhaps a {@link FetchedValue} wrapper of that value
596605
*
597606
* @return a {@link FieldValueInfo}
598607
*
599608
* @throws NonNullableFieldWasNullException in the {@link FieldValueInfo#getFieldValueFuture()} future
600609
* if a nonnull field resolves to a null value
601610
*/
602-
protected FieldValueInfo completeField(ExecutionContext executionContext, ExecutionStrategyParameters parameters, FetchedValue fetchedValue) {
611+
protected FieldValueInfo completeField(ExecutionContext executionContext,
612+
ExecutionStrategyParameters parameters,
613+
@DuckTyped(shape = "Object | FetchedValue")
614+
Object fetchedValue) {
603615
executionContext.throwIfCancelled();
604616

605617
Field field = parameters.getField().getSingleField();
@@ -608,7 +620,7 @@ protected FieldValueInfo completeField(ExecutionContext executionContext, Execut
608620
return completeField(fieldDef, executionContext, parameters, fetchedValue);
609621
}
610622

611-
private FieldValueInfo completeField(GraphQLFieldDefinition fieldDef, ExecutionContext executionContext, ExecutionStrategyParameters parameters, FetchedValue fetchedValue) {
623+
private FieldValueInfo completeField(GraphQLFieldDefinition fieldDef, ExecutionContext executionContext, ExecutionStrategyParameters parameters, Object fetchedValue) {
612624
GraphQLObjectType parentType = (GraphQLObjectType) parameters.getExecutionStepInfo().getUnwrappedNonNullType();
613625
ExecutionStepInfo executionStepInfo = createExecutionStepInfo(executionContext, parameters, fieldDef, parentType);
614626

@@ -618,9 +630,11 @@ private FieldValueInfo completeField(GraphQLFieldDefinition fieldDef, ExecutionC
618630
instrumentationParams, executionContext.getInstrumentationState()
619631
));
620632

621-
ExecutionStrategyParameters newParameters = parameters.transform(executionStepInfo,
622-
fetchedValue.getLocalContext(),
623-
fetchedValue.getFetchedValue());
633+
ExecutionStrategyParameters newParameters = parameters.transform(
634+
executionStepInfo,
635+
FetchedValue.getLocalContext(fetchedValue, parameters.getLocalContext()),
636+
FetchedValue.getFetchedValue(fetchedValue)
637+
);
624638

625639
FieldValueInfo fieldValueInfo = completeValue(executionContext, newParameters);
626640
ctxCompleteField.onDispatched();
@@ -777,12 +791,14 @@ protected FieldValueInfo completeValueForList(ExecutionContext executionContext,
777791

778792
ExecutionStepInfo stepInfoForListElement = executionStepInfoFactory.newExecutionStepInfoForListElement(executionStepInfo, indexedPath);
779793

780-
FetchedValue value = unboxPossibleDataFetcherResult(executionContext, parameters, item);
794+
Object fetchedValue = unboxPossibleDataFetcherResult(executionContext, parameters, item);
781795

782-
ExecutionStrategyParameters newParameters = parameters.transform(stepInfoForListElement,
796+
ExecutionStrategyParameters newParameters = parameters.transform(
797+
stepInfoForListElement,
783798
indexedPath,
784-
value.getLocalContext(),
785-
value.getFetchedValue());
799+
FetchedValue.getLocalContext(fetchedValue, parameters.getLocalContext()),
800+
FetchedValue.getFetchedValue(fetchedValue)
801+
);
786802

787803
fieldValueInfos.add(completeValue(executionContext, newParameters));
788804
index++;

‎src/main/java/graphql/execution/FetchedValue.java

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import java.util.List;
99

1010
/**
11-
* Note: This is returned by {@link InstrumentationFieldCompleteParameters#getFetchedValue()}
11+
* Note: This MAY be returned by {@link InstrumentationFieldCompleteParameters#getFetchedObject()}
1212
* and therefore part of the public despite never used in a method signature.
1313
*/
1414
@PublicApi
@@ -17,6 +17,39 @@ public class FetchedValue {
1717
private final Object localContext;
1818
private final ImmutableList<GraphQLError> errors;
1919

20+
/**
21+
* This allows you to get to the underlying fetched value depending on whether the source
22+
* value is a {@link FetchedValue} or not
23+
*
24+
* @param sourceValue the source value in play
25+
*
26+
* @return the {@link FetchedValue#getFetchedValue()} if its wrapped otherwise the source value itself
27+
*/
28+
public static Object getFetchedValue(Object sourceValue) {
29+
if (sourceValue instanceof FetchedValue) {
30+
return ((FetchedValue) sourceValue).fetchedValue;
31+
} else {
32+
return sourceValue;
33+
}
34+
}
35+
36+
/**
37+
* This allows you to get to the local context depending on whether the source
38+
* value is a {@link FetchedValue} or not
39+
*
40+
* @param sourceValue the source value in play
41+
* @param defaultLocalContext the default local context to use
42+
*
43+
* @return the {@link FetchedValue#getFetchedValue()} if its wrapped otherwise the default local context
44+
*/
45+
public static Object getLocalContext(Object sourceValue, Object defaultLocalContext) {
46+
if (sourceValue instanceof FetchedValue) {
47+
return ((FetchedValue) sourceValue).localContext;
48+
} else {
49+
return defaultLocalContext;
50+
}
51+
}
52+
2053
public FetchedValue(Object fetchedValue, List<GraphQLError> errors, Object localContext) {
2154
this.fetchedValue = fetchedValue;
2255
this.errors = ImmutableList.copyOf(errors);

‎src/main/java/graphql/execution/SubscriptionExecutionStrategy.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,9 @@ private boolean keepOrdered(GraphQLContext graphQLContext) {
109109
private CompletableFuture<Publisher<Object>> createSourceEventStream(ExecutionContext executionContext, ExecutionStrategyParameters parameters) {
110110
ExecutionStrategyParameters newParameters = firstFieldOfSubscriptionSelection(executionContext, parameters, false);
111111

112-
CompletableFuture<FetchedValue> fieldFetched = Async.toCompletableFuture(fetchField(executionContext, newParameters));
112+
CompletableFuture<Object> fieldFetched = Async.toCompletableFuture(fetchField(executionContext, newParameters));
113113
return fieldFetched.thenApply(fetchedValue -> {
114-
Object publisher = fetchedValue.getFetchedValue();
114+
Object publisher = FetchedValue.getFetchedValue(fetchedValue);
115115
return mkReactivePublisher(publisher);
116116
});
117117
}
@@ -168,7 +168,7 @@ private CompletableFuture<ExecutionResult> executeSubscriptionEvent(ExecutionCon
168168
i13nFieldParameters, executionContext.getInstrumentationState()
169169
));
170170

171-
FetchedValue fetchedValue = unboxPossibleDataFetcherResult(newExecutionContext, newParameters, eventPayload);
171+
Object fetchedValue = unboxPossibleDataFetcherResult(newExecutionContext, newParameters, eventPayload);
172172
FieldValueInfo fieldValueInfo = completeField(newExecutionContext, newParameters, fetchedValue);
173173
executionContext.getDataLoaderDispatcherStrategy().newSubscriptionExecution(fieldValueInfo, newParameters.getDeferredCallContext());
174174
CompletableFuture<ExecutionResult> overallResult = fieldValueInfo

‎src/main/java/graphql/execution/instrumentation/parameters/InstrumentationFieldCompleteParameters.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,13 @@ public ExecutionStepInfo getExecutionStepInfo() {
4848
return executionStepInfo.get();
4949
}
5050

51-
public Object getFetchedValue() {
51+
/**
52+
* This returns the object that was fetched, ready to be completed as a value. This can sometimes be a {@link graphql.execution.FetchedValue} object
53+
* but most often it's a simple POJO.
54+
*
55+
* @return the object was fetched, ready to be completed as a value.
56+
*/
57+
public Object getFetchedObject() {
5258
return fetchedValue;
5359
}
5460

‎src/test/groovy/graphql/execution/BreadthFirstExecutionTestStrategy.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,19 @@ public BreadthFirstExecutionTestStrategy() {
2222
public CompletableFuture<ExecutionResult> execute(ExecutionContext executionContext, ExecutionStrategyParameters parameters) throws NonNullableFieldWasNullException {
2323
MergedSelectionSet fields = parameters.getFields();
2424

25-
Map<String, FetchedValue> fetchedValues = new LinkedHashMap<>();
25+
Map<String, Object> fetchedValues = new LinkedHashMap<>();
2626

2727
// first fetch every value
2828
for (String fieldName : fields.keySet()) {
29-
FetchedValue fetchedValue = fetchField(executionContext, parameters, fields, fieldName);
29+
Object fetchedValue = fetchField(executionContext, parameters, fields, fieldName);
3030
fetchedValues.put(fieldName, fetchedValue);
3131
}
3232

3333
// then for every fetched value, complete it
3434
Map<String, Object> results = new LinkedHashMap<>();
3535
for (String fieldName : fetchedValues.keySet()) {
3636
MergedField currentField = fields.getSubField(fieldName);
37-
FetchedValue fetchedValue = fetchedValues.get(fieldName);
37+
Object fetchedValue = fetchedValues.get(fieldName);
3838

3939
ResultPath fieldPath = parameters.getPath().segment(fieldName);
4040
ExecutionStrategyParameters newParameters = parameters
@@ -51,17 +51,17 @@ public CompletableFuture<ExecutionResult> execute(ExecutionContext executionCont
5151
return CompletableFuture.completedFuture(new ExecutionResultImpl(results, executionContext.getErrors()));
5252
}
5353

54-
private FetchedValue fetchField(ExecutionContext executionContext, ExecutionStrategyParameters parameters, MergedSelectionSet fields, String fieldName) {
54+
private Object fetchField(ExecutionContext executionContext, ExecutionStrategyParameters parameters, MergedSelectionSet fields, String fieldName) {
5555
MergedField currentField = fields.getSubField(fieldName);
5656

5757
ResultPath fieldPath = parameters.getPath().segment(fieldName);
5858
ExecutionStrategyParameters newParameters = parameters
5959
.transform(builder -> builder.field(currentField).path(fieldPath));
6060

61-
return Async.<FetchedValue>toCompletableFuture(fetchField(executionContext, newParameters)).join();
61+
return Async.toCompletableFuture(fetchField(executionContext, newParameters)).join();
6262
}
6363

64-
private void completeValue(ExecutionContext executionContext, Map<String, Object> results, String fieldName, FetchedValue fetchedValue, ExecutionStrategyParameters newParameters) {
64+
private void completeValue(ExecutionContext executionContext, Map<String, Object> results, String fieldName, Object fetchedValue, ExecutionStrategyParameters newParameters) {
6565
Object resolvedResult = completeField(executionContext, newParameters, fetchedValue).getFieldValueFuture().join();
6666
results.put(fieldName, resolvedResult);
6767
}

0 commit comments

Comments
 (0)