diff --git a/src/schema.rs b/src/schema.rs index 133c978..4948939 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -733,9 +733,6 @@ impl Compare { Ok(ret) } - // NOTE: Single-element allOf schemas are flattened by `try_compare_flattened` - // before reaching this function. Multi-element allOf would require semantic - // merging for proper comparison, so we just do an equality check. fn compare_schema_type_all_of( &mut self, comparison: SchemaComparison, @@ -743,18 +740,42 @@ impl Compare { old_all_of: Contextual<'_, &Vec>>, new_all_of: Contextual<'_, &Vec>>, ) -> anyhow::Result { - if old_all_of.as_ref() != new_all_of.as_ref() { - self.schema_push_change( + let old_schemas = old_all_of.as_ref(); + let new_schemas = new_all_of.as_ref(); + if old_schemas.len() != new_schemas.len() { + return self.schema_push_change( dry_run, - "unhandled, 'allOf' schema", + "allOf schema count changed", &old_all_of, &new_all_of, comparison, ChangeClass::Unhandled, ChangeDetails::UnknownDifference, + ); + } + + if old_schemas.len() == 1 { + let old_single_schema = old_all_of.append_deref(old_all_of.first().unwrap(), "0"); + let new_single_schema = new_all_of.append_deref(new_all_of.first().unwrap(), "0"); + + self.compare_schema_ref_helper( + dry_run, + comparison, + old_single_schema, + new_single_schema, ) - } else { + } else if old_schemas == new_schemas { Ok(true) + } else { + self.schema_push_change( + dry_run, + "allOf with multiple schemas is unhandled", + &old_all_of, + &new_all_of, + comparison, + ChangeClass::Unhandled, + ChangeDetails::UnknownDifference, + ) } } @@ -813,6 +834,7 @@ impl SchemaKindTag { } /// Classification of a schema reference for flattening purposes. +#[derive(Debug)] enum SchemaRefKind<'a> { /// A bare $ref. BareRef, diff --git a/tests/cases/simple/base.json b/tests/cases/simple/base.json index 9b0c33a..8318c57 100644 --- a/tests/cases/simple/base.json +++ b/tests/cases/simple/base.json @@ -345,6 +345,14 @@ } ] }, + "via_clone": { + "description": "Via clone of SubType using allOf.", + "allOf": [ + { + "$ref": "#/components/schemas/SubTypeClone" + } + ] + }, "not_a_number": { "not": { "type": "number" @@ -363,6 +371,14 @@ } } }, + "SubTypeClone": { + "description": "...", + "allOf": [ + { + "$ref": "#/components/schemas/SubType" + } + ] + }, "Tree": { "type": "object", "properties": { @@ -381,7 +397,9 @@ "type": "string" } }, - "required": ["name"] + "required": [ + "name" + ] }, "UpdateItem": { "type": "object", @@ -401,7 +419,10 @@ "type": "integer" } }, - "required": ["message", "code"] + "required": [ + "message", + "code" + ] }, "ArrayWithConstraints": { "type": "array", diff --git a/tests/cases/simple/output/allof-ref-to-identical-type.out b/tests/cases/simple/output/allof-ref-to-identical-type.out new file mode 100644 index 0000000..938dbad --- /dev/null +++ b/tests/cases/simple/output/allof-ref-to-identical-type.out @@ -0,0 +1,27 @@ +--- allof-ref-to-identical-type.json ++++ patched +@@ + "SubTypeClone": { + "allOf": [ + { +- "$ref": "#/components/schemas/SubType" ++ "$ref": "#/components/schemas/SubTypeV2" + } + ], + "description": "..." + }, ++ "SubTypeV2": { ++ "properties": { ++ "value": { ++ "type": "string" ++ } ++ }, ++ "type": "object" ++ }, + "Tree": { + "properties": { + "children": { + + +Result for patch: +[] diff --git a/tests/cases/simple/output/anyof-to-allof.out b/tests/cases/simple/output/anyof-to-allof.out index 7a6b7f9..54603e5 100644 --- a/tests/cases/simple/output/anyof-to-allof.out +++ b/tests/cases/simple/output/anyof-to-allof.out @@ -13,8 +13,8 @@ - "description": "Via anyOf." + "description": "Via allOf." }, - "via_oneof": { - "description": "Via oneOf.", + "via_clone": { + "allOf": [ Result for patch: diff --git a/tests/cases/simple/output/anyof-to-oneof.out b/tests/cases/simple/output/anyof-to-oneof.out index 6081cb5..7ff5610 100644 --- a/tests/cases/simple/output/anyof-to-oneof.out +++ b/tests/cases/simple/output/anyof-to-oneof.out @@ -14,8 +14,8 @@ - "description": "Via anyOf." + ] }, - "via_oneof": { - "description": "Via oneOf.", + "via_clone": { + "allOf": [ Result for patch: diff --git a/tests/cases/simple/output/anyof-to-ref.out b/tests/cases/simple/output/anyof-to-ref.out index 76839b1..d748325 100644 --- a/tests/cases/simple/output/anyof-to-ref.out +++ b/tests/cases/simple/output/anyof-to-ref.out @@ -12,8 +12,8 @@ - "description": "Via anyOf." + "$ref": "#/components/schemas/SubType" }, - "via_oneof": { - "description": "Via oneOf.", + "via_clone": { + "allOf": [ Result for patch: diff --git a/tests/cases/simple/output/multi-allof-variant-change.out b/tests/cases/simple/output/multi-allof-variant-change.out index ebb7f6e..9cf3348 100644 --- a/tests/cases/simple/output/multi-allof-variant-change.out +++ b/tests/cases/simple/output/multi-allof-variant-change.out @@ -14,7 +14,7 @@ Result for patch: [ Change { - message: "unhandled, 'allOf' schema", + message: "allOf with multiple schemas is unhandled", old_path: [ "#/components/schemas/MultiAllOf/allOf", "#/paths/~1allof/get/responses/200/content/application~1json/schema/$ref", diff --git a/tests/cases/simple/output/oneof-to-allof.out b/tests/cases/simple/output/oneof-to-allof.out index 5f3e337..7f44ff1 100644 --- a/tests/cases/simple/output/oneof-to-allof.out +++ b/tests/cases/simple/output/oneof-to-allof.out @@ -1,7 +1,7 @@ --- oneof-to-allof.json +++ patched @@ - "description": "Via anyOf." + "description": "Via clone of SubType using allOf." }, "via_oneof": { - "description": "Via oneOf.", diff --git a/tests/cases/simple/output/oneof-to-anyof.out b/tests/cases/simple/output/oneof-to-anyof.out index 16f2fb8..627d55f 100644 --- a/tests/cases/simple/output/oneof-to-anyof.out +++ b/tests/cases/simple/output/oneof-to-anyof.out @@ -1,7 +1,7 @@ --- oneof-to-anyof.json +++ patched @@ - "description": "Via anyOf." + "description": "Via clone of SubType using allOf." }, "via_oneof": { - "description": "Via oneOf.", diff --git a/tests/cases/simple/output/oneof-to-ref.out b/tests/cases/simple/output/oneof-to-ref.out index b859544..22ef9d6 100644 --- a/tests/cases/simple/output/oneof-to-ref.out +++ b/tests/cases/simple/output/oneof-to-ref.out @@ -1,7 +1,7 @@ --- oneof-to-ref.json +++ patched @@ - "description": "Via anyOf." + "description": "Via clone of SubType using allOf." }, "via_oneof": { - "description": "Via oneOf.", diff --git a/tests/cases/simple/output/schema-kind-type-to-oneof.out b/tests/cases/simple/output/schema-kind-type-to-oneof.out index da0321f..6c05f23 100644 --- a/tests/cases/simple/output/schema-kind-type-to-oneof.out +++ b/tests/cases/simple/output/schema-kind-type-to-oneof.out @@ -17,8 +17,8 @@ - "type": "object" + ] }, - "Tree": { - "properties": { + "SubTypeClone": { + "allOf": [ Result for patch: diff --git a/tests/cases/simple/patch/allof-ref-to-identical-type.json b/tests/cases/simple/patch/allof-ref-to-identical-type.json new file mode 100644 index 0000000..da98731 --- /dev/null +++ b/tests/cases/simple/patch/allof-ref-to-identical-type.json @@ -0,0 +1,21 @@ +[ + { + "op": "add", + "path": "/components/schemas/SubTypeV2", + "value": { + "type": "object", + "properties": { + "value": { + "type": "string" + } + } + } + }, + { + "op": "replace", + "path": "/components/schemas/SubTypeClone/allOf/0", + "value": { + "$ref": "#/components/schemas/SubTypeV2" + } + } +] diff --git a/tests/test_changes.rs b/tests/test_changes.rs index c222a9f..59d565b 100644 --- a/tests/test_changes.rs +++ b/tests/test_changes.rs @@ -24,7 +24,12 @@ fn test_change() { // Start by making sure that the base compares cleanly against // itself. let result = compare(&base_value, &base_value).expect("fatal failure in comparison"); - assert!(result.is_empty()); + if !result.is_empty() { + panic!( + "base.json does not compare cleanly against itself: {:#?}", + result + ); + } // Iterate through every file in the "patch" subdirectory. let patch_dir = path.join("patch");