chore: test structured output

crates/rover-client/src/operations/graph/publish/mod.rs

@@ -2,4 +2,6 @@ mod runner;
 mod types;
 
 pub use runner::run;
-pub use types::{GraphPublishInput, GraphPublishResponse};
+pub use types::{
+    ChangeSummary, FieldChanges, GraphPublishInput, GraphPublishResponse, TypeChanges,
+};

crates/rover-client/src/operations/graph/publish/runner.rs

@@ -1,4 +1,5 @@
 use crate::blocking::StudioClient;
+use crate::operations::graph::publish::types::{ChangeSummary, FieldChanges, TypeChanges};
 use crate::operations::graph::publish::{GraphPublishInput, GraphPublishResponse};
 use crate::shared::GraphRef;
 use crate::RoverClientError;
@@ -75,9 +76,20 @@ fn build_response(
     // which very well may have changes. For this, we'll just look at the code
     // first and handle the response as if there was `None` for the diff
     let change_summary = if publish_response.code == "NO_CHANGES" {
-        build_change_summary(None)
+        ChangeSummary::none()
     } else {
-        build_change_summary(publish_response.tag.unwrap().diff_to_previous)
+        let diff = publish_response
+            .tag
+            .ok_or_else(|| RoverClientError::MalformedResponse {
+                null_field: "service.upload_schema.tag".to_string(),
+            })?
+            .diff_to_previous;
+
+        if let Some(diff) = diff {
+            build_change_summary(diff)
+        } else {
+            ChangeSummary::none()
+        }
     };
 
     Ok(GraphPublishResponse {
@@ -86,25 +98,21 @@ fn build_response(
     })
 }
 
-type ChangeDiff = graph_publish_mutation::GraphPublishMutationServiceUploadSchemaTagDiffToPrevious;
+type QueryChangeDiff =
+    graph_publish_mutation::GraphPublishMutationServiceUploadSchemaTagDiffToPrevious;
 
-/// builds a string-representation of the diff between two schemas
-/// e.g. ` [Fields: +2 -1 △0, Types: +4 -0 △7]` or `[No Changes]`
-fn build_change_summary(diff: Option<ChangeDiff>) -> String {
-    match diff {
-        None => "[No Changes]".to_string(),
-        Some(diff) => {
-            let changes = diff.change_summary;
-            let fields = format!(
-                "Fields: +{} -{} △ {}",
-                changes.field.additions, changes.field.removals, changes.field.edits
-            );
-            let types = format!(
-                "Types: +{} -{} △ {}",
-                changes.type_.additions, changes.type_.removals, changes.type_.edits
-            );
-            format!("[{}, {}]", fields, types)
-        }
+fn build_change_summary(diff: QueryChangeDiff) -> ChangeSummary {
+    ChangeSummary {
+        field_changes: FieldChanges {
+            additions: diff.change_summary.field.additions as u64,
+            removals: diff.change_summary.field.removals as u64,
+            edits: diff.change_summary.field.edits as u64,
+        },
+        type_changes: TypeChanges {
+            additions: diff.change_summary.type_.additions as u64,
+            removals: diff.change_summary.type_.removals as u64,
+            edits: diff.change_summary.type_.edits as u64,
+        },
     }
 }
 
@@ -200,7 +208,7 @@ mod tests {
             output.unwrap(),
             GraphPublishResponse {
                 schema_hash: "123456".to_string(),
-                change_summary: "[No Changes]".to_string(),
+                change_summary: ChangeSummary::none(),
             }
         );
     }
@@ -251,14 +259,20 @@ mod tests {
                 }
             }
         });
