compare anyOf based on handmade diff score (#32)

6293 · untitaker · web-flow · commit 0276ef28685c · 2023-05-17T18:35:53.000+02:00
Co-authored-by: Markus Unterwaditzer &lt;markus-tarpit+git@unterwaditzer.net&gt;
diff --git a/Cargo.toml b/Cargo.toml
@@ -25,6 +25,7 @@ schemars = { version = "0.8.12", default_features = false }
 serde = "1.0.158"
 serde_json = "1.0.94"
 thiserror = "1.0.40"
+pathfinding = "4.2.1"
 
 [features]
 build-binary = ["clap", "anyhow"]
diff --git a/src/diff_walker.rs b/src/diff_walker.rs
@@ -8,13 +8,21 @@ use serde_json::Value;
 
 use crate::{Change, ChangeKind, Error, JsonSchemaType, Range};
 
-pub struct DiffWalker {
-    pub changes: Vec<Change>,
+pub struct DiffWalker<F: FnMut(Change)> {
+    pub cb: F,
     pub lhs_root: RootSchema,
     pub rhs_root: RootSchema,
 }
 
-impl DiffWalker {
+impl<F: FnMut(Change)> DiffWalker<F> {
+    pub fn new(cb: F, lhs_root: RootSchema, rhs_root: RootSchema) -> Self {
+        Self {
+            cb,
+            lhs_root,
+            rhs_root,
+        }
+    }
+
     fn diff_any_of(
         &mut self,
         json_path: &str,
@@ -27,21 +35,47 @@ impl DiffWalker {
         if let (Some(lhs_any_of), Some(rhs_any_of)) =
             (&mut lhs.subschemas().any_of, &mut rhs.subschemas().any_of)
         {
-            lhs_any_of.sort_by_cached_key(|x| format!("{x:?}"));
-            rhs_any_of.sort_by_cached_key(|x| format!("{x:?}"));
-
-            for (i, (lhs_inner, rhs_inner)) in
-                lhs_any_of.iter_mut().zip(rhs_any_of.iter_mut()).enumerate()
-            {
+            match (lhs_any_of.len(), rhs_any_of.len()) {
+                (l, r) if l <= r => {
+                    lhs_any_of.append(&mut vec![Schema::Bool(false); r - l]);
+                }
+                (l, r) => {
+                    rhs_any_of.append(&mut vec![Schema::Bool(false); l - r]);
+                }
+            }
+            let max_len = lhs_any_of.len().max(rhs_any_of.len());
+            lhs_any_of.resize(max_len, Schema::Bool(false));
+            rhs_any_of.resize(max_len, Schema::Bool(false));
+
+            let mut mat = pathfinding::matrix::Matrix::new(max_len, max_len, 0i32);
+            for (i, l) in lhs_any_of.iter_mut().enumerate() {
+                for (j, r) in rhs_any_of.iter_mut().enumerate() {
+                    let mut count = 0;
+                    let counter = |_change: Change| count += 1;
+                    DiffWalker::new(
+                        Box::new(counter) as Box<dyn FnMut(Change)>,
+                        self.lhs_root.clone(),
+                        self.rhs_root.clone(),
+                    )
+                    .diff(
+                        "",
+                        &mut l.clone().into_object(),
+                        &mut r.clone().into_object(),
+                    )?;
+                    mat[(i, j)] = count;
+                }
+            }
+            let pairs = pathfinding::kuhn_munkres::kuhn_munkres_min(&mat).1;
+            for i in 0..max_len {
                 let new_path = match is_rhs_split {
                     true => json_path.to_owned(),
-                    false => format!("{json_path}.<anyOf:{i}>"),
+                    false => format!("{json_path}.<anyOf:{}>", pairs[i]),
                 };
                 self.do_diff(
                     &new_path,
                     true,
-                    &mut lhs_inner.clone().into_object(),
-                    &mut rhs_inner.clone().into_object(),
+                    &mut lhs_any_of[i].clone().into_object(),
+                    &mut rhs_any_of[pairs[i]].clone().into_object(),
                 )?;
             }
         }
@@ -59,7 +93,7 @@ impl DiffWalker {
         let rhs_ty = rhs.effective_type().into_set();
 
         for removed in lhs_ty.difference(&rhs_ty) {
-            self.changes.push(Change {
+            (self.cb)(Change {
                 path: json_path.to_owned(),
                 change: ChangeKind::TypeRemove {
                     removed: removed.clone(),
@@ -68,7 +102,7 @@ impl DiffWalker {
         }
 
         for added in rhs_ty.difference(&lhs_ty) {
-            self.changes.push(Change {
+            (self.cb)(Change {
                 path: json_path.to_owned(),
                 change: ChangeKind::TypeAdd {
                     added: added.clone(),
@@ -81,25 +115,25 @@ impl DiffWalker {
         Self::normalize_const(lhs);
         Self::normalize_const(rhs);
         match (&lhs.const_value, &rhs.const_value) {
-            (Some(value), None) => self.changes.push(Change {
+            (Some(value), None) => (self.cb)(Change {
                 path: json_path.to_owned(),
                 change: ChangeKind::ConstRemove {
                     removed: value.clone(),
                 },
             }),
-            (None, Some(value)) => self.changes.push(Change {
+            (None, Some(value)) => (self.cb)(Change {
                 path: json_path.to_owned(),
                 change: ChangeKind::ConstAdd {
                     added: value.clone(),
                 },
             }),
             (Some(l), Some(r)) if l != r => {
                 if l.is_object() && r.is_object() {}
-                self.changes.push(Change {
+                (self.cb)(Change {
                     path: json_path.to_owned(),
                     change: ChangeKind::ConstRemove { removed: l.clone() },
                 });
-                self.changes.push(Change {
+                (self.cb)(Change {
                     path: json_path.to_owned(),
                     change: ChangeKind::ConstAdd { added: r.clone() },
                 });
@@ -124,7 +158,7 @@ impl DiffWalker {
             .map_or(true, |x| x.clone().into_object().is_true());
 
         for removed in lhs_props.difference(&rhs_props) {
-            self.changes.push(Change {
+            (self.cb)(Change {
                 path: json_path.to_owned(),
                 change: ChangeKind::PropertyRemove {
                     lhs_additional_properties,
@@ -134,7 +168,7 @@ impl DiffWalker {
         }
 
         for added in rhs_props.difference(&lhs_props) {
-            self.changes.push(Change {
+            (self.cb)(Change {
                 path: json_path.to_owned(),
                 change: ChangeKind::PropertyAdd {
                     lhs_additional_properties,
@@ -218,14 +252,14 @@ impl DiffWalker {
             rhs.number_validation().minimum,
             Range::Minimum,
         ) {
-            self.changes.push(diff)
+            (self.cb)(diff)
         }
         if let Some(diff) = diff(
             lhs.number_validation().maximum,
             rhs.number_validation().maximum,
             Range::Maximum,
         ) {
-            self.changes.push(diff)
+            (self.cb)(diff)
         }
         Ok(())
     }
@@ -239,7 +273,7 @@ impl DiffWalker {
         match (&lhs.array().items, &rhs.array().items) {
             (Some(SingleOrVec::Vec(lhs_items)), Some(SingleOrVec::Vec(rhs_items))) => {
                 if lhs_items.len() != rhs_items.len() {
-                    self.changes.push(Change {
+                    (self.cb)(Change {
                         path: json_path.to_owned(),
                         change: ChangeKind::TupleChange {
                             new_length: rhs_items.len(),
@@ -267,7 +301,7 @@ impl DiffWalker {
                 )?;
             }
             (Some(SingleOrVec::Single(lhs_inner)), Some(SingleOrVec::Vec(rhs_items))) => {
-                self.changes.push(Change {
+                (self.cb)(Change {
                     path: json_path.to_owned(),
                     change: ChangeKind::ArrayToTuple {
                         new_length: rhs_items.len(),
@@ -284,7 +318,7 @@ impl DiffWalker {
                 }
             }
             (Some(SingleOrVec::Vec(lhs_items)), Some(SingleOrVec::Single(rhs_inner))) => {
-                self.changes.push(Change {
+                (self.cb)(Change {
                     path: json_path.to_owned(),
                     change: ChangeKind::TupleToArray {
                         old_length: lhs_items.len(),
@@ -321,7 +355,7 @@ impl DiffWalker {
         let rhs_required = &rhs.object().required;
 
         for removed in lhs_required.difference(rhs_required) {
-            self.changes.push(Change {
+            (self.cb)(Change {
                 path: json_path.to_owned(),
                 change: ChangeKind::RequiredRemove {
                     property: removed.clone(),
@@ -330,7 +364,7 @@ impl DiffWalker {
         }
 
         for added in rhs_required.difference(lhs_required) {
-            self.changes.push(Change {
+            (self.cb)(Change {
                 path: json_path.to_owned(),
                 change: ChangeKind::RequiredAdd {
                     property: added.clone(),
@@ -532,6 +566,7 @@ impl JsonSchemaExt for SchemaObject {
             self.subschemas()
                 .any_of
                 .as_ref()
+                .filter(|schemas| schemas.len() == 1)
                 .and_then(|a| a.get(0))
                 .map(|subschema| subschema.clone().into_object().number().clone())
                 .unwrap_or_default()
diff --git a/src/lib.rs b/src/lib.rs
@@ -14,20 +14,20 @@ pub use types::*;
 ///
 /// `lhs` (left-hand side) is the old schema, `rhs` (right-hand side) is the new schema.
 pub fn diff(lhs: Value, rhs: Value) -> Result<Vec<Change>, Error> {
-    let changes = Vec::new();
     let lhs_root: RootSchema = serde_json::from_value(lhs)?;
     let rhs_root: RootSchema = serde_json::from_value(rhs)?;
 
-    let mut walker = diff_walker::DiffWalker {
-        changes,
-        lhs_root,
-        rhs_root,
+    let mut changes = vec![];
+    let cb = |change: Change| {
+        changes.push(change);
     };
+    let mut walker = diff_walker::DiffWalker::new(Box::new(cb), lhs_root, rhs_root);
     walker.diff(
         "",
         &mut walker.lhs_root.schema.clone(),
         &mut walker.rhs_root.schema.clone(),
     )?;
+    drop(walker);
 
-    Ok(walker.changes)
+    Ok(changes)
 }
diff --git a/tests/fixtures/any_of/number_1.json b/tests/fixtures/any_of/number_1.json
@@ -0,0 +1,16 @@
+{
+  "lhs": {
+    "anyOf": [
+      {"type": "number", "maximum": 10},
+      {"type": "number", "minimum": 1},
+      {"type": "number", "minimum": 100, "maximum": 200}
+    ]
+  },
+  "rhs": {
+    "anyOf": [
+      {"type": "number", "minimum": 7, "maximum": 14},
+      {"type": "number", "maximum": 3},
+      {"type": "number", "minimum": 2}
+    ]
+  }
+}
diff --git a/tests/fixtures/any_of/objects_1.json b/tests/fixtures/any_of/objects_1.json
@@ -0,0 +1,18 @@
+{
+  "lhs": {
+    "anyOf": [
+      {"properties": {"foo": {}}},
+      {"properties": {"type": {"const": "bar"}}}
+    ]
+  },
+  "rhs": {
+    "anyOf": [
+      {
+        "title": "replay_recording",
+        "type": "object",
+        "properties": {"foo": {}}
+      },
+      {"properties": {"type": {"const": "bar"}}}
+    ]
+  }
+}
diff --git a/tests/fixtures/any_of/objects_2.json b/tests/fixtures/any_of/objects_2.json
@@ -0,0 +1,19 @@
+{
+  "lhs": {
+    "anyOf": [
+      {"properties": {"foo": {}}},
+      {"properties": {"type": {"const": "bar"}}}
+    ]
+  },
+  "rhs": {
+    "anyOf": [
+      { "type": "boolean" },
+      {
+        "title": "replay_recording",
+        "type": "object",
+        "properties": {"foo": {}}
+      },
+      {"properties": {"type": {"const": "bar"}}}
+    ]
+  }
+}
diff --git a/tests/fixtures/any_of/objects_3.json b/tests/fixtures/any_of/objects_3.json
@@ -0,0 +1,19 @@
+{
+  "lhs": {
+    "anyOf": [
+      { "type": "boolean" },
+      {"properties": {"foo": {}}},
+      {"properties": {"type": {"const": "bar"}}}
+    ]
+  },
+  "rhs": {
+    "anyOf": [
+      {
+        "title": "replay_recording",
+        "type": "object",
+        "properties": {"foo": {}}
+      },
+      {"properties": {"type": {"const": "bar"}}}
+    ]
+  }
+}
diff --git a/tests/snapshots/test__from_fixtures@any_of__number_1.json.snap b/tests/snapshots/test__from_fixtures@any_of__number_1.json.snap
@@ -0,0 +1,58 @@
+---
+source: tests/test.rs
+expression: diff
+info:
+  lhs:
+    anyOf:
+      - maximum: 10
+        type: number
+      - minimum: 1
+        type: number
+      - maximum: 200
+        minimum: 100
+        type: number
+  rhs:
+    anyOf:
+      - maximum: 14
+        minimum: 7
+        type: number
+      - maximum: 3
+        type: number
+      - minimum: 2
+        type: number
+input_file: tests/fixtures/any_of/number_1.json
+---
+[
+    Change {
+        path: ".<anyOf:1>",
+        change: RangeChange {
+            changed: Maximum,
+            old_value: 10.0,
+            new_value: 3.0,
+        },
+    },
+    Change {
+        path: ".<anyOf:2>",
+        change: RangeChange {
+            changed: Minimum,
+            old_value: 1.0,
+            new_value: 2.0,
+        },
+    },
+    Change {
+        path: ".<anyOf:0>",
+        change: RangeChange {
+            changed: Minimum,
+            old_value: 100.0,
+            new_value: 7.0,
+        },
+    },
+    Change {
+        path: ".<anyOf:0>",
+        change: RangeChange {
+            changed: Maximum,
+            old_value: 200.0,
+            new_value: 14.0,
+        },
+    },
+]
diff --git a/tests/snapshots/test__from_fixtures@any_of__objects_1.json.snap b/tests/snapshots/test__from_fixtures@any_of__objects_1.json.snap
diff --git a/tests/snapshots/test__from_fixtures@any_of__objects_2.json.snap b/tests/snapshots/test__from_fixtures@any_of__objects_2.json.snap
diff --git a/tests/snapshots/test__from_fixtures@any_of__objects_3.json.snap b/tests/snapshots/test__from_fixtures@any_of__objects_3.json.snap
