mcintyre321 / JsonDiffPatch

Apache License 2.0
50 stars 25 forks source link

Incorrect (?) diffpatch when modifying collections #4

Open hcd opened 6 years ago

hcd commented 6 years ago

Hi,

I have an issue when creating diffpatches on my objects after I have modified a sub-collection on those objects. It all works fine as long as I add or remove items to the collection, but not when I both and and remove items. I've create a very simple unit test that demonstrates this. Is this an issue or "expected behavior" ?

Test:

` public class MyClass { public List MyList { get; set; } }

[TestClass]
public class AddRemoveDiffPatchTests
{
    [TestMethod]
    public void Test()
    {
        var differ = new JsonDiffer();
        var myVar = new MyClass();
        myVar.MyList= new List<string>() { "Value1", "Value2", "Value3", "Value4", "Value5" };
        JToken original = JToken.Parse(myVar.ToJson());
        //first let's remove a value from the list
        myVar.MyList.Remove("Value2");
        JToken modified1 = JToken.Parse(myVar.ToJson());
        var diff1 = differ.Diff(original, modified1);
        Assert.AreEqual(1, diff1.Operations.Count);//correct - 1 "remove" operation
        //now add a value to the list
        myVar.MyList.Add("Value6");
        JToken modified2 = JToken.Parse(myVar.ToJson());
        var diff2 = differ.Diff(modified1, modified2);
        Assert.AreEqual(1, diff2.Operations.Count);//correct - 1 "add" operation
        //now both remove a value and add a value 
        myVar.MyList.Remove("Value3");
        myVar.MyList.Add("Value7");
        JToken modified3 = JToken.Parse(myVar.ToJson());
        var diff3 = differ.Diff(modified2, modified3);
        //i would expect 2 operations, 1 remove and 1 add
        Assert.AreEqual(2, diff3.Operations.Count);//FAILS - there are 7 operations:  3 remove, 1 replace and 3 add

    }

}

`

Kind regards, Sven.

mcintyre321 commented 6 years ago

I've had a quick look, and it does seem to produce a patch document that can be used to make the correct final document, but with a highly non-optimal number of operations.

I think that the JsonDiffer probably needs extension to work with strings - I added an LCS algorithm which can produce good results for arrays, but need to be passed a suitable equality comparer.

I've only developed the library enough to work with the use cases I've encountered, but I'm very happy to accept pull requests, and give you some pointers/help if you would like to work on it

CodeGlitcher commented 3 years ago

We had the same problem, @idobosman has made a fix. See pr: 11