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

Modify the set operation to handle nested object values #663

Closed
chacha912 opened this issue Nov 7, 2023 · 2 comments · Fixed by yorkie-team/yorkie-js-sdk#691 or #756
Closed

Modify the set operation to handle nested object values #663

chacha912 opened this issue Nov 7, 2023 · 2 comments · Fixed by yorkie-team/yorkie-js-sdk#691 or #756
Labels
cleanup 🧹 Paying off technical debt enhancement 🌟 New feature or request sdk ⚒️

Comments

@chacha912
Copy link
Contributor

chacha912 commented Nov 7, 2023

Description:

Currently, the set operation is designed to create an empty object when the value is an object, which means it cannot handle nested objects. As a result, when generating a reverse operation for the set operation during an undo operation, if the existing value is an object, it doesn't set the value correctly.

For example, suppose the document is {"shape":{"point":{"x":0,"y":0}}}. If you update the "point" using doc.update((root) => {root.shape.point = { x: 1, y: 1 }}), the undo operation results in [ 'Remove x', 'Remove y', 'Set point: {x:0, y:0}' ]. When this undo operation is applied to another peer, the value point: {x:0, y:0} is not set correctly. (Refer to the test code (which should ensure convergence of peer's document after undoing nested objects) for more information.)

To address this issue, two approaches can be considered. One option is to refactor Set point: {x:0, y:0} into ['Set point: {}', 'Set point.x:0', 'Set point.y:0'] by breaking it down. Alternatively, we can modify the set operation to support setting CRDTObject, which is the approach we have chosen to resolve this issue.
Therefore, it is necessary to modify the set operation to enable the setting of nested objects, similar to how CRDTTree handles it.

function fromElementSimple(pbElementSimple: PbJSONElementSimple): CRDTElement {
  switch (pbElementSimple.getType()) {
    case PbValueType.VALUE_TYPE_JSON_OBJECT:
      // As-is: CRDTObject is created with an empty RHT.
      return CRDTObject.create(fromTimeTicket(pbElementSimple.getCreatedAt())!);
    case PbValueType.VALUE_TYPE_TREE:
      return bytesToTree(pbElementSimple.getValue_asU8())!;
    // ...
  }
}

function bytesToTree(bytes?: Uint8Array): CRDTTree {
  const pbElement = PbJSONElement.deserializeBinary(bytes);
  return fromTree(pbElement.getTree()!);
}

+) When deleting an object, 'removedAt' marking is not applied to nested objects.
The undo for moving a rectangle targets the nested object 'Point.' It should be checked to prevent the creation of a reverse operation.

Related to #652

Why:

When undoing, the set operation should be able to set the CRDT object.

@hackerwins
Copy link
Member

Related to #431.

@hackerwins hackerwins transferred this issue from yorkie-team/yorkie-js-sdk Nov 7, 2023
hackerwins added a commit to yorkie-team/yorkie-js-sdk that referenced this issue Nov 15, 2023
1. Define reverse operations of Object.Set Object.Remove:
   1.1 Modified the set operation to compare `executedAt` with
       `getPositionedAt()` during value setting, addressing issues
       during undo.
   1.2 Set `movedAt` when the value is reassigned during undo.

2. Handle edge cases:
   When the target element is deleted by other peers
   Implemented handling to skip execution of reverse operations
   during undo and redo if the element has been deleted.

3. Add whiteboard example:
   3.1. Feature Introduction - Added a whiteboard example for testing
        undo redo operations.
   3.2. Variables Added for Displaying Yorkie Data - Added
        `CRDTRoot.opsForTest` and `CRDTElement.toJSForTest`.
   3.3. Additional Features to Apply (Todo) - Planned features include
        implementing history pause/resume, subscribing to history
        changes, and applying reverse operations for `array.add` and `array.remove`.

4. TODOs
  4.1. Set nested objects (yorkie-team/yorkie#663)
        - Modified set operation to handle nested objects.
  4.2. When the target element is garbage-collected (yorkie-team/yorkie#664)
        - Considering elements referenced by operations in the
          undo/redo stack during garbage collection.

---------

Co-authored-by: Youngteac Hong <[email protected]>
@github-project-automation github-project-automation bot moved this from Todo to Done in Yorkie Project Nov 23, 2023
@chacha912 chacha912 reopened this Jan 5, 2024
@github-project-automation github-project-automation bot moved this from Done to Todo in Yorkie Project Jan 5, 2024
@chacha912
Copy link
Contributor Author

SDKs:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup 🧹 Paying off technical debt enhancement 🌟 New feature or request sdk ⚒️
Projects
No open projects
Status: Done
2 participants