From de2e61b7b15d81dfec6bbdd668ca843aaf88df11 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Tue, 19 Oct 2021 12:02:24 -0500 Subject: [PATCH] feat(composition): default to routing url from graph registry (#873) --- crates/rover-client/package-lock.json | 62 ++++++++++--------- .../subgraph/fetch/fetch_query.graphql | 1 + .../src/operations/subgraph/fetch/runner.rs | 31 ++++++---- .../rover-client/src/shared/fetch_response.rs | 2 +- src/command/output.rs | 6 +- src/command/supergraph/compose/do_compose.rs | 40 +++++++----- 6 files changed, 83 insertions(+), 59 deletions(-) diff --git a/crates/rover-client/package-lock.json b/crates/rover-client/package-lock.json index 8b0b401d2..4c239314a 100644 --- a/crates/rover-client/package-lock.json +++ b/crates/rover-client/package-lock.json @@ -1172,18 +1172,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/eslint/node_modules/glob-parent": { - "version": "6.0.2", - "resolved": "https://registry.npmjs.org/glob-parent/-/glob-parent-6.0.2.tgz", - "integrity": "sha512-XxwI8EOhVQgWp6iDL+3b0r86f4d6AX6zSU55HfB4ydCEuXLXc5FcYeOu+nnGftS4TEju/11rt4KJPTMgbfmv4A==", - "dev": true, - "dependencies": { - "is-glob": "^4.0.3" - }, - "engines": { - "node": ">=10.13.0" - } - }, "node_modules/eslint/node_modules/globals": { "version": "13.11.0", "resolved": "https://registry.npmjs.org/globals/-/globals-13.11.0.tgz", @@ -1338,6 +1326,18 @@ "node": ">=8" } }, + "node_modules/fast-glob/node_modules/glob-parent": { + "version": "5.1.2", + "resolved": "https://registry.npmjs.org/glob-parent/-/glob-parent-5.1.2.tgz", + "integrity": "sha512-AOIgSQCepiJYwP3ARnGx+5VnTu2HBYdzbGP45eLw1vr3zB3vZLeyed1sC9hnbcOc9/SrMyM5RPQrkGz4aS9Zow==", + "dev": true, + "dependencies": { + "is-glob": "^4.0.1" + }, + "engines": { + "node": ">= 6" + } + }, "node_modules/fast-json-stable-stringify": { "version": "2.1.0", "resolved": "https://registry.npmjs.org/fast-json-stable-stringify/-/fast-json-stable-stringify-2.1.0.tgz", @@ -1449,15 +1449,15 @@ } }, "node_modules/glob-parent": { - "version": "5.1.2", - "resolved": "https://registry.npmjs.org/glob-parent/-/glob-parent-5.1.2.tgz", - "integrity": "sha512-AOIgSQCepiJYwP3ARnGx+5VnTu2HBYdzbGP45eLw1vr3zB3vZLeyed1sC9hnbcOc9/SrMyM5RPQrkGz4aS9Zow==", + "version": "6.0.2", + "resolved": "https://registry.npmjs.org/glob-parent/-/glob-parent-6.0.2.tgz", + "integrity": "sha512-XxwI8EOhVQgWp6iDL+3b0r86f4d6AX6zSU55HfB4ydCEuXLXc5FcYeOu+nnGftS4TEju/11rt4KJPTMgbfmv4A==", "dev": true, "dependencies": { - "is-glob": "^4.0.1" + "is-glob": "^4.0.3" }, "engines": { - "node": ">= 6" + "node": ">=10.13.0" } }, "node_modules/globals": { @@ -3438,15 +3438,6 @@ "integrity": "sha512-TtpcNJ3XAzx3Gq8sWRzJaVajRs0uVxA2YAkdb1jm2YkPz4G6egUFAyA3n5vtEIZefPk5Wa4UXbKuS5fKkJWdgA==", "dev": true }, - "glob-parent": { - "version": "6.0.2", - "resolved": "https://registry.npmjs.org/glob-parent/-/glob-parent-6.0.2.tgz", - "integrity": "sha512-XxwI8EOhVQgWp6iDL+3b0r86f4d6AX6zSU55HfB4ydCEuXLXc5FcYeOu+nnGftS4TEju/11rt4KJPTMgbfmv4A==", - "dev": true, - "requires": { - "is-glob": "^4.0.3" - } - }, "globals": { "version": "13.11.0", "resolved": "https://registry.npmjs.org/globals/-/globals-13.11.0.tgz", @@ -3588,6 +3579,17 @@ "glob-parent": "^5.1.2", "merge2": "^1.3.0", "micromatch": "^4.0.4" + }, + "dependencies": { + "glob-parent": { + "version": "5.1.2", + "resolved": "https://registry.npmjs.org/glob-parent/-/glob-parent-5.1.2.tgz", + "integrity": "sha512-AOIgSQCepiJYwP3ARnGx+5VnTu2HBYdzbGP45eLw1vr3zB3vZLeyed1sC9hnbcOc9/SrMyM5RPQrkGz4aS9Zow==", + "dev": true, + "requires": { + "is-glob": "^4.0.1" + } + } } }, "fast-json-stable-stringify": { @@ -3683,12 +3685,12 @@ } }, "glob-parent": { - "version": "5.1.2", - "resolved": "https://registry.npmjs.org/glob-parent/-/glob-parent-5.1.2.tgz", - "integrity": "sha512-AOIgSQCepiJYwP3ARnGx+5VnTu2HBYdzbGP45eLw1vr3zB3vZLeyed1sC9hnbcOc9/SrMyM5RPQrkGz4aS9Zow==", + "version": "6.0.2", + "resolved": "https://registry.npmjs.org/glob-parent/-/glob-parent-6.0.2.tgz", + "integrity": "sha512-XxwI8EOhVQgWp6iDL+3b0r86f4d6AX6zSU55HfB4ydCEuXLXc5FcYeOu+nnGftS4TEju/11rt4KJPTMgbfmv4A==", "dev": true, "requires": { - "is-glob": "^4.0.1" + "is-glob": "^4.0.3" } }, "globals": { diff --git a/crates/rover-client/src/operations/subgraph/fetch/fetch_query.graphql b/crates/rover-client/src/operations/subgraph/fetch/fetch_query.graphql index 631177641..9cf9abb28 100644 --- a/crates/rover-client/src/operations/subgraph/fetch/fetch_query.graphql +++ b/crates/rover-client/src/operations/subgraph/fetch/fetch_query.graphql @@ -5,6 +5,7 @@ query SubgraphFetchQuery($graph_id: ID!, $variant: String!) { ... on FederatedImplementingServices { services { name + url activePartialSchema { sdl } diff --git a/crates/rover-client/src/operations/subgraph/fetch/runner.rs b/crates/rover-client/src/operations/subgraph/fetch/runner.rs index dd0a1e2ac..380421ff1 100644 --- a/crates/rover-client/src/operations/subgraph/fetch/runner.rs +++ b/crates/rover-client/src/operations/subgraph/fetch/runner.rs @@ -35,11 +35,13 @@ fn get_sdl_from_response_data( ) -> Result { let graph_ref = input.graph_ref.clone(); let service_list = get_services_from_response_data(graph_ref, response_data)?; - let sdl_contents = get_sdl_for_service(&input.subgraph, service_list)?; + let subgraph = get_subgraph(&input.subgraph, service_list)?; Ok(FetchResponse { sdl: Sdl { - contents: sdl_contents, - r#type: SdlType::Subgraph, + contents: subgraph.sdl, + r#type: SdlType::Subgraph { + routing_url: subgraph.url, + }, }, }) } @@ -74,17 +76,22 @@ fn get_services_from_response_data( } } -fn get_sdl_for_service( - subgraph_name: &str, - services: ServiceList, -) -> Result { +struct Subgraph { + url: Option, + sdl: String, +} + +fn get_subgraph(subgraph_name: &str, services: ServiceList) -> Result { // find the right service by name let service = services.iter().find(|svc| svc.name == subgraph_name); - // if there is a service, get it's active sdl, otherwise, error and list + // if there is a service, get its active sdl, otherwise, error and list // available services to fetch if let Some(service) = service { - Ok(service.active_partial_schema.sdl.clone()) + Ok(Subgraph { + url: service.url.clone(), + sdl: service.active_partial_schema.sdl.clone(), + }) } else { let valid_subgraphs: Vec = services.iter().map(|svc| svc.name.clone()).collect(); @@ -175,9 +182,9 @@ mod tests { } ]); let service_list: ServiceList = serde_json::from_value(json_service_list).unwrap(); - let output = get_sdl_for_service("accounts2", service_list); + let output = get_subgraph("accounts2", service_list); assert_eq!( - output.unwrap(), + output.unwrap().sdl, "extend type User @key(fields: \"id\") {\n id: ID! @external\n age: Int\n}\n" .to_string() ); @@ -200,7 +207,7 @@ mod tests { } ]); let service_list: ServiceList = serde_json::from_value(json_service_list).unwrap(); - let output = get_sdl_for_service("harambe-was-an-inside-job", service_list); + let output = get_subgraph("harambe-was-an-inside-job", service_list); assert!(output.is_err()); } diff --git a/crates/rover-client/src/shared/fetch_response.rs b/crates/rover-client/src/shared/fetch_response.rs index b5ddc80bb..feb434d0f 100644 --- a/crates/rover-client/src/shared/fetch_response.rs +++ b/crates/rover-client/src/shared/fetch_response.rs @@ -16,6 +16,6 @@ pub struct Sdl { #[serde(rename_all(serialize = "lowercase"))] pub enum SdlType { Graph, - Subgraph, + Subgraph { routing_url: Option }, Supergraph, } diff --git a/src/command/output.rs b/src/command/output.rs index 80c164141..460ff5b42 100644 --- a/src/command/output.rs +++ b/src/command/output.rs @@ -75,7 +75,7 @@ impl RoverOutput { } RoverOutput::FetchResponse(fetch_response) => { match fetch_response.sdl.r#type { - SdlType::Graph | SdlType::Subgraph => print_descriptor("SDL"), + SdlType::Graph | SdlType::Subgraph { .. } => print_descriptor("SDL"), SdlType::Supergraph => print_descriptor("Supergraph SDL"), } print_content(&fetch_response.sdl.contents); @@ -469,7 +469,9 @@ mod tests { let mock_fetch_response = FetchResponse { sdl: Sdl { contents: "sdl contents".to_string(), - r#type: SdlType::Subgraph, + r#type: SdlType::Subgraph { + routing_url: Some("http://localhost:8000/graphql".to_string()), + }, }, }; let actual_json: JsonOutput = RoverOutput::FetchResponse(mock_fetch_response).into(); diff --git a/src/command/supergraph/compose/do_compose.rs b/src/command/supergraph/compose/do_compose.rs index 48d86e84e..55f3db01c 100644 --- a/src/command/supergraph/compose/do_compose.rs +++ b/src/command/supergraph/compose/do_compose.rs @@ -66,6 +66,13 @@ pub(crate) fn get_subgraph_definitions( ) -> Result> { let mut subgraphs = Vec::new(); + let err_no_routing_url = || { + let err = anyhow!("No routing_url found for schema file."); + let mut err = RoverError::new(err); + err.set_suggestion(Suggestion::ValidComposeRoutingUrl); + err + }; + for (subgraph_name, subgraph_data) in &supergraph_config.subgraphs { match &subgraph_data.schema { SchemaSource::File { file } => { @@ -85,12 +92,10 @@ pub(crate) fn get_subgraph_definitions( err })?; - let url = &subgraph_data.routing_url.clone().ok_or_else(|| { - let err = anyhow!("No routing_url found for schema file."); - let mut err = RoverError::new(err); - err.set_suggestion(Suggestion::ValidComposeRoutingUrl); - err - })?; + let url = &subgraph_data + .routing_url + .clone() + .ok_or_else(err_no_routing_url)?; let subgraph_definition = SubgraphDefinition::new(subgraph_name, url, &schema); subgraphs.push(subgraph_definition); @@ -111,8 +116,8 @@ pub(crate) fn get_subgraph_definitions( )?; let schema = introspection_response.result; - // We don't require a routing_url for this variant of a schema, - // if none are provided, just use an empty string. + // 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() @@ -136,12 +141,19 @@ pub(crate) fn get_subgraph_definitions( &client, )?; - // We don't require a routing_url for this variant of a schema, - // if none are provided, just use an empty string. - // - // TODO: this should eventually get the url from the registry - // and use that when no routing_url is provided. - let url = &subgraph_data.routing_url.clone().unwrap_or_default(); + // We don't require a routing_url in config for this variant of a schema, + // if one isn't provided, just use the routing URL from the graph registry (if it exists). + let url = if let rover_client::shared::SdlType::Subgraph { + routing_url: Some(graph_registry_routing_url), + } = result.sdl.r#type + { + Ok(subgraph_data + .routing_url + .clone() + .unwrap_or(graph_registry_routing_url)) + } else { + Err(err_no_routing_url()) + }?; let subgraph_definition = SubgraphDefinition::new(subgraph_name, url, &result.sdl.contents);