chore: refactor subgraph publish (#630)

EverlastingBugstopper · web-flow · commit e124da58e377 · 2021-06-25T12:10:23.000-05:00
diff --git a/crates/rover-client/src/query/subgraph/check/types.rs b/crates/rover-client/src/query/subgraph/check/types.rs
@@ -9,6 +9,7 @@ type QueryVariables = subgraph_check_query::Variables;
 type QueryChangeSeverity = subgraph_check_query::ChangeSeverity;
 type QuerySchema = subgraph_check_query::PartialSchemaInput;
 type QueryConfig = subgraph_check_query::HistoricQueryParameters;
+type GitContextInput = subgraph_check_query::GitContextInput;
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct SubgraphCheckInput {
@@ -99,3 +100,15 @@ pub struct CompositionError {
     pub message: String,
     pub code: Option<String>,
 }
+
+impl From<GitContext> for GitContextInput {
+    fn from(git_context: GitContext) -> GitContextInput {
+        GitContextInput {
+            branch: git_context.branch,
+            commit: git_context.commit,
+            committer: git_context.author,
+            remote_url: git_context.remote_url,
+            message: None,
+        }
+    }
+}
diff --git a/crates/rover-client/src/query/subgraph/publish/mod.rs b/crates/rover-client/src/query/subgraph/publish/mod.rs
@@ -0,0 +1,3 @@
+pub mod mutation_runner;
+pub(crate) mod types;
+pub use types::{SubgraphPublishInput, SubgraphPublishResponse};
diff --git a/crates/rover-client/src/query/subgraph/publish/mutation_runner.rs b/crates/rover-client/src/query/subgraph/publish/mutation_runner.rs
@@ -1,4 +1,4 @@
-// PublishPartialSchemaMutation
+use super::types::*;
 use crate::blocking::StudioClient;
 use crate::query::config::is_federated;
 use crate::RoverClientError;
@@ -8,38 +8,30 @@ 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/publish.graphql",
+    query_path = "src/query/subgraph/publish/publish_mutation.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. publish_partial_schema_mutation
-pub struct PublishPartialSchemaMutation;
-
-#[derive(Debug, PartialEq)]
-pub struct PublishPartialSchemaResponse {
-    pub schema_hash: Option<String>,
-    pub did_update_gateway: bool,
-    pub service_was_created: bool,
-    pub composition_errors: Option<Vec<String>>,
-}
+/// Snake case of this name is the mod name. i.e. subgraph_publish_mutation
+pub struct SubgraphPublishMutation;
 
 pub fn run(
-    variables: publish_partial_schema_mutation::Variables,
+    input: SubgraphPublishInput,
     client: &StudioClient,
-    convert_to_federated_graph: bool,
-) -> Result<PublishPartialSchemaResponse, RoverClientError> {
-    let graph = variables.graph_id.clone();
+) -> Result<SubgraphPublishResponse, RoverClientError> {
+    let variables: MutationVariables = input.clone().into();
+    let graph = input.graph_id.clone();
     // We don't want to implicitly convert non-federated graph to supergraphs.
     // Error here if no --convert flag is passed _and_ the current context
     // is non-federated. Add a suggestion to require a --convert flag.
-    if !convert_to_federated_graph {
+    if !input.convert_to_federated_graph {
         let is_federated = is_federated::run(
             is_federated::is_federated_graph::Variables {
-                graph_id: variables.graph_id.clone(),
-                graph_variant: variables.graph_variant.clone(),
+                graph_id: input.graph_id.clone(),
+                graph_variant: input.variant,
             },
             &client,
         )?;
@@ -51,24 +43,21 @@ pub fn run(
             });
         }
     }
-    let data = client.post::<PublishPartialSchemaMutation>(variables)?;
+    let data = client.post::<SubgraphPublishMutation>(variables)?;
     let publish_response = get_publish_response_from_data(data, graph)?;
     Ok(build_response(publish_response))
 }
 
-// alias this return type since it's disgusting
-type UpdateResponse = publish_partial_schema_mutation::PublishPartialSchemaMutationServiceUpsertImplementingServiceAndTriggerComposition;
-
 fn get_publish_response_from_data(
-    data: publish_partial_schema_mutation::ResponseData,
+    data: ResponseData,
     graph: String,
 ) -> Result<UpdateResponse, RoverClientError> {
     let service_data = data.service.ok_or(RoverClientError::NoService { graph })?;
 
     Ok(service_data.upsert_implementing_service_and_trigger_composition)
 }
 
-fn build_response(publish_response: UpdateResponse) -> PublishPartialSchemaResponse {
+fn build_response(publish_response: UpdateResponse) -> SubgraphPublishResponse {
     let composition_errors: Vec<String> = publish_response
         .errors
         .iter()
@@ -82,7 +71,7 @@ fn build_response(publish_response: UpdateResponse) -> PublishPartialSchemaRespo
         None
     };
 
-    PublishPartialSchemaResponse {
+    SubgraphPublishResponse {
         schema_hash: match publish_response.composition_config {
             Some(config) => Some(config.schema_hash),
             None => None,
@@ -114,7 +103,7 @@ mod tests {
 
         assert_eq!(
             output,
-            PublishPartialSchemaResponse {
+            SubgraphPublishResponse {
                 schema_hash: Some("5gf564".to_string()),
                 composition_errors: Some(vec![
                     "[Accounts] User -> composition error".to_string(),
@@ -139,7 +128,7 @@ mod tests {
 
         assert_eq!(
             output,
-            PublishPartialSchemaResponse {
+            SubgraphPublishResponse {
                 schema_hash: Some("5gf564".to_string()),
                 composition_errors: None,
                 did_update_gateway: true,
@@ -163,7 +152,7 @@ mod tests {
 
         assert_eq!(
             output,
-            PublishPartialSchemaResponse {
+            SubgraphPublishResponse {
                 schema_hash: None,
                 composition_errors: Some(
                     vec!["[Accounts] -> Things went really wrong".to_string()]
diff --git a/crates/rover-client/src/query/subgraph/publish/publish_mutation.graphql b/crates/rover-client/src/query/subgraph/publish/publish_mutation.graphql
@@ -1,19 +1,19 @@
-mutation PublishPartialSchemaMutation(
+mutation SubgraphPublishMutation(
   $graphId: ID!
-  $graphVariant: String!
-  $name: String!
+  $variant: String!
+  $subgraph: String!
   $url: String
   $revision: String!
-  $activePartialSchema: PartialSchemaInput!
+  $schema: PartialSchemaInput!
   $gitContext: GitContextInput!
 ) {
   service(id: $graphId) {
     upsertImplementingServiceAndTriggerComposition(
-      name: $name
+      name: $subgraph
       url: $url
       revision: $revision
-      activePartialSchema: $activePartialSchema
-      graphVariant: $graphVariant
+      activePartialSchema: $schema
+      graphVariant: $variant
       gitContext: $gitContext
     ) {
       compositionConfig {
diff --git a/crates/rover-client/src/query/subgraph/publish/types.rs b/crates/rover-client/src/query/subgraph/publish/types.rs
@@ -0,0 +1,58 @@
+use super::mutation_runner::subgraph_publish_mutation;
+
+use crate::utils::GitContext;
+
+pub(crate) type ResponseData = subgraph_publish_mutation::ResponseData;
+pub(crate) type MutationVariables = subgraph_publish_mutation::Variables;
+pub(crate) type UpdateResponse = subgraph_publish_mutation::SubgraphPublishMutationServiceUpsertImplementingServiceAndTriggerComposition;
+
+type SchemaInput = subgraph_publish_mutation::PartialSchemaInput;
+type GitContextInput = subgraph_publish_mutation::GitContextInput;
+
+#[derive(Debug, Clone, PartialEq)]
+pub struct SubgraphPublishInput {
+    pub graph_id: String,
+    pub variant: String,
+    pub subgraph: String,
+    pub url: Option<String>,
+    pub schema: String,
+    pub git_context: GitContext,
+    pub convert_to_federated_graph: bool,
+}
+
+#[derive(Debug, PartialEq)]
+pub struct SubgraphPublishResponse {
+    pub schema_hash: Option<String>,
+    pub did_update_gateway: bool,
+    pub service_was_created: bool,
+    pub composition_errors: Option<Vec<String>>,
+}
+
+impl From<SubgraphPublishInput> for MutationVariables {
+    fn from(publish_input: SubgraphPublishInput) -> Self {
+        Self {
+            graph_id: publish_input.graph_id,
+            variant: publish_input.variant,
+            subgraph: publish_input.subgraph,
+            url: publish_input.url,
+            schema: SchemaInput {
+                sdl: Some(publish_input.schema),
+                hash: None,
+            },
+            git_context: publish_input.git_context.into(),
+            revision: "".to_string(),
+        }
+    }
+}
+
+impl From<GitContext> for GitContextInput {
+    fn from(git_context: GitContext) -> GitContextInput {
+        GitContextInput {
+            branch: git_context.branch,
+            commit: git_context.commit,
+            committer: git_context.author,
+            remote_url: git_context.remote_url,
+            message: None,
+        }
+    }
+}
diff --git a/crates/rover-client/src/utils/git.rs b/crates/rover-client/src/utils/git.rs
@@ -1,4 +1,4 @@
-use crate::query::{graph, subgraph};
+use crate::query::graph;
 
 use std::env;
 
@@ -163,34 +163,6 @@ impl From<GitContext> for GraphCheckContextInput {
     }
 }
 
-type SubgraphPublishContextInput =
-    subgraph::publish::publish_partial_schema_mutation::GitContextInput;
-impl From<GitContext> for SubgraphPublishContextInput {
-    fn from(git_context: GitContext) -> SubgraphPublishContextInput {
-        SubgraphPublishContextInput {
-            branch: git_context.branch,
-            commit: git_context.commit,
-            committer: git_context.author,
-            remote_url: git_context.remote_url,
-            message: None,
-        }
-    }
-}
-
-type SubgraphCheckContextInput =
-    subgraph::check::query_runner::subgraph_check_query::GitContextInput;
-impl From<GitContext> for SubgraphCheckContextInput {
-    fn from(git_context: GitContext) -> SubgraphCheckContextInput {
-        SubgraphCheckContextInput {
-            branch: git_context.branch,
-            commit: git_context.commit,
-            committer: git_context.author,
-            remote_url: git_context.remote_url,
-            message: None,
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
diff --git a/src/command/subgraph/publish.rs b/src/command/subgraph/publish.rs
@@ -10,7 +10,7 @@ use crate::utils::{
 };
 use crate::Result;
 
-use rover_client::query::subgraph::publish::{self, PublishPartialSchemaResponse};
+use rover_client::query::subgraph::publish::{self, SubgraphPublishInput, SubgraphPublishResponse};
 use rover_client::utils::GitContext;
 
 #[derive(Debug, Serialize, StructOpt)]
@@ -64,34 +64,29 @@ impl Publish {
             Yellow.normal().paint(&self.profile_name)
         );
 
-        let schema_document = load_schema_from_flag(&self.schema, std::io::stdin())?;
+        let schema = load_schema_from_flag(&self.schema, std::io::stdin())?;
 
-        tracing::debug!("Publishing \n{}", &schema_document);
+        tracing::debug!("Publishing \n{}", &schema);
 
-        let publish_response = publish::run(
-            publish::publish_partial_schema_mutation::Variables {
+        let publish_response = publish::mutation_runner::run(
+            SubgraphPublishInput {
                 graph_id: self.graph.name.clone(),
-                graph_variant: self.graph.variant.clone(),
-                name: self.subgraph.clone(),
-                active_partial_schema:
-                    publish::publish_partial_schema_mutation::PartialSchemaInput {
-                        sdl: Some(schema_document),
-                        hash: None,
-                    },
-                revision: "".to_string(),
+                variant: self.graph.variant.clone(),
+                subgraph: self.subgraph.clone(),
                 url: self.routing_url.clone(),
-                git_context: git_context.into(),
+                schema,
+                git_context,
+                convert_to_federated_graph: self.convert,
             },
             &client,
-            self.convert,
         )?;
 
         handle_publish_response(publish_response, &self.subgraph, &self.graph.name);
         Ok(RoverStdout::None)
     }
 }
 
-fn handle_publish_response(response: PublishPartialSchemaResponse, subgraph: &str, graph: &str) {
+fn handle_publish_response(response: SubgraphPublishResponse, subgraph: &str, graph: &str) {
     if response.service_was_created {
         eprintln!(
             "A new subgraph called '{}' for the '{}' graph was created",
@@ -125,13 +120,13 @@ fn handle_publish_response(response: PublishPartialSchemaResponse, subgraph: &st
 
 #[cfg(test)]
 mod tests {
-    use super::{handle_publish_response, PublishPartialSchemaResponse};
+    use super::{handle_publish_response, SubgraphPublishResponse};
 
     // this test is a bit weird, since we can't test the output. We just verify it
     // doesn't error
     #[test]
     fn handle_response_doesnt_error_with_all_successes() {
-        let response = PublishPartialSchemaResponse {
+        let response = SubgraphPublishResponse {
             schema_hash: Some("123456".to_string()),
             did_update_gateway: true,
             service_was_created: true,
@@ -143,7 +138,7 @@ mod tests {
 
     #[test]
     fn handle_response_doesnt_error_with_all_failures() {
-        let response = PublishPartialSchemaResponse {
+        let response = SubgraphPublishResponse {
             schema_hash: None,
             did_update_gateway: false,
             service_was_created: false,

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