Skip to content

Commit f4dcaf6

Browse files
fix: removes unwrap from is_federated query
1 parent a9c81d5 commit f4dcaf6

15 files changed

Lines changed: 132 additions & 137 deletions

File tree

crates/rover-client/src/error.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,16 @@ pub enum RoverClientError {
109109
#[error("The response from Apollo Studio was malformed. Response body contains `null` value for \"{null_field}\"")]
110110
MalformedResponse { null_field: String },
111111

112-
#[error("The graph `{graph}` is a non-federated graph. This operation is only possible for federated graphs")]
113-
ExpectedFederatedGraph { graph: String },
112+
/// This error occurs when an operation expected a federated graph but a non-federated
113+
/// graph was supplied.
114+
/// `can_operation_convert` is only set to true when a non-federated graph
115+
/// was encountered during an operation that could potentially convert a non-federated graph
116+
/// to a federated graph.
117+
#[error("The graph `{graph}` is a non-federated graph. This operation is only possible for federated graphs.")]
118+
ExpectedFederatedGraph {
119+
graph: String,
120+
can_operation_convert: bool,
121+
},
114122

115123
/// The API returned an invalid ChangeSeverity value
116124
#[error("Invalid ChangeSeverity.")]

crates/rover-client/src/query/config/is_federated.rs

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,35 +15,31 @@ use graphql_client::*;
1515
/// This struct is used to generate the module containing `Variables` and
1616
/// `ResponseData` structs.
1717
/// Snake case of this name is the mod name. i.e. publish_partial_schema_mutation
18-
pub struct IsFederatedGraph;
18+
pub(crate) struct IsFederatedGraph;
1919

20-
#[derive(Debug, PartialEq)]
21-
pub struct IsFederatedGraphResponse {
22-
pub result: bool,
23-
}
24-
25-
pub fn run(
20+
pub(crate) fn run(
2621
variables: is_federated_graph::Variables,
2722
client: &StudioClient,
28-
) -> Result<IsFederatedGraphResponse, RoverClientError> {
23+
) -> Result<bool, RoverClientError> {
24+
let graph = variables.graph_id.clone();
2925
let data = client.post::<IsFederatedGraph>(variables)?;
30-
let is_federated_response = data.service.unwrap();
31-
Ok(build_response(is_federated_response))
26+
build_response(data, graph)
3227
}
3328

34-
type FederatedResponse = is_federated_graph::IsFederatedGraphService;
3529
type ImplementingServices = is_federated_graph::IsFederatedGraphServiceImplementingServices;
3630

37-
fn build_response(service: FederatedResponse) -> IsFederatedGraphResponse {
31+
fn build_response(
32+
data: is_federated_graph::ResponseData,
33+
graph: String,
34+
) -> Result<bool, RoverClientError> {
35+
let service = data.service.ok_or(RoverClientError::NoService { graph })?;
3836
match service.implementing_services {
39-
Some(typename) => match typename {
40-
ImplementingServices::FederatedImplementingServices => {
41-
IsFederatedGraphResponse { result: true }
42-
}
43-
ImplementingServices::NonFederatedImplementingService => {
44-
IsFederatedGraphResponse { result: false }
45-
}
46-
},
47-
None => IsFederatedGraphResponse { result: false },
37+
Some(typename) => Ok(match typename {
38+
ImplementingServices::FederatedImplementingServices => true,
39+
ImplementingServices::NonFederatedImplementingService => false,
40+
}),
41+
None => Err(RoverClientError::MalformedResponse {
42+
null_field: "implementing_services".to_string(),
43+
}),
4844
}
4945
}

crates/rover-client/src/query/graph/fetch.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,9 @@ fn get_schema_from_response_data(
4141
graph: String,
4242
invalid_variant: String,
4343
) -> Result<String, RoverClientError> {
44-
let service_data = match response_data.service {
45-
Some(data) => Ok(data),
46-
None => Err(RoverClientError::NoService {
47-
graph: graph.clone(),
48-
}),
49-
}?;
44+
let service_data = response_data.service.ok_or(RoverClientError::NoService {
45+
graph: graph.clone(),
46+
})?;
5047

5148
let mut valid_variants = Vec::new();
5249

crates/rover-client/src/query/graph/publish.rs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,13 @@ fn get_publish_response_from_data(
3939
graph: String,
4040
) -> Result<publish_schema_mutation::PublishSchemaMutationServiceUploadSchema, RoverClientError> {
4141
// then, from the response data, get .service?.upload_schema?
42-
let service_data = match data.service {
43-
Some(data) => data,
44-
None => return Err(RoverClientError::NoService { graph }),
45-
};
42+
let service_data = data.service.ok_or(RoverClientError::NoService { graph })?;
4643

47-
if let Some(opt_data) = service_data.upload_schema {
48-
Ok(opt_data)
49-
} else {
50-
Err(RoverClientError::MalformedResponse {
44+
service_data
45+
.upload_schema
46+
.ok_or(RoverClientError::MalformedResponse {
5147
null_field: "service.upload_schema".to_string(),
5248
})
53-
}
5449
}
5550

5651
fn build_response(

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
use crate::blocking::StudioClient;
2+
use crate::query::config::is_federated;
23
use crate::RoverClientError;
4+
35
use graphql_client::*;
46

57
use reqwest::Url;
@@ -27,6 +29,20 @@ pub fn run(
2729
client: &StudioClient,
2830
) -> Result<CheckResponse, RoverClientError> {
2931
let graph = variables.graph_id.clone();
32+
// This response is used to check whether or not the current graph is federated.
33+
let is_federated = is_federated::run(
34+
is_federated::is_federated_graph::Variables {
35+
graph_id: variables.graph_id.clone(),
36+
graph_variant: variables.variant.clone(),
37+
},
38+
&client,
39+
)?;
40+
if is_federated {
41+
return Err(RoverClientError::ExpectedFederatedGraph {
42+
graph,
43+
can_operation_convert: false,
44+
});
45+
}
3046
let data = client.post::<CheckPartialSchemaQuery>(variables)?;
3147
get_check_response_from_data(data, graph)
3248
}

crates/rover-client/src/query/subgraph/delete.rs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,10 +43,9 @@ fn get_delete_data_from_response(
4343
response_data: delete_service_mutation::ResponseData,
4444
graph: String,
4545
) -> Result<RawMutationResponse, RoverClientError> {
46-
let service_data = match response_data.service {
47-
Some(data) => Ok(data),
48-
None => Err(RoverClientError::NoService { graph }),
49-
}?;
46+
let service_data = response_data
47+
.service
48+
.ok_or(RoverClientError::NoService { graph })?;
5049

5150
Ok(service_data.remove_implementing_service_and_trigger_composition)
5251
}
@@ -76,7 +75,7 @@ mod tests {
7675
use super::*;
7776
use serde_json::json;
7877

79-
type RawCompositionErrrors = delete_service_mutation::DeleteServiceMutationServiceRemoveImplementingServiceAndTriggerCompositionErrors;
78+
type RawCompositionErrors = delete_service_mutation::DeleteServiceMutationServiceRemoveImplementingServiceAndTriggerCompositionErrors;
8079

8180
#[test]
8281
fn get_delete_data_from_response_works() {
@@ -100,11 +99,11 @@ mod tests {
10099

101100
let expected_response = RawMutationResponse {
102101
errors: vec![
103-
Some(RawCompositionErrrors {
102+
Some(RawCompositionErrors {
104103
message: "wow".to_string(),
105104
}),
106105
None,
107-
Some(RawCompositionErrrors {
106+
Some(RawCompositionErrors {
108107
message: "boo".to_string(),
109108
}),
110109
],
@@ -117,11 +116,11 @@ mod tests {
117116
fn build_response_works_with_successful_responses() {
118117
let response = RawMutationResponse {
119118
errors: vec![
120-
Some(RawCompositionErrrors {
119+
Some(RawCompositionErrors {
121120
message: "wow".to_string(),
122121
}),
123122
None,
124-
Some(RawCompositionErrrors {
123+
Some(RawCompositionErrors {
125124
message: "boo".to_string(),
126125
}),
127126
],

crates/rover-client/src/query/subgraph/fetch.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,9 @@ fn get_services_from_response_data(
3636
response_data: fetch_subgraph_query::ResponseData,
3737
graph: String,
3838
) -> Result<ServiceList, RoverClientError> {
39-
let service_data = match response_data.service {
40-
Some(data) => Ok(data),
41-
None => Err(RoverClientError::NoService {
42-
graph: graph.clone(),
43-
}),
44-
}?;
39+
let service_data = response_data.service.ok_or(RoverClientError::NoService {
40+
graph: graph.clone(),
41+
})?;
4542

4643
// get list of services
4744
let services = match service_data.implementing_services {
@@ -52,6 +49,7 @@ fn get_services_from_response_data(
5249
// wont' for long. Check on this later (Jake) :)
5350
None => Err(RoverClientError::ExpectedFederatedGraph {
5451
graph: graph.clone(),
52+
can_operation_convert: false,
5553
}),
5654
}?;
5755

@@ -60,7 +58,7 @@ fn get_services_from_response_data(
6058
Ok(services.services)
6159
},
6260
fetch_subgraph_query::FetchSubgraphQueryServiceImplementingServices::NonFederatedImplementingService => {
63-
Err(RoverClientError::ExpectedFederatedGraph { graph })
61+
Err(RoverClientError::ExpectedFederatedGraph { graph, can_operation_convert: false })
6462
}
6563
}
6664
}

crates/rover-client/src/query/subgraph/introspect.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,11 @@ pub fn run(
4141
}
4242

4343
fn build_response(
44-
response: introspection_query::ResponseData,
44+
data: introspection_query::ResponseData,
4545
) -> Result<IntrospectionResponse, RoverClientError> {
46-
let service_data = match response.service {
47-
Some(data) => Ok(data),
48-
None => Err(RoverClientError::IntrospectionError {
49-
msg: "No introspection response available.".to_string(),
50-
}),
51-
}?;
46+
let service_data = data.service.ok_or(RoverClientError::IntrospectionError {
47+
msg: "No introspection response available.".to_string(),
48+
})?;
5249

5350
Ok(IntrospectionResponse {
5451
result: service_data.sdl,

crates/rover-client/src/query/subgraph/list.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,9 @@ fn get_subgraphs_from_response_data(
5454
response_data: list_subgraphs_query::ResponseData,
5555
graph: String,
5656
) -> Result<Vec<RawSubgraphInfo>, RoverClientError> {
57-
let service_data = match response_data.service {
58-
Some(data) => Ok(data),
59-
None => Err(RoverClientError::NoService {
60-
graph: graph.clone(),
61-
}),
62-
}?;
57+
let service_data = response_data.service.ok_or(RoverClientError::NoService {
58+
graph: graph.clone(),
59+
})?;
6360

6461
// get list of services
6562
let services = match service_data.implementing_services {
@@ -79,7 +76,7 @@ fn get_subgraphs_from_response_data(
7976
Ok(services.services)
8077
},
8178
list_subgraphs_query::ListSubgraphsQueryServiceImplementingServices::NonFederatedImplementingService => {
82-
Err(RoverClientError::ExpectedFederatedGraph { graph })
79+
Err(RoverClientError::ExpectedFederatedGraph { graph, can_operation_convert: false })
8380
}
8481
}
8582
}

crates/rover-client/src/query/subgraph/publish.rs

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
// PublishPartialSchemaMutation
22
use crate::blocking::StudioClient;
3+
use crate::query::config::is_federated;
34
use crate::RoverClientError;
45
use graphql_client::*;
56

@@ -28,8 +29,28 @@ pub struct PublishPartialSchemaResponse {
2829
pub fn run(
2930
variables: publish_partial_schema_mutation::Variables,
3031
client: &StudioClient,
32+
convert_to_federated_graph: bool,
3133
) -> Result<PublishPartialSchemaResponse, RoverClientError> {
3234
let graph = variables.graph_id.clone();
35+
// We don't want to implicitly convert non-federated graph to supergraphs.
36+
// Error here if no --convert flag is passed _and_ the current context
37+
// is non-federated. Add a suggestion to require a --convert flag.
38+
if !convert_to_federated_graph {
39+
let is_federated = is_federated::run(
40+
is_federated::is_federated_graph::Variables {
41+
graph_id: variables.graph_id.clone(),
42+
graph_variant: variables.graph_variant.clone(),
43+
},
44+
&client,
45+
)?;
46+
47+
if !is_federated {
48+
return Err(RoverClientError::ExpectedFederatedGraph {
49+
graph,
50+
can_operation_convert: true,
51+
});
52+
}
53+
}
3354
let data = client.post::<PublishPartialSchemaMutation>(variables)?;
3455
let publish_response = get_publish_response_from_data(data, graph)?;
3556
Ok(build_response(publish_response))
@@ -42,10 +63,7 @@ fn get_publish_response_from_data(
4263
data: publish_partial_schema_mutation::ResponseData,
4364
graph: String,
4465
) -> Result<UpdateResponse, RoverClientError> {
45-
let service_data = match data.service {
46-
Some(data) => data,
47-
None => return Err(RoverClientError::NoService { graph }),
48-
};
66+
let service_data = data.service.ok_or(RoverClientError::NoService { graph })?;
4967

5068
Ok(service_data.upsert_implementing_service_and_trigger_composition)
5169
}

0 commit comments

Comments
 (0)