Fix invalid version vector check logic for backward compatibility (#1105

) This commit modifies the version check logic for operations, as the previous SDK created operations that utilized an empty map for the version vector instead of nil. With the introduction of version vector from v0.5.7, we now need to differentiate between the Concurrent-Causal relationship while still supporting the older operations generated by versions prior to v0.5.6. The adjustment ensures that the logic appropriately processes an empty map for version vector. --------- Co-authored-by: Youngteac Hong <[email protected]>
yorkie-team · Dec 19, 2024 · 3a615c6 · 3a615c6
1 parent 2bd6d39
commit 3a615c6
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 15 deletions.
diff --git a/pkg/document/crdt/rga_tree_split.go b/pkg/document/crdt/rga_tree_split.go
@@ -527,6 +527,8 @@ func (s *RGATreeSplit[V]) deleteNodes(
 ) (map[string]*time.Ticket, map[string]*RGATreeSplitNode[V]) {
 	createdAtMapByActor := make(map[string]*time.Ticket)
 	removedNodeMap := make(map[string]*RGATreeSplitNode[V])
+	isVersionVectorEmpty := len(versionVector) == 0
+	isMaxCreatedAtMapByActorEmpty := len(maxCreatedAtMapByActor) == 0
 
 	if len(candidates) == 0 {
 		return createdAtMapByActor, removedNodeMap
@@ -545,17 +547,19 @@ func (s *RGATreeSplit[V]) deleteNodes(
 
 		var maxCreatedAt *time.Ticket
 		var clientLamportAtChange int64
-		if versionVector == nil && maxCreatedAtMapByActor == nil {
-			// Local edit - use version vector comparison
+		if isVersionVectorEmpty && isMaxCreatedAtMapByActorEmpty {
+			// Case 1: local editing from json package
 			clientLamportAtChange = time.MaxLamport
-		} else if len(versionVector) > 0 {
+		} else if !isVersionVectorEmpty {
+			// Case 2: from operation with version vector(After v0.5.7)
 			lamport, ok := versionVector.Get(actorID)
 			if ok {
 				clientLamportAtChange = lamport
 			} else {
 				clientLamportAtChange = 0
 			}
 		} else {
+			// Case 3: from operation without version vector(Before v0.5.6)
 			createdAt, ok := maxCreatedAtMapByActor[actorIDHex]
 			if ok {
 				maxCreatedAt = createdAt

diff --git a/pkg/document/crdt/text.go b/pkg/document/crdt/text.go
@@ -312,6 +312,9 @@ func (t *Text) Style(
 	// 02. style nodes between from and to
 	nodes := t.rgaTreeSplit.findBetween(fromRight, toRight)
 	createdAtMapByActor := make(map[string]*time.Ticket)
+	isVersionVectorEmpty := len(versionVector) == 0
+	isMaxCreatedAtMapByActorEmpty := len(maxCreatedAtMapByActor) == 0
+
 	var toBeStyled []*RGATreeSplitNode[*TextValue]
 
 	for _, node := range nodes {
@@ -320,17 +323,19 @@ func (t *Text) Style(
 
 		var maxCreatedAt *time.Ticket
 		var clientLamportAtChange int64
-		if versionVector == nil && maxCreatedAtMapByActor == nil {
-			// Local edit - use version vector comparison
+		if isVersionVectorEmpty && isMaxCreatedAtMapByActorEmpty {
+			// Case 1: local editing from json package
 			clientLamportAtChange = time.MaxLamport
-		} else if versionVector != nil {
+		} else if !isVersionVectorEmpty {
+			// Case 2: from operation with version vector(After v0.5.7)
 			lamport, ok := versionVector.Get(actorID)
 			if ok {
 				clientLamportAtChange = lamport
 			} else {
 				clientLamportAtChange = 0
 			}
 		} else {
+			// Case 3: from operation without version vector(Before v0.5.6)
 			createdAt, ok := maxCreatedAtMapByActor[actorIDHex]
 			if ok {
 				maxCreatedAt = createdAt

diff --git a/pkg/document/crdt/tree.go b/pkg/document/crdt/tree.go
@@ -842,6 +842,9 @@ func (t *Tree) collectBetween(
 	var toBeRemoveds []*TreeNode
 	var toBeMovedToFromParents []*TreeNode
 	createdAtMapByActor := make(map[string]*time.Ticket)
+	isVersionVectorEmpty := len(versionVector) == 0
+	isMaxCreatedAtMapByActorEmpty := len(maxCreatedAtMapByActor) == 0
+
 	if err := t.traverseInPosRange(
 		fromParent, fromLeft,
 		toParent, toLeft,
@@ -869,17 +872,20 @@ func (t *Tree) collectBetween(
 
 			var maxCreatedAt *time.Ticket
 			var clientLamportAtChange int64
-			if versionVector == nil && maxCreatedAtMapByActor == nil {
-				// Local edit - use version vector comparison
+
+			if isVersionVectorEmpty && isMaxCreatedAtMapByActorEmpty {
+				// Case 1: local editing from json package
 				clientLamportAtChange = time.MaxLamport
-			} else if versionVector != nil {
+			} else if !isVersionVectorEmpty {
+				// Case 2: from operation with version vector(After v0.5.7)
 				lamport, ok := versionVector.Get(actorID)
 				if ok {
 					clientLamportAtChange = lamport
 				} else {
 					clientLamportAtChange = 0
 				}
 			} else {
+				// Case 3: from operation without version vector(Before v0.5.6)
 				createdAt, ok := maxCreatedAtMapByActor[actorIDHex]
 				if ok {
 					maxCreatedAt = createdAt
@@ -1000,6 +1006,9 @@ func (t *Tree) Style(
 		return nil, nil, err
 	}
 
+	isVersionVectorEmpty := len(versionVector) == 0
+	isMaxCreatedAtMapByActorEmpty := len(maxCreatedAtMapByActor) == 0
+
 	var pairs []GCPair
 	createdAtMapByActor := make(map[string]*time.Ticket)
 	if err = t.traverseInPosRange(fromParent, fromLeft, toParent, toLeft, func(token index.TreeToken[*TreeNode], _ bool) {
@@ -1009,17 +1018,19 @@ func (t *Tree) Style(
 
 		var maxCreatedAt *time.Ticket
 		var clientLamportAtChange int64
-		if versionVector == nil && maxCreatedAtMapByActor == nil {
-			// Local edit - use version vector comparison
+		if isVersionVectorEmpty && isMaxCreatedAtMapByActorEmpty {
+			// Case 1: local editing from json package
 			clientLamportAtChange = time.MaxLamport
-		} else if versionVector != nil {
+		} else if !isVersionVectorEmpty {
+			// Case 2: from operation with version vector(After v0.5.7)
 			lamport, ok := versionVector.Get(actorID)
 			if ok {
 				clientLamportAtChange = lamport
 			} else {
 				clientLamportAtChange = 0
 			}
 		} else {
+			// Case 3: from operation without version vector(Before v0.5.6)
 			createdAt, ok := maxCreatedAtMapByActor[actorIDHex]
 			if ok {
 				maxCreatedAt = createdAt
@@ -1069,6 +1080,9 @@ func (t *Tree) RemoveStyle(
 		return nil, nil, err
 	}
 
+	isVersionVectorEmpty := len(versionVector) == 0
+	isMaxCreatedAtMapByActorEmpty := len(maxCreatedAtMapByActor) == 0
+
 	var pairs []GCPair
 	createdAtMapByActor := make(map[string]*time.Ticket)
 	if err = t.traverseInPosRange(fromParent, fromLeft, toParent, toLeft, func(token index.TreeToken[*TreeNode], _ bool) {
@@ -1078,17 +1092,19 @@ func (t *Tree) RemoveStyle(
 
 		var maxCreatedAt *time.Ticket
 		var clientLamportAtChange int64
-		if versionVector == nil && maxCreatedAtMapByActor == nil {
-			// Local edit - use version vector comparison
+		if isVersionVectorEmpty && isMaxCreatedAtMapByActorEmpty {
+			// Case 1: local editing from json package
 			clientLamportAtChange = time.MaxLamport
-		} else if versionVector != nil {
+		} else if !isVersionVectorEmpty {
+			// Case 2: from operation with version vector(After v0.5.7)
 			lamport, ok := versionVector.Get(actorID)
 			if ok {
 				clientLamportAtChange = lamport
 			} else {
 				clientLamportAtChange = 0
 			}
 		} else {
+			// Case 3: from operation without version vector(Before v0.5.6)
 			createdAt, ok := maxCreatedAtMapByActor[actorIDHex]
 			if ok {
 				maxCreatedAt = createdAt