Add test for node deletion after client detach

Add test case verifying behavior when client A attempts to delete nodes created by client B after client B is detached. This test is added to discuss on how to handle this scenario.
yorkie-team · Dec 5, 2024 · 98f8d3b · 98f8d3b
1 parent 05d433a
commit 98f8d3b
Show file tree

Hide file tree

Showing 4 changed files with 152 additions and 6 deletions.
diff --git a/pkg/document/change/change.go b/pkg/document/change/change.go
@@ -52,10 +52,16 @@ func New(id ID, message string, operations []operations.Operation, p *innerprese
 }
 
 // Execute applies this change to the given JSON root.
-func (c *Change) Execute(root *crdt.Root, presences *innerpresence.Map) error {
+func (c *Change) Execute(root *crdt.Root, presences *innerpresence.Map, opSource string) error {
 	for _, op := range c.operations {
-		if err := op.Execute(root, operations.WithVersionVector(c.ID().versionVector)); err != nil {
-			return err
+		if opSource == "remote" {
+			if err := op.Execute(root, operations.WithVersionVector(c.ID().versionVector)); err != nil {
+				return err
+			}
+		} else {
+			if err := op.Execute(root); err != nil {
+				return err
+			}
 		}
 	}
 

diff --git a/pkg/document/document.go b/pkg/document/document.go
@@ -170,7 +170,7 @@ func (d *Document) Update(
 
 	if ctx.HasChange() {
 		c := ctx.ToChange()
-		if err := c.Execute(d.doc.root, d.doc.presences); err != nil {
+		if err := c.Execute(d.doc.root, d.doc.presences, "local"); err != nil {
 			return err
 		}
 
@@ -245,7 +245,7 @@ func (d *Document) applyChanges(changes []*change.Change) error {
 	}
 
 	for _, c := range changes {
-		if err := c.Execute(d.cloneRoot, d.clonePresences); err != nil {
+		if err := c.Execute(d.cloneRoot, d.clonePresences, "remote"); err != nil {
 			return err
 		}
 	}

diff --git a/pkg/document/internal_document.go b/pkg/document/internal_document.go
@@ -326,7 +326,7 @@ func (d *InternalDocument) ApplyChanges(changes ...*change.Change) ([]DocEvent,
 			}
 		}
 
-		if err := c.Execute(d.root, d.presences); err != nil {
+		if err := c.Execute(d.root, d.presences, "remote"); err != nil {
 			return nil, err
 		}
 

diff --git a/test/integration/gc_test.go b/test/integration/gc_test.go
@@ -1192,4 +1192,144 @@ func TestGarbageCollection(t *testing.T) {
 		assert.Equal(t, 0, d2.GarbageLen())
 		assert.Equal(t, 1, len(d2.VersionVector()))
 	})
+
+	t.Run("attach > pushpull > detach lifecycle version vector test (run gc at last client detaches document)", func(t *testing.T) {
+		ctx := context.Background()
+		d1 := document.New(helper.TestDocKey(t))
+		assert.NoError(t, c1.Attach(ctx, d1))
+		// d2.vv =[c1:1], minvv =[c1:1], db.vv {c1: [c1:1]}
+		assert.Equal(t, true, checkVV(d1.VersionVector(), versionOf(d1.ActorID(), 1)))
+
+		d2 := document.New(helper.TestDocKey(t))
+		assert.NoError(t, c2.Attach(ctx, d2))
+		// d2.vv =[c1:1, c2:2], minvv =[c1:0, c2:0], db.vv {c1: [c1:1], c2: [c2:1]}
+		assert.Equal(t, true, checkVV(d2.VersionVector(), versionOf(d1.ActorID(), 1), versionOf(d2.ActorID(), 2)))
+
+		err := d1.Update(func(root *json.Object, p *presence.Presence) error {
+			root.SetNewText("text").Edit(0, 0, "a").Edit(1, 1, "b").Edit(2, 2, "c")
+			return nil
+		}, "sets text")
+		// d1/vv = [c1:2]
+		assert.Equal(t, true, checkVV(d1.VersionVector(), versionOf(d1.ActorID(), 2)))
+		assert.NoError(t, err)
+
+		assert.NoError(t, c1.Sync(ctx))
+		// d1.vv = [c1:3, c2:1], minvv = [c1:0, c2:0], db.vv {c1: [c1:2], c2: [c2:1]}
+		assert.Equal(t, true, checkVV(d1.VersionVector(), versionOf(d1.ActorID(), 3), versionOf(d2.ActorID(), 1)))
+
+		assert.NoError(t, c2.Sync(ctx))
+		// d2.vv = [c1:2, c2:3], minvv = [c1:1, c2:0], db.vv {c1: [c1:2], c2: [c1:1, c2:2]}
+		assert.Equal(t, true, checkVV(d2.VersionVector(), versionOf(d1.ActorID(), 2), versionOf(d2.ActorID(), 3)))
+
+		err = d2.Update(func(root *json.Object, p *presence.Presence) error {
+			root.GetText("text").Edit(2, 2, "c")
+			return nil
+		}, "insert c")
+		//d2.vv = [c1:2, c2:4]
+		assert.Equal(t, true, checkVV(d2.VersionVector(), versionOf(d1.ActorID(), 2), versionOf(d2.ActorID(), 4)))
+		assert.NoError(t, err)
+
+		err = d1.Update(func(root *json.Object, p *presence.Presence) error {
+			root.GetText("text").Edit(1, 3, "")
+			return nil
+		}, "delete bc")
+		//d1.vv = [c1:4, c2:1]
+		assert.Equal(t, true, checkVV(d1.VersionVector(), versionOf(d1.ActorID(), 4), versionOf(d2.ActorID(), 1)))
+		assert.NoError(t, err)
+		assert.Equal(t, 2, d1.GarbageLen())
+		assert.Equal(t, 0, d2.GarbageLen())
+
+		assert.NoError(t, c1.Sync(ctx))
+		assert.Equal(t, true, checkVV(d1.VersionVector(), versionOf(d1.ActorID(), 4), versionOf(d2.ActorID(), 1)))
+
+		assert.NoError(t, c2.Sync(ctx))
+		assert.Equal(t, true, checkVV(d2.VersionVector(), versionOf(d1.ActorID(), 4), versionOf(d2.ActorID(), 5)))
+
+		assert.Equal(t, 2, d1.GarbageLen())
+		assert.Equal(t, 2, d2.GarbageLen())
+
+		err = d2.Update(func(root *json.Object, p *presence.Presence) error {
+			root.GetText("text").Edit(2, 2, "1")
+			return nil
+		}, "insert c")
+		assert.Equal(t, true, checkVV(d2.VersionVector(), versionOf(d1.ActorID(), 4), versionOf(d2.ActorID(), 6)))
+		assert.NoError(t, err)
+
+		assert.NoError(t, c2.Sync(ctx))
+		assert.Equal(t, true, checkVV(d2.VersionVector(), versionOf(d1.ActorID(), 4), versionOf(d2.ActorID(), 6)))
+
+		assert.Equal(t, 2, d1.GarbageLen())
+		assert.Equal(t, 0, d2.GarbageLen())
+
+		assert.NoError(t, c1.Sync(ctx))
+		assert.Equal(t, true, checkVV(d1.VersionVector(), versionOf(d1.ActorID(), 7), versionOf(d2.ActorID(), 6)))
+
+		assert.NoError(t, c1.Detach(ctx, d1))
+
+		assert.NoError(t, c2.Sync(ctx))
+		assert.Equal(t, true, checkVV(d2.VersionVector(), versionOf(d2.ActorID(), 9)))
+		assert.Equal(t, `{"text":[{"val":"a"},{"val":"c"},{"val":"1"}]}`, d2.Marshal())
+
+		err = d2.Update(func(root *json.Object, p *presence.Presence) error {
+			root.GetText("text").Edit(0, 3, "")
+			return nil
+		}, "delete all")
+		assert.NoError(t, err)
+		assert.Equal(t, true, checkVV(d2.VersionVector(), versionOf(d2.ActorID(), 10)))
+		assert.Equal(t, `{"text":[]}`, d2.Marshal())
+
+		assert.Equal(t, 3, d2.GarbageLen())
+		assert.NoError(t, c2.Detach(ctx, d2))
+		assert.Equal(t, 0, d2.GarbageLen())
+	})
+
+	t.Run("detached client node deletion test", func(t *testing.T) {
+		t.Skip("remove this after implementing node deletion")
+		clients := activeClients(t, 3)
+		c1, c2, c3 := clients[0], clients[1], clients[2]
+		defer deactivateAndCloseClients(t, clients)
+		ctx := context.Background()
+		d1 := document.New(helper.TestDocKey(t))
+		d2 := document.New(helper.TestDocKey(t))
+		d3 := document.New(helper.TestDocKey(t))
+
+		assert.NoError(t, c1.Attach(ctx, d1))
+		assert.NoError(t, c2.Attach(ctx, d2))
+		assert.NoError(t, c3.Attach(ctx, d3))
+
+		err := d1.Update(func(root *json.Object, p *presence.Presence) error {
+			root.SetNewText("text").Edit(0, 0, "a") // a
+			return nil
+		}, "insert abc")
+		assert.NoError(t, err)
+
+		assert.NoError(t, c1.Sync(ctx))
+		assert.NoError(t, c2.Sync(ctx))
+		assert.NoError(t, c3.Sync(ctx))
+
+		err = d3.Update(func(root *json.Object, p *presence.Presence) error {
+			root.GetText("text").Edit(0, 0, "1") // 1a
+			return nil
+		})
+		assert.NoError(t, err)
+
+		assert.NoError(t, c3.Sync(ctx))
+		assert.NoError(t, c1.Sync(ctx))
+		assert.NoError(t, c2.Sync(ctx))
+
+		assert.NoError(t, c3.Detach(ctx, d3))
+		assert.NoError(t, c1.Sync(ctx))
+		assert.NoError(t, c2.Sync(ctx))
+
+		err = d2.Update(func(root *json.Object, p *presence.Presence) error {
+			root.GetText("text").Edit(0, 1, "x") // xa
+			return nil
+		}, "delete 123 and insert x")
+		assert.NoError(t, err)
+
+		assert.NoError(t, c2.Sync(ctx))
+		assert.NoError(t, c1.Sync(ctx))
+		assert.Equal(t, `{"text":[{"val":"x"},{"val":"a"}]}`, d2.Marshal())
+		assert.Equal(t, `{"text":[{"val":"x"},{"val":"a"}]}`, d1.Marshal()) // "x1a": cannot delete 1
+	})
 }
-Original file line number
+Diff line change
@@ Expand Up @@
     			}
     		}
-    		if err := c.Execute(d.root, d.presences); err != nil {
+    		if err := c.Execute(d.root, d.presences, "remote"); err != nil {
     			return nil, err
     		}
@@ Expand Down @@