From 3d02d42a44c728422000ee25cb1139dff3d39c74 Mon Sep 17 00:00:00 2001 From: James Bellenger Date: Wed, 8 Apr 2026 11:04:36 -0700 Subject: [PATCH 1/3] checkpoint --- .../InputObjectHasUnbreakableCycle.java | 132 ++++++ .../validation/NoUnbrokenInputCycles.java | 91 ---- .../validation/OneOfInputObjectRules.java | 44 -- .../validation/SchemaValidationErrorType.java | 1 - .../schema/validation/SchemaValidator.java | 2 +- .../InputObjectHasUnbreakableCycleTest.groovy | 408 ++++++++++++++++++ .../NoUnbrokenInputCyclesTest.groovy | 46 -- .../OneOfInputObjectRulesTest.groovy | 153 ------- .../validation/SchemaValidatorTest.groovy | 2 +- 9 files changed, 542 insertions(+), 337 deletions(-) create mode 100644 src/main/java/graphql/schema/validation/InputObjectHasUnbreakableCycle.java delete mode 100644 src/main/java/graphql/schema/validation/NoUnbrokenInputCycles.java create mode 100644 src/test/groovy/graphql/schema/validation/InputObjectHasUnbreakableCycleTest.groovy delete mode 100644 src/test/groovy/graphql/schema/validation/NoUnbrokenInputCyclesTest.groovy diff --git a/src/main/java/graphql/schema/validation/InputObjectHasUnbreakableCycle.java b/src/main/java/graphql/schema/validation/InputObjectHasUnbreakableCycle.java new file mode 100644 index 0000000000..90511b76b1 --- /dev/null +++ b/src/main/java/graphql/schema/validation/InputObjectHasUnbreakableCycle.java @@ -0,0 +1,132 @@ +package graphql.schema.validation; + +import graphql.Internal; +import graphql.schema.GraphQLInputObjectField; +import graphql.schema.GraphQLInputObjectType; +import graphql.schema.GraphQLInputType; +import graphql.schema.GraphQLSchemaElement; +import graphql.schema.GraphQLTypeVisitorStub; +import graphql.util.TraversalControl; +import graphql.util.TraverserContext; + +import java.util.LinkedHashSet; +import java.util.Set; + +import static graphql.schema.GraphQLTypeUtil.isList; +import static graphql.schema.GraphQLTypeUtil.isNonNull; +import static graphql.schema.GraphQLTypeUtil.unwrapNonNullAs; +import static java.lang.String.format; + +/** + * Implements the spec's {@code InputObjectHasUnbreakableCycle} validation algorithm. + *

+ * This unifies the old "no unbroken non-null input cycles" check with OneOf input + * inhabitability into a single rule. An input object has an unbreakable cycle when + * no finite value can be constructed for it: + *

+ * + * @see graphql-spec PR #1211 + */ +@Internal +public class InputObjectHasUnbreakableCycle extends GraphQLTypeVisitorStub { + + private final Set knownNoCycle = new LinkedHashSet<>(); + + @Override + public TraversalControl visitGraphQLInputObjectType(GraphQLInputObjectType inputObjectType, + TraverserContext context) { + if (hasUnbreakableCycle(inputObjectType, new LinkedHashSet<>())) { + SchemaValidationErrorCollector errorCollector = + context.getVarFromParents(SchemaValidationErrorCollector.class); + String message = format( + "Input Object %s forms an unbreakable cycle and cannot be constructed.", + inputObjectType.getName()); + errorCollector.addError( + new SchemaValidationError(SchemaValidationErrorType.UnbrokenInputCycle, message)); + } + return TraversalControl.CONTINUE; + } + + /** + * Determines whether the given input object type has an unbreakable cycle, + * meaning no finite value can ever be constructed for it. + * + * @param inputObject the input object type to check + * @param visited the set of input object types already visited in this traversal path + * @return true if the type has an unbreakable cycle + */ + private boolean hasUnbreakableCycle(GraphQLInputObjectType inputObject, + Set visited) { + if (knownNoCycle.contains(inputObject)) { + return false; + } + if (visited.contains(inputObject)) { + return true; + } + + visited.add(inputObject); + + boolean result; + + if (inputObject.isOneOf()) { + // OneOf: unbreakable cycle if EVERY field has one (all escape routes blocked) + result = true; + for (GraphQLInputObjectField field : inputObject.getFieldDefinitions()) { + if (!fieldTypeHasUnbreakableCycle(field.getType(), visited)) { + result = false; + break; + } + } + } else { + // Regular: unbreakable cycle if ANY non-null field has one + result = false; + for (GraphQLInputObjectField field : inputObject.getFieldDefinitions()) { + if (isNonNull(field.getType())) { + GraphQLInputType nullableType = unwrapNonNullAs(field.getType()); + if (fieldTypeHasUnbreakableCycle(nullableType, visited)) { + result = true; + break; + } + } + } + } + + visited.remove(inputObject); + + if (!result) { + knownNoCycle.add(inputObject); + } + return result; + } + + /** + * Determines whether a field's type participates in an unbreakable cycle. + * + * @param fieldType the field type to check + * @param visited the set of input object types already visited in this traversal path + * @return true if the field type has an unbreakable cycle + */ + private boolean fieldTypeHasUnbreakableCycle(GraphQLInputType fieldType, + Set visited) { + if (isList(fieldType)) { + // Lists break cycles -- an empty list is always a valid value + return false; + } + if (isNonNull(fieldType)) { + return fieldTypeHasUnbreakableCycle(unwrapNonNullAs(fieldType), visited); + } + if (!(fieldType instanceof GraphQLInputObjectType)) { + // Scalars, enums, etc. break cycles + return false; + } + return hasUnbreakableCycle((GraphQLInputObjectType) fieldType, visited); + } +} diff --git a/src/main/java/graphql/schema/validation/NoUnbrokenInputCycles.java b/src/main/java/graphql/schema/validation/NoUnbrokenInputCycles.java deleted file mode 100644 index 22be3f7148..0000000000 --- a/src/main/java/graphql/schema/validation/NoUnbrokenInputCycles.java +++ /dev/null @@ -1,91 +0,0 @@ -package graphql.schema.validation; - -import graphql.Internal; -import graphql.schema.GraphQLArgument; -import graphql.schema.GraphQLFieldDefinition; -import graphql.schema.GraphQLInputObjectField; -import graphql.schema.GraphQLInputObjectType; -import graphql.schema.GraphQLInputType; -import graphql.schema.GraphQLList; -import graphql.schema.GraphQLNonNull; -import graphql.schema.GraphQLSchemaElement; -import graphql.schema.GraphQLType; -import graphql.schema.GraphQLTypeVisitorStub; -import graphql.util.TraversalControl; -import graphql.util.TraverserContext; - -import java.util.ArrayList; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Set; - -import static graphql.schema.GraphQLTypeUtil.isList; -import static graphql.schema.GraphQLTypeUtil.isNonNull; -import static graphql.schema.GraphQLTypeUtil.unwrapAll; - -/** - * Schema validation rule ensuring no input type forms an unbroken non-nullable recursion, - * as such a type would be impossible to satisfy - */ -@Internal -public class NoUnbrokenInputCycles extends GraphQLTypeVisitorStub { - - @Override - public TraversalControl visitGraphQLFieldDefinition(GraphQLFieldDefinition fieldDef, TraverserContext context) { - SchemaValidationErrorCollector validationErrorCollector = context.getVarFromParents(SchemaValidationErrorCollector.class); - for (GraphQLArgument argument : fieldDef.getArguments()) { - GraphQLInputType argumentType = argument.getType(); - if (argumentType instanceof GraphQLInputObjectType) { - List path = new ArrayList<>(); - check((GraphQLInputObjectType) argumentType, new LinkedHashSet<>(), path, validationErrorCollector); - } - } - return TraversalControl.CONTINUE; - } - - private void check(GraphQLInputObjectType type, Set seen, List path, SchemaValidationErrorCollector validationErrorCollector) { - if (seen.contains(type)) { - validationErrorCollector.addError(new SchemaValidationError(SchemaValidationErrorType.UnbrokenInputCycle, getErrorMessage(path))); - return; - } - seen.add(type); - - for (GraphQLInputObjectField field : type.getFieldDefinitions()) { - if (isNonNull(field.getType())) { - GraphQLType unwrapped = unwrapNonNull((GraphQLNonNull) field.getType()); - if (unwrapped instanceof GraphQLInputObjectType) { - path = new ArrayList<>(path); - path.add(field.getName() + "!"); - check((GraphQLInputObjectType) unwrapped, new LinkedHashSet<>(seen), path, validationErrorCollector); - } - } - } - } - - private GraphQLType unwrapNonNull(GraphQLNonNull type) { - if (isList(type.getWrappedType())) { - //we only care about [type!]! i.e. non-null lists of non-nulls - GraphQLList listType = (GraphQLList) type.getWrappedType(); - if (isNonNull(listType.getWrappedType())) { - return unwrapAll(listType.getWrappedType()); - } else { - return type.getWrappedType(); - } - } else { - return unwrapAll(type.getWrappedType()); - } - } - - private String getErrorMessage(List path) { - StringBuilder message = new StringBuilder(); - message.append("["); - for (int i = 0; i < path.size(); i++) { - if (i != 0) { - message.append("."); - } - message.append(path.get(i)); - } - message.append("] forms an unsatisfiable cycle"); - return message.toString(); - } -} diff --git a/src/main/java/graphql/schema/validation/OneOfInputObjectRules.java b/src/main/java/graphql/schema/validation/OneOfInputObjectRules.java index b41a7e570b..87e39fc321 100644 --- a/src/main/java/graphql/schema/validation/OneOfInputObjectRules.java +++ b/src/main/java/graphql/schema/validation/OneOfInputObjectRules.java @@ -4,16 +4,11 @@ import graphql.schema.GraphQLInputObjectField; import graphql.schema.GraphQLInputObjectType; import graphql.schema.GraphQLSchemaElement; -import graphql.schema.GraphQLType; import graphql.schema.GraphQLTypeUtil; import graphql.schema.GraphQLTypeVisitorStub; -import graphql.schema.GraphQLUnmodifiedType; import graphql.util.TraversalControl; import graphql.util.TraverserContext; -import java.util.LinkedHashSet; -import java.util.Set; - import static java.lang.String.format; /* @@ -24,45 +19,6 @@ @ExperimentalApi public class OneOfInputObjectRules extends GraphQLTypeVisitorStub { - @Override - public TraversalControl visitGraphQLInputObjectType(GraphQLInputObjectType inputObjectType, TraverserContext context) { - if (!inputObjectType.isOneOf()) { - return TraversalControl.CONTINUE; - } - SchemaValidationErrorCollector errorCollector = context.getVarFromParents(SchemaValidationErrorCollector.class); - if (!canBeProvidedAFiniteValue(inputObjectType, new LinkedHashSet<>())) { - String message = format("OneOf Input Object %s must be inhabited but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle.", inputObjectType.getName()); - errorCollector.addError(new SchemaValidationError(SchemaValidationErrorType.OneOfNotInhabited, message)); - } - return TraversalControl.CONTINUE; - } - - private boolean canBeProvidedAFiniteValue(GraphQLInputObjectType oneOfInputObject, Set visited) { - if (visited.contains(oneOfInputObject)) { - return false; - } - Set nextVisited = new LinkedHashSet<>(visited); - nextVisited.add(oneOfInputObject); - for (GraphQLInputObjectField field : oneOfInputObject.getFieldDefinitions()) { - GraphQLType fieldType = field.getType(); - if (GraphQLTypeUtil.isList(fieldType)) { - return true; - } - GraphQLUnmodifiedType namedFieldType = GraphQLTypeUtil.unwrapAll(fieldType); - if (!(namedFieldType instanceof GraphQLInputObjectType)) { - return true; - } - GraphQLInputObjectType inputFieldType = (GraphQLInputObjectType) namedFieldType; - if (!inputFieldType.isOneOf()) { - return true; - } - if (canBeProvidedAFiniteValue(inputFieldType, nextVisited)) { - return true; - } - } - return false; - } - @Override public TraversalControl visitGraphQLInputObjectField(GraphQLInputObjectField inputObjectField, TraverserContext context) { GraphQLInputObjectType inputObjectType = (GraphQLInputObjectType) context.getParentNode(); diff --git a/src/main/java/graphql/schema/validation/SchemaValidationErrorType.java b/src/main/java/graphql/schema/validation/SchemaValidationErrorType.java index b1caecc8e4..b8392b18d7 100644 --- a/src/main/java/graphql/schema/validation/SchemaValidationErrorType.java +++ b/src/main/java/graphql/schema/validation/SchemaValidationErrorType.java @@ -22,7 +22,6 @@ public enum SchemaValidationErrorType implements SchemaValidationErrorClassifica InputTypeUsedInOutputTypeContext, OneOfDefaultValueOnField, OneOfNonNullableField, - OneOfNotInhabited, RequiredInputFieldCannotBeDeprecated, RequiredFieldArgumentCannotBeDeprecated, RequiredDirectiveArgumentCannotBeDeprecated diff --git a/src/main/java/graphql/schema/validation/SchemaValidator.java b/src/main/java/graphql/schema/validation/SchemaValidator.java index 1f676fb77e..4f9ba48463 100644 --- a/src/main/java/graphql/schema/validation/SchemaValidator.java +++ b/src/main/java/graphql/schema/validation/SchemaValidator.java @@ -18,7 +18,7 @@ public class SchemaValidator { private final List rules = new ArrayList<>(); public SchemaValidator() { - rules.add(new NoUnbrokenInputCycles()); + rules.add(new InputObjectHasUnbreakableCycle()); rules.add(new TypesImplementInterfaces()); rules.add(new TypeAndFieldRule()); rules.add(new DefaultValuesAreValid()); diff --git a/src/test/groovy/graphql/schema/validation/InputObjectHasUnbreakableCycleTest.groovy b/src/test/groovy/graphql/schema/validation/InputObjectHasUnbreakableCycleTest.groovy new file mode 100644 index 0000000000..4aa07ff4eb --- /dev/null +++ b/src/test/groovy/graphql/schema/validation/InputObjectHasUnbreakableCycleTest.groovy @@ -0,0 +1,408 @@ +package graphql.schema.validation + +import graphql.TestUtil +import graphql.schema.idl.SchemaGenerator +import graphql.schema.idl.SchemaParser +import spock.lang.Specification + +/** + * Tests for the unified InputObjectHasUnbreakableCycle validation rule, + * which implements the spec's InputObjectHasUnbreakableCycle algorithm. + * + * Ported from graphql-js PR #4564 and the existing graphql-java tests. + * + * @see graphql-spec PR #1211 + * @see graphql-js PR #4564 + */ +class InputObjectHasUnbreakableCycleTest extends Specification { + + // ------------------------------------------------------------------ + // Valid schemas: breakable cycles and inhabited types + // ------------------------------------------------------------------ + + def "accepts an Input Object with breakable circular reference"() { + def sdl = """ + type Query { f(arg: SomeInputObject): String } + input SomeInputObject { + self: SomeInputObject + arrayOfSelf: [SomeInputObject] + nonNullArrayOfSelf: [SomeInputObject]! + nonNullArrayOfNonNullSelf: [SomeInputObject!]! + intermediateSelf: AnotherInputObject + } + input AnotherInputObject { + parent: SomeInputObject + } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + noExceptionThrown() + } + + def "accepts a OneOf Input Object with a scalar field"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf { a: Int } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + noExceptionThrown() + } + + def "accepts a OneOf Input Object with a recursive list field"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf { a: [A] } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + noExceptionThrown() + } + + def "accepts a OneOf Input Object referencing a non-OneOf input object"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf { b: B } + input B { x: Int } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + noExceptionThrown() + } + + def "accepts a OneOf/OneOf cycle with a scalar escape"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf { b: B, escape: Int } + input B @oneOf { a: A } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + noExceptionThrown() + } + + def "accepts a OneOf/non-OneOf cycle with a nullable escape"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf { b: B } + input B { a: A } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + noExceptionThrown() + } + + def "accepts a non-OneOf/non-OneOf cycle with a nullable escape"() { + def sdl = """ + type Query { f(arg: A): String } + input A { b: B! } + input B { a: A } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + noExceptionThrown() + } + + def "accepts a non-OneOf/non-OneOf cycle with a list escape"() { + def sdl = """ + type Query { f(arg: A): String } + input A { b: [B!]! } + input B { a: A! } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + noExceptionThrown() + } + + def "accepts a OneOf/non-OneOf with scalar escape"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf { b: B, escape: Int } + input B { a: A! } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + noExceptionThrown() + } + + def "accepts multiple fields with chained oneOf escape"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf { b: B, c: C } + input B @oneOf { a: A } + input C @oneOf { a: A, escape: String } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + noExceptionThrown() + } + + def "accepts a OneOf with enum field"() { + def sdl = """ + type Query { f(arg: A): String } + enum Color { RED GREEN BLUE } + input A @oneOf { a: Color } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + noExceptionThrown() + } + + def "accepts oneOf with scalar fields"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf { a: String, b: Int } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + noExceptionThrown() + } + + def "accepts mutually referencing oneOf types with scalar escape"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf { b: B } + input B @oneOf { a: A, escape: Int } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + noExceptionThrown() + } + + def "accepts oneOf referencing non-oneOf with back-reference"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf { b: RegularInput } + input RegularInput { back: A } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + noExceptionThrown() + } + + def "accepts a OneOf Input Object with a non-null recursive list field"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf { a: [A!] } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + noExceptionThrown() + } + + // ------------------------------------------------------------------ + // Invalid schemas: unbreakable cycles + // ------------------------------------------------------------------ + + def "rejects an Input Object with non-breakable self-reference"() { + def sdl = """ + type Query { f(arg: SomeInputObject): String } + input SomeInputObject { + nonNullSelf: SomeInputObject! + } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + def schemaProblem = thrown(InvalidSchemaException) + schemaProblem.errors.any { + it.classification == SchemaValidationErrorType.UnbrokenInputCycle + } + } + + def "rejects Input Objects with non-breakable circular reference spread across them"() { + def sdl = """ + type Query { f(arg: SomeInputObject): String } + input SomeInputObject { + startLoop: AnotherInputObject! + } + input AnotherInputObject { + nextInLoop: YetAnotherInputObject! + } + input YetAnotherInputObject { + closeLoop: SomeInputObject! + } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + def schemaProblem = thrown(InvalidSchemaException) + schemaProblem.errors.every { + it.classification == SchemaValidationErrorType.UnbrokenInputCycle + } + // Each type in the cycle gets its own error + schemaProblem.errors.size() == 3 + } + + def "rejects Input Objects with multiple non-breakable circular references"() { + def sdl = """ + type Query { f(arg: SomeInputObject): String } + input SomeInputObject { + startLoop: AnotherInputObject! + } + input AnotherInputObject { + closeLoop: SomeInputObject! + startSecondLoop: YetAnotherInputObject! + } + input YetAnotherInputObject { + closeSecondLoop: AnotherInputObject! + nonNullSelf: YetAnotherInputObject! + } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + def schemaProblem = thrown(InvalidSchemaException) + schemaProblem.errors.every { + it.classification == SchemaValidationErrorType.UnbrokenInputCycle + } + // All three types participate in unbreakable cycles + schemaProblem.errors.size() == 3 + } + + def "rejects a self-referencing OneOf type with no escapes"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf { self: A } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + def schemaProblem = thrown(InvalidSchemaException) + schemaProblem.errors.size() == 1 + schemaProblem.errors[0].classification == SchemaValidationErrorType.UnbrokenInputCycle + } + + def "rejects a mixed OneOf/non-OneOf cycle with no escapes"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf { b: B } + input B { a: A! } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + def schemaProblem = thrown(InvalidSchemaException) + schemaProblem.errors.every { + it.classification == SchemaValidationErrorType.UnbrokenInputCycle + } + // Both A and B have unbreakable cycles + schemaProblem.errors.size() == 2 + } + + def "rejects a larger mixed OneOf/non-OneOf cycle with no escapes"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf { b: B } + input B { c: C! } + input C @oneOf { a: A } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + def schemaProblem = thrown(InvalidSchemaException) + schemaProblem.errors.every { + it.classification == SchemaValidationErrorType.UnbrokenInputCycle + } + // All three types participate in the cycle + schemaProblem.errors.size() == 3 + } + + def "rejects multiple oneOf types forming a cycle with no escapes"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf { b: B } + input B @oneOf { c: C } + input C @oneOf { a: A } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + def schemaProblem = thrown(InvalidSchemaException) + schemaProblem.errors.every { + it.classification == SchemaValidationErrorType.UnbrokenInputCycle + } + schemaProblem.errors.size() == 3 + } +} diff --git a/src/test/groovy/graphql/schema/validation/NoUnbrokenInputCyclesTest.groovy b/src/test/groovy/graphql/schema/validation/NoUnbrokenInputCyclesTest.groovy deleted file mode 100644 index 2ead346116..0000000000 --- a/src/test/groovy/graphql/schema/validation/NoUnbrokenInputCyclesTest.groovy +++ /dev/null @@ -1,46 +0,0 @@ -package graphql.schema.validation - -import graphql.schema.GraphQLArgument -import graphql.schema.GraphQLFieldDefinition -import graphql.schema.GraphQLInputObjectField -import graphql.schema.GraphQLInputObjectType -import graphql.util.TraverserContext -import spock.lang.Specification - -import static graphql.Scalars.GraphQLBoolean -import static graphql.schema.GraphQLFieldDefinition.newFieldDefinition -import static graphql.schema.GraphQLInputObjectType.newInputObject -import static graphql.schema.GraphQLNonNull.nonNull -import static graphql.schema.GraphQLTypeReference.typeRef - -class NoUnbrokenInputCyclesTest extends Specification { - - SchemaValidationErrorCollector errorCollector = new SchemaValidationErrorCollector() - - def "infinitely recursive input type results in error"() { - given: - GraphQLInputObjectType PersonInputType = newInputObject() - .name("Person") - .field(GraphQLInputObjectField.newInputObjectField() - .name("friend") - .type(typeRef("Person")) - .build()) - .build() - - GraphQLFieldDefinition field = newFieldDefinition() - .name("exists") - .type(GraphQLBoolean) - .argument(GraphQLArgument.newArgument() - .name("person") - .type(PersonInputType)) - .build() - - PersonInputType.getFieldDefinition("friend").replacedType = nonNull(PersonInputType) - def context = Mock(TraverserContext) - context.getVarFromParents(SchemaValidationErrorCollector) >> errorCollector - when: - new NoUnbrokenInputCycles().visitGraphQLFieldDefinition(field, context) - then: - errorCollector.containsValidationError(SchemaValidationErrorType.UnbrokenInputCycle) - } -} diff --git a/src/test/groovy/graphql/schema/validation/OneOfInputObjectRulesTest.groovy b/src/test/groovy/graphql/schema/validation/OneOfInputObjectRulesTest.groovy index f91dca44e2..776c4cbdae 100644 --- a/src/test/groovy/graphql/schema/validation/OneOfInputObjectRulesTest.groovy +++ b/src/test/groovy/graphql/schema/validation/OneOfInputObjectRulesTest.groovy @@ -34,157 +34,4 @@ class OneOfInputObjectRulesTest extends Specification { schemaProblem.errors[1].classification == SchemaValidationErrorType.OneOfDefaultValueOnField } - def "oneOf with scalar fields is inhabited"() { - def sdl = """ - type Query { f(arg: A): String } - input A @oneOf { a: String, b: Int } - """ - - when: - def registry = new SchemaParser().parse(sdl) - new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) - - then: - noExceptionThrown() - } - - def "oneOf with enum field is inhabited"() { - def sdl = """ - type Query { f(arg: A): String } - enum Color { RED GREEN BLUE } - input A @oneOf { a: Color } - """ - - when: - def registry = new SchemaParser().parse(sdl) - new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) - - then: - noExceptionThrown() - } - - def "oneOf with list field is inhabited"() { - def sdl = """ - type Query { f(arg: A): String } - input A @oneOf { a: [A] } - """ - - when: - def registry = new SchemaParser().parse(sdl) - new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) - - then: - noExceptionThrown() - } - - def "oneOf referencing non-oneOf input is inhabited"() { - def sdl = """ - type Query { f(arg: A): String } - input A @oneOf { a: RegularInput } - input RegularInput { x: String } - """ - - when: - def registry = new SchemaParser().parse(sdl) - new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) - - then: - noExceptionThrown() - } - - def "oneOf with escape field is inhabited"() { - def sdl = """ - type Query { f(arg: A): String } - input A @oneOf { b: B, escape: String } - input B @oneOf { a: A } - """ - - when: - def registry = new SchemaParser().parse(sdl) - new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) - - then: - noExceptionThrown() - } - - def "mutually referencing oneOf types with scalar escape is inhabited"() { - def sdl = """ - type Query { f(arg: A): String } - input A @oneOf { b: B } - input B @oneOf { a: A, escape: Int } - """ - - when: - def registry = new SchemaParser().parse(sdl) - new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) - - then: - noExceptionThrown() - } - - def "oneOf referencing non-oneOf with back-reference is inhabited"() { - def sdl = """ - type Query { f(arg: A): String } - input A @oneOf { b: RegularInput } - input RegularInput { back: A } - """ - - when: - def registry = new SchemaParser().parse(sdl) - new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) - - then: - noExceptionThrown() - } - - def "multiple fields with chained oneOf escape is inhabited"() { - def sdl = """ - type Query { f(arg: A): String } - input A @oneOf { b: B, c: C } - input B @oneOf { a: A } - input C @oneOf { a: A, escape: String } - """ - - when: - def registry = new SchemaParser().parse(sdl) - new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) - - then: - noExceptionThrown() - } - - def "single oneOf self-reference cycle is not inhabited"() { - def sdl = """ - type Query { f(arg: A): String } - input A @oneOf { self: A } - """ - - when: - def registry = new SchemaParser().parse(sdl) - new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) - - then: - def schemaProblem = thrown(InvalidSchemaException) - schemaProblem.errors.size() == 1 - schemaProblem.errors[0].description == "OneOf Input Object A must be inhabited but all fields recursively reference only other OneOf Input Objects forming an unresolvable cycle." - schemaProblem.errors[0].classification == SchemaValidationErrorType.OneOfNotInhabited - } - - def "multiple oneOf types forming cycle are not inhabited"() { - def sdl = """ - type Query { f(arg: A): String } - input A @oneOf { b: B } - input B @oneOf { c: C } - input C @oneOf { a: A } - """ - - when: - def registry = new SchemaParser().parse(sdl) - new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) - - then: - def schemaProblem = thrown(InvalidSchemaException) - schemaProblem.errors.size() == 3 - schemaProblem.errors.every { it.classification == SchemaValidationErrorType.OneOfNotInhabited } - } } diff --git a/src/test/groovy/graphql/schema/validation/SchemaValidatorTest.groovy b/src/test/groovy/graphql/schema/validation/SchemaValidatorTest.groovy index 706542df8d..b9a7596688 100644 --- a/src/test/groovy/graphql/schema/validation/SchemaValidatorTest.groovy +++ b/src/test/groovy/graphql/schema/validation/SchemaValidatorTest.groovy @@ -12,7 +12,7 @@ class SchemaValidatorTest extends Specification { def rules = validator.rules then: rules.size() == 9 - rules[0] instanceof NoUnbrokenInputCycles + rules[0] instanceof InputObjectHasUnbreakableCycle rules[1] instanceof TypesImplementInterfaces rules[2] instanceof TypeAndFieldRule rules[3] instanceof DefaultValuesAreValid From 2de6a558e58dbf1624519b274899d2eaad298a7c Mon Sep 17 00:00:00 2001 From: James Bellenger Date: Sat, 22 Aug 2026 05:56:22 -0700 Subject: [PATCH 2/3] Align input object cycle detection with graphql-js --- .../InputObjectHasUnbreakableCycle.java | 390 ++++++++++++++---- .../InputObjectHasUnbreakableCycleTest.groovy | 195 ++++++++- 2 files changed, 478 insertions(+), 107 deletions(-) diff --git a/src/main/java/graphql/schema/validation/InputObjectHasUnbreakableCycle.java b/src/main/java/graphql/schema/validation/InputObjectHasUnbreakableCycle.java index 90511b76b1..097f48b131 100644 --- a/src/main/java/graphql/schema/validation/InputObjectHasUnbreakableCycle.java +++ b/src/main/java/graphql/schema/validation/InputObjectHasUnbreakableCycle.java @@ -1,132 +1,348 @@ package graphql.schema.validation; import graphql.Internal; +import graphql.collect.ImmutableKit; import graphql.schema.GraphQLInputObjectField; import graphql.schema.GraphQLInputObjectType; import graphql.schema.GraphQLInputType; +import graphql.schema.GraphQLNonNull; +import graphql.schema.GraphQLSchema; import graphql.schema.GraphQLSchemaElement; +import graphql.schema.GraphQLType; import graphql.schema.GraphQLTypeVisitorStub; import graphql.util.TraversalControl; import graphql.util.TraverserContext; +import org.jspecify.annotations.Nullable; +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Deque; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; import java.util.Set; -import static graphql.schema.GraphQLTypeUtil.isList; -import static graphql.schema.GraphQLTypeUtil.isNonNull; -import static graphql.schema.GraphQLTypeUtil.unwrapNonNullAs; import static java.lang.String.format; /** - * Implements the spec's {@code InputObjectHasUnbreakableCycle} validation algorithm. - *

- * This unifies the old "no unbroken non-null input cycles" check with OneOf input - * inhabitability into a single rule. An input object has an unbreakable cycle when - * no finite value can be constructed for it: - *

    - *
  • For regular input objects: any non-null field leading to an unbreakable cycle - * makes the type uninhabitable (all non-null fields must be satisfied).
    • - *
  • For OneOf input objects: all fields leading to unbreakable cycles makes the - * type uninhabitable (only one field must be provided, but if every option - * leads to a cycle, there is no escape).
    • - *
  • List types always break cycles (an empty list is a valid value).
    • - *
  • Scalar and enum types always break cycles.
    • - *
- * - * @see graphql-spec PR #1211 + * Validates that every input object type can be provided a finite value. */ @Internal public class InputObjectHasUnbreakableCycle extends GraphQLTypeVisitorStub { - private final Set knownNoCycle = new LinkedHashSet<>(); + /** + * The fields on each input object that lead to another input object and can keep a cycle going. + * For example, given these types: + * input A @oneOf { b: B, value: Int } + * input B { value: Int } + * This contains only A.b for A. + */ + private final Map> targetFields = new LinkedHashMap<>(); + + /** + * The input object reached by each target field. + * For example, given these types: + * input A @oneOf { b: B } + * input B { value: Int } + * A.b maps to B. + */ + private final Map fieldTargets = new LinkedHashMap<>(); + + /** + * The input objects that point to a given input object. + * For example, given these types: + * input A { b: B! } + * input B { value: Int } + * B lists A as a dependent. + */ + private final Map> dependents = new LinkedHashMap<>(); + + /** The total number of fields on each input object, including scalar, enum, and list fields. */ + private final Map fieldCounts = new LinkedHashMap<>(); + + /** + * For each regular input object, the number of required input object fields + * that are not yet known to have a finite value. + */ + private final Map unresolvedTargetCounts = new LinkedHashMap<>(); + + /** The input objects for which a finite value is known to exist. */ + private final Set typesWithFiniteValues = new LinkedHashSet<>(); + + /** Input objects newly known to have a finite value, but whose dependents have not yet been updated. */ + private final Deque typesToPropagate = new ArrayDeque<>(); + + /** + * True after this validator has checked the whole schema. + * The schema visitor calls this validator once for each input object, but the first call checks every input + * object in the schema. This prevents the same work from running again on later calls. + */ + private boolean complete; @Override - public TraversalControl visitGraphQLInputObjectType(GraphQLInputObjectType inputObjectType, - TraverserContext context) { - if (hasUnbreakableCycle(inputObjectType, new LinkedHashSet<>())) { - SchemaValidationErrorCollector errorCollector = - context.getVarFromParents(SchemaValidationErrorCollector.class); - String message = format( - "Input Object %s forms an unbreakable cycle and cannot be constructed.", - inputObjectType.getName()); - errorCollector.addError( - new SchemaValidationError(SchemaValidationErrorType.UnbrokenInputCycle, message)); + public TraversalControl visitGraphQLInputObjectType( + GraphQLInputObjectType inputObjectType, + TraverserContext context + ) { + // This validator runs once per schema. Continue if we've already run. + if (complete) { + return TraversalControl.CONTINUE; } + complete = true; + + GraphQLSchema schema = context.getVarFromParents(GraphQLSchema.class); + SchemaValidationErrorCollector errorCollector = context.getVarFromParents(SchemaValidationErrorCollector.class); + List inputObjectTypes = getInputObjectTypes(schema); + detectInputObjectNonFiniteValues(inputObjectTypes, errorCollector); return TraversalControl.CONTINUE; } - /** - * Determines whether the given input object type has an unbreakable cycle, - * meaning no finite value can ever be constructed for it. - * - * @param inputObject the input object type to check - * @param visited the set of input object types already visited in this traversal path - * @return true if the type has an unbreakable cycle - */ - private boolean hasUnbreakableCycle(GraphQLInputObjectType inputObject, - Set visited) { - if (knownNoCycle.contains(inputObject)) { - return false; + private List getInputObjectTypes(GraphQLSchema schema) { + return ImmutableKit.filterAndMap( + schema.getTypeMap().values(), + GraphQLInputObjectType.class::isInstance, + GraphQLInputObjectType.class::cast + ); + } + + private void detectInputObjectNonFiniteValues( + List inputObjectTypes, + SchemaValidationErrorCollector errorCollector + ) { + /* + * Finds input objects that cannot be given a finite value, using a worklist algorithm. + * See https://en.wikipedia.org/wiki/Data-flow_analysis#The_work_list_algorithm. + * + * First, this records which input objects depend on other input objects. It then starts with the types that + * clearly have a finite value and adds them to a work list: + * - A regular input object has a finite value when all of its required input object fields have + * finite values. + * - A OneOf input object has a finite value when any one of its fields has a finite value. + * + * Each new finite type may therefore make more types finite. This continues until the work list is empty. + * We can then rescan the total list of input object types, and any type that hasn't been proven to have a + * finite value is inferred to have a non-finite value. + * + * Performance note: + * If I is the number of input object types and F is the total number of fields on them, then the analysis takes + * O(I + F) time and uses O(I + F) extra space. + */ + + // Prepare a place to store the state of each input object. + initializeInputObjectStates(inputObjectTypes); + + // Record the input object fields that can keep a cycle going. + recordFiniteValueTargets(inputObjectTypes); + + // Start with the input objects that clearly have a finite value. + initializeFiniteValueWorklist(inputObjectTypes); + + // Use those finite values to find other input objects with finite values. + propagateFiniteValues(); + + // Report cycles among the input objects that are still not known to have a finite value. + reportUnbreakableCycles(inputObjectTypes, errorCollector); + } + + private void initializeInputObjectStates(List inputObjectTypes) { + for (GraphQLInputObjectType inputObjectType : inputObjectTypes) { + targetFields.put(inputObjectType, new ArrayList<>()); + dependents.put(inputObjectType, new ArrayList<>()); + unresolvedTargetCounts.put(inputObjectType, 0); + } + } + + private void recordFiniteValueTargets(List inputObjectTypes) { + for (GraphQLInputObjectType inputObjectType : inputObjectTypes) { + List fields = inputObjectType.getFieldDefinitions(); + fieldCounts.put(inputObjectType, fields.size()); + for (GraphQLInputObjectField field : fields) { + recordFiniteValueTarget(inputObjectType, field); + } + } + } + + private void recordFiniteValueTarget( + GraphQLInputObjectType inputObjectType, + GraphQLInputObjectField field + ) { + GraphQLInputObjectType target = getFiniteValueTarget(inputObjectType, field.getType()); + if (target == null) { + return; } - if (visited.contains(inputObject)) { - return true; + + targetFields.get(inputObjectType).add(field); + fieldTargets.put(field, target); + List targetDependents = dependents.get(target); + if (targetDependents == null) { + return; } + targetDependents.add(inputObjectType); + } - visited.add(inputObject); + private void initializeFiniteValueWorklist(List inputObjectTypes) { + for (GraphQLInputObjectType inputObjectType : inputObjectTypes) { + List targets = targetFields.get(inputObjectType); + if (inputObjectType.isOneOf()) { + initializeOneOfFiniteValue(inputObjectType, targets); + continue; + } - boolean result; + unresolvedTargetCounts.put(inputObjectType, targets.size()); + if (targets.isEmpty()) { + markInputObjectHasFiniteValue(inputObjectType); + } + } + } + + private void initializeOneOfFiniteValue( + GraphQLInputObjectType inputObjectType, + List targets + ) { + int fieldCount = fieldCounts.get(inputObjectType); + if (fieldCount == 0 || targets.size() < fieldCount) { + markInputObjectHasFiniteValue(inputObjectType); + } + } - if (inputObject.isOneOf()) { - // OneOf: unbreakable cycle if EVERY field has one (all escape routes blocked) - result = true; - for (GraphQLInputObjectField field : inputObject.getFieldDefinitions()) { - if (!fieldTypeHasUnbreakableCycle(field.getType(), visited)) { - result = false; - break; - } + private void propagateFiniteValues() { + while (!typesToPropagate.isEmpty()) { + GraphQLInputObjectType finiteType = typesToPropagate.removeLast(); + for (GraphQLInputObjectType dependent : dependents.get(finiteType)) { + propagateFiniteValueToDependent(dependent); } - } else { - // Regular: unbreakable cycle if ANY non-null field has one - result = false; - for (GraphQLInputObjectField field : inputObject.getFieldDefinitions()) { - if (isNonNull(field.getType())) { - GraphQLInputType nullableType = unwrapNonNullAs(field.getType()); - if (fieldTypeHasUnbreakableCycle(nullableType, visited)) { - result = true; - break; - } - } + } + } + + private void propagateFiniteValueToDependent(GraphQLInputObjectType dependent) { + if (typesWithFiniteValues.contains(dependent)) { + return; + } + if (dependent.isOneOf()) { + markInputObjectHasFiniteValue(dependent); + return; + } + + int unresolvedTargetCount = unresolvedTargetCounts.get(dependent) - 1; + unresolvedTargetCounts.put(dependent, unresolvedTargetCount); + if (unresolvedTargetCount == 0) { + markInputObjectHasFiniteValue(dependent); + } + } + + private void markInputObjectHasFiniteValue(GraphQLInputObjectType inputObjectType) { + if (!typesWithFiniteValues.add(inputObjectType)) { + return; + } + typesToPropagate.add(inputObjectType); + } + + @Nullable + private GraphQLInputObjectType getFiniteValueTarget( + GraphQLInputObjectType inputObjectType, + GraphQLInputType fieldType + ) { + if (inputObjectType.isOneOf()) { + if (fieldType instanceof GraphQLInputObjectType) { + return (GraphQLInputObjectType) fieldType; } + return null; + } + if (!(fieldType instanceof GraphQLNonNull)) { + return null; } - visited.remove(inputObject); + GraphQLType nullableType = ((GraphQLNonNull) fieldType).getWrappedType(); + if (nullableType instanceof GraphQLInputObjectType) { + return (GraphQLInputObjectType) nullableType; + } + return null; + } - if (!result) { - knownNoCycle.add(inputObject); + private void reportUnbreakableCycles( + List inputObjectTypes, + SchemaValidationErrorCollector errorCollector + ) { + Set visitedTypes = new LinkedHashSet<>(); + List fieldPath = new ArrayList<>(); + Map fieldPathIndexByType = new LinkedHashMap<>(); + for (GraphQLInputObjectType inputObjectType : inputObjectTypes) { + if (typesWithFiniteValues.contains(inputObjectType)) { + continue; + } + reportCycleRecursive( + inputObjectType, + visitedTypes, + fieldPath, + fieldPathIndexByType, + errorCollector + ); } - return result; } - /** - * Determines whether a field's type participates in an unbreakable cycle. - * - * @param fieldType the field type to check - * @param visited the set of input object types already visited in this traversal path - * @return true if the field type has an unbreakable cycle - */ - private boolean fieldTypeHasUnbreakableCycle(GraphQLInputType fieldType, - Set visited) { - if (isList(fieldType)) { - // Lists break cycles -- an empty list is always a valid value - return false; + private void reportCycleRecursive( + GraphQLInputObjectType inputObjectType, + Set visitedTypes, + List fieldPath, + Map fieldPathIndexByType, + SchemaValidationErrorCollector errorCollector + ) { + if (!visitedTypes.add(inputObjectType)) { + return; } - if (isNonNull(fieldType)) { - return fieldTypeHasUnbreakableCycle(unwrapNonNullAs(fieldType), visited); + fieldPathIndexByType.put(inputObjectType, fieldPath.size()); + + for (GraphQLInputObjectField field : targetFields.get(inputObjectType)) { + reportTargetCycle( + inputObjectType, + field, + visitedTypes, + fieldPath, + fieldPathIndexByType, + errorCollector + ); } - if (!(fieldType instanceof GraphQLInputObjectType)) { - // Scalars, enums, etc. break cycles - return false; + fieldPathIndexByType.remove(inputObjectType); + } + + private void reportTargetCycle( + GraphQLInputObjectType inputObjectType, + GraphQLInputObjectField field, + Set visitedTypes, + List fieldPath, + Map fieldPathIndexByType, + SchemaValidationErrorCollector errorCollector + ) { + GraphQLInputObjectType target = fieldTargets.get(field); + if (!unresolvedTargetCounts.containsKey(target) || typesWithFiniteValues.contains(target)) { + return; } - return hasUnbreakableCycle((GraphQLInputObjectType) fieldType, visited); + + Integer cycleIndex = fieldPathIndexByType.get(target); + fieldPath.add(inputObjectType.getName() + "." + field.getName()); + if (cycleIndex == null) { + reportCycleRecursive(target, visitedTypes, fieldPath, fieldPathIndexByType, errorCollector); + } else { + addCycleError(target, cycleIndex, fieldPath, errorCollector); + } + fieldPath.remove(fieldPath.size() - 1); + } + + private void addCycleError( + GraphQLInputObjectType inputObjectType, + int cycleIndex, + List fieldPath, + SchemaValidationErrorCollector errorCollector + ) { + List cyclePath = fieldPath.subList(cycleIndex, fieldPath.size()); + String message = format( + "Input Object %s cannot be provided a finite value because it references itself through fields: %s.", + inputObjectType.getName(), + String.join(", ", cyclePath) + ); + errorCollector.addError( + new SchemaValidationError(SchemaValidationErrorType.UnbrokenInputCycle, message) + ); } } diff --git a/src/test/groovy/graphql/schema/validation/InputObjectHasUnbreakableCycleTest.groovy b/src/test/groovy/graphql/schema/validation/InputObjectHasUnbreakableCycleTest.groovy index 4aa07ff4eb..bff1155d32 100644 --- a/src/test/groovy/graphql/schema/validation/InputObjectHasUnbreakableCycleTest.groovy +++ b/src/test/groovy/graphql/schema/validation/InputObjectHasUnbreakableCycleTest.groovy @@ -86,6 +86,37 @@ class InputObjectHasUnbreakableCycleTest extends Specification { noExceptionThrown() } + def "accepts a OneOf Input Object referencing an already checked input object"() { + def sdl = """ + type Query { f(arg: A): String } + input B { value: Int } + input A @oneOf { b: B } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + noExceptionThrown() + } + + def "accepts a OneOf Input Object with multiple acyclic input object fields"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf { b: B, c: C } + input B { value: Int } + input C { value: Int } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + noExceptionThrown() + } + def "accepts a OneOf/OneOf cycle with a scalar escape"() { def sdl = """ type Query { f(arg: A): String } @@ -268,9 +299,9 @@ class InputObjectHasUnbreakableCycleTest extends Specification { then: def schemaProblem = thrown(InvalidSchemaException) - schemaProblem.errors.any { - it.classification == SchemaValidationErrorType.UnbrokenInputCycle - } + schemaProblem.errors.size() == 1 + schemaProblem.errors[0].classification == SchemaValidationErrorType.UnbrokenInputCycle + schemaProblem.errors[0].description == "Input Object SomeInputObject cannot be provided a finite value because it references itself through fields: SomeInputObject.nonNullSelf." } def "rejects Input Objects with non-breakable circular reference spread across them"() { @@ -293,11 +324,9 @@ class InputObjectHasUnbreakableCycleTest extends Specification { then: def schemaProblem = thrown(InvalidSchemaException) - schemaProblem.errors.every { - it.classification == SchemaValidationErrorType.UnbrokenInputCycle - } - // Each type in the cycle gets its own error - schemaProblem.errors.size() == 3 + schemaProblem.errors.size() == 1 + schemaProblem.errors[0].classification == SchemaValidationErrorType.UnbrokenInputCycle + schemaProblem.errors[0].description == "Input Object AnotherInputObject cannot be provided a finite value because it references itself through fields: AnotherInputObject.nextInLoop, YetAnotherInputObject.closeLoop, SomeInputObject.startLoop." } def "rejects Input Objects with multiple non-breakable circular references"() { @@ -325,8 +354,12 @@ class InputObjectHasUnbreakableCycleTest extends Specification { schemaProblem.errors.every { it.classification == SchemaValidationErrorType.UnbrokenInputCycle } - // All three types participate in unbreakable cycles schemaProblem.errors.size() == 3 + schemaProblem.errors*.description as Set == [ + "Input Object AnotherInputObject cannot be provided a finite value because it references itself through fields: AnotherInputObject.closeLoop, SomeInputObject.startLoop.", + "Input Object AnotherInputObject cannot be provided a finite value because it references itself through fields: AnotherInputObject.startSecondLoop, YetAnotherInputObject.closeSecondLoop.", + "Input Object YetAnotherInputObject cannot be provided a finite value because it references itself through fields: YetAnotherInputObject.nonNullSelf.", + ] as Set } def "rejects a self-referencing OneOf type with no escapes"() { @@ -343,6 +376,7 @@ class InputObjectHasUnbreakableCycleTest extends Specification { def schemaProblem = thrown(InvalidSchemaException) schemaProblem.errors.size() == 1 schemaProblem.errors[0].classification == SchemaValidationErrorType.UnbrokenInputCycle + schemaProblem.errors[0].description == "Input Object A cannot be provided a finite value because it references itself through fields: A.self." } def "rejects a mixed OneOf/non-OneOf cycle with no escapes"() { @@ -358,11 +392,9 @@ class InputObjectHasUnbreakableCycleTest extends Specification { then: def schemaProblem = thrown(InvalidSchemaException) - schemaProblem.errors.every { - it.classification == SchemaValidationErrorType.UnbrokenInputCycle - } - // Both A and B have unbreakable cycles - schemaProblem.errors.size() == 2 + schemaProblem.errors.size() == 1 + schemaProblem.errors[0].classification == SchemaValidationErrorType.UnbrokenInputCycle + schemaProblem.errors[0].description == "Input Object A cannot be provided a finite value because it references itself through fields: A.b, B.a." } def "rejects a larger mixed OneOf/non-OneOf cycle with no escapes"() { @@ -379,11 +411,9 @@ class InputObjectHasUnbreakableCycleTest extends Specification { then: def schemaProblem = thrown(InvalidSchemaException) - schemaProblem.errors.every { - it.classification == SchemaValidationErrorType.UnbrokenInputCycle - } - // All three types participate in the cycle - schemaProblem.errors.size() == 3 + schemaProblem.errors.size() == 1 + schemaProblem.errors[0].classification == SchemaValidationErrorType.UnbrokenInputCycle + schemaProblem.errors[0].description == "Input Object A cannot be provided a finite value because it references itself through fields: A.b, B.c, C.a." } def "rejects multiple oneOf types forming a cycle with no escapes"() { @@ -400,9 +430,134 @@ class InputObjectHasUnbreakableCycleTest extends Specification { then: def schemaProblem = thrown(InvalidSchemaException) + schemaProblem.errors.size() == 1 + schemaProblem.errors[0].classification == SchemaValidationErrorType.UnbrokenInputCycle + schemaProblem.errors[0].description == "Input Object A cannot be provided a finite value because it references itself through fields: A.b, B.c, C.a." + } + + def "reports only the underlying cycle when a regular Input Object requires an unbreakable OneOf"() { + def sdl = """ + type Query { f(arg: A): String } + input T @oneOf { self: T } + input A { t: T! } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + def schemaProblem = thrown(InvalidSchemaException) + schemaProblem.errors.size() == 1 + schemaProblem.errors[0].classification == SchemaValidationErrorType.UnbrokenInputCycle + schemaProblem.errors[0].description == "Input Object T cannot be provided a finite value because it references itself through fields: T.self." + } + + def "reports each distinct cycle through multiple OneOf branches"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf { b: B, c: C } + input B { a: A! } + input C { a: A! } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + def schemaProblem = thrown(InvalidSchemaException) + schemaProblem.errors.size() == 2 schemaProblem.errors.every { it.classification == SchemaValidationErrorType.UnbrokenInputCycle } - schemaProblem.errors.size() == 3 + schemaProblem.errors*.description as Set == [ + "Input Object A cannot be provided a finite value because it references itself through fields: A.b, B.a.", + "Input Object A cannot be provided a finite value because it references itself through fields: A.c, C.a.", + ] as Set + } + + def "ignores required scalar list and finite input fields when reporting a cycle"() { + def sdl = """ + type Query { f(arg: A): String } + input A { + list: [B]! + finite: Finite! + b: B! + } + input B { + value: Int! + a: A! + } + input Finite { + value: Int! + } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + def schemaProblem = thrown(InvalidSchemaException) + schemaProblem.errors.size() == 1 + schemaProblem.errors[0].classification == SchemaValidationErrorType.UnbrokenInputCycle + schemaProblem.errors[0].description == "Input Object A cannot be provided a finite value because it references itself through fields: A.b, B.a." + } + + def "reports a shared unbreakable OneOf subgraph once"() { + def chainLength = 20 + def inputTypes = (1..chainLength) + .collect { index -> "input T${index} @oneOf { a: T${index - 1}, b: T${index - 1} }" } + .join("\n") + def sdl = """ + type Query { f(arg: T${chainLength}): String } + input T0 @oneOf { self: T0 } + ${inputTypes} + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + def schemaProblem = thrown(InvalidSchemaException) + schemaProblem.errors.size() == 1 + schemaProblem.errors[0].classification == SchemaValidationErrorType.UnbrokenInputCycle + schemaProblem.errors[0].description == "Input Object T0 cannot be provided a finite value because it references itself through fields: T0.self." + } + + def "does not add an unbreakable cycle error for an empty OneOf"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + def schemaProblem = thrown(InvalidSchemaException) + schemaProblem.errors.size() == 1 + schemaProblem.errors[0].classification == SchemaValidationErrorType.InputObjectTypeLackOfFieldError + schemaProblem.errors[0].description == '"A" must define one or more fields.' + } + + def "does not add an unbreakable cycle error for a non-null OneOf field"() { + def sdl = """ + type Query { f(arg: A): String } + input A @oneOf { self: A! } + """ + + when: + def registry = new SchemaParser().parse(sdl) + new SchemaGenerator().makeExecutableSchema(registry, TestUtil.getMockRuntimeWiring()) + + then: + def schemaProblem = thrown(InvalidSchemaException) + schemaProblem.errors.size() == 1 + schemaProblem.errors[0].classification == SchemaValidationErrorType.OneOfNonNullableField + schemaProblem.errors[0].description == "OneOf input field A.self must be nullable." } } From b616984d4aac17cf44fb0d27c8c57b0a88c3759c Mon Sep 17 00:00:00 2001 From: James Bellenger Date: Sun, 23 Aug 2026 06:11:35 -0700 Subject: [PATCH 3/3] Add input object cycle validation benchmarks --- ...putObjectHasUnbreakableCycleBenchmark.java | 112 +++++++++++++++ .../InputObjectHasUnbreakableCycle.java | 18 +-- .../finiteValuePropagation.graphqls | 46 ++++++ .../tenTypeMixedInputCycle.graphqls | 43 ++++++ .../tenTypeTenFieldsInputCycle.graphqls | 133 ++++++++++++++++++ .../twoTypeMixedInputCycle.graphqls | 11 ++ 6 files changed, 348 insertions(+), 15 deletions(-) create mode 100644 src/jmh/java/benchmark/InputObjectHasUnbreakableCycleBenchmark.java create mode 100644 src/test/resources/InputObjectHasUnbreakableCycleBenchmark/finiteValuePropagation.graphqls create mode 100644 src/test/resources/InputObjectHasUnbreakableCycleBenchmark/tenTypeMixedInputCycle.graphqls create mode 100644 src/test/resources/InputObjectHasUnbreakableCycleBenchmark/tenTypeTenFieldsInputCycle.graphqls create mode 100644 src/test/resources/InputObjectHasUnbreakableCycleBenchmark/twoTypeMixedInputCycle.graphqls diff --git a/src/jmh/java/benchmark/InputObjectHasUnbreakableCycleBenchmark.java b/src/jmh/java/benchmark/InputObjectHasUnbreakableCycleBenchmark.java new file mode 100644 index 0000000000..8d1320972e --- /dev/null +++ b/src/jmh/java/benchmark/InputObjectHasUnbreakableCycleBenchmark.java @@ -0,0 +1,112 @@ +package benchmark; + +import graphql.schema.GraphQLSchema; +import graphql.schema.GraphQLTypeVisitor; +import graphql.schema.SchemaTraverser; +import graphql.schema.idl.FastSchemaGenerator; +import graphql.schema.idl.RuntimeWiring; +import graphql.schema.idl.SchemaGenerator; +import graphql.schema.idl.SchemaParser; +import graphql.schema.idl.TypeDefinitionRegistry; +import graphql.schema.validation.InputObjectHasUnbreakableCycle; +import graphql.schema.validation.SchemaValidationError; +import graphql.schema.validation.SchemaValidationErrorCollector; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Level; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.infra.Blackhole; + +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +/** Measures full-schema traversal with only the input object cycle validator. */ +@State(Scope.Thread) +@BenchmarkMode(Mode.AverageTime) +@Warmup(iterations = 2, time = 5) +@Measurement(iterations = 3) +@Fork(2) +@OutputTimeUnit(TimeUnit.MICROSECONDS) +public class InputObjectHasUnbreakableCycleBenchmark { + + private GraphQLSchema twoTypeMixedInputCycle; + private GraphQLSchema tenTypeMixedInputCycle; + private GraphQLSchema tenTypeTenFieldsInputCycle; + private GraphQLSchema finiteValuePropagation; + private SchemaTraverser schemaTraverser; + private List cycleValidators; + private SchemaValidationErrorCollector errorCollector; + private Map, Object> rootVariables; + + @Setup(Level.Trial) + public void setUpSchemas() { + twoTypeMixedInputCycle = loadSchema( + "InputObjectHasUnbreakableCycleBenchmark/twoTypeMixedInputCycle.graphqls" + ); + tenTypeMixedInputCycle = loadSchema( + "InputObjectHasUnbreakableCycleBenchmark/tenTypeMixedInputCycle.graphqls" + ); + tenTypeTenFieldsInputCycle = loadSchema( + "InputObjectHasUnbreakableCycleBenchmark/tenTypeTenFieldsInputCycle.graphqls" + ); + finiteValuePropagation = loadSchema( + "InputObjectHasUnbreakableCycleBenchmark/finiteValuePropagation.graphqls" + ); + schemaTraverser = new SchemaTraverser(); + } + + @Setup(Level.Invocation) + public void setUpInvocation() { + cycleValidators = Collections.singletonList(new InputObjectHasUnbreakableCycle()); + errorCollector = new SchemaValidationErrorCollector(); + rootVariables = new LinkedHashMap<>(); + rootVariables.put(SchemaValidationErrorCollector.class, errorCollector); + } + + @Benchmark + public void twoTypeMixedInputCycle(Blackhole blackhole) { + validate(twoTypeMixedInputCycle, blackhole); + } + + @Benchmark + public void tenTypeMixedInputCycle(Blackhole blackhole) { + validate(tenTypeMixedInputCycle, blackhole); + } + + @Benchmark + public void tenTypeTenFieldsInputCycle(Blackhole blackhole) { + validate(tenTypeTenFieldsInputCycle, blackhole); + } + + @Benchmark + public void finiteValuePropagation(Blackhole blackhole) { + validate(finiteValuePropagation, blackhole); + } + + private void validate(GraphQLSchema schema, Blackhole blackhole) { + schemaTraverser.depthFirstFullSchema(cycleValidators, schema, rootVariables); + for (SchemaValidationError error : errorCollector.getErrors()) { + blackhole.consume(error); + } + } + + private GraphQLSchema loadSchema(String schemaPath) { + String schemaString = BenchmarkUtils.loadResource(schemaPath); + TypeDefinitionRegistry typeRegistry = new SchemaParser().parse(schemaString); + return new FastSchemaGenerator().makeExecutableSchema( + SchemaGenerator.Options.defaultOptions().withValidation(false), + typeRegistry, + RuntimeWiring.MOCKED_WIRING + ); + } +} diff --git a/src/main/java/graphql/schema/validation/InputObjectHasUnbreakableCycle.java b/src/main/java/graphql/schema/validation/InputObjectHasUnbreakableCycle.java index 097f48b131..073869819d 100644 --- a/src/main/java/graphql/schema/validation/InputObjectHasUnbreakableCycle.java +++ b/src/main/java/graphql/schema/validation/InputObjectHasUnbreakableCycle.java @@ -111,22 +111,10 @@ private void detectInputObjectNonFiniteValues( SchemaValidationErrorCollector errorCollector ) { /* - * Finds input objects that cannot be given a finite value, using a worklist algorithm. - * See https://en.wikipedia.org/wiki/Data-flow_analysis#The_work_list_algorithm. + * Finds input objects that cannot be given a finite value. * - * First, this records which input objects depend on other input objects. It then starts with the types that - * clearly have a finite value and adds them to a work list: - * - A regular input object has a finite value when all of its required input object fields have - * finite values. - * - A OneOf input object has a finite value when any one of its fields has a finite value. - * - * Each new finite type may therefore make more types finite. This continues until the work list is empty. - * We can then rescan the total list of input object types, and any type that hasn't been proven to have a - * finite value is inferred to have a non-finite value. - * - * Performance note: - * If I is the number of input object types and F is the total number of fields on them, then the analysis takes - * O(I + F) time and uses O(I + F) extra space. + * Start with types known to have a finite value, then repeatedly mark types that become finite because of them. + * Once no more types can be marked, report cycles among the remaining types. */ // Prepare a place to store the state of each input object. diff --git a/src/test/resources/InputObjectHasUnbreakableCycleBenchmark/finiteValuePropagation.graphqls b/src/test/resources/InputObjectHasUnbreakableCycleBenchmark/finiteValuePropagation.graphqls new file mode 100644 index 0000000000..c7456f0be6 --- /dev/null +++ b/src/test/resources/InputObjectHasUnbreakableCycleBenchmark/finiteValuePropagation.graphqls @@ -0,0 +1,46 @@ +type Query { + check(input: Finite7): Boolean +} + +input Seed { + value: Int! +} + +input Finite1 { + previous: Seed! +} + +input Finite2 @oneOf { + previous: Finite1 + trap: TrapA +} + +input Finite3 { + previous: Finite2! +} + +input Finite4 @oneOf { + previous: Finite3 + trap: TrapA +} + +input Finite5 { + previous: Finite4! +} + +input Finite6 @oneOf { + previous: Finite5 + trap: TrapA +} + +input Finite7 { + previous: Finite6! +} + +input TrapA @oneOf { + next: TrapB +} + +input TrapB { + next: TrapA! +} diff --git a/src/test/resources/InputObjectHasUnbreakableCycleBenchmark/tenTypeMixedInputCycle.graphqls b/src/test/resources/InputObjectHasUnbreakableCycleBenchmark/tenTypeMixedInputCycle.graphqls new file mode 100644 index 0000000000..647c288bf9 --- /dev/null +++ b/src/test/resources/InputObjectHasUnbreakableCycleBenchmark/tenTypeMixedInputCycle.graphqls @@ -0,0 +1,43 @@ +type Query { + check(input: Cycle0): Boolean +} + +input Cycle0 @oneOf { + next: Cycle1 +} + +input Cycle1 { + next: Cycle2! +} + +input Cycle2 @oneOf { + next: Cycle3 +} + +input Cycle3 { + next: Cycle4! +} + +input Cycle4 @oneOf { + next: Cycle5 +} + +input Cycle5 { + next: Cycle6! +} + +input Cycle6 @oneOf { + next: Cycle7 +} + +input Cycle7 { + next: Cycle8! +} + +input Cycle8 @oneOf { + next: Cycle9 +} + +input Cycle9 { + next: Cycle0! +} diff --git a/src/test/resources/InputObjectHasUnbreakableCycleBenchmark/tenTypeTenFieldsInputCycle.graphqls b/src/test/resources/InputObjectHasUnbreakableCycleBenchmark/tenTypeTenFieldsInputCycle.graphqls new file mode 100644 index 0000000000..aec629776f --- /dev/null +++ b/src/test/resources/InputObjectHasUnbreakableCycleBenchmark/tenTypeTenFieldsInputCycle.graphqls @@ -0,0 +1,133 @@ +type Query { + check(input: Wide0): Boolean +} + +input Wide0 { + requiredNext: Wide1! + reference1: Wide1 + reference2: Wide2 + reference3: Wide3 + reference4: Wide4 + reference5: Wide5 + reference6: Wide6 + reference7: Wide7 + reference8: Wide8 + reference9: Wide9 +} + +input Wide1 { + requiredNext: Wide2! + reference1: Wide2 + reference2: Wide3 + reference3: Wide4 + reference4: Wide5 + reference5: Wide6 + reference6: Wide7 + reference7: Wide8 + reference8: Wide9 + reference9: Wide0 +} + +input Wide2 { + requiredNext: Wide3! + reference1: Wide3 + reference2: Wide4 + reference3: Wide5 + reference4: Wide6 + reference5: Wide7 + reference6: Wide8 + reference7: Wide9 + reference8: Wide0 + reference9: Wide1 +} + +input Wide3 { + requiredNext: Wide4! + reference1: Wide4 + reference2: Wide5 + reference3: Wide6 + reference4: Wide7 + reference5: Wide8 + reference6: Wide9 + reference7: Wide0 + reference8: Wide1 + reference9: Wide2 +} + +input Wide4 { + requiredNext: Wide5! + reference1: Wide5 + reference2: Wide6 + reference3: Wide7 + reference4: Wide8 + reference5: Wide9 + reference6: Wide0 + reference7: Wide1 + reference8: Wide2 + reference9: Wide3 +} + +input Wide5 { + requiredNext: Wide6! + reference1: Wide6 + reference2: Wide7 + reference3: Wide8 + reference4: Wide9 + reference5: Wide0 + reference6: Wide1 + reference7: Wide2 + reference8: Wide3 + reference9: Wide4 +} + +input Wide6 { + requiredNext: Wide7! + reference1: Wide7 + reference2: Wide8 + reference3: Wide9 + reference4: Wide0 + reference5: Wide1 + reference6: Wide2 + reference7: Wide3 + reference8: Wide4 + reference9: Wide5 +} + +input Wide7 { + requiredNext: Wide8! + reference1: Wide8 + reference2: Wide9 + reference3: Wide0 + reference4: Wide1 + reference5: Wide2 + reference6: Wide3 + reference7: Wide4 + reference8: Wide5 + reference9: Wide6 +} + +input Wide8 { + requiredNext: Wide9! + reference1: Wide9 + reference2: Wide0 + reference3: Wide1 + reference4: Wide2 + reference5: Wide3 + reference6: Wide4 + reference7: Wide5 + reference8: Wide6 + reference9: Wide7 +} + +input Wide9 { + requiredNext: Wide0! + reference1: Wide0 + reference2: Wide1 + reference3: Wide2 + reference4: Wide3 + reference5: Wide4 + reference6: Wide5 + reference7: Wide6 + reference8: Wide7 + reference9: Wide8 +} diff --git a/src/test/resources/InputObjectHasUnbreakableCycleBenchmark/twoTypeMixedInputCycle.graphqls b/src/test/resources/InputObjectHasUnbreakableCycleBenchmark/twoTypeMixedInputCycle.graphqls new file mode 100644 index 0000000000..232f67fa39 --- /dev/null +++ b/src/test/resources/InputObjectHasUnbreakableCycleBenchmark/twoTypeMixedInputCycle.graphqls @@ -0,0 +1,11 @@ +type Query { + check(input: Cycle0): Boolean +} + +input Cycle0 @oneOf { + next: Cycle1 +} + +input Cycle1 { + next: Cycle0! +}