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

fix: removes unwrap from is_federated query #549

Merged
merged 1 commit into from
May 19, 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
12 changes: 10 additions & 2 deletions crates/rover-client/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,16 @@ 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")]
ExpectedFederatedGraph { graph: String },
/// This error occurs when an operation expected a federated graph but a non-federated
/// graph was supplied.
/// `can_operation_convert` is only set to true when a non-federated graph
/// was encountered during an operation that could potentially convert a non-federated graph
/// to a federated graph.
#[error("The graph `{graph}` is a non-federated graph. This operation is only possible for federated graphs.")]
ExpectedFederatedGraph {
graph: String,
can_operation_convert: bool,
},

/// The API returned an invalid ChangeSeverity value
#[error("Invalid ChangeSeverity.")]
Expand Down
38 changes: 17 additions & 21 deletions crates/rover-client/src/query/config/is_federated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,35 +15,31 @@ use graphql_client::*;
/// This struct is used to generate the module containing `Variables` and
/// `ResponseData` structs.
/// Snake case of this name is the mod name. i.e. publish_partial_schema_mutation
pub struct IsFederatedGraph;
pub(crate) struct IsFederatedGraph;

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

pub fn run(
pub(crate) fn run(
variables: is_federated_graph::Variables,
client: &StudioClient,
) -> Result<IsFederatedGraphResponse, RoverClientError> {
) -> Result<bool, 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<bool, RoverClientError> {
let service = data.service.ok_or(RoverClientError::NoService { graph })?;
match service.implementing_services {
Some(typename) => match typename {
ImplementingServices::FederatedImplementingServices => {
IsFederatedGraphResponse { result: true }
}
ImplementingServices::NonFederatedImplementingService => {
IsFederatedGraphResponse { result: false }
}
},
None => IsFederatedGraphResponse { result: false },
Some(typename) => Ok(match typename {
ImplementingServices::FederatedImplementingServices => true,
ImplementingServices::NonFederatedImplementingService => false,
}),
None => Err(RoverClientError::MalformedResponse {
null_field: "implementing_services".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 })?;
EverlastingBugstopper marked this conversation as resolved.
Show resolved Hide resolved

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
16 changes: 16 additions & 0 deletions crates/rover-client/src/query/subgraph/check.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::blocking::StudioClient;
use crate::query::config::is_federated;
use crate::RoverClientError;

use graphql_client::*;

use reqwest::Url;
Expand Down Expand Up @@ -27,6 +29,20 @@ pub fn run(
client: &StudioClient,
) -> Result<CheckResponse, 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(
is_federated::is_federated_graph::Variables {
graph_id: variables.graph_id.clone(),
graph_variant: variables.variant.clone(),
},
&client,
)?;
if is_federated {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should be !is_federated

return Err(RoverClientError::ExpectedFederatedGraph {
graph,
can_operation_convert: false,
});
}
let data = client.post::<CheckPartialSchemaQuery>(variables)?;
get_check_response_from_data(data, graph)
}
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
12 changes: 5 additions & 7 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 All @@ -52,6 +49,7 @@ fn get_services_from_response_data(
// wont' for long. Check on this later (Jake) :)
None => Err(RoverClientError::ExpectedFederatedGraph {
graph: graph.clone(),
can_operation_convert: false,
}),
}?;

Expand All @@ -60,7 +58,7 @@ fn get_services_from_response_data(
Ok(services.services)
},
fetch_subgraph_query::FetchSubgraphQueryServiceImplementingServices::NonFederatedImplementingService => {
Err(RoverClientError::ExpectedFederatedGraph { graph })
Err(RoverClientError::ExpectedFederatedGraph { graph, can_operation_convert: false })
}
}
}
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
11 changes: 4 additions & 7 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 All @@ -79,7 +76,7 @@ fn get_subgraphs_from_response_data(
Ok(services.services)
},
list_subgraphs_query::ListSubgraphsQueryServiceImplementingServices::NonFederatedImplementingService => {
Err(RoverClientError::ExpectedFederatedGraph { graph })
Err(RoverClientError::ExpectedFederatedGraph { graph, can_operation_convert: false })
}
}
}
Expand Down
26 changes: 22 additions & 4 deletions crates/rover-client/src/query/subgraph/publish.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// PublishPartialSchemaMutation
use crate::blocking::StudioClient;
use crate::query::config::is_federated;
use crate::RoverClientError;
use graphql_client::*;

Expand Down Expand Up @@ -28,8 +29,28 @@ pub struct PublishPartialSchemaResponse {
pub fn run(
variables: publish_partial_schema_mutation::Variables,
client: &StudioClient,
convert_to_federated_graph: bool,
) -> Result<PublishPartialSchemaResponse, RoverClientError> {
let graph = variables.graph_id.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 !convert_to_federated_graph {
let is_federated = is_federated::run(
is_federated::is_federated_graph::Variables {
graph_id: variables.graph_id.clone(),
graph_variant: variables.graph_variant.clone(),
},
&client,
)?;

if !is_federated {
return Err(RoverClientError::ExpectedFederatedGraph {
graph,
can_operation_convert: true,
});
}
}
let data = client.post::<PublishPartialSchemaMutation>(variables)?;
let publish_response = get_publish_response_from_data(data, graph)?;
Ok(build_response(publish_response))
Expand All @@ -42,10 +63,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
25 changes: 16 additions & 9 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 All @@ -54,7 +51,10 @@ fn get_supergraph_sdl_from_response_data(
})
}
} else {
Err(RoverClientError::ExpectedFederatedGraph { graph })
Err(RoverClientError::ExpectedFederatedGraph {
graph,
can_operation_convert: false,
})
}
} else if let Some(most_recent_composition_publish) =
service_data.most_recent_composition_publish
Expand Down Expand Up @@ -83,7 +83,10 @@ fn get_supergraph_sdl_from_response_data(
frontend_url_root: response_data.frontend_url_root,
})
} else {
Err(RoverClientError::ExpectedFederatedGraph { graph })
Err(RoverClientError::ExpectedFederatedGraph {
graph,
can_operation_convert: false,
})
}
}
}
Expand Down Expand Up @@ -213,7 +216,11 @@ mod tests {
let data: fetch_supergraph_query::ResponseData =
serde_json::from_value(json_response).unwrap();
let output = get_supergraph_sdl_from_response_data(data, graph.clone(), variant.clone());
let expected_error = RoverClientError::ExpectedFederatedGraph { graph }.to_string();
let expected_error = RoverClientError::ExpectedFederatedGraph {
graph,
can_operation_convert: false,
}
.to_string();
let actual_error = output.unwrap_err().to_string();
assert_eq!(actual_error, expected_error);
}
Expand Down
Loading