fix(compose, dev): improve --graph-ref option (#2101)

Make `rover supergraph compose --graph-ref REF --config CONFIG` work as documented (and don't remove `routing_url`s fetched from GraphOS if they're missing from the local file). Make `rover supergraph compose --graph-ref REF` and `rover dev --graph-ref REF` use the graph ref's federation version. Fix docs. (Individual commits have more detailed messages.)
apollographql · Sep 6, 2024 · 2011fab · 2011fab
1 parent 44718d5
commit 2011fab
Show file tree

Hide file tree

Showing 9 changed files with 172 additions and 156 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -63,7 +63,7 @@ apollo-parser = "0.8"
 apollo-encoder = "0.8"
 
 # https://github.com/apollographql/federation-rs
-apollo-federation-types = "0.13.1"
+apollo-federation-types = "0.13.2"
 
 # crates.io dependencies
 ariadne = "0.4"
@@ -72,7 +72,7 @@ assert_cmd = "2"
 assert-json-diff = "2"
 anyhow = "1"
 backtrace = "0.3"
-backoff = { version = "0.4", features = [ "tokio" ]}
+backoff = { version = "0.4", features = ["tokio"] }
 base64 = "0.22"
 billboard = "0.2"
 buildstructor = "0.5.4"
@@ -102,7 +102,9 @@ interprocess = { version = "2", default-features = false }
 indoc = "2"
 lazycell = "1"
 lazy_static = "1.4"
-notify = { version = "6", default-features = false, features = ["macos_kqueue"] }
+notify = { version = "6", default-features = false, features = [
+    "macos_kqueue",
+] }
 notify-debouncer-full = "0.3.1"
 opener = "0.7"
 os_info = "3.7"
@@ -201,7 +203,7 @@ assert_fs = { workspace = true }
 assert-json-diff = { workspace = true }
 dircpy = "0.3.19"
 duct = "0.13.7"
-git2 = { workspace = true, features = ["https"]}
+git2 = { workspace = true, features = ["https"] }
 graphql-schema-diff = "0.2.0"
 httpmock = { workspace = true }
 indoc = { workspace = true }

diff --git a/crates/rover-client/src/operations/subgraph/fetch_all/fetch_all_query.graphql b/crates/rover-client/src/operations/subgraph/fetch_all/fetch_all_query.graphql
@@ -9,6 +9,14 @@ query SubgraphFetchAllQuery($graph_ref: ID!) {
           sdl
         }
       }
+      latestLaunch {
+        buildInput {
+          __typename
+          ... on CompositionBuildInput {
+            version
+          }
+        }
+      }
       sourceVariant {
         subgraphs {
           name
@@ -17,6 +25,14 @@ query SubgraphFetchAllQuery($graph_ref: ID!) {
             sdl
           }
         }
+        latestLaunch {
+          buildInput {
+            __typename
+            ... on CompositionBuildInput {
+              version
+            }
+          }
+        }
       }
     }
   }

diff --git a/crates/rover-client/src/operations/subgraph/fetch_all/mod.rs b/crates/rover-client/src/operations/subgraph/fetch_all/mod.rs
@@ -2,4 +2,4 @@ mod runner;
 mod types;
 
 pub use runner::run;
