Skip to content

Implicitly upgrade federation version to the max required by any used directive #2929

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Feb 5, 2024
6 changes: 6 additions & 0 deletions .changeset/forty-dolls-type.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@apollo/composition": minor
"@apollo/federation-internals": minor
---

When a linked directive requires a federation version higher than the linked federation spec, upgrade with a hint
4 changes: 0 additions & 4 deletions composition-js/src/__tests__/compose.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5101,10 +5101,6 @@ describe('@source* directives', () => {

const messages = result.errors!.map(e => e.message);

expect(messages).toContain(
'[bad] Schemas that @link to https://specs.apollo.dev/source must also @link to federation version v2.7 or later (found v2.5)'
);

expect(messages).toContain(
'[bad] @sourceAPI(name: "A?!") must specify name using only [a-zA-Z0-9-_] characters'
);
Expand Down
116 changes: 116 additions & 0 deletions composition-js/src/__tests__/hints.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
import { MergeResult, mergeSubgraphs } from '../merging';
import { composeAsFed2Subgraphs } from './testHelper';
import { formatExpectedToMatchReceived } from './matchers/toMatchString';
import { composeServices } from '../compose';

function mergeDocuments(...documents: DocumentNode[]): MergeResult {
const subgraphs = new Subgraphs();
Expand Down Expand Up @@ -1177,3 +1178,118 @@ describe('when shared field has intersecting but non equal runtime types in diff
);
});
});

describe('when a directive causes an implicit federation version upgrade', () => {
const olderFederationSchema = gql`
extend schema
@link(url: "https://specs.apollo.dev/federation/v2.5", import: ["@key"])

type Query {
a: String!
}
`;

const newerFederationSchema = gql`
extend schema
@link(url: "https://specs.apollo.dev/federation/v2.7", import: ["@key"])

type Query {
b: String!
}
`;

const autoUpgradedSchema = gql`
extend schema
@link(url: "https://specs.apollo.dev/federation/v2.5", import: ["@key", "@shareable"])
@link(url: "https://specs.apollo.dev/source/v0.1", import: [
"@sourceAPI"
"@sourceType"
"@sourceField"
])
@sourceAPI(
name: "A"
http: { baseURL: "https://api.a.com/v1" }
)
{
query: Query
}

type Query @shareable {
resources: [Resource!]! @sourceField(
api: "A"
http: { GET: "/resources" }
)
}

type Resource @shareable @key(fields: "id") @sourceType(
api: "A"
http: { GET: "/resources/{id}" }
selection: "id description"
) {
id: ID!
description: String!
}
`;

it('should hint that the version was upgraded to satisfy directive requirements', () => {
const result = composeServices([
{
name: 'already-newest',
typeDefs: newerFederationSchema,
},
{
name: 'old-but-not-upgraded',
typeDefs: olderFederationSchema,
},
{
name: 'upgraded',
typeDefs: autoUpgradedSchema,
}
]);

expect(result).toRaiseHint(
HINTS.IMPLICITLY_UPGRADED_FEDERATION_VERSION,
'Subgraph upgraded has been implicitly upgraded from federation v2.5 to v2.7',
'@link'
);
});

it('should show separate hints for each upgraded subgraph', () => {
const result = composeServices([
{
name: 'upgraded-1',
typeDefs: autoUpgradedSchema,
},
{
name: 'upgraded-2',
typeDefs: autoUpgradedSchema
},
]);

expect(result).toRaiseHint(
HINTS.IMPLICITLY_UPGRADED_FEDERATION_VERSION,
'Subgraph upgraded-1 has been implicitly upgraded from federation v2.5 to v2.7',
'@link'
);
expect(result).toRaiseHint(
HINTS.IMPLICITLY_UPGRADED_FEDERATION_VERSION,
'Subgraph upgraded-2 has been implicitly upgraded from federation v2.5 to v2.7',
'@link'
);
});

it('should not raise hints if the only upgrade is caused by a link directly to the federation spec', () => {
const result = composeServices([
{
name: 'already-newest',
typeDefs: newerFederationSchema,
},
{
name: 'old-but-not-upgraded',
typeDefs: olderFederationSchema,
},
]);

expect(result).toNotRaiseHints();
});
})
8 changes: 8 additions & 0 deletions composition-js/src/hints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,13 @@ const INCONSISTENT_RUNTIME_TYPES_FOR_SHAREABLE_RETURN = makeCodeDefinition({
description: 'Indicates that a @shareable field returns different sets of runtime types in the different subgraphs in which it is defined.',
});

