diff --git a/.changeset/popular-chairs-happen.md b/.changeset/popular-chairs-happen.md new file mode 100644 index 000000000..3f7d7a122 --- /dev/null +++ b/.changeset/popular-chairs-happen.md @@ -0,0 +1,11 @@ +--- +"apollo-federation-integration-testsuite": patch +"@apollo/query-planner": patch +"@apollo/query-graphs": patch +"@apollo/composition": patch +"@apollo/federation-internals": patch +"@apollo/subgraph": patch +"@apollo/gateway": patch +--- + +Various set context bugfixes diff --git a/composition-js/src/merging/merge.ts b/composition-js/src/merging/merge.ts index de8fa2877..40abaab41 100644 --- a/composition-js/src/merging/merge.ts +++ b/composition-js/src/merging/merge.ts @@ -1661,11 +1661,11 @@ class Merger { return true; } - // if there is a @fromContext directive on one of the sources, we need a join__field + // if there is a @fromContext directive on one of the source's arguments, we need a join__field if (sources.some((s, idx) => { const fromContextDirective = this.subgraphs.values()[idx].metadata().fromContextDirective(); - if (isFederationDirectiveDefinedInSchema(fromContextDirective)) { - return (s?.appliedDirectivesOf(fromContextDirective).length ?? 0) > 0; + if (s && isFederationDirectiveDefinedInSchema(fromContextDirective)) { + return s.kind === 'FieldDefinition' && s.arguments().some(arg => arg.appliedDirectivesOf(fromContextDirective).length > 0); } return false; })) { diff --git a/internals-js/src/federation.ts b/internals-js/src/federation.ts index bc36a73a0..02607c819 100644 --- a/internals-js/src/federation.ts +++ b/internals-js/src/federation.ts @@ -413,11 +413,12 @@ const validateFieldValueType = ({ } const { element, selectionSet: childSelectionSet } = selection; assert(element.definition.type, 'Element type definition should exist'); - const type = element.definition.type; + let type = element.definition.type; + if (childSelectionSet) { - assert(isCompositeType(type), 'Child selection sets should only exist on composite types'); + assert(isCompositeType(baseType(type)), 'Child selection sets should only exist on composite types'); const { resolvedType } = validateFieldValueType({ - currentType: type, + currentType: baseType(type) as CompositeType, selectionSet: childSelectionSet, errorCollector, metadata, diff --git a/internals-js/src/operations.ts b/internals-js/src/operations.ts index 4d80d98f5..3ac780491 100644 --- a/internals-js/src/operations.ts +++ b/internals-js/src/operations.ts @@ -368,10 +368,23 @@ export class Field ex if (this.name === typenameFieldName) { return parentType.typenameField()?.type; } - - return this.canRebaseOn(parentType) + + const returnType = this.canRebaseOn(parentType) ? parentType.field(this.name)?.type : undefined; + + // If the field has an argument with fromContextDirective on it. We should not rebase it. + const fromContextDirective = federationMetadata(parentType.schema())?.fromContextDirective(); + if (fromContextDirective && isFederationDirectiveDefinedInSchema(fromContextDirective)) { + const fieldInParent = parentType.field(this.name); + if (fieldInParent && fieldInParent.arguments() + .some(arg => arg.appliedDirectivesOf(fromContextDirective).length > 0 && (!this.args || this.args[arg.name] === undefined)) + ) { + return undefined; + } + } + + return returnType; } hasDefer(): boolean { diff --git a/query-graphs-js/src/graphPath.ts b/query-graphs-js/src/graphPath.ts index 1a1c198a9..eed54b0b8 100644 --- a/query-graphs-js/src/graphPath.ts +++ b/query-graphs-js/src/graphPath.ts @@ -653,6 +653,12 @@ export class GraphPath 0) { + return undefined; + } // Usually, the starting subgraph in which we want to look for a direct path is the head of // `subgraphEnteringEdge`, that is, where we were just before coming to the current subgraph. diff --git a/query-planner-js/src/__tests__/buildPlan.test.ts b/query-planner-js/src/__tests__/buildPlan.test.ts index 8c91fd560..af995c066 100644 --- a/query-planner-js/src/__tests__/buildPlan.test.ts +++ b/query-planner-js/src/__tests__/buildPlan.test.ts @@ -9643,4 +9643,396 @@ describe('@fromContext impacts on query planning', () => { }, ]); }); + + it('fromContext accesses a different top level query', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query @context(name: "topLevelQuery") { + me: User! + product: Product + } + + type User @key(fields: "id") { + id: ID! + locale: String! + } + + type Product @key(fields: "id") { + id: ID! + price( + locale: String + @fromContext(field: "$topLevelQuery { me { locale } }") + ): Int! + } + `, + }; + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type Query { + randomId: ID! + } + + type Product @key(fields: "id") { + id: ID! + } + `, + }; + + const asFed2Service = (service: ServiceDefinition) => { + return { + ...service, + typeDefs: asFed2SubgraphDocument(service.typeDefs, { + includeAllImports: true, + }), + }; + }; + + const composeAsFed2Subgraphs = (services: ServiceDefinition[]) => { + return composeServices(services.map((s) => asFed2Service(s))); + }; + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + expect(result.errors).toBeUndefined(); + const [api, queryPlanner] = [ + result.schema!.toAPISchema(), + new QueryPlanner(Supergraph.buildForTests(result.supergraphSdl!)), + ]; + // const [api, queryPlanner] = composeFed2SubgraphsAndCreatePlanner(subgraph1, subgraph2); + const operation = operationFromDocument( + api, + gql` + { + product { + price + } + } + `, + ); + + const plan = queryPlanner.buildQueryPlan(operation); + expect(plan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "Subgraph1") { + { + __typename + me { + locale + } + product { + __typename + id + } + } + }, + Flatten(path: "product") { + Fetch(service: "Subgraph1") { + { + ... on Product { + __typename + id + } + } => + { + ... on Product { + price(locale: $contextualArgument_1_0) + } + } + }, + }, + }, + } + `); + expect((plan as any).node.nodes[1].node.contextRewrites).toEqual([ + { + kind: 'KeyRenamer', + path: ['..', 'me', 'locale'], + renameKeyTo: 'contextualArgument_1_0', + }, + ]); + }); + + it('fromContext type does not exist on other subgraph', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + t: T! + } + type T @key(fields: "id") @context(name: "context") { + id: ID! + u: U! + prop: String! + } + type U @key(fields: "id") { + id: ID! + b: String! + field(a: String @fromContext(field: "$context { prop }")): Int! + } + `, + }; + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type Query { + randomId: ID! + } + `, + }; + + const asFed2Service = (service: ServiceDefinition) => { + return { + ...service, + typeDefs: asFed2SubgraphDocument(service.typeDefs, { + includeAllImports: true, + }), + }; + }; + + const composeAsFed2Subgraphs = (services: ServiceDefinition[]) => { + return composeServices(services.map((s) => asFed2Service(s))); + }; + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + expect(result.errors).toBeUndefined(); + const [api, queryPlanner] = [ + result.schema!.toAPISchema(), + new QueryPlanner(Supergraph.buildForTests(result.supergraphSdl!)), + ]; + // const [api, queryPlanner] = composeFed2SubgraphsAndCreatePlanner(subgraph1, subgraph2); + const operation = operationFromDocument( + api, + gql` + { + t { + u { + field + } + } + } + `, + ); + + const plan = queryPlanner.buildQueryPlan(operation); + expect(plan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "Subgraph1") { + { + t { + __typename + prop + u { + __typename + id + } + } + } + }, + Flatten(path: "t.u") { + Fetch(service: "Subgraph1") { + { + ... on U { + __typename + id + } + } => + { + ... on U { + field(a: $contextualArgument_1_0) + } + } + }, + }, + }, + } + `); + expect((plan as any).node.nodes[1].node.contextRewrites).toEqual([ + { + kind: 'KeyRenamer', + path: ['..', '... on T', 'prop'], + renameKeyTo: 'contextualArgument_1_0', + }, + ]); + }); + + it('fromContext required field is several levels deep going back and forth between subgraphs', () => { + const subgraph1 = { + name: 'Subgraph1', + url: 'https://Subgraph1', + typeDefs: gql` + type Query { + t: T! + } + + type A @key(fields: "id") { + id: ID! + b: B! @external + } + + type B @key(fields: "id") { + id: ID! + c: C! + } + + type C @key(fields: "id") { + id: ID! + prop: String! + } + + type T @key(fields: "id") @context(name: "context") { + id: ID! + u: U! + a: A! + } + type U @key(fields: "id") { + id: ID! + b: String! + field( + a: String @fromContext(field: "$context { a { b { c { prop }}} }") + ): Int! + } + `, + }; + + const subgraph2 = { + name: 'Subgraph2', + url: 'https://Subgraph2', + typeDefs: gql` + type Query { + randomId: ID! + } + + type A @key(fields: "id") { + id: ID! + b: B! + } + + type B @key(fields: "id") { + id: ID! + } + `, + }; + + const asFed2Service = (service: ServiceDefinition) => { + return { + ...service, + typeDefs: asFed2SubgraphDocument(service.typeDefs, { + includeAllImports: true, + }), + }; + }; + + const composeAsFed2Subgraphs = (services: ServiceDefinition[]) => { + return composeServices(services.map((s) => asFed2Service(s))); + }; + + const result = composeAsFed2Subgraphs([subgraph1, subgraph2]); + expect(result.errors).toBeUndefined(); + const [api, queryPlanner] = [ + result.schema!.toAPISchema(), + new QueryPlanner(Supergraph.buildForTests(result.supergraphSdl!)), + ]; + // const [api, queryPlanner] = composeFed2SubgraphsAndCreatePlanner(subgraph1, subgraph2); + const operation = operationFromDocument( + api, + gql` + { + t { + u { + field + } + } + } + `, + ); + + const plan = queryPlanner.buildQueryPlan(operation); + expect(plan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "Subgraph1") { + { + t { + __typename + a { + __typename + id + } + u { + __typename + id + } + } + } + }, + Flatten(path: "t.a") { + Fetch(service: "Subgraph2") { + { + ... on A { + __typename + id + } + } => + { + ... on A { + b { + __typename + id + } + } + } + }, + }, + Flatten(path: "t.a.b") { + Fetch(service: "Subgraph1") { + { + ... on B { + __typename + id + } + } => + { + ... on B { + c { + prop + } + } + } + }, + }, + Flatten(path: "t.u") { + Fetch(service: "Subgraph1") { + { + ... on U { + __typename + id + } + } => + { + ... on U { + field(a: $contextualArgument_1_0) + } + } + }, + }, + }, + } + `); + expect((plan as any).node.nodes[3].node.contextRewrites).toEqual([ + { + kind: 'KeyRenamer', + path: ['..', '... on T', 'a', 'b', 'c', 'prop'], + renameKeyTo: 'contextualArgument_1_0', + }, + ]); + }); }); diff --git a/query-planner-js/src/buildPlan.ts b/query-planner-js/src/buildPlan.ts index d81eafe7a..e61b2381f 100644 --- a/query-planner-js/src/buildPlan.ts +++ b/query-planner-js/src/buildPlan.ts @@ -1673,22 +1673,23 @@ type FieldToAlias = { alias: string, } -function selectionSetAsKeyRenamers(selectionSet: SelectionSet, relPath: string[], alias: string): FetchDataKeyRenamer[] { +function selectionSetAsKeyRenamers(selectionSet: SelectionSet | undefined, relPath: string[], alias: string): FetchDataKeyRenamer[] { + if (!selectionSet || selectionSet.isEmpty()) { + return [ + { + kind: 'KeyRenamer', + path: relPath, + renameKeyTo: alias, + } + ]; + } + return selectionSet.selections().map((selection: Selection): FetchDataKeyRenamer[] | undefined => { if (selection.kind === 'FieldSelection') { - // We always have at least one '..' in the relative path. - if (relPath[relPath.length - 1] === '..') { - return possibleRuntimeTypes(selectionSet.parentType).map((t) => ({ - kind: 'KeyRenamer', - path: [...relPath, `... on ${t.name}`, selection.element.name], - renameKeyTo: alias, - })); + if (relPath[relPath.length - 1] === '..' && selectionSet.parentType.name !== 'Query') { + return possibleRuntimeTypes(selectionSet.parentType).map((t) => selectionSetAsKeyRenamers(selectionSet, [...relPath, `... on ${t.name}`], alias)).flat(); } else { - return [{ - kind: 'KeyRenamer', - path: [...relPath, selection.element.name], - renameKeyTo: alias, - }]; + return selectionSetAsKeyRenamers(selection.selectionSet, [...relPath, selection.element.name], alias); } } else if (selection.kind === 'FragmentSelection') { const element = selection.element;