chore: structure subgraph delete

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,

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(),
         })
     }
 }

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<CompositionError> = 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,
             }
         );
     }

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<CompositionErrors>,
+    #[serde(skip_serializing)]
+    pub supergraph_was_updated: bool,
+
+    #[serde(flatten)]
+    pub composition_errors: CompositionErrors,
 }
 
 impl From<SubgraphDeleteInput> for MutationVariables {

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<CompositionError> = publish_response
+    let composition_errors: CompositionErrors = publish_response
         .errors
         .iter()
         .filter_map(|error| {
@@ -71,23 +71,14 @@ 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),
             None => None,
         },
         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,
             }

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();

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<CompositionError>,
+    composition_errors: Vec<CompositionError>,
 }
 
 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::Item> {
+        self.composition_errors.clone().into_iter().next()
+    }
 }
 
 impl Display for CompositionErrors {
@@ -50,5 +73,23 @@ impl Display for CompositionErrors {
     }
 }
 
+impl From<Vec<CompositionError>> for CompositionErrors {
+    fn from(composition_errors: Vec<CompositionError>) -> Self {
+        CompositionErrors { composition_errors }
+    }
+}
+
+impl FromIterator<CompositionError> for CompositionErrors {
+    fn from_iter<I: IntoIterator<Item = CompositionError>>(iter: I) -> Self {
+        let mut c = CompositionErrors::new();
+
+        for i in iter {
+            c.push(i);
+        }
+
+        c
+    }
+}
+
 impl Error for CompositionError {}
 impl Error for CompositionErrors {}