From d30abf344381d967665970f61b2aa00da2fb309d Mon Sep 17 00:00:00 2001 From: kokodak Date: Thu, 22 Aug 2024 00:30:49 +0900 Subject: [PATCH 1/4] Optimize FindChangeInfosBetweenServerSeqs method --- server/backend/database/mongo/client.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/backend/database/mongo/client.go b/server/backend/database/mongo/client.go index 3d65ff029..5fd67891a 100644 --- a/server/backend/database/mongo/client.go +++ b/server/backend/database/mongo/client.go @@ -1030,6 +1030,9 @@ func (c *Client) FindChangeInfosBetweenServerSeqs( from int64, to int64, ) ([]*database.ChangeInfo, error) { + if from > to { + return nil, nil + } cursor, err := c.collection(ColChanges).Find(ctx, bson.M{ "project_id": docRefKey.ProjectID, "doc_id": docRefKey.DocID, From ab3888c43d5692a974063c7c30966919a865b396 Mon Sep 17 00:00:00 2001 From: kokodak Date: Fri, 23 Aug 2024 18:23:59 +0900 Subject: [PATCH 2/4] Add test code --- server/backend/database/memory/database.go | 3 ++ .../backend/database/memory/database_test.go | 4 ++ server/backend/database/mongo/client_test.go | 4 ++ .../backend/database/testcases/testcases.go | 37 +++++++++++++++++++ test/sharding/mongo_client_test.go | 4 ++ 5 files changed, 52 insertions(+) diff --git a/server/backend/database/memory/database.go b/server/backend/database/memory/database.go index 08b2b1364..c550af255 100644 --- a/server/backend/database/memory/database.go +++ b/server/backend/database/memory/database.go @@ -1051,6 +1051,9 @@ func (d *DB) FindChangeInfosBetweenServerSeqs( txn := d.db.Txn(false) defer txn.Abort() + if from > to { + return nil, nil + } var infos []*database.ChangeInfo iterator, err := txn.LowerBound( diff --git a/server/backend/database/memory/database_test.go b/server/backend/database/memory/database_test.go index 5ee6ad0a1..d308940a7 100644 --- a/server/backend/database/memory/database_test.go +++ b/server/backend/database/memory/database_test.go @@ -60,6 +60,10 @@ func TestDB(t *testing.T) { testcases.RunFindChangesBetweenServerSeqsTest(t, db, projectID) }) + t.Run("RunFindChangeInfosBetweenServerSeqsTest test", func(t *testing.T) { + testcases.RunFindChangeInfosBetweenServerSeqsTest(t, db, projectID) + }) + t.Run("RunFindClosestSnapshotInfo test", func(t *testing.T) { testcases.RunFindClosestSnapshotInfoTest(t, db, projectID) }) diff --git a/server/backend/database/mongo/client_test.go b/server/backend/database/mongo/client_test.go index 03e596840..6e0803b5d 100644 --- a/server/backend/database/mongo/client_test.go +++ b/server/backend/database/mongo/client_test.go @@ -76,6 +76,10 @@ func TestClient(t *testing.T) { testcases.RunFindChangesBetweenServerSeqsTest(t, cli, dummyProjectID) }) + t.Run("RunFindChangeInfosBetweenServerSeqsTest test", func(t *testing.T) { + testcases.RunFindChangeInfosBetweenServerSeqsTest(t, cli, dummyProjectID) + }) + t.Run("RunFindClosestSnapshotInfo test", func(t *testing.T) { testcases.RunFindClosestSnapshotInfoTest(t, cli, dummyProjectID) }) diff --git a/server/backend/database/testcases/testcases.go b/server/backend/database/testcases/testcases.go index 06a54bd20..1775b829f 100644 --- a/server/backend/database/testcases/testcases.go +++ b/server/backend/database/testcases/testcases.go @@ -343,6 +343,43 @@ func RunFindChangesBetweenServerSeqsTest( }) } +// RunFindChangeInfosBetweenServerSeqsTest runs the FindChangeInfosBetweenServerSeqs test for the given db. +func RunFindChangeInfosBetweenServerSeqsTest( + t *testing.T, + db database.Database, + projectID types.ID, +) { + t.Run("continues editing without any interference from other users test", func(t *testing.T) { + ctx := context.Background() + + docKey := key.Key(fmt.Sprintf("tests$%s", t.Name())) + + clientInfo, _ := db.ActivateClient(ctx, projectID, t.Name()) + docInfo, _ := db.FindDocInfoByKeyAndOwner(ctx, clientInfo.RefKey(), docKey, true) + assert.NoError(t, clientInfo.AttachDocument(docInfo.ID, false)) + assert.NoError(t, db.UpdateClientInfoAfterPushPull(ctx, clientInfo, docInfo)) + + updatedClientInfo, err := db.FindClientInfoByRefKey(ctx, clientInfo.RefKey()) + + // Record the serverSeq value at the time the PushPull request came in. + initialServerSeq := docInfo.ServerSeq + + // The serverSeq of the checkpoint that the server has should always be the same as + // the serverSeq of the user's checkpoint that came in as a request, if no other user interfered. + reqPackCheckpointServerSeq := updatedClientInfo.Checkpoint(docInfo.ID).ServerSeq + + changeInfos, err := db.FindChangeInfosBetweenServerSeqs( + ctx, + docInfo.RefKey(), + reqPackCheckpointServerSeq+1, + initialServerSeq, + ) + + assert.NoError(t, err) + assert.Len(t, changeInfos, 0) + }) +} + // RunFindClosestSnapshotInfoTest runs the FindClosestSnapshotInfo test for the given db. func RunFindClosestSnapshotInfoTest(t *testing.T, db database.Database, projectID types.ID) { t.Run("store and find snapshots test", func(t *testing.T) { diff --git a/test/sharding/mongo_client_test.go b/test/sharding/mongo_client_test.go index 0653e0200..5cb2bffe6 100644 --- a/test/sharding/mongo_client_test.go +++ b/test/sharding/mongo_client_test.go @@ -85,6 +85,10 @@ func TestClientWithShardedDB(t *testing.T) { testcases.RunFindChangesBetweenServerSeqsTest(t, cli, dummyProjectID) }) + t.Run("RunFindChangeInfosBetweenServerSeqsTest test", func(t *testing.T) { + testcases.RunFindChangeInfosBetweenServerSeqsTest(t, cli, dummyProjectID) + }) + t.Run("RunFindClosestSnapshotInfo test", func(t *testing.T) { testcases.RunFindClosestSnapshotInfoTest(t, cli, dummyProjectID) }) From 6fe24d6ba4a346f907c41424574fef657c44d192 Mon Sep 17 00:00:00 2001 From: kokodak Date: Fri, 23 Aug 2024 18:30:20 +0900 Subject: [PATCH 3/4] Resolve lint --- server/backend/database/testcases/testcases.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/backend/database/testcases/testcases.go b/server/backend/database/testcases/testcases.go index 1775b829f..6ecf09abd 100644 --- a/server/backend/database/testcases/testcases.go +++ b/server/backend/database/testcases/testcases.go @@ -359,7 +359,7 @@ func RunFindChangeInfosBetweenServerSeqsTest( assert.NoError(t, clientInfo.AttachDocument(docInfo.ID, false)) assert.NoError(t, db.UpdateClientInfoAfterPushPull(ctx, clientInfo, docInfo)) - updatedClientInfo, err := db.FindClientInfoByRefKey(ctx, clientInfo.RefKey()) + updatedClientInfo, _ := db.FindClientInfoByRefKey(ctx, clientInfo.RefKey()) // Record the serverSeq value at the time the PushPull request came in. initialServerSeq := docInfo.ServerSeq From 1f7dbd355bf9c996c70236038ab9d6cf3ca33eab Mon Sep 17 00:00:00 2001 From: kokodak Date: Mon, 26 Aug 2024 15:14:39 +0900 Subject: [PATCH 4/4] Add test case for `from > to` and `from <= to` condition --- .../backend/database/testcases/testcases.go | 142 ++++++++++++++++++ 1 file changed, 142 insertions(+) diff --git a/server/backend/database/testcases/testcases.go b/server/backend/database/testcases/testcases.go index 6ecf09abd..ef626724d 100644 --- a/server/backend/database/testcases/testcases.go +++ b/server/backend/database/testcases/testcases.go @@ -378,6 +378,148 @@ func RunFindChangeInfosBetweenServerSeqsTest( assert.NoError(t, err) assert.Len(t, changeInfos, 0) }) + + t.Run("retrieving a document with snapshot that reflect the latest doc info test", func(t *testing.T) { + ctx := context.Background() + + docKey := key.Key(fmt.Sprintf("tests$%s", t.Name())) + + clientInfo, _ := db.ActivateClient(ctx, projectID, t.Name()) + docInfo, _ := db.FindDocInfoByKeyAndOwner(ctx, clientInfo.RefKey(), docKey, true) + docRefKey := docInfo.RefKey() + assert.NoError(t, clientInfo.AttachDocument(docInfo.ID, false)) + assert.NoError(t, db.UpdateClientInfoAfterPushPull(ctx, clientInfo, docInfo)) + + initialServerSeq := docInfo.ServerSeq + + // 01. Create a document and store changes + bytesID, _ := clientInfo.ID.Bytes() + actorID, _ := time.ActorIDFromBytes(bytesID) + doc := document.New(key.Key(t.Name())) + doc.SetActor(actorID) + assert.NoError(t, doc.Update(func(root *json.Object, p *presence.Presence) error { + root.SetNewArray("array") + return nil + })) + for idx := 0; idx < 5; idx++ { + assert.NoError(t, doc.Update(func(root *json.Object, p *presence.Presence) error { + root.GetArray("array").AddInteger(idx) + return nil + })) + } + + pack := doc.CreateChangePack() + for _, c := range pack.Changes { + serverSeq := docInfo.IncreaseServerSeq() + c.SetServerSeq(serverSeq) + } + + err := db.CreateChangeInfos( + ctx, + projectID, + docInfo, + initialServerSeq, + pack.Changes, + false, + ) + assert.NoError(t, err) + + // 02. Create a snapshot that reflect the latest doc info + updatedDocInfo, _ := db.FindDocInfoByRefKey(ctx, docRefKey) + assert.Equal(t, int64(6), updatedDocInfo.ServerSeq) + + pack = change.NewPack( + updatedDocInfo.Key, + change.InitialCheckpoint.NextServerSeq(updatedDocInfo.ServerSeq), + nil, + nil, + ) + assert.NoError(t, doc.ApplyChangePack(pack)) + assert.Equal(t, int64(6), doc.Checkpoint().ServerSeq) + + assert.NoError(t, db.CreateSnapshotInfo(ctx, docRefKey, doc.InternalDocument())) + + // 03. Find changeInfos with snapshot that reflect the latest doc info + snapshotInfo, _ := db.FindClosestSnapshotInfo( + ctx, + docRefKey, + updatedDocInfo.ServerSeq, + false, + ) + + changeInfos, _ := db.FindChangeInfosBetweenServerSeqs( + ctx, + docRefKey, + snapshotInfo.ServerSeq+1, + updatedDocInfo.ServerSeq, + ) + + assert.Len(t, changeInfos, 0) + }) + + t.Run("store changes and find changes test", func(t *testing.T) { + ctx := context.Background() + + docKey := key.Key(fmt.Sprintf("tests$%s", t.Name())) + + clientInfo, _ := db.ActivateClient(ctx, projectID, t.Name()) + docInfo, _ := db.FindDocInfoByKeyAndOwner(ctx, clientInfo.RefKey(), docKey, true) + docRefKey := docInfo.RefKey() + assert.NoError(t, clientInfo.AttachDocument(docInfo.ID, false)) + assert.NoError(t, db.UpdateClientInfoAfterPushPull(ctx, clientInfo, docInfo)) + + initialServerSeq := docInfo.ServerSeq + + // 01. Create a document and store changes + bytesID, _ := clientInfo.ID.Bytes() + actorID, _ := time.ActorIDFromBytes(bytesID) + doc := document.New(key.Key(t.Name())) + doc.SetActor(actorID) + assert.NoError(t, doc.Update(func(root *json.Object, p *presence.Presence) error { + root.SetNewArray("array") + return nil + })) + for idx := 0; idx < 5; idx++ { + assert.NoError(t, doc.Update(func(root *json.Object, p *presence.Presence) error { + root.GetArray("array").AddInteger(idx) + return nil + })) + } + pack := doc.CreateChangePack() + for _, c := range pack.Changes { + serverSeq := docInfo.IncreaseServerSeq() + c.SetServerSeq(serverSeq) + } + + err := db.CreateChangeInfos( + ctx, + projectID, + docInfo, + initialServerSeq, + pack.Changes, + false, + ) + assert.NoError(t, err) + + // 02. Find changes + changeInfos, err := db.FindChangeInfosBetweenServerSeqs( + ctx, + docRefKey, + 1, + 6, + ) + assert.NoError(t, err) + assert.Len(t, changeInfos, 6) + + changeInfos, err = db.FindChangeInfosBetweenServerSeqs( + ctx, + docRefKey, + 3, + 3, + ) + assert.NoError(t, err) + assert.Len(t, changeInfos, 1) + }) } // RunFindClosestSnapshotInfoTest runs the FindClosestSnapshotInfo test for the given db.