const IMPLICITLY_UPGRADED_FEDERATION_VERSION = makeCodeDefinition({
code: 'IMPLICITLY_UPGRADED_FEDERATION_VERSION',
level: HintLevel.INFO,
description: 'Indicates that a directive requires a higher federation version than is explicitly linked.'
+ ' In this case, the supergraph uses the federation version required by the directive.'
});

export const HINTS = {
INCONSISTENT_BUT_COMPATIBLE_FIELD_TYPE,
INCONSISTENT_BUT_COMPATIBLE_ARGUMENT_TYPE,
Expand Down Expand Up @@ -227,6 +234,7 @@ export const HINTS = {
DIRECTIVE_COMPOSITION_INFO,
DIRECTIVE_COMPOSITION_WARN,
INCONSISTENT_RUNTIME_TYPES_FOR_SHAREABLE_RETURN,
IMPLICITLY_UPGRADED_FEDERATION_VERSION,
}

export class CompositionHint {
Expand Down
57 changes: 48 additions & 9 deletions composition-js/src/merging/merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ import {
LinkDirectiveArgs,
sourceIdentity,
FeatureUrl,
CoreFeature,
Subgraph,
} from "@apollo/federation-internals";
import { ASTNode, GraphQLError, DirectiveLocation } from "graphql";
import {
Expand Down Expand Up @@ -343,19 +345,56 @@ class Merger {
}

private getLatestFederationVersionUsed(): FeatureVersion {
const latestVersion = this.subgraphs.values().reduce((latest: FeatureVersion | undefined, subgraph) => {
const version = subgraph.metadata()?.federationFeature()?.url?.version;
if (!latest) {
return version;
const versions = this.subgraphs.values()
.map((s) => this.getLatestFederationVersionUsedInSubgraph(s))
.filter(isDefined);

return FeatureVersion.max(versions) ?? FEDERATION_VERSIONS.latest().version;
}

private getLatestFederationVersionUsedInSubgraph(subgraph: Subgraph): FeatureVersion | undefined {
const linkedFederationVersion = subgraph.metadata()?.federationFeature()?.url.version;
if (!linkedFederationVersion) {
return undefined;
}

// Check if any of the directives imply a newer version of federation than is explicitly linked
const versionsFromFeatures: FeatureVersion[] = [];
for (const feature of subgraph.schema.coreFeatures?.allFeatures() ?? []) {
const version = feature.minimumFederationVersion();
if (version) {
versionsFromFeatures.push(version);
}
if (!version) {
return latest;
}
const impliedFederationVersion = FeatureVersion.max(versionsFromFeatures);
if (!impliedFederationVersion?.satisfies(linkedFederationVersion) || linkedFederationVersion >= impliedFederationVersion) {
return linkedFederationVersion;
}

// If some of the directives are causing an implicit upgrade, put one in the hint
let featureCausingUpgrade: CoreFeature | undefined;
for (const feature of subgraph.schema.coreFeatures?.allFeatures() ?? []) {
if (feature.minimumFederationVersion() == impliedFederationVersion) {
featureCausingUpgrade = feature;
break;
}
return latest >= version ? latest : version;
}, undefined);
return latestVersion ?? FEDERATION_VERSIONS.latest().version;
}

if (featureCausingUpgrade) {
this.hints.push(new CompositionHint(
HINTS.IMPLICITLY_UPGRADED_FEDERATION_VERSION,
`Subgraph ${subgraph.name} has been implicitly upgraded from federation ${linkedFederationVersion} to ${impliedFederationVersion}`,
featureCausingUpgrade.directive.definition,
featureCausingUpgrade.directive.sourceAST ?
addSubgraphToASTNode(featureCausingUpgrade.directive.sourceAST, subgraph.name) :
undefined
));
}

return impliedFederationVersion;
}


private prepareSupergraph(): Map<string, string> {
// TODO: we will soon need to look for name conflicts for @core and @join with potentially user-defined directives and
// pass a `as` to the methods below if necessary. However, as we currently don't propagate any subgraph directives to
Expand Down
6 changes: 6 additions & 0 deletions internals-js/src/definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
CoreSpecDefinition,
extractCoreFeatureImports,
FeatureUrl,
FeatureVersion,
findCoreSpecVersion,
isCoreSpecDirectiveApplication,
removeAllCoreFeatures,
Expand All @@ -53,6 +54,7 @@ import { validateSchema } from "./validate";
import { createDirectiveSpecification, createScalarTypeSpecification, DirectiveSpecification, TypeSpecification } from "./directiveAndTypeSpecification";
import { didYouMean, suggestionList } from "./suggestions";
import { aggregateError, ERRORS, withModifiedErrorMessage } from "./error";
import { coreFeatureDefinitionIfKnown } from "./knownCoreFeatures";

const validationErrorCode = 'GraphQLValidationFailed';
const DEFAULT_VALIDATION_ERROR_MESSAGE = 'The schema is not a valid GraphQL schema.';
Expand Down Expand Up @@ -1014,6 +1016,10 @@ export class CoreFeature {
const elementImport = this.imports.find((i) => i.name === name);
return elementImport ? (elementImport.as ?? name) : this.nameInSchema + '__' + name;
}

minimumFederationVersion(): FeatureVersion | undefined {
return coreFeatureDefinitionIfKnown(this.url)?.minimumFederationVersion;
}
}

export class CoreFeatures {
Expand Down
22 changes: 22 additions & 0 deletions internals-js/src/specs/coreSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,28 @@ export class FeatureVersion {
return new this(+match[1], +match[2])
}

/**
* Find the maximum version in a collection of versions, returning undefined in the case
* that the collection is empty.
*
* # Example
* ```
* expect(FeatureVersion.max([new FeatureVersion(1, 0), new FeatureVersion(2, 0)])).toBe(new FeatureVersion(2, 0))
* expect(FeatureVersion.max([])).toBe(undefined)
* ```
*/
public static max(versions: Iterable<FeatureVersion>): FeatureVersion | undefined {
let max: FeatureVersion | undefined;

for (const version of versions) {
if (!max || version > max) {
max = version;
}
}

return max;
}

/**
* Return true if and only if this FeatureVersion satisfies the `required` version
*
Expand Down
23 changes: 2 additions & 21 deletions internals-js/src/specs/sourceSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,15 +147,13 @@ export class SourceSpecDefinition extends FeatureDefinition {
return this.directive<SourceFieldDirectiveArgs>(schema, 'sourceField')!;
}

private getSourceDirectives(schema: Schema, errors: GraphQLError[]) {
private getSourceDirectives(schema: Schema) {
const result: {
sourceAPI?: DirectiveDefinition<SourceAPIDirectiveArgs>;
sourceType?: DirectiveDefinition<SourceTypeDirectiveArgs>;
sourceField?: DirectiveDefinition<SourceFieldDirectiveArgs>;
} = {};

let federationVersion: FeatureVersion | undefined;

schema.schemaDefinition.appliedDirectivesOf<LinkDirectiveArgs>('link')
.forEach(linkDirective => {
const { url, import: imports } = linkDirective.arguments();
Expand All @@ -175,25 +173,8 @@ export class SourceSpecDefinition extends FeatureDefinition {
}
});
}
if (featureUrl && featureUrl.name === 'federation') {
federationVersion = featureUrl.version;
}
});

if (result.sourceAPI || result.sourceType || result.sourceField) {
// Since this subgraph uses at least one of the @source{API,Type,Field}
// directives, it must also use v2.7 or later of federation.
if (!federationVersion || federationVersion.lt(this.minimumFederationVersion)) {
errors.push(ERRORS.SOURCE_FEDERATION_VERSION_REQUIRED.err(
`Schemas that @link to ${
sourceIdentity
} must also @link to federation version ${
this.minimumFederationVersion
} or later (found ${federationVersion})`,
));
}
}

return result;
}

Expand All @@ -203,7 +184,7 @@ export class SourceSpecDefinition extends FeatureDefinition {
sourceAPI,
sourceType,
sourceField,
} = this.getSourceDirectives(schema, errors);
} = this.getSourceDirectives(schema);

if (!(sourceAPI || sourceType || sourceField)) {
// If none of the @source* directives are present, nothing needs
Expand Down