IdentityMap.remember throws a NPE.

[johnfreier]

We are receiving a NPE in the IdentityMap.remember method, line 102.

neo4j-ogm/core/src/main/java/org/neo4j/ogm/context/IdentityMap.java

Lines 81 to 105 in 5f95e57

	/**
	* determines whether the specified has already
	* been memorised. The object must not be null. An object
	* is regarded as memorised if its hash value in the memo hash
	* is identical to a recalculation of its hash value.
	*
	* @param object the object whose persistable properties we want to check
	* @param entityId the native id of the entity
	* @return true if the object hasn't changed since it was remembered, false otherwise
	*/
	boolean remembered(Object object, Long entityId) {
	// Bail out early if the native id is null or neither the hashes of nodes nor relationships contain the entity id.
	if (entityId == null || !(nodeHashes.containsKey(entityId) || relEntityHashes.containsKey(entityId))) {
	return false;
	}
	ClassInfo classInfo = metaData.classInfo(object);
	boolean isRelEntity = metaData.isRelationshipEntity(classInfo.name());
	long actual = hash(object, classInfo);
	long expected = isRelEntity ? relEntityHashes.get(entityId) : nodeHashes.get(entityId);
	return actual == expected;
	}

Because a node and a relationship could both have the same Id, it passes the initial guard condition, but when it attempts to retrieve the "expected" Id, the code looks in the wrong map, and throws an error.

Expected Behavior

I would expect the code to check if the Id exists before assigning the Id to a primitive.

Steps to Reproduce (for bugs)

To reproduce the issue, I wrote a simple unit test that is based on the current ogm unit test. IdentityMapTest.class(https://github.com/neo4j/neo4j-ogm/blob/master/neo4j-ogm-tests/neo4j-ogm-integration-tests/src/test/java/org/neo4j/ogm/context/IdentityMapTest.java)

import static org.assertj.core.api.Assertions.assertThat;

import org.junit.Before;
import org.junit.Test;
import org.neo4j.ogm.annotation.Id;
import org.neo4j.ogm.annotation.NodeEntity;
import org.neo4j.ogm.annotation.RelationshipEntity;
import org.neo4j.ogm.context.MappingContext;
import org.neo4j.ogm.metadata.MetaData;

public class IdentityMapTest {

    private static final MetaData metaData = new MetaData(IdentityMapTest.class.getPackage().getName());
    private static final MappingContext mappingContext = new MappingContext(metaData);

    @Before
    public void setUp() {
        mappingContext.clear();
    }

    @Test
    public void testNodeAndRelationshipWithSameId() {

        // Create a Node and set the Id.
        Teacher mrsJones = new Teacher();
        mrsJones.setId(1);

        // Remember the entity.
        mappingContext.addNodeEntity(mrsJones);

        // Create a Relationship with the same Id.
        TeachesAt teachesAtRelationship = new TeachesAt();
        teachesAtRelationship.setId(1);

        // Checking if the relationship is dirty will throw a NPE.
        assertThat(mappingContext.isDirty(teachesAtRelationship)).isFalse();
    }

    @NodeEntity
    class Teacher {

        @Id
        private Long id;

        private long getId() { return id; }

        private void setId(long id) { this.id = id; }

    }

    @RelationshipEntity
    class TeachesAt {

        @Id
        private Long id;

        private long getId() { return id; }

        private void setId(long id) { this.id = id; }

    }

}

Your Environment

• OGM Version used: 3.2.1
• Java Version used: 1.8
• Neo4J Version used: 3.2
• Bolt Driver Version used (if applicable):
• Operating System and Version: OSX
• Link to your project:

[michael-simons]

Thanks a lot for the report and detailed analysis, @johnfreier. I can confirm the bug and the point in time when it was introduced. This is fixed on 3.2.x and I also back ported the tests to 3.1.x. Expect a patch release early next week.

[michael-simons]

This is fixed in 3.2.3, release is on it's way to central.

Remember to that version upgrade in Spring Boot is done via Maven or Gradle property (<neo4j-ogm.version>3.2.3</neo4j-ogm.version>)