Skip to content

Commit

Permalink
Remove panic from crdt.Text (#522)
Browse files Browse the repository at this point in the history
json.Text is used by SDK and does not run on the server. So this commit
remains panic from json.Text to keep usability.
  • Loading branch information
emplam27 authored Apr 26, 2023
1 parent 8edb7b0 commit 66c17d6
Show file tree
Hide file tree
Showing 20 changed files with 148 additions and 74 deletions.
3 changes: 1 addition & 2 deletions api/converter/from_bytes.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ func fromJSONText(
}

rgaTreeSplit := crdt.NewRGATreeSplit(crdt.InitialTextNode())

current := rgaTreeSplit.InitialHead()
for _, pbNode := range pbText.Nodes {
textNode, err := fromTextNode(pbNode)
Expand All @@ -190,7 +189,7 @@ func fromJSONText(
if insPrevID != nil {
insPrevNode := rgaTreeSplit.FindNode(insPrevID)
if insPrevNode == nil {
panic("insPrevNode should be presence")
return nil, fmt.Errorf("insPrevNode should be presence")
}
current.SetInsPrev(insPrevNode)
}
Expand Down
10 changes: 7 additions & 3 deletions pkg/document/crdt/array.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,20 @@ func (a *Array) StructureAsString() string {
}

// DeepCopy copies itself deeply.
func (a *Array) DeepCopy() Element {
func (a *Array) DeepCopy() (Element, error) {
elements := NewRGATreeList()

for _, node := range a.elements.Nodes() {
elements.Add(node.elem.DeepCopy())
copiedNode, err := node.elem.DeepCopy()
if err != nil {
return nil, err
}
elements.Add(copiedNode)
}

array := NewArray(elements, a.createdAt)
array.removedAt = a.removedAt
return array
return array, nil
}

// CreatedAt returns the creation time of this array.
Expand Down
4 changes: 2 additions & 2 deletions pkg/document/crdt/counter.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,9 @@ func (p *Counter) Marshal() string {
}

// DeepCopy copies itself deeply.
func (p *Counter) DeepCopy() Element {
func (p *Counter) DeepCopy() (Element, error) {
counter := *p
return &counter
return &counter, nil
}

// CreatedAt returns the creation time.
Expand Down
2 changes: 1 addition & 1 deletion pkg/document/crdt/element.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ type Element interface {
Marshal() string

// DeepCopy copies itself deeply.
DeepCopy() Element
DeepCopy() (Element, error)

// CreatedAt returns the creation time of this element.
CreatedAt() *time.Ticket
Expand Down
10 changes: 7 additions & 3 deletions pkg/document/crdt/object.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,20 @@ func (o *Object) Marshal() string {
}

// DeepCopy copies itself deeply.
func (o *Object) DeepCopy() Element {
func (o *Object) DeepCopy() (Element, error) {
members := NewElementRHT()

for _, node := range o.memberNodes.Nodes() {
members.Set(node.key, node.elem.DeepCopy())
copiedNode, err := node.elem.DeepCopy()
if err != nil {
return nil, err
}
members.Set(node.key, copiedNode)
}

obj := NewObject(members, o.createdAt)
obj.removedAt = o.removedAt
return obj
return obj, nil
}

// CreatedAt returns the creation time of this object.
Expand Down
4 changes: 2 additions & 2 deletions pkg/document/crdt/primitive.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,9 @@ func (p *Primitive) Marshal() string {
}

// DeepCopy copies itself deeply.
func (p *Primitive) DeepCopy() Element {
func (p *Primitive) DeepCopy() (Element, error) {
primitive := *p
return &primitive
return &primitive, nil
}

// CreatedAt returns the creation time.
Expand Down
3 changes: 2 additions & 1 deletion pkg/document/crdt/primitive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ func TestPrimitive(t *testing.T) {
assert.Equal(t, prim.Value(), crdt.ValueFromBytes(prim.ValueType(), prim.Bytes()))
assert.Equal(t, prim.Marshal(), test.marshal)

copied := prim.DeepCopy()
copied, err := prim.DeepCopy()
assert.NoError(t, err)
assert.Equal(t, prim.CreatedAt(), copied.CreatedAt())
assert.Equal(t, prim.MovedAt(), copied.MovedAt())
assert.Equal(t, prim.Marshal(), copied.Marshal())
Expand Down
49 changes: 31 additions & 18 deletions pkg/document/crdt/rga_tree_split.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ func NewRGATreeSplitNodeID(createdAt *time.Ticket, offset int) *RGATreeSplitNode

// Compare returns an integer comparing two ID. The result will be 0 if
// id==other, -1 if id < other, and +1 if id > other. If the receiver or
// argument is nil, it would panic at runtime.
// argument is nil, it would panic at runtime. This method is following
// golang standard interface.
func (id *RGATreeSplitNodeID) Compare(other llrb.Key) int {
if id == nil || other == nil {
panic("RGATreeSplitNodeID cannot be null")
Expand Down Expand Up @@ -330,47 +331,53 @@ func (s *RGATreeSplit[V]) findNodePos(index int) *RGATreeSplitNodePos {
func (s *RGATreeSplit[V]) findNodeWithSplit(
pos *RGATreeSplitNodePos,
updatedAt *time.Ticket,
) (*RGATreeSplitNode[V], *RGATreeSplitNode[V]) {
) (*RGATreeSplitNode[V], *RGATreeSplitNode[V], error) {
absoluteID := pos.getAbsoluteID()
node := s.findFloorNodePreferToLeft(absoluteID)
node, err := s.findFloorNodePreferToLeft(absoluteID)
if err != nil {
return nil, nil, err
}

relativeOffset := absoluteID.offset - node.id.offset

s.splitNode(node, relativeOffset)
_, err = s.splitNode(node, relativeOffset)
if err != nil {
return nil, nil, err
}

for node.next != nil && node.next.createdAt().After(updatedAt) {
node = node.next
}

return node, node.next
return node, node.next, nil
}

func (s *RGATreeSplit[V]) findFloorNodePreferToLeft(id *RGATreeSplitNodeID) *RGATreeSplitNode[V] {
func (s *RGATreeSplit[V]) findFloorNodePreferToLeft(id *RGATreeSplitNodeID) (*RGATreeSplitNode[V], error) {
node := s.findFloorNode(id)
if node == nil {
panic("the node of the given id should be found: " + s.StructureAsString())
return nil, fmt.Errorf("the node of the given id should be found: " + s.StructureAsString())
}

if id.offset > 0 && node.id.offset == id.offset {
// NOTE: InsPrev may not be present due to GC.
if node.insPrev == nil {
return node
return node, nil
}
node = node.insPrev
}

return node
return node, nil
}

func (s *RGATreeSplit[V]) splitNode(node *RGATreeSplitNode[V], offset int) *RGATreeSplitNode[V] {
func (s *RGATreeSplit[V]) splitNode(node *RGATreeSplitNode[V], offset int) (*RGATreeSplitNode[V], error) {
if offset > node.contentLen() {
panic("offset should be less than or equal to length: " + s.StructureAsString())
return nil, fmt.Errorf("offset should be less than or equal to length: " + s.StructureAsString())
}

if offset == 0 {
return node
return node, nil
} else if offset == node.contentLen() {
return node.next
return node.next, nil
}

splitNode := node.split(offset)
Expand All @@ -383,7 +390,7 @@ func (s *RGATreeSplit[V]) splitNode(node *RGATreeSplitNode[V], offset int) *RGAT
}
splitNode.SetInsPrev(node)

return splitNode
return splitNode, nil
}

// InsertAfter inserts the given node after the given previous node.
Expand Down Expand Up @@ -439,10 +446,16 @@ func (s *RGATreeSplit[V]) edit(
latestCreatedAtMapByActor map[string]*time.Ticket,
content V,
editedAt *time.Ticket,
) (*RGATreeSplitNodePos, map[string]*time.Ticket) {
) (*RGATreeSplitNodePos, map[string]*time.Ticket, error) {
// 01. Split nodes with from and to
toLeft, toRight := s.findNodeWithSplit(to, editedAt)
fromLeft, fromRight := s.findNodeWithSplit(from, editedAt)
toLeft, toRight, err := s.findNodeWithSplit(to, editedAt)
if err != nil {
return nil, nil, err
}
fromLeft, fromRight, err := s.findNodeWithSplit(from, editedAt)
if err != nil {
return nil, nil, err
}

// 02. delete between from and to
nodesToDelete := s.findBetween(fromRight, toRight)
Expand All @@ -467,7 +480,7 @@ func (s *RGATreeSplit[V]) edit(
s.removedNodeMap[key] = removedNode
}

return caretPos, latestCreatedAtMap
return caretPos, latestCreatedAtMap, nil
}

func (s *RGATreeSplit[V]) findBetween(from, to *RGATreeSplitNode[V]) []*RGATreeSplitNode[V] {
Expand Down
8 changes: 6 additions & 2 deletions pkg/document/crdt/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,12 @@ func (r *Root) RegisterTextElementWithGarbage(textType TextElement) {
}

// DeepCopy copies itself deeply.
func (r *Root) DeepCopy() *Root {
return NewRoot(r.object.DeepCopy().(*Object))
func (r *Root) DeepCopy() (*Root, error) {
copiedObject, err := r.object.DeepCopy()
if err != nil {
return nil, err
}
return NewRoot(copiedObject.(*Object)), nil
}

// GarbageCollect purge elements that were removed before the given time.
Expand Down
24 changes: 16 additions & 8 deletions pkg/document/crdt/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,25 +59,29 @@ func TestRoot(t *testing.T) {
text := crdt.NewText(crdt.NewRGATreeSplit(crdt.InitialTextNode()), ctx.IssueTimeTicket())

fromPos, toPos := text.CreateRange(0, 0)
text.Edit(fromPos, toPos, nil, "Hello World", nil, ctx.IssueTimeTicket())
_, _, err := text.Edit(fromPos, toPos, nil, "Hello World", nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
registerTextElementWithGarbage(fromPos, toPos, root, text)
assert.Equal(t, "Hello World", text.String())
assert.Equal(t, 0, root.GarbageLen())

fromPos, toPos = text.CreateRange(5, 10)
text.Edit(fromPos, toPos, nil, "Yorkie", nil, ctx.IssueTimeTicket())
_, _, err = text.Edit(fromPos, toPos, nil, "Yorkie", nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
registerTextElementWithGarbage(fromPos, toPos, root, text)
assert.Equal(t, "HelloYorkied", text.String())
assert.Equal(t, 1, root.GarbageLen())

fromPos, toPos = text.CreateRange(0, 5)
text.Edit(fromPos, toPos, nil, "", nil, ctx.IssueTimeTicket())
_, _, err = text.Edit(fromPos, toPos, nil, "", nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
registerTextElementWithGarbage(fromPos, toPos, root, text)
assert.Equal(t, "Yorkied", text.String())
assert.Equal(t, 2, root.GarbageLen())

fromPos, toPos = text.CreateRange(6, 7)
text.Edit(fromPos, toPos, nil, "", nil, ctx.IssueTimeTicket())
_, _, err = text.Edit(fromPos, toPos, nil, "", nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
registerTextElementWithGarbage(fromPos, toPos, root, text)
assert.Equal(t, "Yorkie", text.String())
assert.Equal(t, 3, root.GarbageLen())
Expand Down Expand Up @@ -115,7 +119,8 @@ func TestRoot(t *testing.T) {

for _, tc := range tests {
fromPos, toPos := text.CreateRange(tc.from, tc.to)
text.Edit(fromPos, toPos, nil, tc.content, nil, ctx.IssueTimeTicket())
_, _, err := text.Edit(fromPos, toPos, nil, tc.content, nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
registerTextElementWithGarbage(fromPos, toPos, root, text)
assert.Equal(t, tc.want, text.String())
assert.Equal(t, tc.garbage, root.GarbageLen())
Expand All @@ -131,19 +136,22 @@ func TestRoot(t *testing.T) {
text := crdt.NewText(crdt.NewRGATreeSplit(crdt.InitialTextNode()), ctx.IssueTimeTicket())

fromPos, toPos := text.CreateRange(0, 0)
text.Edit(fromPos, toPos, nil, "Hello World", nil, ctx.IssueTimeTicket())
_, _, err := text.Edit(fromPos, toPos, nil, "Hello World", nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
registerTextElementWithGarbage(fromPos, toPos, root, text)
assert.Equal(t, `[{"val":"Hello World"}]`, text.Marshal())
assert.Equal(t, 0, root.GarbageLen())

fromPos, toPos = text.CreateRange(6, 11)
text.Edit(fromPos, toPos, nil, "Yorkie", nil, ctx.IssueTimeTicket())
_, _, err = text.Edit(fromPos, toPos, nil, "Yorkie", nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
registerTextElementWithGarbage(fromPos, toPos, root, text)
assert.Equal(t, `[{"val":"Hello "},{"val":"Yorkie"}]`, text.Marshal())
assert.Equal(t, 1, root.GarbageLen())

fromPos, toPos = text.CreateRange(0, 6)
text.Edit(fromPos, toPos, nil, "", nil, ctx.IssueTimeTicket())
_, _, err = text.Edit(fromPos, toPos, nil, "", nil, ctx.IssueTimeTicket())
assert.NoError(t, err)
registerTextElementWithGarbage(fromPos, toPos, root, text)
assert.Equal(t, `[{"val":"Yorkie"}]`, text.Marshal())
assert.Equal(t, 2, root.GarbageLen())
Expand Down
27 changes: 16 additions & 11 deletions pkg/document/crdt/text.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,23 +166,23 @@ func (t *Text) Marshal() string {
}

// DeepCopy copies itself deeply.
func (t *Text) DeepCopy() Element {
func (t *Text) DeepCopy() (Element, error) {
rgaTreeSplit := NewRGATreeSplit(InitialTextNode())

current := rgaTreeSplit.InitialHead()

for _, node := range t.Nodes() {
current = rgaTreeSplit.InsertAfter(current, node.DeepCopy())
insPrevID := node.InsPrevID()
if insPrevID != nil {
insPrevNode := rgaTreeSplit.FindNode(insPrevID)
if insPrevNode == nil {
panic("insPrevNode should be presence")
return nil, fmt.Errorf("insPrevNode should be presence")
}
current.SetInsPrev(insPrevNode)
}
}

return NewText(rgaTreeSplit, t.createdAt)
return NewText(rgaTreeSplit, t.createdAt), nil
}

// CreatedAt returns the creation time of this Text.
Expand Down Expand Up @@ -233,21 +233,19 @@ func (t *Text) Edit(
content string,
attributes map[string]string,
executedAt *time.Ticket,
) (*RGATreeSplitNodePos, map[string]*time.Ticket) {
) (*RGATreeSplitNodePos, map[string]*time.Ticket, error) {
val := NewTextValue(content, NewRHT())
for key, value := range attributes {
val.attrs.Set(key, value, executedAt)
}

cursorPos, latestCreatedAtMapByActor := t.rgaTreeSplit.edit(
return t.rgaTreeSplit.edit(
from,
to,
latestCreatedAtMapByActor,
val,
executedAt,
)

return cursorPos, latestCreatedAtMapByActor
}

// Style applies the given attributes of the given range.
Expand All @@ -256,10 +254,16 @@ func (t *Text) Style(
to *RGATreeSplitNodePos,
attributes map[string]string,
executedAt *time.Ticket,
) {
) error {
// 01. Split nodes with from and to
_, toRight := t.rgaTreeSplit.findNodeWithSplit(to, executedAt)
_, fromRight := t.rgaTreeSplit.findNodeWithSplit(from, executedAt)
_, toRight, err := t.rgaTreeSplit.findNodeWithSplit(to, executedAt)
if err != nil {
return err
}
_, fromRight, err := t.rgaTreeSplit.findNodeWithSplit(from, executedAt)
if err != nil {
return err
}

// 02. style nodes between from and to
nodes := t.rgaTreeSplit.findBetween(fromRight, toRight)
Expand All @@ -269,6 +273,7 @@ func (t *Text) Style(
val.attrs.Set(key, value, executedAt)
}
}
return nil
}

// Select stores that the given range has been selected.
Expand Down
Loading

0 comments on commit 66c17d6

Please sign in to comment.