From d9990c4bc184fcd98f208e647bc8d722ac1751a2 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 29 Jul 2024 12:13:46 +0200 Subject: [PATCH 1/2] Parse supergraph schema early to avoid re-parsing --- apollo-router/src/axum_factory/tests.rs | 14 +++-- apollo-router/src/orbiter/mod.rs | 2 +- .../cost_calculator/static_cost.rs | 4 +- .../src/plugins/include_subgraph_errors.rs | 4 +- .../src/plugins/record_replay/record.rs | 2 +- apollo-router/src/plugins/test.rs | 16 +++--- .../src/plugins/traffic_shaping/mod.rs | 5 +- .../src/query_planner/bridge_query_planner.rs | 47 +++++++++-------- .../bridge_query_planner_pool.rs | 19 ++----- apollo-router/src/router_factory.rs | 32 +++++------- apollo-router/src/spec/schema.rs | 41 +++++++-------- apollo-router/src/state_machine.rs | 20 ++++---- apollo-router/src/test_harness.rs | 4 +- .../src/uplink/license_enforcement.rs | 51 +++++++++---------- ...ent_directive_arg_version_in_range.graphql | 45 ++++++++++++++++ ...directive_arg_version_out_of_range.graphql | 35 +++++++++++++ ..._enforcement_spec_version_in_range.graphql | 21 ++++++++ ...orcement_spec_version_out_of_range.graphql | 21 ++++++++ .../src/uplink/testdata/unix_socket.graphql | 5 ++ 19 files changed, 246 insertions(+), 142 deletions(-) diff --git a/apollo-router/src/axum_factory/tests.rs b/apollo-router/src/axum_factory/tests.rs index dde3852032..5cd6b1a549 100644 --- a/apollo-router/src/axum_factory/tests.rs +++ b/apollo-router/src/axum_factory/tests.rs @@ -85,6 +85,7 @@ use crate::services::RouterResponse; use crate::services::SupergraphResponse; use crate::services::MULTIPART_DEFER_ACCEPT; use crate::services::MULTIPART_DEFER_CONTENT_TYPE; +use crate::spec::Schema; use crate::test_harness::http_client; use crate::test_harness::http_client::MaybeMultipart; use crate::uplink::license_enforcement::LicenseState; @@ -2309,14 +2310,11 @@ async fn test_supergraph_timeout() { let conf: Arc = Arc::new(serde_json::from_value(config).unwrap()); let schema = include_str!("..//testdata/minimal_supergraph.graphql"); - let planner = BridgeQueryPlannerPool::new( - schema.to_string(), - conf.clone(), - NonZeroUsize::new(1).unwrap(), - ) - .await - .unwrap(); - let schema = planner.schema(); + let schema = Arc::new(Schema::parse(schema, &conf).unwrap()); + let planner = + BridgeQueryPlannerPool::new(schema.clone(), conf.clone(), NonZeroUsize::new(1).unwrap()) + .await + .unwrap(); // we do the entire supergraph rebuilding instead of using `from_supergraph_mock_callback_and_configuration` // because we need the plugins to apply on the supergraph diff --git a/apollo-router/src/orbiter/mod.rs b/apollo-router/src/orbiter/mod.rs index ebd3383752..a326e15d48 100644 --- a/apollo-router/src/orbiter/mod.rs +++ b/apollo-router/src/orbiter/mod.rs @@ -97,7 +97,7 @@ impl RouterSuperServiceFactory for OrbiterRouterSuperServiceFactory { &'a mut self, is_telemetry_disabled: bool, configuration: Arc, - schema: String, + schema: Arc, previous_router: Option<&'a Self::RouterFactory>, extra_plugins: Option)>>, ) -> Result { diff --git a/apollo-router/src/plugins/demand_control/cost_calculator/static_cost.rs b/apollo-router/src/plugins/demand_control/cost_calculator/static_cost.rs index f84a4fcd0a..1156c6df7d 100644 --- a/apollo-router/src/plugins/demand_control/cost_calculator/static_cost.rs +++ b/apollo-router/src/plugins/demand_control/cost_calculator/static_cost.rs @@ -462,9 +462,9 @@ mod tests { async fn planned_cost(schema_str: &str, query_str: &str) -> f64 { let config: Arc = Arc::new(Default::default()); - let (_schema, query) = parse_schema_and_operation(schema_str, query_str, &config); + let (schema, query) = parse_schema_and_operation(schema_str, query_str, &config); - let mut planner = BridgeQueryPlanner::new(schema_str.to_string(), config.clone(), None) + let mut planner = BridgeQueryPlanner::new(schema.into(), config.clone(), None) .await .unwrap(); diff --git a/apollo-router/src/plugins/include_subgraph_errors.rs b/apollo-router/src/plugins/include_subgraph_errors.rs index 66ffa53917..56aae8045c 100644 --- a/apollo-router/src/plugins/include_subgraph_errors.rs +++ b/apollo-router/src/plugins/include_subgraph_errors.rs @@ -100,6 +100,7 @@ mod test { use crate::services::HasSchema; use crate::services::PluggableSupergraphServiceBuilder; use crate::services::SupergraphRequest; + use crate::spec::Schema; use crate::Configuration; static UNREDACTED_PRODUCT_RESPONSE: Lazy = Lazy::new(|| { @@ -191,8 +192,9 @@ mod test { let schema = include_str!("../../../apollo-router-benchmarks/benches/fixtures/supergraph.graphql"); + let schema = Schema::parse(schema, &Default::default()).unwrap(); let planner = BridgeQueryPlannerPool::new( - schema.to_string(), + schema.into(), Default::default(), NonZeroUsize::new(1).unwrap(), ) diff --git a/apollo-router/src/plugins/record_replay/record.rs b/apollo-router/src/plugins/record_replay/record.rs index e821b016b2..f9dc97b52a 100644 --- a/apollo-router/src/plugins/record_replay/record.rs +++ b/apollo-router/src/plugins/record_replay/record.rs @@ -67,7 +67,7 @@ impl Plugin for Record { enabled: init.config.enabled, supergraph_sdl: init.supergraph_sdl.clone(), storage_path: storage_path.clone().into(), - schema: Arc::new(Schema::parse(&init.supergraph_sdl, &Default::default())?), + schema: Arc::new(Schema::parse_arc(init.supergraph_sdl, &Default::default())?), }; if init.config.enabled { diff --git a/apollo-router/src/plugins/test.rs b/apollo-router/src/plugins/test.rs index c31ae9acc7..523e07f253 100644 --- a/apollo-router/src/plugins/test.rs +++ b/apollo-router/src/plugins/test.rs @@ -18,6 +18,7 @@ use crate::services::http; use crate::services::router; use crate::services::subgraph; use crate::services::supergraph; +use crate::spec::Schema; use crate::Configuration; use crate::Notify; @@ -88,17 +89,16 @@ impl PluginTestHarness { .unwrap_or(Value::Object(Default::default())); let (supergraph_sdl, parsed_schema, subgraph_schemas) = if let Some(schema) = schema { - let planner = BridgeQueryPlanner::new(schema.to_string(), Arc::new(config), None) + let schema = Schema::parse(schema, &config).unwrap(); + let sdl = schema.raw_sdl.clone(); + let supergraph = schema.supergraph_schema().clone(); + let planner = BridgeQueryPlanner::new(schema.into(), Arc::new(config), None) .await .unwrap(); - ( - schema.to_string(), - planner.schema().supergraph_schema().clone(), - planner.subgraph_schemas(), - ) + (sdl, supergraph, planner.subgraph_schemas()) } else { ( - "".to_string(), + "".to_string().into(), Valid::assume_valid(apollo_compiler::Schema::new()), Default::default(), ) @@ -106,7 +106,7 @@ impl PluginTestHarness { let plugin_init = PluginInit::builder() .config(config_for_plugin.clone()) - .supergraph_sdl(Arc::new(supergraph_sdl)) + .supergraph_sdl(supergraph_sdl) .supergraph_schema(Arc::new(parsed_schema)) .subgraph_schemas(subgraph_schemas) .notify(Notify::default()) diff --git a/apollo-router/src/plugins/traffic_shaping/mod.rs b/apollo-router/src/plugins/traffic_shaping/mod.rs index a3ddda0e6d..e4e2eabfa1 100644 --- a/apollo-router/src/plugins/traffic_shaping/mod.rs +++ b/apollo-router/src/plugins/traffic_shaping/mod.rs @@ -484,6 +484,7 @@ mod test { use crate::services::PluggableSupergraphServiceBuilder; use crate::services::SupergraphRequest; use crate::services::SupergraphResponse; + use crate::spec::Schema; use crate::Configuration; static EXPECTED_RESPONSE: Lazy = Lazy::new(|| { @@ -568,14 +569,14 @@ mod test { .unwrap(); let config = Arc::new(config); + let schema = Arc::new(Schema::parse(schema, &config).unwrap()); let planner = BridgeQueryPlannerPool::new( - schema.to_string(), + schema.clone(), config.clone(), NonZeroUsize::new(1).unwrap(), ) .await .unwrap(); - let schema = planner.schema(); let subgraph_schemas = planner.subgraph_schemas(); let mut builder = diff --git a/apollo-router/src/query_planner/bridge_query_planner.rs b/apollo-router/src/query_planner/bridge_query_planner.rs index eb5990e43e..f2348ef11b 100644 --- a/apollo-router/src/query_planner/bridge_query_planner.rs +++ b/apollo-router/src/query_planner/bridge_query_planner.rs @@ -323,11 +323,10 @@ impl PlannerMode { impl BridgeQueryPlanner { pub(crate) async fn new( - schema: String, + schema: Arc, configuration: Arc, old_planner: Option>>, ) -> Result { - let schema = Schema::parse(&schema, &configuration)?; let planner = PlannerMode::new(&schema, &configuration, old_planner).await?; let subgraph_schemas = Arc::new(planner.subgraphs().await?); @@ -353,7 +352,7 @@ impl BridgeQueryPlanner { Ok(Self { planner, - schema: Arc::new(schema), + schema, subgraph_schemas, introspection, enable_authorization_directives, @@ -369,6 +368,7 @@ impl BridgeQueryPlanner { .clone() } + #[cfg(test)] pub(crate) fn schema(&self) -> Arc { self.schema.clone() } @@ -988,13 +988,12 @@ mod tests { #[test(tokio::test)] async fn federation_versions() { async { - let _planner = BridgeQueryPlanner::new( - include_str!("../testdata/minimal_supergraph.graphql").into(), - Default::default(), - None, - ) - .await - .unwrap(); + let sdl = include_str!("../testdata/minimal_supergraph.graphql"); + let config = Arc::default(); + let schema = Schema::parse(sdl, &config).unwrap(); + let _planner = BridgeQueryPlanner::new(schema.into(), config, None) + .await + .unwrap(); assert_gauge!( "apollo.router.supergraph.federation", @@ -1006,13 +1005,12 @@ mod tests { .await; async { - let _planner = BridgeQueryPlanner::new( - include_str!("../testdata/minimal_fed2_supergraph.graphql").into(), - Default::default(), - None, - ) - .await - .unwrap(); + let sdl = include_str!("../testdata/minimal_fed2_supergraph.graphql"); + let config = Arc::default(); + let schema = Schema::parse(sdl, &config).unwrap(); + let _planner = BridgeQueryPlanner::new(schema.into(), config, None) + .await + .unwrap(); assert_gauge!( "apollo.router.supergraph.federation", @@ -1026,10 +1024,10 @@ mod tests { #[test(tokio::test)] async fn empty_query_plan_should_be_a_planner_error() { - let schema = Schema::parse(EXAMPLE_SCHEMA, &Default::default()).unwrap(); + let schema = Arc::new(Schema::parse(EXAMPLE_SCHEMA, &Default::default()).unwrap()); let query = include_str!("testdata/unknown_introspection_query.graphql"); - let planner = BridgeQueryPlanner::new(EXAMPLE_SCHEMA.to_string(), Default::default(), None) + let planner = BridgeQueryPlanner::new(schema.clone(), Default::default(), None) .await .unwrap(); @@ -1128,10 +1126,10 @@ mod tests { configuration.supergraph.introspection = true; let configuration = Arc::new(configuration); - let planner = - BridgeQueryPlanner::new(EXAMPLE_SCHEMA.to_string(), configuration.clone(), None) - .await - .unwrap(); + let schema = Schema::parse(EXAMPLE_SCHEMA, &configuration).unwrap(); + let planner = BridgeQueryPlanner::new(schema.into(), configuration.clone(), None) + .await + .unwrap(); macro_rules! s { ($query: expr) => { @@ -1436,7 +1434,8 @@ mod tests { configuration.supergraph.introspection = true; let configuration = Arc::new(configuration); - let planner = BridgeQueryPlanner::new(schema.to_string(), configuration.clone(), None) + let schema = Schema::parse(schema, &configuration).unwrap(); + let planner = BridgeQueryPlanner::new(schema.into(), configuration.clone(), None) .await .unwrap(); diff --git a/apollo-router/src/query_planner/bridge_query_planner_pool.rs b/apollo-router/src/query_planner/bridge_query_planner_pool.rs index 5da661c4d2..19e80b539e 100644 --- a/apollo-router/src/query_planner/bridge_query_planner_pool.rs +++ b/apollo-router/src/query_planner/bridge_query_planner_pool.rs @@ -40,16 +40,16 @@ pub(crate) struct BridgeQueryPlannerPool { impl BridgeQueryPlannerPool { pub(crate) async fn new( - sdl: String, + schema: Arc, configuration: Arc, size: NonZeroUsize, ) -> Result { - Self::new_from_planners(Default::default(), sdl, configuration, size).await + Self::new_from_planners(Default::default(), schema, configuration, size).await } pub(crate) async fn new_from_planners( old_planners: Vec>>, - schema: String, + schema: Arc, configuration: Arc, size: NonZeroUsize, ) -> Result { @@ -63,12 +63,12 @@ impl BridgeQueryPlannerPool { let mut old_planners_iterator = old_planners.into_iter(); (0..size.into()).for_each(|_| { - let sdl = schema.clone(); + let schema = schema.clone(); let configuration = configuration.clone(); let old_planner = old_planners_iterator.next(); join_set.spawn(async move { - BridgeQueryPlanner::new(sdl, configuration, old_planner).await + BridgeQueryPlanner::new(schema, configuration, old_planner).await }); }); @@ -80,15 +80,6 @@ impl BridgeQueryPlannerPool { bridge_query_planners.push(bridge_query_planner); } - let schema = bridge_query_planners - .first() - .ok_or_else(|| { - ServiceBuildError::QueryPlannerError(QueryPlannerError::PoolProcessing( - "There should be at least 1 Query Planner service in pool".to_string(), - )) - })? - .schema(); - let subgraph_schemas = bridge_query_planners .first() .ok_or_else(|| { diff --git a/apollo-router/src/router_factory.rs b/apollo-router/src/router_factory.rs index ca1dd4cb7b..bb9badd19f 100644 --- a/apollo-router/src/router_factory.rs +++ b/apollo-router/src/router_factory.rs @@ -141,7 +141,7 @@ pub(crate) trait RouterSuperServiceFactory: Send + Sync + 'static { &'a mut self, is_telemetry_disabled: bool, configuration: Arc, - schema: String, + schema: Arc, previous_router: Option<&'a Self::RouterFactory>, extra_plugins: Option)>>, ) -> Result; @@ -159,7 +159,7 @@ impl RouterSuperServiceFactory for YamlRouterFactory { &'a mut self, _is_telemetry_disabled: bool, configuration: Arc, - schema: String, + schema: Arc, previous_router: Option<&'a Self::RouterFactory>, extra_plugins: Option)>>, ) -> Result { @@ -179,17 +179,13 @@ impl RouterSuperServiceFactory for YamlRouterFactory { .get("telemetry") .cloned(); if let Some(plugin_config) = &mut telemetry_config { - inject_schema_id(Some(&Schema::schema_id(&schema)), plugin_config); + inject_schema_id(Some(&schema.schema_id), plugin_config); match factory .create_instance( PluginInit::builder() .config(plugin_config.clone()) - .supergraph_sdl(Arc::new(schema.clone())) - .supergraph_schema(Arc::new( - apollo_compiler::validation::Valid::assume_valid( - apollo_compiler::Schema::new(), - ), - )) + .supergraph_sdl(schema.raw_sdl.clone()) + .supergraph_schema(Arc::new(schema.supergraph_schema().clone())) .notify(configuration.notify.clone()) .build(), ) @@ -227,7 +223,7 @@ impl YamlRouterFactory { async fn inner_create<'a>( &'a mut self, configuration: Arc, - schema: String, + schema: Arc, previous_router: Option<&'a RouterCreator>, initial_telemetry_plugin: Option>, extra_plugins: Option)>>, @@ -302,7 +298,7 @@ impl YamlRouterFactory { pub(crate) async fn inner_create_supergraph<'a>( &'a mut self, configuration: Arc, - schema: String, + schema: Arc, previous_supergraph: Option<&'a SupergraphCreator>, initial_telemetry_plugin: Option>, extra_plugins: Option)>>, @@ -339,7 +335,7 @@ impl YamlRouterFactory { }; let schema_changed = previous_supergraph - .map(|supergraph_creator| supergraph_creator.schema().raw_sdl.as_ref() == &schema) + .map(|supergraph_creator| supergraph_creator.schema().raw_sdl == schema.raw_sdl) .unwrap_or_default(); let config_changed = previous_supergraph @@ -519,16 +515,11 @@ fn load_certs(certificates: &str) -> io::Result> { /// not meant to be used directly pub async fn create_test_service_factory_from_yaml(schema: &str, configuration: &str) { let config: Configuration = serde_yaml::from_str(configuration).unwrap(); + let schema = Arc::new(Schema::parse(schema, &config).unwrap()); let is_telemetry_disabled = false; let service = YamlRouterFactory - .create( - is_telemetry_disabled, - Arc::new(config), - schema.to_string(), - None, - None, - ) + .create(is_telemetry_disabled, Arc::new(config), schema, None, None) .await; assert_eq!( service.map(|_| ()).unwrap_err().to_string().as_str(), @@ -974,13 +965,14 @@ mod test { async fn create_service(config: Configuration) -> Result<(), BoxError> { let schema = include_str!("testdata/supergraph.graphql"); + let schema = Schema::parse(schema, &config)?; let is_telemetry_disabled = false; let service = YamlRouterFactory .create( is_telemetry_disabled, Arc::new(config), - schema.to_string(), + Arc::new(schema), None, None, ) diff --git a/apollo-router/src/spec/schema.rs b/apollo-router/src/spec/schema.rs index 07f13f746a..05546910ca 100644 --- a/apollo-router/src/spec/schema.rs +++ b/apollo-router/src/spec/schema.rs @@ -5,7 +5,6 @@ use std::str::FromStr; use std::sync::Arc; use std::time::Instant; -use apollo_compiler::ast; use apollo_compiler::schema::Implementers; use apollo_compiler::validation::Valid; use apollo_compiler::Name; @@ -38,32 +37,30 @@ pub(crate) struct Schema { pub(crate) struct ApiSchema(pub(crate) ValidFederationSchema); impl Schema { - pub(crate) fn parse_ast(sdl: &str) -> Result { + pub(crate) fn parse(raw_sdl: &str, config: &Configuration) -> Result { + Self::parse_arc(raw_sdl.to_owned().into(), config) + } + + pub(crate) fn parse_arc( + raw_sdl: Arc, + config: &Configuration, + ) -> Result { + let start = Instant::now(); let mut parser = apollo_compiler::parser::Parser::new(); - let result = parser.parse_ast(sdl, "schema.graphql"); + let result = parser.parse_ast(raw_sdl.as_ref(), "schema.graphql"); // Trace log recursion limit data let recursion_limit = parser.recursion_reached(); tracing::trace!(?recursion_limit, "recursion limit data"); - result.map_err(|invalid| { - SchemaError::Parse(ParseErrors { - errors: invalid.errors, - }) - }) - } - - pub(crate) fn parse_compiler_schema( - sdl: &str, - ) -> Result, SchemaError> { - Self::parse_ast(sdl)? + let definitions = result + .map_err(|invalid| { + SchemaError::Parse(ParseErrors { + errors: invalid.errors, + }) + })? .to_schema_validate() - .map_err(|errors| SchemaError::Validate(errors.into())) - } - - pub(crate) fn parse(sdl: &str, config: &Configuration) -> Result { - let start = Instant::now(); - let definitions = Self::parse_compiler_schema(sdl)?; + .map_err(|errors| SchemaError::Validate(errors.into()))?; let mut subgraphs = HashMap::new(); // TODO: error if not found? @@ -111,7 +108,7 @@ impl Schema { let implementers_map = definitions.implementers_map(); let supergraph = Supergraph::from_schema(definitions)?; - let schema_id = Arc::new(Schema::schema_id(sdl)); + let schema_id = Arc::new(Schema::schema_id(&raw_sdl)); let api_schema = supergraph .to_api_schema(ApiSchemaOptions { @@ -125,7 +122,7 @@ impl Schema { })?; Ok(Schema { - raw_sdl: Arc::new(sdl.to_owned()), + raw_sdl, supergraph, subgraphs, implementers_map, diff --git a/apollo-router/src/state_machine.rs b/apollo-router/src/state_machine.rs index 0a141e1669..e3ce6c3a67 100644 --- a/apollo-router/src/state_machine.rs +++ b/apollo-router/src/state_machine.rs @@ -308,7 +308,7 @@ impl State { server_handle: &mut Option, previous_router_service_factory: Option<&FA::RouterFactory>, configuration: Arc, - schema: Arc, + sdl: Arc, license: LicenseState, listen_addresses_guard: &mut OwnedRwLockWriteGuard, mut all_connections_stopped_signals: Vec>, @@ -317,12 +317,12 @@ impl State { S: HttpServerFactory, FA: RouterSuperServiceFactory, { - let report = { - let ast = Schema::parse_ast(&schema) - .map_err(|e| ServiceCreationError(e.to_string().into()))?; - // Check the license - LicenseEnforcementReport::build(&configuration, &ast) - }; + let schema = Arc::new( + Schema::parse_arc(sdl.clone(), &configuration) + .map_err(|e| ServiceCreationError(e.to_string().into()))?, + ); + // Check the license + let report = LicenseEnforcementReport::build(&configuration, &schema); match license { LicenseState::Licensed => { @@ -362,7 +362,7 @@ impl State { .create( state_machine.is_telemetry_disabled, configuration.clone(), - schema.to_string(), + schema, previous_router_service_factory, None, ) @@ -422,7 +422,7 @@ impl State { Ok(Running { configuration, _metrics: metrics, - schema, + schema: sdl, license, server_handle: Some(server_handle), router_service_factory, @@ -1119,7 +1119,7 @@ mod tests { &'a mut self, is_telemetry_disabled: bool, configuration: Arc, - schema: String, + schema: Arc, previous_router_service_factory: Option<&'a MockMyRouterFactory>, extra_plugins: Option)>>, ) -> Result; diff --git a/apollo-router/src/test_harness.rs b/apollo-router/src/test_harness.rs index 7e921ffaf3..a0b5384489 100644 --- a/apollo-router/src/test_harness.rs +++ b/apollo-router/src/test_harness.rs @@ -34,6 +34,7 @@ use crate::services::subgraph; use crate::services::supergraph; use crate::services::HasSchema; use crate::services::SupergraphCreator; +use crate::spec::Schema; use crate::uplink::license_enforcement::LicenseState; /// Mocks for services the Apollo Router must integrate with. @@ -291,10 +292,11 @@ impl<'a> TestHarness<'a> { let config = builder.configuration.unwrap_or_default(); let canned_schema = include_str!("../testing_schema.graphql"); let schema = builder.schema.unwrap_or(canned_schema); + let schema = Arc::new(Schema::parse(schema, &config)?); let supergraph_creator = YamlRouterFactory .inner_create_supergraph( config.clone(), - schema.to_string(), + schema, None, None, Some(builder.extra_plugins), diff --git a/apollo-router/src/uplink/license_enforcement.rs b/apollo-router/src/uplink/license_enforcement.rs index 21ab68d970..ea75f03f3c 100644 --- a/apollo-router/src/uplink/license_enforcement.rs +++ b/apollo-router/src/uplink/license_enforcement.rs @@ -11,9 +11,8 @@ use std::time::Duration; use std::time::SystemTime; use std::time::UNIX_EPOCH; -use apollo_compiler::ast::Definition; use apollo_compiler::schema::Directive; -use apollo_compiler::Node; +use apollo_compiler::schema::ExtendedType; use buildstructor::Builder; use displaydoc::Display; use itertools::Itertools; @@ -31,6 +30,7 @@ use thiserror::Error; use url::Url; use crate::plugins::authentication::convert_key_algorithm; +use crate::spec::Schema; use crate::spec::LINK_AS_ARGUMENT; use crate::spec::LINK_DIRECTIVE_NAME; use crate::spec::LINK_URL_ARGUMENT; @@ -101,7 +101,7 @@ struct ParsedLinkSpec { impl ParsedLinkSpec { fn from_link_directive( - link_directive: &Node, + link_directive: &Directive, ) -> Option> { link_directive .argument_by_name(LINK_URL_ARGUMENT) @@ -157,7 +157,7 @@ impl LicenseEnforcementReport { pub(crate) fn build( configuration: &Configuration, - schema: &apollo_compiler::ast::Document, + schema: &Schema, ) -> LicenseEnforcementReport { LicenseEnforcementReport { restricted_config_in_use: Self::validate_configuration( @@ -197,14 +197,14 @@ impl LicenseEnforcementReport { } fn validate_schema( - schema: &apollo_compiler::ast::Document, + schema: &Schema, schema_restrictions: &Vec, ) -> Vec { let link_specs = schema - .definitions - .iter() - .filter_map(|def| def.as_schema_definition()) - .flat_map(|def| def.directives.get_all(LINK_DIRECTIVE_NAME)) + .supergraph_schema() + .schema_definition + .directives + .get_all(LINK_DIRECTIVE_NAME) .filter_map(|link| { ParsedLinkSpec::from_link_directive(link).map(|maybe_spec| { maybe_spec.ok().map(|spec| (spec.spec_url.to_owned(), spec)) @@ -214,18 +214,8 @@ impl LicenseEnforcementReport { let mut schema_violations: Vec = Vec::new(); - for subgraph_url in schema - .definitions - .iter() - .filter_map(|def| def.as_enum_type_definition()) - .filter(|def| def.name == "join__Graph") - .flat_map(|def| def.values.iter()) - .flat_map(|val| val.directives.iter()) - .filter(|d| d.name == "join__graph") - .filter_map(|dir| (dir.arguments.iter().find(|arg| arg.name == "url"))) - .filter_map(|arg| arg.value.as_str()) - { - if subgraph_url.starts_with("unix://") { + for (_subgraph_name, subgraph_url) in schema.subgraphs() { + if subgraph_url.scheme_str() == Some("unix") { schema_violations.push(SchemaViolation::DirectiveArgument { url: "https://specs.apollo.dev/join/v0.3".to_string(), name: "join__Graph".to_string(), @@ -262,16 +252,19 @@ impl LicenseEnforcementReport { if version_req.matches(&link_spec.version) { let directive_name = link_spec.directive_name(name); if schema - .definitions - .iter() + .supergraph_schema() + .types + .values() .flat_map(|def| match def { // To traverse additional directive locations, add match arms for the respective definition types required. // As of writing this, this is only implemented for finding usages of progressive override on object type fields, but it can be extended to other directive locations trivially. - Definition::ObjectTypeDefinition(object_type_def) => { - let directives_on_object = - object_type_def.directives.get_all(&directive_name); + ExtendedType::Object(object_type_def) => { + let directives_on_object = object_type_def + .directives + .get_all(&directive_name) + .map(|component| &component.node); let directives_on_fields = - object_type_def.fields.iter().flat_map(|field| { + object_type_def.fields.values().flat_map(|field| { field.directives.get_all(&directive_name) }); @@ -682,9 +675,11 @@ mod test { use crate::uplink::license_enforcement::OneOrMany; use crate::Configuration; + #[track_caller] fn check(router_yaml: &str, supergraph_schema: &str) -> LicenseEnforcementReport { let config = Configuration::from_str(router_yaml).expect("router config must be valid"); - let schema = Schema::parse_ast(supergraph_schema).expect("supergraph schema must be valid"); + let schema = + Schema::parse(supergraph_schema, &config).expect("supergraph schema must be valid"); LicenseEnforcementReport::build(&config, &schema) } diff --git a/apollo-router/src/uplink/testdata/schema_enforcement_directive_arg_version_in_range.graphql b/apollo-router/src/uplink/testdata/schema_enforcement_directive_arg_version_in_range.graphql index 563cc14c98..381d34ddd5 100644 --- a/apollo-router/src/uplink/testdata/schema_enforcement_directive_arg_version_in_range.graphql +++ b/apollo-router/src/uplink/testdata/schema_enforcement_directive_arg_version_in_range.graphql @@ -4,6 +4,51 @@ schema query: Query } + +directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE + +directive @join__field( + graph: join__Graph + requires: join__FieldSet + provides: join__FieldSet + type: String + external: Boolean + override: String + usedOverridden: Boolean + overrideLabel: String +) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION + +directive @join__graph(name: String!, url: String!) on ENUM_VALUE + +directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE + +directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR + +directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION + +directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA + +scalar join__FieldSet + +enum join__Graph { + SUBGRAPH1 @join__graph(name: "products", url: "http://localhost:4001/") + SUBGRAPH2 @join__graph(name: "reviews", url: "http://localhost:4002/") +} + +scalar link__Import + +enum link__Purpose { + """ + `SECURITY` features provide metadata necessary to securely resolve fields. + """ + SECURITY + + """ + `EXECUTION` features provide metadata necessary for operation execution. + """ + EXECUTION +} + type Query @join__type(graph: SUBGRAPH1) @join__type(graph: SUBGRAPH2) { t: T @join__field(graph: SUBGRAPH1) } diff --git a/apollo-router/src/uplink/testdata/schema_enforcement_directive_arg_version_out_of_range.graphql b/apollo-router/src/uplink/testdata/schema_enforcement_directive_arg_version_out_of_range.graphql index a62a24953a..586124f47a 100644 --- a/apollo-router/src/uplink/testdata/schema_enforcement_directive_arg_version_out_of_range.graphql +++ b/apollo-router/src/uplink/testdata/schema_enforcement_directive_arg_version_out_of_range.graphql @@ -4,6 +4,41 @@ schema query: Query } +directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE + +directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION + +directive @join__graph(name: String!, url: String!) on ENUM_VALUE + +directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE + +directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR + +directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION + +directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA + +scalar join__FieldSet + +enum join__Graph { + SUBGRAPH1 @join__graph(name: "products", url: "http://localhost:4001/") + SUBGRAPH2 @join__graph(name: "reviews", url: "http://localhost:4002/") +} + +scalar link__Import + +enum link__Purpose { + """ + `SECURITY` features provide metadata necessary to securely resolve fields. + """ + SECURITY + + """ + `EXECUTION` features provide metadata necessary for operation execution. + """ + EXECUTION +} + type Query @join__type(graph: SUBGRAPH1) @join__type(graph: SUBGRAPH2) { t: T @join__field(graph: SUBGRAPH1) } diff --git a/apollo-router/src/uplink/testdata/schema_enforcement_spec_version_in_range.graphql b/apollo-router/src/uplink/testdata/schema_enforcement_spec_version_in_range.graphql index a265252bd4..ded63469ef 100644 --- a/apollo-router/src/uplink/testdata/schema_enforcement_spec_version_in_range.graphql +++ b/apollo-router/src/uplink/testdata/schema_enforcement_spec_version_in_range.graphql @@ -1,5 +1,26 @@ schema @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) @link(url: "https://specs.apollo.dev/authenticated/v0.1", for: SECURITY) { query: Query } + +directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA + +enum link__Purpose { + """ + `SECURITY` features provide metadata necessary to securely resolve fields. + """ + SECURITY + + """ + `EXECUTION` features provide metadata necessary for operation execution. + """ + EXECUTION +} + +scalar link__Import + +type Query { + field: Int +} diff --git a/apollo-router/src/uplink/testdata/schema_enforcement_spec_version_out_of_range.graphql b/apollo-router/src/uplink/testdata/schema_enforcement_spec_version_out_of_range.graphql index 5266cfeb47..231cb51c48 100644 --- a/apollo-router/src/uplink/testdata/schema_enforcement_spec_version_out_of_range.graphql +++ b/apollo-router/src/uplink/testdata/schema_enforcement_spec_version_out_of_range.graphql @@ -1,5 +1,26 @@ schema @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) @link(url: "https://specs.apollo.dev/authenticated/v0.2", for: SECURITY) { query: Query } + +directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA + +enum link__Purpose { + """ + `SECURITY` features provide metadata necessary to securely resolve fields. + """ + SECURITY + + """ + `EXECUTION` features provide metadata necessary for operation execution. + """ + EXECUTION +} + +scalar link__Import + +type Query { + field: Int +} diff --git a/apollo-router/src/uplink/testdata/unix_socket.graphql b/apollo-router/src/uplink/testdata/unix_socket.graphql index 910a221a90..5ea6507f76 100644 --- a/apollo-router/src/uplink/testdata/unix_socket.graphql +++ b/apollo-router/src/uplink/testdata/unix_socket.graphql @@ -20,6 +20,11 @@ directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA +directive @authenticated on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM + +scalar federation__Scope +directive @requiresScopes(scopes: [[federation__Scope!]!]!) on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM + scalar join__FieldSet enum join__Graph { From 09f600597b111b1f97b32af7d19e4da17702c1fb Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 29 Jul 2024 17:11:22 +0200 Subject: [PATCH 2/2] Windows --- apollo-router/src/uplink/license_enforcement.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/apollo-router/src/uplink/license_enforcement.rs b/apollo-router/src/uplink/license_enforcement.rs index ea75f03f3c..2d77fb3683 100644 --- a/apollo-router/src/uplink/license_enforcement.rs +++ b/apollo-router/src/uplink/license_enforcement.rs @@ -725,6 +725,7 @@ mod test { } #[test] + #[cfg(not(windows))] // http::uri::Uri parsing appears to reject unix:// on Windows fn test_restricted_unix_socket_via_schema() { let report = check( include_str!("testdata/oss.router.yaml"),