From 4afb263289485897fdfec37aaf6d5f1e5451dcb3 Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Tue, 10 May 2016 14:50:26 -0700 Subject: [PATCH] Validation: improving overlapping fields quality (#386) This improves the overlapping fields validation performance and improves error reporting quality by separating the concepts of checking fields "within" a single collection of fields from checking fields "between" two different collections of fields. This ensures for deeply overlapping fields that nested fields are not checked against each other repeatedly. Extending this concept further, fragment spreads are no longer expanded inline before looking for conflicts, instead the fields within a fragment are compared to the fields with the selection set which contained the referencing fragment spread. e.g. ```graphql { same: a same: b ...X } fragment X on T { same: c same: d } ``` In the above example, the initial query body is checked "within" so `a` is compared to `b`. Also, the fragment `X` is checked "within" so `c` is compared to `d`. Because of the fragment spread, the query body and fragment `X` are checked "between" so that `a` and `b` are each compared to `c` and `d`. In this trivial example, no fewer checks are performed, but in the case where fragments are referenced multiple times, this reduces the overall number of checks (regardless of memoization). **BREAKING**: This can change the order of fields reported when a conflict arises when fragment spreads are involved. If you are checking the precise output of errors (e.g. for unit tests), you may find existing errors change from `"a" and "c" are different fields` to `"c" and "a" are different fields`. From a perf point of view, this is fairly minor as the memoization "PairSet" was already keeping these repeated checks from consuming time, however this will reduce the number of memoized hits because of the algorithm improvement. From an error reporting point of view, this reports nearest-common-ancestor issues when found in a fragment that comes later in the validation process. I've added a test which fails with the existing impl and now passes, as well as changed a comment. This also fixes an error where validation issues could be missed because of an over-eager memoization. I've also modified the `PairSet` to be aware of both forms of memoization, also represented by a previously failing test. --- .../OverlappingFieldsCanBeMerged-test.js | 165 ++++- .../rules/OverlappingFieldsCanBeMerged.js | 669 ++++++++++++++---- 2 files changed, 677 insertions(+), 157 deletions(-) diff --git a/src/validation/__tests__/OverlappingFieldsCanBeMerged-test.js b/src/validation/__tests__/OverlappingFieldsCanBeMerged-test.js index df4e1d8ca1..3da107a50d 100644 --- a/src/validation/__tests__/OverlappingFieldsCanBeMerged-test.js +++ b/src/validation/__tests__/OverlappingFieldsCanBeMerged-test.js @@ -247,10 +247,10 @@ describe('Validate: Overlapping fields can be merged', () => { `, [ { message: fieldsConflictMessage('x', 'a and b are different fields'), locations: [ { line: 18, column: 9 }, { line: 21, column: 9 } ] }, - { message: fieldsConflictMessage('x', 'a and c are different fields'), - locations: [ { line: 18, column: 9 }, { line: 14, column: 11 } ] }, - { message: fieldsConflictMessage('x', 'b and c are different fields'), - locations: [ { line: 21, column: 9 }, { line: 14, column: 11 } ] } + { message: fieldsConflictMessage('x', 'c and a are different fields'), + locations: [ { line: 14, column: 11 }, { line: 18, column: 9 } ] }, + { message: fieldsConflictMessage('x', 'c and b are different fields'), + locations: [ { line: 14, column: 11 }, { line: 21, column: 9 } ] } ]); }); @@ -363,6 +363,97 @@ describe('Validate: Overlapping fields can be merged', () => { ]); }); + it('reports deep conflict to nearest common ancestor in fragments', () => { + expectFailsRule(OverlappingFieldsCanBeMerged, ` + { + field { + ...F + } + field { + ...F + } + } + fragment F on T { + deepField { + deeperField { + x: a + } + deeperField { + x: b + } + }, + deepField { + deeperField { + y + } + } + } + `, [ + { message: fieldsConflictMessage( + 'deeperField', [ [ 'x', 'a and b are different fields' ] ] + ), + locations: [ + { line: 12, column: 11 }, + { line: 13, column: 13 }, + { line: 15, column: 11 }, + { line: 16, column: 13 } ] }, + ]); + }); + + it('reports deep conflict in nested fragments', () => { + expectFailsRule(OverlappingFieldsCanBeMerged, ` + { + field { + ...F + } + field { + ...I + } + } + fragment F on T { + x: a + ...G + } + fragment G on T { + y: c + } + fragment I on T { + y: d + ...J + } + fragment J on T { + x: b + } + `, [ + { message: fieldsConflictMessage( + 'field', [ [ 'x', 'a and b are different fields' ], + [ 'y', 'c and d are different fields' ] ] + ), + locations: [ + { line: 3, column: 9 }, + { line: 11, column: 9 }, + { line: 15, column: 9 }, + { line: 6, column: 9 }, + { line: 22, column: 9 }, + { line: 18, column: 9 } ] }, + ]); + }); + + it('ignores unknown fragments', () => { + expectPassesRule(OverlappingFieldsCanBeMerged, ` + { + field + ...Unknown + ...Known + } + + fragment Known on T { + field + ...OtherUnknown + } + `); + }); + describe('return types must be unambiguous', () => { const SomeBox = new GraphQLInterfaceType({ @@ -537,6 +628,64 @@ describe('Validate: Overlapping fields can be merged', () => { ]); }); + it('reports correctly when a non-exclusive follows an exclusive', () => { + expectFailsRuleWithSchema(schema, OverlappingFieldsCanBeMerged, ` + { + someBox { + ... on IntBox { + deepBox { + ...X + } + } + } + someBox { + ... on StringBox { + deepBox { + ...Y + } + } + } + memoed: someBox { + ... on IntBox { + deepBox { + ...X + } + } + } + memoed: someBox { + ... on StringBox { + deepBox { + ...Y + } + } + } + other: someBox { + ...X + } + other: someBox { + ...Y + } + } + fragment X on SomeBox { + scalar + } + fragment Y on SomeBox { + scalar: unrelatedField + } + `, [ + { message: fieldsConflictMessage( + 'other', + [ [ 'scalar', 'scalar and unrelatedField are different fields' ] ] + ), + locations: [ + { line: 31, column: 11 }, + { line: 39, column: 11 }, + { line: 34, column: 11 }, + { line: 42, column: 11 }, + ] } + ]); + }); + it('disallows differing return type nullability despite no overlap', () => { expectFailsRuleWithSchema(schema, OverlappingFieldsCanBeMerged, ` { @@ -725,15 +874,15 @@ describe('Validate: Overlapping fields can be merged', () => { `, [ { message: fieldsConflictMessage( 'edges', - [ [ 'node', [ [ 'id', 'id and name are different fields' ] ] ] ] + [ [ 'node', [ [ 'id', 'name and id are different fields' ] ] ] ] ), locations: [ - { line: 14, column: 11 }, - { line: 15, column: 13 }, - { line: 16, column: 15 }, { line: 5, column: 13 }, { line: 6, column: 15 }, { line: 7, column: 17 }, + { line: 14, column: 11 }, + { line: 15, column: 13 }, + { line: 16, column: 15 }, ] } ]); }); diff --git a/src/validation/rules/OverlappingFieldsCanBeMerged.js b/src/validation/rules/OverlappingFieldsCanBeMerged.js index bdc9bd3c93..52d9896f64 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMerged.js +++ b/src/validation/rules/OverlappingFieldsCanBeMerged.js @@ -15,6 +15,7 @@ import type { SelectionSet, Field, Argument, + FragmentDefinition, } from '../../language/ast'; import { FIELD, INLINE_FRAGMENT, FRAGMENT_SPREAD } from '../../language/kinds'; import { print } from '../../language/printer'; @@ -61,28 +62,32 @@ function reasonMessage(reason: ConflictReasonMessage): string { * without ambiguity. */ export function OverlappingFieldsCanBeMerged(context: ValidationContext): any { - const comparedSet = new PairSet(); + // A memoization for when two fragments are compared "between" each other for + // conflicts. Two fragments may be compared many times, so memoizing this can + // dramatically improve the performance of this validator. + const comparedFragments = new PairSet(); + + // A cache for the "field map" and list of fragment names found in any given + // selection set. Selection sets may be asked for this information multiple + // times, so this improves the performance of this validator. + const cachedFieldsAndFragmentNames = new Map(); return { - SelectionSet: { - // Note: we validate on the reverse traversal so deeper conflicts will be - // caught first, for correct calculation of mutual exclusivity and for - // clearer error messages. - leave(selectionSet) { - const fieldMap = collectFieldASTsAndDefs( - context, - context.getParentType(), - selectionSet - ); - const conflicts = findConflicts(context, false, fieldMap, comparedSet); - conflicts.forEach( - ([ [ responseName, reason ], fields1, fields2 ]) => - context.reportError(new GraphQLError( - fieldsConflictMessage(responseName, reason), - fields1.concat(fields2) - )) - ); - } + SelectionSet(selectionSet) { + const conflicts = findConflictsWithinSelectionSet( + context, + cachedFieldsAndFragmentNames, + comparedFragments, + context.getParentType(), + selectionSet + ); + conflicts.forEach( + ([ [ responseName, reason ], fields1, fields2 ]) => + context.reportError(new GraphQLError( + fieldsConflictMessage(responseName, reason), + fields1.concat(fields2) + )) + ); } }; } @@ -98,27 +103,365 @@ type AstAndDef = [ GraphQLCompositeType, Field, ?GraphQLFieldDefinition ]; type AstAndDefCollection = { [key: string]: Array }; /** - * Find all Conflicts within a collection of fields. + * Algorithm: + * + * Conflicts occur when two fields exist in a query which will produce the same + * response name, but represent differing values, thus creating a conflict. + * The algorithm below finds all conflicts via making a series of comparisons + * between fields. In order to compare as few fields as possible, this makes + * a series of comparisons "within" sets of fields and "between" sets of fields. + * + * Given any selection set, a collection produces both a set of fields by + * also including all inline fragments, as well as a list of fragments + * referenced by fragment spreads. + * + * A) Each selection set represented in the document first compares "within" its + * collected set of fields, finding any conflicts between every pair of + * overlapping fields. + * Note: This is the *only time* that a the fields "within" a set are compared + * to each other. After this only fields "between" sets are compared. + * + * B) Also, if any fragment is referenced in a selection set, then a + * comparison is made "between" the original set of fields and the + * referenced fragment. + * + * C) Also, if multiple fragments are referenced, then comparisons + * are made "between" each referenced fragment. + * + * D) When comparing "between" a set of fields and a referenced fragment, first + * a comparison is made between each field in the original set of fields and + * each field in the the referenced set of fields. + * + * E) Also, if any fragment is referenced in the referenced selection set, + * then a comparison is made "between" the original set of fields and the + * referenced fragment (recursively referring to step D). + * + * F) When comparing "between" two fragments, first a comparison is made between + * each field in the first referenced set of fields and each field in the the + * second referenced set of fields. + * + * G) Also, any fragments referenced by the first must be compared to the + * second, and any fragments referenced by the second must be compared to the + * first (recursively referring to step F). + * + * H) When comparing two fields, if both have selection sets, then a comparison + * is made "between" both selection sets, first comparing the set of fields in + * the first selection set with the set of fields in the second. + * + * I) Also, if any fragment is referenced in either selection set, then a + * comparison is made "between" the other set of fields and the + * referenced fragment. + * + * J) Also, if two fragments are referenced in both selection sets, then a + * comparison is made "between" the two fragments. + * */ -function findConflicts( + +// Find all conflicts found "within" a selection set, including those found +// via spreading in fragments. Called when visiting each SelectionSet in the +// GraphQL Document. +function findConflictsWithinSelectionSet( context: ValidationContext, - parentFieldsAreMutuallyExclusive: boolean, + cachedFieldsAndFragmentNames, + comparedFragments: PairSet, + parentType: ?GraphQLNamedType, + selectionSet: SelectionSet +): Array { + const conflicts = []; + + const [ fieldMap, fragmentNames ] = getFieldsAndFragmentNames( + context, + cachedFieldsAndFragmentNames, + parentType, + selectionSet + ); + + // (A) Find find all conflicts "within" the fields of this selection set. + // Note: this is the *only place* `collectConflictsWithin` is called. + collectConflictsWithin( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragments, + fieldMap + ); + + // (B) Then collect conflicts between these fields and those represented by + // each spread fragment name found. + for (let i = 0; i < fragmentNames.length; i++) { + collectConflictsBetweenFieldsAndFragment( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragments, + false, + fieldMap, + fragmentNames[i] + ); + // (C) Then compare this fragment with all other fragments found in this + // selection set to collect conflicts between fragments spread together. + // This compares each item in the list of fragment names to every other item + // in that same list (except for itself). + for (let j = i + 1; j < fragmentNames.length; j++) { + collectConflictsBetweenFragments( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragments, + false, + fragmentNames[i], + fragmentNames[j] + ); + } + } + return conflicts; +} + +// Collect all conflicts found between a set of fields and a fragment reference +// including via spreading in any nested fragments. +function collectConflictsBetweenFieldsAndFragment( + context: ValidationContext, + conflicts: Array, + cachedFieldsAndFragmentNames, + comparedFragments: PairSet, + areMutuallyExclusive: boolean, fieldMap: AstAndDefCollection, - comparedSet: PairSet + fragmentName: string +): void { + const fragment = context.getFragment(fragmentName); + if (!fragment) { + return; + } + + const [ fieldMap2, fragmentNames2 ] = getReferencedFieldsAndFragmentNames( + context, + cachedFieldsAndFragmentNames, + fragment + ); + + // (D) First collect any conflicts between the provided collection of fields + // and the collection of fields represented by the given fragment. + collectConflictsBetween( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragments, + areMutuallyExclusive, + fieldMap, + fieldMap2 + ); + + // (E) Then collect any conflicts between the provided collection of fields + // and any fragment names found in the given fragment. + for (let i = 0; i < fragmentNames2.length; i++) { + collectConflictsBetweenFieldsAndFragment( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragments, + areMutuallyExclusive, + fieldMap, + fragmentNames2[i] + ); + } +} + +// Collect all conflicts found between two fragments, including via spreading in +// any nested fragments. +function collectConflictsBetweenFragments( + context: ValidationContext, + conflicts: Array, + cachedFieldsAndFragmentNames, + comparedFragments: PairSet, + areMutuallyExclusive: boolean, + fragmentName1: string, + fragmentName2: string +): void { + const fragment1 = context.getFragment(fragmentName1); + const fragment2 = context.getFragment(fragmentName2); + if (!fragment1 || !fragment2) { + return; + } + + // No need to compare a fragment to itself. + if (fragment1 === fragment2) { + return; + } + + // Memoize so two fragments are not compared for conflicts more than once. + if ( + comparedFragments.has(fragmentName1, fragmentName2, areMutuallyExclusive) + ) { + return; + } + comparedFragments.add(fragmentName1, fragmentName2, areMutuallyExclusive); + + const [ fieldMap1, fragmentNames1 ] = getReferencedFieldsAndFragmentNames( + context, + cachedFieldsAndFragmentNames, + fragment1 + ); + const [ fieldMap2, fragmentNames2 ] = getReferencedFieldsAndFragmentNames( + context, + cachedFieldsAndFragmentNames, + fragment2 + ); + + // (F) First, collect all conflicts between these two collections of fields + // (not including any nested fragments). + collectConflictsBetween( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragments, + areMutuallyExclusive, + fieldMap1, + fieldMap2 + ); + + // (G) Then collect conflicts between the first fragment and any nested + // fragments spread in the second fragment. + for (let j = 0; j < fragmentNames2.length; j++) { + collectConflictsBetweenFragments( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragments, + areMutuallyExclusive, + fragmentName1, + fragmentNames2[j] + ); + } + + // (G) Then collect conflicts between the second fragment and any nested + // fragments spread in the first fragment. + for (let i = 0; i < fragmentNames1.length; i++) { + collectConflictsBetweenFragments( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragments, + areMutuallyExclusive, + fragmentNames1[i], + fragmentName2 + ); + } +} + +// Find all conflicts found between two selection sets, including those found +// via spreading in fragments. Called when determining if conflicts exist +// between the sub-fields of two overlapping fields. +function findConflictsBetweenSubSelectionSets( + context: ValidationContext, + cachedFieldsAndFragmentNames, + comparedFragments: PairSet, + areMutuallyExclusive: boolean, + parentType1: ?GraphQLNamedType, + selectionSet1: SelectionSet, + parentType2: ?GraphQLNamedType, + selectionSet2: SelectionSet ): Array { const conflicts = []; + + const [ fieldMap1, fragmentNames1 ] = getFieldsAndFragmentNames( + context, + cachedFieldsAndFragmentNames, + parentType1, + selectionSet1 + ); + const [ fieldMap2, fragmentNames2 ] = getFieldsAndFragmentNames( + context, + cachedFieldsAndFragmentNames, + parentType2, + selectionSet2 + ); + + // (H) First, collect all conflicts between these two collections of field. + collectConflictsBetween( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragments, + areMutuallyExclusive, + fieldMap1, + fieldMap2 + ); + + // (I) Then collect conflicts between the first collection of fields and + // those referenced by each fragment name associated with the second. + for (let j = 0; j < fragmentNames2.length; j++) { + collectConflictsBetweenFieldsAndFragment( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragments, + areMutuallyExclusive, + fieldMap1, + fragmentNames2[j] + ); + } + + // (I) Then collect conflicts between the second collection of fields and + // those referenced by each fragment name associated with the first. + for (let i = 0; i < fragmentNames1.length; i++) { + collectConflictsBetweenFieldsAndFragment( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragments, + areMutuallyExclusive, + fieldMap2, + fragmentNames1[i] + ); + } + + // (J) Also collect conflicts between any fragment names by the first and + // fragment names by the second. This compares each item in the first set of + // names to each item in the second set of names. + for (let i = 0; i < fragmentNames1.length; i++) { + for (let j = 0; j < fragmentNames2.length; j++) { + collectConflictsBetweenFragments( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragments, + areMutuallyExclusive, + fragmentNames1[i], + fragmentNames2[j] + ); + } + } + return conflicts; +} + +// Collect all Conflicts "within" one collection of fields. +function collectConflictsWithin( + context: ValidationContext, + conflicts: Array, + cachedFieldsAndFragmentNames, + comparedFragments: PairSet, + fieldMap: AstAndDefCollection +): void { + // A field map is a keyed collection, where each key represents a response + // name and the value at that key is a list of all fields which provide that + // response name. For every response name, if there are multiple fields, they + // must be compared to find a potential conflict. Object.keys(fieldMap).forEach(responseName => { const fields = fieldMap[responseName]; + // This compares every field in the list to every other field in this list + // (except to itself). If the list only has one item, nothing needs to + // be compared. if (fields.length > 1) { for (let i = 0; i < fields.length; i++) { - for (let j = i; j < fields.length; j++) { + for (let j = i + 1; j < fields.length; j++) { const conflict = findConflict( context, - parentFieldsAreMutuallyExclusive, + cachedFieldsAndFragmentNames, + comparedFragments, + false, // within one collection is never mutually exclusive responseName, fields[i], - fields[j], - comparedSet + fields[j] ); if (conflict) { conflicts.push(conflict); @@ -127,46 +470,65 @@ function findConflicts( } } }); - return conflicts; } -/** - * Determines if there is a conflict between two particular fields. - */ +// Collect all Conflicts between two collections of fields. This is similar to, +// but different from the `collectConflictsWithin` function above. This check +// assumes that `collectConflictsWithin` has already been called on each +// provided collection of fields. This is true because this validator traverses +// each individual selection set. +function collectConflictsBetween( + context: ValidationContext, + conflicts: Array, + cachedFieldsAndFragmentNames, + comparedFragments: PairSet, + parentFieldsAreMutuallyExclusive: boolean, + fieldMap1: AstAndDefCollection, + fieldMap2: AstAndDefCollection +): void { + // A field map is a keyed collection, where each key represents a response + // name and the value at that key is a list of all fields which provide that + // response name. For any response name which appears in both provided field + // maps, each field from the first field map must be compared to every field + // in the second field map to find potential conflicts. + Object.keys(fieldMap1).forEach(responseName => { + const fields2 = fieldMap2[responseName]; + if (fields2) { + const fields1 = fieldMap1[responseName]; + for (let i = 0; i < fields1.length; i++) { + for (let j = 0; j < fields2.length; j++) { + const conflict = findConflict( + context, + cachedFieldsAndFragmentNames, + comparedFragments, + parentFieldsAreMutuallyExclusive, + responseName, + fields1[i], + fields2[j] + ); + if (conflict) { + conflicts.push(conflict); + } + } + } + } + }); +} + +// Determines if there is a conflict between two particular fields, including +// comparing their sub-fields. function findConflict( context: ValidationContext, + cachedFieldsAndFragmentNames, + comparedFragments: PairSet, parentFieldsAreMutuallyExclusive: boolean, responseName: string, field1: AstAndDef, - field2: AstAndDef, - comparedSet: PairSet + field2: AstAndDef ): ?Conflict { const [ parentType1, ast1, def1 ] = field1; const [ parentType2, ast2, def2 ] = field2; - // Not a pair. - if (ast1 === ast2) { - return; - } - - // Memoize, do not report the same issue twice. - // Note: Two overlapping ASTs could be encountered both when - // `parentFieldsAreMutuallyExclusive` is true and is false, which could - // produce different results (when `true` being a subset of `false`). - // However we do not need to include this piece of information when - // memoizing since this rule visits leaf fields before their parent fields, - // ensuring that `parentFieldsAreMutuallyExclusive` is `false` the first - // time two overlapping fields are encountered, ensuring that the full - // set of validation rules are always checked when necessary. - if (comparedSet.has(ast1, ast2)) { - return; - } - comparedSet.add(ast1, ast2); - - // The return type for each field. - const type1 = def1 && def1.type; - const type2 = def2 && def2.type; - // If it is known that two fields could not possibly apply at the same // time, due to the parent types, then it is safe to permit them to diverge // in aliased field or arguments used as they will not present any ambiguity @@ -175,13 +537,17 @@ function findConflict( // different Object types. Interface or Union types might overlap - if not // in the current state of the schema, then perhaps in some future version, // thus may not safely diverge. - const fieldsAreMutuallyExclusive = + const areMutuallyExclusive = parentFieldsAreMutuallyExclusive || parentType1 !== parentType2 && parentType1 instanceof GraphQLObjectType && parentType2 instanceof GraphQLObjectType; - if (!fieldsAreMutuallyExclusive) { + // The return type for each field. + const type1 = def1 && def1.type; + const type2 = def2 && def2.type; + + if (!areMutuallyExclusive) { // Two aliases must refer to the same field. const name1 = ast1.name.value; const name2 = ast2.name.value; @@ -211,13 +577,21 @@ function findConflict( ]; } - const subfieldMap = getSubfieldMap(context, ast1, type1, ast2, type2); - if (subfieldMap) { - const conflicts = findConflicts( + // Collect and compare sub-fields. Use the same "visited fragment names" list + // for both collections so fields in a fragment reference are never + // compared to themselves. + const selectionSet1 = ast1.selectionSet; + const selectionSet2 = ast2.selectionSet; + if (selectionSet1 && selectionSet2) { + const conflicts = findConflictsBetweenSubSelectionSets( context, - fieldsAreMutuallyExclusive, - subfieldMap, - comparedSet + cachedFieldsAndFragmentNames, + comparedFragments, + areMutuallyExclusive, + getNamedType(type1), + selectionSet1, + getNamedType(type2), + selectionSet2 ); return subfieldConflicts(conflicts, responseName, ast1, ast2); } @@ -279,54 +653,61 @@ function doTypesConflict( return false; } -/** - * Given two overlapping fields, produce the combined collection of subfields. - */ -function getSubfieldMap( +// Given a selection set, return the collection of fields (a mapping of response +// name to field ASTs and definitions) as well as a list of fragment names +// referenced via fragment spreads. +function getFieldsAndFragmentNames( context: ValidationContext, - ast1: Field, - type1: ?GraphQLOutputType, - ast2: Field, - type2: ?GraphQLOutputType -): ?AstAndDefCollection { - const selectionSet1 = ast1.selectionSet; - const selectionSet2 = ast2.selectionSet; - if (selectionSet1 && selectionSet2) { - const visitedFragmentNames = {}; - let subfieldMap = collectFieldASTsAndDefs( - context, - getNamedType(type1), - selectionSet1, - visitedFragmentNames - ); - subfieldMap = collectFieldASTsAndDefs( + cachedFieldsAndFragmentNames, + parentType: ?GraphQLNamedType, + selectionSet: SelectionSet +): [ AstAndDefCollection, Array ] { + let cached = cachedFieldsAndFragmentNames.get(selectionSet); + if (!cached) { + const astAndDefs = {}; + const fragmentNames = {}; + _collectFieldsAndFragmentNames( context, - getNamedType(type2), - selectionSet2, - visitedFragmentNames, - subfieldMap + parentType, + selectionSet, + astAndDefs, + fragmentNames ); - return subfieldMap; + cached = [ astAndDefs, Object.keys(fragmentNames) ]; + cachedFieldsAndFragmentNames.set(selectionSet, cached); } + return cached; } -/** - * Given a selectionSet, adds all of the fields in that selection to - * the passed in map of fields, and returns it at the end. - * - * Note: This is not the same as execution's collectFields because at static - * time we do not know what object type will be used, so we unconditionally - * spread in all fragments. - */ -function collectFieldASTsAndDefs( +// Given a reference to a fragment, return the represented collection of fields +// as well as a list of nested fragment names referenced via fragment spreads. +function getReferencedFieldsAndFragmentNames( + context: ValidationContext, + cachedFieldsAndFragmentNames, + fragment: FragmentDefinition +) { + // Short-circuit building a type from the AST if possible. + const cached = cachedFieldsAndFragmentNames.get(fragment.selectionSet); + if (cached) { + return cached; + } + + const fragmentType = typeFromAST(context.getSchema(), fragment.typeCondition); + return getFieldsAndFragmentNames( + context, + cachedFieldsAndFragmentNames, + ((fragmentType: any): GraphQLNamedType), + fragment.selectionSet + ); +} + +function _collectFieldsAndFragmentNames( context: ValidationContext, parentType: ?GraphQLNamedType, selectionSet: SelectionSet, - visitedFragmentNames?: {[key: string]: boolean}, - astAndDefs?: AstAndDefCollection -): AstAndDefCollection { - const _visitedFragmentNames = visitedFragmentNames || {}; - let _astAndDefs = astAndDefs || {}; + astAndDefs, + fragmentNames +): void { for (let i = 0; i < selectionSet.selections.length; i++) { const selection = selectionSet.selections[i]; switch (selection.kind) { @@ -339,53 +720,33 @@ function collectFieldASTsAndDefs( } const responseName = selection.alias ? selection.alias.value : fieldName; - if (!_astAndDefs[responseName]) { - _astAndDefs[responseName] = []; + if (!astAndDefs[responseName]) { + astAndDefs[responseName] = []; } - _astAndDefs[responseName].push([ parentType, selection, fieldDef ]); + astAndDefs[responseName].push([ parentType, selection, fieldDef ]); + break; + case FRAGMENT_SPREAD: + fragmentNames[selection.name.value] = true; break; case INLINE_FRAGMENT: const typeCondition = selection.typeCondition; const inlineFragmentType = typeCondition ? typeFromAST(context.getSchema(), selection.typeCondition) : parentType; - _astAndDefs = collectFieldASTsAndDefs( + _collectFieldsAndFragmentNames( context, ((inlineFragmentType: any): GraphQLNamedType), selection.selectionSet, - _visitedFragmentNames, - _astAndDefs - ); - break; - case FRAGMENT_SPREAD: - const fragName = selection.name.value; - if (_visitedFragmentNames[fragName]) { - continue; - } - _visitedFragmentNames[fragName] = true; - const fragment = context.getFragment(fragName); - if (!fragment) { - continue; - } - const fragmentType = - typeFromAST(context.getSchema(), fragment.typeCondition); - _astAndDefs = collectFieldASTsAndDefs( - context, - ((fragmentType: any): GraphQLNamedType), - fragment.selectionSet, - _visitedFragmentNames, - _astAndDefs + astAndDefs, + fragmentNames ); break; } } - return _astAndDefs; } -/** - * Given a series of Conflicts which occurred between two sub-fields, generate - * a single Conflict. - */ +// Given a series of Conflicts which occurred between two sub-fields, generate +// a single Conflict. function subfieldConflicts( conflicts: Array, responseName: string, @@ -412,28 +773,38 @@ function subfieldConflicts( * not matter. We do this by maintaining a sort of double adjacency sets. */ class PairSet { - _data: Map>; + _data: {[a: string]: {[b: string]: boolean}}; constructor() { - this._data = new Map(); + this._data = Object.create(null); } - has(a, b) { - const first = this._data.get(a); - return first && first.has(b); + has(a: string, b: string, areMutuallyExclusive: boolean) { + const first = this._data[a]; + const result = first && first[b]; + if (result === undefined) { + return false; + } + // areMutuallyExclusive being false is a superset of being true, + // hence if we want to know if this PairSet "has" these two with no + // exclusivity, we have to ensure it was added as such. + if (areMutuallyExclusive === false) { + return result === false; + } + return true; } - add(a, b) { - _pairSetAdd(this._data, a, b); - _pairSetAdd(this._data, b, a); + add(a: string, b: string, areMutuallyExclusive: boolean) { + _pairSetAdd(this._data, a, b, areMutuallyExclusive); + _pairSetAdd(this._data, b, a, areMutuallyExclusive); } } -function _pairSetAdd(data, a, b) { - let set = data.get(a); - if (!set) { - set = new Set(); - data.set(a, set); +function _pairSetAdd(data, a, b, areMutuallyExclusive) { + let map = data[a]; + if (!map) { + map = Object.create(null); + data[a] = map; } - set.add(b); + map[b] = areMutuallyExclusive; }