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

SDN4: Unable to persist rich relationship [DATAGRAPH-763] #1330

Closed
spring-projects-issues opened this issue Sep 23, 2015 · 6 comments
Closed
Labels
in: core status: declined type: bug

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented Sep 23, 2015

Eric Spiegelberg opened DATAGRAPH-763 and commented

Having upgraded our application from Spring Data Neo4j (SDN) 3.x to SDN 4.0.GA, our application is no longer able to persist rich relationships. Trying to isolate the problem and learn what we’re doing wrong on our end, we have modified the sdn4-unversity sample project to mirror our approach and, to our surprise, are seeing the same issue.

The attached sdn4-unversity has had a rich relationship, named TeacherAndStudentRelationship and annotated with @RelationshipEntity, and its associated repository and service added. The TeacherController has been modified to add a method that demonstrates the issue.

To demonstrate that entities (ie: nodes) are being persisted correctly:

  1. With an empty database, start Neo4j (we’re using 2.2.2 and 2.2.5)

  2. Download, extract, enter, and start the attached sdn4-university with the following command line:
    a. mvn clean test spring-boot:run -Drun.jvmArguments="-Xdebug -Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=5005 -Dusername=[YOUR_NEO4J_USERNAME] -Dpassword=[YOUR_NEO4J_PASSWORD]"

  3. Attach your remote debugger, such as through Eclipse

  4. Set a debug breakpoint on GenericService, line 32, where the entity is about to be save

  5. Using a REST tool, such as Chrome’s Advanced REST Client, submit a “application/json” POST request to http://localhost.capella.edu:8080/api/teachers with the following JSON payload: {"name": "Demo Teacher"}

  6. When your debugger catches on GenericService, line 32, notice that the id of the new Teacher is null

  7. Step over the save operation on line 32

  8. Notice that the id of the new Teacher is now populated with a Long value, indicating it has been persisted to Neo4j

  9. Let your debugger run, fully completing the request

  10. Use the Neo4j console to validate the teacher was persisted, validating the teacher with the name “Demo Teacher” exists

  11. Notice that “Teacher” is correctly listed within Neo4j’s “Node Labels” section

  12. Populate the sample reference data by using a web browser to hit: http://localhost:8080/api/reload

Now, to reproduce the relationship issue we are seeing:

  1. Use a web browser to hit: http://localhost:8080/api/teachers/relate/11/33 . This invokes our customized code in TeacherController that instantiates and persists our rich relationship class

  2. When your debugger catches on GenericService, line 32, notice that the id of the new TeacherAndStudentRelationship is currently null, as expected

  3. Step over the save operation on line 32

  4. Notice that the id of the new TeacherAndStudentRelationship is still null, indicating it has not been correctly persisted

  5. Using the Neo4j console, notice that TeacherAndStudentRelationship is not listed in the “Relationship Types” section

sdn4-unveristy, as it currently sits in github, is making use of spring-boot 1.3.0.M1. We have upgraded our sd4n-university to make use of spring-boot 1.3.0.M5 (the latest), retested, and unfortunately have the same incorrect results.

The ability to persist rich relationships is obviously an important feature of SDN/Neo4j. We expected to make these modifications to sdn4-unversity to demonstrate the problem was on our end in our application, yet were surprised to be able to seemingly reproduce the issue. We’ll be thrilled to find out the problem is something we’re doing wrong, if someone can help us out and show us what it is. Having raised the issue, we will continue to investigate but would greatly appreciate a second set of eyes from the community.

Along with the above reference to my sdn4-university github fork demonstrating my code, attached is the src directory of my changes (src directory only to stay under the 10meg upload limit).


Affects: 4.0 GA

Reference URL: https://github.com/espiegelberg/sdn4-university

Attachments:

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 24, 2015

Vince Bickers commented

It is important to ensure that rich relationships are wired up correctly.

In the current entity->graph mapping logic, the OGM does not directly save relationships. Instead, when you try to persist a rich relationship object the OGM will select the rich relations's @StartNode entity, and persist that, walking the object graph as it goes. For this technique to work however, the start node and end nodes should maintain a reference to the rich relationship, or it won't be found.

If your wire up the source and target objects so that they maintain a reference to the rich relationship, I think it should solve the problem

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 24, 2015

Eric Spiegelberg commented

If I understand you correctly, we need to add a reference to the rich relationship as a property of both the start and end nodes. We’ve done just that in the updated attached code. When performing a test to create and persist the entities and relationship, the first time the code is executed everything and the relationship is correctly peristed. However, executing the same controller endpoint the the relationship itself is deleted. Performing the operation multiple times demonstrates that the relationship is created on the first execution, deleted on the subsequent execution, created on the third execution, and so on.

To reproduce what we’re seeing:

  • Download and extract the attached source; replace your existing sdn4-university code with these files
  • With an empty database, start Neo4j (we’re using 2.2.2 and 2.2.5)
  • Enter and start the sdn4-university with the following command line:
    • mvn clean test spring-boot:run -Drun.jvmArguments="-Xdebug -Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=5005 -Dusername=[YOUR_NEO4J_USERNAME] -Dpassword=[YOUR_NEO4J_PASSWORD]"
  • Populate the sample reference data by using a web browser to hit: http://localhost:8080/api/reload
  • Use a web browser to hit: http://localhost:8080/api/teachers/relate/11/33 . This invokes our customized code in TeacherController that instantiates, wires the relationships as you suggest, and persists the rich relationship class
  • Using a refreshed Neo4j console, notice that:
    • The TeacherAndStudentRelationship is correctly listed in the “Relationship Types” section
    • When clicking the “TEACHER_AND_STUDENT_RELATIONSHIP” query within the “Relationship Types” section the results correctly return the start and end nodes with the relationship
  • Once again, use a web browser to hit: http://localhost:8080/api/teachers/relate/11/33
  • Using a refreshed Neo4j console, notice that:
    • The TeacherAndStudentRelationship is still correctly listed in the “Relationship Types” section
    • When clicking the “TEACHER_AND_STUDENT_RELATIONSHIP” query within the “Relationship Types” section the query now returns no results (i.e.: the relationship no longer exists)
    • A query for Student and Teachers shows that the start and end node still exist

