Comparing cloned lists

[K-Menki]

Hello,

thanks for this really helpful project. It really helped me!

Now to my question:

I have created a clone from a graph of JPA mapped entities to implement a versioning of that graph. When I compared the cloned one with the original one, I saw that every list inside my root object was marked as changed. After analysing the problem, I realized that the equals() implementations of the objects inside the lists are the reason for that. The CollectionDiffer uses the equals() Methods of these Objects inside the Collections.filteredCopyOf() Method.

I tried to implement a ComparisonStrategy, but I was unhappy about the missing differDispatcher, so I stopped this approach.

How would you describe a best practice for my problem (Collection Diff only comparing unexcluded fields)?

Best regards

Kim

[SQiShER]

Can you elaborate on what's wrong with the equals method of the objects in the lists? java-object-diff relies heavily on the constraints of the standard collection types, which in turn expect items to implement equals and hashCode (and in some cases compareTo) properly, because otherwise none of the lookup functions (contains, remove, etc.) would work reliably.

A ComparisonStrategy wouldn't help you in this case, because they work on a higher abstraction level. When working with collection types there is just no escape when it comes to implementing equals and hashCode of the items. So if I were you, I'd try to do that no matter what.

If you can't make equals work on the objects directly, maybe you can wrap them in a decorator that implements it properly? If that doesn't work, do you have an idea what needs to be done to deal with this kind of scenario? I'd be happy to hear your thoughts, because right now, I'm not really sure.

However, maybe I don't yet fully understand your problem. If you could come up with a nice little (executable) example or even better, a unit test, that reproduces the problem, that would make it much clearer to me.

[K-Menki]

My problem is that in my case equals is implemented by comparing the id field of the entity classes. So if I clone one of these objects and persist them into database by using em.merge(object) the clone objects will contain the new primary keys generated from the database. This works in the way I want it.

The problem results if I compare 2 object graphs. Even if nothing is changed, the id's will be different in the cloned version. I annotated the getter methods of the id fields with @ObjectDiffProperty(excluded = true). The diff for one of the objects will result in UNTOUCHED if I just diff every object inside the list for itself. But the list will have state CHANGED. This is inconsistent with facts that every element in the list is UNTOUCHED, the list has them same number of contained items and the order of the items has not changed.

Your idea to wrap the objects in a decorator is one of the best solutions for my problem that remains on the list. I could perhaps implement an generic decorator that implements equals and hashcode only by comparing the unexcluded 'not annotated with @ObjectDiffProperty(excluded = true)) properties of the decorated class. But applying this decorator to every object of the obect graph seems to be much more work.

I have made some unit tests to reproduce my problem. Perhaps it is helpful to understand the problem:

package de.eis.fba2.util;

import de.danielbechler.diff.introspection.ObjectDiffProperty;
import de.danielbechler.diff.node.DiffNode;
import org.junit.Before;
import org.junit.Test;

import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import static org.junit.Assert.assertEquals;

public class DiffToolTest {

    public static class Foo {

        private Integer id;
        private String text;
        private Integer number;
        private List<Bar> bars;

        public Foo() {
        }

        public Foo(Foo other) {
            this.id = other.id;
            this.text = other.text;
            this.number = other.number;
            this.bars = cloneFooList(other.bars, Bar.class, this);
        }

        @ObjectDiffProperty(excluded = true)
        public Integer getId() {
            return id;
        }

        public void setId(Integer id) {
            this.id = id;
        }

        @ObjectDiffProperty(excluded = false)
        public String getText() {
            return text;
        }

        public void setText(String text) {
            this.text = text;
        }

        @ObjectDiffProperty(excluded = false)
        public Integer getNumber() {
            return number;
        }

        public void setNumber(Integer number) {
            this.number = number;
        }

        @ObjectDiffProperty(excluded = false)
        public List<Bar> getBars() {
            return bars;
        }

        public void setBars(List<Bar> bars) {
            this.bars = bars;
        }

        public void resetIdRecursive() {
            this.id = null;
            for(Bar bar : getBars()) {
                bar.resetIdRecursive();
            }
        }

