From 7ed3b19f781e7f27c8087a33e7276194b609c9ac Mon Sep 17 00:00:00 2001 From: Duckki Oe Date: Thu, 1 Aug 2024 18:58:01 -0700 Subject: [PATCH] fix(federation): fixed a mismatch after merging PR #5743 - FetchNode's `variable_usages` field incorrectly included the "representations" variable. --- .../src/query_plan/fetch_dependency_graph.rs | 18 ++++++++---------- .../handles_operations_with_directives.rs | 3 +++ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/apollo-federation/src/query_plan/fetch_dependency_graph.rs b/apollo-federation/src/query_plan/fetch_dependency_graph.rs index 9eb638def7..1b6bedb55b 100644 --- a/apollo-federation/src/query_plan/fetch_dependency_graph.rs +++ b/apollo-federation/src/query_plan/fetch_dependency_graph.rs @@ -2362,6 +2362,14 @@ impl FetchDependencyGraphNode { .cloned() .collect::>() }; + let variable_usages = { + let mut list = variable_definitions + .iter() + .map(|var_def| var_def.name.clone()) + .collect::>(); + list.sort(); + list + }; let mut operation = if self.is_entity_fetch { operation_for_entities_fetch( @@ -2387,16 +2395,6 @@ impl FetchDependencyGraphNode { operation.reuse_fragments(fragments)?; } - let variable_usages = { - let mut list = operation - .variables - .iter() - .map(|variable| variable.name.clone()) - .collect::>(); - list.sort(); - list - }; - let operation_document = operation.try_into()?; let node = super::PlanNode::Fetch(Box::new(super::FetchNode { diff --git a/apollo-federation/tests/query_plan/build_query_plan_tests/handles_operations_with_directives.rs b/apollo-federation/tests/query_plan/build_query_plan_tests/handles_operations_with_directives.rs index 7a79e150b2..23508b7d02 100644 --- a/apollo-federation/tests/query_plan/build_query_plan_tests/handles_operations_with_directives.rs +++ b/apollo-federation/tests/query_plan/build_query_plan_tests/handles_operations_with_directives.rs @@ -148,6 +148,9 @@ fn test_if_directives_at_the_operation_level_are_passed_down_to_subgraph_queries } } "#); + // This checks a regression where the `variable_usages` included the `representations` variable. + assert_eq!(b_fetch_nodes[0].variable_usages.len(), 0); + assert_eq!(b_fetch_nodes[1].variable_usages.len(), 0); } #[test]