Skip to content

Commit e470658

Browse files
tnineslingclaude
andcommitted
refactor: move @OneOf validation to pre_merge_validations
Validates @OneOf consistency across subgraphs before merging begins, matching the pattern used by other cross-subgraph validations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent f22ea9e commit e470658

4 files changed

Lines changed: 62 additions & 37 deletions

File tree

‎apollo-federation/src/composition/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use crate::connectors::expand::expand_connectors;
1313
use crate::error::CompositionError;
1414
use crate::merger::merge::Merger;
1515
pub use crate::schema::schema_upgrader::upgrade_subgraphs_if_necessary;
16+
use crate::schema::validators::one_of::validate_one_of_consistency;
1617
use crate::schema::validators::root_fields::validate_consistent_root_fields;
1718
use crate::subgraph::typestate::Expanded;
1819
use crate::subgraph::typestate::Initial;
@@ -136,6 +137,7 @@ pub fn expand_subgraphs(
136137
#[instrument(skip(subgraphs))]
137138
pub fn pre_merge_validations(subgraphs: &[Subgraph<Validated>]) -> Result<(), CompositionFailure> {
138139
validate_consistent_root_fields(subgraphs).map_err(CompositionFailure::from_errors)?;
140+
validate_one_of_consistency(subgraphs).map_err(CompositionFailure::from_errors)?;
139141
// TODO: (FED-713) Implement any pre-merge validations that require knowledge of all subgraphs.
140142
Ok(())
141143
}

‎apollo-federation/src/merger/merge_input.rs‎

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@ impl Merger {
2626
sources: &Sources<Node<InputObjectType>>,
2727
dest: &InputObjectTypeDefinitionPosition,
2828
) -> Result<(), FederationError> {
29-
self.validate_one_of_consistency(sources, dest);
30-
3129
// Like for other inputs, we add all the fields found in any subgraphs initially as a simple mean to have a complete list of
3230
// field to iterate over, but we will remove those that are not in all subgraphs.
3331
let added = self.add_input_fields_shallow(sources, dest)?;
@@ -278,39 +276,4 @@ impl Merger {
278276
self.merge_default_value(sources, dest_field)?;
279277
Ok(())
280278
}
281-
282-
fn validate_one_of_consistency(
283-
&mut self,
284-
sources: &Sources<Node<InputObjectType>>,
285-
dest: &InputObjectTypeDefinitionPosition,
286-
) {
287-
let mut with_one_of: Vec<String> = Vec::new();
288-
let mut without_one_of: Vec<String> = Vec::new();
289-
290-
for (idx, source) in sources {
291-
let Some(source) = source else {
292-
continue;
293-
};
294-
let has_one_of = source.directives.has("oneOf");
295-
let subgraph_name = self.names[*idx].to_string();
296-
if has_one_of {
297-
with_one_of.push(subgraph_name);
298-
} else {
299-
without_one_of.push(subgraph_name);
300-
}
301-
}
302-
303-
if !with_one_of.is_empty() && !without_one_of.is_empty() {
304-
let with_str = human_readable_subgraph_names(with_one_of.iter());
305-
let without_str = human_readable_subgraph_names(without_one_of.iter());
306-
self.error_reporter
307-
.add_error(CompositionError::InputObjectOneOfMismatch {
308-
message: format!(
309-
"Input object type \"{}\" is marked with @oneOf in {} but not in {}",
310-
dest.type_name, with_str, without_str,
311-
),
312-
locations: self.source_locations(sources),
313-
});
314-
}
315-
}
316279
}

‎apollo-federation/src/schema/validators/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ pub(crate) mod interface_object;
3030
pub(crate) mod key;
3131
pub(crate) mod list_size;
3232
pub(crate) mod merged;
33+
pub(crate) mod one_of;
3334
pub(crate) mod provides;
3435
pub(crate) mod requires;
3536
pub(crate) mod root_fields;
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
use apollo_compiler::schema::ExtendedType;
2+
3+
use crate::error::CompositionError;
4+
use crate::subgraph::typestate::HasMetadata;
5+
use crate::subgraph::typestate::Subgraph;
6+
use crate::utils::human_readable::human_readable_subgraph_names;
7+
8+
/// Validates that all subgraphs defining the same input object type agree on
9+
/// whether `@oneOf` is applied. Disagreement would produce an invalid
10+
/// supergraph, so we reject it before merging.
11+
pub(crate) fn validate_one_of_consistency<T: HasMetadata>(
12+
subgraphs: &[Subgraph<T>],
13+
) -> Result<(), Vec<CompositionError>> {
14+
let mut errors = Vec::new();
15+
16+
// Collect all input object type names across subgraphs.
17+
let mut input_type_names = std::collections::BTreeSet::new();
18+
for subgraph in subgraphs {
19+
for (name, ty) in &subgraph.schema().schema().types {
20+
if matches!(ty, ExtendedType::InputObject(_)) {
21+
input_type_names.insert(name.clone());
22+
}
23+
}
24+
}
25+
26+
for type_name in &input_type_names {
27+
let mut with_one_of: Vec<&str> = Vec::new();
28+
let mut without_one_of: Vec<&str> = Vec::new();
29+
30+
for subgraph in subgraphs {
31+
if let Some(ExtendedType::InputObject(input_obj)) =
32+
subgraph.schema().schema().types.get(type_name)
33+
{
34+
if input_obj.directives.has("oneOf") {
35+
with_one_of.push(&subgraph.name);
36+
} else {
37+
without_one_of.push(&subgraph.name);
38+
}
39+
}
40+
}
41+
42+
if !with_one_of.is_empty() && !without_one_of.is_empty() {
43+
let with_str = human_readable_subgraph_names(with_one_of.iter());
44+
let without_str = human_readable_subgraph_names(without_one_of.iter());
45+
errors.push(CompositionError::InputObjectOneOfMismatch {
46+
message: format!(
47+
"Input object type \"{type_name}\" is marked with @oneOf in {with_str} but not in {without_str}",
48+
),
49+
locations: Vec::new(),
50+
});
51+
}
52+
}
53+
54+
if errors.is_empty() {
55+
Ok(())
56+
} else {
57+
Err(errors)
58+
}
59+
}

0 commit comments

Comments
 (0)