chore: refactor subgraph fetch (#575)

EverlastingBugstopper · EverlastingBugstopper · commit 24f960880153 · 2021-06-23T15:46:36.000-05:00
diff --git a/crates/rover-client/src/query/subgraph/fetch/fetch_query.graphql b/crates/rover-client/src/query/subgraph/fetch/fetch_query.graphql
@@ -1,5 +1,5 @@
-query FetchSubgraphQuery($variant: String!, $graphID: ID!) {
-  service(id: $graphID) {
+query SubgraphFetchQuery($graph_id: ID!, $variant: String!) {
+  service(id: $graph_id) {
     implementingServices(graphVariant: $variant) {
       __typename
       ... on FederatedImplementingServices {
diff --git a/crates/rover-client/src/query/subgraph/fetch/mod.rs b/crates/rover-client/src/query/subgraph/fetch/mod.rs
@@ -0,0 +1,3 @@
+pub mod query_runner;
+pub(crate) mod types;
+pub use types::{SubgraphFetchInput, SubgraphFetchResponse};
diff --git a/crates/rover-client/src/query/subgraph/fetch/query_runner.rs b/crates/rover-client/src/query/subgraph/fetch/query_runner.rs
@@ -1,3 +1,4 @@
+use super::types::*;
 use crate::blocking::StudioClient;
 use crate::RoverClientError;
 use graphql_client::*;
@@ -6,66 +7,69 @@ use graphql_client::*;
 // The paths are relative to the directory where your `Cargo.toml` is located.
 // Both json and the GraphQL schema language are supported as sources for the schema
 #[graphql(
-    query_path = "src/query/subgraph/fetch.graphql",
+    query_path = "src/query/subgraph/fetch/fetch_query.graphql",
     schema_path = ".schema/schema.graphql",
     response_derives = "PartialEq, Debug, Serialize, Deserialize",
     deprecated = "warn"
 )]
 /// This struct is used to generate the module containing `Variables` and
 /// `ResponseData` structs.
-/// Snake case of this name is the mod name. i.e. fetch_subgraph_query
-pub struct FetchSubgraphQuery;
+/// Snake case of this name is the mod name. i.e. subgraph_fetch_query
+pub(crate) struct SubgraphFetchQuery;
 
 /// Fetches a schema from apollo studio and returns its SDL (String)
 pub fn run(
-    variables: fetch_subgraph_query::Variables,
+    input: SubgraphFetchInput,
     client: &StudioClient,
-    // we can't specify this as a variable in the op, so we have to filter the
-    // operation response by this name
-    subgraph: &str,
-) -> Result<String, RoverClientError> {
-    let graph = variables.graph_id.clone();
-    let response_data = client.post::<FetchSubgraphQuery>(variables)?;
-    let services = get_services_from_response_data(response_data, graph)?;
-    get_sdl_for_service(services, subgraph)
-    // if we want json, we can parse & serialize it here
+) -> Result<SubgraphFetchResponse, RoverClientError> {
+    let variables: SubgraphFetchVariables = input.clone().into();
+    let response_data = client.post::<SubgraphFetchQuery>(variables.into())?;
+    get_sdl_from_response_data(input, response_data)
+}
+
+fn get_sdl_from_response_data(
+    input: SubgraphFetchInput,
+    response_data: SubgraphFetchResponseData,
+) -> Result<SubgraphFetchResponse, RoverClientError> {
+    let service_list = get_services_from_response_data(&input.graph_id, response_data)?;
+    let sdl = get_sdl_for_service(&input.subgraph, service_list)?;
+    Ok(SubgraphFetchResponse { sdl })
 }
 
-type ServiceList = Vec<fetch_subgraph_query::FetchSubgraphQueryServiceImplementingServicesOnFederatedImplementingServicesServices>;
 fn get_services_from_response_data(
-    response_data: fetch_subgraph_query::ResponseData,
-    graph: String,
+    graph_id: &str,
+    response_data: SubgraphFetchResponseData,
 ) -> Result<ServiceList, RoverClientError> {
     let service_data = response_data.service.ok_or(RoverClientError::NoService {
-        graph: graph.clone(),
+        graph: graph_id.to_string(),
     })?;
 
     // get list of services
     let services = match service_data.implementing_services {
         Some(services) => Ok(services),
-        // this case may be removable in the near future as unreachable, since
-        // you should still get an `implementingServices` response in the case
-        // of a non-federated graph. Fow now, this case still exists, but
-        // wont' for long. Check on this later (Jake) :)
         None => Err(RoverClientError::ExpectedFederatedGraph {
-            graph: graph.clone(),
+            graph: graph_id.to_string(),
             can_operation_convert: false,
         }),
     }?;
 
     match services {
-        fetch_subgraph_query::FetchSubgraphQueryServiceImplementingServices::FederatedImplementingServices (services) => {
-            Ok(services.services)
-        },
-        fetch_subgraph_query::FetchSubgraphQueryServiceImplementingServices::NonFederatedImplementingService => {
-            Err(RoverClientError::ExpectedFederatedGraph { graph, can_operation_convert: false })
+        Services::FederatedImplementingServices(services) => Ok(services.services),
+        Services::NonFederatedImplementingService => {
+            Err(RoverClientError::ExpectedFederatedGraph {
+                graph: graph_id.to_string(),
+                can_operation_convert: false,
+            })
         }
     }
 }
 
-fn get_sdl_for_service(services: ServiceList, subgraph: &str) -> Result<String, RoverClientError> {
+fn get_sdl_for_service(
+    subgraph_name: &str,
+    services: ServiceList,
+) -> Result<String, RoverClientError> {
     // find the right service by name
-    let service = services.iter().find(|svc| svc.name == subgraph);
+    let service = services.iter().find(|svc| svc.name == subgraph_name);
 
     // if there is a service, get it's active sdl, otherwise, error and list
     // available services to fetch
@@ -75,7 +79,7 @@ fn get_sdl_for_service(services: ServiceList, subgraph: &str) -> Result<String,
         let valid_subgraphs: Vec<String> = services.iter().map(|svc| svc.name.clone()).collect();
 
         Err(RoverClientError::NoSubgraphInGraph {
-            invalid_subgraph: subgraph.to_string(),
+            invalid_subgraph: subgraph_name.to_string(),
             valid_subgraphs,
         })
     }
@@ -109,9 +113,8 @@ mod tests {
                 }
             }
         });
-        let data: fetch_subgraph_query::ResponseData =
-            serde_json::from_value(json_response).unwrap();
-        let output = get_services_from_response_data(data, "mygraph".to_string());
+        let data: SubgraphFetchResponseData = serde_json::from_value(json_response).unwrap();
+        let output = get_services_from_response_data("mygraph", data);
 
         let expected_json = json!([
           {
@@ -140,9 +143,8 @@ mod tests {
                 "implementingServices": null
             }
         });
-        let data: fetch_subgraph_query::ResponseData =
-            serde_json::from_value(json_response).unwrap();
-        let output = get_services_from_response_data(data, "mygraph".to_string());
+        let data: SubgraphFetchResponseData = serde_json::from_value(json_response).unwrap();
+        let output = get_services_from_response_data("mygraph", data);
         assert!(output.is_err());
     }
 
@@ -163,7 +165,7 @@ mod tests {
           }
         ]);
         let service_list: ServiceList = serde_json::from_value(json_service_list).unwrap();
-        let output = get_sdl_for_service(service_list, "accounts2");
+        let output = get_sdl_for_service("accounts2", service_list);
         assert_eq!(
             output.unwrap(),
             "extend type User @key(fields: \"id\") {\n  id: ID! @external\n  age: Int\n}\n"
@@ -188,7 +190,7 @@ mod tests {
           }
         ]);
         let service_list: ServiceList = serde_json::from_value(json_service_list).unwrap();
-        let output = get_sdl_for_service(service_list, "harambe-was-an-inside-job");
+        let output = get_sdl_for_service("harambe-was-an-inside-job", service_list);
         assert!(output.is_err());
     }
 }
