Skip to content

Commit

Permalink
Only run validateOverride when the coordinate has an override in some…
Browse files Browse the repository at this point in the history
… subgraph
  • Loading branch information
clenfest authored and sachindshinde committed Jul 9, 2024
1 parent 3dff8a3 commit bf362da
Showing 1 changed file with 39 additions and 9 deletions.
48 changes: 39 additions & 9 deletions composition-js/src/merging/merge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ import {
StaticArgumentsTransform,
isNullableType,
isFieldDefinition,
Post20FederationDirectiveDefinition,
} from "@apollo/federation-internals";
import { ASTNode, GraphQLError, DirectiveLocation } from "graphql";
import {
Expand Down Expand Up @@ -368,13 +369,15 @@ class Merger {
private joinDirectiveIdentityURLs = new Set<string>();
private schemaToImportNameToFeatureUrl = new Map<Schema, Map<string, FeatureUrl>>();
private fieldsWithFromContext: Set<string>;
private fieldsWithOverride: Set<string>;

constructor(readonly subgraphs: Subgraphs, readonly options: CompositionOptions) {
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();

this.names = subgraphs.names();
this.composeDirectiveManager = new ComposeDirectiveManager(
Expand Down Expand Up @@ -1346,6 +1349,9 @@ class Merger {
*/
private validateOverride(sources: Sources<FieldDefinition<any>>, dest: FieldDefinition<any>): FieldMergeContext {
const result = new FieldMergeContext(sources);
if (!this.fieldsWithOverride.has(dest.coordinate)) {
return result;
}

// For any field, we can't have more than one @override directive
type MappedValue = {
Expand Down Expand Up @@ -3437,20 +3443,44 @@ class Merger {
}

private getFieldsWithFromContextDirective(): Set<string> {
const fieldsWithFromContext = new Set<string>();
return this.getFieldsWithAppliedDirective(
(subgraph: Subgraph) => subgraph.metadata().fromContextDirective(),
(application: Directive<SchemaElement<any,any>>) => {
const field = application.parent.parent;
assert(isFieldDefinition(field), () => `Expected ${application.parent} to be a field`);
return field;
},
);
}

private getFieldsWithOverrideDirective(): Set<string> {
return this.getFieldsWithAppliedDirective(
(subgraph: Subgraph) => subgraph.metadata().overrideDirective(),
(application: Directive<SchemaElement<any,any>>) => {
const field = application.parent;
assert(isFieldDefinition(field), () => `Expected ${application.parent} to be a field`);
return field;
}
);
}

private getFieldsWithAppliedDirective(
getDirective: (subgraph: Subgraph) => Post20FederationDirectiveDefinition<any>,
getField: (application: Directive<SchemaElement<any, any>>) => FieldDefinition<any>,
) {
const fields = new Set<string>();
for (const subgraph of this.subgraphs) {
const fromContextDirective = subgraph.metadata().fromContextDirective();
if (isFederationDirectiveDefinedInSchema(fromContextDirective)) {
for (const application of fromContextDirective.applications()) {
const field = application.parent.parent;
assert(isFieldDefinition(field), () => `Expected ${application.parent.parent} to be a field`);
const directive = getDirective(subgraph);
if (isFederationDirectiveDefinedInSchema(directive)) {
for (const application of directive.applications()) {
const field = getField(application);
const coordinate = field.coordinate;
if (!fieldsWithFromContext.has(coordinate)) {
fieldsWithFromContext.add(coordinate);
if (!fields.has(coordinate)) {
fields.add(coordinate);
}
}
}
}
return fieldsWithFromContext;
return fields;
}
}

0 comments on commit bf362da

Please sign in to comment.