-        let diff_to_previous: ChangeDiff = serde_json::from_value(json_diff).unwrap();
-        let output = build_change_summary(Some(diff_to_previous));
-        assert_eq!(output, "[Fields: +3 -1 △ 0, Types: +4 -0 △ 2]".to_string())
+        let diff_to_previous: QueryChangeDiff = serde_json::from_value(json_diff).unwrap();
+        let output = build_change_summary(diff_to_previous);
+        assert_eq!(
+            output.to_string(),
+            "[Fields: +3 -1 △ 0, Types: +4 -0 △ 2]".to_string()
+        )
     }
 
     #[test]
     fn build_change_summary_works_with_no_changes() {
-        assert_eq!(build_change_summary(None), "[No Changes]".to_string())
+        assert_eq!(
+            ChangeSummary::none().to_string(),
+            "[No Changes]".to_string()
+        )
     }
 
     fn mock_graph_ref() -> GraphRef {

crates/rover-client/src/operations/graph/publish/types.rs

@@ -3,6 +3,8 @@ use crate::shared::{GitContext, GraphRef};
 
 use serde::Serialize;
 
+use std::fmt;
+
 #[derive(Clone, Debug, PartialEq)]
 pub struct GraphPublishInput {
     pub graph_ref: GraphRef,
@@ -38,5 +40,91 @@ impl From<GitContext> for GraphPublishContextInput {
 #[derive(Clone, Serialize, Debug, PartialEq)]
 pub struct GraphPublishResponse {
     pub schema_hash: String,
-    pub change_summary: String,
+    #[serde(flatten)]
+    pub change_summary: ChangeSummary,
+}
+
+#[derive(Clone, Serialize, Debug, PartialEq)]
+pub struct ChangeSummary {
+    pub field_changes: FieldChanges,
+    pub type_changes: TypeChanges,
+}
+
+impl ChangeSummary {
+    pub(crate) fn none() -> ChangeSummary {
+        ChangeSummary {
+            field_changes: FieldChanges::none(),
+            type_changes: TypeChanges::none(),
+        }
+    }
+}
+
+impl fmt::Display for ChangeSummary {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        if self.field_changes.additions == 0
+            && self.field_changes.removals == 0
+            && self.field_changes.edits == 0
+            && self.type_changes.additions == 0
+            && self.type_changes.removals == 0
+            && self.type_changes.edits == 0
+        {
+            write!(f, "[No Changes]")
+        } else {
+            write!(f, "[{}, {}]", &self.field_changes, &self.type_changes)
+        }
+    }
+}
+
+#[derive(Clone, Serialize, Debug, PartialEq)]
+pub struct FieldChanges {
+    pub additions: u64,
+    pub removals: u64,
+    pub edits: u64,
+}
+
+impl FieldChanges {
+    pub(crate) fn none() -> FieldChanges {
+        FieldChanges {
+            additions: 0,
+            removals: 0,
+            edits: 0,
+        }
+    }
+}
+
+impl fmt::Display for FieldChanges {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(
+            f,
+            "Fields: +{} -{} △ {}",
+            &self.additions, &self.removals, &self.edits
+        )
+    }
+}
+
+#[derive(Clone, Serialize, Debug, PartialEq)]
+pub struct TypeChanges {
+    pub additions: u64,
+    pub removals: u64,
+    pub edits: u64,
+}
+
+impl TypeChanges {
+    pub(crate) fn none() -> TypeChanges {
+        TypeChanges {
+            additions: 0,
+            removals: 0,
+            edits: 0,
+        }
+    }
+}
+
+impl fmt::Display for TypeChanges {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(
+            f,
+            "Types: +{} -{} △ {}",
+            &self.additions, &self.removals, &self.edits
+        )
+    }
 }

crates/rover-client/src/operations/subgraph/list/types.rs

