diff --git a/crates/rover-client/src/error.rs b/crates/rover-client/src/error.rs index 5d38c1b4d0..555be19cfd 100644 --- a/crates/rover-client/src/error.rs +++ b/crates/rover-client/src/error.rs @@ -105,10 +105,10 @@ pub enum RoverClientError { source: CompositionErrors, }, - #[error("Encountered {} while trying to compose a supergraph.", .source.get_num_errors())] + #[error("Encountered {} while trying to compose a supergraph.", .source.len())] CompositionErrors { source: CompositionErrors }, - #[error("Encountered {} while trying to compose subgraph \"{subgraph}\" into supergraph \"{graph_ref}\".", .source.get_num_errors())] + #[error("Encountered {} while trying to compose subgraph \"{subgraph}\" into supergraph \"{graph_ref}\".", .source.len())] SubgraphCompositionErrors { subgraph: String, graph_ref: GraphRef, diff --git a/crates/rover-client/src/operations/subgraph/check/runner.rs b/crates/rover-client/src/operations/subgraph/check/runner.rs index ecbe70d6c1..e57a0bd3e4 100644 --- a/crates/rover-client/src/operations/subgraph/check/runner.rs +++ b/crates/rover-client/src/operations/subgraph/check/runner.rs @@ -4,7 +4,7 @@ use crate::operations::{ config::is_federated::{self, IsFederatedInput}, subgraph::check::types::MutationResponseData, }; -use crate::shared::{CheckResponse, CompositionError, CompositionErrors, GraphRef, SchemaChange}; +use crate::shared::{CheckResponse, CompositionError, GraphRef, SchemaChange}; use crate::RoverClientError; use graphql_client::*; @@ -112,7 +112,7 @@ fn get_check_response_from_data( Err(RoverClientError::SubgraphCompositionErrors { subgraph, graph_ref, - source: CompositionErrors { composition_errors }, + source: composition_errors.into(), }) } } diff --git a/crates/rover-client/src/operations/subgraph/delete/runner.rs b/crates/rover-client/src/operations/subgraph/delete/runner.rs index 2704ed539f..719fc2694c 100644 --- a/crates/rover-client/src/operations/subgraph/delete/runner.rs +++ b/crates/rover-client/src/operations/subgraph/delete/runner.rs @@ -43,7 +43,7 @@ fn get_delete_data_from_response( } fn build_response(response: MutationComposition) -> SubgraphDeleteResponse { - let composition_errors: Vec = response + let composition_errors: CompositionErrors = response .errors .iter() .filter_map(|error| { @@ -54,15 +54,8 @@ fn build_response(response: MutationComposition) -> SubgraphDeleteResponse { }) .collect(); - // if there are no errors, just return None - let composition_errors = if !composition_errors.is_empty() { - Some(CompositionErrors { composition_errors }) - } else { - None - }; - SubgraphDeleteResponse { - updated_gateway: response.updated_gateway, + supergraph_was_updated: response.updated_gateway, composition_errors, } } @@ -136,19 +129,18 @@ mod tests { assert_eq!( parsed, SubgraphDeleteResponse { - composition_errors: Some(CompositionErrors { - composition_errors: vec![ - CompositionError { - message: "wow".to_string(), - code: None - }, - CompositionError { - message: "boo".to_string(), - code: Some("BOO".to_string()) - } - ] - }), - updated_gateway: false, + composition_errors: vec![ + CompositionError { + message: "wow".to_string(), + code: None + }, + CompositionError { + message: "boo".to_string(), + code: Some("BOO".to_string()) + } + ] + .into(), + supergraph_was_updated: false, } ); } @@ -164,8 +156,8 @@ mod tests { assert_eq!( parsed, SubgraphDeleteResponse { - composition_errors: None, - updated_gateway: true, + composition_errors: CompositionErrors::new(), + supergraph_was_updated: true, } ); } diff --git a/crates/rover-client/src/operations/subgraph/delete/types.rs b/crates/rover-client/src/operations/subgraph/delete/types.rs index 7bc8942b0e..28f809106f 100644 --- a/crates/rover-client/src/operations/subgraph/delete/types.rs +++ b/crates/rover-client/src/operations/subgraph/delete/types.rs @@ -6,6 +6,8 @@ use crate::{ pub(crate) type MutationComposition = subgraph_delete_mutation::SubgraphDeleteMutationServiceRemoveImplementingServiceAndTriggerComposition; pub(crate) type MutationVariables = subgraph_delete_mutation::Variables; +use serde::Serialize; + #[cfg(test)] pub(crate) type MutationCompositionErrors = subgraph_delete_mutation::SubgraphDeleteMutationServiceRemoveImplementingServiceAndTriggerCompositionErrors; @@ -20,10 +22,13 @@ pub struct SubgraphDeleteInput { /// `updated_gateway` is true when composition succeeds and the gateway config /// is updated for the gateway to consume. `composition_errors` is just a list /// of strings for when there are composition errors as a result of the delete. -#[derive(Debug, PartialEq)] +#[derive(Debug, Clone, Serialize, PartialEq)] pub struct SubgraphDeleteResponse { - pub updated_gateway: bool, - pub composition_errors: Option, + #[serde(skip_serializing)] + pub supergraph_was_updated: bool, + + #[serde(flatten)] + pub composition_errors: CompositionErrors, } impl From for MutationVariables { diff --git a/crates/rover-client/src/operations/subgraph/publish/runner.rs b/crates/rover-client/src/operations/subgraph/publish/runner.rs index e58ff5a662..1284a7e8f4 100644 --- a/crates/rover-client/src/operations/subgraph/publish/runner.rs +++ b/crates/rover-client/src/operations/subgraph/publish/runner.rs @@ -60,7 +60,7 @@ fn get_publish_response_from_data( } fn build_response(publish_response: UpdateResponse) -> SubgraphPublishResponse { - let composition_errors: Vec = publish_response + let composition_errors: CompositionErrors = publish_response .errors .iter() .filter_map(|error| { @@ -71,13 +71,6 @@ fn build_response(publish_response: UpdateResponse) -> SubgraphPublishResponse { }) .collect(); - // if there are no errors, just return None - let composition_errors = if !composition_errors.is_empty() { - Some(CompositionErrors { composition_errors }) - } else { - None - }; - SubgraphPublishResponse { schema_hash: match publish_response.composition_config { Some(config) => Some(config.schema_hash), @@ -85,9 +78,7 @@ fn build_response(publish_response: UpdateResponse) -> SubgraphPublishResponse { }, supergraph_was_updated: publish_response.did_update_gateway, subgraph_was_created: publish_response.service_was_created, - composition_errors: composition_errors.unwrap_or_else(|| CompositionErrors { - composition_errors: vec![], - }), + composition_errors, } } @@ -120,18 +111,17 @@ mod tests { output, SubgraphPublishResponse { schema_hash: Some("5gf564".to_string()), - composition_errors: CompositionErrors { - composition_errors: vec![ - CompositionError { - message: "[Accounts] User -> composition error".to_string(), - code: None - }, - CompositionError { - message: "[Products] Product -> another one".to_string(), - code: Some("ERROR".to_string()) - } - ] - }, + composition_errors: vec![ + CompositionError { + message: "[Accounts] User -> composition error".to_string(), + code: None + }, + CompositionError { + message: "[Products] Product -> another one".to_string(), + code: Some("ERROR".to_string()) + } + ] + .into(), supergraph_was_updated: false, subgraph_was_created: true, } @@ -153,9 +143,7 @@ mod tests { output, SubgraphPublishResponse { schema_hash: Some("5gf564".to_string()), - composition_errors: CompositionErrors { - composition_errors: vec![] - }, + composition_errors: CompositionErrors::new(), supergraph_was_updated: true, subgraph_was_created: true, } @@ -182,12 +170,11 @@ mod tests { output, SubgraphPublishResponse { schema_hash: None, - composition_errors: CompositionErrors { - composition_errors: vec![CompositionError { - message: "[Accounts] -> Things went really wrong".to_string(), - code: None - }] - }, + composition_errors: vec![CompositionError { + message: "[Accounts] -> Things went really wrong".to_string(), + code: None + }] + .into(), supergraph_was_updated: false, subgraph_was_created: false, } diff --git a/crates/rover-client/src/operations/supergraph/fetch/runner.rs b/crates/rover-client/src/operations/supergraph/fetch/runner.rs index 4e17f8cd4d..8be9e25568 100644 --- a/crates/rover-client/src/operations/supergraph/fetch/runner.rs +++ b/crates/rover-client/src/operations/supergraph/fetch/runner.rs @@ -67,7 +67,7 @@ fn get_supergraph_sdl_from_response_data( } else if let Some(most_recent_composition_publish) = service_data.most_recent_composition_publish { - let composition_errors = most_recent_composition_publish + let composition_errors: CompositionErrors = most_recent_composition_publish .errors .into_iter() .map(|error| CompositionError { @@ -77,7 +77,7 @@ fn get_supergraph_sdl_from_response_data( .collect(); Err(RoverClientError::NoCompositionPublishes { graph_ref, - source: CompositionErrors { composition_errors }, + source: composition_errors, }) } else { let mut valid_variants = Vec::new(); @@ -190,7 +190,7 @@ mod tests { let output = get_supergraph_sdl_from_response_data(data, graph_ref.clone()); let expected_error = RoverClientError::NoCompositionPublishes { graph_ref, - source: CompositionErrors { composition_errors }, + source: composition_errors.into(), } .to_string(); let actual_error = output.unwrap_err().to_string(); diff --git a/crates/rover-client/src/shared/composition_error.rs b/crates/rover-client/src/shared/composition_error.rs index 9444051e08..932ae5e87d 100644 --- a/crates/rover-client/src/shared/composition_error.rs +++ b/crates/rover-client/src/shared/composition_error.rs @@ -1,6 +1,7 @@ use std::{ error::Error, fmt::{self, Display}, + iter::FromIterator, }; use serde::Serialize; @@ -22,13 +23,19 @@ impl Display for CompositionError { } } -#[derive(Debug, Serialize, Clone, PartialEq)] +#[derive(Debug, Default, Serialize, Clone, PartialEq)] pub struct CompositionErrors { - pub composition_errors: Vec, + composition_errors: Vec, } impl CompositionErrors { - pub fn get_num_errors(&self) -> String { + pub fn new() -> Self { + CompositionErrors { + composition_errors: Vec::new(), + } + } + + pub fn len(&self) -> String { let num_failures = self.composition_errors.len(); if num_failures == 0 { unreachable!("No composition errors were encountered while composing the supergraph."); @@ -39,6 +46,22 @@ impl CompositionErrors { _ => format!("{} composition errors", num_failures), } } + + pub fn push(&mut self, error: CompositionError) { + self.composition_errors.push(error); + } + + pub fn is_empty(&self) -> bool { + self.composition_errors.is_empty() + } +} + +impl Iterator for CompositionErrors { + type Item = CompositionError; + + fn next(&mut self) -> Option { + self.composition_errors.clone().into_iter().next() + } } impl Display for CompositionErrors { @@ -50,5 +73,23 @@ impl Display for CompositionErrors { } } +impl From> for CompositionErrors { + fn from(composition_errors: Vec) -> Self { + CompositionErrors { composition_errors } + } +} + +impl FromIterator for CompositionErrors { + fn from_iter>(iter: I) -> Self { + let mut c = CompositionErrors::new(); + + for i in iter { + c.push(i); + } + + c + } +} + impl Error for CompositionError {} impl Error for CompositionErrors {} diff --git a/src/command/output.rs b/src/command/output.rs index a9615a2a1a..05cc7198bf 100644 --- a/src/command/output.rs +++ b/src/command/output.rs @@ -5,12 +5,13 @@ use crate::error::RoverError; use crate::utils::table::{self, cell, row}; use ansi_term::{ - Colour::{Red, Yellow}, + Colour::{Cyan, Red, Yellow}, Style, }; use atty::Stream; use crossterm::style::Attribute::Underlined; use rover_client::operations::graph::publish::GraphPublishResponse; +use rover_client::operations::subgraph::delete::SubgraphDeleteResponse; use rover_client::operations::subgraph::list::SubgraphListResponse; use rover_client::operations::subgraph::publish::SubgraphPublishResponse; use rover_client::shared::{ChangeSeverity, CheckResponse, FetchResponse, GraphRef, SdlType}; @@ -42,6 +43,12 @@ pub enum RoverOutput { subgraph: String, publish_response: SubgraphPublishResponse, }, + SubgraphDeleteResponse { + graph_ref: GraphRef, + subgraph: String, + dry_run: bool, + delete_response: SubgraphDeleteResponse, + }, Profiles(Vec), Introspection(String), ErrorExplanation(String), @@ -109,18 +116,64 @@ impl RoverOutput { ); } - if !publish_response - .composition_errors - .composition_errors - .is_empty() - { + if !publish_response.composition_errors.is_empty() { let warn_prefix = Red.normal().paint("WARN:"); eprintln!("{} The following composition errors occurred:", warn_prefix,); - for error in &publish_response.composition_errors.composition_errors { + for error in publish_response.composition_errors.clone() { eprintln!("{}", &error); } } } + RoverOutput::SubgraphDeleteResponse { + graph_ref, + subgraph, + dry_run, + delete_response, + } => { + let warn_prefix = Red.normal().paint("WARN:"); + if *dry_run { + if !delete_response.composition_errors.is_empty() { + eprintln!( + "{} Deleting the {} subgraph from {} would result in the following composition errors:", + warn_prefix, + Cyan.normal().paint(subgraph), + Cyan.normal().paint(graph_ref.to_string()), + ); + for error in delete_response.composition_errors.clone() { + eprintln!("{}", &error); + } + eprintln!("{} This is only a prediction. If the graph changes before confirming, these errors could change.", warn_prefix); + } else { + eprintln!("{} At the time of checking, there would be no composition errors resulting from the deletion of this subgraph.", warn_prefix); + eprintln!("{} This is only a prediction. If the graph changes before confirming, there could be composition errors.", warn_prefix) + } + } else { + if delete_response.supergraph_was_updated { + eprintln!( + "The {} subgraph was removed from {}. Remaining subgraphs were composed.", + Cyan.normal().paint(subgraph), + Cyan.normal().paint(graph_ref.to_string()), + ) + } else { + eprintln!( + "{} The gateway for {} was not updated. See errors below.", + warn_prefix, + Cyan.normal().paint(graph_ref.to_string()) + ) + } + + if !delete_response.composition_errors.is_empty() { + eprintln!( + "{} There were composition errors as a result of deleting the subgraph:", + warn_prefix, + ); + + for error in delete_response.composition_errors.clone() { + eprintln!("{}", &error); + } + } + } + } RoverOutput::CoreSchema(csdl) => { print_descriptor("CoreSchema"); print_content(&csdl); @@ -190,7 +243,7 @@ impl RoverOutput { pub(crate) fn get_internal_json(&self) -> Value { match self { RoverOutput::DocsList(shortlinks) => { - let mut shortlink_vec = vec![]; + let mut shortlink_vec = Vec::with_capacity(shortlinks.len()); for (shortlink_slug, shortlink_description) in shortlinks { shortlink_vec.push( json!({"slug": shortlink_slug, "description": shortlink_description }), @@ -209,14 +262,22 @@ impl RoverOutput { subgraph: _, publish_response, } => json!(publish_response), + RoverOutput::SubgraphDeleteResponse { + graph_ref: _, + subgraph: _, + dry_run: _, + delete_response, + } => { + json!(delete_response) + } RoverOutput::SubgraphList(list_response) => json!(list_response), RoverOutput::CheckResponse(check_response) => json!(check_response), RoverOutput::Profiles(profiles) => json!({ "profiles": profiles }), RoverOutput::Introspection(introspection_response) => { json!({ "introspection_response": introspection_response }) } - RoverOutput::ErrorExplanation(explanation) => { - json!({ "explanation": explanation }) + RoverOutput::ErrorExplanation(explanation_markdown) => { + json!({ "explanation_markdown": explanation_markdown }) } RoverOutput::EmptySuccess => json!(null), } @@ -287,6 +348,14 @@ impl From for JsonOutput { } = output { publish_response.supergraph_was_updated + } else if let RoverOutput::SubgraphDeleteResponse { + graph_ref: _, + subgraph: _, + dry_run: _, + delete_response, + } = output + { + delete_response.supergraph_was_updated } else { true } @@ -312,7 +381,10 @@ mod tests { use rover_client::{ operations::{ graph::publish::{ChangeSummary, FieldChanges, TypeChanges}, - subgraph::list::{SubgraphInfo, SubgraphUpdatedAt}, + subgraph::{ + delete::SubgraphDeleteResponse, + list::{SubgraphInfo, SubgraphUpdatedAt}, + }, }, shared::{CompositionError, CompositionErrors, SchemaChange, Sdl}, }; @@ -438,24 +510,97 @@ mod tests { assert_eq!(expected_json.to_string(), actual_json.to_string()); } + #[test] + fn subgraph_delete_success_json() { + let mock_subgraph_delete = SubgraphDeleteResponse { + supergraph_was_updated: true, + composition_errors: CompositionErrors::new(), + }; + let actual_json: JsonOutput = RoverOutput::SubgraphDeleteResponse { + delete_response: mock_subgraph_delete, + subgraph: "subgraph".to_string(), + dry_run: false, + graph_ref: GraphRef { + name: "name".to_string(), + variant: "current".to_string(), + }, + } + .into(); + let expected_json = json!({ + "data": { + "composition_errors": [], + "success": true, + }, + "error": null + }); + assert_eq!(expected_json.to_string(), actual_json.to_string()); + } + + #[test] + fn subgraph_delete_failure_json() { + let mock_subgraph_delete = SubgraphDeleteResponse { + supergraph_was_updated: false, + composition_errors: vec![ + CompositionError { + message: "[Accounts] -> Things went really wrong".to_string(), + code: Some("AN_ERROR_CODE".to_string()), + }, + CompositionError { + message: "[Films] -> Something else also went wrong".to_string(), + code: None, + }, + ] + .into(), + }; + let actual_json: JsonOutput = RoverOutput::SubgraphDeleteResponse { + delete_response: mock_subgraph_delete, + subgraph: "subgraph".to_string(), + dry_run: true, + graph_ref: GraphRef { + name: "name".to_string(), + variant: "current".to_string(), + }, + } + .into(); + let expected_json = json!({ + "data": { + "composition_errors": [ + { + "message": "[Accounts] -> Things went really wrong", + "code": "AN_ERROR_CODE" + }, + { + "message": "[Films] -> Something else also went wrong", + "code": null + } + ], + "success": false, + }, + "error": null + }); + assert_eq!(expected_json.to_string(), actual_json.to_string()); + } + #[test] fn check_success_response_json() { let mock_check_response = CheckResponse { - target_url: Some("https://studio.apollographql.com/graph/my-graph/composition/big-hash?variant=current".to_string()), - operation_check_count: 10, - changes: vec![SchemaChange { - code: "SOMETHING_HAPPENED".to_string(), - description: "beeg yoshi".to_string(), - severity: ChangeSeverity::PASS, - }, - SchemaChange { - code: "WOW".to_string(), - description: "that was so cool".to_string(), - severity: ChangeSeverity::PASS, - }], - result: ChangeSeverity::PASS, - failure_count: 0, - }; + target_url: Some("https://studio.apollographql.com/graph/my-graph/composition/big-hash?variant=current".to_string()), + operation_check_count: 10, + changes: vec![ + SchemaChange { + code: "SOMETHING_HAPPENED".to_string(), + description: "beeg yoshi".to_string(), + severity: ChangeSeverity::PASS, + }, + SchemaChange { + code: "WOW".to_string(), + description: "that was so cool".to_string(), + severity: ChangeSeverity::PASS, + } + ], + result: ChangeSeverity::PASS, + failure_count: 0, + }; let actual_json: JsonOutput = RoverOutput::CheckResponse(mock_check_response).into(); let expected_json = json!( { @@ -485,21 +630,23 @@ mod tests { #[test] fn check_failure_response_json() { let mock_check_response = CheckResponse { - target_url: Some("https://studio.apollographql.com/graph/my-graph/composition/big-hash?variant=current".to_string()), - operation_check_count: 10, - changes: vec![SchemaChange { - code: "SOMETHING_HAPPENED".to_string(), - description: "beeg yoshi".to_string(), - severity: ChangeSeverity::FAIL, - }, - SchemaChange { - code: "WOW".to_string(), - description: "that was so cool".to_string(), - severity: ChangeSeverity::FAIL, - }], - result: ChangeSeverity::FAIL, - failure_count: 2, - }; + target_url: Some("https://studio.apollographql.com/graph/my-graph/composition/big-hash?variant=current".to_string()), + operation_check_count: 10, + changes: vec![ + SchemaChange { + code: "SOMETHING_HAPPENED".to_string(), + description: "beeg yoshi".to_string(), + severity: ChangeSeverity::FAIL, + }, + SchemaChange { + code: "WOW".to_string(), + description: "that was so cool".to_string(), + severity: ChangeSeverity::FAIL, + } + ], + result: ChangeSeverity::FAIL, + failure_count: 2, + }; let actual_json: JsonOutput = RoverOutput::CheckResponse(mock_check_response).into(); let expected_json = json!({ "data": { @@ -576,9 +723,7 @@ mod tests { let mock_publish_response = SubgraphPublishResponse { schema_hash: Some("123456".to_string()), - composition_errors: CompositionErrors { - composition_errors: vec![], - }, + composition_errors: CompositionErrors::new(), supergraph_was_updated: true, subgraph_was_created: true, }; @@ -609,18 +754,17 @@ mod tests { let mock_publish_response = SubgraphPublishResponse { schema_hash: None, - composition_errors: CompositionErrors { - composition_errors: vec![ - CompositionError { - message: "[Accounts] -> Things went really wrong".to_string(), - code: Some("AN_ERROR_CODE".to_string()), - }, - CompositionError { - message: "[Films] -> Something else also went wrong".to_string(), - code: None, - }, - ], - }, + composition_errors: vec![ + CompositionError { + message: "[Accounts] -> Things went really wrong".to_string(), + code: Some("AN_ERROR_CODE".to_string()), + }, + CompositionError { + message: "[Films] -> Something else also went wrong".to_string(), + code: None, + }, + ] + .into(), supergraph_was_updated: false, subgraph_was_created: false, }; @@ -634,23 +778,23 @@ mod tests { } .into(); let expected_json = json!({ - "data": { - "schema_hash": null, - "subgraph_was_created": false, - "composition_errors": [ - { - "message": "[Accounts] -> Things went really wrong", - "code": "AN_ERROR_CODE" + "data": { + "schema_hash": null, + "subgraph_was_created": false, + "composition_errors": [ + { + "message": "[Accounts] -> Things went really wrong", + "code": "AN_ERROR_CODE" + }, + { + "message": "[Films] -> Something else also went wrong", + "code": null + } + ], + "success": false }, - { - "message": "[Films] -> Something else also went wrong", - "code": null - } - ], - "success": false - }, - "error": null - }); + "error": null + }); assert_eq!(expected_json.to_string(), actual_json.to_string()); } diff --git a/src/command/subgraph/delete.rs b/src/command/subgraph/delete.rs index dcd0fc49b0..27e447cc94 100644 --- a/src/command/subgraph/delete.rs +++ b/src/command/subgraph/delete.rs @@ -1,4 +1,4 @@ -use ansi_term::Colour::{Cyan, Red, Yellow}; +use ansi_term::Colour::{Cyan, Yellow}; use serde::Serialize; use structopt::StructOpt; @@ -6,9 +6,7 @@ use crate::command::RoverOutput; use crate::utils::client::StudioClientConfig; use crate::Result; -use rover_client::operations::subgraph::delete::{ - self, SubgraphDeleteInput, SubgraphDeleteResponse, -}; +use rover_client::operations::subgraph::delete::{self, SubgraphDeleteInput}; use rover_client::shared::GraphRef; #[derive(Debug, Serialize, StructOpt)] @@ -39,28 +37,34 @@ pub struct Delete { impl Delete { pub fn run(&self, client_config: StudioClientConfig) -> Result { let client = client_config.get_authenticated_client(&self.profile_name)?; - let graph_ref = self.graph.to_string(); eprintln!( "Checking for composition errors resulting from deleting subgraph {} from {} using credentials from the {} profile.", Cyan.normal().paint(&self.subgraph), - Cyan.normal().paint(&graph_ref), + Cyan.normal().paint(self.graph.to_string()), Yellow.normal().paint(&self.profile_name) ); // this is probably the normal path -- preview a subgraph delete // and make the user confirm it manually. if !self.confirm { + let dry_run = true; // run delete with dryRun, so we can preview composition errors let delete_dry_run_response = delete::run( SubgraphDeleteInput { graph_ref: self.graph.clone(), subgraph: self.subgraph.clone(), - dry_run: true, + dry_run, }, &client, )?; - handle_dry_run_response(delete_dry_run_response, &self.subgraph, &graph_ref); + RoverOutput::SubgraphDeleteResponse { + graph_ref: self.graph.clone(), + subgraph: self.subgraph.clone(), + dry_run, + delete_response: delete_dry_run_response, + } + .print(); // I chose not to error here, since this is a perfectly valid path if !confirm_delete()? { @@ -69,36 +73,23 @@ impl Delete { } } + let dry_run = false; + let delete_response = delete::run( SubgraphDeleteInput { graph_ref: self.graph.clone(), subgraph: self.subgraph.clone(), - dry_run: false, + dry_run, }, &client, )?; - handle_response(delete_response, &self.subgraph, &graph_ref); - Ok(RoverOutput::EmptySuccess) - } -} - -fn handle_dry_run_response(response: SubgraphDeleteResponse, subgraph: &str, graph_ref: &str) { - let warn_prefix = Red.normal().paint("WARN:"); - if let Some(composition_errors) = response.composition_errors { - eprintln!( - "{} Deleting the {} subgraph from {} would result in the following composition errors:", - warn_prefix, - Cyan.normal().paint(subgraph), - Cyan.normal().paint(graph_ref), - ); - for error in composition_errors.composition_errors { - eprintln!("{}", &error); - } - eprintln!("{} This is only a prediction. If the graph changes before confirming, these errors could change.", warn_prefix); - } else { - eprintln!("{} At the time of checking, there would be no composition errors resulting from the deletion of this subgraph.", warn_prefix); - eprintln!("{} This is only a prediction. If the graph changes before confirming, there could be composition errors.", warn_prefix) + Ok(RoverOutput::SubgraphDeleteResponse { + graph_ref: self.graph.clone(), + subgraph: self.subgraph.clone(), + dry_run, + delete_response, + }) } } @@ -112,71 +103,3 @@ fn confirm_delete() -> Result { Ok(false) } } - -fn handle_response(response: SubgraphDeleteResponse, subgraph: &str, graph_ref: &str) { - let warn_prefix = Red.normal().paint("WARN:"); - if response.updated_gateway { - eprintln!( - "The {} subgraph was removed from {}. Remaining subgraphs were composed.", - Cyan.normal().paint(subgraph), - Cyan.normal().paint(graph_ref), - ) - } else { - eprintln!( - "{} The gateway for {} was not updated. See errors below.", - warn_prefix, - Cyan.normal().paint(graph_ref) - ) - } - - if let Some(composition_errors) = response.composition_errors { - eprintln!( - "{} There were composition errors as a result of deleting the subgraph:", - warn_prefix, - ); - - for error in composition_errors.composition_errors { - eprintln!("{}", &error); - } - } -} - -#[cfg(test)] -mod tests { - use super::{handle_response, SubgraphDeleteResponse}; - use rover_client::shared::{CompositionError, CompositionErrors}; - - #[test] - fn handle_response_doesnt_error_with_all_successes() { - let response = SubgraphDeleteResponse { - composition_errors: None, - updated_gateway: true, - }; - - handle_response(response, "accounts", "my-graph@current"); - } - - #[test] - fn handle_response_doesnt_error_with_all_failures() { - let response = SubgraphDeleteResponse { - composition_errors: Some(CompositionErrors { - composition_errors: vec![ - CompositionError { - message: "a bad thing happened".to_string(), - code: None, - }, - CompositionError { - message: "another bad thing".to_string(), - code: None, - }, - ], - }), - updated_gateway: false, - }; - - handle_response(response, "accounts", "my-graph@prod"); - } - - // TODO: test the actual output of the logs whenever we do design work - // for the commands :) -} diff --git a/src/command/supergraph/compose/do_compose.rs b/src/command/supergraph/compose/do_compose.rs index 5a85fea267..1a3e428a7d 100644 --- a/src/command/supergraph/compose/do_compose.rs +++ b/src/command/supergraph/compose/do_compose.rs @@ -5,7 +5,7 @@ use crate::{anyhow, command::RoverOutput, error::RoverError, Result, Suggestion} use rover_client::blocking::GraphQLClient; use rover_client::operations::subgraph::fetch::{self, SubgraphFetchInput}; use rover_client::operations::subgraph::introspect::{self, SubgraphIntrospectInput}; -use rover_client::shared::{CompositionError, CompositionErrors, GraphRef}; +use rover_client::shared::{CompositionError, GraphRef}; use rover_client::RoverClientError; use camino::Utf8PathBuf; @@ -41,20 +41,18 @@ impl Compose { match harmonizer::harmonize(subgraph_definitions) { Ok(core_schema) => Ok(RoverOutput::CoreSchema(core_schema)), Err(harmonizer_composition_errors) => { - let mut client_composition_errors = + let mut composition_errors = Vec::with_capacity(harmonizer_composition_errors.len()); for harmonizer_composition_error in harmonizer_composition_errors { if let Some(message) = &harmonizer_composition_error.message { - client_composition_errors.push(CompositionError { + composition_errors.push(CompositionError { message: message.to_string(), code: Some(harmonizer_composition_error.code().to_string()), }); } } Err(RoverError::new(RoverClientError::CompositionErrors { - source: CompositionErrors { - composition_errors: client_composition_errors, - }, + source: composition_errors.into(), })) } } diff --git a/src/error/metadata/mod.rs b/src/error/metadata/mod.rs index 05bd92126a..227ba60b3c 100644 --- a/src/error/metadata/mod.rs +++ b/src/error/metadata/mod.rs @@ -73,7 +73,7 @@ impl From<&mut anyhow::Error> for Metadata { }), Some(Code::E029), ), - RoverClientError::CompositionErrors { source: _ } => { + RoverClientError::CompositionErrors { .. } => { (Some(Suggestion::FixCompositionErrors), Some(Code::E029)) } RoverClientError::OperationCheckFailure { diff --git a/src/error/mod.rs b/src/error/mod.rs index bfc11e7ff9..f3916e4e0e 100644 --- a/src/error/mod.rs +++ b/src/error/mod.rs @@ -44,7 +44,7 @@ where { let mut data = serializer.serialize_struct(struct_name, 2)?; data.serialize_field(message_field_name, &error.to_string())?; - data.serialize_field("composition_errors", &composition_errors.composition_errors)?; + data.serialize_field("composition_errors", composition_errors)?; return data.end(); } }