From ab4a4c5eec08ea378ab6b3d8f4f864a0afe65aa1 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Mon, 17 May 2021 15:12:16 -0500 Subject: [PATCH] fix: removes unwrap from is_federated query --- crates/rover-client/src/error.rs | 2 +- .../src/query/config/is_federated.rs | 30 ++++++------- crates/rover-client/src/query/graph/fetch.rs | 9 ++-- .../rover-client/src/query/graph/publish.rs | 13 ++---- .../rover-client/src/query/subgraph/delete.rs | 17 ++++--- .../rover-client/src/query/subgraph/fetch.rs | 9 ++-- .../src/query/subgraph/introspect.rs | 11 ++--- .../rover-client/src/query/subgraph/list.rs | 9 ++-- .../src/query/subgraph/publish.rs | 5 +-- .../src/query/supergraph/fetch.rs | 9 ++-- src/command/subgraph/check.rs | 13 +----- src/command/subgraph/publish.rs | 44 +++++++++---------- src/error/metadata/suggestion.rs | 2 +- 13 files changed, 68 insertions(+), 105 deletions(-) diff --git a/crates/rover-client/src/error.rs b/crates/rover-client/src/error.rs index 217e6a2e12..d31ef089ca 100644 --- 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 index 2f3db43758..fe8e435de1 100644 --- 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 { +) -> Result<(), RoverClientError> { + let graph = variables.graph_id.clone(); let data = client.post::(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 index e13243248d..2890334e09 100644 --- 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 { - 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 index 88558e7c13..3c5955600b 100644 --- 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 { // 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 index b455c712f1..7e735b2ea6 100644 --- 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 { - 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 index dd7fd54481..d3b2f0d1cd 100644 --- 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 { - 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 index dab13d9629..98e034cea2 100644 --- 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 { - 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 index 7a73cb5270..bd64aceef3 100644 --- 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, 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 index e95aaf7e9d..e4a0f832dd 100644 --- 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 { - 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 index 89f7854dea..82490d1292 100644 --- 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 { - 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 index 185fe4663c..85869d91b6 100644 --- 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,7 +66,7 @@ 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(), @@ -74,15 +74,6 @@ impl Check { &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 index 04e75e4e75..5933ed9167 100644 --- 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::utils::{ + client::StudioClientConfig, + git::GitContext, + loaders::load_schema_from_flag, + parsers::{parse_graph_ref, parse_schema_source, GraphRef, SchemaSource}, }; +use crate::Result; +use crate::{command::RoverStdout, error::RoverError, Suggestion}; use rover_client::query::{ config::is_federated, @@ -74,23 +71,22 @@ 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 index 6143d68a00..36a234eb49 100644 --- 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()