Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

compare anyOf based on handmade diff score #32

Merged
merged 9 commits into from
May 17, 2023
+284 −33
Merged

compare anyOf based on handmade diff score #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6c59eec
fill subschemas on length mismatch
6293 May 16, 2023
47ed7cd
compare anyOf based on handmade diff score
6293 May 16, 2023
e3d2f8e
add test on multiple number schemas within anyOf
6293 May 16, 2023
b0ff30c
use `diff` to compute the score
6293 May 17, 2023
deb50d9
`hungarian` is dead; use `pathfinding` instead
6293 May 17, 2023
0ebbfe3
make use of `Vec::resize`
6293 May 17, 2023
7b4cc56
make `Diffwalker` take a callback
6293 May 17, 2023
35fa7c1
make generic
untitaker May 17, 2023
6b315ac
fmt
untitaker May 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
89 changes: 62 additions & 27 deletions src/diff_walker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems you're trying to get the vectors to be the same length by padding the shorter one with false.

if that is the case I think you can do this:

let max_len = cmp::max(lhs_any_of.len(), rhs_any_of.len());
lhs_any_of.resize(max_len, Schema::Bool(false));
rhs_any_of.resize(max_len, Schema::Bool(false));

}
(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(),
)?;
}
}
Expand All @@ -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(),
Expand All @@ -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(),
Expand All @@ -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() },
});
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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(())
}
Expand All @@ -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(),
Expand Down Expand Up @@ -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(),
Expand All @@ -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(),
Expand Down Expand Up @@ -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(),
Expand All @@ -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(),
Expand Down Expand Up @@ -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()
Expand Down
12 changes: 6 additions & 6 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
16 changes: 16 additions & 0 deletions tests/fixtures/any_of/number_1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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}
]
}
}
18 changes: 18 additions & 0 deletions tests/fixtures/any_of/objects_1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"}}}
]
}
}
19 changes: 19 additions & 0 deletions tests/fixtures/any_of/objects_2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"}}}
]
}
}
19 changes: 19 additions & 0 deletions tests/fixtures/any_of/objects_3.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"}}}
]
}
}
58 changes: 58 additions & 0 deletions tests/snapshots/test__from_fixtures@any_of__number_1.json.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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,
},
},
]
Loading
Loading