From 01fe3f836c08805c1c53b14c745a5117c678866d Mon Sep 17 00:00:00 2001 From: Sylvain Lebresne Date: Thu, 30 Mar 2023 11:07:53 +0200 Subject: [PATCH] Improve fragment reuse code (#2497) The existing code was only able to reuse named fragments in a small subset of cases (essentially when the fragment was matched by a full sub-selection). This patch makes the code much more able to reuse fragments by allowing to match sub-selections, handle intersecting fragments, etc... --- .changeset/rich-kings-obey.md | 11 + .../__tests__/integration/requires.test.ts | 4 +- .../__tests__/integration/value-types.test.ts | 29 +- internals-js/src/__tests__/operations.test.ts | 585 ++++++++++++++++-- internals-js/src/operations.ts | 378 ++++++++--- .../features/basic/value-types.feature | 2 +- 6 files changed, 858 insertions(+), 151 deletions(-) create mode 100644 .changeset/rich-kings-obey.md diff --git a/.changeset/rich-kings-obey.md b/.changeset/rich-kings-obey.md new file mode 100644 index 000000000..6b09639f0 --- /dev/null +++ b/.changeset/rich-kings-obey.md @@ -0,0 +1,11 @@ +--- +"@apollo/query-planner": patch +"@apollo/federation-internals": patch +"@apollo/gateway": patch +--- + +Improves reuse of named fragments in subgraph fetches. When a question has named fragments, the code tries to reuse +those fragment in subgraph fetches is those can apply (so when the fragment is fully queried in a single subgraph fetch). +However, the existing was only able to reuse those fragment in a small subset of cases. This change makes it much more +likely that _if_ a fragment can be reused, it will be. + diff --git a/gateway-js/src/__tests__/integration/requires.test.ts b/gateway-js/src/__tests__/integration/requires.test.ts index 9f60a7a41..3e24d1b6f 100644 --- a/gateway-js/src/__tests__/integration/requires.test.ts +++ b/gateway-js/src/__tests__/integration/requires.test.ts @@ -218,13 +218,13 @@ it('collapses nested requires with user-defined fragments', async () => { { user { __typename - id preferences { favorites { - animal color + animal } } + id } } }, diff --git a/gateway-js/src/__tests__/integration/value-types.test.ts b/gateway-js/src/__tests__/integration/value-types.test.ts index 58811c0d9..fdc638e56 100644 --- a/gateway-js/src/__tests__/integration/value-types.test.ts +++ b/gateway-js/src/__tests__/integration/value-types.test.ts @@ -98,14 +98,7 @@ describe('value types', () => { reviews { metadata { __typename - ... on KeyValue { - key - value - } - ... on Error { - code - message - } + ...Metadata } } } @@ -113,18 +106,22 @@ describe('value types', () => { reviews { metadata { __typename - ... on KeyValue { - key - value - } - ... on Error { - code - message - } + ...Metadata } } } } + + fragment Metadata on MetadataOrError { + ... on KeyValue { + key + value + } + ... on Error { + code + message + } + } }, }, Flatten(path: "topProducts.@") { diff --git a/internals-js/src/__tests__/operations.test.ts b/internals-js/src/__tests__/operations.test.ts index 0f38ed531..da70ddc84 100644 --- a/internals-js/src/__tests__/operations.test.ts +++ b/internals-js/src/__tests__/operations.test.ts @@ -32,6 +32,34 @@ function astSSet(...selections: SelectionNode[]): SelectionSetNode { } describe('fragments optimization', () => { + // Takes a query with fragments as inputs, expand all those fragments, and ensures that all the + // fragments gets optimized back, and that we get back the exact same query. + function testFragmentsRoundtrip({ + schema, + query, + expanded, + }: { + schema: Schema, + query: string, + expanded: string, + }) { + const operation = parseOperation(schema, query); + // We call `trimUnsatisfiableBranches` because the selections we care about in the query planner + // will effectively have had gone through that function (and even if that function wasn't called, + // the query planning algorithm would still end up removing unsatisfiable branches anyway), so + // it is a more interesting test. + const withoutFragments = operation.expandAllFragments().trimUnsatisfiableBranches(); + + expect(withoutFragments.toString()).toMatchString(expanded); + + // We force keeping all reused fragments, even if they are used only once, because the tests using + // this are just about testing the reuse of fragments and this make things shorter/easier to write. + // There is tests in `buildPlan.test.ts` that double-check that we don't reuse fragments used only + // once in actual query plans. + const optimized = withoutFragments.optimize(operation.selectionSet.fragments!, 1); + expect(optimized.toString()).toMatchString(operation.toString()); + } + test('handles fragments using other fragments', () => { const schema = parseSchema(` type Query { @@ -126,9 +154,8 @@ describe('fragments optimization', () => { `); const optimized = withoutFragments.optimize(operation.selectionSet.fragments!); - // Note that we expect onU to *not* be recreated because, by default, optimize only - // add add back a fragment if it is used at least twice (otherwise, the fragment just - // make the query bigger). + // Note that while we didn't use `onU` for `t` in the query, it's technically ok to use + // it and it makes the query smaller, so it gets used. expect(optimized.toString()).toMatchString(` fragment OnT1 on T1 { a @@ -144,15 +171,17 @@ describe('fragments optimization', () => { b } + fragment OnU on U { + ...OnI + ...OnT1 + ...OnT2 + } + { t { - ...OnT1 - ...OnT2 - ...OnI + ...OnU u { - ...OnI - ...OnT1 - ...OnT2 + ...OnU } } } @@ -176,69 +205,537 @@ describe('fragments optimization', () => { } `); - const operation = parseOperation(schema, ` - fragment OnT1 on T1 { - t2 { - x + testFragmentsRoundtrip({ + schema, + query: ` + fragment OnT1 on T1 { + t2 { + x + } } - } - query { - t1a { - ...OnT1 - t2 { - y + query { + t1a { + ...OnT1 + t2 { + y + } + } + t2a { + ...OnT1 } } - t2a { - ...OnT1 + `, + expanded: ` + { + t1a { + t2 { + x + y + } + } + t2a { + t2 { + x + } + } } + `, + }); + }); + + test('handles nested fragments with field intersection', () => { + const schema = parseSchema(` + type Query { + t: T + } + + type T { + a: A + b: Int + } + + type A { + x: String + y: String + z: String } `); - const withoutFragments = parseOperation(schema, operation.toString(true, true)); - expect(withoutFragments.toString()).toMatchString(` - { - t1a { - ... on T1 { - t2 { + + // The subtlety here is that `FA` contains `__typename` and so after we're reused it, the + // selection will look like: + // { + // t { + // a { + // ...FA + // } + // } + // } + // But to recognize that `FT` can be reused from there, we need to be able to see that + // the `__typename` that `FT` wants is inside `FA` (and since FA applies on the parent type `A` + // directly, it is fine to reuse). + testFragmentsRoundtrip({ + schema, + query: ` + fragment FA on A { + __typename + x + y + } + + fragment FT on T { + a { + __typename + ...FA + } + } + + query { + t { + ...FT + } + } + `, + expanded: ` + { + t { + a { + __typename x + y } } - t2 { - y + } + `, + }); + }); + + test('handles fragment matching subset of field selection', () => { + const schema = parseSchema(` + type Query { + t: T + } + + type T { + a: String + b: B + c: Int + d: D + } + + type B { + x: String + y: String + } + + type D { + m: String + n: String + } + `); + + testFragmentsRoundtrip({ + schema, + query: ` + fragment FragT on T { + b { + __typename + x + } + c + d { + m } } - t2a { - ... on T1 { - t2 { + + { + t { + ...FragT + d { + n + } + a + } + } + `, + expanded: ` + { + t { + b { + __typename x } + c + d { + m + n + } + a } } + `, + }); + }); + + test('handles fragment matching subset of inline fragment selection', () => { + // Pretty much the same test than the previous one, but matching inside a fragment selection inside + // of inside a field selection. + const schema = parseSchema(` + type Query { + i: I + } + + interface I { + a: String + } + + type T { + a: String + b: B + c: Int + d: D + } + + type B { + x: String + y: String + } + + type D { + m: String + n: String } `); - const optimized = withoutFragments.optimize(operation.selectionSet.fragments!); - expect(optimized.toString()).toMatchString(` - fragment OnT1 on T1 { - t2 { - x + testFragmentsRoundtrip({ + schema, + query: ` + fragment FragT on T { + b { + __typename + x + } + c + d { + m + } + } + + { + i { + ... on T { + ...FragT + d { + n + } + a + } + } } + `, + expanded: ` + { + i { + ... on T { + b { + __typename + x + } + c + d { + m + n + } + a + } + } + } + `, + }); + }); + + test('intersecting fragments', () => { + const schema = parseSchema(` + type Query { + t: T } - { - t1a { - ...OnT1 - t2 { - y + type T { + a: String + b: B + c: Int + d: D + } + + type B { + x: String + y: String + } + + type D { + m: String + n: String + } + `); + + testFragmentsRoundtrip({ + schema, + // Note: the code that reuse fragments iterates on fragments in the order they are defined in the document, but when it reuse + // a fragment, it puts it at the beginning of the selection (somewhat random, it just feel often easier to read), so the net + // effect on this example is that `Frag2`, which will be reused after `Frag1` will appear first in the re-optimized selection. + // So we put it first in the input too so that input and output actually match (the `testFragmentsRoundtrip` compares strings, + // so it is sensible to ordering; we could theoretically use `Operation.equals` instead of string equality, which wouldn't + // really on ordering, but `Operation.equals` is not entirely trivial and comparing strings make problem a bit more obvious). + query: ` + fragment Frag1 on T { + b { + x + } + c + d { + m } } - t2a { - ...OnT1 + + fragment Frag2 on T { + a + b { + __typename + x + } + d { + m + n + } } + + { + t { + ...Frag2 + ...Frag1 + } + } + `, + expanded: ` + { + t { + a + b { + __typename + x + } + d { + m + n + } + c + } + } + `, + }); + }); + + test('fragments whose application makes a type condition trivial', () => { + const schema = parseSchema(` + type Query { + t: T + } + + interface I { + x: String + } + + type T implements I { + x: String + a: String } `); + + testFragmentsRoundtrip({ + schema, + query: ` + fragment FragI on I { + x + ... on T { + a + } + } + + { + t { + ...FragI + } + } + `, + expanded: ` + { + t { + x + a + } + } + `, + }); + }); + + describe('applied directives', () => { + test('reuse fragments with directives on the fragment, but only when there is those directives', () => { + const schema = parseSchema(` + type Query { + t1: T + t2: T + t3: T + } + + type T { + a: Int + b: Int + c: Int + d: Int + } + `); + + testFragmentsRoundtrip({ + schema, + query: ` + fragment DirectiveOnDef on T @include(if: $cond1) { + a + } + + query myQuery($cond1: Boolean!, $cond2: Boolean!) { + t1 { + ...DirectiveOnDef + } + t2 { + ... on T @include(if: $cond2) { + a + } + } + t3 { + ...DirectiveOnDef @include(if: $cond2) + } + } + `, + expanded: ` + query myQuery($cond1: Boolean!, $cond2: Boolean!) { + t1 { + ... on T @include(if: $cond1) { + a + } + } + t2 { + ... on T @include(if: $cond2) { + a + } + } + t3 { + ... on T @include(if: $cond1) @include(if: $cond2) { + a + } + } + } + `, + }); + }); + + test('reuse fragments with directives in the fragment selection, but only when there is those directives', () => { + const schema = parseSchema(` + type Query { + t1: T + t2: T + t3: T + } + + type T { + a: Int + b: Int + c: Int + d: Int + } + `); + + testFragmentsRoundtrip({ + schema, + query: ` + fragment DirectiveInDef on T { + a @include(if: $cond1) + } + + query myQuery($cond1: Boolean!, $cond2: Boolean!) { + t1 { + a + } + t2 { + ...DirectiveInDef + } + t3 { + a @include(if: $cond2) + } + } + `, + expanded: ` + query myQuery($cond1: Boolean!, $cond2: Boolean!) { + t1 { + a + } + t2 { + a @include(if: $cond1) + } + t3 { + a @include(if: $cond2) + } + } + `, + }); + }); + + test('reuse fragments with directives on spread, but only when there is those directives', () => { + const schema = parseSchema(` + type Query { + t1: T + t2: T + t3: T + } + + type T { + a: Int + b: Int + c: Int + d: Int + } + `); + + testFragmentsRoundtrip({ + schema, + query: ` + fragment NoDirectiveDef on T { + a + } + + query myQuery($cond1: Boolean!) { + t1 { + ...NoDirectiveDef + } + t2 { + ...NoDirectiveDef @include(if: $cond1) + } + } + `, + expanded: ` + query myQuery($cond1: Boolean!) { + t1 { + a + } + t2 { + ... on T @include(if: $cond1) { + a + } + } + } + `, + }); + }); }); }); diff --git a/internals-js/src/operations.ts b/internals-js/src/operations.ts index fddc62360..27a20ba27 100644 --- a/internals-js/src/operations.ts +++ b/internals-js/src/operations.ts @@ -25,7 +25,6 @@ import { isCompositeType, isInterfaceType, isNullableType, - isUnionType, ObjectType, runtimeTypesIntersects, Schema, @@ -50,8 +49,8 @@ import { } from "./definitions"; import { isInterfaceObjectType } from "./federation"; import { ERRORS } from "./error"; -import { isDirectSubtype, sameType } from "./types"; -import { assert, mapEntries, mapValues, MapWithCachedArrays, MultiMap, SetMultiMap } from "./utils"; +import { sameType } from "./types"; +import { assert, isDefined, mapEntries, mapValues, MapWithCachedArrays, MultiMap, SetMultiMap } from "./utils"; import { argumentsEquals, argumentsFromAST, isValidValue, valueToAST, valueToString } from "./values"; import { v1 as uuidv1 } from 'uuid'; @@ -889,15 +888,13 @@ export class NamedFragmentDefinition extends DirectiveTargetElement((prev, val) => prev?.minus(val), thisSelection); + if (selectionDiff) { + updated.add(selectionDiff); } } } @@ -1768,7 +1812,6 @@ export function selectionOfElement(element: OperationElement, subSelection?: Sel } export type Selection = FieldSelection | FragmentSelection; - abstract class AbstractSelection> { constructor( readonly element: TElement, @@ -1839,6 +1882,63 @@ abstract class AbstractSelection boolean, + }): SelectionSet | FragmentSpreadSelection { + let candidates = fragments.maybeApplyingAtType(parentType); + if (fragmentFilter) { + candidates = candidates.filter(fragmentFilter); + } + let shouldTryAgain: boolean; + do { + const { spread, optimizedSelection, hasDiff } = this.tryOptimizeSubselectionOnce({ parentType, subSelection, candidates, fragments }); + if (optimizedSelection) { + subSelection = optimizedSelection; + } else if (spread) { + return spread; + } + shouldTryAgain = !!spread && !!hasDiff; + if (shouldTryAgain) { + candidates = candidates.filter((c) => c !== spread?.namedFragment) + } + } while (shouldTryAgain); + return subSelection; + } } export class FieldSelection extends AbstractSelection, undefined, FieldSelection> { @@ -1868,41 +1968,21 @@ export class FieldSelection extends AbstractSelection, undefined, Fie } optimize(fragments: NamedFragments): Selection { - const optimizedSelection = this.selectionSet ? this.selectionSet.optimize(fragments) : undefined; + let optimizedSelection = this.selectionSet ? this.selectionSet.optimize(fragments) : undefined; const fieldBaseType = baseType(this.element.definition.type!); if (isCompositeType(fieldBaseType) && optimizedSelection) { - for (const candidate of fragments.maybeApplyingAtType(fieldBaseType)) { - // TODO: Checking `equals` here is very simple, but somewhat restrictive in theory. That is, if a query - // is: - // { - // t { - // a - // b - // c - // } - // } - // and we have: - // fragment X on T { - // t { - // a - // b - // } - // } - // then the current code will not use the fragment because `c` is not in the fragment, but in relatity, - // we could use it and make the result be: - // { - // ...X - // t { - // c - // } - // } - // To do that, we can change that `equals` to `contains`, but then we should also "extract" the remainder - // of `optimizedSelection` that isn't covered by the fragment, and that is the part slighly more involved. - if (optimizedSelection.equals(candidate.selectionSet)) { - const fragmentSelection = new FragmentSpreadSelection(fieldBaseType, fragments, candidate, []); - return new FieldSelection(this.element, selectionSetOf(fieldBaseType, fragmentSelection)); - } - } + const optimized = this.tryOptimizeSubselectionWithFragments({ + parentType: fieldBaseType, + subSelection: optimizedSelection, + fragments, + // We can never apply a fragments that has directives on it at the field level (but when those are expanded, + // their type condition would always be preserved due to said applied directives, so they will always + // be handled by `InlineFragmentSelection.optimize` anyway). + fragmentFilter: (f) => f.appliedDirectives.length === 0, + }); + + assert(!(optimized instanceof FragmentSpreadSelection), 'tryOptimizeSubselectionOnce should never return only a spread'); + optimizedSelection = optimized; } return this.selectionSet === optimizedSelection @@ -1910,6 +1990,37 @@ export class FieldSelection extends AbstractSelection, undefined, Fie : new FieldSelection(this.element, optimizedSelection); } + protected tryOptimizeSubselectionOnce({ + parentType, + subSelection, + candidates, + fragments, + }: { + parentType: CompositeType, + subSelection: SelectionSet, + candidates: NamedFragmentDefinition[], + fragments: NamedFragments, + }): { + spread?: FragmentSpreadSelection, + optimizedSelection?: SelectionSet, + hasDiff?: boolean, + }{ + let optimizedSelection = subSelection; + for (const candidate of candidates) { + const { contains, diff } = optimizedSelection.diffIfContains(candidate.selectionSet); + if (contains) { + // We can optimize the selection with this fragment. The replaced sub-selection will be + // comprised of this new spread and the remaining `diff` if there is any. + const spread = new FragmentSpreadSelection(parentType, fragments, candidate, []); + optimizedSelection = diff + ? new SelectionSetUpdates().add(spread).add(diff).toSelectionSet(parentType, fragments) + : selectionSetOf(parentType, spread); + return { spread, optimizedSelection, hasDiff: !!diff } + } + } + return {}; + } + filter(predicate: (selection: Selection) => boolean): FieldSelection | undefined { if (!this.selectionSet) { return predicate(this) ? this : undefined; @@ -2066,6 +2177,11 @@ export class FieldSelection extends AbstractSelection, undefined, Fie return !!this.selectionSet && this.selectionSet.contains(that.selectionSet); } + isUnecessaryInlineFragment(_: CompositeType): this is InlineFragmentSelection { + // Overridden by inline fragments + return false; + } + toString(expandFragments: boolean = true, indent?: string): string { return (indent ?? '') + this.element + (this.selectionSet ? ' ' + this.selectionSet.toString(expandFragments, true, indent) : ''); } @@ -2106,20 +2222,19 @@ export abstract class FragmentSelection extends AbstractSelection t.name === parentType.name)) + ); } + } class InlineFragmentSelection extends FragmentSelection { @@ -2205,37 +2320,83 @@ class InlineFragmentSelection extends FragmentSelection { let optimizedSelection = this.selectionSet.optimize(fragments); const typeCondition = this.element.typeCondition; if (typeCondition) { - for (const candidate of fragments.maybeApplyingAtType(typeCondition)) { - // See comment in `FieldSelection.optimize` about the `equals`: this fully apply here too. - if (optimizedSelection.equals(candidate.selectionSet)) { - let spreadDirectives: Directive[] = []; - if (this.element.appliedDirectives) { + const optimized = this.tryOptimizeSubselectionWithFragments({ + parentType: typeCondition, + subSelection: optimizedSelection, + fragments, + }); + if (optimized instanceof FragmentSpreadSelection) { + // This means the whole inline fragment can be replaced by the spread. + return optimized; + } + optimizedSelection = optimized; + } + return this.selectionSet === optimizedSelection + ? this + : new InlineFragmentSelection(this.element, optimizedSelection); + } + + protected tryOptimizeSubselectionOnce({ + parentType, + subSelection, + candidates, + fragments, + }: { + parentType: CompositeType, + subSelection: SelectionSet, + candidates: NamedFragmentDefinition[], + fragments: NamedFragments, + }): { + spread?: FragmentSpreadSelection, + optimizedSelection?: SelectionSet, + hasDiff?: boolean, + }{ + let optimizedSelection = subSelection; + for (const candidate of candidates) { + const { contains, diff } = optimizedSelection.diffIfContains(candidate.selectionSet); + if (contains) { + // The candidate selection is included in our sub-selection. One remaining thing to take into account + // is applied directives: if the candidate has directives, then we can only use it if 1) there is + // no `diff`, 2) the type condition of this fragment matches the candidate one and 3) the directives + // in question are also on this very fragment. In that case, we can replace this whole inline fragment + // by a spread of the candidate. + if (!diff && sameType(this.element.typeCondition!, candidate.typeCondition)) { + // We can potentially replace the whole fragment by the candidate; but as said above, still needs + // to check the directives. + let spreadDirectives: Directive[] = this.element.appliedDirectives; + if (candidate.appliedDirectives.length > 0) { const { isSubset, difference } = diffDirectives(this.element.appliedDirectives, candidate.appliedDirectives); if (!isSubset) { - // This means that while the named fragments matches the sub-selection, that name fragment also include some - // directives that are _not_ on our element, so we cannot use it. + // While the candidate otherwise match, it has directives that are not on this element, so we + // cannot reuse it. continue; } + // Otherwise, any directives on this element that are not on the candidate should be kept and used + // on the spread created. spreadDirectives = difference; } + // Returning a spread without a subselection will make the code "replace" this whole inline fragment + // by the spread, which is what we want. Do not that as we're replacing the whole inline fragment, + // we use `this.parentType` instead of `parentType` (the later being `this.element.typeCondition` basically). + return { + spread: new FragmentSpreadSelection(this.parentType, fragments, candidate, spreadDirectives), + }; + } - const newSelection = new FragmentSpreadSelection(this.parentType, fragments, candidate, spreadDirectives); - // We use the fragment when the fragments condition is either the same, or a supertype of our current condition. - // If it's the same type, then we don't really want to preserve the current condition, it is included in the - // spread and we can return it directly. But if the fragment condition is a superset, then we should preserve - // our current condition since it restricts the selection more than the fragment actual does. - if (sameType(typeCondition, candidate.typeCondition)) { - return newSelection; - } - - optimizedSelection = selectionSetOf(this.parentType, newSelection); - break; + // We're already dealt with the one case where we might be able to handle a candidate that has directives. + if (candidate.appliedDirectives.length > 0) { + continue; } + + const spread = new FragmentSpreadSelection(parentType, fragments, candidate, []); + optimizedSelection = diff + ? new SelectionSetUpdates().add(spread).add(diff).toSelectionSet(parentType, fragments) + : selectionSetOf(parentType, spread); + + return { spread, optimizedSelection, hasDiff: !!diff }; } } - return this.selectionSet === optimizedSelection - ? this - : new InlineFragmentSelection(this.element, optimizedSelection); + return {}; } withoutDefer(labelsToRemove?: Set): InlineFragmentSelection | SelectionSet { @@ -2353,6 +2514,22 @@ class InlineFragmentSelection extends FragmentSelection { return this.mapToSelectionSet((s) => s.expandFragments(names, updatedFragments)); } + equals(that: Selection): boolean { + if (this === that) { + return true; + } + + return (that instanceof FragmentSelection) + && this.element.equals(that.element) + && this.selectionSet.equals(that.selectionSet); + } + + contains(that: Selection): boolean { + return (that instanceof FragmentSelection) + && this.element.equals(that.element) + && this.selectionSet.contains(that.selectionSet); + } + toString(expandFragments: boolean = true, indent?: string): string { return (indent ?? '') + this.element + ' ' + this.selectionSet.toString(expandFragments, true, indent); } @@ -2372,7 +2549,7 @@ class FragmentSpreadSelection extends FragmentSelection { constructor( sourceType: CompositeType, private readonly fragments: NamedFragments, - private readonly namedFragment: NamedFragmentDefinition, + readonly namedFragment: NamedFragmentDefinition, private readonly spreadDirectives: readonly Directive[], ) { super(new FragmentElement(sourceType, namedFragment.typeCondition, namedFragment.appliedDirectives.concat(spreadDirectives))); @@ -2478,6 +2655,31 @@ class FragmentSpreadSelection extends FragmentSelection { assert(false, 'Unsupported, see `Operation.withAllDeferLabelled`'); } + minus(that: Selection): undefined { + assert(this.equals(that), () => `Invalid operation for ${this.toString(false)} and ${that.toString(false)}`); + return undefined; + } + + equals(that: Selection): boolean { + if (this === that) { + return true; + } + + return (that instanceof FragmentSpreadSelection) + && this.namedFragment.name === that.namedFragment.name + && sameDirectiveApplications(this.spreadDirectives, that.spreadDirectives); + } + + contains(that: Selection): boolean { + if (this.equals(that)) { + return true; + } + + return (that instanceof FragmentSelection) + && this.element.equals(that.element) + && this.selectionSet.contains(that.selectionSet); + } + toString(expandFragments: boolean = true, indent?: string): string { if (expandFragments) { return (indent ?? '') + this.element + ' ' + this.selectionSet.toString(true, true, indent); diff --git a/query-planner-js/src/__tests__/features/basic/value-types.feature b/query-planner-js/src/__tests__/features/basic/value-types.feature index a7fc9a96a..edde843a8 100644 --- a/query-planner-js/src/__tests__/features/basic/value-types.feature +++ b/query-planner-js/src/__tests__/features/basic/value-types.feature @@ -79,7 +79,7 @@ Scenario: resolves value types within their respective services ], "variableUsages": [], "operationKind": "query", - "operation": "query ProducsWithMetadata__reviews__1($representations:[_Any!]!){_entities(representations:$representations){...on Book{reviews{metadata{__typename ...on KeyValue{key value}...on Error{code message}}}}...on Furniture{reviews{metadata{__typename ...on KeyValue{key value}...on Error{code message}}}}}}", + "operation": "query ProducsWithMetadata__reviews__1($representations:[_Any!]!){_entities(representations:$representations){...on Book{reviews{metadata{__typename ...Metadata}}}...on Furniture{reviews{metadata{__typename ...Metadata}}}}}fragment Metadata on MetadataOrError{...on KeyValue{key value}...on Error{code message}}", "operationName": "ProducsWithMetadata__reviews__1" } },