From 9d9b6d04b0243accfefe79e92762d4dea61025b7 Mon Sep 17 00:00:00 2001 From: jonathanrainer Date: Wed, 24 Jul 2024 14:08:32 +0100 Subject: [PATCH 1/8] ROVER-69 Unify supergraph_config parsing We do this in both supergraph commands and dev and having it in two places is not helping. --- src/command/dev/do_dev.rs | 53 +--- src/command/dev/mod.rs | 9 +- src/command/dev/remote_subgraphs.rs | 55 ---- src/command/supergraph/compose/do_compose.rs | 173 +++--------- src/command/supergraph/mod.rs | 11 +- src/utils/mod.rs | 2 + src/utils/parsers.rs | 24 +- .../supergraph_config.rs} | 255 +++++++++++++++++- 8 files changed, 321 insertions(+), 261 deletions(-) delete mode 100644 src/command/dev/remote_subgraphs.rs rename src/{command/supergraph/resolve_config.rs => utils/supergraph_config.rs} (60%) diff --git a/src/command/dev/do_dev.rs b/src/command/dev/do_dev.rs index 9d08c6bcd..c2fa58538 100644 --- a/src/command/dev/do_dev.rs +++ b/src/command/dev/do_dev.rs @@ -3,15 +3,14 @@ use apollo_federation_types::config::FederationVersion; use camino::Utf8PathBuf; use crossbeam_channel::bounded as sync_channel; -use rover_std::{Emoji, Fs}; +use rover_std::Emoji; use crate::command::dev::protocol::FollowerMessage; -use crate::command::supergraph::expand_supergraph_yaml; use crate::utils::client::StudioClientConfig; +use crate::utils::supergraph_config::get_supergraph_config; use crate::{RoverError, RoverOutput, RoverResult}; use super::protocol::{FollowerChannel, FollowerMessenger, LeaderChannel, LeaderSession}; -use super::remote_subgraphs::RemoteSubgraphs; use super::router::RouterConfigHandler; use super::Dev; @@ -36,42 +35,18 @@ impl Dev { let leader_channel = LeaderChannel::new(); let follower_channel = FollowerChannel::new(); - // Read in Remote subgraphs - let remote_subgraphs = match &self.opts.supergraph_opts.graph_ref { - Some(graph_ref) => Some(RemoteSubgraphs::fetch( - &client_config.get_authenticated_client(&self.opts.plugin_opts.profile)?, - &self - .opts - .supergraph_opts - .federation_version - .clone() - .unwrap_or(FederationVersion::LatestFedTwo), - graph_ref, - )?), - None => None, - }; - - // Read in Local Supergraph Config - let supergraph_config = - if let Some(config_path) = &self.opts.supergraph_opts.supergraph_config_path { - let config_content = Fs::read_file(config_path)?; - Some(expand_supergraph_yaml(&config_content)?) - } else { - None - }; - - // Merge Remote and Local Supergraph Configs - let supergraph_config = match remote_subgraphs { - Some(remote_subgraphs) => match supergraph_config { - Some(supergraph_config) => { - let mut merged_supergraph_config = remote_subgraphs.inner().clone(); - merged_supergraph_config.merge_subgraphs(&supergraph_config); - Some(merged_supergraph_config) - } - None => Some(remote_subgraphs.inner().clone()), - }, - None => supergraph_config, - }; + let supergraph_config = get_supergraph_config( + &self.opts.supergraph_opts.graph_ref, + &self.opts.supergraph_opts.supergraph_config_path, + &self + .opts + .supergraph_opts + .federation_version + .clone() + .unwrap_or(FederationVersion::LatestFedTwo), + client_config.clone(), + &self.opts.plugin_opts.profile, + )?; // Build a Rayon Thread pool let tp = rayon::ThreadPoolBuilder::new() diff --git a/src/command/dev/mod.rs b/src/command/dev/mod.rs index b057dc9ca..ecd129ea1 100644 --- a/src/command/dev/mod.rs +++ b/src/command/dev/mod.rs @@ -3,10 +3,12 @@ use std::net::IpAddr; use apollo_federation_types::config::FederationVersion; use camino::Utf8PathBuf; use clap::Parser; -use rover_client::shared::GraphRef; use serde::Serialize; +use rover_client::shared::GraphRef; + use crate::options::{OptionalSubgraphOpts, PluginOpts}; +use crate::utils::parsers::FileDescriptorType; #[cfg(feature = "composition-js")] mod compose; @@ -20,9 +22,6 @@ mod introspect; #[cfg(feature = "composition-js")] mod protocol; -#[cfg(feature = "composition-js")] -mod remote_subgraphs; - #[cfg(feature = "composition-js")] mod router; @@ -91,7 +90,7 @@ pub struct SupergraphOpts { long = "supergraph-config", conflicts_with_all = ["subgraph_name", "subgraph_url", "subgraph_schema_path"] )] - supergraph_config_path: Option, + supergraph_config_path: Option, /// A [`GraphRef`] that is accessible in Apollo Studio. /// This is used to initialize your supergraph with the values contained in this variant. diff --git a/src/command/dev/remote_subgraphs.rs b/src/command/dev/remote_subgraphs.rs deleted file mode 100644 index 1e585db74..000000000 --- a/src/command/dev/remote_subgraphs.rs +++ /dev/null @@ -1,55 +0,0 @@ -use apollo_federation_types::config::{ - FederationVersion, SchemaSource, SubgraphConfig, SupergraphConfig, -}; -use rover_client::{ - blocking::StudioClient, - operations::subgraph::{self, list::SubgraphListInput}, - shared::GraphRef, -}; - -use crate::RoverResult; - -/// Nominal type that captures the behavior of collecting remote subgraphs into a -/// [`SupergraphConfig`] representation -#[derive(Clone, Debug)] -pub struct RemoteSubgraphs(SupergraphConfig); - -impl RemoteSubgraphs { - /// Fetches [`RemoteSubgraphs`] from Studio - pub fn fetch( - client: &StudioClient, - federation_version: &FederationVersion, - graph_ref: &GraphRef, - ) -> RoverResult { - let subgraphs = subgraph::list::run( - SubgraphListInput { - graph_ref: graph_ref.clone(), - }, - client, - )?; - let subgraphs = subgraphs - .subgraphs - .iter() - .map(|subgraph| { - ( - subgraph.name.clone(), - SubgraphConfig { - routing_url: subgraph.url.clone(), - schema: SchemaSource::Subgraph { - graphref: graph_ref.clone().to_string(), - subgraph: subgraph.name.clone(), - }, - }, - ) - }) - .collect(); - let supergraph_config = SupergraphConfig::new(subgraphs, Some(federation_version.clone())); - let remote_subgraphs = RemoteSubgraphs(supergraph_config); - Ok(remote_subgraphs) - } - - /// Provides a reference to the inner value of this representation - pub fn inner(&self) -> &SupergraphConfig { - &self.0 - } -} diff --git a/src/command/supergraph/compose/do_compose.rs b/src/command/supergraph/compose/do_compose.rs index ac1da583f..e8de81524 100644 --- a/src/command/supergraph/compose/do_compose.rs +++ b/src/command/supergraph/compose/do_compose.rs @@ -1,6 +1,7 @@ use std::{fs::File, io::Write, process::Command, str}; use anyhow::{anyhow, Context}; +use apollo_federation_types::config::FederationVersion::LatestFedTwo; use apollo_federation_types::config::SupergraphConfig; use apollo_federation_types::{ build::BuildResult, @@ -10,10 +11,11 @@ use camino::Utf8PathBuf; use clap::Parser; use serde::Serialize; +use rover_client::shared::GraphRef; use rover_client::RoverClientError; use rover_std::{Emoji, Style}; -use crate::command::supergraph::resolve_supergraph_yaml; +use crate::utils::supergraph_config::get_supergraph_config; use crate::utils::{client::StudioClientConfig, parsers::FileDescriptorType}; use crate::{ command::{ @@ -24,22 +26,45 @@ use crate::{ RoverError, RoverErrorSuggestion, RoverOutput, RoverResult, }; -#[derive(Debug, Clone, Serialize, Parser)] +#[derive(Debug, Serialize, Parser)] pub struct Compose { + #[clap(flatten)] + opts: SupergraphComposeOpts, +} + +#[derive(Debug, Serialize, Parser)] +pub struct SupergraphComposeOpts { + #[clap(flatten)] + pub plugin_opts: PluginOpts, + /// The relative path to the supergraph configuration file. You can pass `-` to use stdin instead of a file. #[serde(skip_serializing)] #[arg(long = "config")] supergraph_yaml: FileDescriptorType, - #[clap(flatten)] - opts: PluginOpts, + /// A [`GraphRef`] that is accessible in Apollo Studio. + /// This is used to initialize your supergraph with the values contained in this variant. + /// + /// This is analogous to providing a supergraph.yaml file with references to your graph variant in studio. + /// + /// If used in conjunction with `--supergraph-config`, the values presented in the supergraph.yaml will take precedence over these values. + #[arg(long = "graph-ref")] + graph_ref: Option, + + /// The version of Apollo Federation to use for composition + #[arg(long = "federation-version")] + federation_version: Option, } impl Compose { pub fn new(compose_opts: PluginOpts) -> Self { Self { - supergraph_yaml: FileDescriptorType::File("RAM".into()), - opts: compose_opts, + opts: SupergraphComposeOpts { + plugin_opts: compose_opts, + supergraph_yaml: FileDescriptorType::File("RAM".into()), + graph_ref: None, + federation_version: Some(LatestFedTwo), + }, } } @@ -52,6 +77,7 @@ impl Compose { let plugin = Plugin::Supergraph(federation_version.clone()); if federation_version.is_fed_two() { self.opts + .plugin_opts .elv2_license_accepter .require_elv2_license(&client_config)?; } @@ -60,14 +86,14 @@ impl Compose { let install_command = Install { force: false, plugin: Some(plugin), - elv2_license_accepter: self.opts.elv2_license_accepter, + elv2_license_accepter: self.opts.plugin_opts.elv2_license_accepter, }; // maybe do the install, maybe find a pre-existing installation, maybe fail let plugin_exe = install_command.get_versioned_plugin( override_install_path, client_config, - self.opts.skip_update, + self.opts.plugin_opts.skip_update, )?; Ok(plugin_exe) } @@ -80,13 +106,16 @@ impl Compose { eprintln!( "{}resolving SDL for subgraphs defined in {}", Emoji::Hourglass, - Style::Path.paint(self.supergraph_yaml.to_string()) + Style::Path.paint(self.opts.supergraph_yaml.to_string()) ); - let mut supergraph_config = resolve_supergraph_yaml( - &self.supergraph_yaml, + let mut supergraph_config = get_supergraph_config( + &self.opts.graph_ref, + &Some(self.opts.supergraph_yaml.clone()), + &self.opts.federation_version.clone().unwrap_or(LatestFedTwo), client_config.clone(), - &self.opts.profile, - )?; + &self.opts.plugin_opts.profile, + )? + .unwrap(); self.compose(override_install_path, client_config, &mut supergraph_config) } @@ -187,130 +216,12 @@ impl Compose { #[cfg(test)] mod tests { use std::convert::TryFrom; - use std::fs; - use assert_fs::TempDir; use rstest::rstest; use speculoos::assert_that; - use houston as houston_config; - use houston_config::Config; - - use crate::options::ProfileOpt; - use crate::utils::client::ClientBuilder; - use super::*; - fn get_studio_config() -> StudioClientConfig { - let tmp_home = TempDir::new().unwrap(); - let tmp_path = Utf8PathBuf::try_from(tmp_home.path().to_path_buf()).unwrap(); - StudioClientConfig::new( - None, - Config::new(Some(&tmp_path), None).unwrap(), - false, - ClientBuilder::default(), - ) - } - - #[rstest] - fn it_errs_on_invalid_subgraph_path() { - let raw_good_yaml = r#"subgraphs: - films: - routing_url: https://films.example.com - schema: - file: ./films-do-not-exist.graphql - people: - routing_url: https://people.example.com - schema: - file: ./people-do-not-exist.graphql"#; - let tmp_home = TempDir::new().unwrap(); - let mut config_path = Utf8PathBuf::try_from(tmp_home.path().to_path_buf()).unwrap(); - config_path.push("config.yaml"); - fs::write(&config_path, raw_good_yaml).unwrap(); - assert!(resolve_supergraph_yaml( - &FileDescriptorType::File(config_path), - get_studio_config(), - &ProfileOpt { - profile_name: "profile".to_string() - } - ) - .is_err()) - } - - #[rstest] - fn it_can_get_subgraph_definitions_from_fs() { - let raw_good_yaml = r#"subgraphs: - films: - routing_url: https://films.example.com - schema: - file: ./films.graphql - people: - routing_url: https://people.example.com - schema: - file: ./people.graphql"#; - let tmp_home = TempDir::new().unwrap(); - let mut config_path = Utf8PathBuf::try_from(tmp_home.path().to_path_buf()).unwrap(); - config_path.push("config.yaml"); - fs::write(&config_path, raw_good_yaml).unwrap(); - let tmp_dir = config_path.parent().unwrap().to_path_buf(); - let films_path = tmp_dir.join("films.graphql"); - let people_path = tmp_dir.join("people.graphql"); - fs::write(films_path, "there is something here").unwrap(); - fs::write(people_path, "there is also something here").unwrap(); - assert!(resolve_supergraph_yaml( - &FileDescriptorType::File(config_path), - get_studio_config(), - &ProfileOpt { - profile_name: "profile".to_string() - } - ) - .is_ok()) - } - - #[rstest] - fn it_can_compute_relative_schema_paths() { - let raw_good_yaml = r#"subgraphs: - films: - routing_url: https://films.example.com - schema: - file: ../../films.graphql - people: - routing_url: https://people.example.com - schema: - file: ../../people.graphql"#; - let tmp_home = TempDir::new().unwrap(); - let tmp_dir = Utf8PathBuf::try_from(tmp_home.path().to_path_buf()).unwrap(); - let mut config_path = tmp_dir.clone(); - config_path.push("layer"); - config_path.push("layer"); - fs::create_dir_all(&config_path).unwrap(); - config_path.push("config.yaml"); - fs::write(&config_path, raw_good_yaml).unwrap(); - let films_path = tmp_dir.join("films.graphql"); - let people_path = tmp_dir.join("people.graphql"); - fs::write(films_path, "there is something here").unwrap(); - fs::write(people_path, "there is also something here").unwrap(); - let subgraph_definitions = resolve_supergraph_yaml( - &FileDescriptorType::File(config_path), - get_studio_config(), - &ProfileOpt { - profile_name: "profile".to_string(), - }, - ) - .unwrap() - .get_subgraph_definitions() - .unwrap(); - let film_subgraph = subgraph_definitions.first().unwrap(); - let people_subgraph = subgraph_definitions.get(1).unwrap(); - - assert_eq!(film_subgraph.name, "films"); - assert_eq!(film_subgraph.url, "https://films.example.com"); - assert_eq!(film_subgraph.sdl, "there is something here"); - assert_eq!(people_subgraph.name, "people"); - assert_eq!(people_subgraph.url, "https://people.example.com"); - assert_eq!(people_subgraph.sdl, "there is also something here"); - } - #[rstest] #[case::simple_binary("a/b/c/d/supergraph-v2.8.5", "v2.8.5")] #[case::simple_windows_binary("a/b/supergraph-v2.9.1.exe", "v2.9.1")] diff --git a/src/command/supergraph/mod.rs b/src/command/supergraph/mod.rs index decf897de..9488b0998 100644 --- a/src/command/supergraph/mod.rs +++ b/src/command/supergraph/mod.rs @@ -1,11 +1,3 @@ -pub(crate) mod compose; -mod fetch; - -#[cfg(feature = "composition-js")] -mod resolve_config; -#[cfg(feature = "composition-js")] -pub(crate) use resolve_config::{expand_supergraph_yaml, resolve_supergraph_yaml}; - use camino::Utf8PathBuf; use clap::Parser; use serde::Serialize; @@ -13,6 +5,9 @@ use serde::Serialize; use crate::utils::client::StudioClientConfig; use crate::{RoverOutput, RoverResult}; +pub(crate) mod compose; +mod fetch; + #[derive(Debug, Serialize, Parser)] pub struct Supergraph { #[clap(subcommand)] diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 18cc386a7..9242cd376 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -3,6 +3,8 @@ pub mod env; pub mod parsers; pub mod pkg; pub mod stringify; +#[cfg(feature = "composition-js")] +pub mod supergraph_config; pub mod table; pub mod telemetry; pub mod version; diff --git a/src/utils/parsers.rs b/src/utils/parsers.rs index 7cfd3358f..309cd91d3 100644 --- a/src/utils/parsers.rs +++ b/src/utils/parsers.rs @@ -1,16 +1,18 @@ -use anyhow::{anyhow, Context}; -use camino::{Utf8Path, Utf8PathBuf}; -use rover_std::Fs; - -use crate::{RoverError, RoverErrorSuggestion, RoverResult}; - use std::{ fmt, io::{self, Read}, str::FromStr, }; -#[derive(Debug, Clone, Eq, PartialEq)] +use anyhow::{anyhow, Context}; +use camino::{Utf8Path, Utf8PathBuf}; +use serde::Serialize; + +use rover_std::Fs; + +use crate::{RoverError, RoverErrorSuggestion, RoverResult}; + +#[derive(Debug, Clone, Eq, PartialEq, Serialize)] pub enum FileDescriptorType { Stdin, File(Utf8PathBuf), @@ -111,12 +113,14 @@ pub fn parse_header(header: &str) -> std::result::Result<(String, String), io::E #[cfg(test)] mod tests { - use super::FileDescriptorType; - use assert_fs::prelude::*; - use camino::Utf8PathBuf; use std::convert::TryFrom; use std::str::FromStr; + use assert_fs::prelude::*; + use camino::Utf8PathBuf; + + use super::FileDescriptorType; + #[test] fn it_correctly_parses_stdin_flag() { let fd = FileDescriptorType::from_str("-").unwrap(); diff --git a/src/command/supergraph/resolve_config.rs b/src/utils/supergraph_config.rs similarity index 60% rename from src/command/supergraph/resolve_config.rs rename to src/utils/supergraph_config.rs index 3aa660755..fced29c92 100644 --- a/src/command/supergraph/resolve_config.rs +++ b/src/utils/supergraph_config.rs @@ -1,26 +1,122 @@ use std::str::FromStr; use anyhow::anyhow; -use apollo_federation_types::{ - build::{BuildError, BuildErrors, SubgraphDefinition}, - config::{FederationVersion, SchemaSource, SubgraphConfig, SupergraphConfig}, +use apollo_federation_types::build::{BuildError, BuildErrors, SubgraphDefinition}; +use apollo_federation_types::config::{ + FederationVersion, SchemaSource, SubgraphConfig, SupergraphConfig, }; use apollo_parser::{cst, Parser}; use rayon::iter::{IntoParallelIterator, ParallelIterator}; -use rover_client::operations::subgraph::fetch::{self, SubgraphFetchInput}; -use rover_client::operations::subgraph::introspect::{self, SubgraphIntrospectInput}; +use rover_client::blocking::GraphQLClient; +use rover_client::operations::subgraph; +use rover_client::operations::subgraph::fetch::SubgraphFetchInput; +use rover_client::operations::subgraph::introspect::SubgraphIntrospectInput; +use rover_client::operations::subgraph::list::SubgraphListInput; +use rover_client::operations::subgraph::{fetch, introspect}; use rover_client::shared::GraphRef; -use rover_client::{blocking::GraphQLClient, RoverClientError}; +use rover_client::RoverClientError; use rover_std::{Fs, Style}; -use crate::{ - options::ProfileOpt, - utils::{client::StudioClientConfig, expansion::expand, parsers::FileDescriptorType}, -}; +use crate::options::ProfileOpt; +use crate::utils::client::StudioClientConfig; +use crate::utils::expansion::expand; +use crate::utils::parsers::FileDescriptorType; use crate::{RoverError, RoverErrorSuggestion, RoverResult}; -pub(crate) fn expand_supergraph_yaml(content: &str) -> RoverResult { +/// Nominal type that captures the behavior of collecting remote subgraphs into a +/// [`SupergraphConfig`] representation +#[derive(Clone, Debug)] +pub struct RemoteSubgraphs(SupergraphConfig); + +impl RemoteSubgraphs { + /// Fetches [`RemoteSubgraphs`] from Studio + pub fn fetch( + client_config: &StudioClientConfig, + profile_opt: &ProfileOpt, + federation_version: &FederationVersion, + graph_ref: &GraphRef, + ) -> RoverResult { + let client = &client_config.get_authenticated_client(profile_opt)?; + + let subgraphs = subgraph::list::run( + SubgraphListInput { + graph_ref: graph_ref.clone(), + }, + client, + )?; + let subgraphs = subgraphs + .subgraphs + .iter() + .map(|subgraph| { + ( + subgraph.name.clone(), + SubgraphConfig { + routing_url: subgraph.url.clone(), + schema: SchemaSource::Subgraph { + graphref: graph_ref.clone().to_string(), + subgraph: subgraph.name.clone(), + }, + }, + ) + }) + .collect(); + let supergraph_config = SupergraphConfig::new(subgraphs, Some(federation_version.clone())); + let remote_subgraphs = RemoteSubgraphs(supergraph_config); + Ok(remote_subgraphs) + } + + /// Provides a reference to the inner value of this representation + pub fn inner(&self) -> &SupergraphConfig { + &self.0 + } +} + +pub fn get_supergraph_config( + graph_ref: &Option, + supergraph_config_path: &Option, + federation_version: &FederationVersion, + client_config: StudioClientConfig, + profile_opt: &ProfileOpt, +) -> Result, RoverError> { + // Read in Remote subgraphs + let remote_subgraphs = match graph_ref { + Some(graph_ref) => Some(RemoteSubgraphs::fetch( + &client_config, + profile_opt, + federation_version, + graph_ref, + )?), + None => None, + }; + + // Read in Local Supergraph Config + let supergraph_config = if let Some(file_descriptor) = &supergraph_config_path { + Some(resolve_supergraph_yaml( + file_descriptor, + client_config, + profile_opt, + )?) + } else { + None + }; + + // Merge Remote and Local Supergraph Configs + let supergraph_config = match remote_subgraphs { + Some(remote_subgraphs) => match supergraph_config { + Some(supergraph_config) => { + let mut merged_supergraph_config = remote_subgraphs.inner().clone(); + merged_supergraph_config.merge_subgraphs(&supergraph_config); + Some(merged_supergraph_config) + } + None => Some(remote_subgraphs.inner().clone()), + }, + None => supergraph_config, + }; + Ok(supergraph_config) +} + +fn expand_supergraph_yaml(content: &str) -> RoverResult { serde_yaml::from_str(content) .map_err(RoverError::from) .and_then(expand) @@ -270,14 +366,26 @@ pub(crate) fn resolve_supergraph_yaml( } #[cfg(test)] -mod test_expand_supergraph_yaml { +mod test { + use std::fs; + use apollo_federation_types::config::FederationVersion; + use assert_fs::TempDir; + use camino::Utf8PathBuf; + use rstest::{fixture, rstest}; + + use houston::Config; + + use crate::options::ProfileOpt; + use crate::utils::client::{ClientBuilder, StudioClientConfig}; + use crate::utils::parsers::FileDescriptorType; + use crate::utils::supergraph_config::resolve_supergraph_yaml; #[test] fn test_supergraph_yaml_int_version() { let yaml = r#" federation_version: 1 -subgraphs: +subgraphs: "#; let config = super::expand_supergraph_yaml(yaml).unwrap(); assert_eq!( @@ -285,4 +393,125 @@ subgraphs: Some(FederationVersion::LatestFedOne) ); } + + #[fixture] + fn client_config() -> StudioClientConfig { + let tmp_home = TempDir::new().unwrap(); + let tmp_path = Utf8PathBuf::try_from(tmp_home.path().to_path_buf()).unwrap(); + StudioClientConfig::new( + None, + Config::new(Some(&tmp_path), None).unwrap(), + false, + ClientBuilder::default(), + ) + } + + #[fixture] + fn profile_opt() -> ProfileOpt { + ProfileOpt { + profile_name: "profile".to_string(), + } + } + + #[rstest] + fn it_errs_on_invalid_subgraph_path( + client_config: StudioClientConfig, + profile_opt: ProfileOpt, + ) { + let raw_good_yaml = r#"subgraphs: + films: + routing_url: https://films.example.com + schema: + file: ./films-do-not-exist.graphql + people: + routing_url: https://people.example.com + schema: + file: ./people-do-not-exist.graphql"#; + let tmp_home = TempDir::new().unwrap(); + let mut config_path = Utf8PathBuf::try_from(tmp_home.path().to_path_buf()).unwrap(); + config_path.push("config.yaml"); + fs::write(&config_path, raw_good_yaml).unwrap(); + assert!(resolve_supergraph_yaml( + &FileDescriptorType::File(config_path), + client_config, + &profile_opt + ) + .is_err()) + } + + #[rstest] + fn it_can_get_subgraph_definitions_from_fs( + client_config: StudioClientConfig, + profile_opt: ProfileOpt, + ) { + let raw_good_yaml = r#"subgraphs: + films: + routing_url: https://films.example.com + schema: + file: ./films.graphql + people: + routing_url: https://people.example.com + schema: + file: ./people.graphql"#; + let tmp_home = TempDir::new().unwrap(); + let mut config_path = Utf8PathBuf::try_from(tmp_home.path().to_path_buf()).unwrap(); + config_path.push("config.yaml"); + fs::write(&config_path, raw_good_yaml).unwrap(); + let tmp_dir = config_path.parent().unwrap().to_path_buf(); + let films_path = tmp_dir.join("films.graphql"); + let people_path = tmp_dir.join("people.graphql"); + fs::write(films_path, "there is something here").unwrap(); + fs::write(people_path, "there is also something here").unwrap(); + assert!(resolve_supergraph_yaml( + &FileDescriptorType::File(config_path), + client_config, + &profile_opt + ) + .is_ok()) + } + + #[rstest] + fn it_can_compute_relative_schema_paths( + client_config: StudioClientConfig, + profile_opt: ProfileOpt, + ) { + let raw_good_yaml = r#"subgraphs: + films: + routing_url: https://films.example.com + schema: + file: ../../films.graphql + people: + routing_url: https://people.example.com + schema: + file: ../../people.graphql"#; + let tmp_home = TempDir::new().unwrap(); + let tmp_dir = Utf8PathBuf::try_from(tmp_home.path().to_path_buf()).unwrap(); + let mut config_path = tmp_dir.clone(); + config_path.push("layer"); + config_path.push("layer"); + fs::create_dir_all(&config_path).unwrap(); + config_path.push("config.yaml"); + fs::write(&config_path, raw_good_yaml).unwrap(); + let films_path = tmp_dir.join("films.graphql"); + let people_path = tmp_dir.join("people.graphql"); + fs::write(films_path, "there is something here").unwrap(); + fs::write(people_path, "there is also something here").unwrap(); + let subgraph_definitions = resolve_supergraph_yaml( + &FileDescriptorType::File(config_path), + client_config, + &profile_opt, + ) + .unwrap() + .get_subgraph_definitions() + .unwrap(); + let film_subgraph = subgraph_definitions.first().unwrap(); + let people_subgraph = subgraph_definitions.get(1).unwrap(); + + assert_eq!(film_subgraph.name, "films"); + assert_eq!(film_subgraph.url, "https://films.example.com"); + assert_eq!(film_subgraph.sdl, "there is something here"); + assert_eq!(people_subgraph.name, "people"); + assert_eq!(people_subgraph.url, "https://people.example.com"); + assert_eq!(people_subgraph.sdl, "there is also something here"); + } } From e8f58b21d68440ba4ee57877ecc7d7138c8861d0 Mon Sep 17 00:00:00 2001 From: jonathanrainer Date: Thu, 25 Jul 2024 14:03:55 +0100 Subject: [PATCH 2/8] ROVER-69 Migrate tests from #1977 --- Cargo.lock | 1 + Cargo.toml | 1 + src/command/supergraph/compose/do_compose.rs | 2 - src/utils/supergraph_config.rs | 415 ++++++++++++++++++- 4 files changed, 411 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2fbf96e40..e3b9f4f64 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4585,6 +4585,7 @@ dependencies = [ "heck 0.5.0", "houston", "httpmock", + "indoc", "interprocess", "lazy_static", "lazycell", diff --git a/Cargo.toml b/Cargo.toml index ebd63645d..fe9f1ad0c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -203,6 +203,7 @@ duct = "0.13.7" git2 = { workspace = true, features = ["https"]} graphql-schema-diff = "0.2.0" httpmock = { workspace = true } +indoc = { workspace = true } mime = "0.3.17" portpicker = "0.1.1" predicates = { workspace = true } diff --git a/src/command/supergraph/compose/do_compose.rs b/src/command/supergraph/compose/do_compose.rs index e8de81524..a2681a59f 100644 --- a/src/command/supergraph/compose/do_compose.rs +++ b/src/command/supergraph/compose/do_compose.rs @@ -215,8 +215,6 @@ impl Compose { #[cfg(test)] mod tests { - use std::convert::TryFrom; - use rstest::rstest; use speculoos::assert_that; diff --git a/src/utils/supergraph_config.rs b/src/utils/supergraph_config.rs index fced29c92..4db7236ac 100644 --- a/src/utils/supergraph_config.rs +++ b/src/utils/supergraph_config.rs @@ -368,25 +368,35 @@ pub(crate) fn resolve_supergraph_yaml( #[cfg(test)] mod test { use std::fs; + use std::io::Write; + use std::string::ToString; - use apollo_federation_types::config::FederationVersion; + use anyhow::Result; + use apollo_federation_types::config::{FederationVersion, SchemaSource, SubgraphConfig}; use assert_fs::TempDir; use camino::Utf8PathBuf; + use httpmock::MockServer; + use indoc::indoc; use rstest::{fixture, rstest}; + use serde_json::json; + use speculoos::assert_that; + use speculoos::prelude::{ResultAssertions, VecAssertions}; use houston::Config; use crate::options::ProfileOpt; use crate::utils::client::{ClientBuilder, StudioClientConfig}; use crate::utils::parsers::FileDescriptorType; - use crate::utils::supergraph_config::resolve_supergraph_yaml; + + use super::*; #[test] fn test_supergraph_yaml_int_version() { - let yaml = r#" -federation_version: 1 -subgraphs: -"#; + let yaml = indoc! {r#" + federation_version: 1 + subgraphs: +"# + }; let config = super::expand_supergraph_yaml(yaml).unwrap(); assert_eq!( config.get_federation_version(), @@ -514,4 +524,397 @@ subgraphs: assert_eq!(people_subgraph.url, "https://people.example.com"); assert_eq!(people_subgraph.sdl, "there is also something here"); } + + const INTROSPECTION_SDL: &str = r#"directive @key(fields: _FieldSet!, resolvable: Boolean = true) repeatable on OBJECT | INTERFACE + +directive @requires(fields: _FieldSet!) on FIELD_DEFINITION + +directive @provides(fields: _FieldSet!) on FIELD_DEFINITION + +directive @external(reason: String) on OBJECT | FIELD_DEFINITION + +directive @tag(name: String!) repeatable on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION + +directive @extends on OBJECT | INTERFACE + +type Query {\n test: String!\n _service: _Service!\n} + +scalar _FieldSet + +scalar _Any + +type _Service {\n sdl: String\n}"#; + + #[fixture] + fn schema() -> String { + String::from(indoc! {r#" + type Query { + test: String! + } + "# + }) + } + + #[fixture] + #[once] + fn home_dir() -> Utf8PathBuf { + tempfile::tempdir() + .unwrap() + .path() + .to_path_buf() + .try_into() + .unwrap() + } + + #[fixture] + #[once] + fn api_key() -> String { + uuid::Uuid::new_v4().as_simple().to_string() + } + + #[fixture] + fn config(home_dir: &Utf8PathBuf, api_key: &String) -> Config { + Config::new(Some(home_dir), Some(api_key.to_string())).unwrap() + } + + #[fixture] + fn studio_client_config(config: Config) -> StudioClientConfig { + StudioClientConfig::new(None, config, false, ClientBuilder::default()) + } + + #[rstest] + fn test_subgraph_file_resolution( + schema: String, + profile_opt: ProfileOpt, + studio_client_config: StudioClientConfig, + ) -> Result<()> { + let mut schema_path = tempfile::NamedTempFile::new()?; + schema_path + .as_file_mut() + .write_all(&schema.clone().into_bytes())?; + let supergraph_config = format!( + indoc! {r#" + federation_version: 2 + subgraphs: + products: + routing_url: http://localhost:8000/ + schema: + file: {} +"# + }, + schema_path.path().to_str().unwrap() + ); + + let mut supergraph_config_path = tempfile::NamedTempFile::new()?; + supergraph_config_path + .as_file_mut() + .write_all(&supergraph_config.into_bytes())?; + + let unresolved_supergraph_config = + FileDescriptorType::File(supergraph_config_path.path().to_path_buf().try_into()?); + + let resolved_config = super::resolve_supergraph_yaml( + &unresolved_supergraph_config, + studio_client_config, + &profile_opt, + ); + + assert_that!(resolved_config).is_ok(); + let resolved_config = resolved_config.unwrap(); + + let subgraphs = resolved_config.into_iter().collect::>(); + assert_that!(subgraphs).has_length(1); + let subgraph = &subgraphs[0]; + assert_that!(subgraph).is_equal_to(&( + "products".to_string(), + SubgraphConfig { + routing_url: Some("http://localhost:8000/".to_string()), + schema: SchemaSource::Sdl { + sdl: schema.to_string(), + }, + }, + )); + + Ok(()) + } + + #[rstest] + fn test_subgraph_introspection_resolution( + profile_opt: ProfileOpt, + studio_client_config: StudioClientConfig, + ) -> Result<()> { + let server = MockServer::start(); + + let mock = server.mock(|when, then| { + let body = json!({ + "data": { + "_service": { + "sdl": INTROSPECTION_SDL + } + } + }); + when.method(httpmock::Method::POST).path("/"); + then.status(200) + .header("content-type", "application/json") + .json_body(body); + }); + + let supergraph_config = format!( + indoc! {r#" + federation_version: 2 + subgraphs: + products: + routing_url: {} + schema: + subgraph_url: {} +"# + }, + server.base_url(), + server.base_url() + ); + + let mut supergraph_config_path = tempfile::NamedTempFile::new()?; + supergraph_config_path + .as_file_mut() + .write_all(&supergraph_config.into_bytes())?; + + let unresolved_supergraph_config = + FileDescriptorType::File(supergraph_config_path.path().to_path_buf().try_into()?); + + let resolved_config = super::resolve_supergraph_yaml( + &unresolved_supergraph_config, + studio_client_config, + &profile_opt, + ); + + mock.assert_hits(1); + + assert_that!(resolved_config).is_ok(); + let resolved_config = resolved_config.unwrap(); + + let subgraphs = resolved_config.into_iter().collect::>(); + assert_that!(subgraphs).has_length(1); + let subgraph = &subgraphs[0]; + assert_that!(subgraph).is_equal_to(&( + "products".to_string(), + SubgraphConfig { + routing_url: Some(server.base_url()), + schema: SchemaSource::Sdl { + sdl: INTROSPECTION_SDL.to_string(), + }, + }, + )); + + Ok(()) + } + + #[rstest] + fn test_subgraph_studio_resolution(profile_opt: ProfileOpt, config: Config) -> Result<()> { + let graph_id = "testgraph"; + let variant = "current"; + let graphref = format!("{}@{}", graph_id, variant); + let server = MockServer::start(); + + let subgraph_fetch_mock = server.mock(|when, then| { + let body = json!({ + "data": { + "variant": { + "__typename": "GraphVariant", + "subgraph": { + "url": server.base_url(), + "activePartialSchema": { + "sdl": INTROSPECTION_SDL + } + }, + "subgraphs": [ + { + "name": "products" + } + ] + } + } + }); + when.method(httpmock::Method::POST) + .path("/") + .json_body_obj(&json!({ + "query": indoc!{ + r#" + query SubgraphFetchQuery($graph_ref: ID!, $subgraph_name: ID!) { + variant(ref: $graph_ref) { + __typename + ... on GraphVariant { + subgraph(name: $subgraph_name) { + url, + activePartialSchema { + sdl + } + } + subgraphs { + name + } + } + } + } + "# + }, + "variables": { + "graph_ref": graphref, + "subgraph_name": "products" + }, + "operationName": "SubgraphFetchQuery" + })); + then.status(200) + .header("content-type", "application/json") + .json_body(body); + }); + + let is_federated_mock = server.mock(|when, then| { + let body = json!({ + "data": { + "graph": { + "variant": { + "subgraphs": [ + { + "name": "products" + } + ] + } + } + } + }); + when.method(httpmock::Method::POST) + .path("/") + .json_body_obj(&json!({ + "query": indoc!{ + r#" + query IsFederatedGraph($graph_id: ID!, $variant: String!) { + graph(id: $graph_id) { + variant(name: $variant) { + subgraphs { + name + } + } + } + } + "# + }, + "variables": { + "graph_id": graph_id, + "variant": variant + }, + "operationName": "IsFederatedGraph" + })); + then.status(200) + .header("content-type", "application/json") + .json_body(body); + }); + + let supergraph_config = format!( + indoc! {r#" + federation_version: 2 + subgraphs: + products: + schema: + graphref: {} + subgraph: products +"# + }, + graphref + ); + + let studio_client_config = StudioClientConfig::new( + Some(server.base_url()), + config, + false, + ClientBuilder::default(), + ); + + let mut supergraph_config_path = tempfile::NamedTempFile::new()?; + supergraph_config_path + .as_file_mut() + .write_all(&supergraph_config.into_bytes())?; + + let unresolved_supergraph_config = + FileDescriptorType::File(supergraph_config_path.path().to_path_buf().try_into()?); + + let resolved_config = super::resolve_supergraph_yaml( + &unresolved_supergraph_config, + studio_client_config, + &profile_opt, + ); + + assert_that!(resolved_config).is_ok(); + let resolved_config = resolved_config.unwrap(); + + is_federated_mock.assert_hits(1); + subgraph_fetch_mock.assert_hits(1); + + let subgraphs = resolved_config.into_iter().collect::>(); + assert_that!(subgraphs).has_length(1); + let subgraph = &subgraphs[0]; + assert_that!(subgraph).is_equal_to(&( + "products".to_string(), + SubgraphConfig { + routing_url: Some(server.base_url()), + schema: SchemaSource::Sdl { + sdl: INTROSPECTION_SDL.to_string(), + }, + }, + )); + + Ok(()) + } + + #[rstest] + fn test_subgraph_sdl_resolution( + schema: String, + profile_opt: ProfileOpt, + studio_client_config: StudioClientConfig, + ) -> Result<()> { + let supergraph_config = format!( + indoc! { + r#" + federation_version: 2 + subgraphs: + products: + routing_url: http://localhost:8000/ + schema: + sdl: "{}" + "# + }, + schema.escape_default() + ); + + let mut supergraph_config_path = tempfile::NamedTempFile::new()?; + supergraph_config_path + .as_file_mut() + .write_all(&supergraph_config.into_bytes())?; + + let unresolved_supergraph_config = + FileDescriptorType::File(supergraph_config_path.path().to_path_buf().try_into()?); + + let resolved_config = super::resolve_supergraph_yaml( + &unresolved_supergraph_config, + studio_client_config, + &profile_opt, + ); + + assert_that!(resolved_config).is_ok(); + let resolved_config = resolved_config.unwrap(); + + let subgraphs = resolved_config.into_iter().collect::>(); + assert_that!(subgraphs).has_length(1); + let subgraph = &subgraphs[0]; + assert_that!(subgraph).is_equal_to(&( + "products".to_string(), + SubgraphConfig { + routing_url: Some("http://localhost:8000/".to_string()), + schema: SchemaSource::Sdl { + sdl: schema.to_string(), + }, + }, + )); + + Ok(()) + } } From 59d6f449defd65d1a3123f21d3a80ebcf90db690 Mon Sep 17 00:00:00 2001 From: jonathanrainer Date: Thu, 25 Jul 2024 07:29:03 +0100 Subject: [PATCH 3/8] ROVER-69 Fix Federation Versions to remove WARNs --- src/utils/supergraph_config.rs | 164 +++++++++++++++++++-------------- 1 file changed, 96 insertions(+), 68 deletions(-) diff --git a/src/utils/supergraph_config.rs b/src/utils/supergraph_config.rs index 4db7236ac..2f8e1f35a 100644 --- a/src/utils/supergraph_config.rs +++ b/src/utils/supergraph_config.rs @@ -116,13 +116,6 @@ pub fn get_supergraph_config( Ok(supergraph_config) } -fn expand_supergraph_yaml(content: &str) -> RoverResult { - serde_yaml::from_str(content) - .map_err(RoverError::from) - .and_then(expand) - .and_then(|v| serde_yaml::from_value(v).map_err(RoverError::from)) -} - pub(crate) fn resolve_supergraph_yaml( unresolved_supergraph_yaml: &FileDescriptorType, client_config: StudioClientConfig, @@ -365,10 +358,19 @@ pub(crate) fn resolve_supergraph_yaml( Ok(resolved_supergraph_config) } +fn expand_supergraph_yaml(content: &str) -> RoverResult { + serde_yaml::from_str(content) + .map_err(RoverError::from) + .and_then(expand) + .and_then(|v| serde_yaml::from_value(v).map_err(RoverError::from)) +} + #[cfg(test)] -mod test { +mod test_resolve_supergraph_yaml { use std::fs; + use std::fs::File; use std::io::Write; + use std::path::PathBuf; use std::string::ToString; use anyhow::Result; @@ -378,7 +380,8 @@ mod test { use httpmock::MockServer; use indoc::indoc; use rstest::{fixture, rstest}; - use serde_json::json; + use semver::Version; + use serde_json::{json, Value}; use speculoos::assert_that; use speculoos::prelude::{ResultAssertions, VecAssertions}; @@ -390,6 +393,59 @@ mod test { use super::*; + #[fixture] + fn profile_opt() -> ProfileOpt { + ProfileOpt { + profile_name: "profile".to_string(), + } + } + + #[fixture] + #[once] + fn home_dir() -> Utf8PathBuf { + tempfile::tempdir() + .unwrap() + .path() + .to_path_buf() + .try_into() + .unwrap() + } + + #[fixture] + #[once] + fn api_key() -> String { + uuid::Uuid::new_v4().as_simple().to_string() + } + + #[fixture] + fn config(home_dir: &Utf8PathBuf, api_key: &String) -> Config { + Config::new(Some(home_dir), Some(api_key.to_string())).unwrap() + } + + #[fixture] + fn client_config(config: Config) -> StudioClientConfig { + StudioClientConfig::new(None, config, false, ClientBuilder::default()) + } + + #[fixture] + #[once] + fn latest_fed2_version() -> FederationVersion { + let d = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("latest_plugin_versions.json"); + let fp = File::open(d).expect("could not open version file"); + let raw_version_file: Value = serde_json::from_reader(fp).expect("malformed JSON"); + let raw_version = raw_version_file + .get("supergraph") + .unwrap() + .get("versions") + .unwrap() + .get("latest-2") + .unwrap() + .as_str() + .unwrap(); + let version = Version::from_str(&raw_version.replace("v", "")).unwrap(); + FederationVersion::ExactFedTwo(version) + } + #[test] fn test_supergraph_yaml_int_version() { let yaml = indoc! {r#" @@ -404,25 +460,6 @@ mod test { ); } - #[fixture] - fn client_config() -> StudioClientConfig { - let tmp_home = TempDir::new().unwrap(); - let tmp_path = Utf8PathBuf::try_from(tmp_home.path().to_path_buf()).unwrap(); - StudioClientConfig::new( - None, - Config::new(Some(&tmp_path), None).unwrap(), - false, - ClientBuilder::default(), - ) - } - - #[fixture] - fn profile_opt() -> ProfileOpt { - ProfileOpt { - profile_name: "profile".to_string(), - } - } - #[rstest] fn it_errs_on_invalid_subgraph_path( client_config: StudioClientConfig, @@ -453,8 +490,12 @@ mod test { fn it_can_get_subgraph_definitions_from_fs( client_config: StudioClientConfig, profile_opt: ProfileOpt, + latest_fed2_version: &FederationVersion, ) { - let raw_good_yaml = r#"subgraphs: + let raw_good_yaml = format!( + r#" +federation_version: {} +subgraphs: films: routing_url: https://films.example.com schema: @@ -462,7 +503,9 @@ mod test { people: routing_url: https://people.example.com schema: - file: ./people.graphql"#; + file: ./people.graphql"#, + latest_fed2_version.to_string() + ); let tmp_home = TempDir::new().unwrap(); let mut config_path = Utf8PathBuf::try_from(tmp_home.path().to_path_buf()).unwrap(); config_path.push("config.yaml"); @@ -484,8 +527,12 @@ mod test { fn it_can_compute_relative_schema_paths( client_config: StudioClientConfig, profile_opt: ProfileOpt, + latest_fed2_version: &FederationVersion, ) { - let raw_good_yaml = r#"subgraphs: + let raw_good_yaml = format!( + r#" +federation_version: {} +subgraphs: films: routing_url: https://films.example.com schema: @@ -493,7 +540,9 @@ mod test { people: routing_url: https://people.example.com schema: - file: ../../people.graphql"#; + file: ../../people.graphql"#, + latest_fed2_version.to_string() + ); let tmp_home = TempDir::new().unwrap(); let tmp_dir = Utf8PathBuf::try_from(tmp_home.path().to_path_buf()).unwrap(); let mut config_path = tmp_dir.clone(); @@ -555,38 +604,12 @@ type _Service {\n sdl: String\n}"#; }) } - #[fixture] - #[once] - fn home_dir() -> Utf8PathBuf { - tempfile::tempdir() - .unwrap() - .path() - .to_path_buf() - .try_into() - .unwrap() - } - - #[fixture] - #[once] - fn api_key() -> String { - uuid::Uuid::new_v4().as_simple().to_string() - } - - #[fixture] - fn config(home_dir: &Utf8PathBuf, api_key: &String) -> Config { - Config::new(Some(home_dir), Some(api_key.to_string())).unwrap() - } - - #[fixture] - fn studio_client_config(config: Config) -> StudioClientConfig { - StudioClientConfig::new(None, config, false, ClientBuilder::default()) - } - #[rstest] fn test_subgraph_file_resolution( schema: String, profile_opt: ProfileOpt, - studio_client_config: StudioClientConfig, + client_config: StudioClientConfig, + latest_fed2_version: &FederationVersion, ) -> Result<()> { let mut schema_path = tempfile::NamedTempFile::new()?; schema_path @@ -594,7 +617,7 @@ type _Service {\n sdl: String\n}"#; .write_all(&schema.clone().into_bytes())?; let supergraph_config = format!( indoc! {r#" - federation_version: 2 + federation_version: {} subgraphs: products: routing_url: http://localhost:8000/ @@ -602,6 +625,7 @@ type _Service {\n sdl: String\n}"#; file: {} "# }, + latest_fed2_version.to_string(), schema_path.path().to_str().unwrap() ); @@ -615,7 +639,7 @@ type _Service {\n sdl: String\n}"#; let resolved_config = super::resolve_supergraph_yaml( &unresolved_supergraph_config, - studio_client_config, + client_config, &profile_opt, ); @@ -641,7 +665,8 @@ type _Service {\n sdl: String\n}"#; #[rstest] fn test_subgraph_introspection_resolution( profile_opt: ProfileOpt, - studio_client_config: StudioClientConfig, + client_config: StudioClientConfig, + latest_fed2_version: &FederationVersion, ) -> Result<()> { let server = MockServer::start(); @@ -661,7 +686,7 @@ type _Service {\n sdl: String\n}"#; let supergraph_config = format!( indoc! {r#" - federation_version: 2 + federation_version: {} subgraphs: products: routing_url: {} @@ -669,6 +694,7 @@ type _Service {\n sdl: String\n}"#; subgraph_url: {} "# }, + latest_fed2_version.to_string(), server.base_url(), server.base_url() ); @@ -683,7 +709,7 @@ type _Service {\n sdl: String\n}"#; let resolved_config = super::resolve_supergraph_yaml( &unresolved_supergraph_config, - studio_client_config, + client_config, &profile_opt, ); @@ -869,12 +895,13 @@ type _Service {\n sdl: String\n}"#; fn test_subgraph_sdl_resolution( schema: String, profile_opt: ProfileOpt, - studio_client_config: StudioClientConfig, + client_config: StudioClientConfig, + latest_fed2_version: &FederationVersion, ) -> Result<()> { let supergraph_config = format!( indoc! { r#" - federation_version: 2 + federation_version: {} subgraphs: products: routing_url: http://localhost:8000/ @@ -882,6 +909,7 @@ type _Service {\n sdl: String\n}"#; sdl: "{}" "# }, + latest_fed2_version.to_string(), schema.escape_default() ); @@ -895,7 +923,7 @@ type _Service {\n sdl: String\n}"#; let resolved_config = super::resolve_supergraph_yaml( &unresolved_supergraph_config, - studio_client_config, + client_config, &profile_opt, ); From 7d888f5aad50aba677a7bab1429a7d71828c037e Mon Sep 17 00:00:00 2001 From: jonathanrainer Date: Thu, 25 Jul 2024 07:30:58 +0100 Subject: [PATCH 4/8] ROVER-69 Fix docs --- src/command/supergraph/compose/do_compose.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/command/supergraph/compose/do_compose.rs b/src/command/supergraph/compose/do_compose.rs index a2681a59f..a85f846da 100644 --- a/src/command/supergraph/compose/do_compose.rs +++ b/src/command/supergraph/compose/do_compose.rs @@ -47,7 +47,7 @@ pub struct SupergraphComposeOpts { /// /// This is analogous to providing a supergraph.yaml file with references to your graph variant in studio. /// - /// If used in conjunction with `--supergraph-config`, the values presented in the supergraph.yaml will take precedence over these values. + /// If used in conjunction with `--config`, the values presented in the supergraph.yaml will take precedence over these values. #[arg(long = "graph-ref")] graph_ref: Option, From ae69a95bbc64a75e9e868a1b750d2cf919629828 Mon Sep 17 00:00:00 2001 From: jonathanrainer Date: Thu, 25 Jul 2024 07:39:30 +0100 Subject: [PATCH 5/8] ROVER-69 Optimise --graph-ref uses Rather than fetching each subgraph one at a time, make a single GraphQL request to get them all at once. This combines the work from #1985 --- Cargo.lock | 58 ++++ Cargo.toml | 2 + crates/rover-client/Cargo.toml | 2 + .../fetch_all/fetch_all_query.graphql | 14 + .../src/operations/subgraph/fetch_all/mod.rs | 5 + .../operations/subgraph/fetch_all/runner.rs | 123 +++++++ .../operations/subgraph/fetch_all/types.rs | 31 ++ .../src/operations/subgraph/mod.rs | 3 + src/utils/supergraph_config.rs | 311 ++++++++++++++++-- 9 files changed, 525 insertions(+), 24 deletions(-) create mode 100644 crates/rover-client/src/operations/subgraph/fetch_all/fetch_all_query.graphql create mode 100644 crates/rover-client/src/operations/subgraph/fetch_all/mod.rs create mode 100644 crates/rover-client/src/operations/subgraph/fetch_all/runner.rs create mode 100644 crates/rover-client/src/operations/subgraph/fetch_all/types.rs diff --git a/Cargo.lock b/Cargo.lock index e3b9f4f64..af4e551f0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -775,6 +775,21 @@ version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40e38929add23cdf8a366df9b0e088953150724bcbe5fc330b0d8eb3b328eec8" +[[package]] +name = "buildstructor" +version = "0.5.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c3907aac66c65520545ae3cb3c195306e20d5ed5c90bfbb992e061cf12a104d0" +dependencies = [ + "lazy_static", + "proc-macro2", + "quote", + "str_inflector", + "syn 2.0.72", + "thiserror", + "try_match", +] + [[package]] name = "bumpalo" version = "3.16.0" @@ -1593,6 +1608,17 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "derive-getters" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0a6433aac097572ea8ccc60b3f2e756c661c9aeed9225cdd4d0cb119cb7ff6ba" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.72", +] + [[package]] name = "derive_arbitrary" version = "1.3.2" @@ -4637,8 +4663,10 @@ dependencies = [ "apollo-parser 0.8.0", "ariadne", "backoff", + "buildstructor", "camino", "chrono", + "derive-getters", "git-url-parse", "git2", "graphql_client", @@ -5348,6 +5376,16 @@ dependencies = [ "wsl", ] +[[package]] +name = "str_inflector" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec0b848d5a7695b33ad1be00f84a3c079fe85c9278a325ff9159e6c99cef4ef7" +dependencies = [ + "lazy_static", + "regex", +] + [[package]] name = "strict" version = "0.2.0" @@ -6016,6 +6054,26 @@ version = "0.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b" +[[package]] +name = "try_match" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b065c869a3f832418e279aa4c1d7088f9d5d323bde15a60a08e20c2cd4549082" +dependencies = [ + "try_match_inner", +] + +[[package]] +name = "try_match_inner" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9c81686f7ab4065ccac3df7a910c4249f8c0f3fb70421d6ddec19b9311f63f9" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.72", +] + [[package]] name = "typed-builder" version = "0.18.2" diff --git a/Cargo.toml b/Cargo.toml index fe9f1ad0c..37c607838 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -75,6 +75,7 @@ backtrace = "0.3" backoff = "0.4" base64 = "0.22" billboard = "0.2" +buildstructor = "0.5.4" cargo_metadata = "0.18" calm_io = "0.1" camino = "1" @@ -84,6 +85,7 @@ ci_info = "0.14" console = "0.15" crossbeam-channel = "0.5" ctrlc = "3" +derive-getters = "0.4.0" dialoguer = "0.11" directories-next = "2.0" flate2 = "1" diff --git a/crates/rover-client/Cargo.toml b/crates/rover-client/Cargo.toml index 8be0b2f55..a69a5ccd3 100644 --- a/crates/rover-client/Cargo.toml +++ b/crates/rover-client/Cargo.toml @@ -13,7 +13,9 @@ apollo-federation-types = { workspace = true } apollo-parser = { workspace = true } apollo-encoder = { workspace = true } backoff = { workspace = true } +buildstructor = { workspace = true } chrono = { workspace = true, features = ["serde"] } +derive-getters = { workspace = true } git-url-parse = { workspace = true } git2 = { workspace = true, features = [ "vendored-openssl", diff --git a/crates/rover-client/src/operations/subgraph/fetch_all/fetch_all_query.graphql b/crates/rover-client/src/operations/subgraph/fetch_all/fetch_all_query.graphql new file mode 100644 index 000000000..6233fde6d --- /dev/null +++ b/crates/rover-client/src/operations/subgraph/fetch_all/fetch_all_query.graphql @@ -0,0 +1,14 @@ +query SubgraphFetchAllQuery($graph_ref: ID!) { + variant(ref: $graph_ref) { + __typename + ... on GraphVariant { + subgraphs { + name + url + activePartialSchema { + sdl + } + } + } + } +} diff --git a/crates/rover-client/src/operations/subgraph/fetch_all/mod.rs b/crates/rover-client/src/operations/subgraph/fetch_all/mod.rs new file mode 100644 index 000000000..3c7d12e25 --- /dev/null +++ b/crates/rover-client/src/operations/subgraph/fetch_all/mod.rs @@ -0,0 +1,5 @@ +mod runner; +mod types; + +pub use runner::run; +pub use types::SubgraphFetchAllInput; diff --git a/crates/rover-client/src/operations/subgraph/fetch_all/runner.rs b/crates/rover-client/src/operations/subgraph/fetch_all/runner.rs new file mode 100644 index 000000000..33af7da27 --- /dev/null +++ b/crates/rover-client/src/operations/subgraph/fetch_all/runner.rs @@ -0,0 +1,123 @@ +use graphql_client::*; + +use crate::blocking::StudioClient; +use crate::operations::config::is_federated::{self, IsFederatedInput}; +use crate::RoverClientError; + +use super::types::*; + +#[derive(GraphQLQuery)] +// 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/operations/subgraph/fetch_all/fetch_all_query.graphql", + schema_path = ".schema/schema.graphql", + response_derives = "Eq, 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. subgraph_fetch_all_query +pub(crate) struct SubgraphFetchAllQuery; + +/// For a given graph return all of its subgraphs as a list +pub fn run( + input: SubgraphFetchAllInput, + client: &StudioClient, +) -> Result, RoverClientError> { + let variables = input.clone().into(); + let response_data = client.post::(variables)?; + get_subgraphs_from_response_data(input, response_data) +} + +fn get_subgraphs_from_response_data( + input: SubgraphFetchAllInput, + response_data: SubgraphFetchAllResponseData, +) -> Result, RoverClientError> { + if let Some(maybe_variant) = response_data.variant { + match maybe_variant { + SubgraphFetchAllGraphVariant::GraphVariant(variant) => { + if let Some(subgraphs) = variant.subgraphs { + Ok(subgraphs + .into_iter() + .map(|subgraph| { + Subgraph::builder() + .name(subgraph.name.clone()) + .and_url(subgraph.url) + .sdl(subgraph.active_partial_schema.sdl) + .build() + }) + .collect()) + } else { + Err(RoverClientError::ExpectedFederatedGraph { + graph_ref: input.graph_ref, + can_operation_convert: true, + }) + } + } + _ => Err(RoverClientError::InvalidGraphRef), + } + } else { + Err(RoverClientError::GraphNotFound { + graph_ref: input.graph_ref, + }) + } +} + +#[cfg(test)] +mod tests { + use serde_json::json; + + use crate::shared::GraphRef; + + use super::*; + + #[test] + fn get_services_from_response_data_works() { + let sdl = "extend type User @key(fields: \"id\") {\n id: ID! @external\n age: Int\n}\n" + .to_string(); + let url = "http://my.subgraph.com".to_string(); + let input = mock_input(); + let json_response = json!({ + "variant": { + "__typename": "GraphVariant", + "subgraphs": [ + { + "name": "accounts", + "url": &url, + "activePartialSchema": { + "sdl": &sdl + } + }, + ] + } + }); + let data: SubgraphFetchAllResponseData = serde_json::from_value(json_response).unwrap(); + let expected_subgraph = Subgraph::builder() + .url(url) + .sdl(sdl) + .name("accounts".to_string()) + .build(); + let output = get_subgraphs_from_response_data(input, data); + + assert!(output.is_ok()); + assert_eq!(output.unwrap(), vec![expected_subgraph]); + } + + #[test] + fn get_services_from_response_data_errs_with_no_variant() { + let json_response = json!({ "variant": null }); + let data: SubgraphFetchAllResponseData = serde_json::from_value(json_response).unwrap(); + let output = get_subgraphs_from_response_data(mock_input(), data); + assert!(output.is_err()); + } + + fn mock_input() -> SubgraphFetchAllInput { + let graph_ref = GraphRef { + name: "mygraph".to_string(), + variant: "current".to_string(), + }; + + SubgraphFetchAllInput { graph_ref } + } +} diff --git a/crates/rover-client/src/operations/subgraph/fetch_all/types.rs b/crates/rover-client/src/operations/subgraph/fetch_all/types.rs new file mode 100644 index 000000000..7462dd6ee --- /dev/null +++ b/crates/rover-client/src/operations/subgraph/fetch_all/types.rs @@ -0,0 +1,31 @@ +use buildstructor::Builder; +use derive_getters::Getters; + +use crate::shared::GraphRef; + +use super::runner::subgraph_fetch_all_query; + +pub(crate) type SubgraphFetchAllResponseData = subgraph_fetch_all_query::ResponseData; +pub(crate) type SubgraphFetchAllGraphVariant = + subgraph_fetch_all_query::SubgraphFetchAllQueryVariant; +pub(crate) type QueryVariables = subgraph_fetch_all_query::Variables; + +#[derive(Debug, Clone, Eq, PartialEq)] +pub struct SubgraphFetchAllInput { + pub graph_ref: GraphRef, +} + +impl From for QueryVariables { + fn from(input: SubgraphFetchAllInput) -> Self { + Self { + graph_ref: input.graph_ref.to_string(), + } + } +} + +#[derive(Clone, Builder, Debug, Eq, Getters, PartialEq)] +pub struct Subgraph { + name: String, + url: Option, + sdl: String, +} diff --git a/crates/rover-client/src/operations/subgraph/mod.rs b/crates/rover-client/src/operations/subgraph/mod.rs index a9f057c56..61c110f2b 100644 --- a/crates/rover-client/src/operations/subgraph/mod.rs +++ b/crates/rover-client/src/operations/subgraph/mod.rs @@ -10,6 +10,9 @@ pub mod check; /// "subgraph fetch" command execution pub mod fetch; +/// "subgraph fetch_all" command execution +pub mod fetch_all; + /// "subgraph publish" command execution pub mod publish; diff --git a/src/utils/supergraph_config.rs b/src/utils/supergraph_config.rs index 2f8e1f35a..5fce3bfd5 100644 --- a/src/utils/supergraph_config.rs +++ b/src/utils/supergraph_config.rs @@ -8,11 +8,11 @@ use apollo_federation_types::config::{ use apollo_parser::{cst, Parser}; use rayon::iter::{IntoParallelIterator, ParallelIterator}; -use rover_client::blocking::GraphQLClient; +use rover_client::blocking::{GraphQLClient, StudioClient}; use rover_client::operations::subgraph; use rover_client::operations::subgraph::fetch::SubgraphFetchInput; +use rover_client::operations::subgraph::fetch_all::SubgraphFetchAllInput; use rover_client::operations::subgraph::introspect::SubgraphIntrospectInput; -use rover_client::operations::subgraph::list::SubgraphListInput; use rover_client::operations::subgraph::{fetch, introspect}; use rover_client::shared::GraphRef; use rover_client::RoverClientError; @@ -32,30 +32,25 @@ pub struct RemoteSubgraphs(SupergraphConfig); impl RemoteSubgraphs { /// Fetches [`RemoteSubgraphs`] from Studio pub fn fetch( - client_config: &StudioClientConfig, - profile_opt: &ProfileOpt, + client: &StudioClient, federation_version: &FederationVersion, graph_ref: &GraphRef, ) -> RoverResult { - let client = &client_config.get_authenticated_client(profile_opt)?; - - let subgraphs = subgraph::list::run( - SubgraphListInput { + let subgraphs = subgraph::fetch_all::run( + SubgraphFetchAllInput { graph_ref: graph_ref.clone(), }, client, )?; let subgraphs = subgraphs - .subgraphs .iter() .map(|subgraph| { ( - subgraph.name.clone(), + subgraph.name().clone(), SubgraphConfig { - routing_url: subgraph.url.clone(), - schema: SchemaSource::Subgraph { - graphref: graph_ref.clone().to_string(), - subgraph: subgraph.name.clone(), + routing_url: subgraph.url().clone(), + schema: SchemaSource::Sdl { + sdl: subgraph.sdl().clone(), }, }, ) @@ -81,12 +76,14 @@ pub fn get_supergraph_config( ) -> Result, RoverError> { // Read in Remote subgraphs let remote_subgraphs = match graph_ref { - Some(graph_ref) => Some(RemoteSubgraphs::fetch( - &client_config, - profile_opt, - federation_version, - graph_ref, - )?), + Some(graph_ref) => { + let studio_client = client_config.get_authenticated_client(profile_opt)?; + Some(RemoteSubgraphs::fetch( + &studio_client, + federation_version, + graph_ref, + )?) + } None => None, }; @@ -116,6 +113,268 @@ pub fn get_supergraph_config( Ok(supergraph_config) } +#[cfg(test)] +mod test_get_supergraph_config { + use std::fs::File; + use std::io::Write; + use std::path::PathBuf; + use std::str::FromStr; + + use apollo_federation_types::config::FederationVersion; + use camino::Utf8PathBuf; + use httpmock::MockServer; + use indoc::indoc; + use rstest::{fixture, rstest}; + use semver::Version; + use serde_json::{json, Value}; + use speculoos::assert_that; + use speculoos::prelude::OptionAssertions; + + use houston::Config; + use rover_client::shared::GraphRef; + + use crate::options::ProfileOpt; + use crate::utils::client::{ClientBuilder, StudioClientConfig}; + use crate::utils::parsers::FileDescriptorType; + use crate::utils::supergraph_config::get_supergraph_config; + + #[fixture] + #[once] + fn home_dir() -> Utf8PathBuf { + tempfile::tempdir() + .unwrap() + .path() + .to_path_buf() + .try_into() + .unwrap() + } + + #[fixture] + #[once] + fn api_key() -> String { + uuid::Uuid::new_v4().as_simple().to_string() + } + + #[fixture] + fn config(home_dir: &Utf8PathBuf, api_key: &String) -> Config { + Config::new(Some(home_dir), Some(api_key.to_string())).unwrap() + } + + #[fixture] + fn profile_opt() -> ProfileOpt { + ProfileOpt { + profile_name: "profile".to_string(), + } + } + + #[fixture] + #[once] + fn latest_fed2_version() -> FederationVersion { + let d = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("latest_plugin_versions.json"); + let fp = File::open(d).expect("could not open version file"); + let raw_version_file: Value = serde_json::from_reader(fp).expect("malformed JSON"); + let raw_version = raw_version_file + .get("supergraph") + .unwrap() + .get("versions") + .unwrap() + .get("latest-2") + .unwrap() + .as_str() + .unwrap(); + let version = Version::from_str(&raw_version.replace("v", "")).unwrap(); + FederationVersion::ExactFedTwo(version) + } + + #[rstest] + #[case::no_subgraphs_at_all(None, None, None)] + #[case::only_remote_subgraphs(Some(String::from("products")), None, Some(vec![(String::from("products"), String::from("remote"))]))] + #[case::only_local_subgraphs(None, Some(String::from("pandas")), Some(vec![(String::from("pandas"), String::from("local"))]))] + #[case::both_local_and_remote_subgraphs(Some(String::from("products")), Some(String::from("pandas")), Some(vec![(String::from("pandas"), String::from("local")), (String::from("products"), String::from("remote"))]))] + #[case::local_takes_precedence(Some(String::from("pandas")), Some(String::from("pandas")), Some(vec![(String::from("pandas"), String::from("local"))]))] + fn test_get_supergraph_config( + config: Config, + profile_opt: ProfileOpt, + latest_fed2_version: &FederationVersion, + #[case] remote_subgraph: Option, + #[case] local_subgraph: Option, + #[case] expected: Option>, + ) { + let server = MockServer::start(); + let sdl = "extend type User @key(fields: \"id\") {\n id: ID! @external\n age: Int\n}\n" + .to_string(); + let graphref = if let Some(name) = remote_subgraph { + let variant = String::from("current"); + let graphref_raw = format!("{name}@{variant}"); + let url = format!("http://{}.remote.com", name); + server.mock(|when, then| { + let body = json!({ + "data": { + "variant": { + "__typename": "GraphVariant", + "subgraphs": [ + { + "name": name, + "url": url, + "activePartialSchema": { + "sdl": sdl + } + } + ] + } + } + }); + when.method(httpmock::Method::POST) + .path("/") + .json_body_obj(&json!({ + "query": indoc!{ + r#" + query SubgraphFetchAllQuery($graph_ref: ID!) { + variant(ref: $graph_ref) { + __typename + ... on GraphVariant { + subgraphs { + name + url + activePartialSchema { + sdl + } + } + } + } + } + "# + }, + "variables": { + "graph_ref": graphref_raw, + }, + "operationName": "SubgraphFetchAllQuery" + })); + then.status(200) + .header("content-type", "application/json") + .json_body(body); + }); + + server.mock(|when, then| { + let body = json!({ + "data": { + "graph": { + "variant": { + "subgraphs": [ + { + "name": name + } + ] + } + } + } + }); + when.method(httpmock::Method::POST) + .path("/") + .json_body_obj(&json!({ + "query": indoc!{ + r#" + query IsFederatedGraph($graph_id: ID!, $variant: String!) { + graph(id: $graph_id) { + variant(name: $variant) { + subgraphs { + name + } + } + } + } + "# + }, + "variables": { + "graph_id": name, + "variant": "current" + }, + "operationName": "IsFederatedGraph" + })); + then.status(200) + .header("content-type", "application/json") + .json_body(body); + }); + Some(GraphRef::new(name, Some(variant)).unwrap()) + } else { + None + }; + + let studio_client_config = StudioClientConfig::new( + Some(server.base_url()), + config, + false, + ClientBuilder::default(), + ); + + let actual_result = if let Some(name) = local_subgraph { + let supergraph_config = format!( + indoc! { + r#" + federation_version: {} + subgraphs: + {}: + routing_url: http://{}.local.com + schema: + sdl: "{}" + "# + }, + latest_fed2_version.to_string(), + name, + name, + sdl.escape_default() + ); + let mut supergraph_config_path = + tempfile::NamedTempFile::new().expect("Could not create temporary file"); + supergraph_config_path + .as_file_mut() + .write_all(&supergraph_config.into_bytes()) + .expect("Could not write to temporary file"); + + get_supergraph_config( + &graphref, + &Some(FileDescriptorType::File( + Utf8PathBuf::from_path_buf(supergraph_config_path.path().to_path_buf()) + .unwrap(), + )), + latest_fed2_version, + studio_client_config, + &profile_opt, + ) + .expect("Could not construct SupergraphConfig") + } else { + get_supergraph_config( + &graphref, + &None, + latest_fed2_version, + studio_client_config, + &profile_opt, + ) + .expect("Could not construct SupergraphConfig") + }; + + if expected.is_none() { + assert_that!(actual_result).is_none() + } else { + assert_that(&actual_result).is_some(); + for (idx, subgraph) in actual_result + .unwrap() + .get_subgraph_definitions() + .unwrap() + .iter() + .enumerate() + { + let expected_result = expected.as_ref().unwrap(); + assert_that!(subgraph.name).is_equal_to(&expected_result[idx].0); + assert_that!(subgraph.url).is_equal_to(format!( + "http://{}.{}.com", + expected_result[idx].0, expected_result[idx].1 + )) + } + } + } +} + pub(crate) fn resolve_supergraph_yaml( unresolved_supergraph_yaml: &FileDescriptorType, client_config: StudioClientConfig, @@ -735,7 +994,11 @@ type _Service {\n sdl: String\n}"#; } #[rstest] - fn test_subgraph_studio_resolution(profile_opt: ProfileOpt, config: Config) -> Result<()> { + fn test_subgraph_studio_resolution( + profile_opt: ProfileOpt, + config: Config, + latest_fed2_version: &FederationVersion, + ) -> Result<()> { let graph_id = "testgraph"; let variant = "current"; let graphref = format!("{}@{}", graph_id, variant); @@ -837,7 +1100,7 @@ type _Service {\n sdl: String\n}"#; let supergraph_config = format!( indoc! {r#" - federation_version: 2 + federation_version: {} subgraphs: products: schema: @@ -845,7 +1108,7 @@ type _Service {\n sdl: String\n}"#; subgraph: products "# }, - graphref + latest_fed2_version, graphref ); let studio_client_config = StudioClientConfig::new( @@ -863,7 +1126,7 @@ type _Service {\n sdl: String\n}"#; let unresolved_supergraph_config = FileDescriptorType::File(supergraph_config_path.path().to_path_buf().try_into()?); - let resolved_config = super::resolve_supergraph_yaml( + let resolved_config = resolve_supergraph_yaml( &unresolved_supergraph_config, studio_client_config, &profile_opt, From 2a277b5a7629e8067d6dcbdb690e4d6e68af078c Mon Sep 17 00:00:00 2001 From: jonathanrainer Date: Tue, 30 Jul 2024 16:45:39 +0100 Subject: [PATCH 6/8] ROVER-69 Update to account for #2004 --- src/utils/supergraph_config.rs | 42 ---------------------------------- 1 file changed, 42 deletions(-) diff --git a/src/utils/supergraph_config.rs b/src/utils/supergraph_config.rs index 5fce3bfd5..34fb48196 100644 --- a/src/utils/supergraph_config.rs +++ b/src/utils/supergraph_config.rs @@ -1057,47 +1057,6 @@ type _Service {\n sdl: String\n}"#; .json_body(body); }); - let is_federated_mock = server.mock(|when, then| { - let body = json!({ - "data": { - "graph": { - "variant": { - "subgraphs": [ - { - "name": "products" - } - ] - } - } - } - }); - when.method(httpmock::Method::POST) - .path("/") - .json_body_obj(&json!({ - "query": indoc!{ - r#" - query IsFederatedGraph($graph_id: ID!, $variant: String!) { - graph(id: $graph_id) { - variant(name: $variant) { - subgraphs { - name - } - } - } - } - "# - }, - "variables": { - "graph_id": graph_id, - "variant": variant - }, - "operationName": "IsFederatedGraph" - })); - then.status(200) - .header("content-type", "application/json") - .json_body(body); - }); - let supergraph_config = format!( indoc! {r#" federation_version: {} @@ -1135,7 +1094,6 @@ type _Service {\n sdl: String\n}"#; assert_that!(resolved_config).is_ok(); let resolved_config = resolved_config.unwrap(); - is_federated_mock.assert_hits(1); subgraph_fetch_mock.assert_hits(1); let subgraphs = resolved_config.into_iter().collect::>(); From 3b2b205edb1e4d62a757e97c4bedfae41376b9cf Mon Sep 17 00:00:00 2001 From: jonathanrainer Date: Wed, 31 Jul 2024 08:47:24 +0100 Subject: [PATCH 7/8] ROVER-69 Fix argument parsing --- crates/rover-std/src/emoji.rs | 6 ++- src/command/supergraph/compose/do_compose.rs | 42 +++++++++++--------- src/utils/supergraph_config.rs | 18 +++++++-- 3 files changed, 42 insertions(+), 24 deletions(-) diff --git a/crates/rover-std/src/emoji.rs b/crates/rover-std/src/emoji.rs index 4e131bb87..49920bf0b 100644 --- a/crates/rover-std/src/emoji.rs +++ b/crates/rover-std/src/emoji.rs @@ -9,6 +9,7 @@ pub enum Emoji { Hourglass, Listen, Memo, + Merge, New, Note, Person, @@ -30,9 +31,10 @@ impl Emoji { match self { Action => "🎬 ", Compose => "🎢 ", - Hourglass => "βŒ› ", + Hourglass => "βŒ› ", Listen => "πŸ‘‚ ", Memo => "πŸ“ ", + Merge => "β›™ ", New => "🐀 ", Note => "πŸ—’οΈ ", Person => "πŸ§‘ ", @@ -42,7 +44,7 @@ impl Emoji { Sparkle => "✨ ", Start => "πŸ›« ", Stop => "βœ‹ ", - Success => "βœ… ", + Success => "βœ… ", Warn => "⚠️ ", Watch => "πŸ‘€ ", Web => "πŸ•ΈοΈ ", diff --git a/src/command/supergraph/compose/do_compose.rs b/src/command/supergraph/compose/do_compose.rs index a85f846da..0aac71456 100644 --- a/src/command/supergraph/compose/do_compose.rs +++ b/src/command/supergraph/compose/do_compose.rs @@ -8,12 +8,12 @@ use apollo_federation_types::{ config::{FederationVersion, PluginVersion}, }; use camino::Utf8PathBuf; -use clap::Parser; +use clap::{Args, Parser}; use serde::Serialize; use rover_client::shared::GraphRef; use rover_client::RoverClientError; -use rover_std::{Emoji, Style}; +use rover_std::Emoji; use crate::utils::supergraph_config::get_supergraph_config; use crate::utils::{client::StudioClientConfig, parsers::FileDescriptorType}; @@ -32,15 +32,13 @@ pub struct Compose { opts: SupergraphComposeOpts, } -#[derive(Debug, Serialize, Parser)] -pub struct SupergraphComposeOpts { - #[clap(flatten)] - pub plugin_opts: PluginOpts, - +#[derive(Args, Debug, Serialize)] +#[group(required = true)] +pub struct SupergraphConfigSource { /// The relative path to the supergraph configuration file. You can pass `-` to use stdin instead of a file. #[serde(skip_serializing)] - #[arg(long = "config")] - supergraph_yaml: FileDescriptorType, + #[arg(long = "config", conflicts_with = "graph_ref")] + supergraph_yaml: Option, /// A [`GraphRef`] that is accessible in Apollo Studio. /// This is used to initialize your supergraph with the values contained in this variant. @@ -48,8 +46,17 @@ pub struct SupergraphComposeOpts { /// This is analogous to providing a supergraph.yaml file with references to your graph variant in studio. /// /// If used in conjunction with `--config`, the values presented in the supergraph.yaml will take precedence over these values. - #[arg(long = "graph-ref")] + #[arg(long = "graph-ref", conflicts_with = "supergraph_yaml")] graph_ref: Option, +} + +#[derive(Debug, Serialize, Parser)] +pub struct SupergraphComposeOpts { + #[clap(flatten)] + pub plugin_opts: PluginOpts, + + #[clap(flatten)] + pub supergraph_config_source: SupergraphConfigSource, /// The version of Apollo Federation to use for composition #[arg(long = "federation-version")] @@ -61,9 +68,11 @@ impl Compose { Self { opts: SupergraphComposeOpts { plugin_opts: compose_opts, - supergraph_yaml: FileDescriptorType::File("RAM".into()), - graph_ref: None, federation_version: Some(LatestFedTwo), + supergraph_config_source: SupergraphConfigSource { + supergraph_yaml: Some(FileDescriptorType::File("RAM".into())), + graph_ref: None, + }, }, } } @@ -103,14 +112,9 @@ impl Compose { override_install_path: Option, client_config: StudioClientConfig, ) -> RoverResult { - eprintln!( - "{}resolving SDL for subgraphs defined in {}", - Emoji::Hourglass, - Style::Path.paint(self.opts.supergraph_yaml.to_string()) - ); let mut supergraph_config = get_supergraph_config( - &self.opts.graph_ref, - &Some(self.opts.supergraph_yaml.clone()), + &self.opts.supergraph_config_source.graph_ref, + &self.opts.supergraph_config_source.supergraph_yaml.clone(), &self.opts.federation_version.clone().unwrap_or(LatestFedTwo), client_config.clone(), &self.opts.plugin_opts.profile, diff --git a/src/utils/supergraph_config.rs b/src/utils/supergraph_config.rs index 34fb48196..ea929d3e2 100644 --- a/src/utils/supergraph_config.rs +++ b/src/utils/supergraph_config.rs @@ -16,7 +16,7 @@ use rover_client::operations::subgraph::introspect::SubgraphIntrospectInput; use rover_client::operations::subgraph::{fetch, introspect}; use rover_client::shared::GraphRef; use rover_client::RoverClientError; -use rover_std::{Fs, Style}; +use rover_std::{Emoji, Fs, Style}; use crate::options::ProfileOpt; use crate::utils::client::StudioClientConfig; @@ -78,17 +78,27 @@ pub fn get_supergraph_config( let remote_subgraphs = match graph_ref { Some(graph_ref) => { let studio_client = client_config.get_authenticated_client(profile_opt)?; - Some(RemoteSubgraphs::fetch( + let remote_subgraphs = Some(RemoteSubgraphs::fetch( &studio_client, federation_version, graph_ref, - )?) + )?); + eprintln!( + "{}retrieving subgraphs remotely from {}", + Emoji::Hourglass, + graph_ref + ); + remote_subgraphs } None => None, }; // Read in Local Supergraph Config let supergraph_config = if let Some(file_descriptor) = &supergraph_config_path { + eprintln!( + "{}resolving SDL for subgraphs defined in supergraph schema file", + Emoji::Hourglass, + ); Some(resolve_supergraph_yaml( file_descriptor, client_config, @@ -104,12 +114,14 @@ pub fn get_supergraph_config( Some(supergraph_config) => { let mut merged_supergraph_config = remote_subgraphs.inner().clone(); merged_supergraph_config.merge_subgraphs(&supergraph_config); + eprintln!("{}merging supergraph schema files", Emoji::Merge,); Some(merged_supergraph_config) } None => Some(remote_subgraphs.inner().clone()), }, None => supergraph_config, }; + eprintln!("{}supergraph config loaded successfully", Emoji::Success,); Ok(supergraph_config) } From e08de0d6a1eb47e953fde583b83dcbc88dccefde Mon Sep 17 00:00:00 2001 From: jonathanrainer Date: Wed, 31 Jul 2024 10:14:22 +0100 Subject: [PATCH 8/8] ROVER-69 Respond to PR comments --- Cargo.lock | 1 + crates/rover-client/Cargo.toml | 1 + .../operations/subgraph/fetch_all/runner.rs | 64 +++++++++---------- .../operations/subgraph/fetch_all/types.rs | 10 +++ src/utils/supergraph_config.rs | 37 ++++------- 5 files changed, 56 insertions(+), 57 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index af4e551f0..ee4f57a84 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4680,6 +4680,7 @@ dependencies = [ "regex", "reqwest 0.12.5", "rover-std", + "rstest", "semver", "serde", "serde_json", diff --git a/crates/rover-client/Cargo.toml b/crates/rover-client/Cargo.toml index a69a5ccd3..ca0f8b8ee 100644 --- a/crates/rover-client/Cargo.toml +++ b/crates/rover-client/Cargo.toml @@ -57,3 +57,4 @@ indoc = { workspace = true} httpmock = { workspace = true } pretty_assertions = { workspace = true } strip-ansi-escapes = { workspace = true } +rstest = { workspace = true } diff --git a/crates/rover-client/src/operations/subgraph/fetch_all/runner.rs b/crates/rover-client/src/operations/subgraph/fetch_all/runner.rs index 33af7da27..64cab5f8c 100644 --- a/crates/rover-client/src/operations/subgraph/fetch_all/runner.rs +++ b/crates/rover-client/src/operations/subgraph/fetch_all/runner.rs @@ -1,7 +1,6 @@ use graphql_client::*; use crate::blocking::StudioClient; -use crate::operations::config::is_federated::{self, IsFederatedInput}; use crate::RoverClientError; use super::types::*; @@ -34,50 +33,48 @@ fn get_subgraphs_from_response_data( input: SubgraphFetchAllInput, response_data: SubgraphFetchAllResponseData, ) -> Result, RoverClientError> { - if let Some(maybe_variant) = response_data.variant { - match maybe_variant { - SubgraphFetchAllGraphVariant::GraphVariant(variant) => { - if let Some(subgraphs) = variant.subgraphs { - Ok(subgraphs - .into_iter() - .map(|subgraph| { - Subgraph::builder() - .name(subgraph.name.clone()) - .and_url(subgraph.url) - .sdl(subgraph.active_partial_schema.sdl) - .build() - }) - .collect()) - } else { - Err(RoverClientError::ExpectedFederatedGraph { - graph_ref: input.graph_ref, - can_operation_convert: true, - }) - } - } - _ => Err(RoverClientError::InvalidGraphRef), - } - } else { - Err(RoverClientError::GraphNotFound { + match response_data.variant { + None => Err(RoverClientError::GraphNotFound { graph_ref: input.graph_ref, - }) + }), + Some(SubgraphFetchAllGraphVariant::GraphVariant(variant)) => variant.subgraphs.map_or_else( + || { + Err(RoverClientError::ExpectedFederatedGraph { + graph_ref: input.graph_ref, + can_operation_convert: true, + }) + }, + |subgraphs| { + Ok(subgraphs + .into_iter() + .map(|subgraph| { + Subgraph::builder() + .name(subgraph.name.clone()) + .and_url(subgraph.url) + .sdl(subgraph.active_partial_schema.sdl) + .build() + }) + .collect()) + }, + ), + _ => Err(RoverClientError::InvalidGraphRef), } } #[cfg(test)] mod tests { + use rstest::{fixture, rstest}; use serde_json::json; use crate::shared::GraphRef; use super::*; - #[test] - fn get_services_from_response_data_works() { + #[rstest] + fn get_services_from_response_data_works(#[from(mock_input)] input: SubgraphFetchAllInput) { let sdl = "extend type User @key(fields: \"id\") {\n id: ID! @external\n age: Int\n}\n" .to_string(); let url = "http://my.subgraph.com".to_string(); - let input = mock_input(); let json_response = json!({ "variant": { "__typename": "GraphVariant", @@ -104,14 +101,15 @@ mod tests { assert_eq!(output.unwrap(), vec![expected_subgraph]); } - #[test] - fn get_services_from_response_data_errs_with_no_variant() { + #[rstest] + fn get_services_from_response_data_errs_with_no_variant(mock_input: SubgraphFetchAllInput) { let json_response = json!({ "variant": null }); let data: SubgraphFetchAllResponseData = serde_json::from_value(json_response).unwrap(); - let output = get_subgraphs_from_response_data(mock_input(), data); + let output = get_subgraphs_from_response_data(mock_input, data); assert!(output.is_err()); } + #[fixture] fn mock_input() -> SubgraphFetchAllInput { let graph_ref = GraphRef { name: "mygraph".to_string(), diff --git a/crates/rover-client/src/operations/subgraph/fetch_all/types.rs b/crates/rover-client/src/operations/subgraph/fetch_all/types.rs index 7462dd6ee..67b511a58 100644 --- a/crates/rover-client/src/operations/subgraph/fetch_all/types.rs +++ b/crates/rover-client/src/operations/subgraph/fetch_all/types.rs @@ -1,3 +1,4 @@ +use apollo_federation_types::config::{SchemaSource, SubgraphConfig}; use buildstructor::Builder; use derive_getters::Getters; @@ -29,3 +30,12 @@ pub struct Subgraph { url: Option, sdl: String, } + +impl From for SubgraphConfig { + fn from(value: Subgraph) -> Self { + Self { + routing_url: value.url, + schema: SchemaSource::Sdl { sdl: value.sdl }, + } + } +} diff --git a/src/utils/supergraph_config.rs b/src/utils/supergraph_config.rs index ea929d3e2..69702dfca 100644 --- a/src/utils/supergraph_config.rs +++ b/src/utils/supergraph_config.rs @@ -43,18 +43,8 @@ impl RemoteSubgraphs { client, )?; let subgraphs = subgraphs - .iter() - .map(|subgraph| { - ( - subgraph.name().clone(), - SubgraphConfig { - routing_url: subgraph.url().clone(), - schema: SchemaSource::Sdl { - sdl: subgraph.sdl().clone(), - }, - }, - ) - }) + .into_iter() + .map(|subgraph| (subgraph.name().clone(), subgraph.into())) .collect(); let supergraph_config = SupergraphConfig::new(subgraphs, Some(federation_version.clone())); let remote_subgraphs = RemoteSubgraphs(supergraph_config); @@ -97,7 +87,7 @@ pub fn get_supergraph_config( let supergraph_config = if let Some(file_descriptor) = &supergraph_config_path { eprintln!( "{}resolving SDL for subgraphs defined in supergraph schema file", - Emoji::Hourglass, + Emoji::Hourglass ); Some(resolve_supergraph_yaml( file_descriptor, @@ -109,17 +99,16 @@ pub fn get_supergraph_config( }; // Merge Remote and Local Supergraph Configs - let supergraph_config = match remote_subgraphs { - Some(remote_subgraphs) => match supergraph_config { - Some(supergraph_config) => { - let mut merged_supergraph_config = remote_subgraphs.inner().clone(); - merged_supergraph_config.merge_subgraphs(&supergraph_config); - eprintln!("{}merging supergraph schema files", Emoji::Merge,); - Some(merged_supergraph_config) - } - None => Some(remote_subgraphs.inner().clone()), - }, - None => supergraph_config, + let supergraph_config = match (remote_subgraphs, supergraph_config) { + (Some(remote_subgraphs), Some(supergraph_config)) => { + let mut merged_supergraph_config = remote_subgraphs.inner().clone(); + merged_supergraph_config.merge_subgraphs(&supergraph_config); + eprintln!("{}merging supergraph schema files", Emoji::Merge); + Some(merged_supergraph_config) + } + (Some(remote_subgraphs), None) => Some(remote_subgraphs.inner().clone()), + (None, Some(supergraph_config)) => Some(supergraph_config), + (None, None) => None, }; eprintln!("{}supergraph config loaded successfully", Emoji::Success,); Ok(supergraph_config)