diff --git a/Cargo.lock b/Cargo.lock index 407cefb8f1..e8685f6cce 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -200,34 +200,35 @@ checksum = "a4668cab20f66d8d020e1fbc0ebe47217433c1b6c8f2040faf858554e394ace6" [[package]] name = "apollo-compiler" -version = "0.10.0" +version = "0.11.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4593ed026f92c0e79bc11a050f060af72a4fadaccdfefd211263d85f9be8048b" +checksum = "bb2436eb22464134efc641e508b33229361b27a3d5b6f03242b66b170ab8786c" dependencies = [ - "apollo-parser 0.5.3", + "apollo-parser 0.6.3", "ariadne", - "indexmap 1.9.3", - "ordered-float 3.9.0", + "indexmap 2.0.2", + "ordered-float 4.1.0", "rowan", "salsa", + "serde", "thiserror", "uuid", ] [[package]] name = "apollo-compiler" -version = "0.11.3" +version = "1.0.0-beta.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bb2436eb22464134efc641e508b33229361b27a3d5b6f03242b66b170ab8786c" +checksum = "a5ff474a85ea7b22c944aba74bab8863366adde503d00864d01da2284c670aa8" dependencies = [ - "apollo-parser 0.6.3", + "apollo-parser 0.7.1", "ariadne", "indexmap 2.0.2", - "ordered-float 4.1.0", "rowan", "salsa", "serde", "thiserror", + "triomphe", "uuid", ] @@ -244,19 +245,20 @@ dependencies = [ [[package]] name = "apollo-parser" -version = "0.5.3" +version = "0.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ec05c087cc9d21576b18337d9dbaadc87cea1b33c4f2b8d59e735ed90e880984" +checksum = "4b3cd0ce63c32c83e035757af12e3ae30566ce5d1ae183c02a2dd089a0df65aa" dependencies = [ + "memchr", "rowan", "thiserror", ] [[package]] name = "apollo-parser" -version = "0.6.3" +version = "0.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4b3cd0ce63c32c83e035757af12e3ae30566ce5d1ae183c02a2dd089a0df65aa" +checksum = "c382f531987366a6954ac687305652b1428b049ca1294b4502b0a9dbbf0d37b8" dependencies = [ "memchr", "rowan", @@ -269,9 +271,7 @@ version = "1.33.0" dependencies = [ "access-json", "anyhow", - "apollo-compiler 0.11.3", - "apollo-encoder", - "apollo-parser 0.6.3", + "apollo-compiler 1.0.0-beta.4", "arc-swap", "askama", "async-compression", @@ -6197,6 +6197,12 @@ dependencies = [ "der", ] +[[package]] +name = "stable_deref_trait" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" + [[package]] name = "static_assertions" version = "1.1.0" @@ -6282,7 +6288,7 @@ name = "supergraph_sdl" version = "0.1.0" dependencies = [ "anyhow", - "apollo-compiler 0.10.0", + "apollo-compiler 1.0.0-beta.4", "apollo-router", "async-trait", "tower", @@ -7018,6 +7024,16 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "triomphe" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0eee8098afad3fb0c54a9007aab6804558410503ad676d4633f9c2559a00ac0f" +dependencies = [ + "serde", + "stable_deref_trait", +] + [[package]] name = "try-lock" version = "0.2.4" diff --git a/apollo-router/Cargo.toml b/apollo-router/Cargo.toml index 720a82d915..b9050e24ba 100644 --- a/apollo-router/Cargo.toml +++ b/apollo-router/Cargo.toml @@ -53,9 +53,7 @@ features = ["docs_rs"] askama = "0.12.1" access-json = "0.1.0" anyhow = "1.0.75" -apollo-compiler = "0.11.3" -apollo-encoder = "0.7.0" -apollo-parser = "0.6.3" +apollo-compiler = "=1.0.0-beta.4" arc-swap = "1.6.0" async-compression = { version = "0.4.4", features = [ "tokio", @@ -136,7 +134,7 @@ once_cell = "1.18.0" opentelemetry = { version = "0.20.0", features = [ "rt-tokio", "metrics", - "testing" + "testing", ] } opentelemetry_api = "0.20.0" opentelemetry-aws = "0.8.0" @@ -206,7 +204,7 @@ tonic = { version = "0.9.2", features = [ "transport", "tls", "tls-roots", - "gzip" + "gzip", ] } tower = { version = "0.4.13", features = ["full"] } tower-http = { version = "0.4.4", features = [ @@ -233,7 +231,9 @@ uuid = { version = "1.4.1", features = ["serde", "v4"] } yaml-rust = "0.4.5" wiremock = "0.5.19" wsl = "0.1.0" -tokio-tungstenite = { version = "0.20.1", features = ["rustls-tls-native-roots"] } +tokio-tungstenite = { version = "0.20.1", features = [ + "rustls-tls-native-roots", +] } tokio-rustls = "0.24.1" http-serde = "1.1.3" hmac = "0.12.1" diff --git a/apollo-router/src/error.rs b/apollo-router/src/error.rs index e4f93f391d..58d4998a48 100644 --- a/apollo-router/src/error.rs +++ b/apollo-router/src/error.rs @@ -1,7 +1,6 @@ //! Router errors. use std::sync::Arc; -use apollo_compiler::ApolloDiagnostic; use displaydoc::Display; use lazy_static::__Deref; use router_bridge::introspect::IntrospectionError; @@ -475,7 +474,7 @@ impl From for QueryPlannerError { impl From for QueryPlannerError { fn from(err: ValidationErrors) -> Self { QueryPlannerError::OperationValidationErrors( - err.errors.iter().map(ApolloDiagnostic::to_json).collect(), + err.errors.iter().map(|e| e.to_json()).collect(), ) } } @@ -554,9 +553,9 @@ pub(crate) enum SchemaError { } /// Collection of schema validation errors. -#[derive(Clone, Debug)] +#[derive(Debug)] pub(crate) struct ParseErrors { - pub(crate) errors: Vec, + pub(crate) errors: apollo_compiler::Diagnostics, } impl std::fmt::Display for ParseErrors { @@ -566,8 +565,7 @@ impl std::fmt::Display for ParseErrors { if i > 0 { f.write_str("\n")?; } - // TODO(@goto-bus-stop): display line/column once that is exposed from apollo-rs - write!(f, "at index {}: {}", error.index(), error.message())?; + write!(f, "{}", error)?; } let remaining = errors.count(); if remaining > 0 { @@ -578,19 +576,19 @@ impl std::fmt::Display for ParseErrors { } /// Collection of schema validation errors. -#[derive(Clone, Debug)] +#[derive(Debug)] pub(crate) struct ValidationErrors { - pub(crate) errors: Vec, + pub(crate) errors: apollo_compiler::Diagnostics, } impl IntoGraphQLErrors for ValidationErrors { fn into_graphql_errors(self) -> Result, Self> { Ok(self .errors - .into_iter() + .iter() .map(|diagnostic| { Error::builder() - .message(diagnostic.data.to_string()) + .message(diagnostic.message().to_string()) .locations( diagnostic .get_line_column() @@ -616,9 +614,15 @@ impl std::fmt::Display for ValidationErrors { f.write_str("\n")?; } if let Some(location) = error.get_line_column() { - write!(f, "[{}:{}] {}", location.line, location.column, error.data)?; + write!( + f, + "[{}:{}] {}", + location.line, + location.column, + error.message() + )?; } else { - write!(f, "{}", error.data)?; + write!(f, "{}", error.message())?; } } Ok(()) diff --git a/apollo-router/src/plugins/authorization/authenticated.rs b/apollo-router/src/plugins/authorization/authenticated.rs index e487eee2fb..32ea7ee895 100644 --- a/apollo-router/src/plugins/authorization/authenticated.rs +++ b/apollo-router/src/plugins/authorization/authenticated.rs @@ -1,84 +1,68 @@ //! Authorization plugin -use apollo_compiler::hir; -use apollo_compiler::hir::FieldDefinition; -use apollo_compiler::hir::TypeDefinition; -use apollo_compiler::ApolloCompiler; -use apollo_compiler::FileId; -use apollo_compiler::HirDatabase; +use std::collections::HashMap; +use std::collections::HashSet; + +use apollo_compiler::ast; +use apollo_compiler::schema; +use apollo_compiler::schema::Name; use tower::BoxError; use crate::json_ext::Path; use crate::json_ext::PathElement; use crate::spec::query::transform; -use crate::spec::query::transform::get_field_type; use crate::spec::query::traverse; pub(crate) const AUTHENTICATED_DIRECTIVE_NAME: &str = "authenticated"; pub(crate) struct AuthenticatedCheckVisitor<'a> { - compiler: &'a ApolloCompiler, - file_id: FileId, + schema: &'a schema::Schema, + fragments: HashMap<&'a ast::Name, &'a ast::FragmentDefinition>, pub(crate) found: bool, } impl<'a> AuthenticatedCheckVisitor<'a> { - pub(crate) fn new(compiler: &'a ApolloCompiler, file_id: FileId) -> Self { + pub(crate) fn new(schema: &'a schema::Schema, executable: &'a ast::Document) -> Self { Self { - compiler, - file_id, + schema, + fragments: transform::collect_fragments(executable), found: false, } } - fn is_field_authenticated(&self, field: &FieldDefinition) -> bool { - field - .directive_by_name(AUTHENTICATED_DIRECTIVE_NAME) - .is_some() - || field - .ty() - .type_def(&self.compiler.db) - .map(|t| self.is_type_authenticated(&t)) - .unwrap_or(false) + fn is_field_authenticated(&self, field: &schema::FieldDefinition) -> bool { + field.directives.has(AUTHENTICATED_DIRECTIVE_NAME) + || self + .schema + .types + .get(field.ty.inner_named_type()) + .is_some_and(|t| self.is_type_authenticated(t)) } - fn is_type_authenticated(&self, t: &TypeDefinition) -> bool { - t.directive_by_name(AUTHENTICATED_DIRECTIVE_NAME).is_some() + fn is_type_authenticated(&self, t: &schema::ExtendedType) -> bool { + t.directives().has(AUTHENTICATED_DIRECTIVE_NAME) } } impl<'a> traverse::Visitor for AuthenticatedCheckVisitor<'a> { - fn compiler(&self) -> &ApolloCompiler { - self.compiler - } - - fn operation(&mut self, node: &hir::OperationDefinition) -> Result<(), BoxError> { - traverse::operation(self, node) - } - - fn field(&mut self, parent_type: &str, node: &hir::Field) -> Result<(), BoxError> { - let field_name = node.name(); - - if self - .compiler - .db - .types_definitions_by_name() - .get(parent_type) - .and_then(|def| def.field(&self.compiler.db, field_name)) - .is_some_and(|field| self.is_field_authenticated(field)) - { + fn field( + &mut self, + _parent_type: &str, + field_def: &ast::FieldDefinition, + node: &ast::Field, + ) -> Result<(), BoxError> { + if self.is_field_authenticated(field_def) { self.found = true; return Ok(()); } - traverse::field(self, parent_type, node) + traverse::field(self, field_def, node) } - fn fragment_definition(&mut self, node: &hir::FragmentDefinition) -> Result<(), BoxError> { + fn fragment_definition(&mut self, node: &ast::FragmentDefinition) -> Result<(), BoxError> { if self - .compiler - .db - .types_definitions_by_name() - .get(node.type_condition()) + .schema + .types + .get(&node.type_condition) .is_some_and(|type_definition| self.is_type_authenticated(type_definition)) { self.found = true; @@ -87,17 +71,16 @@ impl<'a> traverse::Visitor for AuthenticatedCheckVisitor<'a> { traverse::fragment_definition(self, node) } - fn fragment_spread(&mut self, node: &hir::FragmentSpread) -> Result<(), BoxError> { - let fragments = self.compiler.db.fragments(self.file_id); - let condition = fragments - .get(node.name()) - .ok_or("MissingFragmentDefinition")? - .type_condition(); + fn fragment_spread(&mut self, node: &ast::FragmentSpread) -> Result<(), BoxError> { + let condition = &self + .fragments + .get(&node.fragment_name) + .ok_or("MissingFragment")? + .type_condition; if self - .compiler - .db - .types_definitions_by_name() + .schema + .types .get(condition) .is_some_and(|type_definition| self.is_type_authenticated(type_definition)) { @@ -110,13 +93,12 @@ impl<'a> traverse::Visitor for AuthenticatedCheckVisitor<'a> { fn inline_fragment( &mut self, parent_type: &str, - node: &hir::InlineFragment, + node: &ast::InlineFragment, ) -> Result<(), BoxError> { - if let Some(name) = node.type_condition() { + if let Some(name) = &node.type_condition { if self - .compiler - .db - .types_definitions_by_name() + .schema + .types .get(name) .is_some_and(|type_definition| self.is_type_authenticated(type_definition)) { @@ -127,79 +109,91 @@ impl<'a> traverse::Visitor for AuthenticatedCheckVisitor<'a> { traverse::inline_fragment(self, parent_type, node) } + + fn schema(&self) -> &apollo_compiler::Schema { + self.schema + } } pub(crate) struct AuthenticatedVisitor<'a> { - compiler: &'a ApolloCompiler, - file_id: FileId, + schema: &'a schema::Schema, + fragments: HashMap<&'a ast::Name, &'a ast::FragmentDefinition>, + implementers_map: &'a HashMap>, pub(crate) query_requires_authentication: bool, pub(crate) unauthorized_paths: Vec, current_path: Path, } impl<'a> AuthenticatedVisitor<'a> { - pub(crate) fn new(compiler: &'a ApolloCompiler, file_id: FileId) -> Self { + pub(crate) fn new( + schema: &'a schema::Schema, + executable: &'a ast::Document, + implementers_map: &'a HashMap>, + ) -> Self { Self { - compiler, - file_id, + schema, + fragments: transform::collect_fragments(executable), + implementers_map, query_requires_authentication: false, unauthorized_paths: Vec::new(), current_path: Path::default(), } } - fn is_field_authenticated(&self, field: &FieldDefinition) -> bool { - field - .directive_by_name(AUTHENTICATED_DIRECTIVE_NAME) - .is_some() - || field - .ty() - .type_def(&self.compiler.db) - .map(|t| self.is_type_authenticated(&t)) - .unwrap_or(false) + fn is_field_authenticated(&self, field: &schema::FieldDefinition) -> bool { + field.directives.has(AUTHENTICATED_DIRECTIVE_NAME) + || self + .schema + .types + .get(field.ty.inner_named_type()) + .is_some_and(|t| self.is_type_authenticated(t)) } - fn is_type_authenticated(&self, t: &TypeDefinition) -> bool { - t.directive_by_name(AUTHENTICATED_DIRECTIVE_NAME).is_some() + fn is_type_authenticated(&self, t: &schema::ExtendedType) -> bool { + t.directives().has(AUTHENTICATED_DIRECTIVE_NAME) } fn implementors_with_different_requirements( &self, - parent_type: &str, - node: &hir::Field, + field_def: &ast::FieldDefinition, + node: &ast::Field, ) -> bool { // if all selections under the interface field are fragments with type conditions // then we don't need to check that they have the same authorization requirements - if node.selection_set().fields().is_empty() { + if node.selection_set.iter().all(|sel| { + matches!( + sel, + ast::Selection::FragmentSpread(_) | ast::Selection::InlineFragment(_) + ) + }) { return false; } - if let Some(type_definition) = get_field_type(self, parent_type, node.name()) - .and_then(|ty| self.compiler.db.find_type_definition_by_name(ty)) - { - if self.implementors_with_different_type_requirements(&type_definition) { + let type_name = field_def.ty.inner_named_type(); + if let Some(type_definition) = self.schema.types.get(type_name) { + if self.implementors_with_different_type_requirements(type_name, type_definition) { return true; } } false } - fn implementors_with_different_type_requirements(&self, t: &TypeDefinition) -> bool { - if t.is_interface_type_definition() { + fn implementors_with_different_type_requirements( + &self, + type_name: &str, + t: &schema::ExtendedType, + ) -> bool { + if t.is_interface() { let mut is_authenticated: Option = None; for ty in self - .compiler - .db - .subtype_map() - .get(t.name()) + .implementers_map + .get(type_name) .into_iter() .flatten() - .cloned() - .filter_map(|ty| self.compiler.db.find_type_definition_by_name(ty)) + .filter_map(|ty| self.schema.types.get(ty)) { - let ty_is_authenticated = - ty.directive_by_name(AUTHENTICATED_DIRECTIVE_NAME).is_some(); + let ty_is_authenticated = ty.directives().has(AUTHENTICATED_DIRECTIVE_NAME); match is_authenticated { None => is_authenticated = Some(ty_is_authenticated), Some(other_ty_is_authenticated) => { @@ -217,29 +211,15 @@ impl<'a> AuthenticatedVisitor<'a> { fn implementors_with_different_field_requirements( &self, parent_type: &str, - field: &hir::Field, + field: &ast::Field, ) -> bool { - if let Some(t) = self - .compiler - .db - .find_type_definition_by_name(parent_type.to_string()) - { - if t.is_interface_type_definition() { + if let Some(t) = self.schema.types.get(parent_type) { + if t.is_interface() { let mut is_authenticated: Option = None; - for ty in self - .compiler - .db - .subtype_map() - .get(t.name()) - .into_iter() - .flatten() - .cloned() - .filter_map(|ty| self.compiler.db.find_type_definition_by_name(ty)) - { - if let Some(f) = ty.field(&self.compiler.db, field.name()) { - let field_is_authenticated = - f.directive_by_name(AUTHENTICATED_DIRECTIVE_NAME).is_some(); + for ty in self.implementers_map.get(parent_type).into_iter().flatten() { + if let Ok(f) = self.schema.type_field(ty, &field.name) { + let field_is_authenticated = f.directives.has(AUTHENTICATED_DIRECTIVE_NAME); match is_authenticated { Some(other) => { if field_is_authenticated != other { @@ -259,57 +239,45 @@ impl<'a> AuthenticatedVisitor<'a> { } impl<'a> transform::Visitor for AuthenticatedVisitor<'a> { - fn compiler(&self) -> &ApolloCompiler { - self.compiler - } - fn operation( &mut self, - node: &hir::OperationDefinition, - ) -> Result, BoxError> { - let operation_requires_authentication = node - .object_type(&self.compiler.db) - .map(|ty| ty.directive_by_name(AUTHENTICATED_DIRECTIVE_NAME).is_some()) - .unwrap_or(false); + root_type: &str, + node: &ast::OperationDefinition, + ) -> Result, BoxError> { + let operation_requires_authentication = self + .schema + .get_object(root_type) + .is_some_and(|ty| ty.directives.has(AUTHENTICATED_DIRECTIVE_NAME)); if operation_requires_authentication { self.unauthorized_paths.push(self.current_path.clone()); self.query_requires_authentication = true; Ok(None) } else { - transform::operation(self, node) + transform::operation(self, root_type, node) } } fn field( &mut self, parent_type: &str, - node: &hir::Field, - ) -> Result, BoxError> { - let field_name = node.name(); - - let mut is_field_list = false; - - let field_requires_authentication = self - .compiler - .db - .types_definitions_by_name() - .get(parent_type) - .and_then(|def| def.field(&self.compiler.db, field_name)) - .is_some_and(|field| { - if field.ty().is_list() { - is_field_list = true; - } - self.is_field_authenticated(field) - }); + field_def: &ast::FieldDefinition, + node: &ast::Field, + ) -> Result, BoxError> { + let field_name = &node.name; + + let is_field_list = field_def.ty.is_list(); + + let field_requires_authentication = self.is_field_authenticated(field_def); - self.current_path.push(PathElement::Key(field_name.into())); + self.current_path + .push(PathElement::Key(field_name.as_str().into())); if is_field_list { self.current_path.push(PathElement::Flatten); } let implementors_with_different_requirements = - self.implementors_with_different_requirements(parent_type, node); + self.implementors_with_different_requirements(field_def, node); let implementors_with_different_field_requirements = self.implementors_with_different_field_requirements(parent_type, node); @@ -322,7 +290,7 @@ impl<'a> transform::Visitor for AuthenticatedVisitor<'a> { self.query_requires_authentication = true; Ok(None) } else { - transform::field(self, parent_type, node) + transform::field(self, field_def, node) }; if is_field_list { @@ -335,13 +303,12 @@ impl<'a> transform::Visitor for AuthenticatedVisitor<'a> { fn fragment_definition( &mut self, - node: &hir::FragmentDefinition, - ) -> Result, BoxError> { + node: &ast::FragmentDefinition, + ) -> Result, BoxError> { let fragment_requires_authentication = self - .compiler - .db - .types_definitions_by_name() - .get(node.type_condition()) + .schema + .types + .get(&node.type_condition) .is_some_and(|type_definition| self.is_type_authenticated(type_definition)); if fragment_requires_authentication { @@ -353,20 +320,19 @@ impl<'a> transform::Visitor for AuthenticatedVisitor<'a> { fn fragment_spread( &mut self, - node: &hir::FragmentSpread, - ) -> Result, BoxError> { - let fragments = self.compiler.db.fragments(self.file_id); - let condition = fragments - .get(node.name()) - .ok_or("MissingFragmentDefinition")? - .type_condition(); + node: &ast::FragmentSpread, + ) -> Result, BoxError> { + let condition = &self + .fragments + .get(&node.fragment_name) + .ok_or("MissingFragment")? + .type_condition; self.current_path - .push(PathElement::Fragment(condition.into())); + .push(PathElement::Fragment(condition.as_str().into())); let fragment_requires_authentication = self - .compiler - .db - .types_definitions_by_name() + .schema + .types .get(condition) .is_some_and(|type_definition| self.is_type_authenticated(type_definition)); @@ -386,10 +352,9 @@ impl<'a> transform::Visitor for AuthenticatedVisitor<'a> { fn inline_fragment( &mut self, parent_type: &str, - - node: &hir::InlineFragment, - ) -> Result, BoxError> { - match node.type_condition() { + node: &ast::InlineFragment, + ) -> Result, BoxError> { + match &node.type_condition { None => { self.current_path.push(PathElement::Fragment(String::new())); let res = transform::inline_fragment(self, parent_type, node); @@ -397,12 +362,12 @@ impl<'a> transform::Visitor for AuthenticatedVisitor<'a> { res } Some(name) => { - self.current_path.push(PathElement::Fragment(name.into())); + self.current_path + .push(PathElement::Fragment(name.as_str().into())); let fragment_requires_authentication = self - .compiler - .db - .types_definitions_by_name() + .schema + .types .get(name) .is_some_and(|type_definition| self.is_type_authenticated(type_definition)); @@ -420,11 +385,16 @@ impl<'a> transform::Visitor for AuthenticatedVisitor<'a> { } } } + + fn schema(&self) -> &apollo_compiler::Schema { + self.schema + } } #[cfg(test)] mod tests { - use apollo_compiler::ApolloCompiler; + use apollo_compiler::ast; + use apollo_compiler::Schema; use multimap::MultiMap; use serde_json_bytes::json; use tower::ServiceExt; @@ -486,29 +456,24 @@ mod tests { "#; #[track_caller] - fn filter(schema: &str, query: &str) -> (apollo_encoder::Document, Vec) { - let mut compiler = ApolloCompiler::new(); - - let _schema_id = compiler.add_type_system(schema, "schema.graphql"); - let file_id = compiler.add_executable(query, "query.graphql"); - - let diagnostics = compiler.validate(); - for diagnostic in &diagnostics { - println!("{diagnostic}"); - } - assert!(diagnostics.is_empty()); + fn filter(schema: &str, query: &str) -> (ast::Document, Vec) { + let schema = Schema::parse(schema, "schema.graphql"); + let doc = ast::Document::parse(query, "query.graphql"); + schema.validate().unwrap(); + doc.to_executable(&schema).validate(&schema).unwrap(); - let mut visitor = AuthenticatedVisitor::new(&compiler, file_id); + let map = schema.implementers_map(); + let mut visitor = AuthenticatedVisitor::new(&schema, &doc, &map); ( - transform::document(&mut visitor, file_id).unwrap(), + transform::document(&mut visitor, &doc).unwrap(), visitor.unauthorized_paths, ) } struct TestResult<'a> { query: &'a str, - result: apollo_encoder::Document, + result: ast::Document, paths: Vec, } diff --git a/apollo-router/src/plugins/authorization/mod.rs b/apollo-router/src/plugins/authorization/mod.rs index ba5fbcf11e..bf993b0c25 100644 --- a/apollo-router/src/plugins/authorization/mod.rs +++ b/apollo-router/src/plugins/authorization/mod.rs @@ -3,16 +3,13 @@ use std::collections::HashMap; use std::collections::HashSet; use std::ops::ControlFlow; -use std::sync::Arc; -use apollo_compiler::ApolloCompiler; -use apollo_compiler::InputDatabase; +use apollo_compiler::ast; use http::StatusCode; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; use serde_json_bytes::Value; -use tokio::sync::Mutex; use tower::BoxError; use tower::ServiceBuilder; use tower::ServiceExt; @@ -42,7 +39,6 @@ use crate::services::execution; use crate::services::supergraph; use crate::spec::query::transform; use crate::spec::query::traverse; -use crate::spec::query::QUERY_EXECUTABLE; use crate::spec::Query; use crate::spec::Schema; use crate::spec::SpecError; @@ -101,19 +97,16 @@ impl AuthorizationPlugin { .and_then(|(_, v)| v.get("preview_directives").and_then(|v| v.as_object())) .and_then(|v| v.get("enabled").and_then(|v| v.as_bool())); let has_authorization_directives = schema - .type_system .definitions - .directives + .directive_definitions .contains_key(AUTHENTICATED_DIRECTIVE_NAME) || schema - .type_system .definitions - .directives + .directive_definitions .contains_key(REQUIRES_SCOPES_DIRECTIVE_NAME) || schema - .type_system .definitions - .directives + .directive_definitions .contains_key(POLICY_DIRECTIVE_NAME); match has_config { @@ -134,23 +127,24 @@ impl AuthorizationPlugin { configuration: &Configuration, context: &Context, ) { - let (compiler, file_id) = Query::make_compiler(query, schema, configuration); + let doc = Query::parse_document(query, schema, configuration); + let ast = &doc.ast; - let mut visitor = AuthenticatedCheckVisitor::new(&compiler, file_id); + let mut visitor = AuthenticatedCheckVisitor::new(&schema.definitions, ast); // 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, file_id).is_ok() && visitor.found { + if traverse::document(&mut visitor, ast).is_ok() && visitor.found { context.insert(AUTHENTICATED_KEY, true).unwrap(); } - let mut visitor = ScopeExtractionVisitor::new(&compiler, file_id); + let mut visitor = ScopeExtractionVisitor::new(&schema.definitions, ast); // 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, file_id).is_ok() { + if traverse::document(&mut visitor, ast).is_ok() { let scopes: Vec = visitor.extracted_scopes.into_iter().collect(); if !scopes.is_empty() { @@ -160,12 +154,12 @@ impl AuthorizationPlugin { // TODO: @policy is out of scope for preview, this will be reactivated later if false { - let mut visitor = PolicyExtractionVisitor::new(&compiler, file_id); + let mut visitor = PolicyExtractionVisitor::new(&schema.definitions, ast); // 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, file_id).is_ok() { + if traverse::document(&mut visitor, ast).is_ok() { let policies: HashMap> = visitor .extracted_policies .into_iter() @@ -233,12 +227,12 @@ impl AuthorizationPlugin { key: &QueryKey, schema: &Schema, ) -> Result, QueryPlannerError> { - // we create a compiler to filter the query. The filtered query will then be used + // The filtered query will then be used // to generate selections for response formatting, to execute introspection and // generating a query plan - let mut compiler = ApolloCompiler::new(); - compiler.set_type_system_hir(schema.type_system.clone()); - let _id = compiler.add_executable(&key.filtered_query, "query"); + + // TODO: do we need to (re)parse here? + let doc = ast::Document::parse(&key.filtered_query, "filtered_query"); let is_authenticated = key.metadata.is_authenticated; let scopes = &key.metadata.scopes; @@ -247,97 +241,76 @@ impl AuthorizationPlugin { let mut is_filtered = false; let mut unauthorized_paths: Vec = vec![]; - let filter_res = Self::authenticated_filter_query(&compiler, is_authenticated)?; + let filter_res = Self::authenticated_filter_query(schema, &doc, is_authenticated)?; - let compiler = match filter_res { - None => compiler, - Some((query, paths)) => { + let doc = match filter_res { + None => doc, + Some((filtered_doc, paths)) => { unauthorized_paths.extend(paths); - if query.is_empty() { + // FIXME: consider only `filtered_doc.get_operation(key.operation_name)`? + if filtered_doc.definitions.is_empty() { return Err(QueryPlannerError::Unauthorized(unauthorized_paths)); } is_filtered = true; - let mut compiler = ApolloCompiler::new(); - compiler.set_type_system_hir(schema.type_system.clone()); - let _id = compiler.add_executable(&query, "query"); - compiler + filtered_doc } }; - let filter_res = Self::scopes_filter_query(&compiler, scopes)?; + let filter_res = Self::scopes_filter_query(schema, &doc, scopes)?; - let compiler = match filter_res { - None => compiler, - Some((query, paths)) => { + let doc = match filter_res { + None => doc, + Some((filtered_doc, paths)) => { unauthorized_paths.extend(paths); - if query.is_empty() { + // FIXME: consider only `filtered_doc.get_operation(key.operation_name)`? + if filtered_doc.definitions.is_empty() { return Err(QueryPlannerError::Unauthorized(unauthorized_paths)); } is_filtered = true; - let mut compiler = ApolloCompiler::new(); - compiler.set_type_system_hir(schema.type_system.clone()); - let _id = compiler.add_executable(&query, "query"); - compiler + filtered_doc } }; - let filter_res = Self::policies_filter_query(&compiler, policies)?; + let filter_res = Self::policies_filter_query(schema, &doc, policies)?; - let compiler = match filter_res { - None => compiler, - Some((query, paths)) => { + let doc = match filter_res { + None => doc, + Some((filtered_doc, paths)) => { unauthorized_paths.extend(paths); - if query.is_empty() { + // FIXME: consider only `filtered_doc.get_operation(key.operation_name)`? + if filtered_doc.definitions.is_empty() { return Err(QueryPlannerError::Unauthorized(unauthorized_paths)); } is_filtered = true; - let mut compiler = ApolloCompiler::new(); - compiler.set_type_system_hir(schema.type_system.clone()); - let _id = compiler.add_executable(&query, "query"); - compiler + filtered_doc } }; if is_filtered { - let file_id = compiler - .db - .source_file(QUERY_EXECUTABLE.into()) - .ok_or_else(|| QueryPlannerError::SpecError(SpecError::UnknownFileId))?; - let filtered_query = compiler.db.source_code(file_id).to_string(); - - Ok(Some(( - filtered_query, - unauthorized_paths, - Arc::new(Mutex::new(compiler)), - ))) + Ok(Some((unauthorized_paths, doc))) } else { Ok(None) } } fn authenticated_filter_query( - compiler: &ApolloCompiler, + schema: &Schema, + doc: &ast::Document, is_authenticated: bool, - ) -> Result)>, QueryPlannerError> { - let id = compiler - .db - .executable_definition_files() - .pop() - .expect("the query was added to the compiler earlier"); - - let mut visitor = AuthenticatedVisitor::new(compiler, id); - let modified_query = transform::document(&mut visitor, id) - .map_err(|e| SpecError::ParsingError(e.to_string()))? - .to_string(); + ) -> Result)>, QueryPlannerError> { + let mut visitor = + AuthenticatedVisitor::new(&schema.definitions, doc, &schema.implementers_map); + let modified_query = transform::document(&mut visitor, doc) + .map_err(|e| SpecError::ParsingError(e.to_string()))?; if visitor.query_requires_authentication { if is_authenticated { @@ -359,21 +332,19 @@ impl AuthorizationPlugin { } fn scopes_filter_query( - compiler: &ApolloCompiler, + schema: &Schema, + doc: &ast::Document, scopes: &[String], - ) -> Result)>, QueryPlannerError> { - let id = compiler - .db - .executable_definition_files() - .pop() - .expect("the query was added to the compiler earlier"); - - let mut visitor = - ScopeFilteringVisitor::new(compiler, id, scopes.iter().cloned().collect()); + ) -> Result)>, QueryPlannerError> { + let mut visitor = ScopeFilteringVisitor::new( + &schema.definitions, + doc, + &schema.implementers_map, + scopes.iter().cloned().collect(), + ); - let modified_query = transform::document(&mut visitor, id) - .map_err(|e| SpecError::ParsingError(e.to_string()))? - .to_string(); + let modified_query = transform::document(&mut visitor, doc) + .map_err(|e| SpecError::ParsingError(e.to_string()))?; if visitor.query_requires_scopes { tracing::debug!("the query required scopes, the requests present scopes: {scopes:?}, modified query:\n{modified_query}\nunauthorized paths: {:?}", @@ -391,21 +362,19 @@ impl AuthorizationPlugin { } fn policies_filter_query( - compiler: &ApolloCompiler, + schema: &Schema, + doc: &ast::Document, policies: &[String], - ) -> Result)>, QueryPlannerError> { - let id = compiler - .db - .executable_definition_files() - .pop() - .expect("the query was added to the compiler earlier"); - - let mut visitor = - PolicyFilteringVisitor::new(compiler, id, policies.iter().cloned().collect()); - - let modified_query = transform::document(&mut visitor, id) - .map_err(|e| SpecError::ParsingError(e.to_string()))? - .to_string(); + ) -> Result)>, QueryPlannerError> { + let mut visitor = PolicyFilteringVisitor::new( + &schema.definitions, + doc, + &schema.implementers_map, + policies.iter().cloned().collect(), + ); + + let modified_query = transform::document(&mut visitor, doc) + .map_err(|e| SpecError::ParsingError(e.to_string()))?; if visitor.query_requires_policies { tracing::debug!("the query required policies, the requests present policies: {policies:?}, modified query:\n{modified_query}\nunauthorized paths: {:?}", diff --git a/apollo-router/src/plugins/authorization/policy.rs b/apollo-router/src/plugins/authorization/policy.rs index 807bee0bd5..c7dff8f834 100644 --- a/apollo-router/src/plugins/authorization/policy.rs +++ b/apollo-router/src/plugins/authorization/policy.rs @@ -5,26 +5,22 @@ //! ```graphql //! directive @policy(policies: [String!]!) on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM //! ``` +use std::collections::HashMap; use std::collections::HashSet; -use apollo_compiler::hir; -use apollo_compiler::hir::FieldDefinition; -use apollo_compiler::hir::TypeDefinition; -use apollo_compiler::hir::Value; -use apollo_compiler::ApolloCompiler; -use apollo_compiler::FileId; -use apollo_compiler::HirDatabase; +use apollo_compiler::ast; +use apollo_compiler::schema; +use apollo_compiler::schema::Name; use tower::BoxError; use crate::json_ext::Path; use crate::json_ext::PathElement; use crate::spec::query::transform; -use crate::spec::query::transform::get_field_type; use crate::spec::query::traverse; pub(crate) struct PolicyExtractionVisitor<'a> { - compiler: &'a ApolloCompiler, - file_id: FileId, + schema: &'a schema::Schema, + fragments: HashMap<&'a ast::Name, &'a ast::FragmentDefinition>, pub(crate) extracted_policies: HashSet, } @@ -32,98 +28,80 @@ pub(crate) const POLICY_DIRECTIVE_NAME: &str = "policy"; impl<'a> PolicyExtractionVisitor<'a> { #[allow(dead_code)] - pub(crate) fn new(compiler: &'a ApolloCompiler, file_id: FileId) -> Self { + pub(crate) fn new(schema: &'a schema::Schema, executable: &'a ast::Document) -> Self { Self { - compiler, - file_id, + schema, + fragments: transform::collect_fragments(executable), extracted_policies: HashSet::new(), } } - fn get_policies_from_field(&mut self, field: &FieldDefinition) { + fn get_policies_from_field(&mut self, field: &schema::FieldDefinition) { self.extracted_policies - .extend(policy_argument(field.directive_by_name(POLICY_DIRECTIVE_NAME)).cloned()); + .extend(policy_argument(field.directives.get(POLICY_DIRECTIVE_NAME))); - if let Some(ty) = field.ty().type_def(&self.compiler.db) { - self.get_policies_from_type(&ty) + if let Some(ty) = self.schema.types.get(field.ty.inner_named_type()) { + self.get_policies_from_type(ty) } } - fn get_policies_from_type(&mut self, ty: &TypeDefinition) { + fn get_policies_from_type(&mut self, ty: &schema::ExtendedType) { self.extracted_policies - .extend(policy_argument(ty.directive_by_name(POLICY_DIRECTIVE_NAME)).cloned()); + .extend(policy_argument(ty.directives().get(POLICY_DIRECTIVE_NAME))); } } -fn policy_argument(opt_directive: Option<&hir::Directive>) -> impl Iterator { +fn policy_argument( + opt_directive: Option<&impl AsRef>, +) -> impl Iterator + '_ { opt_directive - .and_then(|directive| directive.argument_by_name("policies")) - .and_then(|value| match value { - Value::List { value, .. } => Some(value), - _ => None, - }) + .and_then(|directive| directive.as_ref().argument_by_name("policies")) + .and_then(|value| value.as_list()) .into_iter() .flatten() - .filter_map(|v| match v { - Value::String { value, .. } => Some(value), - _ => None, - }) + .filter_map(|v| v.as_str().map(str::to_owned)) } impl<'a> traverse::Visitor for PolicyExtractionVisitor<'a> { - fn compiler(&self) -> &ApolloCompiler { - self.compiler - } - - fn operation(&mut self, node: &hir::OperationDefinition) -> Result<(), BoxError> { - if let Some(ty) = node.object_type(&self.compiler.db) { + fn operation( + &mut self, + root_type: &str, + node: &ast::OperationDefinition, + ) -> Result<(), BoxError> { + if let Some(ty) = self.schema.types.get(root_type) { self.extracted_policies - .extend(policy_argument(ty.directive_by_name(POLICY_DIRECTIVE_NAME)).cloned()); + .extend(policy_argument(ty.directives().get(POLICY_DIRECTIVE_NAME))); } - traverse::operation(self, node) + traverse::operation(self, root_type, node) } - fn field(&mut self, parent_type: &str, node: &hir::Field) -> Result<(), BoxError> { - if let Some(ty) = self - .compiler - .db - .types_definitions_by_name() - .get(parent_type) - { - if let Some(field) = ty.field(&self.compiler.db, node.name()) { - self.get_policies_from_field(field); - } - } + fn field( + &mut self, + _parent_type: &str, + field_def: &ast::FieldDefinition, + node: &ast::Field, + ) -> Result<(), BoxError> { + self.get_policies_from_field(field_def); - traverse::field(self, parent_type, node) + traverse::field(self, field_def, node) } - fn fragment_definition(&mut self, node: &hir::FragmentDefinition) -> Result<(), BoxError> { - if let Some(ty) = self - .compiler - .db - .types_definitions_by_name() - .get(node.type_condition()) - { + fn fragment_definition(&mut self, node: &ast::FragmentDefinition) -> Result<(), BoxError> { + if let Some(ty) = self.schema.types.get(&node.type_condition) { self.get_policies_from_type(ty); } traverse::fragment_definition(self, node) } - fn fragment_spread(&mut self, node: &hir::FragmentSpread) -> Result<(), BoxError> { - let fragments = self.compiler.db.fragments(self.file_id); - let type_condition = fragments - .get(node.name()) - .ok_or("MissingFragmentDefinition")? - .type_condition(); - - if let Some(ty) = self - .compiler - .db - .types_definitions_by_name() - .get(type_condition) - { + fn fragment_spread(&mut self, node: &ast::FragmentSpread) -> Result<(), BoxError> { + let type_condition = &self + .fragments + .get(&node.fragment_name) + .ok_or("MissingFragment")? + .type_condition; + + if let Some(ty) = self.schema.types.get(type_condition) { self.get_policies_from_type(ty); } traverse::fragment_spread(self, node) @@ -132,26 +110,25 @@ impl<'a> traverse::Visitor for PolicyExtractionVisitor<'a> { fn inline_fragment( &mut self, parent_type: &str, - - node: &hir::InlineFragment, + node: &ast::InlineFragment, ) -> Result<(), BoxError> { - if let Some(type_condition) = node.type_condition() { - if let Some(ty) = self - .compiler - .db - .types_definitions_by_name() - .get(type_condition) - { + if let Some(type_condition) = &node.type_condition { + if let Some(ty) = self.schema.types.get(type_condition) { self.get_policies_from_type(ty); } } traverse::inline_fragment(self, parent_type, node) } + + fn schema(&self) -> &apollo_compiler::Schema { + self.schema + } } pub(crate) struct PolicyFilteringVisitor<'a> { - compiler: &'a ApolloCompiler, - file_id: FileId, + schema: &'a schema::Schema, + fragments: HashMap<&'a ast::Name, &'a ast::FragmentDefinition>, + implementers_map: &'a HashMap>, request_policies: HashSet, pub(crate) query_requires_policies: bool, pub(crate) unauthorized_paths: Vec, @@ -160,13 +137,15 @@ pub(crate) struct PolicyFilteringVisitor<'a> { impl<'a> PolicyFilteringVisitor<'a> { pub(crate) fn new( - compiler: &'a ApolloCompiler, - file_id: FileId, + schema: &'a schema::Schema, + executable: &'a ast::Document, + implementers_map: &'a HashMap>, successful_policies: HashSet, ) -> Self { Self { - compiler, - file_id, + schema, + fragments: transform::collect_fragments(executable), + implementers_map, request_policies: successful_policies, query_requires_policies: false, unauthorized_paths: vec![], @@ -174,10 +153,9 @@ impl<'a> PolicyFilteringVisitor<'a> { } } - fn is_field_authorized(&mut self, field: &FieldDefinition) -> bool { - let field_policies = policy_argument(field.directive_by_name(POLICY_DIRECTIVE_NAME)) - .cloned() - .collect::>(); + fn is_field_authorized(&mut self, field: &schema::FieldDefinition) -> bool { + let field_policies = + policy_argument(field.directives.get(POLICY_DIRECTIVE_NAME)).collect::>(); // The field is authorized if any of the policies succeeds if !field_policies.is_empty() @@ -190,17 +168,16 @@ impl<'a> PolicyFilteringVisitor<'a> { return false; } - if let Some(ty) = field.ty().type_def(&self.compiler.db) { - self.is_type_authorized(&ty) + if let Some(ty) = self.schema.types.get(field.ty.inner_named_type()) { + self.is_type_authorized(ty) } else { false } } - fn is_type_authorized(&self, ty: &TypeDefinition) -> bool { - let type_policies = policy_argument(ty.directive_by_name(POLICY_DIRECTIVE_NAME)) - .cloned() - .collect::>(); + fn is_type_authorized(&self, ty: &schema::ExtendedType) -> bool { + let type_policies = + policy_argument(ty.directives().get(POLICY_DIRECTIVE_NAME)).collect::>(); // The field is authorized if any of the policies succeeds type_policies.is_empty() || self @@ -212,48 +189,52 @@ impl<'a> PolicyFilteringVisitor<'a> { fn implementors_with_different_requirements( &self, - parent_type: &str, - node: &hir::Field, + field_def: &ast::FieldDefinition, + node: &ast::Field, ) -> bool { // if all selections under the interface field are fragments with type conditions // then we don't need to check that they have the same authorization requirements - if node.selection_set().fields().is_empty() { + if node.selection_set.iter().all(|sel| { + matches!( + sel, + ast::Selection::FragmentSpread(_) | ast::Selection::InlineFragment(_) + ) + }) { return false; } - if let Some(type_definition) = get_field_type(self, parent_type, node.name()) - .and_then(|ty| self.compiler.db.find_type_definition_by_name(ty)) - { - if self.implementors_with_different_type_requirements(&type_definition) { + let type_name = field_def.ty.inner_named_type(); + if let Some(type_definition) = self.schema.types.get(type_name) { + if self.implementors_with_different_type_requirements(type_name, type_definition) { return true; } } false } - fn implementors_with_different_type_requirements(&self, t: &TypeDefinition) -> bool { - if t.is_interface_type_definition() { + fn implementors_with_different_type_requirements( + &self, + type_name: &str, + t: &schema::ExtendedType, + ) -> bool { + if t.is_interface() { let mut policies: Option> = None; for ty in self - .compiler - .db - .subtype_map() - .get(t.name()) + .implementers_map + .get(type_name) .into_iter() .flatten() - .cloned() - .filter_map(|ty| self.compiler.db.find_type_definition_by_name(ty)) + .filter_map(|ty| self.schema.types.get(ty)) { // aggregate the list of scope sets // we transform to a common representation of sorted vectors because the element order // of hashsets is not stable let field_policies = ty - .directive_by_name(POLICY_DIRECTIVE_NAME) + .directives() + .get(POLICY_DIRECTIVE_NAME) .map(|directive| { - let mut v = policy_argument(Some(directive)) - .cloned() - .collect::>(); + let mut v = policy_argument(Some(directive)).collect::>(); v.sort(); v }) @@ -276,36 +257,22 @@ impl<'a> PolicyFilteringVisitor<'a> { fn implementors_with_different_field_requirements( &self, parent_type: &str, - field: &hir::Field, + field: &ast::Field, ) -> bool { - if let Some(t) = self - .compiler - .db - .find_type_definition_by_name(parent_type.to_string()) - { - if t.is_interface_type_definition() { + if let Some(t) = self.schema.types.get(parent_type) { + if t.is_interface() { let mut policies: Option> = None; - for ty in self - .compiler - .db - .subtype_map() - .get(t.name()) - .into_iter() - .flatten() - .cloned() - .filter_map(|ty| self.compiler.db.find_type_definition_by_name(ty)) - { - if let Some(f) = ty.field(&self.compiler.db, field.name()) { + for ty in self.implementers_map.get(parent_type).into_iter().flatten() { + if let Ok(f) = self.schema.type_field(ty, &field.name) { // aggregate the list of scope sets // we transform to a common representation of sorted vectors because the element order // of hashsets is not stable let field_policies = f - .directive_by_name(POLICY_DIRECTIVE_NAME) + .directives + .get(POLICY_DIRECTIVE_NAME) .map(|directive| { - let mut v = policy_argument(Some(directive)) - .cloned() - .collect::>(); + let mut v = policy_argument(Some(directive)).collect::>(); v.sort(); v }) @@ -328,21 +295,16 @@ impl<'a> PolicyFilteringVisitor<'a> { } impl<'a> transform::Visitor for PolicyFilteringVisitor<'a> { - fn compiler(&self) -> &ApolloCompiler { - self.compiler - } - fn operation( &mut self, - node: &hir::OperationDefinition, - ) -> Result, BoxError> { - let is_authorized = if let Some(ty) = node.object_type(&self.compiler.db) { - match ty.directive_by_name(POLICY_DIRECTIVE_NAME) { + root_type: &str, + node: &ast::OperationDefinition, + ) -> Result, BoxError> { + let is_authorized = if let Some(ty) = self.schema.get_object(root_type) { + match ty.directives.get(POLICY_DIRECTIVE_NAME) { None => true, Some(directive) => { - let type_policies = policy_argument(Some(directive)) - .cloned() - .collect::>(); + let type_policies = policy_argument(Some(directive)).collect::>(); // The field is authorized if any of the policies succeeds type_policies.is_empty() || self @@ -357,7 +319,7 @@ impl<'a> transform::Visitor for PolicyFilteringVisitor<'a> { }; if is_authorized { - transform::operation(self, node) + transform::operation(self, root_type, node) } else { self.unauthorized_paths.push(self.current_path.clone()); self.query_requires_policies = true; @@ -368,34 +330,22 @@ impl<'a> transform::Visitor for PolicyFilteringVisitor<'a> { fn field( &mut self, parent_type: &str, - node: &hir::Field, - ) -> Result, BoxError> { - let field_name = node.name(); - let mut is_field_list = false; - - let is_authorized = self - .compiler - .db - .types_definitions_by_name() - .get(parent_type) - .is_some_and(|def| { - if let Some(field) = def.field(&self.compiler.db, field_name) { - if field.ty().is_list() { - is_field_list = true; - } - self.is_field_authorized(field) - } else { - false - } - }); + field_def: &ast::FieldDefinition, + node: &ast::Field, + ) -> Result, BoxError> { + let field_name = &node.name; + let is_field_list = field_def.ty.is_list(); + + let is_authorized = self.is_field_authorized(field_def); let implementors_with_different_requirements = - self.implementors_with_different_requirements(parent_type, node); + self.implementors_with_different_requirements(field_def, node); let implementors_with_different_field_requirements = self.implementors_with_different_field_requirements(parent_type, node); - self.current_path.push(PathElement::Key(field_name.into())); + self.current_path + .push(PathElement::Key(field_name.as_str().into())); if is_field_list { self.current_path.push(PathElement::Flatten); } @@ -404,7 +354,7 @@ impl<'a> transform::Visitor for PolicyFilteringVisitor<'a> { && !implementors_with_different_requirements && !implementors_with_different_field_requirements { - transform::field(self, parent_type, node) + transform::field(self, field_def, node) } else { self.unauthorized_paths.push(self.current_path.clone()); self.query_requires_policies = true; @@ -421,13 +371,12 @@ impl<'a> transform::Visitor for PolicyFilteringVisitor<'a> { fn fragment_definition( &mut self, - node: &hir::FragmentDefinition, - ) -> Result, BoxError> { + node: &ast::FragmentDefinition, + ) -> Result, BoxError> { let fragment_is_authorized = self - .compiler - .db - .types_definitions_by_name() - .get(node.type_condition()) + .schema + .types + .get(&node.type_condition) .is_some_and(|ty| self.is_type_authorized(ty)); if !fragment_is_authorized { @@ -439,20 +388,19 @@ impl<'a> transform::Visitor for PolicyFilteringVisitor<'a> { fn fragment_spread( &mut self, - node: &hir::FragmentSpread, - ) -> Result, BoxError> { - let fragments = self.compiler.db.fragments(self.file_id); - let condition = fragments - .get(node.name()) - .ok_or("MissingFragmentDefinition")? - .type_condition(); + node: &ast::FragmentSpread, + ) -> Result, BoxError> { + let condition = &self + .fragments + .get(&node.fragment_name) + .ok_or("MissingFragment")? + .type_condition; self.current_path - .push(PathElement::Fragment(condition.into())); + .push(PathElement::Fragment(condition.as_str().into())); let fragment_is_authorized = self - .compiler - .db - .types_definitions_by_name() + .schema + .types .get(condition) .is_some_and(|ty| self.is_type_authorized(ty)); @@ -472,10 +420,9 @@ impl<'a> transform::Visitor for PolicyFilteringVisitor<'a> { fn inline_fragment( &mut self, parent_type: &str, - - node: &hir::InlineFragment, - ) -> Result, BoxError> { - match node.type_condition() { + node: &ast::InlineFragment, + ) -> Result, BoxError> { + match &node.type_condition { None => { self.current_path.push(PathElement::Fragment(String::new())); let res = transform::inline_fragment(self, parent_type, node); @@ -483,12 +430,12 @@ impl<'a> transform::Visitor for PolicyFilteringVisitor<'a> { res } Some(name) => { - self.current_path.push(PathElement::Fragment(name.into())); + self.current_path + .push(PathElement::Fragment(name.as_str().into())); let fragment_is_authorized = self - .compiler - .db - .types_definitions_by_name() + .schema + .types .get(name) .is_some_and(|ty| self.is_type_authorized(ty)); @@ -506,6 +453,10 @@ impl<'a> transform::Visitor for PolicyFilteringVisitor<'a> { } } } + + fn schema(&self) -> &apollo_compiler::Schema { + self.schema + } } #[cfg(test)] @@ -513,8 +464,8 @@ mod tests { use std::collections::BTreeSet; use std::collections::HashSet; - use apollo_compiler::ApolloCompiler; - use apollo_encoder::Document; + use apollo_compiler::ast; + use apollo_compiler::Schema; use crate::json_ext::Path; use crate::plugins::authorization::policy::PolicyExtractionVisitor; @@ -563,19 +514,12 @@ mod tests { "#; fn extract(query: &str) -> BTreeSet { - let mut compiler = ApolloCompiler::new(); - - let _schema_id = compiler.add_type_system(BASIC_SCHEMA, "schema.graphql"); - let id = compiler.add_executable(query, "query.graphql"); - - let diagnostics = compiler.validate(); - for diagnostic in &diagnostics { - println!("{diagnostic}"); - } - assert!(diagnostics.is_empty()); - - let mut visitor = PolicyExtractionVisitor::new(&compiler, id); - traverse::document(&mut visitor, id).unwrap(); + let schema = Schema::parse(BASIC_SCHEMA, "schema.graphql"); + let doc = ast::Document::parse(query, "query.graphql"); + schema.validate().unwrap(); + doc.to_executable(&schema).validate(&schema).unwrap(); + let mut visitor = PolicyExtractionVisitor::new(&schema, &doc); + traverse::document(&mut visitor, &doc).unwrap(); visitor.extracted_policies.into_iter().collect() } @@ -600,21 +544,15 @@ mod tests { } #[track_caller] - fn filter(schema: &str, query: &str, policies: HashSet) -> (Document, Vec) { - let mut compiler = ApolloCompiler::new(); - - let _schema_id = compiler.add_type_system(schema, "schema.graphql"); - let file_id = compiler.add_executable(query, "query.graphql"); - - let diagnostics = compiler.validate(); - for diagnostic in &diagnostics { - println!("{diagnostic}"); - } - assert!(diagnostics.is_empty()); - - let mut visitor = PolicyFilteringVisitor::new(&compiler, file_id, policies); + fn filter(schema: &str, query: &str, policies: HashSet) -> (ast::Document, Vec) { + let schema = Schema::parse(schema, "schema.graphql"); + let doc = ast::Document::parse(query, "query.graphql"); + schema.validate().unwrap(); + doc.to_executable(&schema).validate(&schema).unwrap(); + let map = schema.implementers_map(); + let mut visitor = PolicyFilteringVisitor::new(&schema, &doc, &map, policies); ( - transform::document(&mut visitor, file_id).unwrap(), + transform::document(&mut visitor, &doc).unwrap(), visitor.unauthorized_paths, ) } diff --git a/apollo-router/src/plugins/authorization/scopes.rs b/apollo-router/src/plugins/authorization/scopes.rs index 9664876828..a27ec08083 100644 --- a/apollo-router/src/plugins/authorization/scopes.rs +++ b/apollo-router/src/plugins/authorization/scopes.rs @@ -5,26 +5,22 @@ //! ```graphql //! directive @requiresScopes(scopes: [[String!]!]!) on OBJECT | FIELD_DEFINITION | INTERFACE | SCALAR | ENUM //! ``` +use std::collections::HashMap; use std::collections::HashSet; -use apollo_compiler::hir; -use apollo_compiler::hir::FieldDefinition; -use apollo_compiler::hir::TypeDefinition; -use apollo_compiler::hir::Value; -use apollo_compiler::ApolloCompiler; -use apollo_compiler::FileId; -use apollo_compiler::HirDatabase; +use apollo_compiler::ast; +use apollo_compiler::schema; +use apollo_compiler::schema::Name; use tower::BoxError; use crate::json_ext::Path; use crate::json_ext::PathElement; use crate::spec::query::transform; -use crate::spec::query::transform::get_field_type; use crate::spec::query::traverse; pub(crate) struct ScopeExtractionVisitor<'a> { - compiler: &'a ApolloCompiler, - file_id: FileId, + schema: &'a schema::Schema, + fragments: HashMap<&'a ast::Name, &'a ast::FragmentDefinition>, pub(crate) extracted_scopes: HashSet, } @@ -32,107 +28,87 @@ pub(crate) const REQUIRES_SCOPES_DIRECTIVE_NAME: &str = "requiresScopes"; impl<'a> ScopeExtractionVisitor<'a> { #[allow(dead_code)] - pub(crate) fn new(compiler: &'a ApolloCompiler, file_id: FileId) -> Self { + pub(crate) fn new(schema: &'a schema::Schema, executable: &'a ast::Document) -> Self { Self { - compiler, - file_id, + schema, + fragments: transform::collect_fragments(executable), extracted_scopes: HashSet::new(), } } - fn scopes_from_field(&mut self, field: &FieldDefinition) { - self.extracted_scopes.extend( - scopes_argument(field.directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME)).cloned(), - ); + fn scopes_from_field(&mut self, field: &schema::FieldDefinition) { + self.extracted_scopes.extend(scopes_argument( + field.directives.get(REQUIRES_SCOPES_DIRECTIVE_NAME), + )); - if let Some(ty) = field.ty().type_def(&self.compiler.db) { - self.scopes_from_type(&ty) + if let Some(ty) = self.schema.types.get(field.ty.inner_named_type()) { + self.scopes_from_type(ty) } } - fn scopes_from_type(&mut self, ty: &TypeDefinition) { - self.extracted_scopes - .extend(scopes_argument(ty.directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME)).cloned()); + fn scopes_from_type(&mut self, ty: &schema::ExtendedType) { + self.extracted_scopes.extend(scopes_argument( + ty.directives().get(REQUIRES_SCOPES_DIRECTIVE_NAME), + )); } } -fn scopes_argument(opt_directive: Option<&hir::Directive>) -> impl Iterator { +fn scopes_argument( + opt_directive: Option<&impl AsRef>, +) -> impl Iterator + '_ { opt_directive - .and_then(|directive| directive.argument_by_name("scopes")) + .and_then(|directive| directive.as_ref().argument_by_name("scopes")) // outer array - .and_then(|value| match value { - Value::List { value, .. } => Some(value), - _ => None, - }) + .and_then(|value| value.as_list()) .into_iter() .flatten() // inner array - .filter_map(|value| match value { - Value::List { value, .. } => Some(value), - _ => None, - }) + .filter_map(|value| value.as_list()) .flatten() - .filter_map(|v| match v { - Value::String { value, .. } => Some(value), - _ => None, - }) + .filter_map(|value| value.as_str().map(str::to_owned)) } impl<'a> traverse::Visitor for ScopeExtractionVisitor<'a> { - fn compiler(&self) -> &ApolloCompiler { - self.compiler - } - - fn operation(&mut self, node: &hir::OperationDefinition) -> Result<(), BoxError> { - if let Some(ty) = node.object_type(&self.compiler.db) { - self.extracted_scopes.extend( - scopes_argument(ty.directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME)).cloned(), - ); + fn operation( + &mut self, + root_type: &str, + node: &ast::OperationDefinition, + ) -> Result<(), BoxError> { + if let Some(ty) = self.schema.types.get(root_type) { + self.extracted_scopes.extend(scopes_argument( + ty.directives().get(REQUIRES_SCOPES_DIRECTIVE_NAME), + )); } - traverse::operation(self, node) + traverse::operation(self, root_type, node) } - fn field(&mut self, parent_type: &str, node: &hir::Field) -> Result<(), BoxError> { - if let Some(ty) = self - .compiler - .db - .types_definitions_by_name() - .get(parent_type) - { - if let Some(field) = ty.field(&self.compiler.db, node.name()) { - self.scopes_from_field(field); - } - } + fn field( + &mut self, + _parent_type: &str, + field_def: &ast::FieldDefinition, + node: &ast::Field, + ) -> Result<(), BoxError> { + self.scopes_from_field(field_def); - traverse::field(self, parent_type, node) + traverse::field(self, field_def, node) } - fn fragment_definition(&mut self, node: &hir::FragmentDefinition) -> Result<(), BoxError> { - if let Some(ty) = self - .compiler - .db - .types_definitions_by_name() - .get(node.type_condition()) - { + fn fragment_definition(&mut self, node: &ast::FragmentDefinition) -> Result<(), BoxError> { + if let Some(ty) = self.schema.types.get(&node.type_condition) { self.scopes_from_type(ty); } traverse::fragment_definition(self, node) } - fn fragment_spread(&mut self, node: &hir::FragmentSpread) -> Result<(), BoxError> { - let fragments = self.compiler.db.fragments(self.file_id); - let type_condition = fragments - .get(node.name()) - .ok_or("MissingFragmentDefinition")? - .type_condition(); - - if let Some(ty) = self - .compiler - .db - .types_definitions_by_name() - .get(type_condition) - { + fn fragment_spread(&mut self, node: &ast::FragmentSpread) -> Result<(), BoxError> { + let type_condition = &self + .fragments + .get(&node.fragment_name) + .ok_or("MissingFragment")? + .type_condition; + + if let Some(ty) = self.schema.types.get(type_condition) { self.scopes_from_type(ty); } traverse::fragment_spread(self, node) @@ -141,52 +117,42 @@ impl<'a> traverse::Visitor for ScopeExtractionVisitor<'a> { fn inline_fragment( &mut self, parent_type: &str, - - node: &hir::InlineFragment, + node: &ast::InlineFragment, ) -> Result<(), BoxError> { - if let Some(type_condition) = node.type_condition() { - if let Some(ty) = self - .compiler - .db - .types_definitions_by_name() - .get(type_condition) - { + if let Some(type_condition) = &node.type_condition { + if let Some(ty) = self.schema.types.get(type_condition) { self.scopes_from_type(ty); } } traverse::inline_fragment(self, parent_type, node) } + + fn schema(&self) -> &apollo_compiler::Schema { + self.schema + } } -fn scopes_sets_argument(directive: &hir::Directive) -> impl Iterator> + '_ { +fn scopes_sets_argument(directive: &ast::Directive) -> impl Iterator> + '_ { directive .argument_by_name("scopes") // outer array - .and_then(|value| match value { - Value::List { value, .. } => Some(value), - _ => None, - }) + .and_then(|value| value.as_list()) .into_iter() .flatten() // inner array - .filter_map(|value| match value { - Value::List { value, .. } => Some( - value - .iter() - .filter_map(|v| match v { - Value::String { value, .. } => Some(value), - _ => None, - }) - .cloned() - .collect(), - ), - _ => None, + .filter_map(|value| { + value.as_list().map(|list| { + list.iter() + .filter_map(|value| value.as_str().map(str::to_owned)) + .collect() + }) }) } pub(crate) struct ScopeFilteringVisitor<'a> { - compiler: &'a ApolloCompiler, - file_id: FileId, + schema: &'a schema::Schema, + fragments: HashMap<&'a ast::Name, &'a ast::FragmentDefinition>, + implementers_map: &'a HashMap>, request_scopes: HashSet, pub(crate) query_requires_scopes: bool, pub(crate) unauthorized_paths: Vec, @@ -195,13 +161,15 @@ pub(crate) struct ScopeFilteringVisitor<'a> { impl<'a> ScopeFilteringVisitor<'a> { pub(crate) fn new( - compiler: &'a ApolloCompiler, - file_id: FileId, + schema: &'a schema::Schema, + executable: &'a ast::Document, + implementers_map: &'a HashMap>, scopes: HashSet, ) -> Self { Self { - compiler, - file_id, + schema, + fragments: transform::collect_fragments(executable), + implementers_map, request_scopes: scopes, query_requires_scopes: false, unauthorized_paths: vec![], @@ -209,8 +177,8 @@ impl<'a> ScopeFilteringVisitor<'a> { } } - fn is_field_authorized(&mut self, field: &FieldDefinition) -> bool { - if let Some(directive) = field.directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME) { + fn is_field_authorized(&mut self, field: &schema::FieldDefinition) -> bool { + if let Some(directive) = field.directives.get(REQUIRES_SCOPES_DIRECTIVE_NAME) { let mut field_scopes_sets = scopes_sets_argument(directive); // The outer array acts like a logical OR: if any of the inner arrays of scopes matches, the field @@ -226,15 +194,15 @@ impl<'a> ScopeFilteringVisitor<'a> { } } - if let Some(ty) = field.ty().type_def(&self.compiler.db) { - self.is_type_authorized(&ty) + if let Some(ty) = self.schema.types.get(field.ty.inner_named_type()) { + self.is_type_authorized(ty) } else { false } } - fn is_type_authorized(&self, ty: &TypeDefinition) -> bool { - match ty.directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME) { + fn is_type_authorized(&self, ty: &schema::ExtendedType) -> bool { + match ty.directives().get(REQUIRES_SCOPES_DIRECTIVE_NAME) { None => true, Some(directive) => { let mut type_scopes_sets = scopes_sets_argument(directive); @@ -255,44 +223,51 @@ impl<'a> ScopeFilteringVisitor<'a> { fn implementors_with_different_requirements( &self, - parent_type: &str, - node: &hir::Field, + field_def: &ast::FieldDefinition, + node: &ast::Field, ) -> bool { // if all selections under the interface field are fragments with type conditions // then we don't need to check that they have the same authorization requirements - if node.selection_set().fields().is_empty() { + if node.selection_set.iter().all(|sel| { + matches!( + sel, + ast::Selection::FragmentSpread(_) | ast::Selection::InlineFragment(_) + ) + }) { return false; } - if let Some(type_definition) = get_field_type(self, parent_type, node.name()) - .and_then(|ty| self.compiler.db.find_type_definition_by_name(ty)) - { - if self.implementors_with_different_type_requirements(&type_definition) { + let field_type = field_def.ty.inner_named_type(); + if let Some(type_definition) = self.schema.types.get(field_type) { + if self.implementors_with_different_type_requirements(field_def, type_definition) { return true; } } false } - fn implementors_with_different_type_requirements(&self, t: &TypeDefinition) -> bool { - if t.is_interface_type_definition() { + fn implementors_with_different_type_requirements( + &self, + field_def: &ast::FieldDefinition, + t: &schema::ExtendedType, + ) -> bool { + if t.is_interface() { let mut scope_sets = None; + let type_name = field_def.ty.inner_named_type(); for ty in self - .compiler - .db - .subtype_map() - .get(t.name()) + .implementers_map + .get(type_name) .into_iter() .flatten() - .cloned() - .filter_map(|ty| self.compiler.db.find_type_definition_by_name(ty)) + .filter_map(|ty| self.schema.types.get(ty)) { // aggregate the list of scope sets // we transform to a common representation of sorted vectors because the element order // of hashsets is not stable let ty_scope_sets = ty - .directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME) + .directives() + .get(REQUIRES_SCOPES_DIRECTIVE_NAME) .map(|directive| { let mut v = scopes_sets_argument(directive) .map(|h| { @@ -323,32 +298,20 @@ impl<'a> ScopeFilteringVisitor<'a> { fn implementors_with_different_field_requirements( &self, parent_type: &str, - field: &hir::Field, + field: &ast::Field, ) -> bool { - if let Some(t) = self - .compiler - .db - .find_type_definition_by_name(parent_type.to_string()) - { - if t.is_interface_type_definition() { + if let Some(t) = self.schema.types.get(parent_type) { + if t.is_interface() { let mut scope_sets = None; - for ty in self - .compiler - .db - .subtype_map() - .get(t.name()) - .into_iter() - .flatten() - .cloned() - .filter_map(|ty| self.compiler.db.find_type_definition_by_name(ty)) - { - if let Some(f) = ty.field(&self.compiler.db, field.name()) { + for ty in self.implementers_map.get(parent_type).into_iter().flatten() { + if let Ok(f) = self.schema.type_field(ty, &field.name) { // aggregate the list of scope sets // we transform to a common representation of sorted vectors because the element order // of hashsets is not stable let field_scope_sets = f - .directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME) + .directives + .get(REQUIRES_SCOPES_DIRECTIVE_NAME) .map(|directive| { let mut v = scopes_sets_argument(directive) .map(|h| { @@ -380,16 +343,13 @@ impl<'a> ScopeFilteringVisitor<'a> { } impl<'a> transform::Visitor for ScopeFilteringVisitor<'a> { - fn compiler(&self) -> &ApolloCompiler { - self.compiler - } - fn operation( &mut self, - node: &hir::OperationDefinition, - ) -> Result, BoxError> { - let is_authorized = if let Some(ty) = node.object_type(&self.compiler.db) { - match ty.directive_by_name(REQUIRES_SCOPES_DIRECTIVE_NAME) { + root_type: &str, + node: &ast::OperationDefinition, + ) -> Result, BoxError> { + let is_authorized = if let Some(ty) = self.schema.types.get(root_type) { + match ty.directives().get(REQUIRES_SCOPES_DIRECTIVE_NAME) { None => true, Some(directive) => { let mut type_scopes_sets = scopes_sets_argument(directive); @@ -411,7 +371,7 @@ impl<'a> transform::Visitor for ScopeFilteringVisitor<'a> { }; if is_authorized { - transform::operation(self, node) + transform::operation(self, root_type, node) } else { self.unauthorized_paths.push(self.current_path.clone()); self.query_requires_scopes = true; @@ -422,35 +382,23 @@ impl<'a> transform::Visitor for ScopeFilteringVisitor<'a> { fn field( &mut self, parent_type: &str, - node: &hir::Field, - ) -> Result, BoxError> { - let field_name = node.name(); - - let mut is_field_list = false; - - let is_authorized = self - .compiler - .db - .types_definitions_by_name() - .get(parent_type) - .is_some_and(|def| { - if let Some(field) = def.field(&self.compiler.db, field_name) { - if field.ty().is_list() { - is_field_list = true; - } - self.is_field_authorized(field) - } else { - false - } - }); + field_def: &ast::FieldDefinition, + node: &ast::Field, + ) -> Result, BoxError> { + let field_name = &node.name; + + let is_field_list = field_def.ty.is_list(); + + let is_authorized = self.is_field_authorized(field_def); let implementors_with_different_requirements = - self.implementors_with_different_requirements(parent_type, node); + self.implementors_with_different_requirements(field_def, node); let implementors_with_different_field_requirements = self.implementors_with_different_field_requirements(parent_type, node); - self.current_path.push(PathElement::Key(field_name.into())); + self.current_path + .push(PathElement::Key(field_name.as_str().into())); if is_field_list { self.current_path.push(PathElement::Flatten); } @@ -459,7 +407,7 @@ impl<'a> transform::Visitor for ScopeFilteringVisitor<'a> { && !implementors_with_different_requirements && !implementors_with_different_field_requirements { - transform::field(self, parent_type, node) + transform::field(self, field_def, node) } else { self.unauthorized_paths.push(self.current_path.clone()); self.query_requires_scopes = true; @@ -476,13 +424,12 @@ impl<'a> transform::Visitor for ScopeFilteringVisitor<'a> { fn fragment_definition( &mut self, - node: &hir::FragmentDefinition, - ) -> Result, BoxError> { + node: &ast::FragmentDefinition, + ) -> Result, BoxError> { let fragment_is_authorized = self - .compiler - .db - .types_definitions_by_name() - .get(node.type_condition()) + .schema + .types + .get(&node.type_condition) .is_some_and(|ty| self.is_type_authorized(ty)); // FIXME: if a field was removed inside a fragment definition, then we should add an unauthorized path @@ -499,20 +446,19 @@ impl<'a> transform::Visitor for ScopeFilteringVisitor<'a> { fn fragment_spread( &mut self, - node: &hir::FragmentSpread, - ) -> Result, BoxError> { - let fragments = self.compiler.db.fragments(self.file_id); - let condition = fragments - .get(node.name()) - .ok_or("MissingFragmentDefinition")? - .type_condition(); + node: &ast::FragmentSpread, + ) -> Result, BoxError> { + let condition = &self + .fragments + .get(&node.fragment_name) + .ok_or("MissingFragment")? + .type_condition; self.current_path - .push(PathElement::Fragment(condition.into())); + .push(PathElement::Fragment(condition.as_str().into())); let fragment_is_authorized = self - .compiler - .db - .types_definitions_by_name() + .schema + .types .get(condition) .is_some_and(|ty| self.is_type_authorized(ty)); @@ -532,10 +478,9 @@ impl<'a> transform::Visitor for ScopeFilteringVisitor<'a> { fn inline_fragment( &mut self, parent_type: &str, - - node: &hir::InlineFragment, - ) -> Result, BoxError> { - match node.type_condition() { + node: &ast::InlineFragment, + ) -> Result, BoxError> { + match &node.type_condition { None => { self.current_path.push(PathElement::Fragment(String::new())); let res = transform::inline_fragment(self, parent_type, node); @@ -543,12 +488,12 @@ impl<'a> transform::Visitor for ScopeFilteringVisitor<'a> { res } Some(name) => { - self.current_path.push(PathElement::Fragment(name.into())); + self.current_path + .push(PathElement::Fragment(name.as_str().into())); let fragment_is_authorized = self - .compiler - .db - .types_definitions_by_name() + .schema + .types .get(name) .is_some_and(|ty| self.is_type_authorized(ty)); @@ -566,6 +511,10 @@ impl<'a> transform::Visitor for ScopeFilteringVisitor<'a> { } } } + + fn schema(&self) -> &apollo_compiler::Schema { + self.schema + } } #[cfg(test)] @@ -573,8 +522,8 @@ mod tests { use std::collections::BTreeSet; use std::collections::HashSet; - use apollo_compiler::ApolloCompiler; - use apollo_encoder::Document; + use apollo_compiler::ast::Document; + use apollo_compiler::Schema; use crate::json_ext::Path; use crate::plugins::authorization::scopes::ScopeExtractionVisitor; @@ -624,23 +573,12 @@ mod tests { "#; fn extract(schema: &str, query: &str) -> BTreeSet { - let mut compiler = ApolloCompiler::new(); - - let _schema_id = compiler.add_type_system(schema, "schema.graphql"); - let id = compiler.add_executable(query, "query.graphql"); - - let diagnostics = compiler - .validate() - .into_iter() - .filter(|err| err.data.is_error()) - .collect::>(); - for diagnostic in &diagnostics { - println!("{diagnostic}"); - } - assert!(diagnostics.is_empty()); - - let mut visitor = ScopeExtractionVisitor::new(&compiler, id); - traverse::document(&mut visitor, id).unwrap(); + let schema = Schema::parse(schema, "schema.graphql"); + let doc = Document::parse(query, "query.graphql"); + schema.validate().unwrap(); + doc.to_executable(&schema).validate(&schema).unwrap(); + let mut visitor = ScopeExtractionVisitor::new(&schema, &doc); + traverse::document(&mut visitor, &doc).unwrap(); visitor.extracted_scopes.into_iter().collect() } @@ -667,24 +605,15 @@ mod tests { #[track_caller] fn filter(schema: &str, query: &str, scopes: HashSet) -> (Document, Vec) { - let mut compiler = ApolloCompiler::new(); - - let _schema_id = compiler.add_type_system(schema, "schema.graphql"); - let file_id = compiler.add_executable(query, "query.graphql"); - - let diagnostics = compiler - .validate() - .into_iter() - .filter(|err| err.data.is_error()) - .collect::>(); - for diagnostic in &diagnostics { - println!("{diagnostic}"); - } - assert!(diagnostics.is_empty()); + let schema = Schema::parse(schema, "schema.graphql"); + let doc = Document::parse(query, "query.graphql"); + schema.validate().unwrap(); + doc.to_executable(&schema).validate(&schema).unwrap(); - let mut visitor = ScopeFilteringVisitor::new(&compiler, file_id, scopes); + let map = schema.implementers_map(); + let mut visitor = ScopeFilteringVisitor::new(&schema, &doc, &map, scopes); ( - transform::document(&mut visitor, file_id).unwrap(), + transform::document(&mut visitor, &doc).unwrap(), visitor.unauthorized_paths, ) } @@ -692,7 +621,7 @@ mod tests { struct TestResult<'a> { query: &'a str, extracted_scopes: &'a BTreeSet, - result: apollo_encoder::Document, + result: Document, scopes: Vec, paths: Vec, } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__array.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__array.snap index 20a3c8e19b..fd8526b2bc 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__array.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__array.snap @@ -17,7 +17,7 @@ query: } filtered: -query { +{ topProducts { type publicReviews { diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__defer.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__defer.snap index 0ae1fefd61..dc5ca1c07f 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__defer.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__defer.snap @@ -15,7 +15,7 @@ query: } filtered: -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_field-2.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_field-2.snap index 48558ebd59..5b0c5f5f96 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_field-2.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_field-2.snap @@ -20,7 +20,7 @@ query: } filtered: -query { +{ test itf { ... on A { diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_field.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_field.snap index 2cc8e91ab0..ebeef40e6d 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_field.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_field.snap @@ -13,7 +13,7 @@ query: } filtered: -query { +{ test itf { other diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_fragment.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_fragment.snap index b969b6615f..b6102639fc 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_fragment.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_fragment.snap @@ -19,7 +19,7 @@ query: } filtered: -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_inline_fragment.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_inline_fragment.snap index 10a80d5b05..779c84b511 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_inline_fragment.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_inline_fragment.snap @@ -17,7 +17,7 @@ query: } filtered: -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_type-2.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_type-2.snap index 73480e9d5e..d8cc218483 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_type-2.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_type-2.snap @@ -18,7 +18,7 @@ query: } filtered: -query { +{ test itf { ... on A { diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_type.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_type.snap index 1d06b6a5f0..b37cb0ec4c 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_type.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__interface_type.snap @@ -12,7 +12,7 @@ query: } filtered: -query { +{ test } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__query_field.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__query_field.snap index d4691a6c97..13245efb9c 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__query_field.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__query_field.snap @@ -15,7 +15,7 @@ query: } filtered: -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__query_field_alias.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__query_field_alias.snap index bb3ccaeec0..96482646ff 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__query_field_alias.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__query_field_alias.snap @@ -15,7 +15,7 @@ query: } filtered: -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__scalar.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__scalar.snap index 22bb2f4d4c..3039207ca7 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__scalar.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__scalar.snap @@ -12,7 +12,7 @@ query: } filtered: -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__test.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__test.snap index 25c169cd20..cdc952100c 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__test.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__test.snap @@ -18,7 +18,7 @@ query: } filtered: -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__union.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__union.snap index 98a7d35b73..392105f13d 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__union.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__authenticated__tests__union.snap @@ -17,7 +17,7 @@ query: } filtered: -query { +{ test uni { ... on A { diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__array-2.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__array-2.snap index 34750f1eca..9d88387b78 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__array-2.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__array-2.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__filter_basic_query-2.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__filter_basic_query-2.snap index 34750f1eca..9d88387b78 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__filter_basic_query-2.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__filter_basic_query-2.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__filter_basic_query-4.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__filter_basic_query-4.snap index 6753fadea3..f8535242ca 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__filter_basic_query-4.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__filter_basic_query-4.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ topProducts { type internal diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__filter_basic_query-6.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__filter_basic_query-6.snap index a9beb34c39..1ced6d1c8a 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__filter_basic_query-6.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__filter_basic_query-6.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ topProducts { type internal diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__filter_basic_query-8.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__filter_basic_query-8.snap index 1f729768c1..5fdc027b95 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__filter_basic_query-8.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__filter_basic_query-8.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_field-3.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_field-3.snap index 2f4521d824..ba4ae925aa 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_field-3.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_field-3.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ test itf { ... on A { diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_field.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_field.snap index 86c5f1940d..98f5bd99e0 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_field.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_field.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ test itf { other diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_fragment-2.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_fragment-2.snap index aac80f5c14..2888977a7f 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_fragment-2.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_fragment-2.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_fragment-4.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_fragment-4.snap index 9633b5a7dd..80c1f870ee 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_fragment-4.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_fragment-4.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ topProducts { type } @@ -11,6 +11,7 @@ query { ...F } } + fragment F on User { name } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_inline_fragment-2.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_inline_fragment-2.snap index aac80f5c14..2888977a7f 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_inline_fragment-2.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_inline_fragment-2.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type-3.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type-3.snap index b78cca30ee..fc3562f306 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type-3.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type-3.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ test } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type-5.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type-5.snap index b78cca30ee..fc3562f306 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type-5.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type-5.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ test } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type-7.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type-7.snap index b78cca30ee..fc3562f306 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type-7.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type-7.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ test } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type-9.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type-9.snap index 5994e8567a..5f8ebfd3e8 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type-9.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type-9.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ test itf { ... on A { diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type.snap index b78cca30ee..fc3562f306 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__interface_type.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ test } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__query_field-2.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__query_field-2.snap index 34750f1eca..9d88387b78 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__query_field-2.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__query_field-2.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__query_field_alias-2.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__query_field_alias-2.snap index 34750f1eca..9d88387b78 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__query_field_alias-2.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__query_field_alias-2.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__scalar-2.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__scalar-2.snap index 34750f1eca..9d88387b78 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__scalar-2.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__scalar-2.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__union.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__union.snap index e968662f05..181a094e66 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__union.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__policy__tests__union.snap @@ -2,7 +2,7 @@ source: apollo-router/src/plugins/authorization/policy.rs expression: doc --- -query { +{ test uni { ... on A { diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__array.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__array.snap index 1dfb5b2ef0..9b1c4e4893 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__array.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__array.snap @@ -19,7 +19,7 @@ query: extracted_scopes: {"read:user", "read:username", "review"} request scopes: [] filtered: -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__filter_basic_query-2.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__filter_basic_query-2.snap index c349120848..edbe80b323 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__filter_basic_query-2.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__filter_basic_query-2.snap @@ -19,7 +19,7 @@ query: extracted_scopes: {"internal", "profile", "read:user", "read:username", "test"} request scopes: ["profile", "internal"] filtered: -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__filter_basic_query-3.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__filter_basic_query-3.snap index 90a1b954d9..8b42b9c2a9 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__filter_basic_query-3.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__filter_basic_query-3.snap @@ -19,7 +19,7 @@ query: extracted_scopes: {"internal", "profile", "read:user", "read:username", "test"} request scopes: ["profile", "read:user", "internal", "test"] filtered: -query { +{ topProducts { type internal diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__filter_basic_query-4.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__filter_basic_query-4.snap index dc9f551e7c..65d7a8cfd3 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__filter_basic_query-4.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__filter_basic_query-4.snap @@ -19,7 +19,7 @@ query: extracted_scopes: {"internal", "profile", "read:user", "read:username", "test"} request scopes: ["profile", "read:user", "read:username"] filtered: -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__filter_basic_query.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__filter_basic_query.snap index cc094cab8a..08c511b8bd 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__filter_basic_query.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__filter_basic_query.snap @@ -19,7 +19,7 @@ query: extracted_scopes: {"internal", "profile", "read:user", "read:username", "test"} request scopes: [] filtered: -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_field-2.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_field-2.snap index d872f7d72e..0900c2a00e 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_field-2.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_field-2.snap @@ -21,7 +21,7 @@ query: extracted_scopes: {"a", "b", "c", "d"} request scopes: [] filtered: -query { +{ test itf { ... on A { diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_field.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_field.snap index ff68fe0481..3891bb0637 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_field.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_field.snap @@ -15,7 +15,7 @@ query: extracted_scopes: {} request scopes: [] filtered: -query { +{ test itf { other diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_fragment-2.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_fragment-2.snap index 4a9ecd227f..1d3ce16f04 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_fragment-2.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_fragment-2.snap @@ -22,7 +22,7 @@ query: extracted_scopes: {"read:user", "read:username"} request scopes: ["read:user"] filtered: -query { +{ topProducts { type } @@ -31,6 +31,7 @@ query { ...F } } + fragment F on User { id2: id } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_fragment-3.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_fragment-3.snap index 0061051f93..410f0562cb 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_fragment-3.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_fragment-3.snap @@ -22,7 +22,7 @@ query: extracted_scopes: {"read:user", "read:username"} request scopes: ["read:user", "read:username"] filtered: -query { +{ topProducts { type } @@ -31,6 +31,7 @@ query { ...F } } + fragment F on User { id2: id name diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_fragment.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_fragment.snap index 40d8661819..24f991e076 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_fragment.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_fragment.snap @@ -22,7 +22,7 @@ query: extracted_scopes: {"read:user", "read:username"} request scopes: [] filtered: -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_inline_fragment-2.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_inline_fragment-2.snap index 21a2667724..bb786b9339 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_inline_fragment-2.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_inline_fragment-2.snap @@ -20,7 +20,7 @@ query: extracted_scopes: {"read:user", "read:username"} request scopes: ["read:user"] filtered: -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_inline_fragment-3.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_inline_fragment-3.snap index e737d787ff..0d65e2f24d 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_inline_fragment-3.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_inline_fragment-3.snap @@ -20,7 +20,7 @@ query: extracted_scopes: {"read:user", "read:username"} request scopes: ["read:user", "read:username"] filtered: -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_inline_fragment.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_inline_fragment.snap index 9c3b1d28f0..777a1b4d9e 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_inline_fragment.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_inline_fragment.snap @@ -20,7 +20,7 @@ query: extracted_scopes: {"read:user", "read:username"} request scopes: [] filtered: -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type-2.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type-2.snap index de3cbb4aa1..cdbba76b5d 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type-2.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type-2.snap @@ -14,7 +14,7 @@ query: extracted_scopes: {"itf"} request scopes: ["itf"] filtered: -query { +{ test } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type-3.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type-3.snap index 0a3890e0cd..2c8e9f1bed 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type-3.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type-3.snap @@ -19,7 +19,7 @@ query: extracted_scopes: {"a", "b", "c", "d", "itf"} request scopes: [] filtered: -query { +{ test } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type-4.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type-4.snap index f8a64ef87a..af1808c4f6 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type-4.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type-4.snap @@ -19,7 +19,7 @@ query: extracted_scopes: {"a", "b", "c", "d", "itf"} request scopes: ["itf"] filtered: -query { +{ test } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type-5.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type-5.snap index 542b466aaf..3c564d43c5 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type-5.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type-5.snap @@ -19,7 +19,7 @@ query: extracted_scopes: {"a", "b", "c", "d", "itf"} request scopes: ["itf", "a", "b"] filtered: -query { +{ test itf { ... on A { diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type.snap index 40b0eb974c..911c15ebb2 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__interface_type.snap @@ -14,7 +14,7 @@ query: extracted_scopes: {"itf"} request scopes: [] filtered: -query { +{ test } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__query_field.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__query_field.snap index bca00335b4..cdaca6809d 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__query_field.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__query_field.snap @@ -17,7 +17,7 @@ query: extracted_scopes: {"profile", "read:user", "read:username"} request scopes: [] filtered: -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__query_field_alias.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__query_field_alias.snap index 09af582e5c..cbab8ea249 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__query_field_alias.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__query_field_alias.snap @@ -17,7 +17,7 @@ query: extracted_scopes: {"profile", "read:user", "read:username"} request scopes: [] filtered: -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__scalar.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__scalar.snap index 050c8b6d15..1a20eac24b 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__scalar.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__scalar.snap @@ -14,7 +14,7 @@ query: extracted_scopes: {"internal", "test"} request scopes: [] filtered: -query { +{ topProducts { type } diff --git a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__union.snap b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__union.snap index 294035e909..3990bb0bd0 100644 --- a/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__union.snap +++ b/apollo-router/src/plugins/authorization/snapshots/apollo_router__plugins__authorization__scopes__tests__union.snap @@ -19,7 +19,7 @@ query: extracted_scopes: {"a", "b", "c", "d"} request scopes: ["a", "b"] filtered: -query { +{ test uni { ... on A { diff --git a/apollo-router/src/plugins/telemetry/metrics/apollo.rs b/apollo-router/src/plugins/telemetry/metrics/apollo.rs index fdae0b536e..b5c22fbf51 100644 --- a/apollo-router/src/plugins/telemetry/metrics/apollo.rs +++ b/apollo-router/src/plugins/telemetry/metrics/apollo.rs @@ -310,7 +310,7 @@ mod test { #[tokio::test(flavor = "multi_thread")] async fn apollo_metrics_validation_failure() -> Result<(), BoxError> { - let query = "query {topProducts{unknown}}"; + let query = "query {topProducts(minStarRating: 4.7){name}}"; let results = get_metrics_for_request(query, None, None, false).await?; let mut settings = insta::Settings::clone_current(); settings.set_sort_maps(true); diff --git a/apollo-router/src/query_planner/bridge_query_planner.rs b/apollo-router/src/query_planner/bridge_query_planner.rs index 9e62ac21c9..aebe83418c 100644 --- a/apollo-router/src/query_planner/bridge_query_planner.rs +++ b/apollo-router/src/query_planner/bridge_query_planner.rs @@ -5,8 +5,7 @@ use std::fmt::Debug; use std::sync::Arc; use std::time::Instant; -use apollo_compiler::ApolloCompiler; -use apollo_compiler::InputDatabase; +use apollo_compiler::ast; use futures::future::BoxFuture; use router_bridge::planner::IncrementalDeliverySupport; use router_bridge::planner::PlanSuccess; @@ -16,7 +15,6 @@ use router_bridge::planner::UsageReporting; use serde::Deserialize; use serde_json_bytes::Map; use serde_json_bytes::Value; -use tokio::sync::Mutex; use tower::Service; use super::PlanNode; @@ -31,11 +29,11 @@ use crate::json_ext::Path; use crate::plugins::authorization::AuthorizationPlugin; use crate::plugins::authorization::CacheKeyMetadata; use crate::query_planner::labeler::add_defer_labels; -use crate::services::layers::query_analysis::Compiler; +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::QUERY_EXECUTABLE; use crate::spec::Query; use crate::spec::Schema; use crate::spec::SpecError; @@ -198,36 +196,29 @@ impl BridgeQueryPlanner { async fn parse_selections( &self, query: String, - operation_name: Option, - compiler: Arc>, + operation_name: Option<&str>, + doc: &ParsedDocument, ) -> Result { - let compiler_guard = compiler.lock().await; - + let ast = &doc.ast; + let executable = &doc.executable; crate::spec::operation_limits::check( &self.configuration, &query, - &compiler_guard, - operation_name.clone(), + executable, + operation_name, )?; - let file_id = compiler_guard - .db - .source_file(QUERY_EXECUTABLE.into()) - .ok_or_else(|| QueryPlannerError::SpecError(SpecError::UnknownFileId))?; - - Query::check_errors(&compiler_guard, file_id)?; + Query::check_errors(ast)?; let validation_error = match self.configuration.experimental_graphql_validation_mode { GraphQLValidationMode::Legacy => None, GraphQLValidationMode::New => { - Query::validate_query(&compiler_guard, file_id)?; + Query::validate_query(&self.schema, executable)?; None } - GraphQLValidationMode::Both => Query::validate_query(&compiler_guard, file_id).err(), + GraphQLValidationMode::Both => Query::validate_query(&self.schema, executable).err(), }; let (fragments, operations, defer_stats) = - Query::extract_query_information(&compiler_guard, &self.schema)?; - - drop(compiler_guard); + Query::extract_query_information(&self.schema, executable)?; let subselections = crate::spec::query::subselections::collect_subselections( &self.configuration, @@ -320,13 +311,15 @@ impl BridgeQueryPlanner { } } - let planner_result = self + let planner_result = match self .planner .plan(filtered_query.clone(), operation.clone()) .await .map_err(QueryPlannerError::RouterBridgeError)? .into_result() - .map_err(|err| { + { + Ok(plan) => plan, + Err(err) => { if matches!( self.configuration.experimental_graphql_validation_mode, GraphQLValidationMode::Both @@ -334,13 +327,13 @@ impl BridgeQueryPlanner { compare_validation_errors(Some(&err), selections.validation_error.as_ref()); // If we had a validation error from apollo-rs, return it now. - if let Some(validation_error) = &selections.validation_error { - return QueryPlannerError::from(validation_error.clone()); + if let Some(errors) = selections.validation_error { + return Err(QueryPlannerError::from(errors)); } } - - QueryPlannerError::from(err) - })?; + return Err(QueryPlannerError::from(err)); + } + }; if matches!( self.configuration.experimental_graphql_validation_mode, @@ -434,45 +427,39 @@ impl Service for BridgeQueryPlanner { let fut = async move { let start = Instant::now(); - let compiler = match context.private_entries.lock().get::() { + let mut doc = match context.private_entries.lock().get::() { None => return Err(QueryPlannerError::SpecError(SpecError::UnknownFileId)), - Some(c) => c.0.clone(), + Some(d) => d.clone(), }; - let mut compiler_guard = compiler.lock().await; - let file_id = compiler_guard - .db - .source_file(QUERY_EXECUTABLE.into()) - .ok_or(QueryPlannerError::SpecError(SpecError::UnknownFileId))?; - match add_defer_labels(file_id, &compiler_guard) { + let schema = &this.schema.api_schema().definitions; + match add_defer_labels(schema, &doc.ast) { Err(e) => { return Err(QueryPlannerError::SpecError(SpecError::ParsingError( e.to_string(), ))) } Ok(modified_query) => { - // We’ve already checked the original query against the configured token limit - // when first parsing it. - // We’ve now serialized a modified query (with labels added) and are about - // to re-parse it, but that’s an internal detail that should not affect - // which original queries are rejected because of the token limit. - compiler_guard.db.set_token_limit(None); - compiler_guard.update_executable(file_id, &modified_query); + doc = Arc::new(ParsedDocumentInner { + executable: modified_query.to_executable(schema), + ast: modified_query, + }); + context + .private_entries + .lock() + .insert::(doc.clone()); } } - let filtered_query = compiler_guard.db.source_code(file_id); - drop(compiler_guard); - let res = this .get( QueryKey { original_query, - filtered_query: filtered_query.to_string(), + filtered_query: doc.ast.to_string(), operation_name: operation_name.to_owned(), metadata, }, - compiler, + doc, ) .await; let duration = start.elapsed().as_secs_f64(); @@ -510,13 +497,13 @@ impl Service for BridgeQueryPlanner { } // Appease clippy::type_complexity -pub(crate) type FilteredQuery = (String, Vec, Arc>); +pub(crate) type FilteredQuery = (Vec, ast::Document); impl BridgeQueryPlanner { async fn get( &self, mut key: QueryKey, - mut compiler: Arc>, + mut doc: ParsedDocument, ) -> Result { let filter_res = if self.enable_authorization_directives { match AuthorizationPlugin::filter_query(&key, &self.schema) { @@ -549,14 +536,17 @@ impl BridgeQueryPlanner { let mut selections = self .parse_selections( key.original_query.clone(), - key.operation_name.clone(), - compiler.clone(), + key.operation_name.as_deref(), + &doc, ) .await?; - if let Some((filtered_query, unauthorized_paths, new_compiler)) = filter_res { - key.filtered_query = filtered_query; - compiler = new_compiler; + if let Some((unauthorized_paths, new_doc)) = filter_res { + key.filtered_query = new_doc.to_string(); + doc = Arc::new(ParsedDocumentInner { + executable: new_doc.to_executable(&self.schema.api_schema().definitions), + ast: new_doc, + }); selections.unauthorized_paths = unauthorized_paths; } @@ -587,8 +577,8 @@ impl BridgeQueryPlanner { let mut filtered = self .parse_selections( key.filtered_query.clone(), - key.operation_name.clone(), - compiler, + key.operation_name.as_deref(), + &doc, ) .await?; filtered.is_original = false; @@ -695,10 +685,10 @@ mod tests { .await .unwrap(); - let compiler = Query::make_compiler(query, &schema, &Configuration::default()).0; + let doc = Query::parse_document(query, &schema, &Configuration::default()); let selections = planner - .parse_selections(query.to_string(), None, Arc::new(Mutex::new(compiler))) + .parse_selections(query.to_string(), None, &doc) .await .unwrap(); let err = @@ -1034,7 +1024,6 @@ mod tests { } } - dbg!(query); let result = plan(EXAMPLE_SCHEMA, query, query, None).await.unwrap(); if let QueryPlannerContent::Plan { plan, .. } = result { check_query_plan_coverage(&plan.root, &Path::empty(), None, &plan.query.subselections); @@ -1070,7 +1059,7 @@ mod tests { .await .unwrap(); - let (compiler, _) = Query::make_compiler( + let doc = Query::parse_document( original_query, planner.schema().api_schema(), &configuration, @@ -1084,7 +1073,7 @@ mod tests { operation_name, metadata: CacheKeyMetadata::default(), }, - Arc::new(Mutex::new(compiler)), + doc, ) .await } diff --git a/apollo-router/src/query_planner/caching_query_planner.rs b/apollo-router/src/query_planner/caching_query_planner.rs index 18e20d387a..1c92f1084e 100644 --- a/apollo-router/src/query_planner/caching_query_planner.rs +++ b/apollo-router/src/query_planner/caching_query_planner.rs @@ -3,7 +3,6 @@ use std::ops::Deref; use std::sync::Arc; use std::task; -use apollo_compiler::InputDatabase; use futures::future::BoxFuture; use indexmap::IndexMap; use query_planner::QueryPlannerPlugin; @@ -13,7 +12,6 @@ use router_bridge::planner::Planner; use router_bridge::planner::UsageReporting; use sha2::Digest; use sha2::Sha256; -use tokio::sync::Mutex; use tower::ServiceBuilder; use tower::ServiceExt; use tower_service::Service; @@ -29,13 +27,12 @@ use crate::query_planner::labeler::add_defer_labels; use crate::query_planner::BridgeQueryPlanner; use crate::query_planner::QueryPlanResult; use crate::services::layers::persisted_queries::PersistedQueryLayer; -use crate::services::layers::query_analysis::Compiler; +use crate::services::layers::query_analysis::ParsedDocument; use crate::services::layers::query_analysis::QueryAnalysisLayer; use crate::services::query_planner; use crate::services::QueryPlannerContent; use crate::services::QueryPlannerRequest; use crate::services::QueryPlannerResponse; -use crate::spec::query::QUERY_EXECUTABLE; use crate::spec::Query; use crate::spec::Schema; use crate::spec::SpecError; @@ -175,22 +172,20 @@ where let entry = self.cache.get(&caching_key).await; if entry.is_first() { - let (compiler, file_id) = query_analysis.make_compiler(&query); - let err_res = Query::check_errors(&compiler, file_id); + let doc = query_analysis.parse_document(&query); + let err_res = Query::check_errors(&doc.ast); if let Err(error) = err_res { let e = Arc::new(QueryPlannerError::SpecError(error)); entry.insert(Err(e)).await; continue; } - if let Ok(modified_query) = add_defer_labels(file_id, &compiler) { - query = modified_query; + let schema = &self.schema.api_schema().definitions; + if let Ok(modified_query) = add_defer_labels(schema, &doc.ast) { + query = modified_query.to_string(); } - context - .private_entries - .lock() - .insert(Compiler(Arc::new(Mutex::new(compiler)))); + context.private_entries.lock().insert::(doc); context.private_entries.lock().insert(caching_key.metadata); @@ -311,30 +306,21 @@ where context, } = request; - let compiler = match context.private_entries.lock().get::() { + let doc = match context.private_entries.lock().get::() { None => { return Err(CacheResolverError::RetrievalError(Arc::new( QueryPlannerError::SpecError(SpecError::ParsingError( - "missing compiler".to_string(), + "missing parsed document".to_string(), )), ))) } - Some(c) => c.0.clone(), + Some(d) => d.clone(), }; - let compiler_guard = compiler.lock().await; - let file_id = compiler_guard - .db - .source_file(QUERY_EXECUTABLE.into()) - .ok_or(QueryPlannerError::SpecError(SpecError::ParsingError( - "missing input file for query".to_string(), - ))) - .map_err(|e| CacheResolverError::RetrievalError(Arc::new(e)))?; - - if let Ok(modified_query) = add_defer_labels(file_id, &compiler_guard) { - query = modified_query; + let schema = &self.schema.api_schema().definitions; + if let Ok(modified_query) = add_defer_labels(schema, &doc.ast) { + query = modified_query.to_string(); } - drop(compiler_guard); let request = QueryPlannerRequest::builder() .query(query) @@ -348,12 +334,7 @@ where // of restarting the query planner until another timeout tokio::task::spawn( async move { - // we need to isolate the compiler guard here, otherwise rustc might believe we still hold it - // when inserting the error in the entry - let err_res = { - let compiler_guard = compiler.lock().await; - Query::check_errors(&compiler_guard, file_id) - }; + let err_res = Query::check_errors(&doc.ast); if let Err(error) = err_res { request @@ -583,12 +564,13 @@ mod tests { let schema = Schema::parse(include_str!("testdata/schema.graphql"), &configuration).unwrap(); - let compiler1 = Arc::new(Mutex::new( - Query::make_compiler("query Me { me { username } }", &schema, &configuration).0, - )); + let doc1 = Query::parse_document("query Me { me { username } }", &schema, &configuration); let context = Context::new(); - context.private_entries.lock().insert(Compiler(compiler1)); + context + .private_entries + .lock() + .insert::(doc1); for _ in 0..5 { assert!(planner @@ -600,17 +582,17 @@ mod tests { .await .is_err()); } - let compiler2 = Arc::new(Mutex::new( - Query::make_compiler( - "query Me { me { name { first } } }", - &schema, - &configuration, - ) - .0, - )); + let doc2 = Query::parse_document( + "query Me { me { name { first } } }", + &schema, + &configuration, + ); let context = Context::new(); - context.private_entries.lock().insert(Compiler(compiler2)); + context + .private_entries + .lock() + .insert::(doc2); assert!(planner .call(query_planner::CachingRequest::new( @@ -660,16 +642,14 @@ mod tests { let schema = Schema::parse(include_str!("testdata/schema.graphql"), &configuration).unwrap(); - let compiler = Arc::new(Mutex::new( - Query::make_compiler("query Me { me { username } }", &schema, &configuration).0, - )); + let doc = Query::parse_document("query Me { me { username } }", &schema, &configuration); let mut planner = CachingQueryPlanner::new(delegate, Arc::new(schema), &configuration, IndexMap::new()) .await; let context = Context::new(); - context.private_entries.lock().insert(Compiler(compiler)); + context.private_entries.lock().insert::(doc); for _ in 0..5 { assert!(planner diff --git a/apollo-router/src/query_planner/fetch.rs b/apollo-router/src/query_planner/fetch.rs index 598fe203f3..fd034802b5 100644 --- a/apollo-router/src/query_planner/fetch.rs +++ b/apollo-router/src/query_planner/fetch.rs @@ -61,6 +61,26 @@ impl OperationKind { } } +impl From for apollo_compiler::ast::OperationType { + fn from(value: OperationKind) -> Self { + match value { + OperationKind::Query => apollo_compiler::ast::OperationType::Query, + OperationKind::Mutation => apollo_compiler::ast::OperationType::Mutation, + OperationKind::Subscription => apollo_compiler::ast::OperationType::Subscription, + } + } +} + +impl From for OperationKind { + fn from(value: apollo_compiler::ast::OperationType) -> Self { + match value { + apollo_compiler::ast::OperationType::Query => OperationKind::Query, + apollo_compiler::ast::OperationType::Mutation => OperationKind::Mutation, + apollo_compiler::ast::OperationType::Subscription => OperationKind::Subscription, + } + } +} + /// A fetch node. #[derive(Debug, Clone, PartialEq, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] diff --git a/apollo-router/src/query_planner/labeler.rs b/apollo-router/src/query_planner/labeler.rs index 45eac5215c..b4d214a31f 100644 --- a/apollo-router/src/query_planner/labeler.rs +++ b/apollo-router/src/query_planner/labeler.rs @@ -1,15 +1,14 @@ //! Query Transformer implementation adding labels to @defer directives to identify deferred responses //! -use apollo_compiler::hir; -use apollo_compiler::ApolloCompiler; -use apollo_compiler::FileId; +use apollo_compiler::ast; +use apollo_compiler::Node; +use apollo_compiler::Schema; use tower::BoxError; use crate::spec::query::subselections::DEFER_DIRECTIVE_NAME; use crate::spec::query::transform; use crate::spec::query::transform::document; -use crate::spec::query::transform::selection_set; use crate::spec::query::transform::Visitor; const LABEL_NAME: &str = "label"; @@ -18,22 +17,22 @@ const LABEL_NAME: &str = "label"; /// /// This is used to uniquely identify deferred responses pub(crate) fn add_defer_labels( - file_id: FileId, - compiler: &ApolloCompiler, -) -> Result { + schema: &Schema, + doc: &ast::Document, +) -> Result { let mut visitor = Labeler { - compiler, next_label: 0, + schema, }; - let encoder_document = document(&mut visitor, file_id)?; - Ok(encoder_document.to_string()) + document(&mut visitor, doc) } + pub(crate) struct Labeler<'a> { - compiler: &'a ApolloCompiler, + schema: &'a Schema, next_label: u32, } -impl<'a> Labeler<'a> { +impl Labeler<'_> { fn generate_label(&mut self) -> String { let label = self.next_label.to_string(); self.next_label += 1; @@ -41,94 +40,78 @@ impl<'a> Labeler<'a> { } } -impl<'a> Visitor for Labeler<'a> { - fn compiler(&self) -> &apollo_compiler::ApolloCompiler { - self.compiler - } - +impl Visitor for Labeler<'_> { fn fragment_spread( &mut self, - hir: &hir::FragmentSpread, - ) -> Result, BoxError> { - let name = hir.name(); - let mut encoder_node = apollo_encoder::FragmentSpread::new(name.into()); - for hir in hir.directives() { - encoder_node.directive(directive(self, hir)?); - } - Ok(Some(encoder_node)) + def: &ast::FragmentSpread, + ) -> Result, BoxError> { + let mut new = transform::fragment_spread(self, def)?.unwrap(); + directives(self, &mut new.directives)?; + Ok(Some(new)) } fn inline_fragment( &mut self, parent_type: &str, - hir: &hir::InlineFragment, - ) -> Result, BoxError> { - let parent_type = hir.type_condition().unwrap_or(parent_type); - - let Some(selection_set) = selection_set(self, hir.selection_set(), parent_type)? else { - return Ok(None); - }; - - let mut encoder_node = apollo_encoder::InlineFragment::new(selection_set); - - encoder_node.type_condition( - hir.type_condition() - .map(|name| apollo_encoder::TypeCondition::new(name.into())), - ); + def: &ast::InlineFragment, + ) -> Result, BoxError> { + let mut new = transform::inline_fragment(self, parent_type, def)?.unwrap(); + directives(self, &mut new.directives)?; + Ok(Some(new)) + } - for hir in hir.directives() { - encoder_node.directive(directive(self, hir)?); - } - Ok(Some(encoder_node)) + fn schema(&self) -> &apollo_compiler::Schema { + self.schema } } -pub(crate) fn directive( - visitor: &mut Labeler<'_>, - hir: &hir::Directive, -) -> Result { - let name = hir.name().into(); - let is_defer = name == DEFER_DIRECTIVE_NAME; - let mut encoder_directive = apollo_encoder::Directive::new(name); - - let mut has_label = false; - for arg in hir.arguments() { - // Add a prefix to existing labels - let value = if is_defer && arg.name() == LABEL_NAME { - has_label = true; - if let Some(label) = arg.value().as_str() { - apollo_encoder::Value::String(format!("_{label}")) - } else { - return Err("@defer with a non-string label".into()); +fn directives( + visitor: &mut Labeler, + directives: &mut [Node], +) -> Result<(), BoxError> { + for directive in directives { + if directive.name != DEFER_DIRECTIVE_NAME { + continue; + } + let directive = directive.make_mut(); + let mut has_label = false; + for arg in &mut directive.arguments { + if arg.name == LABEL_NAME { + has_label = true; + if let ast::Value::String(label) = arg.make_mut().value.make_mut() { + // Add a prefix to existing labels + *label = format!("_{label}").into(); + } else { + return Err("@defer with a non-string label".into()); + } } - } else { - transform::value(arg.value())? - }; - encoder_directive.arg(apollo_encoder::Argument::new(arg.name().into(), value)); - } - // Add a generated label if there wasn’t one already - if is_defer && !has_label { - encoder_directive.arg(apollo_encoder::Argument::new( - LABEL_NAME.into(), - apollo_encoder::Value::String(visitor.generate_label()), - )); + } + // Add a generated label if there wasn’t one already + if !has_label { + directive.arguments.push( + ast::Argument { + name: LABEL_NAME.into(), + value: visitor.generate_label().into(), + } + .into(), + ); + } } - - Ok(encoder_directive) + Ok(()) } #[cfg(test)] mod tests { - use apollo_compiler::ApolloCompiler; use super::add_defer_labels; #[test] fn large_float_written_as_int() { - let mut compiler = ApolloCompiler::new(); - compiler.add_type_system("type Query { field(id: Float): String! }", "schema.graphql"); - let file_id = compiler.add_executable(r#"{ field(id: 1234567890123) }"#, "query.graphql"); - let result = add_defer_labels(file_id, &compiler).unwrap(); + let schema = "type Query { field(id: Float): String! }"; + let query = r#"{ field(id: 1234567890123) }"#; + let schema = apollo_compiler::Schema::parse(schema, "schema.graphql"); + let doc = apollo_compiler::ast::Document::parse(query, "query.graphql"); + let result = add_defer_labels(&schema, &doc).unwrap().to_string(); insta::assert_snapshot!(result); } } diff --git a/apollo-router/src/query_planner/snapshots/apollo_router__query_planner__bridge_query_planner__tests__plan_invalid_query_errors.snap b/apollo-router/src/query_planner/snapshots/apollo_router__query_planner__bridge_query_planner__tests__plan_invalid_query_errors.snap index d7471be21d..bc605b4423 100644 --- a/apollo-router/src/query_planner/snapshots/apollo_router__query_planner__bridge_query_planner__tests__plan_invalid_query_errors.snap +++ b/apollo-router/src/query_planner/snapshots/apollo_router__query_planner__bridge_query_planner__tests__plan_invalid_query_errors.snap @@ -4,7 +4,7 @@ expression: errors --- [ GraphQLError { - message: "fragment `UnusedTestFragment` is unused", + message: "compiler error: fragment `UnusedTestFragment` must be used in an operation", locations: [ GraphQLLocation { line: 1, diff --git a/apollo-router/src/query_planner/snapshots/apollo_router__query_planner__labeler__tests__large_float_written_as_int.snap b/apollo-router/src/query_planner/snapshots/apollo_router__query_planner__labeler__tests__large_float_written_as_int.snap index 8be08a97cc..3cd16463ee 100644 --- a/apollo-router/src/query_planner/snapshots/apollo_router__query_planner__labeler__tests__large_float_written_as_int.snap +++ b/apollo-router/src/query_planner/snapshots/apollo_router__query_planner__labeler__tests__large_float_written_as_int.snap @@ -2,7 +2,7 @@ source: apollo-router/src/query_planner/labeler.rs expression: result --- -query { +{ field(id: 1234567890123) } diff --git a/apollo-router/src/router/mod.rs b/apollo-router/src/router/mod.rs index dc6e92b700..6fffb70dac 100644 --- a/apollo-router/src/router/mod.rs +++ b/apollo-router/src/router/mod.rs @@ -487,11 +487,11 @@ mod tests { let response = router_handle.request(request).await.unwrap(); assert_eq!( - "cannot query field 'name' on type 'User'", - response.errors[0].message + "parsing error: no field `name` in type `User`", response.errors[0].message, + "{response:?}" ); assert_eq!( - "INVALID_FIELD", + "PARSING_ERROR", response.errors[0].extensions.get("code").unwrap() ); @@ -549,11 +549,11 @@ mod tests { let response = router_handle.request(request).await.unwrap(); assert_eq!( - "cannot query field 'name' on type 'User'", + "parsing error: no field `name` in type `User`", response.errors[0].message ); assert_eq!( - "INVALID_FIELD", + "PARSING_ERROR", response.errors[0].extensions.get("code").unwrap() ); router_handle.shutdown().await.unwrap(); 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 ed1ad14fc1..b9bf018ba7 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 @@ -4,9 +4,7 @@ use std::ops::ControlFlow; -use apollo_compiler::hir::OperationType; -use apollo_compiler::HirDatabase; -use apollo_compiler::InputDatabase; +use apollo_compiler::ast::OperationType; use futures::future::BoxFuture; use http::header::HeaderName; use http::HeaderValue; @@ -17,14 +15,13 @@ use tower::Layer; use tower::Service; use tower::ServiceBuilder; -use super::query_analysis::Compiler; +use super::query_analysis::ParsedDocument; use crate::graphql::Error; use crate::json_ext::Object; use crate::layers::async_checkpoint::OneShotAsyncCheckpointService; use crate::layers::ServiceBuilderExt; use crate::services::SupergraphRequest; use crate::services::SupergraphResponse; -use crate::spec::query::QUERY_EXECUTABLE; #[derive(Default)] pub(crate) struct AllowOnlyHttpPostMutationsLayer {} @@ -51,11 +48,11 @@ where return Ok(ControlFlow::Continue(req)); } - let compiler = match req.context.private_entries.lock().get::() { + let doc = match req.context.private_entries.lock().get::() { None => { let errors = vec![Error::builder() - .message("Cannot find compiler".to_string()) - .extension_code("MISSING_COMPILER") + .message("Cannot find executable document".to_string()) + .extension_code("MISSING_EXECUTABLE_DOCUMENT") .build()]; let res = SupergraphResponse::builder() .errors(errors) @@ -66,22 +63,15 @@ where return Ok(ControlFlow::Break(res)); } - Some(c) => c.0.clone(), + Some(c) => c.clone(), }; - let c = compiler.lock().await.snapshot(); - let file_id = c - .source_file(QUERY_EXECUTABLE.into()) - .expect("the query is already loaded in the compiler"); - - let op = c.find_operation( - file_id, - req.supergraph_request.body().operation_name.clone(), - ); - drop(c); + let op = doc + .executable + .get_operation(req.supergraph_request.body().operation_name.as_deref()); match op { - None => { + Err(_) => { let errors = vec![Error::builder() .message("Cannot find operation".to_string()) .extension_code("MISSING_OPERATION") @@ -95,8 +85,8 @@ where Ok(ControlFlow::Break(res)) } - Some(op) => { - if op.operation_ty() == OperationType::Mutation { + Ok(op) => { + if op.operation_type == OperationType::Mutation { let errors = vec![Error::builder() .message( "Mutations can only be sent over HTTP POST".to_string(), @@ -133,8 +123,7 @@ where mod forbid_http_get_mutations_tests { use std::sync::Arc; - use apollo_compiler::ApolloCompiler; - use tokio::sync::Mutex; + use apollo_compiler::ast; use tower::ServiceExt; use super::*; @@ -142,6 +131,7 @@ mod forbid_http_get_mutations_tests { use crate::graphql::Response; use crate::plugin::test::MockSupergraphService; use crate::query_planner::fetch::OperationKind; + use crate::services::layers::query_analysis::ParsedDocumentInner; use crate::Context; #[tokio::test] @@ -264,20 +254,20 @@ mod forbid_http_get_mutations_tests { fn create_request(method: Method, operation_kind: OperationKind) -> SupergraphRequest { let query = match operation_kind { - OperationKind::Query => "query { a }", - OperationKind::Mutation => "mutation { a }", + OperationKind::Query => "query { a }\n type Query { a: Int }", + OperationKind::Mutation => "mutation { a }\n type Mutation { a: Int }", - OperationKind::Subscription => "subscription { a }", + OperationKind::Subscription => "subscription { a }\n type Subscription { a: Int }", }; - let mut compiler = ApolloCompiler::new(); - compiler.add_executable(query, QUERY_EXECUTABLE); + let ast = ast::Document::parse(query, ""); + let (_schema, executable) = ast.to_mixed(); let context = Context::new(); context .private_entries .lock() - .insert(Compiler(Arc::new(Mutex::new(compiler)))); + .insert::(Arc::new(ParsedDocumentInner { ast, executable })); let request = SupergraphRequest::fake_builder() .method(method) diff --git a/apollo-router/src/services/layers/persisted_queries/manifest_poller.rs b/apollo-router/src/services/layers/persisted_queries/manifest_poller.rs index 409277ea63..9fd829f278 100644 --- a/apollo-router/src/services/layers/persisted_queries/manifest_poller.rs +++ b/apollo-router/src/services/layers/persisted_queries/manifest_poller.rs @@ -5,10 +5,7 @@ use std::collections::HashSet; use std::sync::Arc; use std::sync::RwLock; -use apollo_parser::ast; -use apollo_parser::ast::AstNode; -use apollo_parser::Parser; -use apollo_parser::SyntaxTree; +use apollo_compiler::ast; use futures::prelude::*; use reqwest::Client; use serde::Deserialize; @@ -63,7 +60,7 @@ impl FreeformGraphQLBehavior { fn action_for_freeform_graphql( &self, body_from_request: &str, - ast: SyntaxTree, + ast: &ast::Document, ) -> FreeformGraphQLAction { match self { FreeformGraphQLBehavior::AllowAll { .. } => FreeformGraphQLAction::Allow, @@ -137,12 +134,13 @@ impl FreeformGraphQLSafelist { } fn insert_from_manifest(&mut self, body_from_manifest: &str) { - self.normalized_bodies.insert( - self.normalize_body(body_from_manifest, Parser::new(body_from_manifest).parse()), - ); + self.normalized_bodies.insert(self.normalize_body( + body_from_manifest, + &ast::Document::parse(body_from_manifest, "from_manifest"), + )); } - fn is_allowed(&self, body_from_request: &str, ast: SyntaxTree) -> bool { + fn is_allowed(&self, body_from_request: &str, ast: &ast::Document) -> bool { // Note: consider adding an LRU cache that caches this function's return // value based solely on body_from_request without needing to normalize // the body. @@ -150,55 +148,39 @@ impl FreeformGraphQLSafelist { .contains(&self.normalize_body(body_from_request, ast)) } - fn normalize_body(&self, body_from_request: &str, ast: SyntaxTree) -> String { - if ast.errors().peekable().peek().is_some() { + fn normalize_body(&self, body_from_request: &str, ast: &ast::Document) -> String { + if ast.check_parse_errors().is_err() { // If we can't parse the operation (whether from the PQ list or the // incoming request), then we can't normalize it. We keep it around // unnormalized, so that it at least works as a byte-for-byte - // safelist entry. (We also do this if we get any other errors - // below.) + // safelist entry. body_from_request.to_string() } else { - let mut encoder_document = apollo_encoder::Document::new(); - let mut operations = vec![]; let mut fragments = vec![]; - for definition in ast.document().syntax().children() { - if ast::OperationDefinition::can_cast(definition.kind()) { - operations.push(ast::OperationDefinition::cast(definition).unwrap()) - } else if ast::FragmentDefinition::can_cast(definition.kind()) { - fragments.push(ast::FragmentDefinition::cast(definition).unwrap()) + for definition in &ast.definitions { + match definition { + ast::Definition::OperationDefinition(def) => operations.push(def.clone()), + ast::Definition::FragmentDefinition(def) => fragments.push(def.clone()), + _ => {} } } - // First include operation definitions, sorted by name. - operations.sort_by_key(|x| x.name().map(|n| n.text().to_string())); - for parser_operation in operations { - let encoder_operation = match parser_operation.try_into() { - Ok(op) => op, - Err(_) => return body_from_request.to_string(), - }; + let mut new_document = ast::Document::new(); - encoder_document.operation(encoder_operation) - } + // First include operation definitions, sorted by name. + operations.sort_by_key(|x| x.name.clone()); + new_document + .definitions + .extend(operations.into_iter().map(Into::into)); // Next include fragment definitions, sorted by name. - fragments.sort_by_key(|x| { - x.fragment_name() - .and_then(|n| n.name()) - .map(|n| n.text().to_string()) - }); - for parser_fragment in fragments { - let encoder_fragment = match parser_fragment.try_into() { - Ok(op) => op, - Err(_) => return body_from_request.to_string(), - }; - - encoder_document.fragment(encoder_fragment) - } - - encoder_document.to_string() + fragments.sort_by_key(|x| x.name.clone()); + new_document + .definitions + .extend(fragments.into_iter().map(Into::into)); + new_document.to_string() } } } @@ -296,7 +278,7 @@ impl PersistedQueryManifestPoller { pub(crate) fn action_for_freeform_graphql( &self, query: &str, - ast: SyntaxTree, + ast: &ast::Document, ) -> FreeformGraphQLAction { let state = self .state @@ -679,7 +661,7 @@ mod tests { ])); let is_allowed = - |body: &str| -> bool { safelist.is_allowed(body, Parser::new(body).parse()) }; + |body: &str| -> bool { safelist.is_allowed(body, &ast::Document::parse(body, "")) }; // Precise string matches. assert!(is_allowed( diff --git a/apollo-router/src/services/layers/persisted_queries/mod.rs b/apollo-router/src/services/layers/persisted_queries/mod.rs index 27c92ba13f..9580ed2417 100644 --- a/apollo-router/src/services/layers/persisted_queries/mod.rs +++ b/apollo-router/src/services/layers/persisted_queries/mod.rs @@ -4,9 +4,6 @@ mod manifest_poller; #[cfg(test)] use std::sync::Arc; -use apollo_compiler::AstDatabase; -use apollo_compiler::HirDatabase; -use apollo_compiler::InputDatabase; use http::header::CACHE_CONTROL; use http::HeaderValue; use id_extractor::PersistedQueryIdExtractor; @@ -14,11 +11,10 @@ pub(crate) use manifest_poller::PersistedQueryManifestPoller; use tower::BoxError; use self::manifest_poller::FreeformGraphQLAction; -use super::query_analysis::Compiler; +use super::query_analysis::ParsedDocument; use crate::graphql::Error as GraphQLError; use crate::services::SupergraphRequest; use crate::services::SupergraphResponse; -use crate::spec::query::QUERY_EXECUTABLE; use crate::Configuration; const DONT_CACHE_RESPONSE_VALUE: &str = "private, no-cache, must-revalidate"; @@ -165,7 +161,7 @@ impl PersistedQueryLayer { Some(ob) => ob, }; - let compiler = { + let doc = { let context_guard = request.context.private_entries.lock(); if context_guard.get::().is_some() { @@ -175,33 +171,20 @@ impl PersistedQueryLayer { return Ok(request); } - match context_guard.get::() { + match context_guard.get::() { None => { drop(context_guard); - // For some reason, QueryAnalysisLayer didn't give us a Compiler? + // For some reason, QueryAnalysisLayer didn't give us a document? return Err(supergraph_err( graphql_err( "MISSING_PARSED_OPERATION", - "internal error: compiler missing", + "internal error: executable document missing", ), request, ErrorCacheStrategy::DontCache, )); } - Some(c) => c.0.clone(), - } - }; - - let compiler_guard = compiler.lock().await; - let db = &compiler_guard.db; - let file_id = match db.source_file(QUERY_EXECUTABLE.into()) { - Some(file_id) => file_id, - None => { - return Err(supergraph_err( - graphql_err("MISSING_PARSED_OPERATION", "missing input file for query"), - request, - ErrorCacheStrategy::DontCache, - )) + Some(d) => d.clone(), } }; @@ -211,16 +194,16 @@ impl PersistedQueryLayer { // __type/__schema/__typename.) We do want to make sure the document // parsed properly before poking around at it, though. if self.introspection_enabled - && db.ast(file_id).errors().peekable().peek().is_none() - && db - .operations(file_id) - .iter() - .all(|op| op.is_introspection(db)) + && doc.ast.check_parse_errors().is_ok() + && doc + .executable + .all_operations() + .all(|op| op.is_introspection(&doc.executable)) { return Ok(request); } - match manifest_poller.action_for_freeform_graphql(operation_body, db.ast(file_id)) { + match manifest_poller.action_for_freeform_graphql(operation_body, &doc.ast) { FreeformGraphQLAction::Allow => { tracing::info!(monotonic_counter.apollo.router.operations.persisted_queries = 1u64,); Ok(request) @@ -677,7 +660,7 @@ mod tests { async fn pq_layer_freeform_graphql_with_safelist() { let manifest = HashMap::from([( "valid-syntax".to_string(), - "fragment A on T { a } query SomeOp { ...A ...B } fragment,,, B on U{b c } # yeah" + "fragment A on Query { me { id } } query SomeOp { ...A ...B } fragment,,, B on Query{me{name,username} } # yeah" .to_string(), ), ( "invalid-syntax".to_string(), @@ -715,7 +698,7 @@ mod tests { denied_by_safelist( &pq_layer, &query_analysis_layer, - "query SomeQuery { hooray }", + "query SomeQuery { me { id } }", ) .await; @@ -723,7 +706,7 @@ mod tests { allowed_by_safelist( &pq_layer, &query_analysis_layer, - "fragment A on T { a } query SomeOp { ...A ...B } fragment,,, B on U{b c } # yeah", + "fragment A on Query { me { id } } query SomeOp { ...A ...B } fragment,,, B on Query{me{name,username} } # yeah", ) .await; @@ -731,14 +714,14 @@ mod tests { allowed_by_safelist( &pq_layer, &query_analysis_layer, - "#comment\n fragment, B on U , { b c } query SomeOp { ...A ...B } fragment \nA on T { a }" + "#comment\n fragment, B on Query , { me{name username} } query SomeOp { ...A ...B } fragment \nA on Query { me{ id} }" ).await; // Reordering fields does not match! denied_by_safelist( &pq_layer, &query_analysis_layer, - "fragment A on T { a } query SomeOp { ...A ...B } fragment,,, B on U{c b } # yeah" + "fragment A on Query { me { id } } query SomeOp { ...A ...B } fragment,,, B on Query{me{username,name} } # yeah" ).await; // Documents with invalid syntax don't match... @@ -767,7 +750,7 @@ mod tests { denied_by_safelist( &pq_layer, &query_analysis_layer, - r#"fragment F on Query { __typename foo: __schema { __typename } bla } query Q { __type(name: "foo") { name } ...F }"#, + r#"fragment F on Query { __typename foo: __schema { __typename } me { id } } query Q { __type(name: "foo") { name } ...F }"#, ).await; } diff --git a/apollo-router/src/services/layers/query_analysis.rs b/apollo-router/src/services/layers/query_analysis.rs index 8a23afc163..85352c7b05 100644 --- a/apollo-router/src/services/layers/query_analysis.rs +++ b/apollo-router/src/services/layers/query_analysis.rs @@ -1,8 +1,7 @@ use std::sync::Arc; -use apollo_compiler::ApolloCompiler; -use apollo_compiler::FileId; -use apollo_compiler::HirDatabase; +use apollo_compiler::ast; +use apollo_compiler::ExecutableDocument; use http::StatusCode; use lru::LruCache; use tokio::sync::Mutex; @@ -20,9 +19,9 @@ use crate::Context; #[derive(Clone)] #[allow(clippy::type_complexity)] pub(crate) struct QueryAnalysisLayer { - schema: Arc, + pub(crate) schema: Arc, configuration: Arc, - cache: Arc>)>>>, + cache: Arc>>, enable_authorization_directives: bool, } @@ -51,8 +50,8 @@ impl QueryAnalysisLayer { } } - pub(crate) fn make_compiler(&self, query: &str) -> (ApolloCompiler, FileId) { - Query::make_compiler(query, self.schema.api_schema(), &self.configuration) + pub(crate) fn parse_document(&self, query: &str) -> ParsedDocument { + Query::parse_document(query, self.schema.api_schema(), &self.configuration) } pub(crate) async fn supergraph_request( @@ -98,20 +97,18 @@ impl QueryAnalysisLayer { }) .cloned(); - let (context, compiler) = match entry { + let (context, doc) = match entry { None => { let span = tracing::info_span!("parse_query", "otel.kind" = "INTERNAL"); - let (compiler, file_id) = span.in_scope(|| self.make_compiler(&query)); + let doc = span.in_scope(|| self.parse_document(&query)); - let compiler = Arc::new(Mutex::new(compiler)); let context = Context::new(); - let operation_name = compiler - .lock() - .await - .db - .find_operation(file_id, op_name.clone()) - .and_then(|operation| operation.name().map(|s| s.to_owned())); + let operation_name = doc + .executable + .get_operation(op_name.as_deref()) + .ok() + .and_then(|operation| operation.name().map(|s| s.as_str().to_owned())); context.insert(OPERATION_NAME, operation_name).unwrap(); @@ -130,10 +127,10 @@ impl QueryAnalysisLayer { query, operation_name: op_name, }, - (context.clone(), compiler.clone()), + (context.clone(), doc.clone()), ); - (context, compiler) + (context, doc) } Some(c) => c, }; @@ -143,7 +140,7 @@ impl QueryAnalysisLayer { .context .private_entries .lock() - .insert(Compiler(compiler)); + .insert::(doc); Ok(SupergraphRequest { supergraph_request: request.supergraph_request, @@ -152,4 +149,9 @@ impl QueryAnalysisLayer { } } -pub(crate) struct Compiler(pub(crate) Arc>); +pub(crate) type ParsedDocument = Arc; + +pub(crate) struct ParsedDocumentInner { + pub(crate) ast: ast::Document, + pub(crate) executable: ExecutableDocument, +} diff --git a/apollo-router/src/services/supergraph_service.rs b/apollo-router/src/services/supergraph_service.rs index 75a68256f5..d45fd1db6e 100644 --- a/apollo-router/src/services/supergraph_service.rs +++ b/apollo-router/src/services/supergraph_service.rs @@ -15,7 +15,6 @@ use indexmap::IndexMap; use router_bridge::planner::Planner; use router_bridge::planner::UsageReporting; use tokio::sync::mpsc; -use tokio::sync::Mutex; use tower::BoxError; use tower::Layer; use tower::ServiceBuilder; @@ -29,7 +28,7 @@ use super::execution::QueryPlan; use super::layers::allow_only_http_post_mutations::AllowOnlyHttpPostMutationsLayer; use super::layers::content_negotiation; use super::layers::persisted_queries::PersistedQueryLayer; -use super::layers::query_analysis::Compiler; +use super::layers::query_analysis::ParsedDocument; use super::layers::query_analysis::QueryAnalysisLayer; use super::new_service::ServiceFactory; use super::router::ClientRequestAccepts; @@ -569,16 +568,17 @@ async fn plan_query( query_str: String, ) -> Result { // FIXME: we have about 80 tests creating a supergraph service and crafting a supergraph request for it - // none of those tests create a compiler to put it in the context, and the compiler cannot be created + // none of those tests create an executable document to put it in the context, and the document cannot be created // from inside the supergraph request fake builder, because it needs a schema matching the query. - // So while we are updating the tests to create a compiler manually, this here will make sure current + // So while we are updating the tests to create a document manually, this here will make sure current // tests will pass { let mut entries = context.private_entries.lock(); - if !entries.contains_key::() { - let (compiler, _) = - Query::make_compiler(&query_str, &schema, &Configuration::default()); - entries.insert(Compiler(Arc::new(Mutex::new(compiler)))); + if !entries.contains_key::() { + let doc = Query::parse_document(&query_str, &schema, &Configuration::default()); + Query::validate_query(&schema, &doc.executable) + .map_err(crate::error::QueryPlannerError::from)?; + entries.insert::(doc); } drop(entries); } @@ -818,6 +818,7 @@ mod tests { @core(feature: "https://specs.apollo.dev/inaccessible/v0.1") { query: Query + subscription: Subscription } directive @core(feature: String!) repeatable on SCHEMA directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet) on FIELD_DEFINITION @@ -1379,7 +1380,6 @@ mod tests { .unwrap(); let mut stream = service.oneshot(request).await.unwrap(); let res = stream.next_response().await.unwrap(); - assert_eq!(&res.data, &Some(serde_json_bytes::Value::Null)); insta::assert_json_snapshot!(res); notify.broadcast(graphql::Response::builder().data(serde_json_bytes::json!({"userWasCreated": { "name": "test", "activeOrganization": { "__typename": "Organization", "id": "0" }}})).build()).await.unwrap(); insta::assert_json_snapshot!(stream.next_response().await.unwrap()); @@ -1451,7 +1451,6 @@ mod tests { .unwrap(); let mut stream = service.oneshot(request).await.unwrap(); let res = stream.next_response().await.unwrap(); - assert_eq!(&res.data, &Some(serde_json_bytes::Value::Null)); insta::assert_json_snapshot!(res); notify.broadcast(graphql::Response::builder().data(serde_json_bytes::json!({"userWasCreated": { "name": "test", "activeOrganization": { "__typename": "Organization", "id": "0" }}})).build()).await.unwrap(); insta::assert_json_snapshot!(stream.next_response().await.unwrap()); @@ -1519,8 +1518,6 @@ mod tests { .unwrap(); let mut stream = service.ready().await.unwrap().call(request).await.unwrap(); let res = stream.next_response().await.unwrap(); - assert_eq!(&res.data, &Some(serde_json_bytes::Value::Null)); - assert!(res.errors.is_empty()); insta::assert_json_snapshot!(res); notify.broadcast(graphql::Response::builder().data(serde_json_bytes::json!({"userWasCreated": { "name": "test", "activeOrganization": { "__typename": "Organization", "id": "0" }}})).build()).await.unwrap(); insta::assert_json_snapshot!(stream.next_response().await.unwrap()); @@ -1710,7 +1707,8 @@ mod tests { .unwrap(); let mut stream = service.oneshot(request).await.unwrap(); - let _res = stream.next_response().await.unwrap(); + let res = stream.next_response().await.unwrap(); + assert_eq!(res.errors, []); let res = stream.next_response().await.unwrap(); assert_eq!( res.incremental diff --git a/apollo-router/src/spec/field_type.rs b/apollo-router/src/spec/field_type.rs index 529571126a..90172facfd 100644 --- a/apollo-router/src/spec/field_type.rs +++ b/apollo-router/src/spec/field_type.rs @@ -1,4 +1,4 @@ -use apollo_compiler::hir; +use apollo_compiler::schema; use serde::Deserialize; use serde::Serialize; @@ -11,9 +11,9 @@ use crate::spec::Schema; pub(crate) struct InvalidValue; #[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub(crate) struct FieldType(pub(crate) hir::Type); +pub(crate) struct FieldType(pub(crate) schema::Type); -// hir::Type does not implement Serialize or Deserialize, +// schema::Type does not implement Serialize or Deserialize, // and seems not to work for recursive types. // Instead have explicit `impl`s that are based on derived impl of purpose-built types. @@ -22,7 +22,7 @@ impl Serialize for FieldType { where S: serde::Serializer, { - struct BorrowedFieldType<'a>(&'a hir::Type); + struct BorrowedFieldType<'a>(&'a schema::Type); impl<'a> Serialize for BorrowedFieldType<'a> { fn serialize(&self, serializer: S) -> Result @@ -31,14 +31,18 @@ impl Serialize for FieldType { { #[derive(Serialize)] enum NestedBorrowed<'a> { - NonNull(BorrowedFieldType<'a>), - List(BorrowedFieldType<'a>), Named(&'a str), + NonNullNamed(&'a str), + List(BorrowedFieldType<'a>), + NonNullList(BorrowedFieldType<'a>), } match &self.0 { - hir::Type::NonNull { ty, .. } => NestedBorrowed::NonNull(BorrowedFieldType(ty)), - hir::Type::List { ty, .. } => NestedBorrowed::List(BorrowedFieldType(ty)), - hir::Type::Named { name, .. } => NestedBorrowed::Named(name), + schema::Type::Named(name) => NestedBorrowed::Named(name), + schema::Type::NonNullNamed(name) => NestedBorrowed::NonNullNamed(name), + schema::Type::List(ty) => NestedBorrowed::List(BorrowedFieldType(ty)), + schema::Type::NonNullList(ty) => { + NestedBorrowed::NonNullList(BorrowedFieldType(ty)) + } } .serialize(serializer) } @@ -55,20 +59,18 @@ impl<'de> Deserialize<'de> for FieldType { { #[derive(Deserialize)] enum WithoutLocation { - NonNull(FieldType), - List(FieldType), Named(String), + NonNullNamed(String), + List(FieldType), + NonNullList(FieldType), } WithoutLocation::deserialize(deserializer).map(|ty| match ty { - WithoutLocation::NonNull(ty) => FieldType(hir::Type::NonNull { - ty: Box::new(ty.0), - loc: None, - }), - WithoutLocation::List(ty) => FieldType(hir::Type::List { - ty: Box::new(ty.0), - loc: None, - }), - WithoutLocation::Named(name) => FieldType(hir::Type::Named { name, loc: None }), + WithoutLocation::Named(name) => FieldType(schema::Type::new_named(&name)), + WithoutLocation::NonNullNamed(name) => { + FieldType(schema::Type::new_named(&name).non_null()) + } + WithoutLocation::List(ty) => FieldType(ty.0.list()), + WithoutLocation::NonNullList(ty) => FieldType(ty.0.list().non_null()), }) } } @@ -80,114 +82,83 @@ impl std::fmt::Display for FieldType { } fn validate_input_value( - ty: &hir::Type, + ty: &schema::Type, value: &Value, schema: &Schema, ) -> Result<(), InvalidValue> { - match (ty, value) { - (hir::Type::Named { name, .. }, Value::String(_)) if name == "String" => Ok(()), - // Spec: https://spec.graphql.org/June2018/#sec-Int - (hir::Type::Named { name, .. }, maybe_int) if name == "Int" => { - if maybe_int == &Value::Null || maybe_int.is_valid_int_input() { - Ok(()) + if value.is_null() { + return match ty { + schema::Type::Named(_) | schema::Type::List(_) => Ok(()), + schema::Type::NonNullNamed(_) | schema::Type::NonNullList(_) => Err(InvalidValue), + }; + } + let type_name = match ty { + schema::Type::Named(name) | schema::Type::NonNullNamed(name) => name, + schema::Type::List(inner_type) | schema::Type::NonNullList(inner_type) => { + return if let Value::Array(vec) = value { + vec.iter() + .try_for_each(|x| validate_input_value(inner_type, x, schema)) } else { - Err(InvalidValue) - } + // For coercion from single value to list + validate_input_value(inner_type, value, schema) + }; } + }; + let from_bool = |condition| if condition { Ok(()) } else { Err(InvalidValue) }; + match type_name.as_str() { + "String" => return from_bool(value.is_string()), + // Spec: https://spec.graphql.org/June2018/#sec-Int + "Int" => return from_bool(value.is_valid_int_input()), // Spec: https://spec.graphql.org/draft/#sec-Float.Input-Coercion - (hir::Type::Named { name, .. }, maybe_float) if name == "Float" => { - if maybe_float == &Value::Null || maybe_float.is_valid_float_input() { - Ok(()) - } else { - Err(InvalidValue) - } - } + "Float" => return from_bool(value.is_valid_float_input()), // "The ID scalar type represents a unique identifier, often used to refetch an object // or as the key for a cache. The ID type is serialized in the same way as a String; // however, it is not intended to be human-readable. While it is often numeric, it // should always serialize as a String." // // In practice it seems Int works too - (hir::Type::Named { name, .. }, Value::String(_)) if name == "ID" => Ok(()), - (hir::Type::Named { name, .. }, value) if name == "ID" => { - if value == &Value::Null || value.is_valid_id_input() { - Ok(()) - } else { - Err(InvalidValue) - } - } - (hir::Type::Named { name, .. }, Value::Bool(_)) if name == "Boolean" => Ok(()), - (hir::Type::List { ty: inner_ty, .. }, Value::Array(vec)) => vec - .iter() - .try_for_each(|x| validate_input_value(inner_ty, x, schema)), - // For coercion from single value to list - (hir::Type::List { ty: inner_ty, .. }, val) if val != &Value::Null => { - validate_input_value(inner_ty, val, schema) - } - (hir::Type::NonNull { ty: inner_ty, .. }, value) => { - if value.is_null() { - Err(InvalidValue) - } else { - validate_input_value(inner_ty, value, schema) - } - } - (hir::Type::Named { name, .. }, _) - if schema - .type_system - .definitions - .scalars - .get(name) - .map(|def| !def.is_built_in()) - .unwrap_or(false) - || schema.type_system.definitions.enums.contains_key(name) => - { - Ok(()) - } - // NOTE: graphql's types are all optional by default - (_, Value::Null) => Ok(()), - (hir::Type::Named { name, .. }, value) => { - if let Some(value) = value.as_object() { - if let Some(object_ty) = schema.type_system.definitions.input_objects.get(name) { - object_ty.fields().try_for_each(|field| { - let default: Value; - let value = match value.get(field.name()) { - Some(&Value::Null) | None => { - default = field - .default_value() - .and_then(parse_hir_value) - .unwrap_or(Value::Null); - &default - } - Some(value) => value, - }; - validate_input_value(field.ty(), value, schema) - }) - } else { - Err(InvalidValue) - } - } else { - Err(InvalidValue) - } + "ID" => return from_bool(value.is_valid_id_input()), + "Boolean" => return from_bool(value.is_boolean()), + _ => {} + } + let type_def = schema + .definitions + .types + .get(type_name) + .ok_or(InvalidValue)?; + match (type_def, value) { + // Custom scalar: accept any JSON value + (schema::ExtendedType::Scalar(_), _) => Ok(()), + + // TODO: check enum value? + // (schema::ExtendedType::Enum(def), Value::String(s)) => { + // from_bool(def.values.contains_key(s)) + // }, + (schema::ExtendedType::Enum(_), _) => Ok(()), + + (schema::ExtendedType::InputObject(def), Value::Object(obj)) => { + // TODO: check keys in `obj` but not in `def.fields`? + def.fields + .values() + .try_for_each(|field| match obj.get(field.name.as_str()) { + Some(&Value::Null) | None => { + let default = field + .default_value + .as_ref() + .and_then(|v| parse_hir_value(v)) + .unwrap_or(Value::Null); + validate_input_value(&field.ty, &default, schema) + } + Some(value) => validate_input_value(&field.ty, value, schema), + }) } _ => Err(InvalidValue), } } -/// TODO: make `hir::Type::name` return &str instead of String -fn hir_type_name(ty: &hir::Type) -> &str { - match ty { - hir::Type::NonNull { ty, .. } => hir_type_name(ty), - hir::Type::List { ty, .. } => hir_type_name(ty), - hir::Type::Named { name, .. } => name, - } -} - impl FieldType { - pub(crate) fn new_named(name: impl Into) -> Self { - Self(hir::Type::Named { - name: name.into(), - loc: None, - }) + pub(crate) fn new_named(name: &str) -> Self { + Self(schema::Type::new_named(name)) } // This function validates input values according to the graphql specification. @@ -200,37 +171,13 @@ impl FieldType { validate_input_value(&self.0, value, schema) } - /// return the name of the type on which selections happen - /// - /// Example if we get the field `list: [User!]!`, it will return "User" - pub(crate) fn inner_type_name(&self) -> Option<&str> { - let name = hir_type_name(&self.0); - if is_built_in(name) { - None // TODO: is this case important or could this method return unconditional `&str`? - } else { - Some(name) - } - } - - pub(crate) fn is_builtin_scalar(&self) -> bool { - match &self.0 { - hir::Type::NonNull { .. } => false, - hir::Type::List { .. } => false, - hir::Type::Named { name, .. } => is_built_in(name), - } - } - pub(crate) fn is_non_null(&self) -> bool { self.0.is_non_null() } } -fn is_built_in(name: &str) -> bool { - matches!(name, "String" | "Int" | "Float" | "ID" | "Boolean") -} - -impl From<&'_ hir::Type> for FieldType { - fn from(ty: &'_ hir::Type) -> Self { +impl From<&'_ schema::Type> for FieldType { + fn from(ty: &'_ schema::Type) -> Self { Self(ty.clone()) } } @@ -238,16 +185,7 @@ impl From<&'_ hir::Type> for FieldType { /// Make sure custom Serialize and Deserialize impls are compatible with each other #[test] fn test_field_type_serialization() { - let ty = FieldType(hir::Type::NonNull { - ty: Box::new(hir::Type::List { - ty: Box::new(hir::Type::Named { - name: "ID".into(), - loc: None, - }), - loc: None, - }), - loc: None, - }); + let ty = FieldType(schema::Type::new_named("ID").list().non_null()); assert_eq!( serde_json::from_str::(&serde_json::to_string(&ty).unwrap()).unwrap(), ty diff --git a/apollo-router/src/spec/fragments.rs b/apollo-router/src/spec/fragments.rs index 4c426a4e09..2b0f5004cc 100644 --- a/apollo-router/src/spec/fragments.rs +++ b/apollo-router/src/spec/fragments.rs @@ -1,12 +1,10 @@ use std::collections::HashMap; -use apollo_compiler::ApolloCompiler; -use apollo_compiler::HirDatabase; +use apollo_compiler::ExecutableDocument; use serde::Deserialize; use serde::Serialize; use crate::spec::query::DeferStats; -use crate::spec::FieldType; use crate::spec::Schema; use crate::spec::Selection; use crate::spec::SpecError; @@ -18,30 +16,28 @@ pub(crate) struct Fragments { impl Fragments { pub(crate) fn from_hir( - compiler: &ApolloCompiler, + doc: &ExecutableDocument, schema: &Schema, defer_stats: &mut DeferStats, ) -> Result { - let map = compiler - .db - .all_fragments() + let map = doc + .fragments .iter() .map(|(name, fragment)| { - let type_condition = fragment.type_condition().to_owned(); - let current_type = FieldType::new_named(type_condition.clone()); + let type_condition = fragment.type_condition(); let fragment = Fragment { - type_condition, + type_condition: type_condition.as_str().to_owned(), selection_set: fragment - .selection_set() - .selection() + .selection_set + .selections .iter() .filter_map(|selection| { - Selection::from_hir(selection, ¤t_type, schema, 0, defer_stats) + Selection::from_hir(selection, type_condition, schema, 0, defer_stats) .transpose() }) .collect::, _>>()?, }; - Ok((name.clone(), fragment)) + Ok((name.as_str().to_owned(), fragment)) }) .collect::>()?; Ok(Fragments { map }) diff --git a/apollo-router/src/spec/operation_limits.rs b/apollo-router/src/spec/operation_limits.rs index c6815382a9..be369abc2b 100644 --- a/apollo-router/src/spec/operation_limits.rs +++ b/apollo-router/src/spec/operation_limits.rs @@ -1,11 +1,8 @@ use std::collections::HashMap; use std::collections::HashSet; -use apollo_compiler::hir; -use apollo_compiler::ApolloCompiler; -use apollo_compiler::FileId; -use apollo_compiler::HirDatabase; -use apollo_compiler::InputDatabase; +use apollo_compiler::executable; +use apollo_compiler::ExecutableDocument; use serde::Deserialize; use serde::Serialize; @@ -61,8 +58,8 @@ impl OperationLimits { pub(crate) fn check( configuration: &Configuration, query: &str, - compiler: &ApolloCompiler, - operation_name: Option, + document: &ExecutableDocument, + operation_name: Option<&str>, ) -> Result<(), OperationLimits> { let config_limits = &configuration.limits; let max = OperationLimits { @@ -76,12 +73,7 @@ pub(crate) fn check( return Ok(()); } - let ids = compiler.db.executable_definition_files(); - // We create a new compiler for each query - debug_assert_eq!(ids.len(), 1); - let query_id = ids[0]; - - let Some(operation) = compiler.db.find_operation(query_id, operation_name.clone()) else { + let Ok(operation) = document.get_operation(operation_name) else { // Undefined or ambiguous operation name. // The request is invalid and will be rejected by some other part of the router, // if it wasn’t already before we got to this code path. @@ -89,12 +81,7 @@ pub(crate) fn check( }; let mut fragment_cache = HashMap::new(); - let measured = count( - compiler, - query_id, - &mut fragment_cache, - operation.selection_set(), - ); + let measured = count(document, &mut fragment_cache, &operation.selection_set); let exceeded = max.combine(measured, |_, config, measured| { if let Some(limit) = config { measured > limit @@ -129,11 +116,10 @@ enum Computation { } /// Recursively measure the given selection set against each limit -fn count( - compiler: &ApolloCompiler, - query_id: FileId, - fragment_cache: &mut HashMap>>, - selection_set: &hir::SelectionSet, +fn count<'a>( + document: &'a executable::ExecutableDocument, + fragment_cache: &mut HashMap<&'a executable::Name, Computation>>, + selection_set: &'a executable::SelectionSet, ) -> OperationLimits { let mut counts = OperationLimits { depth: 0, @@ -142,21 +128,21 @@ fn count( aliases: 0, }; let mut fields_seen = HashSet::new(); - for selection in selection_set.selection() { + for selection in &selection_set.selections { match selection { - hir::Selection::Field(field) => { - let nested = count(compiler, query_id, fragment_cache, field.selection_set()); + executable::Selection::Field(field) => { + let nested = count(document, fragment_cache, &field.selection_set); counts.depth = counts.depth.max(1 + nested.depth); counts.height += nested.height; counts.aliases += nested.aliases; // Multiple aliases for the same field could use different arguments // Until we do full merging for limit checking purpose, // approximate measured height with an upper bound rather than a lower bound. - let used_name = if let Some(alias) = field.alias() { + let used_name = if let Some(alias) = &field.alias { counts.aliases += 1; - alias.name() + alias } else { - field.name() + &field.name }; let not_seen_before = fields_seen.insert(used_name); if not_seen_before { @@ -164,28 +150,21 @@ fn count( counts.root_fields += 1; } } - hir::Selection::InlineFragment(fragment) => { - let nested = count(compiler, query_id, fragment_cache, fragment.selection_set()); + executable::Selection::InlineFragment(fragment) => { + let nested = count(document, fragment_cache, &fragment.selection_set); counts.depth = counts.depth.max(nested.depth); counts.height += nested.height; counts.aliases += nested.aliases; } - hir::Selection::FragmentSpread(fragment) => { - let name = fragment.name(); + executable::Selection::FragmentSpread(fragment) => { + let name = &fragment.fragment_name; let nested; match fragment_cache.get(name) { None => { - if let Some(definition) = - compiler.db.find_fragment_by_name(query_id, name.to_owned()) - { - fragment_cache.insert(name.to_owned(), Computation::InProgress); - nested = count( - compiler, - query_id, - fragment_cache, - definition.selection_set(), - ); - fragment_cache.insert(name.to_owned(), Computation::Done(nested)); + if let Some(definition) = document.fragments.get(name) { + fragment_cache.insert(name, Computation::InProgress); + nested = count(document, fragment_cache, &definition.selection_set); + fragment_cache.insert(name, Computation::Done(nested)); } else { // Undefined fragment. The operation invalid // and will be rejected by some other part of the router, diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index da5b66d4c3..0151b925ea 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -7,12 +7,10 @@ use std::collections::HashMap; use std::collections::HashSet; use std::sync::Arc; -use apollo_compiler::hir; -use apollo_compiler::validation::ValidationDatabase; -use apollo_compiler::ApolloCompiler; -use apollo_compiler::AstDatabase; -use apollo_compiler::FileId; -use apollo_compiler::HirDatabase; +use apollo_compiler::ast; +use apollo_compiler::executable; +use apollo_compiler::schema::ExtendedType; +use apollo_compiler::ExecutableDocument; use derivative::Derivative; use indexmap::IndexSet; use serde::Deserialize; @@ -33,6 +31,8 @@ use crate::json_ext::Path; use crate::json_ext::ResponsePathElement; use crate::json_ext::Value; use crate::query_planner::fetch::OperationKind; +use crate::services::layers::query_analysis::ParsedDocument; +use crate::services::layers::query_analysis::ParsedDocumentInner; use crate::spec::FieldType; use crate::spec::Fragments; use crate::spec::InvalidValue; @@ -46,7 +46,6 @@ pub(crate) mod transform; pub(crate) mod traverse; pub(crate) const TYPENAME: &str = "__typename"; -pub(crate) const QUERY_EXECUTABLE: &str = "query"; /// A GraphQL query. #[derive(Derivative, Serialize, Deserialize)] @@ -261,17 +260,22 @@ impl Query { vec![] } - pub(crate) fn make_compiler( + pub(crate) fn parse_document( query: &str, schema: &Schema, configuration: &Configuration, - ) -> (ApolloCompiler, FileId) { - let mut compiler = ApolloCompiler::new() + ) -> ParsedDocument { + let parser = &mut apollo_compiler::Parser::new() .recursion_limit(configuration.limits.parser_max_recursion) .token_limit(configuration.limits.parser_max_tokens); - compiler.set_type_system_hir(schema.type_system.clone()); - let id = compiler.add_executable(query, QUERY_EXECUTABLE); - (compiler, id) + let ast = parser.parse_ast(query, "query.graphql"); + let executable = ast.to_executable(&schema.api_schema().definitions); + + // Trace log recursion limit data + let recursion_limit = parser.recursion_reached(); + tracing::trace!(?recursion_limit, "recursion limit data"); + + Arc::new(ParsedDocumentInner { ast, executable }) } #[cfg(test)] @@ -282,10 +286,10 @@ impl Query { ) -> Result { let query = query.into(); - let (compiler, id) = Self::make_compiler(&query, schema, configuration); - Self::check_errors(&compiler, id)?; + let doc = Self::parse_document(&query, schema, configuration); + Self::check_errors(&doc.ast)?; let (fragments, operations, defer_stats) = - Self::extract_query_information(&compiler, schema)?; + Self::extract_query_information(schema, &doc.executable)?; Ok(Query { string: query, @@ -301,59 +305,35 @@ impl Query { } /// Check for parse errors in a query in the compiler. - pub(crate) fn check_errors(compiler: &ApolloCompiler, id: FileId) -> Result<(), SpecError> { - let ast = compiler.db.ast(id); - // Trace log recursion limit data - let recursion_limit = ast.recursion_limit(); - tracing::trace!(?recursion_limit, "recursion limit data"); - - let mut parse_errors = ast.errors().peekable(); - if parse_errors.peek().is_some() { - let text = parse_errors - .map(|err| err.to_string()) - .collect::>() - .join("\n"); - failfast_debug!("parsing error(s): {}", text); - return Err(SpecError::ParsingError(text)); - } - - Ok(()) + pub(crate) fn check_errors(document: &ast::Document) -> Result<(), SpecError> { + document + .check_parse_errors() + .map_err(|errors| SpecError::ParsingError(errors.to_string_no_color())) } /// Check for validation errors in a query in the compiler. pub(crate) fn validate_query( - compiler: &ApolloCompiler, - id: FileId, + schema: &Schema, + document: &ExecutableDocument, ) -> Result<(), ValidationErrors> { - // Bail out on validation errors, only if the input is expected to be valid - let diagnostics = compiler.db.validate_executable(id); - let errors = diagnostics - .into_iter() - .filter(|err| err.data.is_error()) - .collect::>(); - - if errors.is_empty() { - return Ok(()); - } - - Err(ValidationErrors { errors }) + document + .validate(&schema.api_schema().definitions) + .map_err(|errors| ValidationErrors { errors }) } /// Extract serializable data structures from the apollo-compiler HIR. pub(crate) fn extract_query_information( - compiler: &ApolloCompiler, schema: &Schema, + document: &ExecutableDocument, ) -> Result<(Fragments, Vec, DeferStats), SpecError> { let mut defer_stats = DeferStats { has_defer: false, has_unconditional_defer: false, conditional_defer_variable_names: IndexSet::new(), }; - let fragments = Fragments::from_hir(compiler, schema, &mut defer_stats)?; - let operations = compiler - .db + let fragments = Fragments::from_hir(document, schema, &mut defer_stats)?; + let operations = document .all_operations() - .iter() .map(|operation| Operation::from_hir(operation, schema, &mut defer_stats)) .collect::, SpecError>>()?; @@ -364,11 +344,11 @@ impl Query { fn format_value<'a: 'b, 'b>( &'a self, parameters: &mut FormatParameters, - field_type: &hir::Type, + field_type: &executable::Type, input: &mut Value, output: &mut Value, path: &mut Vec>, - parent_type: &hir::Type, + parent_type: &executable::Type, selection_set: &'a [Selection], ) -> Result<(), InvalidValue> { // for every type, if we have an invalid value, we will replace it with null @@ -377,10 +357,15 @@ impl Query { // for non null types, we validate with the inner type, then if we get an InvalidValue // we set it to null and immediately return an error instead of Ok(()), because we // want the error to go up until the next nullable parent - hir::Type::NonNull { ty: inner_type, .. } => { + executable::Type::NonNullNamed(_) | executable::Type::NonNullList(_) => { + let inner_type = match field_type { + executable::Type::NonNullList(ty) => ty.clone().list(), + executable::Type::NonNullNamed(name) => executable::Type::Named(name.clone()), + _ => unreachable!(), + }; match self.format_value( parameters, - inner_type, + &inner_type, input, output, path, @@ -417,7 +402,7 @@ impl Query { // and should replace the entire list with null // if the types are nullable, the inner call to filter_errors will take care // of setting the current entry to null - hir::Type::List { ty: inner_type, .. } => match input { + executable::Type::List(inner_type) => match input { Value::Array(input_array) => { if output.is_null() { *output = Value::Array( @@ -454,7 +439,7 @@ impl Query { } _ => Ok(()), }, - hir::Type::Named { name, .. } if name == "Int" => { + executable::Type::Named(name) if name == "Int" => { let opt = if input.is_i64() { input.as_i64().and_then(|i| i32::try_from(i).ok()) } else if input.is_u64() { @@ -472,7 +457,7 @@ impl Query { } Ok(()) } - hir::Type::Named { name, .. } if name == "Float" => { + executable::Type::Named(name) if name == "Float" => { if input.as_f64().is_some() { *output = input.clone(); } else { @@ -480,7 +465,7 @@ impl Query { } Ok(()) } - hir::Type::Named { name, .. } if name == "Boolean" => { + executable::Type::Named(name) if name == "Boolean" => { if input.as_bool().is_some() { *output = input.clone(); } else { @@ -488,7 +473,7 @@ impl Query { } Ok(()) } - hir::Type::Named { name, .. } if name == "String" => { + executable::Type::Named(name) if name == "String" => { if input.as_str().is_some() { *output = input.clone(); } else { @@ -496,7 +481,7 @@ impl Query { } Ok(()) } - hir::Type::Named { name, .. } if name == "Id" => { + executable::Type::Named(name) if name == "Id" => { if input.is_string() || input.is_i64() || input.is_u64() || input.is_f64() { *output = input.clone(); } else { @@ -504,31 +489,32 @@ impl Query { } Ok(()) } - hir::Type::Named { - name: type_name, .. - } => { - let definitions = ¶meters.schema.type_system.definitions; + executable::Type::Named(type_name) => { // we cannot know about the expected format of custom scalars // so we must pass them directly to the client - if definitions.scalars.contains_key(type_name) { - *output = input.clone(); - return Ok(()); - } else if let Some(enum_type) = definitions.enums.get(type_name) { - return match input.as_str() { - Some(s) => { - if enum_type.value(s).is_some() { - *output = input.clone(); - Ok(()) - } else { + match parameters.schema.definitions.types.get(type_name) { + Some(ExtendedType::Scalar(_)) => { + *output = input.clone(); + return Ok(()); + } + Some(ExtendedType::Enum(enum_type)) => { + return match input.as_str() { + Some(s) => { + if enum_type.values.contains_key(s) { + *output = input.clone(); + Ok(()) + } else { + *output = Value::Null; + Ok(()) + } + } + None => { *output = Value::Null; Ok(()) } - } - None => { - *output = Value::Null; - Ok(()) - } - }; + }; + } + _ => {} } match input { @@ -536,18 +522,17 @@ impl Query { if let Some(input_type) = input_object.get(TYPENAME).and_then(|val| val.as_str()) { - let definitions = ¶meters.schema.type_system.definitions; // If there is a __typename, make sure the pointed type is a valid type of the schema. Otherwise, something is wrong, and in case we might // be inadvertently leaking some data for an @inacessible type or something, nullify the whole object. However, do note that due to `@interfaceObject`, // some subgraph can have returned a __typename that is the name of an interface in the supergraph, and this is fine (that is, we should not // return such a __typename to the user, but as long as it's not returned, having it in the internal data is ok and sometimes expected). - if !definitions.objects.contains_key(input_type) - && !definitions.interfaces.contains_key(input_type) - { + let Some(ExtendedType::Object(_) | ExtendedType::Interface(_)) = + parameters.schema.definitions.types.get(input_type) + else { parameters.nullified.push(Path::from_response_slice(path)); *output = Value::Null; return Ok(()); - } + }; } if output.is_null() { @@ -589,7 +574,7 @@ impl Query { input: &mut Object, output: &mut Object, path: &mut Vec>, - parent_type: &hir::Type, + parent_type: &executable::Type, ) -> Result<(), InvalidValue> { // For skip and include, using .unwrap_or is legit here because // validate_variables should have already checked that @@ -626,10 +611,9 @@ impl Query { if let Some(input_str) = input_value.as_str() { if parameters .schema - .type_system .definitions - .objects - .contains_key(input_str) + .get_object(input_str) + .is_some() { *output_value = input_value.clone(); } else { @@ -1096,31 +1080,32 @@ pub(crate) struct Variable { impl Operation { pub(crate) fn from_hir( - operation: &hir::OperationDefinition, + operation: executable::OperationRef<'_>, schema: &Schema, defer_stats: &mut DeferStats, ) -> Result { - let name = operation.name().map(|s| s.to_owned()); - let kind = operation.operation_ty().into(); + let name = operation.name().map(|s| s.as_str().to_owned()); + let kind = operation.operation_type.into(); let type_name = schema.root_operation_name(kind).to_owned(); - let current_field_type = FieldType::new_named(&type_name); let selection_set = operation - .selection_set() - .selection() + .selection_set + .selections .iter() .filter_map(|selection| { - Selection::from_hir(selection, ¤t_field_type, schema, 0, defer_stats) - .transpose() + Selection::from_hir(selection, &type_name, schema, 0, defer_stats).transpose() }) .collect::>()?; let variables = operation - .variables() + .variables .iter() .map(|variable| { - let name = variable.name().into(); + let name = variable.name.as_str().into(); let variable = Variable { - field_type: variable.ty().into(), - default_value: variable.default_value().and_then(parse_hir_value), + field_type: variable.ty.as_ref().into(), + default_value: variable + .default_value + .as_ref() + .and_then(|v| parse_hir_value(v)), }; Ok((name, variable)) }) @@ -1176,29 +1161,19 @@ impl Operation { } } -impl From for OperationKind { - fn from(operation_type: hir::OperationType) -> Self { - match operation_type { - hir::OperationType::Query => Self::Query, - hir::OperationType::Mutation => Self::Mutation, - hir::OperationType::Subscription => Self::Subscription, - } - } -} - -pub(crate) fn parse_hir_value(value: &hir::Value) -> Option { +pub(crate) fn parse_hir_value(value: &executable::Value) -> Option { match value { - hir::Value::Variable(_) => None, - hir::Value::Int { value, .. } => Some((value.get() as i64).into()), - hir::Value::Float { value, .. } => Some(value.get().into()), - hir::Value::Null { .. } => Some(Value::Null), - hir::Value::String { value, .. } => Some(value.as_str().into()), - hir::Value::Boolean { value, .. } => Some((*value).into()), - hir::Value::Enum { value, .. } => Some(value.src().into()), - hir::Value::List { value, .. } => value.iter().map(parse_hir_value).collect(), - hir::Value::Object { value, .. } => value + executable::Value::Variable(_) => None, + executable::Value::Int(value) => Some(value.as_str().parse::().ok()?.into()), + executable::Value::Float(value) => Some(value.try_to_f64().ok()?.into()), + executable::Value::Null => Some(Value::Null), + executable::Value::String(value) => Some(value.as_str().into()), + executable::Value::Boolean(value) => Some((*value).into()), + executable::Value::Enum(value) => Some(value.as_str().into()), + executable::Value::List(value) => value.iter().map(|v| parse_hir_value(v)).collect(), + executable::Value::Object(value) => value .iter() - .map(|(k, v)| Some((k.src(), parse_hir_value(v)?))) + .map(|(k, v)| Some((k.as_str(), parse_hir_value(v)?))) .collect(), } } diff --git a/apollo-router/src/spec/query/subselections.rs b/apollo-router/src/spec/query/subselections.rs index 4bbb16ac45..c2c7038dd6 100644 --- a/apollo-router/src/spec/query/subselections.rs +++ b/apollo-router/src/spec/query/subselections.rs @@ -211,7 +211,7 @@ impl Shared<'_> { alias: None, selection_set: Some(selection_set), field_type: path_type.clone(), - include_skip: IncludeSkip::parse(&[]), + include_skip: IncludeSkip::default(), }]; } selection_set @@ -288,7 +288,7 @@ fn collect_from_selection_set<'a>( }, SubSelectionValue { selection_set: shared.reconstruct_up_to_root(nested), - type_name: fragment_type.0.name(), + type_name: fragment_type.0.inner_named_type().as_str().to_owned(), }, ); } else { diff --git a/apollo-router/src/spec/query/tests.rs b/apollo-router/src/spec/query/tests.rs index e3493baf6d..c63c86c0f0 100644 --- a/apollo-router/src/spec/query/tests.rs +++ b/apollo-router/src/spec/query/tests.rs @@ -522,9 +522,9 @@ fn reformat_response_data_best_effort() { "baz": "2", }, "array": [ - {}, + {"bar": null, "baz": "3"}, null, - {} + {"bar": "5", "baz": null} ], "other": null, }, @@ -3097,13 +3097,14 @@ fn filter_errors_top_level_fragment() { .schema(schema) .query(query) .response(json! {{ + "__typename": "Query", "get": { "name": "a", "other": "b" } }}) .expected(json! {{ - "__typename": null, + "__typename": "Query", "get": { "name": "a", } @@ -3115,10 +3116,11 @@ fn filter_errors_top_level_fragment() { .schema(schema) .query(query) .response(json! {{ + "__typename": "Query", "get": {"name": null, "other": "b"} }}) .expected(json! {{ - "__typename": null, + "__typename": "Query", "get": { "name": null, } @@ -3131,10 +3133,11 @@ fn filter_errors_top_level_fragment() { .schema(schema) .query(query2) .response(json! {{ + "__typename": "Query", "get": {"name2": "a", "other": "b"} }}) .expected(json! {{ - "__typename": null, + "__typename": "Query", "get": { "name2": "a", } @@ -3146,10 +3149,11 @@ fn filter_errors_top_level_fragment() { .schema(schema) .query(query2) .response(json! {{ + "__typename": "Query", "get": {"name2": null, "other": "b"} }}) .expected(json! {{ - "__typename": null, + "__typename": "Query", "get": null }}) .test(); @@ -3160,10 +3164,11 @@ fn filter_errors_top_level_fragment() { .schema(schema) .query(query3) .response(json! {{ + "__typename": "Query", "get": {"name": "a", "other": "b"} }}) .expected(json! {{ - "__typename": null, + "__typename": "Query", "get": { "name": "a", } @@ -3175,10 +3180,11 @@ fn filter_errors_top_level_fragment() { .schema(schema) .query(query3) .response(json! {{ + "__typename": "Query", "get": {"name": null, "other": "b"} }}) .expected(json! {{ - "__typename": null, + "__typename": "Query", "get": { "name": null, } @@ -3191,10 +3197,11 @@ fn filter_errors_top_level_fragment() { .schema(schema) .query(query4) .response(json! {{ + "__typename": "Query", "get": {"name2": "a", "other": "b"} }}) .expected(json! {{ - "__typename": null, + "__typename": "Query", "get": { "name2": "a", } @@ -3206,10 +3213,11 @@ fn filter_errors_top_level_fragment() { .schema(schema) .query(query4) .response(json! {{ + "__typename": "Query", "get": {"name2": null, "other": "b"} }}) .expected(json! {{ - "__typename": null, + "__typename": "Query", "get": null, }}) .test(); @@ -3322,12 +3330,15 @@ fn it_parses_default_floats() { ); let schema = Schema::parse_test(&schema, &Default::default()).unwrap(); - let value = schema.type_system.definitions.input_objects["WithAllKindsOfFloats"] - .field("a_float_that_doesnt_fit_an_int") + let value = schema + .definitions + .get_input_object("WithAllKindsOfFloats") .unwrap() - .default_value() + .fields["a_float_that_doesnt_fit_an_int"] + .default_value + .as_ref() .unwrap(); - assert_eq!(f64::try_from(value).unwrap() as i64, 9876543210); + assert_eq!(value.to_f64().unwrap() as i64, 9876543210); } #[test] @@ -5575,7 +5586,7 @@ fn test_query_not_named_query() { matches!( selection, Selection::Field { - field_type: FieldType(hir::Type::Named { name, .. }), + field_type: FieldType(apollo_compiler::executable::Type::Named(name)), .. } if name == "Boolean" @@ -5630,10 +5641,9 @@ fn filtered_defer_fragment() { } }"; - let mut compiler = ApolloCompiler::new(); - compiler.add_executable(query, "query.graphql"); + let doc = ExecutableDocument::parse(&schema.definitions, query, "query.graphql"); let (fragments, operations, defer_stats) = - Query::extract_query_information(&compiler, &schema).unwrap(); + Query::extract_query_information(&schema, &doc).unwrap(); let subselections = crate::spec::query::subselections::collect_subselections( &config, @@ -5654,10 +5664,13 @@ fn filtered_defer_fragment() { validation_error: None, }; - let mut compiler = ApolloCompiler::new(); - compiler.add_executable(filtered_query, "filtered_query.graphql"); + let doc = ExecutableDocument::parse( + &schema.definitions, + filtered_query, + "filtered_query.graphql", + ); let (fragments, operations, defer_stats) = - Query::extract_query_information(&compiler, &schema).unwrap(); + Query::extract_query_information(&schema, &doc).unwrap(); let subselections = crate::spec::query::subselections::collect_subselections( &config, diff --git a/apollo-router/src/spec/query/transform.rs b/apollo-router/src/spec/query/transform.rs index 2a5571fc37..1710e2444b 100644 --- a/apollo-router/src/spec/query/transform.rs +++ b/apollo-router/src/spec/query/transform.rs @@ -1,35 +1,45 @@ -#![cfg_attr(not(test), allow(unused))] // TODO: remove when used +use std::collections::HashMap; -use apollo_compiler::hir; -use apollo_compiler::ApolloCompiler; -use apollo_compiler::HirDatabase; +use apollo_compiler::ast; +use apollo_compiler::schema::FieldLookupError; use tower::BoxError; -/// Transform an executable document with the given visitor. +/// Transform a document with the given visitor. pub(crate) fn document( visitor: &mut impl Visitor, - file_id: apollo_compiler::FileId, -) -> Result { - let mut encoder_node = apollo_encoder::Document::new(); - - for def in visitor.compiler().db.operations(file_id).iter() { - if let Some(op) = visitor.operation(def)? { - encoder_node.operation(op) - } - } - for def in visitor.compiler().db.fragments(file_id).values() { - if let Some(f) = visitor.fragment_definition(def)? { - encoder_node.fragment(f) + document: &ast::Document, +) -> Result { + let mut new = ast::Document { + sources: document.sources.clone(), + definitions: Vec::new(), + }; + for definition in &document.definitions { + match definition { + ast::Definition::OperationDefinition(def) => { + let root_type = visitor + .schema() + .root_operation(def.operation_type) + .ok_or("missing root operation definition")? + .clone(); + if let Some(new_def) = visitor.operation(&root_type, def)? { + new.definitions + .push(ast::Definition::OperationDefinition(new_def.into())) + } + } + ast::Definition::FragmentDefinition(def) => { + if let Some(new_def) = visitor.fragment_definition(def)? { + new.definitions + .push(ast::Definition::FragmentDefinition(new_def.into())) + } + } + _ => {} } } - - Ok(encoder_node) + Ok(new) } pub(crate) trait Visitor: Sized { - /// A compiler that contains both the executable document to transform - /// and the corresponding type system definitions. - fn compiler(&self) -> &ApolloCompiler; + fn schema(&self) -> &apollo_compiler::Schema; /// Transform an operation definition. /// @@ -37,9 +47,10 @@ pub(crate) trait Visitor: Sized { /// Return `Ok(None)` to remove this operation. fn operation( &mut self, - hir: &hir::OperationDefinition, - ) -> Result, BoxError> { - operation(self, hir) + root_type: &str, + def: &ast::OperationDefinition, + ) -> Result, BoxError> { + operation(self, root_type, def) } /// Transform a fragment definition. @@ -48,9 +59,9 @@ pub(crate) trait Visitor: Sized { /// Return `Ok(None)` to remove this fragment. fn fragment_definition( &mut self, - hir: &hir::FragmentDefinition, - ) -> Result, BoxError> { - fragment_definition(self, hir) + def: &ast::FragmentDefinition, + ) -> Result, BoxError> { + fragment_definition(self, def) } /// Transform a field within a selection set. @@ -59,10 +70,11 @@ pub(crate) trait Visitor: Sized { /// Return `Ok(None)` to remove this field. fn field( &mut self, - parent_type: &str, - hir: &hir::Field, - ) -> Result, BoxError> { - field(self, parent_type, hir) + _parent_type: &str, + field_def: &ast::FieldDefinition, + def: &ast::Field, + ) -> Result, BoxError> { + field(self, field_def, def) } /// Transform a fragment spread within a selection set. @@ -71,9 +83,9 @@ pub(crate) trait Visitor: Sized { /// Return `Ok(None)` to remove this fragment spread. fn fragment_spread( &mut self, - hir: &hir::FragmentSpread, - ) -> Result, BoxError> { - fragment_spread(self, hir) + def: &ast::FragmentSpread, + ) -> Result, BoxError> { + fragment_spread(self, def) } /// Transform a inline fragment within a selection set. @@ -83,24 +95,9 @@ pub(crate) trait Visitor: Sized { fn inline_fragment( &mut self, parent_type: &str, - hir: &hir::InlineFragment, - ) -> Result, BoxError> { - inline_fragment(self, parent_type, hir) - } - - /// Transform a selection within a selection set. - /// - /// Call the [`selection`] free function for the default behavior. - /// Return `Ok(None)` to remove this selection from the selection set. - /// - /// Compared to `field`, `fragment_spread`, or `inline_fragment` trait methods, - /// this allows returning a different kind of selection. - fn selection( - &mut self, - hir: &hir::Selection, - parent_type: &str, - ) -> Result, BoxError> { - selection(self, hir, parent_type) + def: &ast::InlineFragment, + ) -> Result, BoxError> { + inline_fragment(self, parent_type, def) } } @@ -109,37 +106,20 @@ pub(crate) trait Visitor: Sized { /// Never returns `Ok(None)`, the `Option` is for `Visitor` impl convenience. pub(crate) fn operation( visitor: &mut impl Visitor, - def: &hir::OperationDefinition, -) -> Result, BoxError> { - let object_type = def - .object_type(&visitor.compiler().db) - .ok_or("ObjectTypeDefMissing")?; - let type_name = object_type.name(); - - let Some(selection_set) = selection_set(visitor, def.selection_set(), type_name)? else { + root_type: &str, + def: &ast::OperationDefinition, +) -> Result, BoxError> { + let Some(selection_set) = selection_set(visitor, root_type, &def.selection_set)? else { return Ok(None); }; - let mut encoder_node = - apollo_encoder::OperationDefinition::new(operation_type(def.operation_ty()), selection_set); - - if let Some(name) = def.name() { - encoder_node.name(Some(name.to_string())); - } - - for def in def.variables() { - if let Some(v) = variable_definition(def)? { - encoder_node.variable_definition(v) - } - } - - for hir in def.directives() { - if let Some(d) = directive(hir)? { - encoder_node.directive(d) - } - } - - Ok(Some(encoder_node)) + Ok(Some(ast::OperationDefinition { + name: def.name.clone(), + operation_type: def.operation_type, + variables: def.variables.clone(), + directives: def.directives.clone(), + selection_set, + })) } /// The default behavior for transforming a fragment definition. @@ -147,25 +127,18 @@ pub(crate) fn operation( /// Never returns `Ok(None)`, the `Option` is for `Visitor` impl convenience. pub(crate) fn fragment_definition( visitor: &mut impl Visitor, - hir: &hir::FragmentDefinition, -) -> Result, BoxError> { - let name = hir.name(); - let type_condition = hir.type_condition(); - - let Some(selection_set) = selection_set(visitor, hir.selection_set(), type_condition)? else { + def: &ast::FragmentDefinition, +) -> Result, BoxError> { + let Some(selection_set) = selection_set(visitor, &def.type_condition, &def.selection_set)? + else { return Ok(None); }; - - let type_condition = apollo_encoder::TypeCondition::new(type_condition.into()); - let mut encoder_node = - apollo_encoder::FragmentDefinition::new(name.into(), type_condition, selection_set); - for hir in hir.directives() { - if let Some(d) = directive(hir)? { - encoder_node.directive(d) - } - } - - Ok(Some(encoder_node)) + Ok(Some(ast::FragmentDefinition { + name: def.name.clone(), + type_condition: def.type_condition.clone(), + directives: def.directives.clone(), + selection_set, + })) } /// The default behavior for transforming a field within a selection set. @@ -173,42 +146,21 @@ pub(crate) fn fragment_definition( /// Returns `Ok(None)` if the field had nested selections and they’re all removed. pub(crate) fn field( visitor: &mut impl Visitor, - parent_type: &str, - hir: &hir::Field, -) -> Result, BoxError> { - let name = hir.name(); - - let mut encoder_node = apollo_encoder::Field::new(name.into()); - - if let Some(alias) = hir.alias() { - encoder_node.alias(Some(alias.name().into())); - } - - for arg in hir.arguments() { - encoder_node.argument(apollo_encoder::Argument::new( - arg.name().into(), - value(arg.value())?, - )); - } - - for hir in hir.directives() { - if let Some(d) = directive(hir)? { - encoder_node.directive(d) - } - } - - let selections = hir.selection_set(); - if !selections.selection().is_empty() { - let field_type = get_field_type(visitor, parent_type, name) - .ok_or_else(|| format!("cannot query field '{name}' on type '{parent_type}'"))?; - match selection_set(visitor, selections, &field_type)? { - // we expected some fields on that object but got none: that field should be removed - None => return Ok(None), - Some(selection_set) => encoder_node.selection_set(Some(selection_set)), - } - } - - Ok(Some(encoder_node)) + field_def: &ast::FieldDefinition, + def: &ast::Field, +) -> Result, BoxError> { + let Some(selection_set) = + selection_set(visitor, field_def.ty.inner_named_type(), &def.selection_set)? + else { + return Ok(None); + }; + Ok(Some(ast::Field { + alias: def.alias.clone(), + name: def.name.clone(), + arguments: def.arguments.clone(), + directives: def.directives.clone(), + selection_set, + })) } /// The default behavior for transforming a fragment spread. @@ -216,17 +168,10 @@ pub(crate) fn field( /// Never returns `Ok(None)`, the `Option` is for `Visitor` impl convenience. pub(crate) fn fragment_spread( visitor: &mut impl Visitor, - hir: &hir::FragmentSpread, -) -> Result, BoxError> { + def: &ast::FragmentSpread, +) -> Result, BoxError> { let _ = visitor; // Unused, but matches trait method signature - let name = hir.name(); - let mut encoder_node = apollo_encoder::FragmentSpread::new(name.into()); - for hir in hir.directives() { - if let Some(d) = directive(hir)? { - encoder_node.directive(d) - } - } - Ok(Some(encoder_node)) + Ok(Some(def.clone())) } /// The default behavior for transforming an inline fragment. @@ -235,182 +180,103 @@ pub(crate) fn fragment_spread( pub(crate) fn inline_fragment( visitor: &mut impl Visitor, parent_type: &str, - hir: &hir::InlineFragment, -) -> Result, BoxError> { - let parent_type = hir.type_condition().unwrap_or(parent_type); - - let Some(selection_set) = selection_set(visitor, hir.selection_set(), parent_type)? else { + def: &ast::InlineFragment, +) -> Result, BoxError> { + let Some(selection_set) = selection_set(visitor, parent_type, &def.selection_set)? else { return Ok(None); }; - - let mut encoder_node = apollo_encoder::InlineFragment::new(selection_set); - - encoder_node.type_condition( - hir.type_condition() - .map(|name| apollo_encoder::TypeCondition::new(name.into())), - ); - - for hir in hir.directives() { - if let Some(d) = directive(hir)? { - encoder_node.directive(d) - } - } - - Ok(Some(encoder_node)) -} - -pub(crate) fn get_field_type( - visitor: &impl Visitor, - parent: &str, - field_name: &str, -) -> Option { - Some(if field_name == "__typename" { - "String".into() - } else { - let db = &visitor.compiler().db; - db.types_definitions_by_name() - .get(parent)? - .field(db, field_name)? - .ty() - .name() - }) + Ok(Some(ast::InlineFragment { + type_condition: def.type_condition.clone(), + directives: def.directives.clone(), + selection_set, + })) } pub(crate) fn selection_set( visitor: &mut impl Visitor, - hir: &hir::SelectionSet, - parent_type: &str, -) -> Result, BoxError> { - let selections = hir - .selection() - .iter() - .filter_map(|hir| visitor.selection(hir, parent_type).transpose()) - .collect::, _>>()?; - Ok((!selections.is_empty()).then(|| apollo_encoder::SelectionSet::with_selections(selections))) -} - -pub(crate) fn selection( - visitor: &mut impl Visitor, - hir: &hir::Selection, parent_type: &str, -) -> Result, BoxError> { - Ok(match hir { - hir::Selection::Field(hir) => visitor - .field(parent_type, hir)? - .map(apollo_encoder::Selection::Field), - hir::Selection::FragmentSpread(hir) => visitor - .fragment_spread(hir)? - .map(apollo_encoder::Selection::FragmentSpread), - hir::Selection::InlineFragment(hir) => visitor - .inline_fragment(parent_type, hir)? - .map(apollo_encoder::Selection::InlineFragment), - }) -} - -pub(crate) fn variable_definition( - hir: &hir::VariableDefinition, -) -> Result, BoxError> { - let name = hir.name(); - let ty = ty(hir.ty()); - - let mut encoder_node = apollo_encoder::VariableDefinition::new(name.into(), ty); - - if let Some(default_value) = hir.default_value() { - encoder_node.default_value(value(default_value)?); + set: &[ast::Selection], +) -> Result>, BoxError> { + if set.is_empty() { + return Ok(Some(Vec::new())); } - - for hir in hir.directives() { - if let Some(d) = directive(hir)? { - encoder_node.directive(d) + let mut selections = Vec::new(); + for sel in set { + match sel { + ast::Selection::Field(def) => { + let field_def = visitor + .schema() + .type_field(parent_type, &def.name) + .map_err(|e| match e { + FieldLookupError::NoSuchType => format!("type `{parent_type}` not defined"), + FieldLookupError::NoSuchField(_, _) => { + format!("no field `{}` in type `{parent_type}`", &def.name) + } + })? + .clone(); + if let Some(sel) = visitor.field(parent_type, &field_def, def)? { + selections.push(ast::Selection::Field(sel.into())) + } + } + ast::Selection::FragmentSpread(def) => { + if let Some(sel) = visitor.fragment_spread(def)? { + selections.push(ast::Selection::FragmentSpread(sel.into())) + } + } + ast::Selection::InlineFragment(def) => { + let fragment_type = def.type_condition.as_deref().unwrap_or(parent_type); + if let Some(sel) = visitor.inline_fragment(fragment_type, def)? { + selections.push(ast::Selection::InlineFragment(sel.into())) + } + } } } - - Ok(Some(encoder_node)) -} - -pub(crate) fn directive( - hir: &hir::Directive, -) -> Result, BoxError> { - let name = hir.name().into(); - let mut encoder_directive = apollo_encoder::Directive::new(name); - - for arg in hir.arguments() { - encoder_directive.arg(apollo_encoder::Argument::new( - arg.name().into(), - value(arg.value())?, - )); - } - - Ok(Some(encoder_directive)) -} - -// FIXME: apollo-rs should provide these three conversions, or unify types - -pub(crate) fn operation_type(hir: hir::OperationType) -> apollo_encoder::OperationType { - match hir { - hir::OperationType::Query => apollo_encoder::OperationType::Query, - hir::OperationType::Mutation => apollo_encoder::OperationType::Mutation, - hir::OperationType::Subscription => apollo_encoder::OperationType::Subscription, - } + Ok((!selections.is_empty()).then_some(selections)) } -pub(crate) fn ty(hir: &hir::Type) -> apollo_encoder::Type_ { - match hir { - hir::Type::NonNull { ty: hir, .. } => apollo_encoder::Type_::NonNull { - ty: Box::new(ty(hir)), - }, - hir::Type::List { ty: hir, .. } => apollo_encoder::Type_::List { - ty: Box::new(ty(hir)), - }, - hir::Type::Named { name, .. } => apollo_encoder::Type_::NamedType { name: name.clone() }, - } -} - -pub(crate) fn value(hir: &hir::Value) -> Result { - Ok(match hir { - hir::Value::Variable(val) => apollo_encoder::Value::Variable(val.name().into()), - hir::Value::Int { value, .. } => value - .to_i32_checked() - .map(apollo_encoder::Value::Int) - .unwrap_or_else(|| apollo_encoder::Value::Float(value.get())), - hir::Value::Float { value, .. } => apollo_encoder::Value::Float(value.get()), - hir::Value::String { value, .. } => apollo_encoder::Value::String(value.clone()), - hir::Value::Boolean { value, .. } => apollo_encoder::Value::Boolean(*value), - hir::Value::Null { .. } => apollo_encoder::Value::Null, - hir::Value::Enum { value, .. } => apollo_encoder::Value::Enum(value.src().into()), - hir::Value::List { value: list, .. } => { - apollo_encoder::Value::List(list.iter().map(value).collect::, _>>()?) - } - hir::Value::Object { value: obj, .. } => apollo_encoder::Value::Object( - obj.iter() - .map(|(k, v)| Ok::<_, BoxError>((k.src().to_string(), value(v)?))) - .collect::, _>>()?, - ), - }) +pub(crate) fn collect_fragments( + executable: &ast::Document, +) -> HashMap<&ast::Name, &ast::FragmentDefinition> { + executable + .definitions + .iter() + .filter_map(|def| match def { + ast::Definition::FragmentDefinition(frag) => Some((&frag.name, frag.as_ref())), + _ => None, + }) + .collect() } #[test] fn test_add_directive_to_fields() { - struct AddDirective<'a>(&'a ApolloCompiler); - - impl<'a> Visitor for AddDirective<'a> { - fn compiler(&self) -> &ApolloCompiler { - self.0 - } + struct AddDirective { + schema: apollo_compiler::Schema, + } + impl Visitor for AddDirective { fn field( &mut self, - parent_type: &str, - hir: &hir::Field, - ) -> Result, BoxError> { - Ok(field(self, parent_type, hir)?.map(|mut encoder_node| { - encoder_node.directive(apollo_encoder::Directive::new("added".into())); - encoder_node + _parent_type: &str, + field_def: &ast::FieldDefinition, + def: &ast::Field, + ) -> Result, BoxError> { + Ok(field(self, field_def, def)?.map(|mut new| { + new.directives.push( + ast::Directive { + name: "added".into(), + arguments: Vec::new(), + } + .into(), + ); + new })) } + + fn schema(&self) -> &apollo_compiler::Schema { + &self.schema + } } - let mut compiler = ApolloCompiler::new(); let graphql = " type Query { a: String @@ -432,8 +298,9 @@ fn test_add_directive_to_fields() { } } "; - let file_id = compiler.add_document(graphql, ""); - let mut visitor = AddDirective(&compiler); + let ast = apollo_compiler::ast::Document::parse(graphql, ""); + let (schema, _doc) = ast.to_mixed(); + let mut visitor = AddDirective { schema }; let expected = "query($id: ID = null) { a @added ... @defer { @@ -441,14 +308,12 @@ fn test_add_directive_to_fields() { } ...F } + fragment F on Query { next @added { a @added } } "; - assert_eq!( - document(&mut visitor, file_id).unwrap().to_string(), - expected - ) + assert_eq!(document(&mut visitor, &ast).unwrap().to_string(), expected) } diff --git a/apollo-router/src/spec/query/traverse.rs b/apollo-router/src/spec/query/traverse.rs index 46902020c9..2a8760cd1e 100644 --- a/apollo-router/src/spec/query/traverse.rs +++ b/apollo-router/src/spec/query/traverse.rs @@ -1,198 +1,188 @@ -#![cfg_attr(not(test), allow(unused))] // TODO: remove when used - -use apollo_compiler::hir; -use apollo_compiler::ApolloCompiler; -use apollo_compiler::HirDatabase; +use apollo_compiler::ast; +use apollo_compiler::schema::FieldLookupError; use tower::BoxError; -/// Transform an executable document with the given visitor. +/// Traverse a document with the given visitor. pub(crate) fn document( visitor: &mut impl Visitor, - file_id: apollo_compiler::FileId, + document: &ast::Document, ) -> Result<(), BoxError> { - for def in visitor.compiler().db.operations(file_id).iter() { - visitor.operation(def)?; - } - for def in visitor.compiler().db.fragments(file_id).values() { - visitor.fragment_definition(def)?; - } - Ok(()) + 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(()), + }) } pub(crate) trait Visitor: Sized { - /// A compiler that contains both the executable document to transform - /// and the corresponding type system definitions. - fn compiler(&self) -> &ApolloCompiler; + fn schema(&self) -> &apollo_compiler::Schema; - /// Transform an operation definition. + /// Traverse an operation definition. /// /// Call the [`operation`] free function for the default behavior. /// Return `Ok(None)` to remove this operation. - fn operation(&mut self, hir: &hir::OperationDefinition) -> Result<(), BoxError> { - operation(self, hir) + fn operation( + &mut self, + root_type: &str, + def: &ast::OperationDefinition, + ) -> Result<(), BoxError> { + operation(self, root_type, def) } - /// Transform a fragment definition. + /// Traverse a fragment definition. /// /// Call the [`fragment_definition`] free function for the default behavior. /// Return `Ok(None)` to remove this fragment. - fn fragment_definition(&mut self, hir: &hir::FragmentDefinition) -> Result<(), BoxError> { - fragment_definition(self, hir) + fn fragment_definition(&mut self, def: &ast::FragmentDefinition) -> Result<(), BoxError> { + fragment_definition(self, def) } - /// Transform a field within a selection set. + /// Traverse a field within a selection set. /// /// Call the [`field`] free function for the default behavior. /// Return `Ok(None)` to remove this field. - fn field(&mut self, parent_type: &str, hir: &hir::Field) -> Result<(), BoxError> { - field(self, parent_type, hir) + fn field( + &mut self, + _parent_type: &str, + field_def: &ast::FieldDefinition, + def: &ast::Field, + ) -> Result<(), BoxError> { + field(self, field_def, def) } - /// Transform a fragment spread within a selection set. + /// Traverse a fragment spread within a selection set. /// /// Call the [`fragment_spread`] free function for the default behavior. /// Return `Ok(None)` to remove this fragment spread. - fn fragment_spread(&mut self, hir: &hir::FragmentSpread) -> Result<(), BoxError> { - fragment_spread(self, hir) + fn fragment_spread(&mut self, def: &ast::FragmentSpread) -> Result<(), BoxError> { + fragment_spread(self, def) } - /// Transform a inline fragment within a selection set. + /// Traverse a inline fragment within a selection set. /// /// Call the [`inline_fragment`] free function for the default behavior. /// Return `Ok(None)` to remove this inline fragment. fn inline_fragment( &mut self, parent_type: &str, - hir: &hir::InlineFragment, + def: &ast::InlineFragment, ) -> Result<(), BoxError> { - inline_fragment(self, parent_type, hir) + inline_fragment(self, parent_type, def) } } -/// The default behavior for transforming an operation. +/// The default behavior for traversing an operation. /// /// Never returns `Ok(None)`, the `Option` is for `Visitor` impl convenience. pub(crate) fn operation( visitor: &mut impl Visitor, - def: &hir::OperationDefinition, + root_type: &str, + def: &ast::OperationDefinition, ) -> Result<(), BoxError> { - let object_type = def - .object_type(&visitor.compiler().db) - .ok_or("ObjectTypeDefMissing")?; - let type_name = object_type.name(); - selection_set(visitor, def.selection_set(), type_name)?; - Ok(()) + selection_set(visitor, root_type, &def.selection_set) } -/// The default behavior for transforming a fragment definition. +/// The default behavior for traversing a fragment definition. /// /// Never returns `Ok(None)`, the `Option` is for `Visitor` impl convenience. pub(crate) fn fragment_definition( visitor: &mut impl Visitor, - hir: &hir::FragmentDefinition, + def: &ast::FragmentDefinition, ) -> Result<(), BoxError> { - let type_condition = hir.type_condition(); - selection_set(visitor, hir.selection_set(), type_condition)?; + selection_set(visitor, &def.type_condition, &def.selection_set)?; Ok(()) } -/// The default behavior for transforming a field within a selection set. +/// The default behavior for traversing a field within a selection set. /// /// Returns `Ok(None)` if the field had nested selections and they’re all removed. pub(crate) fn field( visitor: &mut impl Visitor, - parent_type: &str, - hir: &hir::Field, + field_def: &ast::FieldDefinition, + def: &ast::Field, ) -> Result<(), BoxError> { - let name = hir.name(); - let selections = hir.selection_set(); - if !selections.selection().is_empty() { - let field_type = get_field_type(visitor, parent_type, name) - .ok_or_else(|| format!("cannot query field '{name}' on type '{parent_type}'"))?; - selection_set(visitor, selections, &field_type)? - } - Ok(()) + selection_set(visitor, field_def.ty.inner_named_type(), &def.selection_set) } -/// The default behavior for transforming a fragment spread. +/// The default behavior for traversing a fragment spread. /// /// Never returns `Ok(None)`, the `Option` is for `Visitor` impl convenience. pub(crate) fn fragment_spread( visitor: &mut impl Visitor, - hir: &hir::FragmentSpread, + def: &ast::FragmentSpread, ) -> Result<(), BoxError> { - let _ = (visitor, hir); // Unused, but matches trait method signature + let _ = (visitor, def); // Unused, but matches trait method signature Ok(()) } -/// The default behavior for transforming an inline fragment. +/// The default behavior for traversing an inline fragment. /// /// Returns `Ok(None)` if all selections within the fragment are removed. pub(crate) fn inline_fragment( visitor: &mut impl Visitor, parent_type: &str, - hir: &hir::InlineFragment, + def: &ast::InlineFragment, ) -> Result<(), BoxError> { - let parent_type = hir.type_condition().unwrap_or(parent_type); - selection_set(visitor, hir.selection_set(), parent_type)?; - Ok(()) -} - -fn get_field_type(visitor: &impl Visitor, parent: &str, field_name: &str) -> Option { - Some(if field_name == "__typename" { - "String".into() - } else { - let db = &visitor.compiler().db; - db.types_definitions_by_name() - .get(parent)? - .field(db, field_name)? - .ty() - .name() - }) + selection_set(visitor, parent_type, &def.selection_set) } fn selection_set( visitor: &mut impl Visitor, - hir: &hir::SelectionSet, - parent_type: &str, -) -> Result<(), BoxError> { - hir.selection() - .iter() - .try_for_each(|hir| selection(visitor, hir, parent_type)) -} - -fn selection( - visitor: &mut impl Visitor, - selection: &hir::Selection, parent_type: &str, + set: &[ast::Selection], ) -> Result<(), BoxError> { - match selection { - hir::Selection::Field(hir) => visitor.field(parent_type, hir)?, - hir::Selection::FragmentSpread(hir) => visitor.fragment_spread(hir)?, - hir::Selection::InlineFragment(hir) => visitor.inline_fragment(parent_type, hir)?, - } - Ok(()) + set.iter().try_for_each(|def| match def { + ast::Selection::Field(def) => { + let field_def = &visitor + .schema() + .type_field(parent_type, &def.name) + .map_err(|e| match e { + FieldLookupError::NoSuchType => format!("type `{parent_type}` not defined"), + FieldLookupError::NoSuchField(_, _) => { + format!("no field `{}` in type `{parent_type}`", &def.name) + } + })? + .clone(); + visitor.field(parent_type, field_def, def) + } + ast::Selection::FragmentSpread(def) => visitor.fragment_spread(def), + ast::Selection::InlineFragment(def) => { + let fragment_type = def.type_condition.as_deref().unwrap_or(parent_type); + visitor.inline_fragment(fragment_type, def) + } + }) } #[test] fn test_count_fields() { - struct CountFields<'a> { - compiler: &'a ApolloCompiler, + struct CountFields { + schema: apollo_compiler::Schema, fields: u32, } - impl<'a> Visitor for CountFields<'a> { - fn compiler(&self) -> &ApolloCompiler { - self.compiler + impl Visitor for CountFields { + fn field( + &mut self, + _parent_type: &str, + field_def: &ast::FieldDefinition, + def: &ast::Field, + ) -> Result<(), BoxError> { + self.fields += 1; + field(self, field_def, def) } - fn field(&mut self, parent_type: &str, hir: &hir::Field) -> Result<(), BoxError> { - self.fields += 1; - field(self, parent_type, hir) + fn schema(&self) -> &apollo_compiler::Schema { + &self.schema } } - let mut compiler = ApolloCompiler::new(); let graphql = " type Query { a: String @@ -215,11 +205,9 @@ fn test_count_fields() { } } "; - let file_id = compiler.add_document(graphql, ""); - let mut visitor = CountFields { - compiler: &compiler, - fields: 0, - }; - document(&mut visitor, file_id).unwrap(); + let ast = apollo_compiler::ast::Document::parse(graphql, ""); + let (schema, _doc) = ast.to_mixed(); + let mut visitor = CountFields { fields: 0, schema }; + document(&mut visitor, &ast).unwrap(); assert_eq!(visitor.fields, 4) } diff --git a/apollo-router/src/spec/schema.rs b/apollo-router/src/spec/schema.rs index 1703484b9d..0a5706c0d3 100644 --- a/apollo-router/src/spec/schema.rs +++ b/apollo-router/src/spec/schema.rs @@ -1,20 +1,17 @@ //! GraphQL schema. use std::collections::HashMap; +use std::collections::HashSet; use std::str::FromStr; use std::sync::Arc; use std::time::Instant; -use apollo_compiler::diagnostics::ApolloDiagnostic; -use apollo_compiler::ApolloCompiler; -use apollo_compiler::AstDatabase; -use apollo_compiler::HirDatabase; -use apollo_compiler::InputDatabase; +use apollo_compiler::ast; +use apollo_compiler::Diagnostics; use http::Uri; use sha2::Digest; use sha2::Sha256; -use super::FieldType; use crate::configuration::GraphQLValidationMode; use crate::error::ParseErrors; use crate::error::SchemaError; @@ -26,10 +23,11 @@ use crate::Configuration; #[derive(Debug)] pub(crate) struct Schema { pub(crate) raw_sdl: Arc, - pub(crate) type_system: Arc, + pub(crate) definitions: apollo_compiler::Schema, /// Stored for comparison with the validation errors from query planning. - diagnostics: Vec, + diagnostics: Option, subgraphs: HashMap, + pub(crate) implementers_map: HashMap>, api_schema: Option>, pub(crate) schema_id: Option, } @@ -63,53 +61,39 @@ impl Schema { pub(crate) fn parse(sdl: &str, configuration: &Configuration) -> Result { let start = Instant::now(); - let mut compiler = ApolloCompiler::new(); - let id = compiler.add_type_system(sdl, "schema.graphql"); - - let ast = compiler.db.ast(id); + let mut parser = apollo_compiler::Parser::new(); + let ast = parser.parse_ast(sdl, "schema.graphql"); // Trace log recursion limit data - let recursion_limit = ast.recursion_limit(); + let recursion_limit = parser.recursion_reached(); tracing::trace!(?recursion_limit, "recursion limit data"); - let mut parse_errors = ast.errors().peekable(); - if parse_errors.peek().is_some() { - let errors = parse_errors.cloned().collect::>(); - return Err(SchemaError::Parse(ParseErrors { errors })); - } + ast.check_parse_errors() + .map_err(|errors| SchemaError::Parse(ParseErrors { errors }))?; + + let definitions = ast.to_schema(); let diagnostics = if configuration.experimental_graphql_validation_mode == GraphQLValidationMode::Legacy { - vec![] + None } else { - compiler - .validate() - .into_iter() - .filter(|err| err.data.is_error()) - .collect::>() + definitions.validate().err() }; - if !diagnostics.is_empty() { - let errors = ValidationErrors { - errors: diagnostics.clone(), - }; - - // Only error out if new validation is used: with `Both`, we take the legacy - // validation as authoritative and only use the new result for comparison - if configuration.experimental_graphql_validation_mode == GraphQLValidationMode::New { - return Err(SchemaError::Validate(errors)); + // Only error out if new validation is used: with `Both`, we take the legacy + // validation as authoritative and only use the new result for comparison + if configuration.experimental_graphql_validation_mode == GraphQLValidationMode::New { + if let Some(errors) = diagnostics { + return Err(SchemaError::Validate(ValidationErrors { errors })); } } let mut subgraphs = HashMap::new(); // TODO: error if not found? - if let Some(join_enum) = compiler.db.find_enum_by_name("join__Graph".into()) { - for (name, url) in join_enum.values().filter_map(|value| { - let join_directive = value - .directives() - .iter() - .find(|directive| directive.name() == "join__graph")?; + if let Some(join_enum) = definitions.get_enum("join__Graph") { + for (name, url) in join_enum.values.iter().filter_map(|(_name, value)| { + let join_directive = value.directives.get("join__graph")?; let name = join_directive.argument_by_name("name")?.as_str()?; let url = join_directive.argument_by_name("url")?.as_str()?; Some((name, url)) @@ -127,7 +111,6 @@ impl Schema { } } - let sdl = compiler.db.source_code(id); let mut hasher = Sha256::new(); hasher.update(sdl.as_bytes()); let schema_id = Some(format!("{:x}", hasher.finalize())); @@ -135,11 +118,14 @@ impl Schema { histogram.apollo.router.schema.load.duration = start.elapsed().as_secs_f64() ); + let implementers_map = definitions.implementers_map(); + Ok(Schema { - raw_sdl: Arc::new(sdl.to_string()), - type_system: compiler.db.type_system(), + raw_sdl: Arc::new(sdl.to_owned()), + definitions, diagnostics, subgraphs, + implementers_map, api_schema: None, schema_id, }) @@ -158,41 +144,27 @@ impl Schema { } pub(crate) fn is_subtype(&self, abstract_type: &str, maybe_subtype: &str) -> bool { - self.type_system - .subtype_map - .get(abstract_type) - .map(|x| x.contains(maybe_subtype)) - .unwrap_or(false) + self.definitions.is_subtype(abstract_type, maybe_subtype) } pub(crate) fn is_implementation(&self, interface: &str, implementor: &str) -> bool { - self.type_system - .definitions - .interfaces - .get(interface) + self.definitions + .get_interface(interface) .map(|interface| { - interface - .implements_interfaces() - .any(|i| i.interface() == implementor) + // FIXME: this looks backwards + interface.implements_interfaces.contains(implementor) }) .unwrap_or(false) } pub(crate) fn is_interface(&self, abstract_type: &str) -> bool { - self.type_system - .definitions - .interfaces - .contains_key(abstract_type) + self.definitions.get_interface(abstract_type).is_some() } // given two field, returns the one that implements the other, if applicable - pub(crate) fn most_precise<'f>( - &self, - a: &'f FieldType, - b: &'f FieldType, - ) -> Option<&'f FieldType> { - let typename_a = a.inner_type_name().unwrap_or_default(); - let typename_b = b.inner_type_name().unwrap_or_default(); + pub(crate) fn most_precise<'f>(&self, a: &'f str, b: &'f str) -> Option<&'f str> { + let typename_a = a; + let typename_b = b; if typename_a == typename_b { return Some(a); } @@ -227,17 +199,15 @@ impl Schema { } pub(crate) fn root_operation_name(&self, kind: OperationKind) -> &str { - let schema_def = &self.type_system.definitions.schema; - match kind { - OperationKind::Query => schema_def.query(), - OperationKind::Mutation => schema_def.mutation(), - OperationKind::Subscription => schema_def.subscription(), + if let Some(name) = self.definitions.root_operation(kind.into()) { + name.as_str() + } else { + kind.as_str() } - .unwrap_or_else(|| kind.as_str()) } pub(crate) fn has_errors(&self) -> bool { - !self.diagnostics.is_empty() + self.diagnostics.is_some() } } @@ -421,9 +391,12 @@ mod tests { let schema = include_str!("../testdata/contract_schema.graphql"); let schema = Schema::parse_test(schema, &Default::default()).unwrap(); let has_in_stock_field = |schema: &Schema| { - schema.type_system.definitions.objects["Product"] - .fields() - .any(|f| f.name() == "inStock") + schema + .definitions + .get_object("Product") + .unwrap() + .fields + .contains_key("inStock") }; assert!(has_in_stock_field(&schema)); assert!(!has_in_stock_field(schema.api_schema.as_ref().unwrap())); diff --git a/apollo-router/src/spec/selection.rs b/apollo-router/src/spec/selection.rs index e1d6ee7386..4084af02fc 100644 --- a/apollo-router/src/spec/selection.rs +++ b/apollo-router/src/spec/selection.rs @@ -1,4 +1,4 @@ -use apollo_compiler::hir; +use apollo_compiler::executable; use serde::Deserialize; use serde::Serialize; use serde_json_bytes::ByteString; @@ -42,8 +42,8 @@ pub(crate) enum Selection { impl Selection { pub(crate) fn from_hir( - selection: &hir::Selection, - current_type: &FieldType, + selection: &executable::Selection, + current_type: &str, schema: &Schema, mut count: usize, defer_stats: &mut DeferStats, @@ -59,99 +59,61 @@ impl Selection { count += 1; Ok(match selection { // Spec: https://spec.graphql.org/draft/#Field - hir::Selection::Field(field) => { - let include_skip = IncludeSkip::parse(field.directives()); + executable::Selection::Field(field) => { + let include_skip = IncludeSkip::parse(&field.directives); if include_skip.statically_skipped() { return Ok(None); } - let field_type = match field.name() { - TYPENAME => FieldType::new_named("String"), - "__schema" => FieldType::new_named("__Schema"), - "__type" => FieldType::new_named("__Type"), - field_name => { - let name = current_type - .inner_type_name() - .ok_or_else(|| SpecError::InvalidType(current_type.to_string()))?; - let definitions = &schema.type_system.definitions; - //looking into object types - definitions - .objects - .get(name) - .and_then(|ty| ty.fields().find(|f| f.name() == field_name)) - // otherwise, it might be an interface - .or_else(|| { - definitions - .interfaces - .get(name) - .and_then(|ty| ty.fields().find(|f| f.name() == field_name)) - }) - .ok_or_else(|| { - SpecError::InvalidField( - field_name.to_owned(), - current_type - .inner_type_name() - .map(ToString::to_string) - .unwrap_or_else(|| current_type.to_string()), - ) - })? - .ty() - .into() - } - }; + let field_type = FieldType::from(field.ty()); - let alias = field.alias().map(|x| x.0.as_str().into()); + let alias = field.alias.as_ref().map(|x| x.as_str().into()); - let selection_set = if field_type.is_builtin_scalar() { + let selection_set = if field.selection_set.selections.is_empty() { None } else { - let selection = field.selection_set().selection(); - if selection.is_empty() { - None - } else { - Some( - selection - .iter() - .filter_map(|selection| { - Selection::from_hir( - selection, - &field_type, - schema, - count, - defer_stats, - ) - .transpose() - }) - .collect::>()?, - ) - } + Some( + field + .selection_set + .selections + .iter() + .filter_map(|selection| { + Selection::from_hir( + selection, + field_type.0.inner_named_type(), + schema, + count, + defer_stats, + ) + .transpose() + }) + .collect::>()?, + ) }; Some(Self::Field { alias, - name: field.name().into(), + name: field.name.as_str().into(), selection_set, field_type, include_skip, }) } // Spec: https://spec.graphql.org/draft/#InlineFragment - hir::Selection::InlineFragment(inline_fragment) => { - let include_skip = IncludeSkip::parse(inline_fragment.directives()); + executable::Selection::InlineFragment(inline_fragment) => { + let include_skip = IncludeSkip::parse(&inline_fragment.directives); if include_skip.statically_skipped() { return Ok(None); } - let (defer, defer_label) = parse_defer(inline_fragment.directives(), defer_stats); + let (defer, defer_label) = parse_defer(&inline_fragment.directives, defer_stats); let type_condition = inline_fragment - .type_condition() - .map(|s| s.to_owned()) - // if we can't get a type name from the current type, that means we're applying - // a fragment onto a scalar - .or_else(|| current_type.inner_type_name().map(|s| s.to_string())) - .ok_or_else(|| SpecError::InvalidType(current_type.to_string()))?; + .type_condition + .as_ref() + .map(|s| s.as_str()) + .unwrap_or(current_type) + .to_owned(); - let fragment_type = FieldType::new_named(type_condition.clone()); - let known_type = current_type.inner_type_name().map(|s| s.to_string()); + let fragment_type = &type_condition; // this is the type we pass when extracting the fragment's selections // If the type condition is a union or interface and the current type implements it, then we want @@ -163,25 +125,26 @@ impl Selection { debug_assert!( schema.is_subtype( type_condition.as_str(), - current_type.inner_type_name().unwrap_or("") + current_type ) || schema.is_implementation( type_condition.as_str(), - current_type.inner_type_name().unwrap_or("")) + current_type + ) || // if the current type and the type condition are both the same interface, it is still valid type_condition.as_str() - == current_type.inner_type_name().unwrap_or("") + == current_type ); - let relevant_type = schema.most_precise(current_type, &fragment_type); + let relevant_type = schema.most_precise(current_type, fragment_type); debug_assert!(relevant_type.is_some()); - relevant_type.unwrap_or(&fragment_type) + relevant_type.unwrap_or(fragment_type) } else { - &fragment_type + fragment_type }; let selection_set = inline_fragment - .selection_set() - .selection() + .selection_set + .selections .iter() .filter_map(|selection| { Selection::from_hir(selection, relevant_type, schema, count, defer_stats) @@ -189,6 +152,8 @@ impl Selection { }) .collect::>()?; + let known_type = Some(inline_fragment.selection_set.ty.as_str().to_owned()); + Some(Self::InlineFragment { type_condition, selection_set, @@ -199,15 +164,16 @@ impl Selection { }) } // Spec: https://spec.graphql.org/draft/#FragmentSpread - hir::Selection::FragmentSpread(fragment_spread) => { - let include_skip = IncludeSkip::parse(fragment_spread.directives()); + executable::Selection::FragmentSpread(fragment_spread) => { + let include_skip = IncludeSkip::parse(&fragment_spread.directives); if include_skip.statically_skipped() { return Ok(None); } - let (defer, defer_label) = parse_defer(fragment_spread.directives(), defer_stats); + let (defer, defer_label) = parse_defer(&fragment_spread.directives, defer_stats); + Some(Self::FragmentSpread { - name: fragment_spread.name().to_owned(), - known_type: current_type.inner_type_name().map(|s| s.to_string()), + name: fragment_spread.fragment_name.as_str().to_owned(), + known_type: Some(current_type.to_owned()), include_skip, defer, defer_label, @@ -319,48 +285,47 @@ pub(crate) enum Condition { /// Returns the `if` condition and the `label` fn parse_defer( - directives: &[hir::Directive], + directives: &executable::Directives, defer_stats: &mut DeferStats, ) -> (Condition, Option) { - for directive in directives { - if directive.name() == DEFER_DIRECTIVE_NAME { - let condition = Condition::parse(directive).unwrap_or(Condition::Yes); - match &condition { - Condition::Yes => { - defer_stats.has_defer = true; - defer_stats.has_unconditional_defer = true; - } - Condition::No => {} - Condition::Variable(name) => { - defer_stats.has_defer = true; - defer_stats - .conditional_defer_variable_names - .insert(name.clone()); - } + if let Some(directive) = directives.get(DEFER_DIRECTIVE_NAME) { + let condition = Condition::parse(directive).unwrap_or(Condition::Yes); + match &condition { + Condition::Yes => { + defer_stats.has_defer = true; + defer_stats.has_unconditional_defer = true; + } + Condition::No => {} + Condition::Variable(name) => { + defer_stats.has_defer = true; + defer_stats + .conditional_defer_variable_names + .insert(name.clone()); } - let label = if condition != Condition::No { - directive - .argument_by_name("label") - .and_then(|value| value.as_str()) - .map(|str| str.to_owned()) - } else { - None - }; - return (condition, label); } + let label = if condition != Condition::No { + directive + .argument_by_name("label") + .and_then(|value| value.as_str()) + .map(|str| str.to_owned()) + } else { + None + }; + (condition, label) + } else { + (Condition::No, None) } - (Condition::No, None) } impl IncludeSkip { - pub(crate) fn parse(directives: &[hir::Directive]) -> Self { + pub(crate) fn parse(directives: &executable::Directives) -> Self { let mut include = None; let mut skip = None; - for directive in directives { - if include.is_none() && directive.name() == "include" { + for directive in &directives.0 { + if include.is_none() && directive.name == "include" { include = Condition::parse(directive) } - if skip.is_none() && directive.name() == "skip" { + if skip.is_none() && directive.name == "skip" { skip = Condition::parse(directive) } } @@ -370,6 +335,13 @@ impl IncludeSkip { } } + pub(crate) fn default() -> Self { + Self { + include: Condition::Yes, + skip: Condition::No, + } + } + pub(crate) fn statically_skipped(&self) -> bool { matches!(self.skip, Condition::Yes) || matches!(self.include, Condition::No) } @@ -383,11 +355,13 @@ impl IncludeSkip { } impl Condition { - pub(crate) fn parse(directive: &hir::Directive) -> Option { - match directive.argument_by_name("if")? { - hir::Value::Boolean { value: true, .. } => Some(Condition::Yes), - hir::Value::Boolean { value: false, .. } => Some(Condition::No), - hir::Value::Variable(variable) => Some(Condition::Variable(variable.name().to_owned())), + pub(crate) fn parse(directive: &executable::Directive) -> Option { + match directive.argument_by_name("if")?.as_ref() { + executable::Value::Boolean(true) => Some(Condition::Yes), + executable::Value::Boolean(false) => Some(Condition::No), + executable::Value::Variable(variable) => { + Some(Condition::Variable(variable.as_str().to_owned())) + } _ => None, } } diff --git a/apollo-router/tests/integration_tests.rs b/apollo-router/tests/integration_tests.rs index eca03bd8c6..4e6670bd9e 100644 --- a/apollo-router/tests/integration_tests.rs +++ b/apollo-router/tests/integration_tests.rs @@ -110,20 +110,21 @@ async fn api_schema_hides_field() { let (actual, _) = query_rust(request).await; - assert!(actual.errors[0] - .message - .as_str() - .contains(r#"cannot query field `inStock` on type `Product`"#)); + let message = &actual.errors[0].message; + assert!( + message.contains("no field `inStock` in type `Product`"), + "{message}" + ); assert_eq!( actual.errors[0].extensions["code"].as_str(), - Some("GRAPHQL_VALIDATION_FAILED"), + Some("PARSING_ERROR"), ); } #[tokio::test(flavor = "multi_thread")] async fn validation_errors_from_rust() { let request = supergraph::Request::fake_builder() - .query(r#"{ topProducts { name inStock } notAField }"#) + .query(r#"{ topProducts { name(notAnArg: true) } } fragment Unused on Product { upc }"#) .build() .expect("expecting valid request"); diff --git a/apollo-router/tests/snapshots/integration_tests__defer_path_with_disabled_config.snap b/apollo-router/tests/snapshots/integration_tests__defer_path_with_disabled_config.snap index 26541b985c..a9aba23865 100644 --- a/apollo-router/tests/snapshots/integration_tests__defer_path_with_disabled_config.snap +++ b/apollo-router/tests/snapshots/integration_tests__defer_path_with_disabled_config.snap @@ -5,11 +5,11 @@ expression: stream.next().await.unwrap().unwrap() { "errors": [ { - "message": "directive `@defer` is not defined", + "message": "compiler error: cannot find directive `defer` in this document", "locations": [ { "line": 4, - "column": 9 + "column": 20 } ], "extensions": { diff --git a/apollo-router/tests/snapshots/integration_tests__validation_errors_from_rust.snap b/apollo-router/tests/snapshots/integration_tests__validation_errors_from_rust.snap index 7b2a5354d9..b225176b7a 100644 --- a/apollo-router/tests/snapshots/integration_tests__validation_errors_from_rust.snap +++ b/apollo-router/tests/snapshots/integration_tests__validation_errors_from_rust.snap @@ -4,23 +4,17 @@ expression: response.errors --- [ { - "message": "cannot query field `inStock` on type `Product`", - "locations": [ - { - "line": 4, - "column": 5 - } - ], + "message": "compiler error: fragment `Unused` must be used in an operation", "extensions": { "code": "GRAPHQL_VALIDATION_FAILED" } }, { - "message": "cannot query field `notAField` on type `Query`", + "message": "compiler error: the argument `notAnArg` is not supported", "locations": [ { - "line": 6, - "column": 3 + "line": 1, + "column": 22 } ], "extensions": { diff --git a/examples/supergraph-sdl/rust/Cargo.toml b/examples/supergraph-sdl/rust/Cargo.toml index 49baba750c..06fe186d26 100644 --- a/examples/supergraph-sdl/rust/Cargo.toml +++ b/examples/supergraph-sdl/rust/Cargo.toml @@ -5,7 +5,7 @@ edition = "2021" [dependencies] anyhow = "1" -apollo-compiler = "0.10.0" +apollo-compiler = "=1.0.0-beta.4" apollo-router = { path = "../../../apollo-router" } async-trait = "0.1" tower = { version = "0.4", features = ["full"] } diff --git a/examples/supergraph-sdl/rust/src/supergraph_sdl.rs b/examples/supergraph-sdl/rust/src/supergraph_sdl.rs index 6141e6c43f..50fd740f91 100644 --- a/examples/supergraph-sdl/rust/src/supergraph_sdl.rs +++ b/examples/supergraph-sdl/rust/src/supergraph_sdl.rs @@ -1,6 +1,7 @@ use std::sync::Arc; -use apollo_compiler::ApolloCompiler; +use apollo_compiler::ExecutableDocument; +use apollo_compiler::Schema; use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; use apollo_router::register_plugin; @@ -9,11 +10,10 @@ use tower::BoxError; use tower::ServiceBuilder; use tower::ServiceExt; -#[derive(Default)] // Global state for our plugin would live here. -// We keep our supergraph sdl here as a string. +// We keep our parsed supergraph schema in a reference-counted pointer struct SupergraphSDL { - supergraph_sdl: Arc, + schema: Arc, } #[async_trait::async_trait] @@ -23,13 +23,13 @@ impl Plugin for SupergraphSDL { async fn new(init: PluginInit) -> Result { Ok(SupergraphSDL { - supergraph_sdl: init.supergraph_sdl, + schema: Arc::new(Schema::parse(&*init.supergraph_sdl, "schema.graphql")), }) } fn supergraph_service(&self, service: supergraph::BoxService) -> supergraph::BoxService { - // Clone our supergraph_sdl for use in map_request - let supergraph_sdl = self.supergraph_sdl.clone(); + // Clone our parsed schema for use in map_request + let schema = self.schema.clone(); // `ServiceBuilder` provides us with `map_request` and `map_response` methods. // // These allow basic interception and transformation of request and response messages. @@ -37,16 +37,14 @@ impl Plugin for SupergraphSDL { .map_request(move |req: supergraph::Request| { // If we have a query if let Some(query) = &req.supergraph_request.body().query { - // Compile our supergraph_sdl and query - let mut ctx = ApolloCompiler::new(); - ctx.add_type_system(&supergraph_sdl, "supergraph_sdl"); - ctx.add_executable(query, "query"); + // Parse our query against the schema + let doc = ExecutableDocument::parse(&schema, query, "query.graphql"); // Do we have any diagnostics we'd like to print? - let diagnostics = ctx.validate(); - for diagnostic in diagnostics { - tracing::warn!(%diagnostic, "compiler diagnostics"); + if let Err(diagnostics) = doc.validate(&schema) { + let diagnostics = diagnostics.to_string_no_color(); + tracing::warn!(%diagnostics, "validation diagnostics"); } - // TODO: Whatever else we want to do with our compiler context + // TODO: Whatever else we want to do with our parsed schema and document } req })