From 51db09b9099e11b7b77e231690803ad87bafbb61 Mon Sep 17 00:00:00 2001 From: bbaker Date: Fri, 25 Jul 2025 16:25:32 +1000 Subject: [PATCH 1/3] A smidge faster unwrap non-null --- .../graphql/execution/ExecutionStepInfo.java | 12 ++++++++ .../execution/ExecutionStepInfoFactory.java | 2 +- .../graphql/execution/ExecutionStrategy.java | 10 +++---- .../SubscriptionExecutionStrategy.java | 2 +- .../java/graphql/schema/GraphQLTypeUtil.java | 8 +++-- .../graphql/schema/GraphQLTypeUtilTest.groovy | 30 ++++++++++++++++++- 6 files changed, 54 insertions(+), 10 deletions(-) diff --git a/src/main/java/graphql/execution/ExecutionStepInfo.java b/src/main/java/graphql/execution/ExecutionStepInfo.java index eefa8a81cc..c9f27c38c4 100644 --- a/src/main/java/graphql/execution/ExecutionStepInfo.java +++ b/src/main/java/graphql/execution/ExecutionStepInfo.java @@ -126,6 +126,18 @@ public GraphQLOutputType getUnwrappedNonNullType() { return (GraphQLOutputType) GraphQLTypeUtil.unwrapNonNull(this.type); } + /** + * This returns the type which is unwrapped if it was {@link GraphQLNonNull} wrapped + * and then cast to the target type. + * + * @param for two + * + * @return the graphql type in question + */ + public T getUnwrappedNonNullTypeAs() { + return GraphQLTypeUtil.unwrapNonNullAs(this.type); + } + /** * This returns the field definition that is in play when this type info was created or null * if the type is a root query type diff --git a/src/main/java/graphql/execution/ExecutionStepInfoFactory.java b/src/main/java/graphql/execution/ExecutionStepInfoFactory.java index ec2716aec3..286106a7dc 100644 --- a/src/main/java/graphql/execution/ExecutionStepInfoFactory.java +++ b/src/main/java/graphql/execution/ExecutionStepInfoFactory.java @@ -25,7 +25,7 @@ public class ExecutionStepInfoFactory { public ExecutionStepInfo newExecutionStepInfoForListElement(ExecutionStepInfo executionInfo, ResultPath indexedPath) { - GraphQLList fieldType = (GraphQLList) executionInfo.getUnwrappedNonNullType(); + GraphQLList fieldType = executionInfo.getUnwrappedNonNullTypeAs(); GraphQLOutputType typeInList = (GraphQLOutputType) fieldType.getWrappedType(); return executionInfo.transform(typeInList, executionInfo, indexedPath); } diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index a16f0f80f1..feb831b6d6 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -394,7 +394,7 @@ protected Object resolveFieldWithInfo(ExecutionContext executionContext, Executi @DuckTyped(shape = "CompletableFuture | ") protected Object fetchField(ExecutionContext executionContext, ExecutionStrategyParameters parameters) { MergedField field = parameters.getField(); - GraphQLObjectType parentType = (GraphQLObjectType) parameters.getExecutionStepInfo().getUnwrappedNonNullType(); + GraphQLObjectType parentType = parameters.getExecutionStepInfo().getUnwrappedNonNullTypeAs(); GraphQLFieldDefinition fieldDef = getFieldDef(executionContext.getGraphQLSchema(), parentType, field.getSingleField()); return fetchField(fieldDef, executionContext, parameters); } @@ -408,7 +408,7 @@ private Object fetchField(GraphQLFieldDefinition fieldDef, ExecutionContext exec } MergedField field = parameters.getField(); - GraphQLObjectType parentType = (GraphQLObjectType) parameters.getExecutionStepInfo().getUnwrappedNonNullType(); + GraphQLObjectType parentType = parameters.getExecutionStepInfo().getUnwrappedNonNullTypeAs(); // if the DF (like PropertyDataFetcher) does not use the arguments or execution step info then dont build any @@ -615,13 +615,13 @@ protected FieldValueInfo completeField(ExecutionContext executionContext, executionContext.throwIfCancelled(); Field field = parameters.getField().getSingleField(); - GraphQLObjectType parentType = (GraphQLObjectType) parameters.getExecutionStepInfo().getUnwrappedNonNullType(); + GraphQLObjectType parentType = parameters.getExecutionStepInfo().getUnwrappedNonNullTypeAs(); GraphQLFieldDefinition fieldDef = getFieldDef(executionContext.getGraphQLSchema(), parentType, field); return completeField(fieldDef, executionContext, parameters, fetchedValue); } private FieldValueInfo completeField(GraphQLFieldDefinition fieldDef, ExecutionContext executionContext, ExecutionStrategyParameters parameters, Object fetchedValue) { - GraphQLObjectType parentType = (GraphQLObjectType) parameters.getExecutionStepInfo().getUnwrappedNonNullType(); + GraphQLObjectType parentType = parameters.getExecutionStepInfo().getUnwrappedNonNullTypeAs(); ExecutionStepInfo executionStepInfo = createExecutionStepInfo(executionContext, parameters, fieldDef, parentType); Instrumentation instrumentation = executionContext.getInstrumentation(); @@ -1008,7 +1008,7 @@ private boolean incrementAndCheckMaxNodesExceeded(ExecutionContext executionCont * @return a {@link GraphQLFieldDefinition} */ protected GraphQLFieldDefinition getFieldDef(ExecutionContext executionContext, ExecutionStrategyParameters parameters, Field field) { - GraphQLObjectType parentType = (GraphQLObjectType) parameters.getExecutionStepInfo().getUnwrappedNonNullType(); + GraphQLObjectType parentType = parameters.getExecutionStepInfo().getUnwrappedNonNullTypeAs(); return getFieldDef(executionContext.getGraphQLSchema(), parentType, field); } diff --git a/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java b/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java index 45e2192248..d2da978471 100644 --- a/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java +++ b/src/main/java/graphql/execution/SubscriptionExecutionStrategy.java @@ -226,7 +226,7 @@ private ExecutionStrategyParameters firstFieldOfSubscriptionSelection(ExecutionC private ExecutionStepInfo createSubscribedFieldStepInfo(ExecutionContext executionContext, ExecutionStrategyParameters parameters) { Field field = parameters.getField().getSingleField(); - GraphQLObjectType parentType = (GraphQLObjectType) parameters.getExecutionStepInfo().getUnwrappedNonNullType(); + GraphQLObjectType parentType = parameters.getExecutionStepInfo().getUnwrappedNonNullTypeAs(); GraphQLFieldDefinition fieldDef = getFieldDef(executionContext.getGraphQLSchema(), parentType, field); return createExecutionStepInfo(executionContext, parameters, fieldDef, parentType); } diff --git a/src/main/java/graphql/schema/GraphQLTypeUtil.java b/src/main/java/graphql/schema/GraphQLTypeUtil.java index c2a44d641c..af649ff5fc 100644 --- a/src/main/java/graphql/schema/GraphQLTypeUtil.java +++ b/src/main/java/graphql/schema/GraphQLTypeUtil.java @@ -219,15 +219,19 @@ private static GraphQLType unwrapAllImpl(GraphQLType type) { /** - * Unwraps all non nullable layers of the type until it reaches a type that is not {@link GraphQLNonNull} + * Unwraps all non-nullable layers of the type until it reaches a type that is not {@link GraphQLNonNull} * * @param type the type to unwrap * * @return the underlying type that is not {@link GraphQLNonNull} */ public static GraphQLType unwrapNonNull(GraphQLType type) { + // nominally its illegal to have a type that is a non null wrapping a non-null + // but the code is like this just in case and anyway it has to do 1 non-null check + // so this works even if it wont really loop while (isNonNull(type)) { - type = unwrapOne(type); + // is cheaper doing this direct rather than calling #unwrapOne + type = ((GraphQLNonNull) type).getWrappedType(); } return type; } diff --git a/src/test/groovy/graphql/schema/GraphQLTypeUtilTest.groovy b/src/test/groovy/graphql/schema/GraphQLTypeUtilTest.groovy index 39763a0b0d..a2d23ac6a8 100644 --- a/src/test/groovy/graphql/schema/GraphQLTypeUtilTest.groovy +++ b/src/test/groovy/graphql/schema/GraphQLTypeUtilTest.groovy @@ -6,7 +6,7 @@ import static graphql.Scalars.GraphQLString import static graphql.schema.GraphQLList.list import static graphql.schema.GraphQLNonNull.nonNull import static graphql.schema.GraphQLObjectType.newObject -import static graphql.schema.GraphQLTypeReference.* +import static graphql.schema.GraphQLTypeReference.typeRef class GraphQLTypeUtilTest extends Specification { @@ -205,4 +205,32 @@ class GraphQLTypeUtilTest extends Specification { then: !GraphQLTypeUtil.isInput(type) } + + def "can unwrap non null-ness"() { + + when: + def type = GraphQLTypeUtil.unwrapNonNull(nonNull(GraphQLString)) + + then: + (type as GraphQLNamedType).getName() == "String" + + when: + type = GraphQLTypeUtil.unwrapNonNull(nonNull(list(GraphQLString))) + + then: + type instanceof GraphQLList + + when: + type = GraphQLTypeUtil.unwrapNonNull(list(GraphQLString)) + + then: + type instanceof GraphQLList + + when: + type = GraphQLTypeUtil.unwrapNonNull(GraphQLString) + + then: + (type as GraphQLNamedType).getName() == "String" + + } } From 4dc1f344b641c212f76617d3b02646fdc631c9e0 Mon Sep 17 00:00:00 2001 From: bbaker Date: Fri, 25 Jul 2025 17:02:25 +1000 Subject: [PATCH 2/3] A smidge faster unwrap non-null - updated to only do 1 check --- src/main/java/graphql/schema/GraphQLNonNull.java | 4 ++-- src/main/java/graphql/schema/GraphQLTypeUtil.java | 14 +++++++------- .../graphql/schema/GraphQLTypeUtilTest.groovy | 6 +++--- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/main/java/graphql/schema/GraphQLNonNull.java b/src/main/java/graphql/schema/GraphQLNonNull.java index 12f131b428..6de1ba61d3 100644 --- a/src/main/java/graphql/schema/GraphQLNonNull.java +++ b/src/main/java/graphql/schema/GraphQLNonNull.java @@ -46,8 +46,8 @@ public GraphQLNonNull(GraphQLType wrappedType) { } private void assertNonNullWrapping(GraphQLType wrappedType) { - assertTrue(!GraphQLTypeUtil.isNonNull(wrappedType), - "A non null type cannot wrap an existing non null type '%s'", GraphQLTypeUtil.simplePrint(wrappedType)); + assertTrue(!GraphQLTypeUtil.isNonNull(wrappedType), () -> + String.format("A non null type cannot wrap an existing non null type '%s'", GraphQLTypeUtil.simplePrint(wrappedType))); } @Override diff --git a/src/main/java/graphql/schema/GraphQLTypeUtil.java b/src/main/java/graphql/schema/GraphQLTypeUtil.java index af649ff5fc..ef933eb291 100644 --- a/src/main/java/graphql/schema/GraphQLTypeUtil.java +++ b/src/main/java/graphql/schema/GraphQLTypeUtil.java @@ -219,17 +219,17 @@ private static GraphQLType unwrapAllImpl(GraphQLType type) { /** - * Unwraps all non-nullable layers of the type until it reaches a type that is not {@link GraphQLNonNull} + * Unwraps a single non-nullable layer of the type if its present. Note there can + * only ever be one non-nullable wrapping of a type and this is enforced by {@link GraphQLNonNull} * * @param type the type to unwrap * * @return the underlying type that is not {@link GraphQLNonNull} */ public static GraphQLType unwrapNonNull(GraphQLType type) { - // nominally its illegal to have a type that is a non null wrapping a non-null - // but the code is like this just in case and anyway it has to do 1 non-null check - // so this works even if it wont really loop - while (isNonNull(type)) { + // its illegal to have a type that is a non-null wrapping a non-null type + // and GraphQLNonNull has code that prevents it so we can just check once during the unwrapping + if (isNonNull(type)) { // is cheaper doing this direct rather than calling #unwrapOne type = ((GraphQLNonNull) type).getWrappedType(); } @@ -237,8 +237,8 @@ public static GraphQLType unwrapNonNull(GraphQLType type) { } /** - * Unwraps all non nullable layers of the type until it reaches a type that is not {@link GraphQLNonNull} - * and then cast to the target type. + * Unwraps a single non-nullable layer of the type if its present and then cast to the target type. Note there can + * only ever be one non-nullable wrapping of a type and this is enforced by {@link GraphQLNonNull} * * @param type the type to unwrap * @param for two diff --git a/src/test/groovy/graphql/schema/GraphQLTypeUtilTest.groovy b/src/test/groovy/graphql/schema/GraphQLTypeUtilTest.groovy index a2d23ac6a8..bc8a08aff1 100644 --- a/src/test/groovy/graphql/schema/GraphQLTypeUtilTest.groovy +++ b/src/test/groovy/graphql/schema/GraphQLTypeUtilTest.groovy @@ -215,19 +215,19 @@ class GraphQLTypeUtilTest extends Specification { (type as GraphQLNamedType).getName() == "String" when: - type = GraphQLTypeUtil.unwrapNonNull(nonNull(list(GraphQLString))) + type = GraphQLTypeUtil.unwrapNonNull(nonNull(list(GraphQLString))) then: type instanceof GraphQLList when: - type = GraphQLTypeUtil.unwrapNonNull(list(GraphQLString)) + type = GraphQLTypeUtil.unwrapNonNull(list(GraphQLString)) then: type instanceof GraphQLList when: - type = GraphQLTypeUtil.unwrapNonNull(GraphQLString) + type = GraphQLTypeUtil.unwrapNonNull(GraphQLString) then: (type as GraphQLNamedType).getName() == "String" From 7e0a66ef021ce4ec27cfc53515a82c173efd6b99 Mon Sep 17 00:00:00 2001 From: bbaker Date: Fri, 25 Jul 2025 17:12:36 +1000 Subject: [PATCH 3/3] A smidge faster unwrap non-null - asssert tweak --- src/main/java/graphql/schema/GraphQLNonNull.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/graphql/schema/GraphQLNonNull.java b/src/main/java/graphql/schema/GraphQLNonNull.java index 6de1ba61d3..44762d1017 100644 --- a/src/main/java/graphql/schema/GraphQLNonNull.java +++ b/src/main/java/graphql/schema/GraphQLNonNull.java @@ -47,7 +47,7 @@ public GraphQLNonNull(GraphQLType wrappedType) { private void assertNonNullWrapping(GraphQLType wrappedType) { assertTrue(!GraphQLTypeUtil.isNonNull(wrappedType), () -> - String.format("A non null type cannot wrap an existing non null type '%s'", GraphQLTypeUtil.simplePrint(wrappedType))); + "A non null type cannot wrap an existing non null type"); } @Override