        @Override
        public boolean equals(Object o) {
            if (this == o) return true;
            if (o == null || getClass() != o.getClass()) return false;

            Foo foo = (Foo) o;

            // Equals is implemented on field id because id contains the primary identifier from the database table
            if (id != null ? !id.equals(foo.id) : foo.id != null) return false;

            return true;
        }

        @Override
        public int hashCode() {
            return id != null ? id.hashCode() : 0;
        }
    }

    public static class Bar {

        Integer id;
        Foo foo;
        String text;
        Integer number;

        public Bar() {
        }

        public Bar(Bar other) {
            this.id = other.id;
            this.foo = other.getFoo();
            this.text = other.text;
            this.number = other.number;
        }

        public Bar(Bar other, Foo parent) {
            this(other);
            this.foo = parent;
        }

        @ObjectDiffProperty(excluded = true)
        public Integer getId() {
            return id;
        }

        public void setId(Integer id) {
            this.id = id;
        }

        public Foo getFoo() {
            return foo;
        }

        public void setFoo(Foo foo) {
            this.foo = foo;
        }

        public String getText() {
            return text;
        }

        public void setText(String text) {
            this.text = text;
        }

        public Integer getNumber() {
            return number;
        }

        public void setNumber(Integer number) {
            this.number = number;
        }

        public void resetIdRecursive() {
            this.id = null;
        }

        @Override
        public boolean equals(Object o) {
            if (this == o) return true;
            if (o == null || getClass() != o.getClass()) return false;

            Bar bar = (Bar) o;

            // Equals is implemented on field id because id contains the primary identifier from the database table
            if (id != null ? !id.equals(bar.id) : bar.id != null) return false;

            return true;
        }

        @Override
        public int hashCode() {
            return id != null ? id.hashCode() : 0;
        }
    }

    public static <E> List cloneFooList(List<E> source, Class<E> sourceClass, Foo parent) {
        return cloneChildList(source, sourceClass, parent, parent.getClass());
    }

    private static <E, F> List<E> cloneChildList(List<E> source, Class<E> childClass, F parent, Class<? extends F> parentClass) {
        Constructor<? extends E> constructor;
        try {
            constructor = childClass.getConstructor(childClass, parentClass);
        } catch (NoSuchMethodException e) {
            throw new RuntimeException(e);
        }
        ArrayList<E> result = new ArrayList<E>();
        if (source != null) {
            for (E element : source) {
                try {
                    result.add(constructor.newInstance(element, parent));
                } catch (InstantiationException e) {
                    throw new RuntimeException(e);
                } catch (IllegalAccessException e) {
                    throw new RuntimeException(e);
                } catch (InvocationTargetException e) {
                    throw new RuntimeException(e);
                }
            }
        }
        return result;
    }

    Foo foo;
    Bar bar1, bar2;

    @Before
    public void setUp() throws Exception {
        foo = new Foo();
        foo.setId(1);
        foo.setNumber(4711);
        foo.setText("foo for ever");

        bar1 = new Bar();
        bar1.setId(10);
        bar1.setNumber(815);
        bar1.setText("bar for ever 1");
        bar1.setFoo(foo);

        bar2 = new Bar();
        bar1.setId(11);
        bar1.setNumber(816);
        bar1.setText("bar for ever 2");
        bar1.setFoo(foo);

        foo.setBars(Arrays.asList(bar1, bar2));
    }

    @Test
    public void testBarUntouchedAfterClone() throws Exception {

        bar1 = new Bar();
        bar1.setId(10);
        bar1.setNumber(815);
        bar1.setText("bar for ever 1");

        Bar clonedBar = new Bar(bar1);

        clonedBar.setId(bar1.getId() + 1);

        DiffNode diffNode = DiffTool.compareObjects(clonedBar, bar1);
        assertEquals(DiffNode.State.UNTOUCHED, diffNode.getState());
    }

    @Test
    public void testUntouchedAfterClone() throws Exception {

        // Clone Object
        Foo clonedFoo = new Foo(foo);

        // Clear IDs, so em.merge() will give the clones new IDs
        clonedFoo.resetIdRecursive();

        // Simulation new IDs in Test Case
        clonedFoo.setId(2);
        clonedFoo.getBars().get(0).setId(20);
        clonedFoo.getBars().get(1).setId(21);

        DiffNode diffNode = DiffTool.compareObjects(clonedFoo, foo);
        assertEquals(DiffNode.State.UNTOUCHED, diffNode.getState());
    }