@@ -26,7 +26,11 @@ impl From<SubgraphListInput> for QueryVariables {
 #[derive(Clone, Serialize, PartialEq, Debug)]
 pub struct SubgraphListResponse {
     pub subgraphs: Vec<SubgraphInfo>,
+
+    #[serde(skip_serializing)]
     pub root_url: String,
+
+    #[serde(skip_serializing)]
     pub graph_ref: GraphRef,
 }
 

crates/rover-client/src/operations/subgraph/publish/types.rs

@@ -24,8 +24,14 @@ pub struct SubgraphPublishInput {
 #[derive(Debug, Clone, Serialize, PartialEq)]
 pub struct SubgraphPublishResponse {
     pub schema_hash: Option<String>,
+
+    // we skip serializing this field as it is merged with "success"
+    // at the top level
+    #[serde(skip_serializing)]
     pub supergraph_was_updated: bool,
+
     pub subgraph_was_created: bool,
+
     #[serde(flatten)]
     pub composition_errors: CompositionErrors,
 }

crates/rover-client/src/shared/check_response.rs

@@ -17,6 +17,7 @@ pub struct CheckResponse {
     pub target_url: Option<String>,
     pub operation_check_count: u64,
     pub changes: Vec<SchemaChange>,
+    #[serde(skip_serializing)]
     pub result: ChangeSeverity,
     pub failure_count: u64,
 }

crates/rover-client/src/shared/fetch_response.rs

@@ -8,6 +8,7 @@ pub struct FetchResponse {
 #[derive(Debug, Clone, Serialize, PartialEq)]
 pub struct Sdl {
     pub contents: String,
+    #[serde(skip_serializing)]
     pub r#type: SdlType,
 }
 

src/cli.rs

@@ -115,15 +115,15 @@ impl Rover {
         match rover_output {
             Ok(output) => {
                 if self.json {
-                    println!("{}", JsonOutput::success(output));
+                    println!("{}", JsonOutput::from(output));
                 } else {
                     output.print();
                 }
                 process::exit(0);
             }
             Err(error) => {
                 if self.json {
-                    println!("{}", JsonOutput::error(error));
+                    println!("{}", JsonOutput::from(error));
                 } else {
                     tracing::debug!(?error);
                     eprint!("{}", error);

src/command/config/auth.rs

@@ -32,7 +32,7 @@ impl Auth {
         Profile::get_credential(&self.profile_name, &config).map(|_| {
             eprintln!("Successfully saved API key.");
         })?;
-        Ok(RoverOutput::None)
+        Ok(RoverOutput::EmptySuccess)
     }
 }
 

src/command/config/clear.rs

@@ -16,6 +16,6 @@ impl Clear {
     pub fn run(&self, config: config::Config) -> Result<RoverOutput> {
         config.clear()?;
         eprintln!("Successfully cleared all configuration.");
-        Ok(RoverOutput::None)
+        Ok(RoverOutput::EmptySuccess)
     }
 }

src/command/config/delete.rs

@@ -23,6 +23,6 @@ impl Delete {
     pub fn run(&self, config: config::Config) -> Result<RoverOutput> {
         config::Profile::delete(&self.name, &config)?;
         eprintln!("Successfully deleted profile \"{}\"", &self.name);
-        Ok(RoverOutput::None)
+        Ok(RoverOutput::EmptySuccess)
     }
 }

src/command/config/whoami.rs

@@ -80,6 +80,6 @@ impl WhoAmI {
 
         eprintln!("{}", message);
 
-        Ok(RoverOutput::None)
+        Ok(RoverOutput::EmptySuccess)
     }
 }

src/command/docs/open.rs

@@ -40,6 +40,6 @@ impl Open {
             Ok(())
         }?;
 
-        Ok(RoverOutput::None)
+        Ok(RoverOutput::EmptySuccess)
     }
 }

src/command/docs/shortlinks.rs

@@ -1,9 +1,9 @@
 pub const URL_BASE: &str = "https://go.apollo.dev/r";
 
-use std::collections::HashMap;
+use std::collections::BTreeMap;
 
-pub fn get_shortlinks_with_description() -> HashMap<&'static str, &'static str> {
-    let mut links = HashMap::new();
+pub fn get_shortlinks_with_description() -> BTreeMap<&'static str, &'static str> {
+    let mut links = BTreeMap::new();
     links.insert("docs", "Rover's Documentation Homepage");
     links.insert("api-keys", "Understanding Apollo's API Keys");
     links.insert("contributing", "Contributing to Rover");

src/command/explain.rs

@@ -14,6 +14,6 @@ pub struct Explain {
 impl Explain {
     pub fn run(&self) -> Result<RoverOutput> {
         let explanation = &self.code.explain();
-        Ok(RoverOutput::Markdown(explanation.clone()))
+        Ok(RoverOutput::ErrorExplanation(explanation.clone()))
     }
 }

src/command/info.rs

@@ -28,6 +28,6 @@ impl Info {
             PKG_VERSION, location, os, shell
         );
 
-        Ok(RoverOutput::None)
+        Ok(RoverOutput::EmptySuccess)
     }
 }

src/command/install/mod.rs

@@ -68,7 +68,7 @@ impl Install {
             } else {
                 eprintln!("{} was not installed. To override the existing installation, you can pass the `--force` flag to the installer.", &binary_name);
             }
-            Ok(RoverOutput::None)
+            Ok(RoverOutput::EmptySuccess)
         } else {
             Err(anyhow!("Failed to get the current executable's path.").into())
         }