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

chore: refactor subgraph publish #630

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
13 changes: 13 additions & 0 deletions crates/rover-client/src/query/subgraph/check/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ type QueryVariables = subgraph_check_query::Variables;
type QueryChangeSeverity = subgraph_check_query::ChangeSeverity;
type QuerySchema = subgraph_check_query::PartialSchemaInput;
type QueryConfig = subgraph_check_query::HistoricQueryParameters;
type GitContextInput = subgraph_check_query::GitContextInput;

#[derive(Debug, Clone, PartialEq)]
pub struct SubgraphCheckInput {
Expand Down Expand Up @@ -99,3 +100,15 @@ pub struct CompositionError {
pub message: String,
pub code: Option<String>,
}

impl From<GitContext> for GitContextInput {
fn from(git_context: GitContext) -> GitContextInput {
GitContextInput {
branch: git_context.branch,
commit: git_context.commit,
committer: git_context.author,
remote_url: git_context.remote_url,
message: None,
}
}
}
3 changes: 3 additions & 0 deletions crates/rover-client/src/query/subgraph/publish/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pub mod mutation_runner;
pub(crate) mod types;
pub use types::{SubgraphPublishInput, SubgraphPublishResponse};
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// PublishPartialSchemaMutation
use super::types::*;
use crate::blocking::StudioClient;
use crate::query::config::is_federated;
use crate::RoverClientError;
Expand All @@ -8,38 +8,30 @@ use graphql_client::*;
// The paths are relative to the directory where your `Cargo.toml` is located.
// Both json and the GraphQL schema language are supported as sources for the schema
#[graphql(
query_path = "src/query/subgraph/publish.graphql",
query_path = "src/query/subgraph/publish/publish_mutation.graphql",
schema_path = ".schema/schema.graphql",
response_derives = "PartialEq, Debug, Serialize, Deserialize",
deprecated = "warn"
)]
/// This struct is used to generate the module containing `Variables` and
/// `ResponseData` structs.
/// Snake case of this name is the mod name. i.e. publish_partial_schema_mutation
pub struct PublishPartialSchemaMutation;

#[derive(Debug, PartialEq)]
pub struct PublishPartialSchemaResponse {
pub schema_hash: Option<String>,
pub did_update_gateway: bool,
pub service_was_created: bool,
pub composition_errors: Option<Vec<String>>,
}
/// Snake case of this name is the mod name. i.e. subgraph_publish_mutation
pub struct SubgraphPublishMutation;

pub fn run(
variables: publish_partial_schema_mutation::Variables,
input: SubgraphPublishInput,
client: &StudioClient,
convert_to_federated_graph: bool,
) -> Result<PublishPartialSchemaResponse, RoverClientError> {
let graph = variables.graph_id.clone();
) -> Result<SubgraphPublishResponse, RoverClientError> {
let variables: MutationVariables = input.clone().into();
let graph = input.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 {
if !input.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(),
graph_id: input.graph_id.clone(),
graph_variant: input.variant,
},
&client,
)?;
Expand All @@ -51,24 +43,21 @@ pub fn run(
});
}
}
let data = client.post::<PublishPartialSchemaMutation>(variables)?;
let data = client.post::<SubgraphPublishMutation>(variables)?;
let publish_response = get_publish_response_from_data(data, graph)?;
Ok(build_response(publish_response))
}

// alias this return type since it's disgusting
type UpdateResponse = publish_partial_schema_mutation::PublishPartialSchemaMutationServiceUpsertImplementingServiceAndTriggerComposition;

fn get_publish_response_from_data(
data: publish_partial_schema_mutation::ResponseData,
data: ResponseData,
graph: String,
) -> Result<UpdateResponse, RoverClientError> {
let service_data = data.service.ok_or(RoverClientError::NoService { graph })?;

Ok(service_data.upsert_implementing_service_and_trigger_composition)
}

