Implement Tree.RemoveStyle (#748)

This commit added tests for Tree behavior in concurrency scenarios:

Styles by 2 users (4 * 6 * 6 = 144 cases):
  - Ranges(4)
    - A = B
    - A contains B
    - A and B are intersecting
    - not intersecting
  - Operations for both A and B(6)
    - `RemoveStyle({'bold'})`
    - `Style({'bold': 'aa'})`
    - `Style({'bold': 'bb'})`
    - `RemoveStyle({'italic'})`
    - `Style({'italic': 'aa'})`
    - `Style({'italic': 'bb'})`

Edit and Style (6 * 5 * 2 = 60 cases):
  - Ranges(6)
    - A = B
    - A contains B
    - B contains A
    - A and B are intersecting
    - B is next to A
    - A is next to B.
  - Operations for A(5)
    - Insert front of range
    - Insert back of range
    - Insert middle of range
    - Delete
    - Change
  - Operations for B(2)
    - `RemoveStyle({'bold'})`
    - `Style({'bold': 'aa'})`

api/converter/from_pb.go

@@ -511,6 +511,16 @@ func fromTreeStyle(pbTreeStyle *api.Operation_TreeStyle) (*operations.TreeStyle,
 		return nil, err
 	}
 
+	if len(pbTreeStyle.AttributesToRemove) > 0 {
+		return operations.NewTreeStyleRemove(
+			parentCreatedAt,
+			from,
+			to,
+			pbTreeStyle.AttributesToRemove,
+			executedAt,
+		), nil
+	}
+
 	return operations.NewTreeStyle(
 		parentCreatedAt,
 		from,

api/converter/to_pb.go

@@ -364,11 +364,12 @@ func toTreeEdit(e *operations.TreeEdit) (*api.Operation_TreeEdit_, error) {
 func toTreeStyle(style *operations.TreeStyle) (*api.Operation_TreeStyle_, error) {
 	return &api.Operation_TreeStyle_{
 		TreeStyle: &api.Operation_TreeStyle{
-			ParentCreatedAt: ToTimeTicket(style.ParentCreatedAt()),
-			From:            toTreePos(style.FromPos()),
-			To:              toTreePos(style.ToPos()),
-			Attributes:      style.Attributes(),
-			ExecutedAt:      ToTimeTicket(style.ExecutedAt()),
+			ParentCreatedAt:    ToTimeTicket(style.ParentCreatedAt()),
+			From:               toTreePos(style.FromPos()),
+			To:                 toTreePos(style.ToPos()),
+			Attributes:         style.Attributes(),
+			ExecutedAt:         ToTimeTicket(style.ExecutedAt()),
+			AttributesToRemove: style.AttributesToRemove(),
 		},
 	}, nil
 }

api/yorkie/v1/resources.proto

@@ -133,6 +133,7 @@ message Operation {
     TreePos to = 3;
     map<string, string> attributes = 4;
     TimeTicket executed_at = 5;
+    repeated string attributes_to_remove = 6;
   }
 
   oneof body {

pkg/document/crdt/rht.go

@@ -106,7 +106,15 @@ func (rht *RHT) Set(k, v string, executedAt *time.Ticket) {
 
 // Remove removes the Element of the given key.
 func (rht *RHT) Remove(k string, executedAt *time.Ticket) string {
-	if node, ok := rht.nodeMapByKey[k]; ok && executedAt.After(node.updatedAt) {
+	if node, ok := rht.nodeMapByKey[k]; !ok || executedAt.After(node.updatedAt) {
+		// NOTE(justiceHui): Even if key is not existed, we must set flag `isRemoved` for concurrency
+		if node == nil {
+			rht.numberOfRemovedElement++
+			newNode := newRHTNode(k, ``, executedAt, true)
+			rht.nodeMapByKey[k] = newNode
+			return ""
+		}
+
 		alreadyRemoved := node.isRemoved
 		if !alreadyRemoved {
 			rht.numberOfRemovedElement++

pkg/document/crdt/tree.go

@@ -876,6 +876,38 @@ func (t *Tree) Style(from, to *TreePos, attributes map[string]string, editedAt *
 	return nil
 }
 
+// RemoveStyle removes the given attributes of the given range.
+func (t *Tree) RemoveStyle(from, to *TreePos, attributesToRemove []string, editedAt *time.Ticket) error {
+	// 01. split text nodes at the given range if needed.
+	fromParent, fromLeft, err := t.FindTreeNodesWithSplitText(from, editedAt)
+	if err != nil {
+		return err
+	}
+	toParent, toLeft, err := t.FindTreeNodesWithSplitText(to, editedAt)
+	if err != nil {
+		return err
+	}
+
+	err = t.traverseInPosRange(fromParent, fromLeft, toParent, toLeft,
+		func(token index.TreeToken[*TreeNode], _ bool) {
+			node := token.Node
+			// NOTE(justiceHui): Even if key is not existed, we must set flag `isRemoved` for concurrency
+			if !node.IsRemoved() && !node.IsText() {
+				if node.Attrs == nil {
+					node.Attrs = NewRHT()
+				}
+				for _, value := range attributesToRemove {
+					node.Attrs.Remove(value, editedAt)
+				}
+			}
+		})
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
 // FindTreeNodesWithSplitText finds TreeNode of the given crdt.TreePos and
 // splits the text node if the position is in the middle of the text node.
 // crdt.TreePos is a position in the CRDT perspective. This is different

pkg/document/json/tree.go

@@ -207,6 +207,10 @@ func (t *Tree) Style(fromIdx, toIdx int, attributes map[string]string) bool {
 		panic("from should be less than or equal to to")
 	}
 
+	if len(attributes) == 0 {
+		return true
+	}
+
 	fromPos, err := t.Tree.FindPos(fromIdx)
 	if err != nil {
 		panic(err)
@@ -232,6 +236,41 @@ func (t *Tree) Style(fromIdx, toIdx int, attributes map[string]string) bool {
 	return true
 }
 
+// RemoveStyle sets the attributes to the elements of the given range.
+func (t *Tree) RemoveStyle(fromIdx, toIdx int, attributesToRemove []string) bool {
+	if fromIdx > toIdx {
+		panic("from should be less than or equal to to")
+	}
+
+	if len(attributesToRemove) == 0 {
+		return true
+	}
+
+	fromPos, err := t.Tree.FindPos(fromIdx)
+	if err != nil {
+		panic(err)
+	}
+	toPos, err := t.Tree.FindPos(toIdx)
+	if err != nil {
+		panic(err)
+	}
+
+	ticket := t.context.IssueTimeTicket()
+	if err := t.Tree.RemoveStyle(fromPos, toPos, attributesToRemove, ticket); err != nil {
+		panic(err)
+	}
+
+	t.context.Push(operations.NewTreeStyleRemove(
+		t.CreatedAt(),
+		fromPos,
+		toPos,
+		attributesToRemove,
+		ticket,
+	))
+
+	return true
+}
+
 // Len returns the length of this tree.
 func (t *Tree) Len() int {
 	return t.IndexTree.Root().Len()

pkg/document/operations/tree_style.go

@@ -32,9 +32,12 @@ type TreeStyle struct {
 	// toPos represents the end point of the editing range.
 	to *crdt.TreePos
 
-	// attributes represents the tree style.
+	// attributes represents the tree style to be added.
 	attributes map[string]string
 
+	// attributesToRemove represents the tree style to be removed.
+	attributesToRemove []string
+
 	// executedAt is the time the operation was executed.
 	executedAt *time.Ticket
 }
@@ -48,11 +51,30 @@ func NewTreeStyle(
 	executedAt *time.Ticket,
 ) *TreeStyle {
 	return &TreeStyle{
-		parentCreatedAt: parentCreatedAt,
-		from:            from,
-		to:              to,
-		attributes:      attributes,
-		executedAt:      executedAt,
+		parentCreatedAt:    parentCreatedAt,
+		from:               from,
+		to:                 to,
+		attributes:         attributes,
+		attributesToRemove: []string{},
+		executedAt:         executedAt,
+	}
+}
+
+// NewTreeStyleRemove creates a new instance of TreeStyle.
+func NewTreeStyleRemove(
+	parentCreatedAt *time.Ticket,
+	from *crdt.TreePos,
+	to *crdt.TreePos,
+	attributesToRemove []string,
+	executedAt *time.Ticket,
+) *TreeStyle {
+	return &TreeStyle{
+		parentCreatedAt:    parentCreatedAt,
+		from:               from,
+		to:                 to,
+		attributes:         map[string]string{},
+		attributesToRemove: attributesToRemove,
+		executedAt:         executedAt,
 	}
 }
 
@@ -64,7 +86,11 @@ func (e *TreeStyle) Execute(root *crdt.Root) error {
 		return ErrNotApplicableDataType
 	}
 
-	return obj.Style(e.from, e.to, e.attributes, e.executedAt)
+	if len(e.attributes) > 0 {
+		return obj.Style(e.from, e.to, e.attributes, e.executedAt)
+	}
+
+	return obj.RemoveStyle(e.from, e.to, e.attributesToRemove, e.executedAt)
 }
 
 // FromPos returns the start point of the editing range.
@@ -96,3 +122,8 @@ func (e *TreeStyle) ParentCreatedAt() *time.Ticket {
 func (e *TreeStyle) Attributes() map[string]string {
 	return e.attributes
 }
+
+// AttributesToRemove returns the content of Style.
+func (e *TreeStyle) AttributesToRemove() []string {
+	return e.attributesToRemove
+}

test/integration/main_test.go

@@ -88,6 +88,37 @@ func syncClientsThenAssertEqual(t *testing.T, pairs []clientAndDocPair) {
 	}
 }
 
+func syncClientsThenCheckEqual(t *testing.T, pairs []clientAndDocPair) bool {
+	assert.True(t, len(pairs) > 1)
+	ctx := context.Background()
+	// Save own changes and get previous changes.
+	for i, pair := range pairs {
+		fmt.Printf("before d%d: %s\n", i+1, pair.doc.Marshal())
+		err := pair.cli.Sync(ctx)
+		assert.NoError(t, err)
+	}
+
+	// Get last client changes.
+	// Last client get all precede changes in above loop.
+	for _, pair := range pairs[:len(pairs)-1] {
+		err := pair.cli.Sync(ctx)
+		assert.NoError(t, err)
+	}
+
+	// Assert start.
+	expected := pairs[0].doc.Marshal()
+	fmt.Printf("after d1: %s\n", expected)
+	for i, pair := range pairs[1:] {
+		v := pair.doc.Marshal()
+		fmt.Printf("after d%d: %s\n", i+2, v)
+		if expected != v {
+			return false
+		}
+	}
+
+	return true
+}
+
 // activeClients creates and activates the given number of clients.
 func activeClients(t *testing.T, n int) (clients []*client.Client) {
 	for i := 0; i < n; i++ {