diff --git a/composition-js/src/__tests__/compose.directiveArgumentMergeStrategies.test.ts b/composition-js/src/__tests__/compose.directiveArgumentMergeStrategies.test.ts index b1be5c54c..c83466e93 100644 --- a/composition-js/src/__tests__/compose.directiveArgumentMergeStrategies.test.ts +++ b/composition-js/src/__tests__/compose.directiveArgumentMergeStrategies.test.ts @@ -89,17 +89,19 @@ describe('composition of directive with non-trivial argument strategies', () => t: 2, k: 1, b: 4, }, }, { - name: 'sum', - type: (schema: Schema) => new NonNullType(schema.intType()), - compositionStrategy: ARGUMENT_COMPOSITION_STRATEGIES.SUM, - argValues: { - s1: { t: 3, k: 1 }, - s2: { t: 2, k: 5, b: 4 }, - }, - resultValues: { - t: 5, k: 6, b: 4, - }, - }, { + // NOTE: See the note for the SUM strategy in argumentCompositionStrategies.ts + // for more information on why this is commented out. + // name: 'sum', + // type: (schema: Schema) => new NonNullType(schema.intType()), + // compositionStrategy: ARGUMENT_COMPOSITION_STRATEGIES.SUM, + // argValues: { + // s1: { t: 3, k: 1 }, + // s2: { t: 2, k: 5, b: 4 }, + // }, + // resultValues: { + // t: 5, k: 6, b: 4, + // }, + // }, { name: 'intersection', type: (schema: Schema) => new NonNullType(new ListType(new NonNullType(schema.stringType()))), compositionStrategy: ARGUMENT_COMPOSITION_STRATEGIES.INTERSECTION, @@ -219,7 +221,7 @@ describe('composition of directive with non-trivial argument strategies', () => const s = result.schema; expect(directiveStrings(s.schemaDefinition, name)).toStrictEqual([ - `@link(url: "https://specs.apollo.dev/${name}/v0.1", import: ["@${name}"])` + `@link(url: "https://specs.apollo.dev/${name}/v0.1")` ]); const t = s.type('T') as ObjectType; diff --git a/composition-js/src/merging/merge.ts b/composition-js/src/merging/merge.ts index 294e54cdc..01b3430dd 100644 --- a/composition-js/src/merging/merge.ts +++ b/composition-js/src/merging/merge.ts @@ -31,7 +31,6 @@ import { CompositeType, Subgraphs, JOIN_VERSIONS, - INACCESSIBLE_VERSIONS, NamedSchemaElement, errorCauses, isObjectType, @@ -69,7 +68,6 @@ import { CoreSpecDefinition, FeatureVersion, FEDERATION_VERSIONS, - InaccessibleSpecDefinition, LinkDirectiveArgs, sourceIdentity, FeatureUrl, @@ -81,6 +79,10 @@ import { isNullableType, isFieldDefinition, Post20FederationDirectiveDefinition, + DirectiveCompositionSpecification, + FeatureDefinition, + CoreImport, + inaccessibleIdentity, } from "@apollo/federation-internals"; import { ASTNode, GraphQLError, DirectiveLocation } from "graphql"; import { @@ -345,6 +347,12 @@ interface OverrideArgs { label?: string; } +interface MergedDirectiveInfo { + definition: DirectiveDefinition; + argumentsMerger?: ArgumentMerger; + staticArgumentTransform?: StaticArgumentsTransform; +} + class Merger { readonly names: readonly string[]; readonly subgraphsSchema: readonly Schema[]; @@ -353,7 +361,8 @@ class Merger { readonly merged: Schema = new Schema(); readonly subgraphNamesToJoinSpecName: Map; readonly mergedFederationDirectiveNames = new Set(); - readonly mergedFederationDirectiveInSupergraph = new Map(); + readonly mergedFederationDirectiveInSupergraphByDirectiveName = + new Map(); readonly enumUsages = new Map(); private composeDirectiveManager: ComposeDirectiveManager; private mismatchReporter: MismatchReporter; @@ -364,7 +373,7 @@ class Merger { }[]; private joinSpec: JoinSpecDefinition; private linkSpec: CoreSpecDefinition; - private inaccessibleSpec: InaccessibleSpecDefinition; + private inaccessibleDirectiveInSupergraph?: DirectiveDefinition; private latestFedVersionUsed: FeatureVersion; private joinDirectiveIdentityURLs = new Set(); private schemaToImportNameToFeatureUrl = new Map>(); @@ -375,7 +384,6 @@ class Merger { this.latestFedVersionUsed = this.getLatestFederationVersionUsed(); this.joinSpec = JOIN_VERSIONS.getMinimumRequiredVersion(this.latestFedVersionUsed); this.linkSpec = LINK_VERSIONS.getMinimumRequiredVersion(this.latestFedVersionUsed); - this.inaccessibleSpec = INACCESSIBLE_VERSIONS.getMinimumRequiredVersion(this.latestFedVersionUsed); this.fieldsWithFromContext = this.getFieldsWithFromContextDirective(); this.fieldsWithOverride = this.getFieldsWithOverrideDirective(); @@ -470,59 +478,127 @@ class Merger { assert(errors.length === 0, "We shouldn't have errors adding the join spec to the (still empty) supergraph schema"); const directivesMergeInfo = collectCoreDirectivesToCompose(this.subgraphs); - for (const mergeInfo of directivesMergeInfo) { - this.validateAndMaybeAddSpec(mergeInfo); - } - + this.validateAndMaybeAddSpecs(directivesMergeInfo); return this.joinSpec.populateGraphEnum(this.merged, this.subgraphs); } - private validateAndMaybeAddSpec({url, name, definitionsPerSubgraph, compositionSpec}: CoreDirectiveInSubgraphs) { - // Not composition specification means that it shouldn't be composed. - if (!compositionSpec) { - return; - } - - let nameInSupergraph: string | undefined; - for (const subgraph of this.subgraphs) { - const directive = definitionsPerSubgraph.get(subgraph.name); - if (!directive) { - continue; + private validateAndMaybeAddSpecs(directivesMergeInfo: CoreDirectiveInSubgraphs[]) { + const supergraphInfoByIdentity = new Map< + string, + { + specInSupergraph: FeatureDefinition; + directives: { + nameInFeature: string; + nameInSupergraph: string; + compositionSpec: DirectiveCompositionSpecification; + }[]; } + >; - if (!nameInSupergraph) { - nameInSupergraph = directive.name; - } else if (nameInSupergraph !== directive.name) { - this.mismatchReporter.reportMismatchError( - ERRORS.LINK_IMPORT_NAME_MISMATCH, - `The "@${name}" directive (from ${url}) is imported with mismatched name between subgraphs: it is imported as `, - directive, - sourcesFromArray(this.subgraphs.values().map((s) => definitionsPerSubgraph.get(s.name))), - (def) => `"@${def.name}"`, - ); + for (const {url, name, definitionsPerSubgraph, compositionSpec} of directivesMergeInfo) { + // No composition specification means that it shouldn't be composed. + if (!compositionSpec) { return; } + + let nameInSupergraph: string | undefined; + for (const subgraph of this.subgraphs) { + const directive = definitionsPerSubgraph.get(subgraph.name); + if (!directive) { + continue; + } + + if (!nameInSupergraph) { + nameInSupergraph = directive.name; + } else if (nameInSupergraph !== directive.name) { + this.mismatchReporter.reportMismatchError( + ERRORS.LINK_IMPORT_NAME_MISMATCH, + `The "@${name}" directive (from ${url}) is imported with mismatched name between subgraphs: it is imported as `, + directive, + sourcesFromArray(this.subgraphs.values().map((s) => definitionsPerSubgraph.get(s.name))), + (def) => `"@${def.name}"`, + ); + return; + } + } + + // If we get here with `nameInSupergraph` unset, it means there is no usage for the directive at all and we + // don't bother adding the spec to the supergraph. + if (nameInSupergraph) { + const specInSupergraph = compositionSpec.supergraphSpecification(this.latestFedVersionUsed); + let supergraphInfo = supergraphInfoByIdentity.get(specInSupergraph.url.identity); + if (supergraphInfo) { + assert( + specInSupergraph.url.equals(supergraphInfo.specInSupergraph.url), + `Spec ${specInSupergraph.url} directives disagree on version for supergraph`, + ); + } else { + supergraphInfo = { + specInSupergraph, + directives: [], + }; + supergraphInfoByIdentity.set(specInSupergraph.url.identity, supergraphInfo); + } + supergraphInfo.directives.push({ + nameInFeature: name, + nameInSupergraph, + compositionSpec, + }); + } } - // If we get here with `nameInSupergraph` unset, it means there is no usage for the directive at all and we - // don't bother adding the spec to the supergraph. - if (nameInSupergraph) { - const specInSupergraph = compositionSpec.supergraphSpecification(this.latestFedVersionUsed); - const errors = this.linkSpec.applyFeatureAsLink(this.merged, specInSupergraph, specInSupergraph.defaultCorePurpose, [{ name, as: name === nameInSupergraph ? undefined : nameInSupergraph }], ); - assert(errors.length === 0, "We shouldn't have errors adding the join spec to the (still empty) supergraph schema"); - const feature = this.merged?.coreFeatures?.getByIdentity(specInSupergraph.url.identity); + for (const { specInSupergraph, directives } of supergraphInfoByIdentity.values()) { + const imports: CoreImport[] = []; + for (const { nameInFeature, nameInSupergraph } of directives) { + const defaultNameInSupergraph = CoreFeature.directiveNameInSchemaForCoreArguments( + specInSupergraph.url, + specInSupergraph.url.name, + [], + nameInFeature, + ); + if (nameInSupergraph !== defaultNameInSupergraph) { + imports.push(nameInFeature === nameInSupergraph + ? { name: `@${nameInFeature}` } + : { name: `@${nameInFeature}`, as: `@${nameInSupergraph}` } + ); + } + } + const errors = this.linkSpec.applyFeatureToSchema( + this.merged, + specInSupergraph, + undefined, + specInSupergraph.defaultCorePurpose, + imports, + ); + assert( + errors.length === 0, + "We shouldn't have errors adding the join spec to the (still empty) supergraph schema" + ); + const feature = this.merged.coreFeatures?.getByIdentity(specInSupergraph.url.identity); assert(feature, 'Should have found the feature we just added'); - const argumentsMerger = compositionSpec.argumentsMerger?.call(null, this.merged, feature); - if (argumentsMerger instanceof GraphQLError) { - // That would mean we made a mistake in the declaration of a hard-coded directive, so we just throw right away so this can be caught and corrected. - throw argumentsMerger; - } - this.mergedFederationDirectiveNames.add(nameInSupergraph); - this.mergedFederationDirectiveInSupergraph.set(name, { - definition: this.merged.directive(nameInSupergraph)!, - argumentsMerger, - staticArgumentTransform: compositionSpec.staticArgumentTransform, - }); + for (const { nameInFeature, nameInSupergraph, compositionSpec } of directives) { + const argumentsMerger = compositionSpec.argumentsMerger?.call(null, this.merged, feature); + if (argumentsMerger instanceof GraphQLError) { + // That would mean we made a mistake in the declaration of a hard-coded directive, + // so we just throw right away so this can be caught and corrected. + throw argumentsMerger; + } + this.mergedFederationDirectiveNames.add(nameInSupergraph); + this.mergedFederationDirectiveInSupergraphByDirectiveName.set(nameInSupergraph, { + definition: this.merged.directive(nameInSupergraph)!, + argumentsMerger, + staticArgumentTransform: compositionSpec.staticArgumentTransform, + }); + // If we encounter the @inaccessible directive, we need to record its + // definition so certain merge validations that care about @inaccessible + // can act accordingly. + if ( + specInSupergraph.identity === inaccessibleIdentity + && nameInFeature === specInSupergraph.url.name + ) { + this.inaccessibleDirectiveInSupergraph = this.merged.directive(nameInSupergraph)!; + } + } } } @@ -2464,8 +2540,8 @@ class Merger { this.recordAppliedDirectivesToMerge(valueSources, value); this.addJoinEnumValue(valueSources, value); - const inaccessibleInSupergraph = this.mergedFederationDirectiveInSupergraph.get(this.inaccessibleSpec.inaccessibleDirectiveSpec.name); - const isInaccessible = inaccessibleInSupergraph && value.hasAppliedDirective(inaccessibleInSupergraph.definition); + const isInaccessible = this.inaccessibleDirectiveInSupergraph + && value.hasAppliedDirective(this.inaccessibleDirectiveInSupergraph); // The merging strategy depends on the enum type usage: // - if it is _only_ used in position of Input type, we merge it with an "intersection" strategy (like other input types/things). // - if it is _only_ used in position of Output type, we merge it with an "union" strategy (like other output types/things). @@ -2562,8 +2638,6 @@ class Merger { } private mergeInput(inputSources: Sources, dest: InputObjectType) { - const inaccessibleInSupergraph = this.mergedFederationDirectiveInSupergraph.get(this.inaccessibleSpec.inaccessibleDirectiveSpec.name); - // Like for other inputs, we add all the fields found in any subgraphs initially as a simple mean to have a complete list of // field to iterate over, but we will remove those that are not in all subgraphs. const added = this.addFieldsShallow(inputSources, dest); @@ -2572,7 +2646,8 @@ class Merger { // compatibility between definitions and 2) we actually want to see if the result is marked inaccessible or not and it makes // that easier. this.mergeInputField(subgraphFields, destField); - const isInaccessible = inaccessibleInSupergraph && destField.hasAppliedDirective(inaccessibleInSupergraph.definition); + const isInaccessible = this.inaccessibleDirectiveInSupergraph + && destField.hasAppliedDirective(this.inaccessibleDirectiveInSupergraph); // Note that if the field is manually marked @inaccessible, we can always accept it to be inconsistent between subgraphs since // it won't be exposed in the API, and we don't hint about it because we're just doing what the user is explicitely asking. if (!isInaccessible && someSources(subgraphFields, field => !field)) { @@ -2840,8 +2915,7 @@ class Merger { // is @inaccessible, which is necessary to exist in the supergraph for EnumValues to properly // determine whether the fact that a value is both input / output will matter private recordAppliedDirectivesToMerge(sources: Sources>, dest: SchemaElement) { - const inaccessibleInSupergraph = this.mergedFederationDirectiveInSupergraph.get(this.inaccessibleSpec.inaccessibleDirectiveSpec.name); - const inaccessibleName = inaccessibleInSupergraph?.definition.name; + const inaccessibleName = this.inaccessibleDirectiveInSupergraph?.name; const names = this.gatherAppliedDirectiveNames(sources); if (inaccessibleName && names.has(inaccessibleName)) { @@ -2905,7 +2979,7 @@ class Merger { return; } - const directiveInSupergraph = this.mergedFederationDirectiveInSupergraph.get(name); + const directiveInSupergraph = this.mergedFederationDirectiveInSupergraphByDirectiveName.get(name); if (dest.schema().directive(name)?.repeatable) { // For repeatable directives, we simply include each application found but with exact duplicates removed @@ -2945,7 +3019,7 @@ class Merger { if (differentApplications.length === 1) { dest.applyDirective(name, differentApplications[0].arguments(false)); } else { - const info = this.mergedFederationDirectiveInSupergraph.get(name); + const info = this.mergedFederationDirectiveInSupergraphByDirectiveName.get(name); if (info && info.argumentsMerger) { const mergedArguments = Object.create(null); const applicationsArguments = differentApplications.map((a) => a.arguments(true)); diff --git a/gateway-js/src/__tests__/gateway/lifecycle-hooks.test.ts b/gateway-js/src/__tests__/gateway/lifecycle-hooks.test.ts index 458175876..43b65e309 100644 --- a/gateway-js/src/__tests__/gateway/lifecycle-hooks.test.ts +++ b/gateway-js/src/__tests__/gateway/lifecycle-hooks.test.ts @@ -149,7 +149,7 @@ describe('lifecycle hooks', () => { // the supergraph (even just formatting differences), this ID will change // and this test will have to updated. expect(secondCall[0]!.compositionId).toMatchInlineSnapshot( - `"4aa2278e35df345ff5959a30546d2e9ef9e997204b4ffee4a42344b578b36068"`, + `"6dc1bde2b9818fabec62208c5d8825abaa1bae89635fa6f3a5ffea7b78fc6d82"`, ); // second call should have previous info in the second arg expect(secondCall[1]!.compositionId).toEqual(expectedFirstId); diff --git a/internals-js/src/__tests__/values.test.ts b/internals-js/src/__tests__/values.test.ts index eaab39aa7..6e1d167fb 100644 --- a/internals-js/src/__tests__/values.test.ts +++ b/internals-js/src/__tests__/values.test.ts @@ -414,5 +414,6 @@ describe('objectEquals tests', () => { expect(valueEquals({ foo: 'foo', bar: undefined }, { foo: 'foo' })).toBe( false, ); + expect(valueEquals({}, null)).toBe(false); }); }); diff --git a/internals-js/src/argumentCompositionStrategies.ts b/internals-js/src/argumentCompositionStrategies.ts index 85a870773..f9d60ef3f 100644 --- a/internals-js/src/argumentCompositionStrategies.ts +++ b/internals-js/src/argumentCompositionStrategies.ts @@ -13,87 +13,105 @@ export type ArgumentCompositionStrategy = { function supportFixedTypes(types: (schema: Schema) => InputType[]): TypeSupportValidator { return (schema, type) => { const supported = types(schema); - if (!supported.some((t) => sameType(t, type))) { - return { valid: false, supportedMsg: `type(s) ${supported.join(', ')}` }; - } - return { valid: true }; + return supported.some((t) => sameType(t, type)) + ? { valid: true } + : { valid: false, supportedMsg: `type(s) ${supported.join(', ')}` }; }; } function supportAnyNonNullArray(): TypeSupportValidator { - return (_, type) => { - if (!isNonNullType(type) || !isListType(type.ofType)) { - return { valid: false, supportedMsg: 'non nullable list types of any type'}; - } - return { valid: true }; + return (_, type) => isNonNullType(type) && isListType(type.ofType) + ? { valid: true } + : { valid: false, supportedMsg: 'non nullable list types of any type' } +} + +function supportAnyArray(): TypeSupportValidator { + return (_, type) => isListType(type) || (isNonNullType(type) && isListType(type.ofType)) + ? { valid: true } + : { valid: false, supportedMsg: 'list types of any type' }; +} + +// NOTE: This function makes the assumption that for the directive argument +// being merged, it is not "nullable with non-null default" in the supergraph +// schema (this kind of type/default combo is confusing and should be avoided, +// if possible). This assumption allows this function to replace null with +// undefined, which makes for a cleaner supergraph schema. +function mergeNullableValues( + mergeValues: (values: T[]) => T +): (values: (T | null | undefined)[]) => T | undefined { + return (values: (T | null | undefined)[]) => { + const nonNullValues = values.filter((v) => v !== null && v !== undefined) as T[]; + return nonNullValues.length > 0 + ? mergeValues(nonNullValues) + : undefined; }; } +function unionValues(values: any[]): any { + return values.reduce((acc, next) => { + const newValues = next.filter((v1: any) => !acc.some((v2: any) => valueEquals(v1, v2))); + return acc.concat(newValues); + }, []); +} + export const ARGUMENT_COMPOSITION_STRATEGIES = { MAX: { name: 'MAX', isTypeSupported: supportFixedTypes((schema: Schema) => [new NonNullType(schema.intType())]), - mergeValues: (values: any[]) => Math.max(...values), + mergeValues: (values: number[]) => Math.max(...values), }, MIN: { name: 'MIN', isTypeSupported: supportFixedTypes((schema: Schema) => [new NonNullType(schema.intType())]), - mergeValues: (values: any[]) => Math.min(...values), - }, - SUM: { - name: 'SUM', - isTypeSupported: supportFixedTypes((schema: Schema) => [new NonNullType(schema.intType())]), - mergeValues: (values: any[]) => values.reduce((acc, val) => acc + val, 0), + mergeValues: (values: number[]) => Math.min(...values), }, + // NOTE: This doesn't work today because directive applications are de-duped + // before being merged, we'd need to modify merge logic if we need this kind + // of behavior. + // SUM: { + // name: 'SUM', + // isTypeSupported: supportFixedTypes((schema: Schema) => [new NonNullType(schema.intType())]), + // mergeValues: (values: any[]) => values.reduce((acc, val) => acc + val, 0), + // }, INTERSECTION: { name: 'INTERSECTION', isTypeSupported: supportAnyNonNullArray(), - mergeValues: (values: any[]) => values.reduce((acc, val) => acc.filter((v1: any) => val.some((v2: any) => valueEquals(v1, v2))), values[0]), + mergeValues: (values: any[]) => values.reduce((acc, next) => { + if (acc === undefined) { + return next; + } else { + return acc.filter((v1: any) => next.some((v2: any) => valueEquals(v1, v2))); + } + }, undefined) ?? [], }, UNION: { name: 'UNION', isTypeSupported: supportAnyNonNullArray(), - mergeValues: (values: any[]) => - values.reduce((acc, val) => { - const newValues = val.filter((v1: any) => !acc.some((v2: any) => valueEquals(v1, v2))); - return acc.concat(newValues); - }, []), + mergeValues: unionValues, }, NULLABLE_AND: { name: 'NULLABLE_AND', - isTypeSupported: supportFixedTypes((schema: Schema) => [schema.booleanType()]), - mergeValues: (values: (boolean | null | undefined)[]) => values.reduce((acc, next) => { - if (acc === null || acc === undefined) { - return next; - } else if (next === null || next === undefined) { - return acc; - } else { - return acc && next; - } - }, undefined), + isTypeSupported: supportFixedTypes((schema: Schema) => [ + schema.booleanType(), + new NonNullType(schema.booleanType()) + ]), + mergeValues: mergeNullableValues( + (values: boolean[]) => values.every((v) => v) + ), }, NULLABLE_MAX: { name: 'NULLABLE_MAX', - isTypeSupported: supportFixedTypes((schema: Schema) => [schema.intType(), new NonNullType(schema.intType())]), - mergeValues: (values: any[]) => values.reduce((a: any, b: any) => a !== undefined && b !== undefined ? Math.max(a, b) : a ?? b, undefined), + isTypeSupported: supportFixedTypes((schema: Schema) => [ + schema.intType(), + new NonNullType(schema.intType()) + ]), + mergeValues: mergeNullableValues( + (values: number[]) => Math.max(...values) + ) }, NULLABLE_UNION: { name: 'NULLABLE_UNION', - isTypeSupported: (_: Schema, type: InputType) => ({ valid: isListType(type) }), - mergeValues: (values: any[]) => { - if (values.every((v) => v === undefined)) { - return undefined; - } - - const combined = new Set(); - for (const subgraphValues of values) { - if (Array.isArray(subgraphValues)) { - for (const value of subgraphValues) { - combined.add(value); - } - } - } - return Array.from(combined); - } + isTypeSupported: supportAnyArray(), + mergeValues: mergeNullableValues(unionValues), } } diff --git a/internals-js/src/definitions.ts b/internals-js/src/definitions.ts index e52013047..cdd8e09dd 100644 --- a/internals-js/src/definitions.ts +++ b/internals-js/src/definitions.ts @@ -984,12 +984,28 @@ export class CoreFeature { } directiveNameInSchema(name: string): string { - const elementImport = this.imports.find((i) => i.name.charAt(0) === '@' && i.name.slice(1) === name); + return CoreFeature.directiveNameInSchemaForCoreArguments( + this.url, + this.nameInSchema, + this.imports, + name, + ); + } + + static directiveNameInSchemaForCoreArguments( + specUrl: FeatureUrl, + specNameInSchema: string, + imports: CoreImport[], + directiveNameInSpec: string, + ): string { + const elementImport = imports.find((i) => + i.name.charAt(0) === '@' && i.name.slice(1) === directiveNameInSpec + ); return elementImport - ? (elementImport.as?.slice(1) ?? name) - : (name === this.url.name - ? this.nameInSchema - : this.nameInSchema + '__' + name + ? (elementImport.as?.slice(1) ?? directiveNameInSpec) + : (directiveNameInSpec === specUrl.name + ? specNameInSchema + : specNameInSchema + '__' + directiveNameInSpec ); } @@ -1064,7 +1080,7 @@ export class CoreFeatures { const feature = this.byAlias.get(splitted[0]); return feature ? { feature, - nameInFeature: splitted[1], + nameInFeature: splitted.slice(1).join('__'), isImported: false, } : undefined; } else { @@ -1076,7 +1092,7 @@ export class CoreFeatures { if ((as ?? name) === importName) { return { feature, - nameInFeature: name.slice(1), + nameInFeature: isDirective ? name.slice(1) : name, isImported: true, }; } @@ -1088,8 +1104,8 @@ export class CoreFeatures { if (directFeature && isDirective) { return { feature: directFeature, - nameInFeature: directFeature.imports.find(imp => imp.as === `@${element.name}`)?.name.slice(1) ?? element.name, - isImported: true, + nameInFeature: element.name, + isImported: false, }; } diff --git a/internals-js/src/extractSubgraphsFromSupergraph.ts b/internals-js/src/extractSubgraphsFromSupergraph.ts index 3d412d039..421c66bee 100644 --- a/internals-js/src/extractSubgraphsFromSupergraph.ts +++ b/internals-js/src/extractSubgraphsFromSupergraph.ts @@ -40,7 +40,7 @@ import { parseSelectionSet } from "./operations"; import fs from 'fs'; import path from 'path'; import { validateStringContainsBoolean } from "./utils"; -import { CONTEXT_VERSIONS, ContextSpecDefinition, DirectiveDefinition, FeatureUrl, FederationDirectiveName, SchemaElement, errorCauses, isFederationDirectiveDefinedInSchema, printErrors } from "."; +import { ContextSpecDefinition, CostSpecDefinition, SchemaElement, errorCauses, isFederationDirectiveDefinedInSchema, printErrors } from "."; function filteredTypes( supergraph: Schema, @@ -194,7 +194,7 @@ function typesUsedInFederationDirective(fieldSet: string | undefined, parentType } export function extractSubgraphsFromSupergraph(supergraph: Schema, validateExtractedSubgraphs: boolean = true): [Subgraphs, Map] { - const [coreFeatures, joinSpec] = validateSupergraph(supergraph); + const [coreFeatures, joinSpec, contextSpec, costSpec] = validateSupergraph(supergraph); const isFed1 = joinSpec.version.equals(new FeatureVersion(0, 1)); try { // We first collect the subgraphs (creating an empty schema that we'll populate next for each). @@ -224,13 +224,13 @@ export function extractSubgraphsFromSupergraph(supergraph: Schema, validateExtra } const types = filteredTypes(supergraph, joinSpec, coreFeatures.coreDefinition); - const originalDirectiveNames = getApolloDirectiveNames(supergraph); const args: ExtractArguments = { supergraph, subgraphs, joinSpec, + contextSpec, + costSpec, filteredTypes: types, - originalDirectiveNames, getSubgraph, getSubgraphEnumValue, }; @@ -293,8 +293,9 @@ type ExtractArguments = { supergraph: Schema, subgraphs: Subgraphs, joinSpec: JoinSpecDefinition, + contextSpec: ContextSpecDefinition | undefined, + costSpec: CostSpecDefinition | undefined, filteredTypes: NamedType[], - originalDirectiveNames: Record, getSubgraph: (application: Directive) => Subgraph | undefined, getSubgraphEnumValue: (subgraphName: string) => string } @@ -352,7 +353,9 @@ function addAllEmptySubgraphTypes(args: ExtractArguments): TypesInfo { const subgraph = getSubgraph(application); assert(subgraph, () => `Should have found the subgraph for ${application}`); const subgraphType = subgraph.schema.addType(newNamedType(type.kind, type.name)); - propagateDemandControlDirectives(type, subgraphType, subgraph, args.originalDirectiveNames); + if (args.costSpec) { + propagateDemandControlDirectives(type, subgraphType, subgraph, args.costSpec); + } } break; } @@ -401,17 +404,8 @@ function addEmptyType( } } } - - const coreFeatures = supergraph.coreFeatures; - assert(coreFeatures, 'Should have core features'); - const contextFeature = coreFeatures.getByIdentity(ContextSpecDefinition.identity); - let supergraphContextDirective: DirectiveDefinition<{ name: string}> | undefined; - if (contextFeature) { - const contextSpec = CONTEXT_VERSIONS.find(contextFeature.url.version); - assert(contextSpec, 'Should have context spec'); - supergraphContextDirective = contextSpec.contextDirective(supergraph); - } - + + const supergraphContextDirective = args.contextSpec?.contextDirective(supergraph); if (supergraphContextDirective) { const contextApplications = type.appliedDirectivesOf(supergraphContextDirective); // for every application, apply the context directive to the correct subgraph @@ -438,8 +432,6 @@ function extractObjOrItfContent(args: ExtractArguments, info: TypeInfo 1; for (const { type: subgraphType, subgraph } of subgraphsInfo.values()) { - addSubgraphField({ field, type: subgraphType, subgraph, isShareable, originalDirectiveNames }); + addSubgraphField({ + field, + type: subgraphType, + subgraph, + isShareable, + costSpec: args.costSpec + }); } } else { const isShareable = isObjectType(type) @@ -478,58 +478,21 @@ function extractObjOrItfContent(args: ExtractArguments, info: TypeInfo renamedCost` with the `@` prefix removed. - * - * If the directive is imported under its default name, that also results in an entry. So, - * ```graphql - * @link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@cost"]) - * ``` - * results in a map entry of `cost -> cost`. This duals as a way to check if a directive - * is included in the supergraph schema. - * - * **Important:** This map does _not_ include directives imported from identities other - * than `specs.apollo.dev`. This helps us avoid extracting directives to subgraphs - * when a custom directive's name conflicts with that of a default one. - */ -function getApolloDirectiveNames(supergraph: Schema): Record { - const originalDirectiveNames: Record = {}; - for (const linkDirective of supergraph.schemaDefinition.appliedDirectivesOf("link")) { - if (linkDirective.arguments().url && linkDirective.arguments().import) { - const url = FeatureUrl.maybeParse(linkDirective.arguments().url); - if (!url?.identity.includes("specs.apollo.dev")) { - continue; - } - - for (const importedDirective of linkDirective.arguments().import) { - if (importedDirective.name && importedDirective.as) { - originalDirectiveNames[importedDirective.name.replace('@', '')] = importedDirective.as.replace('@', ''); - } else if (typeof importedDirective === 'string') { - originalDirectiveNames[importedDirective.replace('@', '')] = importedDirective.replace('@', ''); + addSubgraphField({ + field, + type: subgraphType, + subgraph, isShareable, + joinFieldArgs, + costSpec: args.costSpec + }); } } } } - - return originalDirectiveNames; } function extractInputObjContent(args: ExtractArguments, info: TypeInfo[]) { const fieldDirective = args.joinSpec.fieldDirective(args.supergraph); - const originalDirectiveNames = args.originalDirectiveNames; for (const { type, subgraphsInfo } of info) { for (const field of type.fields()) { @@ -537,19 +500,30 @@ function extractInputObjContent(args: ExtractArguments, info: TypeInfo[]) { // This was added in join 0.3, so it can genuinely be undefined. const enumValueDirective = args.joinSpec.enumValueDirective(args.supergraph); - const originalDirectiveNames = args.originalDirectiveNames; for (const { type, subgraphsInfo } of info) { - for (const { type: subgraphType, subgraph } of subgraphsInfo.values()) { - propagateDemandControlDirectives(type, subgraphType, subgraph, originalDirectiveNames); + if (args.costSpec) { + for (const { type: subgraphType, subgraph } of subgraphsInfo.values()) { + propagateDemandControlDirectives(type, subgraphType, subgraph, args.costSpec); + } } for (const value of type.values) { @@ -678,20 +653,20 @@ function maybeDumpSubgraphSchema(subgraph: Subgraph): string { } } -function propagateDemandControlDirectives(source: SchemaElement, dest: SchemaElement, subgraph: Subgraph, originalDirectiveNames?: Record) { - const costDirectiveName = originalDirectiveNames?.[FederationDirectiveName.COST]; - if (costDirectiveName) { - const costDirective = source.appliedDirectivesOf(costDirectiveName).pop(); - if (costDirective) { - dest.applyDirective(subgraph.metadata().costDirective().name, costDirective.arguments()); +function propagateDemandControlDirectives(source: SchemaElement, dest: SchemaElement, subgraph: Subgraph, costSpec: CostSpecDefinition) { + const costDirective = costSpec.costDirective(source.schema()); + if (costDirective) { + const application = source.appliedDirectivesOf(costDirective)[0]; + if (application) { + dest.applyDirective(subgraph.metadata().costDirective().name, application.arguments()); } } - const listSizeDirectiveName = originalDirectiveNames?.[FederationDirectiveName.LIST_SIZE]; - if (listSizeDirectiveName) { - const listSizeDirective = source.appliedDirectivesOf(listSizeDirectiveName).pop(); - if (listSizeDirective) { - dest.applyDirective(subgraph.metadata().listSizeDirective().name, listSizeDirective.arguments()); + const listSizeDirective = costSpec.listSizeDirective(source.schema()); + if (listSizeDirective) { + const application = source.appliedDirectivesOf(listSizeDirective)[0]; + if (application) { + dest.applyDirective(subgraph.metadata().listSizeDirective().name, application.arguments()); } } } @@ -707,14 +682,14 @@ function addSubgraphField({ subgraph, isShareable, joinFieldArgs, - originalDirectiveNames, + costSpec, }: { field: FieldDefinition, type: ObjectType | InterfaceType, subgraph: Subgraph, isShareable: boolean, joinFieldArgs?: JoinFieldDirectiveArguments, - originalDirectiveNames?: Record, + costSpec?: CostSpecDefinition, }): FieldDefinition { const copiedFieldType = joinFieldArgs?.type ? decodeType(joinFieldArgs.type, subgraph.schema, subgraph.name) @@ -723,7 +698,9 @@ function addSubgraphField({ const subgraphField = type.addField(field.name, copiedFieldType); for (const arg of field.arguments()) { const argDef = subgraphField.addArgument(arg.name, copyType(arg.type!, subgraph.schema, subgraph.name), arg.defaultValue); - propagateDemandControlDirectives(arg, argDef, subgraph, originalDirectiveNames) + if (costSpec) { + propagateDemandControlDirectives(arg, argDef, subgraph, costSpec); + } } if (joinFieldArgs?.requires) { subgraphField.applyDirective(subgraph.metadata().requiresDirective(), {'fields': joinFieldArgs.requires}); @@ -769,7 +746,9 @@ function addSubgraphField({ subgraphField.applyDirective(subgraph.metadata().shareableDirective()); } - propagateDemandControlDirectives(field, subgraphField, subgraph, originalDirectiveNames); + if (costSpec) { + propagateDemandControlDirectives(field, subgraphField, subgraph, costSpec); + } return subgraphField; } @@ -779,13 +758,13 @@ function addSubgraphInputField({ type, subgraph, joinFieldArgs, - originalDirectiveNames, + costSpec, }: { field: InputFieldDefinition, type: InputObjectType, subgraph: Subgraph, joinFieldArgs?: JoinFieldDirectiveArguments, - originalDirectiveNames?: Record + costSpec?: CostSpecDefinition, }): InputFieldDefinition { const copiedType = joinFieldArgs?.type ? decodeType(joinFieldArgs?.type, subgraph.schema, subgraph.name) @@ -794,7 +773,9 @@ function addSubgraphInputField({ const inputField = type.addField(field.name, copiedType); inputField.defaultValue = field.defaultValue - propagateDemandControlDirectives(field, inputField, subgraph, originalDirectiveNames); + if (costSpec) { + propagateDemandControlDirectives(field, inputField, subgraph, costSpec); + } return inputField; } diff --git a/internals-js/src/specs/coreSpec.ts b/internals-js/src/specs/coreSpec.ts index 909769ef4..aaeb155cb 100644 --- a/internals-js/src/specs/coreSpec.ts +++ b/internals-js/src/specs/coreSpec.ts @@ -195,7 +195,8 @@ export type CoreDirectiveArgs = { url: undefined, feature: string, as?: string, - for?: string + for?: string, + import: undefined, } export type LinkDirectiveArgs = { @@ -203,7 +204,7 @@ export type LinkDirectiveArgs = { feature: undefined, as?: string, for?: string, - import?: (string | CoreImport)[] + import?: (string | CoreImport)[], } export type CoreOrLinkDirectiveArgs = CoreDirectiveArgs | LinkDirectiveArgs; @@ -539,36 +540,36 @@ export class CoreSpecDefinition extends FeatureDefinition { return feature.url.version; } - applyFeatureToSchema(schema: Schema, feature: FeatureDefinition, as?: string, purpose?: CorePurpose): GraphQLError[] { + applyFeatureToSchema( + schema: Schema, + feature: FeatureDefinition, + as?: string, + purpose?: CorePurpose, + imports?: CoreImport[], + ): GraphQLError[] { const coreDirective = this.coreDirective(schema); const args = { [this.urlArgName()]: feature.toString(), as, - } as CoreDirectiveArgs; - if (this.supportPurposes() && purpose) { - args.for = purpose; - } - schema.schemaDefinition.applyDirective(coreDirective, args); - return feature.addElementsToSchema(schema); - } - - applyFeatureAsLink(schema: Schema, feature: FeatureDefinition, purpose?: CorePurpose, imports?: CoreImport[]): GraphQLError[] { - const existing = schema.schemaDefinition.appliedDirectivesOf(linkDirectiveDefaultName).find((link) => link.arguments().url === feature.toString()); - if (existing) { - existing.remove(); + } as CoreOrLinkDirectiveArgs; + if (purpose) { + if (this.supportPurposes()) { + args.for = purpose; + } else { + return [new GraphQLError( + `Cannot apply feature ${feature} with purpose since the schema's @core/@link version does not support it.` + )]; + } } - - const coreDirective = this.coreDirective(schema); - const args: LinkDirectiveArgs = { - url: feature.toString(), - import: (existing?.arguments().import ?? []).concat(imports?.map((i) => i.as ? { name: `@${i.name}`, as: `@${i.as}` } : `@${i.name}`)), - feature: undefined, - }; - - if (this.supportPurposes() && purpose) { - args.for = purpose; + if (imports && imports.length > 0) { + if (this.supportImport()) { + args.import = imports.map(i => i.as ? i : i.name); + } else { + return [new GraphQLError( + `Cannot apply feature ${feature} with imports since the schema's @core/@link version does not support it.` + )]; + } } - schema.schemaDefinition.applyDirective(coreDirective, args); return feature.addElementsToSchema(schema); } diff --git a/internals-js/src/specs/costSpec.ts b/internals-js/src/specs/costSpec.ts index f6f1bda54..ff15c9aa5 100644 --- a/internals-js/src/specs/costSpec.ts +++ b/internals-js/src/specs/costSpec.ts @@ -1,7 +1,7 @@ import { DirectiveLocation } from 'graphql'; import { createDirectiveSpecification } from '../directiveAndTypeSpecification'; import { FeatureDefinition, FeatureDefinitions, FeatureUrl, FeatureVersion } from './coreSpec'; -import { ListType, NonNullType } from '../definitions'; +import { DirectiveDefinition, ListType, NonNullType, Schema } from '../definitions'; import { registerKnownFeature } from '../knownCoreFeatures'; import { ARGUMENT_COMPOSITION_STRATEGIES } from '../argumentCompositionStrategies'; @@ -41,6 +41,14 @@ export class CostSpecDefinition extends FeatureDefinition { supergraphSpecification: (fedVersion) => COST_VERSIONS.getMinimumRequiredVersion(fedVersion) })); } + + costDirective(schema: Schema): DirectiveDefinition | undefined { + return this.directive(schema, 'cost'); + } + + listSizeDirective(schema: Schema): DirectiveDefinition | undefined { + return this.directive(schema, 'listSize'); + } } export const COST_VERSIONS = new FeatureDefinitions(costIdentity) diff --git a/internals-js/src/supergraphs.ts b/internals-js/src/supergraphs.ts index 3f37a103a..da4d52751 100644 --- a/internals-js/src/supergraphs.ts +++ b/internals-js/src/supergraphs.ts @@ -1,7 +1,9 @@ import { DocumentNode, GraphQLError } from "graphql"; -import { ErrCoreCheckFailed, FeatureUrl, FeatureVersion } from "./specs/coreSpec"; import { CoreFeatures, Schema, sourceASTs } from "./definitions"; +import { ErrCoreCheckFailed, FeatureUrl, FeatureVersion } from "./specs/coreSpec"; import { joinIdentity, JoinSpecDefinition, JOIN_VERSIONS } from "./specs/joinSpec"; +import { CONTEXT_VERSIONS, ContextSpecDefinition } from "./specs/contextSpec"; +import { COST_VERSIONS, costIdentity, CostSpecDefinition } from "./specs/costSpec"; import { buildSchema, buildSchemaFromAST } from "./buildSchema"; import { extractSubgraphsNamesAndUrlsFromSupergraph, extractSubgraphsFromSupergraph } from "./extractSubgraphsFromSupergraph"; import { ERRORS } from "./error"; @@ -81,11 +83,17 @@ function checkFeatureSupport(coreFeatures: CoreFeatures, supportedFeatures: Set< } } -export function validateSupergraph(supergraph: Schema): [CoreFeatures, JoinSpecDefinition] { +export function validateSupergraph(supergraph: Schema): [ + CoreFeatures, + JoinSpecDefinition, + ContextSpecDefinition | undefined, + CostSpecDefinition | undefined, +] { const coreFeatures = supergraph.coreFeatures; if (!coreFeatures) { throw ERRORS.INVALID_FEDERATION_SUPERGRAPH.err("Invalid supergraph: must be a core schema"); } + const joinFeature = coreFeatures.getByIdentity(joinIdentity); if (!joinFeature) { throw ERRORS.INVALID_FEDERATION_SUPERGRAPH.err("Invalid supergraph: must use the join spec"); @@ -95,7 +103,27 @@ export function validateSupergraph(supergraph: Schema): [CoreFeatures, JoinSpecD throw ERRORS.INVALID_FEDERATION_SUPERGRAPH.err( `Invalid supergraph: uses unsupported join spec version ${joinFeature.url.version} (supported versions: ${JOIN_VERSIONS.versions().join(', ')})`); } - return [coreFeatures, joinSpec]; + + const contextFeature = coreFeatures.getByIdentity(ContextSpecDefinition.identity); + let contextSpec = undefined; + if (contextFeature) { + contextSpec = CONTEXT_VERSIONS.find(contextFeature.url.version); + if (!contextSpec) { + throw ERRORS.INVALID_FEDERATION_SUPERGRAPH.err( + `Invalid supergraph: uses unsupported context spec version ${contextFeature.url.version} (supported versions: ${CONTEXT_VERSIONS.versions().join(', ')})`); + } + } + + const costFeature = coreFeatures.getByIdentity(costIdentity); + let costSpec = undefined; + if (costFeature) { + costSpec = COST_VERSIONS.find(costFeature.url.version); + if (!costSpec) { + throw ERRORS.INVALID_FEDERATION_SUPERGRAPH.err( + `Invalid supergraph: uses unsupported cost spec version ${costFeature.url.version} (supported versions: ${COST_VERSIONS.versions().join(', ')})`); + } + } + return [coreFeatures, joinSpec, contextSpec, costSpec]; } export function isFed1Supergraph(supergraph: Schema): boolean { diff --git a/internals-js/src/values.ts b/internals-js/src/values.ts index 11b2968e5..62de2a08e 100644 --- a/internals-js/src/values.ts +++ b/internals-js/src/values.ts @@ -135,8 +135,10 @@ export function valueEquals(a: any, b: any): boolean { if (Array.isArray(a)) { return Array.isArray(b) && arrayValueEquals(a, b) ; } - if (typeof a === 'object') { - return typeof b === 'object' && objectEquals(a, b); + // Note that typeof null === 'object', so we have to manually rule that out + // here. + if (a !== null && typeof a === 'object') { + return b !== null && typeof b === 'object' && objectEquals(a, b); } return a === b; } @@ -224,8 +226,10 @@ function applyDefaultValues(value: any, type: InputType): any { if (fieldValue === undefined) { if (field.defaultValue !== undefined) { updated[field.name] = applyDefaultValues(field.defaultValue, field.type); - } else if (isNonNullType(field.type)) { - throw ERRORS.INVALID_GRAPHQL.err(`Field "${field.name}" of required type ${type} was not provided.`); + } else if (!isNonNullType(field.type)) { + updated[field.name] = null; + } else { + throw ERRORS.INVALID_GRAPHQL.err(`Required field "${field.name}" of type ${type} was not provided.`); } } else { updated[field.name] = applyDefaultValues(fieldValue, field.type); @@ -249,8 +253,12 @@ export function withDefaultValues(value: any, argument: ArgumentDefinition) throw buildError(`Cannot compute default value for argument ${argument} as the type is undefined`); } if (value === undefined) { - if (argument.defaultValue) { + if (argument.defaultValue !== undefined) { return applyDefaultValues(argument.defaultValue, argument.type); + } else if (!isNonNullType(argument.type)) { + return null; + } else { + throw ERRORS.INVALID_GRAPHQL.err(`Required argument "${argument.coordinate}" was not provided.`); } } return applyDefaultValues(value, argument.type);