From 1aaedece0c5040bbeac96cba92843d4af00b9504 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Thu, 3 Oct 2024 12:49:14 +0200 Subject: [PATCH 1/3] chore(federation): do not generate fragments without type condition This is to match the JavaScript implementation. We can get rid of it later. --- apollo-federation/src/operation/optimize.rs | 14 ++++++ .../fragment_autogeneration.rs | 47 +++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/apollo-federation/src/operation/optimize.rs b/apollo-federation/src/operation/optimize.rs index e9e897304f..bde0b6dfa9 100644 --- a/apollo-federation/src/operation/optimize.rs +++ b/apollo-federation/src/operation/optimize.rs @@ -1684,6 +1684,20 @@ impl FragmentGenerator { SelectionValue::InlineFragment(mut candidate) => { self.visit_selection_set(candidate.get_selection_set_mut())?; + // XXX(@goto-bus-stop): This is temporary to support mismatch testing with JS! + // JS federation does not consider fragments without a type condition. + if candidate + .get() + .inline_fragment + .type_condition_position + .is_none() + { + new_selection_set.add_local_selection(&Selection::InlineFragment( + Arc::clone(candidate.get()), + ))?; + continue; + } + let directives = &candidate.get().inline_fragment.directives; let skip_include = directives .iter() diff --git a/apollo-federation/tests/query_plan/build_query_plan_tests/fragment_autogeneration.rs b/apollo-federation/tests/query_plan/build_query_plan_tests/fragment_autogeneration.rs index 876334fa5d..cd0de19455 100644 --- a/apollo-federation/tests/query_plan/build_query_plan_tests/fragment_autogeneration.rs +++ b/apollo-federation/tests/query_plan/build_query_plan_tests/fragment_autogeneration.rs @@ -177,6 +177,9 @@ fn it_handles_fragments_with_one_non_leaf_field() { ); } +/// XXX(@goto-bus-stop): this test is meant to check that fragments with @skip and @include *are* +/// migrated. But we are currently matching JS behavior, where they are not. This test should be +/// updated when we remove JS compatibility. #[test] fn it_migrates_skip_include() { let planner = planner!( @@ -348,6 +351,50 @@ fn fragments_that_share_a_hash_but_are_not_identical_generate_their_own_fragment ); } +#[test] +fn same_as_js_router798() { + let planner = planner!( + config = QueryPlannerConfig { generate_query_fragments: true, reuse_query_fragments: false, ..Default::default() }, + Subgraph1: r#" + interface Element { a: Int } + type Y implements Element { a: Int b: Int } + type Z implements Element { a: Int c: Int } + + type Query { + elementsByProject(projectId: Int!): Element + } + "#, + ); + assert_plan!( + &planner, + r#" + query($var0: Boolean! = true) { + ... @skip(if: $var0) { + field0: elementsByProject(projectId: 0) { + field1: __typename + } + } + } + "#, + @r###" + QueryPlan { + Skip(if: $var0) { + Fetch(service: "Subgraph1") { + { + ... { + field0: elementsByProject(projectId: 0) { + __typename + field1: __typename + } + } + } + }, + }, + } + "### + ); +} + #[test] fn works_with_key_chains() { let planner = planner!( From 17af9e70ca312671b0f9136fc34de095b4b86104 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Thu, 3 Oct 2024 13:08:01 +0200 Subject: [PATCH 2/3] don't forget to check in the supergraph! --- .../supergraphs/same_as_js_router798.graphql | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 apollo-federation/tests/query_plan/supergraphs/same_as_js_router798.graphql diff --git a/apollo-federation/tests/query_plan/supergraphs/same_as_js_router798.graphql b/apollo-federation/tests/query_plan/supergraphs/same_as_js_router798.graphql new file mode 100644 index 0000000000..17fab12a27 --- /dev/null +++ b/apollo-federation/tests/query_plan/supergraphs/same_as_js_router798.graphql @@ -0,0 +1,73 @@ +# Composed from subgraphs with hash: aa93183462e2b32e88609b0ccf81621fff86c4e9 +schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.4", for: EXECUTION) +{ + query: Query +} + +directive @join__directive(graphs: [join__Graph!], name: String!, args: join__DirectiveArguments) repeatable on SCHEMA | OBJECT | INTERFACE | FIELD_DEFINITION + +directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE + +directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean, overrideLabel: String) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION + +directive @join__graph(name: String!, url: String!) on ENUM_VALUE + +directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE + +directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR + +directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION + +directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA + +interface Element + @join__type(graph: SUBGRAPH1) +{ + a: Int +} + +scalar join__DirectiveArguments + +scalar join__FieldSet + +enum join__Graph { + SUBGRAPH1 @join__graph(name: "Subgraph1", url: "none") +} + +scalar link__Import + +enum link__Purpose { + """ + `SECURITY` features provide metadata necessary to securely resolve fields. + """ + SECURITY + + """ + `EXECUTION` features provide metadata necessary for operation execution. + """ + EXECUTION +} + +type Query + @join__type(graph: SUBGRAPH1) +{ + elementsByProject(projectId: Int!): Element +} + +type Y implements Element + @join__implements(graph: SUBGRAPH1, interface: "Element") + @join__type(graph: SUBGRAPH1) +{ + a: Int + b: Int +} + +type Z implements Element + @join__implements(graph: SUBGRAPH1, interface: "Element") + @join__type(graph: SUBGRAPH1) +{ + a: Int + c: Int +} From 9457306cd60fb76d3b7f131798dc2855a935e59f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Thu, 3 Oct 2024 14:53:11 +0200 Subject: [PATCH 3/3] tweak --- .../fragment_autogeneration.rs | 12 ++++++------ .../supergraphs/same_as_js_router798.graphql | 14 +++++++------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/apollo-federation/tests/query_plan/build_query_plan_tests/fragment_autogeneration.rs b/apollo-federation/tests/query_plan/build_query_plan_tests/fragment_autogeneration.rs index cd0de19455..03b43c6245 100644 --- a/apollo-federation/tests/query_plan/build_query_plan_tests/fragment_autogeneration.rs +++ b/apollo-federation/tests/query_plan/build_query_plan_tests/fragment_autogeneration.rs @@ -356,12 +356,12 @@ fn same_as_js_router798() { let planner = planner!( config = QueryPlannerConfig { generate_query_fragments: true, reuse_query_fragments: false, ..Default::default() }, Subgraph1: r#" - interface Element { a: Int } - type Y implements Element { a: Int b: Int } - type Z implements Element { a: Int c: Int } + interface Interface { a: Int } + type Y implements Interface { a: Int b: Int } + type Z implements Interface { a: Int c: Int } type Query { - elementsByProject(projectId: Int!): Element + interfaces(id: Int!): Interface } "#, ); @@ -370,7 +370,7 @@ fn same_as_js_router798() { r#" query($var0: Boolean! = true) { ... @skip(if: $var0) { - field0: elementsByProject(projectId: 0) { + field0: interfaces(id: 0) { field1: __typename } } @@ -382,7 +382,7 @@ fn same_as_js_router798() { Fetch(service: "Subgraph1") { { ... { - field0: elementsByProject(projectId: 0) { + field0: interfaces(id: 0) { __typename field1: __typename } diff --git a/apollo-federation/tests/query_plan/supergraphs/same_as_js_router798.graphql b/apollo-federation/tests/query_plan/supergraphs/same_as_js_router798.graphql index 17fab12a27..6895c4bf96 100644 --- a/apollo-federation/tests/query_plan/supergraphs/same_as_js_router798.graphql +++ b/apollo-federation/tests/query_plan/supergraphs/same_as_js_router798.graphql @@ -1,4 +1,4 @@ -# Composed from subgraphs with hash: aa93183462e2b32e88609b0ccf81621fff86c4e9 +# Composed from subgraphs with hash: ee7fce9eb672edf9b036a25bcae0b056ccf5f451 schema @link(url: "https://specs.apollo.dev/link/v1.0") @link(url: "https://specs.apollo.dev/join/v0.4", for: EXECUTION) @@ -22,7 +22,7 @@ directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA -interface Element +interface Interface @join__type(graph: SUBGRAPH1) { a: Int @@ -53,19 +53,19 @@ enum link__Purpose { type Query @join__type(graph: SUBGRAPH1) { - elementsByProject(projectId: Int!): Element + interfaces(id: Int!): Interface } -type Y implements Element - @join__implements(graph: SUBGRAPH1, interface: "Element") +type Y implements Interface + @join__implements(graph: SUBGRAPH1, interface: "Interface") @join__type(graph: SUBGRAPH1) { a: Int b: Int } -type Z implements Element - @join__implements(graph: SUBGRAPH1, interface: "Element") +type Z implements Interface + @join__implements(graph: SUBGRAPH1, interface: "Interface") @join__type(graph: SUBGRAPH1) { a: Int