Skip to content

Commit 26fcb01

Browse files
committed
fix(federation): update field types and member references when renaming a union
`UnionTypeDefinitionPosition::rename` only re-keyed the union's reference record. Object and interface fields that return the union kept the old, removed name, making the schema invalid. The reference index also kept the old name in each member object's `union_types` set and in the union's `__typename` field position. The union rename now rewrites the types of referencing fields, preserving list and non-null wrappers, and `Referencers::rename_union_type` remaps the member and `__typename` positions. The regression compares the index with one rebuilt from the edited schema.
1 parent c3829d4 commit 26fcb01

2 files changed

Lines changed: 220 additions & 0 deletions

File tree

‎apollo-federation/src/schema/position.rs‎

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5722,6 +5722,12 @@ impl UnionTypeDefinitionPosition {
57225722
if let Some(union_type_referencers) =
57235723
schema.referencers.union_types.swap_remove(&self.type_name)
57245724
{
5725+
for pos in union_type_referencers.object_fields.iter() {
5726+
pos.rename_type(schema, new_name.clone())?;
5727+
}
5728+
for pos in union_type_referencers.interface_fields.iter() {
5729+
pos.rename_type(schema, new_name.clone())?;
5730+
}
57255731
schema
57265732
.referencers
57275733
.union_types
@@ -8383,4 +8389,189 @@ mod tests {
83838389
}
83848390
"#);
83858391
}
8392+
8393+
/// Flattens a referencer index into an order-independent set of facts, so an incrementally
8394+
/// maintained index can be compared with one rebuilt from scratch.
8395+
fn referencer_facts(referencers: &Referencers) -> std::collections::BTreeSet<String> {
8396+
fn add<T: Debug>(
8397+
facts: &mut std::collections::BTreeSet<String>,
8398+
kind: &str,
8399+
key: &Name,
8400+
slot: &str,
8401+
positions: impl IntoIterator<Item = T>,
8402+
) {
8403+
facts.insert(format!("{kind} {key}"));
8404+
for position in positions {
8405+
facts.insert(format!("{kind} {key} {slot} {position:?}"));
8406+
}
8407+
}
8408+
let mut facts = std::collections::BTreeSet::new();
8409+
let f = &mut facts;
8410+
for (key, r) in &referencers.scalar_types {
8411+
add(f, "scalar", key, "object_fields", &r.object_fields);
8412+
add(
8413+
f,
8414+
"scalar",
8415+
key,
8416+
"object_field_arguments",
8417+
&r.object_field_arguments,
8418+
);
8419+
add(f, "scalar", key, "interface_fields", &r.interface_fields);
8420+
add(
8421+
f,
8422+
"scalar",
8423+
key,
8424+
"interface_field_arguments",
8425+
&r.interface_field_arguments,
8426+
);
8427+
add(f, "scalar", key, "union_fields", &r.union_fields);
8428+
add(
8429+
f,
8430+
"scalar",
8431+
key,
8432+
"input_object_fields",
8433+
&r.input_object_fields,
8434+
);
8435+
add(
8436+
f,
8437+
"scalar",
8438+
key,
8439+
"directive_arguments",
8440+
&r.directive_arguments,
8441+
);
8442+
}
8443+
for (key, r) in &referencers.object_types {
8444+
add(f, "object", key, "schema_roots", &r.schema_roots);
8445+
add(f, "object", key, "object_fields", &r.object_fields);
8446+
add(f, "object", key, "interface_fields", &r.interface_fields);
8447+
add(f, "object", key, "union_types", &r.union_types);
8448+
}
8449+
for (key, r) in &referencers.interface_types {
8450+
add(f, "interface", key, "object_types", &r.object_types);
8451+
add(f, "interface", key, "object_fields", &r.object_fields);
8452+
add(f, "interface", key, "interface_types", &r.interface_types);
8453+
add(f, "interface", key, "interface_fields", &r.interface_fields);
8454+
}
8455+
for (key, r) in &referencers.union_types {
8456+
add(f, "union", key, "object_fields", &r.object_fields);
8457+
add(f, "union", key, "interface_fields", &r.interface_fields);
8458+
}
8459+
for (key, r) in &referencers.enum_types {
8460+
add(f, "enum", key, "object_fields", &r.object_fields);
8461+
add(
8462+
f,
8463+
"enum",
8464+
key,
8465+
"object_field_arguments",
8466+
&r.object_field_arguments,
8467+
);
8468+
add(f, "enum", key, "interface_fields", &r.interface_fields);
8469+
add(
8470+
f,
8471+
"enum",
8472+
key,
8473+
"interface_field_arguments",
8474+
&r.interface_field_arguments,
8475+
);
8476+
add(
8477+
f,
8478+
"enum",
8479+
key,
8480+
"input_object_fields",
8481+
&r.input_object_fields,
8482+
);
8483+
add(
8484+
f,
8485+
"enum",
8486+
key,
8487+
"directive_arguments",
8488+
&r.directive_arguments,
8489+
);
8490+
}
8491+
for (key, r) in &referencers.input_object_types {
8492+
add(
8493+
f,
8494+
"input",
8495+
key,
8496+
"object_field_arguments",
8497+
&r.object_field_arguments,
8498+
);
8499+
add(
8500+
f,
8501+
"input",
8502+
key,
8503+
"interface_field_arguments",
8504+
&r.interface_field_arguments,
8505+
);
8506+
add(
8507+
f,
8508+
"input",
8509+
key,
8510+
"input_object_fields",
8511+
&r.input_object_fields,
8512+
);
8513+
add(
8514+
f,
8515+
"input",
8516+
key,
8517+
"directive_arguments",
8518+
&r.directive_arguments,
8519+
);
8520+
}
8521+
for (key, r) in &referencers.directives {
8522+
add(f, "directive", key, "targets", r.iter());
8523+
}
8524+
facts
8525+
}
8526+
8527+
/// Renames a type through the public entry point, then checks the edit contract: the compiler
8528+
/// schema is still valid, and the incrementally maintained referencers equal referencers
8529+
/// rebuilt from scratch from the edited schema.
8530+
fn rename_and_check(sdl: &str, old_name: Name, new_name: Name) -> FederationSchema {
8531+
let mut schema = FederationSchema::new(
8532+
Schema::parse_and_validate(sdl, "rename.graphql")
8533+
.unwrap()
8534+
.into_inner(),
8535+
)
8536+
.unwrap();
8537+
schema
8538+
.get_type(&old_name)
8539+
.unwrap()
8540+
.rename(&mut schema, new_name)
8541+
.unwrap();
8542+
let valid = schema.schema().clone().validate();
8543+
assert!(valid.is_ok(), "renamed schema is invalid: {valid:?}");
8544+
let rebuilt = FederationSchema::new(schema.schema().clone()).unwrap();
8545+
let actual = referencer_facts(schema.referencers());
8546+
let expected = referencer_facts(rebuilt.referencers());
8547+
let stale: Vec<_> = actual.difference(&expected).collect();
8548+
let missing: Vec<_> = expected.difference(&actual).collect();
8549+
assert!(
8550+
stale.is_empty() && missing.is_empty(),
8551+
"referencers diverge from a rebuild after renaming {old_name}\n\
8552+
stale: {stale:#?}\nmissing: {missing:#?}"
8553+
);
8554+
schema
8555+
}
8556+
8557+
#[test]
8558+
fn renamed_union_updates_output_field_types() {
8559+
let schema = rename_and_check(
8560+
"type Member { id: ID! } union Choice = Member \
8561+
interface Holder { held: [Choice!] } \
8562+
type Query implements Holder { result: Choice! held: [Choice!] }",
8563+
name!("Choice"),
8564+
name!("MovedChoice"),
8565+
);
8566+
let query = ObjectTypeDefinitionPosition::new(name!("Query"));
8567+
let result = query.field(name!("result")).get(schema.schema()).unwrap();
8568+
assert_eq!(result.ty.to_string(), "MovedChoice!");
8569+
let held = InterfaceTypeDefinitionPosition {
8570+
type_name: name!("Holder"),
8571+
}
8572+
.field(name!("held"))
8573+
.get(schema.schema())
8574+
.unwrap();
8575+
assert_eq!(held.ty.to_string(), "[MovedChoice!]");
8576+
}
83868577
}

‎apollo-federation/src/schema/referencer.rs‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,18 @@ impl Referencers {
345345
}
346346

347347
pub(crate) fn rename_union_type(&mut self, old_name: &Name, new_name: &Name) {
348+
for (_scalar_name, scalar_refs) in self.scalar_types.iter_mut() {
349+
Self::update_union_typename_field_positions(
350+
&mut scalar_refs.union_fields,
351+
old_name,
352+
new_name,
353+
);
354+
}
355+
356+
for (_object_name, object_refs) in self.object_types.iter_mut() {
357+
Self::update_union_type_positions(&mut object_refs.union_types, old_name, new_name);
358+
}
359+
348360
for (_directive_name, directive_refs) in self.directives.iter_mut() {
349361
Self::update_union_type_positions(&mut directive_refs.union_types, old_name, new_name);
350362
}
@@ -367,6 +379,23 @@ impl Referencers {
367379
types.extend(updated_types);
368380
}
369381

382+
fn update_union_typename_field_positions(
383+
fields: &mut IndexSet<UnionTypenameFieldDefinitionPosition>,
384+
old_type_name: &Name,
385+
new_type_name: &Name,
386+
) {
387+
let updated_fields: Vec<_> = fields
388+
.iter()
389+
.filter(|f| &f.type_name == old_type_name)
390+
.map(|_| UnionTypenameFieldDefinitionPosition {
391+
type_name: new_type_name.clone(),
392+
})
393+
.collect();
394+
395+
fields.retain(|f| &f.type_name != old_type_name);
396+
fields.extend(updated_fields);
397+
}
398+
370399
pub(crate) fn rename_enum_type(&mut self, old_name: &Name, new_name: &Name) {
371400
for (_directive_name, directive_refs) in self.directives.iter_mut() {
372401
Self::update_enum_type_positions(&mut directive_refs.enum_types, old_name, new_name);

0 commit comments

Comments
 (0)