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..b8493d629 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..a8227d24e 100644 --- a/crates/apollo-compiler/src/schema/mod.rs +++ b/crates/apollo-compiler/src/schema/mod.rs @@ -44,7 +44,7 @@ pub struct Schema { /// /// 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 +332,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 +386,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 +515,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 +825,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/tests/extensions.rs b/crates/apollo-compiler/tests/extensions.rs index 90de0ba67..8234e2d1d 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(); } 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..82051f361 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]