    @Test
    public void testChangedAfterClone() throws Exception {

        // Clone Object
        Foo clonedFoo = new Foo(foo);

        // Clear IDs, so em.merge() will give the clones new IDs
        clonedFoo.resetIdRecursive();

        // Simulation new IDs in Test Case
        clonedFoo.setId(2);
        clonedFoo.getBars().get(0).setId(20);
        clonedFoo.getBars().get(1).setId(21);

        // Change something
        clonedFoo.getBars().get(0).setText("changedBar");

        DiffNode diffNode = DiffTool.compareObjects(clonedFoo, foo);
        assertEquals(DiffNode.State.CHANGED, diffNode.getState());
    }
}

DiffTool.compareObjects is just this:

public class DiffTool {

    public static DiffNode compareObjects(Object now, Object before) {

        Object realNow, realBefore;
        if(now instanceof HibernateProxy) {
            realNow = ((HibernateProxy) now).getHibernateLazyInitializer()
                    .getImplementation();
        } else {
            realNow = now;
        }

        if(before instanceof HibernateProxy) {
            realBefore = ((HibernateProxy) before).getHibernateLazyInitializer()
                    .getImplementation();
        } else {
            realBefore = before;
        }

        return ObjectDifferBuilder
                .startBuilding()
                .inclusion().exclude().type(org.hibernate.proxy.pojo.javassist.JavassistLazyInitializer.class).and()
                .inclusion().exclude().propertyName("hibernateLazyInitializer").and()
                .inclusion().exclude().propertyName("erstelltAm").and()
                .inclusion().exclude().propertyName("erstelltDurch").and()
                .inclusion().exclude().propertyName("geaendertAm").and()
                .inclusion().exclude().propertyName("geaendertDurch").and()
                .inclusion().exclude().propertyName("vlc").and()
                .filtering().returnNodesWithState(DiffNode.State.ADDED).and()
                .filtering().returnNodesWithState(DiffNode.State.CHANGED).and()
                .filtering().returnNodesWithState(DiffNode.State.REMOVED).and()
                .filtering().returnNodesWithState(DiffNode.State.UNTOUCHED).and()
                .introspection().setDefaultIntrospector(new HibernateLazyIntrospector()).and()
                .build()
                .compare(realNow, realBefore);
    }

[massfords]

Does your problem go away if you remove the db generated ID from equals and hashCode? If so, there's your solution.

Whether this is preferable or not is not suitable for this list but if you skim some of the Hibernate/JPA books or google JPA entities equals/hashCode you'll see lots of similar recommendations.

On Aug 2, 2014, at 2:03 AM, K-Menki notifications@github.com wrote:

My problem is that in my case equals is implemented by comparing the id field of the entity classes. So if I clone one of these objects and persist them into database by using em.merge(object) the clone objects will contain the new primary keys generated from the database. This works in the way I want it.

The problem results if I compare 2 object graphs. Even if nothing is changed, the id's will be different in the cloned version. I annotated the getter methods of the id fields with @ObjectDiffProperty(excluded = true). The diff for one of the objects will result in UNTOUCHED if I just diff every object inside the list for itself. But the list will have state CHANGED. This is inconsistent with facts that every element in the list is UNTOUCHED, the list has them same number of contained items and the order of the items has not changed.

Your idea to wrap the objects in a decorator is one of the best solutions for my problem that remains on the list. I could perhaps implement an generic decorator that implements equals and hashcode only by comparing the unexcluded 'not annotated with @ObjectDiffProperty(excluded = true)) properties of the decorated class. But applying this decorator to every object of the obect graph seems to be much more work.

I have made some unit tests to reproduce my problem. Perhaps it is helpful to understand the problem:

package de.eis.fba2.util;

import de.danielbechler.diff.introspection.ObjectDiffProperty;
import de.danielbechler.diff.node.DiffNode;
import org.junit.Before;
import org.junit.Test;

import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import static org.junit.Assert.assertEquals;

public class DiffToolTest {

public static class Foo {

