diff --git a/.changesets/feat_geal_schema_hash_requires.md b/.changesets/feat_geal_schema_hash_requires.md new file mode 100644 index 0000000000..2c46a6da9b --- /dev/null +++ b/.changesets/feat_geal_schema_hash_requires.md @@ -0,0 +1,14 @@ +### reuse cached query plans across schema updates if possible ([Issue #4834](https://github.com/apollographql/router/issues/4834)) + +This extends the schema aware query hashing introduced in entity caching, to reduce the amount of work when reloading the router. That hash is designed to stay the same for a same query across schema updates if the update does not affect that query. If query planner cache warm up is configured, then it can reuse previous cache entries for which the hash does not change, which will reduce CPU usage and make reloads faster. + +This can be activated with the following option: + +```yaml title="router.yaml" +supergraph: + query_planning: + warmed_up_queries: 100 + experimental_reuse_query_plans: true +``` + +By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/4883 \ No newline at end of file diff --git a/apollo-router/src/cache/mod.rs b/apollo-router/src/cache/mod.rs index 4c487d8968..e6d571d79d 100644 --- a/apollo-router/src/cache/mod.rs +++ b/apollo-router/src/cache/mod.rs @@ -8,6 +8,7 @@ use tokio::sync::Mutex; use tower::BoxError; use self::storage::CacheStorage; +use self::storage::InMemoryCache; use self::storage::KeyType; use self::storage::ValueType; use crate::configuration::RedisCache; @@ -113,6 +114,10 @@ where self.storage.insert(key, value).await; } + pub(crate) async fn insert_in_memory(&self, key: K, value: V) { + self.storage.insert_in_memory(key, value).await; + } + async fn send(&self, sender: broadcast::Sender, key: &K, value: V) { // Lock the wait map to prevent more subscribers racing with our send // notification @@ -121,8 +126,8 @@ where let _ = sender.send(value); } - pub(crate) async fn in_memory_keys(&self) -> Vec { - self.storage.in_memory_keys().await + pub(crate) fn in_memory_cache(&self) -> InMemoryCache { + self.storage.in_memory_cache() } } diff --git a/apollo-router/src/cache/storage.rs b/apollo-router/src/cache/storage.rs index e8b4a9696d..85581bc45c 100644 --- a/apollo-router/src/cache/storage.rs +++ b/apollo-router/src/cache/storage.rs @@ -41,6 +41,8 @@ where // It has the functions it needs already } +pub(crate) type InMemoryCache = Arc>>; + // placeholder storage module // // this will be replaced by the multi level (in memory + redis/memcached) once we find @@ -178,13 +180,19 @@ where ); } - pub(crate) async fn in_memory_keys(&self) -> Vec { - self.inner - .lock() - .await - .iter() - .map(|(k, _)| k.clone()) - .collect() + pub(crate) async fn insert_in_memory(&self, key: K, value: V) { + let mut in_memory = self.inner.lock().await; + in_memory.put(key, value); + let size = in_memory.len() as u64; + tracing::info!( + value.apollo_router_cache_size = size, + kind = %self.caller, + storage = &tracing::field::display(CacheStorageName::Memory), + ); + } + + pub(crate) fn in_memory_cache(&self) -> InMemoryCache { + self.inner.clone() } #[cfg(test)] diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index 479d13a80e..cff536608c 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -896,6 +896,10 @@ pub(crate) struct QueryPlanning { /// /// The default value is None, which specifies no limit. pub(crate) experimental_paths_limit: Option, + + /// If cache warm up is configured, this will allow the router to keep a query plan created with + /// the old schema, if it determines that the schema update does not affect the corresponding query + pub(crate) experimental_reuse_query_plans: bool, } /// Cache configuration diff --git a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap index 98ebb36d81..b986bd32f1 100644 --- a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap +++ b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap @@ -2629,7 +2629,8 @@ expression: "&schema" }, "warmed_up_queries": null, "experimental_plans_limit": null, - "experimental_paths_limit": null + "experimental_paths_limit": null, + "experimental_reuse_query_plans": false }, "early_cancel": false, "experimental_log_on_broken_pipe": false @@ -2692,7 +2693,8 @@ expression: "&schema" }, "warmed_up_queries": null, "experimental_plans_limit": null, - "experimental_paths_limit": null + "experimental_paths_limit": null, + "experimental_reuse_query_plans": false }, "type": "object", "properties": { @@ -2839,6 +2841,11 @@ expression: "&schema" "minimum": 0.0, "nullable": true }, + "experimental_reuse_query_plans": { + "description": "If cache warm up is configured, this will allow the router to keep a query plan created with the old schema, if it determines that the schema update does not affect the corresponding query", + "default": false, + "type": "boolean" + }, "warmed_up_queries": { "description": "Warms up the cache on reloads by running the query plan over a list of the most used queries (from the in memory cache) Configures the number of queries warmed up. Defaults to 1/3 of the in memory cache", "default": null, diff --git a/apollo-router/src/context/mod.rs b/apollo-router/src/context/mod.rs index a73e2e1140..1c8a19ff3c 100644 --- a/apollo-router/src/context/mod.rs +++ b/apollo-router/src/context/mod.rs @@ -418,6 +418,7 @@ mod test { crate::services::layers::query_analysis::ParsedDocumentInner { ast: Default::default(), executable: Default::default(), + hash: Default::default(), parse_errors: Default::default(), validation_errors: Default::default(), }, diff --git a/apollo-router/src/plugins/authorization/authenticated.rs b/apollo-router/src/plugins/authorization/authenticated.rs index 6e193f7249..f857e6f209 100644 --- a/apollo-router/src/plugins/authorization/authenticated.rs +++ b/apollo-router/src/plugins/authorization/authenticated.rs @@ -3,9 +3,11 @@ use std::collections::HashMap; use apollo_compiler::ast; +use apollo_compiler::executable; use apollo_compiler::schema; use apollo_compiler::schema::Implementers; use apollo_compiler::schema::Name; +use apollo_compiler::Node; use tower::BoxError; use crate::json_ext::Path; @@ -21,7 +23,7 @@ pub(crate) const AUTHENTICATED_SPEC_VERSION_RANGE: &str = ">=0.1.0, <=0.1.0"; pub(crate) struct AuthenticatedCheckVisitor<'a> { schema: &'a schema::Schema, - fragments: HashMap<&'a ast::Name, &'a ast::FragmentDefinition>, + fragments: HashMap<&'a ast::Name, &'a Node>, pub(crate) found: bool, authenticated_directive_name: String, entity_query: bool, @@ -30,13 +32,13 @@ pub(crate) struct AuthenticatedCheckVisitor<'a> { impl<'a> AuthenticatedCheckVisitor<'a> { pub(crate) fn new( schema: &'a schema::Schema, - executable: &'a ast::Document, + executable: &'a executable::ExecutableDocument, entity_query: bool, ) -> Option { Some(Self { schema, entity_query, - fragments: transform::collect_fragments(executable), + fragments: executable.fragments.iter().collect(), found: false, authenticated_directive_name: Schema::directive_name( schema, @@ -60,22 +62,22 @@ impl<'a> AuthenticatedCheckVisitor<'a> { t.directives().has(&self.authenticated_directive_name) } - fn entities_operation(&mut self, node: &ast::OperationDefinition) -> Result<(), BoxError> { + fn entities_operation(&mut self, node: &executable::Operation) -> Result<(), BoxError> { use crate::spec::query::traverse::Visitor; - if node.selection_set.len() != 1 { + if node.selection_set.selections.len() != 1 { return Err("invalid number of selections for _entities query".into()); } - match node.selection_set.first() { - Some(ast::Selection::Field(field)) => { + match node.selection_set.selections.first() { + Some(executable::Selection::Field(field)) => { if field.name.as_str() != "_entities" { return Err("expected _entities field".into()); } - for selection in &field.selection_set { + for selection in &field.selection_set.selections { match selection { - ast::Selection::InlineFragment(f) => { + executable::Selection::InlineFragment(f) => { match f.type_condition.as_ref() { None => { return Err("expected type condition".into()); @@ -94,11 +96,7 @@ impl<'a> AuthenticatedCheckVisitor<'a> { } impl<'a> traverse::Visitor for AuthenticatedCheckVisitor<'a> { - fn operation( - &mut self, - root_type: &str, - node: &ast::OperationDefinition, - ) -> Result<(), BoxError> { + fn operation(&mut self, root_type: &str, node: &executable::Operation) -> Result<(), BoxError> { if !self.entity_query { traverse::operation(self, root_type, node) } else { @@ -109,7 +107,7 @@ impl<'a> traverse::Visitor for AuthenticatedCheckVisitor<'a> { &mut self, _parent_type: &str, field_def: &ast::FieldDefinition, - node: &ast::Field, + node: &executable::Field, ) -> Result<(), BoxError> { if self.is_field_authenticated(field_def) { self.found = true; @@ -118,25 +116,25 @@ impl<'a> traverse::Visitor for AuthenticatedCheckVisitor<'a> { traverse::field(self, field_def, node) } - fn fragment_definition(&mut self, node: &ast::FragmentDefinition) -> Result<(), BoxError> { + fn fragment(&mut self, node: &executable::Fragment) -> Result<(), BoxError> { if self .schema .types - .get(&node.type_condition) + .get(node.type_condition()) .is_some_and(|type_definition| self.is_type_authenticated(type_definition)) { self.found = true; return Ok(()); } - traverse::fragment_definition(self, node) + traverse::fragment(self, node) } - fn fragment_spread(&mut self, node: &ast::FragmentSpread) -> Result<(), BoxError> { - let condition = &self + fn fragment_spread(&mut self, node: &executable::FragmentSpread) -> Result<(), BoxError> { + let condition = self .fragments .get(&node.fragment_name) .ok_or("MissingFragment")? - .type_condition; + .type_condition(); if self .schema @@ -153,7 +151,7 @@ impl<'a> traverse::Visitor for AuthenticatedCheckVisitor<'a> { fn inline_fragment( &mut self, parent_type: &str, - node: &ast::InlineFragment, + node: &executable::InlineFragment, ) -> Result<(), BoxError> { if let Some(name) = &node.type_condition { if self diff --git a/apollo-router/src/plugins/authorization/mod.rs b/apollo-router/src/plugins/authorization/mod.rs index 3d7a65a280..85445a652c 100644 --- a/apollo-router/src/plugins/authorization/mod.rs +++ b/apollo-router/src/plugins/authorization/mod.rs @@ -5,7 +5,7 @@ use std::collections::HashSet; use std::ops::ControlFlow; use apollo_compiler::ast; -use apollo_compiler::ast::Document; +use apollo_compiler::ExecutableDocument; use http::StatusCode; use schemars::JsonSchema; use serde::Deserialize; @@ -176,18 +176,23 @@ impl AuthorizationPlugin { pub(crate) fn query_analysis( query: &str, + operation_name: Option<&str>, schema: &Schema, configuration: &Configuration, context: &Context, - ) { - let doc = Query::parse_document(query, schema, configuration); - let ast = &doc.ast; + ) -> Result<(), SpecError> { + let doc = Query::parse_document(query, operation_name, schema, configuration)?; let CacheKeyMetadata { is_authenticated, scopes, policies, - } = Self::generate_cache_metadata(ast, &schema.definitions, false); + } = Self::generate_cache_metadata( + &doc.executable, + operation_name, + &schema.definitions, + false, + ); if is_authenticated { context.insert(AUTHENTICATED_KEY, true).unwrap(); } @@ -201,39 +206,42 @@ impl AuthorizationPlugin { policies.into_iter().map(|policy| (policy, None)).collect(); context.insert(REQUIRED_POLICIES_KEY, policies).unwrap(); } + + Ok(()) } pub(crate) fn generate_cache_metadata( - ast: &Document, + document: &ExecutableDocument, + operation_name: Option<&str>, schema: &apollo_compiler::Schema, entity_query: bool, ) -> CacheKeyMetadata { let mut is_authenticated = false; - if let Some(mut visitor) = AuthenticatedCheckVisitor::new(schema, ast, entity_query) { + if let Some(mut visitor) = AuthenticatedCheckVisitor::new(schema, document, entity_query) { // if this fails, the query is invalid and will fail at the query planning phase. // We do not return validation errors here for now because that would imply a huge // refactoring of telemetry and tests - if traverse::document(&mut visitor, ast).is_ok() && visitor.found { + if traverse::document(&mut visitor, document, operation_name).is_ok() && visitor.found { is_authenticated = true; } } let mut scopes = Vec::new(); - if let Some(mut visitor) = ScopeExtractionVisitor::new(schema, ast, entity_query) { + if let Some(mut visitor) = ScopeExtractionVisitor::new(schema, document, entity_query) { // if this fails, the query is invalid and will fail at the query planning phase. // We do not return validation errors here for now because that would imply a huge // refactoring of telemetry and tests - if traverse::document(&mut visitor, ast).is_ok() { + if traverse::document(&mut visitor, document, operation_name).is_ok() { scopes = visitor.extracted_scopes.into_iter().collect(); } } let mut policies: Vec = Vec::new(); - if let Some(mut visitor) = PolicyExtractionVisitor::new(schema, ast, entity_query) { + if let Some(mut visitor) = PolicyExtractionVisitor::new(schema, document, entity_query) { // if this fails, the query is invalid and will fail at the query planning phase. // We do not return validation errors here for now because that would imply a huge // refactoring of telemetry and tests - if traverse::document(&mut visitor, ast).is_ok() { + if traverse::document(&mut visitor, document, operation_name).is_ok() { policies = visitor.extracted_policies.into_iter().collect(); } } diff --git a/apollo-router/src/plugins/authorization/policy.rs b/apollo-router/src/plugins/authorization/policy.rs index 37050428b2..17a900c6fe 100644 --- a/apollo-router/src/plugins/authorization/policy.rs +++ b/apollo-router/src/plugins/authorization/policy.rs @@ -10,9 +10,11 @@ use std::collections::HashMap; use std::collections::HashSet; use apollo_compiler::ast; +use apollo_compiler::executable; use apollo_compiler::schema; use apollo_compiler::schema::Implementers; use apollo_compiler::schema::Name; +use apollo_compiler::Node; use tower::BoxError; use crate::json_ext::Path; @@ -24,7 +26,7 @@ use crate::spec::TYPENAME; pub(crate) struct PolicyExtractionVisitor<'a> { schema: &'a schema::Schema, - fragments: HashMap<&'a ast::Name, &'a ast::FragmentDefinition>, + fragments: HashMap<&'a ast::Name, &'a Node>, pub(crate) extracted_policies: HashSet, policy_directive_name: String, entity_query: bool, @@ -38,13 +40,13 @@ impl<'a> PolicyExtractionVisitor<'a> { #[allow(dead_code)] pub(crate) fn new( schema: &'a schema::Schema, - executable: &'a ast::Document, + executable: &'a executable::ExecutableDocument, entity_query: bool, ) -> Option { Some(Self { schema, entity_query, - fragments: transform::collect_fragments(executable), + fragments: executable.fragments.iter().collect(), extracted_policies: HashSet::new(), policy_directive_name: Schema::directive_name( schema, @@ -71,22 +73,22 @@ impl<'a> PolicyExtractionVisitor<'a> { )); } - fn entities_operation(&mut self, node: &ast::OperationDefinition) -> Result<(), BoxError> { + fn entities_operation(&mut self, node: &executable::Operation) -> Result<(), BoxError> { use crate::spec::query::traverse::Visitor; - if node.selection_set.len() != 1 { + if node.selection_set.selections.len() != 1 { return Err("invalid number of selections for _entities query".into()); } - match node.selection_set.first() { - Some(ast::Selection::Field(field)) => { + match node.selection_set.selections.first() { + Some(executable::Selection::Field(field)) => { if field.name.as_str() != "_entities" { return Err("expected _entities field".into()); } - for selection in &field.selection_set { + for selection in &field.selection_set.selections { match selection { - ast::Selection::InlineFragment(f) => { + executable::Selection::InlineFragment(f) => { match f.type_condition.as_ref() { None => { return Err("expected type condition".into()); @@ -120,11 +122,7 @@ fn policy_argument( } impl<'a> traverse::Visitor for PolicyExtractionVisitor<'a> { - fn operation( - &mut self, - root_type: &str, - node: &ast::OperationDefinition, - ) -> Result<(), BoxError> { + fn operation(&mut self, root_type: &str, node: &executable::Operation) -> Result<(), BoxError> { if let Some(ty) = self.schema.types.get(root_type) { self.extracted_policies.extend(policy_argument( ty.directives().get(&self.policy_directive_name), @@ -142,26 +140,26 @@ impl<'a> traverse::Visitor for PolicyExtractionVisitor<'a> { &mut self, _parent_type: &str, field_def: &ast::FieldDefinition, - node: &ast::Field, + node: &executable::Field, ) -> Result<(), BoxError> { self.get_policies_from_field(field_def); traverse::field(self, field_def, node) } - fn fragment_definition(&mut self, node: &ast::FragmentDefinition) -> Result<(), BoxError> { - if let Some(ty) = self.schema.types.get(&node.type_condition) { + fn fragment(&mut self, node: &executable::Fragment) -> Result<(), BoxError> { + if let Some(ty) = self.schema.types.get(node.type_condition()) { self.get_policies_from_type(ty); } - traverse::fragment_definition(self, node) + traverse::fragment(self, node) } - fn fragment_spread(&mut self, node: &ast::FragmentSpread) -> Result<(), BoxError> { - let type_condition = &self + fn fragment_spread(&mut self, node: &executable::FragmentSpread) -> Result<(), BoxError> { + let type_condition = self .fragments .get(&node.fragment_name) .ok_or("MissingFragment")? - .type_condition; + .type_condition(); if let Some(ty) = self.schema.types.get(type_condition) { self.get_policies_from_type(ty); @@ -172,7 +170,7 @@ impl<'a> traverse::Visitor for PolicyExtractionVisitor<'a> { fn inline_fragment( &mut self, parent_type: &str, - node: &ast::InlineFragment, + node: &executable::InlineFragment, ) -> Result<(), BoxError> { if let Some(type_condition) = &node.type_condition { if let Some(ty) = self.schema.types.get(type_condition) { @@ -643,6 +641,7 @@ mod tests { use apollo_compiler::ast; use apollo_compiler::ast::Document; + use apollo_compiler::ExecutableDocument; use apollo_compiler::Schema; use crate::json_ext::Path; @@ -714,10 +713,9 @@ mod tests { fn extract(schema: &str, query: &str) -> BTreeSet { let schema = Schema::parse_and_validate(schema, "schema.graphql").unwrap(); - let doc = ast::Document::parse(query, "query.graphql").unwrap(); - doc.to_executable_validate(&schema).unwrap(); + let doc = ExecutableDocument::parse(&schema, query, "query.graphql").unwrap(); let mut visitor = PolicyExtractionVisitor::new(&schema, &doc, false).unwrap(); - traverse::document(&mut visitor, &doc).unwrap(); + traverse::document(&mut visitor, &doc, None).unwrap(); visitor.extracted_policies.into_iter().collect() } diff --git a/apollo-router/src/plugins/authorization/scopes.rs b/apollo-router/src/plugins/authorization/scopes.rs index 3dcac9d1c3..8f07c64762 100644 --- a/apollo-router/src/plugins/authorization/scopes.rs +++ b/apollo-router/src/plugins/authorization/scopes.rs @@ -10,9 +10,11 @@ use std::collections::HashMap; use std::collections::HashSet; use apollo_compiler::ast; +use apollo_compiler::executable; use apollo_compiler::schema; use apollo_compiler::schema::Implementers; use apollo_compiler::schema::Name; +use apollo_compiler::Node; use tower::BoxError; use crate::json_ext::Path; @@ -24,7 +26,7 @@ use crate::spec::TYPENAME; pub(crate) struct ScopeExtractionVisitor<'a> { schema: &'a schema::Schema, - fragments: HashMap<&'a ast::Name, &'a ast::FragmentDefinition>, + fragments: HashMap<&'a ast::Name, &'a Node>, pub(crate) extracted_scopes: HashSet, requires_scopes_directive_name: String, entity_query: bool, @@ -38,13 +40,13 @@ impl<'a> ScopeExtractionVisitor<'a> { #[allow(dead_code)] pub(crate) fn new( schema: &'a schema::Schema, - executable: &'a ast::Document, + executable: &'a executable::ExecutableDocument, entity_query: bool, ) -> Option { Some(Self { schema, entity_query, - fragments: transform::collect_fragments(executable), + fragments: executable.fragments.iter().collect(), extracted_scopes: HashSet::new(), requires_scopes_directive_name: Schema::directive_name( schema, @@ -71,22 +73,22 @@ impl<'a> ScopeExtractionVisitor<'a> { )); } - fn entities_operation(&mut self, node: &ast::OperationDefinition) -> Result<(), BoxError> { + fn entities_operation(&mut self, node: &executable::Operation) -> Result<(), BoxError> { use crate::spec::query::traverse::Visitor; - if node.selection_set.len() != 1 { + if node.selection_set.selections.len() != 1 { return Err("invalid number of selections for _entities query".into()); } - match node.selection_set.first() { - Some(ast::Selection::Field(field)) => { + match node.selection_set.selections.first() { + Some(executable::Selection::Field(field)) => { if field.name.as_str() != "_entities" { return Err("expected _entities field".into()); } - for selection in &field.selection_set { + for selection in &field.selection_set.selections { match selection { - ast::Selection::InlineFragment(f) => { + executable::Selection::InlineFragment(f) => { match f.type_condition.as_ref() { None => { return Err("expected type condition".into()); @@ -120,11 +122,7 @@ fn scopes_argument( } impl<'a> traverse::Visitor for ScopeExtractionVisitor<'a> { - fn operation( - &mut self, - root_type: &str, - node: &ast::OperationDefinition, - ) -> Result<(), BoxError> { + fn operation(&mut self, root_type: &str, node: &executable::Operation) -> Result<(), BoxError> { if let Some(ty) = self.schema.types.get(root_type) { self.extracted_scopes.extend(scopes_argument( ty.directives().get(&self.requires_scopes_directive_name), @@ -142,26 +140,26 @@ impl<'a> traverse::Visitor for ScopeExtractionVisitor<'a> { &mut self, _parent_type: &str, field_def: &ast::FieldDefinition, - node: &ast::Field, + node: &executable::Field, ) -> Result<(), BoxError> { self.scopes_from_field(field_def); traverse::field(self, field_def, node) } - fn fragment_definition(&mut self, node: &ast::FragmentDefinition) -> Result<(), BoxError> { - if let Some(ty) = self.schema.types.get(&node.type_condition) { + fn fragment(&mut self, node: &executable::Fragment) -> Result<(), BoxError> { + if let Some(ty) = self.schema.types.get(node.type_condition()) { self.scopes_from_type(ty); } - traverse::fragment_definition(self, node) + traverse::fragment(self, node) } - fn fragment_spread(&mut self, node: &ast::FragmentSpread) -> Result<(), BoxError> { - let type_condition = &self + fn fragment_spread(&mut self, node: &executable::FragmentSpread) -> Result<(), BoxError> { + let type_condition = self .fragments .get(&node.fragment_name) .ok_or("MissingFragment")? - .type_condition; + .type_condition(); if let Some(ty) = self.schema.types.get(type_condition) { self.scopes_from_type(ty); @@ -172,7 +170,7 @@ impl<'a> traverse::Visitor for ScopeExtractionVisitor<'a> { fn inline_fragment( &mut self, parent_type: &str, - node: &ast::InlineFragment, + node: &executable::InlineFragment, ) -> Result<(), BoxError> { if let Some(type_condition) = &node.type_condition { if let Some(ty) = self.schema.types.get(type_condition) { @@ -717,9 +715,9 @@ mod tests { fn extract(schema: &str, query: &str) -> BTreeSet { let schema = Schema::parse_and_validate(schema, "schema.graphql").unwrap(); let doc = Document::parse(query, "query.graphql").unwrap(); - doc.to_executable_validate(&schema).unwrap(); - let mut visitor = ScopeExtractionVisitor::new(&schema, &doc, false).unwrap(); - traverse::document(&mut visitor, &doc).unwrap(); + let exec = doc.to_executable_validate(&schema).unwrap(); + let mut visitor = ScopeExtractionVisitor::new(&schema, &exec, false).unwrap(); + traverse::document(&mut visitor, &exec, None).unwrap(); visitor.extracted_scopes.into_iter().collect() } diff --git a/apollo-router/src/plugins/cache/tests.rs b/apollo-router/src/plugins/cache/tests.rs index f24f4e0126..f63d50a2d5 100644 --- a/apollo-router/src/plugins/cache/tests.rs +++ b/apollo-router/src/plugins/cache/tests.rs @@ -86,12 +86,12 @@ impl Mocks for Mock1 { match &*command.cmd { "GET" => { if let Some(RedisValue::Bytes(b)) = command.args.first() { - if b == &b"subgraph:user:Query:146a735f805c55554b5233253c17756deaa6ffd06696fafa4d6e3186e6efe592:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c"[..]{ + if b == &b"subgraph:user:Query:3c13e047244d0fc7283c4f09328c841ebd5aeeae17625ddf4b6173b1d94a8ff6:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c"[..]{ let set = self.set.lock(); if *set { return Ok(RedisValue::Bytes(Bytes::from(USER_RESPONSE))); } - } else if b == &b"subgraph:orga:Organization:5811967f540d300d249ab30ae681359a7815fdb5d3dc71a94be1d491006a6b27:655f22a6af21d7ffe671d3ce4b33464a76ddfea0bf179740b15e804b11983c04:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c"[..] { + } else if b == &b"subgraph:orga:Organization:5811967f540d300d249ab30ae681359a7815fdb5d3dc71a94be1d491006a6b27:14f7294f5ca99ecfc3a4c6d9acc249aa33bd03446be80e60eb3fdc42d5293c47:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c"[..] { return Ok(RedisValue::Bytes(Bytes::from(ORGA_RESPONSE))); } } @@ -99,7 +99,7 @@ impl Mocks for Mock1 { "SET" => { if let Some(RedisValue::Bytes(b)) = command.args.first() { if b == - &b"subgraph:user:Query:146a735f805c55554b5233253c17756deaa6ffd06696fafa4d6e3186e6efe592:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c"[..] { + &b"subgraph:user:Query:3c13e047244d0fc7283c4f09328c841ebd5aeeae17625ddf4b6173b1d94a8ff6:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c"[..] { let mut set = self.set.lock(); *set = true; diff --git a/apollo-router/src/plugins/progressive_override/mod.rs b/apollo-router/src/plugins/progressive_override/mod.rs index a339e99d35..69c5c8ae32 100644 --- a/apollo-router/src/plugins/progressive_override/mod.rs +++ b/apollo-router/src/plugins/progressive_override/mod.rs @@ -205,7 +205,7 @@ impl Plugin for ProgressiveOverridePlugin { .or_insert_with(|| { OverrideLabelVisitor::new(&schema) .map(|mut visitor| { - let _ = traverse::document(&mut visitor, &parsed_doc.ast); + let _ = traverse::document(&mut visitor, &parsed_doc.executable, operation_name.as_deref()); visitor.override_labels.into_iter().collect::>() }) .unwrap_or_default() diff --git a/apollo-router/src/plugins/progressive_override/snapshots/apollo_router__plugins__progressive_override__tests__non_overridden_field_yields_expected_query_plan.snap b/apollo-router/src/plugins/progressive_override/snapshots/apollo_router__plugins__progressive_override__tests__non_overridden_field_yields_expected_query_plan.snap index 905007adba..1961250dd2 100644 --- a/apollo-router/src/plugins/progressive_override/snapshots/apollo_router__plugins__progressive_override__tests__non_overridden_field_yields_expected_query_plan.snap +++ b/apollo-router/src/plugins/progressive_override/snapshots/apollo_router__plugins__progressive_override__tests__non_overridden_field_yields_expected_query_plan.snap @@ -1,6 +1,5 @@ --- source: apollo-router/src/plugins/progressive_override/tests.rs -assertion_line: 253 expression: query_plan --- { @@ -19,7 +18,7 @@ expression: query_plan "id": null, "inputRewrites": null, "outputRewrites": null, - "schemaAwareHash": "098137301a64979dbc957ad4134e40a6c4bcf3be50a08e984661e576c78d4a1b", + "schemaAwareHash": "9358047754b11522aac502a3c6a668cd4286c07d489680834e63d6e033db4eb5", "authorization": { "is_authenticated": false, "scopes": [], diff --git a/apollo-router/src/plugins/progressive_override/snapshots/apollo_router__plugins__progressive_override__tests__overridden_field_yields_expected_query_plan.snap b/apollo-router/src/plugins/progressive_override/snapshots/apollo_router__plugins__progressive_override__tests__overridden_field_yields_expected_query_plan.snap index 6db5314f4a..5d4e064a6d 100644 --- a/apollo-router/src/plugins/progressive_override/snapshots/apollo_router__plugins__progressive_override__tests__overridden_field_yields_expected_query_plan.snap +++ b/apollo-router/src/plugins/progressive_override/snapshots/apollo_router__plugins__progressive_override__tests__overridden_field_yields_expected_query_plan.snap @@ -1,6 +1,5 @@ --- source: apollo-router/src/plugins/progressive_override/tests.rs -assertion_line: 262 expression: query_plan --- { @@ -24,7 +23,7 @@ expression: query_plan "id": null, "inputRewrites": null, "outputRewrites": null, - "schemaAwareHash": "3560295fe31a2e33da9dba4d775a6fd64727cb953cf790575bc83c27a486f8d2", + "schemaAwareHash": "8f445761c0bcdda90b8da35ccd13fd98e474514f3efc071bd2c39495b5af94e5", "authorization": { "is_authenticated": false, "scopes": [], @@ -62,7 +61,7 @@ expression: query_plan "id": null, "inputRewrites": null, "outputRewrites": null, - "schemaAwareHash": "0eb7c697f1b6125db1db0b099e0707e3a2984e4efb4aaee1d3bde8430b34bc6f", + "schemaAwareHash": "9a1feab7ee8c57c8a4ab4db29712412a9cfe94009bfcb40dc0d22ea54c410865", "authorization": { "is_authenticated": false, "scopes": [], diff --git a/apollo-router/src/plugins/progressive_override/tests.rs b/apollo-router/src/plugins/progressive_override/tests.rs index c8853a9c3f..303157ca7a 100644 --- a/apollo-router/src/plugins/progressive_override/tests.rs +++ b/apollo-router/src/plugins/progressive_override/tests.rs @@ -1,6 +1,7 @@ use std::sync::Arc; use apollo_compiler::ast::Document; +use apollo_compiler::Schema; use tower::ServiceExt; use crate::metrics::FutureMetricsExt; @@ -137,8 +138,12 @@ async fn assert_expected_and_absent_labels_for_supergraph_service( // plugin depends on the parsed document being in the context so we'll add // it ourselves for testing purposes + let schema = Schema::parse_and_validate(SCHEMA, "").unwrap(); + let document = Document::parse(query, "query.graphql").unwrap(); + let executable = document.to_executable(&schema).unwrap(); let parsed_doc: ParsedDocument = Arc::from(ParsedDocumentInner { - ast: Document::parse(query, "query.graphql").unwrap(), + ast: document, + executable: Arc::new(executable), ..Default::default() }); @@ -206,8 +211,12 @@ async fn plugin_supergraph_service_trims_0pc_label() { } async fn get_json_query_plan(query: &str) -> serde_json::Value { + let schema = Schema::parse_and_validate(SCHEMA, "").unwrap(); + let document = Document::parse(query, "query.graphql").unwrap(); + let executable = document.to_executable(&schema).unwrap(); let parsed_doc: ParsedDocument = Arc::from(ParsedDocumentInner { - ast: Document::parse(query, "query.graphql").unwrap(), + ast: document, + executable: Arc::new(executable), ..Default::default() }); @@ -279,8 +288,12 @@ async fn query_with_labels(query: &str, labels_from_coprocessors: Vec<&str>) { // plugin depends on the parsed document being in the context so we'll add // it ourselves for testing purposes + let schema = Schema::parse_and_validate(SCHEMA, "").unwrap(); + let document = Document::parse(query, "query.graphql").unwrap(); + let executable = document.to_executable(&schema).unwrap(); let parsed_doc: ParsedDocument = Arc::from(ParsedDocumentInner { - ast: Document::parse(query, "query.graphql").unwrap(), + ast: document, + executable: Arc::new(executable), ..Default::default() }); diff --git a/apollo-router/src/plugins/progressive_override/visitor.rs b/apollo-router/src/plugins/progressive_override/visitor.rs index 540db9e469..d17cd07aec 100644 --- a/apollo-router/src/plugins/progressive_override/visitor.rs +++ b/apollo-router/src/plugins/progressive_override/visitor.rs @@ -3,6 +3,7 @@ use std::collections::HashSet; use std::sync::Arc; use apollo_compiler::ast; +use apollo_compiler::executable; use apollo_compiler::schema; use tower::BoxError; @@ -33,11 +34,7 @@ impl<'a> traverse::Visitor for OverrideLabelVisitor<'a> { self.schema } - fn operation( - &mut self, - root_type: &str, - node: &ast::OperationDefinition, - ) -> Result<(), BoxError> { + fn operation(&mut self, root_type: &str, node: &executable::Operation) -> Result<(), BoxError> { traverse::operation(self, root_type, node) } @@ -45,7 +42,7 @@ impl<'a> traverse::Visitor for OverrideLabelVisitor<'a> { &mut self, _parent_type: &str, field_def: &ast::FieldDefinition, - node: &ast::Field, + node: &executable::Field, ) -> Result<(), BoxError> { let new_override_labels = field_def .directives @@ -80,7 +77,8 @@ pub(crate) struct OverrideLabelVisitor<'a> { mod tests { use std::sync::Arc; - use apollo_compiler::ast::Document; + use apollo_compiler::validation::Valid; + use apollo_compiler::ExecutableDocument; use apollo_compiler::Schema; use crate::plugins::progressive_override::visitor::OverrideLabelVisitor; @@ -151,12 +149,16 @@ mod tests { fn collects() { let schema = Schema::parse(SCHEMA, "supergraph.graphql").expect("parse schema"); let operation_string = "{ t { k a b } }"; - let operation = - Document::parse(operation_string, "query.graphql").expect("parse operation"); + let operation = ExecutableDocument::parse( + Valid::assume_valid_ref(&schema), + operation_string, + "query.graphql", + ) + .expect("parse operation"); let mut visitor = OverrideLabelVisitor::new(&schema).expect("create visitor"); - traverse::document(&mut visitor, &operation).unwrap(); + traverse::document(&mut visitor, &operation, None).unwrap(); assert_eq!( visitor.override_labels, @@ -168,12 +170,16 @@ mod tests { fn collects2() { let schema = Schema::parse(SCHEMA, "supergraph.graphql").expect("parse schema"); let operation_string = "{ t { k a b } t2 }"; - let operation = - Document::parse(operation_string, "query.graphql").expect("parse operation"); + let operation = ExecutableDocument::parse( + Valid::assume_valid_ref(&schema), + operation_string, + "query.graphql", + ) + .expect("parse operation"); let mut visitor = OverrideLabelVisitor::new(&schema).expect("create visitor"); - traverse::document(&mut visitor, &operation).unwrap(); + traverse::document(&mut visitor, &operation, None).unwrap(); assert_eq!( visitor.override_labels, diff --git a/apollo-router/src/plugins/record_replay/record.rs b/apollo-router/src/plugins/record_replay/record.rs index fbd9150bda..7179c8db50 100644 --- a/apollo-router/src/plugins/record_replay/record.rs +++ b/apollo-router/src/plugins/record_replay/record.rs @@ -161,6 +161,7 @@ impl Plugin for Record { .query .clone() .unwrap_or_default(), + req.supergraph_request.body().operation_name.as_deref(), schema.clone(), ) { return req; @@ -308,8 +309,8 @@ async fn write_file(dir: Arc, path: &PathBuf, contents: &[u8]) -> Result<( Ok(()) } -fn is_introspection(query: String, schema: Arc) -> bool { - Query::parse(query, &schema, &Configuration::default()) +fn is_introspection(query: String, operation_name: Option<&str>, schema: Arc) -> bool { + Query::parse(query, operation_name, &schema, &Configuration::default()) .map(|q| q.contains_introspection()) .unwrap_or_default() } diff --git a/apollo-router/src/plugins/snapshots/apollo_router__plugins__expose_query_plan__tests__it_expose_query_plan-2.snap b/apollo-router/src/plugins/snapshots/apollo_router__plugins__expose_query_plan__tests__it_expose_query_plan-2.snap index 8121dc0467..0767d22f80 100644 --- a/apollo-router/src/plugins/snapshots/apollo_router__plugins__expose_query_plan__tests__it_expose_query_plan-2.snap +++ b/apollo-router/src/plugins/snapshots/apollo_router__plugins__expose_query_plan__tests__it_expose_query_plan-2.snap @@ -68,7 +68,7 @@ expression: "serde_json::to_value(response).unwrap()" "id": null, "inputRewrites": null, "outputRewrites": null, - "schemaAwareHash": "330b684002dfeb54a17451591d1ac7834c6004d641c9c69599c0cf1ddf75d005", + "schemaAwareHash": "34be619a78867ab9d0670048f4c93574e38cd9253e9cc032f567078355b25086", "authorization": { "is_authenticated": false, "scopes": [], @@ -107,7 +107,7 @@ expression: "serde_json::to_value(response).unwrap()" "id": null, "inputRewrites": null, "outputRewrites": null, - "schemaAwareHash": "c78203a67f808f38117410251f2bd629d13f960c6d7dd9b38ec1bfe906cac897", + "schemaAwareHash": "f1582d942020b23347d84f6ae46c018492ae7c59c9b1472e0b442121ddf16368", "authorization": { "is_authenticated": false, "scopes": [], @@ -153,7 +153,7 @@ expression: "serde_json::to_value(response).unwrap()" "id": null, "inputRewrites": null, "outputRewrites": null, - "schemaAwareHash": "a4799bf4639ea9c24898cd72ede3a27b939fcb7976a43ef27378641763a7a4c1", + "schemaAwareHash": "6fa5a74c5af2b18f343e9e69bbcbc9335e9faaa46c3d8964d199002dfeb0026f", "authorization": { "is_authenticated": false, "scopes": [], @@ -196,7 +196,7 @@ expression: "serde_json::to_value(response).unwrap()" "id": null, "inputRewrites": null, "outputRewrites": null, - "schemaAwareHash": "36d755acdab9a67a3d42cd572fdf9a82c95602e1b657a191c028f6da21fe4f66", + "schemaAwareHash": "6fa5a74c5af2b18f343e9e69bbcbc9335e9faaa46c3d8964d199002dfeb0026f", "authorization": { "is_authenticated": false, "scopes": [], diff --git a/apollo-router/src/plugins/snapshots/apollo_router__plugins__expose_query_plan__tests__it_expose_query_plan.snap b/apollo-router/src/plugins/snapshots/apollo_router__plugins__expose_query_plan__tests__it_expose_query_plan.snap index 8121dc0467..0767d22f80 100644 --- a/apollo-router/src/plugins/snapshots/apollo_router__plugins__expose_query_plan__tests__it_expose_query_plan.snap +++ b/apollo-router/src/plugins/snapshots/apollo_router__plugins__expose_query_plan__tests__it_expose_query_plan.snap @@ -68,7 +68,7 @@ expression: "serde_json::to_value(response).unwrap()" "id": null, "inputRewrites": null, "outputRewrites": null, - "schemaAwareHash": "330b684002dfeb54a17451591d1ac7834c6004d641c9c69599c0cf1ddf75d005", + "schemaAwareHash": "34be619a78867ab9d0670048f4c93574e38cd9253e9cc032f567078355b25086", "authorization": { "is_authenticated": false, "scopes": [], @@ -107,7 +107,7 @@ expression: "serde_json::to_value(response).unwrap()" "id": null, "inputRewrites": null, "outputRewrites": null, - "schemaAwareHash": "c78203a67f808f38117410251f2bd629d13f960c6d7dd9b38ec1bfe906cac897", + "schemaAwareHash": "f1582d942020b23347d84f6ae46c018492ae7c59c9b1472e0b442121ddf16368", "authorization": { "is_authenticated": false, "scopes": [], @@ -153,7 +153,7 @@ expression: "serde_json::to_value(response).unwrap()" "id": null, "inputRewrites": null, "outputRewrites": null, - "schemaAwareHash": "a4799bf4639ea9c24898cd72ede3a27b939fcb7976a43ef27378641763a7a4c1", + "schemaAwareHash": "6fa5a74c5af2b18f343e9e69bbcbc9335e9faaa46c3d8964d199002dfeb0026f", "authorization": { "is_authenticated": false, "scopes": [], @@ -196,7 +196,7 @@ expression: "serde_json::to_value(response).unwrap()" "id": null, "inputRewrites": null, "outputRewrites": null, - "schemaAwareHash": "36d755acdab9a67a3d42cd572fdf9a82c95602e1b657a191c028f6da21fe4f66", + "schemaAwareHash": "6fa5a74c5af2b18f343e9e69bbcbc9335e9faaa46c3d8964d199002dfeb0026f", "authorization": { "is_authenticated": false, "scopes": [], diff --git a/apollo-router/src/query_planner/bridge_query_planner.rs b/apollo-router/src/query_planner/bridge_query_planner.rs index 333c0209e6..63a9b86da1 100644 --- a/apollo-router/src/query_planner/bridge_query_planner.rs +++ b/apollo-router/src/query_planner/bridge_query_planner.rs @@ -41,12 +41,14 @@ use crate::plugins::authorization::AuthorizationPlugin; use crate::plugins::authorization::CacheKeyMetadata; use crate::plugins::authorization::UnauthorizedPaths; use crate::plugins::progressive_override::LABELS_TO_OVERRIDE_KEY; +use crate::query_planner::fetch::QueryHash; use crate::query_planner::labeler::add_defer_labels; use crate::services::layers::query_analysis::ParsedDocument; use crate::services::layers::query_analysis::ParsedDocumentInner; use crate::services::QueryPlannerContent; use crate::services::QueryPlannerRequest; use crate::services::QueryPlannerResponse; +use crate::spec::query::change::QueryHashVisitor; use crate::spec::Query; use crate::spec::Schema; use crate::spec::SpecError; @@ -375,7 +377,7 @@ impl BridgeQueryPlanner { }; let (fragments, operations, defer_stats, schema_aware_hash) = - Query::extract_query_information(&self.schema, executable, &doc.ast)?; + Query::extract_query_information(&self.schema, executable, operation_name)?; let subselections = crate::spec::query::subselections::collect_subselections( &self.configuration, @@ -488,9 +490,7 @@ impl BridgeQueryPlanner { .into_result() { Ok(mut plan) => { - plan.data - .query_plan - .hash_subqueries(&self.schema.definitions); + plan.data.query_plan.hash_subqueries(&self.subgraph_schemas); plan.data .query_plan .extract_authorization_metadata(&self.schema.definitions, &key); @@ -620,9 +620,16 @@ impl Service for BridgeQueryPlanner { .to_executable(schema) // Assume transformation creates a valid document: ignore conversion errors .unwrap_or_else(|invalid| invalid.partial); + let hash = QueryHashVisitor::hash_query( + schema, + &executable_document, + operation_name.as_deref(), + ) + .map_err(|e| SpecError::QueryHashing(e.to_string()))?; doc = Arc::new(ParsedDocumentInner { executable: Arc::new(executable_document), ast: modified_query, + hash: Arc::new(QueryHash(hash)), // Carry errors from previous ParsedDocument // and assume transformation doesn’t introduce new errors. // TODO: check the latter? @@ -740,9 +747,16 @@ impl BridgeQueryPlanner { .to_executable(&self.schema.api_schema().definitions) // Assume transformation creates a valid document: ignore conversion errors .unwrap_or_else(|invalid| invalid.partial); + let hash = QueryHashVisitor::hash_query( + &self.schema.definitions, + &executable_document, + key.operation_name.as_deref(), + ) + .map_err(|e| SpecError::QueryHashing(e.to_string()))?; doc = Arc::new(ParsedDocumentInner { executable: Arc::new(executable_document), ast: new_doc, + hash: Arc::new(QueryHash(hash)), // Carry errors from previous ParsedDocument // and assume transformation doesn’t introduce new errors. // TODO: check the latter? @@ -831,9 +845,9 @@ struct QueryPlan { } impl QueryPlan { - fn hash_subqueries(&mut self, schema: &apollo_compiler::Schema) { + fn hash_subqueries(&mut self, schemas: &HashMap>>) { if let Some(node) = self.node.as_mut() { - node.hash_subqueries(schema); + node.hash_subqueries(schemas); } } @@ -1118,7 +1132,7 @@ mod tests { .await .unwrap(); - let doc = Query::parse_document(query, &schema, &Configuration::default()); + let doc = Query::parse_document(query, None, &schema, &Configuration::default()).unwrap(); let selections = planner .parse_selections(query.to_string(), None, &doc) @@ -1501,9 +1515,11 @@ mod tests { let doc = Query::parse_document( original_query, + operation_name.as_deref(), planner.schema().api_schema(), &configuration, - ); + ) + .unwrap(); planner .get( diff --git a/apollo-router/src/query_planner/caching_query_planner.rs b/apollo-router/src/query_planner/caching_query_planner.rs index ded0df4648..66b5508133 100644 --- a/apollo-router/src/query_planner/caching_query_planner.rs +++ b/apollo-router/src/query_planner/caching_query_planner.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::hash::Hash; use std::ops::Deref; use std::sync::Arc; use std::task; @@ -20,6 +21,8 @@ use tower::ServiceExt; use tower_service::Service; use tracing::Instrument; +use super::fetch::QueryHash; +use crate::cache::storage::InMemoryCache; use crate::cache::DeduplicatingCache; use crate::error::CacheResolverError; use crate::error::QueryPlannerError; @@ -45,6 +48,8 @@ use crate::Context; /// An [`IndexMap`] of available plugins. pub(crate) type Plugins = IndexMap>; +pub(crate) type InMemoryCachePlanner = + InMemoryCache>>; /// A query planner wrapper that caches results. /// @@ -95,32 +100,23 @@ where }) } - pub(crate) async fn cache_keys(&self, count: Option) -> Vec { - let keys = self.cache.in_memory_keys().await; - let count = count.unwrap_or(keys.len() / 3); - keys.into_iter() - .take(count) - .map(|key| WarmUpCachingQueryKey { - query: key.query, - operation: key.operation, - metadata: key.metadata, - plan_options: key.plan_options, - }) - .collect() + pub(crate) fn previous_cache(&self) -> InMemoryCachePlanner { + self.cache.in_memory_cache() } pub(crate) async fn warm_up( &mut self, query_analysis: &QueryAnalysisLayer, persisted_query_layer: &PersistedQueryLayer, - mut cache_keys: Vec, + previous_cache: InMemoryCachePlanner, + count: Option, + experimental_reuse_query_plans: bool, ) { let _timer = Timer::new(|duration| { ::tracing::info!( histogram.apollo.router.query.planning.warmup.duration = duration.as_secs_f64() ); }); - let schema_id = self.schema.schema_id.clone(); let mut service = ServiceBuilder::new().service( self.plugins @@ -131,6 +127,35 @@ where }), ); + let mut cache_keys = { + let cache = previous_cache.lock().await; + + let count = count.unwrap_or(cache.len() / 3); + + cache + .iter() + .map( + |( + CachingQueryKey { + query, + operation, + hash, + metadata, + plan_options, + }, + _, + )| WarmUpCachingQueryKey { + query: query.clone(), + operation: operation.clone(), + hash: Some(hash.clone()), + metadata: metadata.clone(), + plan_options: plan_options.clone(), + }, + ) + .take(count) + .collect::>() + }; + cache_keys.shuffle(&mut thread_rng()); let persisted_queries_operations = persisted_query_layer.all_operations(); @@ -153,6 +178,7 @@ where all_cache_keys.push(WarmUpCachingQueryKey { query, operation: None, + hash: None, metadata: CacheKeyMetadata::default(), plan_options: PlanOptions::default(), }); @@ -162,25 +188,46 @@ where all_cache_keys.extend(cache_keys.into_iter()); let mut count = 0usize; + let mut reused = 0usize; for WarmUpCachingQueryKey { mut query, operation, + hash, metadata, plan_options, } in all_cache_keys { + let context = Context::new(); + let doc = match query_analysis.parse_document(&query, operation.as_deref()) { + Ok(doc) => doc, + Err(_) => continue, + }; + let caching_key = CachingQueryKey { - schema_id: schema_id.clone(), query: query.clone(), operation: operation.clone(), + hash: doc.hash.clone(), metadata, plan_options, }; - let context = Context::new(); + + if experimental_reuse_query_plans { + // if the query hash did not change with the schema update, we can reuse the previously cached entry + if let Some(hash) = hash { + if hash == doc.hash { + if let Some(entry) = + { previous_cache.lock().await.get(&caching_key).cloned() } + { + self.cache.insert_in_memory(caching_key, entry).await; + reused += 1; + continue; + } + } + } + } let entry = self.cache.get(&caching_key).await; if entry.is_first() { - let doc = query_analysis.parse_document(&query); let err_res = Query::check_errors(&doc); if let Err(error) = err_res { let e = Arc::new(QueryPlannerError::SpecError(error)); @@ -230,7 +277,7 @@ where } } - tracing::debug!("warmed up the query planner cache with {count} queries"); + tracing::debug!("warmed up the query planner cache with {count} queries planned and {reused} queries reused"); } } @@ -306,8 +353,6 @@ where mut self, request: query_planner::CachingRequest, ) -> Result<>::Response, CacheResolverError> { - let schema_id = self.schema.schema_id.clone(); - if self.enable_authorization_directives { AuthorizationPlugin::update_cache_key(&request.context); } @@ -320,10 +365,21 @@ where .unwrap_or_default(), }; + let doc = match request.context.extensions().lock().get::() { + None => { + return Err(CacheResolverError::RetrievalError(Arc::new( + QueryPlannerError::SpecError(SpecError::ParsingError( + "missing parsed document".to_string(), + )), + ))) + } + Some(d) => d.clone(), + }; + let caching_key = CachingQueryKey { - schema_id, query: request.query.clone(), operation: request.operation_name.to_owned(), + hash: doc.hash.clone(), metadata: request .context .extensions() @@ -343,17 +399,6 @@ where context, } = request; - let doc = match context.extensions().lock().get::() { - None => { - return Err(CacheResolverError::RetrievalError(Arc::new( - QueryPlannerError::SpecError(SpecError::ParsingError( - "missing parsed document".to_string(), - )), - ))) - } - Some(d) => d.clone(), - }; - let schema = &self.schema.api_schema().definitions; if let Ok(modified_query) = add_defer_labels(schema, &doc.ast) { query = modified_query.to_string(); @@ -490,11 +535,11 @@ fn stats_report_key_hash(stats_report_key: &str) -> String { hex::encode(result) } -#[derive(Debug, Clone, Hash, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct CachingQueryKey { - pub(crate) schema_id: Option, pub(crate) query: String, pub(crate) operation: Option, + pub(crate) hash: Arc, pub(crate) metadata: CacheKeyMetadata, pub(crate) plan_options: PlanOptions, } @@ -503,14 +548,6 @@ const FEDERATION_VERSION: &str = std::env!("FEDERATION_VERSION"); impl std::fmt::Display for CachingQueryKey { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let mut hasher = Sha256::new(); - hasher.update(&self.query); - let query = hex::encode(hasher.finalize()); - - let mut hasher = Sha256::new(); - hasher.update(self.operation.as_deref().unwrap_or("-")); - let operation = hex::encode(hasher.finalize()); - let mut hasher = Sha256::new(); hasher.update(&serde_json::to_vec(&self.metadata).expect("serialization should not fail")); hasher.update( @@ -518,15 +555,15 @@ impl std::fmt::Display for CachingQueryKey { ); let metadata = hex::encode(hasher.finalize()); - write!( - f, - "plan:{}:{}:{}:{}:{}", - FEDERATION_VERSION, - self.schema_id.as_deref().unwrap_or("-"), - query, - operation, - metadata, - ) + write!(f, "plan:{}:{}:{}", FEDERATION_VERSION, self.hash, metadata,) + } +} + +impl Hash for CachingQueryKey { + fn hash(&self, state: &mut H) { + self.hash.0.hash(state); + self.metadata.hash(state); + self.plan_options.hash(state); } } @@ -534,6 +571,7 @@ impl std::fmt::Display for CachingQueryKey { pub(crate) struct WarmUpCachingQueryKey { pub(crate) query: String, pub(crate) operation: Option, + pub(crate) hash: Option>, pub(crate) metadata: CacheKeyMetadata, pub(crate) plan_options: PlanOptions, } @@ -619,7 +657,13 @@ mod tests { let schema = Schema::parse(include_str!("testdata/schema.graphql"), &configuration).unwrap(); - let doc1 = Query::parse_document("query Me { me { username } }", &schema, &configuration); + let doc1 = Query::parse_document( + "query Me { me { username } }", + None, + &schema, + &configuration, + ) + .unwrap(); let context = Context::new(); context.extensions().lock().insert::(doc1); @@ -636,9 +680,11 @@ mod tests { } let doc2 = Query::parse_document( "query Me { me { name { first } } }", + None, &schema, &configuration, - ); + ) + .unwrap(); let context = Context::new(); context.extensions().lock().insert::(doc2); @@ -692,7 +738,13 @@ mod tests { let schema = Schema::parse(include_str!("testdata/schema.graphql"), &configuration).unwrap(); - let doc = Query::parse_document("query Me { me { username } }", &schema, &configuration); + let doc = Query::parse_document( + "query Me { me { username } }", + None, + &schema, + &configuration, + ) + .unwrap(); let mut planner = CachingQueryPlanner::new(delegate, Arc::new(schema), &configuration, IndexMap::new()) diff --git a/apollo-router/src/query_planner/fetch.rs b/apollo-router/src/query_planner/fetch.rs index a4c88c546d..07207b23b1 100644 --- a/apollo-router/src/query_planner/fetch.rs +++ b/apollo-router/src/query_planner/fetch.rs @@ -1,7 +1,8 @@ use std::fmt::Display; use std::sync::Arc; -use apollo_compiler::ast::Document; +use apollo_compiler::validation::Valid; +use apollo_compiler::ExecutableDocument; use indexmap::IndexSet; use serde::Deserialize; use serde::Serialize; @@ -27,7 +28,6 @@ use crate::plugins::authorization::AuthorizationPlugin; use crate::plugins::authorization::CacheKeyMetadata; use crate::services::SubgraphRequest; use crate::spec::query::change::QueryHashVisitor; -use crate::spec::query::traverse; use crate::spec::Schema; /// GraphQL operation type. @@ -129,7 +129,7 @@ pub(crate) struct FetchNode { pub(crate) authorization: Arc, } -#[derive(Clone, Default, PartialEq, Deserialize, Serialize)] +#[derive(Clone, Default, Hash, PartialEq, Eq, Deserialize, Serialize)] pub(crate) struct QueryHash(#[serde(with = "hex")] pub(crate) Vec); impl std::fmt::Debug for QueryHash { @@ -140,6 +140,12 @@ impl std::fmt::Debug for QueryHash { } } +impl Display for QueryHash { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", hex::encode(&self.0)) + } +} + pub(crate) struct Variables { pub(crate) variables: Object, pub(crate) inverted_paths: Vec>, @@ -491,14 +497,13 @@ impl FetchNode { &self.operation_kind } - pub(crate) fn hash_subquery(&mut self, schema: &apollo_compiler::Schema) { - let doc = Document::parse(&self.operation, "query.graphql") + pub(crate) fn hash_subquery(&mut self, schema: &Valid) { + let doc = ExecutableDocument::parse(schema, &self.operation, "query.graphql") .expect("subgraph queries should be valid"); - let mut visitor = QueryHashVisitor::new(schema, &doc); - visitor.subgraph_query = !self.requires.is_empty(); - if traverse::document(&mut visitor, &doc).is_ok() { - self.schema_aware_hash = Arc::new(QueryHash(visitor.finish())); + if let Ok(hash) = QueryHashVisitor::hash_query(schema, &doc, self.operation_name.as_deref()) + { + self.schema_aware_hash = Arc::new(QueryHash(hash)); } } @@ -507,11 +512,19 @@ impl FetchNode { schema: &apollo_compiler::Schema, global_authorisation_cache_key: &CacheKeyMetadata, ) { - let doc = Document::parse(&self.operation, "query.graphql") - // Assume query planing creates a valid document: ignore parse errors - .unwrap_or_else(|invalid| invalid.partial); - let subgraph_query_cache_key = - AuthorizationPlugin::generate_cache_metadata(&doc, schema, !self.requires.is_empty()); + let doc = ExecutableDocument::parse( + Valid::assume_valid_ref(schema), + &self.operation, + "query.graphql", + ) + // Assume query planing creates a valid document: ignore parse errors + .unwrap_or_else(|invalid| invalid.partial); + let subgraph_query_cache_key = AuthorizationPlugin::generate_cache_metadata( + &doc, + self.operation_name.as_deref(), + schema, + !self.requires.is_empty(), + ); // we need to intersect the cache keys because the global key already takes into account // the scopes and policies from the client request diff --git a/apollo-router/src/query_planner/plan.rs b/apollo-router/src/query_planner/plan.rs index 1504e8c8b4..06e3519484 100644 --- a/apollo-router/src/query_planner/plan.rs +++ b/apollo-router/src/query_planner/plan.rs @@ -1,5 +1,7 @@ +use std::collections::HashMap; use std::sync::Arc; +use apollo_compiler::validation::Valid; use router_bridge::planner::PlanOptions; use router_bridge::planner::UsageReporting; use serde::Deserialize; @@ -225,38 +227,43 @@ impl PlanNode { } } - pub(crate) fn hash_subqueries(&mut self, schema: &apollo_compiler::Schema) { + pub(crate) fn hash_subqueries( + &mut self, + schemas: &HashMap>>, + ) { match self { PlanNode::Fetch(fetch_node) => { - fetch_node.hash_subquery(schema); + if let Some(schema) = schemas.get(&fetch_node.service_name) { + fetch_node.hash_subquery(schema); + } } PlanNode::Sequence { nodes } => { for node in nodes { - node.hash_subqueries(schema); + node.hash_subqueries(schemas); } } PlanNode::Parallel { nodes } => { for node in nodes { - node.hash_subqueries(schema); + node.hash_subqueries(schemas); } } - PlanNode::Flatten(flatten) => flatten.node.hash_subqueries(schema), + PlanNode::Flatten(flatten) => flatten.node.hash_subqueries(schemas), PlanNode::Defer { primary, deferred } => { if let Some(node) = primary.node.as_mut() { - node.hash_subqueries(schema); + node.hash_subqueries(schemas); } for deferred_node in deferred { if let Some(node) = deferred_node.node.take() { let mut new_node = (*node).clone(); - new_node.hash_subqueries(schema); + new_node.hash_subqueries(schemas); deferred_node.node = Some(Arc::new(new_node)); } } } PlanNode::Subscription { primary: _, rest } => { if let Some(node) = rest.as_mut() { - node.hash_subqueries(schema); + node.hash_subqueries(schemas); } } PlanNode::Condition { @@ -265,10 +272,10 @@ impl PlanNode { else_clause, } => { if let Some(node) = if_clause.as_mut() { - node.hash_subqueries(schema); + node.hash_subqueries(schemas); } if let Some(node) = else_clause.as_mut() { - node.hash_subqueries(schema); + node.hash_subqueries(schemas); } } } diff --git a/apollo-router/src/query_planner/snapshots/apollo_router__query_planner__bridge_query_planner__tests__plan_root.snap b/apollo-router/src/query_planner/snapshots/apollo_router__query_planner__bridge_query_planner__tests__plan_root.snap index 308d3371dc..5e07a0cfd1 100644 --- a/apollo-router/src/query_planner/snapshots/apollo_router__query_planner__bridge_query_planner__tests__plan_root.snap +++ b/apollo-router/src/query_planner/snapshots/apollo_router__query_planner__bridge_query_planner__tests__plan_root.snap @@ -14,7 +14,7 @@ Fetch( input_rewrites: None, output_rewrites: None, schema_aware_hash: QueryHash( - "8d5fa7b9c03ada5f622a0b8904666d3209ffc753a1ae3a647cc1c61b6892f220", + "68a86d7602ea2876e77b84d7942f585ef6b6101887bb2979d1f0af3b28c9a0ed", ), authorization: CacheKeyMetadata { is_authenticated: false, diff --git a/apollo-router/src/query_planner/tests.rs b/apollo-router/src/query_planner/tests.rs index 561321c5e2..21ff529b05 100644 --- a/apollo-router/src/query_planner/tests.rs +++ b/apollo-router/src/query_planner/tests.rs @@ -435,6 +435,7 @@ async fn defer_if_condition() { query: Arc::new( Query::parse( query, + Some("Me"), &schema, &Configuration::fake_builder().build().unwrap(), ) diff --git a/apollo-router/src/router_factory.rs b/apollo-router/src/router_factory.rs index e126d65d6c..5fa305e38f 100644 --- a/apollo-router/src/router_factory.rs +++ b/apollo-router/src/router_factory.rs @@ -244,12 +244,19 @@ impl YamlRouterFactory { let persisted_query_layer = Arc::new(PersistedQueryLayer::new(&configuration).await?); if let Some(previous_router) = previous_router { - let cache_keys = previous_router - .cache_keys(configuration.supergraph.query_planning.warmed_up_queries) - .await; + let previous_cache = previous_router.previous_cache(); supergraph_creator - .warm_up_query_planner(&query_analysis_layer, &persisted_query_layer, cache_keys) + .warm_up_query_planner( + &query_analysis_layer, + &persisted_query_layer, + previous_cache, + configuration.supergraph.query_planning.warmed_up_queries, + configuration + .supergraph + .query_planning + .experimental_reuse_query_plans, + ) .await; }; RouterCreator::new( @@ -850,7 +857,10 @@ mod test { let schema = include_str!("testdata/starstuff@current.graphql"); let mut config = json!({ "apollo": {} }); let schema = Schema::parse_test(schema, &Default::default()).unwrap(); - inject_schema_id(schema.api_schema().schema_id.as_deref(), &mut config); + inject_schema_id( + Some(&Schema::schema_id(&schema.api_schema().raw_sdl)), + &mut config, + ); let config = serde_json::from_value::(config).unwrap(); assert_eq!( diff --git a/apollo-router/src/services/layers/allow_only_http_post_mutations.rs b/apollo-router/src/services/layers/allow_only_http_post_mutations.rs index 6b173fb9a1..619685c5f4 100644 --- a/apollo-router/src/services/layers/allow_only_http_post_mutations.rs +++ b/apollo-router/src/services/layers/allow_only_http_post_mutations.rs @@ -287,6 +287,7 @@ mod forbid_http_get_mutations_tests { .insert::(Arc::new(ParsedDocumentInner { ast, executable: Arc::new(executable), + hash: Default::default(), parse_errors: None, validation_errors: None, })); diff --git a/apollo-router/src/services/layers/query_analysis.rs b/apollo-router/src/services/layers/query_analysis.rs index e5b6da7224..07630df06b 100644 --- a/apollo-router/src/services/layers/query_analysis.rs +++ b/apollo-router/src/services/layers/query_analysis.rs @@ -12,12 +12,16 @@ use tokio::sync::Mutex; use crate::context::OPERATION_KIND; use crate::context::OPERATION_NAME; +use crate::graphql::Error; +use crate::graphql::ErrorExtension; use crate::plugins::authorization::AuthorizationPlugin; +use crate::query_planner::fetch::QueryHash; use crate::query_planner::OperationKind; use crate::services::SupergraphRequest; use crate::services::SupergraphResponse; use crate::spec::Query; use crate::spec::Schema; +use crate::spec::SpecError; use crate::Configuration; use crate::Context; @@ -56,8 +60,17 @@ impl QueryAnalysisLayer { } } - pub(crate) fn parse_document(&self, query: &str) -> ParsedDocument { - Query::parse_document(query, self.schema.api_schema(), &self.configuration) + pub(crate) fn parse_document( + &self, + query: &str, + operation_name: Option<&str>, + ) -> Result { + Query::parse_document( + query, + operation_name, + self.schema.api_schema(), + &self.configuration, + ) } pub(crate) async fn supergraph_request( @@ -107,7 +120,20 @@ impl QueryAnalysisLayer { let (context, doc) = match entry { None => { let span = tracing::info_span!("parse_query", "otel.kind" = "INTERNAL"); - let doc = span.in_scope(|| self.parse_document(&query)); + let doc = match span.in_scope(|| self.parse_document(&query, op_name.as_deref())) { + Ok(doc) => doc, + Err(err) => { + return Err(SupergraphResponse::builder() + .errors(vec![Error::builder() + .message(err.to_string()) + .extension_code(err.extension_code()) + .build()]) + .status_code(StatusCode::BAD_REQUEST) + .context(request.context) + .build() + .expect("response is valid")); + } + }; let context = Context::new(); @@ -123,12 +149,23 @@ impl QueryAnalysisLayer { .expect("cannot insert operation kind in the context; this is a bug"); if self.enable_authorization_directives { - AuthorizationPlugin::query_analysis( + if let Err(err) = AuthorizationPlugin::query_analysis( &query, + op_name.as_deref(), &self.schema, &self.configuration, &context, - ); + ) { + return Err(SupergraphResponse::builder() + .errors(vec![Error::builder() + .message(err.to_string()) + .extension_code(err.extension_code()) + .build()]) + .status_code(StatusCode::BAD_REQUEST) + .context(request.context) + .build() + .expect("response is valid")); + } } (*self.cache.lock().await).put( @@ -164,6 +201,7 @@ pub(crate) type ParsedDocument = Arc; pub(crate) struct ParsedDocumentInner { pub(crate) ast: ast::Document, pub(crate) executable: Arc, + pub(crate) hash: Arc, pub(crate) parse_errors: Option, pub(crate) validation_errors: Option, } @@ -176,7 +214,7 @@ impl Display for ParsedDocumentInner { impl Hash for ParsedDocumentInner { fn hash(&self, state: &mut H) { - self.ast.hash(state); + self.hash.0.hash(state); } } diff --git a/apollo-router/src/services/router/service.rs b/apollo-router/src/services/router/service.rs index ad271f86ed..2591fc3aca 100644 --- a/apollo-router/src/services/router/service.rs +++ b/apollo-router/src/services/router/service.rs @@ -45,7 +45,7 @@ use crate::http_ext; use crate::plugin::test::MockSupergraphService; use crate::protocols::multipart::Multipart; use crate::protocols::multipart::ProtocolMode; -use crate::query_planner::WarmUpCachingQueryKey; +use crate::query_planner::InMemoryCachePlanner; use crate::router_factory::RouterFactory; use crate::services::layers::apq::APQLayer; use crate::services::layers::content_negotiation; @@ -841,7 +841,7 @@ impl RouterCreator { } impl RouterCreator { - pub(crate) async fn cache_keys(&self, count: Option) -> Vec { - self.supergraph_creator.cache_keys(count).await + pub(crate) fn previous_cache(&self) -> InMemoryCachePlanner { + self.supergraph_creator.previous_cache() } } diff --git a/apollo-router/src/services/supergraph/service.rs b/apollo-router/src/services/supergraph/service.rs index 3db4dc718c..4a87809820 100644 --- a/apollo-router/src/services/supergraph/service.rs +++ b/apollo-router/src/services/supergraph/service.rs @@ -27,6 +27,7 @@ use tracing_futures::Instrument; use crate::configuration::Batching; use crate::context::OPERATION_NAME; use crate::error::CacheResolverError; +use crate::error::QueryPlannerError; use crate::graphql; use crate::graphql::IntoGraphQLErrors; use crate::graphql::Response; @@ -42,8 +43,8 @@ use crate::query_planner::subscription::OPENED_SUBSCRIPTIONS; use crate::query_planner::subscription::SUBSCRIPTION_EVENT_SPAN_NAME; use crate::query_planner::BridgeQueryPlanner; use crate::query_planner::CachingQueryPlanner; +use crate::query_planner::InMemoryCachePlanner; use crate::query_planner::QueryPlanResult; -use crate::query_planner::WarmUpCachingQueryKey; use crate::router_factory::create_plugins; use crate::router_factory::create_subgraph_services; use crate::services::execution::QueryPlan; @@ -599,7 +600,13 @@ async fn plan_query( // During a regular request, `ParsedDocument` is already populated during query analysis. // Some tests do populate the document, so we only do it if it's not already there. if !context.extensions().lock().contains_key::() { - let doc = Query::parse_document(&query_str, &schema, &Configuration::default()); + let doc = Query::parse_document( + &query_str, + operation_name.as_deref(), + &schema, + &Configuration::default(), + ) + .map_err(QueryPlannerError::SpecError)?; Query::check_errors(&doc).map_err(crate::error::QueryPlannerError::from)?; Query::validate_query(&doc).map_err(crate::error::QueryPlannerError::from)?; context.extensions().lock().insert::(doc); @@ -832,8 +839,8 @@ impl SupergraphCreator { ) } - pub(crate) async fn cache_keys(&self, count: Option) -> Vec { - self.query_planner_service.cache_keys(count).await + pub(crate) fn previous_cache(&self) -> InMemoryCachePlanner { + self.query_planner_service.previous_cache() } pub(crate) fn planner(&self) -> Arc> { @@ -844,10 +851,18 @@ impl SupergraphCreator { &mut self, query_parser: &QueryAnalysisLayer, persisted_query_layer: &PersistedQueryLayer, - cache_keys: Vec, + previous_cache: InMemoryCachePlanner, + count: Option, + experimental_reuse_query_plans: bool, ) { self.query_planner_service - .warm_up(query_parser, persisted_query_layer, cache_keys) + .warm_up( + query_parser, + persisted_query_layer, + previous_cache, + count, + experimental_reuse_query_plans, + ) .await } } diff --git a/apollo-router/src/spec/mod.rs b/apollo-router/src/spec/mod.rs index cf7c798930..add1927b27 100644 --- a/apollo-router/src/spec/mod.rs +++ b/apollo-router/src/spec/mod.rs @@ -47,6 +47,8 @@ pub(crate) enum SpecError { UnknownOperation(String), /// subscription operation is not supported SubscriptionNotSupported, + /// query hashing failed: {0} + QueryHashing(String), } pub(crate) const GRAPHQL_VALIDATION_FAILURE_ERROR_KEY: &str = "## GraphQLValidationFailure\n"; @@ -75,6 +77,7 @@ impl ErrorExtension for SpecError { SpecError::ValidationError(_) => "GRAPHQL_VALIDATION_FAILED", SpecError::UnknownOperation(_) => "GRAPHQL_VALIDATION_FAILED", SpecError::SubscriptionNotSupported => "SUBSCRIPTION_NOT_SUPPORTED", + SpecError::QueryHashing(_) => "QUERY_HASHING", } .to_string() } diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index ba2754d102..6ef3333211 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -7,7 +7,6 @@ use std::collections::HashMap; use std::collections::HashSet; use std::sync::Arc; -use apollo_compiler::ast; use apollo_compiler::executable; use apollo_compiler::schema::ExtendedType; use apollo_compiler::validation::WithErrors; @@ -17,6 +16,7 @@ use indexmap::IndexSet; use serde::Deserialize; use serde::Serialize; use serde_json_bytes::ByteString; +use tower::BoxError; use tracing::level_filters::LevelFilter; use self::change::QueryHashVisitor; @@ -35,6 +35,7 @@ use crate::json_ext::ResponsePathElement; use crate::json_ext::Value; use crate::plugins::authorization::UnauthorizedPaths; use crate::query_planner::fetch::OperationKind; +use crate::query_planner::fetch::QueryHash; use crate::services::layers::query_analysis::ParsedDocument; use crate::services::layers::query_analysis::ParsedDocumentInner; use crate::spec::FieldType; @@ -278,9 +279,10 @@ impl Query { pub(crate) fn parse_document( query: &str, + operation_name: Option<&str>, schema: &Schema, configuration: &Configuration, - ) -> ParsedDocument { + ) -> Result { let parser = &mut apollo_compiler::Parser::new() .recursion_limit(configuration.limits.parser_max_recursion) .token_limit(configuration.limits.parser_max_tokens); @@ -308,25 +310,30 @@ impl Query { let recursion_limit = parser.recursion_reached(); tracing::trace!(?recursion_limit, "recursion limit data"); - Arc::new(ParsedDocumentInner { + let hash = QueryHashVisitor::hash_query(schema, &executable_document, operation_name) + .map_err(|e| SpecError::QueryHashing(e.to_string()))?; + + Ok(Arc::new(ParsedDocumentInner { ast, executable: Arc::new(executable_document), + hash: Arc::new(QueryHash(hash)), parse_errors, validation_errors, - }) + })) } pub(crate) fn parse( query: impl Into, + operation_name: Option<&str>, schema: &Schema, configuration: &Configuration, - ) -> Result { + ) -> Result { let query = query.into(); - let doc = Self::parse_document(&query, schema, configuration); + let doc = Self::parse_document(&query, operation_name, schema, configuration)?; Self::check_errors(&doc)?; let (fragments, operations, defer_stats, schema_aware_hash) = - Self::extract_query_information(schema, &doc.executable, &doc.ast)?; + Self::extract_query_information(schema, &doc.executable, operation_name)?; Ok(Query { string: query, @@ -362,7 +369,7 @@ impl Query { pub(crate) fn extract_query_information( schema: &Schema, document: &ExecutableDocument, - ast: &ast::Document, + operation_name: Option<&str>, ) -> Result<(Fragments, Vec, DeferStats, Vec), SpecError> { let mut defer_stats = DeferStats { has_defer: false, @@ -375,8 +382,8 @@ impl Query { .map(|operation| Operation::from_hir(operation, schema, &mut defer_stats, &fragments)) .collect::, SpecError>>()?; - let mut visitor = QueryHashVisitor::new(&schema.definitions, ast); - traverse::document(&mut visitor, ast).map_err(|e| { + let mut visitor = QueryHashVisitor::new(&schema.definitions, document); + traverse::document(&mut visitor, document, operation_name).map_err(|e| { SpecError::ParsingError(format!("could not calculate the query hash: {e}")) })?; let hash = visitor.finish(); diff --git a/apollo-router/src/spec/query/change.rs b/apollo-router/src/spec/query/change.rs index 43bf4227f8..bebb8efa72 100644 --- a/apollo-router/src/spec/query/change.rs +++ b/apollo-router/src/spec/query/change.rs @@ -4,17 +4,28 @@ use std::hash::Hash; use std::hash::Hasher; use apollo_compiler::ast; -use apollo_compiler::ast::Selection; +use apollo_compiler::ast::Argument; +use apollo_compiler::ast::FieldDefinition; +use apollo_compiler::ast::Name; +use apollo_compiler::executable; use apollo_compiler::schema; +use apollo_compiler::schema::DirectiveList; use apollo_compiler::schema::ExtendedType; +use apollo_compiler::validation::Valid; use apollo_compiler::Node; +use apollo_compiler::NodeStr; +use apollo_compiler::Parser; use sha2::Digest; use sha2::Sha256; use tower::BoxError; -use super::transform; use super::traverse; -use crate::plugins::cache::entity::ENTITIES; +use super::traverse::Visitor; +use crate::plugins::progressive_override::JOIN_FIELD_DIRECTIVE_NAME; +use crate::plugins::progressive_override::JOIN_SPEC_BASE_URL; +use crate::spec::Schema; + +pub(crate) const JOIN_TYPE_DIRECTIVE_NAME: &str = "join__type"; /// Calculates a hash of the query and the schema, but only looking at the parts of the /// schema which affect the query. @@ -23,25 +34,51 @@ use crate::plugins::cache::entity::ENTITIES; pub(crate) struct QueryHashVisitor<'a> { schema: &'a schema::Schema, hasher: Sha256, - fragments: HashMap<&'a ast::Name, &'a ast::FragmentDefinition>, + fragments: HashMap<&'a ast::Name, &'a Node>, hashed_types: HashSet, // name, field hashed_fields: HashSet<(String, String)>, - pub(crate) subgraph_query: bool, + join_field_directive_name: Option, + join_type_directive_name: Option, } impl<'a> QueryHashVisitor<'a> { - pub(crate) fn new(schema: &'a schema::Schema, executable: &'a ast::Document) -> Self { + pub(crate) fn new( + schema: &'a schema::Schema, + executable: &'a executable::ExecutableDocument, + ) -> Self { Self { schema, hasher: Sha256::new(), - fragments: transform::collect_fragments(executable), + fragments: executable.fragments.iter().collect(), hashed_types: HashSet::new(), hashed_fields: HashSet::new(), - subgraph_query: false, + // should we just return an error if we do not find those directives? + join_field_directive_name: Schema::directive_name( + schema, + JOIN_SPEC_BASE_URL, + ">=0.1.0", + JOIN_FIELD_DIRECTIVE_NAME, + ), + join_type_directive_name: Schema::directive_name( + schema, + JOIN_SPEC_BASE_URL, + ">=0.1.0", + JOIN_TYPE_DIRECTIVE_NAME, + ), } } + pub(crate) fn hash_query( + schema: &'a schema::Schema, + executable: &'a executable::ExecutableDocument, + operation_name: Option<&str>, + ) -> Result, BoxError> { + let mut visitor = QueryHashVisitor::new(schema, executable); + traverse::document(&mut visitor, executable, operation_name)?; + Ok(visitor.finish()) + } + pub(crate) fn finish(self) -> Vec { self.hasher.finalize().as_slice().into() } @@ -104,19 +141,20 @@ impl<'a> QueryHashVisitor<'a> { } } - fn hash_type_by_name(&mut self, t: &str) { + fn hash_type_by_name(&mut self, t: &str) -> Result<(), BoxError> { if self.hashed_types.contains(t) { - return; + return Ok(()); } self.hashed_types.insert(t.to_string()); if let Some(ty) = self.schema.types.get(t) { - self.hash_extended_type(ty); + self.hash_extended_type(ty)?; } + Ok(()) } - fn hash_extended_type(&mut self, t: &'a ExtendedType) { + fn hash_extended_type(&mut self, t: &'a ExtendedType) -> Result<(), BoxError> { match t { ExtendedType::Scalar(s) => { for directive in &s.directives { @@ -127,11 +165,14 @@ impl<'a> QueryHashVisitor<'a> { for directive in &o.directives { self.hash_directive(&directive.node); } + + self.hash_join_type(&o.name, &o.directives)?; } ExtendedType::Interface(i) => { for directive in &i.directives { self.hash_directive(&directive.node); } + self.hash_join_type(&i.name, &i.directives)?; } ExtendedType::Union(u) => { for directive in &u.directives { @@ -139,7 +180,7 @@ impl<'a> QueryHashVisitor<'a> { } for member in &u.members { - self.hash_type_by_name(member.as_str()); + self.hash_type_by_name(member.as_str())?; } } ExtendedType::Enum(e) => { @@ -162,14 +203,15 @@ impl<'a> QueryHashVisitor<'a> { for (name, ty) in &o.fields { if ty.default_value.is_some() { name.hash(self); - self.hash_input_value_definition(&ty.node); + self.hash_input_value_definition(&ty.node)?; } } } } + Ok(()) } - fn hash_type(&mut self, t: &ast::Type) { + fn hash_type(&mut self, t: &ast::Type) -> Result<(), BoxError> { match t { schema::Type::Named(name) => self.hash_type_by_name(name.as_str()), schema::Type::NonNullNamed(name) => { @@ -178,58 +220,116 @@ impl<'a> QueryHashVisitor<'a> { } schema::Type::List(t) => { "[]".hash(self); - self.hash_type(t); + self.hash_type(t) } schema::Type::NonNullList(t) => { "[]!".hash(self); - self.hash_type(t); + self.hash_type(t) } } } - fn hash_input_value_definition(&mut self, t: &Node) { - self.hash_type(&t.ty); + fn hash_field( + &mut self, + parent_type: String, + type_name: String, + field_def: &FieldDefinition, + arguments: &[Node], + ) -> Result<(), BoxError> { + if self.hashed_fields.insert((parent_type.clone(), type_name)) { + self.hash_type_by_name(&parent_type)?; + + field_def.name.hash(self); + + for argument in &field_def.arguments { + self.hash_input_value_definition(argument)?; + } + + for argument in arguments { + self.hash_argument(argument); + } + + self.hash_type(&field_def.ty)?; + + for directive in &field_def.directives { + self.hash_directive(directive); + } + + self.hash_join_field(&parent_type, &field_def.directives)?; + } + Ok(()) + } + + fn hash_input_value_definition( + &mut self, + t: &Node, + ) -> Result<(), BoxError> { + self.hash_type(&t.ty)?; for directive in &t.directives { self.hash_directive(directive); } if let Some(value) = t.default_value.as_ref() { self.hash_value(value); } + Ok(()) } - fn hash_entities_operation(&mut self, node: &ast::OperationDefinition) -> Result<(), BoxError> { - use crate::spec::query::traverse::Visitor; - - if node.selection_set.len() != 1 { - return Err("invalid number of selections for _entities query".into()); + fn hash_join_type(&mut self, name: &Name, directives: &DirectiveList) -> Result<(), BoxError> { + if let Some(dir_name) = self.join_type_directive_name.as_deref() { + if let Some(dir) = directives.get(dir_name) { + if let Some(key) = dir.argument_by_name("key").and_then(|arg| arg.as_str()) { + let mut parser = Parser::new(); + if let Ok(field_set) = parser.parse_field_set( + Valid::assume_valid_ref(self.schema), + name.clone(), + key, + std::path::Path::new("schema.graphql"), + ) { + traverse::selection_set( + self, + name.as_str(), + &field_set.selection_set.selections[..], + )?; + } + } + } } - match node.selection_set.first() { - Some(Selection::Field(field)) => { - if field.name.as_str() != ENTITIES { - return Err("expected _entities field".into()); - } + Ok(()) + } - "_entities".hash(self); - - for selection in &field.selection_set { - match selection { - Selection::InlineFragment(f) => { - match f.type_condition.as_ref() { - None => { - return Err("expected type condition".into()); - } - Some(condition) => self.inline_fragment(condition.as_str(), f)?, - }; + fn hash_join_field( + &mut self, + parent_type: &str, + directives: &ast::DirectiveList, + ) -> Result<(), BoxError> { + if let Some(dir_name) = self.join_field_directive_name.as_deref() { + if let Some(dir) = directives.get(dir_name) { + if let Some(requires) = dir + .argument_by_name("requires") + .and_then(|arg| arg.as_str()) + { + if let Ok(parent_type) = Name::new(NodeStr::new(parent_type)) { + let mut parser = Parser::new(); + + if let Ok(field_set) = parser.parse_field_set( + Valid::assume_valid_ref(self.schema), + parent_type.clone(), + requires, + std::path::Path::new("schema.graphql"), + ) { + traverse::selection_set( + self, + parent_type.as_str(), + &field_set.selection_set.selections[..], + )?; } - Selection::FragmentSpread(f) => self.fragment_spread(f)?, - _ => return Err("expected inline fragment".into()), } } - Ok(()) } - _ => Err("expected _entities field".into()), } + + Ok(()) } } @@ -243,69 +343,45 @@ impl<'a> Hasher for QueryHashVisitor<'a> { } } -impl<'a> traverse::Visitor for QueryHashVisitor<'a> { - fn operation( - &mut self, - root_type: &str, - node: &ast::OperationDefinition, - ) -> Result<(), BoxError> { +impl<'a> Visitor for QueryHashVisitor<'a> { + fn operation(&mut self, root_type: &str, node: &executable::Operation) -> Result<(), BoxError> { root_type.hash(self); - self.hash_type_by_name(root_type); + self.hash_type_by_name(root_type)?; - if !self.subgraph_query { - traverse::operation(self, root_type, node) - } else { - self.hash_entities_operation(node) - } + traverse::operation(self, root_type, node) } fn field( &mut self, parent_type: &str, field_def: &ast::FieldDefinition, - node: &ast::Field, + node: &executable::Field, ) -> Result<(), BoxError> { - let parent = parent_type.to_string(); - let name = field_def.name.as_str().to_string(); - - if self.hashed_fields.insert((parent, name)) { - self.hash_type_by_name(parent_type); - - field_def.name.hash(self); - - for argument in &field_def.arguments { - self.hash_input_value_definition(argument); - } - - for argument in &node.arguments { - self.hash_argument(argument); - } - - self.hash_type(&field_def.ty); - - for directive in &field_def.directives { - self.hash_directive(directive); - } - } + self.hash_field( + parent_type.to_string(), + field_def.name.as_str().to_string(), + field_def, + &node.arguments, + )?; traverse::field(self, field_def, node) } - fn fragment_definition(&mut self, node: &ast::FragmentDefinition) -> Result<(), BoxError> { + fn fragment(&mut self, node: &executable::Fragment) -> Result<(), BoxError> { node.name.hash(self); - self.hash_type_by_name(&node.type_condition); + self.hash_type_by_name(node.type_condition())?; - traverse::fragment_definition(self, node) + traverse::fragment(self, node) } - fn fragment_spread(&mut self, node: &ast::FragmentSpread) -> Result<(), BoxError> { + fn fragment_spread(&mut self, node: &executable::FragmentSpread) -> Result<(), BoxError> { node.fragment_name.hash(self); let type_condition = &self .fragments .get(&node.fragment_name) .ok_or("MissingFragment")? - .type_condition; - self.hash_type_by_name(type_condition); + .type_condition(); + self.hash_type_by_name(type_condition)?; traverse::fragment_spread(self, node) } @@ -313,10 +389,10 @@ impl<'a> traverse::Visitor for QueryHashVisitor<'a> { fn inline_fragment( &mut self, parent_type: &str, - node: &ast::InlineFragment, + node: &executable::InlineFragment, ) -> Result<(), BoxError> { if let Some(type_condition) = &node.type_condition { - self.hash_type_by_name(type_condition); + self.hash_type_by_name(type_condition)?; } traverse::inline_fragment(self, parent_type, node) } @@ -330,6 +406,7 @@ impl<'a> traverse::Visitor for QueryHashVisitor<'a> { mod tests { use apollo_compiler::ast::Document; use apollo_compiler::schema::Schema; + use apollo_compiler::validation::Valid; use super::QueryHashVisitor; use crate::spec::query::traverse; @@ -342,24 +419,28 @@ mod tests { .unwrap(); let doc = Document::parse(query, "query.graphql").unwrap(); - doc.to_executable(&schema) + let exec = doc + .to_executable(&schema) .unwrap() .validate(&schema) .unwrap(); - let mut visitor = QueryHashVisitor::new(&schema, &doc); - traverse::document(&mut visitor, &doc).unwrap(); + let mut visitor = QueryHashVisitor::new(&schema, &exec); + traverse::document(&mut visitor, &exec, None).unwrap(); hex::encode(visitor.finish()) } #[track_caller] fn hash_subgraph_query(schema: &str, query: &str) -> String { - let schema = Schema::parse(schema, "schema.graphql").unwrap(); + let schema = Valid::assume_valid(Schema::parse(schema, "schema.graphql").unwrap()); let doc = Document::parse(query, "query.graphql").unwrap(); - //doc.to_executable(&schema); - let mut visitor = QueryHashVisitor::new(&schema, &doc); - visitor.subgraph_query = true; - traverse::document(&mut visitor, &doc).unwrap(); + let exec = doc + .to_executable(&schema) + .unwrap() + .validate(&schema) + .unwrap(); + let mut visitor = QueryHashVisitor::new(&schema, &exec); + traverse::document(&mut visitor, &exec, None).unwrap(); hex::encode(visitor.finish()) } @@ -555,7 +636,12 @@ mod tests { query: Query } + scalar _Any + + union _Entity = User + type Query { + _entities(representations: [_Any!]!): [_Entity]! me: User customer: User } @@ -571,7 +657,12 @@ mod tests { query: Query } + scalar _Any + + union _Entity = User + type Query { + _entities(representations: [_Any!]!): [_Entity]! me: User } @@ -619,4 +710,244 @@ mod tests { assert_eq!(hash1, hash2); } + + #[test] + fn join_type_key() { + let schema1: &str = r#" + schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) { + query: Query + } + directive @test on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM + directive @join__type(graph: join__Graph!, key: join__FieldSet) repeatable on OBJECT | INTERFACE + directive @join__graph(name: String!, url: String!) on ENUM_VALUE + directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA + + scalar join__FieldSet + + 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 + } + + enum join__Graph { + ACCOUNTS @join__graph(name: "accounts", url: "https://accounts.demo.starstuff.dev") + INVENTORY @join__graph(name: "inventory", url: "https://inventory.demo.starstuff.dev") + PRODUCTS @join__graph(name: "products", url: "https://products.demo.starstuff.dev") + REVIEWS @join__graph(name: "reviews", url: "https://reviews.demo.starstuff.dev") + } + + type Query { + me: User + customer: User + itf: I + } + + type User @join__type(graph: ACCOUNTS, key: "id") { + id: ID! + name: String + } + + interface I @join__type(graph: ACCOUNTS, key: "id") { + id: ID! + name :String + } + + union U = User + "#; + + let schema2: &str = r#" + schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) { + query: Query + } + directive @test on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM + directive @join__type(graph: join__Graph!, key: join__FieldSet) repeatable on OBJECT | INTERFACE + directive @join__graph(name: String!, url: String!) on ENUM_VALUE + directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA + + scalar join__FieldSet + + 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 + } + + enum join__Graph { + ACCOUNTS @join__graph(name: "accounts", url: "https://accounts.demo.starstuff.dev") + INVENTORY @join__graph(name: "inventory", url: "https://inventory.demo.starstuff.dev") + PRODUCTS @join__graph(name: "products", url: "https://products.demo.starstuff.dev") + REVIEWS @join__graph(name: "reviews", url: "https://reviews.demo.starstuff.dev") + } + + type Query { + me: User + customer: User @test + itf: I + } + + type User @join__type(graph: ACCOUNTS, key: "id") { + id: ID! @test + name: String + } + + interface I @join__type(graph: ACCOUNTS, key: "id") { + id: ID! @test + name :String + } + "#; + let query = "query { me { name } }"; + assert_ne!(hash(schema1, query), hash(schema2, query)); + + let query = "query { itf { name } }"; + assert_ne!(hash(schema1, query), hash(schema2, query)); + } + + #[test] + fn join_field_requires() { + let schema1: &str = r#" + schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) { + query: Query + } + directive @test on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM + directive @join__type(graph: join__Graph!, key: join__FieldSet) repeatable on OBJECT | INTERFACE + 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 @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA + + scalar join__FieldSet + + 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 + } + + enum join__Graph { + ACCOUNTS @join__graph(name: "accounts", url: "https://accounts.demo.starstuff.dev") + INVENTORY @join__graph(name: "inventory", url: "https://inventory.demo.starstuff.dev") + PRODUCTS @join__graph(name: "products", url: "https://products.demo.starstuff.dev") + REVIEWS @join__graph(name: "reviews", url: "https://reviews.demo.starstuff.dev") + } + + type Query { + me: User + customer: User + itf: I + } + + type User @join__type(graph: ACCOUNTS, key: "id") { + id: ID! + name: String + username: String @join__field(graph:ACCOUNTS, requires: "name") + a: String @join__field(graph:ACCOUNTS, requires: "itf { ... on A { name } }") + itf: I + } + + interface I @join__type(graph: ACCOUNTS, key: "id") { + id: ID! + name: String + } + + type A implements I @join__type(graph: ACCOUNTS, key: "id") { + id: ID! + name: String + } + "#; + + let schema2: &str = r#" + schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) { + query: Query + } + directive @test on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM + directive @join__type(graph: join__Graph!, key: join__FieldSet) repeatable on OBJECT | INTERFACE + 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 @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA + + scalar join__FieldSet + + 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 + } + + enum join__Graph { + ACCOUNTS @join__graph(name: "accounts", url: "https://accounts.demo.starstuff.dev") + INVENTORY @join__graph(name: "inventory", url: "https://inventory.demo.starstuff.dev") + PRODUCTS @join__graph(name: "products", url: "https://products.demo.starstuff.dev") + REVIEWS @join__graph(name: "reviews", url: "https://reviews.demo.starstuff.dev") + } + + type Query { + me: User + customer: User @test + itf: I + } + + type User @join__type(graph: ACCOUNTS, key: "id") { + id: ID! + name: String @test + username: String @join__field(graph:ACCOUNTS, requires: "name") + a: String @join__field(graph:ACCOUNTS, requires: "itf { ... on A { name } }") + itf: I + } + + interface I @join__type(graph: ACCOUNTS, key: "id") { + id: ID! + name: String @test + } + + type A implements I @join__type(graph: ACCOUNTS, key: "id") { + id: ID! + name: String @test + } + "#; + let query = "query { me { username } }"; + assert_ne!(hash(schema1, query), hash(schema2, query)); + + let query = "query { me { a } }"; + assert_ne!(hash(schema1, query), hash(schema2, query)); + } } diff --git a/apollo-router/src/spec/query/tests.rs b/apollo-router/src/spec/query/tests.rs index bed4cd8ead..65c6d56c92 100644 --- a/apollo-router/src/spec/query/tests.rs +++ b/apollo-router/src/spec/query/tests.rs @@ -127,7 +127,7 @@ impl FormatTest { let api_schema = schema.api_schema(); let query = - Query::parse(query, &schema, &Default::default()).expect("could not parse query"); + Query::parse(query, None, &schema, &Default::default()).expect("could not parse query"); let mut response = Response::builder().data(response).build(); query.format_response( @@ -1405,6 +1405,7 @@ macro_rules! run_validation { .query .as_ref() .expect("query has been added right above; qed"), + None, &schema, &Default::default(), ) @@ -3501,6 +3502,7 @@ fn it_statically_includes() { name } }", + None, &schema, &Default::default(), ) @@ -3523,6 +3525,7 @@ fn it_statically_includes() { name } }", + None, &schema, &Default::default(), ) @@ -3553,6 +3556,7 @@ fn it_statically_includes() { name } }", + None, &schema, &Default::default(), ) @@ -3588,6 +3592,7 @@ fn it_statically_includes() { ...ProductName @include(if: false) } }", + None, &schema, &Default::default(), ) @@ -3646,6 +3651,7 @@ fn it_statically_skips() { name } }", + None, &schema, &Default::default(), ) @@ -3668,6 +3674,7 @@ fn it_statically_skips() { name } }", + None, &schema, &Default::default(), ) @@ -3698,6 +3705,7 @@ fn it_statically_skips() { name } }", + None, &schema, &Default::default(), ) @@ -3733,6 +3741,7 @@ fn it_statically_skips() { ...ProductName @skip(if: true) } }", + None, &schema, &Default::default(), ) @@ -3778,6 +3787,7 @@ fn it_should_fail_with_empty_selection_set() { product { } }", + None, &schema, &Default::default(), ) @@ -5120,7 +5130,8 @@ fn fragment_on_interface_on_query() { let schema = Schema::parse_test(schema, &Default::default()).expect("could not parse schema"); let api_schema = schema.api_schema(); - let query = Query::parse(query, &schema, &Default::default()).expect("could not parse query"); + let query = + Query::parse(query, None, &schema, &Default::default()).expect("could not parse query"); let mut response = Response::builder() .data(json! {{ "object": { @@ -5324,7 +5335,7 @@ fn parse_introspection_query() { } } }"; - assert!(Query::parse(query, api_schema, &Default::default()) + assert!(Query::parse(query, None, api_schema, &Default::default()) .unwrap() .operations .first() @@ -5339,7 +5350,7 @@ fn parse_introspection_query() { } }"; - assert!(Query::parse(query, api_schema, &Default::default()) + assert!(Query::parse(query, None, api_schema, &Default::default()) .unwrap() .operations .first() @@ -5350,7 +5361,7 @@ fn parse_introspection_query() { __typename }"; - assert!(Query::parse(query, api_schema, &Default::default()) + assert!(Query::parse(query, None, api_schema, &Default::default()) .unwrap() .operations .first() @@ -5723,6 +5734,7 @@ fn test_error_path_works_across_inline_fragments() { } } }"#, + Some("MyQueryThatContainsFragments"), &schema, &Default::default(), ) @@ -5760,7 +5772,7 @@ fn test_query_not_named_query() { &config, ) .unwrap(); - let query = Query::parse("{ example }", &schema, &config).unwrap(); + let query = Query::parse("{ example }", None, &schema, &config).unwrap(); let selection = &query.operations[0].selection_set[0]; assert!( matches!( @@ -5826,7 +5838,7 @@ fn filtered_defer_fragment() { .unwrap(); let doc = ast.to_executable(&schema.definitions).unwrap(); let (fragments, operations, defer_stats, schema_aware_hash) = - Query::extract_query_information(&schema, &doc, &ast).unwrap(); + Query::extract_query_information(&schema, &doc, None).unwrap(); let subselections = crate::spec::query::subselections::collect_subselections( &config, @@ -5853,7 +5865,7 @@ fn filtered_defer_fragment() { .unwrap(); let doc = ast.to_executable(&schema.definitions).unwrap(); let (fragments, operations, defer_stats, schema_aware_hash) = - Query::extract_query_information(&schema, &doc, &ast).unwrap(); + Query::extract_query_information(&schema, &doc, None).unwrap(); let subselections = crate::spec::query::subselections::collect_subselections( &config, diff --git a/apollo-router/src/spec/query/traverse.rs b/apollo-router/src/spec/query/traverse.rs index 46494cb960..f8a90d8fc7 100644 --- a/apollo-router/src/spec/query/traverse.rs +++ b/apollo-router/src/spec/query/traverse.rs @@ -1,24 +1,24 @@ use apollo_compiler::ast; +use apollo_compiler::executable; use apollo_compiler::schema::FieldLookupError; +use apollo_compiler::ExecutableDocument; use tower::BoxError; /// Traverse a document with the given visitor. pub(crate) fn document( visitor: &mut impl Visitor, - document: &ast::Document, + document: &ExecutableDocument, + operation_name: Option<&str>, ) -> Result<(), BoxError> { - document.definitions.iter().try_for_each(|def| match def { - ast::Definition::OperationDefinition(def) => { - let root_type = visitor - .schema() - .root_operation(def.operation_type) - .ok_or("missing root operation definition")? - .clone(); - visitor.operation(&root_type, def) - } - ast::Definition::FragmentDefinition(def) => visitor.fragment_definition(def), - _ => Ok(()), - }) + if let Ok(operation) = document.get_operation(operation_name) { + visitor.operation(operation.object_type().as_str(), operation)?; + } + + for fragment in document.fragments.values() { + visitor.fragment(fragment)?; + } + + Ok(()) } pub(crate) trait Visitor: Sized { @@ -28,11 +28,7 @@ pub(crate) trait Visitor: Sized { /// /// Call the [`operation`] free function for the default behavior. /// Return `Ok(None)` to remove this operation. - fn operation( - &mut self, - root_type: &str, - def: &ast::OperationDefinition, - ) -> Result<(), BoxError> { + fn operation(&mut self, root_type: &str, def: &executable::Operation) -> Result<(), BoxError> { operation(self, root_type, def) } @@ -40,8 +36,8 @@ pub(crate) trait Visitor: Sized { /// /// Call the [`fragment_definition`] free function for the default behavior. /// Return `Ok(None)` to remove this fragment. - fn fragment_definition(&mut self, def: &ast::FragmentDefinition) -> Result<(), BoxError> { - fragment_definition(self, def) + fn fragment(&mut self, def: &executable::Fragment) -> Result<(), BoxError> { + fragment(self, def) } /// Traverse a field within a selection set. @@ -52,7 +48,7 @@ pub(crate) trait Visitor: Sized { &mut self, _parent_type: &str, field_def: &ast::FieldDefinition, - def: &ast::Field, + def: &executable::Field, ) -> Result<(), BoxError> { field(self, field_def, def) } @@ -61,7 +57,7 @@ pub(crate) trait Visitor: Sized { /// /// Call the [`fragment_spread`] free function for the default behavior. /// Return `Ok(None)` to remove this fragment spread. - fn fragment_spread(&mut self, def: &ast::FragmentSpread) -> Result<(), BoxError> { + fn fragment_spread(&mut self, def: &executable::FragmentSpread) -> Result<(), BoxError> { fragment_spread(self, def) } @@ -72,7 +68,7 @@ pub(crate) trait Visitor: Sized { fn inline_fragment( &mut self, parent_type: &str, - def: &ast::InlineFragment, + def: &executable::InlineFragment, ) -> Result<(), BoxError> { inline_fragment(self, parent_type, def) } @@ -84,19 +80,20 @@ pub(crate) trait Visitor: Sized { pub(crate) fn operation( visitor: &mut impl Visitor, root_type: &str, - def: &ast::OperationDefinition, + def: &executable::Operation, ) -> Result<(), BoxError> { - selection_set(visitor, root_type, &def.selection_set) + //FIXME: we should look at directives etc on operation + selection_set(visitor, root_type, &def.selection_set.selections) } /// The default behavior for traversing a fragment definition. /// /// Never returns `Ok(None)`, the `Option` is for `Visitor` impl convenience. -pub(crate) fn fragment_definition( +pub(crate) fn fragment( visitor: &mut impl Visitor, - def: &ast::FragmentDefinition, + def: &executable::Fragment, ) -> Result<(), BoxError> { - selection_set(visitor, &def.type_condition, &def.selection_set)?; + selection_set(visitor, def.type_condition(), &def.selection_set.selections)?; Ok(()) } @@ -106,9 +103,13 @@ pub(crate) fn fragment_definition( pub(crate) fn field( visitor: &mut impl Visitor, field_def: &ast::FieldDefinition, - def: &ast::Field, + def: &executable::Field, ) -> Result<(), BoxError> { - selection_set(visitor, field_def.ty.inner_named_type(), &def.selection_set) + selection_set( + visitor, + field_def.ty.inner_named_type(), + &def.selection_set.selections, + ) } /// The default behavior for traversing a fragment spread. @@ -116,7 +117,7 @@ pub(crate) fn field( /// Never returns `Ok(None)`, the `Option` is for `Visitor` impl convenience. pub(crate) fn fragment_spread( visitor: &mut impl Visitor, - def: &ast::FragmentSpread, + def: &executable::FragmentSpread, ) -> Result<(), BoxError> { let _ = (visitor, def); // Unused, but matches trait method signature Ok(()) @@ -128,18 +129,18 @@ pub(crate) fn fragment_spread( pub(crate) fn inline_fragment( visitor: &mut impl Visitor, parent_type: &str, - def: &ast::InlineFragment, + def: &executable::InlineFragment, ) -> Result<(), BoxError> { - selection_set(visitor, parent_type, &def.selection_set) + selection_set(visitor, parent_type, &def.selection_set.selections) } -fn selection_set( +pub(crate) fn selection_set( visitor: &mut impl Visitor, parent_type: &str, - set: &[ast::Selection], + set: &[executable::Selection], ) -> Result<(), BoxError> { set.iter().try_for_each(|def| match def { - ast::Selection::Field(def) => { + executable::Selection::Field(def) => { let field_def = &visitor .schema() .type_field(parent_type, &def.name) @@ -152,8 +153,8 @@ fn selection_set( .clone(); visitor.field(parent_type, field_def, def) } - ast::Selection::FragmentSpread(def) => visitor.fragment_spread(def), - ast::Selection::InlineFragment(def) => { + executable::Selection::FragmentSpread(def) => visitor.fragment_spread(def), + executable::Selection::InlineFragment(def) => { let fragment_type = def .type_condition .as_ref() @@ -166,6 +167,8 @@ fn selection_set( #[test] fn test_count_fields() { + use apollo_compiler::validation::Valid; + struct CountFields { schema: apollo_compiler::Schema, fields: u32, @@ -176,7 +179,7 @@ fn test_count_fields() { &mut self, _parent_type: &str, field_def: &ast::FieldDefinition, - def: &ast::Field, + def: &executable::Field, ) -> Result<(), BoxError> { self.fields += 1; field(self, field_def, def) @@ -187,14 +190,14 @@ fn test_count_fields() { } } - let graphql = " - type Query { - a(id: ID): String - b: Int - next: Query - } - directive @defer(label: String, if: Boolean! = true) on FRAGMENT_SPREAD | INLINE_FRAGMENT - + let schema = " + type Query { + a(id: ID): String + b: Int + next: Query + } + directive @defer(label: String, if: Boolean! = true) on FRAGMENT_SPREAD | INLINE_FRAGMENT"; + let query = " query($id: ID = null) { a(id: $id) ... @defer { @@ -210,10 +213,12 @@ fn test_count_fields() { } } "; - let ast = apollo_compiler::ast::Document::parse(graphql, "").unwrap(); - let (schema, _doc) = ast.to_mixed_validate().unwrap(); + let ast = apollo_compiler::ast::Document::parse(schema, "").unwrap(); + let schema = ast.to_schema_validate().unwrap(); let schema = schema.into_inner(); + let executable = + ExecutableDocument::parse(Valid::assume_valid_ref(&schema), query, "").unwrap(); let mut visitor = CountFields { fields: 0, schema }; - document(&mut visitor, &ast).unwrap(); + document(&mut visitor, &executable, None).unwrap(); assert_eq!(visitor.fields, 4) } diff --git a/apollo-router/src/spec/schema.rs b/apollo-router/src/spec/schema.rs index f10d7b34a4..c1fffc5b98 100644 --- a/apollo-router/src/spec/schema.rs +++ b/apollo-router/src/spec/schema.rs @@ -33,7 +33,6 @@ pub(crate) struct Schema { subgraphs: HashMap, pub(crate) implementers_map: HashMap, api_schema: Option>, - pub(crate) schema_id: Option, } impl Schema { @@ -133,7 +132,6 @@ impl Schema { } } - let schema_id = Some(Self::schema_id(sdl)); tracing::info!( histogram.apollo.router.schema.load.duration = start.elapsed().as_secs_f64() ); @@ -147,7 +145,6 @@ impl Schema { subgraphs, implementers_map, api_schema: None, - schema_id, }) } @@ -585,17 +582,13 @@ mod tests { let schema = Schema::parse_test(schema, &Default::default()).unwrap(); assert_eq!( - schema.schema_id, - Some( - "8e2021d131b23684671c3b85f82dfca836908c6a541bbd5c3772c66e7f8429d8".to_string() - ) + Schema::schema_id(&schema.raw_sdl), + "8e2021d131b23684671c3b85f82dfca836908c6a541bbd5c3772c66e7f8429d8".to_string() ); assert_eq!( - schema.api_schema().schema_id, - Some( - "6af283f857f47055b0069547a8ee21c942c2c72ceebbcaabf78a42f0d1786318".to_string() - ) + Schema::schema_id(&schema.api_schema().raw_sdl), + "6af283f857f47055b0069547a8ee21c942c2c72ceebbcaabf78a42f0d1786318".to_string() ); } } diff --git a/apollo-router/tests/integration/redis.rs b/apollo-router/tests/integration/redis.rs index bacea7c657..2ad68cc94f 100644 --- a/apollo-router/tests/integration/redis.rs +++ b/apollo-router/tests/integration/redis.rs @@ -24,7 +24,7 @@ mod test { // 2. run `docker compose up -d` and connect to the redis container by running `docker exec -ti /bin/bash`. // 3. Run the `redis-cli` command from the shell and start the redis `monitor` command. // 4. Run this test and yank the updated cache key from the redis logs. - let known_cache_key = "plan:v2.7.1:5abb5fecf7df056396fb90fdf38d430b8c1fec55ec132fde878161608af18b76:4c45433039407593557f8a982dafd316a66ec03f0e1ed5fa1b7ef8060d76e8ec:3973e022e93220f9212c18d0d0c543ae7c309e46640da93a4a0314de999f5112:2bf7810d3a47b31d8a77ebb09cdc784a3f77306827dc55b06770030a858167c7"; + let known_cache_key = "plan:v2.7.1:af1ee357bc75cfbbcc6adda41089a56e7d1d52f6d44c049739dde2c259314f58:2bf7810d3a47b31d8a77ebb09cdc784a3f77306827dc55b06770030a858167c7"; let config = RedisConfig::from_url("redis://127.0.0.1:6379")?; let client = RedisClient::new(config, None, None, None); @@ -362,16 +362,13 @@ mod test { insta::assert_json_snapshot!(response); let s:String = client - .get("subgraph:products:Query:530d594c46b838e725b87d64fd6384b82f6ff14bd902b57bba9dcc34ce684b76:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") + .get("subgraph:products:Query:07bd08ba4eb8b85451edd3b3aae3c3ad3dc0892d86deedde6e6d53f6415f807f:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") .await .unwrap(); let v: Value = serde_json::from_str(&s).unwrap(); insta::assert_json_snapshot!(v.as_object().unwrap().get("data").unwrap()); - let s:String = client - .get("subgraph:reviews:Product:4911f7a9dbad8a47b8900d65547503a2f3c0359f65c0bc5652ad9b9843281f66:98424704ece0e377929efa619bce2cbd5246281199c72a0902da863270f5839c:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") - .await - .unwrap(); + let s: String = client.get("subgraph:reviews:Product:4911f7a9dbad8a47b8900d65547503a2f3c0359f65c0bc5652ad9b9843281f66:826d5cf03645266e30655c7475530e2d40e0d5978595b0ab16318b1ce87c0fe1:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c").await.unwrap(); let v: Value = serde_json::from_str(&s).unwrap(); insta::assert_json_snapshot!(v.as_object().unwrap().get("data").unwrap()); @@ -466,7 +463,7 @@ mod test { insta::assert_json_snapshot!(response); let s:String = client - .get("subgraph:reviews:Product:d9a4cd73308dd13ca136390c10340823f94c335b9da198d2339c886c738abf0d:98424704ece0e377929efa619bce2cbd5246281199c72a0902da863270f5839c:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") + .get("subgraph:reviews:Product:d9a4cd73308dd13ca136390c10340823f94c335b9da198d2339c886c738abf0d:826d5cf03645266e30655c7475530e2d40e0d5978595b0ab16318b1ce87c0fe1:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") .await .unwrap(); let v: Value = serde_json::from_str(&s).unwrap(); @@ -676,7 +673,7 @@ mod test { insta::assert_json_snapshot!(response); let s:String = client - .get("subgraph:products:Query:530d594c46b838e725b87d64fd6384b82f6ff14bd902b57bba9dcc34ce684b76:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") + .get("subgraph:products:Query:07bd08ba4eb8b85451edd3b3aae3c3ad3dc0892d86deedde6e6d53f6415f807f:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") .await .unwrap(); let v: Value = serde_json::from_str(&s).unwrap(); @@ -697,7 +694,7 @@ mod test { ); let s: String = client - .get("subgraph:reviews:Product:4911f7a9dbad8a47b8900d65547503a2f3c0359f65c0bc5652ad9b9843281f66:98424704ece0e377929efa619bce2cbd5246281199c72a0902da863270f5839c:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") + .get("subgraph:reviews:Product:4911f7a9dbad8a47b8900d65547503a2f3c0359f65c0bc5652ad9b9843281f66:826d5cf03645266e30655c7475530e2d40e0d5978595b0ab16318b1ce87c0fe1:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") .await .unwrap(); let v: Value = serde_json::from_str(&s).unwrap(); @@ -741,7 +738,7 @@ mod test { insta::assert_json_snapshot!(response); let s:String = client - .get("subgraph:reviews:Product:4911f7a9dbad8a47b8900d65547503a2f3c0359f65c0bc5652ad9b9843281f66:dc8e1fb584d7ad114b3e836a5fe4f642732b82eb39bb8d6dff000d844d0e3baf:f1d914240cfd0c60d5388f3f2d2ae00b5f1e2400ef2c9320252439f354515ce9") + .get("subgraph:reviews:Product:4911f7a9dbad8a47b8900d65547503a2f3c0359f65c0bc5652ad9b9843281f66:c75297b98da101021e30020db99a3a11c2f9ac2008de94ce410c47940162e304:d9d84a3c7ffc27b0190a671212f3740e5b8478e84e23825830e97822e25cf05c") .await .unwrap(); let v: Value = serde_json::from_str(&s).unwrap(); diff --git a/apollo-router/tests/integration/snapshots/integration_tests__integration__redis__test__query_planner.snap b/apollo-router/tests/integration/snapshots/integration_tests__integration__redis__test__query_planner.snap index ade0ce1932..1fad999d1a 100644 --- a/apollo-router/tests/integration/snapshots/integration_tests__integration__redis__test__query_planner.snap +++ b/apollo-router/tests/integration/snapshots/integration_tests__integration__redis__test__query_planner.snap @@ -1,5 +1,5 @@ --- -source: apollo-router/tests/redis_test.rs +source: apollo-router/tests/integration/redis.rs expression: query_plan --- { @@ -12,7 +12,7 @@ expression: query_plan "id": null, "inputRewrites": null, "outputRewrites": null, - "schemaAwareHash": "7026c8330d32f98093b2f24fe09aeff9526e19780a595d09c6c440aaf0cbd6c1", + "schemaAwareHash": "af1ee357bc75cfbbcc6adda41089a56e7d1d52f6d44c049739dde2c259314f58", "authorization": { "is_authenticated": false, "scopes": [], diff --git a/docs/source/configuration/in-memory-caching.mdx b/docs/source/configuration/in-memory-caching.mdx index 6b7741a1b8..ef60b5bf59 100644 --- a/docs/source/configuration/in-memory-caching.mdx +++ b/docs/source/configuration/in-memory-caching.mdx @@ -82,6 +82,20 @@ then look at `apollo_router_schema_loading_time` and `apollo_router_query_planni #### Cache warm-up with distributed caching If the Router is using distributed caching for query plans, the warm-up phase will also store the new query plans in Redis. Since all Router instances might have the same distributions of queries in their in-memory cache, the list of queries is shuffled before warm-up, so each Router instance can plan queries in a different order and share their results through the cache. + +#### Schema aware query hashing + +The query plan cache key uses a hashing algorithm specifically designed for GraphQL queries, using the schema. If a schema update does not affect a query (example: a field was added), then the query hash will stay the same. The query plan cache can use that key during warm up to check if a cached entry can be reused instead of planning it again. + +It can be activated through this option: + +```yaml title="router.yaml" +supergraph: + query_planning: + warmed_up_queries: 100 + experimental_reuse_query_plans: true +``` + ## Caching automatic persisted queries (APQ) [Automatic Persisted Queries (**APQ**)](/apollo-server/performance/apq/) enable GraphQL clients to send a server the _hash_ of their query string, _instead of_ sending the query string itself. When query strings are very large, this can significantly reduce network usage.