From 8d0ea76f119cedaf160fdcff8d8ee40a285e4bfe Mon Sep 17 00:00:00 2001 From: Simon Lind Date: Fri, 25 Sep 2026 17:41:27 +0000 Subject: [PATCH 1/4] feat: route same-named root fields by enum argument, merge enums as unions Same-named types from different subgraphs were merged with no conflict detection: an enum kept the first subgraph's values, an object type got its field lists concatenated, and a Query or Mutation field published by several subgraphs kept the last subgraph's resolver. A call could then validate against one subgraph's enum and execute against another. - an enum is now the union of the values of every subgraph, each value keeping the description of the subgraph that declared it - object fields are merged by name, the first declaration wins - a shared Query/Mutation field is routed by an enum argument: it must be declared with the same enum type in every subgraph, as Enum or Enum!, with every value declared by exactly one subgraph; the value the client passes selects the subgraph - composition fails when a shared root field cannot be routed: no such argument, a value declared by two subgraphs, or more than one argument that could route it Co-Authored-By: Claude Fable 5.1 --- README.md | 42 ++++++++ lib/composer.js | 102 +++++++++++++++++++ lib/fields.js | 26 ++++- test/shared-root-fields.test.js | 172 ++++++++++++++++++++++++++++++++ 4 files changed, 340 insertions(+), 2 deletions(-) create mode 100644 test/shared-root-fields.test.js diff --git a/README.md b/README.md index c77bf1f..3de3ece 100644 --- a/README.md +++ b/README.md @@ -174,6 +174,48 @@ async function main () { main() ``` +## Same-named types across subgraphs + +Subgraphs may publish types with the same name. They are merged as follows: + +- an **enum** becomes the union of the values declared by every subgraph, each value keeping the description of the subgraph that declared it +- an **object type** becomes the union of the fields by name; the first declaration of a field wins +- any other type keeps the first declaration + +A `Query` or `Mutation` field published by more than one subgraph is **routed by an enum argument**. The argument must be declared with the same enum type in every subgraph, as `Enum` or `Enum!`, and every value must be declared by exactly one subgraph. The value a client passes selects the subgraph that receives the call. + +Given a `Books` subgraph: + +```graphql +enum IndexedEntity { BOOK } +type IndexResult { indexed: Int! } +type Mutation { reindex(entity: IndexedEntity!): IndexResult! } +``` + +and a `Reviews` subgraph: + +```graphql +enum IndexedEntity { REVIEW AUTHOR } +type IndexResult { indexed: Int! } +type Mutation { reindex(entity: IndexedEntity!): IndexResult! } +``` + +the composed schema is: + +```graphql +enum IndexedEntity { BOOK REVIEW AUTHOR } +type IndexResult { indexed: Int! } +type Mutation { reindex(entity: IndexedEntity!): IndexResult! } +``` + +`reindex(entity: BOOK)` is sent to `Books`, `reindex(entity: AUTHOR)` to `Reviews`, and a value outside the enum is rejected by the composed schema before any subgraph is called. + +Composition fails, rather than picking a subgraph silently, when a shared root field cannot be routed: + +- no argument is an enum with a distinct set of values per subgraph, for example two subgraphs both publishing `Query.status: String` +- a value is declared by more than one subgraph +- more than one enum argument could route the field + ## API ### `compose(config)` diff --git a/lib/composer.js b/lib/composer.js index 796b847..5f5e82e 100644 --- a/lib/composer.js +++ b/lib/composer.js @@ -121,6 +121,7 @@ class Composer { } this.buildMergedSchema() + this.buildSharedFieldResolvers() } // TODO return a copy to avoid writes on resolvers @@ -454,6 +455,100 @@ class Composer { } } + /** + * A Query or Mutation field published by more than one subgraph is routed by an enum argument: + * every subgraph declares the argument with the same enum type and its own values, the merged + * schema carries the union of the values (see `mergeTypes`), and the value a client sends picks + * the subgraph. Without this pass the field silently kept the last subgraph's resolver while the + * enum kept the first subgraph's values, so a call could validate against one subgraph and run + * against another. + * + * Composition fails when a shared field has no such argument, or when a value is declared by + * more than one subgraph: either way there is no single subgraph that owns the call. + */ + buildSharedFieldResolvers () { + const mainTypeNames = new Set() + for (const mainType of this.mainTypes.values()) { + mainTypeNames.add(this.schemaOptions[mainType].name) + } + + for (const fieldId of Object.keys(this.fields)) { + const typeName = fieldId.slice(0, fieldId.indexOf('.')) + if (!mainTypeNames.has(typeName)) { continue } + + const subgraphNames = Object.keys(this.fields[fieldId]).filter(s => s !== COMPOSER_SUBGRAPH_NAME) + if (subgraphNames.length < 2) { continue } + + const { argName, routes } = this.sharedFieldDiscriminator(fieldId, subgraphNames) + this.logger.debug({ fieldId, argName, routes: Object.fromEntries(routes) }, 'route shared field by enum argument') + + this.resolvers[typeName][fieldId.slice(typeName.length + 1)] = this.createRoutedResolver({ typeName, fieldId, argName, routes }) + } + } + + /** + * find the one argument of a shared field that routes it: an enum, of the same type name in + * every subgraph, whose values are declared by exactly one subgraph each + * @returns {{ argName: string, routes: Map }} routes: enum value -> subgraph name + */ + sharedFieldDiscriminator (fieldId, subgraphNames) { + const candidates = [] + const firstField = this.fields[fieldId][subgraphNames[0]].src + + for (const firstArg of firstField.args ?? []) { + const enumName = enumArgTypeName(firstArg.type) + if (!enumName) { continue } + + const routes = new Map() + let disjoint = true + + for (const subgraphName of subgraphNames) { + const arg = this.fields[fieldId][subgraphName].src.args?.find(a => a.name === firstArg.name) + if (!arg || enumArgTypeName(arg.type) !== enumName) { + disjoint = false + break + } + + const values = this.types[enumName]?.[subgraphName]?.src.enumValues ?? [] + for (const { name } of values) { + if (routes.has(name)) { + disjoint = false + break + } + routes.set(name, subgraphName) + } + if (!disjoint) { break } + } + + if (disjoint && routes.size > 0) { + candidates.push({ argName: firstArg.name, routes }) + } + } + + if (candidates.length === 1) { + return candidates[0] + } + + const where = `${fieldId} is published by subgraphs ${subgraphNames.join(', ')}` + if (candidates.length === 0) { + throw new Error(`Cannot compose ${where}: no enum argument with a distinct set of values per subgraph to route by`) + } + throw new Error(`Cannot compose ${where}: more than one enum argument could route it (${candidates.map(c => c.argName).join(', ')})`) + } + + createRoutedResolver ({ typeName, fieldId, argName, routes }) { + return async (parent, args, context, info) => { + const value = args?.[argName] + const subgraphName = routes.get(value) + if (!subgraphName) { + throw new Error(`No subgraph resolves ${fieldId} for ${argName}: ${String(value)}`) + } + + const fieldSrc = this.fields[fieldId][subgraphName].src + return this.runResolver({ typeName, subgraphName, fieldSrc, parent, args, context, info }) + } + } + async runResolver ({ typeName, subgraphName, fieldSrc, parent, args, context, info }) { const { queries, order } = this.collectQueries({ typeName, @@ -783,6 +878,13 @@ class Composer { } } +// the enum type name of an argument declared as `Enum` or `Enum!`; lists and other kinds do not +// route, a call has to land on exactly one subgraph +function enumArgTypeName (type) { + const t = type.kind === 'NON_NULL' ? type.ofType : type + return t.kind === 'ENUM' ? t.name : undefined +} + // TODO improve code logic, add unit tests // key should be selected by parent type, as it is for parent/many function parentKey (keys, path, type) { diff --git a/lib/fields.js b/lib/fields.js index 4f9383e..af3ad10 100644 --- a/lib/fields.js +++ b/lib/fields.js @@ -4,13 +4,24 @@ const QUERY_TYPE = 'QUERY' const MUTATION_TYPE = 'MUTATION' /** - * merge only entity types + * merge two definitions of the same type coming from different subgraphs + * - ENUM: the union of the values, by name + * - OBJECT: the union of the fields, by name; the first definition of a field wins + * - anything else: the first definition wins + * + * Same-named fields on Query or Mutation are resolved by `Composer#buildSharedFieldResolvers`, + * which routes each call to one subgraph by the value of an enum argument. */ function mergeTypes (t1, t2) { + if (t1.src.kind === 'ENUM' && Array.isArray(t1.src.enumValues) && Array.isArray(t2.src.enumValues)) { + t1.src.enumValues = unionByName(t1.src.enumValues, t2.src.enumValues) + return t1 + } + if (t1.src.kind !== 'OBJECT' || !Array.isArray(t1.src.fields)) { return t1 } - t1.src.fields = t1.src.fields.concat(t2.src.fields) + t1.src.fields = unionByName(t1.src.fields, t2.src.fields ?? []) // TODO t1.fields = t1.fields.concat(t2.fields) // TODO fields.resolvers @@ -18,6 +29,17 @@ function mergeTypes (t1, t2) { return t1 } +function unionByName (first, second) { + const names = new Set(first.map(item => item.name)) + const union = first.slice() + for (const item of second) { + if (names.has(item.name)) { continue } + names.add(item.name) + union.push(item) + } + return union +} + // return Query or Mutation if type is one of them // TODO Subscription function getMainType (schema, type) { diff --git a/test/shared-root-fields.test.js b/test/shared-root-fields.test.js new file mode 100644 index 0000000..3802c97 --- /dev/null +++ b/test/shared-root-fields.test.js @@ -0,0 +1,172 @@ +'use strict' + +const assert = require('node:assert') +const { test } = require('node:test') +const dedent = require('dedent') +const { createGraphqlServices, createComposerService, graphqlRequest } = require('./helper') +const { compose } = require('../') + +// Two subgraphs publish the same Mutation field with the same enum argument, each declaring +// its own values: the merged enum is the union, the field appears once, and a call is routed +// to the subgraph that declares the value the client passed. +function indexingSubgraph (entities, owner, calls) { + const values = entities.map(entity => `"""${owner}""" ${entity}`).join(' ') + return { + mercurius: { + schema: ` + enum IndexedEntity { ${values} } + type IndexResult { indexed: Int! } + type Query { ${owner}Status: String } + type Mutation { reindex(entity: IndexedEntity!): IndexResult! } + `, + resolvers: { + Query: { [`${owner}Status`]: () => 'ok' }, + Mutation: { + reindex: (_, { entity }) => { + calls.push(entity) + return { indexed: entities.indexOf(entity) + 1 } + } + } + } + }, + listen: true + } +} + +async function composeServices (t, configs) { + const services = await createGraphqlServices(t, configs) + const options = { + subgraphs: services.map(service => ({ name: service.name, server: { host: service.host } })) + } + return { services, options } +} + +test.describe('same-named root fields across subgraphs', () => { + test('should merge a same-named enum as the union of the values and publish shared types once', async (t) => { + const { options } = await composeServices(t, [ + { name: 'books', ...indexingSubgraph(['BOOK'], 'books', []) }, + { name: 'reviews', ...indexingSubgraph(['REVIEW', 'AUTHOR'], 'reviews', []) } + ]) + + const composer = await compose(options) + + assert.strictEqual(composer.toSdl(), dedent` + enum IndexedEntity { + """books""" + BOOK + + """reviews""" + REVIEW + + """reviews""" + AUTHOR + } + + type IndexResult { + indexed: Int! + } + + type Query { + booksStatus: String + reviewsStatus: String + } + + type Mutation { + reindex(entity: IndexedEntity!): IndexResult! + }`) + }) + + test('should route a shared mutation to the subgraph that declares the enum value', async (t) => { + const books = [] + const reviews = [] + const { options } = await composeServices(t, [ + { name: 'books', ...indexingSubgraph(['BOOK'], 'books', books) }, + { name: 'reviews', ...indexingSubgraph(['REVIEW', 'AUTHOR'], 'reviews', reviews) } + ]) + const { service } = await createComposerService(t, { compose, options }) + + const literal = await graphqlRequest(service, 'mutation { reindex(entity: AUTHOR) { indexed } }') + assert.deepStrictEqual(literal, { reindex: { indexed: 2 } }) + + const variable = await graphqlRequest(service, + 'mutation ($entity: IndexedEntity!) { reindex(entity: $entity) { indexed } }', + { entity: 'BOOK' }) + assert.deepStrictEqual(variable, { reindex: { indexed: 1 } }) + + assert.deepStrictEqual({ books, reviews }, { books: ['BOOK'], reviews: ['AUTHOR'] }) + + // fields published by one subgraph only are untouched + const own = await graphqlRequest(service, '{ booksStatus reviewsStatus }') + assert.deepStrictEqual(own, { booksStatus: 'ok', reviewsStatus: 'ok' }) + }) + + test('should reject a value no subgraph declares before reaching any subgraph', async (t) => { + const books = [] + const reviews = [] + const { options } = await composeServices(t, [ + { name: 'books', ...indexingSubgraph(['BOOK'], 'books', books) }, + { name: 'reviews', ...indexingSubgraph(['REVIEW'], 'reviews', reviews) } + ]) + const { service } = await createComposerService(t, { compose, options }) + + const response = await service.inject({ + path: '/graphql', + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ query: 'mutation { reindex(entity: PUBLISHER) { indexed } }' }) + }) + const { data, errors } = response.json() + + assert.strictEqual(data, null) + assert.match(errors[0].message, /"PUBLISHER" does not exist in "IndexedEntity" enum/) + assert.deepStrictEqual({ books, reviews }, { books: [], reviews: [] }) + }) + + test('should fail to compose when two subgraphs declare the same enum value', async (t) => { + const { options } = await composeServices(t, [ + { name: 'books', ...indexingSubgraph(['BOOK'], 'books', []) }, + { name: 'catalog', ...indexingSubgraph(['SERIES', 'BOOK'], 'catalog', []) } + ]) + + await assert.rejects(compose(options), + /Cannot compose Mutation\.reindex is published by subgraphs books, catalog: no enum argument with a distinct set of values per subgraph to route by/) + }) + + test('should fail to compose a shared root field with no enum argument to route by', async (t) => { + const status = label => ({ + mercurius: { + schema: 'type Query { status: String }', + resolvers: { Query: { status: () => label } } + }, + listen: true + }) + const { options } = await composeServices(t, [ + { name: 'books', ...status('books') }, + { name: 'reviews', ...status('reviews') } + ]) + + await assert.rejects(compose(options), + /Cannot compose Query\.status is published by subgraphs books, reviews: no enum argument/) + }) + + test('should fail to compose when more than one enum argument could route a shared field', async (t) => { + const lookup = (kind, region) => ({ + mercurius: { + schema: ` + enum Kind { ${kind} } + enum Region { ${region} } + type Query { lookup(kind: Kind!, region: Region!): String } + `, + resolvers: { Query: { lookup: () => `${kind}/${region}` } } + }, + listen: true + }) + const { options } = await composeServices(t, [ + { name: 'one', ...lookup('A', 'EU') }, + { name: 'two', ...lookup('B', 'US') } + ]) + + await assert.rejects(compose(options), + /Cannot compose Query\.lookup .* more than one enum argument could route it \(kind, region\)/) + }) +}) From d8568bfe01739b20e89154095877cb9811213f9e Mon Sep 17 00:00:00 2001 From: Simon Lind Date: Fri, 25 Sep 2026 18:14:35 +0000 Subject: [PATCH 2/4] feat: make the handling of same-named types and root fields an option Adds `onConflict` to the composer options: "error" (default) fails composition on a shared Query or Mutation field, "first" and "last" pick that subgraph's resolver and enum values with a warning, and "route" merges the enum and routes each call by the enum argument. Routing now checks what a routed call relies on and fails composition otherwise: the subgraphs must agree on the return type, on every argument's type and on the defaults of the non-routing arguments; the routing argument must be non-null or have a default; and no subgraph outside the field's publishers may add values to the enum, since such a value would pass validation with nowhere to go. The shared-field check uses the root type names actually seen in the subgraphs rather than the configured ones. Also fixes a pre-existing crash when a client leaves an argument to its schema default: graphql-js fills the value but there is no AST node for it, and the query builder dereferenced it. The argument is now left out of the subgraph query so the subgraph applies the same default. Co-Authored-By: Claude Fable 5.1 --- README.md | 22 ++- lib/composer.js | 199 ++++++++++++++++------ lib/fields.js | 42 +++-- lib/utils.js | 6 + lib/validation.js | 7 + test/shared-root-fields.test.js | 291 ++++++++++++++++++++++++++------ 6 files changed, 446 insertions(+), 121 deletions(-) diff --git a/README.md b/README.md index 3de3ece..8f78788 100644 --- a/README.md +++ b/README.md @@ -176,13 +176,20 @@ main() ## Same-named types across subgraphs -Subgraphs may publish types with the same name. They are merged as follows: +Subgraphs may publish types and root fields with the same name. The `onConflict` option decides how they are merged: -- an **enum** becomes the union of the values declared by every subgraph, each value keeping the description of the subgraph that declared it -- an **object type** becomes the union of the fields by name; the first declaration of a field wins -- any other type keeps the first declaration +| `onConflict` | same-named enum | field both declare on a same-named object type | shared `Query` / `Mutation` field | +| --- | --- | --- | --- | +| `'error'` (default) | union of the values | first declaration | composition fails, naming the field and the subgraphs | +| `'first'` | first subgraph's values | first declaration | first subgraph's resolver, with a warning | +| `'last'` | last subgraph's values | last declaration | last subgraph's resolver, with a warning | +| `'route'` | union of the values | first declaration | routed to one subgraph by an enum argument | -A `Query` or `Mutation` field published by more than one subgraph is **routed by an enum argument**. The argument must be declared with the same enum type in every subgraph, as `Enum` or `Enum!`, and every value must be declared by exactly one subgraph. The value a client passes selects the subgraph that receives the call. +"First" and "last" follow the order of `subgraphs`. A merged enum value keeps the description of the subgraph that declared it. + +### Routing a shared root field + +With `onConflict: 'route'`, a `Query` or `Mutation` field published by more than one subgraph is routed by one of its arguments. Every subgraph must declare the field with the same return type and the same argument types, and one argument must be an enum, declared as `Enum!` or with a default value, whose values are declared by exactly one subgraph each. The value a client passes selects the subgraph that receives the call. Given a `Books` subgraph: @@ -212,9 +219,11 @@ type Mutation { reindex(entity: IndexedEntity!): IndexResult! } Composition fails, rather than picking a subgraph silently, when a shared root field cannot be routed: -- no argument is an enum with a distinct set of values per subgraph, for example two subgraphs both publishing `Query.status: String` +- the subgraphs disagree on the return type or on an argument's type, or on the default of any argument other than the routing one +- no argument is an enum with a distinct set of values per subgraph; a nullable enum argument without a default does not qualify, since a client may leave it out - a value is declared by more than one subgraph - more than one enum argument could route the field +- a subgraph that does not publish the field adds values to the enum, since such a value would pass validation with nowhere to go ## API @@ -255,6 +264,7 @@ Composition fails, rather than picking a subgraph silently, when a shared root f - `onSubgraphError` (function, optional) - Hook called when an error occurs getting schema from a subgraph. The default function will throw the error. The arguments are: - `error` (error) - The error. - `subgraph` (string) - The erroring subgraph name. + - `onConflict` (string, optional) - How a type or root field that more than one subgraph publishes is merged: `'error'`, `'first'`, `'last'` or `'route'`, see [same-named types across subgraphs](#same-named-types-across-subgraphs). **Default:** `'error'`. - `queryTypeName` (string, optional) - The name of the `Query` type in the composed schema. **Default:** `'Query'`. - `mutationTypeName` (string, optional) - The name of the `Mutation` type in the composed schema. **Default:** `'Mutation'`. diff --git a/lib/composer.js b/lib/composer.js index 5f5e82e..6f67e6a 100644 --- a/lib/composer.js +++ b/lib/composer.js @@ -48,6 +48,8 @@ class Composer { this.mergedSchema = { _built: false } this.schemas = [] this.mainTypes = new Set() + // the Query and Mutation type names actually seen in the subgraphs + this.mainTypeNames = new Set() this.resolvers = Object.create(null) const v = validateComposerOptions(options) @@ -65,6 +67,7 @@ class Composer { } this.addEntitiesResolvers = v.addEntitiesResolvers this.onSubgraphError = v.onSubgraphError + this.onConflict = v.onConflict this.subgraphs = v.subgraphs this.defaultArgsAdapter = v.defaultArgsAdapter this.headersAdapter = createDefaultHeadersAdapter() @@ -165,8 +168,8 @@ class Composer { const t = this.mergedSchema.types.get(typeName) - // TODO handle conflicts by name or type, add option to rename, mask or hide - this.mergedSchema.types.set(typeName, t ? mergeTypes(t, type) : objectDeepClone(type)) + // TODO add option to rename, mask or hide conflicting fields + this.mergedSchema.types.set(typeName, t ? mergeTypes(t, type, this.onConflict) : objectDeepClone(type)) } } @@ -290,6 +293,7 @@ class Composer { const mainType = getMainType(schema, schemaType) if (mainType) { this.mainTypes.add(mainType) + this.mainTypeNames.add(typeName) } if (!Array.isArray(schemaType.fields)) { @@ -455,62 +459,130 @@ class Composer { } } + async runResolver ({ typeName, subgraphName, fieldSrc, parent, args, context, info }) { + const { queries, order } = this.collectQueries({ + typeName, + subgraphName, + fieldSrc, + parent, + args, + context, + info + }) + + const result = await this.runQueries(queries, order, context) + return result[fieldSrc.name] + } + /** - * A Query or Mutation field published by more than one subgraph is routed by an enum argument: - * every subgraph declares the argument with the same enum type and its own values, the merged - * schema carries the union of the values (see `mergeTypes`), and the value a client sends picks - * the subgraph. Without this pass the field silently kept the last subgraph's resolver while the - * enum kept the first subgraph's values, so a call could validate against one subgraph and run - * against another. + * A Query or Mutation field published by more than one subgraph is resolved according to the + * `onConflict` strategy: + * - "error": composition fails, naming the field and the subgraphs + * - "first" / "last": that subgraph's resolver answers, and a warning names the others + * - "route": every subgraph declares the field with the same signature and an enum argument + * whose values are disjoint per subgraph; the merged enum is the union (see `mergeTypes`) and + * the value a client passes picks the subgraph * - * Composition fails when a shared field has no such argument, or when a value is declared by - * more than one subgraph: either way there is no single subgraph that owns the call. + * Without this pass the field silently kept the last subgraph's resolver while a shared enum + * kept the first subgraph's values, so a call could validate against one subgraph and run + * against another. */ buildSharedFieldResolvers () { - const mainTypeNames = new Set() - for (const mainType of this.mainTypes.values()) { - mainTypeNames.add(this.schemaOptions[mainType].name) - } - for (const fieldId of Object.keys(this.fields)) { const typeName = fieldId.slice(0, fieldId.indexOf('.')) - if (!mainTypeNames.has(typeName)) { continue } + if (!this.mainTypeNames.has(typeName)) { continue } const subgraphNames = Object.keys(this.fields[fieldId]).filter(s => s !== COMPOSER_SUBGRAPH_NAME) if (subgraphNames.length < 2) { continue } - const { argName, routes } = this.sharedFieldDiscriminator(fieldId, subgraphNames) + const fieldName = fieldId.slice(typeName.length + 1) + const where = `${fieldId} is published by subgraphs ${subgraphNames.join(', ')}` + + if (this.onConflict === 'error') { + throw new Error(`Cannot compose ${where}: set onConflict to "route", "first" or "last" to resolve it`) + } + + if (this.onConflict === 'first' || this.onConflict === 'last') { + const winner = this.onConflict === 'first' ? subgraphNames[0] : subgraphNames[subgraphNames.length - 1] + this.logger.warn({ fieldId, subgraphs: subgraphNames, winner }, `${where}: resolved by subgraph ${winner} (onConflict: ${this.onConflict})`) + this.resolvers[typeName][fieldName] = this.fields[fieldId][winner].resolver + continue + } + + this.assertSameSignature(fieldId, subgraphNames, where) + const { argName, routes } = this.sharedFieldDiscriminator(fieldId, subgraphNames, where) + this.assertSameDefaults(fieldId, subgraphNames, where, argName) this.logger.debug({ fieldId, argName, routes: Object.fromEntries(routes) }, 'route shared field by enum argument') - this.resolvers[typeName][fieldId.slice(typeName.length + 1)] = this.createRoutedResolver({ typeName, fieldId, argName, routes }) + this.resolvers[typeName][fieldName] = this.createRoutedResolver({ typeName, fieldId, argName, routes }) + } + } + + /** + * a routed field is validated once, against the merged schema, then sent to one subgraph: + * the subgraphs must agree on the return type and on every argument's type, or a call that + * passes validation can fail in the subgraph that receives it + */ + assertSameSignature (fieldId, subgraphNames, where) { + const signatures = new Map() + for (const subgraphName of subgraphNames) { + signatures.set(subgraphName, printFieldSignature(this.fields[fieldId][subgraphName].src)) + } + + if (new Set(signatures.values()).size > 1) { + const listed = Array.from(signatures, ([s, sig]) => `${s}: ${sig}`).join('; ') + throw new Error(`Cannot compose ${where}: the signatures differ (${listed})`) + } + } + + /** + * an argument the client leaves out is filled from the merged schema's default, which is the + * first subgraph's, and sent without a value so the receiving subgraph applies its own: the + * defaults must agree. The routing argument is the exception: each subgraph can only default + * it to a value it declares, and a call left to the merged default is routed to that very + * subgraph. + */ + assertSameDefaults (fieldId, subgraphNames, where, routingArgName) { + const defaults = new Map() + for (const subgraphName of subgraphNames) { + const args = (this.fields[fieldId][subgraphName].src.args ?? []) + .filter(arg => arg.name !== routingArgName && arg.defaultValue != null) + .map(arg => `${arg.name} = ${arg.defaultValue}`) + .sort() + defaults.set(subgraphName, `(${args.join(', ')})`) + } + + if (new Set(defaults.values()).size > 1) { + const listed = Array.from(defaults, ([s, d]) => `${s}: ${d}`).join('; ') + throw new Error(`Cannot compose ${where}: the argument defaults differ (${listed})`) } } /** - * find the one argument of a shared field that routes it: an enum, of the same type name in - * every subgraph, whose values are declared by exactly one subgraph each + * find the one argument of a shared field that routes it: an enum, declared as `Enum!` or with + * a default value, whose values are declared by exactly one subgraph each * @returns {{ argName: string, routes: Map }} routes: enum value -> subgraph name */ - sharedFieldDiscriminator (fieldId, subgraphNames) { + sharedFieldDiscriminator (fieldId, subgraphNames, where) { const candidates = [] - const firstField = this.fields[fieldId][subgraphNames[0]].src + const optional = [] + // the signatures are identical at this point, so the first subgraph's arguments stand for all + const args = this.fields[fieldId][subgraphNames[0]].src.args ?? [] - for (const firstArg of firstField.args ?? []) { - const enumName = enumArgTypeName(firstArg.type) + for (const arg of args) { + const enumName = enumArgTypeName(arg.type) if (!enumName) { continue } + // a client may omit a nullable argument without a default, and then nothing routes the call + if (arg.type.kind !== 'NON_NULL' && arg.defaultValue == null) { + optional.push(arg.name) + continue + } + const routes = new Map() let disjoint = true - for (const subgraphName of subgraphNames) { - const arg = this.fields[fieldId][subgraphName].src.args?.find(a => a.name === firstArg.name) - if (!arg || enumArgTypeName(arg.type) !== enumName) { - disjoint = false - break - } - - const values = this.types[enumName]?.[subgraphName]?.src.enumValues ?? [] - for (const { name } of values) { + for (const { name } of this.types[enumName][subgraphName].src.enumValues) { if (routes.has(name)) { disjoint = false break @@ -521,19 +593,37 @@ class Composer { } if (disjoint && routes.size > 0) { - candidates.push({ argName: firstArg.name, routes }) + candidates.push({ argName: arg.name, enumName, routes }) } } - if (candidates.length === 1) { - return candidates[0] + if (candidates.length > 1) { + throw new Error(`Cannot compose ${where}: more than one enum argument could route it (${candidates.map(c => c.argName).join(', ')})`) } - const where = `${fieldId} is published by subgraphs ${subgraphNames.join(', ')}` if (candidates.length === 0) { - throw new Error(`Cannot compose ${where}: no enum argument with a distinct set of values per subgraph to route by`) + const hint = optional.length > 0 + ? `; ${optional.map(name => `"${name}"`).join(', ')} is a nullable enum argument without a default value, declare it as non-null or give it a default to route by it` + : '' + throw new Error(`Cannot compose ${where}: no enum argument with a distinct set of values per subgraph to route by${hint}`) + } + + // the merged enum is the union over EVERY subgraph declaring it, not only the field's + // publishers, and a value nobody publishing the field declares would pass validation + // and then have nowhere to go + const { argName, enumName, routes } = candidates[0] + const unroutable = [] + for (const subgraphName of Object.keys(this.types[enumName])) { + if (subgraphName === COMPOSER_SUBGRAPH_NAME || subgraphNames.includes(subgraphName)) { continue } + for (const { name } of this.types[enumName][subgraphName].src.enumValues) { + if (!routes.has(name)) { unroutable.push(`${name} (${subgraphName})`) } + } } - throw new Error(`Cannot compose ${where}: more than one enum argument could route it (${candidates.map(c => c.argName).join(', ')})`) + if (unroutable.length > 0) { + throw new Error(`Cannot compose ${where}: enum ${enumName} has values declared by subgraphs that do not publish the field, ${unroutable.join(', ')}`) + } + + return { argName, routes } } createRoutedResolver ({ typeName, fieldId, argName, routes }) { @@ -549,21 +639,6 @@ class Composer { } } - async runResolver ({ typeName, subgraphName, fieldSrc, parent, args, context, info }) { - const { queries, order } = this.collectQueries({ - typeName, - subgraphName, - fieldSrc, - parent, - args, - context, - info - }) - - const result = await this.runQueries(queries, order, context) - return result[fieldSrc.name] - } - /** * collect queries starting from the resolver fn */ @@ -885,6 +960,22 @@ function enumArgTypeName (type) { return t.kind === 'ENUM' ? t.name : undefined } +// an introspection type reference as SDL, for example `[Book!]!` +function printTypeRef (type) { + if (type.kind === 'NON_NULL') { return `${printTypeRef(type.ofType)}!` } + if (type.kind === 'LIST') { return `[${printTypeRef(type.ofType)}]` } + return type.name +} + +// a field's return type and argument types as SDL, argument order and defaults aside, to compare +// across subgraphs +function printFieldSignature (field) { + const args = (field.args ?? []) + .map(arg => `${arg.name}: ${printTypeRef(arg.type)}`) + .sort() + return `(${args.join(', ')}): ${printTypeRef(field.type)}` +} + // TODO improve code logic, add unit tests // key should be selected by parent type, as it is for parent/many function parentKey (keys, path, type) { diff --git a/lib/fields.js b/lib/fields.js index af3ad10..fcc7ac8 100644 --- a/lib/fields.js +++ b/lib/fields.js @@ -4,24 +4,30 @@ const QUERY_TYPE = 'QUERY' const MUTATION_TYPE = 'MUTATION' /** - * merge two definitions of the same type coming from different subgraphs - * - ENUM: the union of the values, by name - * - OBJECT: the union of the fields, by name; the first definition of a field wins - * - anything else: the first definition wins + * merge two definitions of the same type coming from different subgraphs, t1 being the merged + * definition so far and t2 the next subgraph's one, according to the `onConflict` strategy + * - ENUM: "first" keeps t1's values, "last" takes t2's, "route" and "error" take the union by name, + * each value keeping the description of the subgraph that declared it + * - OBJECT: the union of the fields by name; a field both declare is t2's under "last", t1's otherwise + * - anything else: t1 * - * Same-named fields on Query or Mutation are resolved by `Composer#buildSharedFieldResolvers`, - * which routes each call to one subgraph by the value of an enum argument. + * A field on Query or Mutation that more than one subgraph publishes is then resolved by + * `Composer#buildSharedFieldResolvers`, following the same strategy. */ -function mergeTypes (t1, t2) { +function mergeTypes (t1, t2, onConflict = 'error') { if (t1.src.kind === 'ENUM' && Array.isArray(t1.src.enumValues) && Array.isArray(t2.src.enumValues)) { - t1.src.enumValues = unionByName(t1.src.enumValues, t2.src.enumValues) + if (onConflict === 'last') { + t1.src.enumValues = t2.src.enumValues.slice() + } else if (onConflict !== 'first') { + t1.src.enumValues = unionByName(t1.src.enumValues, t2.src.enumValues) + } return t1 } if (t1.src.kind !== 'OBJECT' || !Array.isArray(t1.src.fields)) { return t1 } - t1.src.fields = unionByName(t1.src.fields, t2.src.fields ?? []) + t1.src.fields = unionByName(t1.src.fields, t2.src.fields ?? [], onConflict === 'last') // TODO t1.fields = t1.fields.concat(t2.fields) // TODO fields.resolvers @@ -29,13 +35,21 @@ function mergeTypes (t1, t2) { return t1 } -function unionByName (first, second) { - const names = new Set(first.map(item => item.name)) +/** + * union of two lists of named items; an item both lists name is the second's when + * `preferSecond`, the first's otherwise, in the first's position either way + */ +function unionByName (first, second, preferSecond = false) { + const index = new Map(first.map((item, i) => [item.name, i])) const union = first.slice() for (const item of second) { - if (names.has(item.name)) { continue } - names.add(item.name) - union.push(item) + const at = index.get(item.name) + if (at === undefined) { + index.set(item.name, union.length) + union.push(item) + } else if (preferSecond) { + union[at] = item + } } return union } diff --git a/lib/utils.js b/lib/utils.js index f4f18b1..6e032aa 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -87,6 +87,12 @@ function collectPlainArgs (args, nodeArguments, info, types) { return collectVariableArg(nodeArguments, info, types) } + // graphql-js fills a value the client did not send from the argument's default, so there is + // no node for it; leave it out and the subgraph applies the same default + if (nodeArguments === undefined) { + return + } + if (Array.isArray(args)) { const queryArgs = [] for (let i = 0; i < args.length; i++) { diff --git a/lib/validation.js b/lib/validation.js index 5064b6d..05313d5 100644 --- a/lib/validation.js +++ b/lib/validation.js @@ -5,6 +5,8 @@ const nullLogger = require('abstract-logging') const DEFAULT_COMPOSE_ENDPOINT = '/.well-known/graphql-composition' const DEFAULT_GRAPHQL_ENDPOINT = '/graphql' +const CONFLICT_STRATEGIES = ['first', 'last', 'route', 'error'] +const DEFAULT_CONFLICT_STRATEGY = 'error' function validateArray (value, name, defaultValue) { if (!value && defaultValue) { return defaultValue } @@ -54,6 +56,11 @@ function validateComposerOptions (options) { validateString(validatedOptions.queryTypeName, 'queryTypeName') validateString(validatedOptions.mutationTypeName, 'mutationTypeName') + + validatedOptions.onConflict = options.onConflict ?? DEFAULT_CONFLICT_STRATEGY + if (!CONFLICT_STRATEGIES.includes(validatedOptions.onConflict)) { + throw new TypeError(`onConflict must be one of ${CONFLICT_STRATEGIES.map(s => `"${s}"`).join(', ')}`) + } validatedOptions.onSubgraphError = validateFunction(options.onSubgraphError, 'onSubgraphError', defaultOnError) validatedOptions.defaultArgsAdapter = validateDefaultArgsAdapterOptions(options.defaultArgsAdapter) diff --git a/test/shared-root-fields.test.js b/test/shared-root-fields.test.js index 3802c97..0897c77 100644 --- a/test/shared-root-fields.test.js +++ b/test/shared-root-fields.test.js @@ -6,25 +6,27 @@ const dedent = require('dedent') const { createGraphqlServices, createComposerService, graphqlRequest } = require('./helper') const { compose } = require('../') -// Two subgraphs publish the same Mutation field with the same enum argument, each declaring -// its own values: the merged enum is the union, the field appears once, and a call is routed -// to the subgraph that declares the value the client passed. -function indexingSubgraph (entities, owner, calls) { +// Two subgraphs publish the same Mutation field with the same enum argument, each declaring its +// own values. How the composer resolves that is the `onConflict` option: "error" (the default) +// refuses, "first" and "last" pick a subgraph, "route" merges the enum and sends each call to the +// subgraph that declares the value the client passed. +function indexingSubgraph (owner, entities, calls, { field = 'reindex(entity: IndexedEntity!): IndexResult!', result = 'type IndexResult { indexed: Int! }' } = {}) { const values = entities.map(entity => `"""${owner}""" ${entity}`).join(' ') return { + name: owner, mercurius: { schema: ` enum IndexedEntity { ${values} } - type IndexResult { indexed: Int! } + ${result} type Query { ${owner}Status: String } - type Mutation { reindex(entity: IndexedEntity!): IndexResult! } + type Mutation { ${field} } `, resolvers: { Query: { [`${owner}Status`]: () => 'ok' }, Mutation: { - reindex: (_, { entity }) => { - calls.push(entity) - return { indexed: entities.indexOf(entity) + 1 } + reindex: (_, args) => { + calls.push(args.entity) + return { indexed: entities.indexOf(args.entity) + 1, count: 0 } } } } @@ -33,21 +35,78 @@ function indexingSubgraph (entities, owner, calls) { } } -async function composeServices (t, configs) { +async function composeOptions (t, configs, options = {}) { const services = await createGraphqlServices(t, configs) - const options = { + return { + ...options, subgraphs: services.map(service => ({ name: service.name, server: { host: service.host } })) } - return { services, options } } -test.describe('same-named root fields across subgraphs', () => { - test('should merge a same-named enum as the union of the values and publish shared types once', async (t) => { - const { options } = await composeServices(t, [ - { name: 'books', ...indexingSubgraph(['BOOK'], 'books', []) }, - { name: 'reviews', ...indexingSubgraph(['REVIEW', 'AUTHOR'], 'reviews', []) } +function collectingLogger (lines) { + const push = level => (...args) => lines.push({ level, args }) + return { fatal: push('fatal'), error: push('error'), warn: push('warn'), info: push('info'), debug: push('debug'), trace: push('trace'), child () { return this } } +} + +async function rawRequest (service, query, variables) { + const response = await service.inject({ + path: '/graphql', + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ query, variables }) + }) + return response.json() +} + +test.describe('onConflict option', () => { + test('should reject an unknown strategy', async () => { + await assert.rejects(compose({ onConflict: 'merge' }), /onConflict must be one of "first", "last", "route", "error"/) + }) + + test('should fail to compose a shared root field by default', async (t) => { + const options = await composeOptions(t, [ + indexingSubgraph('books', ['BOOK'], []), + indexingSubgraph('reviews', ['REVIEW'], []) ]) + await assert.rejects(compose(options), + /Cannot compose Mutation\.reindex is published by subgraphs books, reviews: set onConflict to "route", "first" or "last" to resolve it/) + }) + + for (const [strategy, winner, values] of [['first', 'books', ['BOOK']], ['last', 'reviews', ['REVIEW', 'AUTHOR']]]) { + test(`"${strategy}" should let the ${winner} subgraph answer a shared field and keep its enum, with a warning`, async (t) => { + const calls = { books: [], reviews: [] } + const lines = [] + const options = await composeOptions(t, [ + indexingSubgraph('books', ['BOOK'], calls.books), + indexingSubgraph('reviews', ['REVIEW', 'AUTHOR'], calls.reviews) + ], { onConflict: strategy, logger: collectingLogger(lines) }) + const { composer, service } = await createComposerService(t, { compose, options }) + + const sdl = composer.toSdl() + assert.match(sdl, new RegExp(`enum IndexedEntity \\{\\n(?: {2}"""${winner}"""\\n {2}\\w+\\n\\n?)+\\}`)) + for (const value of values) { assert.match(sdl, new RegExp(`\\n {2}${value}\\n`)) } + assert.strictEqual(sdl.match(/reindex\(/g).length, 1) + + const warning = lines.find(line => line.level === 'warn') + assert.match(warning.args[1], new RegExp(`Mutation\\.reindex is published by subgraphs books, reviews: resolved by subgraph ${winner} \\(onConflict: ${strategy}\\)`)) + + await graphqlRequest(service, `mutation { reindex(entity: ${values[0]}) { indexed } }`) + assert.deepStrictEqual(calls[winner], [values[0]]) + assert.deepStrictEqual(calls[winner === 'books' ? 'reviews' : 'books'], []) + }) + } +}) + +test.describe('onConflict: "route"', () => { + const route = { onConflict: 'route' } + + test('should merge a same-named enum as the union of the values and publish shared types once', async (t) => { + const options = await composeOptions(t, [ + indexingSubgraph('books', ['BOOK'], []), + indexingSubgraph('reviews', ['REVIEW', 'AUTHOR'], []) + ], route) + const composer = await compose(options) assert.strictEqual(composer.toSdl(), dedent` @@ -79,10 +138,10 @@ test.describe('same-named root fields across subgraphs', () => { test('should route a shared mutation to the subgraph that declares the enum value', async (t) => { const books = [] const reviews = [] - const { options } = await composeServices(t, [ - { name: 'books', ...indexingSubgraph(['BOOK'], 'books', books) }, - { name: 'reviews', ...indexingSubgraph(['REVIEW', 'AUTHOR'], 'reviews', reviews) } - ]) + const options = await composeOptions(t, [ + indexingSubgraph('books', ['BOOK'], books), + indexingSubgraph('reviews', ['REVIEW', 'AUTHOR'], reviews) + ], route) const { service } = await createComposerService(t, { compose, options }) const literal = await graphqlRequest(service, 'mutation { reindex(entity: AUTHOR) { indexed } }') @@ -100,57 +159,131 @@ test.describe('same-named root fields across subgraphs', () => { assert.deepStrictEqual(own, { booksStatus: 'ok', reviewsStatus: 'ok' }) }) + test('should route a shared query too', async (t) => { + const calls = { books: [], reviews: [] } + const subgraph = (owner, value) => ({ + name: owner, + mercurius: { + schema: `enum Kind { ${value} } type Query { count(kind: Kind!): Int }`, + resolvers: { Query: { count: (_, { kind }) => { calls[owner].push(kind); return kind.length } } } + }, + listen: true + }) + const options = await composeOptions(t, [subgraph('books', 'BOOK'), subgraph('reviews', 'REVIEW')], route) + const { service } = await createComposerService(t, { compose, options }) + + assert.deepStrictEqual(await graphqlRequest(service, '{ count(kind: REVIEW) }'), { count: 6 }) + assert.deepStrictEqual(calls, { books: [], reviews: ['REVIEW'] }) + }) + + // Each subgraph can only default the routing argument to a value it declares, so the defaults + // differ by construction. The merged schema keeps the first subgraph's, and a call left to it + // is routed to that subgraph, which applies the same default itself. + test('should route by a defaulted argument the client left out', async (t) => { + const books = [] + const reviews = [] + const options = await composeOptions(t, [ + indexingSubgraph('books', ['BOOK'], books, { field: 'reindex(entity: IndexedEntity = BOOK): IndexResult!' }), + indexingSubgraph('reviews', ['REVIEW'], reviews, { field: 'reindex(entity: IndexedEntity = REVIEW): IndexResult!' }) + ], route) + const { composer, service } = await createComposerService(t, { compose, options }) + + assert.match(composer.toSdl(), /reindex\(entity: IndexedEntity = BOOK\): IndexResult!/) + assert.deepStrictEqual(await graphqlRequest(service, 'mutation { reindex { indexed } }'), { reindex: { indexed: 1 } }) + assert.deepStrictEqual(await graphqlRequest(service, 'mutation { reindex(entity: REVIEW) { indexed } }'), { reindex: { indexed: 1 } }) + assert.deepStrictEqual({ books, reviews }, { books: ['BOOK'], reviews: ['REVIEW'] }) + }) + + test('should fail to compose when the subgraphs disagree on the default of another argument', async (t) => { + const options = await composeOptions(t, [ + indexingSubgraph('books', ['BOOK'], [], { field: 'reindex(entity: IndexedEntity!, force: Boolean = true): IndexResult!' }), + indexingSubgraph('reviews', ['REVIEW'], [], { field: 'reindex(entity: IndexedEntity!, force: Boolean = false): IndexResult!' }) + ], route) + + await assert.rejects(compose(options), + /Cannot compose Mutation\.reindex .* the argument defaults differ \(books: \(force = true\); reviews: \(force = false\)\)/) + }) + test('should reject a value no subgraph declares before reaching any subgraph', async (t) => { const books = [] const reviews = [] - const { options } = await composeServices(t, [ - { name: 'books', ...indexingSubgraph(['BOOK'], 'books', books) }, - { name: 'reviews', ...indexingSubgraph(['REVIEW'], 'reviews', reviews) } - ]) + const options = await composeOptions(t, [ + indexingSubgraph('books', ['BOOK'], books), + indexingSubgraph('reviews', ['REVIEW'], reviews) + ], route) const { service } = await createComposerService(t, { compose, options }) - const response = await service.inject({ - path: '/graphql', - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ query: 'mutation { reindex(entity: PUBLISHER) { indexed } }' }) - }) - const { data, errors } = response.json() + const { data, errors } = await rawRequest(service, 'mutation { reindex(entity: PUBLISHER) { indexed } }') assert.strictEqual(data, null) assert.match(errors[0].message, /"PUBLISHER" does not exist in "IndexedEntity" enum/) assert.deepStrictEqual({ books, reviews }, { books: [], reviews: [] }) }) + test('should merge a same-named enum of non-shared fields as the union too', async (t) => { + const subgraph = (owner, values) => ({ + name: owner, + mercurius: { + schema: `enum Sort { ${values} } type Query { ${owner}List(sort: Sort): [String] }`, + resolvers: { Query: { [`${owner}List`]: (_, { sort }) => [sort] } } + }, + listen: true + }) + const options = await composeOptions(t, [subgraph('books', 'ASC DESC'), subgraph('reviews', 'ASC DESC RANDOM')], route) + const composer = await compose(options) + + assert.match(composer.toSdl(), /enum Sort \{\n {2}ASC\n {2}DESC\n {2}RANDOM\n\}/) + }) + test('should fail to compose when two subgraphs declare the same enum value', async (t) => { - const { options } = await composeServices(t, [ - { name: 'books', ...indexingSubgraph(['BOOK'], 'books', []) }, - { name: 'catalog', ...indexingSubgraph(['SERIES', 'BOOK'], 'catalog', []) } - ]) + const options = await composeOptions(t, [ + indexingSubgraph('books', ['BOOK'], []), + indexingSubgraph('catalog', ['SERIES', 'BOOK'], []) + ], route) await assert.rejects(compose(options), /Cannot compose Mutation\.reindex is published by subgraphs books, catalog: no enum argument with a distinct set of values per subgraph to route by/) }) test('should fail to compose a shared root field with no enum argument to route by', async (t) => { - const status = label => ({ + const status = owner => ({ + name: owner, mercurius: { schema: 'type Query { status: String }', - resolvers: { Query: { status: () => label } } + resolvers: { Query: { status: () => owner } } }, listen: true }) - const { options } = await composeServices(t, [ - { name: 'books', ...status('books') }, - { name: 'reviews', ...status('reviews') } - ]) + const options = await composeOptions(t, [status('books'), status('reviews')], route) await assert.rejects(compose(options), /Cannot compose Query\.status is published by subgraphs books, reviews: no enum argument/) }) + test('should fail to compose when the only enum argument is nullable without a default', async (t) => { + const field = 'reindex(entity: IndexedEntity): IndexResult!' + const options = await composeOptions(t, [ + indexingSubgraph('books', ['BOOK'], [], { field }), + indexingSubgraph('reviews', ['REVIEW'], [], { field }) + ], route) + + await assert.rejects(compose(options), + /Cannot compose Mutation\.reindex .*"entity" is a nullable enum argument without a default value, declare it as non-null or give it a default/) + }) + + test('should not route by a list of enums', async (t) => { + const field = 'reindex(entity: [IndexedEntity!]!): IndexResult!' + const options = await composeOptions(t, [ + indexingSubgraph('books', ['BOOK'], [], { field }), + indexingSubgraph('reviews', ['REVIEW'], [], { field }) + ], route) + + await assert.rejects(compose(options), /Cannot compose Mutation\.reindex .*no enum argument/) + }) + test('should fail to compose when more than one enum argument could route a shared field', async (t) => { - const lookup = (kind, region) => ({ + const lookup = (owner, kind, region) => ({ + name: owner, mercurius: { schema: ` enum Kind { ${kind} } @@ -161,12 +294,76 @@ test.describe('same-named root fields across subgraphs', () => { }, listen: true }) - const { options } = await composeServices(t, [ - { name: 'one', ...lookup('A', 'EU') }, - { name: 'two', ...lookup('B', 'US') } - ]) + const options = await composeOptions(t, [lookup('one', 'A', 'EU'), lookup('two', 'B', 'US')], route) await assert.rejects(compose(options), /Cannot compose Query\.lookup .* more than one enum argument could route it \(kind, region\)/) }) + + test('should fail to compose when the subgraphs disagree on the return type', async (t) => { + const options = await composeOptions(t, [ + indexingSubgraph('books', ['BOOK'], []), + indexingSubgraph('reviews', ['REVIEW'], [], { field: 'reindex(entity: IndexedEntity!): IndexCount!', result: 'type IndexCount { count: Int! }' }) + ], route) + + await assert.rejects(compose(options), + /Cannot compose Mutation\.reindex .* the signatures differ \(books: \(entity: IndexedEntity!\): IndexResult!; reviews: \(entity: IndexedEntity!\): IndexCount!\)/) + }) + + test('should fail to compose when one subgraph declares an extra argument', async (t) => { + const options = await composeOptions(t, [ + indexingSubgraph('books', ['BOOK'], [], { field: 'reindex(entity: IndexedEntity!, force: Boolean): IndexResult!' }), + indexingSubgraph('reviews', ['REVIEW'], []) + ], route) + + await assert.rejects(compose(options), /the signatures differ \(books: \(entity: IndexedEntity!, force: Boolean\): IndexResult!; reviews: \(entity: IndexedEntity!\): IndexResult!\)/) + }) + + test('should fail to compose when the subgraphs disagree on the nullability of the routing argument', async (t) => { + const options = await composeOptions(t, [ + indexingSubgraph('books', ['BOOK'], []), + indexingSubgraph('reviews', ['REVIEW'], [], { field: 'reindex(entity: IndexedEntity = REVIEW): IndexResult!' }) + ], route) + + await assert.rejects(compose(options), /the signatures differ/) + }) + + test('should fail to compose when a subgraph that does not publish the field adds values to the enum', async (t) => { + const stats = { + name: 'stats', + mercurius: { + schema: 'enum IndexedEntity { COMMENT } type Query { statsCount(entity: IndexedEntity!): Int }', + resolvers: { Query: { statsCount: () => 0 } } + }, + listen: true + } + const options = await composeOptions(t, [ + indexingSubgraph('books', ['BOOK'], []), + indexingSubgraph('reviews', ['REVIEW'], []), + stats + ], route) + + await assert.rejects(compose(options), + /Cannot compose Mutation\.reindex .* enum IndexedEntity has values declared by subgraphs that do not publish the field, COMMENT \(stats\)/) + }) +}) + +test.describe('same-named object types', () => { + const pizza = (owner, id, extra) => ({ + name: owner, + mercurius: { + schema: `type Pizza { id: ${id} ${extra} } type Query { ${owner}Pizza: Pizza }`, + resolvers: { Query: { [`${owner}Pizza`]: () => null } } + }, + listen: true + }) + + for (const [strategy, id] of [['first', 'ID'], ['route', 'ID'], ['error', 'ID'], ['last', 'Int!']]) { + test(`should publish each field once, the ${strategy === 'last' ? 'last' : 'first'} declaration of a shared one under "${strategy}"`, async (t) => { + const options = await composeOptions(t, [pizza('a', 'ID', 'name: String'), pizza('b', 'Int!', 'size: Int')], { onConflict: strategy }) + const composer = await compose(options) + + assert.match(composer.toSdl(), new RegExp(`type Pizza \\{\\n {2}id: ${id.replace('!', '\\!')}\\n {2}name: String\\n {2}size: Int\\n\\}`)) + }) + } }) From 3e73b68773753a22af1c6b305d7722a9e4966a3f Mon Sep 17 00:00:00 2001 From: Simon Lind Date: Fri, 25 Sep 2026 19:52:08 +0000 Subject: [PATCH 3/4] feat: keep the previous merge when onConflict is not set An unset option keeps the composer's behaviour as it was: the first subgraph's values for a shared enum, the concatenated field lists for a shared object type, and the last subgraph's resolver for a shared root field. The only addition is a warning that names the field and the subgraphs and points at the option, so upgrading is not a breaking change. The four strategies are opt-in; the default can move in a future major version. Co-Authored-By: Claude Fable 5.1 --- README.md | 7 +++++-- lib/composer.js | 12 ++++++++---- lib/fields.js | 12 +++++++++--- lib/validation.js | 6 +++--- test/shared-root-fields.test.js | 31 +++++++++++++++++++++++++++++-- 5 files changed, 54 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 8f78788..140a1ae 100644 --- a/README.md +++ b/README.md @@ -180,13 +180,16 @@ Subgraphs may publish types and root fields with the same name. The `onConflict` | `onConflict` | same-named enum | field both declare on a same-named object type | shared `Query` / `Mutation` field | | --- | --- | --- | --- | -| `'error'` (default) | union of the values | first declaration | composition fails, naming the field and the subgraphs | +| unset (default) | first subgraph's values | last declaration | last subgraph's resolver, with a warning | +| `'error'` | union of the values | first declaration | composition fails, naming the field and the subgraphs | | `'first'` | first subgraph's values | first declaration | first subgraph's resolver, with a warning | | `'last'` | last subgraph's values | last declaration | last subgraph's resolver, with a warning | | `'route'` | union of the values | first declaration | routed to one subgraph by an enum argument | "First" and "last" follow the order of `subgraphs`. A merged enum value keeps the description of the subgraph that declared it. +The default keeps the merge as it was before the option existed. Note that it is inconsistent: a shared enum keeps the first subgraph's values while a shared root field is answered by the last subgraph, so a call can validate against one subgraph and run against another. Set `onConflict` to choose a consistent strategy; the default will change in a future major version. + ### Routing a shared root field With `onConflict: 'route'`, a `Query` or `Mutation` field published by more than one subgraph is routed by one of its arguments. Every subgraph must declare the field with the same return type and the same argument types, and one argument must be an enum, declared as `Enum!` or with a default value, whose values are declared by exactly one subgraph each. The value a client passes selects the subgraph that receives the call. @@ -264,7 +267,7 @@ Composition fails, rather than picking a subgraph silently, when a shared root f - `onSubgraphError` (function, optional) - Hook called when an error occurs getting schema from a subgraph. The default function will throw the error. The arguments are: - `error` (error) - The error. - `subgraph` (string) - The erroring subgraph name. - - `onConflict` (string, optional) - How a type or root field that more than one subgraph publishes is merged: `'error'`, `'first'`, `'last'` or `'route'`, see [same-named types across subgraphs](#same-named-types-across-subgraphs). **Default:** `'error'`. + - `onConflict` (string, optional) - How a type or root field that more than one subgraph publishes is merged: `'error'`, `'first'`, `'last'` or `'route'`, see [same-named types across subgraphs](#same-named-types-across-subgraphs). **Default:** unset, which keeps the merge as it was before the option existed. - `queryTypeName` (string, optional) - The name of the `Query` type in the composed schema. **Default:** `'Query'`. - `mutationTypeName` (string, optional) - The name of the `Mutation` type in the composed schema. **Default:** `'Mutation'`. diff --git a/lib/composer.js b/lib/composer.js index 6f67e6a..2686af7 100644 --- a/lib/composer.js +++ b/lib/composer.js @@ -482,10 +482,9 @@ class Composer { * - "route": every subgraph declares the field with the same signature and an enum argument * whose values are disjoint per subgraph; the merged enum is the union (see `mergeTypes`) and * the value a client passes picks the subgraph - * - * Without this pass the field silently kept the last subgraph's resolver while a shared enum - * kept the first subgraph's values, so a call could validate against one subgraph and run - * against another. + * - unset: as before the option existed, the last subgraph's resolver answers while a shared + * enum keeps the first subgraph's values, so a call can validate against one subgraph and run + * against another; a warning names the field and points at the option */ buildSharedFieldResolvers () { for (const fieldId of Object.keys(this.fields)) { @@ -498,6 +497,11 @@ class Composer { const fieldName = fieldId.slice(typeName.length + 1) const where = `${fieldId} is published by subgraphs ${subgraphNames.join(', ')}` + if (this.onConflict === undefined) { + this.logger.warn({ fieldId, subgraphs: subgraphNames }, `${where}: resolved by the last subgraph, set onConflict to choose how`) + continue + } + if (this.onConflict === 'error') { throw new Error(`Cannot compose ${where}: set onConflict to "route", "first" or "last" to resolve it`) } diff --git a/lib/fields.js b/lib/fields.js index fcc7ac8..c897773 100644 --- a/lib/fields.js +++ b/lib/fields.js @@ -11,14 +11,18 @@ const MUTATION_TYPE = 'MUTATION' * - OBJECT: the union of the fields by name; a field both declare is t2's under "last", t1's otherwise * - anything else: t1 * + * With no strategy set the merge is what it was before the option existed: t1 for an enum, and the + * two field lists concatenated for an object type (graphql-js then keeps the last of a duplicated + * field name when it builds the schema). + * * A field on Query or Mutation that more than one subgraph publishes is then resolved by * `Composer#buildSharedFieldResolvers`, following the same strategy. */ -function mergeTypes (t1, t2, onConflict = 'error') { +function mergeTypes (t1, t2, onConflict) { if (t1.src.kind === 'ENUM' && Array.isArray(t1.src.enumValues) && Array.isArray(t2.src.enumValues)) { if (onConflict === 'last') { t1.src.enumValues = t2.src.enumValues.slice() - } else if (onConflict !== 'first') { + } else if (onConflict === 'route' || onConflict === 'error') { t1.src.enumValues = unionByName(t1.src.enumValues, t2.src.enumValues) } return t1 @@ -27,7 +31,9 @@ function mergeTypes (t1, t2, onConflict = 'error') { if (t1.src.kind !== 'OBJECT' || !Array.isArray(t1.src.fields)) { return t1 } - t1.src.fields = unionByName(t1.src.fields, t2.src.fields ?? [], onConflict === 'last') + t1.src.fields = onConflict === undefined + ? t1.src.fields.concat(t2.src.fields) + : unionByName(t1.src.fields, t2.src.fields ?? [], onConflict === 'last') // TODO t1.fields = t1.fields.concat(t2.fields) // TODO fields.resolvers diff --git a/lib/validation.js b/lib/validation.js index 05313d5..e0c02d9 100644 --- a/lib/validation.js +++ b/lib/validation.js @@ -6,7 +6,6 @@ const nullLogger = require('abstract-logging') const DEFAULT_COMPOSE_ENDPOINT = '/.well-known/graphql-composition' const DEFAULT_GRAPHQL_ENDPOINT = '/graphql' const CONFLICT_STRATEGIES = ['first', 'last', 'route', 'error'] -const DEFAULT_CONFLICT_STRATEGY = 'error' function validateArray (value, name, defaultValue) { if (!value && defaultValue) { return defaultValue } @@ -57,8 +56,9 @@ function validateComposerOptions (options) { validateString(validatedOptions.queryTypeName, 'queryTypeName') validateString(validatedOptions.mutationTypeName, 'mutationTypeName') - validatedOptions.onConflict = options.onConflict ?? DEFAULT_CONFLICT_STRATEGY - if (!CONFLICT_STRATEGIES.includes(validatedOptions.onConflict)) { + // unset keeps the merge as it was before the option existed, see Composer#buildSharedFieldResolvers + validatedOptions.onConflict = options.onConflict + if (validatedOptions.onConflict !== undefined && !CONFLICT_STRATEGIES.includes(validatedOptions.onConflict)) { throw new TypeError(`onConflict must be one of ${CONFLICT_STRATEGIES.map(s => `"${s}"`).join(', ')}`) } validatedOptions.onSubgraphError = validateFunction(options.onSubgraphError, 'onSubgraphError', defaultOnError) diff --git a/test/shared-root-fields.test.js b/test/shared-root-fields.test.js index 0897c77..58f4a45 100644 --- a/test/shared-root-fields.test.js +++ b/test/shared-root-fields.test.js @@ -63,11 +63,31 @@ test.describe('onConflict option', () => { await assert.rejects(compose({ onConflict: 'merge' }), /onConflict must be one of "first", "last", "route", "error"/) }) - test('should fail to compose a shared root field by default', async (t) => { + // unset keeps what the composer did before the option: the first subgraph's enum values, the + // last subgraph's resolver, and now a warning that names the field + test('should keep the previous merge when the option is not set, with a warning', async (t) => { + const calls = { books: [], reviews: [] } + const lines = [] + const options = await composeOptions(t, [ + indexingSubgraph('books', ['BOOK'], calls.books), + indexingSubgraph('reviews', ['REVIEW'], calls.reviews) + ], { logger: collectingLogger(lines) }) + const { composer, service } = await createComposerService(t, { compose, options }) + + assert.match(composer.toSdl(), /enum IndexedEntity \{\n {2}"""books"""\n {2}BOOK\n\}/) + const warning = lines.find(line => line.level === 'warn') + assert.match(warning.args[1], /Mutation\.reindex is published by subgraphs books, reviews: resolved by the last subgraph, set onConflict to choose how/) + + const { errors } = await rawRequest(service, 'mutation { reindex(entity: BOOK) { indexed } }') + assert.match(errors[0].message, /"BOOK" does not exist in "IndexedEntity" enum/) + assert.deepStrictEqual(calls, { books: [], reviews: [] }) + }) + + test('"error" should fail to compose a shared root field', async (t) => { const options = await composeOptions(t, [ indexingSubgraph('books', ['BOOK'], []), indexingSubgraph('reviews', ['REVIEW'], []) - ]) + ], { onConflict: 'error' }) await assert.rejects(compose(options), /Cannot compose Mutation\.reindex is published by subgraphs books, reviews: set onConflict to "route", "first" or "last" to resolve it/) @@ -358,6 +378,13 @@ test.describe('same-named object types', () => { listen: true }) + test('should keep the previous merge when the option is not set: the last declaration of a shared field wins', async (t) => { + const options = await composeOptions(t, [pizza('a', 'ID', 'name: String'), pizza('b', 'Int!', 'size: Int')]) + const composer = await compose(options) + + assert.match(composer.toSdl(), /type Pizza \{\n {2}id: Int!\n {2}name: String\n {2}size: Int\n\}/) + }) + for (const [strategy, id] of [['first', 'ID'], ['route', 'ID'], ['error', 'ID'], ['last', 'Int!']]) { test(`should publish each field once, the ${strategy === 'last' ? 'last' : 'first'} declaration of a shared one under "${strategy}"`, async (t) => { const options = await composeOptions(t, [pizza('a', 'ID', 'name: String'), pizza('b', 'Int!', 'size: Int')], { onConflict: strategy }) From 8d0ad62cda82d1d23065077a35d7594c49be35e6 Mon Sep 17 00:00:00 2001 From: Simon Lind Date: Fri, 25 Sep 2026 19:57:20 +0000 Subject: [PATCH 4/4] docs: state the merge rules as they are Comments and README describe the mechanism that exists rather than narrating what changed; the test names say what unset does. Co-Authored-By: Claude Fable 5.1 --- README.md | 4 ++-- lib/composer.js | 35 ++++++++++----------------------- lib/fields.js | 17 ++++++---------- lib/validation.js | 2 +- test/shared-root-fields.test.js | 14 ++++++------- 5 files changed, 26 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index 140a1ae..4ddced5 100644 --- a/README.md +++ b/README.md @@ -188,7 +188,7 @@ Subgraphs may publish types and root fields with the same name. The `onConflict` "First" and "last" follow the order of `subgraphs`. A merged enum value keeps the description of the subgraph that declared it. -The default keeps the merge as it was before the option existed. Note that it is inconsistent: a shared enum keeps the first subgraph's values while a shared root field is answered by the last subgraph, so a call can validate against one subgraph and run against another. Set `onConflict` to choose a consistent strategy; the default will change in a future major version. +With the option unset, a shared enum keeps the first subgraph's values while a shared root field is answered by the last subgraph, so a call can validate against one subgraph and run against another. Set `onConflict` to choose a consistent strategy; the default will change in a future major version. ### Routing a shared root field @@ -267,7 +267,7 @@ Composition fails, rather than picking a subgraph silently, when a shared root f - `onSubgraphError` (function, optional) - Hook called when an error occurs getting schema from a subgraph. The default function will throw the error. The arguments are: - `error` (error) - The error. - `subgraph` (string) - The erroring subgraph name. - - `onConflict` (string, optional) - How a type or root field that more than one subgraph publishes is merged: `'error'`, `'first'`, `'last'` or `'route'`, see [same-named types across subgraphs](#same-named-types-across-subgraphs). **Default:** unset, which keeps the merge as it was before the option existed. + - `onConflict` (string, optional) - How a type or root field that more than one subgraph publishes is merged: `'error'`, `'first'`, `'last'` or `'route'`, see [same-named types across subgraphs](#same-named-types-across-subgraphs). **Default:** unset: the first subgraph's values for a shared enum and the last subgraph's resolver for a shared root field, with a warning. - `queryTypeName` (string, optional) - The name of the `Query` type in the composed schema. **Default:** `'Query'`. - `mutationTypeName` (string, optional) - The name of the `Mutation` type in the composed schema. **Default:** `'Mutation'`. diff --git a/lib/composer.js b/lib/composer.js index 2686af7..7487248 100644 --- a/lib/composer.js +++ b/lib/composer.js @@ -475,16 +475,9 @@ class Composer { } /** - * A Query or Mutation field published by more than one subgraph is resolved according to the - * `onConflict` strategy: - * - "error": composition fails, naming the field and the subgraphs - * - "first" / "last": that subgraph's resolver answers, and a warning names the others - * - "route": every subgraph declares the field with the same signature and an enum argument - * whose values are disjoint per subgraph; the merged enum is the union (see `mergeTypes`) and - * the value a client passes picks the subgraph - * - unset: as before the option existed, the last subgraph's resolver answers while a shared - * enum keeps the first subgraph's values, so a call can validate against one subgraph and run - * against another; a warning names the field and points at the option + * resolve a Query or Mutation field that more than one subgraph publishes, by `onConflict`; + * unset warns and takes the last subgraph's resolver while `mergeTypes` keeps the first + * subgraph's enum values, so a call can validate against one subgraph and run against another */ buildSharedFieldResolvers () { for (const fieldId of Object.keys(this.fields)) { @@ -522,11 +515,8 @@ class Composer { } } - /** - * a routed field is validated once, against the merged schema, then sent to one subgraph: - * the subgraphs must agree on the return type and on every argument's type, or a call that - * passes validation can fail in the subgraph that receives it - */ + // a routed call is validated against the merged schema, which carries the first publisher's + // signature, then sent to whichever subgraph owns the value: a valid call must be valid there too assertSameSignature (fieldId, subgraphNames, where) { const signatures = new Map() for (const subgraphName of subgraphNames) { @@ -539,13 +529,9 @@ class Composer { } } - /** - * an argument the client leaves out is filled from the merged schema's default, which is the - * first subgraph's, and sent without a value so the receiving subgraph applies its own: the - * defaults must agree. The routing argument is the exception: each subgraph can only default - * it to a value it declares, and a call left to the merged default is routed to that very - * subgraph. - */ + // an argument the client leaves out is sent without a value, so the receiving subgraph applies + // its own default: they must agree. The routing argument is exempt: a subgraph can only default + // it to a value it owns, and a call left to the merged (first) default is routed there assertSameDefaults (fieldId, subgraphNames, where, routingArgName) { const defaults = new Map() for (const subgraphName of subgraphNames) { @@ -612,9 +598,8 @@ class Composer { throw new Error(`Cannot compose ${where}: no enum argument with a distinct set of values per subgraph to route by${hint}`) } - // the merged enum is the union over EVERY subgraph declaring it, not only the field's - // publishers, and a value nobody publishing the field declares would pass validation - // and then have nowhere to go + // the merged enum spans every subgraph declaring it, not only the field's publishers: a value + // from elsewhere passes validation with no subgraph to go to const { argName, enumName, routes } = candidates[0] const unroutable = [] for (const subgraphName of Object.keys(this.types[enumName])) { diff --git a/lib/fields.js b/lib/fields.js index c897773..072b165 100644 --- a/lib/fields.js +++ b/lib/fields.js @@ -4,19 +4,14 @@ const QUERY_TYPE = 'QUERY' const MUTATION_TYPE = 'MUTATION' /** - * merge two definitions of the same type coming from different subgraphs, t1 being the merged - * definition so far and t2 the next subgraph's one, according to the `onConflict` strategy - * - ENUM: "first" keeps t1's values, "last" takes t2's, "route" and "error" take the union by name, - * each value keeping the description of the subgraph that declared it + * merge two definitions of the same type from different subgraphs, t1 the merged one so far and + * t2 the next subgraph's, by `onConflict` + * - ENUM: "first" keeps t1's values, "last" takes t2's, "route" and "error" take the union by name * - OBJECT: the union of the fields by name; a field both declare is t2's under "last", t1's otherwise + * - unset: t1 for an enum; for an object type the field lists concatenated, and graphql-js keeps + * the last of a duplicated name when it builds the schema * - anything else: t1 - * - * With no strategy set the merge is what it was before the option existed: t1 for an enum, and the - * two field lists concatenated for an object type (graphql-js then keeps the last of a duplicated - * field name when it builds the schema). - * - * A field on Query or Mutation that more than one subgraph publishes is then resolved by - * `Composer#buildSharedFieldResolvers`, following the same strategy. + * A Query or Mutation field both publish is resolved by `Composer#buildSharedFieldResolvers`. */ function mergeTypes (t1, t2, onConflict) { if (t1.src.kind === 'ENUM' && Array.isArray(t1.src.enumValues) && Array.isArray(t2.src.enumValues)) { diff --git a/lib/validation.js b/lib/validation.js index e0c02d9..da40a60 100644 --- a/lib/validation.js +++ b/lib/validation.js @@ -56,7 +56,7 @@ function validateComposerOptions (options) { validateString(validatedOptions.queryTypeName, 'queryTypeName') validateString(validatedOptions.mutationTypeName, 'mutationTypeName') - // unset keeps the merge as it was before the option existed, see Composer#buildSharedFieldResolvers + // unset is a valid choice, see Composer#buildSharedFieldResolvers validatedOptions.onConflict = options.onConflict if (validatedOptions.onConflict !== undefined && !CONFLICT_STRATEGIES.includes(validatedOptions.onConflict)) { throw new TypeError(`onConflict must be one of ${CONFLICT_STRATEGIES.map(s => `"${s}"`).join(', ')}`) diff --git a/test/shared-root-fields.test.js b/test/shared-root-fields.test.js index 58f4a45..5d930f3 100644 --- a/test/shared-root-fields.test.js +++ b/test/shared-root-fields.test.js @@ -7,9 +7,9 @@ const { createGraphqlServices, createComposerService, graphqlRequest } = require const { compose } = require('../') // Two subgraphs publish the same Mutation field with the same enum argument, each declaring its -// own values. How the composer resolves that is the `onConflict` option: "error" (the default) -// refuses, "first" and "last" pick a subgraph, "route" merges the enum and sends each call to the -// subgraph that declares the value the client passed. +// own values. How the composer resolves that is the `onConflict` option: unset (the default) warns +// and takes the last subgraph's resolver, "error" refuses, "first" and "last" pick a subgraph, +// "route" merges the enum and sends each call to the subgraph that declares the value passed. function indexingSubgraph (owner, entities, calls, { field = 'reindex(entity: IndexedEntity!): IndexResult!', result = 'type IndexResult { indexed: Int! }' } = {}) { const values = entities.map(entity => `"""${owner}""" ${entity}`).join(' ') return { @@ -63,9 +63,9 @@ test.describe('onConflict option', () => { await assert.rejects(compose({ onConflict: 'merge' }), /onConflict must be one of "first", "last", "route", "error"/) }) - // unset keeps what the composer did before the option: the first subgraph's enum values, the - // last subgraph's resolver, and now a warning that names the field - test('should keep the previous merge when the option is not set, with a warning', async (t) => { + // unset: the first subgraph's enum values, the last subgraph's resolver, and a warning that + // names the field + test('should take the first enum and the last resolver when the option is not set, with a warning', async (t) => { const calls = { books: [], reviews: [] } const lines = [] const options = await composeOptions(t, [ @@ -378,7 +378,7 @@ test.describe('same-named object types', () => { listen: true }) - test('should keep the previous merge when the option is not set: the last declaration of a shared field wins', async (t) => { + test('should publish each field once, the last declaration of a shared one, when the option is not set', async (t) => { const options = await composeOptions(t, [pizza('a', 'ID', 'name: String'), pizza('b', 'Int!', 'size: Int')]) const composer = await compose(options)