fix(koreader): stop a rebuilt library silently dropping reading history

Deleting a library cascades away its books, files, page stats and reading sessions, and a rescan
mints new rows. The device's sync state is keyed by the file's partial md5, so an unchanged file
kept its cursors across that swap: the plugin asked only for events newer than a watermark earned
against rows the server no longer had, and nothing recorded before the swap was ever resent.
"Recheck all book matches" did not help, because it funnels back through setMatched.

Treat a changed book file id or book id as a fresh sync target and keep only the local file path.
Both ids matter: statistics hang off the file row while highlights, status, rating and review hang
off the book row, and an existing-folder merge re-parents a file to another book without changing
its id. Resending costs nothing because the page-stat dedup key makes a replay a server-side no-op.

Order both hash resolvers by book file id so content that exists as two files always resolves to
the same row. They previously took whichever row Postgres returned first, so they could disagree
with each other and, with the reset above, move a device's sync target under it.

Closes #1049
This commit is contained in:
neon
2026-08-21 16:38:02 +00:00
parent 0250a61858
commit 1e5fcedb2a
6 changed files with 252 additions and 14 deletions
@@ -82,13 +82,20 @@ function BookOrbitState:matchedFileMaps()
end
-- Every match-producing path funnels through here (lifecycle drain, open-book
-- match, sweep and the bulk manifest link), so this is the only place that can
-- stamp match freshness without leaving some path silently unfresh. The token
-- the match was verified against is stored alongside the time: without it a
-- single library change would force a request on every later book sync until
-- the user ran a full sweep.
-- match, sweep and the bulk manifest link). A different server book or file is
-- a fresh sync target even when the local digest is unchanged, so only its
-- local path may survive that transition. Both ids are checked because the
-- cursors below are split across them: reading statistics hang off the file
-- row, while highlights, status, rating and review hang off the book row, and a
-- folder merge re-parents a file to another book without changing its id. The
-- token the match was verified against is stored alongside the time so one
-- library change does not force a request on every later book sync until the
-- user runs a full sweep.
function BookOrbitState:setMatched(md5, book_file_id, book_id, file, verified_version)
local book = self.books[md5] or {}
local previous = self.books[md5]
local identity_changed = previous ~= nil
and (previous.fileId ~= book_file_id or previous.bookId ~= book_id)
local book = identity_changed and { file = file or previous.file } or (previous or {})
book.fileId = book_file_id
book.bookId = book_id
book.file = file or book.file
@@ -50,6 +50,120 @@ assert(BookOrbitState.isMatchFresh(book, state.global, now + MAX_AGE + 1) == fal
assert(BookOrbitState.isMatchFresh(book, state.global, now - 60) == false,
"a stamp in the device's future is treated as expired")
-- A routine rematch to the same server book and file must preserve incremental
-- state; a rematch is the common case and re-uploading on every one would undo
-- the whole point of the cursors.
local retained = newState({ libraryVersion = "lib-v1" })
retained.books["same"] = {
bookId = 2,
fileId = 11,
file = "/books/same.epub",
statsWatermark = 900,
annWatermark = "2026-01-01 10:00:00",
annCount = 3,
sidecarMtime = 100,
annSignature = "ann-signature",
annExchangedAt = 200,
bmSignature = "bookmark-signature",
bmExchangedAt = 300,
progressPushedPct = 0.5,
ratingSyncedKnown = true,
}
retained:setMatched("same", 11, 2, "/books/same.epub", "lib-v1")
local retained_book = retained:getBook("same")
assert(retained_book.fileId == 11 and retained_book.bookId == 2, "an unchanged rematch keeps the server ids")
assert(retained_book.statsWatermark == 900, "an unchanged rematch preserves the statistics watermark")
assert(retained_book.annWatermark == "2026-01-01 10:00:00", "an unchanged rematch preserves the annotation watermark")
assert(retained_book.annCount == 3, "an unchanged rematch preserves the annotation count")
assert(retained_book.sidecarMtime == 100, "an unchanged rematch preserves the sidecar marker")
assert(retained_book.annSignature == "ann-signature", "an unchanged rematch preserves the annotation signature")
assert(retained_book.bmSignature == "bookmark-signature", "an unchanged rematch preserves the bookmark signature")
assert(retained_book.progressPushedPct == 0.5, "an unchanged rematch preserves the progress marker")
assert(retained_book.ratingSyncedKnown == true, "an unchanged rematch preserves state acknowledgments")
-- A new server file must not inherit acknowledgments earned against the old
-- row. Its local file path is the only safe part of the old record.
local replaced = newState({ libraryVersion = "lib-v2" })
replaced.books["moved"] = {
bookId = 1,
fileId = 11,
file = "/books/moved.epub",
statsWatermark = 900,
annWatermark = "2026-01-01 10:00:00",
annCount = 3,
sidecarMtime = 100,
annSignature = "ann-signature",
annExchangedAt = 200,
bmSignature = "bookmark-signature",
bmExchangedAt = 300,
progressPushedPct = 0.5,
ratingSyncedKnown = true,
reviewSyncedKnown = true,
statusSyncedModified = "2026-01-01 10:00:00",
}
replaced:setMatched("moved", 22, 2, nil, "lib-v2")
local replaced_book = replaced:getBook("moved")
assert(replaced_book.fileId == 22 and replaced_book.bookId == 2, "an identity change stores the new server ids")
assert(replaced_book.file == "/books/moved.epub", "an identity change preserves the local file path")
assert(replaced_book.statsWatermark == 0, "an identity change resets the statistics watermark")
assert(replaced_book.annWatermark == "", "an identity change resets the annotation watermark")
assert(replaced_book.annCount == 0, "an identity change resets the annotation count")
assert(replaced_book.sidecarMtime == nil, "an identity change expires the sidecar marker")
assert(replaced_book.annSignature == nil and replaced_book.annExchangedAt == nil,
"an identity change expires annotation exchange markers")
assert(replaced_book.bmSignature == nil and replaced_book.bmExchangedAt == nil,
"an identity change expires bookmark exchange markers")
assert(replaced_book.progressPushedPct == nil, "an identity change expires the progress marker")
assert(replaced_book.ratingSyncedKnown == nil and replaced_book.reviewSyncedKnown == nil,
"an identity change expires rating and review acknowledgments")
assert(replaced_book.statusSyncedModified == nil, "an identity change expires the status acknowledgment")
-- A server-side folder merge re-parents a file row to another book without
-- changing its id, taking the old book's highlights, status, rating and review
-- with it. The file-scoped statistics survive that, but the book-scoped
-- acknowledgments must not, so a changed book id alone is an identity change.
local reparented = newState({ libraryVersion = "lib-v1" })
reparented.books["merged"] = {
bookId = 1,
fileId = 11,
file = "/books/merged.epub",
statsWatermark = 900,
annWatermark = "2026-01-01 10:00:00",
annCount = 3,
annSignature = "ann-signature",
ratingSyncedKnown = true,
reviewSyncedKnown = true,
statusSyncedModified = "2026-01-01 10:00:00",
}
reparented:setMatched("merged", 11, 2, nil, "lib-v1")
local reparented_book = reparented:getBook("merged")
assert(reparented_book.fileId == 11 and reparented_book.bookId == 2, "a re-parented file stores the new book id")
assert(reparented_book.file == "/books/merged.epub", "a re-parented file preserves the local file path")
assert(reparented_book.annWatermark == "" and reparented_book.annCount == 0,
"a re-parented file resets the annotation cursors")
assert(reparented_book.annSignature == nil, "a re-parented file expires the annotation signature")
assert(reparented_book.ratingSyncedKnown == nil and reparented_book.reviewSyncedKnown == nil,
"a re-parented file expires rating and review acknowledgments")
assert(reparented_book.statusSyncedModified == nil, "a re-parented file expires the status acknowledgment")
assert(reparented_book.statsWatermark == 0,
"a re-parented file resets the statistics watermark too, because one record carries both scopes")
-- A legacy matched record without a server file id has no safe cursor scope.
local unscoped = newState({ libraryVersion = "lib-v1" })
unscoped.books["legacy-cursor"] = {
file = "/books/legacy-cursor.epub",
statsWatermark = 700,
annWatermark = "2025-01-01 10:00:00",
}
unscoped:setMatched("legacy-cursor", 33, 3, nil, "lib-v1")
assert(unscoped:getBook("legacy-cursor").statsWatermark == 0,
"a legacy unscoped statistics cursor is reset on its first match")
assert(unscoped:getBook("legacy-cursor").annWatermark == "",
"a legacy unscoped annotation cursor is reset on its first match")
replaced:setUnmatched("moved")
assert(replaced:getBook("moved") == nil, "an unmatched response removes the complete matched record")
-- State written before the stamp existed must recheck once, not be trusted.
local legacy = newState({ libraryVersion = "lib-v1" })
legacy.books["legacy1"] = { bookId = 3, fileId = 4, file = "/books/legacy.epub" }
@@ -167,6 +167,72 @@ do
"a candidate-less matched book with no stamp is refreshed by the sweep")
end
-- A library recreation can replace a server file without an intervening
-- unmatched response. The rematch must reset the old target's cursor before
-- the statistics queue is built, then retain the new server acknowledgment.
do
local state = {
books = {
aaa = {
bookId = 1,
fileId = 11,
file = "/books/a.epub",
statsWatermark = 1000,
annWatermark = "2026-01-01 10:00:00",
annCount = 2,
sidecarMtime = 100,
matchVerifiedAt = NOW - 2 * DAY,
matchVerifiedVersion = "v1",
},
},
global = { libraryVersion = "v1" },
}
local harness = SweepHarness.install{
books = { { md5 = "aaa", id = 1, title = "A", last_open = 100 } },
events = { [1] = { { page = 4, start_time = 500 } } },
library_version = "v2",
matches = { aaa = { bookId = 2, bookFileId = 22 } },
state = state,
sidecar = {
sidecarMtime = function() return 100 end,
extract = function()
return {
annotations = { { datetime = "2025-01-01 10:00:00", text = "A highlight" } },
annotations_count = 1,
annotations_max_datetime = "2025-01-01 10:00:00",
annotations_signature = "new-signature",
bookmarks = {},
bookmarks_signature = "empty-bookmarks",
}
end,
},
page_stats_response = function(books)
return {
results = { { hash = books[1].hash, watermark = books[1].events[1].startTime } },
unmatched = {},
}
end,
}
local Sweep = require("bookorbit_sweep")
startSweep(harness, Sweep)
harness.scheduler:drain()
assertEqual(#harness.calls.page_stats, 1, "a replacement server file receives historical reading data")
assertEqual(#harness.calls.page_stats[1][1].events, 1, "the historical event is included in the upload")
assertEqual(harness.calls.page_stats[1][1].events[1].startTime, 500, "the event predates the old target's watermark")
assertEqual(harness.state.books.aaa.fileId, 22, "the replacement server file id is stored")
assertEqual(harness.state.books.aaa.statsWatermark, 500, "the replacement target stores the acknowledged watermark")
assertEqual(#harness.calls.annotation_exchanges, 1, "a replacement server file forces an annotation exchange")
assertEqual(#harness.calls.annotation_exchanges[1].annotations, 1, "the replacement exchange carries the local annotation")
assertEqual(harness.state.books.aaa.sidecarMtime, 100, "the replacement target stores the newly delivered sidecar marker")
startSweep(harness, Sweep)
harness.scheduler:drain()
assertEqual(#harness.calls.page_stats, 1, "the acknowledged history is not uploaded again")
assertEqual(#harness.calls.annotation_exchanges, 1, "the acknowledged sidecar is not exchanged again")
end
-- Cancellation stops the run at its next yield: the step already scheduled
-- runs, sees it is no longer the current generation and writes nothing.
do
+16 -3
View File
@@ -85,13 +85,20 @@ local function installClient(handle)
if responses.unmatched[hash] then
-- left out of the response on purpose
else
table.insert(matches, { hash = hash, bookId = 1, bookFileId = 2 })
local configured = responses.matches[hash] or {}
local stored = handle.state and handle.state:getBook(hash) or {}
table.insert(matches, {
hash = hash,
bookId = configured.bookId or stored.bookId or 1,
bookFileId = configured.bookFileId or stored.fileId or 2,
})
end
end
return { matches = matches, libraryVersion = responses.library_version }
end,
uploadPageStats = function(_, books)
table.insert(handle.calls.page_stats, books)
if responses.page_stats then return responses.page_stats(books) end
return { results = {}, unmatched = {} }
end,
uploadBookStates = function(_, batch)
@@ -121,6 +128,8 @@ opts:
- state: initial { books, unmatched, files, global }
- library_version: token every server response carries
- unmatched: set of hashes match-check refuses to match
- matches: map of hashes to custom { bookId, bookFileId } match results
- page_stats_response: function returning the server response for an upload
- sidecar: overrides for the BookOrbitSidecar stub
]]
function SweepHarness.install(opts)
@@ -141,6 +150,8 @@ function SweepHarness.install(opts)
responses = {
library_version = opts.library_version,
unmatched = opts.unmatched or {},
matches = opts.matches or {},
page_stats = opts.page_stats_response,
},
calls = {
match = {},
@@ -148,6 +159,7 @@ function SweepHarness.install(opts)
book_states = {},
progress = {},
legacy_annotations = {},
annotation_exchanges = {},
sweep_complete = 0,
event_queries = 0,
latest_event_queries = 0,
@@ -249,8 +261,9 @@ function SweepHarness.install(opts)
package.loaded["bookorbit_sidecar"] = sidecar
package.loaded["bookorbit_annotations"] = {
exchangeBook = function()
return { uploaded = 0, applied = 0, deleted = 0, failed = 0, had_errors = false }
exchangeBook = function(exchange_opts)
table.insert(handle.calls.annotation_exchanges, exchange_opts)
return { uploaded = #(exchange_opts.annotations or {}), applied = 0, deleted = 0, failed = 0, had_errors = false }
end,
readWatermark = function() return "" end,
advanceWatermark = function() end,