Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

toMany relationshipEntity is detached after first update #868

Closed
gonzalad opened this issue Feb 1, 2021 · 6 comments
Closed

toMany relationshipEntity is detached after first update #868

gonzalad opened this issue Feb 1, 2021 · 6 comments
Assignees

Comments

@gonzalad
Copy link

gonzalad commented Feb 1, 2021

Hello, I have the following issue

If needed there's a sample project to reproduce this issue : https://github.com/gonzalad/neo4j-dependent-relationship-entity-issue/blob/master/src/test/java/com/example/neo4j/IssueTest.java

Issue

When using relationshipEntities to model toMany relations, when executing a series of updates/queries on the root entity (all in the same transactional context), the relationshipEntity can get detached.

We end up with a 2 instances of the same relationshipEntity in the root entity.

Expected Behavior

I would expect that in a given transactional context every entity and every relation is attached (session OGM keeps track of maintaining the references between entity instances).

Current Behavior

My test case is:

  • creation of Actor (entity) with a single Role (relationshipEntity) to Movie (entity)
  • read query to Actor (with all its relations loaded)
  • update to a field of Actor
  • read query to entity Actor (with all its relations loaded)
  • check current size of actor.playedIn (role) relation

Sample test failing:

In the following test, the last line

 assertThat(actor.getPlayedIn()).hasSize(1);

Should return a size of 1 but it instead returns a size of 2.
From what I understand, the entities id are the same, but the instances are different hence from my POV, one of them has been detached under the hoods by the previous save (update) operation.

Testcase

@SpringBootTest
@Transactional
@Testcontainers
class IssueTest {

    @Autowired
    private Session session;

    @Test
    void entityOrRelationshipEntityDetachedAfterUpdate() {
        Movie movie = new Movie();
        movie.setTitle("Lord of the rings");
        Role role = new Role();
        role.setTitle("Saruman");
        role.setMovie(movie);
        Actor actor = new Actor();
        actor.setName("Christopher Lee");
        actor.addPlayedIn(role);

        session.save(actor);
        assertThat(actor).isNotNull();
        assertThat(actor.getId()).isNotNull();
        assertThat(actor.getPlayedIn()).hasSize(1);

        runReadQuery(actor);
        assertThat(actor.getPlayedIn()).hasSize(1);

        // this second save detaches an entity or relationshipEntity
        // we'll get a bad result on next query
        actor.setName("name changed");
        session.save(actor);
        assertThat(actor).isNotNull();
        assertThat(actor.getId()).isNotNull();
        assertThat(actor.getPlayedIn()).hasSize(1);

        runReadQuery(actor);
        // fails, we return childs = 2
        assertThat(actor.getPlayedIn()).hasSize(1);
    }

    private void runReadQuery(Actor entity) {
        Map<String, Object> params = new HashMap<>();
        params.put("id", entity.getId());
        session.query("MATCH path = (a:Actor)-[:PLAYED_IN*0..]->()"
                + " WHERE id(a) = $id"
                + " RETURN nodes(path), relationships(path)", params);
    }

    @TestConfiguration
    static class Config {
        @Bean
        public org.neo4j.ogm.config.Configuration configuration() {
            return new org.neo4j.ogm.config.Configuration.Builder().uri(Neo4j.database().getBoltUrl()).credentials("neo4j", Neo4j.database().getAdminPassword()).build();
        }
    }
}

Entity classes used by the sample (based on Spring Boot sample Actor/Role/Movie, but I changed role cardinality to be ToMany):

/**
 * Sample from https://docs.spring.io/spring-data/neo4j/docs/5.3.6.RELEASE/reference/html/#reference:annotating-entities:relationship-entity
 */
@NodeEntity
public class Actor {

    @Id
    @GeneratedValue
    private Long id;

    @Relationship(type="PLAYED_IN")
    private List<Role> playedIn;

    private String name;

    public Long getId() {
        return id;
    }

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

    public List<Role> getPlayedIn() {
        return playedIn;
    }

    public void addPlayedIn(Role playedIn) {
        if (this.playedIn == null) {
            this.playedIn = new ArrayList<>();
        }
        playedIn.setActor(this);
        this.playedIn.add(playedIn);
    }

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }
}

@RelationshipEntity(type = "PLAYED_IN")
public class Role {
    @Id @GeneratedValue
    private Long relationshipId;
    @Property
    private String title;
    @StartNode
    private Actor actor;
    @EndNode
    private Movie movie;

    public Long getRelationshipId() {
        return relationshipId;
    }

    public void setRelationshipId(Long relationshipId) {
        this.relationshipId = relationshipId;
    }

    public String getTitle() {
        return title;
    }

    public void setTitle(String title) {
        this.title = title;
    }

    public Actor getActor() {
        return actor;
    }

    public void setActor(Actor actor) {
        this.actor = actor;
    }

    public Movie getMovie() {
        return movie;
    }

    public void setMovie(Movie movie) {
        this.movie = movie;
    }
}

@NodeEntity
public class Movie {
    @Id
    @GeneratedValue
    private Long id;
    private String title;

    public Long getId() {
        return id;
    }

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

    public String getTitle() {
        return title;
    }

    public void setTitle(String title) {
        this.title = title;
    }
}

Context

When issuing series of updates/queries in my real application I have duplicated relation instances.

This messes up the application and makes it really hard to use relationshipEntities in ToMany scenarios.

Your Environment

Thanks,
Adrian

@gonzalad
Copy link
Author

gonzalad commented Feb 8, 2021

Similar (or same ?) as spring-projects/spring-data-neo4j#2130

@michael-simons
Copy link
Collaborator

Hi. OGM needs to flush the session cache on custom queries on which it can not reliable decide whether it's a read or write query (we don't do client Cypher parsing).

This can be fixed with a query hint: /*+ OGM_READ_ONLY */ , which looks like this in your case:

 private void runReadQuery(Actor entity) {
        Map<String, Object> params = new HashMap<>();
        params.put("id", entity.getId());
        session.query("MATCH /*+ OGM_READ_ONLY */ path = (a:Actor)-[:PLAYED_IN*0..]->()"
                + " WHERE id(a) = $id"
                + " RETURN nodes(path), relationships(path)", params);
    }

Please see this #839

Please let me now if this works for you.

@michael-simons michael-simons self-assigned this Feb 9, 2021
gonzalad pushed a commit to gonzalad/neo4j-dependent-relationship-entity-issue that referenced this issue Feb 9, 2021
@gonzalad
Copy link
Author

gonzalad commented Feb 9, 2021

@michael-simons
Copy link
Collaborator

I am sorry for this longtime until "needs investigation" was concluded.

If you look into the database, all relationships are consisted. OGM however is not able to figure out that the one relationship has already been loaded.

You need to add a proper pair of equals/hashCode to the Role class like this:

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

    @Override 
    public int hashCode() {
        return Objects.hash(relationshipId);
    }

Of course, a better implementation is to do this for Actor and Movie and include those attributes of Role in the above methods. I will demonstrate this in a commit referencing this issue.

Read only hint is not needed btw but I recommend it nevertheless.

Thanks again for opening this up.

@gonzalad
Copy link
Author

Thanks very much @michael-simons

My issue (in my real project) was resolved by using the 2 instructions you gave me:

  • using /*+ OGM_READ_ONLY */
  • implementing equals/hashcode in my neo4j entities/relationsEntities (I've based them on my primary business key instead of neo4j id, but it works fine)

Thanks once more !!!

@michael-simons
Copy link
Collaborator

Thanks a lot for your feedback, @gonzalad much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants