Skip to content

Commit d3d96b6

Browse files
GREsauGREsau
authored
Improve handling of keyword ordering in schema serialization (#445)
Previously, all objects within schemas were effectively assumed to be schemas themselves.
1 parent d8efe53 commit d3d96b6

File tree

3 files changed

+200
-8
lines changed

3 files changed

+200
-8
lines changed

‎schemars/src/schema.rs

Lines changed: 69 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ impl Serialize for Schema {
8181
where
8282
S: serde::Serializer,
8383
{
84-
ser::OrderedKeywordWrapper(&self.0).serialize(serializer)
84+
ser::OrderedKeywordWrapper::from(&self.0).serialize(serializer)
8585
}
8686
}
8787

@@ -482,47 +482,108 @@ mod ser {
482482
];
483483
const ORDERED_KEYWORDS_END: [&str; 2] = ["$defs", "definitions"];
484484

485-
pub(super) struct OrderedKeywordWrapper<'a>(pub &'a Value);
485+
// `no_reorder` is true when the value is expected to be an object that is NOT a schema,
486+
// but the object's property values are expected to be schemas. In this case, we do not
487+
// reorder the object's direct properties, but we do reorder nested (subschema) properties.
488+
//
489+
// When `no_reorder` is false, then the value is expected to be one of:
490+
// - a JSON schema object
491+
// - an array of JSON schemas
492+
// - a JSON primitive value (null/string/number/bool)
493+
//
494+
// If any of these expectations are not met, then the value should still be serialized in a
495+
// valid way, but the property ordering may be unclear.
496+
pub(super) struct OrderedKeywordWrapper<'a> {
497+
value: &'a Value,
498+
no_reorder: bool,
499+
}
500+
501+
impl<'a> From<&'a Value> for OrderedKeywordWrapper<'a> {
502+
fn from(value: &'a Value) -> Self {
503+
Self {
504+
value,
505+
no_reorder: false,
506+
}
507+
}
508+
}
486509

487510
impl Serialize for OrderedKeywordWrapper<'_> {
488511
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
489512
where
490513
S: serde::Serializer,
491514
{
492-
match self.0 {
515+
fn serialize_schema_property<S>(
516+
map: &mut S::SerializeMap,
517+
key: &str,
518+
value: &Value,
519+
) -> Result<(), S::Error>
520+
where
521+
S: serde::Serializer,
522+
{
523+
if matches!(key, "examples" | "default") || key.starts_with("x-") {
524+
// Value(s) of `examples`/`default` are plain values, not schemas.
525+
// Also don't reorder values of custom properties.
526+
map.serialize_entry(key, value)
527+
} else {
528+
let no_reorder = matches!(
529+
key,
530+
"properties"
531+
| "patternProperties"
532+
| "dependentSchemas"
533+
| "$defs"
534+
| "definitions"
535+
);
536+
map.serialize_entry(key, &OrderedKeywordWrapper { value, no_reorder })
537+
}
538+
}
539+
540+
match self.value {
493541
Value::Array(array) => {
494542
let mut seq = serializer.serialize_seq(Some(array.len()))?;
495543
for value in array {
496-
seq.serialize_element(&OrderedKeywordWrapper(value))?;
544+
seq.serialize_element(&OrderedKeywordWrapper::from(value))?;
497545
}
498546
seq.end()
499547
}
548+
Value::Object(object) if self.no_reorder => {
549+
let mut map = serializer.serialize_map(Some(object.len()))?;
550+
551+
for (key, value) in object {
552+
// Don't use `serialize_schema_property` because `object` is NOT expected
553+
// to be a schema (but `value` is expected to be a schema)
554+
map.serialize_entry(key, &OrderedKeywordWrapper::from(value))?;
555+
}
556+
557+
map.end()
558+
}
500559
Value::Object(object) => {
501560
let mut map = serializer.serialize_map(Some(object.len()))?;
502561

503562
for key in ORDERED_KEYWORDS_START {
504563
if let Some(value) = object.get(key) {
505-
map.serialize_entry(key, &OrderedKeywordWrapper(value))?;
564+
serialize_schema_property::<S>(&mut map, key, value)?;
506565
}
507566
}
508567

509568
for (key, value) in object {
510569
if !ORDERED_KEYWORDS_START.contains(&key.as_str())
511570
&& !ORDERED_KEYWORDS_END.contains(&key.as_str())
512571
{
513-
map.serialize_entry(key, &OrderedKeywordWrapper(value))?;
572+
serialize_schema_property::<S>(&mut map, key, value)?;
514573
}
515574
}
516575

517576
for key in ORDERED_KEYWORDS_END {
518577
if let Some(value) = object.get(key) {
519-
map.serialize_entry(key, &OrderedKeywordWrapper(value))?;
578+
serialize_schema_property::<S>(&mut map, key, value)?;
520579
}
521580
}
522581

523582
map.end()
524583
}
525-
_ => self.0.serialize(serializer),
584+
Value::Null | Value::Bool(_) | Value::Number(_) | Value::String(_) => {
585+
self.value.serialize(serializer)
586+
}
526587
}
527588
}
528589
}

