Update tests to account for schema name case changes, and fix flattening to make it consistent with previous (possibly buggy?) behaviour

Author: GREsau

schemars/src/_private.rs

@@ -89,7 +89,6 @@ pub fn new_internally_tagged_enum(
             }
         },
         "required": [tag_name],
-        "additionalProperties": false,
     });
 
     if deny_unknown_fields {

schemars/src/flatten.rs

@@ -1,3 +1,4 @@
+use serde_json::map::Entry;
 use serde_json::Value;
 
 use crate::schema::*;
@@ -8,16 +9,64 @@ impl Schema {
     /// It should not be considered part of the public API.
     #[doc(hidden)]
     pub fn flatten(mut self, other: Self) -> Schema {
-        if let Value::Object(obj2) = other.into() {
+        // This special null-type-schema handling is here for backward-compatibility, but needs reviewing.
+        // I think it's only needed to make internally-tagged enum unit variants behave correctly, but that
+        // should be handled entirely within schemars_derive.
+        if other
+            .as_object()
+            .and_then(|o| o.get("type"))
+            .and_then(|t| t.as_str())
+            == Some("null")
+        {
+            return self;
+        }
+
+        if let Value::Object(mut obj2) = other.to_value() {
             let obj1 = self.ensure_object();
 
+            let ap2 = obj2.remove("additionalProperties");
+            if let Entry::Occupied(mut ap1) = obj1.entry("additionalProperties") {
+                match ap2 {
+                    Some(ap2) => {
+                        flatten_additional_properties(ap1.get_mut(), ap2);
+                    }
+                    None => {
+                        ap1.remove();
+                    }
+                }
+            }
+
             for (key, value2) in obj2 {
                 match obj1.entry(key) {
-                    serde_json::map::Entry::Vacant(vacant) => {
+                    Entry::Vacant(vacant) => {
                         vacant.insert(value2);
                     }
-                    serde_json::map::Entry::Occupied(mut occupied) => {
+                    Entry::Occupied(mut occupied) => {
                         match occupied.key().as_str() {
+                            // This special "type" handling can probably be removed once the enum variant `with`/`schema_with` behaviour is fixed
+                            "type" => match (occupied.get_mut(), value2) {
+                                (Value::Array(a1), Value::Array(mut a2)) => {
+                                    a2.retain(|v2| !a1.contains(v2));
+                                    a1.extend(a2);
+                                }
+                                (v1, Value::Array(mut a2)) => {
+                                    if !a2.contains(v1) {
+                                        a2.push(std::mem::take(v1));
+                                        *occupied.get_mut() = Value::Array(a2);
+                                    }
+                                }
+                                (Value::Array(a1), v2) => {
+                                    if !a1.contains(&v2) {
+                                        a1.push(v2);
+                                    }
+                                }
+                                (v1, v2) => {
+                                    if v1 != &v2 {
+                                        *occupied.get_mut() =
+                                            Value::Array(vec![std::mem::take(v1), v2]);
+                                    }
+                                }
+                            },
                             "required" => {
                                 if let Value::Array(a1) = occupied.into_mut() {
                                     if let Value::Array(a2) = value2 {
@@ -32,10 +81,6 @@ impl Schema {
                                     }
                                 }
                             }
-                            "additionalProperties" => {
-                                let value1 = std::mem::take(occupied.get_mut());
-                                occupied.insert(flatten_additional_properties(value1, value2));
-                            }
                             _ => {
                                 // leave the original value as it is (don't modify `self`)
                             }
@@ -49,14 +94,18 @@ impl Schema {
     }
 }
 
-fn flatten_additional_properties(v1: Value, v2: Value) -> Value {
+// TODO validate behaviour when flattening a normal struct into a struct with deny_unknown_fields
+fn flatten_additional_properties(v1: &mut Value, v2: Value) {
     match (v1, v2) {
-        (Value::Bool(true), _) | (_, Value::Bool(true)) => Value::Bool(true),
-        (Value::Bool(false), v) | (v, Value::Bool(false)) => v,
-        (Value::Object(mut o1), Value::Object(o2)) => {
+        (v1, Value::Bool(true)) => {
+            *v1 = Value::Bool(true);
+        }
+        (v1 @ Value::Bool(false), v2) => {
+            *v1 = v2;
+        }
+        (Value::Object(o1), Value::Object(o2)) => {
             o1.extend(o2);
-            Value::Object(o1)
         }
-        (v, _) => v,
+        _ => {}
     }
 }

schemars/tests/enum.rs

@@ -31,6 +31,7 @@ enum External {
     },
     UnitTwo,
     Tuple(i32, bool),
+    // FIXME this should probably only replace the "payload" of the enum
     #[schemars(with = "i32")]
     WithInt,
 }
@@ -53,6 +54,7 @@ enum Internal {
         bar: bool,
     },
     UnitTwo,
+    // FIXME this should probably only replace the "payload" of the enum
     #[schemars(with = "i32")]
     WithInt,
 }
@@ -75,6 +77,7 @@ enum Untagged {
         bar: bool,
     },
     Tuple(i32, bool),
+    // FIXME this should probably only replace the "payload" of the enum
     #[schemars(with = "i32")]
     WithInt,
 }
@@ -98,6 +101,7 @@ enum Adjacent {
     },
     Tuple(i32, bool),
     UnitTwo,
+    // FIXME this should probably only replace the "payload" of the enum
     #[schemars(with = "i32")]
     WithInt,
 }

schemars/tests/enum_deny_unknown_fields.rs

@@ -33,6 +33,7 @@ enum External {
     },
     UnitTwo,
     Tuple(i32, bool),
+    // FIXME this should probably only replace the "payload" of the enum
     #[schemars(with = "i32")]
     WithInt,
 }
@@ -56,6 +57,7 @@ enum Internal {
         bar: bool,
     },
     UnitTwo,
+    // FIXME this should only replace the "payload" of the enum (which doesn't even make sense for unit enums!)
     #[schemars(with = "i32")]
     WithInt,
 }
@@ -79,6 +81,7 @@ enum Untagged {
         bar: bool,
     },
     Tuple(i32, bool),
+    // FIXME this should probably only replace the "payload" of the enum
     #[schemars(with = "i32")]
     WithInt,
 }
@@ -103,6 +106,7 @@ enum Adjacent {
     },
     Tuple(i32, bool),
     UnitTwo,
+    // FIXME this should probably only replace the "payload" of the enum
     #[schemars(with = "i32")]
     WithInt,
 }

schemars/tests/expected/enum-internal-duf.json

@@ -127,8 +127,7 @@
             "WithInt"
           ]
         }
-      },
-      "additionalProperties": false
+      }
     }
   ]
 }

