From 9bae09b87ad3a53745dbfbbf11cfb758e1674d70 Mon Sep 17 00:00:00 2001 From: Sylvain Lebresne Date: Wed, 26 Oct 2022 11:21:46 +0200 Subject: [PATCH 1/4] Implements @interfaceObject and @key on interfaces. Add support for the newly added `@interfaceObject` directive and for `@key` on interfaces, in order to allow using interfaces as abstraction accross subgraphs. See #2277 for details. Fixes #2277. --- .../compose.composeDirective.test.ts.snap | 6 +- composition-js/src/__tests__/compose.test.ts | 248 ++++- .../src/__tests__/validation_errors.test.ts | 136 ++- composition-js/src/merging/merge.ts | 194 +++- composition-js/src/validate.ts | 12 +- docs/source/errors.md | 5 +- .../src/__tests__/executeQueryPlan.test.ts | 419 +++++++++ .../__tests__/gateway/lifecycle-hooks.test.ts | 2 +- gateway-js/src/executeQueryPlan.ts | 46 +- .../src/__tests__/schemaUpgrader.test.ts | 43 + .../src/__tests__/subgraphValidation.test.ts | 155 ++- internals-js/src/__tests__/testUtils.ts | 28 + internals-js/src/__tests__/values.test.ts | 2 +- internals-js/src/definitions.ts | 12 + internals-js/src/error.ts | 24 +- .../src/extractSubgraphsFromSupergraph.ts | 18 +- internals-js/src/federation.ts | 127 ++- internals-js/src/federationSpec.ts | 11 +- internals-js/src/joinSpec.ts | 32 +- internals-js/src/operations.ts | 38 +- internals-js/src/schemaUpgrader.ts | 13 + internals-js/src/supergraphs.ts | 1 + query-graphs-js/src/graphPath.ts | 456 ++++++--- query-graphs-js/src/querygraph.ts | 173 +++- query-graphs-js/src/transition.ts | 73 +- query-planner-js/src/QueryPlan.ts | 6 + .../src/__tests__/buildPlan.defer.test.ts | 2 +- .../buildPlan.interfaceObject.test.ts | 412 ++++++++ .../src/__tests__/buildPlan.test.ts | 24 +- query-planner-js/src/__tests__/testHelper.ts | 45 + query-planner-js/src/buildPlan.ts | 886 ++++++++++++------ query-planner-js/src/index.ts | 40 +- .../src/__tests__/buildSubgraphSchema.test.ts | 69 +- subgraph-js/src/schemaExtensions.ts | 2 +- subgraph-js/src/types.ts | 144 ++- 35 files changed, 3256 insertions(+), 648 deletions(-) create mode 100644 internals-js/src/__tests__/testUtils.ts create mode 100644 query-planner-js/src/__tests__/buildPlan.interfaceObject.test.ts create mode 100644 query-planner-js/src/__tests__/testHelper.ts diff --git a/composition-js/src/__tests__/__snapshots__/compose.composeDirective.test.ts.snap b/composition-js/src/__tests__/__snapshots__/compose.composeDirective.test.ts.snap index 8ec5c03b0..26d1b2e58 100644 --- a/composition-js/src/__tests__/__snapshots__/compose.composeDirective.test.ts.snap +++ b/composition-js/src/__tests__/__snapshots__/compose.composeDirective.test.ts.snap @@ -3,7 +3,7 @@ exports[`composing custom core directives custom tag directive works when federation tag is renamed 1`] = ` "schema @link(url: \\"https://specs.apollo.dev/link/v1.0\\") - @link(url: \\"https://specs.apollo.dev/join/v0.2\\", for: EXECUTION) + @link(url: \\"https://specs.apollo.dev/join/v0.3\\", for: EXECUTION) @link(url: \\"https://specs.apollo.dev/tag/v0.2\\", as: \\"mytag\\") @link(url: \\"https://custom.dev/tag/v1.0\\", import: [\\"@tag\\"]) { @@ -14,9 +14,9 @@ directive @link(url: String, as: String, for: link__Purpose, import: [link__Impo directive @join__graph(name: String!, url: String!) on ENUM_VALUE -directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR +directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR -directive @join__field(graph: join__Graph!, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION +directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE diff --git a/composition-js/src/__tests__/compose.test.ts b/composition-js/src/__tests__/compose.test.ts index f38ad8f46..65bf69049 100644 --- a/composition-js/src/__tests__/compose.test.ts +++ b/composition-js/src/__tests__/compose.test.ts @@ -56,18 +56,18 @@ describe('composition', () => { expect(result.supergraphSdl).toMatchString(` schema @link(url: "https://specs.apollo.dev/link/v1.0") - @link(url: "https://specs.apollo.dev/join/v0.2", for: EXECUTION) + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) { query: Query } - directive @join__field(graph: join__Graph!, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION + directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION directive @join__graph(name: String!, url: String!) on ENUM_VALUE directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE - directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR + directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA @@ -2135,18 +2135,18 @@ describe('composition', () => { expect(printSchema(supergraph)).toMatchString(` schema @link(url: \"https://specs.apollo.dev/link/v1.0\") - @link(url: \"https://specs.apollo.dev/join/v0.2\", for: EXECUTION) + @link(url: \"https://specs.apollo.dev/join/v0.3\", for: EXECUTION) { query: Query } - directive @join__field(graph: join__Graph!, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION + directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION directive @join__graph(name: String!, url: String!) on ENUM_VALUE directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE - directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR + directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA @@ -3260,4 +3260,240 @@ describe('composition', () => { } `); }); + + describe('@interfaceObject', () => { + it('composes valid @interfaceObject usages correctly', () => { + const subgraphA = { + typeDefs: gql` + type Query { + iFromA: I + } + + interface I @key(fields: "id") { + id: ID! + x: Int + } + + type A implements I @key(fields: "id") { + id: ID! + x: Int + w: Int + } + + type B implements I @key(fields: "id") { + id: ID! + x: Int + z: Int + } + `, + name: 'subgraphA', + }; + + const subgraphB = { + typeDefs: gql` + type Query { + iFromB: I + } + + type I @interfaceObject @key(fields: "id") { + id: ID! + y: Int + } + `, + name: 'subgraphB', + }; + + const result = composeAsFed2Subgraphs([subgraphA, subgraphB]); + assertCompositionSuccess(result); + + const [_, api] = schemas(result); + expect(printSchema(api)).toMatchString(` + type A implements I { + id: ID! + x: Int + w: Int + y: Int + } + + type B implements I { + id: ID! + x: Int + z: Int + y: Int + } + + interface I { + id: ID! + x: Int + y: Int + } + + type Query { + iFromA: I + iFromB: I + } + `); + }); + + it('errors if @interfaceObject is used with no corresponding interface', () => { + const subgraphA = { + typeDefs: gql` + type Query { + iFromA: I + } + + type I @interfaceObject @key(fields: "id") { + id: ID! + x: Int + } + `, + name: 'subgraphA', + }; + + const subgraphB = { + typeDefs: gql` + type Query { + iFromB: I + } + + type I @interfaceObject @key(fields: "id") { + id: ID! + y: Int + } + `, + name: 'subgraphB', + }; + + const result = composeAsFed2Subgraphs([subgraphA, subgraphB]); + expect(result.errors).toBeDefined(); + expect(errors(result)).toStrictEqual([[ + 'INTERFACE_OBJECT_USAGE_ERROR', + 'Type "I" is declared with @interfaceObject in all the subgraphs in which is is defined (it is defined in subgraphs "subgraphA" and "subgraphB" but should be defined as an interface in at least one subgraph)' + ]]); + }); + + it('errors if @interfaceObject is missing in some subgraph', () => { + const subgraphA = { + typeDefs: gql` + type Query { + iFromA: I + } + + interface I @key(fields: "id") { + id: ID! + x: Int + } + + type A implements I @key(fields: "id") { + id: ID! + x: Int + } + `, + name: 'subgraphA', + }; + + const subgraphB = { + typeDefs: gql` + type Query { + iFromB: I + } + + type I @interfaceObject @key(fields: "id") { + id: ID! + y: Int + } + `, + name: 'subgraphB', + }; + + const subgraphC = { + typeDefs: gql` + type Query { + iFromC: I + } + + type I @key(fields: "id") { + id: ID! + z: Int + } + `, + name: 'subgraphC', + }; + + const result = composeAsFed2Subgraphs([subgraphA, subgraphB, subgraphC]); + expect(result.errors).toBeDefined(); + // Note: the error is a bit of a mouthful, but it should be clear enough and making it more compact requires + // a bit more special code on the error generation side and it's not clear it's worth the trouble (since again, + // the error should point to the problem well enough). + expect(errors(result)).toStrictEqual([[ + 'TYPE_KIND_MISMATCH', + 'Type "I" has mismatched kind: it is defined as Interface Type in subgraph "subgraphA" but Interface Object Type (Object Type with @interfaceObject) in subgraph "subgraphB" and Object Type in subgraph "subgraphC"', + ]]); + }); + + it('errors if an interface has a @key but the subgraph do not know all implementations', () => { + const subgraphA = { + typeDefs: gql` + type Query { + iFromA: I + } + + interface I @key(fields: "id") { + id: ID! + x: Int + } + + type A implements I @key(fields: "id") { + id: ID! + x: Int + w: Int + } + + type B implements I @key(fields: "id") { + id: ID! + x: Int + z: Int + } + `, + name: 'subgraphA', + }; + + const subgraphB = { + typeDefs: gql` + type Query { + iFromB: I + } + + type I @interfaceObject @key(fields: "id") { + id: ID! + y: Int + } + `, + name: 'subgraphB', + }; + + const subgraphC = { + typeDefs: gql` + interface I { + id: ID! + x: Int + } + + type C implements I @key(fields: "id") { + id: ID! + x: Int + w: Int + } + `, + name: 'subgraphC', + } + + const result = composeAsFed2Subgraphs([subgraphA, subgraphB, subgraphC]); + expect(result.errors).toBeDefined(); + expect(errors(result)).toStrictEqual([[ + 'INTERFACE_KEY_MISSING_IMPLEMENTATION_TYPE', + '[subgraphA] Interface type "I" has a resolvable key (@key(fields: "id")) in subgraph "subgraphA" but that subgraph is missing some of the supergraph implementation types of "I". Subgraph "subgraphA" should define type "C" (and have it implement "I").', + ]]); + }); + }); }); diff --git a/composition-js/src/__tests__/validation_errors.test.ts b/composition-js/src/__tests__/validation_errors.test.ts index 7f909f1d6..22308a1d9 100644 --- a/composition-js/src/__tests__/validation_errors.test.ts +++ b/composition-js/src/__tests__/validation_errors.test.ts @@ -115,8 +115,6 @@ describe('@requires', () => { describe('non-resolvable keys', () => { it('fails if key is declared non-resolvable but would be needed', () => { - global.console = require('console'); - const subgraphA = { name: 'A', typeDefs: gql` @@ -158,3 +156,137 @@ describe('non-resolvable keys', () => { ]); }); }); + +describe('@interfaceObject', () => { + it('fails on @interfaceObject usage with missing @key on interface', () => { + const subgraphA = { + typeDefs: gql` + interface I { + id: ID! + x: Int + } + + type A implements I @key(fields: "id") { + id: ID! + x: Int + } + + type B implements I @key(fields: "id") { + id: ID! + x: Int + } + `, + name: 'subgraphA', + }; + + const subgraphB = { + typeDefs: gql` + type Query { + iFromB: I + } + + type I @interfaceObject @key(fields: "id") { + id: ID! + y: Int + } + `, + name: 'subgraphB', + }; + + const result = composeAsFed2Subgraphs([subgraphA, subgraphB]); + expect(result.errors).toBeDefined(); + expect(errorMessages(result)).toMatchStringArray([ + ` + The following supergraph API query: + { + iFromB { + ... on A { + ... + } + } + } + cannot be satisfied by the subgraphs because: + - from subgraph "subgraphB": no subgraph can be reached to resolve the implementation type of @interfaceObject type "I". + `, + ` + The following supergraph API query: + { + iFromB { + ... on B { + ... + } + } + } + cannot be satisfied by the subgraphs because: + - from subgraph "subgraphB": no subgraph can be reached to resolve the implementation type of @interfaceObject type "I". + `, + ]); + }); + + it('fails on @interfaceObject with some unreachable implementation', () => { + const subgraphA = { + typeDefs: gql` + interface I @key(fields: "id") { + id: ID! + x: Int + } + + type A implements I @key(fields: "id") { + id: ID! + x: Int + } + + type B implements I @key(fields: "id") { + id: ID! + x: Int + } + `, + name: 'subgraphA', + }; + + const subgraphB = { + typeDefs: gql` + type Query { + iFromB: I + } + + type I @interfaceObject @key(fields: "id") { + id: ID! + y: Int + } + `, + name: 'subgraphB', + }; + + const subgraphC = { + typeDefs: gql` + type A { + z: Int + } + `, + name: 'subgraphC', + }; + + const result = composeAsFed2Subgraphs([subgraphA, subgraphB, subgraphC]); + expect(result.errors).toBeDefined(); + expect(errorMessages(result)).toMatchStringArray([ + ` + The following supergraph API query: + { + iFromB { + ... on A { + z + } + } + } + cannot be satisfied by the subgraphs because: + - from subgraph "subgraphB": + - cannot find implementation type "A" (supergraph interface "I" is declared with @interfaceObject in "subgraphB"). + - cannot move to subgraph "subgraphC", which has field "A.z", because interface "I" is not defined in this subgraph (to jump to "subgraphC", it would need to both define interface "I" and have a @key on it). + - from subgraph "subgraphA": + - cannot find field "A.z". + - cannot move to subgraph "subgraphC", which has field "A.z", because type "A" has no @key defined in subgraph "subgraphC". + ` + ]); + }); +}); diff --git a/composition-js/src/merging/merge.ts b/composition-js/src/merging/merge.ts index 1697cb576..98742dd4c 100644 --- a/composition-js/src/merging/merge.ts +++ b/composition-js/src/merging/merge.ts @@ -69,6 +69,7 @@ import { isCompositeType, isDefined, addSubgraphToError, + printHumanReadableList, } from "@apollo/federation-internals"; import { ASTNode, GraphQLError, DirectiveLocation } from "graphql"; import { @@ -256,10 +257,6 @@ function descriptionString(toIndent: string, indentation: string): string { return indentation + '"""\n' + indentation + toIndent.replace('\n', '\n' + indentation) + '\n' + indentation + '"""'; } -function typeKindToString(t: NamedType): string { - return t.kind.replace("Type", " Type"); -} - function locationString(locations: DirectiveLocation[]): string { if (locations.length === 0) { return ""; @@ -447,6 +444,15 @@ class Merger { this.mergeAllAppliedDirectives(); + + // When @interfaceObject is used in a subgraph, then that subgraph essentially provides fields both + // to the interface but also to all it's implementation. But so far, we only merged the type definition + // itself, so we now need to potentially add the field to the implementations if missing. + // Note that we do this after everything else have been merged because this method will essentially + // copy things from interface in the merged schema into their implementation in that same schema so + // we want to make sure everything is ready. + this.addMissingInterfaceObjectFieldsToImplementations(); + // If we already encountered errors, `this.merged` is probably incomplete. Let's not risk adding errors that // are only an artifact of that incompleteness as it's confusing. if (this.errors.length === 0) { @@ -492,22 +498,52 @@ class Merger { // and report errors otherwise. private addTypesShallow() { const mismatchedTypes = new Set(); - for (const subgraph of this.subgraphsSchema) { + const typesWithInterfaceObject = new Set(); + for (const subgraph of this.subgraphs) { + const metadata = subgraph.metadata(); + // We include the built-ins in general (even if we skip some federation specific ones): if a subgraph built-in // is not a supergraph built-in, we should add it as a normal type. - for (const type of subgraph.allTypes()) { + for (const type of subgraph.schema.allTypes()) { if (!isMergedType(type)) { continue; } + + let expectedKind = type.kind; + if (metadata.isInterfaceObjectType(type)) { + expectedKind = 'InterfaceType'; + typesWithInterfaceObject.add(type.name); + } const previous = this.merged.type(type.name); if (!previous) { - this.merged.addType(newNamedType(type.kind, type.name)); - } else if (previous.kind !== type.kind) { + this.merged.addType(newNamedType(expectedKind, type.name)); + } else if (previous.kind !== expectedKind) { mismatchedTypes.add(type.name); } } } mismatchedTypes.forEach(t => this.reportMismatchedTypeDefinitions(t)); + + // Most invalid use of @interfaceObject are reported as a mismatch above, but one exception is the + // case where a type is used only with @interfaceObject, but there is no corresponding interface + // definition in any subgraph. + for (const itfObjectType of typesWithInterfaceObject) { + if (mismatchedTypes.has(itfObjectType)) { + continue; + } + + if (!this.subgraphsSchema.some((s) => s.type(itfObjectType)?.kind === 'InterfaceType')) { + const subgraphsWithType = this.subgraphs.values().filter((s) => s.schema.type(itfObjectType) !== undefined); + // Note that there is meaningful way in which the supergraph could work in this situation, expect maybe if + // the type is unused, because validation composition would complain it cannot find the `__typename` in path + // leading to that type. But the error here is a bit more "direct"/user friendly than what post-merging + // validation would return, so we make this a hard error, not just a warning. + this.errors.push(ERRORS.INTERFACE_OBJECT_USAGE_ERROR.err( + `Type "${itfObjectType}" is declared with @interfaceObject in all the subgraphs in which is is defined (it is defined in ${printSubgraphNames(subgraphsWithType.map((s) => s.name))} but should be defined as an interface in at least one subgraph)`, + { nodes: sourceASTs(...subgraphsWithType.map((s) => s.schema.type(itfObjectType))) }, + )); + } + } } private addCoreFeatures() { @@ -549,6 +585,14 @@ class Merger { private reportMismatchedTypeDefinitions(mismatchedType: string) { const supergraphType = this.merged.type(mismatchedType)!; + const typeKindToString = (t: NamedType) => { + const metadata = federationMetadata(t.schema()); + if (metadata?.isInterfaceObjectType(t)) { + return 'Interface Object Type (Object Type with @interfaceObject)'; + } else { + return t.kind.replace("Type", " Type"); + } + }; this.mismatchReporter.reportMismatchError( ERRORS.TYPE_KIND_MISMATCH, `Type "${mismatchedType}" has mismatched kind: it is defined as `, @@ -559,12 +603,17 @@ class Merger { } private subgraphsTypes(supergraphType: T): (T | undefined)[] { - return this.subgraphsSchema.map((subgraph) => { - const type = subgraph.type(supergraphType.name); + return this.subgraphs.values().map((subgraph) => { + const type = subgraph.schema.type(supergraphType.name); + if (!type) { + return undefined; + } + // At this point, we have already reported errors for type mismatches (and so composition // will fail, we just try to gather more errors), so simply ignore versions of the type - // that don't have the proper kind. - if (!type || type.kind !== supergraphType.kind) { + // that don't have the "proper" kind. + const kind = subgraph.metadata().isInterfaceObjectType(type) ? 'InterfaceType' : type.kind; + if (kind !== supergraphType.kind) { return undefined; } return type as T; @@ -657,7 +706,8 @@ class Merger { this.mergeObject(sources as (ObjectType | undefined)[], dest); break; case 'InterfaceType': - this.mergeInterface(sources as (InterfaceType | undefined)[], dest); + // Note that due to @interfaceObject, we can have some ObjectType in the sources, not just interfaces. + this.mergeInterface(sources as (InterfaceType | ObjectType | undefined)[], dest); break; case 'UnionType': this.mergeUnion(sources as (UnionType | undefined)[], dest); @@ -711,15 +761,19 @@ class Merger { // There is either 1 join__type per-key, or if there is no key, just one for the type. const sourceMetadata = this.subgraphs.values()[idx].metadata(); + // Note that mechanically we don't need to substitue `undefined` for `false` below (`false` is the + // default value), but doing so 1) yield smaller supergraph (because the parameter isn't included) + // and 2) this avoid needless disrepancies compared to supergraphs generated before @interfaceObject was added. + const isInterfaceObject = sourceMetadata.isInterfaceObjectType(source) ? true : undefined; const keys = source.appliedDirectivesOf(sourceMetadata.keyDirective()); const name = this.joinSpecName(idx); if (!keys.length) { - dest.applyDirective(joinTypeDirective, { graph: name }); + dest.applyDirective(joinTypeDirective, { graph: name, isInterfaceObject }); } else { for (const key of keys) { const extension = key.ofExtension() || source.hasAppliedDirective(sourceMetadata.extendsDirective()) ? true : undefined; const { resolvable } = key.arguments(); - dest.applyDirective(joinTypeDirective, { graph: name, key: key.arguments().fields, extension, resolvable }); + dest.applyDirective(joinTypeDirective, { graph: name, key: key.arguments().fields, extension, resolvable, isInterfaceObject }); } } } @@ -813,6 +867,71 @@ class Merger { } } + private addMissingInterfaceObjectFieldsToImplementations() { + // For each merged object types, we check if we're missing a field from one of the implemented interface. + // If we do, then we look if one of the subgraph provides that field as a (non-external) interface object + // type, and if that's the case, we add the field to the object. + for (const type of this.merged.objectTypes()) { + for (const implementedItf of type.interfaces()) { + for (const itfField of implementedItf.fields()) { + if (type.field(itfField.name)) { + continue; + } + + // Note that we don't blindly add the field yet, that would be incorrect in many cases (and we + // have a specific validation that return a user-friendly error in such incorrect cases, see + // `postMergeValidations`). We must first check that there is some subgraph that implement + // that field as an "interface object", since in that case the field will genuinely be provided + // by that subgraph at runtime. + if (this.isFieldProvidedByAnInterfaceObject(itfField.name, implementedItf.name)) { + // Note it's possible that interface is abstracted away (as an interface object) in multiple + // subgraphs, so we don't bother with the field definition in those subgraphs, but rather + // just copy the merged definition from the interface. + const implemField = type.addField(itfField.name, itfField.type); + // Cases could probably be made for both either copying or not copying the description + // and applied directives from the interface field, but we copy both here as it feels + // more likely to be what user expects (assume they care either way). It's unlikely + // this will be an issue to anyone, but we can always make this behaviour tunable + // "somehow" later if the need arise. Feels highly overkill at this point though. + implemField.description = itfField.description; + this.copyNonJoinAppliedDirectives(itfField, implemField); + for (const itfArg of itfField.arguments()) { + const implemArg = implemField.addArgument(itfArg.name, itfArg.type, itfArg.defaultValue); + implemArg.description = itfArg.description; + this.copyNonJoinAppliedDirectives(itfArg, implemArg); + } + + // We add a special @join__field for those added field with no `graph` target. This + // clarify to the later extraction process that this particular field doesn't come + // from any particular subgraph (it comes indirectly from an @interfaceObject type, + // but it's very much indirect so ...). + implemField.applyDirective(joinSpec.fieldDirective(this.merged), { graph: undefined }); + } + } + } + } + } + + private copyNonJoinAppliedDirectives(source: SchemaElement, dest: SchemaElement) { + // This method is used to copy "user provided" applied directives from interface fields to some + // implementation type when @interfaceObject is used. But we shouldn't copy the `join` spec directive + // as those are for the interface field but are invalid for the implementation field. + source.appliedDirectives.forEach((d) => { + if (!joinSpec.isSpecDirective(d.definition!)) { + dest.applyDirective(d.name, {...d.arguments()}) + } + }); + } + + private isFieldProvidedByAnInterfaceObject(fieldName: string, interfaceName: string): boolean { + return this.subgraphs.values().some((s) => { + const meta = s.metadata(); + const type = s.schema.type(interfaceName); + const field = type && meta.isInterfaceObjectType(type) ? type.field(fieldName) : undefined; + return field && !meta.isFieldExternal(field); + }); + } + private addFieldsShallow(sources: (T | undefined)[], dest: T) { for (const source of sources) { if (!source) { @@ -1506,7 +1625,9 @@ class Merger { } } - private mergeInterface(sources: (InterfaceType | undefined)[], dest: InterfaceType) { + private mergeInterface(sources: (InterfaceType | ObjectType | undefined)[], dest: InterfaceType) { + this.validateInterfaceKeys(sources, dest); + this.addFieldsShallow(sources, dest); for (const destField of dest.fields()) { this.hintOnInconsistentValueTypeField(sources, dest, destField); @@ -1515,6 +1636,47 @@ class Merger { } } + private validateInterfaceKeys(sources: (InterfaceType | ObjectType | undefined)[], dest: InterfaceType) { + // Remark: it might be ok to filter @inaccessible types in `supergraphImplementations`, but this require + // some more thinking (and I'm not even sure it makes a practical difference given the rules for validaty + // of @inacessible) and it will be backward compatible to filter them later, while the reverse wouldn't + // technically be, so we stay on the safe side. + const supergraphImplementations = dest.possibleRuntimeTypes(); + + // Validate that if a source defines a (resolvable) @key on an interface, then that subgraph defines + // all the implementations of that interface in the supergraph. + for (const [idx, source] of sources.entries()) { + if (!source || !isInterfaceType(source)) { + continue; + } + const sourceMetadata = this.subgraphs.values()[idx].metadata(); + const resolvableKey = source.appliedDirectivesOf(sourceMetadata.keyDirective()).find((k) => k.arguments().resolvable !== false); + if (!resolvableKey) { + continue; + } + + const implementationsInSubgraph = source.possibleRuntimeTypes(); + if (implementationsInSubgraph.length < supergraphImplementations.length) { + const missingImplementations = supergraphImplementations.filter((superImpl) => !implementationsInSubgraph.some((subgImpl) => superImpl.name === subgImpl.name)); + const typesString = printHumanReadableList( + missingImplementations.map((i) => `"${i.coordinate}"`), + { + prefix: 'type', + prefixPlural: 'types', + } + ); + this.errors.push(addSubgraphToError( + ERRORS.INTERFACE_KEY_MISSING_IMPLEMENTATION_TYPE.err( + `Interface type "${source.coordinate}" has a resolvable key (${resolvableKey}) in subgraph "${this.names[idx]}" but that subgraph is missing some of the supergraph implementation types of "${dest.coordinate}". ` + + `Subgraph "${this.names[idx]}" should define ${typesString} (and have ${missingImplementations.length > 1 ? 'them' : 'it'} implement "${source.coordinate}").`, + { nodes: resolvableKey.sourceAST}, + ), + this.names[idx], + )); + } + } + } + private mergeUnion(sources: (UnionType | undefined)[], dest: UnionType) { for (const source of sources) { if (!source) { diff --git a/composition-js/src/validate.ts b/composition-js/src/validate.ts index 92fe2c322..c8f6ace55 100644 --- a/composition-js/src/validate.ts +++ b/composition-js/src/validate.ts @@ -98,8 +98,12 @@ function displayReasons(reasons: Unadvanceables[]): string { if (reasons.length === 1) { msg += ' ' + reasons[0].details + '.'; } else { - for (const reason of reasons) { - msg += '\n - ' + reason.details + '.'; + // We put all the reasons into a set because it's possible multiple paths of the algorithm + // had the same "dead end". Typically, without this, there is cases where we end up with + // multiple "cannot find field x" messages (for the same "x"). + const allDetails = new Set(reasons.map((r) => r.details)); + for (const details of allDetails) { + msg += '\n - ' + details + '.'; } } return msg; @@ -155,7 +159,9 @@ function buildWitnessNextStep(edges: Edge[], index: number): SelectionSet | unde case 'SubgraphEnteringTransition': case 'KeyResolution': case 'RootTypeResolution': - return subSelection; + case 'InterfaceObjectFakeDownCast': + // Witnesses are build from a path on the supergraph, so we shouldn't have any of those edges. + assert(false, `Invalid edge ${edge} found in supergraph path`); } // If we get here, the edge is either a downcast or a field, so the edge head must be selectable. const selectionSet = new SelectionSet(edge.head.type as CompositeType); diff --git a/docs/source/errors.md b/docs/source/errors.md index 3d248fd10..da357b00d 100644 --- a/docs/source/errors.md +++ b/docs/source/errors.md @@ -44,6 +44,9 @@ The following errors might be raised during composition: | `INPUT_FIELD_DEFAULT_MISMATCH` | An input field has a default value that is incompatible with other declarations of that field in other subgraphs. | 2.0.0 | | | `INTERFACE_FIELD_IMPLEM_TYPE_MISMATCH` | For an interface field, some of its concrete implementations have @external or @requires and there is difference in those implementations return type (which is currently not supported; see https://github.com/apollographql/federation/issues/1257) | 2.0.0 | | | `INTERFACE_FIELD_NO_IMPLEM` | After subgraph merging, an implementation is missing a field of one of the interface it implements (which can happen for valid subgraphs). | 2.0.0 | | +| `INTERFACE_KEY_MISSING_IMPLEMENTATION_TYPE` | A subgraph has a `@key` on an interface type, but that subgraph does not define all the implementation (in the supergraph) of that interface | 2.3.0 | | +| `INTERFACE_KEY_NOT_ON_IMPLEMENTATION` | A `@key` is defined on an interface type, but is not defined (or is not resolvable) on at least one of the interface implementations | 2.3.0 | | +| `INTERFACE_OBJECT_USAGE_ERROR` | Error in the usage of the @interfaceObject directive. | 2.3.0 | | | `INVALID_FEDERATION_SUPERGRAPH` | Indicates that a schema provided for an Apollo Federation supergraph is not a valid supergraph schema. | 2.1.0 | | | `INVALID_FIELD_SHARING` | A field that is non-shareable in at least one subgraph is resolved by multiple subgraphs. | 2.0.0 | | | `INVALID_GRAPHQL` | A schema is invalid GraphQL: it violates one of the rule of the specification. | 2.0.0 | | @@ -56,7 +59,7 @@ The following errors might be raised during composition: | `KEY_FIELDS_SELECT_INVALID_TYPE` | The `fields` argument of `@key` directive includes a field whose type is a list, interface, or union type. Fields of these types cannot be part of a `@key` | 0.x | | | `KEY_INVALID_FIELDS_TYPE` | The value passed to the `fields` argument of a `@key` directive is not a string. | 2.0.0 | | | `KEY_INVALID_FIELDS` | The `fields` argument of a `@key` directive is invalid (it has invalid syntax, includes unknown fields, ...). | 2.0.0 | | -| `KEY_UNSUPPORTED_ON_INTERFACE` | A `@key` directive is used on an interface, which is not (yet) supported. | 2.0.0 | | +| `KEY_UNSUPPORTED_ON_INTERFACE` | A `@key` directive is used on an interface, which is only supported when @linking to federation 2.3+. | 2.0.0 | | | `LINK_IMPORT_NAME_MISMATCH` | The import name for a merged directive (as declared by the relevant `@link(import:)` argument) is inconsistent between subgraphs. | 2.0.0 | | | `MERGED_DIRECTIVE_APPLICATION_ON_EXTERNAL` | In a subgraph, a field is both marked @external and has a merged directive applied to it | 2.0.0 | | | `NO_QUERIES` | None of the composed subgraphs expose any query. | 2.0.0 | | diff --git a/gateway-js/src/__tests__/executeQueryPlan.test.ts b/gateway-js/src/__tests__/executeQueryPlan.test.ts index 36e479fc0..645d049f2 100644 --- a/gateway-js/src/__tests__/executeQueryPlan.test.ts +++ b/gateway-js/src/__tests__/executeQueryPlan.test.ts @@ -3516,4 +3516,423 @@ describe('executeQueryPlan', () => { `); }); }); + + describe('@interfaceObject', () => { + const defineSchema = ({ + s1, + }: { + s1?: { + iResolversExtra?: any, + hasIResolveReference?: boolean, + iResolveReferenceExtra?: (id: string) => { [k: string]: any }, + aResolversExtra?: any, + bResolversExtra?: any, + } + }) => { + + // The example uses 2 entities: + // - one of type A with id='idA' (x=1, y=2, z=3) + // - one of type B with id='idB' (x=10, y=20, w=30) + + const s1IBaseResolvers = (s1?.hasIResolveReference ?? true) + ? { + __resolveReference(ref: { id: string }) { + const extraFct = s1?.iResolveReferenceExtra; + const extraData = extraFct ? extraFct(ref.id) : {}; + return ref.id === 'idA' + ? { id: ref.id, x: 1, z: 3, ...extraData } + : { id: ref.id, x: 10, w: 30, ...extraData }; + } + } + : {}; + + const subgraph1 = { + name: 'S1', + typeDefs: gql` + extend schema + @link(url: "https://specs.apollo.dev/federation/v2.3", import: ["@key"]) + + type Query { + iFromS1: I + } + + interface I @key(fields: "id") { + id: ID! + x: Int + } + + type A implements I @key(fields: "id") { + id: ID! + x: Int + z: Int + } + + type B implements I @key(fields: "id") { + id: ID! + x: Int + w: Int + } + `, + resolvers: { + Query: { + iFromS1() { + return { __typename: 'A', id: 'idA' }; + } + }, + I: { + ...s1IBaseResolvers, + ...(s1?.iResolversExtra ?? {}), + }, + A: { + ...(s1?.aResolversExtra ?? {}), + }, + B: { + ...(s1?.bResolversExtra ?? {}), + }, + } + } + + const subgraph2 = { + name: 'S2', + typeDefs: gql` + extend schema + @link(url: "https://specs.apollo.dev/federation/v2.3", import: ["@key", "@interfaceObject"]) + + type Query { + iFromS2: I + } + + type I @interfaceObject @key(fields: "id") { + id: ID! + y: Int + } + `, + resolvers: { + Query: { + iFromS2() { + return { + __typename: 'I', + id: 'idB', + y: 20, + }; + } + }, + I: { + __resolveReference(ref: { id: string }) { + return { + id: ref.id, + y: ref.id === 'idA' ? 2 : 20, + } + }, + }, + } + } + + const { serviceMap, schema, queryPlanner } = getFederatedTestingSchema([ subgraph1, subgraph2 ]); + return async (op: string): Promise<{ plan: QueryPlan, response: GatewayExecutionResult }> => { + const operation = parseOp(op, schema); + const plan = buildPlan(operation, queryPlanner); + const response = await executePlan(plan, operation, undefined, schema, serviceMap); + return { plan, response }; + }; + } + + + test('handles __typename rewriting when using @key to @interfaceObject', async () => { + // We don't need extra resolving from S1 in this case. + const tester = defineSchema({}); + + let { plan, response } = await tester(` + query { + iFromS1 { + __typename + y + } + } + `); + + expect(plan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "S1") { + { + iFromS1 { + __typename + id + } + } + }, + Flatten(path: "iFromS1") { + Fetch(service: "S2") { + { + ... on I { + __typename + id + } + } => + { + ... on I { + y + } + } + }, + }, + }, + } + `); + + expect(response.errors).toBeUndefined(); + expect(response.data).toMatchInlineSnapshot(` + Object { + "iFromS1": Object { + "__typename": "A", + "y": 2, + }, + } + `); + + // Same, but with an explicit cast to A + ({ plan, response } = await tester(` + query { + iFromS1 { + ... on A { + y + } + } + } + `)); + + expect(plan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "S1") { + { + iFromS1 { + __typename + ... on A { + __typename + id + } + } + } + }, + Flatten(path: "iFromS1") { + Fetch(service: "S2") { + { + ... on A { + __typename + id + } + } => + { + ... on I { + y + } + } + }, + }, + }, + } + `); + + expect(response.errors).toBeUndefined(); + expect(response.data).toMatchInlineSnapshot(` + Object { + "iFromS1": Object { + "y": 2, + }, + } + `); + + // And lastly, make sure that we explicitly cast to B, we get nothing + ({ plan, response } = await tester(` + query { + iFromS1 { + ... on B { + y + } + } + } + `)); + + expect(plan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "S1") { + { + iFromS1 { + __typename + ... on B { + __typename + id + } + } + } + }, + Flatten(path: "iFromS1") { + Fetch(service: "S2") { + { + ... on B { + __typename + id + } + } => + { + ... on I { + y + } + } + }, + }, + }, + } + `); + + expect(response.errors).toBeUndefined(); + expect(response.data).toMatchInlineSnapshot(` + Object { + "iFromS1": Object {}, + } + `); + }); + + test.each([{ + name: 'with manual __typename', + s1: { + iResolveReferenceExtra: (id: string) => ({ __typename: id === 'idA' ? 'A' : 'B' }), + }, + }, { + name: 'with __resolveType', + s1: { + iResolversExtra: { + __resolveType(ref: { id: string }) { + return ref.id === 'idA' ? 'A' : 'B'; + } + }, + }, + }, { + name: 'with isTypeOf', + s1: { + aResolversExtra: { + __isTypeOf(ref: { id: string }) { + return ref.id === 'idA'; + } + }, + bResolversExtra: { + __isTypeOf(ref: { id: string }) { + // Same remark as above. + return ref.id === 'idB'; + } + }, + }, + }, { + name: 'with only a __resolveType on the interface but per-runtime-types __resolveReference', + s1: { + hasIResolveReference: false, + iResolversExtra: { + __resolveType(ref: { id: string }) { + return ref.id === 'idA' ? 'A' : 'B'; + } + }, + aResolversExtra: { + __resolveReference(ref: { id: string }) { + return ref.id === 'idA' + ? { id: ref.id, x: 1, z: 3 } + : undefined; + } + }, + bResolversExtra: { + __resolveReference(ref: { id: string }) { + return ref.id === 'idB' + ? { id: ref.id, x: 10, w: 30 } + : undefined; + } + }, + }, + }, { + name: 'errors when nothing provides the runtime type', + expectedErrors: [ + 'Abstract type "I" `__resolveReference` method must resolve to an Object type at runtime. ' + + 'Either the object returned by "I.__resolveReference" must include a valid `__typename` field, ' + + 'or the "I" type should provide a "resolveType" function or each possible type should provide an "isTypeOf" function.' + ], + }])('resolving an interface @key $name', async ({s1, expectedErrors}) => { + const tester = defineSchema({ s1 }); + + const { plan, response } = await tester(` + query { + iFromS2 { + __typename + x + y + ... on A { + z + } + ... on B { + w + } + } + } + `); + + expect(plan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "S2") { + { + iFromS2 { + __typename + id + y + } + } + }, + Flatten(path: "iFromS2") { + Fetch(service: "S1") { + { + ... on I { + __typename + id + } + } => + { + ... on I { + __typename + x + ... on A { + z + } + ... on B { + w + } + } + } + }, + }, + }, + } + `); + + if (expectedErrors) { + expect(response.errors?.map((e) => e.message)).toEqual(expectedErrors); + expect(response.data).toMatchInlineSnapshot(` + Object { + "iFromS2": null, + } + `); + } else { + expect(response.errors).toBeUndefined(); + expect(response.data).toMatchInlineSnapshot(` + Object { + "iFromS2": Object { + "__typename": "B", + "w": 30, + "x": 10, + "y": 20, + }, + } + `); + } + }); + }); }); diff --git a/gateway-js/src/__tests__/gateway/lifecycle-hooks.test.ts b/gateway-js/src/__tests__/gateway/lifecycle-hooks.test.ts index 9a5e61017..7d81a4870 100644 --- a/gateway-js/src/__tests__/gateway/lifecycle-hooks.test.ts +++ b/gateway-js/src/__tests__/gateway/lifecycle-hooks.test.ts @@ -147,7 +147,7 @@ describe('lifecycle hooks', () => { // the supergraph (even just formatting differences), this ID will change // and this test will have to updated. expect(secondCall[0]!.compositionId).toEqual( - '55f11e687010f75c791b917d9a46745b967c5b19d20443463ed075e500bff6c8', + '97f11725be210d703ced94cd941ef00d72ab26a9e04bdb724207b9e89e87359e', ); // second call should have previous info in the second arg expect(secondCall[1]!.compositionId).toEqual(expectedFirstId); diff --git a/gateway-js/src/executeQueryPlan.ts b/gateway-js/src/executeQueryPlan.ts index 8c893ce7f..271d4c00b 100644 --- a/gateway-js/src/executeQueryPlan.ts +++ b/gateway-js/src/executeQueryPlan.ts @@ -24,12 +24,13 @@ import { QueryPlanSelectionNode, QueryPlanFieldNode, getResponseName, + FetchDataRewrite, } from '@apollo/query-planner'; import { deepMerge } from './utilities/deepMerge'; import { isNotNullOrUndefined } from './utilities/array'; import { SpanStatusCode } from "@opentelemetry/api"; import { OpenTelemetrySpanNames, tracer } from "./utilities/opentelemetry"; -import { assert, defaultRootName, errorCodeDef, ERRORS } from '@apollo/federation-internals'; +import { assert, defaultRootName, errorCodeDef, ERRORS, isDefined } from '@apollo/federation-internals'; import { GatewayGraphQLRequestContext, GatewayExecutionResult } from '@apollo/server-gateway-interface'; export type ServiceMap = { @@ -323,6 +324,7 @@ async function executeFetch( context.supergraphSchema, entity, requires, + fetch.inputRewrites, ); if (representation && representation[TypeNameMetaFieldDef.name]) { representations.push(representation); @@ -484,6 +486,34 @@ async function executeFetch( } } +function updateRewrites(rewrites: FetchDataRewrite[] | undefined, pathElement: string): { + updated: FetchDataRewrite[], + completeRewrite?: any, +} | undefined { + if (!rewrites) { + return undefined; + } + + let completeRewrite: any = undefined; + const updated = rewrites + .map((r) => { + let u: FetchDataRewrite | undefined = undefined; + if (r.path[0] === pathElement) { + const updatedPath = r.path.slice(1); + if (updatedPath.length === 0) { + completeRewrite = r.setValueTo; + } else { + u = { ...r, path: updatedPath }; + } + } + return u; + }) + .filter(isDefined); + return updated.length === 0 && completeRewrite === undefined + ? undefined + : { updated, completeRewrite }; +} + /** * * @param source Result of GraphQL execution. @@ -493,6 +523,7 @@ function executeSelectionSet( schema: GraphQLSchema, source: Record | null, selections: QueryPlanSelectionNode[], + activeRewrites?: FetchDataRewrite[], ): Record | null { // If the underlying service has returned null for the parent (source) @@ -522,10 +553,17 @@ function executeSelectionSet( // here. return null; } + + const updatedRewrites = updateRewrites(activeRewrites, responseName); + if (updatedRewrites?.completeRewrite !== undefined) { + result[responseName] = updatedRewrites.completeRewrite; + continue; + } + if (Array.isArray(source[responseName])) { result[responseName] = source[responseName].map((value: any) => selections - ? executeSelectionSet(schema, value, selections) + ? executeSelectionSet(schema, value, selections, updatedRewrites?.updated) : value, ); } else if (selections) { @@ -533,6 +571,7 @@ function executeSelectionSet( schema, source[responseName], selections, + updatedRewrites?.updated, ); } else { result[responseName] = source[responseName]; @@ -545,9 +584,10 @@ function executeSelectionSet( if (!typename) continue; if (doesTypeConditionMatch(schema, selection.typeCondition, typename)) { + const updatedRewrites = activeRewrites ? updateRewrites(activeRewrites, `... on ${selection.typeCondition}`) : undefined; deepMerge( result, - executeSelectionSet(schema, source, selection.selections), + executeSelectionSet(schema, source, selection.selections, updatedRewrites?.updated), ); } break; diff --git a/internals-js/src/__tests__/schemaUpgrader.test.ts b/internals-js/src/__tests__/schemaUpgrader.test.ts index f32dd8401..0c4e82ee8 100644 --- a/internals-js/src/__tests__/schemaUpgrader.test.ts +++ b/internals-js/src/__tests__/schemaUpgrader.test.ts @@ -202,3 +202,46 @@ test('remove tag on external field if found on definition', () => { expect(typeAInS1.field("y")?.appliedDirectivesOf('tag').map((d) => d.toString())).toStrictEqual([]); expect(typeAInS2.field("y")?.appliedDirectivesOf('tag').map((d) => d.toString())).toStrictEqual([ '@tag(name: "a tag")' ]); }) + +test('reject @interfaceObject usage if not all subgraphs are fed2', () => { + // Note that this test both validates the rejection of fed1 subgraph when @interfaceObject is used somewhere, but also + // illustrate why we do so: fed1 schema can use @key on interface for backward compatibility, but it is ignored and + // the schema upgrader removes them. Given that actual support for @key on interfaces is necesarry to make @interfaceObject + // work, it would be really confusing to not reject the example below right away, since it "looks" like it the @key on + // the interface in the 2nd subgraph should work, but it actually won't. + + const s1 = ` + extend schema + @link(url: "https://specs.apollo.dev/federation/v2.3", import: [ "@key", "@interfaceObject"]) + + type Query { + a: A + } + + type A @key(fields: "id") @interfaceObject { + id: String + x: Int + } + `; + + const s2 = ` + interface A @key(fields: "id") { + id: String + y: Int + } + + type X implements A @key(fields: "id") { + id: String + y: Int + } + `; + + const subgraphs = new Subgraphs(); + subgraphs.add(buildSubgraph('s1', 'http://s1', s1)); + subgraphs.add(buildSubgraph('s2', 'http://s2', s2)); + const res = upgradeSubgraphsIfNecessary(subgraphs); + expect(res.errors?.map((e) => e.message)).toStrictEqual([ + 'The @interfaceObject directive can only be used if all subgraphs have federation 2 subgraph schema (schema with a `@link` to "https://specs.apollo.dev/federation" version 2.0 or newer): ' + + '@interfaceObject is used in subgraph "s1" but subgraph "s2" is not a federation 2 subgraph schema.' + ]); +}) diff --git a/internals-js/src/__tests__/subgraphValidation.test.ts b/internals-js/src/__tests__/subgraphValidation.test.ts index c0dd1733d..0605e2487 100644 --- a/internals-js/src/__tests__/subgraphValidation.test.ts +++ b/internals-js/src/__tests__/subgraphValidation.test.ts @@ -1,33 +1,9 @@ import { DocumentNode } from 'graphql'; import gql from 'graphql-tag'; -import { Subgraph, errorCauses } from '..'; -import { asFed2SubgraphDocument, buildSubgraph } from "../federation" +import { Subgraph } from '..'; +import { buildSubgraph } from "../federation" import { defaultPrintOptions, printSchema } from '../print'; -import './matchers'; - -// Builds the provided subgraph (using name 'S' for the subgraph) and, if the -// subgraph is invalid/has errors, return those errors as a list of [code, message]. -// If the subgraph is valid, return undefined. -export function buildForErrors( - subgraphDefs: DocumentNode, - options?: { - subgraphName?: string, - asFed2?: boolean, - } -): [string, string][] | undefined { - try { - const doc = (options?.asFed2 ?? true) ? asFed2SubgraphDocument(subgraphDefs) : subgraphDefs; - const name = options?.subgraphName ?? 'S'; - buildSubgraph(name, `http://${name}`, doc).validate(); - return undefined; - } catch (e) { - const causes = errorCauses(e); - if (!causes) { - throw e; - } - return causes.map((err) => [err.extensions.code as string, err.message]); - } -} +import { buildForErrors } from './testUtils'; describe('fieldset-based directives', () => { it('rejects field defined with arguments in @key', () => { @@ -91,8 +67,11 @@ describe('fieldset-based directives', () => { ]); }); - it('rejects @key on interfaces', () => { + it.each(['2.0', '2.1', '2.2'])('rejects @key on interfaces _in the %p spec_', (version) => { const subgraph = gql` + extend schema + @link(url: "https://specs.apollo.dev/federation/v${version}", import: ["@key"]) + type Query { t: T } @@ -101,7 +80,7 @@ describe('fieldset-based directives', () => { f: Int } ` - expect(buildForErrors(subgraph)).toStrictEqual([ + expect(buildForErrors(subgraph, { asFed2: false })).toStrictEqual([ ['KEY_UNSUPPORTED_ON_INTERFACE', '[S] Cannot use @key on interface "T": @key is not yet supported on interfaces'], ]); }); @@ -1183,3 +1162,121 @@ describe('@shareable', () => { ]]); }); }); + +describe('@interfaceObject/@key on interfaces validation', () => { + it('@key on interfaces require @key on all implementations', () => { + const doc = gql` + interface I @key(fields: "id1") @key(fields: "id2") { + id1: ID! + id2: ID! + } + + type A implements I @key(fields: "id2") { + id1: ID! + id2: ID! + a: Int + } + + type B implements I @key(fields: "id1") @key(fields: "id2") { + id1: ID! + id2: ID! + b: Int + } + + type C implements I @key(fields: "id2") { + id1: ID! + id2: ID! + c: Int + } + `; + + expect(buildForErrors(doc)).toStrictEqual([[ + 'INTERFACE_KEY_NOT_ON_IMPLEMENTATION', + '[S] Key @key(fields: "id1") on interface type "I" is missing on implementation types "A" and "C".', + ]]); + }); + + it('@key on interfaces with @key on some implementation non resolvable', () => { + const doc = gql` + interface I @key(fields: "id1") { + id1: ID! + } + + type A implements I @key(fields: "id1") { + id1: ID! + a: Int + } + + type B implements I @key(fields: "id1") { + id1: ID! + b: Int + } + + type C implements I @key(fields: "id1", resolvable: false) { + id1: ID! + c: Int + } + `; + + expect(buildForErrors(doc)).toStrictEqual([[ + 'INTERFACE_KEY_NOT_ON_IMPLEMENTATION', + '[S] Key @key(fields: "id1") on interface type "I" should be resolvable on all implementation types, but is declared with argument "@key(resolvable:)" set to false in type "C".', + ]]); + }); + + it('ensures order of fields in key does not matter', () => { + const doc = gql` + interface I @key(fields: "a b c") { + a: Int + b: Int + c: Int + } + + type A implements I @key(fields: "c b a") { + a: Int + b: Int + c: Int + } + + type B implements I @key(fields: "a c b") { + a: Int + b: Int + c: Int + } + + type C implements I @key(fields: "a b c") { + a: Int + b: Int + c: Int + } + `; + + expect(buildForErrors(doc)).toBeUndefined(); + }); + + // There is no meaningful way to make @interfaceObject work on a value type at the moment, because + // if you have an @interfaceObject, some other subgraph needs to be able to resolve the concrete + // type, and that imply that you have key to go to that other subgraph. + // To be clear, the @key on the @interfaceObject technically con't need to be "resolvable", and the + // difference between no key and a non-resolvable key is arguably more convention than a genuine + // mechanical difference at the moment, but still a good idea to rely on that convention to help + // catching obvious mistakes early. + it('only allow @interfaceObject on entity types', () => { + const doc = gql` + # This one shouldn't raise an error + type A @key(fields: "id", resolvable: false) @interfaceObject { + id: ID! + } + + # This one should + type B @interfaceObject { + x: Int + } + `; + + expect(buildForErrors(doc)).toStrictEqual([[ + 'INTERFACE_OBJECT_USAGE_ERROR', + '[S] The @interfaceObject directive can only be applied to entity types but type "B" has no @key in this subgraph.' + ]]); + }); +}); diff --git a/internals-js/src/__tests__/testUtils.ts b/internals-js/src/__tests__/testUtils.ts new file mode 100644 index 000000000..b5a0ab2b7 --- /dev/null +++ b/internals-js/src/__tests__/testUtils.ts @@ -0,0 +1,28 @@ +import { DocumentNode } from "graphql"; +import { asFed2SubgraphDocument, buildSubgraph, errorCauses } from ".."; +import './matchers'; + +// Builds the provided subgraph (using name 'S' for the subgraph) and, if the +// subgraph is invalid/has errors, return those errors as a list of [code, message]. +// If the subgraph is valid, return undefined. +export function buildForErrors( + subgraphDefs: DocumentNode, + options?: { + subgraphName?: string, + asFed2?: boolean, + } +): [string, string][] | undefined { + try { + const doc = (options?.asFed2 ?? true) ? asFed2SubgraphDocument(subgraphDefs) : subgraphDefs; + const name = options?.subgraphName ?? 'S'; + buildSubgraph(name, `http://${name}`, doc).validate(); + return undefined; + } catch (e) { + const causes = errorCauses(e); + if (!causes) { + throw e; + } + return causes.map((err) => [err.extensions.code as string, err.message]); + } +} + diff --git a/internals-js/src/__tests__/values.test.ts b/internals-js/src/__tests__/values.test.ts index 948f8a071..021fcddb9 100644 --- a/internals-js/src/__tests__/values.test.ts +++ b/internals-js/src/__tests__/values.test.ts @@ -3,10 +3,10 @@ import { } from '../definitions'; import { buildSchema } from '../buildSchema'; import { parseOperation } from '../operations'; -import { buildForErrors } from './subgraphValidation.test'; import gql from 'graphql-tag'; import { printSchema } from '../print'; import { valueEquals } from '../values'; +import { buildForErrors } from './testUtils'; function parseSchema(schema: string): Schema { try { diff --git a/internals-js/src/definitions.ts b/internals-js/src/definitions.ts index 78df320f0..f0e08b01d 100644 --- a/internals-js/src/definitions.ts +++ b/internals-js/src/definitions.ts @@ -241,6 +241,14 @@ export function runtimeTypesIntersects(t1: CompositeType, t2: CompositeType): bo return false; } +export function supertypes(type: CompositeType): readonly CompositeType[] { + switch (type.kind) { + case 'InterfaceType': return type.interfaces(); + case 'UnionType': return []; + case 'ObjectType': return (type.interfaces() as CompositeType[]).concat(type.unionsWhereMember()); + } +} + export function isConditionalDirective(directive: Directive | DirectiveDefinition): boolean { return ['include', 'skip'].includes(directive.name); } @@ -2098,6 +2106,10 @@ export class ObjectType extends FieldBasedType break; } } + + unionsWhereMember(): readonly UnionType[] { + return this._referencers?.filter((r): r is UnionType => r instanceof BaseNamedType && isUnionType(r)) ?? []; + } } export class InterfaceType extends FieldBasedType { diff --git a/internals-js/src/error.ts b/internals-js/src/error.ts index 719fc79a0..84578aac2 100644 --- a/internals-js/src/error.ts +++ b/internals-js/src/error.ts @@ -235,7 +235,7 @@ const REQUIRES_MISSING_EXTERNAL = DIRECTIVE_FIELDS_MISSING_EXTERNAL.createCode(' const DIRECTIVE_UNSUPPORTED_ON_INTERFACE = makeFederationDirectiveErrorCodeCategory( 'UNSUPPORTED_ON_INTERFACE', - (directive) => `A \`@${directive}\` directive is used on an interface, which is not (yet) supported.`, + (directive) => `A \`@${directive}\` directive is used on an interface, which is ${directive === 'key' ? 'only supported when @linking to federation 2.3+' : 'not (yet) supported'}.`, ); const KEY_UNSUPPORTED_ON_INTERFACE = DIRECTIVE_UNSUPPORTED_ON_INTERFACE.createCode('key'); @@ -531,6 +531,25 @@ const DIRECTIVE_COMPOSITION_ERROR = makeCodeDefinition( { addedIn: '2.1.0' }, ); +const INTERFACE_OBJECT_USAGE_ERROR = makeCodeDefinition( + 'INTERFACE_OBJECT_USAGE_ERROR', + 'Error in the usage of the @interfaceObject directive.', + { addedIn: '2.3.0' }, +); + +const INTERFACE_KEY_NOT_ON_IMPLEMENTATION = makeCodeDefinition( + 'INTERFACE_KEY_NOT_ON_IMPLEMENTATION', + 'A `@key` is defined on an interface type, but is not defined (or is not resolvable) on at least one of the interface implementations', + { addedIn: '2.3.0' }, +); + +const INTERFACE_KEY_MISSING_IMPLEMENTATION_TYPE = makeCodeDefinition( + 'INTERFACE_KEY_MISSING_IMPLEMENTATION_TYPE', + 'A subgraph has a `@key` on an interface type, but that subgraph does not define all the implementation (in the supergraph) of that interface', + { addedIn: '2.3.0' }, +) + + export const ERROR_CATEGORIES = { DIRECTIVE_FIELDS_MISSING_EXTERNAL, DIRECTIVE_UNSUPPORTED_ON_INTERFACE, @@ -614,6 +633,9 @@ export const ERRORS = { PROVIDES_HAS_DIRECTIVE_IN_FIELDS_ARGS, REQUIRES_HAS_DIRECTIVE_IN_FIELDS_ARGS, DIRECTIVE_COMPOSITION_ERROR, + INTERFACE_OBJECT_USAGE_ERROR, + INTERFACE_KEY_NOT_ON_IMPLEMENTATION, + INTERFACE_KEY_MISSING_IMPLEMENTATION_TYPE, }; const codeDefByCode = Object.values(ERRORS).reduce((obj: {[code: string]: ErrorCodeDefinition}, codeDef: ErrorCodeDefinition) => { obj[codeDef.code] = codeDef; return obj; }, {}); diff --git a/internals-js/src/extractSubgraphsFromSupergraph.ts b/internals-js/src/extractSubgraphsFromSupergraph.ts index 290d5d0fd..915419720 100644 --- a/internals-js/src/extractSubgraphsFromSupergraph.ts +++ b/internals-js/src/extractSubgraphsFromSupergraph.ts @@ -202,7 +202,10 @@ export function extractSubgraphsFromSupergraph(supergraph: Schema): Subgraphs { const ownerDirective = joinSpec.ownerDirective(supergraph); const fieldDirective = joinSpec.fieldDirective(supergraph); - const getSubgraph = (application: Directive) => graphEnumNameToSubgraphName.get(application.arguments().graph); + const getSubgraph = (application: Directive) => { + const graph = application.arguments().graph; + return graph ? graphEnumNameToSubgraphName.get(graph) : undefined; + }; /* * Fed2 supergraph have "provenance" information for all types and fields, so we can faithfully extract subgraph relatively easily. @@ -221,7 +224,7 @@ export function extractSubgraphsFromSupergraph(supergraph: Schema): Subgraphs { supergraph, subgraphs.names(), (f, name) => { - const fieldApplications: Directive[] = f.appliedDirectivesOf(fieldDirective); + const fieldApplications: Directive[] = f.appliedDirectivesOf(fieldDirective); if (fieldApplications.length) { const application = fieldApplications.find((application) => getSubgraph(application) === name); if (application) { @@ -274,7 +277,11 @@ export function extractSubgraphsFromSupergraph(supergraph: Schema): Subgraphs { // We can have more than one type directive for a given subgraph let subgraphType = schema.type(type.name); if (!subgraphType) { - subgraphType = schema.addType(newNamedType(type.kind, type.name)); + const kind = args.isInterfaceObject ? 'ObjectType' : type.kind; + subgraphType = schema.addType(newNamedType(kind, type.name)); + if (args.isInterfaceObject) { + subgraphType.applyDirective('interfaceObject'); + } } if (args.key) { const { resolvable } = args; @@ -353,6 +360,11 @@ export function extractSubgraphsFromSupergraph(supergraph: Schema): Subgraphs { for (const application of fieldApplications) { const args = application.arguments(); + // We use a @join__field with no graph to indicates when a field in the supergraph does not come + // directly from any subgraph and there is thus nothing to do to "extract" it. + if (!args.graph) { + continue; + } const subgraph = subgraphs.get(graphEnumNameToSubgraphName.get(args.graph)!)!; const subgraphField = addSubgraphField(field, subgraph, args.type); if (!subgraphField) { diff --git a/internals-js/src/federation.ts b/internals-js/src/federation.ts index 6d53a2615..c40cfb28a 100644 --- a/internals-js/src/federation.ts +++ b/internals-js/src/federation.ts @@ -302,6 +302,7 @@ function validateAllFieldSet>({ isOnParentType = false, allowOnNonExternalLeafFields = false, allowFieldsWithArguments = false, + allowOnInterface = false, onFields, }: { definition: DirectiveDefinition<{fields: any}>, @@ -311,13 +312,14 @@ function validateAllFieldSet>({ isOnParentType?: boolean, allowOnNonExternalLeafFields?: boolean, allowFieldsWithArguments?: boolean, + allowOnInterface?: boolean, onFields?: (field: FieldDefinition) => void, }): void { for (const application of definition.applications()) { const elt = application.parent as TParent; const type = targetTypeExtractor(elt); const parentType = isOnParentType ? type : (elt.parent as NamedType); - if (isInterfaceType(parentType)) { + if (isInterfaceType(parentType) && !allowOnInterface) { const code = ERROR_CATEGORIES.DIRECTIVE_UNSUPPORTED_ON_INTERFACE.get(definition.name); errorCollector.push(code.err( isOnParentType @@ -438,6 +440,69 @@ function validateNoExternalOnInterfaceFields(metadata: FederationMetadata, error } } +function validateKeyOnInterfacesAreAlsoOnAllImplementations(metadata: FederationMetadata, errorCollector: GraphQLError[]): void { + for (const itfType of metadata.schema.interfaceTypes()) { + const implementations = itfType.possibleRuntimeTypes(); + for (const keyApplication of itfType.appliedDirectivesOf(metadata.keyDirective())) { + // Note that we will always have validated all @key fields at this point, so not bothering with extra validation + const fields = parseFieldSetArgument({parentType: itfType, directive: keyApplication, validate: false}); + const isResolvable = !(keyApplication.arguments().resolvable === false); + const implementationsWithKeyButNotResolvable = new Array(); + const implementationsMissingKey = new Array(); + for (const type of implementations) { + const matchingApp = type.appliedDirectivesOf(metadata.keyDirective()).find((app) => { + const appFields = parseFieldSetArgument({parentType: type, directive: app, validate: false}); + return fields.equals(appFields); + }); + if (matchingApp) { + if (isResolvable && matchingApp.arguments().resolvable === false) { + implementationsWithKeyButNotResolvable.push(type); + } + } else { + implementationsMissingKey.push(type); + } + } + + if (implementationsMissingKey.length > 0) { + const typesString = printHumanReadableList( + implementationsMissingKey.map((i) => `"${i.coordinate}"`), + { + prefix: 'type', + prefixPlural: 'types', + } + ); + errorCollector.push(ERRORS.INTERFACE_KEY_NOT_ON_IMPLEMENTATION.err( + `Key ${keyApplication} on interface type "${itfType.coordinate}" is missing on implementation ${typesString}.`, + { nodes: sourceASTs(...implementationsMissingKey) }, + )); + } else if (implementationsWithKeyButNotResolvable.length > 0) { + const typesString = printHumanReadableList( + implementationsWithKeyButNotResolvable.map((i) => `"${i.coordinate}"`), + { + prefix: 'type', + prefixPlural: 'types', + } + ); + errorCollector.push(ERRORS.INTERFACE_KEY_NOT_ON_IMPLEMENTATION.err( + `Key ${keyApplication} on interface type "${itfType.coordinate}" should be resolvable on all implementation types, but is declared with argument "@key(resolvable:)" set to false in ${typesString}.`, + { nodes: sourceASTs(...implementationsWithKeyButNotResolvable) }, + )); + } + } + } +} + +function validateInterfaceObjectsAreOnEntities(metadata: FederationMetadata, errorCollector: GraphQLError[]): void { + for (const application of metadata.interfaceObjectDirective().applications()) { + if (!isEntityType(application.parent)) { + errorCollector.push(ERRORS.INTERFACE_OBJECT_USAGE_ERROR.err( + `The @interfaceObject directive can only be applied to entity types but type "${application.parent.coordinate}" has no @key in this subgraph.`, + { nodes: application.parent.sourceAST } + )); + } + } +} + /** * Register errors when, for an interface field, some of the implementations of that field are @external * _and_ not all of those field implementation have the same type (which otherwise allowed because field @@ -606,6 +671,11 @@ export class FederationMetadata { return this.sharingPredicate()(field); } + isInterfaceObjectType(type: NamedType): type is ObjectType { + return isObjectType(type) + && hasAppliedDirective(type, this.interfaceObjectDirective()); + } + federationDirectiveNameInSchema(name: string): string { if (this.isFed2Schema()) { const coreFeatures = this.schema.coreFeatures; @@ -660,6 +730,15 @@ export class FederationMetadata { return this.schema.directive(this.federationDirectiveNameInSchema(name)) as DirectiveDefinition | undefined; } + private getPost20FederationDirective( + name: FederationDirectiveName + ): Post20FederationDirectiveDefinition { + return this.getFederationDirective(name) ?? { + name, + applications: () => new Array>(), + }; + } + keyDirective(): DirectiveDefinition<{fields: any, resolvable?: boolean}> { return this.getLegacyFederationDirective(FederationDirectiveName.KEY); } @@ -692,17 +771,18 @@ export class FederationMetadata { return this.getLegacyFederationDirective(FederationDirectiveName.TAG); } - composeDirective(): DirectiveDefinition<{name: string}> | FederationDirectiveNotDefinedInSchema<{name: string}> { - return this.getFederationDirective<{name: string}>(FederationDirectiveName.COMPOSE_DIRECTIVE) ?? { - name: FederationDirectiveName.COMPOSE_DIRECTIVE, - applications: () => new Array>(), - }; + composeDirective(): Post20FederationDirectiveDefinition<{name: string}> { + return this.getPost20FederationDirective(FederationDirectiveName.COMPOSE_DIRECTIVE); } inaccessibleDirective(): DirectiveDefinition<{}> { return this.getLegacyFederationDirective(FederationDirectiveName.INACCESSIBLE); } + interfaceObjectDirective(): Post20FederationDirectiveDefinition<{}> { + return this.getPost20FederationDirective(FederationDirectiveName.INTERFACE_OBJECT); + } + allFederationDirectives(): DirectiveDefinition[] { const baseDirectives: DirectiveDefinition[] = [ this.keyDirective(), @@ -723,6 +803,11 @@ export class FederationMetadata { if (isFederationDirectiveDefinedInSchema(composeDirective)) { baseDirectives.push(composeDirective); } + const interfaceObjectDirective = this.interfaceObjectDirective(); + if (isFederationDirectiveDefinedInSchema(interfaceObjectDirective)) { + baseDirectives.push(interfaceObjectDirective); + } + return baseDirectives; } @@ -762,12 +847,20 @@ export type FederationDirectiveNotDefinedInSchema readonly Directive[], } +export type Post20FederationDirectiveDefinition = + DirectiveDefinition + | FederationDirectiveNotDefinedInSchema; + export function isFederationDirectiveDefinedInSchema( - definition: DirectiveDefinition | FederationDirectiveNotDefinedInSchema + definition: Post20FederationDirectiveDefinition ): definition is DirectiveDefinition { return definition instanceof DirectiveDefinition; } +export function hasAppliedDirective(type: NamedType, definition: Post20FederationDirectiveDefinition): boolean { + return isFederationDirectiveDefinedInSchema(definition) && type.hasAppliedDirective(definition); +} + export class FederationBlueprint extends SchemaBlueprint { constructor(private readonly withRootTypeRenaming: boolean) { super(); @@ -872,6 +965,7 @@ export class FederationBlueprint extends SchemaBlueprint { metadata, isOnParentType: true, allowOnNonExternalLeafFields: true, + allowOnInterface: metadata.federationFeature()!.url.version.compareTo(new FeatureVersion(2, 3)) >= 0, onFields: field => { const type = baseType(field.type!); if (isUnionType(type) || isInterfaceType(type)) { @@ -925,6 +1019,8 @@ export class FederationBlueprint extends SchemaBlueprint { validateNoExternalOnInterfaceFields(metadata, errorCollector); validateAllExternalFieldsUsed(metadata, errorCollector); + validateKeyOnInterfacesAreAlsoOnAllImplementations(metadata, errorCollector); + validateInterfaceObjectsAreOnEntities(metadata, errorCollector); // If tag is redefined by the user, make sure the definition is compatible with what we expect const tagDirective = metadata.tagDirective(); @@ -1070,7 +1166,7 @@ export function setSchemaAsFed2Subgraph(schema: Schema) { // This is the full @link declaration as added by `asFed2SubgraphDocument`. It's here primarily for uses by tests that print and match // subgraph schema to avoid having to update 20+ tests every time we use a new directive or the order of import changes ... -export const FEDERATION2_LINK_WITH_FULL_IMPORTS = '@link(url: "https://specs.apollo.dev/federation/v2.2", import: ["@key", "@requires", "@provides", "@external", "@tag", "@extends", "@shareable", "@inaccessible", "@override", "@composeDirective"])'; +export const FEDERATION2_LINK_WITH_FULL_IMPORTS = '@link(url: "https://specs.apollo.dev/federation/v2.3", import: ["@key", "@requires", "@provides", "@external", "@tag", "@extends", "@shareable", "@inaccessible", "@override", "@composeDirective", "@interfaceObject"])'; export function asFed2SubgraphDocument(document: DocumentNode): DocumentNode { const fed2LinkExtension: SchemaExtensionNode = { @@ -1123,13 +1219,21 @@ export function isFederationField(field: FieldDefinition): boolea } export function isEntityType(type: NamedType): boolean { - if (type.kind !== "ObjectType") { + if (!isObjectType(type) && !isInterfaceType(type)) { return false; } const metadata = federationMetadata(type.schema()); return !!metadata && type.hasAppliedDirective(metadata.keyDirective()); } +export function isInterfaceObjectType(type: NamedType): boolean { + if (!isObjectType(type)) { + return false; + } + const metadata = federationMetadata(type.schema()); + return !!metadata && metadata.isInterfaceObjectType(type); +} + export function buildSubgraph( name: string, url: string, @@ -1191,7 +1295,6 @@ function isFedSpecLinkDirective(directive: Directive): directi } function completeFed1SubgraphSchema(schema: Schema): GraphQLError[] { - // We special case @key, @requires and @provides because we've seen existing user schema where those // have been defined in an invalid way, but in a way that fed1 wasn't rejecting. So for convenience, // if we detect one of those case, we just remove the definition and let the code afteward add the @@ -1482,6 +1585,10 @@ export const serviceTypeSpec = createObjectTypeSpecification({ export const entityTypeSpec = createUnionTypeSpecification({ name: '_Entity', membersFct: (schema) => { + // Please note that `_Entity` cannot use "interface entities" since interface types cannot be in unions. + // It is ok in practice because _Entity is only use as return type for `_entities`, and even when interfaces + // are involve, the result of an `_entities` call will always be an object type anyway, and since we force + // all implementations of an interface entity to be entity themselves in a subgraph, we're fine. return schema.objectTypes().filter(isEntityType).map((t) => t.name); }, }); diff --git a/internals-js/src/federationSpec.ts b/internals-js/src/federationSpec.ts index 5f3794c56..9069c193c 100644 --- a/internals-js/src/federationSpec.ts +++ b/internals-js/src/federationSpec.ts @@ -35,6 +35,7 @@ export enum FederationDirectiveName { TAG = 'tag', INACCESSIBLE = 'inaccessible', COMPOSE_DIRECTIVE = 'composeDirective', + INTERFACE_OBJECT = 'interfaceObject', } const fieldSetTypeSpec = createScalarTypeSpecification({ name: FederationTypeName.FIELD_SET }); @@ -153,6 +154,13 @@ export class FederationSpecDefinition extends FeatureDefinition { }), })); } + + if (version >= (new FeatureVersion(2, 3))) { + this.registerDirective(createDirectiveSpecification({ + name: FederationDirectiveName.INTERFACE_OBJECT, + locations: [DirectiveLocation.OBJECT], + })); + } } private registerDirective(spec: DirectiveSpecification) { @@ -195,6 +203,7 @@ export class FederationSpecDefinition extends FeatureDefinition { export const FEDERATION_VERSIONS = new FeatureDefinitions(federationIdentity) .add(new FederationSpecDefinition(new FeatureVersion(2, 0))) .add(new FederationSpecDefinition(new FeatureVersion(2, 1))) - .add(new FederationSpecDefinition(new FeatureVersion(2, 2))); + .add(new FederationSpecDefinition(new FeatureVersion(2, 2))) + .add(new FederationSpecDefinition(new FeatureVersion(2, 3))); registerKnownFeature(FEDERATION_VERSIONS); diff --git a/internals-js/src/joinSpec.ts b/internals-js/src/joinSpec.ts index 05a548d17..192b47134 100644 --- a/internals-js/src/joinSpec.ts +++ b/internals-js/src/joinSpec.ts @@ -64,11 +64,21 @@ export class JoinSpecDefinition extends FeatureDefinition { if (!this.isV01()) { joinType.addArgument('extension', new NonNullType(schema.booleanType()), false); joinType.addArgument('resolvable', new NonNullType(schema.booleanType()), true); + + if (this.version >= (new FeatureVersion(0, 3))) { + joinType.addArgument('isInterfaceObject', new NonNullType(schema.booleanType()), false); + } } const joinField = this.addDirective(schema, 'field').addLocations(DirectiveLocation.FIELD_DEFINITION, DirectiveLocation.INPUT_FIELD_DEFINITION); joinField.repeatable = true; - joinField.addArgument('graph', new NonNullType(graphEnum)); + // The `graph` argument used to be non-nullable, but @interfaceObject makes us add some field in + // the supergraph that don't "directly" come from any subgraph (they indirectly are inherited from + // an `@interfaceObject` type), and to indicate that, we use a `@join__field(graph: null)` annotation. + const graphArgType = this.version >= (new FeatureVersion(0, 3)) + ? graphEnum + : new NonNullType(graphEnum); + joinField.addArgument('graph', graphArgType); joinField.addArgument('requires', joinFieldSet); joinField.addArgument('provides', joinFieldSet); if (!this.isV01()) { @@ -153,7 +163,7 @@ export class JoinSpecDefinition extends FeatureDefinition { return this.directive(schema, 'graph')!; } - typeDirective(schema: Schema): DirectiveDefinition<{graph: string, key?: string, extension?: boolean, resolvable?: boolean}> { + typeDirective(schema: Schema): DirectiveDefinition<{graph: string, key?: string, extension?: boolean, resolvable?: boolean, isInterfaceObject?: boolean}> { return this.directive(schema, 'type')!; } @@ -162,7 +172,7 @@ export class JoinSpecDefinition extends FeatureDefinition { } fieldDirective(schema: Schema): DirectiveDefinition<{ - graph: string, + graph?: string, requires?: string, provides?: string, override?: string, @@ -182,15 +192,15 @@ export class JoinSpecDefinition extends FeatureDefinition { } } -// Note: This declare a no-yet-agreed-upon join spec v0.2, that: -// 1. allows a repeatable join__field (join-spec#15). -// 2. allows the 'key' argument of join__type to be optional (join-spec#13) -// 3. relax conditions on join__type in general so as to not relate to the notion of owner (join-spec#16). -// 4. has join__implements (join-spec#13) -// The changes from join-spec#17 and join-spec#18 are not yet implemented, but probably should be or we may have bugs -// due to the query planner having an invalid understanding of the subgraph services API. +// The versions are as follows: +// - 0.1: this is the version used by federation 1 composition. Federation 2 is still able to read supergraphs +// using that verison for backward compatibility, but never writes this spec version is not expressive enough +// for federation 2 in general. +// - 0.2: this is the original version released with federation 2. +// - 0.3: adds the `isInterfaceObject` argument to `@join__type`, and make the `graph` in `@join__field` skippable. export const JOIN_VERSIONS = new FeatureDefinitions(joinIdentity) .add(new JoinSpecDefinition(new FeatureVersion(0, 1))) - .add(new JoinSpecDefinition(new FeatureVersion(0, 2))); + .add(new JoinSpecDefinition(new FeatureVersion(0, 2))) + .add(new JoinSpecDefinition(new FeatureVersion(0, 3))); registerKnownFeature(JOIN_VERSIONS); diff --git a/internals-js/src/operations.ts b/internals-js/src/operations.ts index e95bce3fc..8dd1bf66d 100644 --- a/internals-js/src/operations.ts +++ b/internals-js/src/operations.ts @@ -139,7 +139,7 @@ export class Field ex selects(definition: FieldDefinition, assumeValid: boolean = false): boolean { // We've already validated that the field selects the definition on which it was built. - if (definition == this.definition) { + if (definition === this.definition) { return true; } @@ -292,10 +292,18 @@ export class FragmentElement extends AbstractOperationElement { } withUpdatedSourceType(newSourceType: CompositeType): FragmentElement { + return this.withUpdatedTypes(newSourceType, this.typeCondition); + } + + withUpdatedCondition(newCondition: CompositeType | undefined): FragmentElement { + return this.withUpdatedTypes(this.sourceType, newCondition); + } + + withUpdatedTypes(newSourceType: CompositeType, newCondition: CompositeType | undefined): FragmentElement { // Note that we pass the type-condition name instead of the type itself, to ensure that if `newSourceType` was from a different // schema (typically, the supergraph) than `this.sourceType` (typically, a subgraph), then the new condition uses the // definition of the proper schema (the supergraph in such cases, instead of the subgraph). - const newFragment = new FragmentElement(newSourceType, this.typeCondition?.name); + const newFragment = new FragmentElement(newSourceType, newCondition?.name); for (const directive of this.appliedDirectives) { newFragment.applyDirective(directive.definition!, directive.arguments()); } @@ -311,12 +319,18 @@ export class FragmentElement extends AbstractOperationElement { const fragmentParent = this.parentType; const typeCondition = this.typeCondition; if (selectionParent != fragmentParent) { - // As long as there an intersection between the type we cast into and the selection parent, it's ok. - validate( - !typeCondition || runtimeTypesIntersects(selectionParent, typeCondition), - () => `Cannot add fragment of parent type "${this.parentType}" to selection set of parent type "${selectionSet.parentType}"` - ); - return this.withUpdatedSourceType(selectionParent); + // This usually imply that the fragment is not from the same sugraph than then selection. So we need + // to update the source type of the fragment, but also "rebase" the condition to the selection set + // schema. + let updatedTypeCondition: CompositeType | undefined = undefined; + if (typeCondition) { + const typeInSchema = selectionParent.schema().type(typeCondition.name); + validate(typeInSchema, () => `Cannot add ${this} to selection of parent type ${selectionParent}: cannot find condition type in schema of parent type`); + validate(isCompositeType(typeInSchema), () => `Cannot add ${this} to selection of parent type ${selectionParent}: condition type in schema is a ${typeInSchema.kind}`); + validate(runtimeTypesIntersects(selectionParent, typeInSchema), () => `Cannot add ${this} to selection of parent type ${selectionParent}: condition type in schema does not intersect ${selectionParent}`); + updatedTypeCondition = typeInSchema; + } + return this.withUpdatedTypes(selectionParent, updatedTypeCondition); } return this; } @@ -1437,6 +1451,10 @@ export class FieldSelection extends Freezable { this.selectionSet = isLeafType(type) ? undefined : (initialSelectionSet ? initialSelectionSet.cloneIfFrozen() : new SelectionSet(type as CompositeType)); } + get parentType(): CompositeType { + return this.field.parentType; + } + protected us(): FieldSelection { return this; } @@ -1711,6 +1729,10 @@ export abstract class FragmentSelection extends Freezable { abstract withUpdatedSubSelection(newSubSelection: SelectionSet | undefined): FragmentSelection; + get parentType(): CompositeType { + return this.element().parentType; + } + protected us(): FragmentSelection { return this; } diff --git a/internals-js/src/schemaUpgrader.ts b/internals-js/src/schemaUpgrader.ts index 849c260d8..b05158fb9 100644 --- a/internals-js/src/schemaUpgrader.ts +++ b/internals-js/src/schemaUpgrader.ts @@ -229,9 +229,13 @@ export function upgradeSubgraphsIfNecessary(inputs: Subgraphs): UpgradeResult { const subgraphs = new Subgraphs(); let errors: GraphQLError[] = []; + const subgraphsUsingInterfaceObject = []; for (const subgraph of inputs.values()) { if (subgraph.isFed2Subgraph()) { subgraphs.add(subgraph); + if (subgraph.metadata().interfaceObjectDirective().applications().length > 0) { + subgraphsUsingInterfaceObject.push(subgraph.name); + } } else { const otherSubgraphs = inputs.values().filter((s) => s.name !== subgraph.name); const res = new SchemaUpgrader(subgraph, otherSubgraphs).upgrade(); @@ -243,6 +247,15 @@ export function upgradeSubgraphsIfNecessary(inputs: Subgraphs): UpgradeResult { } } } + if (errors.length === 0 && subgraphsUsingInterfaceObject.length > 0) { + const fed1Subgraphs = inputs.values().filter((s) => !s.isFed2Subgraph()).map((s) => s.name); + // Note that we exit this method early if everything is a fed2 schema, so we know at least one of them wasn't. + errors = [ ERRORS.INTERFACE_OBJECT_USAGE_ERROR.err( + 'The @interfaceObject directive can only be used if all subgraphs have federation 2 subgraph schema (schema with a `@link` to "https://specs.apollo.dev/federation" version 2.0 or newer): ' + + `@interfaceObject is used in ${printSubgraphNames(subgraphsUsingInterfaceObject)} but ${printSubgraphNames(fed1Subgraphs)} ${fed1Subgraphs.length > 1 ? 'are not' : 'is not a'} federation 2 subgraph schema.`, + )]; + } + return errors.length === 0 ? { subgraphs, changes } : { errors }; } diff --git a/internals-js/src/supergraphs.ts b/internals-js/src/supergraphs.ts index 022dde929..ae5bdf64c 100644 --- a/internals-js/src/supergraphs.ts +++ b/internals-js/src/supergraphs.ts @@ -11,6 +11,7 @@ const SUPPORTED_FEATURES = new Set([ 'https://specs.apollo.dev/core/v0.2', 'https://specs.apollo.dev/join/v0.1', 'https://specs.apollo.dev/join/v0.2', + 'https://specs.apollo.dev/join/v0.3', 'https://specs.apollo.dev/tag/v0.1', 'https://specs.apollo.dev/tag/v0.2', 'https://specs.apollo.dev/inaccessible/v0.1', diff --git a/query-graphs-js/src/graphPath.ts b/query-graphs-js/src/graphPath.ts index d5e3a86fa..d47c206e2 100644 --- a/query-graphs-js/src/graphPath.ts +++ b/query-graphs-js/src/graphPath.ts @@ -32,7 +32,7 @@ import { } from "@apollo/federation-internals"; import { OpPathTree, traversePathTree } from "./pathTree"; import { Vertex, QueryGraph, Edge, RootVertex, isRootVertex, isFederatedGraphRootType, FEDERATED_GRAPH_ROOT_SOURCE } from "./querygraph"; -import { Transition } from "./transition"; +import { DownCast, Transition } from "./transition"; import { PathContext, emptyContext } from "./pathContext"; import { v4 as uuidv4 } from 'uuid'; @@ -65,6 +65,8 @@ function updateRuntimeTypes(currentRuntimeTypes: readonly ObjectType[], edge: Ed const castedType = edge.transition.castedType; const castedRuntimeTypes = possibleRuntimeTypes(castedType); return currentRuntimeTypes.filter(t => castedRuntimeTypes.includes(t)); + case 'InterfaceObjectFakeDownCast': + return currentRuntimeTypes; case 'KeyResolution': const currentType = edge.tail.type as CompositeType; // We've taken a key into a new subgraph, so any of the possible runtime types of the new subgraph could be returned. @@ -295,55 +297,56 @@ export class GraphPath `Shouldn't have conditions paths (got ${conditionsResolution.pathTree}) for edge without conditions (edge: ${edge})`); - if (edge && edge.transition.kind === 'DownCast' && this.props.edgeToTail) { - const previousOperation = this.lastTrigger(); - if (previousOperation instanceof FragmentElement && previousOperation.appliedDirectives.length === 0) { - // This mean we have 2 type-cast back-to-back and that means the previous operation might not be - // useful on this path. More precisely, the previous type-cast was only useful if it restricted - // the possible runtime types of the type on which it applied more than the current type-cast - // does (but note that if the previous type-cast had directives, we keep it not matter what in - // case those directives are important). - // That is, we're in the where we have (somewhere potentially deep in a query): - // f { # field 'f' of type A - // ... on B { - // ... on C { - // # more stuffs - // } - // } - // } - // If the intersection of A and C is non empty and included (or equal) to the intersection of A and B, - // then there is no reason to have `... on B` at all because: - // 1. you can do `... on C` on `f` directly since the intersection of A and C is non-empty. - // 2. `... on C` restricts strictly more than `... on B` and so the latter can't impact the result. - // So if we detect that we're in that situation, we remove the `... on B` (but note that this is an - // optimization, keeping `... on B` wouldn't be incorrect, just useless). - const runtimeTypesWithoutPreviousCast = updateRuntimeTypes(this.props.runtimeTypesBeforeTailIfLastIsCast!, edge); - if (runtimeTypesWithoutPreviousCast.length > 0 - && runtimeTypesWithoutPreviousCast.every(t => this.props.runtimeTypesOfTail.includes(t)) - ) { - // Note that edge is from the vertex we've eliminating from the path. So we need to get the edge goes - // directly from the prior vertex to the new tail for that path. - const updatedEdge = this.graph.outEdges(this.props.edgeToTail!.head).find(e => e.tail.type === edge.tail.type); - if (updatedEdge) { - // We replace the previous operation by the new one. - debug.log(() => `Previous cast ${previousOperation} is made obsolete by new cast ${trigger}, removing from path.`); - return new GraphPath({ - ...this.props, - tail: updatedEdge.tail, - edgeTriggers: withReplacedLastElement(this.props.edgeTriggers, trigger), - edgeIndexes: withReplacedLastElement(this.props.edgeIndexes, updatedEdge.index), - edgeConditions: withReplacedLastElement(this.props.edgeConditions, conditionsResolution.pathTree ?? null), - edgeToTail: updatedEdge, - runtimeTypesOfTail: runtimeTypesWithoutPreviousCast, - deferOnTail: defer, - }); + let subgraphEnteringEdge = this.subgraphEnteringEdge; + + if (edge) { + if (edge.transition.kind === 'DownCast' && this.props.edgeToTail) { + const previousOperation = this.lastTrigger(); + if (previousOperation instanceof FragmentElement && previousOperation.appliedDirectives.length === 0) { + // This mean we have 2 type-cast back-to-back and that means the previous operation might not be + // useful on this path. More precisely, the previous type-cast was only useful if it restricted + // the possible runtime types of the type on which it applied more than the current type-cast + // does (but note that if the previous type-cast had directives, we keep it no matter what in + // case those directives are important). + // That is, we're in the case where we have (somewhere potentially deep in a query): + // f { # field 'f' of type A + // ... on B { + // ... on C { + // # more stuffs + // } + // } + // } + // If the intersection of A and C is non empty and included (or equal) to the intersection of A and B, + // then there is no reason to have `... on B` at all because: + // 1. you can do `... on C` on `f` directly since the intersection of A and C is non-empty. + // 2. `... on C` restricts strictly more than `... on B` and so the latter can't impact the result. + // So if we detect that we're in that situation, we remove the `... on B` (but note that this is an + // optimization, keeping `... on B` wouldn't be incorrect, just useless). + const runtimeTypesWithoutPreviousCast = updateRuntimeTypes(this.props.runtimeTypesBeforeTailIfLastIsCast!, edge); + if (runtimeTypesWithoutPreviousCast.length > 0 + && runtimeTypesWithoutPreviousCast.every(t => this.props.runtimeTypesOfTail.includes(t)) + ) { + // Note that edge is from the vertex we've eliminating from the path. So we need to get the edge goes + // directly from the prior vertex to the new tail for that path. + const updatedEdge = this.graph.outEdges(this.props.edgeToTail!.head).find(e => e.tail.type === edge.tail.type); + if (updatedEdge) { + // We replace the previous operation by the new one. + debug.log(() => `Previous cast ${previousOperation} is made obsolete by new cast ${trigger}, removing from path.`); + return new GraphPath({ + ...this.props, + tail: updatedEdge.tail, + edgeTriggers: withReplacedLastElement(this.props.edgeTriggers, trigger), + edgeIndexes: withReplacedLastElement(this.props.edgeIndexes, updatedEdge.index), + edgeConditions: withReplacedLastElement(this.props.edgeConditions, conditionsResolution.pathTree ?? null), + edgeToTail: updatedEdge, + runtimeTypesOfTail: runtimeTypesWithoutPreviousCast, + deferOnTail: defer, + }); + } } } } - } - let subgraphEnteringEdge = this.subgraphEnteringEdge; - if (edge) { // `subgraphEnteringEdge` is used to be able to eliminate some options when we can detect that going to subgraph // was ineffectient (see `advancePathWithNonCollectingAndTypePreservingTransitions` for details). So first, // we shouldn't change it for a "key to self" due to a @defer since the assumption of `subgraphEnteringEdge` is @@ -357,6 +360,24 @@ export class GraphPath( // // So we need to handle this case and we do this first. Note that the only kind of // transition that can give use this is a 'DownCast' transition. - if (transition.kind === 'DownCast') { + // Also note that if the subgraph type we're on is an @interfaceObject type, then we + // also can't be in this situation as an @interfaceObject type "stands in" for all + // the possible implementations of that interface. And one way to detect if the subgraph + // type an @interfaceObject is to check if the subgraph type is an object type while the + // supergraph type is an interface one. + if (transition.kind === 'DownCast' && !(isInterfaceType(transition.sourceType) && isObjectType(subgraphPath.path.tail.type))) { // If we consider a 'downcast' transition, it means that the target of that cast is composite, but also that the // last type of the subgraph path also is (that type is essentially the "source" of the cast). const supergraphRuntimeTypes = possibleRuntimeTypes(targetType as CompositeType); @@ -952,20 +989,41 @@ export function advancePathWithTransition( const type = schema.type(typeName); if (type && isCompositeType(type) && type.field(fieldName)) { // That subgraph has the type we look for, but we have recorded no dead-ends. This means there is no edge to that type, - // and thus that it has no keys. - const metadata = federationMetadata(schema); - const keys: Directive[] = metadata ? type.appliedDirectivesOf(metadata.keyDirective()) : []; - const allNonResolvable = keys.length > 0 && keys.every((k) => !(k.arguments().resolvable ?? true)); - assert(keys.length === 0 || allNonResolvable, () => `Expected type ${type} in ${subgraph} to have no resolvable keys`); - const explanation = keys.length === 0 - ? `type "${typeName}" has no @key defined in subgraph "${subgraph}"` - : `none of the @key defined on type "${typeName}" in subgraph "${subgraph}" are resolvable (they are all declared with their "resolvable" argument set to false)`; - allDeadEnds.push({ - sourceSubgraph: subgraphPath.path.tail.source, - destSubgraph: subgraph, - reason: UnadvanceableReason.UNREACHABLE_TYPE, - details: `cannot move to subgraph "${subgraph}", which has field "${transition.definition.coordinate}", because ${explanation}` - }); + // and thus that either: + // - it has no keys. + // - the path to advance it an @interfaceObject type, the type we look for is an implementation of that interface, and + // there no key on the interface. + const typenameOfTail = subgraphPath.path.tail.type.name; + const typeOfTailInSubgraph = schema.type(typenameOfTail); + if (!typeOfTailInSubgraph) { + // This means that 1) the type of the path we're trying to advance is different from the transition we're considering, + // and that should only happen if the path is on an @interfaceObject type, and 2) the subgraph we're looking at + // actually doesn't have that interface. To be able to jump to that subgraph, we would need the interface and it + // would need to have a resolvable key, but it has neither. + allDeadEnds.push({ + sourceSubgraph: subgraphPath.path.tail.source, + destSubgraph: subgraph, + reason: UnadvanceableReason.UNREACHABLE_TYPE, + details: `cannot move to subgraph "${subgraph}", which has field "${transition.definition.coordinate}", because interface "${typenameOfTail}" is not defined in this subgraph (to jump to "${subgraph}", it would need to both define interface "${typenameOfTail}" and have a @key on it)`, + }); + } else { + // `typeOfTailInSubgraph` exists, so it's either equal to `type`, or it's an interface of it. In any case, it's composite. + assert(isCompositeType(typeOfTailInSubgraph), () => `Type ${typeOfTailInSubgraph} in ${subgraph} should be composite`); + const metadata = federationMetadata(schema); + const keys: Directive[] = metadata ? typeOfTailInSubgraph.appliedDirectivesOf(metadata.keyDirective()) : []; + const allNonResolvable = keys.length > 0 && keys.every((k) => !(k.arguments().resolvable ?? true)); + assert(keys.length === 0 || allNonResolvable, () => `After ${subgraphPath} and for transition ${transition}, expected type ${type} in ${subgraph} to have no resolvable keys`); + const kindOfType = typeOfTailInSubgraph === type ? 'type' : 'interface'; + const explanation = keys.length === 0 + ? `${kindOfType} "${typenameOfTail}" has no @key defined in subgraph "${subgraph}"` + : `none of the @key defined on ${kindOfType} "${typenameOfTail}" in subgraph "${subgraph}" are resolvable (they are all declared with their "resolvable" argument set to false)`; + allDeadEnds.push({ + sourceSubgraph: subgraphPath.path.tail.source, + destSubgraph: subgraph, + reason: UnadvanceableReason.UNREACHABLE_TYPE, + details: `cannot move to subgraph "${subgraph}", which has field "${transition.definition.coordinate}", because ${explanation}` + }); + } } } } @@ -1068,7 +1126,27 @@ function advancePathWithNonCollectingAndTypePreservingTransitions !e.transition.collectOperationElements); if (nextEdges.length === 0) { - debug.log(() => `Nothing to try for ${toAdvance}: it has no non-collecting outbound edges`); + // The subtlety here is that this may either mean that there is no non-collecting edges from the tail of + // this path, _or_ that there is some but they "trivial" since `nextEdges` above may end up calling + // `QueryGraph.nonTrivialFollowupEdges()`. In the later, this means there is a key we could use, but + // it get us back to the previous vertex in the path, which is useless. But we distinguish that case + // to 1) make the debug more "true" and 2) much more importantly, record a "dead-end" for this path. + const outEdges = toAdvance.graph.outEdges(toAdvance.tail).filter(e => !e.transition.collectOperationElements); + if (outEdges.length > 0) { + debug.log(() => `Nothing to try for ${toAdvance}: it only has "trivial" non-collecting outbound edges`); + for (const edge of outEdges) { + if (edge.tail.source !== toAdvance.tail.source && edge.tail.source !== originalSource) { + deadEnds.push({ + sourceSubgraph: toAdvance.tail.source, + destSubgraph: edge.tail.source, + reason: UnadvanceableReason.IGNORED_INDIRECT_PATH, + details: `ignoring moving to subgraph "${edge.tail.source}" using @key(fields: "${edge.conditions?.toString(true, false)}") of "${edge.head.type}" because there is a more direct path in ${edge.tail.source} that avoids ${toAdvance.tail.source} altogether` + }); + } + } + } else { + debug.log(() => `Nothing to try for ${toAdvance}: it has no non-collecting outbound edges`); + } continue; } debug.group(() => `From ${toAdvance}:`); @@ -1088,13 +1166,6 @@ function advancePathWithNonCollectingAndTypePreservingTransitions( ) : GraphPath[] | Unadvanceables { assert(transition.collectOperationElements, "Supergraphs shouldn't have transitions that don't collect elements"); + if ( + transition.kind === 'FieldCollection' + && transition.definition.parent.name !== path.tail.type.name + && isCompositeType(path.tail.type) + && !path.tailIsInterfaceObject() + ) { + // Usually, when we collect a field, the path should already be on the type of that field. + // But one exception is due to the fact that a type condition may be "absorbed" by an + // @interfaceObject, and once we'we taken a key on the interface to another subgraph + // (the tail is not the interface object anymore), we need to "restore" the type condition + // first. + const updatedPath = advancePathWithDirectTransition( + path, + new DownCast(path.tail.type, transition.definition.parent), + conditionResolver, + ); + // The case we described above should be the only case we capture here, and so the current + // subgraph must have the implementation type (it may not have the field we want, but it + // must have the type) and so we should be able to advance to it. + assert(!isUnadvanceable(updatedPath), () => `Advancing ${path} for ${transition} gave ${updatedPath}`); + // Also note that there is currently no case where we should have more that one option. + assert(updatedPath.length === 1, () => `Expect one path, got ${updatedPath.length}`) + path = updatedPath[0]; + // We can now continue on dealing with the actual field. + } + const options: GraphPath[] = []; const deadEnds: Unadvanceable[] = []; @@ -1304,20 +1401,41 @@ function advancePathWithDirectTransition( if (conditionResolution.satisfied) { options.push(path.add(transition, edge, conditionResolution)); } else { - // The only direct transitions are fields and typeCast, and the later cannot have conditions (the former can, and it - // is a @require). - assert(transition.kind === 'FieldCollection', () => `Shouldn't have conditions on direct transition ${transition}`); - const field = transition.definition; - const parentTypeInSubgraph = path.graph.sources.get(edge.head.source)!.type(field.parent.name)! as CompositeType; - const details = conditionResolution.unsatisfiedConditionReason === UnsatisfiedConditionReason.NO_POST_REQUIRE_KEY - ? `@require condition on field "${field.coordinate}" can be satisfied but missing usable key on "${parentTypeInSubgraph}" in subgraph "${edge.head.source}" to resume query` - : `cannot satisfy @require conditions on field "${field.coordinate}"${warnOnKeyFieldsMarkedExternal(parentTypeInSubgraph)}`; - deadEnds.push({ - sourceSubgraph: edge.head.source, - destSubgraph: edge.head.source, - reason: UnadvanceableReason.UNSATISFIABLE_REQUIRES_CONDITION, - details - }); + switch (edge.transition.kind) { + case 'FieldCollection': + { + // Condition on a field means a @require + const field = edge.transition.definition; + const parentTypeInSubgraph = path.graph.sources.get(edge.head.source)!.type(field.parent.name)! as CompositeType; + const details = conditionResolution.unsatisfiedConditionReason === UnsatisfiedConditionReason.NO_POST_REQUIRE_KEY + ? `@require condition on field "${field.coordinate}" can be satisfied but missing usable key on "${parentTypeInSubgraph}" in subgraph "${edge.head.source}" to resume query` + : `cannot satisfy @require conditions on field "${field.coordinate}"${warnOnKeyFieldsMarkedExternal(parentTypeInSubgraph)}`; + deadEnds.push({ + sourceSubgraph: edge.head.source, + destSubgraph: edge.head.source, + reason: UnadvanceableReason.UNSATISFIABLE_REQUIRES_CONDITION, + details + }); + } + break; + case 'InterfaceObjectFakeDownCast': + { + // The condition on such edge is only __typename, so it essentially means that an @interfaceObject exists but there is no reachable subgraph + // with a @key on an interface to find out proper implementations. + const details = conditionResolution.unsatisfiedConditionReason === UnsatisfiedConditionReason.NO_POST_REQUIRE_KEY + ? `@interfaceObject type "${edge.transition.sourceType.coordinate}" misses a resolvable key to resume query once the implementation type has been resolved` + : `no subgraph can be reached to resolve the implementation type of @interfaceObject type "${edge.transition.sourceType.coordinate}"`; + deadEnds.push({ + sourceSubgraph: edge.head.source, + destSubgraph: edge.head.source, + reason: UnadvanceableReason.UNRESOLVABLE_INTERFACE_OBJECT, + details + }); + } + break; + default: + assert(false, () => `Shouldn't have conditions on direct transition ${transition}`); + } } } if (options.length > 0) { @@ -1329,31 +1447,38 @@ function advancePathWithDirectTransition( const subgraph = path.tail.source; if (transition.kind === 'FieldCollection') { const schema = path.graph.sources.get(subgraph)!; - const typeInSubgraph = schema.type(path.tail.type.name); - const fieldInSubgraph = typeInSubgraph && isCompositeType(typeInSubgraph) - ? typeInSubgraph.field(transition.definition.name) - : undefined; - - if (fieldInSubgraph) { - // the subgraph has the field but no corresponding edge. This should only happen if the field is external. - const externalDirective = fieldInSubgraph.appliedDirectivesOf(federationMetadata(fieldInSubgraph.schema())!.externalDirective()).pop(); - assert( - externalDirective, - () => `${fieldInSubgraph.coordinate} in ${subgraph} is not external but there is no corresponding edge (edges from ${path} = [${path.nextEdges().join(', ')}])` - ); - // but the field is external in the "subgraph-extracted-from-the-supergraph", but it might have been forced to an external - // due to being a used-overriden field, in which case we want to amend the message to avoid confusing the user. - // Note that the subgraph extraction marks such "forced external due to being overriden" by setting the "reason" to "[overridden]". - const overriddingSources = externalDirective.arguments().reason === '[overridden]' - ? findOverriddingSourcesIfOverridden(fieldInSubgraph, subgraph, path.graph.sources) - : []; - if (overriddingSources.length > 0) { - details = `field "${transition.definition.coordinate}" is not resolvable because it is overridden by ${printSubgraphNames(overriddingSources)}`; + const fieldTypeName = transition.definition.parent.name; + const typeInSubgraph = schema.type(fieldTypeName); + if (!typeInSubgraph && path.tail.type.name !== fieldTypeName) { + // This is due to us looking for an implementation field, but the subgraph not having that implementation because + // it uses @interfaceObject on an interface of that implementation. + details = `cannot find implementation type "${fieldTypeName}" (supergraph interface "${path.tail.type.name}" is declared with @interfaceObject in "${subgraph}")`; + } else { + const fieldInSubgraph = typeInSubgraph && isCompositeType(typeInSubgraph) + ? typeInSubgraph.field(transition.definition.name) + : undefined; + + if (fieldInSubgraph) { + // the subgraph has the field but no corresponding edge. This should only happen if the field is external. + const externalDirective = fieldInSubgraph.appliedDirectivesOf(federationMetadata(fieldInSubgraph.schema())!.externalDirective()).pop(); + assert( + externalDirective, + () => `${fieldInSubgraph.coordinate} in ${subgraph} is not external but there is no corresponding edge (edges from ${path} = [${path.nextEdges().join(', ')}])` + ); + // but the field is external in the "subgraph-extracted-from-the-supergraph", but it might have been forced to an external + // due to being a used-overriden field, in which case we want to amend the message to avoid confusing the user. + // Note that the subgraph extraction marks such "forced external due to being overriden" by setting the "reason" to "[overridden]". + const overriddingSources = externalDirective.arguments().reason === '[overridden]' + ? findOverriddingSourcesIfOverridden(fieldInSubgraph, subgraph, path.graph.sources) + : []; + if (overriddingSources.length > 0) { + details = `field "${transition.definition.coordinate}" is not resolvable because it is overridden by ${printSubgraphNames(overriddingSources)}`; + } else { + details = `field "${transition.definition.coordinate}" is not resolvable because marked @external`; + } } else { - details = `field "${transition.definition.coordinate}" is not resolvable because marked @external`; + details = `cannot find field "${transition.definition.coordinate}"`; } - } else { - details = `cannot find field "${transition.definition.coordinate}"`; } } else { assert(transition.kind === 'DownCast', () => `Unhandled direct transition ${transition} of kind ${transition.kind}`); @@ -1587,28 +1712,32 @@ export function advanceSimultaneousPathsWithOperation( debug.group(() => `Computing options for ${path}`); // If we're just entering a deferred section, then we will need to re-enter subgraphs, so we should not consider - // direct options and instead force and indirect path. + // direct options and instead force an indirect path. if (!path.deferOnTail) { debug.group(() => `Direct options`); - options = advanceWithOperation( + const { options: advanceOptions, hasOnlyTypeExplodedResults } = advanceWithOperation( supergraphSchema, path, operation, updatedContext, subgraphSimultaneousPaths.conditionResolver ); + options = advanceOptions; debug.groupEnd(() => advanceOptionsToString(options)); // If we got some options, there is number of cases where there is no point looking for indirect paths: // - if the operation is terminal: this mean we just found a direct edge that is terminal, so no // indirect options could be better (this is no true for non-terminal where the direct route may - // end up being a dead end later). + // end up being a dead end later). One exception however is when `advanceWithOperation` type-exploded (meaning + // that're on an interface), because in that case, the type-exploded options have already taken indirect edges + // into account, so it's possible that an indirect edge _from the interface_ could be better, but only if + // there wasn't a "true" direct edge on the interface, which is what `hasOnlyTypeExplodedResults` tells us. // - if we get options, but an empty set of them, which signifies the operation correspond to unsatisfiable // conditions and we can essentially ignore it. // - if the operation is a fragment in general: if we were able to find a direct option, that means the type // is known in the "current" subgraph, and so we'll still be able to take any indirect edges that we could // take now later, for the follow-up operation. And pushing the decision will give us more context and may // avoid a bunch of state explosion in practice. - if (options && (options.length === 0 || isTerminalOperation(operation) || operation.kind === 'FragmentElement')) { + if (options && (options.length === 0 || (isTerminalOperation(operation) && !hasOnlyTypeExplodedResults) || operation.kind === 'FragmentElement')) { debug.groupEnd(() => `Final options for ${path}: ${advanceOptionsToString(options)}`); // Note that options is empty, that means this particular "branch" is unsatisfiable, so we should just ignore it. if (options.length > 0) { @@ -1628,7 +1757,7 @@ export function advanceSimultaneousPathsWithOperation( debug.group('Validating indirect options:'); for (const pathWithNonCollecting of pathsWithNonCollecting.paths) { debug.group(() => `For indirect path ${pathWithNonCollecting}:`); - const pathWithOperation = advanceWithOperation( + const { options: pathWithOperation } = advanceWithOperation( supergraphSchema, pathWithNonCollecting, operation, @@ -1656,13 +1785,14 @@ export function advanceSimultaneousPathsWithOperation( // to fall back on not deferring. if (options.length === 0 && path.deferOnTail) { debug.group(() => `Cannot defer (no indirect options); falling back to direct options`); - options = advanceWithOperation( + const { options: advanceOptions } = advanceWithOperation( supergraphSchema, path, operation, updatedContext, subgraphSimultaneousPaths.conditionResolver - ) ?? []; + ); + options = advanceOptions ?? []; debug.groupEnd(() => advanceOptionsToString(options)); } @@ -1917,14 +2047,17 @@ function advanceWithOperation( operation: OperationElement, context: PathContext, conditionResolver: ConditionResolver, -) : SimultaneousPaths[] | undefined { +) : { + options: SimultaneousPaths[] | undefined, + hasOnlyTypeExplodedResults?: boolean, +} { debug.group(() => `Trying to advance ${path} directly with ${operation}`); const currentType = path.tail.type; if (isFederatedGraphRootType(currentType)) { // We cannot advance any operation from there: we need to take the initial non-collecting edges first. debug.groupEnd('Cannot advance federated graph root with direct operations'); - return undefined; + return { options: undefined }; } if (operation.kind === 'Field') { @@ -1935,14 +2068,26 @@ function advanceWithOperation( const edge = nextEdgeForField(path, operation); if (!edge) { debug.groupEnd(() => `No edge for field ${field} on object type ${currentType}`); - return undefined; + return { options: undefined }; } + + // If the current type is an @interfaceObject, it's possible that the requested field + // is a field of an implementation of the interface. Because we found an edge, we + // know that the interface has the field and we can use the edge, but we should redact + // the `operation` to use the current type field, so the operation does not continue + // refering to a type that is not in the current subgraph. + if (path.tailIsInterfaceObject() && field.parent.name !== currentType.name) { + const fieldOnCurrentType = currentType.field(field.name); + assert(fieldOnCurrentType, () => `We should not have found edge ${edge} for ${field} from ${path}`) + operation = operation.withUpdatedDefinition(fieldOnCurrentType); + } + const fieldPath = addFieldEdge(path, operation, edge, conditionResolver, context); debug.groupEnd(() => fieldPath ? `Collected field ${field} on object type ${currentType}` : `Cannot satisfy @requires on field ${field} for object type ${currentType}` ); - return pathAsOptions(fieldPath); + return { options: pathAsOptions(fieldPath) }; case 'InterfaceType': // First, we check if there is a direct edge from the interface (which only happens if we're in a subgraph that knows all of the // implementations of that interface globally and all of them resolve the field). @@ -1979,19 +2124,35 @@ function advanceWithOperation( // if the direct edge cannot be satisfied? Probably depends on the exact semantic of @requires on interface fields). if (directPathOverrideTypeExplosion && (isLeafType(field.type!) || !anImplementationIsEntityWithFieldShareable(path, field.name, currentType))) { debug.groupEnd(() => `Collecting (leaf) field ${field} on interface ${currentType} without type-exploding`); - return pathAsOptions(itfPath); + return { options: pathAsOptions(itfPath) }; } debug.log(() => `Collecting field ${field} on interface ${currentType} as 1st option`); } - // Otherwise, that means we need to type explode and descend into every possible implementations (implementations "in the current - // subgraph", since the previous edge to this interface had to come from the current subgraph and can thus only have returned - // local implementations). - // TODO: once we add @key on interfaces, this will have to be updated. - const implementations = path.tailPossibleRuntimeTypes(); - debug.log(() => !itfPath - ? `No direct edge: type exploding interface ${currentType} into possible runtime types [${implementations.join(', ')}]` - : `Type exploding interface ${currentType} into possible runtime types [${implementations.join(', ')}] as 2nd option` - ); + + // There is 2 main cases to handle here: + // - the most common is that it's a field of the interface that is queried, and + // so we should type-explode because either didn't had a direct edge, or @provides + // makes it potentially worthwile to check with type explosion. + // - but we could also have the case where the field queried is actually of one + // of the implementation of the interface: this happens if we just entered the + // subgraph on an interface @key. In that case, we only want to consider that one + // implementation. + let implementations: readonly ObjectType[]; + if (field.parent.name !== currentType.name) { + assert( + isObjectType(field.parent) && path.tailPossibleRuntimeTypes().some((t) => t.name === field.parent.name), + () => `${field.coordinate} requested on ${currentType}, but ${field.parent} is not an implementation` + ); + implementations = [ field.parent ]; + debug.log(() => `Casting into requested type ${field.parent}`); + } else { + implementations = path.tailPossibleRuntimeTypes(); + debug.log(() => !itfPath + ? `No direct edge: type exploding interface ${currentType} into possible runtime types [${implementations.join(', ')}]` + : `Type exploding interface ${currentType} into possible runtime types [${implementations.join(', ')}] as 2nd option` + ); + } + // For all implementations, We need to call advanceSimultaneousPathsWithOperation on a made-up Fragment. If any // gives use empty options, we bail. const optionsByImplems: OpGraphPath[][][] = []; @@ -2007,7 +2168,7 @@ function advanceWithOperation( if (!implemOptions) { debug.groupEnd(); debug.groupEnd(() => `Cannot collect field ${field} from ${implemType}: stopping with options [${itfPath}]`); - return pathAsOptions(itfPath); + return { options: pathAsOptions(itfPath) }; } // If the new fragment makes it so that we're on an unsatisfiable branch, we just ignore that implementation. if (implemOptions.length === 0) { @@ -2038,7 +2199,7 @@ function advanceWithOperation( if (withField.length === 0) { debug.groupEnd(); // implem group debug.groupEnd(() => `Cannot collect field ${field} from ${implemType}: stopping with options [${itfPath}]`); - return pathAsOptions(itfPath); + return { options: pathAsOptions(itfPath) }; } debug.groupEnd(() => `Collected field ${field} from ${implemType}`); optionsByImplems.push(withField); @@ -2051,13 +2212,13 @@ function advanceWithOperation( allOptions = pathAsOptions(itfPath)!.concat(allOptions); } debug.groupEnd(() => `With type-exploded options: ${advanceOptionsToString(allOptions)}`); - return allOptions; + return { options: allOptions, hasOnlyTypeExplodedResults: !itfPath }; case 'UnionType': assert(field.name === typenameFieldName, () => `Invalid field selection ${operation} for union type ${currentType}`); const typenameEdge = nextEdgeForField(path, operation); assert(typenameEdge, `Should always have an edge for __typename edge on an union`); debug.groupEnd(() => `Trivial collection of __typename for union ${currentType}`); - return pathAsOptions(addFieldEdge(path, operation, typenameEdge, conditionResolver, context)); + return { options: pathAsOptions(addFieldEdge(path, operation, typenameEdge, conditionResolver, context)) }; default: // Only object, interfaces and unions (only for __typename) have fields so the query should have been flagged invalid if a field was selected on something else. assert(false, `Unexpected ${currentType.kind} type ${currentType} from ${path.tail} given operation ${operation}`); @@ -2072,7 +2233,7 @@ function advanceWithOperation( const updatedPath = operation.appliedDirectives.length > 0 ? path.add(operation, null, noConditionsResolution, operation.deferDirectiveArgs()) : path; - return [[ updatedPath ]]; + return { options: [[ updatedPath ]] }; } const typeName = operation.typeCondition.name; switch (currentType.kind) { @@ -2083,7 +2244,7 @@ function advanceWithOperation( if (edge) { assert(!edge.conditions, "TypeCast collecting edges shouldn't have conditions"); debug.groupEnd(() => `Using type-casting edge for ${typeName} from current type ${currentType}`); - return [[path.add(operation, edge, noConditionsResolution, operation.deferDirectiveArgs())]]; + return { options: [[path.add(operation, edge, noConditionsResolution, operation.deferDirectiveArgs())]] }; } // Otherwise, checks what is the intersection between the possible runtime types of the current type // and the ones of the cast. We need to be able to go into all those types simultaneously. @@ -2103,7 +2264,7 @@ function advanceWithOperation( if (!implemOptions) { debug.groupEnd(); debug.groupEnd(() => `Cannot advance into ${tName} from ${currentType}: no options for ${operation}.`); - return undefined; + return { options: undefined }; } // If the new fragment makes it so that we're on an unsatisfiable branch, we just ignore that implementation. if (implemOptions.length === 0) { @@ -2115,12 +2276,18 @@ function advanceWithOperation( } const allCastOptions = flatCartesianProduct(optionsByImplems); debug.groupEnd(() => `Type-exploded options: ${advanceOptionsToString(allCastOptions)}`); - return allCastOptions; + return { options: allCastOptions }; case 'ObjectType': // We've already handled the case of a fragment whose condition is this type. But the fragment might // be for either: // - a super-type of the current type: in which case, we're pretty much in the same case than if there // were no particular condition. + // - if the current type is an @interfaceObject, then this can be an implementation type of + // the interface in the supergraph. In that case, the type of the condition is not a + // type known of the subgraph, but the subgraph might still be able to handle some of + // fields, so in that case, we essentially "ignore" the fragment for now. We will re-add + // it back later for fields that are not in the current subgraph after we've taken + // a @key for the interface. // - another, incompatible type. This can happen for a type that intersects a super-type of the // current type (since graphQL allows a fragment as long as there is an intersection). In that // case, the whole operation simply cannot ever return anything. @@ -2130,13 +2297,26 @@ function advanceWithOperation( const updatedPath = operation.appliedDirectives.length > 0 ? path.add(operation, null, noConditionsResolution, operation.deferDirectiveArgs()) : path; - return [[ updatedPath ]]; + return { options: [[ updatedPath ]] }; + } + + if (path.tailIsInterfaceObject()) { + const fakeDownCastEdge = path.nextEdges().find((e) => e.transition.kind === 'InterfaceObjectFakeDownCast' && e.transition.castedTypeName === typeName); + if (fakeDownCastEdge) { + const conditionResolution = canSatisfyConditions(path, fakeDownCastEdge, conditionResolver, context, [], []); + if (!conditionResolution.satisfied) { + return { options: undefined }; + } + const updatedPath = path.add(operation, fakeDownCastEdge, conditionResolution, operation.deferDirectiveArgs()); + return { options: [[ updatedPath ]] }; + } } + // The operation we're dealing with can never return results (the type conditions applies have no intersection). // This means we _can_ fulfill this operation (by doing nothing and returning an empty result), which we indicate // by return an empty list of options. debug.groupEnd(() => `Cannot ever get ${typeName} from current type ${currentType}: returning empty branch`); - return []; + return { options: [] }; default: // We shouldn't have a fragment on a non-selectable type assert(false, `Unexpected ${currentType.kind} type ${currentType} from ${path.tail} given operation ${operation}`); diff --git a/query-graphs-js/src/querygraph.ts b/query-graphs-js/src/querygraph.ts index 51c173601..95b646c70 100644 --- a/query-graphs-js/src/querygraph.ts +++ b/query-graphs-js/src/querygraph.ts @@ -30,9 +30,13 @@ import { federationMetadata, FederationMetadata, DirectiveDefinition, + Directive, + typenameFieldName, + Field, + selectionSetOfElement, } from '@apollo/federation-internals'; import { inspect } from 'util'; -import { DownCast, FieldCollection, subgraphEnteringTransition, SubgraphEnteringTransition, Transition, KeyResolution, RootTypeResolution } from './transition'; +import { DownCast, FieldCollection, subgraphEnteringTransition, SubgraphEnteringTransition, Transition, KeyResolution, RootTypeResolution, InterfaceObjectFakeDownCast } from './transition'; import { preComputeNonTrivialFollowupEdges } from './nonTrivialEdgePrecomputing'; // We use our federation reserved subgraph name to avoid risk of conflict with other subgraph names (wouldn't be a huge @@ -175,6 +179,7 @@ export class Edge { switch (transition.kind) { case 'FieldCollection': return otherTransition.kind === 'FieldCollection' && transition.definition.name === otherTransition.definition.name; case 'DownCast': return otherTransition.kind === 'DownCast' && transition.castedType.name === otherTransition.castedType.name; + case 'InterfaceObjectFakeDownCast': return otherTransition.kind === 'DownCast' && transition.castedTypeName === otherTransition.castedType.name; default: return false; } } @@ -518,7 +523,12 @@ export function buildQueryGraph(name: string, schema: Schema): QueryGraph { return buildGraphInternal(name, schema, false); } -function buildGraphInternal(name: string, schema: Schema, addAdditionalAbstractTypeEdges: boolean, supergraphSchema?: Schema): QueryGraph { +function buildGraphInternal( + name: string, + schema: Schema, + addAdditionalAbstractTypeEdges: boolean, + supergraphSchema?: Schema +): QueryGraph { const builder = new GraphBuilderFromSchema( name, schema, @@ -527,6 +537,9 @@ function buildGraphInternal(name: string, schema: Schema, addAdditionalAbstractT for (const rootType of schema.schemaDefinition.roots()) { builder.addRecursivelyFromRoot(rootType.rootKind, rootType.type); } + if (builder.isFederatedSubgraph) { + builder.addInterfaceEntityEdges(); + } if (addAdditionalAbstractTypeEdges) { builder.addAdditionalAbstractTypeEdges(); } @@ -572,7 +585,7 @@ export function buildFederatedQueryGraph(supergraph: Schema, forQueryPlanning: b for (const subgraph of subgraphs) { graphs.push(buildGraphInternal(subgraph.name, subgraph.schema, forQueryPlanning, supergraph)); } - return federateSubgraphs(graphs); + return federateSubgraphs(supergraph, graphs); } function federatedProperties(subgraphs: QueryGraph[]) : [number, Set, Schema[]] { @@ -588,7 +601,15 @@ function federatedProperties(subgraphs: QueryGraph[]) : [number, Set, + type: NamedType +): Directive[] { + const applications: Directive[] = type.appliedDirectivesOf(keyDirective); + return applications.filter((application) => application.arguments().resolvable ?? true); +} + +function federateSubgraphs(supergraph: Schema, subgraphs: QueryGraph[]): QueryGraph { const [verticesCount, rootKinds, schemas] = federatedProperties(subgraphs); const builder = new GraphBuilder(verticesCount); rootKinds.forEach(k => builder.createRootVertex( @@ -636,11 +657,7 @@ function federateSubgraphs(subgraphs: QueryGraph[]): QueryGraph { subgraph, v => { const type = v.type; - for (const keyApplication of type.appliedDirectivesOf(keyDirective)) { - if (!(keyApplication.arguments().resolvable ?? true)) { - continue; - } - + for (const keyApplication of resolvableKeyApplications(keyDirective, type)) { // The @key directive creates an edge from every subgraphs (having that type) // to the current subgraph. In other words, the fact this subgraph has a @key means // that the service of the current subgraph can be queried for the entity (through @@ -653,6 +670,7 @@ function federateSubgraphs(subgraphs: QueryGraph[]): QueryGraph { // restriction, and this may be useful at least temporarily to allow convert a type to // an entity). assert(isInterfaceType(type) || isObjectType(type), () => `Invalid "@key" application on non Object || Interface type "${type}"`); + const isInterfaceObject = subgraphMetadata.isInterfaceObjectType(type); const conditions = parseFieldSetArgument({ parentType: type, directive: keyApplication }); // Note that each subgraph has a key edge to itself (when i === j below). We usually ignore // this edges, but they exists for the special case of @defer, where we technically may have @@ -662,16 +680,42 @@ function federateSubgraphs(subgraphs: QueryGraph[]): QueryGraph { if (otherVertices.length == 0) { continue; } - // Note that later, when we've handled @provides, this might not be true anymore a provide may create copy of a + // Note that later, when we've handled @provides, this might not be true anymore as @provides may create copy of a // certain type. But for now, it's true. assert( otherVertices.length == 1, () => `Subgraph ${j} should have a single vertex for type ${type.name} but got ${otherVertices.length}: ${inspect(otherVertices)}`); + const otherVertice = otherVertices[0]; // The edge goes from the otherSubgraph to the current one. - const head = copyPointers[j].copiedVertex(otherVertices[0]); + const head = copyPointers[j].copiedVertex(otherVertice); const tail = copyPointers[i].copiedVertex(v); builder.addEdge(head, tail, new KeyResolution(), conditions); + + // Additionally, if the key is on an @interfaceObject and this "other" subgraph has the type as + // a proper interface, then we need an edge from each of those implementation (to the @interfaceObject). + // This is used when an entity of specific implementation is queried first, but then some of the + // requested fields are only provided by that @interfaceObject. + const otherType = otherVertice.type; + if (isInterfaceObject && isInterfaceType(otherType)) { + for (const implemType of otherType.possibleRuntimeTypes()) { + // Note that we're only taking the implementation types from "otherSubgraph", so we're guaranteed + // to have a corresponding vertice (and only one for the same reason than mentioned in the assert above). + const implemVertice = otherSubgraph.verticesForType(implemType.name)[0]; + const implemHead = copyPointers[j].copiedVertex(implemVertice); + // The key goes from the implementation type to the @interfaceObject one, so the conditions + // will be "fetched" on the implementation type, but `conditions` has been parsed on the + // interface type, so it will use fields from the interface, not the implementation type. + // So we re-parse the condition using the implementation type: this could fail, but in + // that case it just mean that key is not usable. + try { + const implConditions = parseFieldSetArgument({ parentType: implemType, directive: keyApplication, validate: false }); + builder.addEdge(implemHead, tail, new KeyResolution(), implConditions); + } catch (e) { + // Ignored on purpose: it just means the key is not usable on this subgraph. + } + } + } } } }, @@ -729,6 +773,60 @@ function federateSubgraphs(subgraphs: QueryGraph[]): QueryGraph { } ); } + + // We now ned to finish handling @interfaceObject types. More precisely, there is cases where only a/some implementation(s) + // of a interface are queried, and that could apply to an interface that is an @interfaceObject in some sugraph. Consider + // the following example: + // ```graphql + // type Query { + // getIs: [I] + // } + // + // type I @key(fields: "id") @interfaceObject { + // id: ID! + // x: Int + // } + // ``` + // where we suppose that `I` has some implementations say, `A`, `B` and `C`, in some other subgraph. + // Now, consider query: + // ```graphql + // { + // getIs { + // ... on B { + // x + // } + // } + // } + // ``` + // So here, we query `x` which the subgraph provides, but we only do so for one of the impelementation. + // So in that case, we essentially need to figure out the `__typename` first (of more precisely, we need + // to know the real __typename "eventually"; we could theoretically query `x` first, and then get the __typename + // to know if we should keep the result or discard it, and that could be more efficient in certain case, + // but as we don't know both 1) if `x` is expansive to resolve and 2) what the ratio of results from `getIs` + // will be `B` versus some other implementation, it is "safer" to get the __typename first and only resolve `x` + // when we need to). + // + // Long story short, to solve this, we create edges from @interfaceObject types to themselves for every implementation + // types of the interface: those edges will be taken when we try to take a `... on B` condition, and those edge + // have __typename has a condition, forcing to find __typename in another subgraph first. + for (const [i, subgraph] of subgraphs.entries()) { + const subgraphSchema = schemas[i]; + const subgraphMetadata = federationMetadata(subgraphSchema); + assert(subgraphMetadata, `Subgraph ${i} is not a valid federation subgraph`); + const interfaceObjectDirective = subgraphMetadata.interfaceObjectDirective(); + for (const application of interfaceObjectDirective.applications()) { + const type = application.parent; + assert(isObjectType(type), '@interfaceObject should have been on an object type'); + const vertex = copyPointers[i].copiedVertex(subgraph.verticesForType(type.name)[0]); + const supergraphItf = supergraph.type(type.name); + assert(supergraphItf && isInterfaceType(supergraphItf), () => `${type} has @interfaceObject in subgraph but has kind ${supergraphItf?.kind} in supergraph`) + const condition = selectionSetOfElement(new Field(type.typenameField()!)); + for (const implementation of supergraphItf.possibleRuntimeTypes()) { + builder.addEdge(vertex, vertex, new InterfaceObjectFakeDownCast(type, implementation.name), condition); + } + } + } + return builder.build(FEDERATED_GRAPH_ROOT_SOURCE); } @@ -948,7 +1046,7 @@ class GraphBuilder { * schema API, but does not handle vertices and edges related to federation). */ class GraphBuilderFromSchema extends GraphBuilder { - private readonly isFederatedSubgraph: boolean; + readonly isFederatedSubgraph: boolean; constructor( private readonly name: string, @@ -960,9 +1058,9 @@ class GraphBuilderFromSchema extends GraphBuilder { assert(!this.isFederatedSubgraph || supergraph, `Missing supergraph schema for building the federated subgraph graph`); } - private hasDirective(field: FieldDefinition, directiveFct: (metadata: FederationMetadata) => DirectiveDefinition): boolean { + private hasDirective(elt: FieldDefinition | NamedType, directiveFct: (metadata: FederationMetadata) => DirectiveDefinition): boolean { const metadata = federationMetadata(this.schema); - return !!metadata && field.hasAppliedDirective(directiveFct(metadata)); + return !!metadata && elt.hasAppliedDirective(directiveFct(metadata)); } private isExternal(field: FieldDefinition): boolean { @@ -1018,12 +1116,18 @@ class GraphBuilderFromSchema extends GraphBuilder { } private addObjectTypeEdges(type: ObjectType, head: Vertex) { + const isInterfaceObject = federationMetadata(this.schema)?.isInterfaceObjectType(type) ?? false; + // We do want all fields, including most built-in. For instance, it's perfectly valid to query __typename manually, so we want // to have an edge for it. Also, the fact we handle the _entities field ensure that all entities are part of the graph, // even if they are not reachable by any other user operations. // We do skip introspection fields however. for (const field of type.allFields()) { - if (field.isSchemaIntrospectionField()) { + // Note that @interfaceObject types are an exception to the rule of "it's perfectly valid to query __typename". More + // precisely, a query can ask for the `__typename` of anything, but it shouldn't be answered by an @interfaceObject + // and so we don't add an edge, ensuring the query planner has to get it from another subgraph (than the one with + // said @interfaceObject). + if (field.isSchemaIntrospectionField() || (isInterfaceObject && field.name === typenameFieldName)) { continue; } @@ -1281,6 +1385,45 @@ class GraphBuilderFromSchema extends GraphBuilder { } } + /** + * In a subgraph, all entity object type will be "automatically" reachable (from the query root) because + * of the `_entities` operation. Indeed, it returns `_Entity`, which is a union of all entity object types, + * making those reachable. + * + * However, we also want entity interface types (interface with a @key) to be reachable in a similar way, + * because the `_entities` operation is also technically the one resolving them, and not having them + * reachable would break plenty of code that assume that by traversing a query graph from root, we get to + * everything that can be queried. + * + * But because graphQL unions cannot have interface types, they are not part of the `_Entity` union (and + * cannot be). This is ok as far as the typing of the schema does, because even when `_entities` is called + * to resolve an interface type, it technically returns a concrete object, and so, since every + * implementation of an entity interface is also an entity, this is captured by the `_Entity` union. + * + * But it does mean we want to manually add the corresponding edges now for interfaces, or @key on + * interfaces wouldn't work properly (at least whenthe interface is not otherwise reachable by a use operation + * in the subgraph). + */ + addInterfaceEntityEdges() { + const subgraphMetadata = federationMetadata(this.schema); + assert(subgraphMetadata, () => `${this.name} does not correspond to a subgraph`); + const entityType = subgraphMetadata.entityType(); + // We can ignore this case because if the subgraph has an interface with a @key, then we force its + // implementations to be marked as entity too and so we know that if `_Entity` is undefined, then + // we have no need for entity edges. + if (!entityType) { + return; + } + const entityTypeVertex = this.addTypeRecursively(entityType); + const keyDirective = subgraphMetadata.keyDirective(); + for (const itfType of this.schema.interfaceTypes()) { + if (resolvableKeyApplications(keyDirective, itfType).length > 0) { + const itfTypeVertex = this.addTypeRecursively(itfType); + this.addEdge(entityTypeVertex, itfTypeVertex, new DownCast(entityType, itfType)); + } + } + } + build(): QueryGraph { return super.build(this.name); } diff --git a/query-graphs-js/src/transition.ts b/query-graphs-js/src/transition.ts index 17c4cffcc..3b71e0b24 100644 --- a/query-graphs-js/src/transition.ts +++ b/query-graphs-js/src/transition.ts @@ -4,26 +4,50 @@ import { FieldDefinition, CompositeType, SchemaRootKind } from "@apollo/federati * The type of query graphs edge "transitions". * * An edge transition encodes what the edges correspond to, in the underlying graphQL - * schema. Edges may correspond to: - * - a field (`FieldCollection`): the edge goes from (a vertex for) the field parent type, to the - * field (base) type. - * - a "downcast" (`DownCast`): the edges goes from an abstract type (interface or union) to a type - * that implements that abstract type (for interfaces) or is a member of that abstract type (for - * unions). - * - a key (`KeyResolution`), only found in "federated" query graphs: the edge goes from an - * entity type in a particular subgraph to the same entity type but in another subgraph. Edge - * with key transition _must_ have `conditions` corresponding to the key fields. - * - a root type (`RootTypeResolution`), only found in "federated" query graphs: the edge goes from - * a root type (query, mutation or subscription) of a subgraph to the (same) root type of another - * subgraph. It encodes the fact that if a subgraph field returns a root type, any subgraph - * can be queried from there. - * - a "subgraph entering" edge: this is a special case only used for the edges out of the root - * vertices of "federated" query graphs. It does not correspond to any physical graphQL elements - * but can be understood as the fact that the gateway is always free to start querying any of - * the subgraph services as needed. - * + * schema. */ -export type Transition = FieldCollection | DownCast | KeyResolution | RootTypeResolution | SubgraphEnteringTransition; +export type Transition = + /** + * A field: the edge goes from (a vertex for) the field parent type, to the field (base) type. + */ + FieldCollection + + /** + * A "downcast": the edges goes from an abstract type (interface or union) to a type that implements that abstract + * type (for interfaces) or is a member of that abstract type (for unions) + */ + | DownCast + + /** + * A key, only found in "federated" query graphs: the edge goes from an entity type in a particular subgraph to the + * same entity type but in another subgraph. Edge with key transition _must_ have `conditions` corresponding to the + * key fields + */ + | KeyResolution + + /** + * A root type, only found in "federated" query graphs: the edge goes from a root type (query, mutation or subscription) + * of a subgraph to the (same) root type of another subgraph. It encodes the fact that if a subgraph field returns a root + * type, any subgraph can be queried from there. + */ + | RootTypeResolution + + /** + * A "subgraph entering" edge: this is a special case only used for the edges out of the root vertices of "federated" + * query graphs. It does not correspond to any physical graphQL elements but can be understood as the fact that the gateway + * is always free to start querying any of the subgraph services as needed. + */ + | SubgraphEnteringTransition + + /** + * A special "fake" downcast for an @interfaceObject type to an implementation, only found in "federated" query graphs: + * this encodes the fact that an @interfaceObject type "stand-in" for any possible implementations (in the supergraph) + * of the corresponding interface. It is "fake" because the corresponding edges stays on the @interfaceObject type (this + * is also why the "casted type" is only a name: that casted type does not actually exists in the subgraph in which + * the corresponding edge will be found). + */ + | InterfaceObjectFakeDownCast +; export class KeyResolution { readonly kind = 'KeyResolution' as const; @@ -80,5 +104,16 @@ export class SubgraphEnteringTransition { } } +export class InterfaceObjectFakeDownCast { + readonly kind = 'InterfaceObjectFakeDownCast' as const; + readonly collectOperationElements = true as const; + + constructor(readonly sourceType: CompositeType, readonly castedTypeName: string) {} + + toString() { + return '... on ' + this.castedTypeName; + } +} + export const subgraphEnteringTransition = new SubgraphEnteringTransition(); diff --git a/query-planner-js/src/QueryPlan.ts b/query-planner-js/src/QueryPlan.ts index 872bc96da..6dfc96ea3 100644 --- a/query-planner-js/src/QueryPlan.ts +++ b/query-planner-js/src/QueryPlan.ts @@ -39,6 +39,12 @@ export interface FetchNode { operationName: string | undefined; operationKind: OperationTypeNode; operationDocumentNode?: DocumentNode; + inputRewrites?: FetchDataRewrite[]; +} + +export interface FetchDataRewrite { + path: string[], + setValueTo?: any, } export interface FlattenNode { diff --git a/query-planner-js/src/__tests__/buildPlan.defer.test.ts b/query-planner-js/src/__tests__/buildPlan.defer.test.ts index 732d296d7..ba4839e00 100644 --- a/query-planner-js/src/__tests__/buildPlan.defer.test.ts +++ b/query-planner-js/src/__tests__/buildPlan.defer.test.ts @@ -1,7 +1,7 @@ import { operationFromDocument, Schema, ServiceDefinition } from '@apollo/federation-internals'; import gql from 'graphql-tag'; import { QueryPlanner } from '@apollo/query-planner'; -import { composeAndCreatePlanner, composeAndCreatePlannerWithOptions } from "./buildPlan.test"; +import { composeAndCreatePlanner, composeAndCreatePlannerWithOptions } from "./testHelper"; function composeAndCreatePlannerWithDefer(...services: ServiceDefinition[]): [Schema, QueryPlanner] { return composeAndCreatePlannerWithOptions(services, { incrementalDelivery: { enableDefer : true }}); diff --git a/query-planner-js/src/__tests__/buildPlan.interfaceObject.test.ts b/query-planner-js/src/__tests__/buildPlan.interfaceObject.test.ts new file mode 100644 index 000000000..62f59441e --- /dev/null +++ b/query-planner-js/src/__tests__/buildPlan.interfaceObject.test.ts @@ -0,0 +1,412 @@ +import { operationFromDocument } from '@apollo/federation-internals'; +import gql from 'graphql-tag'; +import { composeAndCreatePlanner, findFetchNodes } from "./testHelper"; + +describe('basic @key on interface/@interfaceObject handling', () => { + const subgraph1 = { + name: 'S1', + typeDefs: gql` + type Query { + iFromS1: I + } + + interface I @key(fields: "id") { + id: ID! + x: Int + } + + type A implements I @key(fields: "id") { + id: ID! + x: Int + z: Int + } + + type B implements I @key(fields: "id") { + id: ID! + x: Int + w: Int + } + ` + } + + const subgraph2 = { + name: 'S2', + typeDefs: gql` + type Query { + iFromS2: I + } + + type I @interfaceObject @key(fields: "id") { + id: ID! + y: Int + } + ` + } + + const [api, queryPlanner] = composeAndCreatePlanner(subgraph1, subgraph2); + + test('can use a @key on an @interfaceObject type', () => { + // Start by ensuring we can use the key on an @interfaceObject type + const operation = operationFromDocument(api, gql` + { + iFromS1 { + x + y + } + } + `); + + const plan = queryPlanner.buildQueryPlan(operation); + expect(plan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "S1") { + { + iFromS1 { + __typename + id + x + } + } + }, + Flatten(path: "iFromS1") { + Fetch(service: "S2") { + { + ... on I { + __typename + id + } + } => + { + ... on I { + y + } + } + }, + }, + }, + } + `); + }); + + test('can use a @key on an interface "from" an @interfaceObject type', () => { + const operation = operationFromDocument(api, gql` + { + iFromS2 { + x + y + } + } + `); + + const plan = queryPlanner.buildQueryPlan(operation); + expect(plan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "S2") { + { + iFromS2 { + __typename + id + y + } + } + }, + Flatten(path: "iFromS2") { + Fetch(service: "S1") { + { + ... on I { + __typename + id + } + } => + { + ... on I { + x + } + } + }, + }, + }, + } + `); + }); + + test('only uses an @interfaceObject if it can', () => { + const operation = operationFromDocument(api, gql` + { + iFromS2 { + y + } + } + `); + + const plan = queryPlanner.buildQueryPlan(operation); + expect(plan).toMatchInlineSnapshot(` + QueryPlan { + Fetch(service: "S2") { + { + iFromS2 { + y + } + } + }, + } + `); + }); + + test('does not rely on an @interfaceObject directly for `__typename`', () => { + const operation = operationFromDocument(api, gql` + { + iFromS2 { + __typename + y + } + } + `); + + const plan = queryPlanner.buildQueryPlan(operation); + expect(plan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "S2") { + { + iFromS2 { + __typename + id + y + } + } + }, + Flatten(path: "iFromS2") { + Fetch(service: "S1") { + { + ... on I { + __typename + id + } + } => + { + ... on I { + __typename + } + } + }, + }, + }, + } + `); + }); + + test('does not rely on an @interfaceObject directly if a specific implementation is requested', () => { + // Even though `y` is part of the interface and accessible from the 2nd subgraph, the + // fact that we "filter" a single implementation should act as if `__typename` was queried + // (effectively, the gateway/router need that `__typename` to decide if the returned data + // should be included or not. + const operation = operationFromDocument(api, gql` + { + iFromS2 { + ... on A { + y + } + } + } + `); + + const plan = queryPlanner.buildQueryPlan(operation); + expect(plan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "S2") { + { + iFromS2 { + __typename + id + } + } + }, + Flatten(path: "iFromS2") { + Fetch(service: "S1") { + { + ... on I { + __typename + id + } + } => + { + ... on I { + __typename + } + } + }, + }, + Flatten(path: "iFromS2") { + Fetch(service: "S2") { + { + ... on A { + __typename + id + } + } => + { + ... on I { + y + } + } + }, + }, + }, + } + `); + }); + + test('can use a @key on an @interfaceObject type even for a concrete implementation', () => { + const operation = operationFromDocument(api, gql` + { + iFromS1 { + ... on A { + y + } + } + } + `); + + const plan = queryPlanner.buildQueryPlan(operation); + expect(plan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "S1") { + { + iFromS1 { + __typename + ... on A { + __typename + id + } + } + } + }, + Flatten(path: "iFromS1") { + Fetch(service: "S2") { + { + ... on A { + __typename + id + } + } => + { + ... on I { + y + } + } + }, + }, + }, + } + `); + + const rewrites = findFetchNodes('S2', plan.node)[0].inputRewrites; + expect(rewrites).toBeDefined(); + expect(rewrites?.length).toBe(1); + const rewrite = rewrites![0]; + expect(rewrite.path).toEqual(['... on A', '__typename']); + expect(rewrite.setValueTo).toBe('I'); + }); +}); + +it('avoids buffering @interfaceObject results that may have to filtered with lists', () => { + const subgraph1 = { + name: 'S1', + typeDefs: gql` + type Query { + everything: [I] + } + + type I @interfaceObject @key(fields: "id") { + id: ID! + expansiveField: String + } + ` + } + + const subgraph2 = { + name: 'S2', + typeDefs: gql` + interface I @key(fields: "id") { + id: ID! + } + + type A implements I @key(fields: "id") { + id: ID! + a: Int + } + + type B implements I @key(fields: "id") { + id: ID! + b: Int + } + ` + } + + const [api, queryPlanner] = composeAndCreatePlanner(subgraph1, subgraph2); + + const operation = operationFromDocument(api, gql` + { + everything { + ... on A { + a + expansiveField + } + } + } + `); + + const plan = queryPlanner.buildQueryPlan(operation); + expect(plan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "S1") { + { + everything { + __typename + id + } + } + }, + Flatten(path: "everything.@") { + Fetch(service: "S2") { + { + ... on I { + __typename + id + } + } => + { + ... on I { + __typename + ... on A { + a + } + } + } + }, + }, + Flatten(path: "everything.@") { + Fetch(service: "S1") { + { + ... on A { + __typename + id + } + } => + { + ... on I { + expansiveField + } + } + }, + }, + }, + } + `); +}); diff --git a/query-planner-js/src/__tests__/buildPlan.test.ts b/query-planner-js/src/__tests__/buildPlan.test.ts index 2929679c7..e35f7f6b8 100644 --- a/query-planner-js/src/__tests__/buildPlan.test.ts +++ b/query-planner-js/src/__tests__/buildPlan.test.ts @@ -1,28 +1,10 @@ -import { astSerializer, queryPlanSerializer, QueryPlanner, QueryPlannerConfig } from '@apollo/query-planner'; -import { composeServices } from '@apollo/composition'; -import { asFed2SubgraphDocument, assert, buildSchema, operationFromDocument, Schema, ServiceDefinition } from '@apollo/federation-internals'; +import { QueryPlanner } from '@apollo/query-planner'; +import { assert, buildSchema, operationFromDocument, ServiceDefinition } from '@apollo/federation-internals'; import gql from 'graphql-tag'; import { MAX_COMPUTED_PLANS } from '../buildPlan'; import { FetchNode, FlattenNode, SequenceNode } from '../QueryPlan'; import { FieldNode, OperationDefinitionNode, parse } from 'graphql'; - -expect.addSnapshotSerializer(astSerializer); -expect.addSnapshotSerializer(queryPlanSerializer); - -export function composeAndCreatePlanner(...services: ServiceDefinition[]): [Schema, QueryPlanner] { - return composeAndCreatePlannerWithOptions(services, {}); -} - -export function composeAndCreatePlannerWithOptions(services: ServiceDefinition[], config: QueryPlannerConfig): [Schema, QueryPlanner] { - const compositionResults = composeServices( - services.map((s) => ({ ...s, typeDefs: asFed2SubgraphDocument(s.typeDefs) })) - ); - expect(compositionResults.errors).toBeUndefined(); - return [ - compositionResults.schema!.toAPISchema(), - new QueryPlanner(buildSchema(compositionResults.supergraphSdl!), config) - ]; -} +import { composeAndCreatePlanner, composeAndCreatePlannerWithOptions } from './testHelper'; describe('shareable root fields', () => { test('can use same root operation from multiple subgraphs in parallel', () => { diff --git a/query-planner-js/src/__tests__/testHelper.ts b/query-planner-js/src/__tests__/testHelper.ts new file mode 100644 index 000000000..d143366a8 --- /dev/null +++ b/query-planner-js/src/__tests__/testHelper.ts @@ -0,0 +1,45 @@ +import { astSerializer, FetchNode, PlanNode, queryPlanSerializer, QueryPlanner, QueryPlannerConfig } from '@apollo/query-planner'; +import { composeServices } from '@apollo/composition'; +import { asFed2SubgraphDocument, buildSchema, Schema, ServiceDefinition } from '@apollo/federation-internals'; + +expect.addSnapshotSerializer(astSerializer); +expect.addSnapshotSerializer(queryPlanSerializer); + +export function composeAndCreatePlanner(...services: ServiceDefinition[]): [Schema, QueryPlanner] { + return composeAndCreatePlannerWithOptions(services, {}); +} + +export function composeAndCreatePlannerWithOptions(services: ServiceDefinition[], config: QueryPlannerConfig): [Schema, QueryPlanner] { + const compositionResults = composeServices( + services.map((s) => ({ ...s, typeDefs: asFed2SubgraphDocument(s.typeDefs) })) + ); + expect(compositionResults.errors).toBeUndefined(); + return [ + compositionResults.schema!.toAPISchema(), + new QueryPlanner(buildSchema(compositionResults.supergraphSdl!), config) + ]; +} + +export function findFetchNodes(subgraphName: string, node: PlanNode | undefined): FetchNode[] { + if (!node) { + return []; + } + + switch (node.kind) { + case 'Fetch': + return node.serviceName === subgraphName ? [node] : []; + case 'Flatten': + return findFetchNodes(subgraphName, node.node); + case 'Defer': + return findFetchNodes(subgraphName, node.primary?.node).concat( + node.deferred.flatMap((d) => findFetchNodes(subgraphName, d.node)) + ); + case 'Sequence': + case 'Parallel': + return node.nodes.flatMap((n) => findFetchNodes(subgraphName, n)); + case 'Condition': + return findFetchNodes(subgraphName, node.ifClause).concat( + findFetchNodes(subgraphName, node.elseClause) + ); + } +} diff --git a/query-planner-js/src/buildPlan.ts b/query-planner-js/src/buildPlan.ts index 7b5e0bba9..b937fbe77 100644 --- a/query-planner-js/src/buildPlan.ts +++ b/query-planner-js/src/buildPlan.ts @@ -50,6 +50,11 @@ import { typenameFieldName, mapKeys, operationPathToStringPath, + mapValues, + runtimeTypesIntersects, + supertypes, + isInterfaceObjectType, + isInterfaceType, } from "@apollo/federation-internals"; import { advanceSimultaneousPathsWithOperation, @@ -81,10 +86,11 @@ import { terminateWithNonRequestedTypenameField, getLocallySatisfiableKey, createInitialOptions, + buildFederatedQueryGraph, } from "@apollo/query-graphs"; import { stripIgnoredCharacters, print, parse, OperationTypeNode } from "graphql"; -import { DeferredNode } from "."; -import { QueryPlannerConfig } from "./config"; +import { DeferredNode, FetchDataRewrite } from "."; +import { enforceQueryPlannerConfigDefaults, QueryPlannerConfig } from "./config"; import { QueryPlan, ResponsePath, SequenceNode, PlanNode, ParallelNode, FetchNode, trimSelectionNodes } from "./QueryPlan"; const debug = newDebugLogger('plan'); @@ -383,7 +389,7 @@ class QueryPlanningTaversal { private newDependencyGraph(): FetchDependencyGraph { const rootType = this.isTopLevel && this.hasDefers ? this.supergraphSchema.schemaDefinition.rootType(this.rootKind) : undefined; - return FetchDependencyGraph.create(this.subgraphs, this.startFetchIdGen, rootType); + return FetchDependencyGraph.create(this.supergraphSchema, this.subgraphs, this.startFetchIdGen, rootType); } // Moves the first closed branch to after any branch having more options. @@ -594,6 +600,10 @@ class LazySelectionSet { assert(_computed || _toCloneOnWrite, 'Should have one of the argument'); } + parentType(): CompositeType { + return this.forRead().parentType; + } + forRead(): SelectionSet { return this._computed ? this._computed : this._toCloneOnWrite!; } @@ -645,6 +655,11 @@ class FetchGroup { private _id: string | undefined; + // Set in some code-path to indicate that the selection of the group not be optimized away even if it "looks" useless. + mustPreserveSelection: boolean = false; + + private readonly inputRewrites: FetchDataRewrite[] = []; + private constructor( readonly dependencyGraph: FetchDependencyGraph, public index: number, @@ -653,31 +668,46 @@ class FetchGroup { readonly parentType: CompositeType, readonly isEntityFetch: boolean, private readonly _selection: LazySelectionSet, - private readonly _inputs?: LazySelectionSet, + private _inputs?: LazySelectionSet, readonly mergeAt?: ResponsePath, readonly deferRef?: string, ) { } - static create( + static create({ + dependencyGraph, + index, + subgraphName, + rootKind, + parentType, + inputsParentType, + mergeAt, + deferRef, + }: { dependencyGraph: FetchDependencyGraph, index: number, subgraphName: string, rootKind: SchemaRootKind, parentType: CompositeType, - isEntityFetch: boolean, + inputsParentType?: CompositeType, mergeAt?: ResponsePath, deferRef?: string, - ): FetchGroup { + }): FetchGroup { + // Sanity checks about the types provided: + // - the selection parent type must belong to the schema of the subgraph we're querying. + // - the inputs parent type must belong to the supergrah schema (inputs essentially comes from "the current in-memory result set + // maintained by the gateway/router", so it's technically not any specific subgraph, but rather the supergraph). + assert(parentType.schema() === dependencyGraph.subgraphSchemas.get(subgraphName), `Expected parent type ${parentType} to belong to ${subgraphName}`); + assert(!inputsParentType || inputsParentType.schema() === dependencyGraph.supergraphSchema, `Expected inputs parent type ${inputsParentType} to belong to the supergraph schema`); return new FetchGroup( dependencyGraph, index, subgraphName, rootKind, parentType, - isEntityFetch, + !!inputsParentType, new LazySelectionSet(new SelectionSet(parentType)), - isEntityFetch ? new LazySelectionSet(new SelectionSet(parentType)) : undefined, + inputsParentType ? new LazySelectionSet(new SelectionSet(inputsParentType)) : undefined, mergeAt, deferRef, ); @@ -721,10 +751,6 @@ class FetchGroup { return this._inputs?.forRead(); } - clonedInputs(): LazySelectionSet | undefined { - return this._inputs?.clone(); - } - addParents(parents: readonly ParentRelation[]) { for (const parent of parents) { this.addParent(parent); @@ -743,7 +769,7 @@ class FetchGroup { } assert(!parent.group.isParentOf(this), () => `Group ${parent.group} is a parent of ${this}, but the child relationship is broken`); - assert(!parent.group.isChildOf(this), () => `Group ${parent.group} is a child of ${this}: adding is as parent would create a cycle`); + assert(!parent.group.isChildOf(this), () => `Group ${parent.group} is a child of ${this}: adding it as parent would create a cycle`); this.dependencyGraph.onModification(); this._parents.push(parent); @@ -768,6 +794,18 @@ class FetchGroup { return !!this.parentRelation(maybeParent); } + isDescendantOf(maybeAncestor: FetchGroup): boolean { + const children = Array.from(maybeAncestor.children()); + while (children.length > 0) { + const child = children.pop()!; + if (child === this) { + return true; + } + child.children().forEach((c) => children.push(c)); + } + return false; + } + /** * Returns whether this group is both a child of `maybeParent` but also if we can show that the * dependency between the group is "artificial" in the sense that this group inputs do not truly @@ -818,13 +856,63 @@ class FetchGroup { return this._children; } - addInputs(selection: Selection | SelectionSet) { + addInputs(selection: Selection | SelectionSet, rewrites?: FetchDataRewrite[]) { assert(this._inputs, "Shouldn't try to add inputs to a root fetch group"); + // There is subtlety here, that is due to the fact that we sometime want to merge groups that + // are at the same mergeAt but that may currently have "incompatible" input parent types. In + // that case, we still rely on the fact that they have a common "super type" (which they must + // have if they are at the same mergeAt), but this may mean changing `this._inputs` in this + // case. + const thisParentType = this._inputs.parentType(); + const schema = thisParentType.schema(); + const selectionParentType = selection.parentType; + // Note that we check reference inequality first just to avoid the most costly 2nd test in the common case where the parent types are + // already the same (we also use name comparison because if they are from different schema, we're still ok if the name matches) + if (thisParentType.name !== selectionParentType.name && !runtimeTypesIntersects(thisParentType, selectionParentType)) { + assert(this.isEntityFetch, `Cannot add ${selection} of parent type ${selectionParentType} to ${this._inputs} of parent type ${thisParentType}: no common runtime intersections and not an entity fetch`) + // Because it is an entity fetch, we know the top-level selections must be fragments that selects a specific object type. + const extractSelectedTypeName = (s: Selection) => { + assert(s.kind === 'FragmentSelection', () => `Expected ${s} to be a fragment when adding inputs ${selection} to ${this}`); + return s.element().castedType().name; + } + const typesToAccountFor = new Set( + this._inputs.forRead().selections().map(extractSelectedTypeName).concat( + selection instanceof SelectionSet + ? selection.selections().map(extractSelectedTypeName) + : [extractSelectedTypeName(selection)] + ) + ); + + const allSupertypes: (readonly CompositeType[])[] = Array.from(typesToAccountFor).map((name) => supertypes(schema.type(name) as CompositeType)); + const first = allSupertypes[0]; + const rest = allSupertypes.slice(1); + const commonType = first.find((t) => rest.every((st) => st.includes(t))); + assert(commonType, () => `Cannot add ${selection} of parent type ${selectionParentType} to ${this._inputs} of parent type ${thisParentType}: non common supertype`) + + // Note that while there should be a common super type, there can be more than one, and it's a bit hard to find which one truly + // correspond to the group `mergeAt`. We pick the first one, because it works, but this is why this method actually look the + // fragments within the selections: this ensure that if we try to merge the inputs with some other choice later, and we've make + // the "wrong" choice here, it'll still work (because we won't look at the choice of parent made here, but rather the underlying + // object types). + const newInputs = new SelectionSet(commonType); + newInputs.mergeIn(this._inputs.forRead()); + this._inputs = new LazySelectionSet(newInputs); + } + if (selection instanceof SelectionSet) { this._inputs.forWrite().mergeIn(selection); } else { this._inputs.forWrite().add(selection); } + if (rewrites) { + rewrites.forEach((r) => this.inputRewrites.push(r)); + } + } + + copyInputsOf(other: FetchGroup, clone: boolean = false) { + if (other.inputs) { + this.addInputs(clone ? other.inputs.clone() : other.inputs, other.inputRewrites); + } } addSelection(path: OperationPath, onPathEnd?: (finalSelectionSet: SelectionSet | undefined) => void) { @@ -858,7 +946,7 @@ class FetchGroup { } canMergeSiblingIn(sibling: FetchGroup): boolean { - // We only allow merging sibling on the same subgraph, same "mergeAt" and when our common parent is our only parent: + // We only allow merging sibling on the same subgraph, same "mergeAt" and when the common parent is their only parent: // - there is no reason merging siblings of different subgraphs could ever make sense. // - ensuring the same "mergeAt" makes so we can merge the inputs and selections without having to worry about those // not being at the same level (hence the empty path in the call to `mergeInInternal` below). In theory, we could @@ -885,9 +973,7 @@ class FetchGroup { * their _only_ parent. Further `this` and `sibling` must be on the same subgraph and have the same `mergeAt`. */ mergeSiblingIn(sibling: FetchGroup) { - if (sibling.inputs) { - this.addInputs(sibling.inputs); - } + this.copyInputsOf(sibling); this.mergeInInternal(sibling, []); } @@ -935,10 +1021,9 @@ class FetchGroup { assert(this.deferRef === other.deferRef, () => `Can only merge unrelated groups within the same @defer block: cannot merge ${this} and ${other}`); assert(this.subgraphName === other.subgraphName, () => `Can only merge unrelated groups to the same subraphs: cannot merge ${this} and ${other}`); assert(sameMergeAt(this.mergeAt, other.mergeAt), () => `Can only merge unrelated groups at the same "mergeAt": ${this} has mergeAt=${this.mergeAt}, but ${other} has mergeAt=${other.mergeAt}`); + assert(this.inputs?.parentType === other.inputs?.parentType, () => `Can only merge unrelated groups with the same input parent type: ${this} has input parent=${this.inputs?.parentType}, but ${other} has input parent=${other.inputs?.parentType}`); - if (other.inputs) { - this.addInputs(other.inputs); - } + this.copyInputsOf(other); this.mergeInInternal(other, [], true); } @@ -969,6 +1054,10 @@ class FetchGroup { if (mergeParentDependencies) { this.relocateParentsOnMergedIn(merged); } + + if (merged.mustPreserveSelection) { + this.mustPreserveSelection = true; + } this.dependencyGraph.remove(merged); } @@ -1003,9 +1092,18 @@ class FetchGroup { private relocateParentsOnMergedIn(merged: FetchGroup) { for (const parent of merged.parents()) { + // If the parent of the merged is already a parent of ours, don't re-create the already existing relationship. if (parent.group.isParentOf(this)) { continue; } + + // Further, if the parent is a descendant of `this`, we also should ignore that relationship, becuase + // adding it a parent of `this` would create a cycle. And assuming this method is called properly, + // that when `merged` can genuinely be safely merged into `this`, then this just mean the `parent` -> `merged` + // relationship was unecessary after all (which can happen given how groups are generated). + if (parent.group.isDescendantOf(this)) { + continue; + } this.addParent(parent); } } @@ -1057,6 +1155,7 @@ class FetchGroup { operationKind: schemaRootKindToOperationKind(operation.rootKind), operationName: operation.name, operationDocumentNode: queryPlannerConfig.exposeDocumentNodeInFetchNode ? operationDocument : undefined, + inputRewrites: this.inputRewrites.length === 0 ? undefined : this.inputRewrites, }; return this.isTopLevel @@ -1302,6 +1401,7 @@ class FetchDependencyGraph { private fetchIdGen: number; private constructor( + readonly supergraphSchema: Schema, readonly subgraphSchemas: ReadonlyMap, readonly federatedQueryGraph: QueryGraph, readonly startingIdGen: number, @@ -1312,8 +1412,9 @@ class FetchDependencyGraph { this.fetchIdGen = startingIdGen; } - static create(federatedQueryGraph: QueryGraph, startingIdGen: number, rootTypeForDefer: CompositeType | undefined) { + static create(supergraphSchema: Schema, federatedQueryGraph: QueryGraph, startingIdGen: number, rootTypeForDefer: CompositeType | undefined) { return new FetchDependencyGraph( + supergraphSchema, federatedQueryGraph.sources, federatedQueryGraph, startingIdGen, @@ -1337,6 +1438,7 @@ class FetchDependencyGraph { clone(): FetchDependencyGraph { const cloned = new FetchDependencyGraph( + this.supergraphSchema, this.subgraphSchemas, this.federatedQueryGraph, this.startingIdGen, @@ -1402,7 +1504,7 @@ class FetchDependencyGraph { rootKind: SchemaRootKind, parentType: CompositeType, }): FetchGroup { - const group = this.newFetchGroup({ subgraphName, parentType, isEntityFetch: false, rootKind }); + const group = this.newFetchGroup({ subgraphName, parentType, rootKind }); this.rootGroups.set(subgraphName, group); return group; } @@ -1410,41 +1512,43 @@ class FetchDependencyGraph { private newFetchGroup({ subgraphName, parentType, - isEntityFetch, + inputsParentType, rootKind, // always "query" for entity fetches mergeAt, deferRef, }: { subgraphName: string, parentType: CompositeType, - isEntityFetch: boolean, + inputsParentType?: CompositeType, rootKind: SchemaRootKind, mergeAt?: ResponsePath, deferRef?: string, }): FetchGroup { this.onModification(); - const newGroup = FetchGroup.create( - this, - this.groups.length, + const newGroup = FetchGroup.create({ + dependencyGraph: this, + index: this.groups.length, subgraphName, rootKind, parentType, - isEntityFetch, + inputsParentType, mergeAt, deferRef, - ); + }); this.groups.push(newGroup); return newGroup; } getOrCreateKeyFetchGroup({ subgraphName, + inputsTypeName, mergeAt, parent, conditionsGroups, deferRef, }: { subgraphName: string, + inputsTypeName: string, mergeAt: ResponsePath, parent: ParentRelation, conditionsGroups: FetchGroup[], @@ -1458,6 +1562,7 @@ class FetchDependencyGraph { if (existing.subgraphName === subgraphName && existing.mergeAt && sameMergeAt(existing.mergeAt, mergeAt) + && inputsTypeName === existing.inputs?.parentType?.name && !this.isInGroupsOrTheirAncestors(existing, conditionsGroups) && existing.deferRef === deferRef ) { @@ -1473,7 +1578,12 @@ class FetchDependencyGraph { return existing; } } - const newGroup = this.newKeyFetchGroup({ subgraphName, mergeAt, deferRef }); + const newGroup = this.newKeyFetchGroup({ + subgraphName, + inputsTypeName, + mergeAt, + deferRef + }); newGroup.addParent(parent); return newGroup } @@ -1497,7 +1607,13 @@ class FetchDependencyGraph { mergeAt: ResponsePath, deferRef?: string, }): FetchGroup { - return this.newFetchGroup({ subgraphName, parentType, isEntityFetch: false, rootKind, mergeAt, deferRef }); + return this.newFetchGroup({ + subgraphName, + parentType, + rootKind, + mergeAt, + deferRef + }); } // Returns true if `toCheck` is either part of `conditions`, or is one of their ancestors (potentially recursively). @@ -1513,18 +1629,34 @@ class FetchDependencyGraph { return false; } + typeForFetchInputs(name: string): CompositeType { + const type = this.supergraphSchema.type(name); + assert(type, `Type ${name} should exist in the supergraph`) + assert(isCompositeType(type), `Type ${type} should be a composite, but got ${type.kind}`); + return type; + } + newKeyFetchGroup({ subgraphName, + inputsTypeName, mergeAt, deferRef, }: { subgraphName: string, + inputsTypeName: string, mergeAt: ResponsePath, deferRef?: string, }): FetchGroup { const parentType = this.federationMetadata(subgraphName).entityType(); - assert(parentType, () => `Subgraph ${subgraphName} has not entities defined`); - return this.newFetchGroup({ subgraphName, parentType, isEntityFetch: true, rootKind: 'query', mergeAt, deferRef }); + assert(parentType, () => `Subgraph ${subgraphName} has no entities defined`); + return this.newFetchGroup({ + subgraphName, + parentType, + inputsParentType: this.typeForFetchInputs(inputsTypeName), + rootKind: 'query', + mergeAt, + deferRef + }); } remove(toRemove: FetchGroup) { @@ -1639,7 +1771,7 @@ class FetchDependencyGraph { // If a group is such that everything is fetches is already included in the inputs, then // this group does useless fetches and can be removed. - if (group.inputs && group.inputs.contains(group.selection)) { + if (group.inputs && !group.mustPreserveSelection && group.inputs.contains(group.selection)) { // In general, removing a group is a bit tricky because we need to deal with the fact // that the group can have multiple parents and children and no break the "path in parent" // in all those cases. To keep thing relatively easily, we only handle the following @@ -2129,238 +2261,272 @@ interface FetchGroupProcessor { reduceDefer(main: TProcessed, subSelection: SelectionSet, deferredBlocks: TDeferred[]): TProcessed, } +export type PlanningStatistics = { + evaluatedPlanCount: number, +} -/** - * Modify the provided selection set to optimize the handling of __typename selection for query planning. - * - * Explicit querying of __typename can create some inefficiency for the query planning process if not - * handled specially. More precisely, query planning performance is directly proportional to how many possible - * plans a query has, since it compute all those options to compare them. Further, the number of possible - * plans double for every field for which there is a choice, so miminizing the number of field for which we - * have choices is paramount. - * - * And for a given type, __typename can always be provided by any subgraph having that type (it works as a - * kind of "always @shareable" field), so it often creates theoretical choices. In practice it doesn't - * matter which subgraph we use for __typename: we're happy to use whichever subgraph we're using for - * the "other" fields queried for the type. But the default query planning algorithm does not know how - * to do that. - * - * Let's note that this isn't an issue in most cases, because the query planning algorithm knows not to - * consider "obviously" inefficient paths. Typically, querying the __typename of an entity is generally - * ok because when looking at a path, the query planning algorithm always favor getting a field "locally" - * if it can (which it always can for __typename) and ignore alternative that would jump subgraphs. - * - * But this can still be a performance issue when a __typename is queried after a @shareable field: in - * that case, the algorithm would consider getting the __typename from each version of the @shareable - * field and this would add to the options to consider. But as, again, __typename can always be fetched - * from any subgraph, it's more efficient to ignore those options and simply get __typename from whichever - * subgraph we get any other of the other field requested (on the type on which we request __typename). - * - * It is unclear how to do this cleanly with the current planning algorithm however, so this method - * implements an alternative: to avoid the query planning spending time of exploring options for - * __typename, we "remove" the __typename selections from the operation. But of course, we still - * need to ensure that __typename is effectively queried, so as we do that removal, we also "tag" - * one of the "sibling" selection (using `addAttachement`) to remember that __typename needs to - * 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 - * 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. - */ -function optimizeSiblingTypenames(selectionSet: SelectionSet): SelectionSet { - const selections = selectionSet.selections(); - let updatedSelections: Selection[] | undefined = undefined; - let typenameSelection: Selection | undefined = undefined; - // We remember the first non-__typename field selection found. This is the one we'll "tag" if we do find a __typename - // occurrence that we want to remove. We only use for _field_ selections because at the stage where this is applied, - // we cannot be sure the selection set is "minimized" and so some of the inline fragments may end up being eliminated - // (for instance, the fragment condition could be "less precise" than the parent type, in which case query planning - // will ignore it) and tagging those could lose the tagging. - let firstFieldSelection: FieldSelection | undefined = undefined; - for (let i = 0; i < selections.length; i++) { - const selection = selections[i]; - let updated: Selection | undefined; - if (!typenameSelection && selection.kind === 'FieldSelection' && selection.field.name === typenameFieldName) { - // 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_ - // 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. - updated = undefined; - typenameSelection = selection; - } else { - const updatedSubSelection = selection.selectionSet ? optimizeSiblingTypenames(selection.selectionSet) : undefined; - if (updatedSubSelection === selection.selectionSet) { - updated = selection; - } else { - updated = selection.withUpdatedSubSelection(updatedSubSelection); - } - if (!firstFieldSelection && updated.kind === 'FieldSelection') { - firstFieldSelection = updated; - } +export class QueryPlanner { + private readonly config: Concrete; + private readonly federatedQueryGraph: QueryGraph; + private _lastGeneratedPlanStatistics: PlanningStatistics | undefined; + + // A set of the names of interface types for which at least one subgraph use an @interfaceObject to abstract + // that interface. + private readonly interfaceTypesWithInterfaceObjects = new Set(); + + constructor( + public readonly supergraphSchema: Schema, + config?: QueryPlannerConfig + ) { + this.config = enforceQueryPlannerConfigDefaults(config); + this.federatedQueryGraph = buildFederatedQueryGraph(supergraphSchema, true); + this.collectInterfaceTypesWithInterfaceObjects(); + } + + private collectInterfaceTypesWithInterfaceObjects() { + const isInterfaceObject = (name: string, schema: Schema) => { + const typeInSchema = schema.type(name); + return !!typeInSchema && isInterfaceObjectType(typeInSchema); } - // As soon as we find a selection that is discarded or modified, we need to create new selection set so we - // first copy everything up to this selection. - if (updated !== selection && !updatedSelections) { - updatedSelections = []; - for (let j = 0; j < i; j++) { - updatedSelections.push(selections[j]); + for (const itfType of this.supergraphSchema.interfaceTypes()) { + if (mapValues(this.federatedQueryGraph.sources).some((s) => isInterfaceObject(itfType.name, s))) { + this.interfaceTypesWithInterfaceObjects.add(itfType.name); } } - // Record the (potentially updated) selection if we're creating a new selection set, and said selection is not discarded. - if (updatedSelections && !!updated) { - updatedSelections.push(updated); - } } - if (!updatedSelections || updatedSelections.length === 0) { - // No selection was modified at all, or there is no other field selection than __typename one. - // In both case, we just return the current selectionSet unmodified. - return selectionSet; - } + buildQueryPlan(operation: Operation): QueryPlan { + if (operation.selectionSet.isEmpty()) { + return { kind: 'QueryPlan' }; + } - // If we have some __typename selection that was removed but need to be "remembered" for later, - // "tag" whichever first field selection is still part of the operation. - if (typenameSelection) { - if (firstFieldSelection) { - // Note that as we tag the element, we also record the alias used if any since that needs to be preserved. - firstFieldSelection.element().addAttachement(SIBLING_TYPENAME_KEY, typenameSelection.field.alias ? typenameSelection.field.alias : ''); + if (operation.rootKind === 'subscription') { + throw ERRORS.UNSUPPORTED_FEATURE.err( + 'Query planning does not currently support subscriptions.', + { nodes: [parse(operation.toString())] }, + ); + } + + const statistics: PlanningStatistics = { + evaluatedPlanCount: 0, + }; + this._lastGeneratedPlanStatistics = statistics; + + const reuseQueryFragments = this.config.reuseQueryFragments ?? true; + let fragments = operation.selectionSet.fragments + if (fragments && reuseQueryFragments) { + // For all subgraph fetches we query `__typename` on every abstract types (see `FetchGroup.toPlanNode`) so if we want + // to have a chance to reuse fragments, we should make sure those fragments also query `__typename` for every abstract type. + fragments = addTypenameFieldForAbstractTypesInNamedFragments(fragments) } else { - // If we have no other field selection, then we can't optimize __typename and we need to add - // it back to the updated subselections (we add it first because that's usually where we - // put __typename by convention). - updatedSelections = [typenameSelection as Selection].concat(updatedSelections); + fragments = undefined; } - } - return new SelectionSet(selectionSet.parentType, selectionSet.fragments).addAll(updatedSelections) -} -/** - * Applies `optimizeSiblingTypenames` to the provided operation selection set. - */ -function withSiblingTypenameOptimizedAway(operation: Operation): Operation { - const updatedSelectionSet = optimizeSiblingTypenames(operation.selectionSet); - if (updatedSelectionSet === operation.selectionSet) { - return operation; - } - return new Operation( - operation.rootKind, - updatedSelectionSet, - operation.variableDefinitions, - operation.name - ); -} + // We expand all fragments. This might merge a number of common branches and save us some work, and we're + // going to expand everything during the algorithm anyway. We'll re-optimize subgraph fetches with fragments + // later if possible (which is why we saved them above before expansion). + operation = operation.expandAllFragments(); + operation = withoutIntrospection(operation); + operation = this.withSiblingTypenameOptimizedAway(operation); -export type PlanningStatistics = { - evaluatedPlanCount: number, -} + let assignedDeferLabels: Set | undefined = undefined; + let hasDefers: boolean = false; + let deferConditions: SetMultiMap | undefined = undefined; + if (this.config.incrementalDelivery.enableDefer) { + ({ operation, hasDefers, assignedDeferLabels, deferConditions } = operation.withNormalizedDefer()); + } 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 + // to end up passing through a @defer to a subgraph by mistake). + operation = operation.withoutDefer(); + } -export function computeQueryPlan({ - config, - supergraphSchema, - federatedQueryGraph, - operation, -}: { - config: Concrete, - supergraphSchema: Schema, - federatedQueryGraph: QueryGraph, - operation: Operation, -}): { - plan: QueryPlan, - statistics: PlanningStatistics, -} { - if (operation.rootKind === 'subscription') { - throw ERRORS.UNSUPPORTED_FEATURE.err( - 'Query planning does not currently support subscriptions.', - { nodes: [parse(operation.toString())] }, - ); - } + debug.group(() => `Computing plan for\n${operation}`); + if (operation.selectionSet.isEmpty()) { + debug.groupEnd('Empty plan'); + return { kind: 'QueryPlan' }; + } - const statistics: PlanningStatistics = { - evaluatedPlanCount: 0, - }; + const root = this.federatedQueryGraph.root(operation.rootKind); + assert(root, () => `Shouldn't have a ${operation.rootKind} operation if the subgraphs don't have a ${operation.rootKind} root`); + const processor = fetchGroupToPlanProcessor({ + config: this.config, + variableDefinitions: operation.variableDefinitions, + fragments, + operationName: operation.name, + assignedDeferLabels, + }); - const reuseQueryFragments = config.reuseQueryFragments ?? true; - let fragments = operation.selectionSet.fragments - if (fragments && reuseQueryFragments) { - // For all subgraph fetches we query `__typename` on every abstract types (see `FetchGroup.toPlanNode`) so if we want - // to have a chance to reuse fragments, we should make sure those fragments also query `__typename` for every abstract type. - fragments = addTypenameFieldForAbstractTypesInNamedFragments(fragments) - } else { - fragments = undefined; - } - // We expand all fragments. This might merge a number of common branches and save us some work, and we're - // going to expand everything during the algorithm anyway. We'll re-optimize subgraph fetches with fragments - // later if possible (which is why we saved them above before expansion). - operation = operation.expandAllFragments(); - operation = withoutIntrospection(operation); - operation = withSiblingTypenameOptimizedAway(operation); + let rootNode: PlanNode | undefined; + if (deferConditions && deferConditions.size > 0) { + assert(hasDefers, 'Should not have defer conditions without @defer'); + rootNode = computePlanForDeferConditionals({ + supergraphSchema: this.supergraphSchema, + federatedQueryGraph: this.federatedQueryGraph, + operation, + processor, + root, + deferConditions, + statistics, + }) + } else { + rootNode = computePlanInternal({ + supergraphSchema: this.supergraphSchema, + federatedQueryGraph: this.federatedQueryGraph, + operation, + processor, + root, + hasDefers, + statistics, + }); + } - let assignedDeferLabels: Set | undefined = undefined; - let hasDefers: boolean = false; - let deferConditions: SetMultiMap | undefined = undefined; - if (config.incrementalDelivery.enableDefer) { - ({ operation, hasDefers, assignedDeferLabels, deferConditions } = operation.withNormalizedDefer()); - } 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 - // to end up passing through a @defer to a subgraph by mistake). - operation = operation.withoutDefer(); - } + debug.groupEnd('Query plan computed'); - debug.group(() => `Computing plan for\n${operation}`); - if (operation.selectionSet.isEmpty()) { - debug.groupEnd('Empty plan'); - return { - plan: { kind: 'QueryPlan' }, - statistics, - }; + return{ kind: 'QueryPlan', node: rootNode }; } - const root = federatedQueryGraph.root(operation.rootKind); - assert(root, () => `Shouldn't have a ${operation.rootKind} operation if the subgraphs don't have a ${operation.rootKind} root`); - const processor = fetchGroupToPlanProcessor({ - config, - variableDefinitions: operation.variableDefinitions, - fragments, - operationName: operation.name, - assignedDeferLabels, - }); + /** + * Modifies the provided selection set to optimize the handling of __typename selection for query planning. + * + * Explicit querying of __typename can create some inefficiency for the query planning process if not + * handled specially. More precisely, query planning performance is directly proportional to how many possible + * plans a query has, since it compute all those options to compare them. Further, the number of possible + * plans double for every field for which there is a choice, so miminizing the number of field for which we + * have choices is paramount. + * + * And for a given type, __typename can always be provided by any subgraph having that type (it works as a + * kind of "always @shareable" field), so it often creates theoretical choices. In practice it doesn't + * matter which subgraph we use for __typename: we're happy to use whichever subgraph we're using for + * the "other" fields queried for the type. But the default query planning algorithm does not know how + * to do that. + * + * Let's note that this isn't an issue in most cases, because the query planning algorithm knows not to + * consider "obviously" inefficient paths. Typically, querying the __typename of an entity is generally + * ok because when looking at a path, the query planning algorithm always favor getting a field "locally" + * if it can (which it always can for __typename) and ignore alternative that would jump subgraphs. + * + * But this can still be a performance issue when a __typename is queried after a @shareable field: in + * that case, the algorithm would consider getting the __typename from each version of the @shareable + * field and this would add to the options to consider. But as, again, __typename can always be fetched + * from any subgraph, it's more efficient to ignore those options and simply get __typename from whichever + * subgraph we get any other of the other field requested (on the type on which we request __typename). + * + * It is unclear how to do this cleanly with the current planning algorithm however, so this method + * implements an alternative: to avoid the query planning spending time of exploring options for + * __typename, we "remove" the __typename selections from the operation. But of course, we still + * need to ensure that __typename is effectively queried, so as we do that removal, we also "tag" + * one of the "sibling" selection (using `addAttachement`) to remember that __typename needs to + * 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 + * 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. + */ + private optimizeSiblingTypenames(selectionSet: SelectionSet): SelectionSet { + const selections = selectionSet.selections(); + const parentType = selectionSet.parentType; + const parentMaybeInterfaceObject = this.interfaceTypesWithInterfaceObjects.has(parentType.name); + let updatedSelections: Selection[] | undefined = undefined; + let typenameSelection: Selection | undefined = undefined; + // We remember the first non-__typename field selection found. This is the one we'll "tag" if we do find a __typename + // occurrence that we want to remove. We only use for _field_ selections because at the stage where this is applied, + // we cannot be sure the selection set is "minimized" and so some of the inline fragments may end up being eliminated + // (for instance, the fragment condition could be "less precise" than the parent type, in which case query planning + // will ignore it) and tagging those could lose the tagging. + let firstFieldSelection: FieldSelection | undefined = undefined; + for (let i = 0; i < selections.length; i++) { + const selection = selections[i]; + let updated: Selection | undefined; + if ( + !typenameSelection + && selection.kind === 'FieldSelection' + && selection.field.name === typenameFieldName + && !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_ + // 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 + // 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. + updated = undefined; + typenameSelection = selection; + } else { + const updatedSubSelection = selection.selectionSet ? this.optimizeSiblingTypenames(selection.selectionSet) : undefined; + if (updatedSubSelection === selection.selectionSet) { + updated = selection; + } else { + updated = selection.withUpdatedSubSelection(updatedSubSelection); + } + if (!firstFieldSelection && updated.kind === 'FieldSelection') { + firstFieldSelection = updated; + } + } + // As soon as we find a selection that is discarded or modified, we need to create new selection set so we + // first copy everything up to this selection. + if (updated !== selection && !updatedSelections) { + updatedSelections = []; + for (let j = 0; j < i; j++) { + updatedSelections.push(selections[j]); + } + } + // Record the (potentially updated) selection if we're creating a new selection set, and said selection is not discarded. + if (updatedSelections && !!updated) { + updatedSelections.push(updated); + } + } - let rootNode: PlanNode | undefined; - if (deferConditions && deferConditions.size > 0) { - assert(hasDefers, 'Should not have defer conditions without @defer'); - rootNode = computePlanForDeferConditionals({ - supergraphSchema, - federatedQueryGraph, - operation, - processor, - root, - deferConditions, - statistics, - }) - } else { - rootNode = computePlanInternal({ - supergraphSchema, - federatedQueryGraph, - operation, - processor, - root, - hasDefers, - statistics, - }); + if (!updatedSelections || updatedSelections.length === 0) { + // No selection was modified at all, or there is no other field selection than __typename one. + // In both case, we just return the current selectionSet unmodified. + return selectionSet; + } + + // If we have some __typename selection that was removed but need to be "remembered" for later, + // "tag" whichever first field selection is still part of the operation. + if (typenameSelection) { + if (firstFieldSelection) { + // Note that as we tag the element, we also record the alias used if any since that needs to be preserved. + firstFieldSelection.element().addAttachement(SIBLING_TYPENAME_KEY, typenameSelection.field.alias ? typenameSelection.field.alias : ''); + } else { + // If we have no other field selection, then we can't optimize __typename and we need to add + // it back to the updated subselections (we add it first because that's usually where we + // put __typename by convention). + updatedSelections = [typenameSelection as Selection].concat(updatedSelections); + } + } + return new SelectionSet(selectionSet.parentType, selectionSet.fragments).addAll(updatedSelections) } - debug.groupEnd('Query plan computed'); - return { - plan: { kind: 'QueryPlan', node: rootNode }, - statistics, - }; + /** + * Applies `optimizeSiblingTypenames` to the provided operation selection set. + */ + private withSiblingTypenameOptimizedAway(operation: Operation): Operation { + const updatedSelectionSet = this.optimizeSiblingTypenames(operation.selectionSet); + if (updatedSelectionSet === operation.selectionSet) { + return operation; + } + return new Operation( + operation.rootKind, + updatedSelectionSet, + operation.variableDefinitions, + operation.name + ); + } + + lastGeneratedPlanStatistics(): PlanningStatistics | undefined { + return this._lastGeneratedPlanStatistics; + } } function computePlanInternal({ @@ -2575,15 +2741,16 @@ function computeRootParallelBestPlan( const plan = planningTraversal.findBestPlan(); // Getting no plan means the query is essentially unsatisfiable (it's a valid query, but we can prove it will never return a result), // so we just return an empty plan. - return plan ?? createEmptyPlan(federatedQueryGraph, root); + return plan ?? createEmptyPlan(supergraphSchema, federatedQueryGraph, root); } function createEmptyPlan( + supergraphSchema: Schema, federatedQueryGraph: QueryGraph, root: RootVertex ): [FetchDependencyGraph, OpPathTree, number] { return [ - FetchDependencyGraph.create(federatedQueryGraph, 0, undefined), + FetchDependencyGraph.create(supergraphSchema, federatedQueryGraph, 0, undefined), PathTree.createOp(federatedQueryGraph, root), 0 ]; @@ -2623,7 +2790,7 @@ function computeRootSerialDependencyGraph( // } // then we should _not_ merge the 2 `mut1` fields (contrarily to what happens on queried fields). prevPaths = prevPaths.concat(newPaths); - prevDepGraph = computeRootFetchGroups(FetchDependencyGraph.create(federatedQueryGraph, startingFetchId, rootType), prevPaths, root.rootKind); + prevDepGraph = computeRootFetchGroups(FetchDependencyGraph.create(supergraphSchema, federatedQueryGraph, startingFetchId, rootType), prevPaths, root.rootKind); } else { startingFetchId = prevDepGraph.nextFetchId(); graphs.push(prevDepGraph); @@ -2955,39 +3122,56 @@ function computeNonRootFetchGroups(dependencyGraph: FetchDependencyGraph, pathTr return dependencyGraph; } -function wrapEntitySelection( - path: GroupPath, - type: CompositeType, - selections: SelectionSet | undefined, +function wrapInputsSelections( + wrappingType: CompositeType, + selections: SelectionSet, context: PathContext -): { - updatedSelection: Selection, - newPath: GroupPath, -}{ - const typeCast = new FragmentElement(type, type.name); - let updatedSelection = selectionOfElement(typeCast, selections); - let newGroupContext = [typeCast]; +): SelectionSet { + return wrapSelectionWithTypeAndConditions( + wrappingType, + selections, + (fragment, currentSeletions) => selectionSetOf(fragment.parentType, selectionOfElement(fragment, currentSeletions)), + context + ); +} + +function createFetchInitialPath(wrappingType: CompositeType, context: PathContext): OperationPath { + return wrapSelectionWithTypeAndConditions( + wrappingType, + [], + (fragment, path) => [fragment as OperationElement].concat(path), + context, + ); +} + +function wrapSelectionWithTypeAndConditions( + wrappingType: CompositeType, + initialSelection: TSelection, + wrapInFragment: (fragment: FragmentElement, current: TSelection) => TSelection, + context: PathContext +): TSelection { + const typeCast = new FragmentElement(wrappingType, wrappingType.name); + let updatedSelection = wrapInFragment(typeCast, initialSelection); if (context.conditionals.length === 0) { - return { updatedSelection, newPath: path.forNewKeyFetch(newGroupContext) }; + return updatedSelection; } - const schema = type.schema(); + const schema = wrappingType.schema(); // We add the first include/skip to the current typeCast and then wrap in additional type-casts for the next ones // if necessary. Note that we use type-casts (... on ), but, outside of the first one, we could well also // use fragments with no type-condition. We do the former mostly to preverve older behavior, but doing the latter - // would technically procude slightly small query plans. + // would technically produce slightly small query plans. const [name0, ifs0] = context.conditionals[0]; typeCast.applyDirective(schema.directive(name0)!, { 'if': ifs0 }); for (let i = 1; i < context.conditionals.length; i++) { const [name, ifs] = context.conditionals[i]; - const fragment = new FragmentElement(type, type.name); + const fragment = new FragmentElement(wrappingType, wrappingType.name); fragment.applyDirective(schema.directive(name)!, { 'if': ifs }); - updatedSelection = selectionOfElement(fragment, selectionSetOf(type, updatedSelection)); - newGroupContext = [fragment].concat(newGroupContext); + updatedSelection = wrapInFragment(fragment, updatedSelection); } - return {updatedSelection, newPath: path.forNewKeyFetch(newGroupContext) }; + return updatedSelection; } function extractPathInParentForKeyFetch(type: CompositeType, path: GroupPath): OperationPath { @@ -3058,12 +3242,24 @@ function computeGroupsForTree( createdGroups.push(...conditionsGroups); // Then we can "take the edge", creating a new group. That group depends // on the condition ones. - const type = edge.tail.type as CompositeType; // We shouldn't have a key on a non-composite type - const pathInParent = extractPathInParentForKeyFetch(type, path); + const sourceType = edge.head.type as CompositeType; // We shouldn't have a key on a non-composite type + const destType = edge.tail.type as CompositeType; // We shouldn't have a key on a non-composite type + const pathInParent = extractPathInParentForKeyFetch(sourceType, path); const updatedDeferContext = deferContextAfterSubgraphJump(deferContext); + // Note that we use the name of `destType` for the inputs parent type, which can seem strange, but the reason is that we + // 2 kind of cases: + // - either sourceType == destType, which is the case for an object entity key, or for a key from an @interfaceObject + // to an interface key. + // - or sourceType !== destType, and that means the source is an implementation type X of some interface I, and + // destType is an @interfaceObject corresponding to I. But in that case, using I as base for the inputs is a + // bit more flexible as it ensure that if the query uses multiple such key for multiple implementations (so, + // key from X to I, and then Y to I), then the same fetch is properly reused. Note that it is ok to do so + // since 1) inputs are based on the supergraph schema, so I is going to exist there and 2) we wrap the input + // selection properly against `sourceType` below anyway. const newGroup = dependencyGraph.getOrCreateKeyFetchGroup({ subgraphName: edge.tail.source, mergeAt: path.inResponse(), + inputsTypeName: destType.name, parent: { group, path: pathInParent }, conditionsGroups, deferRef: updatedDeferContext.activeDeferRef, @@ -3080,19 +3276,28 @@ function computeGroupsForTree( } return { group: conditionGroup, path }; })); - const inputSelections = newCompositeTypeSelectionSet(type); + // Note that inputs must be based on the supergraph schema, not any particular subgraph, since sometimes key conditions + // are fetched from multiple subgraphs (and so no one subgraph has a type definition with all the proper fields, only + // the supergraph does). + const inputType = dependencyGraph.typeForFetchInputs(sourceType.name); + const inputSelections = newCompositeTypeSelectionSet(inputType); inputSelections.mergeIn(edge.conditions!); - - const {updatedSelection, newPath} = wrapEntitySelection(path, type, inputSelections, newContext); - newGroup.addInputs(updatedSelection); + let rewrites: FetchDataRewrite[] | undefined = undefined; + if (isInterfaceObjectType(destType)) { + rewrites = [{ + path: [ `... on ${inputType.name}`, typenameFieldName ], + setValueTo: destType.name, + }]; + } + newGroup.addInputs(wrapInputsSelections(inputType, inputSelections, newContext), rewrites); // We also ensure to get the __typename of the current type in the "original" group. - group.addSelection(path.inGroup().concat(new Field((edge.head.type as CompositeType).typenameField()!))); + group.addSelection(path.inGroup().concat(new Field(sourceType.typenameField()!))); stack.push({ tree: child, group: newGroup, - path: newPath, + path: path.forNewKeyFetch(createFetchInitialPath(edge.tail.type as CompositeType, newContext)), context: newContext, deferContext: updatedDeferContext, }); @@ -3129,11 +3334,10 @@ function computeGroupsForTree( deferRef: updatedDeferContext.activeDeferRef, }); newGroup.addParent({ group, path: path.inGroup() }); - const { newPath } = wrapEntitySelection(path, type, undefined, newContext); stack.push({ tree: child, group: newGroup, - path: newPath, + path: path.forNewKeyFetch(createFetchInitialPath(type, newContext)), context: newContext, deferContext: updatedDeferContext, }); @@ -3150,8 +3354,8 @@ function computeGroupsForTree( path, }); - // We're now removed any @defer. If the operation contains other directives, we need to preserve those and - // so we add operation. Otherwise, we just skip it as a minor optimization (it makes the subgraph query + // We're now removed any @defer. If the operation contains other directives or a non-trivial + // type condition, we need to preserve it and so we add operation. Otherwise, we just skip it as a minor optimization (it makes the subgraph query // slighly smaller and on complex queries, it might also deduplicate similar selections). let newPath = path; if (updatedOperation && updatedOperation.appliedDirectives.length > 0) { @@ -3213,7 +3417,45 @@ function computeGroupsForTree( createdGroups.push(...requireResult.createdGroups); } - updated.path = updated.path.add(updatedOperation); + if (updatedOperation.kind === 'Field' && updatedOperation.name === typenameFieldName) { + // Because of the optimization done in `QueryPlanner.optimizeSiblingTypenames`, we will rarely get an explicit `__typename` + // edge here. But one case where it can happen is where an @interfaceObject was involved, and we had to force jumping to + // another subgraph for getting the "true" `__typename`. However, this case can sometimes lead to fetch group that only + // exists for that `__typename` resolution and that "look" useless. That, we could have a fetch group that looks like: + // Fetch(service: "Subgraph2") { + // { + // ... on I { + // __typename + // id + // } + // } => + // { + // ... on I { + // __typename + // } + // } + // } + // 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* 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 + // optimization doesn't kick in in this case. + updated.group.mustPreserveSelection = true + } + + if (edge.transition.kind === 'InterfaceObjectFakeDownCast') { + // We shouldn't add the operation "as is" as it's a down-cast but we're "faking it". However, + // if the operation has directives, we should preserve that. + assert(updatedOperation.kind === 'FragmentElement', () => `Unexpected operation ${updatedOperation} for edge ${edge}`); + if (updatedOperation.appliedDirectives.length > 0) { + // We want to keep the directives, but we clear the condition since it's to a type that doesn't exists in the + // subgraph we're currently in. + updated.path = updated.path.add(updatedOperation.withUpdatedCondition(undefined)); + } + } else { + updated.path = updated.path.add(updatedOperation); + } stack.push(updated); } @@ -3337,10 +3579,14 @@ function handleRequires( // We start by computing the groups for the conditions. We do this using a copy of the current // group (with only the inputs) as that allows to modify this copy without modifying `group`. - const originalInputs = group.clonedInputs()!; - const newGroup = dependencyGraph.newKeyFetchGroup({ subgraphName: group.subgraphName, mergeAt: group.mergeAt!, deferRef: group.deferRef}); + const newGroup = dependencyGraph.newKeyFetchGroup({ + subgraphName: group.subgraphName, + inputsTypeName: group.inputs!.parentType.name, + mergeAt: group.mergeAt!, + deferRef: group.deferRef + }); newGroup.addParent(parent); - newGroup.addInputs(originalInputs.forRead()); + newGroup.copyInputsOf(group, true); const createdGroups = computeGroupsForTree(dependencyGraph, requiresConditions, newGroup, path, deferContextForConditions(deferContext)); if (createdGroups.length == 0) { // All conditions were local. Just merge the newly created group back in the current group (we didn't need it) @@ -3448,7 +3694,7 @@ function handleRequires( if (unmergedGroups.length == 0) { // We still need to add the stuffs we require though (but `group` already has a key in its inputs, // we don't need one). - group.addInputs(inputsForRequire(dependencyGraph.federatedQueryGraph, path, entityType, edge, context, false).inputs); + group.addInputs(inputsForRequire(dependencyGraph, entityType, edge, context, false).inputs); return { group, path, createdGroups: [] }; } @@ -3457,7 +3703,12 @@ function handleRequires( // would have been able to merge `newGroup` to `group`'s parent). So the group we should return, which // is the group where the "post-@require" fields will be add, needs to a be a new group that depends // on all those `unmergedGroups`. - const postRequireGroup = dependencyGraph.newKeyFetchGroup({ subgraphName: group.subgraphName, mergeAt: group.mergeAt!, deferRef: group.deferRef}); + const postRequireGroup = dependencyGraph.newKeyFetchGroup({ + subgraphName: group.subgraphName, + inputsTypeName: entityType.name, + mergeAt: group.mergeAt!, + deferRef: group.deferRef + }); // Note that `postRequireGroup` cannot generally be merged in any of the `unmergedGroup` and we don't provide a `path`. postRequireGroup.addParents(unmergedGroups.map((group) => ({ group }))); // That group also need, in general, to depend on the current `group`. That said, if we detected that the @require @@ -3473,7 +3724,7 @@ function handleRequires( // we need the path here, so this will have to do for now, and if this ever breaks in practice, we'll at least have an example to // guide us toward improving/fixing. assert(parent.path, `Missing path-in-parent for @require on ${edge} with group ${group} and parent ${parent}`); - const newPath = addPostRequireInputs( + addPostRequireInputs( dependencyGraph, path.forParentOfGroup(parent.path), entityType, @@ -3484,11 +3735,10 @@ function handleRequires( ); return { group: postRequireGroup, - path: newPath, + path: path.forNewKeyFetch(createFetchInitialPath(entityType, context)), createdGroups: unmergedGroups.concat(postRequireGroup), }; } else { - // We're in the somewhat simpler case where a @require happens somewhere in the middle of a subgraph query (so, not // just after having jumped to that subgraph). In that case, there isn't tons of optimisation we can do: we have to // see what satisfying the @require necessitate, and if it needs anything from another subgraph, we have to stop the @@ -3499,12 +3749,17 @@ function handleRequires( if (createdGroups.length == 0) { return { group, path, createdGroups: []}; } + // We need to create a new group, on the same subgraph `group`, where we resume fetching the field for // which we handle the @requires _after_ we've delt with the `requiresConditionsGroups`. // Note that we know the conditions will include a key for our group so we can resume properly. - const newGroup = dependencyGraph.newKeyFetchGroup({ subgraphName: group.subgraphName, mergeAt: path.inResponse() }); + const newGroup = dependencyGraph.newKeyFetchGroup({ + subgraphName: group.subgraphName, + inputsTypeName: entityType.name, + mergeAt: path.inResponse(), + }); newGroup.addParents(createdGroups.map((group) => ({ group }))); - const newPath = addPostRequireInputs( + addPostRequireInputs( dependencyGraph, path, entityType, @@ -3513,7 +3768,11 @@ function handleRequires( group, newGroup, ); - return { group: newGroup, path: newPath, createdGroups: createdGroups.concat(newGroup) }; + return { + group: newGroup, + path: path.forNewKeyFetch(createFetchInitialPath(entityType, context)), + createdGroups: createdGroups.concat(newGroup), + }; } } @@ -3525,8 +3784,8 @@ function addPostRequireInputs( context: PathContext, preRequireGroup: FetchGroup, postRequireGroup: FetchGroup, -): GroupPath { - const { inputs, newPath, keyInputs } = inputsForRequire(dependencyGraph.federatedQueryGraph, requirePath, entityType, edge, context); +) { + const { inputs, keyInputs } = inputsForRequire(dependencyGraph, entityType, edge, context); postRequireGroup.addInputs(inputs); if (keyInputs) { // It could be the key used to resume fetching after the @require is already fetched in the original group, but we cannot @@ -3536,7 +3795,6 @@ function addPostRequireInputs( endOfPathSet.addAll(keyInputs.selections()); }); } - return newPath; } function newCompositeTypeSelectionSet(type: CompositeType): SelectionSet { @@ -3546,31 +3804,55 @@ function newCompositeTypeSelectionSet(type: CompositeType): SelectionSet { } function inputsForRequire( - graph: QueryGraph, - path: GroupPath, + dependencyGraph: FetchDependencyGraph, entityType: ObjectType, edge: Edge, context: PathContext, includeKeyInputs: boolean = true ): { - inputs: Selection, - newPath: GroupPath, + inputs: SelectionSet, keyInputs: SelectionSet | undefined, }{ - const fullSelectionSet = newCompositeTypeSelectionSet(entityType); + // This method is actually called for to handle conditions of @requires, but also to fetch `__typename` in the + // case of "fake downcast on an @interfaceObject". In that later case, once we're fetched that `__typename`, + // we want to wrap the input into the "downcasted" type, not the @interfaceObject one, so that we don't end + // up querying some fields in the @interfaceObject subgraph for entities that we know won't match a type + // condition of the query. + const isInterfaceObjectDownCast = edge.transition.kind === 'InterfaceObjectFakeDownCast'; + const inputTypeName = isInterfaceObjectDownCast ? edge.transition.castedTypeName : entityType.name; + const inputType = dependencyGraph.supergraphSchema.type(inputTypeName); + assert(inputType && isCompositeType(inputType), () => `Type ${inputTypeName} should exist in the supergraph and be a composite type`); + + const fullSelectionSet = newCompositeTypeSelectionSet(inputType); fullSelectionSet.mergeIn(edge.conditions!); let keyInputs: SelectionSet | undefined = undefined; if (includeKeyInputs) { - const keyCondition = getLocallySatisfiableKey(graph, edge.head); + const keyCondition = getLocallySatisfiableKey(dependencyGraph.federatedQueryGraph, edge.head); assert(keyCondition, () => `Due to @require, validation should have required a key to be present for ${edge}`); - fullSelectionSet.mergeIn(keyCondition); + let keyConditionAsInput = keyCondition; + if (isInterfaceObjectDownCast) { + // This means that conditions parents are on the @interfaceObject type, but we actually want to select only the + // `inputTypeName` implementation, the `mergeIn` below will try to add fields from the interface to one of the + // implementationt type. Which `mergeIn` usually let us do as that's safe, but because `keyCondition` are on + // the @interfaceObject subgraph, the type there is not an interface. To work around this, we "rebase" the + // condition on the supergraph type (which is an interface) first, which lets the `mergeIn` work. + const supergraphItfType = dependencyGraph.supergraphSchema.type(entityType.name); + assert(supergraphItfType && isInterfaceType(supergraphItfType), () => `Type ${entityType} should be an interface in the supergraph`); + const rebasedKeyCondition = new SelectionSet(supergraphItfType); + rebasedKeyCondition.mergeIn(keyConditionAsInput); + keyConditionAsInput = rebasedKeyCondition; + } + fullSelectionSet.mergeIn(keyConditionAsInput); + + // Note that `keyInputs` are used to ensure those input are fetch on the original group, the one having `edge`. In + // the case of an @interfaceObject downcast, that's the subgraph with said @interfaceObject, so in that case we + // should just use `entityType` (that @interfaceObject type), not input type which will be an implementation the + // subgraph does not know in that particular case. keyInputs = newCompositeTypeSelectionSet(entityType); keyInputs.mergeIn(keyCondition); } - const { updatedSelection, newPath } = wrapEntitySelection(path, entityType, fullSelectionSet, context); return { - inputs: updatedSelection, - newPath, + inputs: wrapInputsSelections(inputType, fullSelectionSet, context), keyInputs, }; } diff --git a/query-planner-js/src/index.ts b/query-planner-js/src/index.ts index 9362024a4..a02f2279a 100644 --- a/query-planner-js/src/index.ts +++ b/query-planner-js/src/index.ts @@ -2,44 +2,6 @@ export { queryPlanSerializer, astSerializer } from './snapshotSerializers'; export { prettyFormatQueryPlan } from './prettyFormatQueryPlan'; export * from './QueryPlan'; -import { QueryPlan } from './QueryPlan'; - -import { Schema, Operation, Concrete } from '@apollo/federation-internals'; -import { buildFederatedQueryGraph, QueryGraph } from "@apollo/query-graphs"; -import { computeQueryPlan, PlanningStatistics } from './buildPlan'; -import { enforceQueryPlannerConfigDefaults, QueryPlannerConfig } from './config'; - +export { QueryPlanner } from './buildPlan'; export { QueryPlannerConfig } from './config'; -export class QueryPlanner { - private readonly config: Concrete; - private readonly federatedQueryGraph: QueryGraph; - private _lastGeneratedPlanStatistics: PlanningStatistics | undefined; - - constructor( - public readonly supergraphSchema: Schema, - config?: QueryPlannerConfig - ) { - this.config = enforceQueryPlannerConfigDefaults(config); - this.federatedQueryGraph = buildFederatedQueryGraph(supergraphSchema, true); - } - - buildQueryPlan(operation: Operation): QueryPlan { - if (operation.selectionSet.isEmpty()) { - return { kind: 'QueryPlan' }; - } - - const {plan, statistics} = computeQueryPlan({ - config: this.config, - supergraphSchema: this.supergraphSchema, - federatedQueryGraph: this.federatedQueryGraph, - operation, - }); - this._lastGeneratedPlanStatistics = statistics; - return plan; - } - - lastGeneratedPlanStatistics(): PlanningStatistics | undefined { - return this._lastGeneratedPlanStatistics; - } -} diff --git a/subgraph-js/src/__tests__/buildSubgraphSchema.test.ts b/subgraph-js/src/__tests__/buildSubgraphSchema.test.ts index 935390759..9b1a5448d 100644 --- a/subgraph-js/src/__tests__/buildSubgraphSchema.test.ts +++ b/subgraph-js/src/__tests__/buildSubgraphSchema.test.ts @@ -1160,8 +1160,7 @@ describe('buildSubgraphSchema', () => { }); it('expands federation 2.2 correctly', async () => { - // For 2.2, we expect in particular that: - // - the @composeDirective directive to exists + // For 2.2, we expect everything from 2.1 plus: // - the @shareable directive to be repeatable await testVersion('2.2', ` schema @@ -1223,6 +1222,72 @@ describe('buildSubgraphSchema', () => { } `); }); + + it('expands federation 2.3 correctly', async () => { + // For 2.3, we expect in everything from 2.2 plus: + // - the @interfaceObject directive + await testVersion('2.3', ` + schema + @link(url: \"https://specs.apollo.dev/link/v1.0\") + { + query: Query + } + + extend schema + @link(url: "https://specs.apollo.dev/federation/v2.3", import: ["@key"]) + + directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA + + directive @key(fields: federation__FieldSet!, resolvable: Boolean = true) repeatable on OBJECT | INTERFACE + + directive @federation__requires(fields: federation__FieldSet!) on FIELD_DEFINITION + + directive @federation__provides(fields: federation__FieldSet!) on FIELD_DEFINITION + + directive @federation__external(reason: String) on OBJECT | FIELD_DEFINITION + + directive @federation__tag(name: String!) repeatable on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION + + directive @federation__extends on OBJECT | INTERFACE + + directive @federation__shareable repeatable on OBJECT | FIELD_DEFINITION + + directive @federation__inaccessible on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION + + directive @federation__override(from: String!) on FIELD_DEFINITION + + directive @federation__composeDirective(name: String) repeatable on SCHEMA + + directive @federation__interfaceObject on OBJECT + + type Query { + x: Int + _service: _Service! + } + + enum link__Purpose { + """ + \`SECURITY\` features provide metadata necessary to securely resolve fields. + """ + SECURITY + + """ + \`EXECUTION\` features provide metadata necessary for operation execution. + """ + EXECUTION + } + + scalar link__Import + + scalar federation__FieldSet + + scalar _Any + + type _Service { + sdl: String + } + `); + }); }); }); diff --git a/subgraph-js/src/schemaExtensions.ts b/subgraph-js/src/schemaExtensions.ts index cdd990883..3c7d9d57f 100644 --- a/subgraph-js/src/schemaExtensions.ts +++ b/subgraph-js/src/schemaExtensions.ts @@ -5,7 +5,7 @@ import { GraphQLUnionTypeExtensions } from 'graphql'; -type GraphQLReferenceResolver = ( +export type GraphQLReferenceResolver = ( reference: object, context: TContext, info: GraphQLResolveInfo, diff --git a/subgraph-js/src/types.ts b/subgraph-js/src/types.ts index e5bed8fe5..115e04cc2 100644 --- a/subgraph-js/src/types.ts +++ b/subgraph-js/src/types.ts @@ -11,10 +11,17 @@ import { isNamedType, isObjectType, GraphQLResolveInfo, + isInterfaceType, + defaultTypeResolver, + GraphQLError, + GraphQLAbstractType, + GraphQLSchema, + GraphQLInterfaceType, } from 'graphql'; import { PromiseOrValue } from 'graphql/jsutils/PromiseOrValue'; import { maybeCacheControlFromInfo } from '@apollo/cache-control-types'; -import { ApolloGraphQLObjectTypeExtensions } from './schemaExtensions'; +import { ApolloGraphQLInterfaceTypeExtensions, ApolloGraphQLObjectTypeExtensions, GraphQLReferenceResolver } from './schemaExtensions'; +import { inspect } from 'util'; export type Maybe = null | undefined | T; @@ -62,6 +69,99 @@ function addTypeNameToPossibleReturn( return maybeObject as null | (T & { __typename: string }); } +function maybeAddTypeNameToPossibleReturn( + maybeObject: PromiseOrValue, + typename: string, +): PromiseOrValue { + if (isPromise(maybeObject)) { + return maybeObject.then((x: any) => addTypeNameToPossibleReturn(x, typename)); + } + return addTypeNameToPossibleReturn(maybeObject, typename); +} + +/** + * Copied from GraphQL-js. + * + * For @key on interfaces, we need to check that we can resolve the runtime type of the object returned by the interface + * `__resolveReference`. If we cannot, and we simply don't add any `__typename` to the result, the graphQL-js will end + * erroring out, but the error will not be user friendly as it will say something along the lines of: + * ``` + * Abstract type "_Entity" must resolve to an Object type at runtime for field "Query._entities". Either the "_Entity" type + * should provide a "resolveType" function or each possible type should provide an "isTypeOf" function. + * ``` + * But this is ultimately incorrect, as it is only interface type the user must use "resolveType", add a __typename, or rely + * on "isTypeOf". And so we have to somewhat copy this. As we do, we also adapt the message a bit to be more user friendly. + */ +function ensureValidRuntimeType( + runtimeTypeName: unknown, + schema: GraphQLSchema, + returnType: GraphQLAbstractType, + result: unknown, +): GraphQLObjectType { + if (runtimeTypeName == null) { + throw new GraphQLError( + `Abstract type "${returnType.name}" \`__resolveReference\` method must resolve to an Object type at runtime. Either the object returned by "${returnType}.__resolveReference" must include a valid \`__typename\` field, or the "${returnType.name}" type should provide a "resolveType" function or each possible type should provide an "isTypeOf" function.`, + ); + } + + if (typeof runtimeTypeName !== 'string') { + throw new GraphQLError( + `Abstract type "${returnType.name}" \`__resolveReference\` method must resolve to an Object type at runtime with ` + + `value ${inspect(result)}, received "${inspect(runtimeTypeName)}".`, + ); + } + + const runtimeType = schema.getType(runtimeTypeName); + if (runtimeType == null) { + throw new GraphQLError( + `Abstract type "${returnType.name}" \`__resolveReference\` method resolved to a type "${runtimeTypeName}" that does not exist inside the schema.`, + ); + } + + if (!isObjectType(runtimeType)) { + throw new GraphQLError( + `Abstract type "${returnType.name}" \`__resolveReference\` method resolved to a non-object type "${runtimeTypeName}".`, + ); + } + + if (!schema.isSubType(returnType, runtimeType)) { + throw new GraphQLError( + `Runtime Object type "${runtimeType.name}" \`__resolveReference\` method is not a possible type for "${returnType.name}".`, + ); + } + + return runtimeType; +} + +function withResolvedType({ + type, + value, + context, + info, + callback, +}: { + type: GraphQLInterfaceType, + value: any, + context: any, + info: GraphQLResolveInfo, + callback: (runtimeType: GraphQLObjectType) => PromiseOrValue, +}): PromiseOrValue { + const resolveTypeFn = type.resolveType ?? defaultTypeResolver; + const runtimeType = resolveTypeFn(value, context, info, type); + if (isPromise(runtimeType)) { + return runtimeType.then((name) => ( + callback(ensureValidRuntimeType(name, info.schema, type, value)) + )); + } + + return callback(ensureValidRuntimeType(runtimeType, info.schema, type, value)); +} + +function definedResolveReference(type: GraphQLObjectType | GraphQLInterfaceType): GraphQLReferenceResolver | undefined { + const extensions: ApolloGraphQLObjectTypeExtensions | ApolloGraphQLInterfaceTypeExtensions = type.extensions; + return extensions.apollo?.subgraph?.resolveReference; +} + export function entitiesResolver({ representations, context, @@ -75,9 +175,9 @@ export function entitiesResolver({ const { __typename } = reference; const type = info.schema.getType(__typename); - if (!type || !isObjectType(type)) { + if (!type || !(isObjectType(type) || isInterfaceType(type))) { throw new Error( - `The _entities resolver tried to load an entity for type "${__typename}", but no object type of that name was found in the schema`, + `The _entities resolver tried to load an entity for type "${__typename}", but no object or interface type of that name was found in the schema`, ); } @@ -96,19 +196,41 @@ export function entitiesResolver({ } } - const extensions: ApolloGraphQLObjectTypeExtensions = type.extensions; - const resolveReference = extensions.apollo?.subgraph?.resolveReference ?? (() => reference); + const resolveReference = definedResolveReference(type); // FIXME somehow get this to show up special in Studio traces? - const result = resolveReference(reference, context, info); + const result = resolveReference ? resolveReference(reference, context, info) : reference; - if (isPromise(result)) { - return result.then((x: any) => - addTypeNameToPossibleReturn(x, __typename), - ); + if (isInterfaceType(type)) { + return withResolvedType({ + type, + value: result, + context, + info, + callback: (runtimeType) => { + // If we had no interface-level __resolveReference, then we look for one on the runtime + // type itself, and call it if it exists. If that one also doesn't, we essentially end + // up using the same resolver than for object types, one that does nothing. + let finalResult = maybeAddTypeNameToPossibleReturn(result, runtimeType.name); + if (!resolveReference) { + const runtimeResolveReference = definedResolveReference(runtimeType); + if (runtimeResolveReference) { + // Note that we call the resolver on the reference with the "proper" __typename, + // and then add back the __typename again in case the resolver removed it (which + // ultimately is the behaviour we use with object type __resolveReference in + // general). + finalResult = isPromise(finalResult) + ? finalResult.then((r) => runtimeResolveReference(r, context, info)) + : runtimeResolveReference(finalResult, context, info); + finalResult = maybeAddTypeNameToPossibleReturn(finalResult, runtimeType.name); + } + } + return finalResult; + }, + }); } - return addTypeNameToPossibleReturn(result, __typename); + return maybeAddTypeNameToPossibleReturn(result, __typename); }); } From 5e5d03e5400c60801f4462f826fedc85d920bf04 Mon Sep 17 00:00:00 2001 From: Sylvain Lebresne Date: Thu, 8 Dec 2022 15:35:04 +0100 Subject: [PATCH 2/4] Implements review feedback --- composition-js/src/merging/merge.ts | 12 ++++++------ gateway-js/src/executeQueryPlan.ts | 10 +++++----- internals-js/src/error.ts | 2 +- query-graphs-js/src/transition.ts | 24 ++++++++++++------------ query-planner-js/src/QueryPlan.ts | 27 ++++++++++++++++++++++++--- query-planner-js/src/buildPlan.ts | 9 +++++---- subgraph-js/src/types.ts | 9 +++++---- 7 files changed, 58 insertions(+), 35 deletions(-) diff --git a/composition-js/src/merging/merge.ts b/composition-js/src/merging/merge.ts index 98742dd4c..d50de4038 100644 --- a/composition-js/src/merging/merge.ts +++ b/composition-js/src/merging/merge.ts @@ -446,7 +446,7 @@ class Merger { // When @interfaceObject is used in a subgraph, then that subgraph essentially provides fields both - // to the interface but also to all it's implementation. But so far, we only merged the type definition + // to the interface but also to all its implementations. But so far, we only merged the type definition // itself, so we now need to potentially add the field to the implementations if missing. // Note that we do this after everything else have been merged because this method will essentially // copy things from interface in the merged schema into their implementation in that same schema so @@ -761,9 +761,9 @@ class Merger { // There is either 1 join__type per-key, or if there is no key, just one for the type. const sourceMetadata = this.subgraphs.values()[idx].metadata(); - // Note that mechanically we don't need to substitue `undefined` for `false` below (`false` is the + // Note that mechanically we don't need to substitute `undefined` for `false` below (`false` is the // default value), but doing so 1) yield smaller supergraph (because the parameter isn't included) - // and 2) this avoid needless disrepancies compared to supergraphs generated before @interfaceObject was added. + // and 2) this avoid needless discrepancies compared to supergraphs generated before @interfaceObject was added. const isInterfaceObject = sourceMetadata.isInterfaceObjectType(source) ? true : undefined; const keys = source.appliedDirectivesOf(sourceMetadata.keyDirective()); const name = this.joinSpecName(idx); @@ -1637,9 +1637,9 @@ class Merger { } private validateInterfaceKeys(sources: (InterfaceType | ObjectType | undefined)[], dest: InterfaceType) { - // Remark: it might be ok to filter @inaccessible types in `supergraphImplementations`, but this require - // some more thinking (and I'm not even sure it makes a practical difference given the rules for validaty - // of @inacessible) and it will be backward compatible to filter them later, while the reverse wouldn't + // Remark: it might be ok to filter @inaccessible types in `supergraphImplementations`, but this requires + // some more thinking (and I'm not even sure it makes a practical difference given the rules for validity + // of @inaccessible) and it will be backward compatible to filter them later, while the reverse wouldn't // technically be, so we stay on the safe side. const supergraphImplementations = dest.possibleRuntimeTypes(); diff --git a/gateway-js/src/executeQueryPlan.ts b/gateway-js/src/executeQueryPlan.ts index 271d4c00b..caf0d8ed3 100644 --- a/gateway-js/src/executeQueryPlan.ts +++ b/gateway-js/src/executeQueryPlan.ts @@ -24,7 +24,7 @@ import { QueryPlanSelectionNode, QueryPlanFieldNode, getResponseName, - FetchDataRewrite, + FetchDataInputRewrite, } from '@apollo/query-planner'; import { deepMerge } from './utilities/deepMerge'; import { isNotNullOrUndefined } from './utilities/array'; @@ -486,8 +486,8 @@ async function executeFetch( } } -function updateRewrites(rewrites: FetchDataRewrite[] | undefined, pathElement: string): { - updated: FetchDataRewrite[], +function updateRewrites(rewrites: FetchDataInputRewrite[] | undefined, pathElement: string): { + updated: FetchDataInputRewrite[], completeRewrite?: any, } | undefined { if (!rewrites) { @@ -497,7 +497,7 @@ function updateRewrites(rewrites: FetchDataRewrite[] | undefined, pathElement: s let completeRewrite: any = undefined; const updated = rewrites .map((r) => { - let u: FetchDataRewrite | undefined = undefined; + let u: FetchDataInputRewrite | undefined = undefined; if (r.path[0] === pathElement) { const updatedPath = r.path.slice(1); if (updatedPath.length === 0) { @@ -523,7 +523,7 @@ function executeSelectionSet( schema: GraphQLSchema, source: Record | null, selections: QueryPlanSelectionNode[], - activeRewrites?: FetchDataRewrite[], + activeRewrites?: FetchDataInputRewrite[], ): Record | null { // If the underlying service has returned null for the parent (source) diff --git a/internals-js/src/error.ts b/internals-js/src/error.ts index 84578aac2..053ab1355 100644 --- a/internals-js/src/error.ts +++ b/internals-js/src/error.ts @@ -545,7 +545,7 @@ const INTERFACE_KEY_NOT_ON_IMPLEMENTATION = makeCodeDefinition( const INTERFACE_KEY_MISSING_IMPLEMENTATION_TYPE = makeCodeDefinition( 'INTERFACE_KEY_MISSING_IMPLEMENTATION_TYPE', - 'A subgraph has a `@key` on an interface type, but that subgraph does not define all the implementation (in the supergraph) of that interface', + 'A subgraph has a `@key` on an interface type, but that subgraph does not define an implementation (in the supergraph) of that interface', { addedIn: '2.3.0' }, ) diff --git a/query-graphs-js/src/transition.ts b/query-graphs-js/src/transition.ts index 3b71e0b24..c41ab2f79 100644 --- a/query-graphs-js/src/transition.ts +++ b/query-graphs-js/src/transition.ts @@ -50,8 +50,8 @@ export type Transition = ; export class KeyResolution { - readonly kind = 'KeyResolution' as const; - readonly collectOperationElements = false as const; + readonly kind = 'KeyResolution' + readonly collectOperationElements = false; toString() { return 'key()'; @@ -59,8 +59,8 @@ export class KeyResolution { } export class RootTypeResolution { - readonly kind = 'RootTypeResolution' as const; - readonly collectOperationElements = false as const; + readonly kind = 'RootTypeResolution'; + readonly collectOperationElements = false; constructor(readonly rootKind: SchemaRootKind) { } @@ -71,8 +71,8 @@ export class RootTypeResolution { } export class FieldCollection { - readonly kind = 'FieldCollection' as const; - readonly collectOperationElements = true as const; + readonly kind = 'FieldCollection'; + readonly collectOperationElements = true; constructor( readonly definition: FieldDefinition, @@ -85,8 +85,8 @@ export class FieldCollection { } export class DownCast { - readonly kind = 'DownCast' as const; - readonly collectOperationElements = true as const; + readonly kind = 'DownCast'; + readonly collectOperationElements = true; constructor(readonly sourceType: CompositeType, readonly castedType: CompositeType) {} @@ -96,8 +96,8 @@ export class DownCast { } export class SubgraphEnteringTransition { - readonly kind = 'SubgraphEnteringTransition' as const; - readonly collectOperationElements = false as const; + readonly kind = 'SubgraphEnteringTransition'; + readonly collectOperationElements = false; toString() { return '∅'; @@ -105,8 +105,8 @@ export class SubgraphEnteringTransition { } export class InterfaceObjectFakeDownCast { - readonly kind = 'InterfaceObjectFakeDownCast' as const; - readonly collectOperationElements = true as const; + readonly kind = 'InterfaceObjectFakeDownCast'; + readonly collectOperationElements = true; constructor(readonly sourceType: CompositeType, readonly castedTypeName: string) {} diff --git a/query-planner-js/src/QueryPlan.ts b/query-planner-js/src/QueryPlan.ts index 6dfc96ea3..4e70af686 100644 --- a/query-planner-js/src/QueryPlan.ts +++ b/query-planner-js/src/QueryPlan.ts @@ -39,12 +39,33 @@ export interface FetchNode { operationName: string | undefined; operationKind: OperationTypeNode; operationDocumentNode?: DocumentNode; - inputRewrites?: FetchDataRewrite[]; + // Optionally describe a number of "rewrites" that query plan executors should apply to the data that is sent as input of this fetch. + inputRewrites?: FetchDataInputRewrite[]; } -export interface FetchDataRewrite { +/** + * The type of rewrites currently supported on the input data of fetches. + * + * A rewrite usually identifies some subpart of the input data and some action to perform on that subpart. + * Note that input rewrites should only impact the inputs of the fetch they are applied to (meaning that, as + * those inputs are collected from the current in-memory result, the rewrite should _not_ impact said in-memory + * results, only what is sent in the fetch). + */ +export type FetchDataInputRewrite = FetchDataValueSetter; + +/** + * A rewrite that sets a value at the provided path of the data it is applied to. + */ +export interface FetchDataValueSetter { + kind: 'ValueSetter', + // Path to the key that is set by this "rewrite". It is of the form `[ 'x', '... on A', 'y' ]`, where fragments + // means that the path should only match for objects whose `__typename` is he provided type. + // If the path does not match in the data this is applied to, then this setter should not rewrite the data. + // The path starts at the top of the data it is applied to. So for instance, for fetch data inputs, the path + // start at the root of the object representing those inputs. path: string[], - setValueTo?: any, + // The value to set at `path`. + setValueTo: any, } export interface FlattenNode { diff --git a/query-planner-js/src/buildPlan.ts b/query-planner-js/src/buildPlan.ts index b937fbe77..d24af6025 100644 --- a/query-planner-js/src/buildPlan.ts +++ b/query-planner-js/src/buildPlan.ts @@ -89,7 +89,7 @@ import { buildFederatedQueryGraph, } from "@apollo/query-graphs"; import { stripIgnoredCharacters, print, parse, OperationTypeNode } from "graphql"; -import { DeferredNode, FetchDataRewrite } from "."; +import { DeferredNode, FetchDataInputRewrite } from "."; import { enforceQueryPlannerConfigDefaults, QueryPlannerConfig } from "./config"; import { QueryPlan, ResponsePath, SequenceNode, PlanNode, ParallelNode, FetchNode, trimSelectionNodes } from "./QueryPlan"; @@ -658,7 +658,7 @@ class FetchGroup { // Set in some code-path to indicate that the selection of the group not be optimized away even if it "looks" useless. mustPreserveSelection: boolean = false; - private readonly inputRewrites: FetchDataRewrite[] = []; + private readonly inputRewrites: FetchDataInputRewrite[] = []; private constructor( readonly dependencyGraph: FetchDependencyGraph, @@ -856,7 +856,7 @@ class FetchGroup { return this._children; } - addInputs(selection: Selection | SelectionSet, rewrites?: FetchDataRewrite[]) { + addInputs(selection: Selection | SelectionSet, rewrites?: FetchDataInputRewrite[]) { assert(this._inputs, "Shouldn't try to add inputs to a root fetch group"); // There is subtlety here, that is due to the fact that we sometime want to merge groups that // are at the same mergeAt but that may currently have "incompatible" input parent types. In @@ -3282,9 +3282,10 @@ function computeGroupsForTree( const inputType = dependencyGraph.typeForFetchInputs(sourceType.name); const inputSelections = newCompositeTypeSelectionSet(inputType); inputSelections.mergeIn(edge.conditions!); - let rewrites: FetchDataRewrite[] | undefined = undefined; + let rewrites: FetchDataInputRewrite[] | undefined = undefined; if (isInterfaceObjectType(destType)) { rewrites = [{ + kind: 'ValueSetter', path: [ `... on ${inputType.name}`, typenameFieldName ], setValueTo: destType.name, }]; diff --git a/subgraph-js/src/types.ts b/subgraph-js/src/types.ts index 115e04cc2..58a040289 100644 --- a/subgraph-js/src/types.ts +++ b/subgraph-js/src/types.ts @@ -80,17 +80,18 @@ function maybeAddTypeNameToPossibleReturn( } /** - * Copied from GraphQL-js. + * Copied and adapted from GraphQL-js to provide more tailored error messages (but the equivalent method is also not exported + * by `graphql-js`). * * For @key on interfaces, we need to check that we can resolve the runtime type of the object returned by the interface * `__resolveReference`. If we cannot, and we simply don't add any `__typename` to the result, the graphQL-js will end * erroring out, but the error will not be user friendly as it will say something along the lines of: * ``` - * Abstract type "_Entity" must resolve to an Object type at runtime for field "Query._entities". Either the "_Entity" type - * should provide a "resolveType" function or each possible type should provide an "isTypeOf" function. + * Abstract type "_Entity" must resolve to an Object type at runtime for field "Query._entities". Either the "_Entity" type + * should provide a "resolveType" function or each possible type should provide an "isTypeOf" function. * ``` * But this is ultimately incorrect, as it is only interface type the user must use "resolveType", add a __typename, or rely - * on "isTypeOf". And so we have to somewhat copy this. As we do, we also adapt the message a bit to be more user friendly. + * on "isTypeOf". And so we have to somewhat copy and adapt the logic slightly (mostly to provide a more user friendly method). */ function ensureValidRuntimeType( runtimeTypeName: unknown, From 3b5eee850ac5797d09c3bcda50ca79244ad01a12 Mon Sep 17 00:00:00 2001 From: Sylvain Lebresne Date: Thu, 8 Dec 2022 15:50:44 +0100 Subject: [PATCH 3/4] regen error doc --- docs/source/errors.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/errors.md b/docs/source/errors.md index da357b00d..fb4fa7d3b 100644 --- a/docs/source/errors.md +++ b/docs/source/errors.md @@ -44,7 +44,7 @@ The following errors might be raised during composition: | `INPUT_FIELD_DEFAULT_MISMATCH` | An input field has a default value that is incompatible with other declarations of that field in other subgraphs. | 2.0.0 | | | `INTERFACE_FIELD_IMPLEM_TYPE_MISMATCH` | For an interface field, some of its concrete implementations have @external or @requires and there is difference in those implementations return type (which is currently not supported; see https://github.com/apollographql/federation/issues/1257) | 2.0.0 | | | `INTERFACE_FIELD_NO_IMPLEM` | After subgraph merging, an implementation is missing a field of one of the interface it implements (which can happen for valid subgraphs). | 2.0.0 | | -| `INTERFACE_KEY_MISSING_IMPLEMENTATION_TYPE` | A subgraph has a `@key` on an interface type, but that subgraph does not define all the implementation (in the supergraph) of that interface | 2.3.0 | | +| `INTERFACE_KEY_MISSING_IMPLEMENTATION_TYPE` | A subgraph has a `@key` on an interface type, but that subgraph does not define an implementation (in the supergraph) of that interface | 2.3.0 | | | `INTERFACE_KEY_NOT_ON_IMPLEMENTATION` | A `@key` is defined on an interface type, but is not defined (or is not resolvable) on at least one of the interface implementations | 2.3.0 | | | `INTERFACE_OBJECT_USAGE_ERROR` | Error in the usage of the @interfaceObject directive. | 2.3.0 | | | `INVALID_FEDERATION_SUPERGRAPH` | Indicates that a schema provided for an Apollo Federation supergraph is not a valid supergraph schema. | 2.1.0 | | From 2fb8f7ed531afe08b29e89850516990b1afdaf1b Mon Sep 17 00:00:00 2001 From: Sylvain Lebresne Date: Wed, 14 Dec 2022 11:16:33 +0100 Subject: [PATCH 4/4] changelogs --- gateway-js/CHANGELOG.md | 2 ++ subgraph-js/CHANGELOG.md | 3 +++ 2 files changed, 5 insertions(+) diff --git a/gateway-js/CHANGELOG.md b/gateway-js/CHANGELOG.md index 2a8bdb748..2ef833153 100644 --- a/gateway-js/CHANGELOG.md +++ b/gateway-js/CHANGELOG.md @@ -4,6 +4,8 @@ This CHANGELOG pertains only to Apollo Federation packages in the 2.x range. The ## vNext +- Adds support for `@interfaceObject` and keys on interfaces [PR #2279](https://github.com/apollographql/federation/pull/2279). + ## 2.2.2 diff --git a/subgraph-js/CHANGELOG.md b/subgraph-js/CHANGELOG.md index 0c5ac4966..4b64a3dd7 100644 --- a/subgraph-js/CHANGELOG.md +++ b/subgraph-js/CHANGELOG.md @@ -4,6 +4,9 @@ This CHANGELOG pertains only to Apollo Federation packages in the 2.x range. The ## vNext +- Adds support for the 2.3 version of the federation spec (that is, `@link(url: "https://specs.apollo.dev/federation/v2.3")`), with: + - New `@interfaceObject` directive and support for keys on interfaces. + ## 2.2.0 - Adds support for the 2.2 version of the federation spec (that is, `@link(url: "https://specs.apollo.dev/federation/v2.2")`), which: