Yjs-based Shared Editing [3/4] Implement New Frontend Architecture based on Yjs #1674
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Xiao-zhen-Liu
added
feature
gui
dependencies
# Conflicts: # core/new-gui/src/app/workspace/service/workflow-graph/model/workflow-graph.ts
zuozhiw
reviewed
Oct 4, 2022
Contributor
zuozhiw
left a comment
zuozhiw
left a comment
There was a problem hiding this comment.
Left a few minor comments, I'll continue the review.
...i/src/app/workspace/component/codearea-custom-template/codearea-custom-template.component.ts
Outdated
Show resolved
Hide resolved
...ent/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.html
Show resolved
Hide resolved
core/new-gui/src/app/workspace/types/shared-editing.interface.ts
Outdated
Show resolved
Hide resolved
core/new-gui/src/app/workspace/types/shared-editing.interface.ts
Outdated
Show resolved
Hide resolved
core/new-gui/src/app/workspace/service/workflow-graph/model/shared-model-change-handler.ts
Show resolved
Hide resolved
core/new-gui/src/app/workspace/service/workflow-graph/model/shared-model-change-handler.ts
Outdated
Show resolved
Hide resolved
# Conflicts: # core/new-gui/src/app/workspace/service/execute-workflow/execute-workflow.service.ts
zuozhiw
approved these changes
Oct 5, 2022
Contributor
zuozhiw
left a comment
zuozhiw
left a comment
There was a problem hiding this comment.
left some more minor comments, after that it's good to go
Also we need to modify the wiki instructions to run the shared editing server
# Conflicts: # core/new-gui/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts # core/new-gui/src/app/workspace/service/execute-workflow/execute-workflow.service.ts # core/new-gui/src/app/workspace/service/workflow-graph/model/workflow-action.service.ts # core/new-gui/src/app/workspace/service/workflow-graph/model/workflow-graph.ts # core/new-gui/src/app/workspace/types/command.interface.ts
Xiao-zhen-Liu
added a commit
that referenced
this pull request
Dec 13, 2022
This PR fixes a bug where the following functionalities are not disabled when `workflowModificatonEnabled` is `true`: - Disable operator - Cache operator - Auto Layout - Delete All - Import Workflow - Add Comment - Restore to a Version - Python UDF's Edit button - Python UDF code editor - Context Menu items These bugs are introduced by #1674's deletion of "lock" in previous shared editing implementation. For other components, I use `WorkflowActionService`'s flag. For Python UDF's readonly status I use formly's status, since we allow temporarily enabling modification of properties during execution. Added back the "readonly" button for UDF, and used a workaround similar to #1688 to fix the unit test. It would look like below:  There is one more complexity for version display: since when displaying a version, workflow modification is always disabled, we cannot use this flag to determine whether restore version should be disabled. This has never been handled properly. For now I added a temporary flag to remember the workflow action disabled status before displaying a particular version.
Xiao-zhen-Liu
added a commit
that referenced
this pull request
Apr 29, 2024
On workflow editor, the positions of comment boxes are not persisted because of a wrong filter in #1674. This PR fixes it. Co-authored-by: Xinyuan Lin <xinyual3@uci.edu>
Xiao-zhen-Liu
added a commit
that referenced
this pull request
Oct 10, 2025
…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 minimally reproduced as follows with two users editing the same workflow (or two tabs opened by the same user): 1. User A deletes a link E from operator X to Y on the canvas, 2. User B deletes operator Y. 3. User A clicks "undo", and the workflow reaches an erroneous state, where there is a link E that connects to an operator Y that no longer exists. Note E exists in the frontend data but is not visible on the UI. The following gif shows this process.  ## Shared-editing Architecture Shared editing (#1674) 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. There are two layers of data on each user's Texera frontend, one being the UI data (jointjs), and the other being this shared "Y data". The two layers in each user's UI are synched by our application code, and the Y data between users of a shared-editing sessions are kept in sync with automatic conflict resolution by relying on yjs. The following diagram shows what happens when a user adds a link and how the other user sees this change in real-time.  Yjs's CRDT guarantees the eventual **consistency** of this underlying data model among concurrent editors, i.e., it makes sure this data model is correctly synced in each editor's frontend. ## The core problem Yjs does not offer a "graph" data structure, and currently in Texera, the shared data structures for operators and links are two separate `Map`s: - `operatorIDMap`: `operatorID`->`Operator` - `operatorLinkMap`: `linkID`-> `OperatorLink` There is an application-specific "referential constraint" in Texera's frontend that "a link must connect to an operator that exists", and this kind of sanity checking on the data is not the concern of CRDT. It can only be enforced by the application (i.e., ourselves). Ideally, before making any changes to the shared data model, we should do sanity checking and reject changes that violate our application-specific constraints. As shown below, in each user's frontend, there are 3 paths where the shared data model can be modified.  **Path 1**: The first is path includes those changes initiated by a user's UI actions (e.g., add a link on the UI). For this path, we do have existing sanity checking logic: ``` public addLink(link: OperatorLink): void { this.assertLinkNotExists(link); this.assertLinkIsValid(link); this.sharedModel.operatorLinkMap.set(link.linkID, link); } ``` **Path 2**: Another path is undo/redo, which is purely managed by an `UndoManager`, also offered by Yjs. This module is local to each user's frontend, and it automatically tracks local changes to the shared data model. When a user clicks "undo", `UndoManager` directly applies changes to the shared data model. **The core of the problem is there is no sanity checking on this path.** **Path 3**: The third path is remote changes from another collaborator. There is also no sanity checking on this path, but the correctness of such changes depends on whether the change was sanity-checked on the collaborator's side (i.e., if it is a UI change from User A, the propagated change to User B's frontend would be sanity-checked; if it is a undo change, however, the propagated changed to User B would not be sanity-checked and could cause issues.) ## Cause of the bug The following diagram shows how the bug happens from the perspective of the shared model.  When user A clicks "Undo" after 2), the `UndoManager` simply applies the reverse-operation of "Delete E", and add the link `E` to `operatorLinkMap `. As there is no sanity checking during this process, this operation succeeds, and the shared model reaches a state that violates the constraint. ## Solution Unfortunately, due to the limitations of Yjs's APIs, it is not possible to add sanity checking to Path 2 or 3 **before** a change is applied, as an undo/redo operation on the `UndoManager`'s stack is not exposed as a meaningful action (i.e., there is no way to tell that an action to be applied to the shared model is an `addLink` if it is an undo operation). Nevertheless, we can react to a change to the shared model that is initiated from Path 2 or Path 3 after the change has been applied, and add sanity checking logic there to "repair" unsanitary changes. This places (`SharedModelChangeHandler`) is exactly where we sync the changes from the shared model to the UI: any changes to the shared model not initiated by the UI (i.e., changes from the `UndoManager` or remote changes by other users) go through this place, and such changes are parsed as meaningful changes such as "add a link", "delete an operator", etc.  Currently, the only sanity checking needed is to check if a newly added link connects to operator / ports that exist and that it is not a duplicate link. We add such checking logic in `SharedModelChangeHandler`, and revert unsanitary operations before it is reflected on the UI. ## Demo The following gif shows the experience after the fix. When unsanitary actions caused by undo happens, it would fail and we log it in the console. The workflow JSON no longer gets damaged.  --------- Co-authored-by: Yicong Huang <17627829+Yicong-Huang@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduction
This PR is the "meat" of this series of work. It implements lock free real-time shared editing based on the popular CRDT library, Yjs.
Architectural Change
This is better explained with these two diagrams:
Previous Design
New Final Design
Other Changes Introduced
y-websocketneeds to run for the new frontend to work.Next Step
The final step is adding co-editor awareness on top of this. This PR only allows a workflow to be collaboratively edited, but the user cannot know which other user is making the changes.