    private Integer id;
    private String text;
    private Integer number;
    private List<Bar> bars;

    public Foo() {
    }

    public Foo(Foo other) {
        this.id = other.id;
        this.text = other.text;
        this.number = other.number;
        this.bars = cloneFooList(other.bars, Bar.class, this);
    }

    @ObjectDiffProperty(excluded = true)
    public Integer getId() {
        return id;
    }

    public void setId(Integer id) {
        this.id = id;
    }

    @ObjectDiffProperty(excluded = false)
    public String getText() {
        return text;
    }

    public void setText(String text) {
        this.text = text;
    }

    @ObjectDiffProperty(excluded = false)
    public Integer getNumber() {
        return number;
    }

    public void setNumber(Integer number) {
        this.number = number;
    }

    @ObjectDiffProperty(excluded = false)
    public List<Bar> getBars() {
        return bars;
    }

    public void setBars(List<Bar> bars) {
        this.bars = bars;
    }

    public void resetIdRecursive() {
        this.id = null;
        for(Bar bar : getBars()) {
            bar.resetIdRecursive();
        }
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;

        Foo foo = (Foo) o;

        // Equals is implemented on field id because id contains the primary identifier from the database table
        if (id != null ? !id.equals(foo.id) : foo.id != null) return false;

        return true;
    }

    @Override
    public int hashCode() {
        return id != null ? id.hashCode() : 0;
    }
}

public static class Bar {

    Integer id;
    Foo foo;
    String text;
    Integer number;

    public Bar() {
    }

    public Bar(Bar other) {
        this.id = other.id;
        this.foo = other.getFoo();
        this.text = other.text;
        this.number = other.number;
    }

    public Bar(Bar other, Foo parent) {
        this(other);
        this.foo = parent;
    }

    @ObjectDiffProperty(excluded = true)
    public Integer getId() {
        return id;
    }

    public void setId(Integer id) {
        this.id = id;
    }

    public Foo getFoo() {
        return foo;
    }

    public void setFoo(Foo foo) {
        this.foo = foo;
    }

    public String getText() {
        return text;
    }

    public void setText(String text) {
        this.text = text;
    }

    public Integer getNumber() {
        return number;
    }

    public void setNumber(Integer number) {
        this.number = number;
    }

    public void resetIdRecursive() {
        this.id = null;
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;

        Bar bar = (Bar) o;

        // Equals is implemented on field id because id contains the primary identifier from the database table
        if (id != null ? !id.equals(bar.id) : bar.id != null) return false;

        return true;
    }