‎schemars/tests/integration/snapshots/schemars/tests/integration/structs.rs~property_order.json

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
{
2+
"$schema": "https://json-schema.org/draft/2020-12/schema",
3+
"title": "PropertyOrder",
4+
"type": "object",
5+
"properties": {
6+
"$defs": {
7+
"$ref": "#/$defs/UnitStruct"
8+
},
9+
"examples": {
10+
"$ref": "#/$defs/UnitStruct"
11+
},
12+
"properties": {
13+
"$ref": "#/$defs/UnitStruct"
14+
},
15+
"required": {
16+
"$ref": "#/$defs/UnitStruct"
17+
},
18+
"$schema": {
19+
"$ref": "#/$defs/UnitStruct"
20+
},
21+
"title": {
22+
"$ref": "#/$defs/UnitStruct"
23+
},
24+
"type": {
25+
"$ref": "#/$defs/UnitStruct"
26+
},
27+
"x-custom": {
28+
"$ref": "#/$defs/UnitStruct"
29+
}
30+
},
31+
"required": [
32+
"$defs",
33+
"examples",
34+
"properties",
35+
"required",
36+
"$schema",
37+
"title",
38+
"type",
39+
"x-custom"
40+
],
41+
"examples": [
42+
{
43+
"$defs": null,
44+
"examples": null,
45+
"properties": null,
46+
"required": null,
47+
"$schema": null,
48+
"title": null,
49+
"type": null,
50+
"x-custom": null
51+
}
52+
],
53+
"x-custom": {
54+
"$defs": null,
55+
"examples": null,
56+
"properties": null,
57+
"required": null,
58+
"$schema": null,
59+
"title": null,
60+
"type": null,
61+
"x-custom": null
62+
},
63+
"$defs": {
64+
"UnitStruct": {
65+
"type": "null"
66+
}
67+
}
68+
}

‎schemars/tests/integration/structs.rs

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,3 +87,66 @@ fn deny_unknown_fields() {
8787
})])
8888
.assert_matches_de_roundtrip(arbitrary_values());
8989
}
90+
91+
#[derive(JsonSchema, Deserialize, Serialize, Default)]
92+
#[schemars(example = PropertyOrder::default(), extend("x-custom" = PropertyOrder::default()))]
93+
struct PropertyOrder {
94+
#[serde(rename = "$defs")]
95+
pub defs: UnitStruct,
96+
pub examples: UnitStruct,
97+
pub properties: UnitStruct,
98+
pub required: UnitStruct,
99+
#[serde(rename = "$schema")]
100+
pub schema: UnitStruct,
101+
pub title: UnitStruct,
102+
pub r#type: UnitStruct,
103+
#[serde(rename = "x-custom")]
104+
pub x_custom: UnitStruct,
105+
}
106+
107+
#[cfg_attr(not(feature = "preserve_order"), ignore)]
108+
#[test]
109+
fn property_order() {
110+
test!(PropertyOrder).assert_snapshot().custom(|schema, _| {
111+
fn get_property_keys(value: &Value) -> Vec<&str> {
112+
value
113+
.as_object()
114+
.expect("expected value to be an object")
115+
.keys()
116+
.map(String::as_str)
117+
.collect()
118+
}
119+
120+
let value = serde_json::to_value(schema).unwrap();
121+
122+
assert!(matches!(
123+
get_property_keys(&value).as_slice(),
124+
// order of `examples`, `required` and `x-custom` is unspecified
125+
&["$schema", "title", "type", "properties", .., "$defs"]
126+
));
127+
128+
let field_order = &[
129+
"$defs",
130+
"examples",
131+
"properties",
132+
"required",
133+
"$schema",
134+
"title",
135+
"type",
136+
"x-custom",
137+
];
138+
139+
assert_eq!(
140+
get_property_keys(&value["properties"]).as_slice(),
141+
field_order,
142+
);
143+
assert_eq!(
144+
get_property_keys(&value["examples"][0]).as_slice(),
145+
field_order,
146+
);
147+
assert_eq!(
148+
get_property_keys(&value["x-custom"]).as_slice(),
149+
field_order,
150+
);
151+
});
152+
}

0 commit comments

Comments
 (0)