Skip to content

chore(federation): generate the same fragment names as JS #5821

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
goto-bus-stop merged 3 commits into from
Aug 15, 2024
Merged

chore(federation): generate the same fragment names as JS #5821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c7b1ccd
chore(federation): generate the same fragment names as JS
goto-bus-stop Aug 14, 2024
9c99e23
lint
goto-bus-stop Aug 14, 2024
2067118
Merge branch 'dev' into renee/ROUTER-523-mismatch
goto-bus-stop Aug 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 43 additions & 1 deletion apollo-federation/src/operation/optimize.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1624,13 +1624,42 @@ fn fragment_name(mut index: usize) -> Name {
#[derive(Debug, Default)]
struct FragmentGenerator {
fragments: NamedFragments,
// XXX(@goto-bus-stop): This is temporary to support mismatch testing with JS!
names: HashMap<(String, usize), usize>,
}

impl FragmentGenerator {
fn next_name(&self) -> Name {
fragment_name(self.fragments.len())
}

// XXX(@goto-bus-stop): This is temporary to support mismatch testing with JS!
// In the future, we will just use `.next_name()`.
fn generate_name(&mut self, frag: &InlineFragmentSelection) -> Name {
use std::fmt::Write as _;

let type_condition = frag
.inline_fragment
.type_condition_position
.as_ref()
.map_or_else(
|| "undefined".to_string(),
|condition| condition.to_string(),
);
let selections = frag.selection_set.selections.len();
let mut name = format!("_generated_on{type_condition}_{selections}");

let key = (type_condition, selections);
let index = self
.names
.entry(key)
.and_modify(|index| *index += 1)
.or_default();
_ = write!(&mut name, "_{index}");

Name::new_unchecked(&name)
}

/// Is a selection set worth using for a newly generated named fragment?
fn is_worth_using(selection_set: &SelectionSet) -> bool {
let mut iter = selection_set.iter();
Expand Down Expand Up @@ -1697,6 +1726,17 @@ impl FragmentGenerator {
continue;
};

// XXX(@goto-bus-stop): This is temporary to support mismatch testing with JS!
// JS does not special-case @skip and @include. It never extracts a fragment if
// there's any directives on it. This code duplicates the body from the
// previous condition so it's very easy to remove when we're ready :)
if !skip_include.is_empty() {
new_selection_set.add_local_selection(&Selection::InlineFragment(
Arc::clone(candidate.get()),
))?;
continue;
}

let existing = self.fragments.iter().find(|existing| {
existing.type_condition_position
== candidate.get().inline_fragment.casted_type()
Expand All @@ -1706,7 +1746,9 @@ impl FragmentGenerator {
let existing = if let Some(existing) = existing {
existing
} else {
let name = self.next_name();
// XXX(@goto-bus-stop): This is temporary to support mismatch testing with JS!
// This should be reverted to `self.next_name();` when we're ready.
let name = self.generate_name(candidate.get());
self.fragments.insert(Fragment {
schema: selection_set.schema.clone(),
name: name.clone(),
Expand Down
49 changes: 25 additions & 24 deletions apollo-federation/tests/query_plan/build_query_plan_tests/fragment_autogeneration.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,14 @@ fn it_respects_generate_query_fragments_option() {
{
t {
__typename
...a
..._generated_onA_2_0
... on B {
z
}
}
}

fragment a on A {
fragment _generated_onA_2_0 on A {
x
y
}
Expand Down Expand Up @@ -105,21 +105,21 @@ fn it_handles_nested_fragment_generation() {
{
t {
__typename
...b
..._generated_onA_3_0
}
}

fragment a on A {
fragment _generated_onA_2_0 on A {
x
y
}

fragment b on A {
fragment _generated_onA_3_0 on A {
x
y
t {
__typename
...a
..._generated_onA_2_0
... on B {
z
}
Expand Down Expand Up @@ -159,11 +159,11 @@ fn it_handles_fragments_with_one_non_leaf_field() {
{
t {
__typename
...a
..._generated_onA_1_0
}
}

fragment a on A {
fragment _generated_onA_1_0 on A {
t {
__typename
... on B {
Expand Down Expand Up @@ -219,22 +219,23 @@ fn it_migrates_skip_include() {
{
t {
__typename
...b
..._generated_onA_3_0
}
}

fragment a on A {
x
y
}

fragment b on A {
fragment _generated_onA_3_0 on A {
x
y
t {
__typename
...a @include(if: $var)
...a @skip(if: $var)
... on A @include(if: $var) {
x
y
}
... on A @skip(if: $var) {
x
y
}
... on A @custom {
x
y
Expand Down Expand Up @@ -276,15 +277,15 @@ fn it_identifies_and_reuses_equivalent_fragments_that_arent_identical() {
{
t {
__typename
...a
..._generated_onA_2_0
}
t2 {
__typename
...a
..._generated_onA_2_0
}
}

fragment a on A {
fragment _generated_onA_2_0 on A {
x
y
}
Expand Down Expand Up @@ -324,20 +325,20 @@ fn fragments_that_share_a_hash_but_are_not_identical_generate_their_own_fragment
{
t {
__typename
...a
..._generated_onA_2_0
}
t2 {
__typename
...b
..._generated_onA_2_1
}
}

fragment a on A {
fragment _generated_onA_2_0 on A {
x
y
}

fragment b on A {
fragment _generated_onA_2_1 on A {
y
z
}
Expand Down