Skip to content

Commit fd4545c

Browse files
author
Sylvain Lebresne
authored
Fix not using @join__type for fields with no @join__field (#2654)
When a field is defined in all the subgraphs in which its type is defined (typically the case for value types) and the field has nothing specific (no `@requires`, `@provides`, `@override`, ...), then we don't mark it in the supergraph with a `@join__field`. It's not really needed, and this keeps supergraphs smaller. But the code extracting subgraphs from supergraph was not handling that case gracefully, and it was instead trying to add such fields to all the subgraphs. This didn't break anything because when a subgraph didn't had the type, the code was still giving up on adding the field, but for supergraph with a fair number of subgraphs, this led to a lot of busy work, making the extraction code slower than it needed to be. This commit fix that issue. Note that a reason this probably occured is that the extraction is trying to handle all versions of supergraphs with the same code, but fed1 supergraphs and fed2 supergraphs have actually a fair number of differences, and trying to handle both with the same code is, in hindsight, a bit confusing and we should clean it up. Leaving this as a follow-up however.
1 parent 02eab3a commit fd4545c

2 files changed

Lines changed: 45 additions & 10 deletions

File tree

.changeset/violet-rice-cheer.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"@apollo/federation-internals": patch
3+
---
4+
5+
Fix unnecessary busy work in the code extracting subgraphs from supergraphs. This code is executing when a new
6+
supergraph is deployed, and can impact gateway clients when it runs. This is often not a problem, but for
7+
large supergraphs with lots of subgraphs, an obvious inefficiency could make the code take much longer than
8+
it should have.
9+

internals-js/src/extractSubgraphsFromSupergraph.ts

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -328,15 +328,46 @@ export function extractSubgraphsFromSupergraph(supergraph: Schema): Subgraphs {
328328
}
329329
// Fall-through on purpose.
330330
case 'InputObjectType':
331+
const typeApplications = type.appliedDirectivesOf(typeDirective);
332+
// Note that we can have more that one `@join__type` for a given graph if there is multiple keys, so we collect
333+
// the actual set of subgraphs defining the type.
334+
const subgraphsDefiningType: Subgraph[] = [];
335+
for (const app of typeApplications) {
336+
const subgraph = subgraphs.get(graphEnumNameToSubgraphName.get(app.arguments().graph)!)!;
337+
if (!subgraphsDefiningType.includes(subgraph)) {
338+
subgraphsDefiningType.push(subgraph);
339+
}
340+
}
341+
331342
for (const field of type.fields()) {
332343
const fieldApplications = field.appliedDirectivesOf(fieldDirective);
333344
if (!fieldApplications.length) {
334-
// The meaning of having no join__field depends on whether the parent type has a join__owner.
335-
// If it does, it means the field is only on that owner subgraph. Otherwise, we kind of don't
336-
// know, so we add it to all subgraphs that have the parent type and, if the field base type
337-
// is a named type, know that field type.
345+
// Where there is no join__field, there is roughly 3 main cases:
346+
// 1. if the type has an `@join__owner` directive (old fed1 supergraph), then the field belong to that owner subgraph.
347+
// 2. otherwise, if the type has some `@join__type` directives (which it will have in post-"fed1" supergraph), then
348+
// the field is in all the supergraph in which the type is.
349+
// 2. otherwise, we kind of don't know, so we add it to all subgraphs that have the parent type and, if the
350+
// field base type is a named type, know that field type. Note that this last case only happens for old fed1
351+
// supergraphs which were lacking information and force a bit of guessing. All fed2 generated supergraph
352+
// use `@join__type` systematically on all types, and will always be case 2.
338353
const ownerApplications = ownerDirective ? type.appliedDirectivesOf(ownerDirective) : [];
339-
if (!ownerApplications.length) {
354+
if (ownerApplications.length > 0) {
355+
assert(ownerApplications.length == 1, () => `Found multiple join__owner directives on type ${type}`)
356+
const subgraph = subgraphs.get(graphEnumNameToSubgraphName.get(ownerApplications[0].arguments().graph)!)!;
357+
const subgraphField = addSubgraphField(field, subgraph);
358+
assert(subgraphField, () => `Found join__owner directive on ${type} but no corresponding join__type`);
359+
continue;
360+
}
361+
362+
if (subgraphsDefiningType.length > 0) {
363+
const isShareable = isObjectType(type) && subgraphsDefiningType.length > 1;
364+
for (const subgraph of subgraphsDefiningType) {
365+
const subgraphField = addSubgraphField(field, subgraph);
366+
if (subgraphField && isShareable) {
367+
subgraphField.applyDirective(subgraph.metadata().shareableDirective());
368+
}
369+
}
370+
} else {
340371
const fieldBaseType = baseType(field.type!);
341372
const isShareable = isObjectType(type) && subgraphs.values().filter((s) => s.schema.type(type.name)).length > 1;
342373
for (const subgraph of subgraphs) {
@@ -347,11 +378,6 @@ export function extractSubgraphsFromSupergraph(supergraph: Schema): Subgraphs {
347378
}
348379
}
349380
}
350-
} else {
351-
assert(ownerApplications.length == 1, () => `Found multiple join__owner directives on type ${type}`)
352-
const subgraph = subgraphs.get(graphEnumNameToSubgraphName.get(ownerApplications[0].arguments().graph)!)!;
353-
const subgraphField = addSubgraphField(field, subgraph);
354-
assert(subgraphField, () => `Found join__owner directive on ${type} but no corresponding join__type`);
355381
}
356382
} else {
357383
const isShareable = isObjectType(type)

0 commit comments

Comments
 (0)