diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a3a58108..69d34df0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -107,7 +107,6 @@ jobs: - { feature: chrono-clock, crate: juniper } - { feature: chrono-tz, crate: juniper } - { feature: expose-test-schema, crate: juniper } - - { feature: graphql-parser, crate: juniper } - { feature: rust_decimal, crate: juniper } - { feature: schema-language, crate: juniper } - { feature: time, crate: juniper } diff --git a/book/src/schema/schemas_and_mutations.md b/book/src/schema/schemas_and_mutations.md index 2502f83d..716b76ce 100644 --- a/book/src/schema/schemas_and_mutations.md +++ b/book/src/schema/schemas_and_mutations.md @@ -89,17 +89,17 @@ fn main() { EmptySubscription::<()>::new(), ); - // Convert the Rust schema into the GraphQL Schema Language. - let result = schema.as_schema_language(); + // Convert the Rust schema into the GraphQL Schema Definition Language. + let result = schema.as_sdl(); let expected = "\ -type Query { - hello: String! -} - schema { query: Query } + +type Query { + hello: String! +} "; # #[cfg(not(target_os = "windows"))] assert_eq!(result, expected); diff --git a/juniper/CHANGELOG.md b/juniper/CHANGELOG.md index d57f4fd6..180c7956 100644 --- a/juniper/CHANGELOG.md +++ b/juniper/CHANGELOG.md @@ -55,6 +55,9 @@ All user visible changes to `juniper` crate will be documented in this file. Thi - Made `LookAheadMethods::children()` method to return slice instead of `Vec`. ([#1200]) - Abstracted `Spanning::start` and `Spanning::end` fields into separate struct `Span`. ([#1207], [#1208]) - Added `Span` to `Arguments` and `LookAheadArguments`. ([#1206], [#1209]) +- Removed `graphql-parser-integration` and `graphql-parser` [Cargo feature]s by merging them into `schema-language` [Cargo feature]. ([#1237]) +- Renamed `RootNode::as_schema_language()` method as `RootNode::as_sdl()`. ([#1237]) +- Renamed `RootNode::as_parser_document()` method as `RootNode::as_document()`. ([#1237]) ### Added @@ -89,6 +92,7 @@ All user visible changes to `juniper` crate will be documented in this file. Thi - Incorrect input value coercion with defaults. ([#1080], [#1073]) - Incorrect error when explicit `null` provided for `null`able list input parameter. ([#1086], [#1085]) - Stack overflow on nested GraphQL fragments. ([CVE-2022-31173]) +- Unstable definitions order in schema generated by `RootNode::as_sdl()`. ([#1237], [#1134]) [#103]: /../../issues/103 [#113]: /../../issues/113 @@ -132,6 +136,7 @@ All user visible changes to `juniper` crate will be documented in this file. Thi [#1086]: /../../pull/1086 [#1118]: /../../issues/1118 [#1119]: /../../pull/1119 +[#1134]: /../../issues/1134 [#1138]: /../../issues/1138 [#1145]: /../../pull/1145 [#1147]: /../../pull/1147 @@ -149,6 +154,7 @@ All user visible changes to `juniper` crate will be documented in this file. Thi [#1227]: /../../pull/1227 [#1228]: /../../pull/1228 [#1235]: /../../pull/1235 +[#1237]: /../../pull/1237 [ba1ed85b]: /../../commit/ba1ed85b3c3dd77fbae7baf6bc4e693321a94083 [CVE-2022-31173]: /../../security/advisories/GHSA-4rx6-g5vg-5f3j diff --git a/juniper/Cargo.toml b/juniper/Cargo.toml index fb764707..3b11fc3e 100644 --- a/juniper/Cargo.toml +++ b/juniper/Cargo.toml @@ -39,10 +39,9 @@ chrono = ["dep:chrono"] chrono-clock = ["chrono", "chrono/clock"] chrono-tz = ["dep:chrono-tz", "dep:regex"] expose-test-schema = ["dep:anyhow", "dep:serde_json"] -graphql-parser = ["dep:graphql-parser", "dep:void"] js = ["chrono?/wasmbind", "time?/wasm-bindgen", "uuid?/js"] rust_decimal = ["dep:rust_decimal"] -schema-language = ["graphql-parser"] +schema-language = ["dep:graphql-parser", "dep:void"] time = ["dep:time"] url = ["dep:url"] uuid = ["dep:uuid"] diff --git a/juniper/src/schema/model.rs b/juniper/src/schema/model.rs index 81c37636..7d1eed90 100644 --- a/juniper/src/schema/model.rs +++ b/juniper/src/schema/model.rs @@ -1,7 +1,7 @@ use std::{borrow::Cow, fmt}; use fnv::FnvHashMap; -#[cfg(feature = "graphql-parser")] +#[cfg(feature = "schema-language")] use graphql_parser::schema::Document; use crate::{ @@ -13,9 +13,6 @@ use crate::{ GraphQLEnum, }; -#[cfg(feature = "graphql-parser")] -use crate::schema::translate::{graphql_parser::GraphQLParserTranslator, SchemaTranslator}; - /// Root query node of a schema /// /// This brings the mutation, subscription and query types together, @@ -221,17 +218,40 @@ where } #[cfg(feature = "schema-language")] - /// The schema definition as a `String` in the - /// [GraphQL Schema Language](https://graphql.org/learn/schema/#type-language) - /// format. - pub fn as_schema_language(&self) -> String { - self.as_parser_document().to_string() + /// Returns this [`RootNode`] as a [`String`] containing the schema in [SDL (schema definition language)]. + /// + /// # Sorted + /// + /// The order of the generated definitions is stable and is sorted in the "type-then-name" manner. + /// + /// If another sorting order is required, then the [`as_document()`] method should be used, which allows to sort the + /// returned [`Document`] in the desired manner and then to convert it [`to_string()`]. + /// + /// [`as_document()`]: RootNode::as_document + /// [`to_string()`]: ToString::to_string + /// [0]: https://graphql.org/learn/schema#type-language + #[must_use] + pub fn as_sdl(&self) -> String { + use crate::schema::translate::graphql_parser::sort_schema_document; + + let mut doc = self.as_document(); + sort_schema_document(&mut doc); + doc.to_string() } - #[cfg(feature = "graphql-parser")] - /// The schema definition as a [`graphql_parser`](https://crates.io/crates/graphql-parser) - /// [`Document`](https://docs.rs/graphql-parser/latest/graphql_parser/schema/struct.Document.html). - pub fn as_parser_document(&'a self) -> Document<'a, &'a str> { + #[cfg(feature = "schema-language")] + /// Returns this [`RootNode`] as a [`graphql_parser`]'s [`Document`]. + /// + /// # Unsorted + /// + /// The order of the generated definitions in the returned [`Document`] is NOT stable and may change without any + /// real schema changes. + #[must_use] + pub fn as_document(&'a self) -> Document<'a, &'a str> { + use crate::schema::translate::{ + graphql_parser::GraphQLParserTranslator, SchemaTranslator as _, + }; + GraphQLParserTranslator::translate_schema(&self.schema) } } @@ -666,119 +686,141 @@ impl<'a, S> fmt::Display for TypeType<'a, S> { } #[cfg(test)] -mod test { - - #[cfg(feature = "graphql-parser")] - mod graphql_parser_integration { +mod root_node_test { + #[cfg(feature = "schema-language")] + mod as_document { use crate::{graphql_object, EmptyMutation, EmptySubscription, RootNode}; - #[test] - fn graphql_parser_doc() { - struct Query; - #[graphql_object] - impl Query { - fn blah() -> bool { - true - } + struct Query; + + #[graphql_object] + impl Query { + fn blah() -> bool { + true } + } + + #[test] + fn generates_correct_document() { let schema = RootNode::new( Query, EmptyMutation::<()>::new(), EmptySubscription::<()>::new(), ); let ast = graphql_parser::parse_schema::<&str>( + //language=GraphQL r#" type Query { - blah: Boolean! + blah: Boolean! } schema { query: Query } - "#, + "#, ) .unwrap(); - assert_eq!(ast.to_string(), schema.as_parser_document().to_string()); + + assert_eq!(ast.to_string(), schema.as_document().to_string()); } } #[cfg(feature = "schema-language")] - mod schema_language { + mod as_sdl { use crate::{ graphql_object, EmptyMutation, EmptySubscription, GraphQLEnum, GraphQLInputObject, GraphQLObject, GraphQLUnion, RootNode, }; - #[test] - fn schema_language() { - #[derive(GraphQLObject, Default)] - struct Cake { - fresh: bool, + #[derive(GraphQLObject, Default)] + struct Cake { + fresh: bool, + } + + #[derive(GraphQLObject, Default)] + struct IceCream { + cold: bool, + } + + #[derive(GraphQLUnion)] + enum GlutenFree { + Cake(Cake), + IceCream(IceCream), + } + + #[derive(GraphQLEnum)] + enum Fruit { + Apple, + Orange, + } + + #[derive(GraphQLInputObject)] + struct Coordinate { + latitude: f64, + longitude: f64, + } + + struct Query; + + #[graphql_object] + impl Query { + fn blah() -> bool { + true } - #[derive(GraphQLObject, Default)] - struct IceCream { - cold: bool, + + /// This is whatever's description. + fn whatever() -> String { + "foo".into() } - #[derive(GraphQLUnion)] - enum GlutenFree { - Cake(Cake), - IceCream(IceCream), + + fn arr(stuff: Vec<Coordinate>) -> Option<&'static str> { + (!stuff.is_empty()).then_some("stuff") } - #[derive(GraphQLEnum)] - enum Fruit { - Apple, - Orange, + + fn fruit() -> Fruit { + Fruit::Apple } - #[derive(GraphQLInputObject)] - struct Coordinate { - latitude: f64, - longitude: f64, - } - struct Query; - #[graphql_object] - impl Query { - fn blah() -> bool { - true - } - /// This is whatever's description. - fn whatever() -> String { - "foo".into() - } - fn arr(stuff: Vec<Coordinate>) -> Option<&'static str> { - (!stuff.is_empty()).then_some("stuff") - } - fn fruit() -> Fruit { - Fruit::Apple - } - fn gluten_free(flavor: String) -> GlutenFree { - if flavor == "savory" { - GlutenFree::Cake(Cake::default()) - } else { - GlutenFree::IceCream(IceCream::default()) - } - } - #[deprecated] - fn old() -> i32 { - 42 - } - #[deprecated(note = "This field is deprecated, use another.")] - fn really_old() -> f64 { - 42.0 + + fn gluten_free(flavor: String) -> GlutenFree { + if flavor == "savory" { + GlutenFree::Cake(Cake::default()) + } else { + GlutenFree::IceCream(IceCream::default()) } } - let schema = RootNode::new( + #[deprecated] + fn old() -> i32 { + 42 + } + + #[deprecated(note = "This field is deprecated, use another.")] + fn really_old() -> f64 { + 42.0 + } + } + + #[test] + fn generates_correct_sdl() { + let actual = RootNode::new( Query, EmptyMutation::<()>::new(), EmptySubscription::<()>::new(), ); - let ast = graphql_parser::parse_schema::<&str>( + let expected = graphql_parser::parse_schema::<&str>( + //language=GraphQL r#" - union GlutenFree = Cake | IceCream + schema { + query: Query + } enum Fruit { APPLE ORANGE } + input Coordinate { + latitude: Float! + longitude: Float! + } type Cake { fresh: Boolean! } @@ -795,17 +837,12 @@ mod test { old: Int! @deprecated reallyOld: Float! @deprecated(reason: "This field is deprecated, use another.") } - input Coordinate { - latitude: Float! - longitude: Float! - } - schema { - query: Query - } - "#, + union GlutenFree = Cake | IceCream + "#, ) .unwrap(); - assert_eq!(ast.to_string(), schema.as_schema_language()); + + assert_eq!(actual.as_sdl(), expected.to_string()); } } } diff --git a/juniper/src/schema/translate/graphql_parser.rs b/juniper/src/schema/translate/graphql_parser.rs index 7612b610..0fac42ca 100644 --- a/juniper/src/schema/translate/graphql_parser.rs +++ b/juniper/src/schema/translate/graphql_parser.rs @@ -307,3 +307,113 @@ where vec![] } } + +/// Sorts the provided [`Document`] in the "type-then-name" manner. +pub(crate) fn sort_schema_document<'a, T: Text<'a>>(document: &mut Document<'a, T>) { + document.definitions.sort_by(move |a, b| { + let type_cmp = sort_value::by_type(a).cmp(&sort_value::by_type(b)); + let name_cmp = sort_value::by_is_directive(a) + .cmp(&sort_value::by_is_directive(b)) + .then(sort_value::by_name(a).cmp(&sort_value::by_name(b))) + .then(sort_value::by_directive(a).cmp(&sort_value::by_directive(b))); + type_cmp.then(name_cmp) + }) +} + +/// Evaluation of a [`Definition`] weights for sorting. +mod sort_value { + use graphql_parser::schema::{Definition, Text, TypeDefinition, TypeExtension}; + + /// Returns a [`Definition`] sorting weight by its type. + pub(super) fn by_type<'a, T>(definition: &Definition<'a, T>) -> u8 + where + T: Text<'a>, + { + match definition { + Definition::SchemaDefinition(_) => 0, + Definition::DirectiveDefinition(_) => 1, + Definition::TypeDefinition(t) => match t { + TypeDefinition::Enum(_) => 2, + TypeDefinition::InputObject(_) => 4, + TypeDefinition::Interface(_) => 6, + TypeDefinition::Scalar(_) => 8, + TypeDefinition::Object(_) => 10, + TypeDefinition::Union(_) => 12, + }, + Definition::TypeExtension(e) => match e { + TypeExtension::Enum(_) => 3, + TypeExtension::InputObject(_) => 5, + TypeExtension::Interface(_) => 7, + TypeExtension::Scalar(_) => 9, + TypeExtension::Object(_) => 11, + TypeExtension::Union(_) => 13, + }, + } + } + + /// Returns a [`Definition`] sorting weight by its name. + pub(super) fn by_name<'b, 'a, T>(definition: &'b Definition<'a, T>) -> Option<&'b T::Value> + where + T: Text<'a>, + { + match definition { + Definition::SchemaDefinition(_) => None, + Definition::DirectiveDefinition(d) => Some(&d.name), + Definition::TypeDefinition(t) => match t { + TypeDefinition::Enum(d) => Some(&d.name), + TypeDefinition::InputObject(d) => Some(&d.name), + TypeDefinition::Interface(d) => Some(&d.name), + TypeDefinition::Scalar(d) => Some(&d.name), + TypeDefinition::Object(d) => Some(&d.name), + TypeDefinition::Union(d) => Some(&d.name), + }, + Definition::TypeExtension(e) => match e { + TypeExtension::Enum(d) => Some(&d.name), + TypeExtension::InputObject(d) => Some(&d.name), + TypeExtension::Interface(d) => Some(&d.name), + TypeExtension::Scalar(d) => Some(&d.name), + TypeExtension::Object(d) => Some(&d.name), + TypeExtension::Union(d) => Some(&d.name), + }, + } + } + + /// Returns a [`Definition`] sorting weight by its directive. + pub(super) fn by_directive<'b, 'a, T>(definition: &'b Definition<'a, T>) -> Option<&'b T::Value> + where + T: Text<'a>, + { + match definition { + Definition::SchemaDefinition(_) => None, + Definition::DirectiveDefinition(_) => None, + Definition::TypeDefinition(t) => match t { + TypeDefinition::Enum(d) => d.directives.first().map(|d| &d.name), + TypeDefinition::InputObject(d) => d.directives.first().map(|d| &d.name), + TypeDefinition::Interface(d) => d.directives.first().map(|d| &d.name), + TypeDefinition::Scalar(d) => d.directives.first().map(|d| &d.name), + TypeDefinition::Object(d) => d.directives.first().map(|d| &d.name), + TypeDefinition::Union(d) => d.directives.first().map(|d| &d.name), + }, + Definition::TypeExtension(e) => match e { + TypeExtension::Enum(d) => d.directives.first().map(|d| &d.name), + TypeExtension::InputObject(d) => d.directives.first().map(|d| &d.name), + TypeExtension::Interface(d) => d.directives.first().map(|d| &d.name), + TypeExtension::Scalar(d) => d.directives.first().map(|d| &d.name), + TypeExtension::Object(d) => d.directives.first().map(|d| &d.name), + TypeExtension::Union(d) => d.directives.first().map(|d| &d.name), + }, + } + } + + /// Returns a [`Definition`] sorting weight by whether it represents a directive. + pub(super) fn by_is_directive<'a, T>(definition: &Definition<'a, T>) -> u8 + where + T: Text<'a>, + { + match definition { + Definition::SchemaDefinition(_) => 0, + Definition::DirectiveDefinition(_) => 1, + _ => 2, + } + } +} diff --git a/juniper/src/schema/translate/mod.rs b/juniper/src/schema/translate/mod.rs index 06121850..99b57ed9 100644 --- a/juniper/src/schema/translate/mod.rs +++ b/juniper/src/schema/translate/mod.rs @@ -4,5 +4,5 @@ pub trait SchemaTranslator<'a, T> { fn translate_schema<S: 'a + ScalarValue>(s: &'a SchemaType<S>) -> T; } -#[cfg(feature = "graphql-parser")] +#[cfg(feature = "schema-language")] pub mod graphql_parser; diff --git a/juniper/src/tests/fixtures/starwars/schema_language.rs b/juniper/src/tests/fixtures/starwars/schema_language.rs index 044362f6..66234048 100644 --- a/juniper/src/tests/fixtures/starwars/schema_language.rs +++ b/juniper/src/tests/fixtures/starwars/schema_language.rs @@ -1,6 +1,6 @@ #![allow(missing_docs)] -/// The schema as a static/hardcoded GraphQL Schema Language. +/// The schema as a static/hardcoded GraphQL SDL (schema definition language). pub const STATIC_GRAPHQL_SCHEMA_DEFINITION: &str = include_str!("starwars.graphql"); #[cfg(test)] @@ -24,7 +24,7 @@ mod tests { EmptySubscription::<Database>::new(), ); - dbg!("{}", schema.as_schema_language()); + //dbg!("{}", schema.as_sdl()); // `include_str()` keeps line endings. `git` will sadly by default // convert them, making this test fail without runtime tweaks on @@ -32,11 +32,10 @@ mod tests { // // See https://github.com/rust-lang/rust/pull/63681. #[cfg(windows)] - let expected = &STATIC_GRAPHQL_SCHEMA_DEFINITION.replace("\r\n", "\n"); - + let expected = STATIC_GRAPHQL_SCHEMA_DEFINITION.replace("\r\n", "\n"); #[cfg(not(windows))] let expected = STATIC_GRAPHQL_SCHEMA_DEFINITION; - assert_eq!(expected, &schema.as_schema_language()); + assert_eq!(schema.as_sdl(), expected); } } diff --git a/juniper/src/tests/fixtures/starwars/starwars.graphql b/juniper/src/tests/fixtures/starwars/starwars.graphql index b54a3314..7fa76cba 100644 --- a/juniper/src/tests/fixtures/starwars/starwars.graphql +++ b/juniper/src/tests/fixtures/starwars/starwars.graphql @@ -1,22 +1,5 @@ -"A mechanical creature in the Star Wars universe." -type Droid implements Character { - "The id of the droid" - id: String! - "The name of the droid" - name: String - "The friends of the droid" - friends: [Character!]! - "Which movies they appear in" - appearsIn: [Episode!]! - "The primary function of the droid" - primaryFunction: String -} - -"The root query object of the schema" -type Query { - human("id of the human" id: String!): Human - droid("id of the droid" id: String!): Droid - hero("If omitted, returns the hero of the whole saga. If provided, returns the hero of that particular episode" episode: Episode): Character +schema { + query: Query } enum Episode { @@ -37,6 +20,20 @@ interface Character { appearsIn: [Episode!]! } +"A mechanical creature in the Star Wars universe." +type Droid implements Character { + "The id of the droid" + id: String! + "The name of the droid" + name: String + "The friends of the droid" + friends: [Character!]! + "Which movies they appear in" + appearsIn: [Episode!]! + "The primary function of the droid" + primaryFunction: String +} + "A humanoid creature in the Star Wars universe." type Human implements Character { "The id of the human" @@ -51,6 +48,9 @@ type Human implements Character { homePlanet: String } -schema { - query: Query +"The root query object of the schema" +type Query { + human("id of the human" id: String!): Human + droid("id of the droid" id: String!): Droid + hero("If omitted, returns the hero of the whole saga. If provided, returns the hero of that particular episode" episode: Episode): Character }