From 7100262883ad9958c5d95db174539fe8ef1e4d13 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Wed, 5 Oct 2022 21:59:29 +1100 Subject: [PATCH 1/3] Avoid allocating a type resolve env if the type is already an object type --- .../graphql/execution/ExecutionStrategy.java | 37 +++++++------------ .../java/graphql/execution/ResolveType.java | 26 ++++++------- 2 files changed, 27 insertions(+), 36 deletions(-) diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index 41d80eccf..abfd89eb1 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -349,21 +349,21 @@ protected CompletableFuture handleFetchingException(ExecutionContext exec .build(); try { - return asyncHandleException(dataFetcherExceptionHandler, handlerParameters, executionContext); + return asyncHandleException(dataFetcherExceptionHandler, handlerParameters); } catch (Exception handlerException) { handlerParameters = DataFetcherExceptionHandlerParameters.newExceptionParameters() .dataFetchingEnvironment(environment) .exception(handlerException) .build(); - return asyncHandleException(new SimpleDataFetcherExceptionHandler(), handlerParameters, executionContext); + return asyncHandleException(new SimpleDataFetcherExceptionHandler(), handlerParameters); } } - private CompletableFuture asyncHandleException(DataFetcherExceptionHandler handler, DataFetcherExceptionHandlerParameters handlerParameters, ExecutionContext executionContext) { + private CompletableFuture asyncHandleException(DataFetcherExceptionHandler handler, DataFetcherExceptionHandlerParameters handlerParameters) { //noinspection unchecked - return handler.handleException(handlerParameters) - .thenApply(handlerResult -> (T) DataFetcherResult.newResult().errors(handlerResult.getErrors()).build() - ); + return handler.handleException(handlerParameters).thenApply( + handlerResult -> (T) DataFetcherResult.newResult().errors(handlerResult.getErrors()).build() + ); } /** @@ -620,7 +620,7 @@ protected CompletableFuture completeValueForScalar(ExecutionCon protected CompletableFuture completeValueForEnum(ExecutionContext executionContext, ExecutionStrategyParameters parameters, GraphQLEnumType enumType, Object result) { Object serialized; try { - serialized = enumType.serialize(result); + serialized = enumType.serialize(result, executionContext.getGraphQLContext(), executionContext.getLocale()); } catch (CoercingSerializeException e) { serialized = handleCoercionProblem(executionContext, parameters, e); } @@ -633,7 +633,7 @@ protected CompletableFuture completeValueForEnum(ExecutionConte } /** - * Called to turn an java object value into an graphql object value + * Called to turn a java object value into an graphql object value * * @param executionContext contains the top level execution parameters * @param parameters contains the parameters holding the fields to be executed and source object @@ -649,7 +649,7 @@ protected CompletableFuture completeValueForObject(ExecutionCon .schema(executionContext.getGraphQLSchema()) .objectType(resolvedObjectType) .fragments(executionContext.getFragmentsByName()) - .variables(executionContext.getVariables()) + .variables(executionContext.getCoercedVariables().toMap()) .build(); MergedSelectionSet subFields = fieldCollector.collectFields(collectorParameters, parameters.getField()); @@ -680,20 +680,11 @@ private Object handleCoercionProblem(ExecutionContext context, ExecutionStrategy } - /** - * Converts an object that is known to should be an Iterable into one - * - * @param result the result object - * - * @return an Iterable from that object - * - * @throws java.lang.ClassCastException if it's not an Iterable - */ - protected Iterable toIterable(Object result) { - return FpKit.toIterable(result); - } - protected GraphQLObjectType resolveType(ExecutionContext executionContext, ExecutionStrategyParameters parameters, GraphQLType fieldType) { + // we can avoid a method call and type resolver environment allocation if we know it's an object type + if (fieldType instanceof GraphQLObjectType) { + return (GraphQLObjectType) fieldType; + } return resolvedType.resolveType(executionContext, parameters.getField(), parameters.getSource(), parameters.getExecutionStepInfo(), fieldType, parameters.getLocalContext()); } @@ -743,7 +734,7 @@ protected GraphQLFieldDefinition getFieldDef(GraphQLSchema schema, GraphQLObject } /** - * See (http://facebook.github.io/graphql/#sec-Errors-and-Non-Nullability), + * See (...), *

* If a non nullable child field type actually resolves to a null value and the parent type is nullable * then the parent must in fact become null diff --git a/src/main/java/graphql/execution/ResolveType.java b/src/main/java/graphql/execution/ResolveType.java index fbc9b5f53..82a8465f0 100644 --- a/src/main/java/graphql/execution/ResolveType.java +++ b/src/main/java/graphql/execution/ResolveType.java @@ -1,5 +1,6 @@ package graphql.execution; +import graphql.Assert; import graphql.Internal; import graphql.TypeResolutionEnvironment; import graphql.normalized.ExecutableNormalizedField; @@ -19,11 +20,20 @@ @Internal public class ResolveType { - public GraphQLObjectType resolveType(ExecutionContext executionContext, MergedField field, Object source, ExecutionStepInfo executionStepInfo, GraphQLType fieldType, Object localContext) { - GraphQLObjectType resolvedType; + Assert.assertTrue(fieldType instanceof GraphQLInterfaceType || fieldType instanceof GraphQLUnionType, + () -> "The passed in fieldType MUST be an interface or union type : " + fieldType.getClass().getName()); + TypeResolutionEnvironment env = buildEnv(executionContext, field, source, executionStepInfo, fieldType, localContext); + if (fieldType instanceof GraphQLInterfaceType) { + return resolveTypeForInterface(env, (GraphQLInterfaceType) fieldType); + } else { + return resolveTypeForUnion(env, (GraphQLUnionType) fieldType); + } + } + + private TypeResolutionEnvironment buildEnv(ExecutionContext executionContext, MergedField field, Object source, ExecutionStepInfo executionStepInfo, GraphQLType fieldType, Object localContext) { DataFetchingFieldSelectionSet fieldSelectionSet = buildSelectionSet(executionContext, field, (GraphQLOutputType) fieldType, executionStepInfo); - TypeResolutionEnvironment env = TypeResolutionParameters.newParameters() + return TypeResolutionParameters.newParameters() .field(field) .fieldType(fieldType) .value(source) @@ -34,14 +44,6 @@ public GraphQLObjectType resolveType(ExecutionContext executionContext, MergedFi .localContext(localContext) .schema(executionContext.getGraphQLSchema()) .build(); - if (fieldType instanceof GraphQLInterfaceType) { - resolvedType = resolveTypeForInterface(env, (GraphQLInterfaceType) fieldType); - } else if (fieldType instanceof GraphQLUnionType) { - resolvedType = resolveTypeForUnion(env, (GraphQLUnionType) fieldType); - } else { - resolvedType = (GraphQLObjectType) fieldType; - } - return resolvedType; } private DataFetchingFieldSelectionSet buildSelectionSet(ExecutionContext executionContext, MergedField field, GraphQLOutputType fieldType, ExecutionStepInfo executionStepInfo) { @@ -65,11 +67,9 @@ private GraphQLObjectType resolveAbstractType(TypeResolutionEnvironment env, Typ if (result == null) { throw new UnresolvedTypeException(abstractType); } - if (!env.getSchema().isPossibleType(abstractType, result)) { throw new UnresolvedTypeException(abstractType, result); } - return result; } From 6004840cacc52b9d6e526f8ac70a8c4630cbf281 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Wed, 5 Oct 2022 22:03:24 +1100 Subject: [PATCH 2/3] Avoid allocating a type resolve env if the type is already an object type - restored protected method --- .../java/graphql/execution/ExecutionStrategy.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index abfd89eb1..2286192af 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -679,6 +679,18 @@ private Object handleCoercionProblem(ExecutionContext context, ExecutionStrategy return null; } + /** + * Converts an object that is known to should be an Iterable into one + * + * @param result the result object + * + * @return an Iterable from that object + * + * @throws java.lang.ClassCastException if it's not an Iterable + */ + protected Iterable toIterable(Object result) { + return FpKit.toIterable(result); + } protected GraphQLObjectType resolveType(ExecutionContext executionContext, ExecutionStrategyParameters parameters, GraphQLType fieldType) { // we can avoid a method call and type resolver environment allocation if we know it's an object type From 668c0987f0ae7242b26ae5d597e275f5d0a0e002 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Wed, 5 Oct 2022 22:09:26 +1100 Subject: [PATCH 3/3] Avoid allocating a type resolve env if the type is already an object type - restored direct env allocation code --- src/main/java/graphql/execution/ResolveType.java | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/main/java/graphql/execution/ResolveType.java b/src/main/java/graphql/execution/ResolveType.java index 82a8465f0..3ef9c556f 100644 --- a/src/main/java/graphql/execution/ResolveType.java +++ b/src/main/java/graphql/execution/ResolveType.java @@ -23,17 +23,8 @@ public class ResolveType { public GraphQLObjectType resolveType(ExecutionContext executionContext, MergedField field, Object source, ExecutionStepInfo executionStepInfo, GraphQLType fieldType, Object localContext) { Assert.assertTrue(fieldType instanceof GraphQLInterfaceType || fieldType instanceof GraphQLUnionType, () -> "The passed in fieldType MUST be an interface or union type : " + fieldType.getClass().getName()); - TypeResolutionEnvironment env = buildEnv(executionContext, field, source, executionStepInfo, fieldType, localContext); - if (fieldType instanceof GraphQLInterfaceType) { - return resolveTypeForInterface(env, (GraphQLInterfaceType) fieldType); - } else { - return resolveTypeForUnion(env, (GraphQLUnionType) fieldType); - } - } - - private TypeResolutionEnvironment buildEnv(ExecutionContext executionContext, MergedField field, Object source, ExecutionStepInfo executionStepInfo, GraphQLType fieldType, Object localContext) { DataFetchingFieldSelectionSet fieldSelectionSet = buildSelectionSet(executionContext, field, (GraphQLOutputType) fieldType, executionStepInfo); - return TypeResolutionParameters.newParameters() + TypeResolutionEnvironment env = TypeResolutionParameters.newParameters() .field(field) .fieldType(fieldType) .value(source) @@ -44,6 +35,11 @@ private TypeResolutionEnvironment buildEnv(ExecutionContext executionContext, Me .localContext(localContext) .schema(executionContext.getGraphQLSchema()) .build(); + if (fieldType instanceof GraphQLInterfaceType) { + return resolveTypeForInterface(env, (GraphQLInterfaceType) fieldType); + } else { + return resolveTypeForUnion(env, (GraphQLUnionType) fieldType); + } } private DataFetchingFieldSelectionSet buildSelectionSet(ExecutionContext executionContext, MergedField field, GraphQLOutputType fieldType, ExecutionStepInfo executionStepInfo) {