wip: refactor subgraph check and add line numbers

Author: EverlastingBugstopper

crates/rover-client/src/error.rs

@@ -1,6 +1,8 @@
 use reqwest::Url;
 use thiserror::Error;
 
+use crate::query::subgraph::check::types::CompositionError;
+
 /// RoverClientError represents all possible failures that can occur during a client request.
 #[derive(Error, Debug)]
 pub enum RoverClientError {
@@ -103,6 +105,12 @@ pub enum RoverClientError {
         graph: String,
         composition_errors: Vec<String>,
     },
+    
+    #[error("{}", subgraph_composition_error_msg(.composition_errors))]
+    SubgraphCompositionErrors {
+        graph_name: String,
+        composition_errors: Vec<CompositionError>
+    },
 
     /// This error occurs when the Studio API returns no implementing services for a graph
     /// This response shouldn't be possible!
@@ -141,3 +149,16 @@ pub enum RoverClientError {
     #[error("This endpoint doesn't support subgraph introspection via the Query._service field")]
     SubgraphIntrospectionNotAvailable,
 }
+
+fn subgraph_composition_error_msg(composition_errors: &[CompositionError]) -> String {
+    let num_failures = composition_errors.len();
+    if num_failures == 0 {
+        unreachable!("No composition errors were encountered while composing the supergraph.");
+    }
+    let mut msg = String::new();
+    msg.push_str(&match num_failures {
+        1 => "Encountered 1 composition error while composing the supergraph.".to_string(),
+        _ => format!("Encountered {} composition errors while composing the supergraph.", num_failures)
+    });
+    msg
+}

crates/rover-client/src/query/subgraph/check.graphql crates/rover-client/src/query/subgraph/check/check_query.graphql

@@ -1,4 +1,4 @@
-  mutation CheckPartialSchemaQuery(
+  mutation SubgraphCheckQuery(
     $graph_id: ID!
     $variant: String!
     $implementingServiceName: String!
@@ -17,6 +17,11 @@
         compositionValidationResult {
           errors {
             message
+            code
+            locations {
+              line
+              column
+            }
           }
         }
         checkSchemaResult {

crates/rover-client/src/query/subgraph/check/mod.rs

@@ -0,0 +1,3 @@
+pub mod query_runner;
+pub(crate) mod types;
+pub use types::SubgraphCheckResponse;

crates/rover-client/src/query/subgraph/check.rs crates/rover-client/src/query/subgraph/check/query_runner.rs

@@ -1,33 +1,31 @@
+use graphql_client::*;
 use crate::blocking::StudioClient;
 use crate::query::config::is_federated;
 use crate::RoverClientError;
+use super::types::*;
 
-use graphql_client::*;
-
-use reqwest::Url;
-
-type Timestamp = String;
 #[derive(GraphQLQuery)]
 // The paths are relative to the directory where your `Cargo.toml` is located.
 // Both json and the GraphQL schema language are supported as sources for the schema
 #[graphql(
-    query_path = "src/query/subgraph/check.graphql",
-    schema_path = ".schema/schema.graphql",
-    response_derives = "PartialEq, Debug, Serialize, Deserialize",
-    deprecated = "warn"
+  query_path = "src/query/subgraph/check/check_query.graphql",
+  schema_path = ".schema/schema.graphql",
+  response_derives = "PartialEq, Debug, Serialize, Deserialize",
+  deprecated = "warn"
 )]
 /// This struct is used to generate the module containing `Variables` and
 /// `ResponseData` structs.
-/// Snake case of this name is the mod name. i.e. check_partial_schema_query
-pub struct CheckPartialSchemaQuery;
+/// Snake case of this name is the mod name. i.e. subgraph_check_query
+pub struct SubgraphCheckQuery;
+
 
 /// The main function to be used from this module.
 /// This function takes a proposed schema and validates it against a published
 /// schema.
 pub fn run(
-    variables: check_partial_schema_query::Variables,
+    variables: subgraph_check_query::Variables,
     client: &StudioClient,
-) -> Result<CheckResponse, RoverClientError> {
+) -> Result<SubgraphCheckResponse, RoverClientError> {
     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(
@@ -43,77 +41,67 @@ pub fn run(
             can_operation_convert: false,
         });
     }
-    let data = client.post::<CheckPartialSchemaQuery>(variables)?;
+    let data = client.post::<SubgraphCheckQuery>(variables)?;
     get_check_response_from_data(data, graph)
 }
 
-pub enum CheckResponse {
-    CompositionErrors(Vec<check_partial_schema_query::CheckPartialSchemaQueryServiceCheckPartialSchemaCompositionValidationResultErrors>),
-    CheckResult(CheckResult)
-}
-
-#[derive(Debug)]
-pub struct CheckResult {
-    pub target_url: Option<Url>,
-    pub number_of_checked_operations: i64,
-    pub change_severity: check_partial_schema_query::ChangeSeverity,
-    pub changes: Vec<check_partial_schema_query::CheckPartialSchemaQueryServiceCheckPartialSchemaCheckSchemaResultDiffToPreviousChanges>,
-}
-
 fn get_check_response_from_data(
-    data: check_partial_schema_query::ResponseData,
-    graph: String,
-) -> Result<CheckResponse, RoverClientError> {
-    let service = data.service.ok_or(RoverClientError::NoService { graph })?;
+    data: subgraph_check_query::ResponseData,
+    graph_name: String,
+) -> Result<SubgraphCheckResponse, RoverClientError> {
+    let service = data.service.ok_or(RoverClientError::NoService { graph: graph_name.clone() })?;
 
     // for some reason this is a `Vec<Option<CompositionError>>`
     // we convert this to just `Vec<CompositionError>` because the `None`
     // errors would be useless.
-    let composition_errors: Vec<check_partial_schema_query::CheckPartialSchemaQueryServiceCheckPartialSchemaCompositionValidationResultErrors> = service
+    let query_composition_errors: Vec<subgraph_check_query::SubgraphCheckQueryServiceCheckPartialSchemaCompositionValidationResultErrors> = service
         .check_partial_schema
         .composition_validation_result
         .errors;
 
-    if composition_errors.is_empty() {
+    if query_composition_errors.is_empty() {
         let check_schema_result = service.check_partial_schema.check_schema_result.ok_or(
             RoverClientError::MalformedResponse {
                 null_field: "service.check_partial_schema.check_schema_result".to_string(),
             },
         )?;
 
-        let target_url = get_url(check_schema_result.target_url);
+        let target_url = check_schema_result.target_url.map(|u| u.to_string());
 
         let diff_to_previous = check_schema_result.diff_to_previous;
 
         let number_of_checked_operations =
             diff_to_previous.number_of_checked_operations.unwrap_or(0);
 
-        let change_severity = diff_to_previous.severity;
-        let changes = diff_to_previous.changes;
+        let change_severity = diff_to_previous.severity.into();
+
+        let mut changes = Vec::with_capacity(diff_to_previous.changes.len());
+        for change in diff_to_previous.changes {
+            changes.push(SchemaChange {
+                code: change.code,
+                severity: change.severity.into(),
+                description: change.description
+            });
+        }
 
-        let check_result = CheckResult {
+        let check_result = SubgraphCheckResponse {
             target_url,
             number_of_checked_operations,
-            change_severity,
             changes,
+            change_severity
         };
 
-        Ok(CheckResponse::CheckResult(check_result))
+        Ok(check_result)
     } else {
-        Ok(CheckResponse::CompositionErrors(composition_errors))
-    }
-}
-
-fn get_url(url: Option<String>) -> Option<Url> {
-    match url {
-        Some(url) => {
-            let url = Url::parse(&url);
-            match url {
-                Ok(url) => Some(url),
-                // if the API returns an invalid URL, don't put it in the response
-                Err(_) => None,
-            }
+        let num_failures = query_composition_errors.len();
+
+        let mut composition_errors = Vec::with_capacity(num_failures);
+        for query_composition_error in query_composition_errors {
+            composition_errors.push(CompositionError {
+                message: query_composition_error.message,
+                code: query_composition_error.code
+            });
         }
-        None => None,
+        Err(RoverClientError::SubgraphCompositionErrors { graph_name, composition_errors })
     }
 }

crates/rover-client/src/query/subgraph/check/types.rs

@@ -0,0 +1,53 @@
+use std::fmt;
+
+use super::query_runner::subgraph_check_query;
+
+pub(crate) type Timestamp = String;
+type QueryChangeSeverity = subgraph_check_query::ChangeSeverity;
+
+#[derive(Debug, Clone, PartialEq)]
+pub struct SubgraphCheckResponse {
+    pub target_url: Option<String>,
+    pub number_of_checked_operations: i64,
+    pub changes: Vec<SchemaChange>,
+    pub change_severity: ChangeSeverity
+}
+
+#[derive(Debug, Clone, PartialEq)]
+pub enum ChangeSeverity {
+  PASS,
+  FAIL,
+}
+
+impl fmt::Display for ChangeSeverity {
+  fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+    let msg = match self {
+      ChangeSeverity::PASS => "PASS",
+      ChangeSeverity::FAIL => "FAIL"
+    };
+    write!(f, "{}", msg)
+  }
+}
+
+impl From<QueryChangeSeverity> for ChangeSeverity {
+  fn from(severity: QueryChangeSeverity) -> Self {
+    match severity {
+      QueryChangeSeverity::NOTICE => ChangeSeverity::PASS,
+      QueryChangeSeverity::FAILURE => ChangeSeverity::FAIL,
+      _ => unreachable!("Unknown change severity")
+    }
+  }
+}
+
+#[derive(Debug, Clone, PartialEq)]
+pub struct SchemaChange {
+  pub code: String,
+  pub description: String,
+  pub severity: ChangeSeverity
+}
+
+#[derive(Debug, Clone, PartialEq)]
+pub struct CompositionError {
+  pub message: String,
+  pub code: Option<String>
+}

docs/source/errors.md

@@ -233,4 +233,12 @@ This error occurs when Rover could not connect to an HTTP endpoint.
 
 If you encountered this error while running introspection, you'll want to make sure that you typed the endpoint correctly, your Internet connection is stable, and that your server is responding to requests. You may wish to run the command again with `--log=debug`.
 
+### E029
+
+This error occurs when you propose a subgraph schema that could not be composed.
+
+There are many reasons why you may run into composition errors. This error should include information about _why_ the proposed subgraph schema could not be composed. Error code references can be found [here](https://www.apollographql.com/docs/federation/errors/).
+
+Some composition errors are part of normal workflows. For instance, you may need to publish a subgraph that does not compose if you are trying to [migrate an entity or field](https://www.apollographql.com/docs/federation/entities/#migrating-entities-and-fields-advanced).
+
 

src/command/output.rs

@@ -5,7 +5,7 @@ use crate::utils::table::{self, cell, row};
 use ansi_term::{Colour::Yellow, Style};
 use atty::Stream;
 use crossterm::style::Attribute::Underlined;
-use rover_client::query::subgraph::list::ListDetails;
+use rover_client::query::subgraph::{check::SubgraphCheckResponse, list::ListDetails};
 use termimad::MadSkin;
 
 /// RoverStdout defines all of the different types of data that are printed
@@ -24,6 +24,7 @@ pub enum RoverStdout {
     CoreSchema(String),
     SchemaHash(String),
     SubgraphList(ListDetails),
+    SubgraphCheck(SubgraphCheckResponse),
     VariantList(Vec<String>),
     Profiles(Vec<String>),
     Introspection(String),
@@ -97,6 +98,36 @@ impl RoverStdout {
                     details.root_url, details.graph_name
                 );
             }
+            RoverStdout::SubgraphCheck(check_response) => {
+                let num_changes = check_response.changes.len();
+
+                let msg = match num_changes {
+                    0 => "There were no changes detected in the composed schema.".to_string(),
+                    _ => format!(
+                        "Compared {} schema changes against {} operations",
+                        check_response.changes.len(),
+                        check_response.number_of_checked_operations
+                    ),
+                };
+
+                eprintln!("{}", &msg);
+
+                if !check_response.changes.is_empty() {
+                    let mut table = table::get_table();
+
+                    // bc => sets top row to be bold and center
+                    table.add_row(row![bc => "Change", "Code", "Description"]);
+                    for check in &check_response.changes {
+                        table.add_row(row![check.severity, check.code, check.description]);
+                    }
+
+                    print_content(table.to_string());
+                }
+
+                if let Some(url) = &check_response.target_url {
+                    eprintln!("View full details at {}", url);
+                }
+            }
             RoverStdout::VariantList(variants) => {
                 print_descriptor("Variants");
                 for variant in variants {