diff --git a/crates/rover-client/src/query/subgraph/fetch/types.rs b/crates/rover-client/src/query/subgraph/fetch/types.rs
@@ -0,0 +1,42 @@
+use super::query_runner::subgraph_fetch_query;
+
+pub(crate) type ServiceList = Vec<subgraph_fetch_query::SubgraphFetchQueryServiceImplementingServicesOnFederatedImplementingServicesServices>;
+pub(crate) type SubgraphFetchResponseData = subgraph_fetch_query::ResponseData;
+pub(crate) type Services = subgraph_fetch_query::SubgraphFetchQueryServiceImplementingServices;
+pub(crate) type QueryVariables = subgraph_fetch_query::Variables;
+
+#[derive(Debug, Clone, PartialEq)]
+pub struct SubgraphFetchInput {
+    pub graph_id: String,
+    pub variant: String,
+    pub subgraph: String,
+}
+
+#[derive(Debug, Clone, PartialEq)]
+pub(crate) struct SubgraphFetchVariables {
+    graph_id: String,
+    variant: String,
+}
+
+impl From<SubgraphFetchInput> for SubgraphFetchVariables {
+    fn from(input: SubgraphFetchInput) -> Self {
+        Self {
+            graph_id: input.graph_id,
+            variant: input.variant,
+        }
+    }
+}
+
+impl From<SubgraphFetchVariables> for QueryVariables {
+    fn from(fetch_variables: SubgraphFetchVariables) -> Self {
+        Self {
+            graph_id: fetch_variables.graph_id,
+            variant: fetch_variables.variant,
+        }
+    }
+}
+
+#[derive(Debug, Clone, PartialEq)]
+pub struct SubgraphFetchResponse {
+    pub sdl: String,
+}
diff --git a/src/command/subgraph/fetch.rs b/src/command/subgraph/fetch.rs
@@ -2,7 +2,7 @@ use ansi_term::Colour::{Cyan, Yellow};
 use serde::Serialize;
 use structopt::StructOpt;
 
