diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index c1ed85d7b6..e232e8e77a 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -3,6 +3,7 @@ import { describe, it } from 'mocha'; import { dedent } from '../../__testUtils__/dedent.js'; import { expectJSON } from '../../__testUtils__/expectJSON.js'; +import { spyOnMethod } from '../../__testUtils__/spyOn.js'; import { inspect } from '../../jsutils/inspect.js'; @@ -888,6 +889,24 @@ describe('Type System: Input Objects must have fields', () => { ]); }); + it('rejects a OneOf Input Object type with missing fields', () => { + const schema = buildSchema(` + type Query { + field(arg: SomeInputObject): String + } + + input SomeInputObject @oneOf + `); + + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Input Object type SomeInputObject must define one or more fields.', + locations: [{ line: 6, column: 7 }], + }, + ]); + }); + it('accepts an Input Object with breakable circular reference', () => { const schema = buildSchema(` type Query { @@ -924,7 +943,7 @@ describe('Type System: Input Objects must have fields', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Invalid circular reference. The Input Object SomeInputObject references itself in the non-null field SomeInputObject.nonNullSelf.', + 'Input Object SomeInputObject cannot be provided a finite value because it references itself through fields: SomeInputObject.nonNullSelf.', locations: [{ line: 7, column: 9 }], }, ]); @@ -952,7 +971,7 @@ describe('Type System: Input Objects must have fields', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Invalid circular reference. The Input Object SomeInputObject references itself via the non-null fields: SomeInputObject.startLoop, AnotherInputObject.nextInLoop, YetAnotherInputObject.closeLoop.', + 'Input Object SomeInputObject cannot be provided a finite value because it references itself through fields: SomeInputObject.startLoop, AnotherInputObject.nextInLoop, YetAnotherInputObject.closeLoop.', locations: [ { line: 7, column: 9 }, { line: 11, column: 9 }, @@ -986,7 +1005,7 @@ describe('Type System: Input Objects must have fields', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Invalid circular reference. The Input Object SomeInputObject references itself via the non-null fields: SomeInputObject.startLoop, AnotherInputObject.closeLoop.', + 'Input Object SomeInputObject cannot be provided a finite value because it references itself through fields: SomeInputObject.startLoop, AnotherInputObject.closeLoop.', locations: [ { line: 7, column: 9 }, { line: 11, column: 9 }, @@ -994,7 +1013,7 @@ describe('Type System: Input Objects must have fields', () => { }, { message: - 'Invalid circular reference. The Input Object AnotherInputObject references itself via the non-null fields: AnotherInputObject.startSecondLoop, YetAnotherInputObject.closeSecondLoop.', + 'Input Object AnotherInputObject cannot be provided a finite value because it references itself through fields: AnotherInputObject.startSecondLoop, YetAnotherInputObject.closeSecondLoop.', locations: [ { line: 12, column: 9 }, { line: 16, column: 9 }, @@ -1002,7 +1021,7 @@ describe('Type System: Input Objects must have fields', () => { }, { message: - 'Invalid circular reference. The Input Object YetAnotherInputObject references itself in the non-null field YetAnotherInputObject.nonNullSelf.', + 'Input Object YetAnotherInputObject cannot be provided a finite value because it references itself through fields: YetAnotherInputObject.nonNullSelf.', locations: [{ line: 17, column: 9 }], }, ]); @@ -2409,6 +2428,405 @@ describe('Type System: OneOf Input Object fields must be nullable', () => { }); }); +describe('Type System: Input Objects must not have unbreakable cycles', () => { + it('accepts a OneOf Input Object with a scalar field', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + a: Int + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf Input Object with a recursive list field', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + a: [A!] + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf Input Object referencing a non-OneOf input object', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + b: B + } + + input B { + x: Int + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf Input Object referencing an already checked input object', () => { + const schema = buildSchema(` + type Query { + b(arg: B): Int + a(arg: A): Int + } + + input B { + value: Int + } + + input A @oneOf { + b: B + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf Input Object with multiple acyclic input object fields', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + b: B + c: C + } + + input B { + value: Int + } + + input C { + value: Int + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf Input Object with an input object field and scalar escape', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + b: B + escape: Int + } + + input B { + value: Int + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf/OneOf cycle with a scalar escape', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + b: B + escape: Int + } + + input B @oneOf { + a: A + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a OneOf/non-OneOf cycle with a nullable escape', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + b: B + } + + input B { + a: A + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('rejects a self-referencing OneOf type with no escapes', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + self: A + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Input Object A cannot be provided a finite value because it references itself through fields: A.self.', + locations: [{ line: 7, column: 9 }], + }, + ]); + }); + + it('rejects a normal Input Object requiring an unbreakable OneOf cycle', () => { + const schema = buildSchema(` + type Query { + t(arg: T): Int + a(arg: A): Int + } + + input T @oneOf { + self: T + } + + input A { + t: T! + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Input Object T cannot be provided a finite value because it references itself through fields: T.self.', + locations: [{ line: 8, column: 9 }], + }, + ]); + }); + + it('caches shared unbreakable OneOf subgraphs', () => { + const chainLength = 16; + const types: Array = []; + types[0] = new GraphQLInputObjectType({ + name: 'T0', + isOneOf: true, + fields: () => ({ self: { type: types[0] } }), + }); + + for (let i = 1; i <= chainLength; ++i) { + const previousType = types[i - 1]; + types[i] = new GraphQLInputObjectType({ + name: `T${i}`, + isOneOf: true, + fields: { + a: { type: previousType }, + b: { type: previousType }, + }, + }); + } + + const getFieldsSpies = types.map((type) => spyOnMethod(type, 'getFields')); + + const schema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'Query', + fields: { + test: { + type: GraphQLInt, + args: { input: { type: types[chainLength] } }, + }, + }, + }), + types, + }); + + expect(validateSchema(schema)).to.have.lengthOf(1); + expect( + getFieldsSpies.reduce((sum, spy) => sum + spy.callCount, 0), + ).to.be.lessThan(500); + }); + + it('rejects a mixed OneOf/non-OneOf cycle with no escapes', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + b: B + } + + input B { + a: A! + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Input Object A cannot be provided a finite value because it references itself through fields: A.b, B.a.', + locations: [ + { line: 7, column: 9 }, + { line: 11, column: 9 }, + ], + }, + ]); + }); + + it('rejects multiple OneOf branches without duplicate cycle reports', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + b: B + c: C + } + + input B { + a: A! + } + + input C { + a: A! + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Input Object A cannot be provided a finite value because it references itself through fields: A.b, B.a.', + locations: [ + { line: 7, column: 9 }, + { line: 12, column: 9 }, + ], + }, + { + message: + 'Input Object A cannot be provided a finite value because it references itself through fields: A.c, C.a.', + locations: [ + { line: 8, column: 9 }, + { line: 16, column: 9 }, + ], + }, + ]); + }); + + it('accepts a OneOf/non-OneOf with scalar escape', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + b: B + escape: Int + } + + input B { + a: A! + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a non-OneOf/non-OneOf cycle with a nullable escape', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A { + b: B! + } + + input B { + a: A + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('accepts a non-OneOf/non-OneOf cycle with a list escape', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A { + b: [B!]! + } + + input B { + a: A! + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([]); + }); + + it('rejects a non-OneOf/non-OneOf cycle with required scalar and list fields', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A { + list: [B]! + b: B! + } + + input B { + value: Int! + a: A! + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Input Object A cannot be provided a finite value because it references itself through fields: A.b, B.a.', + locations: [ + { line: 8, column: 9 }, + { line: 13, column: 9 }, + ], + }, + ]); + }); + + it('rejects a larger mixed OneOf/non-OneOf cycle with no escapes', () => { + const schema = buildSchema(` + type Query { + test(arg: A): Int + } + + input A @oneOf { + b: B + } + + input B { + c: C! + } + + input C @oneOf { + a: A + } + `); + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Input Object A cannot be provided a finite value because it references itself through fields: A.b, B.c, C.a.', + locations: [ + { line: 7, column: 9 }, + { line: 11, column: 9 }, + { line: 15, column: 9 }, + ], + }, + ]); + }); +}); + describe('Objects must adhere to Interface they implement', () => { it('accepts an Object which implements an Interface', () => { const schema = buildSchema(` diff --git a/src/type/validate.ts b/src/type/validate.ts index 1ddb4a9b54..39e76074a6 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -373,12 +373,15 @@ function validateName( } function validateTypes(context: SchemaValidationContext): void { - // Ensure Input Objects do not contain non-nullable circular references. - const validateInputObjectNonNullCircularRefs = - createInputObjectNonNullCircularRefsValidator(context); + const inputObjectUnbreakableCycleCheck = + createInputObjectUnbreakableCycleCheck(); const validateInputObjectDefaultValueCircularRefs = createInputObjectDefaultValueCircularRefsValidator(context); const typeMap = context.schema.getTypeMap(); + + // Collect Input Object types that have unbreakable cycles. + const typesWithUnbreakableCycles = new Set(); + for (const type of Object.values(typeMap)) { // Ensure all provided types are in fact GraphQL type. if (!isNamedType(type)) { @@ -416,14 +419,17 @@ function validateTypes(context: SchemaValidationContext): void { // Ensure Input Object fields are valid. validateInputFields(context, type); - // Ensure Input Objects do not contain invalid field circular references. - // Ensure Input Objects do not contain non-nullable circular references. - validateInputObjectNonNullCircularRefs(type); + // Ensure Input Objects do not have unbreakable cycles. + if (inputObjectUnbreakableCycleCheck(type)) { + typesWithUnbreakableCycles.add(type); + } // Ensure Input Objects do not contain invalid default value circular references. validateInputObjectDefaultValueCircularRefs(type); } } + + reportInputObjectUnbreakableCycles(context, typesWithUnbreakableCycles); } function validateFields( @@ -739,65 +745,242 @@ function validateOneOfInputObjectField( } } -function createInputObjectNonNullCircularRefsValidator( +// Implements the spec's InputObjectHasUnbreakableCycle algorithm. +// Tracks already checked types to ensure that types are not redundantly checked. +function createInputObjectUnbreakableCycleCheck(): ( + inputObj: GraphQLInputObjectType, +) => boolean { + const cycleMemo = new Map(); + + return inputObjectHasUnbreakableCycle; + + function inputObjectHasUnbreakableCycle( + inputObj: GraphQLInputObjectType, + ): boolean { + const cachedResult = cycleMemo.get(inputObj); + if (cachedResult !== undefined) { + return cachedResult; + } + + // Unknown reachable types mapped to types that depend on them. + const candidates = new AccumulatorMap< + GraphQLInputObjectType, + GraphQLInputObjectType + >(); + for (const candidate of collectReachableInputObjects(inputObj)) { + candidates.set(candidate, []); + } + + // Tracks unresolved fields for normal Input Objects. + const normalTypeStates = new Map< + GraphQLInputObjectType, + { hasKnownCycleField: boolean; unresolvedFieldCount: number } + >(); + + // Types proven not to have cycles and ready to remove. + const typesToRemove: Array = []; + + for (const candidate of candidates.keys()) { + const fields = Object.values(candidate.getFields()); + + if (candidate.isOneOf) { + // OneOf Input Objects have an unbreakable cycle if every field leads to an unbreakable cycle. + if (fields.length === 0) { + typesToRemove.push(candidate); + continue; + } + + for (const field of fields) { + const target = getUnbreakableCycleTarget(candidate, field.type); + + if (target == null) { + typesToRemove.push(candidate); + break; + } + + const targetResult = cycleMemo.get(target); + if (targetResult === false) { + typesToRemove.push(candidate); + break; + } + + if (targetResult === undefined) { + candidates.add(target, candidate); + } + } + } else { + // Normal Input Objects have an unbreakable cycle if any non-null field has one. + let hasKnownCycleField = false; + let unresolvedFieldCount = 0; + + for (const field of fields) { + const target = getUnbreakableCycleTarget(candidate, field.type); + + if (target == null) { + continue; + } + + const targetResult = cycleMemo.get(target); + if (targetResult === false) { + continue; + } + + if (targetResult === true) { + hasKnownCycleField = true; + } else { + ++unresolvedFieldCount; + candidates.add(target, candidate); + } + } + + normalTypeStates.set(candidate, { + hasKnownCycleField, + unresolvedFieldCount, + }); + + if (!hasKnownCycleField && unresolvedFieldCount === 0) { + typesToRemove.push(candidate); + } + } + } + + while (typesToRemove.length > 0) { + const type = typesToRemove.pop(); + invariant(type != null); + + const dependents = candidates.get(type); + if (dependents === undefined) { + continue; + } + + candidates.delete(type); + cycleMemo.set(type, false); + + for (const dependent of dependents) { + if (!candidates.has(dependent)) { + continue; + } + + if (dependent.isOneOf) { + typesToRemove.push(dependent); + } else { + const state = normalTypeStates.get(dependent); + invariant(state !== undefined); + + --state.unresolvedFieldCount; + if (!state.hasKnownCycleField && state.unresolvedFieldCount === 0) { + typesToRemove.push(dependent); + } + } + } + } + + for (const candidate of candidates.keys()) { + cycleMemo.set(candidate, true); + } + + const result = cycleMemo.get(inputObj); + invariant(result !== undefined); + return result; + } + + function collectReachableInputObjects( + inputObj: GraphQLInputObjectType, + ): Set { + const reachable = new Set(); + const visited = new Set(); + + collect(inputObj); + return reachable; + + function collect(type: GraphQLInputObjectType): void { + if (visited.has(type) || cycleMemo.has(type)) { + return; + } + + visited.add(type); + reachable.add(type); + + for (const field of Object.values(type.getFields())) { + const target = getUnbreakableCycleTarget(type, field.type); + + if (target != null) { + collect(target); + } + } + } + } +} + +function getUnbreakableCycleTarget( + inputObj: GraphQLInputObjectType, + fieldType: GraphQLInputType, +): Maybe { + if (inputObj.isOneOf) { + if (isInputObjectType(fieldType)) { + return fieldType; + } + return undefined; + } + + if (isNonNullType(fieldType) && isInputObjectType(fieldType.ofType)) { + return fieldType.ofType; + } +} + +function reportInputObjectUnbreakableCycles( context: SchemaValidationContext, -): (inputObj: GraphQLInputObjectType) => void { - // Modified copy of algorithm from 'src/validation/rules/NoFragmentCycles.js'. - // Tracks already visited types to maintain O(N) and to ensure that cycles - // are not redundantly reported. + typesWithUnbreakableCycles: ReadonlySet, +): void { + // Tracks already visited types to ensure that cycles are not redundantly + // reported. const visitedTypes = new Set(); - // Array of types nodes used to produce meaningful errors + // Array of fields used to produce meaningful errors. const fieldPath: Array<{ fieldStr: string; astNode: Maybe }> = []; - // Position in the type path - const fieldPathIndexByTypeName: ObjMap = - Object.create(null); + // Position in the field path. + const fieldPathIndexByType = new Map(); - return detectCycleRecursive; + for (const type of typesWithUnbreakableCycles) { + reportCycleRecursive(type); + } - // This does a straight-forward DFS to find cycles. - // It does not terminate when a cycle was found but continues to explore - // the graph to find all possible cycles. - function detectCycleRecursive(inputObj: GraphQLInputObjectType): void { + function reportCycleRecursive(inputObj: GraphQLInputObjectType): void { if (visitedTypes.has(inputObj)) { return; } visitedTypes.add(inputObj); - fieldPathIndexByTypeName[inputObj.name] = fieldPath.length; + fieldPathIndexByType.set(inputObj, fieldPath.length); - const fields = Object.values(inputObj.getFields()); - for (const field of fields) { - if (isNonNullType(field.type) && isInputObjectType(field.type.ofType)) { - const fieldType = field.type.ofType; - const cycleIndex = fieldPathIndexByTypeName[fieldType.name]; + for (const field of Object.values(inputObj.getFields())) { + const target = getUnbreakableCycleTarget(inputObj, field.type); + if (target == null || !typesWithUnbreakableCycles.has(target)) { + continue; + } - fieldPath.push({ - fieldStr: `${inputObj}.${field.name}`, - astNode: field.astNode, - }); - if (cycleIndex === undefined) { - detectCycleRecursive(fieldType); - } else { - const cyclePath = fieldPath.slice(cycleIndex); - const pathStr = cyclePath - .map((fieldObj) => fieldObj.fieldStr) - .join(', '); - context.reportError( - `Invalid circular reference. The Input Object ${fieldType} references itself ${ - cyclePath.length > 1 - ? 'via the non-null fields:' - : 'in the non-null field' - } ${pathStr}.`, - cyclePath.map((fieldObj) => fieldObj.astNode), - ); - } - fieldPath.pop(); + const cycleIndex = fieldPathIndexByType.get(target); + fieldPath.push({ + fieldStr: `${inputObj}.${field.name}`, + astNode: field.astNode, + }); + + if (cycleIndex === undefined) { + reportCycleRecursive(target); + } else { + const cyclePath = fieldPath.slice(cycleIndex); + const pathStr = cyclePath.map((p) => p.fieldStr).join(', '); + context.reportError( + `Input Object ${target} cannot be provided a finite value because it references itself through fields: ${pathStr}.`, + cyclePath.map((p) => p.astNode), + ); } + + fieldPath.pop(); } - fieldPathIndexByTypeName[inputObj.name] = undefined; + fieldPathIndexByType.delete(inputObj); } }