Skip to content

Make state machine for saving process more robust. #2181

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

Closed
wants to merge 4 commits into from

Conversation

meistermeier
Copy link
Collaborator

If two entities and one were saved as
(:A{name:'a1'})-[:KNOWS]->(:A{name:'a2'})
and we would add another back-referencing KNOWS relationship from a2 to a1 in a clean operation
loadA1 (which automatically pulls also a2)
loadA2
createA2KnowsA1
the state machine assumes that aa2 from the existing a1 relationship differs from the directly loaded a2
and wants to re-create it which causes in the scenario of activated optimistic locking an OptimisticLockingException because "one of the two" a2s already got an update of its version.
To mitigate the problem, a registration of initial objects in the state machine to mark them as processed were introduced.

Besides that the state machine got a little bit more robust, I wonder if the approach to check for objects' equality, that results in a PROCESSED_ALL_VALUES state, is the right one. I had to adjust one existing test because there were two "different" objects that were "equal" due to a very simple equals method.

@Nullable String inDatabase) {

Neo4jPersistentProperty requiredIdProperty = targetNodeDescription.getRequiredIdProperty();
PersistentPropertyAccessor<Object> ding = targetNodeDescription.getPropertyAccessor(entity);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming.

@Nullable String inDatabase) {

Neo4jPersistentProperty requiredIdProperty = targetNodeDescription.getRequiredIdProperty();
PersistentPropertyAccessor<Object> ding = targetNodeDescription.getPropertyAccessor(entity);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming.

@@ -284,9 +284,9 @@ private Object convertIdValues(@Nullable Neo4jPersistentProperty idProperty, Obj

String databaseName = getDatabaseName();

Collection<T> entities;
List<T> entities;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not that I mind, but is it necessary?

@@ -324,7 +324,7 @@ private Object convertIdValues(@Nullable Neo4jPersistentProperty idProperty, Obj

// Save related
entitiesToBeSaved.forEach(entityToBeSaved -> processRelations(entityMetaData, entityToBeSaved,
isNewIndicator.get(entitiesToBeSaved.indexOf(entityToBeSaved)), databaseName));
isNewIndicator.get(entitiesToBeSaved.indexOf(entityToBeSaved)), databaseName, entities.get(entitiesToBeSaved.indexOf(entityToBeSaved))));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

k, i See. But please change to only one indexOf operation.

@meistermeier
Copy link
Collaborator Author

Closed with 6aef5d9 and 65ee2ae

@meistermeier meistermeier deleted the issue/gh-2177 branch March 26, 2021 15:43
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 this pull request may close these issues.

2 participants