-use rover_client::query::subgraph::fetch;
+use rover_client::query::subgraph::fetch::{query_runner, SubgraphFetchInput};
 
 use crate::command::RoverStdout;
 use crate::utils::client::StudioClientConfig;
@@ -39,15 +39,15 @@ impl Fetch {
             Yellow.normal().paint(&self.profile_name)
         );
 
-        let sdl = fetch::run(
-            fetch::fetch_subgraph_query::Variables {
+        let result = query_runner::run(
+            SubgraphFetchInput {
                 graph_id: self.graph.name.clone(),
                 variant: self.graph.variant.clone(),
+                subgraph: self.subgraph.clone(),
             },
             &client,
-            &self.subgraph,
         )?;
 
-        Ok(RoverStdout::Sdl(sdl))
+        Ok(RoverStdout::Sdl(result.sdl))
     }
 }
diff --git a/src/command/supergraph/compose/do_compose.rs b/src/command/supergraph/compose/do_compose.rs
@@ -5,6 +5,7 @@ use crate::{anyhow, command::RoverStdout, error::RoverError, Result, Suggestion}
 use ansi_term::Colour::Red;
 use camino::Utf8PathBuf;
 
+use rover_client::query::subgraph::fetch::SubgraphFetchInput;
 use rover_client::{
     blocking::GraphQLClient,
     query::subgraph::{fetch, introspect},
@@ -122,13 +123,13 @@ pub(crate) fn get_subgraph_definitions(
                 // obtain SDL and add it to subgraph_definition.
                 let client = client_config.get_client(&profile_name)?;
                 let graphref = parse_graph_ref(graphref)?;
-                let schema = fetch::run(
-                    fetch::fetch_subgraph_query::Variables {
+                let result = fetch::query_runner::run(
+                    SubgraphFetchInput {
                         graph_id: graphref.name.clone(),
                         variant: graphref.variant.clone(),
+                        subgraph: subgraph.clone(),
                     },
                     &client,
-                    subgraph,
                 )?;
 
                 // We don't require a routing_url for this variant of a schema,
@@ -138,7 +139,7 @@ pub(crate) fn get_subgraph_definitions(
                 // and use that when no routing_url is provided.
                 let url = &subgraph_data.routing_url.clone().unwrap_or_default();
 
-                let subgraph_definition = SubgraphDefinition::new(subgraph_name, url, &schema);
+                let subgraph_definition = SubgraphDefinition::new(subgraph_name, url, &result.sdl);
                 subgraphs.push(subgraph_definition);
             }
         }

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+pub mod query_runner;`
	`2`	`+pub(crate) mod types;`
	`3`	`+pub use types::{SubgraphFetchInput, SubgraphFetchResponse};`