fn build_response(publish_response: UpdateResponse) -> PublishPartialSchemaResponse {
fn build_response(publish_response: UpdateResponse) -> SubgraphPublishResponse {
let composition_errors: Vec<String> = publish_response
.errors
.iter()
Expand All @@ -82,7 +71,7 @@ fn build_response(publish_response: UpdateResponse) -> PublishPartialSchemaRespo
None
};

PublishPartialSchemaResponse {
SubgraphPublishResponse {
schema_hash: match publish_response.composition_config {
Some(config) => Some(config.schema_hash),
None => None,
Expand Down Expand Up @@ -114,7 +103,7 @@ mod tests {

assert_eq!(
output,
PublishPartialSchemaResponse {
SubgraphPublishResponse {
schema_hash: Some("5gf564".to_string()),
composition_errors: Some(vec![
"[Accounts] User -> composition error".to_string(),
Expand All @@ -139,7 +128,7 @@ mod tests {

assert_eq!(
output,
PublishPartialSchemaResponse {
SubgraphPublishResponse {
schema_hash: Some("5gf564".to_string()),
composition_errors: None,
did_update_gateway: true,
Expand All @@ -163,7 +152,7 @@ mod tests {

assert_eq!(
output,
PublishPartialSchemaResponse {
SubgraphPublishResponse {
schema_hash: None,
composition_errors: Some(
vec!["[Accounts] -> Things went really wrong".to_string()]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
mutation PublishPartialSchemaMutation(
mutation SubgraphPublishMutation(
$graphId: ID!
$graphVariant: String!
$name: String!
$variant: String!
$subgraph: String!
$url: String
$revision: String!
$activePartialSchema: PartialSchemaInput!
$schema: PartialSchemaInput!
$gitContext: GitContextInput!
) {
service(id: $graphId) {
upsertImplementingServiceAndTriggerComposition(
name: $name
name: $subgraph
url: $url
revision: $revision
activePartialSchema: $activePartialSchema
graphVariant: $graphVariant
activePartialSchema: $schema
graphVariant: $variant
gitContext: $gitContext
) {
compositionConfig {
Expand Down
58 changes: 58 additions & 0 deletions crates/rover-client/src/query/subgraph/publish/types.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
use super::mutation_runner::subgraph_publish_mutation;

use crate::utils::GitContext;

pub(crate) type ResponseData = subgraph_publish_mutation::ResponseData;
pub(crate) type MutationVariables = subgraph_publish_mutation::Variables;
pub(crate) type UpdateResponse = subgraph_publish_mutation::SubgraphPublishMutationServiceUpsertImplementingServiceAndTriggerComposition;

type SchemaInput = subgraph_publish_mutation::PartialSchemaInput;
type GitContextInput = subgraph_publish_mutation::GitContextInput;

#[derive(Debug, Clone, PartialEq)]
pub struct SubgraphPublishInput {
pub graph_id: String,
pub variant: String,
pub subgraph: String,
pub url: Option<String>,
pub schema: String,
pub git_context: GitContext,
pub convert_to_federated_graph: bool,
}

#[derive(Debug, PartialEq)]
pub struct SubgraphPublishResponse {
pub schema_hash: Option<String>,
pub did_update_gateway: bool,
pub service_was_created: bool,
pub composition_errors: Option<Vec<String>>,
}

impl From<SubgraphPublishInput> for MutationVariables {
fn from(publish_input: SubgraphPublishInput) -> Self {
Self {
graph_id: publish_input.graph_id,
variant: publish_input.variant,
subgraph: publish_input.subgraph,
url: publish_input.url,
schema: SchemaInput {
sdl: Some(publish_input.schema),
hash: None,
},
git_context: publish_input.git_context.into(),
revision: "".to_string(),
}
}
}

impl From<GitContext> for GitContextInput {
fn from(git_context: GitContext) -> GitContextInput {
GitContextInput {
branch: git_context.branch,
commit: git_context.commit,
committer: git_context.author,
remote_url: git_context.remote_url,
message: None,
}
}
}
30 changes: 1 addition & 29 deletions crates/rover-client/src/utils/git.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::query::{graph, subgraph};
use crate::query::graph;

use std::env;

Expand Down Expand Up @@ -163,34 +163,6 @@ impl From<GitContext> for GraphCheckContextInput {
}
}

type SubgraphPublishContextInput =
subgraph::publish::publish_partial_schema_mutation::GitContextInput;
impl From<GitContext> for SubgraphPublishContextInput {
fn from(git_context: GitContext) -> SubgraphPublishContextInput {
SubgraphPublishContextInput {
branch: git_context.branch,
commit: git_context.commit,
committer: git_context.author,
remote_url: git_context.remote_url,
message: None,
}
}
}

type SubgraphCheckContextInput =
subgraph::check::query_runner::subgraph_check_query::GitContextInput;
impl From<GitContext> for SubgraphCheckContextInput {
fn from(git_context: GitContext) -> SubgraphCheckContextInput {
SubgraphCheckContextInput {
branch: git_context.branch,
commit: git_context.commit,
committer: git_context.author,
remote_url: git_context.remote_url,
message: None,
}
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
33 changes: 14 additions & 19 deletions src/command/subgraph/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::utils::{
};
use crate::Result;

use rover_client::query::subgraph::publish::{self, PublishPartialSchemaResponse};
use rover_client::query::subgraph::publish::{self, SubgraphPublishInput, SubgraphPublishResponse};
use rover_client::utils::GitContext;

#[derive(Debug, Serialize, StructOpt)]
Expand Down Expand Up @@ -64,34 +64,29 @@ impl Publish {
Yellow.normal().paint(&self.profile_name)
);

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

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

let publish_response = publish::run(
publish::publish_partial_schema_mutation::Variables {
let publish_response = publish::mutation_runner::run(
SubgraphPublishInput {
graph_id: self.graph.name.clone(),
graph_variant: self.graph.variant.clone(),
name: self.subgraph.clone(),
active_partial_schema:
publish::publish_partial_schema_mutation::PartialSchemaInput {
sdl: Some(schema_document),
hash: None,
},
revision: "".to_string(),
variant: self.graph.variant.clone(),
subgraph: self.subgraph.clone(),
url: self.routing_url.clone(),
git_context: git_context.into(),
schema,
git_context,
convert_to_federated_graph: self.convert,
},
&client,
self.convert,
)?;

handle_publish_response(publish_response, &self.subgraph, &self.graph.name);
Ok(RoverStdout::None)
}
}

fn handle_publish_response(response: PublishPartialSchemaResponse, subgraph: &str, graph: &str) {
fn handle_publish_response(response: SubgraphPublishResponse, subgraph: &str, graph: &str) {
if response.service_was_created {
eprintln!(
"A new subgraph called '{}' for the '{}' graph was created",
Expand Down Expand Up @@ -125,13 +120,13 @@ fn handle_publish_response(response: PublishPartialSchemaResponse, subgraph: &st

#[cfg(test)]
mod tests {
use super::{handle_publish_response, PublishPartialSchemaResponse};
use super::{handle_publish_response, SubgraphPublishResponse};

// this test is a bit weird, since we can't test the output. We just verify it
// doesn't error
#[test]
fn handle_response_doesnt_error_with_all_successes() {
let response = PublishPartialSchemaResponse {
let response = SubgraphPublishResponse {
schema_hash: Some("123456".to_string()),
did_update_gateway: true,
service_was_created: true,
Expand All @@ -143,7 +138,7 @@ mod tests {

#[test]
fn handle_response_doesnt_error_with_all_failures() {
let response = PublishPartialSchemaResponse {
let response = SubgraphPublishResponse {
schema_hash: None,
did_update_gateway: false,
service_was_created: false,
Expand Down