fix: removes unwrap from is_federated query

EverlastingBugstopper · EverlastingBugstopper · commit e39a361e0a97 · 2021-05-17T15:23:03.000-05:00
diff --git a/crates/rover-client/src/error.rs b/crates/rover-client/src/error.rs
@@ -109,7 +109,7 @@ pub enum RoverClientError {
     #[error("The response from Apollo Studio was malformed. Response body contains `null` value for \"{null_field}\"")]
     MalformedResponse { null_field: String },
 
-    #[error("The graph `{graph}` is a non-federated graph. This operation is only possible for federated graphs")]
+    #[error("The graph `{graph}` is a non-federated graph. This operation is only possible for federated graphs.")]
     ExpectedFederatedGraph { graph: String },
 
     /// The API returned an invalid ChangeSeverity value
diff --git a/crates/rover-client/src/query/config/is_federated.rs b/crates/rover-client/src/query/config/is_federated.rs
@@ -17,33 +17,33 @@ use graphql_client::*;
 /// Snake case of this name is the mod name. i.e. publish_partial_schema_mutation
 pub struct IsFederatedGraph;
 
-#[derive(Debug, PartialEq)]
-pub struct IsFederatedGraphResponse {
-    pub result: bool,
-}
-
 pub fn run(
     variables: is_federated_graph::Variables,
     client: &StudioClient,
-) -> Result<IsFederatedGraphResponse, RoverClientError> {
+) -> Result<(), RoverClientError> {
+    let graph = variables.graph_id.clone();
     let data = client.post::<IsFederatedGraph>(variables)?;
-    let is_federated_response = data.service.unwrap();
-    Ok(build_response(is_federated_response))
+    build_response(data, graph)
 }
 
-type FederatedResponse = is_federated_graph::IsFederatedGraphService;
 type ImplementingServices = is_federated_graph::IsFederatedGraphServiceImplementingServices;
 
-fn build_response(service: FederatedResponse) -> IsFederatedGraphResponse {
+fn build_response(
+    data: is_federated_graph::ResponseData,
+    graph: String,
+) -> Result<(), RoverClientError> {
+    let service = data.service.ok_or(RoverClientError::NoService {
+        graph: graph.clone(),
+    })?;
     match service.implementing_services {
         Some(typename) => match typename {
-            ImplementingServices::FederatedImplementingServices => {
-                IsFederatedGraphResponse { result: true }
-            }
+            ImplementingServices::FederatedImplementingServices => Ok(()),
             ImplementingServices::NonFederatedImplementingService => {
-                IsFederatedGraphResponse { result: false }
+                Err(RoverClientError::ExpectedFederatedGraph { graph })
             }
         },
-        None => IsFederatedGraphResponse { result: false },
+        None => Err(RoverClientError::MalformedResponse {
+            null_field: "__typename".to_string(),
+        }),
     }
 }
diff --git a/crates/rover-client/src/query/graph/fetch.rs b/crates/rover-client/src/query/graph/fetch.rs
@@ -41,12 +41,9 @@ fn get_schema_from_response_data(
     graph: String,
     invalid_variant: String,
 ) -> Result<String, RoverClientError> {
-    let service_data = match response_data.service {
-        Some(data) => Ok(data),
-        None => Err(RoverClientError::NoService {
-            graph: graph.clone(),
-        }),
-    }?;
+    let service_data = response_data.service.ok_or(RoverClientError::NoService {
+        graph: graph.clone(),
+    })?;
 
     let mut valid_variants = Vec::new();
 
diff --git a/crates/rover-client/src/query/graph/publish.rs b/crates/rover-client/src/query/graph/publish.rs
@@ -39,18 +39,13 @@ fn get_publish_response_from_data(
     graph: String,
 ) -> Result<publish_schema_mutation::PublishSchemaMutationServiceUploadSchema, RoverClientError> {
     // then, from the response data, get .service?.upload_schema?
-    let service_data = match data.service {
-        Some(data) => data,
-        None => return Err(RoverClientError::NoService { graph }),
-    };
+    let service_data = data.service.ok_or(RoverClientError::NoService { graph })?;
 
-    if let Some(opt_data) = service_data.upload_schema {
-        Ok(opt_data)
-    } else {
-        Err(RoverClientError::MalformedResponse {
+    service_data
+        .upload_schema
+        .ok_or(RoverClientError::MalformedResponse {
             null_field: "service.upload_schema".to_string(),
         })
-    }
 }
 
 fn build_response(
diff --git a/crates/rover-client/src/query/subgraph/delete.rs b/crates/rover-client/src/query/subgraph/delete.rs
@@ -43,10 +43,9 @@ fn get_delete_data_from_response(
     response_data: delete_service_mutation::ResponseData,
     graph: String,
 ) -> Result<RawMutationResponse, RoverClientError> {
-    let service_data = match response_data.service {
-        Some(data) => Ok(data),
-        None => Err(RoverClientError::NoService { graph }),
-    }?;
+    let service_data = response_data
+        .service
+        .ok_or(RoverClientError::NoService { graph })?;
 
     Ok(service_data.remove_implementing_service_and_trigger_composition)
 }
@@ -76,7 +75,7 @@ mod tests {
     use super::*;
     use serde_json::json;
 
-    type RawCompositionErrrors = delete_service_mutation::DeleteServiceMutationServiceRemoveImplementingServiceAndTriggerCompositionErrors;
+    type RawCompositionErrors = delete_service_mutation::DeleteServiceMutationServiceRemoveImplementingServiceAndTriggerCompositionErrors;
 
     #[test]
     fn get_delete_data_from_response_works() {
@@ -100,11 +99,11 @@ mod tests {
 
         let expected_response = RawMutationResponse {
             errors: vec![
-                Some(RawCompositionErrrors {
+                Some(RawCompositionErrors {
                     message: "wow".to_string(),
                 }),
                 None,
-                Some(RawCompositionErrrors {
+                Some(RawCompositionErrors {
                     message: "boo".to_string(),
                 }),
             ],
@@ -117,11 +116,11 @@ mod tests {
     fn build_response_works_with_successful_responses() {
         let response = RawMutationResponse {
             errors: vec![
-                Some(RawCompositionErrrors {
+                Some(RawCompositionErrors {
                     message: "wow".to_string(),
                 }),
                 None,
-                Some(RawCompositionErrrors {
+                Some(RawCompositionErrors {
                     message: "boo".to_string(),
                 }),
             ],
diff --git a/crates/rover-client/src/query/subgraph/fetch.rs b/crates/rover-client/src/query/subgraph/fetch.rs
@@ -36,12 +36,9 @@ fn get_services_from_response_data(
     response_data: fetch_subgraph_query::ResponseData,
     graph: String,
 ) -> Result<ServiceList, RoverClientError> {
-    let service_data = match response_data.service {
-        Some(data) => Ok(data),
-        None => Err(RoverClientError::NoService {
-            graph: graph.clone(),
-        }),
-    }?;
+    let service_data = response_data.service.ok_or(RoverClientError::NoService {
+        graph: graph.clone(),
+    })?;
 
     // get list of services
     let services = match service_data.implementing_services {
diff --git a/crates/rover-client/src/query/subgraph/introspect.rs b/crates/rover-client/src/query/subgraph/introspect.rs
@@ -41,14 +41,11 @@ pub fn run(
 }
 
 fn build_response(
-    response: introspection_query::ResponseData,
+    data: introspection_query::ResponseData,
 ) -> Result<IntrospectionResponse, RoverClientError> {
-    let service_data = match response.service {
-        Some(data) => Ok(data),
-        None => Err(RoverClientError::IntrospectionError {
-            msg: "No introspection response available.".to_string(),
-        }),
-    }?;
+    let service_data = data.service.ok_or(RoverClientError::IntrospectionError {
+        msg: "No introspection response available.".to_string(),
+    })?;
 
     Ok(IntrospectionResponse {
         result: service_data.sdl,
diff --git a/crates/rover-client/src/query/subgraph/list.rs b/crates/rover-client/src/query/subgraph/list.rs
@@ -54,12 +54,9 @@ fn get_subgraphs_from_response_data(
     response_data: list_subgraphs_query::ResponseData,
     graph: String,
 ) -> Result<Vec<RawSubgraphInfo>, RoverClientError> {
-    let service_data = match response_data.service {
-        Some(data) => Ok(data),
-        None => Err(RoverClientError::NoService {
-            graph: graph.clone(),
-        }),
-    }?;
+    let service_data = response_data.service.ok_or(RoverClientError::NoService {
+        graph: graph.clone(),
+    })?;
 
     // get list of services
     let services = match service_data.implementing_services {
diff --git a/crates/rover-client/src/query/subgraph/publish.rs b/crates/rover-client/src/query/subgraph/publish.rs
@@ -42,10 +42,7 @@ fn get_publish_response_from_data(
     data: publish_partial_schema_mutation::ResponseData,
     graph: String,
 ) -> Result<UpdateResponse, RoverClientError> {
-    let service_data = match data.service {
-        Some(data) => data,
-        None => return Err(RoverClientError::NoService { graph }),
-    };
+    let service_data = data.service.ok_or(RoverClientError::NoService { graph })?;
 
     Ok(service_data.upsert_implementing_service_and_trigger_composition)
 }
diff --git a/crates/rover-client/src/query/supergraph/fetch.rs b/crates/rover-client/src/query/supergraph/fetch.rs
@@ -37,12 +37,9 @@ fn get_supergraph_sdl_from_response_data(
     graph: String,
     variant: String,
 ) -> Result<String, RoverClientError> {
-    let service_data = match response_data.service {
-        Some(data) => Ok(data),
-        None => Err(RoverClientError::NoService {
-            graph: graph.clone(),
-        }),
-    }?;
+    let service_data = response_data.service.ok_or(RoverClientError::NoService {
+        graph: graph.clone(),
+    })?;
 
     if let Some(schema_tag) = service_data.schema_tag {
         if let Some(composition_result) = schema_tag.composition_result {
diff --git a/src/command/subgraph/check.rs b/src/command/subgraph/check.rs
@@ -2,7 +2,6 @@ use ansi_term::Colour::Red;
 use serde::Serialize;
 use structopt::StructOpt;
 
-use crate::{anyhow, error::RoverError, Result, Suggestion};
 use rover_client::query::{config::is_federated, subgraph::check};
 
 use crate::command::RoverStdout;
@@ -14,6 +13,7 @@ use crate::utils::parsers::{
     parse_schema_source, parse_validation_period, GraphRef, SchemaSource, ValidationPeriod,
 };
 use crate::utils::table::{self, cell, row};
+use crate::Result;
 
 #[derive(Debug, Serialize, StructOpt)]
 pub struct Check {
@@ -66,23 +66,14 @@ impl Check {
         let sdl = load_schema_from_flag(&self.schema, std::io::stdin())?;
 
         // This response is used to check whether or not the current graph is federated.
-        let federated_response = is_federated::run(
+        is_federated::run(
             is_federated::is_federated_graph::Variables {
                 graph_id: self.graph.name.clone(),
                 graph_variant: self.graph.variant.clone(),
             },
             &client,
         )?;
 
-        // We don't want to run subgraph check on a non-federated graph, so
-        // return an error and recommend running graph check instead.
-        if !federated_response.result {
-            let err = anyhow!("Not able to run subgraph check on a non-federated graph.");
-            let mut err = RoverError::new(err);
-            err.set_suggestion(Suggestion::UseFederatedGraph);
-            return Err(err);
-        }
-
         let partial_schema = check::check_partial_schema_query::PartialSchemaInput {
             sdl: Some(sdl),
             // we never need to send the hash since the back end computes it from SDL
diff --git a/src/command/subgraph/publish.rs b/src/command/subgraph/publish.rs
@@ -2,17 +2,14 @@ use ansi_term::Colour::{Cyan, Red, Yellow};
 use serde::Serialize;
 use structopt::StructOpt;
 
-use crate::{anyhow, Result};
-use crate::{command::RoverStdout, error::RoverError};
-use crate::{
-    utils::{
-        client::StudioClientConfig,
-        git::GitContext,
-        loaders::load_schema_from_flag,
-        parsers::{parse_graph_ref, parse_schema_source, GraphRef, SchemaSource},
-    },
-    Suggestion,
+use crate::{Suggestion, error::RoverError, command::RoverStdout};
+use crate::utils::{
+    client::StudioClientConfig,
+    git::GitContext,
+    loaders::load_schema_from_flag,
+    parsers::{parse_graph_ref, parse_schema_source, GraphRef, SchemaSource},
 };
+use crate::Result;
 
 use rover_client::query::{
     config::is_federated,
@@ -74,23 +71,21 @@ impl Publish {
 
         tracing::debug!("Publishing \n{}", &schema_document);
 
-        // This response is used to check whether or not the current graph is federated.
-        let federated_response = is_federated::run(
-            is_federated::is_federated_graph::Variables {
-                graph_id: self.graph.name.clone(),
-                graph_variant: self.graph.variant.clone(),
-            },
-            &client,
-        )?;
-
         // We don't want to implicitly convert non-federated graph to subgraphs.
         // Error here if no --convert flag is passed _and_ the current context
         // is non-federated. Add a suggestion to require a --convert flag.
-        if !federated_response.result && !self.convert {
-            let err = anyhow!("Could not publish a subgraph to a non-federated graph.");
-            let mut err = RoverError::new(err);
-            err.set_suggestion(Suggestion::ConvertGraphToSubgraph);
-            return Err(err);
+        if !self.convert {
+            is_federated::run(
+                is_federated::is_federated_graph::Variables {
+                    graph_id: self.graph.name.clone(),
+                    graph_variant: self.graph.variant.clone(),
+                },
+                &client,
+            ).map_err(|e| {
+                let mut error = RoverError::from(e);
+                error.set_suggestion(Suggestion::ConvertGraphToSubgraph);
+                error
+            })?;
         }
 
         let publish_response = publish::run(
diff --git a/src/error/metadata/suggestion.rs b/src/error/metadata/suggestion.rs
@@ -68,7 +68,7 @@ impl Display for Suggestion {
                 format!("Try resolving the composition errors in your subgraph(s), and publish them with the {} command.", Yellow.normal().paint("`rover subgraph publish`"))
             }
             Suggestion::UseFederatedGraph => {
-                "Try running the command on a valid federated graph or use the appropriate `rover graph` command instead of `rover subgraph`.".to_string()
+                "Try running the command on a valid federated graph, or use the appropriate `rover graph` command instead of `rover subgraph`.".to_string()
             }
             Suggestion::CheckGraphNameAndAuth => {
                 "Make sure your graph name is typed correctly, and that your API key is valid. (Are you using the right profile?)".to_string()
