A smidge faster unwrap non-null - updated to only do 1 check

bbakerman · bbakerman · commit 4dc1f344b641 · 2025-07-25T17:02:25.000+10:00
diff --git 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
@@ -219,26 +219,26 @@ 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();
         }
         return 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 <T>  for two
diff --git 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"

Original file line number	Diff line number	Diff line change
`@@ -46,8 +46,8 @@ public GraphQLNonNull(GraphQLType wrappedType) {`
`46`	`46`	`}`
`47`	`47`
`48`	`48`	`private void assertNonNullWrapping(GraphQLType wrappedType) {`
`49`		`- assertTrue(!GraphQLTypeUtil.isNonNull(wrappedType),`
`50`		`- "A non null type cannot wrap an existing non null type '%s'", GraphQLTypeUtil.simplePrint(wrappedType));`
	`49`	`+ assertTrue(!GraphQLTypeUtil.isNonNull(wrappedType), () ->`
	`50`	`+ String.format("A non null type cannot wrap an existing non null type '%s'", GraphQLTypeUtil.simplePrint(wrappedType)));`
`51`	`51`	`}`
`52`	`52`
`53`	`53`	`@Override`