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

Node references are visited on unpredictable order leading to unsaved changes. #407

Closed
felipebn opened this issue Sep 25, 2017 · 2 comments · Fixed by #433
Closed

Node references are visited on unpredictable order leading to unsaved changes. #407

felipebn opened this issue Sep 25, 2017 · 2 comments · Fixed by #433
Assignees

Comments

@felipebn
Copy link

felipebn commented Sep 25, 2017

Expected Behavior

I would expect to OGM visit nodes in a graph always in the same order between distinct executions.

Current Behavior

The visiting order changes between executions, leading to unsaved changes.

Possible Solution

The order changes due to the following observations:

  • EntityGraphMapper is visiting nodes randomly between executions in a graph when saving with limited depth.
  • EntityGraphMapper.mapEntityReferences finds references iterating over a collection based on what ClassInfo.relationshipFields returns.
  • As ClassInfo.relationshipFields returns a newly created HashSet based on FieldInfo class that stores the fields in a HashMap the order of the collection is not guaranted which leads to problems when using limited depth operations.

Use collections that guarantee the same order of iteration between executions in the places noted above.

Steps to Reproduce (for bugs)

Quite hard to reproduce as it depends on many factors related to environment or uncontrolled.

Context

This problem was noticed while working with limited depth save operations. When following the unexpected visiting order some changes were simply ignored.

By debugging and unit testing was possible to see that the EntityGraphMapper.mapEntityReferences iterated over collections with distinct sequences leading to some nodes being ignored as they were partially visited.

Example situation:

Saving from the node labeled as Start with depth 3 there's two possible visiting sequence (blue and red), one would expect to only one of them always take place but not randomly switching between.

In case the red sequence is applied, changes to the purple node would not be persisted.

visiting_order
Note: TASK_TO_BAG relationship should not be considered!

Your Environment

  • OGM Version used: 2.1.3
  • Java Version used: 1.8
  • Neo4J Version used: 3.1.6
  • Bolt Driver Version used (if applicable): 3.1.2
  • Operating System and Version: Windows 10 and Ubuntu 14 (problem happens on both)

Edit: Adding note to the graph

@frant-hartm
Copy link
Contributor

Hi,

thank you for reporting this.

Just quick follow up question to make sure I understand this correctly.

In your example graph situation "Sample place" should be stored for depth 3 no matter what, right?

So your suggested solution
Use collections that guarantee the same order of iteration between executions in the places noted above.
wouldn't really work, because the visiting sequence would be deterministic, but it could happen to be the red one (which wouldn't save "Sample place").

@felipebn
Copy link
Author

Hi!

Thanks for the quick reply!

That would be the perfect world I guess...

Yes it could follow the red one, but at least the behavior is consistent and prevents the "it was working yesterday".

About the example, I noticed that I added an extra relationship (TASK_TO_BAG) that does not exist in my real world case, so in this case another solution would be to go through the OUTGOING relationships first, then the INCOMING and at last UNDIRECTED. But even that could lead to problems in other scenarios with more nodes and relationships.

I really can't think of a satisfy all solution, even don't feel sure would be feasible, but I believe the important point in this case is to have a consistent behavior so at least the save operation is always wrong or always right.

@frant-hartm frant-hartm self-assigned this Nov 14, 2017
frant-hartm added a commit that referenced this issue Nov 14, 2017
frant-hartm added a commit that referenced this issue Nov 14, 2017
When a node is reached at horizon 0 it is marked as visited.
After reaching the node again, with horizon >0 its relationships
should be visited.

This commit stores the horizon in CypherContext and uses it to evaluate
if the traversal should continue.

Fixes #407.
frant-hartm added a commit that referenced this issue Nov 15, 2017
frant-hartm added a commit that referenced this issue Nov 15, 2017
When a node is reached at horizon 0 it is marked as visited.
After reaching the node again, with horizon >0 its relationships
should be visited.

This commit stores the horizon in CypherContext and uses it to evaluate
if the traversal should continue.

Fixes #407.
nmervaillie pushed a commit that referenced this issue Nov 15, 2017
* Solves a problem when deep saving an entity which is 
reachable from several paths. 

When a node is reached at horizon 0 it is marked as visited.
After reaching the node again, with horizon >0 its relationships
should be visited.

This commit stores the horizon in CypherContext and uses it to evaluate
if the traversal should continue.

Fixes #407.

* Fix mapping of directed transient relationships defined in both directions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants