mirror of
https://github.com/dnote/dnote
synced 2026-03-14 22:45:50 +01:00
Fix in a more correct way
This commit is contained in:
Sung
parent
fb22e8556e
commit
9323b1e205
2 changed files with 150 additions and 48 deletions
|
|
@ -90,6 +90,7 @@ type syncList struct {
|
|||
ExpungedNotes map[string]bool
|
||||
ExpungedBooks map[string]bool
|
||||
MaxUSN int
|
||||
UserMaxUSN int // Server's actual max USN (for distinguishing empty fragment vs empty server)
|
||||
MaxCurrentTime int64
|
||||
}
|
||||
|
||||
|
|
@ -104,6 +105,7 @@ func processFragments(fragments []client.SyncFragment) (syncList, error) {
|
|||
expungedNotes := map[string]bool{}
|
||||
expungedBooks := map[string]bool{}
|
||||
var maxUSN int
|
||||
var userMaxUSN int
|
||||
var maxCurrentTime int64
|
||||
|
||||
for _, fragment := range fragments {
|
||||
|
|
@ -120,12 +122,11 @@ func processFragments(fragments []client.SyncFragment) (syncList, error) {
|
|||
expungedNotes[uuid] = true
|
||||
}
|
||||
|
||||
// Use UserMaxUSN instead of FragMaxUSN to track the server's actual max USN.
|
||||
// FragMaxUSN is the max USN in THIS fragment (can be 0 if empty),
|
||||
// while UserMaxUSN is the server's current max USN (always accurate).
|
||||
// This prevents last_max_usn from being reset to 0 when we get empty fragments.
|
||||
if fragment.UserMaxUSN > maxUSN {
|
||||
maxUSN = fragment.UserMaxUSN
|
||||
if fragment.FragMaxUSN > maxUSN {
|
||||
maxUSN = fragment.FragMaxUSN
|
||||
}
|
||||
if fragment.UserMaxUSN > userMaxUSN {
|
||||
userMaxUSN = fragment.UserMaxUSN
|
||||
}
|
||||
if fragment.CurrentTime > maxCurrentTime {
|
||||
maxCurrentTime = fragment.CurrentTime
|
||||
|
|
@ -138,6 +139,7 @@ func processFragments(fragments []client.SyncFragment) (syncList, error) {
|
|||
ExpungedNotes: expungedNotes,
|
||||
ExpungedBooks: expungedBooks,
|
||||
MaxUSN: maxUSN,
|
||||
UserMaxUSN: userMaxUSN,
|
||||
MaxCurrentTime: maxCurrentTime,
|
||||
}
|
||||
|
||||
|
|
@ -581,7 +583,7 @@ func fullSync(ctx context.DnoteCtx, tx *database.DB) error {
|
|||
}
|
||||
}
|
||||
|
||||
err = saveSyncState(tx, list.MaxCurrentTime, list.MaxUSN)
|
||||
err = saveSyncState(tx, list.MaxCurrentTime, list.MaxUSN, list.UserMaxUSN)
|
||||
if err != nil {
|
||||
return errors.Wrap(err, "saving sync state")
|
||||
}
|
||||
|
|
@ -625,7 +627,7 @@ func stepSync(ctx context.DnoteCtx, tx *database.DB, afterUSN int) error {
|
|||
}
|
||||
}
|
||||
|
||||
err = saveSyncState(tx, list.MaxCurrentTime, list.MaxUSN)
|
||||
err = saveSyncState(tx, list.MaxCurrentTime, list.MaxUSN, list.UserMaxUSN)
|
||||
if err != nil {
|
||||
return errors.Wrap(err, "saving sync state")
|
||||
}
|
||||
|
|
@ -681,13 +683,9 @@ func sendBooks(ctx context.DnoteCtx, tx *database.DB) (bool, error) {
|
|||
} else {
|
||||
resp, err := client.CreateBook(ctx, book.Label)
|
||||
if err != nil {
|
||||
// If we get a 409 conflict, it means another client uploaded data.
|
||||
if isConflictError(err) {
|
||||
log.Debug("409 conflict creating book %s, will retry after sync\n", book.Label)
|
||||
isBehind = true
|
||||
continue
|
||||
}
|
||||
return isBehind, errors.Wrap(err, "creating a book")
|
||||
log.Debug("error creating book (will retry after stepSync): %v\n", err)
|
||||
isBehind = true
|
||||
continue
|
||||
}
|
||||
|
||||
_, err = tx.Exec("UPDATE notes SET book_uuid = ? WHERE book_uuid = ?", resp.Book.UUID, book.UUID)
|
||||
|
|
@ -792,8 +790,7 @@ func sendNotes(ctx context.DnoteCtx, tx *database.DB) (bool, error) {
|
|||
} else {
|
||||
resp, err := client.CreateNote(ctx, note.BookUUID, note.Body)
|
||||
if err != nil {
|
||||
// If we get a 409 conflict, it means another client uploaded data.
|
||||
log.Debug("error creating note (will retry after sync): %v\n", err)
|
||||
log.Debug("error creating note (will retry after stepSync): %v\n", err)
|
||||
isBehind = true
|
||||
continue
|
||||
}
|
||||
|
|
@ -903,10 +900,24 @@ func updateLastSyncAt(tx *database.DB, val int64) error {
|
|||
return nil
|
||||
}
|
||||
|
||||
func saveSyncState(tx *database.DB, serverTime int64, serverMaxUSN int) error {
|
||||
if err := updateLastMaxUSN(tx, serverMaxUSN); err != nil {
|
||||
return errors.Wrap(err, "updating last max usn")
|
||||
func saveSyncState(tx *database.DB, serverTime int64, serverMaxUSN int, userMaxUSN int) error {
|
||||
// Handle last_max_usn update based on server state:
|
||||
// - If serverMaxUSN > 0: we got data, update to serverMaxUSN
|
||||
// - If serverMaxUSN == 0 && userMaxUSN > 0: empty fragment (caught up), preserve existing
|
||||
// - If serverMaxUSN == 0 && userMaxUSN == 0: empty server, reset to 0
|
||||
if serverMaxUSN > 0 {
|
||||
if err := updateLastMaxUSN(tx, serverMaxUSN); err != nil {
|
||||
return errors.Wrap(err, "updating last max usn")
|
||||
}
|
||||
} else if userMaxUSN == 0 {
|
||||
// Server is empty, reset to 0
|
||||
if err := updateLastMaxUSN(tx, 0); err != nil {
|
||||
return errors.Wrap(err, "updating last max usn")
|
||||
}
|
||||
}
|
||||
// else: empty fragment but server has data, preserve existing last_max_usn
|
||||
|
||||
// Always update last_sync_at (we did communicate with server)
|
||||
if err := updateLastSyncAt(tx, serverTime); err != nil {
|
||||
return errors.Wrap(err, "updating last sync at")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -36,7 +36,6 @@ import (
|
|||
"github.com/pkg/errors"
|
||||
)
|
||||
|
||||
|
||||
func TestProcessFragments(t *testing.T) {
|
||||
fragments := []client.SyncFragment{
|
||||
{
|
||||
|
|
@ -98,6 +97,7 @@ func TestProcessFragments(t *testing.T) {
|
|||
ExpungedNotes: map[string]bool{},
|
||||
ExpungedBooks: map[string]bool{},
|
||||
MaxUSN: 10,
|
||||
UserMaxUSN: 10,
|
||||
MaxCurrentTime: 1550436136,
|
||||
}
|
||||
|
||||
|
|
@ -1796,41 +1796,132 @@ func TestMergeBook(t *testing.T) {
|
|||
}
|
||||
|
||||
func TestSaveServerState(t *testing.T) {
|
||||
// set up
|
||||
db := database.InitTestMemoryDB(t)
|
||||
testutils.LoginDB(t, db)
|
||||
t.Run("with data received", func(t *testing.T) {
|
||||
// set up
|
||||
db := database.InitTestMemoryDB(t)
|
||||
testutils.LoginDB(t, db)
|
||||
|
||||
database.MustExec(t, "inserting last synced at", db, "INSERT INTO system (key, value) VALUES (?, ?)", consts.SystemLastSyncAt, int64(1231108742))
|
||||
database.MustExec(t, "inserting last max usn", db, "INSERT INTO system (key, value) VALUES (?, ?)", consts.SystemLastMaxUSN, 8)
|
||||
database.MustExec(t, "inserting last synced at", db, "INSERT INTO system (key, value) VALUES (?, ?)", consts.SystemLastSyncAt, int64(1231108742))
|
||||
database.MustExec(t, "inserting last max usn", db, "INSERT INTO system (key, value) VALUES (?, ?)", consts.SystemLastMaxUSN, 8)
|
||||
|
||||
// execute
|
||||
tx, err := db.Begin()
|
||||
if err != nil {
|
||||
t.Fatal(errors.Wrap(err, "beginning a transaction").Error())
|
||||
}
|
||||
// execute
|
||||
tx, err := db.Begin()
|
||||
if err != nil {
|
||||
t.Fatal(errors.Wrap(err, "beginning a transaction").Error())
|
||||
}
|
||||
|
||||
serverTime := int64(1541108743)
|
||||
serverMaxUSN := 100
|
||||
serverTime := int64(1541108743)
|
||||
serverMaxUSN := 100
|
||||
userMaxUSN := 100
|
||||
|
||||
err = saveSyncState(tx, serverTime, serverMaxUSN)
|
||||
if err != nil {
|
||||
tx.Rollback()
|
||||
t.Fatal(errors.Wrap(err, "executing").Error())
|
||||
}
|
||||
err = saveSyncState(tx, serverTime, serverMaxUSN, userMaxUSN)
|
||||
if err != nil {
|
||||
tx.Rollback()
|
||||
t.Fatal(errors.Wrap(err, "executing").Error())
|
||||
}
|
||||
|
||||
tx.Commit()
|
||||
tx.Commit()
|
||||
|
||||
// test
|
||||
var lastSyncedAt int64
|
||||
var lastMaxUSN int
|
||||
// test
|
||||
var lastSyncedAt int64
|
||||
var lastMaxUSN int
|
||||
|
||||
database.MustScan(t, "getting system value",
|
||||
db.QueryRow("SELECT value FROM system WHERE key = ?", consts.SystemLastSyncAt), &lastSyncedAt)
|
||||
database.MustScan(t, "getting system value",
|
||||
db.QueryRow("SELECT value FROM system WHERE key = ?", consts.SystemLastMaxUSN), &lastMaxUSN)
|
||||
database.MustScan(t, "getting system value",
|
||||
db.QueryRow("SELECT value FROM system WHERE key = ?", consts.SystemLastSyncAt), &lastSyncedAt)
|
||||
database.MustScan(t, "getting system value",
|
||||
db.QueryRow("SELECT value FROM system WHERE key = ?", consts.SystemLastMaxUSN), &lastMaxUSN)
|
||||
|
||||
assert.Equal(t, lastSyncedAt, serverTime, "last synced at mismatch")
|
||||
assert.Equal(t, lastMaxUSN, serverMaxUSN, "last max usn mismatch")
|
||||
assert.Equal(t, lastSyncedAt, serverTime, "last synced at mismatch")
|
||||
assert.Equal(t, lastMaxUSN, serverMaxUSN, "last max usn mismatch")
|
||||
})
|
||||
|
||||
t.Run("with empty fragment but server has data - preserves last_max_usn", func(t *testing.T) {
|
||||
// This tests the fix for the infinite sync bug where empty fragments
|
||||
// would reset last_max_usn to 0, causing the client to re-download all data.
|
||||
// When serverMaxUSN=0 (no data in fragment) but userMaxUSN>0 (server has data),
|
||||
// we're caught up and should preserve the existing last_max_usn.
|
||||
|
||||
// set up
|
||||
db := database.InitTestMemoryDB(t)
|
||||
testutils.LoginDB(t, db)
|
||||
|
||||
existingLastMaxUSN := 100
|
||||
database.MustExec(t, "inserting last synced at", db, "INSERT INTO system (key, value) VALUES (?, ?)", consts.SystemLastSyncAt, int64(1231108742))
|
||||
database.MustExec(t, "inserting last max usn", db, "INSERT INTO system (key, value) VALUES (?, ?)", consts.SystemLastMaxUSN, existingLastMaxUSN)
|
||||
|
||||
// execute
|
||||
tx, err := db.Begin()
|
||||
if err != nil {
|
||||
t.Fatal(errors.Wrap(err, "beginning a transaction").Error())
|
||||
}
|
||||
|
||||
serverTime := int64(1541108743)
|
||||
serverMaxUSN := 0 // Empty fragment (no data in this sync)
|
||||
userMaxUSN := 150 // Server's actual max USN (higher than ours)
|
||||
|
||||
err = saveSyncState(tx, serverTime, serverMaxUSN, userMaxUSN)
|
||||
if err != nil {
|
||||
tx.Rollback()
|
||||
t.Fatal(errors.Wrap(err, "executing").Error())
|
||||
}
|
||||
|
||||
tx.Commit()
|
||||
|
||||
// test
|
||||
var lastSyncedAt int64
|
||||
var lastMaxUSN int
|
||||
|
||||
database.MustScan(t, "getting system value",
|
||||
db.QueryRow("SELECT value FROM system WHERE key = ?", consts.SystemLastSyncAt), &lastSyncedAt)
|
||||
database.MustScan(t, "getting system value",
|
||||
db.QueryRow("SELECT value FROM system WHERE key = ?", consts.SystemLastMaxUSN), &lastMaxUSN)
|
||||
|
||||
assert.Equal(t, lastSyncedAt, serverTime, "last synced at should be updated")
|
||||
// last_max_usn should NOT be updated to 0, it should preserve the existing value
|
||||
assert.Equal(t, lastMaxUSN, existingLastMaxUSN, "last max usn should be preserved when fragment is empty but server has data")
|
||||
})
|
||||
|
||||
t.Run("with empty server - resets last_max_usn to 0", func(t *testing.T) {
|
||||
// When both serverMaxUSN=0 and userMaxUSN=0, the server is truly empty
|
||||
// and we should reset last_max_usn to 0.
|
||||
|
||||
// set up
|
||||
db := database.InitTestMemoryDB(t)
|
||||
testutils.LoginDB(t, db)
|
||||
|
||||
database.MustExec(t, "inserting last synced at", db, "INSERT INTO system (key, value) VALUES (?, ?)", consts.SystemLastSyncAt, int64(1231108742))
|
||||
database.MustExec(t, "inserting last max usn", db, "INSERT INTO system (key, value) VALUES (?, ?)", consts.SystemLastMaxUSN, 50)
|
||||
|
||||
// execute
|
||||
tx, err := db.Begin()
|
||||
if err != nil {
|
||||
t.Fatal(errors.Wrap(err, "beginning a transaction").Error())
|
||||
}
|
||||
|
||||
serverTime := int64(1541108743)
|
||||
serverMaxUSN := 0 // Empty fragment
|
||||
userMaxUSN := 0 // Server is actually empty
|
||||
|
||||
err = saveSyncState(tx, serverTime, serverMaxUSN, userMaxUSN)
|
||||
if err != nil {
|
||||
tx.Rollback()
|
||||
t.Fatal(errors.Wrap(err, "executing").Error())
|
||||
}
|
||||
|
||||
tx.Commit()
|
||||
|
||||
// test
|
||||
var lastSyncedAt int64
|
||||
var lastMaxUSN int
|
||||
|
||||
database.MustScan(t, "getting system value",
|
||||
db.QueryRow("SELECT value FROM system WHERE key = ?", consts.SystemLastSyncAt), &lastSyncedAt)
|
||||
database.MustScan(t, "getting system value",
|
||||
db.QueryRow("SELECT value FROM system WHERE key = ?", consts.SystemLastMaxUSN), &lastMaxUSN)
|
||||
|
||||
assert.Equal(t, lastSyncedAt, serverTime, "last synced at should be updated")
|
||||
assert.Equal(t, lastMaxUSN, 0, "last max usn should be reset to 0 when server is empty")
|
||||
})
|
||||
}
|
||||
|
||||
// TestSendBooks tests that books are put to correct 'buckets' by running a test server and recording the
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue