diff --git a/crates/rover-client/src/error.rs b/crates/rover-client/src/error.rs index 217e6a2e1..3f92d95bd 100644 --- a/crates/rover-client/src/error.rs +++ b/crates/rover-client/src/error.rs @@ -109,8 +109,16 @@ 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")] - ExpectedFederatedGraph { graph: String }, + /// This error occurs when an operation expected a federated graph but a non-federated + /// graph was supplied. + /// `can_operation_convert` is only set to true when a non-federated graph + /// was encountered during an operation that could potentially convert a non-federated graph + /// to a federated graph. + #[error("The graph `{graph}` is a non-federated graph. This operation is only possible for federated graphs.")] + ExpectedFederatedGraph { + graph: String, + can_operation_convert: bool, + }, /// The API returned an invalid ChangeSeverity value #[error("Invalid ChangeSeverity.")] diff --git a/crates/rover-client/src/query/config/is_federated.rs b/crates/rover-client/src/query/config/is_federated.rs index 2f3db4375..34752f6b7 100644 --- a/crates/rover-client/src/query/config/is_federated.rs +++ b/crates/rover-client/src/query/config/is_federated.rs @@ -15,35 +15,31 @@ use graphql_client::*; /// 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 IsFederatedGraph; +pub(crate) struct IsFederatedGraph; -#[derive(Debug, PartialEq)] -pub struct IsFederatedGraphResponse { - pub result: bool, -} - -pub fn run( +pub(crate) fn run( variables: is_federated_graph::Variables, client: &StudioClient, -) -> Result { +) -> Result { + 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 { + let service = data.service.ok_or(RoverClientError::NoService { graph })?; match service.implementing_services { - Some(typename) => match typename { - ImplementingServices::FederatedImplementingServices => { - IsFederatedGraphResponse { result: true } - } - ImplementingServices::NonFederatedImplementingService => { - IsFederatedGraphResponse { result: false } - } - }, - None => IsFederatedGraphResponse { result: false }, + Some(typename) => Ok(match typename { + ImplementingServices::FederatedImplementingServices => true, + ImplementingServices::NonFederatedImplementingService => false, + }), + None => Err(RoverClientError::MalformedResponse { + null_field: "implementing_services".to_string(), + }), } } diff --git a/crates/rover-client/src/query/graph/fetch.rs b/crates/rover-client/src/query/graph/fetch.rs index e13243248..2890334e0 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 88558e7c1..3c5955600 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/check.rs b/crates/rover-client/src/query/subgraph/check.rs index d37b475ec..6f35bfad6 100644 --- a/crates/rover-client/src/query/subgraph/check.rs +++ b/crates/rover-client/src/query/subgraph/check.rs @@ -1,5 +1,7 @@ use crate::blocking::StudioClient; +use crate::query::config::is_federated; use crate::RoverClientError; + use graphql_client::*; use reqwest::Url; @@ -27,6 +29,20 @@ pub fn run( client: &StudioClient, ) -> Result { let graph = variables.graph_id.clone(); + // This response is used to check whether or not the current graph is federated. + let is_federated = is_federated::run( + is_federated::is_federated_graph::Variables { + graph_id: variables.graph_id.clone(), + graph_variant: variables.variant.clone(), + }, + &client, + )?; + if is_federated { + return Err(RoverClientError::ExpectedFederatedGraph { + graph, + can_operation_convert: false, + }); + } let data = client.post::(variables)?; get_check_response_from_data(data, graph) } diff --git a/crates/rover-client/src/query/subgraph/delete.rs b/crates/rover-client/src/query/subgraph/delete.rs index b455c712f..7e735b2ea 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 dd7fd5448..9509d3951 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 { @@ -52,6 +49,7 @@ fn get_services_from_response_data( // wont' for long. Check on this later (Jake) :) None => Err(RoverClientError::ExpectedFederatedGraph { graph: graph.clone(), + can_operation_convert: false, }), }?; @@ -60,7 +58,7 @@ fn get_services_from_response_data( Ok(services.services) }, fetch_subgraph_query::FetchSubgraphQueryServiceImplementingServices::NonFederatedImplementingService => { - Err(RoverClientError::ExpectedFederatedGraph { graph }) + Err(RoverClientError::ExpectedFederatedGraph { graph, can_operation_convert: false }) } } } diff --git a/crates/rover-client/src/query/subgraph/introspect.rs b/crates/rover-client/src/query/subgraph/introspect.rs index dab13d962..98e034cea 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 7a73cb527..a7752d5f7 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 { @@ -79,7 +76,7 @@ fn get_subgraphs_from_response_data( Ok(services.services) }, list_subgraphs_query::ListSubgraphsQueryServiceImplementingServices::NonFederatedImplementingService => { - Err(RoverClientError::ExpectedFederatedGraph { graph }) + Err(RoverClientError::ExpectedFederatedGraph { graph, can_operation_convert: false }) } } } diff --git a/crates/rover-client/src/query/subgraph/publish.rs b/crates/rover-client/src/query/subgraph/publish.rs index e95aaf7e9..3b08f088a 100644 --- a/crates/rover-client/src/query/subgraph/publish.rs +++ b/crates/rover-client/src/query/subgraph/publish.rs @@ -1,5 +1,6 @@ // PublishPartialSchemaMutation use crate::blocking::StudioClient; +use crate::query::config::is_federated; use crate::RoverClientError; use graphql_client::*; @@ -28,8 +29,28 @@ pub struct PublishPartialSchemaResponse { pub fn run( variables: publish_partial_schema_mutation::Variables, client: &StudioClient, + convert_to_federated_graph: bool, ) -> Result { let graph = variables.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 { + let is_federated = is_federated::run( + is_federated::is_federated_graph::Variables { + graph_id: variables.graph_id.clone(), + graph_variant: variables.graph_variant.clone(), + }, + &client, + )?; + + if !is_federated { + return Err(RoverClientError::ExpectedFederatedGraph { + graph, + can_operation_convert: true, + }); + } + } let data = client.post::(variables)?; let publish_response = get_publish_response_from_data(data, graph)?; Ok(build_response(publish_response)) @@ -42,10 +63,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 89f7854de..dc1b7b235 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 { @@ -54,7 +51,10 @@ fn get_supergraph_sdl_from_response_data( }) } } else { - Err(RoverClientError::ExpectedFederatedGraph { graph }) + Err(RoverClientError::ExpectedFederatedGraph { + graph, + can_operation_convert: false, + }) } } else if let Some(most_recent_composition_publish) = service_data.most_recent_composition_publish @@ -83,7 +83,10 @@ fn get_supergraph_sdl_from_response_data( frontend_url_root: response_data.frontend_url_root, }) } else { - Err(RoverClientError::ExpectedFederatedGraph { graph }) + Err(RoverClientError::ExpectedFederatedGraph { + graph, + can_operation_convert: false, + }) } } } @@ -213,7 +216,11 @@ mod tests { let data: fetch_supergraph_query::ResponseData = serde_json::from_value(json_response).unwrap(); let output = get_supergraph_sdl_from_response_data(data, graph.clone(), variant.clone()); - let expected_error = RoverClientError::ExpectedFederatedGraph { graph }.to_string(); + let expected_error = RoverClientError::ExpectedFederatedGraph { + graph, + can_operation_convert: false, + } + .to_string(); let actual_error = output.unwrap_err().to_string(); assert_eq!(actual_error, expected_error); } diff --git a/src/command/subgraph/check.rs b/src/command/subgraph/check.rs index 185fe4663..d4e85770a 100644 --- a/src/command/subgraph/check.rs +++ b/src/command/subgraph/check.rs @@ -2,8 +2,7 @@ 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 rover_client::query::subgraph::check; use crate::command::RoverStdout; use crate::utils::client::StudioClientConfig; @@ -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 { @@ -65,24 +65,6 @@ 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::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 index 04e75e4e7..6781d50c4 100644 --- a/src/command/subgraph/publish.rs +++ b/src/command/subgraph/publish.rs @@ -2,22 +2,16 @@ 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::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, - subgraph::publish::{self, PublishPartialSchemaResponse}, -}; +use rover_client::query::subgraph::publish::{self, PublishPartialSchemaResponse}; #[derive(Debug, Serialize, StructOpt)] pub struct Publish { @@ -74,25 +68,6 @@ 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); - } - let publish_response = publish::run( publish::publish_partial_schema_mutation::Variables { graph_id: self.graph.name.clone(), @@ -108,6 +83,7 @@ impl Publish { git_context: git_context.into(), }, &client, + self.convert, )?; handle_publish_response(publish_response, &self.subgraph, &self.graph.name); diff --git a/src/error/metadata/mod.rs b/src/error/metadata/mod.rs index cdfda2241..8bc0d67ad 100644 --- a/src/error/metadata/mod.rs +++ b/src/error/metadata/mod.rs @@ -58,10 +58,19 @@ impl From<&mut anyhow::Error> for Metadata { RoverClientError::InvalidSeverity => { (Some(Suggestion::SubmitIssue), Some(Code::E006)) } - RoverClientError::SubgraphIntrospectionNotAvailable - | RoverClientError::ExpectedFederatedGraph { graph: _ } => { + RoverClientError::SubgraphIntrospectionNotAvailable => { (Some(Suggestion::UseFederatedGraph), Some(Code::E007)) } + RoverClientError::ExpectedFederatedGraph { + graph: _, + can_operation_convert, + } => { + if *can_operation_convert { + (Some(Suggestion::ConvertGraphToSubgraph), Some(Code::E007)) + } else { + (Some(Suggestion::UseFederatedGraph), Some(Code::E007)) + } + } RoverClientError::NoSchemaForVariant { graph, invalid_variant, diff --git a/src/error/metadata/suggestion.rs b/src/error/metadata/suggestion.rs index 6143d68a0..36a234eb4 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()