Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rover): rover subgraph check #110

Merged
merged 1 commit into from
Jan 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions crates/rover-client/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ pub enum RoverClientError {
#[error("No graph found. Either the graph@variant combination wasn't found or your API key is invalid.")]
NoService,

/// This error occurs when the Studio API returns no composition errors AND
/// no check result. This response shouldn't be possible!
#[error(
"The response from the server was malformed, there was no data from the check operation."
)]
NoCheckData,

#[error("The graph `{graph_name}` is a non-federated graph. This operation is only possible for federated graphs")]
ExpectedFederatedGraph { graph_name: String },

Expand Down
32 changes: 32 additions & 0 deletions crates/rover-client/src/query/subgraph/check.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
mutation CheckPartialSchemaQuery (
$graph_id: ID!
$variant: String!
$implementingServiceName: String!
$partialSchema: PartialSchemaInput!
) {
service(id: $graph_id) {
checkPartialSchema(
graphVariant: $variant
implementingServiceName: $implementingServiceName
partialSchema: $partialSchema
) {
compositionValidationResult {
errors {
message
}
}
checkSchemaResult {
diffToPrevious {
severity
numberOfCheckedOperations
changes {
severity
code
description
}
}
targetUrl
}
}
}
}
104 changes: 104 additions & 0 deletions crates/rover-client/src/query/subgraph/check.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
use crate::blocking::StudioClient;
use crate::RoverClientError;
use graphql_client::*;

use reqwest::Url;

#[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"
)]
/// 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;

/// The main function to be used from this module.
/// This function takes a proposed schema and validates it against a pushed
/// schema.
pub fn run(
variables: check_partial_schema_query::Variables,
client: &StudioClient,
) -> Result<CheckResponse, RoverClientError> {
let data = client.post::<CheckPartialSchemaQuery>(variables)?;
get_check_response_from_data(data)
}

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,
) -> Result<CheckResponse, RoverClientError> {
let service = data.service.ok_or(RoverClientError::NoService)?;

// 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
.check_partial_schema
.composition_validation_result
.errors
.into_iter()
.filter(|e| e.is_some())
.map(|e| e.unwrap())
.collect();

if composition_errors.is_empty() {
// TODO: fix this error case
let check_schema_result = service
.check_partial_schema
.check_schema_result
.ok_or(RoverClientError::NoCheckData)?;

let target_url = get_url(check_schema_result.target_url);

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 check_result = CheckResult {
target_url,
number_of_checked_operations,
change_severity,
changes,
};

Ok(CheckResponse::CheckResult(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,
}
}
None => None,
}
}
9 changes: 6 additions & 3 deletions crates/rover-client/src/query/subgraph/mod.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
/// "subgraph push" command execution
pub mod push;

/// "subgraph delete" command execution
pub mod delete;

/// "subgraph check" command execution
pub mod check;

/// "subgraph fetch" command execution
pub mod fetch;

/// "subgraph push" command execution
pub mod push;
144 changes: 144 additions & 0 deletions src/command/subgraph/check.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
use anyhow::{Context, Result};
use prettytable::{cell, row, Table};
use serde::Serialize;
use structopt::StructOpt;

use rover_client::query::subgraph::check;

use crate::client::StudioClientConfig;
use crate::command::RoverStdout;
use crate::utils::loaders::load_schema_from_flag;
use crate::utils::parsers::{parse_graph_ref, parse_schema_source, GraphRef, SchemaSource};

#[derive(Debug, Serialize, StructOpt)]
pub struct Check {
/// <NAME>@<VARIANT> of graph in Apollo Studio to validate.
/// @<VARIANT> may be left off, defaulting to @current
#[structopt(name = "GRAPH_REF", parse(try_from_str = parse_graph_ref))]
#[serde(skip_serializing)]
graph: GraphRef,

/// Name of the implementing service to validate
#[structopt(required = true)]
#[serde(skip_serializing)]
service_name: String,

/// Name of configuration profile to use
#[structopt(long = "profile", default_value = "default")]
#[serde(skip_serializing)]
profile_name: String,

/// The schema file to push
/// Can pass `-` to use stdin instead of a file
#[structopt(long, short = "s", parse(try_from_str = parse_schema_source))]
#[serde(skip_serializing)]
schema: SchemaSource,
}