Can you help us understand why the relationship is deleted? We would have expected it to not be.

The approach you’ve outlined — of adding a reference to the rich relationship as a property of both the start and end nodes — does not appear to be documented in the SDN4 reference information. It is also not implemented in either the sdn4-university (which has no rich relationships) or sdn4-cineasts projects. Knowing that 4.0.GA is a total rewrite, is the need for this approach a departure from the functional behavior of Spring Data Neo4j?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 24, 2015

Eric Spiegelberg commented

Updated sample code to demonstrate Vince's approach of adding a reference to the start and end nodes

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 25, 2015

Luanne Misquitta commented

The references look fine now, but the relationship entity has no properties and hence it is an incorrect relationship entity (and you should use a standard @Relationship instead). If for example, you were to add this to TeacherAndStudentRelationship:

private long since;

and then always set it so that the relationships are distinguishable

teacherAndStudentRelationship.setSince(System.currentTimeMillis());

the code works correctly.

Agreed that the documentation is lacking in this respect, we'll make sure to have this taken care of before the next release.
On the object references themselves, I've written a blog post that might be helpful: http://graphaware.com/neo4j/2015/09/03/sdn-4-object-model.html

BTW the sdn4-cineasts project does contain a relationship entity- see https://github.com/neo4j-examples/sdn4-cineasts/blob/master/src/main/java/org/neo4j/cineasts/domain/Actor.java#L33

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 29, 2015

Eric Spiegelberg commented

I've digested the information Luaane provided in updating https://jira.spring.io/browse/DATAGRAPH-763. In doing so, I updated my sample sdn4-university sample project attached to DATAGRAPH-763 by:

  • Updated the TeacherAndStudentRelationship object to contain a few properties; not having them in place was an oversight in creating my example
  • Updated Teacher to contain a Set of Student objects along with a getter/getter for the collection
  • Updated Student to contain a Set of Teacher objects, along with a getter/getter for the collection
  • Updated the TeacherAndStudentRelationship to contain the following constructor:
public TeacherAndStudentRelationship(Teacher teacher, Student student) {
    this.teacher = teacher;
    this.student = student;
    this.teacher.getStudents().add(student);
    this.student.getTeachers().add(teacher);
}
  1. Updated the TeacherController to no longer instantiate and persist the TeacherAndStudentRelationship through the use of the TeacherAndStudentRelationshipService and instead:
TeacherAndStudentRelationship teacherAndStudentRelationship = new TeacherAndStudentRelationship(teacher, student);
teacherAndStudentRelationship.setDate(new Date());
teacherAndStudentRelationship.setName("name");
teacherService.createOrUpdate(teacher);

In short, the above changes now mirror the usage of Role in sdn4-cinecasts as well as Luanne's blog post in http://graphaware.com/neo4j/2015/09/03/sdn-4-object-model.html and the behavior of my sample now works as expected and the relationship, along with it's properties, work properly.

Taking a big step back and suspending any judgement of good or bad, from my view this is a pretty big design and usage change in how rich relationships work in SDN 4 versus SDN 3. Is that a fair statement to make? The fact that while using SDN 3 we were able to make use of "anaemic domain models" (as Luanne referred to them in her blog post) without issue but are now experiencing issues after upgrading to SDN 4 leads me to conclude that, again with out judgment of good or bad, the change in how relationships are designed/used in SDN 4 is a big change. Either way, on behalf of the SDN community -- particularly those that have used rich relationships in SDN 3 -- I think it's pretty important that the (new?) SDN4 design and usage of rich relationships get more coverage in the documentation as well as the SDN 4 example/sample projects.

That said, we wish to move our applicaiton forward in alignment with SDN4 best practices. In our current design we have a fairly small number of entities (a dozen or two) that can participate in what may be on the order of a dozen or two relationships. Should the information in Luanne's blog post be considered the recommend modeling approach of relationships -- both simple and rich -- in SDN4? That is to say, would you recommend we revise our domain model so that entities contain a collection for each relationship it may participate in? This isn't necessarily a bad thing, it's just that we were excited and enjoyed the SDN 3 approach where our entities were largely containers of data without the clutter of relationship collections and their associated getter/setters (I believe that this is what Luanne refers to as "anaemic" domain models in her post). It's not the end of the world if we have to overhaul our domain, we simply want to adopt the recommend approach

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Oct 1, 2015

Eric Spiegelberg commented

For the benefit of the community, those that find this item in the future, and to formally close this thread off, it is accurate to say that this issue is working as expected.

After having some deeper offline conversations with GraphAware:

  1. The rich relationships functionality does function differently between SDN3 and SDN4
  2. This rich relationship behavior is considered by GraphAware to be an area of improvement/enhancement for SDN4 and the improvement/enhancement work is already planned (and perhaps even underway)
  3. In the mean time until this improvement is released, the above approach outlined by Luanne (ie: to avoid "anemic" entities) is considered the recommended short term path forward

@spring-projects-issues spring-projects-issues added type: bug status: declined in: core labels Dec 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core status: declined type: bug
Projects
None yet
Development

No branches or pull requests

1 participant