From a9385bdb0d6f5e9b03f9c143bbc7f34e5b5bf166 Mon Sep 17 00:00:00 2001 From: Chris Lenfest Date: Thu, 9 Mar 2023 14:24:50 -0600 Subject: [PATCH] subscriptions support in the query planner (#2389) * Subscriptions support in the query planner --- .changeset/forty-hairs-flow.md | 7 + .changeset/many-rats-allow.md | 11 - .../__tests__/compose.subscription.test.ts | 135 ++++++++++++ composition-js/src/merging/merge.ts | 41 +++- .../src/matchers/toCallService.ts | 10 +- gateway-js/src/executeQueryPlan.ts | 4 + internals-js/src/definitions.ts | 8 + query-planner-js/src/QueryPlan.ts | 12 +- .../buildPlan.interfaceObject.test.ts | 4 +- .../__tests__/buildPlan.subscription.test.ts | 207 ++++++++++++++++++ query-planner-js/src/__tests__/testHelper.ts | 8 +- query-planner-js/src/buildPlan.ts | 74 +++++-- .../queryPlanSerializer.ts | 20 +- 13 files changed, 491 insertions(+), 50 deletions(-) create mode 100644 .changeset/forty-hairs-flow.md delete mode 100644 .changeset/many-rats-allow.md create mode 100644 composition-js/src/__tests__/compose.subscription.test.ts create mode 100644 query-planner-js/src/__tests__/buildPlan.subscription.test.ts diff --git a/.changeset/forty-hairs-flow.md b/.changeset/forty-hairs-flow.md new file mode 100644 index 000000000..b887a8204 --- /dev/null +++ b/.changeset/forty-hairs-flow.md @@ -0,0 +1,7 @@ +--- +"@apollo/composition": minor +"@apollo/query-planner": minor +--- + +Addition of new query planner node types to enable federated subscriptions support + \ No newline at end of file diff --git a/.changeset/many-rats-allow.md b/.changeset/many-rats-allow.md deleted file mode 100644 index 45e66acdb..000000000 --- a/.changeset/many-rats-allow.md +++ /dev/null @@ -1,11 +0,0 @@ ---- -"@apollo/composition": patch -"apollo-federation-integration-testsuite": patch -"@apollo/gateway": patch -"@apollo/federation-internals": patch -"@apollo/query-graphs": patch -"@apollo/query-planner": patch -"@apollo/subgraph": patch ---- - -Add `changesets` for release management and changelog automation diff --git a/composition-js/src/__tests__/compose.subscription.test.ts b/composition-js/src/__tests__/compose.subscription.test.ts new file mode 100644 index 000000000..59c0a6e23 --- /dev/null +++ b/composition-js/src/__tests__/compose.subscription.test.ts @@ -0,0 +1,135 @@ +import { FEDERATION2_LINK_WITH_FULL_IMPORTS, FieldDefinition } from '@apollo/federation-internals'; +import gql from 'graphql-tag'; +import { composeServices } from '../compose'; + +describe('subscription composition tests', () => { + it('type subscription appears in the supergraph', () => { + const subgraphA = { + name: 'subgraphA', + typeDefs: gql` + extend schema + ${FEDERATION2_LINK_WITH_FULL_IMPORTS} + type Query { + me: User! + } + + type Subscription { + onNewUser: User! + } + + type User { + id: ID! + name: String! + } + `, + }; + + const subgraphB = { + name: 'subgraphB', + typeDefs:gql` + extend schema + ${FEDERATION2_LINK_WITH_FULL_IMPORTS} + type Query { + foo: Int + } + + type Subscription { + bar: Int + } + `, + }; + + const { errors, schema } = composeServices([subgraphA, subgraphB]); + expect(errors).toBeUndefined(); + expect(schema).toBeDefined(); + const onNewUser = schema?.elementByCoordinate('Subscription.onNewUser') as FieldDefinition; + expect(onNewUser.appliedDirectives?.[0].toString()).toBe('@join__field(graph: SUBGRAPHA)'); + }); + + it.each([ + { directive: '@shareable', errorMsg: 'Fields on root level subscription object cannot be marked as shareable'}, + ])('directives that are incompatible with subscriptions wont compose', ({ directive, errorMsg }) => { + const subgraphA = { + name: 'subgraphA', + typeDefs: gql` + extend schema + ${FEDERATION2_LINK_WITH_FULL_IMPORTS} + type Query { + me: User! + } + + type Subscription { + onNewUser: User! ${directive} + } + + type User { + id: ID! + name: String! + } + `, + }; + + const subgraphB = { + name: 'subgraphB', + typeDefs:gql` + extend schema + ${FEDERATION2_LINK_WITH_FULL_IMPORTS} + type Query { + foo: Int + } + + type Subscription { + bar: Int + } + `, + }; + + const { errors, schema } = composeServices([subgraphA, subgraphB]); + expect(errors?.length).toBe(1); + expect(errors?.[0].message).toBe(errorMsg); + expect(schema).toBeUndefined(); + }); + + it('subscription name collisions across subgraphs should not compose', () => { + const subgraphA = { + name: 'subgraphA', + typeDefs: gql` + extend schema + ${FEDERATION2_LINK_WITH_FULL_IMPORTS} + type Query { + me: User! + } + + type Subscription { + onNewUser: User + foo: Int! + } + + type User { + id: ID! + name: String! + } + `, + }; + + const subgraphB = { + name: 'subgraphB', + typeDefs:gql` + extend schema + ${FEDERATION2_LINK_WITH_FULL_IMPORTS} + type Query { + foo: Int + } + + type Subscription { + foo: Int! + } + `, + }; + + const { errors, schema } = composeServices([subgraphA, subgraphB]); + expect(errors?.length).toBe(1); + expect(errors?.[0].message).toBe('Non-shareable field "Subscription.foo" is resolved from multiple subgraphs: it is resolved from subgraphs "subgraphA" and "subgraphB" and defined as non-shareable in all of them'); + expect(schema).toBeUndefined(); + }); +}); diff --git a/composition-js/src/merging/merge.ts b/composition-js/src/merging/merge.ts index 8531aa7c6..a4c261858 100644 --- a/composition-js/src/merging/merge.ts +++ b/composition-js/src/merging/merge.ts @@ -791,6 +791,7 @@ class Merger { private mergeObject(sources: (ObjectType | undefined)[], dest: ObjectType) { const isEntity = this.hintOnInconsistentEntity(sources, dest); const isValueType = !isEntity && !dest.isRootType(); + const isSubscription = dest.isSubscriptionRootType(); this.addFieldsShallow(sources, dest); if (!dest.hasFields()) { @@ -806,7 +807,15 @@ class Merger { const subgraphFields = sources.map(t => t?.field(destField.name)); const mergeContext = this.validateOverride(subgraphFields, destField); - this.mergeField(subgraphFields, destField, mergeContext); + if (isSubscription) { + this.validateSubscriptionField(subgraphFields); + } + + this.mergeField({ + sources: subgraphFields, + dest: destField, + mergeContext, + }); this.validateFieldSharing(subgraphFields, destField, mergeContext); } } @@ -1085,7 +1094,7 @@ class Merger { const { subgraphsWithOverride, subgraphMap } = sources.map((source, idx) => { if (!source) { // While the subgraph may not have the field directly, it could have "stand-in" for that field - // through @interfaceObject, and it is those stand-ins that would be effectively overridden. + // through @interfaceObject, and it is those stand-ins that would be effectively overridden. const interfaceObjectAbstractingFields = this.fieldsInSourceIfAbstractedByInterfaceObject(dest, idx); if (interfaceObjectAbstractingFields.length > 0) { return { @@ -1251,7 +1260,15 @@ class Merger { }).filter(isDefined); } - private mergeField(sources: FieldOrUndefinedArray, dest: FieldDefinition, mergeContext: FieldMergeContext = new FieldMergeContext(sources)) { + private mergeField({ + sources, + dest, + mergeContext = new FieldMergeContext(sources), + }: { + sources: FieldOrUndefinedArray, + dest: FieldDefinition, + mergeContext: FieldMergeContext, + }) { if (sources.every((s, i) => s === undefined ? this.fieldsInSourceIfAbstractedByInterfaceObject(dest, i).every((f) => this.isExternal(i, f)) : this.isExternal(i, s))) { const definingSubgraphs = sources.map((source, i) => { if (source) { @@ -1790,7 +1807,11 @@ class Merger { } const subgraphFields = sources.map(t => t?.field(destField.name)); const mergeContext = this.validateOverride(subgraphFields, destField); - this.mergeField(subgraphFields, destField, mergeContext); + this.mergeField({ + sources: subgraphFields, + dest: destField, + mergeContext, + }); } } @@ -2805,4 +2826,16 @@ class Merger { : err; }); } + + private validateSubscriptionField(sources: FieldOrUndefinedArray) { + // no subgraph marks field as @shareable + const fieldsWithShareable = sources.filter((src, idx) => src && src.appliedDirectivesOf(this.metadata(idx).shareableDirective()).length > 0); + if (fieldsWithShareable.length > 0) { + const nodes = sourceASTs(...fieldsWithShareable); + this.errors.push(ERRORS.INVALID_FIELD_SHARING.err( + `Fields on root level subscription object cannot be marked as shareable`, + { nodes}, + )); + } + } } diff --git a/federation-integration-testsuite-js/src/matchers/toCallService.ts b/federation-integration-testsuite-js/src/matchers/toCallService.ts index 28a51ef78..439f6e463 100644 --- a/federation-integration-testsuite-js/src/matchers/toCallService.ts +++ b/federation-integration-testsuite-js/src/matchers/toCallService.ts @@ -1,4 +1,4 @@ -import { QueryPlan, PlanNode } from '@apollo/query-planner'; +import { QueryPlan, PlanNode, SubscriptionNode } from '@apollo/query-planner'; import { astSerializer, queryPlanSerializer } from '../snapshotSerializers'; import prettyFormat from 'pretty-format'; @@ -41,9 +41,9 @@ function toCallService( let pass = false; // let initialServiceCall = null; // recurse the node, find first match of service name, return - function walkExecutionNode(node?: PlanNode) { + function walkExecutionNode(node?: PlanNode | SubscriptionNode) { if (!node) return; - if (node.kind === 'Fetch' && node.serviceName === service) { + if ((node.kind === 'Fetch') && node.serviceName === service) { pass = true; // initialServiceCall = node; return; @@ -56,6 +56,10 @@ function toCallService( case 'Sequence': node.nodes.forEach(walkExecutionNode); break; + case 'Subscription': + walkExecutionNode(node.primary); + walkExecutionNode(node.rest); + break; default: return; } diff --git a/gateway-js/src/executeQueryPlan.ts b/gateway-js/src/executeQueryPlan.ts index ed1afec05..fbd90d003 100644 --- a/gateway-js/src/executeQueryPlan.ts +++ b/gateway-js/src/executeQueryPlan.ts @@ -129,6 +129,10 @@ export async function executeQueryPlan( requestContext.metrics && requestContext.metrics.captureTraces ); + if (queryPlan.node?.kind === 'Subscription') { + throw new Error('Execution of subscriptions not supported by gateway'); + } + if (queryPlan.node) { const traceNode = await executeNode( context, diff --git a/internals-js/src/definitions.ts b/internals-js/src/definitions.ts index be3cf4d23..4f52d54c3 100644 --- a/internals-js/src/definitions.ts +++ b/internals-js/src/definitions.ts @@ -2091,6 +2091,14 @@ export class ObjectType extends FieldBasedType return schema.schemaDefinition.root('query')?.type === this; } + /** + * Whether this type is the "subscription" root type of the schema (will return false if the type is detached). + */ + isSubscriptionRootType(): boolean { + const schema = this.schema(); + return schema.schemaDefinition.root('subscription')?.type === this; + } + protected removeReferenceRecursive(ref: ObjectTypeReferencer): void { // Note that the ref can also be a`SchemaDefinition`, but don't have anything to do then. switch (ref.kind) { diff --git a/query-planner-js/src/QueryPlan.ts b/query-planner-js/src/QueryPlan.ts index 36515e90a..efeb75a12 100644 --- a/query-planner-js/src/QueryPlan.ts +++ b/query-planner-js/src/QueryPlan.ts @@ -11,7 +11,7 @@ export type ResponsePath = (string | number)[]; export interface QueryPlan { kind: 'QueryPlan'; - node?: PlanNode; + node?: PlanNode | SubscriptionNode; } export type PlanNode = SequenceNode | ParallelNode | FetchNode | FlattenNode | DeferNode | ConditionNode; @@ -26,6 +26,12 @@ export interface ParallelNode { nodes: PlanNode[]; } +export interface SubscriptionNode { + kind: 'Subscription'; + primary: FetchNode; + rest?: PlanNode; +} + export interface FetchNode { kind: 'Fetch'; serviceName: string; @@ -229,3 +235,7 @@ export const trimSelectionNodes = ( return remapped; }; + +export const isPlanNode = (node: PlanNode | SubscriptionNode | undefined): node is PlanNode => { + return !!node && node.kind !== 'Subscription'; +} diff --git a/query-planner-js/src/__tests__/buildPlan.interfaceObject.test.ts b/query-planner-js/src/__tests__/buildPlan.interfaceObject.test.ts index ff6125541..29180822c 100644 --- a/query-planner-js/src/__tests__/buildPlan.interfaceObject.test.ts +++ b/query-planner-js/src/__tests__/buildPlan.interfaceObject.test.ts @@ -1,5 +1,6 @@ -import { operationFromDocument } from '@apollo/federation-internals'; +import { assert, operationFromDocument } from '@apollo/federation-internals'; import gql from 'graphql-tag'; +import { isPlanNode } from '../QueryPlan'; import { composeAndCreatePlanner, findFetchNodes } from "./testHelper"; describe('basic @key on interface/@interfaceObject handling', () => { @@ -306,6 +307,7 @@ describe('basic @key on interface/@interfaceObject handling', () => { } `); + assert(isPlanNode(plan.node), 'buildQueryPlan should return QueryPlan'); const rewrites = findFetchNodes('S2', plan.node)[0].inputRewrites; expect(rewrites).toBeDefined(); expect(rewrites?.length).toBe(1); diff --git a/query-planner-js/src/__tests__/buildPlan.subscription.test.ts b/query-planner-js/src/__tests__/buildPlan.subscription.test.ts new file mode 100644 index 000000000..3f0e6c12a --- /dev/null +++ b/query-planner-js/src/__tests__/buildPlan.subscription.test.ts @@ -0,0 +1,207 @@ +import { operationFromDocument } from '@apollo/federation-internals'; +import gql from 'graphql-tag'; +import { composeAndCreatePlanner, composeAndCreatePlannerWithOptions } from './testHelper'; + +describe('subscription query plan tests', () => { + it('basic subscription query plan', () => { + const subgraphA = { + name: 'subgraphA', + typeDefs: gql` + type Query { + me: User! + } + + type Subscription { + onNewUser: User! + } + + type User @key(fields: "id") { + id: ID! + name: String! + } + `, + }; + + const subgraphB = { + name: 'subgraphB', + typeDefs: gql` + type Query { + foo: Int + } + + type User @key(fields: "id") { + id: ID! + address: String! + } + `, + }; + + const [api, queryPlanner] = composeAndCreatePlanner(subgraphA, subgraphB); + const operation = operationFromDocument( + api, + gql` + subscription MySubscription { + onNewUser { + id + name + address + } + } + `, + ); + + const plan = queryPlanner.buildQueryPlan(operation); + expect(plan).toMatchInlineSnapshot(` + QueryPlan { + Subscription { + Primary: { + Fetch(service: "subgraphA") { + { + onNewUser { + __typename + id + name + } + } + } + }, + Rest: { + Sequence { + Flatten(path: "onNewUser") { + Fetch(service: "subgraphB") { + { + ... on User { + __typename + id + } + } => + { + ... on User { + address + } + } + }, + }, + } + } + }, + } + `); + }); + it('basic subscription query plan, single subgraph', () => { + const subgraphA = { + name: 'subgraphA', + typeDefs: gql` + type Query { + me: User! + } + + type Subscription { + onNewUser: User! + } + + type User @key(fields: "id") { + id: ID! + name: String! + } + `, + }; + + const subgraphB = { + name: 'subgraphB', + typeDefs: gql` + type Query { + foo: Int + } + + type User @key(fields: "id") { + id: ID! + address: String! + } + `, + }; + + const [api, queryPlanner] = composeAndCreatePlanner(subgraphA, subgraphB); + const operation = operationFromDocument( + api, + gql` + subscription MySubscription { + onNewUser { + id + name + } + } + `, + ); + + const plan = queryPlanner.buildQueryPlan(operation); + expect(plan).toMatchInlineSnapshot(` + QueryPlan { + Subscription { + Primary: { + Fetch(service: "subgraphA") { + { + onNewUser { + id + name + } + } + } + }, + } + }, + } + `); + }); + + it('trying to use @defer with a description results in an error', () => { + const subgraphA = { + name: 'subgraphA', + typeDefs: gql` + type Query { + me: User! + } + + type Subscription { + onNewUser: User! + } + + type User @key(fields: "id") { + id: ID! + name: String! + } + `, + }; + + const subgraphB = { + name: 'subgraphB', + typeDefs: gql` + type Query { + foo: Int + } + + type User @key(fields: "id") { + id: ID! + address: String! + } + `, + }; + + const [api, queryPlanner] = composeAndCreatePlannerWithOptions([subgraphA, subgraphB], { incrementalDelivery: { enableDefer: true } }); + const operation = operationFromDocument( + api, + gql` + subscription MySubscription { + onNewUser { + id + ... @defer { + name + } + address + } + } + `, + ); + expect(() => queryPlanner.buildQueryPlan(operation)).toThrow('@defer is not supported on subscriptions'); + }); +}); diff --git a/query-planner-js/src/__tests__/testHelper.ts b/query-planner-js/src/__tests__/testHelper.ts index d143366a8..aceb46dd5 100644 --- a/query-planner-js/src/__tests__/testHelper.ts +++ b/query-planner-js/src/__tests__/testHelper.ts @@ -1,4 +1,4 @@ -import { astSerializer, FetchNode, PlanNode, queryPlanSerializer, QueryPlanner, QueryPlannerConfig } from '@apollo/query-planner'; +import { astSerializer, FetchNode, PlanNode, queryPlanSerializer, QueryPlanner, QueryPlannerConfig, SubscriptionNode } from '@apollo/query-planner'; import { composeServices } from '@apollo/composition'; import { asFed2SubgraphDocument, buildSchema, Schema, ServiceDefinition } from '@apollo/federation-internals'; @@ -20,7 +20,7 @@ export function composeAndCreatePlannerWithOptions(services: ServiceDefinition[] ]; } -export function findFetchNodes(subgraphName: string, node: PlanNode | undefined): FetchNode[] { +export function findFetchNodes(subgraphName: string, node: PlanNode | SubscriptionNode | undefined): FetchNode[] { if (!node) { return []; } @@ -41,5 +41,9 @@ export function findFetchNodes(subgraphName: string, node: PlanNode | undefined) return findFetchNodes(subgraphName, node.ifClause).concat( findFetchNodes(subgraphName, node.elseClause) ); + case 'Subscription': + return findFetchNodes(subgraphName, node.primary).concat( + findFetchNodes(subgraphName, node.rest) + ); } } diff --git a/query-planner-js/src/buildPlan.ts b/query-planner-js/src/buildPlan.ts index 7eb143bf3..c30c8e781 100644 --- a/query-planner-js/src/buildPlan.ts +++ b/query-planner-js/src/buildPlan.ts @@ -40,7 +40,6 @@ import { directiveApplicationsSubstraction, conditionalDirectivesInOperationPath, SetMultiMap, - ERRORS, OperationElement, Concrete, DeferDirectiveArgs, @@ -93,11 +92,11 @@ import { buildFederatedQueryGraph, FEDERATED_GRAPH_ROOT_SOURCE, } from "@apollo/query-graphs"; -import { stripIgnoredCharacters, print, parse, OperationTypeNode } from "graphql"; +import { stripIgnoredCharacters, print, OperationTypeNode } from "graphql"; import { DeferredNode, FetchDataInputRewrite, FetchDataOutputRewrite } from "."; import { enforceQueryPlannerConfigDefaults, QueryPlannerConfig } from "./config"; import { generateAllPlansAndFindBest } from "./generateAllPlans"; -import { QueryPlan, ResponsePath, SequenceNode, PlanNode, ParallelNode, FetchNode, trimSelectionNodes } from "./QueryPlan"; +import { QueryPlan, ResponsePath, SequenceNode, PlanNode, ParallelNode, FetchNode, SubscriptionNode, trimSelectionNodes } from "./QueryPlan"; const debug = newDebugLogger('plan'); @@ -997,7 +996,7 @@ class FetchGroup { if (interfaceInputSelections.length > 0) { return interfaceInputSelections.some((input) => input.selectionSet.contains(subSelectionSet)); } - return implementationInputSelections.length > 0 + return implementationInputSelections.length > 0 && implementationInputSelections.every((input) => input.selectionSet.contains(subSelectionSet)); }); } @@ -1143,7 +1142,7 @@ class FetchGroup { assert(childPathInThis, () => `Cannot remove useless ${child} of ${this}: the path of the former into the later is unknown`); this.dependencyGraph.onModification(); - // Removing the child means atttaching all it's children to the parent, so it's the same relocation than on a "mergeIn". + // Removing the child means atttaching all it's children to the parent, so it's the same relocation than on a "mergeIn". this.relocateChildrenOnMergedIn(child, childPathInThis); this.dependencyGraph.remove(child); } @@ -1205,7 +1204,7 @@ class FetchGroup { queryPlannerConfig: QueryPlannerConfig, variableDefinitions: VariableDefinitions, fragments?: NamedFragments, - operationName?: string + operationName?: string, ) : PlanNode | undefined { if (this.selection.isEmpty()) { return undefined; @@ -2310,7 +2309,7 @@ class FetchDependencyGraph { // a @defer B may be nested inside @defer A "in the query", but be such that we don't need anything fetched within // the deferred part of A to start the deferred part of B). // Long story short, we first collect the groups from `allDeferredGroups` that are _not_ in our current level, if - // any, and pass those to recursion call below so they can be use a their proper level of nestedness. + // any, and pass those to recursion call below so they can be use a their proper level of nestedness. const defersInCurrent = this.deferTracking.defersInParent(currentDeferRef); const handledDefersInCurrent = new Set(defersInCurrent.map((d) => d.label)); const unhandledDefersInCurrent = mapKeys(allDeferredGroups).filter((label) => !handledDefersInCurrent.has(label)); @@ -2485,12 +2484,7 @@ export class QueryPlanner { return { kind: 'QueryPlan' }; } - if (operation.rootKind === 'subscription') { - throw ERRORS.UNSUPPORTED_FEATURE.err( - 'Query planning does not currently support subscriptions.', - { nodes: [parse(operation.toString())] }, - ); - } + const isSubscription = operation.rootKind === 'subscription'; const statistics: PlanningStatistics = { evaluatedPlanCount: 0, @@ -2534,10 +2528,13 @@ export class QueryPlanner { operation = this.withSiblingTypenameOptimizedAway(operation); let assignedDeferLabels: Set | undefined = undefined; - let hasDefers: boolean = false; + let hasDefers = false; let deferConditions: SetMultiMap | undefined = undefined; if (this.config.incrementalDelivery.enableDefer) { ({ operation, hasDefers, assignedDeferLabels, deferConditions } = operation.withNormalizedDefer()); + if (isSubscription && hasDefers) { + throw new Error(`@defer is not supported on subscriptions`); + } } else { // If defer is not enabled, we remove all @defer from the query. This feels cleaner do this once here than // having to guard all the code dealing with defer later, and is probably less error prone too (less likely @@ -2562,7 +2559,7 @@ export class QueryPlanner { }); - let rootNode: PlanNode | undefined; + let rootNode: PlanNode | SubscriptionNode | undefined; if (deferConditions && deferConditions.size > 0) { assert(hasDefers, 'Should not have defer conditions without @defer'); rootNode = computePlanForDeferConditionals({ @@ -2586,6 +2583,36 @@ export class QueryPlanner { }); } + // If this is a subscription, we want to make sure that we return a SubscriptionNode rather than a PlanNode + // We potentially will need to separate "primary" from "rest" + // Note that if it is a subscription, we are guaranteed that nothing is deferred. + if (rootNode && isSubscription) { + switch (rootNode.kind) { + case 'Fetch': { + rootNode = { + kind: 'Subscription', + primary: rootNode, + }; + } + break; + case 'Sequence': { + const [primary, ...rest] = rootNode.nodes; + assert(primary.kind === 'Fetch', 'Primary node of a subscription is not a Fetch'); + rootNode = { + kind: 'Subscription', + primary, + rest: { + kind: 'Sequence', + nodes: rest, + }, + }; + } + break; + default: + throw new Error(`Unexpected top level PlanNode kind: '${rootNode.kind}' when processing subscription`); + } + } + debug.groupEnd('Query plan computed'); return { kind: 'QueryPlan', node: rootNode }; @@ -2625,8 +2652,8 @@ export class QueryPlanner { * be added back eventually. The core query planning algorithm will ignore that tag, and because * __typename has been otherwise removed, we'll save any related work. But as we build the final * query plan, we'll check back for those "tags" (see `getAttachement` in `computeGroupsForTree`), - * and when we fine one, we'll add back the request to __typename. As this only happen after the - * query planning algorithm has computed all choices, we achieve our goal of not considering useless + * and when we fine one, we'll add back the request to __typename. As this only happen after the + * query planning algorithm has computed all choices, we achieve our goal of not considering useless * choices due to __typename. Do note that if __typename is the "only" selection of some selection * set, then we leave it untouched, and let the query planning algorithm treat it as any other * field. We have no other choice in that case, and that's actually what we want. @@ -2653,11 +2680,11 @@ export class QueryPlanner { && !parentMaybeInterfaceObject ) { // The reason we check for `!typenameSelection` is that due to aliasing, there can be more than one __typename selection - // in theory, and so this will only kick in on the first one. This is fine in practice: it only means that if there _is_ + // in theory, and so this will only kick in on the first one. This is fine in practice: it only means that if there _is_ // 2 selection of __typename, then we won't optimise things as much as we could, but there is no practical reason // whatsoever to have 2 selection of __typename in the first place, so not being optimal is moot. // - // Also note that we do not remove __typename if on (interface) types that are implemented by + // Also note that we do not remove __typename if on (interface) types that are implemented by // an @interfaceObject in some subgraph: the reason is that those types are an exception to the rule // that __typename can be resolved from _any_ subgraph, as the __typename of @interfaceObject is not // one we should return externally and so cannot fulfill the user query. @@ -2774,7 +2801,6 @@ function computePlanInternal({ const dependencyGraph = computeRootParallelDependencyGraph(supergraphSchema, operation, federatedQueryGraph, root, 0, hasDefers, statistics); ({ main, deferred } = dependencyGraph.process(processor, operation.rootKind)); primarySelection = dependencyGraph.deferTracking.primarySelection; - } if (deferred.length > 0) { assert(primarySelection, 'Should have had a primary selection created'); @@ -2979,7 +3005,7 @@ function computeRootSerialDependencyGraph( // We have to serially compute a plan for each top-level selection. const splittedRoots = splitTopLevelFields(operation.selectionSet); const graphs: FetchDependencyGraph[] = []; - let startingFetchId: number = 0; + let startingFetchId = 0; let [prevDepGraph, prevPaths] = computeRootParallelBestPlan(supergraphSchema, splittedRoots[0], operation.variableDefinitions, federatedQueryGraph, root, startingFetchId, hasDefers, statistics); let prevSubgraph = onlyRootSubgraph(prevDepGraph); for (let i = 1; i < splittedRoots.length; i++) { @@ -3596,7 +3622,7 @@ function computeGroupsForTree( }); assert(updatedOperation, `Extracting @defer from ${operation} should not have resulted in no operation`); - let updated = { + const updated = { tree: child, group, path, @@ -3638,7 +3664,7 @@ function computeGroupsForTree( // } // } // but the trick is that the `__typename` in the input will be the name of the interface itself (`I` in this case) - // but the one return after the fetch will the name of the actual implementation (some implementation of `I`). + // but the one return after the fetch will the name of the actual implementation (some implementation of `I`). // *But* we later have optimizations that would remove such a group, on the group that the output is included // in the input, which is in general the right thing to do (and genuinely ensure that some useless groups created when // handling complex @require gets eliminated). So we "protect" the group in this case to ensure that later @@ -3838,7 +3864,7 @@ function handleRequires( // Otherwise, we will have to "keep it". // Note: it is to be sure this test is not poluted by other things in `group` that we created `newGroup`. newGroup.removeInputsFromSelection(); - let newGroupIsUnneeded = parent.path && newGroup.selection.canRebaseOn(typeAtPath(parent.group.selection.parentType, parent.path)); + const newGroupIsUnneeded = parent.path && newGroup.selection.canRebaseOn(typeAtPath(parent.group.selection.parentType, parent.path)); const unmergedGroups = []; if (newGroupIsUnneeded) { diff --git a/query-planner-js/src/snapshotSerializers/queryPlanSerializer.ts b/query-planner-js/src/snapshotSerializers/queryPlanSerializer.ts index 0ec91236c..23aa807bb 100644 --- a/query-planner-js/src/snapshotSerializers/queryPlanSerializer.ts +++ b/query-planner-js/src/snapshotSerializers/queryPlanSerializer.ts @@ -1,5 +1,5 @@ import { Config, Plugin, Refs } from 'pretty-format'; -import { DeferredNode, PlanNode, QueryPlan } from '../'; +import { DeferredNode, PlanNode, QueryPlan, SubscriptionNode } from '../'; import { parse, Kind, visit, DocumentNode } from 'graphql'; export default { @@ -31,7 +31,7 @@ export default { } as Plugin; function printNode( - node: PlanNode, + node: PlanNode | SubscriptionNode, config: Config, indentation: string, depth: number, @@ -56,7 +56,7 @@ function printNode( case 'Fetch': const idStr = node.id ? `, id: ${node.id}` : ''; result += - `Fetch(service: "${node.serviceName}"${idStr})` + + `${node.kind}(service: "${node.serviceName}"${idStr})` + ' {' + config.spacingOuter + (node.requires @@ -119,6 +119,18 @@ function printNode( indentation + '}' } break; + case 'Subscription': { + const primary = node.primary; + const rest = node.rest; + const indentationInner = indentationNext + config.indent; + result += 'Subscription {' + + config.spacingOuter + indentationNext + 'Primary: {' + config.spacingOuter + indentationInner + printNode(primary, config, indentationInner, depth, refs, printer) + + config.spacingOuter + indentationNext + '},' + + (rest ? (config.spacingOuter + indentationNext + 'Rest: {' + config.spacingOuter + indentationInner + printNode(rest, config, indentationInner, depth, refs, printer)) : '') + + config.spacingOuter + indentationNext + '}' + + config.spacingOuter + config.indent + '}'; // TODO: Is this right? + break; + } default: result += node.kind; } @@ -135,7 +147,7 @@ function printNode( } function printNodes( - nodes: PlanNode[] | undefined, + nodes: (SubscriptionNode | PlanNode)[] | undefined, config: Config, indentation: string, depth: number,