[neo4j] Natural key not desired in equals/hashCode due to lazy loading?

[vmarcinko]

Hi,

It seems I pinpointed the problem regarding the post:
http://forum.springsource.org/showth...ove(obj)-fails
and also JIRA issue:
https://jira.springsource.org/browse/DATAGRAPH-265

General suggestion is that fields used in equals/hashCode should be immutable for an object, especially when used in hash-related collections, such as HashSet, HashMap etc...
In Spring Neo4j tutorial, it is recommended to include graph ID and natural key in equals/hashCode, but problem is that only graph ID is loaded when having associated entity, and the problem arise when we have collection of associated entities in a form of HashSet. So, Spring Neo4j first adds new entities to this hash collection, but all entities have only graph ID present, whereas, when one fetches all of these to work with them, then their equals/hashCode changes as result of loaded natural key, and behaviour of hash collection becomes invalid (such as given in examples above) ?

Do you have any suggestion to overcome this?

-Vjeran

[MichaelHunger]

Can you point me to the suggestion of including graphId and natural key? I'm not aware of having written that?
I think just using the graphId is best, although even that comes with the problem that it might be not set for new entities and will be set in-place in the entity. I look into changing that so that it remove an re-adds entities when their graphId is set.

[vmarcinko]

Here is quick example of the problem, which doesn't even use Spring Neo4j, but plain java objects.
Lets say I have a myEntity class that has 2 fields: graphId and naturalKey. We'll use public fields for shortness. Equals/hashCode are overriden accordingly.

public class MyEntity {
    public Long graphId;
    public String naturalKey;

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

        MyEntity myEntity = (MyEntity) o;

        if (graphId != null ? !graphId.equals(myEntity.graphId) : myEntity.graphId != null) {
            return false;
        }
        if (naturalKey != null ? !naturalKey.equals(myEntity.naturalKey) : myEntity.naturalKey != null) {
            return false;
        }

        return true;
    }

    @Override
    public int hashCode() {
        int result = graphId != null ? graphId.hashCode() : 0;
        result = 31 * result + (naturalKey != null ? naturalKey.hashCode() : 0);
        return result;
    }

    @Override
    public String toString() {
        return "[MyEntity graphId=" + graphId + ", naturalKey=" + naturalKey + "]";
    }

And if you run this piece of code:

        // this is what happens when Spring Neo4j loads entity collection from DB...
        MyEntity myEntity1 = new MyEntity();
        myEntity1.graphId = 10L;

        MyEntity myEntity2 = new MyEntity();
        myEntity2.graphId = 20L;

        Set<MyEntity> set = new HashSet<MyEntity>();
        set.add(myEntity1);
        set.add(myEntity2);

        System.out.println("set = " + set);

        // this is simulation of what happens when neo4jTemplate.fetch(set) is called on collection ...
        myEntity1.naturalKey = "SomeKey1";
        myEntity2.naturalKey = "SomeKey2";

        System.out.println("set.contains(myEntity1) = " + set.contains(myEntity1));

And final result is that set DOES NOT contain myEntity1, because its equals/hashCode has changed AFTER being put in Set at first:

set = [[MyEntity graphId=10, naturalKey=null], [MyEntity graphId=20, naturalKey=null]]
set.contains(myEntity1) = false

[vmarcinko]

I'm sorry, I could have sworn that I have seen examples in reference docs using graphId and some natural key for equals/hashCode, but now when I look at it, I am baffled by my stupidity.

Anyway, I don't see how only graph ID can be used in equals/hashCode. I see 2 problems:

1. When one adds 2 new different entities to some Set, both of them will have graph ID with null value, thus Set will treat them as equals, and end up with just one element instead of 2
2. Graph ID has no business meaning, its just used by database, so when one works with the model outside of transactions, such as unit tests, it will need equality based on some natural key, and not database surrogate key.

The only sensible option is, as suggested in Hibernate book for example, to use only natural key in equals/hashCode, but that means that framework should enable lazy loading for that field in any situation when it is required (such as equals/hashCode when required by HashSet). Simple mapping doesn't work that way (no lazy loading, only explicit fetching), so my question is if advanced mapping would work then ? (I haven't tried it yet)

[MichaelHunger]

One of the problems is that there is no _the_ natural key. And you run in the same issue again, when the natural key is not yet set in the entity.

In the graphId == null case, it should still return equals==true for the same entity instance as it should check this == other first anyway (as it does in your example).

Michael

[laureta]

General suggestion is that fields used in equals/hashCode should be immutable for an object, especially when used in hash-related collections, such as HashSet, HashMap etc...

[lassewesth]

For completeness, I have written up our recommendations on entity equality here: http://spring.neo4j.org/docs#d5e807