byron / treediff-rs Goto Github PK

Hi there! I'm trying to use this library to patch a Serializable Rust struct with incomming JSON data, but there seems to be a bug/misunderstanding that I am running into with the UpdateFilter implementation I'm trying to use. Here's a minimal failing example:

use serde::{Deserialize, Serialize};
use serde_json::Value;
use treediff::{self, diff, tools::Merger};

#[derive(Serialize, Deserialize, Debug, Clone)]
struct Example {
    username: String,
    password: String,
}
use std::borrow::Cow;
use treediff::tools::MutableFilter;

/// Mutable filter that will not remove deleted items
pub(crate) struct UpdateFilter;

impl MutableFilter for UpdateFilter {
    fn resolve_conflict<'a, K: Clone + std::fmt::Display, V: Clone>(
        &mut self,
        _keys: &[K],
        _old: &'a V,
        new: &'a V,
        _target: &mut V,
    ) -> Option<std::borrow::Cow<'a, V>> {
        println!("resolve_conflict");
        for key in _keys {
            println!("key: {}", key);
        }
        Some(Cow::Borrowed(new))
    }

    fn resolve_removal<'a, K: Clone + std::fmt::Display, V: Clone>(
        &mut self,
        _keys: &[K],
        removed: &'a V,
        target: &mut V,
    ) -> Option<std::borrow::Cow<'a, V>> {
        println!("resolve_removeal");
        for key in _keys {
            println!("key: {}", key);
        }
        Some(Cow::Borrowed(removed))
    }
}

fn main() {
    let original_data = vec![Example {
        username: "admin".into(),
        password: "password".into(),
    }];

    let original_serialized = serde_json::to_value(&original_data).unwrap();

    let diff_json = r#"[
        { "username":  "dude" }
    ]"#;

    let diff_value: Value = serde_json::from_str(diff_json).unwrap();

    let mut merger = Merger::with_filter(Value::Null, UpdateFilter);

    diff(&original_serialized, &diff_value, &mut merger);

    let output = merger.into_inner();

    dbg!(output.clone());

    let patched_data: Vec<Example> = serde_json::from_value(output).unwrap();

    dbg!(patched_data);
}

The output is:

resolve_conflict
key: 0
key: username
resolve_removeal
key: 0
key: password
[examples/treediff_test.rs:65] output.clone() = Array([
    Object({
        "password": String(
            "password",
        ),
    }),
])
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error("missing field `username`", line: 0, column: 0)', examples/treediff_test.rs:67:69
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

For some reason, instead of the output containing the updated user, and the old password, it only contains the old password.

By messing around with different implementations I found that the problem seems to be in the resolve_removal implementation, when I return Some(Cow::Borrowed(removed)) it somehow causes the removed key, i.e. "password" to be the only key returned in that mapping.

This seems like a bug, but I could just be misunderstanding something. It seems there isn't any combination that I can get to work with the current API. Any help would be appreciated!

Edit: I did realize after posting this that I can probably just implement Delegate for my own type to get more control and do what I want. Still I think this current behavior may be a bug so this might still be worth addressing.

Would be nice if a comparison function could be passed. And also be able to ignore the order of things.

In my program i read in json ["hello", "world"] and ["world", "hello"] this will be picked up as two changes, but i don't really care for the order in the array. Would be nice to be able to set per array if it needs to compare indices (the default) or not.

Another thing is that instead of array of strings i will have an array of objects and one property of an object should be ignored during comparison. [{ignore_this:"timestamp", compare_this: "important data"}, { .. another object ..}]

I think to design this in the library will not be so easy because the passed in function probably needs knowledge of what it is looking at (some path from the root struct) + access to parents + access to children (and therefor implicit access to siblings).

I can not fix this by manipulating the data i have up front. For example sorting the data and removing that one property from an object. Consider this these two arrays (string example, not with object example): ["hello", "world"] and ["hello", "lol", "world"]. The diff is as follows:

Unchanged([Index(0)], String("hello"))
Modified([Index(1)], String("world"), String("lol"))
Added([Index(2)], String("world"))

So i would need index insensitive diff and it would change to:

Unchanged(String("hello"))
Added(String("lol"))
Unchanged(String("world"))

Maybe there are already facilities in the library that have some of the things i asked for. But i'm not sure what to look for.

I would like to discriminate next diffs:

• sequences

  • reorder [a,c,b] became [c,b,a] (different from sorted)
  • add/remove/insert (seems support)
  • not set anymore [a,b], became [a,a,b]
  • sorted vs unsorted [b,a] became [a,b]

• maps (sequences of key values entries, sequences of 2-tuples):

  • similar to sequences when applied
  • sorted by key
  • duplicate of key
  • add/remove/insert (seems support)
  • key vs value updates

• considering primitives (like u128) depending on context be primitives (single value in place eq update) vs being vec of boolean numbers

these scenarios are based on realworld diff and equality considerations,

also relevant #5

Hi! This looks like a really cool+useful crate. I've got a question that is not really an issue but this felt like the best place to ask it.

I've got a handful of structs that I would like to diff. They're pretty simple (strings, ints, booleans) and they all implement Serialize+Deserialize from serde. Implementing Value manually on each structs feels like a hassle, so instead I'm serializing to JSON before diffing:

let item1_json: serde_json::Value = item1.serialize(serde_json::value::Serializer)?;
let item2_json: serde_json::Value = item2.serialize(serde_json::value::Serializer)?;
let mut recorder = Recorder::default();
treediff::diff(&item1_json, &item2_json, &mut recorder);

That works, but I'm still quite new to treediff and I'm wondering if that's the best approach. Is there a simpler approach that I'm missing? It would be really cool if treediff automatically worked on anything that implements Serialize+Deserialize (but I'm probably missing a good reason why it can't).

If that is not the case, you have the data, but it's not possible to do much with it.
Things might change with specializations though.

Hi, I am currently trying to update the package of the serde-yaml crate in Debian.
to 0.9.x I tried bumping the dependency in the treediff crate but the build fails with

   error[E0004]: non-exhaustive patterns: `serde_yaml::Value::Tagged(_)` not covered
      --> src/value/serde_yaml.rs:14:15
       |
    14 |         match *self {
       |               ^^^^^ pattern `serde_yaml::Value::Tagged(_)` not covered
       |

I would appreciate it if you can take a look and determine an appropriate way to handle
the new enum variant.

Would be cool to get a process step X of Y steps of the diff so that a progress bar can be shown. Maybe Y will need to be adjusted, but it's better than nothing.

Can only be done reasonably when the feature was stabilized:

rust-lang/rust#33417