impl Check {
pub fn run(&self, client_config: StudioClientConfig) -> Result<RoverStdout> {
let client = client_config.get_client(&self.profile_name)?;

let sdl = load_schema_from_flag(&self.schema, std::io::stdin())?;

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
hash: None,
};
let res = check::run(
check::check_partial_schema_query::Variables {
graph_id: self.graph.name.clone(),
variant: self.graph.variant.clone(),
partial_schema,
implementing_service_name: self.service_name.clone(),
},
&client,
)
.context("Failed to validate schema")?;

tracing::info!(
"Checked the proposed subgraph against {}@{}",
&self.graph.name,
&self.graph.variant
);

match res {
check::CheckResponse::CompositionErrors(composition_errors) => {
handle_composition_errors(&composition_errors)
}
check::CheckResponse::CheckResult(check_result) => handle_checks(check_result),
}
}
}

fn handle_checks(check_result: check::CheckResult) -> Result<RoverStdout> {
let num_changes = check_result.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_result.changes.len(),
check_result.number_of_checked_operations
),
};

tracing::info!("{}", &msg);

let mut num_failures = 0;

if !check_result.changes.is_empty() {
let mut table = Table::new();
table.add_row(row!["Change", "Code", "Description"]);
for check in check_result.changes {
let change = match check.severity {
check::check_partial_schema_query::ChangeSeverity::NOTICE => "PASS",
check::check_partial_schema_query::ChangeSeverity::FAILURE => {
num_failures += 1;
"FAIL"
}
_ => unreachable!("Unknown change severity"),
};
table.add_row(row![change, check.code, check.description]);
}

eprintln!("{}", table);
}

if let Some(url) = check_result.target_url {
tracing::info!("View full details here");
tracing::info!("{}", url.to_string());
}

match num_failures {
0 => Ok(RoverStdout::None),
1 => Err(anyhow::anyhow!(
"Encountered 1 failure while checking your subgraph."
)),
_ => Err(anyhow::anyhow!(
"Encountered {} failures while checking your subgraph.",
num_failures
)),
}
}

fn handle_composition_errors(
composition_errors: &[check::check_partial_schema_query::CheckPartialSchemaQueryServiceCheckPartialSchemaCompositionValidationResultErrors],
) -> Result<RoverStdout> {
let mut num_failures = 0;
for error in composition_errors {
num_failures += 1;
tracing::error!("{}", &error.message);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as a potential future improvement, we could make a table of these errors similar to checks. Composition errors should always be in the format [Service] Type.field? -> Message so maybe we could parse out into two columns like location and message. But this is fine for now :)

Copy link
Contributor Author

@EverlastingBugstopper EverlastingBugstopper Dec 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, the way it prints it is as so:

 ERROR [films] Person -> A @key directive specifies a field which is not found in this service. Add a field to this type with @external.
 ERROR [films] Person -> A @key selects iddd, but Person.iddd could not be found
 ERROR [films] Person -> extends from people but specifies an invalid @key directive. Valid @key directives are specified by the originating type. Available @key directives for this type are:
        @key(fields: "id")
Error: Encountered 3 composition errors while composing the subgraph

i think if we wanted to print it like a table we'd want to have that data returned in separate fields by the API rather than trying to parse out these strings.

}
match num_failures {
0 => Ok(RoverStdout::None),
1 => Err(anyhow::anyhow!(
"Encountered 1 composition error while composing the subgraph."
)),
_ => Err(anyhow::anyhow!(
"Encountered {} composition errors while composing the subgraph.",
num_failures
)),
}
}
9 changes: 7 additions & 2 deletions src/command/subgraph/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
mod check;
mod delete;
mod fetch;
mod push;
Expand All @@ -17,14 +18,17 @@ pub struct Subgraph {

#[derive(Debug, Serialize, StructOpt)]
pub enum Command {
/// Push an implementing service schema from a local file
Push(push::Push),
/// Check changes to an implementing service
Check(check::Check),

/// Delete an implementing service and trigger composition
Delete(delete::Delete),

/// Fetch an implementing service's schema from Apollo Studio
Fetch(fetch::Fetch),

/// Push an implementing service schema from a local file
Push(push::Push),
}

impl Subgraph {
Expand All @@ -33,6 +37,7 @@ impl Subgraph {
Command::Push(command) => command.run(client_config),
Command::Delete(command) => command.run(client_config),
Command::Fetch(command) => command.run(client_config),
Command::Check(command) => command.run(client_config),
}
}
}