schemars/tests/expected/range.json

@@ -15,7 +15,7 @@
       "$ref": "#/definitions/Range_of_double"
     },
     "bound": {
-      "$ref": "#/definitions/Bound_of_String"
+      "$ref": "#/definitions/Bound_of_string"
     }
   },
   "definitions": {
@@ -55,7 +55,7 @@
         }
       }
     },
-    "Bound_of_String": {
+    "Bound_of_string": {
       "oneOf": [
         {
           "type": "object",

schemars/tests/expected/remote_derive_generic.json

@@ -10,10 +10,10 @@
   ],
   "properties": {
     "byte_or_bool2": {
-      "$ref": "#/definitions/Or_for_uint8_and_Boolean"
+      "$ref": "#/definitions/Or_for_uint8_and_boolean"
     },
     "unit_or_t2": {
-      "$ref": "#/definitions/Or_for_Null_and_int32"
+      "$ref": "#/definitions/Or_for_null_and_int32"
     },
     "s": {
       "$ref": "#/definitions/Str"
@@ -30,7 +30,7 @@
     }
   },
   "definitions": {
-    "Or_for_uint8_and_Boolean": {
+    "Or_for_uint8_and_boolean": {
       "anyOf": [
         {
           "type": "integer",
@@ -42,7 +42,7 @@
         }
       ]
     },
-    "Or_for_Null_and_int32": {
+    "Or_for_null_and_int32": {
       "anyOf": [
         {
           "type": "null"

schemars/tests/expected/result.json

@@ -8,14 +8,14 @@
   ],
   "properties": {
     "result1": {
-      "$ref": "#/definitions/Result_of_MyStruct_or_Array_of_String"
+      "$ref": "#/definitions/Result_of_MyStruct_or_Array_of_string"
     },
     "result2": {
-      "$ref": "#/definitions/Result_of_Boolean_or_Null"
+      "$ref": "#/definitions/Result_of_boolean_or_null"
     }
   },
   "definitions": {
-    "Result_of_MyStruct_or_Array_of_String": {
+    "Result_of_MyStruct_or_Array_of_string": {
       "oneOf": [
         {
           "type": "object",
@@ -56,7 +56,7 @@
         }
       }
     },
-    "Result_of_Boolean_or_Null": {
+    "Result_of_boolean_or_null": {
       "oneOf": [
         {
           "type": "object",

schemars/tests/expected/schema-name-custom.json

@@ -1,6 +1,6 @@
 {
   "$schema": "http://json-schema.org/draft-07/schema#",
-  "title": "a-new-name-Array_of_String-int32-int32",
+  "title": "a-new-name-Array_of_string-int32-int32",
   "type": "object",
   "required": [
     "inner",

schemars/tests/expected/schema-name-default.json

@@ -1,15 +1,11 @@
 {
   "$schema": "http://json-schema.org/draft-07/schema#",
-  "title": "MyStruct_for_int32_and_Null_and_Boolean_and_Array_of_String",
+  "title": "MyStruct_for_int32_and_null_and_boolean_and_Array_of_string",
   "type": "object",
-  "required": [
-    "inner",
-    "t",
-    "u",
-    "v",
-    "w"
-  ],
   "properties": {
+    "inner": {
+      "$ref": "#/definitions/MySimpleStruct"
+    },
     "t": {
       "type": "integer",
       "format": "int32"
@@ -25,23 +21,27 @@
       "items": {
         "type": "string"
       }
-    },
-    "inner": {
-      "$ref": "#/definitions/MySimpleStruct"
     }
   },
+  "required": [
+    "inner",
+    "t",
+    "u",
+    "v",
+    "w"
+  ],
   "definitions": {
     "MySimpleStruct": {
       "type": "object",
-      "required": [
-        "foo"
-      ],
       "properties": {
         "foo": {
           "type": "integer",
           "format": "int32"
         }
-      }
+      },
+      "required": [
+        "foo"
+      ]
     }
   }
 }

schemars/tests/expected/schema-name-mixed-generics.json

@@ -1,6 +1,6 @@
 {
   "$schema": "http://json-schema.org/draft-07/schema#",
-  "title": "MixedGenericStruct_for_MyStruct_for_int32_and_Null_and_Boolean_and_Array_of_String_and_42_and_z",
+  "title": "MixedGenericStruct_for_MyStruct_for_int32_and_null_and_boolean_and_Array_of_string_and_42_and_z",
   "type": "object",
   "required": [
     "foo",
@@ -12,7 +12,7 @@
       "format": "int32"
     },
     "generic": {
-      "$ref": "#/definitions/MyStruct_for_int32_and_Null_and_Boolean_and_Array_of_String"
+      "$ref": "#/definitions/MyStruct_for_int32_and_null_and_boolean_and_Array_of_string"
     }
   },
   "definitions": {
@@ -28,7 +28,7 @@
         }
       }
     },
-    "MyStruct_for_int32_and_Null_and_Boolean_and_Array_of_String": {
+    "MyStruct_for_int32_and_null_and_boolean_and_Array_of_string": {
       "type": "object",
       "required": [
         "inner",

schemars/tests/expected/schema_with-enum-internal.json

@@ -22,8 +22,8 @@
     },
     {
       "type": [
-        "boolean",
-        "object"
+        "object",
+        "boolean"
       ],
       "required": [
         "typeProperty"
@@ -39,8 +39,8 @@
     },
     {
       "type": [
-        "boolean",
-        "object"
+        "object",
+        "boolean"
       ],
       "required": [
         "typeProperty"