From 140b91d575f25c9bddd8f446b0c7a3a4505bb67a Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 6 Oct 2023 21:53:55 +0200 Subject: [PATCH 1/3] Add opt in flag to adopt orphan extensions --- crates/apollo-compiler/CHANGELOG.md | 20 +++ crates/apollo-compiler/src/schema/from_ast.rs | 146 +++++++++++++++--- crates/apollo-compiler/tests/extensions.rs | 55 +++++++ crates/apollo-compiler/tests/main.rs | 1 + 4 files changed, 204 insertions(+), 18 deletions(-) create mode 100644 crates/apollo-compiler/tests/extensions.rs diff --git a/crates/apollo-compiler/CHANGELOG.md b/crates/apollo-compiler/CHANGELOG.md index 80b38256b..b8c651f5d 100644 --- a/crates/apollo-compiler/CHANGELOG.md +++ b/crates/apollo-compiler/CHANGELOG.md @@ -19,6 +19,26 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm # [x.x.x] (unreleased) - 2023-mm-dd +## Features + +- Add opt-in configuration for “orphan” extensions to be “adopted”, by [SimonSapin] in [pull/678] + + Type extensions and schema extensions without a corresponding definition + are normally ignored except for recording a validation error. + In this new mode, an implicit empty definition to extend is generated instead. + Because this behavior is non-standard it is not the default. + Configure a schema builder to opt in: + ```rust + let input = "extend type Query { x: Int }"; + let schema = apollo_compiler::Schema::builder() + .adopt_orphan_extensions() + .parse(input, "schema.graphql") + .build(); + schema.validate()?; + ``` + +[pull/678]: https://github.com/apollographql/apollo-rs/pull/678 + ## Fixes - Allow built-in directives to be redefined, by [SimonSapin] in [pull/684], [issue/656] diff --git a/crates/apollo-compiler/src/schema/from_ast.rs b/crates/apollo-compiler/src/schema/from_ast.rs index 49df2b471..bdf7c79da 100644 --- a/crates/apollo-compiler/src/schema/from_ast.rs +++ b/crates/apollo-compiler/src/schema/from_ast.rs @@ -2,6 +2,7 @@ use super::*; use indexmap::map::Entry; pub struct SchemaBuilder { + adopt_orphan_extensions: bool, schema: Schema, orphan_schema_extensions: Vec>, orphan_type_extensions: IndexMap>, @@ -18,6 +19,7 @@ impl SchemaBuilder { /// and introspection types pub fn new() -> Self { let mut builder = SchemaBuilder { + adopt_orphan_extensions: false, schema: Schema { sources: IndexMap::new(), build_errors: Vec::new(), @@ -50,6 +52,14 @@ impl SchemaBuilder { builder } + /// Configure the builder so that “orphan” schema extensions and type extensions + /// (without a corresponding definition) are “adopted”: + /// accepted as if extending an empty definition instead of being rejected as errors. + pub fn adopt_orphan_extensions(mut self) -> Self { + self.adopt_orphan_extensions = true; + self + } + /// Parse an input file with the default configuration as an additional input for this schema. /// /// Create a [`Parser`] to use different parser configuration. @@ -61,6 +71,12 @@ impl SchemaBuilder { /// Add an AST document to the schema being built /// /// Executable definitions, if any, will be silently ignored. + pub fn add_ast(mut self, document: &ast::Document) -> Self { + let executable_definitions_are_errors = true; + self.add_ast_document(document, executable_definitions_are_errors); + self + } + pub(crate) fn add_ast_document( &mut self, document: &ast::Document, @@ -116,6 +132,7 @@ impl SchemaBuilder { if let Err((prev_name, previous)) = insert_sticky(&mut self.schema.types, &def.name, || { ExtendedType::Scalar(ScalarType::from_ast( + &mut self.schema.build_errors, def, self.orphan_type_extensions .remove(&def.name) @@ -252,7 +269,7 @@ impl SchemaBuilder { ast::Definition::ScalarTypeExtension(ext) => { if let Some((_, ty_name, ty)) = self.schema.types.get_full_mut(&ext.name) { if let ExtendedType::Scalar(ty) = ty { - ty.make_mut().extend_ast(ext) + ty.make_mut().extend_ast(&mut self.schema.build_errors, ext) } else { self.schema .build_errors @@ -403,30 +420,118 @@ impl SchemaBuilder { /// was found, or if it is a different kind of type pub fn build(self) -> Schema { let SchemaBuilder { + adopt_orphan_extensions, mut schema, orphan_schema_extensions, orphan_type_extensions, } = self; - schema - .build_errors - .extend(orphan_schema_extensions.into_iter().map(|ext| { - BuildError::OrphanSchemaExtension { - location: ext.location(), - } - })); - schema - .build_errors - .extend(orphan_type_extensions.into_values().flatten().map(|ext| { - let name = ext.name().unwrap().clone(); - BuildError::OrphanTypeExtension { - location: name.location(), - name, + if adopt_orphan_extensions { + if !orphan_schema_extensions.is_empty() { + assert!(schema.schema_definition.is_none()); + let schema_def = schema + .schema_definition + .get_or_insert_with(Default::default) + .make_mut(); + for ext in &orphan_schema_extensions { + schema_def.extend_ast(&mut schema.build_errors, ext) } - })); + } + for (type_name, extensions) in orphan_type_extensions { + let type_def = adopt_type_extensions(&mut schema, &type_name, &extensions); + let previous = schema.types.insert(type_name, type_def); + assert!(previous.is_none()); + } + } else { + schema + .build_errors + .extend(orphan_schema_extensions.into_iter().map(|ext| { + BuildError::OrphanSchemaExtension { + location: ext.location(), + } + })); + schema + .build_errors + .extend(orphan_type_extensions.into_values().flatten().map(|ext| { + let name = ext.name().unwrap().clone(); + BuildError::OrphanTypeExtension { + location: name.location(), + name, + } + })); + } schema } } +fn adopt_type_extensions( + schema: &mut Schema, + type_name: &NodeStr, + extensions: &[ast::Definition], +) -> ExtendedType { + macro_rules! extend { + ($( $ExtensionVariant: path => $describe: literal $empty_def: expr )+) => { + match &extensions[0] { + $( + $ExtensionVariant(_) => { + let mut def = $empty_def; + for ext in extensions { + if let $ExtensionVariant(ext) = ext { + def.extend_ast(&mut schema.build_errors, ext) + } else { + let ext_name = ext.name().unwrap(); + schema + .build_errors + .push(BuildError::TypeExtensionKindMismatch { + location: ext_name.location(), + name: ext_name.clone(), + describe_ext: ext.describe(), + def_location: type_name.location(), + describe_def: $describe, + }) + } + } + def.into() + } + )+ + _ => unreachable!(), + } + }; + } + extend! { + ast::Definition::ScalarTypeExtension => "a scalar type" ScalarType { + description: Default::default(), + directives: Default::default(), + } + ast::Definition::ObjectTypeExtension => "an object type" ObjectType { + description: Default::default(), + implements_interfaces: Default::default(), + directives: Default::default(), + fields: Default::default(), + } + ast::Definition::InterfaceTypeExtension => "an interface type" InterfaceType { + description: Default::default(), + implements_interfaces: Default::default(), + directives: Default::default(), + fields: Default::default(), + } + ast::Definition::UnionTypeExtension => "a union type" UnionType { + description: Default::default(), + directives: Default::default(), + members: Default::default(), + } + ast::Definition::EnumTypeExtension => "an enum type" EnumType { + description: Default::default(), + directives: Default::default(), + values: Default::default(), + } + ast::Definition::InputObjectTypeExtension => "an input object type" InputObjectType { + description: Default::default(), + directives: Default::default(), + fields: Default::default(), + } + } +} + impl SchemaDefinition { fn from_ast( errors: &mut Vec, @@ -493,6 +598,7 @@ impl SchemaDefinition { impl ScalarType { fn from_ast( + errors: &mut [BuildError], definition: &Node, extensions: Vec, ) -> Node { @@ -506,13 +612,17 @@ impl ScalarType { }; for def in &extensions { if let ast::Definition::ScalarTypeExtension(ext) = def { - ty.extend_ast(ext) + ty.extend_ast(errors, ext) } } definition.same_location(ty) } - fn extend_ast(&mut self, extension: &Node) { + fn extend_ast( + &mut self, + _errors: &mut [BuildError], + extension: &Node, + ) { let origin = ComponentOrigin::Extension(ExtensionId::new(extension)); self.directives.extend( extension diff --git a/crates/apollo-compiler/tests/extensions.rs b/crates/apollo-compiler/tests/extensions.rs new file mode 100644 index 000000000..90de0ba67 --- /dev/null +++ b/crates/apollo-compiler/tests/extensions.rs @@ -0,0 +1,55 @@ +use apollo_compiler::Schema; + +#[test] +fn test_orphan_extensions() { + let input = r#" + extend schema @dir + extend type Obj @dir + directive @dir on SCHEMA | OBJECT + "#; + + // By default, orphan extensions are errors: + let schema = Schema::parse(input, "schema.graphql"); + assert!(!schema.schema_definition_directives().has("dir")); + assert!(!schema.types.contains_key("Obj")); + let err = schema.validate().unwrap_err().to_string_no_color(); + assert!( + err.contains("schema extension without a schema definition"), + "{err}" + ); + assert!( + err.contains("type extension for undefined type `Obj`"), + "{err}" + ); + + // Opt in to non-standard behavior of adopting them instead: + let schema2 = Schema::builder() + .adopt_orphan_extensions() + .parse(input, "schema.graphql") + .build(); + assert!(schema2.schema_definition_directives().has("dir")); + assert!(schema2.types["Obj"].directives().has("dir")); + schema2.validate().unwrap(); +} + +#[test] +fn test_orphan_extensions_kind_mismatch() { + let input = r#" + extend type T @dir + extend interface T @dir + directive @dir repeatable on SCHEMA | OBJECT +"#; + + let schema = Schema::builder() + .adopt_orphan_extensions() + .parse(input, "schema.graphql") + .build(); + let type_def = &schema.types["T"]; + assert!(type_def.is_object()); + assert_eq!(type_def.directives().get_all("dir").count(), 1); + let err = schema.validate().unwrap_err().to_string_no_color(); + assert!( + err.contains("adding an interface type extension, but `T` is an object type"), + "{err}" + ); +} diff --git a/crates/apollo-compiler/tests/main.rs b/crates/apollo-compiler/tests/main.rs index b406e8112..b336d8e5c 100644 --- a/crates/apollo-compiler/tests/main.rs +++ b/crates/apollo-compiler/tests/main.rs @@ -1,4 +1,5 @@ mod executable; +mod extensions; mod merge_schemas; /// Formerly in src/lib.rs mod misc; From 24c5aa50d88786add255e60806e088c6e2994eaa Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Tue, 10 Oct 2023 12:22:29 +0200 Subject: [PATCH 2/3] Allow schema extensions to extend an implicit schema definition Fixes https://github.com/apollographql/apollo-rs/issues/682 --- crates/apollo-compiler/CHANGELOG.md | 20 ++- crates/apollo-compiler/src/schema/from_ast.rs | 136 ++++++++++++------ crates/apollo-compiler/src/schema/mod.rs | 92 ++---------- .../apollo-compiler/src/schema/serialize.rs | 67 +++++++-- .../diagnostics/0063_extension_orphan.txt | 12 +- crates/apollo-compiler/tests/extensions.rs | 18 ++- crates/apollo-compiler/tests/merge_schemas.rs | 29 ++-- crates/apollo-compiler/tests/schema.rs | 34 ++++- 8 files changed, 246 insertions(+), 162 deletions(-) diff --git a/crates/apollo-compiler/CHANGELOG.md b/crates/apollo-compiler/CHANGELOG.md index b8c651f5d..d4eceea3c 100644 --- a/crates/apollo-compiler/CHANGELOG.md +++ b/crates/apollo-compiler/CHANGELOG.md @@ -19,6 +19,19 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm # [x.x.x] (unreleased) - 2023-mm-dd +## BREAKING + +Assorted `Schema` API changes by [SimonSapin] in [pull/678]: +- Type of the `schema_definition` field changed + from `Option` to `SchemaDefinition`. + Default root operations based on object type names + are now stored explicitly in `SchemaDefinition`. + Serialization relies on a heuristic to decide on implicit schema definition. +- Removed `schema_definition_directives` method: no longer having an `Option` allows + field `schema.schema_definition.directives` to be accessed directly +- Removed `query_root_operation`, `mutation_root_operation`, and `subscription_root_operation` + methods. Instead `schema.schema_definition.query` etc can be accessed directly. + ## Features - Add opt-in configuration for “orphan” extensions to be “adopted”, by [SimonSapin] in [pull/678] @@ -37,14 +50,17 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm schema.validate()?; ``` -[pull/678]: https://github.com/apollographql/apollo-rs/pull/678 ## Fixes - Allow built-in directives to be redefined, by [SimonSapin] in [pull/684], [issue/656] +- Allow schema extensions to extend a schema definition implied by object types named after default root operations, by [SimonSapin] in [pull/678], [issues/682] -[pull/684]: https://github.com/apollographql/apollo-rs/pull/684 +[SimonSapin]: https://github.com/SimonSapin [issue/656]: https://github.com/apollographql/apollo-rs/issues/656 +[issue/682]: https://github.com/apollographql/apollo-rs/issues/682 +[pull/678]: https://github.com/apollographql/apollo-rs/pull/678 +[pull/684]: https://github.com/apollographql/apollo-rs/pull/684 # [1.0.0-beta.1](https://crates.io/crates/apollo-compiler/1.0.0-beta.1) - 2023-10-05 diff --git a/crates/apollo-compiler/src/schema/from_ast.rs b/crates/apollo-compiler/src/schema/from_ast.rs index bdf7c79da..0e9f66028 100644 --- a/crates/apollo-compiler/src/schema/from_ast.rs +++ b/crates/apollo-compiler/src/schema/from_ast.rs @@ -4,10 +4,17 @@ use indexmap::map::Entry; pub struct SchemaBuilder { adopt_orphan_extensions: bool, schema: Schema, - orphan_schema_extensions: Vec>, + schema_definition: SchemaDefinitionStatus, orphan_type_extensions: IndexMap>, } +enum SchemaDefinitionStatus { + Found, + NoneSoFar { + orphan_extensions: Vec>, + }, +} + impl Default for SchemaBuilder { fn default() -> Self { Self::new() @@ -23,11 +30,19 @@ impl SchemaBuilder { schema: Schema { sources: IndexMap::new(), build_errors: Vec::new(), - schema_definition: None, + schema_definition: Node::new(SchemaDefinition { + description: None, + directives: Directives::default(), + query: None, + mutation: None, + subscription: None, + }), directive_definitions: IndexMap::new(), types: IndexMap::new(), }, - orphan_schema_extensions: Vec::new(), + schema_definition: SchemaDefinitionStatus::NoneSoFar { + orphan_extensions: Vec::new(), + }, orphan_type_extensions: IndexMap::new(), }; @@ -46,8 +61,11 @@ impl SchemaBuilder { debug_assert!( builder.schema.build_errors.is_empty() && builder.orphan_type_extensions.is_empty() - && builder.orphan_schema_extensions.is_empty() - && builder.schema.schema_definition.is_none(), + && matches!( + &builder.schema_definition, + SchemaDefinitionStatus::NoneSoFar { orphan_extensions } + if orphan_extensions.is_empty() + ) ); builder } @@ -87,21 +105,21 @@ impl SchemaBuilder { } for definition in &document.definitions { match definition { - ast::Definition::SchemaDefinition(def) => match &self.schema.schema_definition { - None => { - self.schema.schema_definition = Some(SchemaDefinition::from_ast( + ast::Definition::SchemaDefinition(def) => match &self.schema_definition { + SchemaDefinitionStatus::NoneSoFar { orphan_extensions } => { + self.schema.schema_definition = SchemaDefinition::from_ast( &mut self.schema.build_errors, def, - &self.orphan_schema_extensions, - )); - self.orphan_schema_extensions = Vec::new(); + orphan_extensions, + ); + self.schema_definition = SchemaDefinitionStatus::Found; } - Some(previous) => { + SchemaDefinitionStatus::Found => { self.schema .build_errors .push(BuildError::SchemaDefinitionCollision { location: def.location(), - previous_location: previous.location(), + previous_location: self.schema.schema_definition.location(), }) } }, @@ -258,14 +276,16 @@ impl SchemaBuilder { }) } } - ast::Definition::SchemaExtension(ext) => { - if let Some(root) = &mut self.schema.schema_definition { - root.make_mut() - .extend_ast(&mut self.schema.build_errors, ext) - } else { - self.orphan_schema_extensions.push(ext.clone()) + ast::Definition::SchemaExtension(ext) => match &mut self.schema_definition { + SchemaDefinitionStatus::Found => self + .schema + .schema_definition + .make_mut() + .extend_ast(&mut self.schema.build_errors, ext), + SchemaDefinitionStatus::NoneSoFar { orphan_extensions } => { + orphan_extensions.push(ext.clone()) } - } + }, ast::Definition::ScalarTypeExtension(ext) => { if let Some((_, ty_name, ty)) = self.schema.types.get_full_mut(&ext.name) { if let ExtendedType::Scalar(ty) = ty { @@ -413,42 +433,74 @@ impl SchemaBuilder { } } - /// Returns the schema built from all added documents, and orphan extensions: - /// - /// * `Definition::SchemaExtension` variants if no `Definition::SchemaDefinition` was found - /// * `Definition::*TypeExtension` if no `Definition::*TypeDefinition` with the same name - /// was found, or if it is a different kind of type + /// Returns the schema built from all added documents pub fn build(self) -> Schema { let SchemaBuilder { adopt_orphan_extensions, mut schema, - orphan_schema_extensions, + schema_definition, orphan_type_extensions, } = self; - if adopt_orphan_extensions { - if !orphan_schema_extensions.is_empty() { - assert!(schema.schema_definition.is_none()); - let schema_def = schema - .schema_definition - .get_or_insert_with(Default::default) - .make_mut(); - for ext in &orphan_schema_extensions { - schema_def.extend_ast(&mut schema.build_errors, ext) + match schema_definition { + SchemaDefinitionStatus::Found => {} + SchemaDefinitionStatus::NoneSoFar { orphan_extensions } => { + // This a macro rather than a closure to generate separate `static`s + let mut has_implicit_root_operation = false; + macro_rules! default_root_operation { + ($($operation_type: path: $root_operation: expr,)+) => {{ + $( + let name = $operation_type.default_type_name(); + if let Some(ExtendedType::Object(_)) = schema.types.get(name) { + static OBJECT_TYPE_NAME: OnceLock = OnceLock::new(); + $root_operation = Some(OBJECT_TYPE_NAME.get_or_init(|| { + Name::new(name).to_component(ComponentOrigin::Definition) + }).clone()); + has_implicit_root_operation = true; + } + )+ + }}; + } + let schema_def = schema.schema_definition.make_mut(); + default_root_operation!( + ast::OperationType::Query: schema_def.query, + ast::OperationType::Mutation: schema_def.mutation, + ast::OperationType::Subscription: schema_def.subscription, + ); + + let apply_schema_extensions = + // https://github.com/apollographql/apollo-rs/issues/682 + // If we have no explict `schema` definition but do have object type(s) + // with a default type name for root operations, + // an implicit schema definition is generated with those root operations. + // That implict definition can be extended: + has_implicit_root_operation || + // https://github.com/apollographql/apollo-rs/pull/678 + // In this opt-in mode we unconditionally assume + // an implicit schema definition to extend + adopt_orphan_extensions; + if apply_schema_extensions { + for ext in &orphan_extensions { + schema_def.extend_ast(&mut schema.build_errors, ext) + } + } else { + schema + .build_errors + .extend(orphan_extensions.into_iter().map(|ext| { + BuildError::OrphanSchemaExtension { + location: ext.location(), + } + })); } } + } + // https://github.com/apollographql/apollo-rs/pull/678 + if adopt_orphan_extensions { for (type_name, extensions) in orphan_type_extensions { let type_def = adopt_type_extensions(&mut schema, &type_name, &extensions); let previous = schema.types.insert(type_name, type_def); assert!(previous.is_none()); } } else { - schema - .build_errors - .extend(orphan_schema_extensions.into_iter().map(|ext| { - BuildError::OrphanSchemaExtension { - location: ext.location(), - } - })); schema .build_errors .extend(orphan_type_extensions.into_values().flatten().map(|ext| { diff --git a/crates/apollo-compiler/src/schema/mod.rs b/crates/apollo-compiler/src/schema/mod.rs index 3f531053d..ad40b1395 100644 --- a/crates/apollo-compiler/src/schema/mod.rs +++ b/crates/apollo-compiler/src/schema/mod.rs @@ -41,10 +41,7 @@ pub struct Schema { build_errors: Vec, /// The `schema` definition and its extensions, defining root operations - /// - /// For more convenient access to its directives regardless of `Option`, - /// see [`schema_definition_directives`][Self::schema_definition_directives] - pub schema_definition: Option>, + pub schema_definition: Node, /// Built-in and explicit directive definitions pub directive_definitions: IndexMap>, @@ -332,15 +329,6 @@ impl Schema { } } - /// Directives of the `schema` definition and its extensions - pub fn schema_definition_directives(&self) -> &Directives { - if let Some(def) = &self.schema_definition { - &def.directives - } else { - Directives::EMPTY - } - } - /// Returns the type with the given name, if it is a scalar type pub fn get_scalar(&self, name: &str) -> Option<&Node> { if let Some(ExtendedType::Scalar(ty)) = self.types.get(name) { @@ -395,72 +383,15 @@ impl Schema { } } - /// Returns the name of the object type for the `query` root operation - pub fn query_root_operation(&self) -> Option<&NamedType> { - if let Some(root_operations) = &self.schema_definition { - root_operations - .query - .as_ref() - .map(|component| &component.node) - } else { - self.default_root_operation(ast::OperationType::Query) - } - } - - /// Returns the name of the object type for the `mutation` root operation - pub fn mutation_root_operation(&self) -> Option<&NamedType> { - if let Some(root_operations) = &self.schema_definition { - root_operations - .mutation - .as_ref() - .map(|component| &component.node) - } else { - self.default_root_operation(ast::OperationType::Mutation) - } - } - - /// Returns the name of the object type for the `subscription` root operation - pub fn subscription_root_operation(&self) -> Option<&NamedType> { - if let Some(root_operations) = &self.schema_definition { - root_operations - .subscription - .as_ref() - .map(|component| &component.node) - } else { - self.default_root_operation(ast::OperationType::Subscription) - } - } - /// Returns the name of the object type for the root operation with the given operation kind pub fn root_operation(&self, operation_type: ast::OperationType) -> Option<&NamedType> { - if let Some(root_operations) = &self.schema_definition { - match operation_type { - ast::OperationType::Query => &root_operations.query, - ast::OperationType::Mutation => &root_operations.mutation, - ast::OperationType::Subscription => &root_operations.subscription, - } - .as_ref() - .map(|component| &component.node) - } else { - self.default_root_operation(operation_type) + match operation_type { + ast::OperationType::Query => &self.schema_definition.query, + ast::OperationType::Mutation => &self.schema_definition.mutation, + ast::OperationType::Subscription => &self.schema_definition.subscription, } - } - - fn default_root_operation(&self, operation_type: ast::OperationType) -> Option<&NamedType> { - let name = operation_type.default_type_name(); - macro_rules! as_static { - () => {{ - static OBJECT_TYPE_NAME: OnceLock = OnceLock::new(); - OBJECT_TYPE_NAME.get_or_init(|| Name::new(name)) - }}; - } - self.get_object(name) - .is_some() - .then(|| match operation_type { - ast::OperationType::Query => as_static!(), - ast::OperationType::Mutation => as_static!(), - ast::OperationType::Subscription => as_static!(), - }) + .as_ref() + .map(|component| &component.node) } /// Returns the definition of a type’s explicit field or meta-field. @@ -581,7 +512,12 @@ impl Schema { }), ] }); - if self.query_root_operation().is_some_and(|n| n == type_name) { + if self + .schema_definition + .query + .as_ref() + .is_some_and(|n| n == type_name) + { // __typename: String! // __schema: __Schema! // __type(name: String!): __Type @@ -886,8 +822,6 @@ impl InputObjectType { } impl Directives { - const EMPTY: &Self = &Self::new(); - pub const fn new() -> Self { Self(Vec::new()) } diff --git a/crates/apollo-compiler/src/schema/serialize.rs b/crates/apollo-compiler/src/schema/serialize.rs index 65c86b687..e32f702fb 100644 --- a/crates/apollo-compiler/src/schema/serialize.rs +++ b/crates/apollo-compiler/src/schema/serialize.rs @@ -15,9 +15,7 @@ impl Schema { impl Schema { pub(crate) fn to_ast(&self) -> impl Iterator + '_ { self.schema_definition - .as_ref() - .into_iter() - .flat_map(|root| root.to_ast()) + .to_ast(&self.types) .chain( self.directive_definitions .values() @@ -36,7 +34,47 @@ impl Schema { } impl Node { - fn to_ast(&self) -> impl Iterator + '_ { + fn to_ast( + &self, + types: &IndexMap, + ) -> impl Iterator + '_ { + let SchemaDefinition { + description, + directives, + query, + mutation, + subscription, + } = &**self; + let extensions = self.extensions(); + let implict = description.is_none() + && directives.is_empty() + && extensions.is_empty() + && [ + (query, ast::OperationType::Query), + (mutation, ast::OperationType::Mutation), + (subscription, ast::OperationType::Subscription), + ] + .into_iter() + .all(|(root_operation, operation_type)| { + // If there were no explict `schema` definition, + // what implicit root operation would we get for this operation type? + let default_type_name = operation_type.default_type_name(); + let implicit_root_operation: Option<&str> = types + .get(default_type_name) + .filter(|ty_def| ty_def.is_object()) + .map(|_ty_def| default_type_name); + // What we have + let actual_root_operation = root_operation.as_ref().map(|r| r.as_str()); + // Only allow an implicit `schema` definition if they match + actual_root_operation == implicit_root_operation + }) + // Hack: if there is *nothing*, still emit an empty SchemaDefinition AST node + // that carries a location, so AST-based validation can emit an error + // with `DiagnosticData::QueryRootOperationType`. + // This can be removed after that validation rule is ported to high-level `Schema`. + && [query, mutation, subscription] + .into_iter() + .any(|op| op.is_some()); let root_ops = |ext: Option<&ExtensionId>| -> Vec> { let root_op = |op: &Option, ty| { op.as_ref() @@ -49,14 +87,19 @@ impl Node { .chain(root_op(&self.subscription, OperationType::Subscription)) .collect() }; - std::iter::once(ast::Definition::SchemaDefinition(self.same_location( - ast::SchemaDefinition { - description: self.description.clone(), - directives: ast::Directives(components(&self.directives, None)), - root_operations: root_ops(None), - }, - ))) - .chain(self.extensions().into_iter().map(move |ext| { + if implict { + None + } else { + Some(ast::Definition::SchemaDefinition(self.same_location( + ast::SchemaDefinition { + description: self.description.clone(), + directives: ast::Directives(components(&self.directives, None)), + root_operations: root_ops(None), + }, + ))) + } + .into_iter() + .chain(extensions.into_iter().map(move |ext| { ast::Definition::SchemaExtension(ext.same_location(ast::SchemaExtension { directives: ast::Directives(components(&self.directives, Some(ext))), root_operations: root_ops(Some(ext)), diff --git a/crates/apollo-compiler/test_data/diagnostics/0063_extension_orphan.txt b/crates/apollo-compiler/test_data/diagnostics/0063_extension_orphan.txt index 0314ec9bd..fe7fa1c95 100644 --- a/crates/apollo-compiler/test_data/diagnostics/0063_extension_orphan.txt +++ b/crates/apollo-compiler/test_data/diagnostics/0063_extension_orphan.txt @@ -12,14 +12,12 @@ Error: type extension for undefined type `B` │ ┬ │ ╰── extension here ───╯ -Error: schema extension without a schema definition - ╭─[0063_extension_orphan.graphql:7:1] +Error: duplicate definitions for the `query` root operation type + ╭─[0063_extension_orphan.graphql:8:3] │ - 7 │ ╭─▶ extend schema { - ┆ ┆ - 9 │ ├─▶ } - │ │ - │ ╰─────── extension here + 8 │ query: Query + │ ──────┬───── + │ ╰─────── `query` redefined here ───╯ Error: type extension for undefined type `C` ╭─[0063_extension_orphan.graphql:10:14] diff --git a/crates/apollo-compiler/tests/extensions.rs b/crates/apollo-compiler/tests/extensions.rs index 90de0ba67..fa5a0942d 100644 --- a/crates/apollo-compiler/tests/extensions.rs +++ b/crates/apollo-compiler/tests/extensions.rs @@ -10,7 +10,7 @@ fn test_orphan_extensions() { // By default, orphan extensions are errors: let schema = Schema::parse(input, "schema.graphql"); - assert!(!schema.schema_definition_directives().has("dir")); + assert!(!schema.schema_definition.directives.has("dir")); assert!(!schema.types.contains_key("Obj")); let err = schema.validate().unwrap_err().to_string_no_color(); assert!( @@ -27,7 +27,7 @@ fn test_orphan_extensions() { .adopt_orphan_extensions() .parse(input, "schema.graphql") .build(); - assert!(schema2.schema_definition_directives().has("dir")); + assert!(schema2.schema_definition.directives.has("dir")); assert!(schema2.types["Obj"].directives().has("dir")); schema2.validate().unwrap(); } @@ -53,3 +53,17 @@ fn test_orphan_extensions_kind_mismatch() { "{err}" ); } + +/// https://github.com/apollographql/apollo-rs/issues/682 +#[test] +fn test_extend_implicit_schema() { + let input = r#" + type Query { field: Int } # creates an implicit schema definition that can be extended + extend schema @dir + directive @dir on SCHEMA +"#; + + let schema = Schema::parse(input, "schema.graphql"); + schema.validate().unwrap(); + assert!(schema.schema_definition.directives.has("dir")); +} diff --git a/crates/apollo-compiler/tests/merge_schemas.rs b/crates/apollo-compiler/tests/merge_schemas.rs index c19bc2b16..4609cd941 100644 --- a/crates/apollo-compiler/tests/merge_schemas.rs +++ b/crates/apollo-compiler/tests/merge_schemas.rs @@ -14,18 +14,15 @@ fn merge_schemas(inputs: &[&str]) -> Result { let mut merged = Schema::new(); for &input in inputs { let schema = Schema::parse(input, "schema.graphql"); - merge_options_or( - &mut merged.schema_definition, - &schema.schema_definition, - |merged, new| { - let merged = merged.make_mut(); - merge_options(&mut merged.description, &new.description)?; - merge_vecs(&mut merged.directives, &new.directives)?; - merge_options(&mut merged.query, &new.query)?; - merge_options(&mut merged.mutation, &new.mutation)?; - merge_options(&mut merged.subscription, &new.subscription) - }, - )?; + { + let merged = merged.schema_definition.make_mut(); + let new = &schema.schema_definition; + merge_options(&mut merged.description, &new.description)?; + merge_vecs(&mut merged.directives, &new.directives)?; + merge_options(&mut merged.query, &new.query)?; + merge_options(&mut merged.mutation, &new.mutation)?; + merge_options(&mut merged.subscription, &new.subscription)? + } merge_maps( &mut merged.directive_definitions, &schema.directive_definitions, @@ -258,7 +255,8 @@ fn test_ok() { } "#, ]; - let expected = r#"type Query { + let expected = expect_test::expect![ + r#"type Query { t: T } @@ -278,6 +276,7 @@ enum E { V1 V2 } -"#; - assert_eq!(merge_schemas(&inputs).as_deref(), Ok(expected)) +"# + ]; + expected.assert_eq(&merge_schemas(&inputs).unwrap()); } diff --git a/crates/apollo-compiler/tests/schema.rs b/crates/apollo-compiler/tests/schema.rs index 294bced96..14e983efa 100644 --- a/crates/apollo-compiler/tests/schema.rs +++ b/crates/apollo-compiler/tests/schema.rs @@ -56,7 +56,8 @@ fn test_schema_reserialize() { directive @customDirective on OBJECT; "#; // Order is mostly not preserved - let expected = r#"directive @customDirective on OBJECT + let expected = expect_test::expect![ + r#"directive @customDirective on OBJECT type Query { int: Int @@ -75,9 +76,10 @@ extend type Query { interface Inter { string: String } -"#; +"# + ]; let schema = Schema::parse(input, "schema.graphql"); - assert_eq!(schema.to_string(), expected); + expected.assert_eq(&schema.to_string()); } #[test] @@ -192,3 +194,29 @@ const SUPERGRAPH_BOILERPLATE: &str = r#" } "#; + +/// https://github.com/graphql/graphql-spec/pull/987 +/// https://github.com/apollographql/apollo-rs/issues/682#issuecomment-1752661656 +#[test] +fn test_default_root_op_name_ignored_with_explicit_schema_def() { + let input = r#" + schema { + query: Query + # no mutation here + } + type Query { + viruses: [Virus!] + } + type Virus { + name: String! + knownMutations: [Mutation!]! + } + type Mutation { # happens to use that name but isn't a root operation + name: String! + geneSequence: String! + } + "#; + let schema = Schema::parse(input, "schema.graphql"); + schema.validate().unwrap(); + assert!(schema.schema_definition.mutation.is_none()) +} From 18739ff013a40fe1f2817a09301b4de958774d50 Mon Sep 17 00:00:00 2001 From: Irina Shestak Date: Tue, 10 Oct 2023 14:53:17 +0200 Subject: [PATCH 3/3] chore(compiler): implicit schema definition snapshot tests --- .../0108_implicit_schema_extension.graphql | 7 +++ .../0108_implicit_schema_extension.txt | 8 +++ ..._schema_definition_with_query_type.graphql | 3 + ...icit_schema_definition_with_query_type.txt | 18 ++++++ ...inition_with_several_default_types.graphql | 12 ++++ ..._definition_with_several_default_types.txt | 62 +++++++++++++++++++ ...it_schema_extension_with_directive.graphql | 6 ++ ...plicit_schema_extension_with_directive.txt | 36 +++++++++++ .../0108_implicit_schema_extension.graphql | 7 +++ ..._schema_definition_with_query_type.graphql | 3 + ...inition_with_several_default_types.graphql | 11 ++++ ...it_schema_extension_with_directive.graphql | 7 +++ 12 files changed, 180 insertions(+) create mode 100644 crates/apollo-compiler/test_data/diagnostics/0108_implicit_schema_extension.graphql create mode 100644 crates/apollo-compiler/test_data/diagnostics/0108_implicit_schema_extension.txt create mode 100644 crates/apollo-compiler/test_data/ok/0035_implicit_schema_definition_with_query_type.graphql create mode 100644 crates/apollo-compiler/test_data/ok/0035_implicit_schema_definition_with_query_type.txt create mode 100644 crates/apollo-compiler/test_data/ok/0036_implicit_schema_definition_with_several_default_types.graphql create mode 100644 crates/apollo-compiler/test_data/ok/0036_implicit_schema_definition_with_several_default_types.txt create mode 100644 crates/apollo-compiler/test_data/ok/0037_implicit_schema_extension_with_directive.graphql create mode 100644 crates/apollo-compiler/test_data/ok/0037_implicit_schema_extension_with_directive.txt create mode 100644 crates/apollo-compiler/test_data/serializer/diagnostics/0108_implicit_schema_extension.graphql create mode 100644 crates/apollo-compiler/test_data/serializer/ok/0035_implicit_schema_definition_with_query_type.graphql create mode 100644 crates/apollo-compiler/test_data/serializer/ok/0036_implicit_schema_definition_with_several_default_types.graphql create mode 100644 crates/apollo-compiler/test_data/serializer/ok/0037_implicit_schema_extension_with_directive.graphql diff --git a/crates/apollo-compiler/test_data/diagnostics/0108_implicit_schema_extension.graphql b/crates/apollo-compiler/test_data/diagnostics/0108_implicit_schema_extension.graphql new file mode 100644 index 000000000..e7fdc6af6 --- /dev/null +++ b/crates/apollo-compiler/test_data/diagnostics/0108_implicit_schema_extension.graphql @@ -0,0 +1,7 @@ +type Query { + name: String +} + +extend schema { + query: Query +} \ No newline at end of file diff --git a/crates/apollo-compiler/test_data/diagnostics/0108_implicit_schema_extension.txt b/crates/apollo-compiler/test_data/diagnostics/0108_implicit_schema_extension.txt new file mode 100644 index 000000000..cb8bfe5bb --- /dev/null +++ b/crates/apollo-compiler/test_data/diagnostics/0108_implicit_schema_extension.txt @@ -0,0 +1,8 @@ +Error: duplicate definitions for the `query` root operation type + ╭─[0108_implicit_schema_extension.graphql:6:5] + │ + 6 │ query: Query + │ ──────┬───── + │ ╰─────── `query` redefined here +───╯ + diff --git a/crates/apollo-compiler/test_data/ok/0035_implicit_schema_definition_with_query_type.graphql b/crates/apollo-compiler/test_data/ok/0035_implicit_schema_definition_with_query_type.graphql new file mode 100644 index 000000000..0a2cb427c --- /dev/null +++ b/crates/apollo-compiler/test_data/ok/0035_implicit_schema_definition_with_query_type.graphql @@ -0,0 +1,3 @@ +type Query { + name: String +} \ No newline at end of file diff --git a/crates/apollo-compiler/test_data/ok/0035_implicit_schema_definition_with_query_type.txt b/crates/apollo-compiler/test_data/ok/0035_implicit_schema_definition_with_query_type.txt new file mode 100644 index 000000000..c598ede98 --- /dev/null +++ b/crates/apollo-compiler/test_data/ok/0035_implicit_schema_definition_with_query_type.txt @@ -0,0 +1,18 @@ +0..31 @34 ObjectTypeDefinition { + description: None, + name: "Query", + implements_interfaces: [], + directives: [], + fields: [ + 17..29 @34 FieldDefinition { + description: None, + name: "name", + arguments: [], + ty: Named( + "String", + ), + directives: [], + }, + ], +} + diff --git a/crates/apollo-compiler/test_data/ok/0036_implicit_schema_definition_with_several_default_types.graphql b/crates/apollo-compiler/test_data/ok/0036_implicit_schema_definition_with_several_default_types.graphql new file mode 100644 index 000000000..a191c3303 --- /dev/null +++ b/crates/apollo-compiler/test_data/ok/0036_implicit_schema_definition_with_several_default_types.graphql @@ -0,0 +1,12 @@ +type Query { + name: String +} + +type Mutation { + add(name: String!): Result! +} + +type Result { + id: String +} + diff --git a/crates/apollo-compiler/test_data/ok/0036_implicit_schema_definition_with_several_default_types.txt b/crates/apollo-compiler/test_data/ok/0036_implicit_schema_definition_with_several_default_types.txt new file mode 100644 index 000000000..5077b44e8 --- /dev/null +++ b/crates/apollo-compiler/test_data/ok/0036_implicit_schema_definition_with_several_default_types.txt @@ -0,0 +1,62 @@ +0..31 @35 ObjectTypeDefinition { + description: None, + name: "Query", + implements_interfaces: [], + directives: [], + fields: [ + 17..29 @35 FieldDefinition { + description: None, + name: "name", + arguments: [], + ty: Named( + "String", + ), + directives: [], + }, + ], +} +33..81 @35 ObjectTypeDefinition { + description: None, + name: "Mutation", + implements_interfaces: [], + directives: [], + fields: [ + 52..79 @35 FieldDefinition { + description: None, + name: "add", + arguments: [ + 56..69 @35 InputValueDefinition { + description: None, + name: "name", + ty: 62..69 @35 NonNullNamed( + "String", + ), + default_value: None, + directives: [], + }, + ], + ty: NonNullNamed( + "Result", + ), + directives: [], + }, + ], +} +83..111 @35 ObjectTypeDefinition { + description: None, + name: "Result", + implements_interfaces: [], + directives: [], + fields: [ + 99..109 @35 FieldDefinition { + description: None, + name: "id", + arguments: [], + ty: Named( + "String", + ), + directives: [], + }, + ], +} + diff --git a/crates/apollo-compiler/test_data/ok/0037_implicit_schema_extension_with_directive.graphql b/crates/apollo-compiler/test_data/ok/0037_implicit_schema_extension_with_directive.graphql new file mode 100644 index 000000000..b61122496 --- /dev/null +++ b/crates/apollo-compiler/test_data/ok/0037_implicit_schema_extension_with_directive.graphql @@ -0,0 +1,6 @@ +type Query { + name: String +} + +extend schema @dir +directive @dir on SCHEMA \ No newline at end of file diff --git a/crates/apollo-compiler/test_data/ok/0037_implicit_schema_extension_with_directive.txt b/crates/apollo-compiler/test_data/ok/0037_implicit_schema_extension_with_directive.txt new file mode 100644 index 000000000..85220b61a --- /dev/null +++ b/crates/apollo-compiler/test_data/ok/0037_implicit_schema_extension_with_directive.txt @@ -0,0 +1,36 @@ +0..31 @36 ObjectTypeDefinition { + description: None, + name: "Query", + implements_interfaces: [], + directives: [], + fields: [ + 17..29 @36 FieldDefinition { + description: None, + name: "name", + arguments: [], + ty: Named( + "String", + ), + directives: [], + }, + ], +} +33..51 @36 SchemaExtension { + directives: [ + 47..51 @36 Directive { + name: "dir", + arguments: [], + }, + ], + root_operations: [], +} +52..76 @36 DirectiveDefinition { + description: None, + name: "dir", + arguments: [], + repeatable: false, + locations: [ + "SCHEMA", + ], +} + diff --git a/crates/apollo-compiler/test_data/serializer/diagnostics/0108_implicit_schema_extension.graphql b/crates/apollo-compiler/test_data/serializer/diagnostics/0108_implicit_schema_extension.graphql new file mode 100644 index 000000000..a3478aa80 --- /dev/null +++ b/crates/apollo-compiler/test_data/serializer/diagnostics/0108_implicit_schema_extension.graphql @@ -0,0 +1,7 @@ +type Query { + name: String +} + +extend schema { + query: Query +} diff --git a/crates/apollo-compiler/test_data/serializer/ok/0035_implicit_schema_definition_with_query_type.graphql b/crates/apollo-compiler/test_data/serializer/ok/0035_implicit_schema_definition_with_query_type.graphql new file mode 100644 index 000000000..6efd090a5 --- /dev/null +++ b/crates/apollo-compiler/test_data/serializer/ok/0035_implicit_schema_definition_with_query_type.graphql @@ -0,0 +1,3 @@ +type Query { + name: String +} diff --git a/crates/apollo-compiler/test_data/serializer/ok/0036_implicit_schema_definition_with_several_default_types.graphql b/crates/apollo-compiler/test_data/serializer/ok/0036_implicit_schema_definition_with_several_default_types.graphql new file mode 100644 index 000000000..8dee388a5 --- /dev/null +++ b/crates/apollo-compiler/test_data/serializer/ok/0036_implicit_schema_definition_with_several_default_types.graphql @@ -0,0 +1,11 @@ +type Query { + name: String +} + +type Mutation { + add(name: String!): Result! +} + +type Result { + id: String +} diff --git a/crates/apollo-compiler/test_data/serializer/ok/0037_implicit_schema_extension_with_directive.graphql b/crates/apollo-compiler/test_data/serializer/ok/0037_implicit_schema_extension_with_directive.graphql new file mode 100644 index 000000000..e87dab860 --- /dev/null +++ b/crates/apollo-compiler/test_data/serializer/ok/0037_implicit_schema_extension_with_directive.graphql @@ -0,0 +1,7 @@ +type Query { + name: String +} + +extend schema @dir + +directive @dir on SCHEMA