Xiao-zhen-Liu opened a new pull request, #3836:
URL: https://github.com/apache/texera/pull/3836
## Purpose
#3571 disabled frontend undo/redo due to an existing bug with the undo/redo
manager during shared editing. This PR fixes that bug and re-enables undo/redo.
## Bug with shared editing
The bug can be reproduced as follows with two users editing the same
workflow (or two tabs opened by the same user):
1. User A deletes a link L1 from operator X to Y on the canvas,
2. User B adds a link L2 from X to Y (note: this actually creates a new link
that is not the same as L1 as it will have a different `linkID`),
3. User B deletes operator Y (and link L2 gets deleted automatically),
4. User A clicks "undo", and the workflow reaches an erroneous state, where
there is a link L1 that connects to an operator Y that no longer exists. Note
L1 exists in the frontend data but is not visible on the UI.
## Background
Shared editing is achieved by letting the frontend rely on data structures
from yjs (a CRDT library) as its data model, as any manipulation to these data
structures can be propagated to other users with automatic conflict-resolution.
Conflict resolution only pertains to the same data structure, e.g., two
users editing the same text. However, the shared data structures for operators
and links are two separate `Map`s:
- `operatorIDMap`: `operatorID`->`Operator`
- `operatorLinkMap`: `linkID`-> `OperatorLink`
The "referential constraint" that "a link must connect to an operator that
exists" is not possible to be enforced by the shared data structures
themselves. That is why, when a user tries to add a new link on the frontend we
have code that specifically checks for the validity of such a link:
```
public addLink(link: OperatorLink): void {
this.assertLinkNotExists(link);
this.assertLinkIsValid(link);
this.sharedModel.operatorLinkMap.set(link.linkID, link);
}
```
## Cause of the bug
What happens in step 4 is:
- The "undo" is for step 1 (delete L1), which adds L1 back.
- If this was an `addLink` operation triggered by the user (either of the
two) on the UI via `addLink`, it would be caught by the assertions and this
operation would fail.
- However, as we rely on Yjs's `UndoManager` for handling undo-redo
operations, and undo operation directly modifies the shared structure, and we
sync these changes to the UI. Changes to the shared data structure triggered by
the undo manager do not go through the same kind of integrity check.
## Fix
Add integrity checking for adding links triggered by the undo manager. Note
this can only be done during the process of syncing changes on the shared model
to the UI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
