diff --git a/src/utils/supergraph_config.rs b/src/utils/supergraph_config.rs index 492daf8dd1..c24c23e1ed 100644 --- a/src/utils/supergraph_config.rs +++ b/src/utils/supergraph_config.rs @@ -1,7 +1,8 @@ +use std::collections::BTreeMap; use std::str::FromStr; use anyhow::anyhow; -use apollo_federation_types::build::{BuildError, BuildErrors, SubgraphDefinition}; +use apollo_federation_types::build::{BuildError, BuildErrors}; use apollo_federation_types::config::{ FederationVersion, SchemaSource, SubgraphConfig, SupergraphConfig, }; @@ -86,7 +87,19 @@ pub async fn get_supergraph_config( // In this branch we get a completely resolved config, so all the references in it are // resolved to a concrete SDL that could be printed out to a user. This is what // `supergraph compose` uses. - Some(resolve_supergraph_yaml(file_descriptor, client_config, profile_opt).await?) + Some( + resolve_supergraph_yaml( + file_descriptor, + client_config, + profile_opt, + federation_version.is_none() + && remote_subgraphs + .as_ref() + .and_then(|it| it.inner().get_federation_version()) + .is_none(), + ) + .await?, + ) } else { // Alternatively, we might actually want a more dynamic object so that we can // set up watchers on the subgraph sources. This branch is what `rover dev` uses. @@ -642,13 +655,8 @@ pub(crate) async fn resolve_supergraph_yaml( unresolved_supergraph_yaml: &FileDescriptorType, client_config: StudioClientConfig, profile_opt: &ProfileOpt, + must_determine_federation_version: bool, ) -> RoverResult { - let err_no_routing_url = || { - let err = anyhow!("No routing_url found for schema file."); - let mut err = RoverError::new(err); - err.set_suggestion(RoverErrorSuggestion::ValidComposeRoutingUrl); - err - }; let err_invalid_graph_ref = || { let err = anyhow!("Invalid graph ref."); let mut err = RoverError::new(err); @@ -690,15 +698,7 @@ pub(crate) async fn resolve_supergraph_yaml( err.set_suggestion(RoverErrorSuggestion::ValidComposeFile); err }) - .and_then(|schema| { - subgraph_data - .routing_url - .clone() - .ok_or_else(err_no_routing_url) - .map(|url| { - SubgraphDefinition::new(subgraph_name.clone(), url, &schema) - }) - }) + .map(|schema| (subgraph_data.routing_url.clone(), schema)) } SchemaSource::SubgraphIntrospection { subgraph_url, @@ -726,13 +726,21 @@ pub(crate) async fn resolve_supergraph_yaml( .map(|introspection_response| { let schema = introspection_response.result; - // We don't require a routing_url in config for this variant of a schema, - // if one isn't provided, just use the URL they passed for introspection. - let url = &subgraph_data - .routing_url - .clone() - .unwrap_or_else(|| subgraph_url.to_string()); - SubgraphDefinition::new(subgraph_name.clone(), url, schema) + ( + // We don't require a routing_url in config for + // this variant of a schema, if one isn't + // provided, just use the URL they passed for + // introspection. (This does mean there's no way + // when combining `--graph-ref` and a config + // file to say "fetch the schema from + // introspection but use the routing URL from + // the graph" at the moment.) + subgraph_data + .routing_url + .clone() + .or_else(|| Some(subgraph_url.to_string())), + schema, + ) }) .map_err(RoverError::from) } @@ -770,25 +778,19 @@ pub(crate) async fn resolve_supergraph_yaml( routing_url: Some(graph_registry_routing_url), } = result.sdl.r#type { - let url = subgraph_data - .routing_url - .clone() - .unwrap_or(graph_registry_routing_url); - SubgraphDefinition::new( - subgraph_name.clone(), - url, - &result.sdl.contents, + ( + subgraph_data + .routing_url + .clone() + .or(Some(graph_registry_routing_url)), + result.sdl.contents, ) } else { panic!("whoops: rebase me"); } }) } - SchemaSource::Sdl { sdl } => subgraph_data - .routing_url - .clone() - .ok_or_else(err_no_routing_url) - .map(|url| SubgraphDefinition::new(subgraph_name.clone(), url, sdl)), + SchemaSource::Sdl { sdl } => Ok((subgraph_data.routing_url.clone(), sdl.clone())), }; Ok((cloned_subgraph_name, result)) }); @@ -796,14 +798,45 @@ pub(crate) async fn resolve_supergraph_yaml( let subgraph_definition_results = join_all(futs).await.into_iter(); let num_subgraphs = subgraph_definition_results.len(); - let mut subgraph_definitions = Vec::new(); - let mut subgraph_definition_errors = Vec::new(); + let mut subgraph_configs = BTreeMap::new(); + let mut subgraph_config_errors = Vec::new(); + + let mut fed_two_subgraph_names = Vec::new(); for res in subgraph_definition_results { match res { - Ok((subgraph_name, subgraph_definition_result)) => match subgraph_definition_result { - Ok(subgraph_definition) => subgraph_definitions.push(subgraph_definition), - Err(e) => subgraph_definition_errors.push((subgraph_name, e)), + Ok((subgraph_name, subgraph_config_result)) => match subgraph_config_result { + Ok((routing_url, sdl)) => { + subgraph_configs.insert( + subgraph_name.clone(), + SubgraphConfig { + routing_url, + schema: SchemaSource::Sdl { sdl: sdl.clone() }, + }, + ); + let parser = Parser::new(&sdl); + let parsed_ast = parser.parse(); + let doc = parsed_ast.document(); + 'definitions: for definition in doc.definitions() { + let maybe_directives = match definition { + cst::Definition::SchemaExtension(ext) => ext.directives(), + cst::Definition::SchemaDefinition(def) => def.directives(), + _ => None, + } + .map(|d| d.directives()); + if let Some(directives) = maybe_directives { + for directive in directives { + if let Some(directive_name) = directive.name() { + if "link" == directive_name.text() { + fed_two_subgraph_names.push(subgraph_name); + break 'definitions; + } + } + } + } + } + } + Err(e) => subgraph_config_errors.push((subgraph_name, e)), }, Err(err) => { eprintln!("err: {err}"); @@ -811,9 +844,9 @@ pub(crate) async fn resolve_supergraph_yaml( } } - if !subgraph_definition_errors.is_empty() { + if !subgraph_config_errors.is_empty() { let source = BuildErrors::from( - subgraph_definition_errors + subgraph_config_errors .iter() .map(|(subgraph_name, error)| { let mut message = error.message(); @@ -837,31 +870,8 @@ pub(crate) async fn resolve_supergraph_yaml( })); } - let mut resolved_supergraph_config: SupergraphConfig = subgraph_definitions.into(); - - let mut fed_two_subgraph_names = Vec::new(); - for subgraph_definition in resolved_supergraph_config.get_subgraph_definitions()? { - let parser = Parser::new(&subgraph_definition.sdl); - let parsed_ast = parser.parse(); - let doc = parsed_ast.document(); - for definition in doc.definitions() { - let maybe_directives = match definition { - cst::Definition::SchemaExtension(ext) => ext.directives(), - cst::Definition::SchemaDefinition(def) => def.directives(), - _ => None, - } - .map(|d| d.directives()); - if let Some(directives) = maybe_directives { - for directive in directives { - if let Some(directive_name) = directive.name() { - if "link" == directive_name.text() { - fed_two_subgraph_names.push(subgraph_definition.name.clone()); - } - } - } - } - } - } + let mut resolved_supergraph_config: SupergraphConfig = + SupergraphConfig::new(subgraph_configs, None); let print_inexact_warning = || { eprintln!("{} An exact {} was not specified in '{}'. Future versions of {} will fail without specifying an exact federation version. See {} for more information.", Style::WarningPrefix.paint("WARN:"), Style::Command.paint("federation_version"), &unresolved_supergraph_yaml, Style::Command.paint("`rover supergraph compose`"), Style::Link.paint("https://www.apollographql.com/docs/rover/commands/supergraphs#setting-a-composition-version")) @@ -891,14 +901,16 @@ pub(crate) async fn resolve_supergraph_yaml( // otherwise, set the version to what they set resolved_supergraph_config.set_federation_version(specified_federation_version) - } else if fed_two_subgraph_names.is_empty() { - // if they did not specify a version and no subgraphs contain `@link` directives, use Federation 1 - print_inexact_warning(); - resolved_supergraph_config.set_federation_version(FederationVersion::LatestFedOne) - } else { - // if they did not specify a version and at least one subgraph contains an `@link` directive, use Federation 2 - print_inexact_warning(); - resolved_supergraph_config.set_federation_version(FederationVersion::LatestFedTwo) + } else if must_determine_federation_version { + if fed_two_subgraph_names.is_empty() { + // if they did not specify a version and no subgraphs contain `@link` directives, use Federation 1 + print_inexact_warning(); + resolved_supergraph_config.set_federation_version(FederationVersion::LatestFedOne) + } else { + // if they did not specify a version and at least one subgraph contains an `@link` directive, use Federation 2 + print_inexact_warning(); + resolved_supergraph_config.set_federation_version(FederationVersion::LatestFedTwo) + } } Ok(resolved_supergraph_config) @@ -1035,7 +1047,8 @@ mod test_resolve_supergraph_yaml { assert!(resolve_supergraph_yaml( &FileDescriptorType::File(config_path), client_config, - &profile_opt + &profile_opt, + true ) .await .is_err()) @@ -1074,7 +1087,8 @@ subgraphs: assert!(resolve_supergraph_yaml( &FileDescriptorType::File(config_path), client_config, - &profile_opt + &profile_opt, + true ) .await .is_ok()) @@ -1117,6 +1131,7 @@ subgraphs: &FileDescriptorType::File(config_path), client_config, &profile_opt, + true, ) .await .unwrap() @@ -1201,6 +1216,7 @@ type _Service {\n sdl: String\n}"#; &unresolved_supergraph_config, client_config, &profile_opt, + true, ) .await; @@ -1273,6 +1289,7 @@ type _Service {\n sdl: String\n}"#; &unresolved_supergraph_config, client_config, &profile_opt, + true, ) .await; @@ -1395,6 +1412,7 @@ type _Service {\n sdl: String\n}"#; &unresolved_supergraph_config, studio_client_config, &profile_opt, + true, ) .await; @@ -1454,6 +1472,7 @@ type _Service {\n sdl: String\n}"#; &unresolved_supergraph_config, client_config, &profile_opt, + true, ) .await; @@ -1475,4 +1494,57 @@ type _Service {\n sdl: String\n}"#; Ok(()) } + + #[rstest] + #[case::must_determine_federation_version(true)] + #[case::no_need_to_determine_federation_version(false)] + #[tokio::test] + async fn test_subgraph_federation_version_default( + #[case] must_determine_federation_version: bool, + schema: String, + profile_opt: ProfileOpt, + client_config: StudioClientConfig, + ) -> Result<()> { + let supergraph_config = format!( + indoc! { + r#" + 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, + client_config, + &profile_opt, + must_determine_federation_version, + ) + .await; + + assert_that!(resolved_config).is_ok(); + let resolved_config = resolved_config.unwrap(); + assert_eq!( + resolved_config.get_federation_version(), + if must_determine_federation_version { + Some(FederationVersion::LatestFedOne) + } else { + None + } + ); + + Ok(()) + } }