Skip to content

Commit

Permalink
fix: removes unwrap from is_federated query
Browse files Browse the repository at this point in the history
  • Loading branch information
EverlastingBugstopper committed May 17, 2021
1 parent a9c81d5 commit ab4a4c5
Show file tree
Hide file tree
Showing 13 changed files with 68 additions and 105 deletions.
2 changes: 1 addition & 1 deletion crates/rover-client/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ pub enum RoverClientError {
#[error("The response from Apollo Studio was malformed. Response body contains `null` value for \"{null_field}\"")]
MalformedResponse { null_field: String },

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

/// The API returned an invalid ChangeSeverity value
Expand Down
30 changes: 15 additions & 15 deletions crates/rover-client/src/query/config/is_federated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,33 +17,33 @@ use graphql_client::*;
/// Snake case of this name is the mod name. i.e. publish_partial_schema_mutation
pub struct IsFederatedGraph;

#[derive(Debug, PartialEq)]
pub struct IsFederatedGraphResponse {
pub result: bool,
}

pub fn run(
variables: is_federated_graph::Variables,
client: &StudioClient,
) -> Result<IsFederatedGraphResponse, RoverClientError> {
) -> Result<(), RoverClientError> {
let graph = variables.graph_id.clone();
let data = client.post::<IsFederatedGraph>(variables)?;
let is_federated_response = data.service.unwrap();
Ok(build_response(is_federated_response))
build_response(data, graph)
}

type FederatedResponse = is_federated_graph::IsFederatedGraphService;
type ImplementingServices = is_federated_graph::IsFederatedGraphServiceImplementingServices;

fn build_response(service: FederatedResponse) -> IsFederatedGraphResponse {
fn build_response(
data: is_federated_graph::ResponseData,
graph: String,
) -> Result<(), RoverClientError> {
let service = data.service.ok_or(RoverClientError::NoService {
graph: graph.clone(),
})?;
match service.implementing_services {
Some(typename) => match typename {
ImplementingServices::FederatedImplementingServices => {
IsFederatedGraphResponse { result: true }
}
ImplementingServices::FederatedImplementingServices => Ok(()),
ImplementingServices::NonFederatedImplementingService => {
IsFederatedGraphResponse { result: false }
Err(RoverClientError::ExpectedFederatedGraph { graph })
}
},
None => IsFederatedGraphResponse { result: false },
None => Err(RoverClientError::MalformedResponse {
null_field: "__typename".to_string(),
}),
}
}
9 changes: 3 additions & 6 deletions crates/rover-client/src/query/graph/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,9 @@ fn get_schema_from_response_data(
graph: String,
invalid_variant: String,
) -> Result<String, RoverClientError> {
let service_data = match response_data.service {
Some(data) => Ok(data),
None => Err(RoverClientError::NoService {
graph: graph.clone(),
}),
}?;
let service_data = response_data.service.ok_or(RoverClientError::NoService {
graph: graph.clone(),
})?;

let mut valid_variants = Vec::new();

Expand Down
13 changes: 4 additions & 9 deletions crates/rover-client/src/query/graph/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,13 @@ fn get_publish_response_from_data(
graph: String,
) -> Result<publish_schema_mutation::PublishSchemaMutationServiceUploadSchema, RoverClientError> {
// then, from the response data, get .service?.upload_schema?
let service_data = match data.service {
Some(data) => data,
None => return Err(RoverClientError::NoService { graph }),
};
let service_data = data.service.ok_or(RoverClientError::NoService { graph })?;

if let Some(opt_data) = service_data.upload_schema {
Ok(opt_data)
} else {
Err(RoverClientError::MalformedResponse {
service_data
.upload_schema
.ok_or(RoverClientError::MalformedResponse {
null_field: "service.upload_schema".to_string(),
})
}
}

fn build_response(
Expand Down
17 changes: 8 additions & 9 deletions crates/rover-client/src/query/subgraph/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,9 @@ fn get_delete_data_from_response(
response_data: delete_service_mutation::ResponseData,
graph: String,
) -> Result<RawMutationResponse, RoverClientError> {
let service_data = match response_data.service {
Some(data) => Ok(data),
None => Err(RoverClientError::NoService { graph }),
}?;
let service_data = response_data
.service
.ok_or(RoverClientError::NoService { graph })?;

Ok(service_data.remove_implementing_service_and_trigger_composition)
}
Expand Down Expand Up @@ -76,7 +75,7 @@ mod tests {
use super::*;
use serde_json::json;

type RawCompositionErrrors = delete_service_mutation::DeleteServiceMutationServiceRemoveImplementingServiceAndTriggerCompositionErrors;
type RawCompositionErrors = delete_service_mutation::DeleteServiceMutationServiceRemoveImplementingServiceAndTriggerCompositionErrors;

#[test]
fn get_delete_data_from_response_works() {
Expand All @@ -100,11 +99,11 @@ mod tests {

let expected_response = RawMutationResponse {
errors: vec![
Some(RawCompositionErrrors {
Some(RawCompositionErrors {
message: "wow".to_string(),
}),
None,
Some(RawCompositionErrrors {
Some(RawCompositionErrors {
message: "boo".to_string(),
}),
],
Expand All @@ -117,11 +116,11 @@ mod tests {
fn build_response_works_with_successful_responses() {
let response = RawMutationResponse {
errors: vec![
Some(RawCompositionErrrors {
Some(RawCompositionErrors {
message: "wow".to_string(),
}),
None,
Some(RawCompositionErrrors {
Some(RawCompositionErrors {
message: "boo".to_string(),
}),
],
Expand Down
9 changes: 3 additions & 6 deletions crates/rover-client/src/query/subgraph/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,9 @@ fn get_services_from_response_data(
response_data: fetch_subgraph_query::ResponseData,
graph: String,
) -> Result<ServiceList, RoverClientError> {
let service_data = match response_data.service {
Some(data) => Ok(data),
None => Err(RoverClientError::NoService {
graph: graph.clone(),
}),
}?;
let service_data = response_data.service.ok_or(RoverClientError::NoService {
graph: graph.clone(),
})?;

// get list of services
let services = match service_data.implementing_services {
Expand Down
11 changes: 4 additions & 7 deletions crates/rover-client/src/query/subgraph/introspect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,11 @@ pub fn run(
}

fn build_response(
response: introspection_query::ResponseData,
data: introspection_query::ResponseData,
) -> Result<IntrospectionResponse, RoverClientError> {
let service_data = match response.service {
Some(data) => Ok(data),
None => Err(RoverClientError::IntrospectionError {
msg: "No introspection response available.".to_string(),
}),
}?;
let service_data = data.service.ok_or(RoverClientError::IntrospectionError {
msg: "No introspection response available.".to_string(),
})?;

Ok(IntrospectionResponse {
result: service_data.sdl,
Expand Down
9 changes: 3 additions & 6 deletions crates/rover-client/src/query/subgraph/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,9 @@ fn get_subgraphs_from_response_data(
response_data: list_subgraphs_query::ResponseData,
graph: String,
) -> Result<Vec<RawSubgraphInfo>, RoverClientError> {
let service_data = match response_data.service {
Some(data) => Ok(data),
None => Err(RoverClientError::NoService {
graph: graph.clone(),
}),
}?;
let service_data = response_data.service.ok_or(RoverClientError::NoService {
graph: graph.clone(),
})?;

// get list of services
let services = match service_data.implementing_services {
Expand Down
5 changes: 1 addition & 4 deletions crates/rover-client/src/query/subgraph/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,7 @@ fn get_publish_response_from_data(
data: publish_partial_schema_mutation::ResponseData,
graph: String,
) -> Result<UpdateResponse, RoverClientError> {
let service_data = match data.service {
Some(data) => data,
None => return Err(RoverClientError::NoService { graph }),
};
let service_data = data.service.ok_or(RoverClientError::NoService { graph })?;

Ok(service_data.upsert_implementing_service_and_trigger_composition)
}
Expand Down
9 changes: 3 additions & 6 deletions crates/rover-client/src/query/supergraph/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,9 @@ fn get_supergraph_sdl_from_response_data(
graph: String,
variant: String,
) -> Result<String, RoverClientError> {
let service_data = match response_data.service {
Some(data) => Ok(data),
None => Err(RoverClientError::NoService {
graph: graph.clone(),
}),
}?;
let service_data = response_data.service.ok_or(RoverClientError::NoService {
graph: graph.clone(),
})?;

if let Some(schema_tag) = service_data.schema_tag {
if let Some(composition_result) = schema_tag.composition_result {
Expand Down
13 changes: 2 additions & 11 deletions src/command/subgraph/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use ansi_term::Colour::Red;
use serde::Serialize;
use structopt::StructOpt;

use crate::{anyhow, error::RoverError, Result, Suggestion};
use rover_client::query::{config::is_federated, subgraph::check};

use crate::command::RoverStdout;
Expand All @@ -14,6 +13,7 @@ use crate::utils::parsers::{
parse_schema_source, parse_validation_period, GraphRef, SchemaSource, ValidationPeriod,
};
use crate::utils::table::{self, cell, row};
use crate::Result;

#[derive(Debug, Serialize, StructOpt)]
pub struct Check {
Expand Down Expand Up @@ -66,23 +66,14 @@ impl Check {
let sdl = load_schema_from_flag(&self.schema, std::io::stdin())?;

// This response is used to check whether or not the current graph is federated.
let federated_response = is_federated::run(
is_federated::run(
is_federated::is_federated_graph::Variables {
graph_id: self.graph.name.clone(),
graph_variant: self.graph.variant.clone(),
},
&client,
)?;

// We don't want to run subgraph check on a non-federated graph, so
// return an error and recommend running graph check instead.
if !federated_response.result {
let err = anyhow!("Not able to run subgraph check on a non-federated graph.");
let mut err = RoverError::new(err);
err.set_suggestion(Suggestion::UseFederatedGraph);
return Err(err);
}

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
Expand Down
44 changes: 20 additions & 24 deletions src/command/subgraph/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,14 @@ use ansi_term::Colour::{Cyan, Red, Yellow};
use serde::Serialize;
use structopt::StructOpt;

use crate::{anyhow, Result};
use crate::{command::RoverStdout, error::RoverError};
use crate::{
utils::{
client::StudioClientConfig,
git::GitContext,
loaders::load_schema_from_flag,
parsers::{parse_graph_ref, parse_schema_source, GraphRef, SchemaSource},
},
Suggestion,
use crate::utils::{
client::StudioClientConfig,
git::GitContext,
loaders::load_schema_from_flag,
parsers::{parse_graph_ref, parse_schema_source, GraphRef, SchemaSource},
};
use crate::Result;
use crate::{command::RoverStdout, error::RoverError, Suggestion};

use rover_client::query::{
config::is_federated,
Expand Down Expand Up @@ -74,23 +71,22 @@ impl Publish {

tracing::debug!("Publishing \n{}", &schema_document);

// This response is used to check whether or not the current graph is federated.
let federated_response = is_federated::run(
is_federated::is_federated_graph::Variables {
graph_id: self.graph.name.clone(),
graph_variant: self.graph.variant.clone(),
},
&client,
)?;

// We don't want to implicitly convert non-federated graph to subgraphs.
// Error here if no --convert flag is passed _and_ the current context
// is non-federated. Add a suggestion to require a --convert flag.
if !federated_response.result && !self.convert {
let err = anyhow!("Could not publish a subgraph to a non-federated graph.");
let mut err = RoverError::new(err);
err.set_suggestion(Suggestion::ConvertGraphToSubgraph);
return Err(err);
if !self.convert {
is_federated::run(
is_federated::is_federated_graph::Variables {
graph_id: self.graph.name.clone(),
graph_variant: self.graph.variant.clone(),
},
&client,
)
.map_err(|e| {
let mut error = RoverError::from(e);
error.set_suggestion(Suggestion::ConvertGraphToSubgraph);
error
})?;
}

let publish_response = publish::run(
Expand Down
2 changes: 1 addition & 1 deletion src/error/metadata/suggestion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl Display for Suggestion {
format!("Try resolving the composition errors in your subgraph(s), and publish them with the {} command.", Yellow.normal().paint("`rover subgraph publish`"))
}
Suggestion::UseFederatedGraph => {
"Try running the command on a valid federated graph or use the appropriate `rover graph` command instead of `rover subgraph`.".to_string()
"Try running the command on a valid federated graph, or use the appropriate `rover graph` command instead of `rover subgraph`.".to_string()
}
Suggestion::CheckGraphNameAndAuth => {
"Make sure your graph name is typed correctly, and that your API key is valid. (Are you using the right profile?)".to_string()
Expand Down

0 comments on commit ab4a4c5

Please sign in to comment.