From 6ac35e1742b6ef9aa4dc4e9da43010241b6e74a7 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Wed, 7 Jul 2021 13:56:24 -0500 Subject: [PATCH] chore: move GraphRef to rover-client (#664) --- Cargo.lock | 1 + Cargo.toml | 1 - crates/rover-client/Cargo.toml | 1 + crates/rover-client/src/error.rs | 6 +- .../src/operations/config/who_am_i/mod.rs | 3 +- .../src/operations/graph/check/runner.rs | 2 +- .../src/operations/graph/check/types.rs | 9 +- .../src/operations/subgraph/check/mod.rs | 6 +- .../src/operations/subgraph/check/runner.rs | 8 +- .../src/operations/subgraph/check/types.rs | 15 +-- .../subgraph/delete/delete_mutation.graphql | 1 + .../src/operations/subgraph/delete/mod.rs | 3 +- .../src/operations/subgraph/delete/runner.rs | 37 ++++++- .../src/operations/subgraph/delete/types.rs | 14 ++- .../src/operations/subgraph/fetch/mod.rs | 6 +- .../src/operations/subgraph/fetch/runner.rs | 8 +- .../src/operations/subgraph/fetch/types.rs | 26 +---- .../src/operations/subgraph/list/mod.rs | 3 +- .../src/operations/subgraph/list/runner.rs | 2 +- .../src/operations/subgraph/list/types.rs | 9 +- .../src/operations/subgraph/publish/mod.rs | 6 +- .../subgraph/publish/publish_mutation.graphql | 1 + .../src/operations/subgraph/publish/runner.rs | 56 +++++++--- .../src/operations/subgraph/publish/types.rs | 13 +-- .../src/shared/composition_error.rs | 18 +++ crates/rover-client/src/shared/graph_ref.rs | 104 ++++++++++++++++++ crates/rover-client/src/shared/mod.rs | 4 + src/command/config/whoami.rs | 4 +- src/command/graph/check.rs | 11 +- src/command/graph/fetch.rs | 4 +- src/command/graph/publish.rs | 6 +- src/command/subgraph/check.rs | 15 ++- src/command/subgraph/delete.rs | 44 +++++--- src/command/subgraph/fetch.rs | 11 +- src/command/subgraph/list.rs | 7 +- src/command/subgraph/publish.rs | 37 ++++--- src/command/supergraph/compose/do_compose.rs | 18 +-- src/command/supergraph/config.rs | 2 +- src/command/supergraph/fetch.rs | 4 +- src/error/metadata/mod.rs | 13 +-- src/utils/parsers.rs | 93 +--------------- 41 files changed, 358 insertions(+), 274 deletions(-) create mode 100644 crates/rover-client/src/shared/composition_error.rs create mode 100644 crates/rover-client/src/shared/graph_ref.rs diff --git a/Cargo.lock b/Cargo.lock index f2efddf95..e8c389322 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1977,6 +1977,7 @@ dependencies = [ "indoc", "online", "pretty_assertions", + "regex", "reqwest", "sdl-encoder", "semver 1.0.3", diff --git a/Cargo.toml b/Cargo.toml index 7b430cbd6..4e01982d8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -57,7 +57,6 @@ humantime = "2.1.0" opener = "0.5.0" os_info = "3.0" prettytable-rs = "0.8.0" -regex = "1" serde = "1.0" serde_json = "1.0" serde_yaml = "0.8" diff --git a/crates/rover-client/Cargo.toml b/crates/rover-client/Cargo.toml index 341562437..a471d4c92 100644 --- a/crates/rover-client/Cargo.toml +++ b/crates/rover-client/Cargo.toml @@ -18,6 +18,7 @@ git2 = { version = "0.13.20", default-features = false, features = ["vendored-op graphql_client = "0.9" http = "0.2" reqwest = {version = "0.11", default-features = false, features = ["json", "blocking", "rustls-tls", "gzip"]} +regex = "1" sdl-encoder = {path = "../sdl-encoder"} semver = "1" serde = "1" diff --git a/crates/rover-client/src/error.rs b/crates/rover-client/src/error.rs index 220cf319e..68a34f1f1 100644 --- a/crates/rover-client/src/error.rs +++ b/crates/rover-client/src/error.rs @@ -1,7 +1,7 @@ use reqwest::Url; use thiserror::Error; -use crate::{operations::subgraph::check::types::CompositionError, shared::CheckResponse}; +use crate::shared::{CheckResponse, CompositionError}; /// RoverClientError represents all possible failures that can occur during a client request. #[derive(Error, Debug)] @@ -138,6 +138,10 @@ pub enum RoverClientError { #[error("{}", check_response_error_msg(.check_response))] OperationCheckFailure { check_response: CheckResponse }, + /// This error occurs when a user has a malformed Graph Ref + #[error("Graph IDs must be in the format or @, where can only contain letters, numbers, or the characters `-` or `_`, and must be 64 characters or less. must be 64 characters or less.")] + InvalidGraphRef, + /// This error occurs when a user has a malformed API key #[error( "The API key you provided is malformed. An API key must have three parts separated by a colon." diff --git a/crates/rover-client/src/operations/config/who_am_i/mod.rs b/crates/rover-client/src/operations/config/who_am_i/mod.rs index 9e6871fdd..ccb47268d 100644 --- a/crates/rover-client/src/operations/config/who_am_i/mod.rs +++ b/crates/rover-client/src/operations/config/who_am_i/mod.rs @@ -1,4 +1,5 @@ +mod runner; mod types; -pub mod runner; +pub use runner::run; pub use types::{Actor, ConfigWhoAmIInput, RegistryIdentity}; diff --git a/crates/rover-client/src/operations/graph/check/runner.rs b/crates/rover-client/src/operations/graph/check/runner.rs index 190dec094..69ec084f2 100644 --- a/crates/rover-client/src/operations/graph/check/runner.rs +++ b/crates/rover-client/src/operations/graph/check/runner.rs @@ -29,7 +29,7 @@ pub fn run( input: GraphCheckInput, client: &StudioClient, ) -> Result { - let graph = input.graph_id.clone(); + let graph = input.graph_ref.name.clone(); let data = client.post::(input.into())?; get_check_response_from_data(data, graph) } diff --git a/crates/rover-client/src/operations/graph/check/types.rs b/crates/rover-client/src/operations/graph/check/types.rs index 61c16efac..1a85b5207 100644 --- a/crates/rover-client/src/operations/graph/check/types.rs +++ b/crates/rover-client/src/operations/graph/check/types.rs @@ -1,10 +1,9 @@ use crate::operations::graph::check::runner::graph_check_mutation; -use crate::shared::{ChangeSeverity, CheckConfig, GitContext, SchemaChange}; +use crate::shared::{ChangeSeverity, CheckConfig, GitContext, GraphRef, SchemaChange}; #[derive(Debug, Clone, PartialEq)] pub struct GraphCheckInput { - pub graph_id: String, - pub variant: String, + pub graph_ref: GraphRef, pub proposed_schema: String, pub git_context: GitContext, pub config: CheckConfig, @@ -13,8 +12,8 @@ pub struct GraphCheckInput { impl From for MutationVariables { fn from(input: GraphCheckInput) -> Self { Self { - graph_id: input.graph_id, - variant: Some(input.variant), + graph_id: input.graph_ref.name, + variant: Some(input.graph_ref.variant), proposed_schema: Some(input.proposed_schema), config: input.config.into(), git_context: input.git_context.into(), diff --git a/crates/rover-client/src/operations/subgraph/check/mod.rs b/crates/rover-client/src/operations/subgraph/check/mod.rs index 9734f57b4..ebbc0d6e6 100644 --- a/crates/rover-client/src/operations/subgraph/check/mod.rs +++ b/crates/rover-client/src/operations/subgraph/check/mod.rs @@ -1,3 +1,5 @@ -pub mod runner; -pub(crate) mod types; +mod runner; +mod types; + +pub use runner::run; pub use types::SubgraphCheckInput; diff --git a/crates/rover-client/src/operations/subgraph/check/runner.rs b/crates/rover-client/src/operations/subgraph/check/runner.rs index 94d91bd85..fde0365bb 100644 --- a/crates/rover-client/src/operations/subgraph/check/runner.rs +++ b/crates/rover-client/src/operations/subgraph/check/runner.rs @@ -1,7 +1,7 @@ use super::types::*; use crate::blocking::StudioClient; use crate::operations::{config::is_federated, subgraph::check::types::MutationResponseData}; -use crate::shared::{CheckResponse, SchemaChange}; +use crate::shared::{CheckResponse, CompositionError, SchemaChange}; use crate::RoverClientError; use graphql_client::*; @@ -28,12 +28,12 @@ pub fn run( input: SubgraphCheckInput, client: &StudioClient, ) -> Result { - let graph = input.graph_id.clone(); + let graph = input.graph_ref.name.clone(); // This response is used to check whether or not the current graph is federated. let is_federated = is_federated::run( is_federated::is_federated_graph::Variables { - graph_id: input.graph_id.clone(), - graph_variant: input.variant.clone(), + graph_id: input.graph_ref.name.clone(), + graph_variant: input.graph_ref.variant.clone(), }, &client, )?; diff --git a/crates/rover-client/src/operations/subgraph/check/types.rs b/crates/rover-client/src/operations/subgraph/check/types.rs index 2fdfd258a..dd38b7f57 100644 --- a/crates/rover-client/src/operations/subgraph/check/types.rs +++ b/crates/rover-client/src/operations/subgraph/check/types.rs @@ -1,5 +1,5 @@ use crate::operations::subgraph::check::runner::subgraph_check_mutation; -use crate::shared::{ChangeSeverity, CheckConfig, GitContext}; +use crate::shared::{ChangeSeverity, CheckConfig, GitContext, GraphRef}; type MutationVariables = subgraph_check_mutation::Variables; @@ -36,8 +36,7 @@ impl From for MutationGitContextInput { #[derive(Debug, Clone, PartialEq)] pub struct SubgraphCheckInput { - pub graph_id: String, - pub variant: String, + pub graph_ref: GraphRef, pub subgraph: String, pub proposed_schema: String, pub git_context: GitContext, @@ -47,8 +46,8 @@ pub struct SubgraphCheckInput { impl From for MutationVariables { fn from(input: SubgraphCheckInput) -> Self { Self { - graph_id: input.graph_id, - variant: input.variant, + graph_id: input.graph_ref.name, + variant: input.graph_ref.variant, subgraph: input.subgraph, proposed_schema: MutationSchema { sdl: Some(input.proposed_schema), @@ -68,9 +67,3 @@ impl From for MutationVariables { } } } - -#[derive(Debug, Clone, PartialEq)] -pub struct CompositionError { - pub message: String, - pub code: Option, -} diff --git a/crates/rover-client/src/operations/subgraph/delete/delete_mutation.graphql b/crates/rover-client/src/operations/subgraph/delete/delete_mutation.graphql index f9ba68241..40d22c3d5 100644 --- a/crates/rover-client/src/operations/subgraph/delete/delete_mutation.graphql +++ b/crates/rover-client/src/operations/subgraph/delete/delete_mutation.graphql @@ -12,6 +12,7 @@ mutation SubgraphDeleteMutation( ) { errors { message + code } updatedGateway } diff --git a/crates/rover-client/src/operations/subgraph/delete/mod.rs b/crates/rover-client/src/operations/subgraph/delete/mod.rs index 387f7cbc0..db8cb0622 100644 --- a/crates/rover-client/src/operations/subgraph/delete/mod.rs +++ b/crates/rover-client/src/operations/subgraph/delete/mod.rs @@ -1,6 +1,5 @@ mod runner; - -pub(crate) mod types; +mod types; pub use runner::run; pub use types::{SubgraphDeleteInput, SubgraphDeleteResponse}; diff --git a/crates/rover-client/src/operations/subgraph/delete/runner.rs b/crates/rover-client/src/operations/subgraph/delete/runner.rs index 694a7afc3..4355bd228 100644 --- a/crates/rover-client/src/operations/subgraph/delete/runner.rs +++ b/crates/rover-client/src/operations/subgraph/delete/runner.rs @@ -1,5 +1,6 @@ use crate::blocking::StudioClient; use crate::operations::subgraph::delete::types::*; +use crate::shared::CompositionError; use crate::RoverClientError; use graphql_client::*; @@ -24,7 +25,7 @@ pub fn run( input: SubgraphDeleteInput, client: &StudioClient, ) -> Result { - let graph = input.graph_id.clone(); + let graph = input.graph_ref.name.clone(); let response_data = client.post::(input.into())?; let data = get_delete_data_from_response(response_data, graph)?; Ok(build_response(data)) @@ -42,10 +43,15 @@ fn get_delete_data_from_response( } fn build_response(response: MutationComposition) -> SubgraphDeleteResponse { - let composition_errors: Vec = response + let composition_errors: Vec = response .errors .iter() - .filter_map(|error| error.as_ref().map(|e| e.message.clone())) + .filter_map(|error| { + error.as_ref().map(|e| CompositionError { + message: e.message.clone(), + code: e.code.clone(), + }) + }) .collect(); // if there are no errors, just return None @@ -72,9 +78,15 @@ mod tests { "service": { "removeImplementingServiceAndTriggerComposition": { "errors": [ - { "message": "wow" }, + { + "message": "wow", + "code": null + }, null, - { "message": "boo" } + { + "message": "boo", + "code": "BOO" + } ], "updatedGateway": false, } @@ -90,10 +102,12 @@ mod tests { errors: vec![ Some(MutationCompositionErrors { message: "wow".to_string(), + code: None, }), None, Some(MutationCompositionErrors { message: "boo".to_string(), + code: Some("BOO".to_string()), }), ], updated_gateway: false, @@ -107,10 +121,12 @@ mod tests { errors: vec![ Some(MutationCompositionErrors { message: "wow".to_string(), + code: None, }), None, Some(MutationCompositionErrors { message: "boo".to_string(), + code: Some("BOO".to_string()), }), ], updated_gateway: false, @@ -120,7 +136,16 @@ mod tests { assert_eq!( parsed, SubgraphDeleteResponse { - composition_errors: Some(vec!["wow".to_string(), "boo".to_string()]), + composition_errors: Some(vec![ + CompositionError { + message: "wow".to_string(), + code: None + }, + CompositionError { + message: "boo".to_string(), + code: Some("BOO".to_string()) + } + ]), updated_gateway: false, } ); diff --git a/crates/rover-client/src/operations/subgraph/delete/types.rs b/crates/rover-client/src/operations/subgraph/delete/types.rs index 04ce6c2e6..0bb7b67d2 100644 --- a/crates/rover-client/src/operations/subgraph/delete/types.rs +++ b/crates/rover-client/src/operations/subgraph/delete/types.rs @@ -1,4 +1,7 @@ -use crate::operations::subgraph::delete::runner::subgraph_delete_mutation; +use crate::{ + operations::subgraph::delete::runner::subgraph_delete_mutation, + shared::{CompositionError, GraphRef}, +}; pub(crate) type MutationComposition = subgraph_delete_mutation::SubgraphDeleteMutationServiceRemoveImplementingServiceAndTriggerComposition; pub(crate) type MutationVariables = subgraph_delete_mutation::Variables; @@ -8,8 +11,7 @@ pub(crate) type MutationCompositionErrors = subgraph_delete_mutation::SubgraphDe #[derive(Debug, Clone, PartialEq)] pub struct SubgraphDeleteInput { - pub graph_id: String, - pub variant: String, + pub graph_ref: GraphRef, pub subgraph: String, pub dry_run: bool, } @@ -21,14 +23,14 @@ pub struct SubgraphDeleteInput { #[derive(Debug, PartialEq)] pub struct SubgraphDeleteResponse { pub updated_gateway: bool, - pub composition_errors: Option>, + pub composition_errors: Option>, } impl From for MutationVariables { fn from(input: SubgraphDeleteInput) -> Self { Self { - graph_id: input.graph_id, - variant: input.variant, + graph_id: input.graph_ref.name, + variant: input.graph_ref.variant, subgraph: input.subgraph, dry_run: input.dry_run, } diff --git a/crates/rover-client/src/operations/subgraph/fetch/mod.rs b/crates/rover-client/src/operations/subgraph/fetch/mod.rs index 57ec30cd3..6258a0615 100644 --- a/crates/rover-client/src/operations/subgraph/fetch/mod.rs +++ b/crates/rover-client/src/operations/subgraph/fetch/mod.rs @@ -1,3 +1,5 @@ -pub mod runner; -pub(crate) mod types; +mod runner; +mod types; + +pub use runner::run; pub use types::{SubgraphFetchInput, SubgraphFetchResponse}; diff --git a/crates/rover-client/src/operations/subgraph/fetch/runner.rs b/crates/rover-client/src/operations/subgraph/fetch/runner.rs index e23032c57..29423c600 100644 --- a/crates/rover-client/src/operations/subgraph/fetch/runner.rs +++ b/crates/rover-client/src/operations/subgraph/fetch/runner.rs @@ -22,16 +22,16 @@ pub fn run( input: SubgraphFetchInput, client: &StudioClient, ) -> Result { - let variables: SubgraphFetchVariables = input.clone().into(); - let response_data = client.post::(variables.into())?; - get_sdl_from_response_data(input, response_data) + let input_clone = input.clone(); + let response_data = client.post::(input.into())?; + get_sdl_from_response_data(input_clone, response_data) } fn get_sdl_from_response_data( input: SubgraphFetchInput, response_data: SubgraphFetchResponseData, ) -> Result { - let service_list = get_services_from_response_data(&input.graph_id, response_data)?; + let service_list = get_services_from_response_data(&input.graph_ref.name, response_data)?; let sdl = get_sdl_for_service(&input.subgraph, service_list)?; Ok(SubgraphFetchResponse { sdl }) } diff --git a/crates/rover-client/src/operations/subgraph/fetch/types.rs b/crates/rover-client/src/operations/subgraph/fetch/types.rs index e0554de85..304805e57 100644 --- a/crates/rover-client/src/operations/subgraph/fetch/types.rs +++ b/crates/rover-client/src/operations/subgraph/fetch/types.rs @@ -1,3 +1,5 @@ +use crate::shared::GraphRef; + use super::runner::subgraph_fetch_query; pub(crate) type ServiceList = Vec; @@ -7,31 +9,15 @@ pub(crate) type QueryVariables = subgraph_fetch_query::Variables; #[derive(Debug, Clone, PartialEq)] pub struct SubgraphFetchInput { - pub graph_id: String, - pub variant: String, + pub graph_ref: GraphRef, pub subgraph: String, } -#[derive(Debug, Clone, PartialEq)] -pub(crate) struct SubgraphFetchVariables { - graph_id: String, - variant: String, -} - -impl From for SubgraphFetchVariables { +impl From for QueryVariables { fn from(input: SubgraphFetchInput) -> Self { Self { - graph_id: input.graph_id, - variant: input.variant, - } - } -} - -impl From for QueryVariables { - fn from(fetch_variables: SubgraphFetchVariables) -> Self { - Self { - graph_id: fetch_variables.graph_id, - variant: fetch_variables.variant, + graph_id: input.graph_ref.name, + variant: input.graph_ref.variant, } } } diff --git a/crates/rover-client/src/operations/subgraph/list/mod.rs b/crates/rover-client/src/operations/subgraph/list/mod.rs index aeb6b0a14..912e0a83c 100644 --- a/crates/rover-client/src/operations/subgraph/list/mod.rs +++ b/crates/rover-client/src/operations/subgraph/list/mod.rs @@ -1,6 +1,5 @@ mod runner; - -pub(crate) mod types; +mod types; pub use runner::run; pub use types::{SubgraphListInput, SubgraphListResponse}; diff --git a/crates/rover-client/src/operations/subgraph/list/runner.rs b/crates/rover-client/src/operations/subgraph/list/runner.rs index 0b701d6be..1c1faed5e 100644 --- a/crates/rover-client/src/operations/subgraph/list/runner.rs +++ b/crates/rover-client/src/operations/subgraph/list/runner.rs @@ -24,7 +24,7 @@ pub fn run( input: SubgraphListInput, client: &StudioClient, ) -> Result { - let graph = input.graph_id.clone(); + let graph = input.graph_ref.name.clone(); let response_data = client.post::(input.into())?; let root_url = response_data.frontend_url_root.clone(); let subgraphs = get_subgraphs_from_response_data(response_data, graph.clone())?; diff --git a/crates/rover-client/src/operations/subgraph/list/types.rs b/crates/rover-client/src/operations/subgraph/list/types.rs index 07235cfd0..00b9da8b6 100644 --- a/crates/rover-client/src/operations/subgraph/list/types.rs +++ b/crates/rover-client/src/operations/subgraph/list/types.rs @@ -1,4 +1,4 @@ -use crate::operations::subgraph::list::runner::subgraph_list_query; +use crate::{operations::subgraph::list::runner::subgraph_list_query, shared::GraphRef}; pub(crate) type QuerySubgraphInfo = subgraph_list_query::SubgraphListQueryServiceImplementingServicesOnFederatedImplementingServicesServices; pub(crate) type QueryResponseData = subgraph_list_query::ResponseData; @@ -10,15 +10,14 @@ use chrono::{DateTime, Local}; #[derive(Clone, PartialEq, Debug)] pub struct SubgraphListInput { - pub graph_id: String, - pub variant: String, + pub graph_ref: GraphRef, } impl From for QueryVariables { fn from(input: SubgraphListInput) -> Self { Self { - graph_id: input.graph_id, - variant: input.variant, + graph_id: input.graph_ref.name, + variant: input.graph_ref.variant, } } } diff --git a/crates/rover-client/src/operations/subgraph/publish/mod.rs b/crates/rover-client/src/operations/subgraph/publish/mod.rs index 8ad8f0747..0cf673b54 100644 --- a/crates/rover-client/src/operations/subgraph/publish/mod.rs +++ b/crates/rover-client/src/operations/subgraph/publish/mod.rs @@ -1,3 +1,5 @@ -pub mod runner; -pub(crate) mod types; +mod runner; +mod types; + +pub use runner::run; pub use types::{SubgraphPublishInput, SubgraphPublishResponse}; diff --git a/crates/rover-client/src/operations/subgraph/publish/publish_mutation.graphql b/crates/rover-client/src/operations/subgraph/publish/publish_mutation.graphql index 956de4ba2..c64f45468 100644 --- a/crates/rover-client/src/operations/subgraph/publish/publish_mutation.graphql +++ b/crates/rover-client/src/operations/subgraph/publish/publish_mutation.graphql @@ -21,6 +21,7 @@ mutation SubgraphPublishMutation( } errors { message + code } didUpdateGateway: updatedGateway serviceWasCreated: wasCreated diff --git a/crates/rover-client/src/operations/subgraph/publish/runner.rs b/crates/rover-client/src/operations/subgraph/publish/runner.rs index b85154b5e..e1ed36987 100644 --- a/crates/rover-client/src/operations/subgraph/publish/runner.rs +++ b/crates/rover-client/src/operations/subgraph/publish/runner.rs @@ -1,6 +1,7 @@ use super::types::*; use crate::blocking::StudioClient; use crate::operations::config::is_federated; +use crate::shared::CompositionError; use crate::RoverClientError; use graphql_client::*; @@ -23,15 +24,15 @@ pub fn run( client: &StudioClient, ) -> Result { let variables: MutationVariables = input.clone().into(); - let graph = input.graph_id.clone(); + let graph = input.graph_ref.name.clone(); // We don't want to implicitly convert non-federated graph to supergraphs. // Error here if no --convert flag is passed _and_ the current context // is non-federated. Add a suggestion to require a --convert flag. if !input.convert_to_federated_graph { let is_federated = is_federated::run( is_federated::is_federated_graph::Variables { - graph_id: input.graph_id.clone(), - graph_variant: input.variant, + graph_id: input.graph_ref.name.clone(), + graph_variant: input.graph_ref.variant, }, &client, )?; @@ -58,10 +59,15 @@ fn get_publish_response_from_data( } fn build_response(publish_response: UpdateResponse) -> SubgraphPublishResponse { - let composition_errors: Vec = publish_response + let composition_errors: Vec = publish_response .errors .iter() - .filter_map(|error| error.as_ref().map(|e| e.message.clone())) + .filter_map(|error| { + error.as_ref().map(|e| CompositionError { + message: e.message.clone(), + code: e.code.clone(), + }) + }) .collect(); // if there are no errors, just return None @@ -77,7 +83,7 @@ fn build_response(publish_response: UpdateResponse) -> SubgraphPublishResponse { None => None, }, did_update_gateway: publish_response.did_update_gateway, - service_was_created: publish_response.service_was_created, + subgraph_was_created: publish_response.service_was_created, composition_errors, } } @@ -91,9 +97,15 @@ mod tests { let json_response = json!({ "compositionConfig": { "schemaHash": "5gf564" }, "errors": [ - {"message": "[Accounts] User -> composition error"}, + { + "message": "[Accounts] User -> composition error", + "code": null + }, null, // this is technically allowed in the types - {"message": "[Products] Product -> another one"} + { + "message": "[Products] Product -> another one", + "code": "ERROR" + } ], "didUpdateGateway": false, "serviceWasCreated": true @@ -106,11 +118,17 @@ mod tests { SubgraphPublishResponse { schema_hash: Some("5gf564".to_string()), composition_errors: Some(vec![ - "[Accounts] User -> composition error".to_string(), - "[Products] Product -> another one".to_string() + CompositionError { + message: "[Accounts] User -> composition error".to_string(), + code: None + }, + CompositionError { + message: "[Products] Product -> another one".to_string(), + code: Some("ERROR".to_string()) + } ]), did_update_gateway: false, - service_was_created: true, + subgraph_was_created: true, } ); } @@ -132,7 +150,7 @@ mod tests { schema_hash: Some("5gf564".to_string()), composition_errors: None, did_update_gateway: true, - service_was_created: true, + subgraph_was_created: true, } ); } @@ -143,7 +161,10 @@ mod tests { fn build_response_works_with_failure_and_no_hash() { let json_response = json!({ "compositionConfig": null, - "errors": [{ "message": "[Accounts] -> Things went really wrong" }], + "errors": [{ + "message": "[Accounts] -> Things went really wrong", + "code": null + }], "didUpdateGateway": false, "serviceWasCreated": false }); @@ -154,11 +175,12 @@ mod tests { output, SubgraphPublishResponse { schema_hash: None, - composition_errors: Some( - vec!["[Accounts] -> Things went really wrong".to_string()] - ), + composition_errors: Some(vec![CompositionError { + message: "[Accounts] -> Things went really wrong".to_string(), + code: None + }]), did_update_gateway: false, - service_was_created: false, + subgraph_was_created: false, } ); } diff --git a/crates/rover-client/src/operations/subgraph/publish/types.rs b/crates/rover-client/src/operations/subgraph/publish/types.rs index e51b32d67..6472a70e4 100644 --- a/crates/rover-client/src/operations/subgraph/publish/types.rs +++ b/crates/rover-client/src/operations/subgraph/publish/types.rs @@ -1,6 +1,6 @@ use super::runner::subgraph_publish_mutation; -use crate::shared::GitContext; +use crate::shared::{CompositionError, GitContext, GraphRef}; pub(crate) type ResponseData = subgraph_publish_mutation::ResponseData; pub(crate) type MutationVariables = subgraph_publish_mutation::Variables; @@ -11,8 +11,7 @@ type GitContextInput = subgraph_publish_mutation::GitContextInput; #[derive(Debug, Clone, PartialEq)] pub struct SubgraphPublishInput { - pub graph_id: String, - pub variant: String, + pub graph_ref: GraphRef, pub subgraph: String, pub url: Option, pub schema: String, @@ -24,15 +23,15 @@ pub struct SubgraphPublishInput { pub struct SubgraphPublishResponse { pub schema_hash: Option, pub did_update_gateway: bool, - pub service_was_created: bool, - pub composition_errors: Option>, + pub subgraph_was_created: bool, + pub composition_errors: Option>, } impl From for MutationVariables { fn from(publish_input: SubgraphPublishInput) -> Self { Self { - graph_id: publish_input.graph_id, - variant: publish_input.variant, + graph_id: publish_input.graph_ref.name, + variant: publish_input.graph_ref.variant, subgraph: publish_input.subgraph, url: publish_input.url, schema: SchemaInput { diff --git a/crates/rover-client/src/shared/composition_error.rs b/crates/rover-client/src/shared/composition_error.rs new file mode 100644 index 000000000..b45e51c5a --- /dev/null +++ b/crates/rover-client/src/shared/composition_error.rs @@ -0,0 +1,18 @@ +use std::fmt::{self, Display}; + +#[derive(Debug, Clone, PartialEq)] +pub struct CompositionError { + pub message: String, + pub code: Option, +} + +impl Display for CompositionError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Some(code) = &self.code { + write!(f, "{}: ", code)?; + } else { + write!(f, "UNKNOWN: ")?; + } + write!(f, "{}", &self.message) + } +} diff --git a/crates/rover-client/src/shared/graph_ref.rs b/crates/rover-client/src/shared/graph_ref.rs new file mode 100644 index 000000000..5cad87c3e --- /dev/null +++ b/crates/rover-client/src/shared/graph_ref.rs @@ -0,0 +1,104 @@ +use std::fmt; +use std::str::FromStr; + +use crate::RoverClientError; + +use regex::Regex; + +#[derive(Debug, Clone, PartialEq)] +pub struct GraphRef { + pub name: String, + pub variant: String, +} + +impl fmt::Display for GraphRef { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}@{}", self.name, self.variant) + } +} + +impl FromStr for GraphRef { + type Err = RoverClientError; + + /// NOTE: THIS IS A TEMPORARY SOLUTION. IN THE FUTURE, ALL GRAPH ID PARSING + /// WILL HAPPEN IN THE BACKEND TO KEEP EVERYTHING CONSISTENT. THIS IS AN + /// INCOMPLETE PLACEHOLDER, AND MAY NOT COVER EVERY SINGLE VALID USE CASE + fn from_str(graph_id: &str) -> Result { + let pattern = Regex::new(r"^[a-zA-Z][a-zA-Z0-9_-]{0,63}$").unwrap(); + let variant_pattern = Regex::new(r"^([a-zA-Z][a-zA-Z0-9_-]{0,63})@(.{0,63})$").unwrap(); + + let valid_graph_name_only = pattern.is_match(graph_id); + let valid_graph_with_variant = variant_pattern.is_match(graph_id); + + if valid_graph_name_only { + Ok(GraphRef { + name: graph_id.to_string(), + variant: "current".to_string(), + }) + } else if valid_graph_with_variant { + let matches = variant_pattern.captures(graph_id).unwrap(); + let name = matches.get(1).unwrap().as_str(); + let variant = matches.get(2).unwrap().as_str(); + Ok(GraphRef { + name: name.to_string(), + variant: variant.to_string(), + }) + } else { + Err(RoverClientError::InvalidGraphRef) + } + } +} + +#[cfg(test)] +mod tests { + use super::GraphRef; + use std::str::FromStr; + + #[test] + fn from_str_works() { + assert!(GraphRef::from_str("engine#%^").is_err()); + assert!(GraphRef::from_str( + "1234567890123456789012345678901234567890123456789012345678901234567890" + ) + .is_err()); + assert!(GraphRef::from_str("1boi").is_err()); + assert!(GraphRef::from_str("_eng").is_err()); + assert!(GraphRef::from_str( + "engine@1234567890123456789012345678901234567890123456789012345678901234567890" + ) + .is_err()); + assert!(GraphRef::from_str( + "engine1234567890123456789012345678901234567890123456789012345678901234567890@prod" + ) + .is_err()); + + assert_eq!( + GraphRef::from_str("engine@okay").unwrap(), + GraphRef { + name: "engine".to_string(), + variant: "okay".to_string() + } + ); + assert_eq!( + GraphRef::from_str("studio").unwrap(), + GraphRef { + name: "studio".to_string(), + variant: "current".to_string() + } + ); + assert_eq!( + GraphRef::from_str("this_should_work").unwrap(), + GraphRef { + name: "this_should_work".to_string(), + variant: "current".to_string() + } + ); + assert_eq!( + GraphRef::from_str("it-is-cool@my-special/variant:from$hell").unwrap(), + GraphRef { + name: "it-is-cool".to_string(), + variant: "my-special/variant:from$hell".to_string() + } + ); + } +} diff --git a/crates/rover-client/src/shared/mod.rs b/crates/rover-client/src/shared/mod.rs index e4d6a6267..9a2f6f445 100644 --- a/crates/rover-client/src/shared/mod.rs +++ b/crates/rover-client/src/shared/mod.rs @@ -1,5 +1,9 @@ mod check_response; +mod composition_error; mod git_context; +mod graph_ref; pub use check_response::{ChangeSeverity, CheckConfig, CheckResponse, SchemaChange}; +pub use composition_error::CompositionError; pub use git_context::GitContext; +pub use graph_ref::GraphRef; diff --git a/src/command/config/whoami.rs b/src/command/config/whoami.rs index b79104660..332b2c637 100644 --- a/src/command/config/whoami.rs +++ b/src/command/config/whoami.rs @@ -1,5 +1,5 @@ use ansi_term::Colour::Green; -use rover_client::operations::config::who_am_i::{runner, Actor, ConfigWhoAmIInput}; +use rover_client::operations::config::who_am_i::{self, Actor, ConfigWhoAmIInput}; use serde::Serialize; use structopt::StructOpt; @@ -26,7 +26,7 @@ impl WhoAmI { let client = client_config.get_client(&self.profile_name)?; eprintln!("Checking identity of your API key against the registry."); - let identity = runner::run(ConfigWhoAmIInput {}, &client)?; + let identity = who_am_i::run(ConfigWhoAmIInput {}, &client)?; let mut message = format!( "{}: {:?}\n", diff --git a/src/command/graph/check.rs b/src/command/graph/check.rs index ac2ca8e4d..a8bf3b821 100644 --- a/src/command/graph/check.rs +++ b/src/command/graph/check.rs @@ -2,14 +2,14 @@ use serde::Serialize; use structopt::StructOpt; use rover_client::operations::graph::check::{self, GraphCheckInput}; -use rover_client::shared::{CheckConfig, GitContext}; +use rover_client::shared::{CheckConfig, GitContext, GraphRef}; use crate::command::RoverStdout; use crate::utils::client::StudioClientConfig; use crate::utils::loaders::load_schema_from_flag; use crate::utils::parsers::{ - parse_graph_ref, parse_query_count_threshold, parse_query_percentage_threshold, - parse_schema_source, parse_validation_period, GraphRef, SchemaSource, ValidationPeriod, + parse_query_count_threshold, parse_query_percentage_threshold, parse_schema_source, + parse_validation_period, SchemaSource, ValidationPeriod, }; use crate::Result; @@ -17,7 +17,7 @@ use crate::Result; pub struct Check { /// @ of graph in Apollo Studio to validate. /// @ may be left off, defaulting to @current - #[structopt(name = "GRAPH_REF", parse(try_from_str = parse_graph_ref))] + #[structopt(name = "GRAPH_REF")] #[serde(skip_serializing)] graph: GraphRef, @@ -64,8 +64,7 @@ impl Check { let res = check::run( GraphCheckInput { - graph_id: self.graph.name.clone(), - variant: self.graph.variant.clone(), + graph_ref: self.graph.clone(), proposed_schema, git_context, config: CheckConfig { diff --git a/src/command/graph/fetch.rs b/src/command/graph/fetch.rs index e08383f0f..dd2c21d53 100644 --- a/src/command/graph/fetch.rs +++ b/src/command/graph/fetch.rs @@ -3,17 +3,17 @@ use serde::Serialize; use structopt::StructOpt; use rover_client::operations::graph::fetch; +use rover_client::shared::GraphRef; use crate::command::RoverStdout; use crate::utils::client::StudioClientConfig; -use crate::utils::parsers::{parse_graph_ref, GraphRef}; use crate::Result; #[derive(Debug, Serialize, StructOpt)] pub struct Fetch { /// @ of graph in Apollo Studio to fetch from. /// @ may be left off, defaulting to @current - #[structopt(name = "GRAPH_REF", parse(try_from_str = parse_graph_ref))] + #[structopt(name = "GRAPH_REF")] #[serde(skip_serializing)] graph: GraphRef, diff --git a/src/command/graph/publish.rs b/src/command/graph/publish.rs index 1526f09f1..b20e53f34 100644 --- a/src/command/graph/publish.rs +++ b/src/command/graph/publish.rs @@ -3,19 +3,19 @@ use serde::Serialize; use structopt::StructOpt; use rover_client::operations::graph::publish; -use rover_client::shared::GitContext; +use rover_client::shared::{GitContext, GraphRef}; use crate::command::RoverStdout; use crate::utils::client::StudioClientConfig; use crate::utils::loaders::load_schema_from_flag; -use crate::utils::parsers::{parse_graph_ref, parse_schema_source, GraphRef, SchemaSource}; +use crate::utils::parsers::{parse_schema_source, SchemaSource}; use crate::Result; #[derive(Debug, Serialize, StructOpt)] pub struct Publish { /// @ of graph in Apollo Studio to publish to. /// @ may be left off, defaulting to @current - #[structopt(name = "GRAPH_REF", parse(try_from_str = parse_graph_ref))] + #[structopt(name = "GRAPH_REF")] #[serde(skip_serializing)] graph: GraphRef, diff --git a/src/command/subgraph/check.rs b/src/command/subgraph/check.rs index de6780d87..642268efa 100644 --- a/src/command/subgraph/check.rs +++ b/src/command/subgraph/check.rs @@ -1,15 +1,15 @@ use serde::Serialize; use structopt::StructOpt; -use rover_client::operations::subgraph::check::{runner, SubgraphCheckInput}; -use rover_client::shared::{CheckConfig, GitContext}; +use rover_client::operations::subgraph::check::{self, SubgraphCheckInput}; +use rover_client::shared::{CheckConfig, GitContext, GraphRef}; use crate::command::RoverStdout; use crate::utils::client::StudioClientConfig; use crate::utils::loaders::load_schema_from_flag; use crate::utils::parsers::{ - parse_graph_ref, parse_query_count_threshold, parse_query_percentage_threshold, - parse_schema_source, parse_validation_period, GraphRef, SchemaSource, ValidationPeriod, + parse_query_count_threshold, parse_query_percentage_threshold, parse_schema_source, + parse_validation_period, SchemaSource, ValidationPeriod, }; use crate::Result; @@ -17,7 +17,7 @@ use crate::Result; pub struct Check { /// @ of graph in Apollo Studio to validate. /// @ may be left off, defaulting to @current - #[structopt(name = "GRAPH_REF", parse(try_from_str = parse_graph_ref))] + #[structopt(name = "GRAPH_REF")] #[serde(skip_serializing)] graph: GraphRef, @@ -68,10 +68,9 @@ impl Check { &self.subgraph, &self.graph ); - let res = runner::run( + let res = check::run( SubgraphCheckInput { - graph_id: self.graph.name.clone(), - variant: self.graph.variant.clone(), + graph_ref: self.graph.clone(), proposed_schema, subgraph: self.subgraph.clone(), git_context, diff --git a/src/command/subgraph/delete.rs b/src/command/subgraph/delete.rs index 09bcf5dd3..675138b9e 100644 --- a/src/command/subgraph/delete.rs +++ b/src/command/subgraph/delete.rs @@ -4,18 +4,18 @@ use structopt::StructOpt; use crate::command::RoverStdout; use crate::utils::client::StudioClientConfig; -use crate::utils::parsers::{parse_graph_ref, GraphRef}; use crate::Result; use rover_client::operations::subgraph::delete::{ self, SubgraphDeleteInput, SubgraphDeleteResponse, }; +use rover_client::shared::GraphRef; #[derive(Debug, Serialize, StructOpt)] pub struct Delete { /// @ of federated graph in Apollo Studio to delete subgraph from. /// @ may be left off, defaulting to @current - #[structopt(name = "GRAPH_REF", parse(try_from_str = parse_graph_ref))] + #[structopt(name = "GRAPH_REF")] #[serde(skip_serializing)] graph: GraphRef, @@ -53,8 +53,7 @@ impl Delete { // run delete with dryRun, so we can preview composition errors let delete_dry_run_response = delete::run( SubgraphDeleteInput { - graph_id: self.graph.name.clone(), - variant: self.graph.variant.clone(), + graph_ref: self.graph.clone(), subgraph: self.subgraph.clone(), dry_run: true, }, @@ -72,8 +71,7 @@ impl Delete { let delete_response = delete::run( SubgraphDeleteInput { - graph_id: self.graph.name.clone(), - variant: self.graph.variant.clone(), + graph_ref: self.graph.clone(), subgraph: self.subgraph.clone(), dry_run: false, }, @@ -89,12 +87,14 @@ fn handle_dry_run_response(response: SubgraphDeleteResponse, subgraph: &str, gra let warn_prefix = Red.normal().paint("WARN:"); if let Some(errors) = response.composition_errors { eprintln!( - "{} Deleting the {} subgraph from {} would result in the following composition errors: \n{}", - warn_prefix, - Cyan.normal().paint(subgraph), - Cyan.normal().paint(graph_ref), - errors.join("\n") - ); + "{} 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 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); @@ -131,16 +131,20 @@ fn handle_response(response: SubgraphDeleteResponse, subgraph: &str, graph_ref: if let Some(errors) = response.composition_errors { eprintln!( - "{} There were composition errors as a result of deleting the subgraph: \n{}", + "{} There were composition errors as a result of deleting the subgraph:", warn_prefix, - errors.join("\n") - ) + ); + + for error in errors { + eprintln!("{}", &error); + } } } #[cfg(test)] mod tests { use super::{handle_response, SubgraphDeleteResponse}; + use rover_client::shared::CompositionError; #[test] fn handle_response_doesnt_error_with_all_successes() { @@ -156,8 +160,14 @@ mod tests { fn handle_response_doesnt_error_with_all_failures() { let response = SubgraphDeleteResponse { composition_errors: Some(vec![ - "a bad thing happened".to_string(), - "another bad thing".to_string(), + CompositionError { + message: "a bad thing happened".to_string(), + code: None, + }, + CompositionError { + message: "another bad thing".to_string(), + code: None, + }, ]), updated_gateway: false, }; diff --git a/src/command/subgraph/fetch.rs b/src/command/subgraph/fetch.rs index 08ddada32..a5dafbe84 100644 --- a/src/command/subgraph/fetch.rs +++ b/src/command/subgraph/fetch.rs @@ -2,18 +2,18 @@ use ansi_term::Colour::{Cyan, Yellow}; use serde::Serialize; use structopt::StructOpt; -use rover_client::operations::subgraph::fetch::{runner, SubgraphFetchInput}; +use rover_client::operations::subgraph::fetch::{self, SubgraphFetchInput}; +use rover_client::shared::GraphRef; use crate::command::RoverStdout; use crate::utils::client::StudioClientConfig; -use crate::utils::parsers::{parse_graph_ref, GraphRef}; use crate::Result; #[derive(Debug, Serialize, StructOpt)] pub struct Fetch { /// @ of graph in Apollo Studio to fetch from. /// @ may be left off, defaulting to @current - #[structopt(name = "GRAPH_REF", parse(try_from_str = parse_graph_ref))] + #[structopt(name = "GRAPH_REF")] #[serde(skip_serializing)] graph: GraphRef, @@ -39,10 +39,9 @@ impl Fetch { Yellow.normal().paint(&self.profile_name) ); - let result = runner::run( + let result = fetch::run( SubgraphFetchInput { - graph_id: self.graph.name.clone(), - variant: self.graph.variant.clone(), + graph_ref: self.graph.clone(), subgraph: self.subgraph.clone(), }, &client, diff --git a/src/command/subgraph/list.rs b/src/command/subgraph/list.rs index d24ece7e2..3944a8e51 100644 --- a/src/command/subgraph/list.rs +++ b/src/command/subgraph/list.rs @@ -3,17 +3,17 @@ use serde::Serialize; use structopt::StructOpt; use rover_client::operations::subgraph::list::{self, SubgraphListInput}; +use rover_client::shared::GraphRef; use crate::command::RoverStdout; use crate::utils::client::StudioClientConfig; -use crate::utils::parsers::{parse_graph_ref, GraphRef}; use crate::Result; #[derive(Debug, Serialize, StructOpt)] pub struct List { /// @ of graph in Apollo Studio to list subgraphs from. /// @ may be left off, defaulting to @current - #[structopt(name = "GRAPH_REF", parse(try_from_str = parse_graph_ref))] + #[structopt(name = "GRAPH_REF")] #[serde(skip_serializing)] graph: GraphRef, @@ -35,8 +35,7 @@ impl List { let list_details = list::run( SubgraphListInput { - graph_id: self.graph.name.clone(), - variant: self.graph.variant.clone(), + graph_ref: self.graph.clone(), }, &client, )?; diff --git a/src/command/subgraph/publish.rs b/src/command/subgraph/publish.rs index d84c6a0ae..bd1e0284f 100644 --- a/src/command/subgraph/publish.rs +++ b/src/command/subgraph/publish.rs @@ -6,20 +6,20 @@ use crate::command::RoverStdout; use crate::utils::{ client::StudioClientConfig, loaders::load_schema_from_flag, - parsers::{parse_graph_ref, parse_schema_source, GraphRef, SchemaSource}, + parsers::{parse_schema_source, SchemaSource}, }; use crate::Result; use rover_client::operations::subgraph::publish::{ self, SubgraphPublishInput, SubgraphPublishResponse, }; -use rover_client::shared::GitContext; +use rover_client::shared::{GitContext, GraphRef}; #[derive(Debug, Serialize, StructOpt)] pub struct Publish { /// @ of federated graph in Apollo Studio to publish to. /// @ may be left off, defaulting to @current - #[structopt(name = "GRAPH_REF", parse(try_from_str = parse_graph_ref))] + #[structopt(name = "GRAPH_REF")] #[serde(skip_serializing)] graph: GraphRef, @@ -70,10 +70,9 @@ impl Publish { tracing::debug!("Publishing \n{}", &schema); - let publish_response = publish::runner::run( + let publish_response = publish::run( SubgraphPublishInput { - graph_id: self.graph.name.clone(), - variant: self.graph.variant.clone(), + graph_ref: self.graph.clone(), subgraph: self.subgraph.clone(), url: self.routing_url.clone(), schema, @@ -89,7 +88,7 @@ impl Publish { } fn handle_publish_response(response: SubgraphPublishResponse, subgraph: &str, graph: &str) { - if response.service_was_created { + if response.subgraph_was_created { eprintln!( "A new subgraph called '{}' for the '{}' graph was created", subgraph, graph @@ -112,17 +111,17 @@ fn handle_publish_response(response: SubgraphPublishResponse, subgraph: &str, gr if let Some(errors) = response.composition_errors { let warn_prefix = Red.normal().paint("WARN:"); - eprintln!( - "{} The following composition errors occurred: \n{}", - warn_prefix, - errors.join("\n") - ); + eprintln!("{} The following composition errors occurred:", warn_prefix,); + for error in errors { + eprintln!("{}", &error); + } } } #[cfg(test)] mod tests { use super::{handle_publish_response, SubgraphPublishResponse}; + use rover_client::shared::CompositionError; // this test is a bit weird, since we can't test the output. We just verify it // doesn't error @@ -131,7 +130,7 @@ mod tests { let response = SubgraphPublishResponse { schema_hash: Some("123456".to_string()), did_update_gateway: true, - service_was_created: true, + subgraph_was_created: true, composition_errors: None, }; @@ -143,10 +142,16 @@ mod tests { let response = SubgraphPublishResponse { schema_hash: None, did_update_gateway: false, - service_was_created: false, + subgraph_was_created: false, composition_errors: Some(vec![ - "a bad thing happened".to_string(), - "another bad thing".to_string(), + CompositionError { + message: "a bad thing happened".to_string(), + code: None, + }, + CompositionError { + message: "another bad thing".to_string(), + code: None, + }, ]), }; diff --git a/src/command/supergraph/compose/do_compose.rs b/src/command/supergraph/compose/do_compose.rs index 99ad0b9fc..ba76fd2ec 100644 --- a/src/command/supergraph/compose/do_compose.rs +++ b/src/command/supergraph/compose/do_compose.rs @@ -1,5 +1,5 @@ use crate::command::supergraph::config::{self, SchemaSource, SupergraphConfig}; -use crate::utils::{client::StudioClientConfig, parsers::parse_graph_ref}; +use crate::utils::client::StudioClientConfig; use crate::{anyhow, command::RoverStdout, error::RoverError, Result, Suggestion}; use ansi_term::Colour::Red; @@ -7,12 +7,13 @@ use camino::Utf8PathBuf; use rover_client::operations::subgraph::fetch::SubgraphFetchInput; use rover_client::operations::subgraph::introspect::SubgraphIntrospectInput; +use rover_client::shared::GraphRef; use rover_client::{ blocking::GraphQLClient, operations::subgraph::{fetch, introspect}, }; use serde::Serialize; -use std::{collections::HashMap, fs}; +use std::{collections::HashMap, fs, str::FromStr}; use structopt::StructOpt; use harmonizer::ServiceDefinition as SubgraphDefinition; @@ -124,15 +125,16 @@ pub(crate) fn get_subgraph_definitions( let subgraph_definition = SubgraphDefinition::new(subgraph_name, url, &schema); subgraphs.push(subgraph_definition); } - SchemaSource::Subgraph { graphref, subgraph } => { - // given a graphref and subgraph, run subgraph fetch to + SchemaSource::Subgraph { + graph_ref, + subgraph, + } => { + // given a graph_ref and subgraph, run subgraph fetch to // obtain SDL and add it to subgraph_definition. let client = client_config.get_client(&profile_name)?; - let graphref = parse_graph_ref(graphref)?; - let result = fetch::runner::run( + let result = fetch::run( SubgraphFetchInput { - graph_id: graphref.name.clone(), - variant: graphref.variant.clone(), + graph_ref: GraphRef::from_str(graph_ref)?, subgraph: subgraph.clone(), }, &client, diff --git a/src/command/supergraph/config.rs b/src/command/supergraph/config.rs index 59551a32b..50c08b55f 100644 --- a/src/command/supergraph/config.rs +++ b/src/command/supergraph/config.rs @@ -27,7 +27,7 @@ pub(crate) struct Subgraph { pub(crate) enum SchemaSource { File { file: Utf8PathBuf }, SubgraphIntrospection { subgraph_url: Url }, - Subgraph { graphref: String, subgraph: String }, + Subgraph { graph_ref: String, subgraph: String }, } #[cfg(feature = "composition-js")] diff --git a/src/command/supergraph/fetch.rs b/src/command/supergraph/fetch.rs index d55cdd384..97937af1e 100644 --- a/src/command/supergraph/fetch.rs +++ b/src/command/supergraph/fetch.rs @@ -1,8 +1,8 @@ use crate::utils::client::StudioClientConfig; -use crate::utils::parsers::{parse_graph_ref, GraphRef}; use crate::{command::RoverStdout, Result}; use rover_client::operations::supergraph::fetch; +use rover_client::shared::GraphRef; use ansi_term::Colour::{Cyan, Yellow}; use serde::Serialize; @@ -12,7 +12,7 @@ use structopt::StructOpt; pub struct Fetch { /// @ of graph in Apollo Studio to fetch from. /// @ may be left off, defaulting to @current - #[structopt(name = "GRAPH_REF", parse(try_from_str = parse_graph_ref))] + #[structopt(name = "GRAPH_REF")] #[serde(skip_serializing)] graph: GraphRef, diff --git a/src/error/metadata/mod.rs b/src/error/metadata/mod.rs index ab4ecd1a8..b2f33d9d4 100644 --- a/src/error/metadata/mod.rs +++ b/src/error/metadata/mod.rs @@ -63,14 +63,8 @@ impl From<&mut anyhow::Error> for Metadata { composition_errors, } => { for composition_error in composition_errors { - let mut error = format!("{} ", Red.bold().paint("error:")); - if let Some(code) = &composition_error.code { - error.push_str(&format!("{}: ", code)); - } else { - error.push_str("UNKNOWN: "); - } - error.push_str(&composition_error.message); - eprintln!("{}", &error); + let error_descriptor = format!("{} ", Red.bold().paint("error:")); + eprintln!("{} {}", &error_descriptor, &composition_error); } ( Some(Suggestion::FixSubgraphSchema { @@ -147,6 +141,9 @@ impl From<&mut anyhow::Error> for Metadata { RoverClientError::CouldNotConnect { .. } => { (Some(Suggestion::CheckServerConnection), Some(Code::E028)) } + RoverClientError::InvalidGraphRef { .. } => { + unreachable!("Graph ref parse errors should be caught via structopt") + } }; return Metadata { suggestion, diff --git a/src/utils/parsers.rs b/src/utils/parsers.rs index 0f6c31024..9ede9dba8 100644 --- a/src/utils/parsers.rs +++ b/src/utils/parsers.rs @@ -1,8 +1,7 @@ use camino::Utf8PathBuf; -use regex::Regex; use serde::Serialize; -use std::{convert::TryInto, fmt}; +use std::convert::TryInto; use crate::{error::RoverError, Result}; @@ -25,46 +24,6 @@ pub fn parse_schema_source(loc: &str) -> Result { } } -#[derive(Debug, Clone, PartialEq)] -pub struct GraphRef { - pub name: String, - pub variant: String, -} - -impl fmt::Display for GraphRef { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}@{}", self.name, self.variant) - } -} - -/// NOTE: THIS IS A TEMPORARY SOLUTION. IN THE FUTURE, ALL GRAPH ID PARSING -/// WILL HAPPEN IN THE BACKEND TO KEEP EVERYTHING CONSISTENT. THIS IS AN -/// INCOMPLETE PLACEHOLDER, AND MAY NOT COVER EVERY SINGLE VALID USE CASE -pub fn parse_graph_ref(graph_id: &str) -> Result { - let pattern = Regex::new(r"^[a-zA-Z][a-zA-Z0-9_-]{0,63}$").unwrap(); - let variant_pattern = Regex::new(r"^([a-zA-Z][a-zA-Z0-9_-]{0,63})@(.{0,63})$").unwrap(); - - let valid_graph_name_only = pattern.is_match(graph_id); - let valid_graph_with_variant = variant_pattern.is_match(graph_id); - - if valid_graph_name_only { - Ok(GraphRef { - name: graph_id.to_string(), - variant: "current".to_string(), - }) - } else if valid_graph_with_variant { - let matches = variant_pattern.captures(graph_id).unwrap(); - let name = matches.get(1).unwrap().as_str(); - let variant = matches.get(2).unwrap().as_str(); - Ok(GraphRef { - name: name.to_string(), - variant: variant.to_string(), - }) - } else { - Err(RoverError::parse_error("Graph IDs must be in the format or @, where can only contain letters, numbers, or the characters `-` or `_`, and must be 64 characters or less. must be 64 characters or less.")) - } -} - #[derive(Debug, Serialize, Default, Clone)] pub struct ValidationPeriod { // these timestamps could be represented as i64, but the API expects @@ -140,7 +99,7 @@ pub fn parse_header(header: &str) -> Result<(String, String)> { #[cfg(test)] mod tests { - use super::{parse_graph_ref, parse_schema_source, GraphRef, SchemaSource}; + use super::{parse_schema_source, SchemaSource}; #[test] fn it_correctly_parses_stdin_flag() { @@ -163,52 +122,4 @@ mod tests { let loc = parse_schema_source(""); assert!(loc.is_err()); } - - #[test] - fn parse_graph_ref_works() { - assert!(parse_graph_ref("engine#%^").is_err()); - assert!(parse_graph_ref( - "1234567890123456789012345678901234567890123456789012345678901234567890" - ) - .is_err()); - assert!(parse_graph_ref("1boi").is_err()); - assert!(parse_graph_ref("_eng").is_err()); - assert!(parse_graph_ref( - "engine@1234567890123456789012345678901234567890123456789012345678901234567890" - ) - .is_err()); - assert!(parse_graph_ref( - "engine1234567890123456789012345678901234567890123456789012345678901234567890@prod" - ) - .is_err()); - - assert_eq!( - parse_graph_ref("engine@okay").unwrap(), - GraphRef { - name: "engine".to_string(), - variant: "okay".to_string() - } - ); - assert_eq!( - parse_graph_ref("studio").unwrap(), - GraphRef { - name: "studio".to_string(), - variant: "current".to_string() - } - ); - assert_eq!( - parse_graph_ref("this_should_work").unwrap(), - GraphRef { - name: "this_should_work".to_string(), - variant: "current".to_string() - } - ); - assert_eq!( - parse_graph_ref("it-is-cool@my-special/variant:from$hell").unwrap(), - GraphRef { - name: "it-is-cool".to_string(), - variant: "my-special/variant:from$hell".to_string() - } - ); - } }