Skip to content

Commit

Permalink
Add ability for router to deal with query plans with contextual rewri…
Browse files Browse the repository at this point in the history
…tes (#5097)

Two main things that we're doing in this PR. 
1. We've added a variable to FetchNode called `context_rewrites`. This is a vector of DataRewrite::KeyRenamer that are specifically taking data from their path (which will be relative and can traverse up the data path) and writes the data into an argument that is passed to the selection set.
2. There are two cases. In the most straightforward, the data that is passed to the selection set is the same for every entity. This case is pretty easy and doesn't require any special handling. In the second case, the value of the variable may be different per entity. If that is true, we need to use aliasing and duplication in our query in order to send it to subgraphs. Once graphql/composite-schemas-spec#25 is decided and has subgraph support, this query cloning will be able to go away. 

Co-authored-by: o0Ignition0o <[email protected]>
Co-authored-by: Gary Pennington <[email protected]>
  • Loading branch information
3 people authored May 23, 2024
1 parent d34c70b commit 5a0826e
Show file tree
Hide file tree
Showing 51 changed files with 2,953 additions and 150 deletions.
7 changes: 7 additions & 0 deletions .changesets/feat_clenfest_set_context.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
### Add ability for router to deal with query plans with contextual rewrites ([PR #5097](https://github.com/apollographql/router/pull/5097))

Adds the ability for the router to execute query plans with context rewrites on them. These are generated by the @fromContext directive and are used to map a Value in the collected data JSON onto a variable which will in turn be used as an argument to a field resolver.

⚠️ This ships with a new version of federation, which means distributed caches will be repopulated.

By [@clenfest](https://github.com/clenfest) in https://github.com/apollographql/router/pull/5097
6 changes: 3 additions & 3 deletions .config/nextest.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ path = "junit.xml"
# Integration tests require more than one thread. The default setting of 1 will cause too many integration tests to run
# at the same time and causes tests to fail where timing is involved.
# This filter applies only to to the integration tests in the apollo-router package.
[[profile.default.overrides]]
filter = 'package(apollo-router) & kind(test)'
threads-required = 2
[[profile.ci.overrides]]
filter = 'test(/^apollo-router::/)'
threads-required = 4
54 changes: 27 additions & 27 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1374,9 +1374,9 @@ checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610"

[[package]]
name = "bytes"
version = "1.5.0"
version = "1.6.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a2bd12c1caf447e69cd4528f47f94d203fd2582878ecb9e9465484c4148a8223"
checksum = "514de17de45fdb8dc022b1a7975556c53c86f9f0aa5f534b98977b171857c2c9"

[[package]]
name = "bytes-utils"
Expand Down Expand Up @@ -1940,9 +1940,9 @@ dependencies = [

[[package]]
name = "curve25519-dalek"
version = "4.0.0"
version = "4.1.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f711ade317dd348950a9910f81c5947e3d8907ebd2b83f76203ff1807e6a2bc2"
checksum = "0a677b8922c94e01bdbb12126b0bc852f00447528dee1782229af9c720c3f348"
dependencies = [
"cfg-if",
"cpufeatures",
Expand Down Expand Up @@ -2419,7 +2419,7 @@ dependencies = [
"digest 0.10.7",
"elliptic-curve 0.13.8",
"rfc6979 0.4.0",
"signature 2.0.0",
"signature 2.2.0",
"spki 0.7.2",
]

Expand Down Expand Up @@ -2659,9 +2659,9 @@ dependencies = [

[[package]]
name = "fiat-crypto"
version = "0.1.20"
version = "0.2.9"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e825f6987101665dea6ec934c09ec6d721de7bc1bf92248e1d5810c8cd636b77"
checksum = "28dea519a9695b9977216879a3ebfddf92f1c08c05d984f8996aecd6ecdc811d"

[[package]]
name = "filetime"
Expand Down Expand Up @@ -3954,9 +3954,9 @@ dependencies = [

[[package]]
name = "libz-ng-sys"
version = "1.1.12"
version = "1.1.15"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3dd9f43e75536a46ee0f92b758f6b63846e594e86638c61a9251338a65baea63"
checksum = "c6409efc61b12687963e602df8ecf70e8ddacf95bc6576bcf16e3ac6328083c5"
dependencies = [
"cmake",
"libc",
Expand Down Expand Up @@ -4150,9 +4150,9 @@ checksum = "68354c5c6bd36d73ff3feceb05efa59b6acb7626617f4962be322a825e61f79a"

[[package]]
name = "miniz_oxide"
version = "0.7.1"
version = "0.7.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e7810e0be55b428ada41041c41f32c9f1a42817901b4ccf45fa3d4b6561e74c7"
checksum = "9d811f3e15f28568be3407c8e7fdb6514c1cda3cb30683f15b6a1a1dc4ea14a7"
dependencies = [
"adler",
]
Expand Down Expand Up @@ -5195,7 +5195,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7f4c021e1093a56626774e81216a4ce732a735e5bad4868a03f3ed65ca0c3919"
dependencies = [
"once_cell",
"toml_edit 0.19.14",
"toml_edit 0.19.15",
]

[[package]]
Expand Down Expand Up @@ -5776,9 +5776,9 @@ dependencies = [

[[package]]
name = "router-bridge"
version = "0.5.21+v2.7.5"
version = "0.5.24+v2.8.0-alpha.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b2142445fe3fe2aae7a3c3c5083d1211a448a0dabb489a14dd90d427cf6c0b13"
checksum = "4a4b92a40b68c797d2d624716d5671e80de578f00c5012af37f19c74b175566b"
dependencies = [
"anyhow",
"async-channel 1.9.0",
Expand Down Expand Up @@ -6149,9 +6149,9 @@ checksum = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3"

[[package]]
name = "serde"
version = "1.0.197"
version = "1.0.199"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3fb1c873e1b9b056a4dc4c0c198b24c3ffa059243875552b2bd0933b1aee4ce2"
checksum = "0c9f6e76df036c77cd94996771fb40db98187f096dd0b9af39c6c6e452ba966a"
dependencies = [
"serde_derive",
]
Expand All @@ -6167,9 +6167,9 @@ dependencies = [

[[package]]
name = "serde_derive"
version = "1.0.197"
version = "1.0.199"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7eb0b34b42edc17f6b7cac84a52a1c5f0e1bb2227e997ca9011ea3dd34e8610b"
checksum = "11bd257a6541e141e42ca6d24ae26f7714887b47e89aa739099104c7e4d3b7fc"
dependencies = [
"proc-macro2 1.0.76",
"quote 1.0.35",
Expand Down Expand Up @@ -6201,9 +6201,9 @@ dependencies = [

[[package]]
name = "serde_json"
version = "1.0.114"
version = "1.0.116"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c5f09b1bd632ef549eaa9f60a1f8de742bdbc698e6cee2095fc84dde5f549ae0"
checksum = "3e17db7126d17feb94eb3fad46bf1a96b034e8aacbc2e775fe81505f8b0b2813"
dependencies = [
"indexmap 2.2.3",
"itoa",
Expand Down Expand Up @@ -6385,9 +6385,9 @@ dependencies = [

[[package]]
name = "signature"
version = "2.0.0"
version = "2.2.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8fe458c98333f9c8152221191a77e2a44e8325d0193484af2e9421a53019e57d"
checksum = "77549399552de45a898a580c1b41d445bf730df867cc44e6c0233bbc4b8329de"
dependencies = [
"digest 0.10.7",
"rand_core 0.6.4",
Expand Down Expand Up @@ -7070,9 +7070,9 @@ dependencies = [

[[package]]
name = "toml_edit"
version = "0.19.14"
version = "0.19.15"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f8123f27e969974a3dfba720fdb560be359f57b44302d280ba72e76a74480e8a"
checksum = "1b5bb770da30e5cbfde35a2d7b9b8a2c4b8ef89548a7a6aeab5c9a576e3e7421"
dependencies = [
"indexmap 2.2.3",
"toml_datetime",
Expand Down Expand Up @@ -7609,9 +7609,9 @@ checksum = "92888ba5573ff080736b3648696b70cafad7d250551175acbaa4e0385b3e1460"

[[package]]
name = "unicode-id"
version = "0.3.3"
version = "0.3.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d70b6494226b36008c8366c288d77190b3fad2eb4c10533139c1c1f461127f1a"
checksum = "b1b6def86329695390197b82c1e244a54a131ceb66c996f2088a3876e2ae083f"

[[package]]
name = "unicode-ident"
Expand Down Expand Up @@ -8205,7 +8205,7 @@ version = "2.0.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "fb66477291e7e8d2b0ff1bcb900bf29489a9692816d79874bea351e7a8b6de96"
dependencies = [
"curve25519-dalek 4.0.0",
"curve25519-dalek 4.1.2",
"rand_core 0.6.4",
"serde",
"zeroize",
Expand Down
4 changes: 4 additions & 0 deletions apollo-federation/src/link/join_spec_definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,10 @@ lazy_static! {
Version { major: 0, minor: 3 },
Some(Version { major: 2, minor: 0 }),
));
definitions.add(JoinSpecDefinition::new(
Version { major: 0, minor: 5 },
Some(Version { major: 2, minor: 8 }),
));
definitions
};

Expand Down
1 change: 1 addition & 0 deletions apollo-federation/src/query_plan/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ impl FetchNode {
operation_kind: _,
input_rewrites: _,
output_rewrites: _,
context_rewrites: _,
} = self;
state.write(format_args!("Fetch(service: {subgraph_name:?}"))?;
if let Some(id) = id {
Expand Down
1 change: 1 addition & 0 deletions apollo-federation/src/query_plan/fetch_dependency_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1320,6 +1320,7 @@ impl FetchDependencyGraphNode {
operation_kind: self.root_kind.into(),
input_rewrites: self.input_rewrites.clone(),
output_rewrites,
context_rewrites: Default::default(),
}));

Ok(Some(if let Some(path) = self.merge_at.clone() {
Expand Down
3 changes: 3 additions & 0 deletions apollo-federation/src/query_plan/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ pub struct FetchNode {
/// Similar to `input_rewrites`, but for optional "rewrites" to apply to the data that is
/// received from a fetch (and before it is applied to the current in-memory results).
pub output_rewrites: Vec<Arc<FetchDataRewrite>>,
/// Similar to the other kinds of rewrites. This is a mechanism to convert a contextual path into
/// an argument to a resolver
pub context_rewrites: Vec<Arc<FetchDataRewrite>>,
}

#[derive(Debug, Clone)]
Expand Down
1 change: 1 addition & 0 deletions apollo-federation/src/query_plan/query_planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ impl QueryPlanner {
requires: Default::default(),
input_rewrites: Default::default(),
output_rewrites: Default::default(),
context_rewrites: Default::default(),
};

return Ok(QueryPlan::new(node, statistics));
Expand Down
29 changes: 13 additions & 16 deletions apollo-federation/tests/query_plan/build_query_plan_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,6 @@ fn pick_keys_that_minimize_fetches() {
/// (more precisely, this force the query planner to _consider_ type explosion; the generated
/// query plan still ends up not type-exploding in practice since as it's not necessary).
#[test]
#[should_panic(expected = "snapshot assertion")]
// TODO: investigate this failure
fn field_covariance_and_type_explosion() {
let planner = planner!(
Subgraph1: r#"
Expand Down Expand Up @@ -195,23 +193,22 @@ fn field_covariance_and_type_explosion() {
}
"#,
@r###"
QueryPlan {
Fetch(service: "Subgraph1") {
{
dummy {
QueryPlan {
Fetch(service: "Subgraph1") {
{
dummy {
field {
__typename
... on Object {
field {
__typename
field {
__typename
... on Object {
field {
__typename
}
}
}
}
}
},
}
}
"###
}
},
}
"###
);
}
Loading

0 comments on commit 5a0826e

Please sign in to comment.