Sort order as "type-then-name" in introspection output (#1239, #1134)

This commit is contained in:
Audun Halland 2024-01-17 15:04:17 +01:00 committed by GitHub
parent c54137f7b4
commit a1bc775e00
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 1665 additions and 1627 deletions

View file

@ -103,6 +103,7 @@ All user visible changes to `juniper` crate will be documented in this file. Thi
- Incorrect error when explicit `null` provided for `null`able list input parameter. ([#1086], [#1085]) - Incorrect error when explicit `null` provided for `null`able list input parameter. ([#1086], [#1085])
- Stack overflow on nested GraphQL fragments. ([CVE-2022-31173]) - Stack overflow on nested GraphQL fragments. ([CVE-2022-31173])
- Unstable definitions order in schema generated by `RootNode::as_sdl()`. ([#1237], [#1134]) - Unstable definitions order in schema generated by `RootNode::as_sdl()`. ([#1237], [#1134])
- Unstable definitions order in schema generated by `introspect()` or other introspection queries. ([#1239], [#1134])
[#103]: /../../issues/103 [#103]: /../../issues/103
[#113]: /../../issues/113 [#113]: /../../issues/113
@ -165,6 +166,7 @@ All user visible changes to `juniper` crate will be documented in this file. Thi
[#1228]: /../../pull/1228 [#1228]: /../../pull/1228
[#1235]: /../../pull/1235 [#1235]: /../../pull/1235
[#1237]: /../../pull/1237 [#1237]: /../../pull/1237
[#1239]: /../../pull/1239
[ba1ed85b]: /../../commit/ba1ed85b3c3dd77fbae7baf6bc4e693321a94083 [ba1ed85b]: /../../commit/ba1ed85b3c3dd77fbae7baf6bc4e693321a94083
[CVE-2022-31173]: /../../security/advisories/GHSA-4rx6-g5vg-5f3j [CVE-2022-31173]: /../../security/advisories/GHSA-4rx6-g5vg-5f3j

View file

@ -417,7 +417,13 @@ impl<'a, S> SchemaType<'a, S> {
/// Get a list of types. /// Get a list of types.
pub fn type_list(&self) -> Vec<TypeType<S>> { pub fn type_list(&self) -> Vec<TypeType<S>> {
self.types.values().map(|t| TypeType::Concrete(t)).collect() let mut types = self
.types
.values()
.map(|t| TypeType::Concrete(t))
.collect::<Vec<_>>();
sort_concrete_types(&mut types);
types
} }
/// Get a list of concrete types. /// Get a list of concrete types.
@ -443,7 +449,9 @@ impl<'a, S> SchemaType<'a, S> {
/// Get a list of directives. /// Get a list of directives.
pub fn directive_list(&self) -> Vec<&DirectiveType<S>> { pub fn directive_list(&self) -> Vec<&DirectiveType<S>> {
self.directives.values().collect() let mut directives = self.directives.values().collect::<Vec<_>>();
sort_directives(&mut directives);
directives
} }
/// Get directive by name. /// Get directive by name.
@ -685,6 +693,66 @@ impl<'a, S> fmt::Display for TypeType<'a, S> {
} }
} }
/// Sorts the provided [`TypeType`]s in the "type-then-name" manner.
fn sort_concrete_types<S>(types: &mut [TypeType<S>]) {
types.sort_by(|a, b| {
concrete_type_sort::by_type(a)
.cmp(&concrete_type_sort::by_type(b))
.then_with(|| concrete_type_sort::by_name(a).cmp(&concrete_type_sort::by_name(b)))
});
}
/// Sorts the provided [`DirectiveType`]s by name.
fn sort_directives<S>(directives: &mut [&DirectiveType<S>]) {
directives.sort_by(|a, b| a.name.cmp(&b.name));
}
/// Evaluation of a [`TypeType`] weights for sorting (for concrete types only).
///
/// Used for deterministic introspection output.
mod concrete_type_sort {
use crate::meta::MetaType;
use super::TypeType;
/// Returns a [`TypeType`] sorting weight by its type.
pub fn by_type<S>(t: &TypeType<S>) -> u8 {
match t {
TypeType::Concrete(MetaType::Enum(_)) => 0,
TypeType::Concrete(MetaType::InputObject(_)) => 1,
TypeType::Concrete(MetaType::Interface(_)) => 2,
TypeType::Concrete(MetaType::Scalar(_)) => 3,
TypeType::Concrete(MetaType::Object(_)) => 4,
TypeType::Concrete(MetaType::Union(_)) => 5,
// NOTE: The following types are not part of the introspected types.
TypeType::Concrete(
MetaType::List(_) | MetaType::Nullable(_) | MetaType::Placeholder(_),
) => 6,
// NOTE: Other variants will not appear since we're only sorting concrete types.
TypeType::List(..) | TypeType::NonNull(_) => 7,
}
}
/// Returns a [`TypeType`] sorting weight by its name.
pub fn by_name<'a, S>(t: &'a TypeType<'a, S>) -> Option<&'a str> {
match t {
TypeType::Concrete(MetaType::Enum(meta)) => Some(&meta.name),
TypeType::Concrete(MetaType::InputObject(meta)) => Some(&meta.name),
TypeType::Concrete(MetaType::Interface(meta)) => Some(&meta.name),
TypeType::Concrete(MetaType::Scalar(meta)) => Some(&meta.name),
TypeType::Concrete(MetaType::Object(meta)) => Some(&meta.name),
TypeType::Concrete(MetaType::Union(meta)) => Some(&meta.name),
TypeType::Concrete(
// NOTE: The following types are not part of the introspected types.
MetaType::List(_) | MetaType::Nullable(_) | MetaType::Placeholder(_),
)
// NOTE: Other variants will not appear since we're only sorting concrete types.
| TypeType::List(..)
| TypeType::NonNull(_) => None,
}
}
}
#[cfg(test)] #[cfg(test)]
mod root_node_test { mod root_node_test {
#[cfg(feature = "schema-language")] #[cfg(feature = "schema-language")]

View file

@ -282,11 +282,11 @@ impl<'a, S: ScalarValue + 'a> TypeType<'a, S> {
TypeType::Concrete(&MetaType::Interface(InterfaceMeta { TypeType::Concrete(&MetaType::Interface(InterfaceMeta {
name: ref iface_name, name: ref iface_name,
.. ..
})) => Some( })) => {
context let mut type_names = context
.concrete_type_list() .types
.iter() .values()
.filter_map(|&ct| { .filter_map(|ct| {
if let MetaType::Object(ObjectMeta { if let MetaType::Object(ObjectMeta {
name, name,
interface_names, interface_names,
@ -295,15 +295,21 @@ impl<'a, S: ScalarValue + 'a> TypeType<'a, S> {
{ {
interface_names interface_names
.iter() .iter()
.any(|name| name == iface_name) .any(|iname| iname == iface_name)
.then(|| context.type_by_name(name)) .then(|| name.as_ref())
.flatten()
} else { } else {
None None
} }
}) })
.collect(), .collect::<Vec<_>>();
), type_names.sort();
Some(
type_names
.into_iter()
.filter_map(|n| context.type_by_name(n))
.collect(),
)
}
_ => None, _ => None,
} }
} }

View file

@ -1,5 +1,7 @@
use std::collections::HashSet; use std::collections::HashSet;
use pretty_assertions::assert_eq;
use crate::{ use crate::{
graphql_vars, graphql_vars,
introspection::IntrospectionFormat, introspection::IntrospectionFormat,
@ -184,14 +186,20 @@ async fn test_introspection_directives() {
EmptySubscription::<Database>::new(), EmptySubscription::<Database>::new(),
); );
let mut result = crate::execute(q, None, &schema, &graphql_vars! {}, &database) let result = crate::execute(q, None, &schema, &graphql_vars! {}, &database)
.await .await
.unwrap(); .unwrap();
sort_schema_value(&mut result.0);
let mut expected = graphql_value!({ let expected = graphql_value!({
"__schema": { "__schema": {
"directives": [ "directives": [
{
"name": "deprecated",
"locations": [
"FIELD_DEFINITION",
"ENUM_VALUE",
],
},
{ {
"name": "include", "name": "include",
"locations": [ "locations": [
@ -208,13 +216,6 @@ async fn test_introspection_directives() {
"INLINE_FRAGMENT", "INLINE_FRAGMENT",
], ],
}, },
{
"name": "deprecated",
"locations": [
"FIELD_DEFINITION",
"ENUM_VALUE",
],
},
{ {
"name": "specifiedBy", "name": "specifiedBy",
"locations": [ "locations": [
@ -224,7 +225,6 @@ async fn test_introspection_directives() {
], ],
}, },
}); });
sort_schema_value(&mut expected);
assert_eq!(result, (expected, vec![])); assert_eq!(result, (expected, vec![]));
} }
@ -286,9 +286,9 @@ async fn test_builtin_introspection_query() {
EmptyMutation::<Database>::new(), EmptyMutation::<Database>::new(),
EmptySubscription::<Database>::new(), EmptySubscription::<Database>::new(),
); );
let mut result = crate::introspect(&schema, &database, IntrospectionFormat::default()).unwrap(); let result = crate::introspect(&schema, &database, IntrospectionFormat::default()).unwrap();
sort_schema_value(&mut result.0);
let expected = schema_introspection_result(); let expected = schema_introspection_result();
assert_eq!(result, (expected, vec![])); assert_eq!(result, (expected, vec![]));
} }
@ -301,9 +301,8 @@ async fn test_builtin_introspection_query_without_descriptions() {
EmptySubscription::<Database>::new(), EmptySubscription::<Database>::new(),
); );
let mut result = let result =
crate::introspect(&schema, &database, IntrospectionFormat::WithoutDescriptions).unwrap(); crate::introspect(&schema, &database, IntrospectionFormat::WithoutDescriptions).unwrap();
sort_schema_value(&mut result.0);
let expected = schema_introspection_result_without_descriptions(); let expected = schema_introspection_result_without_descriptions();
assert_eq!(result, (expected, vec![])); assert_eq!(result, (expected, vec![]));

File diff suppressed because it is too large Load diff