From cd5e0e2e2e332bdde82cc02162e47e35708bc6ec Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 20 Oct 2023 15:35:19 +0200 Subject: [PATCH] Port to apollo-compiler 1.0 beta (#4038) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes https://github.com/apollographql/router/issues/2710 # apollo-compiler background The public API of apollo-compiler 1.0 is almost completely different from 0.x. Notably it doesn’t have compiler objects anymore. Instead, users mainly manipulate: * `apollo_compiler::ast::Document` is the result of parsing one input file/string. May contain parse errors. Finding anything requires a linear scan of a `Vec` of top-level definitions. * `apollo_compiler::Schema` has stuff indexed by name, with GraphQL extensions "applied". Conversion from AST may record "build errors" (e.g. for a name collision). Stuff with errors (e.g. that second definition with the same name) may be missing in the main data structure. * `apollo_compiler::ExecutableDocument` similarly has operations and fragments indexed by name, has build errors, and may be missing stuff related to errors. Creating one requires `&Schema`, where it’ll find a field definition to associate with every field selection, and resolve the type of every selection set. `Schema` and `ExecutableDocument` have `validate` methods to run full GraphQL spec validation, but it needs to be called explicitly. A future beta will likely change the API to make it less easy to forget dealing with build errors or validation errors. # This PR Instead of creating and passing around a `Arc>` for every GraphQL request, the Router now creates a `Arc` which contains *both* an `ast::Document` and an `ExecutableDocument`. In a later PR we’ll want to run Rust validation early and have the rest of a request lifecycle only deal with `ExecutableDocument`, but as long as we rely on TypeScript validation some things still need to work at the AST level. ~~This PR starts as a draft because of remaining failing tests:~~ * `cargo test --lib` - [x] plugins::telemetry::metrics::apollo::test::apollo_metrics_validation_failure - [x] router::tests::schema_update_test - [x] spec::query::tests::reformat_response_data_best_effort * `cargo test --test integration_tests` - [x] api_schema_hides_field - [x] defer_path_with_disabled_config - [x] validation_errors_from_rust --- **Checklist** Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review. - [x] Changes are compatible[^1] - [ ] Documentation[^2] completed - [x] Performance impact assessed and acceptable - Tests added and passing[^3] - [ ] Unit Tests - [ ] Integration Tests - [ ] Manual Tests **Exceptions** *Note any exceptions here* **Notes** [^1]: It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this. [^2]: Configuration is an important part of many changes. Where applicable please try to document configuration examples. [^3]: Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions. --- Cargo.lock | 50 +- apollo-router/Cargo.toml | 12 +- apollo-router/src/error.rs | 28 +- .../plugins/authorization/authenticated.rs | 339 ++++++------- .../src/plugins/authorization/mod.rs | 167 +++---- .../src/plugins/authorization/policy.rs | 386 ++++++-------- .../src/plugins/authorization/scopes.rs | 419 +++++++--------- ...rization__authenticated__tests__array.snap | 2 +- ...rization__authenticated__tests__defer.snap | 2 +- ...thenticated__tests__interface_field-2.snap | 2 +- ...authenticated__tests__interface_field.snap | 2 +- ...henticated__tests__interface_fragment.snap | 2 +- ...ted__tests__interface_inline_fragment.snap | 2 +- ...uthenticated__tests__interface_type-2.snap | 2 +- ..._authenticated__tests__interface_type.snap | 2 +- ...on__authenticated__tests__query_field.snap | 2 +- ...thenticated__tests__query_field_alias.snap | 2 +- ...ization__authenticated__tests__scalar.snap | 2 +- ...orization__authenticated__tests__test.snap | 2 +- ...rization__authenticated__tests__union.snap | 2 +- ...authorization__policy__tests__array-2.snap | 2 +- ...__policy__tests__filter_basic_query-2.snap | 2 +- ...__policy__tests__filter_basic_query-4.snap | 2 +- ...__policy__tests__filter_basic_query-6.snap | 2 +- ...__policy__tests__filter_basic_query-8.snap | 2 +- ...ion__policy__tests__interface_field-3.snap | 2 +- ...ation__policy__tests__interface_field.snap | 2 +- ...__policy__tests__interface_fragment-2.snap | 2 +- ...__policy__tests__interface_fragment-4.snap | 3 +- ...y__tests__interface_inline_fragment-2.snap | 2 +- ...tion__policy__tests__interface_type-3.snap | 2 +- ...tion__policy__tests__interface_type-5.snap | 2 +- ...tion__policy__tests__interface_type-7.snap | 2 +- ...tion__policy__tests__interface_type-9.snap | 2 +- ...zation__policy__tests__interface_type.snap | 2 +- ...ization__policy__tests__query_field-2.snap | 2 +- ...n__policy__tests__query_field_alias-2.snap | 2 +- ...uthorization__policy__tests__scalar-2.snap | 2 +- ...__authorization__policy__tests__union.snap | 2 +- ...__authorization__scopes__tests__array.snap | 2 +- ...__scopes__tests__filter_basic_query-2.snap | 2 +- ...__scopes__tests__filter_basic_query-3.snap | 2 +- ...__scopes__tests__filter_basic_query-4.snap | 2 +- ...on__scopes__tests__filter_basic_query.snap | 2 +- ...ion__scopes__tests__interface_field-2.snap | 2 +- ...ation__scopes__tests__interface_field.snap | 2 +- ...__scopes__tests__interface_fragment-2.snap | 3 +- ...__scopes__tests__interface_fragment-3.snap | 3 +- ...on__scopes__tests__interface_fragment.snap | 2 +- ...s__tests__interface_inline_fragment-2.snap | 2 +- ...s__tests__interface_inline_fragment-3.snap | 2 +- ...pes__tests__interface_inline_fragment.snap | 2 +- ...tion__scopes__tests__interface_type-2.snap | 2 +- ...tion__scopes__tests__interface_type-3.snap | 2 +- ...tion__scopes__tests__interface_type-4.snap | 2 +- ...tion__scopes__tests__interface_type-5.snap | 2 +- ...zation__scopes__tests__interface_type.snap | 2 +- ...orization__scopes__tests__query_field.snap | 2 +- ...ion__scopes__tests__query_field_alias.snap | 2 +- ..._authorization__scopes__tests__scalar.snap | 2 +- ...__authorization__scopes__tests__union.snap | 2 +- .../src/plugins/telemetry/metrics/apollo.rs | 2 +- .../src/query_planner/bridge_query_planner.rs | 115 ++--- .../query_planner/caching_query_planner.rs | 80 ++- apollo-router/src/query_planner/fetch.rs | 20 + apollo-router/src/query_planner/labeler.rs | 139 +++--- ...ner__tests__plan_invalid_query_errors.snap | 2 +- ...er__tests__large_float_written_as_int.snap | 2 +- apollo-router/src/router/mod.rs | 10 +- .../layers/allow_only_http_post_mutations.rs | 50 +- .../persisted_queries/manifest_poller.rs | 74 ++- .../services/layers/persisted_queries/mod.rs | 53 +- .../src/services/layers/query_analysis.rs | 42 +- .../src/services/supergraph_service.rs | 24 +- apollo-router/src/spec/field_type.rs | 230 ++++----- apollo-router/src/spec/fragments.rs | 24 +- apollo-router/src/spec/operation_limits.rs | 69 +-- apollo-router/src/spec/query.rs | 223 ++++----- apollo-router/src/spec/query/subselections.rs | 4 +- apollo-router/src/spec/query/tests.rs | 55 +- apollo-router/src/spec/query/transform.rs | 471 +++++++----------- apollo-router/src/spec/query/traverse.rs | 204 ++++---- apollo-router/src/spec/schema.rs | 123 ++--- apollo-router/src/spec/selection.rs | 216 ++++---- apollo-router/tests/integration_tests.rs | 13 +- ...ests__defer_path_with_disabled_config.snap | 4 +- ...on_tests__validation_errors_from_rust.snap | 14 +- examples/supergraph-sdl/rust/Cargo.toml | 2 +- .../supergraph-sdl/rust/src/supergraph_sdl.rs | 28 +- 89 files changed, 1625 insertions(+), 2180 deletions(-) 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 a565a303be..63a2e6ddc3 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; @@ -579,16 +578,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); } @@ -852,6 +852,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 @@ -1413,7 +1414,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()); @@ -1491,7 +1491,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()); @@ -1559,8 +1558,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()); @@ -1750,7 +1747,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 })