diff --git a/composition-js/src/__tests__/compose.custom.test.ts b/composition-js/src/__tests__/compose.custom.test.ts new file mode 100644 index 000000000..092064f78 --- /dev/null +++ b/composition-js/src/__tests__/compose.custom.test.ts @@ -0,0 +1,431 @@ +import { composeServices } from '../compose'; +import { + Schema, +} from '@apollo/federation-internals'; +import gql from 'graphql-tag'; +import './matchers'; + +const expectDirectiveOnElement = (schema: Schema, location: string, directiveName: string, props?: { [key: string]: any }) => { + const elem = schema.elementByCoordinate(location); + expect(elem).toBeDefined(); + const directive = elem?.appliedDirectives.find(d => { + if (d.name !== directiveName) { + return false; + } + if (props) { + for (const prop in props) { + if (d.arguments()[prop] !== props[prop]) { + return false; + } + } + } + return true; + }); + expect(directive).toBeDefined(); +}; + +const expectNoDirectiveOnElement = (schema: Schema, location: string, directiveName: string) => { + const elem = schema.elementByCoordinate(location); + expect(elem).toBeDefined(); + const directive = elem?.appliedDirectives.find(d => d.name === directiveName); + expect(directive).toBeUndefined(); +}; + +describe('composing custom directives', () => { + it('executable directive successful merge, not explicitly exposed', () => { + const subgraphA = { + name: 'subgraphA', + typeDefs: gql` + directive @foo(name: String!) on QUERY | FIELD_DEFINITION + type Query { + a: User + } + + type User @key(fields: "id") { + id: Int + name: String @foo(name: "graphA") + } + `, + }; + + const subgraphB = { + name: 'subgraphB', + typeDefs: gql` + directive @foo(name: String!) on QUERY | FIELD_DEFINITION + type User @key(fields: "id") { + id: Int + description: String @foo(name: "graphB") + } + `, + }; + const result = composeServices([subgraphA, subgraphB]); + expect(result.errors).toBeUndefined(); + expect(result.hints?.length).toBe(0); + const { schema } = result; + expect(schema).toBeDefined(); + if (schema) { + expectNoDirectiveOnElement(schema, 'User.name', 'foo'); + expect(schema.directive('foo')?.name).toBe('foo'); + expect(schema.directive('foo')?.locations).toEqual(['QUERY']); + } + }); + + it('executable directive successful merge, explicitly exposed', () => { + const subgraphA = { + name: 'subgraphA', + typeDefs: gql` + directive @foo(name: String!) on QUERY | FIELD_DEFINITION + type Query { + a: User + } + + type User @key(fields: "id") { + id: Int + name: String @foo(name: "graphA") + } + `, + }; + + const subgraphB = { + name: 'subgraphB', + typeDefs: gql` + directive @foo(name: String!) on QUERY | FIELD_DEFINITION + type User @key(fields: "id") { + id: Int + description: String @foo(name: "graphB") + } + `, + }; + const result = composeServices([subgraphA, subgraphB], { exposeDirectives: ['@foo']}); + expect(result.errors).toBeUndefined(); + expect(result.hints?.length).toBe(0); + const { schema } = result; + expect(schema).toBeDefined(); + if (schema) { + expectDirectiveOnElement(schema, 'User.name', 'foo', { name: 'graphA'}); + expect(schema.directive('foo')?.name).toBe('foo'); + expect(schema.directive('foo')?.locations).toEqual(['QUERY', 'FIELD_DEFINITION']); + } + }); + + it('executable directive, conflicting definitions non-explicit', () => { + const subgraphA = { + name: 'subgraphA', + typeDefs: gql` + directive @foo(name: String!) on QUERY + type Query { + a: User + } + + type User @key(fields: "id") { + id: Int + name: String + } + `, + }; + + const subgraphB = { + name: 'subgraphB', + typeDefs: gql` + directive @foo(name: String!) on QUERY | FIELD_DEFINITION | OBJECT + type User @key(fields: "id") @foo(name: "objectB") { + id: Int + description: String @foo(name: "graphB") + } + `, + }; + const result = composeServices([subgraphA, subgraphB]); + expect(result.errors).toBeUndefined(); + expect(result.hints?.length).toBe(0); + const { schema } = result; + expect(schema).toBeDefined(); + if (schema) { + expectNoDirectiveOnElement(schema, 'User.name', 'foo'); + expect(schema.directive('foo')?.name).toBe('foo'); + expect(schema.directive('foo')?.locations).toEqual(['QUERY']); + } + }); + + it('executable directive, conflicting definitions explicit', () => { + const subgraphA = { + name: 'subgraphA', + typeDefs: gql` + directive @foo(name: String!) on QUERY + type Query { + a: User + } + + type User @key(fields: "id") { + id: Int + name: String + } + `, + }; + + const subgraphB = { + name: 'subgraphB', + typeDefs: gql` + directive @foo(name: String!) on QUERY | FIELD_DEFINITION | OBJECT + type User @key(fields: "id") @foo(name: "objectB") { + id: Int + description: String @foo(name: "graphB") + } + `, + }; + const result = composeServices([subgraphA, subgraphB], { exposeDirectives: ['@foo']}); + expect(result.errors).toBeUndefined(); + expect(result.hints?.length).toBe(0); + const { schema } = result; + expect(schema).toBeDefined(); + if (schema) { + expectDirectiveOnElement(schema, 'User.description', 'foo', { name: 'graphB'}); + expectDirectiveOnElement(schema, 'User', 'foo', { name: 'objectB'}); + expect(schema.directive('foo')?.name).toBe('foo'); + expect(schema.directive('foo')?.locations).toEqual(['QUERY', 'FIELD_DEFINITION', 'OBJECT']); + } + }); + + it('type-system directive, not exposed', () => { + const subgraphA = { + name: 'subgraphA', + typeDefs: gql` + directive @foo(name: String!) on FIELD_DEFINITION + type Query { + a: User + } + + type User @key(fields: "id") { + id: Int + name: String @foo(name: "graphA") + } + `, + }; + + const subgraphB = { + name: 'subgraphB', + typeDefs: gql` + directive @foo(name: String!) on FIELD_DEFINITION + type User @key(fields: "id") { + id: Int + description: String @foo(name: "graphB") + } + `, + }; + const result = composeServices([subgraphA, subgraphB]); + expect(result.errors).toBeUndefined(); + expect(result.hints?.length).toBe(0); + const { schema } = result; + expect(schema).toBeDefined(); + if (schema) { + expectNoDirectiveOnElement(schema, 'User.name', 'foo'); + } + }); + + it('type-system directive, explicitly exposed', () => { + const subgraphA = { + name: 'subgraphA', + typeDefs: gql` + directive @foo(name: String!) on FIELD_DEFINITION + type Query { + a: User + } + + type User @key(fields: "id") { + id: Int + name: String @foo(name: "graphA") + } + `, + }; + + const subgraphB = { + name: 'subgraphB', + typeDefs: gql` + directive @foo(name: String!) on FIELD_DEFINITION + type User @key(fields: "id") { + id: Int + description: String @foo(name: "graphB") + } + `, + }; + const result = composeServices([subgraphA, subgraphB], { exposeDirectives: ['@foo']}); + expect(result.errors).toBeUndefined(); + expect(result.hints?.length).toBe(0); + const { schema } = result; + expect(schema).toBeDefined(); + if (schema) { + expectDirectiveOnElement(schema, 'User.name', 'foo', { name: 'graphA'}); + } + }); + + it('type-system directive, repeatable sometimes', () => { + const subgraphA = { + name: 'subgraphA', + typeDefs: gql` + directive @foo(name: String!) repeatable on FIELD_DEFINITION + type Query { + a: User + } + + type User @key(fields: "id") { + id: Int + name: String @foo(name: "graphA") + } + `, + }; + + const subgraphB = { + name: 'subgraphB', + typeDefs: gql` + directive @foo(name: String!) on FIELD_DEFINITION + type User @key(fields: "id") { + id: Int + description: String @foo(name: "graphB") + } + `, + }; + const result = composeServices([subgraphA, subgraphB], { exposeDirectives: ['@foo']}); + expect(result.errors).toBeUndefined(); + expect(result.hints?.length).toBe(1); + expect(result.hints?.[0].definition.code).toBe('INCONSISTENT_TYPE_SYSTEM_DIRECTIVE_REPEATABLE'); + const { schema } = result; + expect(schema).toBeDefined(); + if (schema) { + expectDirectiveOnElement(schema, 'User.name', 'foo', { name: 'graphA'}); + } + }); + + it('type-system directive, different locations', () => { + const subgraphA = { + name: 'subgraphA', + typeDefs: gql` + directive @foo(name: String!) on FIELD_DEFINITION | OBJECT + type Query { + a: User + } + + type User @key(fields: "id") @foo(name: "objectA") { + id: Int + name: String @foo(name: "graphA") + } + `, + }; + + const subgraphB = { + name: 'subgraphB', + typeDefs: gql` + directive @foo(name: String!) on FIELD_DEFINITION + type User @key(fields: "id") { + id: Int + description: String @foo(name: "graphB") + } + `, + }; + const result = composeServices([subgraphA, subgraphB], { exposeDirectives: ['@foo']}); + expect(result.errors).toBeUndefined(); + expect(result.hints?.length).toBe(0); + const { schema } = result; + expect(schema).toBeDefined(); + if (schema) { + expectDirectiveOnElement(schema, 'User.name', 'foo', { name: 'graphA'}); + expectDirectiveOnElement(schema, 'User', 'foo', { name: 'objectA'}); + } + }); + + it('type-system directive, core feature, not exposed', () => { + const subgraphA = { + name: 'subgraphA', + typeDefs: gql` + extend 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/foo/v1.0", import: ["@foo"]) + + directive @foo(name: String!) on FIELD_DEFINITION + + type Query { + a: User + } + + type User @key(fields: "id") { + id: Int + name: String @foo(name: "graphA") + } + `, + }; + + const subgraphB = { + name: 'subgraphB', + typeDefs: gql` + extend 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/foo/v1.0", import: ["@foo"]) + + directive @foo(name: String!) on FIELD_DEFINITION + + type User @key(fields: "id") { + id: Int + description: String @foo(name: "graphB") + } + `, + }; + const result = composeServices([subgraphA, subgraphB]); + expect(result.errors).toBeUndefined(); + expect(result.hints?.length).toBe(0); + const { schema } = result; + expect(schema).toBeDefined(); + if (schema) { + expectNoDirectiveOnElement(schema, 'User.name', 'foo'); + } + }); + + it('type-system directive, core feature, explicitly exposed', () => { + const subgraphA = { + name: 'subgraphA', + typeDefs: gql` + extend 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/foo/v1.0", import: ["@foo"]) + + directive @foo(name: String!) on FIELD_DEFINITION + + type Query { + a: User + } + + type User @key(fields: "id") { + id: Int + name: String @foo(name: "graphA") + } + `, + }; + + const subgraphB = { + name: 'subgraphB', + typeDefs: gql` + extend 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/foo/v1.0", import: ["@foo"]) + + directive @foo(name: String!) on FIELD_DEFINITION + + type User @key(fields: "id") { + id: Int + description: String @foo(name: "graphB") + } + `, + }; + const result = composeServices([subgraphA, subgraphB], { exposeDirectives: ['@foo']}); + expect(result.errors).toBeUndefined(); + expect(result.hints?.length).toBe(0); + const { schema } = result; + expect(schema).toBeDefined(); + if (schema) { + expectDirectiveOnElement(schema, 'User.name', 'foo', { name: 'graphA'}); + } + }); + + it.todo('expose builtin directive'); +}); diff --git a/composition-js/src/compose.ts b/composition-js/src/compose.ts index d94160e71..4bfb56f7b 100644 --- a/composition-js/src/compose.ts +++ b/composition-js/src/compose.ts @@ -14,6 +14,7 @@ import { buildFederatedQueryGraph, buildSupergraphAPIQueryGraph } from "@apollo/ import { mergeSubgraphs } from "./merging"; import { validateGraphComposition } from "./validate"; import { CompositionHint } from "./hints"; +import { type CompositionOptions } from './types'; export type CompositionResult = CompositionFailure | CompositionSuccess; @@ -31,7 +32,7 @@ export interface CompositionSuccess { errors?: undefined; } -export function compose(subgraphs: Subgraphs): CompositionResult { +export function compose(subgraphs: Subgraphs, options?: CompositionOptions): CompositionResult { const upgradeResult = upgradeSubgraphsIfNecessary(subgraphs); if (upgradeResult.errors) { return { errors: upgradeResult.errors }; @@ -43,7 +44,7 @@ export function compose(subgraphs: Subgraphs): CompositionResult { return { errors: validationErrors }; } - const mergeResult = mergeSubgraphs(toMerge); + const mergeResult = mergeSubgraphs(toMerge, options); if (mergeResult.errors) { return { errors: mergeResult.errors }; } @@ -74,7 +75,7 @@ export function compose(subgraphs: Subgraphs): CompositionResult { }; } -export function composeServices(services: ServiceDefinition[]): CompositionResult { +export function composeServices(services: ServiceDefinition[], options?: CompositionOptions): CompositionResult { const subgraphs = subgraphsFromServiceList(services); if (Array.isArray(subgraphs)) { // Errors in subgraphs are not truly "composition" errors, but it's probably still the best place @@ -82,5 +83,5 @@ export function composeServices(services: ServiceDefinition[]): CompositionResul // include the subgraph name in their message. return { errors: subgraphs }; } - return compose(subgraphs); + return compose(subgraphs, options); } diff --git a/composition-js/src/merging/merge.ts b/composition-js/src/merging/merge.ts index 31bf15a6a..142870542 100644 --- a/composition-js/src/merging/merge.ts +++ b/composition-js/src/merging/merge.ts @@ -17,7 +17,6 @@ import { UnionType, sameType, isStrictSubtype, - SubtypingRule, ListType, NonNullType, Type, @@ -75,6 +74,7 @@ import { HintCodeDefinition, HINTS, } from "../hints"; +import { type CompositionOptions } from '../types'; const linkSpec = LINK_VERSIONS.latest(); type FieldOrUndefinedArray = (FieldDefinition | undefined)[]; @@ -84,11 +84,6 @@ const inaccessibleSpec = INACCESSIBLE_VERSIONS.latest(); export type MergeResult = MergeSuccess | MergeFailure; -// TODO: move somewhere else. -export type CompositionOptions = { - allowedFieldTypeMergingSubtypingRules?: SubtypingRule[] -} - // for each source, specify additional properties that validate functions can set class FieldMergeContext { _props: { usedOverridden: boolean, unusedOverridden: boolean }[]; @@ -248,6 +243,24 @@ function locationString(locations: DirectiveLocation[]): string { return (locations.length === 1 ? 'location ' : 'locations ') + '"' + locations.join(', ') + '"'; } +const getLocationsFromDirectiveDefs = (sources: (DirectiveDefinition | undefined)[]) => { + let consistentLocations = true; + const locationSet = new Set(); + sources + .filter((src): src is DirectiveDefinition => src !== undefined) + .forEach((src, idx) => { + const prevLength = locationSet.size; + src.locations.forEach(locationSet.add, locationSet); + if (idx > 0 && prevLength !== locationSet.size) { + consistentLocations = false; + } + }); + return { + consistentLocations, + locations: Array.from(locationSet), + }; +} + type EnumTypeUsagePosition = 'Input' | 'Output' | 'Both'; type EnumTypeUsage = { position: EnumTypeUsagePosition, @@ -353,6 +366,7 @@ class Merger { // supergraph. This allow to be able to reference those from that point on. this.addTypesShallow(); this.addDirectivesShallow(); + this.addCustomTypeSystemDirectives(); const typesToMerge = this.merged.types() .filter((type) => !linkSpec.isSpecType(type) && !joinSpec.isSpecType(type)); @@ -483,6 +497,40 @@ class Merger { } } + private addCustomTypeSystemDirectives() { + const directiveNameMap = this.exposedDirectives().reduce((acc, name: string) => { + acc[name] = []; + return acc; + }, {} as { [name: string]: DirectiveDefinition[] }); + for (const subgraph of this.subgraphsSchema) { + for (const directive of subgraph.allDirectives()) { + if (directive.name in directiveNameMap) { + directiveNameMap[directive.name].push(directive); + } + } + } + Object.entries(directiveNameMap).forEach(([name, definitions]) => { + // only add if the custom directive is not "executable" + // TODO: Should we generate a hint if definition.length === 0? + if (definitions.length > 0) { + let def: DirectiveDefinition | undefined; + // if it's an executable directive, we probably have already created it + if (definitions.some(d => d.locations.some(loc => executableDirectiveLocations.includes(loc)))) { + def = this.merged.directive(name); + assert(def, `could not find directive '@${name}'`); + const { locations } = getLocationsFromDirectiveDefs(definitions); + def.addLocations(...locations); + } else { + def = new DirectiveDefinition(name); + this.merged.addDirectiveDefinition(def); + this.mergeTypeSystemDirectiveDefinition(definitions, def); + } + } + }); + + // TODO: Check to make sure that these really are custom directives + } + private reportMismatchedTypeDefinitions(mismatchedType: string) { const supergraphType = this.merged.type(mismatchedType)!; this.reportMismatchError( @@ -1810,21 +1858,22 @@ class Merger { // definition is the intersection of all definitions (meaning that if there divergence in // locations, we only expose locations that are common everywhere). this.mergeDescription(sources, dest); + if (sources.some((s) => s && this.isMergedDirective(s))) { this.mergeExecutableDirectiveDefinition(sources, dest); } } // Note: as far as directive definition goes, we currently only merge directive having execution location, and only for - // thos locations. Any type system directive definition that propagates to the supergraph (graphQL built-ins and `@tag`) + // those locations. Any type system directive definition that propagates to the supergraph (graphQL built-ins and `@tag`) // is currently handled in an hard-coded way. This will change very soon however so keeping this code around to be // re-enabled by a future commit. - //private mergeTypeSystemDirectiveDefinition(sources: (DirectiveDefinition | undefined)[], dest: DirectiveDefinition) { - // this.addArgumentsShallow(sources, dest); - // for (const destArg of dest.arguments()) { - // const subgraphArgs = sources.map(f => f?.argument(destArg.name)); - // this.mergeArgument(subgraphArgs, destArg); - // } + private mergeTypeSystemDirectiveDefinition(sources: DirectiveDefinition[], dest: DirectiveDefinition) { + this.addArgumentsShallow(sources, dest); + for (const destArg of dest.arguments()) { + const subgraphArgs = sources.map(f => f?.argument(destArg.name)); + this.mergeArgument(subgraphArgs, destArg); + } // let repeatable: boolean | undefined = undefined; // let inconsistentRepeatable = false; @@ -1855,34 +1904,39 @@ class Merger { // }); // } // } - // dest.repeatable = repeatable!; - // dest.addLocations(...locations!); - - // if (inconsistentRepeatable) { - // this.reportMismatchHint( - // HINTS.INCONSISTENT_TYPE_SYSTEM_DIRECTIVE_REPEATABLE, - // `Type system directive "${dest}" is marked repeatable in the supergraph but it is inconsistently marked repeatable in subgraphs: `, - // dest, - // sources, - // directive => directive.repeatable ? 'yes' : 'no', - // // Note that the first callback is for element that are "like the supergraph". And the supergraph will be repeatable on inconsistencies. - // (_, subgraphs) => `it is repeatable in ${subgraphs}`, - // (_, subgraphs) => ` but not in ${subgraphs}`, - // ); - // } - // if (inconsistentLocations) { - // this.reportMismatchHint( - // HINTS.INCONSISTENT_TYPE_SYSTEM_DIRECTIVE_LOCATIONS, - // `Type system directive "${dest}" has inconsistent locations across subgraphs `, - // dest, - // sources, - // directive => locationString(this.extractLocations(directive)), - // // Note that the first callback is for element that are "like the supergraph". - // (locs, subgraphs) => `and will use ${locs} (union of all subgraphs) in the supergraph, but has: ${subgraphs ? `${locs} in ${subgraphs} and ` : ''}`, - // (locs, subgraphs) => `${locs} in ${subgraphs}`, - // ); - // } - //} + const repeatable = sources[0].repeatable; + const inconsistentRepeatable = sources.some(src => src.repeatable !== repeatable); + const { consistentLocations, locations } = getLocationsFromDirectiveDefs(sources); + + dest.repeatable = repeatable; + dest.addLocations(...locations); + + if (inconsistentRepeatable) { + this.reportMismatchHint( + HINTS.INCONSISTENT_TYPE_SYSTEM_DIRECTIVE_REPEATABLE, + `Type system directive "${dest}" is marked repeatable in the supergraph but it is inconsistently marked repeatable in subgraphs: `, + dest, + sources, + directive => directive.repeatable ? 'yes' : 'no', + // Note that the first callback is for element that are "like the supergraph". And the supergraph will be repeatable on inconsistencies. + (_, subgraphs) => `it is repeatable in ${subgraphs}`, + (_, subgraphs) => ` but not in ${subgraphs}`, + ); + } + if (!consistentLocations) { + console.log('mismatch hint'); + this.reportMismatchHint( + HINTS.INCONSISTENT_TYPE_SYSTEM_DIRECTIVE_LOCATIONS, + `Type system directive "${dest}" has inconsistent locations across subgraphs `, + dest, + sources, + directive => locationString(directive.locations as any), + // Note that the first callback is for element that are "like the supergraph". + (locs, subgraphs) => `and will use ${locs} (union of all subgraphs) in the supergraph, but has: ${subgraphs ? `${locs} in ${subgraphs} and ` : ''}`, + (locs, subgraphs) => `${locs} in ${subgraphs}`, + ); + } + } private mergeExecutableDirectiveDefinition(sources: (DirectiveDefinition | undefined)[], dest: DirectiveDefinition) { let repeatable: boolean | undefined = undefined; @@ -2003,7 +2057,7 @@ class Merger { for (const source of sources) { if (source) { for (const directive of source.appliedDirectives) { - if (this.isMergedDirective(directive)) { + if (this.isMergedDirective(directive) || this.exposedDirectives().includes(directive.name)) { names.add(directive.name); } } @@ -2286,4 +2340,8 @@ class Merger { : err; }); } + + private exposedDirectives() { + return (this.options.exposeDirectives ?? []).map(directive => directive.slice(1)); + } } diff --git a/composition-js/src/types.ts b/composition-js/src/types.ts new file mode 100644 index 000000000..3c1584296 --- /dev/null +++ b/composition-js/src/types.ts @@ -0,0 +1,8 @@ +import { + type SubtypingRule, +} from "@apollo/federation-internals"; + +export type CompositionOptions = { + exposeDirectives?: string[], + allowedFieldTypeMergingSubtypingRules?: SubtypingRule[], +}; diff --git a/gateway-js/src/supergraphManagers/IntrospectAndCompose/index.ts b/gateway-js/src/supergraphManagers/IntrospectAndCompose/index.ts index 4be9ab65e..4884a48d7 100644 --- a/gateway-js/src/supergraphManagers/IntrospectAndCompose/index.ts +++ b/gateway-js/src/supergraphManagers/IntrospectAndCompose/index.ts @@ -24,6 +24,7 @@ export interface IntrospectAndComposeOptions { pollIntervalInMs?: number; logger?: Logger; subgraphHealthCheck?: boolean; + exposeDirectives?: string[]; } type State = @@ -111,7 +112,7 @@ export class IntrospectAndCompose implements SupergraphManager { } private createSupergraphFromSubgraphList(subgraphs: ServiceDefinition[]) { - const compositionResult = composeServices(subgraphs); + const compositionResult = composeServices(subgraphs, { exposeDirectives: this.config.exposeDirectives }); if (compositionResult.errors) { const { errors } = compositionResult; diff --git a/internals-js/src/types.ts b/internals-js/src/types.ts index 4650573da..0353d721e 100644 --- a/internals-js/src/types.ts +++ b/internals-js/src/types.ts @@ -3,7 +3,7 @@ */ import { - AbstractType, + AbstractType, InterfaceType, isInterfaceType, isListType,