Remove panic from crdt.RGATreeList (#596)

api/converter/from_bytes.go

@@ -147,7 +147,9 @@ func fromJSONArray(pbArr *api.JSONElement_JSONArray) (*crdt.Array, error) {
 		if err != nil {
 			return nil, err
 		}
-		elements.Add(elem)
+		if err = elements.Add(elem); err != nil {
+			return nil, err
+		}
 	}
 
 	createdAt, err := fromTimeTicket(pbArr.CreatedAt)

pkg/document/crdt/array.go

@@ -43,9 +43,8 @@ func (a *Array) Purge(elem Element) error {
 }
 
 // Add adds the given element at the last.
-func (a *Array) Add(elem Element) *Array {
-	a.elements.Add(elem)
-	return a
+func (a *Array) Add(elem Element) error {
+	return a.elements.Add(elem)
 }
 
 // Get returns the element of the given index.
@@ -59,7 +58,7 @@ func (a *Array) Get(idx int) (Element, error) {
 
 // FindPrevCreatedAt returns the creation time of the previous element of the
 // given element.
-func (a *Array) FindPrevCreatedAt(createdAt *time.Ticket) *time.Ticket {
+func (a *Array) FindPrevCreatedAt(createdAt *time.Ticket) (*time.Ticket, error) {
 	return a.elements.FindPrevCreatedAt(createdAt)
 }
 
@@ -74,8 +73,8 @@ func (a *Array) Delete(idx int, deletedAt *time.Ticket) (Element, error) {
 
 // MoveAfter moves the given `createdAt` element after the `prevCreatedAt`
 // element.
-func (a *Array) MoveAfter(prevCreatedAt, createdAt, executedAt *time.Ticket) {
-	a.elements.MoveAfter(prevCreatedAt, createdAt, executedAt)
+func (a *Array) MoveAfter(prevCreatedAt, createdAt, executedAt *time.Ticket) error {
+	return a.elements.MoveAfter(prevCreatedAt, createdAt, executedAt)
 }
 
 // Elements returns an array of elements contained in this RGATreeList.
@@ -111,7 +110,9 @@ func (a *Array) DeepCopy() (Element, error) {
 		if err != nil {
 			return nil, err
 		}
-		elements.Add(copiedNode)
+		if err = elements.Add(copiedNode); err != nil {
+			return nil, err
+		}
 	}
 
 	array := NewArray(elements, a.createdAt)
@@ -160,13 +161,17 @@ func (a *Array) LastCreatedAt() *time.Ticket {
 }
 
 // InsertAfter inserts the given element after the given previous element.
-func (a *Array) InsertAfter(prevCreatedAt *time.Ticket, element Element) {
-	a.elements.InsertAfter(prevCreatedAt, element)
+func (a *Array) InsertAfter(prevCreatedAt *time.Ticket, element Element) error {
+	return a.elements.InsertAfter(prevCreatedAt, element)
 }
 
 // DeleteByCreatedAt deletes the given element.
-func (a *Array) DeleteByCreatedAt(createdAt *time.Ticket, deletedAt *time.Ticket) Element {
-	return a.elements.DeleteByCreatedAt(createdAt, deletedAt).elem
+func (a *Array) DeleteByCreatedAt(createdAt *time.Ticket, deletedAt *time.Ticket) (Element, error) {
+	node, err := a.elements.DeleteByCreatedAt(createdAt, deletedAt)
+	if err != nil {
+		return nil, err
+	}
+	return node.elem, nil
 }
 
 // Len returns length of this Array.

pkg/document/crdt/array_test.go

@@ -32,11 +32,14 @@ func TestArray(t *testing.T) {
 
 		a := crdt.NewArray(crdt.NewRGATreeList(), ctx.IssueTimeTicket())
 
-		a.Add(crdt.NewPrimitive("1", ctx.IssueTimeTicket()))
+		err := a.Add(crdt.NewPrimitive("1", ctx.IssueTimeTicket()))
+		assert.NoError(t, err)
 		assert.Equal(t, `["1"]`, a.Marshal())
-		a.Add(crdt.NewPrimitive("2", ctx.IssueTimeTicket()))
+		err = a.Add(crdt.NewPrimitive("2", ctx.IssueTimeTicket()))
+		assert.NoError(t, err)
 		assert.Equal(t, `["1","2"]`, a.Marshal())
-		a.Add(crdt.NewPrimitive("3", ctx.IssueTimeTicket()))
+		err = a.Add(crdt.NewPrimitive("3", ctx.IssueTimeTicket()))
+		assert.NoError(t, err)
 		assert.Equal(t, `["1","2","3"]`, a.Marshal())
 	})
 }

pkg/document/crdt/element.go

@@ -36,7 +36,7 @@ type Container interface {
 	Descendants(callback func(elem Element, parent Container) bool)
 
 	// DeleteByCreatedAt removes the given element from this container.
-	DeleteByCreatedAt(createdAt *time.Ticket, deletedAt *time.Ticket) Element
+	DeleteByCreatedAt(createdAt *time.Ticket, deletedAt *time.Ticket) (Element, error)
 }
 
 // GCElement represents Element which has GC.

pkg/document/crdt/element_rht.go

@@ -129,17 +129,17 @@ func (rht *ElementRHT) Delete(k string, deletedAt *time.Ticket) Element {
 }
 
 // DeleteByCreatedAt deletes the Element of the given creation time.
-func (rht *ElementRHT) DeleteByCreatedAt(createdAt *time.Ticket, deletedAt *time.Ticket) Element {
+func (rht *ElementRHT) DeleteByCreatedAt(createdAt *time.Ticket, deletedAt *time.Ticket) (Element, error) {
 	node, ok := rht.nodeMapByCreatedAt[createdAt.Key()]
 	if !ok {
-		return nil
+		return nil, fmt.Errorf("DeleteByCreatedAt %s: %w", createdAt.Key(), ErrChildNotFound)
 	}
 
 	if !node.Remove(deletedAt) {
-		return nil
+		return nil, nil
 	}
 
-	return node.elem
+	return node.elem, nil
 }
 
 // Elements returns a map of elements because the map easy to use for loop.

pkg/document/crdt/object.go

@@ -63,7 +63,7 @@ func (o *Object) Has(k string) bool {
 }
 
 // DeleteByCreatedAt deletes the element of the given creation time.
-func (o *Object) DeleteByCreatedAt(createdAt *time.Ticket, deletedAt *time.Ticket) Element {
+func (o *Object) DeleteByCreatedAt(createdAt *time.Ticket, deletedAt *time.Ticket) (Element, error) {
 	return o.memberNodes.DeleteByCreatedAt(createdAt, deletedAt)
 }
 

pkg/document/crdt/rga_tree_list.go

@@ -149,8 +149,8 @@ func (a *RGATreeList) Marshal() string {
 }
 
 // Add adds the given element at the last.
-func (a *RGATreeList) Add(elem Element) {
-	a.insertAfter(a.last.CreatedAt(), elem, elem.CreatedAt())
+func (a *RGATreeList) Add(elem Element) error {
+	return a.insertAfter(a.last.CreatedAt(), elem, elem.CreatedAt())
 }
 
 // Nodes returns an array of elements contained in this RGATreeList.
@@ -175,8 +175,8 @@ func (a *RGATreeList) LastCreatedAt() *time.Ticket {
 }
 
 // InsertAfter inserts the given element after the given previous element.
-func (a *RGATreeList) InsertAfter(prevCreatedAt *time.Ticket, elem Element) {
-	a.insertAfter(prevCreatedAt, elem, elem.CreatedAt())
+func (a *RGATreeList) InsertAfter(prevCreatedAt *time.Ticket, elem Element) error {
+	return a.insertAfter(prevCreatedAt, elem, elem.CreatedAt())
 }
 
 // Get returns the element of the given index.
@@ -207,17 +207,17 @@ func (a *RGATreeList) Get(idx int) (*RGATreeListNode, error) {
 }
 
 // DeleteByCreatedAt deletes the given element.
-func (a *RGATreeList) DeleteByCreatedAt(createdAt *time.Ticket, deletedAt *time.Ticket) *RGATreeListNode {
+func (a *RGATreeList) DeleteByCreatedAt(createdAt *time.Ticket, deletedAt *time.Ticket) (*RGATreeListNode, error) {
 	node, ok := a.nodeMapByCreatedAt[createdAt.Key()]
 	if !ok {
-		panic("fail to find the given createdAt: " + createdAt.Key())
+		return nil, fmt.Errorf("DeleteByCreatedAt %s: %w", createdAt.Key(), ErrChildNotFound)
 	}
 
 	alreadyRemoved := node.isRemoved()
 	if node.elem.Remove(deletedAt) && !alreadyRemoved {
 		a.nodeMapByIndex.Splay(node.indexNode)
 	}
-	return node
+	return node, nil
 }
 
 // Len returns length of this RGATreeList.
@@ -237,35 +237,38 @@ func (a *RGATreeList) Delete(idx int, deletedAt *time.Ticket) (*RGATreeListNode,
 	if err != nil {
 		return nil, err
 	}
-	return a.DeleteByCreatedAt(target.CreatedAt(), deletedAt), nil
+	return a.DeleteByCreatedAt(target.CreatedAt(), deletedAt)
 }
 
 // MoveAfter moves the given `createdAt` element after the `prevCreatedAt`
 // element.
-func (a *RGATreeList) MoveAfter(prevCreatedAt, createdAt, executedAt *time.Ticket) {
+func (a *RGATreeList) MoveAfter(prevCreatedAt, createdAt, executedAt *time.Ticket) error {
 	prevNode, ok := a.nodeMapByCreatedAt[prevCreatedAt.Key()]
 	if !ok {
-		panic("fail to find the given prevCreatedAt: " + prevCreatedAt.Key())
+		return fmt.Errorf("MoveAfter %s: %w", prevCreatedAt.Key(), ErrChildNotFound)
 	}
 
 	node, ok := a.nodeMapByCreatedAt[createdAt.Key()]
 	if !ok {
-		panic("fail to find the given createdAt: " + createdAt.Key())
+		return fmt.Errorf("MoveAfter %s: %w", createdAt.Key(), ErrChildNotFound)
 	}
 
 	if node.elem.MovedAt() == nil || executedAt.After(node.elem.MovedAt()) {
 		a.release(node)
-		a.insertAfter(prevNode.CreatedAt(), node.elem, executedAt)
+		if err := a.insertAfter(prevNode.CreatedAt(), node.elem, executedAt); err != nil {
+			return err
+		}
 		node.elem.SetMovedAt(executedAt)
 	}
+	return nil
 }
 
 // FindPrevCreatedAt returns the creation time of the previous element of the
 // given element.
-func (a *RGATreeList) FindPrevCreatedAt(createdAt *time.Ticket) *time.Ticket {
+func (a *RGATreeList) FindPrevCreatedAt(createdAt *time.Ticket) (*time.Ticket, error) {
 	node, ok := a.nodeMapByCreatedAt[createdAt.Key()]
 	if !ok {
-		panic("fail to find the given prevCreatedAt: " + createdAt.Key())
+		return nil, fmt.Errorf("FindPrevCreatedAt %s: %w", createdAt.Key(), ErrChildNotFound)
 	}
 
 	for {
@@ -275,7 +278,7 @@ func (a *RGATreeList) FindPrevCreatedAt(createdAt *time.Ticket) *time.Ticket {
 		}
 	}
 
-	return node.CreatedAt()
+	return node.CreatedAt(), nil
 }
 
 // purge physically purge child element.
@@ -293,17 +296,17 @@ func (a *RGATreeList) purge(elem Element) error {
 func (a *RGATreeList) findNextBeforeExecutedAt(
 	createdAt *time.Ticket,
 	executedAt *time.Ticket,
-) *RGATreeListNode {
+) (*RGATreeListNode, error) {
 	node, ok := a.nodeMapByCreatedAt[createdAt.Key()]
 	if !ok {
-		panic("fail to find the given createdAt: " + createdAt.Key())
+		return nil, fmt.Errorf("findNextBeforeExecutedAt %s: %w", createdAt.Key(), ErrChildNotFound)
 	}
 
 	for node.next != nil && node.next.PositionedAt().After(executedAt) {
 		node = node.next
 	}
 
-	return node
+	return node, nil
 }
 
 func (a *RGATreeList) release(node *RGATreeListNode) {
@@ -325,13 +328,18 @@ func (a *RGATreeList) insertAfter(
 	prevCreatedAt *time.Ticket,
 	value Element,
 	executedAt *time.Ticket,
-) {
-	prevNode := a.findNextBeforeExecutedAt(prevCreatedAt, executedAt)
+) error {
+	prevNode, err := a.findNextBeforeExecutedAt(prevCreatedAt, executedAt)
+	if err != nil {
+		return err
+	}
+
 	newNode := newRGATreeListNodeAfter(prevNode, value)
 	if prevNode == a.last {
 		a.last = newNode
 	}
 
 	a.nodeMapByIndex.InsertAfter(prevNode.indexNode, newNode.indexNode)
 	a.nodeMapByCreatedAt[value.CreatedAt().Key()] = newNode
+	return nil
 }

pkg/document/crdt/rga_tree_list_test.go

@@ -0,0 +1,72 @@
+package crdt_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/yorkie-team/yorkie/pkg/document/crdt"
+	"github.com/yorkie-team/yorkie/test/helper"
+)
+
+func TestRGATreeList(t *testing.T) {
+	t.Run("rga_tree_list operations test", func(t *testing.T) {
+		root := helper.TestRoot()
+		ctx := helper.TextChangeContext(root)
+
+		elements := crdt.NewRGATreeList()
+
+		var err error
+		for _, v := range []string{"1", "2", "3"} {
+			err = elements.Add(crdt.NewPrimitive(v, ctx.IssueTimeTicket()))
+			assert.NoError(t, err)
+		}
+		assert.Equal(t, `["1","2","3"]`, elements.Marshal())
+
+		nodes := elements.Nodes()
+		assert.Equal(t, len(nodes), 3)
+
+		targetElement, err := elements.Get(1)
+		assert.NoError(t, err)
+		assert.Equal(t, `"2"`, targetElement.Element().Marshal())
+
+		prevCreatedAt, err := elements.FindPrevCreatedAt(targetElement.CreatedAt())
+		assert.NoError(t, err)
+		assert.Equal(t, prevCreatedAt.Compare(targetElement.CreatedAt()), -1)
+
+		err = elements.MoveAfter(targetElement.CreatedAt(), prevCreatedAt, ctx.IssueTimeTicket())
+		assert.NoError(t, err)
+		assert.Equal(t, `["2","1","3"]`, elements.Marshal())
+
+		_, err = elements.DeleteByCreatedAt(targetElement.CreatedAt(), ctx.IssueTimeTicket())
+		assert.NoError(t, err)
+		assert.Equal(t, `["1","3"]`, elements.Marshal())
+
+		_, err = elements.Delete(1, ctx.IssueTimeTicket())
+		assert.NoError(t, err)
+		assert.Equal(t, `["1"]`, elements.Marshal())
+
+	})
+
+	t.Run("invalid createdAt test", func(t *testing.T) {
+		root := helper.TestRoot()
+		ctx := helper.TextChangeContext(root)
+
+		validCreatedAt, invalidCreatedAt := ctx.IssueTimeTicket(), ctx.IssueTimeTicket()
+		elements := crdt.NewRGATreeList()
+		err := elements.Add(crdt.NewPrimitive("1", validCreatedAt))
+		assert.NoError(t, err)
+
+		_, err = elements.DeleteByCreatedAt(invalidCreatedAt, ctx.IssueTimeTicket())
+		assert.ErrorIs(t, err, crdt.ErrChildNotFound)
+
+		err = elements.MoveAfter(validCreatedAt, invalidCreatedAt, ctx.IssueTimeTicket())
+		assert.ErrorIs(t, err, crdt.ErrChildNotFound)
+
+		err = elements.MoveAfter(invalidCreatedAt, validCreatedAt, ctx.IssueTimeTicket())
+		assert.ErrorIs(t, err, crdt.ErrChildNotFound)
+
+		_, err = elements.FindPrevCreatedAt(invalidCreatedAt)
+		assert.ErrorIs(t, err, crdt.ErrChildNotFound)
+	})
+}

pkg/document/crdt/root_test.go

@@ -36,15 +36,18 @@ func TestRoot(t *testing.T) {
 	t.Run("garbage collection for array test", func(t *testing.T) {
 		root := helper.TestRoot()
 		ctx := helper.TextChangeContext(root)
-		array := crdt.NewArray(crdt.NewRGATreeList(), ctx.IssueTimeTicket())
 
-		array.Add(crdt.NewPrimitive(0, ctx.IssueTimeTicket()))
-		array.Add(crdt.NewPrimitive(1, ctx.IssueTimeTicket()))
-		array.Add(crdt.NewPrimitive(2, ctx.IssueTimeTicket()))
+		array := crdt.NewArray(crdt.NewRGATreeList(), ctx.IssueTimeTicket())
+		var err error
+		for _, v := range []int{0, 1, 2} {
+			err = array.Add(crdt.NewPrimitive(v, ctx.IssueTimeTicket()))
+			assert.NoError(t, err)
+		}
 		assert.Equal(t, "[0,1,2]", array.Marshal())
 
 		targetElement, _ := array.Get(1)
-		array.DeleteByCreatedAt(targetElement.CreatedAt(), ctx.IssueTimeTicket())
+		_, err = array.DeleteByCreatedAt(targetElement.CreatedAt(), ctx.IssueTimeTicket())
+		assert.NoError(t, err)
 		root.RegisterRemovedElementPair(array, targetElement)
 		assert.Equal(t, "[0,2]", array.Marshal())
 		assert.Equal(t, 1, root.GarbageLen())
@@ -182,10 +185,14 @@ func TestRoot(t *testing.T) {
 
 		obj := root.Object()
 		obj.Set("1", crdt.NewPrimitive(1, ctx.IssueTimeTicket()))
-		arr := crdt.NewArray(crdt.NewRGATreeList(), ctx.IssueTimeTicket()).
-			Add(crdt.NewPrimitive(1, ctx.IssueTimeTicket())).
-			Add(crdt.NewPrimitive(2, ctx.IssueTimeTicket())).
-			Add(crdt.NewPrimitive(3, ctx.IssueTimeTicket()))
+
+		arr := crdt.NewArray(crdt.NewRGATreeList(), ctx.IssueTimeTicket())
+		var err error
+		for _, v := range []int{1, 2, 3} {
+			err = arr.Add(crdt.NewPrimitive(v, ctx.IssueTimeTicket()))
+			assert.NoError(t, err)
+		}
+
 		obj.Set("2", arr)
 		obj.Set("3", crdt.NewPrimitive(3, ctx.IssueTimeTicket()))
 		assert.Equal(t, `{"1":1,"2":[1,2,3],"3":3}`, root.Object().Marshal())