    @Override
    public int hashCode() {
        return id != null ? id.hashCode() : 0;
    }
}

public static <E> List cloneFooList(List<E> source, Class<E> sourceClass, Foo parent) {
    return cloneChildList(source, sourceClass, parent, parent.getClass());
}

private static <E, F> List<E> cloneChildList(List<E> source, Class<E> childClass, F parent, Class<? extends F> parentClass) {
    Constructor<? extends E> constructor;
    try {
        constructor = childClass.getConstructor(childClass, parentClass);
    } catch (NoSuchMethodException e) {
        throw new RuntimeException(e);
    }
    ArrayList<E> result = new ArrayList<E>();
    if (source != null) {
        for (E element : source) {
            try {
                result.add(constructor.newInstance(element, parent));
            } catch (InstantiationException e) {
                throw new RuntimeException(e);
            } catch (IllegalAccessException e) {
                throw new RuntimeException(e);
            } catch (InvocationTargetException e) {
                throw new RuntimeException(e);
            }
        }
    }
    return result;
}

Foo foo;
Bar bar1, bar2;

@Before
public void setUp() throws Exception {
    foo = new Foo();
    foo.setId(1);
    foo.setNumber(4711);
    foo.setText("foo for ever");

    bar1 = new Bar();
    bar1.setId(10);
    bar1.setNumber(815);
    bar1.setText("bar for ever 1");
    bar1.setFoo(foo);

    bar2 = new Bar();
    bar1.setId(11);
    bar1.setNumber(816);
    bar1.setText("bar for ever 2");
    bar1.setFoo(foo);

    foo.setBars(Arrays.asList(bar1, bar2));
}

@Test
public void testBarUntouchedAfterClone() throws Exception {

    bar1 = new Bar();
    bar1.setId(10);
    bar1.setNumber(815);
    bar1.setText("bar for ever 1");

    Bar clonedBar = new Bar(bar1);

    clonedBar.setId(bar1.getId() + 1);

    DiffNode diffNode = DiffTool.compareObjects(clonedBar, bar1);
    assertEquals(DiffNode.State.UNTOUCHED, diffNode.getState());
}

@Test
public void testUntouchedAfterClone() throws Exception {

    // Clone Object
    Foo clonedFoo = new Foo(foo);

    // Clear IDs, so em.merge() will give the clones new IDs
    clonedFoo.resetIdRecursive();

    // Simulation new IDs in Test Case
    clonedFoo.setId(2);
    clonedFoo.getBars().get(0).setId(20);
    clonedFoo.getBars().get(1).setId(21);

    DiffNode diffNode = DiffTool.compareObjects(clonedFoo, foo);
    assertEquals(DiffNode.State.UNTOUCHED, diffNode.getState());
}

@Test
public void testChangedAfterClone() throws Exception {

    // Clone Object
    Foo clonedFoo = new Foo(foo);

    // Clear IDs, so em.merge() will give the clones new IDs
    clonedFoo.resetIdRecursive();

    // Simulation new IDs in Test Case
    clonedFoo.setId(2);
    clonedFoo.getBars().get(0).setId(20);
    clonedFoo.getBars().get(1).setId(21);

    // Change something
    clonedFoo.getBars().get(0).setText("changedBar");

    DiffNode diffNode = DiffTool.compareObjects(clonedFoo, foo);
    assertEquals(DiffNode.State.CHANGED, diffNode.getState());
}

}
DiffTool.compareObjects is just this:

public class DiffTool {

public static DiffNode compareObjects(Object now, Object before) {

    Object realNow, realBefore;
    if(now instanceof HibernateProxy) {
        realNow = ((HibernateProxy) now).getHibernateLazyInitializer()
                .getImplementation();
    } else {
        realNow = now;
    }

    if(before instanceof HibernateProxy) {
        realBefore = ((HibernateProxy) before).getHibernateLazyInitializer()
                .getImplementation();
    } else {
        realBefore = before;
    }

    return ObjectDifferBuilder
            .startBuilding()
            .inclusion().exclude().type(org.hibernate.proxy.pojo.javassist.JavassistLazyInitializer.class).and()
            .inclusion().exclude().propertyName("hibernateLazyInitializer").and()
            .inclusion().exclude().propertyName("erstelltAm").and()
            .inclusion().exclude().propertyName("erstelltDurch").and()
            .inclusion().exclude().propertyName("geaendertAm").and()
            .inclusion().exclude().propertyName("geaendertDurch").and()
            .inclusion().exclude().propertyName("vlc").and()
            .filtering().returnNodesWithState(DiffNode.State.ADDED).and()
            .filtering().returnNodesWithState(DiffNode.State.CHANGED).and()
            .filtering().returnNodesWithState(DiffNode.State.REMOVED).and()
            .filtering().returnNodesWithState(DiffNode.State.UNTOUCHED).and()
            .introspection().setDefaultIntrospector(new HibernateLazyIntrospector()).and()
            .build()
            .compare(realNow, realBefore);
}

—
Reply to this email directly or view it on GitHub.

[SQiShER]

@massfords suggestion sounds promising. I just found this answer on stackoverflow, which adds some more insights (e.g. a link to the Hibernate docs which addresses the same issue.)

It is recommended that you implement equals() and hashCode() using Business key equality. Business key equality means that the equals() method compares only the properties that form the business key. It is a key that would identify our instance in the real world...

They make it sound as if its easy to override the default equals/hashCode implementations, but I can't say for sure, since I never really used Hibernate. Can you give it a try, @K-Menki?

[K-Menki]

Ok, I have found another solution. I converted my entity graph to a presentation graph and implemented the equals() and hashCode() methods on business keys. Then everything worked as expected.

Thanks for your help 👍

[SQiShER]

Great! I'm glad you found a solution that works. And thanks for sharing it with us!