-pub use types::SubgraphFetchAllInput;
+pub use types::{SubgraphFetchAllInput, SubgraphFetchAllResponse};
diff --git a/crates/rover-client/src/operations/subgraph/fetch_all/runner.rs b/crates/rover-client/src/operations/subgraph/fetch_all/runner.rs
@@ -24,7 +24,7 @@ pub(crate) struct SubgraphFetchAllQuery;
 pub async fn run(
     input: SubgraphFetchAllInput,
     client: &StudioClient,
-) -> Result<Vec<Subgraph>, RoverClientError> {
+) -> Result<SubgraphFetchAllResponse, RoverClientError> {
     let variables = input.clone().into();
     let response_data = client.post::<SubgraphFetchAllQuery>(variables).await?;
     get_subgraphs_from_response_data(input, response_data)
@@ -33,7 +33,7 @@ pub async fn run(
 fn get_subgraphs_from_response_data(
     input: SubgraphFetchAllInput,
     response_data: SubgraphFetchAllResponseData,
-) -> Result<Vec<Subgraph>, RoverClientError> {
+) -> Result<SubgraphFetchAllResponse, RoverClientError> {
     match response_data.variant {
         None => Err(RoverClientError::GraphNotFound {
             graph_ref: input.graph_ref,
@@ -47,15 +47,17 @@ fn get_subgraphs_from_response_data(
 fn extract_subgraphs_from_response(
     value: SubgraphFetchAllQueryVariantOnGraphVariant,
     graph_ref: GraphRef,
-) -> Result<Vec<Subgraph>, RoverClientError> {
+) -> Result<SubgraphFetchAllResponse, RoverClientError> {
     match (value.subgraphs, value.source_variant) {
         // If we get null back in both branches or the query, or we get a structure in the
         // sourceVariant half but there are no subgraphs in it. Then we return an error
         // because this isn't a FederatedSubgraph **as far as we can tell**.
         (None, None)
         | (
             None,
-            Some(SubgraphFetchAllQueryVariantOnGraphVariantSourceVariant { subgraphs: None }),
+            Some(SubgraphFetchAllQueryVariantOnGraphVariantSourceVariant {
+                subgraphs: None, ..
+            }),
         ) => Err(RoverClientError::ExpectedFederatedGraph {
             graph_ref,
             can_operation_convert: true,
@@ -66,11 +68,15 @@ fn extract_subgraphs_from_response(
             None,
             Some(SubgraphFetchAllQueryVariantOnGraphVariantSourceVariant {
                 subgraphs: Some(subgraphs),
+                latest_launch,
             }),
-        ) => Ok(subgraphs
-            .into_iter()
-            .map(|subgraph| subgraph.into())
-            .collect()),
+        ) => Ok(SubgraphFetchAllResponse {
+            subgraphs: subgraphs
+                .into_iter()
+                .map(|subgraph| subgraph.into())
+                .collect(),
+            federation_version: latest_launch.and_then(|it| it.into()),
+        }),
         // Here there are three cases where we might want to return the subgraphs we got from
         // directly querying the graphVariant:
         // 1. If we get subgraphs back from the graphVariant directly and nothing from the sourceVariant
@@ -79,16 +85,21 @@ fn extract_subgraphs_from_response(
         // 3. If we get subgraphs back from both 'sides' of the query, we take the results from
         // querying the **graphVariant**, as this is closest to the original behaviour, before
         // we introduced the querying of the sourceVariant.
-        (Some(subgraphs), _) => Ok(subgraphs
-            .into_iter()
-            .map(|subgraph| subgraph.into())
-            .collect()),
+        (Some(subgraphs), _) => Ok(SubgraphFetchAllResponse {
+            subgraphs: subgraphs
+                .into_iter()
+                .map(|subgraph| subgraph.into())
+                .collect(),
+            federation_version: value.latest_launch.and_then(|it| it.into()),
+        }),
     }
 }
 
 #[cfg(test)]
 mod tests {
+    use apollo_federation_types::config::FederationVersion;
     use rstest::{fixture, rstest};
+    use semver::Version;
     use serde_json::{json, Value};
 
     use crate::shared::GraphRef;
@@ -114,10 +125,18 @@ mod tests {
                      }
                 },
             ],
+            "latestLaunch": {
+                "buildInput": {
+                    "__typename": "CompositionBuildInput",
+                    "version": "2.3.4"
+                }
+            },
             "sourceVariant": null
         }
-    }
-    ), Some(vec![Subgraph::builder().url(URL).sdl(SDL).name(SUBGRAPH_NAME).build()]))]
+    }), Some(SubgraphFetchAllResponse {
+        subgraphs: vec![Subgraph::builder().url(URL).sdl(SDL).name(SUBGRAPH_NAME).build()],
+        federation_version: Some(FederationVersion::ExactFedTwo(Version::new(2, 3, 4))),
+    }))]
     #[case::subgraphs_returned_via_source_variant(json!(
     {
         "variant": {
@@ -132,10 +151,19 @@ mod tests {
                         "sdl": SDL
                     }
                 }
-                ]
+                ],
+                "latestLaunch": {
+                    "buildInput": {
+                        "__typename": "CompositionBuildInput",
+                        "version": "2.3.4"
+                    }
+                }
             }
         }
-    }), Some(vec![Subgraph::builder().url(URL).sdl(SDL).name(SUBGRAPH_NAME).build()]))]
+    }), Some(SubgraphFetchAllResponse {
+        subgraphs: vec![Subgraph::builder().url(URL).sdl(SDL).name(SUBGRAPH_NAME).build()],
+        federation_version: Some(FederationVersion::ExactFedTwo(Version::new(2, 3, 4))),
+    }))]
     #[case::no_subgraphs_returned_in_either_case(json!(
     {
         "variant": {
@@ -159,6 +187,12 @@ mod tests {
                 }
             }
         ],
+        "latestLaunch": {
+            "buildInput": {
+                "__typename": "CompositionBuildInput",
+                "version": "2.3.4"
+            }
+        },
         "sourceVariant": {
             "subgraphs": [
                 {
@@ -168,14 +202,23 @@ mod tests {
                         "sdl": SDL
                     }
                  }
-             ]
+             ],
+             "latestLaunch": {
+                "buildInput": {
+                    "__typename": "CompositionBuildInput",
+                    "version": "2.9.9"
+                }
+            }
         }
     }
-    }), Some(vec![Subgraph::builder().url(URL).sdl(SDL).name(SUBGRAPH_NAME).build()]))]
+    }), Some(SubgraphFetchAllResponse {
+        subgraphs: vec![Subgraph::builder().url(URL).sdl(SDL).name(SUBGRAPH_NAME).build()],
+        federation_version: Some(FederationVersion::ExactFedTwo(Version::new(2, 3, 4))),
+    }))]
     fn get_services_from_response_data_works(
         #[from(mock_input)] input: SubgraphFetchAllInput,
         #[case] json_response: Value,
-        #[case] expected_subgraphs: Option<Vec<Subgraph>>,
+        #[case] expected_subgraphs: Option<SubgraphFetchAllResponse>,
     ) {
         let data: SubgraphFetchAllResponseData = serde_json::from_value(json_response).unwrap();
         let output = get_subgraphs_from_response_data(input, data);