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

Handle concurrent editing of Tree.Edit #607

Merged
merged 37 commits into from
Aug 17, 2023
Merged

Conversation

hackerwins
Copy link
Member

@hackerwins hackerwins commented Aug 14, 2023

What this PR does / why we need it:

Handle concurrent editing of Tree.Edit

Tasks:

A. Protocol Update and Converter Implementation

  • Protocol Update and Compilation
  • Converter Implementation

B. Porting Tree.Edit Logic

  • TreeEditOperation
  • index.Tree
    • Remove CRDT Dependency in index.Tree
  • crdt.Tree
    • Coordinate System Change: CRDTTreePos -> CRDTTreeNodeID, new CRDTTreePos
    • Edit Logic Change
  • json.Tree
  • Writing Test Cases: Test Code

C. Testing

  • Integration Testing for JS SDK
  • Testing in ProseMirror Example

Which issue(s) this PR fixes:

Address #559

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


Additional documentation:


Checklist:

  • Added relevant tests or not required
  • Didn't break anything

@codecov
Copy link

codecov bot commented Aug 14, 2023

Codecov Report

Merging #607 (f272647) into main (65c8164) will decrease coverage by 0.91%.
Report is 2 commits behind head on main.
The diff coverage is 46.10%.

@@            Coverage Diff             @@
##             main     #607      +/-   ##
==========================================
- Coverage   51.19%   50.29%   -0.91%     
==========================================
  Files          67       69       +2     
  Lines        7053     7307     +254     
==========================================
+ Hits         3611     3675      +64     
- Misses       2960     3140     +180     
- Partials      482      492      +10     
Files Changed Coverage Δ
api/converter/to_pb.go 55.07% <0.00%> (-0.17%) ⬇️
server/backend/backend.go 0.00% <0.00%> (ø)
api/converter/to_bytes.go 59.77% <16.66%> (-3.17%) ⬇️
api/converter/from_pb.go 47.77% <18.75%> (-1.24%) ⬇️
pkg/index/tree.go 47.52% <49.05%> (-1.34%) ⬇️
pkg/document/crdt/tree.go 47.59% <50.21%> (-7.79%) ⬇️

... and 5 files with indirect coverage changes

@hackerwins hackerwins changed the title Port concurrent editing of Tree.Edit from JS SDK Handle concurrent editing of Tree.Edit Aug 15, 2023
@hackerwins hackerwins marked this pull request as ready for review August 16, 2023 10:39
@hackerwins hackerwins merged commit bd3250b into main Aug 17, 2023
1 check passed
@hackerwins hackerwins deleted the concurrent-case-handling branch August 17, 2023 03:43
@hackerwins hackerwins added protocol changed 📝 Whether the protocol has changed sdk ⚒️ labels Aug 17, 2023
hackerwins added a commit that referenced this pull request Aug 17, 2023
Introduced new logical timestamp for identifying the position in
local/remote editing, and ensures commutative editing in concurrent
cases.

This logical timestamp consists of {parentID, leftSiblingID}. This
allows editing at the front of text nodes by using a reference to the
parent, as well as getting rid of the local offset used to access an
element node's children previously by using the leftSiblingID.

---------

Co-authored-by: MoonGyu1 <[email protected]>
Co-authored-by: sejongk <[email protected]>
hackerwins added a commit that referenced this pull request Aug 18, 2023
Introduced new logical timestamp for identifying the position in
local/remote editing, and ensures commutative editing in concurrent
cases.

This logical timestamp consists of {parentID, leftSiblingID}. This
allows editing at the front of text nodes by using a reference to the
parent, as well as getting rid of the local offset used to access an
element node's children previously by using the leftSiblingID.

---------

Co-authored-by: MoonGyu1 <[email protected]>
Co-authored-by: sejongk <[email protected]>
Wu22e pushed a commit to Wu22e/yorkie that referenced this pull request Sep 3, 2023
Introduced new logical timestamp for identifying the position in
local/remote editing, and ensures commutative editing in concurrent
cases.

This logical timestamp consists of {parentID, leftSiblingID}. This
allows editing at the front of text nodes by using a reference to the
parent, as well as getting rid of the local offset used to access an
element node's children previously by using the leftSiblingID.

---------

Co-authored-by: MoonGyu1 <[email protected]>
Co-authored-by: sejongk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protocol changed 📝 Whether the protocol has changed sdk ⚒️
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants