Files
readest/apps/readest.koplugin/spec/library/syncbooks_spec.lua
T
Huang Xin 72233e1c6a feat: sync reading status across devices and with KOReader (#4634) (#4656)
* docs(sync): design spec for syncing reading status (#4634)

Field-level LWW for reading_status (dedicated reading_status_updated_at),
a new 'abandoned' status in the Readest UI, and a koplugin bridge to
KOReader's native summary.status (whole-library apply + capture).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(sync): implementation plan for syncing reading status (#4634)

Bite-sized TDD tasks across 3 parts: A) cloud field-level LWW
(reading_status_updated_at on server upsert + client pull-merge),
B) 'abandoned'/On-hold status in the Readest UI, C) koplugin bridge
to KOReader summary.status (mapping + reconcile + whole-library
apply/capture).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(sync): add reading_status_updated_at for field-level status LWW (#4634)

* feat(sync): stamp readingStatusUpdatedAt on status change in updateBookProgress

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(sync): stamp status timestamp on explicit library status edits

* feat(sync): resolve reading status by its own timestamp in client pull-merge

* feat(sync): resolve reading status by its own timestamp in server upsert (#4634)

* fix(sync): tighten reading-status merge typing + strengthen test (A5 review)

Replace as-unknown-as double-casts at read sites with typed locals
(clientBook/serverBook); retain a single as-unknown-as only at the
server-wins construction site where the static type is too narrow.
Strengthen test 3 to assert both fields with toEqual.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(library): render the 'On hold' (abandoned) status badge

* feat(library): add 'Mark as On hold' actions + i18n for abandoned status

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* i18n: translate 'On hold' and 'Mark as On hold' for the abandoned status (#4634)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(koplugin): add reading-status mapping + reconcile between Readest and KOReader

* feat(koplugin): persist + sync reading_status_updated_at in LibraryStore

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(koplugin): bridge reading status to KOReader summary.status on library sync (#4634)

* test(sync): cover koplugin v1->v2 migration + tighten status-sync tests (final review)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(sync): redesign KOReader first-sync (decisive-only + bootstrap) (#4634)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(koplugin): safe first-sync of reading status (decisive-only + bootstrap) (#4634)

KOReader auto-sets summary.status='reading' on first open, and legacy Readest
statuses have reading_status_updated_at=0, so pure timestamp LWW let opening a
finished book downgrade it. Restrict sync to deliberate statuses (finished/
complete, abandoned/on-hold, unread->clear); never capture KO 'reading'/'New'.
On the unsynced baseline (Readest ts=0) conflicts resolve Readest-authoritative,
then stamp now_ms to exit bootstrap into steady-state LWW. reconcile now returns
write_ko/write_store flags; statussync captures now_ms once and equalizes both
sides (convergent, idempotent, resumable).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(koplugin): cover remaining first-sync graph cells + document sort effect (review)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-19 05:00:40 +02:00

421 lines
19 KiB
Lua

-- syncbooks_spec.lua
-- Pure-function tests for library/syncbooks.lua. The network-touching parts
-- (pullBooks, downloadBook, downloadCover) require Spore + httpclient +
-- NetworkMgr + UIManager.looper at require-time and aren't unit-testable;
-- they're covered by the manual matrix. These specs lock down the bits we
-- CAN exercise in isolation: the cloud fileKey / local-filename builders
-- that the UI tier depends on.
require("spec_helper")
describe("library.syncbooks", function()
local syncbooks
before_each(function()
package.loaded["library.syncbooks"] = nil
syncbooks = require("library.syncbooks")
end)
-- =====================================================================
-- build_file_key — cloud download fileKey for a book file
-- =====================================================================
describe("build_file_key", function()
it("returns the canonical {user_id}/Readest/Books/{hash}/{hash}.{ext} shape", function()
local key = syncbooks.build_file_key({
user_id = "u-alice",
hash = "abc123",
format = "EPUB",
})
assert.are.equal("u-alice/Readest/Books/abc123/abc123.epub", key)
end)
it("derives extension from format via library.exts", function()
local cases = {
EPUB = "epub", PDF = "pdf", MOBI = "mobi", AZW3 = "azw3",
CBZ = "cbz", FB2 = "fb2", TXT = "txt", MD = "md",
}
for fmt, ext in pairs(cases) do
local key = syncbooks.build_file_key({
user_id = "u",
hash = "h",
format = fmt,
})
assert.are.equal("u/Readest/Books/h/h." .. ext, key,
"wrong key for format " .. fmt)
end
end)
it("returns nil for an unknown format (caller decides what to do)", function()
local key = syncbooks.build_file_key({
user_id = "u",
hash = "h",
format = "ROT13",
})
assert.is_nil(key)
end)
it("returns nil when user_id is missing or empty", function()
assert.is_nil(syncbooks.build_file_key({ hash = "h", format = "EPUB" }))
assert.is_nil(syncbooks.build_file_key({ user_id = "", hash = "h", format = "EPUB" }))
end)
it("returns nil when hash is missing or empty", function()
assert.is_nil(syncbooks.build_file_key({ user_id = "u", format = "EPUB" }))
assert.is_nil(syncbooks.build_file_key({ user_id = "u", hash = "", format = "EPUB" }))
end)
end)
-- =====================================================================
-- build_cover_key — cloud download fileKey for a cover image
-- =====================================================================
describe("build_cover_key", function()
it("returns {user_id}/Readest/Books/{hash}/cover.png — same on R2 and S3", function()
local key = syncbooks.build_cover_key({ user_id = "u-bob", hash = "deadbeef" })
assert.are.equal("u-bob/Readest/Books/deadbeef/cover.png", key)
end)
it("returns nil with missing user_id or hash", function()
assert.is_nil(syncbooks.build_cover_key({ hash = "h" }))
assert.is_nil(syncbooks.build_cover_key({ user_id = "u" }))
assert.is_nil(syncbooks.build_cover_key({ user_id = "", hash = "h" }))
assert.is_nil(syncbooks.build_cover_key({ user_id = "u", hash = "" }))
end)
end)
-- =====================================================================
-- build_local_filename — what we save the cloud download as on disk
-- =====================================================================
-- Per design doc: local layout is FLAT (KOReader users prefer flat
-- dirs). Filename derived from source_title || title, sanitized for
-- filesystem-illegal chars. No JS-parity port required.
describe("build_local_filename", function()
it("uses source_title when present, else title", function()
assert.are.equal("Foundation.epub", syncbooks.build_local_filename({
title = "anything", source_title = "Foundation", format = "EPUB",
}))
assert.are.equal("Dune.pdf", syncbooks.build_local_filename({
title = "Dune", format = "PDF",
}))
end)
it("replaces filesystem-illegal chars with _ (FAT/NTFS-safe)", function()
assert.are.equal("Sci-Fi_Classics_Foundation.epub", syncbooks.build_local_filename({
title = "Sci-Fi/Classics/Foundation", format = "EPUB",
}))
assert.are.equal("Title_with_pipe_and_quote.epub", syncbooks.build_local_filename({
title = 'Title|with|pipe"and"quote', format = "EPUB",
}))
end)
it("strips control characters", function()
assert.are.equal("Foo_Bar.epub", syncbooks.build_local_filename({
title = "Foo\1Bar", format = "EPUB",
}))
end)
it("clamps long names (no Lua-side surprise on 1KB titles)", function()
local long = string.rep("a", 500)
local out = syncbooks.build_local_filename({ title = long, format = "EPUB" })
-- 200 byte body + ".epub" extension = 205 max
assert.is_true(#out <= 205, "filename too long: " .. #out)
assert.is_true(out:match("%.epub$") ~= nil, "extension lost")
end)
it("preserves UTF-8 (CJK + emoji) without mangling code points", function()
local out = syncbooks.build_local_filename({
title = "三体 — 第一部 📖", format = "EPUB",
})
-- Em dash is allowed; CJK + emoji preserved verbatim.
assert.is_true(out:match("三体") ~= nil)
assert.is_true(out:match("📖") ~= nil)
assert.is_true(out:match("%.epub$") ~= nil)
end)
it("returns 'book.{ext}' when title is empty/missing", function()
assert.are.equal("book.epub", syncbooks.build_local_filename({ format = "EPUB" }))
assert.are.equal("book.epub", syncbooks.build_local_filename({ title = "", format = "EPUB" }))
end)
it("returns nil for unknown format (matches build_file_key behavior)", function()
assert.is_nil(syncbooks.build_local_filename({ title = "X", format = "ROT13" }))
end)
end)
-- =====================================================================
-- _row_to_wire — converts our snake_case store row to the camelCase
-- Book the server expects on POST /sync. Locks the field mapping
-- against drift from transformBookToDB at
-- apps/readest-app/src/utils/transform.ts:66-105.
-- =====================================================================
describe("_row_to_wire", function()
it("maps snake_case row fields to camelCase wire fields", function()
local out = syncbooks._row_to_wire({
hash = "h1",
meta_hash = "m1",
format = "EPUB",
title = "Foundation",
source_title = "Foundation (Original)",
author = "Asimov",
group_id = "gid1",
group_name = "Sci-Fi",
reading_status = "reading",
created_at = 1700000000000,
updated_at = 1770000000000,
deleted_at = nil,
uploaded_at = 1750000000000,
})
assert.are.equal("h1", out.bookHash)
assert.are.equal("h1", out.hash)
assert.are.equal("m1", out.metaHash)
assert.are.equal("EPUB", out.format)
assert.are.equal("Foundation", out.title)
assert.are.equal("Foundation (Original)", out.sourceTitle)
assert.are.equal("Asimov", out.author)
assert.are.equal("gid1", out.groupId)
assert.are.equal("Sci-Fi", out.groupName)
assert.are.equal("reading", out.readingStatus)
assert.are.equal(1700000000000, out.createdAt)
assert.are.equal(1770000000000, out.updatedAt)
assert.is_nil(out.deletedAt)
assert.are.equal(1750000000000, out.uploadedAt)
end)
it("parses metadata_json back to a table for the server", function()
local out = syncbooks._row_to_wire({
hash = "h1", title = "T",
metadata_json = '{"series":"Foundation","seriesIndex":1}',
})
assert.is_table(out.metadata)
assert.are.equal("Foundation", out.metadata.series)
assert.are.equal(1, out.metadata.seriesIndex)
end)
it("parses progress_lib back to a [cur, total] tuple", function()
local out = syncbooks._row_to_wire({
hash = "h1", title = "T",
progress_lib = "[42,250]",
})
assert.is_table(out.progress)
assert.are.equal(42, out.progress[1])
assert.are.equal(250, out.progress[2])
end)
it("survives missing/malformed metadata + progress without crashing", function()
local out = syncbooks._row_to_wire({
hash = "h1", title = "T",
metadata_json = "{not valid json",
progress_lib = "garbage",
})
assert.is_nil(out.metadata)
assert.is_nil(out.progress)
end)
it("returns nil for nil input (defensive)", function()
assert.is_nil(syncbooks._row_to_wire(nil))
end)
it("emits readingStatusUpdatedAt in the wire payload", function()
local wire = syncbooks._row_to_wire({ hash = "h1", title = "T",
reading_status = "finished", reading_status_updated_at = 1750000000000 })
assert.are.equal("finished", wire.readingStatus)
assert.are.equal(1750000000000, wire.readingStatusUpdatedAt)
end)
end)
-- =====================================================================
-- resolve_collision — bumps {name}.ext → {name} (1).ext on collision
-- =====================================================================
-- Pure helper; takes a list of existing filenames + a candidate, returns
-- a non-colliding name. Real downloadBook calls lfs.attributes; this
-- helper takes a "does it exist" predicate so we can test it.
describe("resolve_collision", function()
it("returns the input when nothing collides", function()
local out = syncbooks.resolve_collision("Dune.epub", function() return false end)
assert.are.equal("Dune.epub", out)
end)
it("appends (1) on first collision", function()
local existing = { ["Dune.epub"] = true }
local out = syncbooks.resolve_collision("Dune.epub",
function(name) return existing[name] == true end)
assert.are.equal("Dune (1).epub", out)
end)
it("walks (1)..(N) until a free slot is found", function()
local existing = {}
existing["Dune.epub"] = true
existing["Dune (1).epub"] = true
existing["Dune (2).epub"] = true
local out = syncbooks.resolve_collision("Dune.epub",
function(name) return existing[name] == true end)
assert.are.equal("Dune (3).epub", out)
end)
it("handles names without an extension", function()
local out = syncbooks.resolve_collision("README",
function(name) return name == "README" end)
assert.are.equal("README (1)", out)
end)
end)
-- =====================================================================
-- extractLocalCover(file_path, dst_png) — render a book's embedded cover
-- to dst_png as PNG via coverbrowser's BookInfo:getCoverImage so
-- uploadBook can ship a cover for books that originated on this device
-- (issue #4374), not just ones previously downloaded from the cloud.
-- The blitbuffer/document work is live-KOReader-only, so we inject a fake
-- BookInfo via package.loaded and assert the success/failure wiring.
-- =====================================================================
describe("extractLocalCover", function()
local BI_KEY = "apps/filemanager/filemanagerbookinfo"
local saved_bookinfo
before_each(function()
saved_bookinfo = package.loaded[BI_KEY]
end)
after_each(function()
package.loaded[BI_KEY] = saved_bookinfo
end)
it("writes the cover as PNG and returns true when the book has a cover", function()
local wrote, freed
local fake_bb = {
writeToFile = function(_self, path, fmt)
wrote = { path = path, fmt = fmt }
return true
end,
free = function() freed = true end,
}
package.loaded[BI_KEY] = {
getCoverImage = function(_self, document, file)
-- Called with a nil document + the book's path so BookInfo
-- opens the file itself (matches calibre.koplugin's usage).
assert.is_nil(document)
assert.are.equal("/books/foo.epub", file)
return fake_bb
end,
}
local ok = syncbooks.extractLocalCover("/books/foo.epub", "/cache/abc.png")
assert.is_true(ok)
assert.are.equal("/cache/abc.png", wrote.path)
assert.are.equal("png", wrote.fmt)
assert.is_true(freed, "the cover blitbuffer must be freed")
end)
it("returns false when the book has no extractable cover", function()
package.loaded[BI_KEY] = {
getCoverImage = function() return nil end,
}
assert.is_false(syncbooks.extractLocalCover("/books/foo.epub", "/cache/abc.png"))
end)
it("returns false when the PNG write fails", function()
package.loaded[BI_KEY] = {
getCoverImage = function()
return { writeToFile = function() return false end, free = function() end }
end,
}
assert.is_false(syncbooks.extractLocalCover("/books/foo.epub", "/cache/abc.png"))
end)
it("returns false when coverbrowser's BookInfo isn't available", function()
-- Neither package.loaded nor package.path resolves the module in
-- the test env, so require() errors and the pcall guard kicks in.
package.loaded[BI_KEY] = nil
assert.is_false(syncbooks.extractLocalCover("/books/foo.epub", "/cache/abc.png"))
end)
it("returns false for missing arguments", function()
assert.is_false(syncbooks.extractLocalCover(nil, "/cache/abc.png"))
assert.is_false(syncbooks.extractLocalCover("/books/foo.epub", nil))
end)
end)
-- =====================================================================
-- syncBooks(opts, mode, cb, before_push) — bidirectional orchestration.
--
-- The order matters: pull must run BEFORE push in "both" mode so the
-- local row has fresh cloud-side fields (uploaded_at, metadata, etc.)
-- before we touch + push it. Otherwise the push would send a row with
-- those fields nil, and the server's transformBookToDB explicit-nulls
-- them on the cloud — wiping out group/upload state on every device
-- that pulls afterward. Regression guard for issue #4138.
-- =====================================================================
describe("syncBooks", function()
-- Stub the network-touching halves so we can record call order and
-- assert it without standing up Spore + a fake server.
local function with_stubs(fn)
local original_pull = syncbooks.pullBooks
local original_push = syncbooks.pushChangedBooks
local calls = {}
syncbooks.pullBooks = function(_opts, cb)
table.insert(calls, "pull")
if cb then cb(true, 0) end
end
syncbooks.pushChangedBooks = function(_opts, cb)
table.insert(calls, "push")
if cb then cb(true, 0) end
end
local ok, err = pcall(fn, calls)
syncbooks.pullBooks = original_pull
syncbooks.pushChangedBooks = original_push
if not ok then error(err) end
end
it("runs pull before push in 'both' mode", function()
with_stubs(function(calls)
local before_push_at
syncbooks.syncBooks({}, "both",
function() end,
function() before_push_at = #calls end)
assert.are.same({ "pull", "push" }, calls)
-- before_push runs AFTER pull and BEFORE push — i.e. with
-- exactly one call ("pull") recorded so far.
assert.are.equal(1, before_push_at)
end)
end)
it("invokes before_push between pull and push in 'both' mode", function()
with_stubs(function(calls)
local before_push_called = 0
syncbooks.syncBooks({}, "both",
function() end,
function() before_push_called = before_push_called + 1 end)
assert.are.equal(1, before_push_called)
end)
end)
it("invokes before_push and then push in 'push' mode", function()
with_stubs(function(calls)
local before_push_at
syncbooks.syncBooks({}, "push",
function() end,
function() before_push_at = #calls end)
assert.are.same({ "push" }, calls)
assert.are.equal(0, before_push_at)
end)
end)
it("skips before_push in 'pull' mode (no push happens)", function()
with_stubs(function(calls)
local before_push_called = 0
syncbooks.syncBooks({}, "pull",
function() end,
function() before_push_called = before_push_called + 1 end)
assert.are.same({ "pull" }, calls)
assert.are.equal(0, before_push_called)
end)
end)
it("tolerates a missing before_push callback", function()
with_stubs(function(calls)
-- No before_push passed; orchestration should still work.
syncbooks.syncBooks({}, "both", function() end)
assert.are.same({ "pull", "push" }, calls)
end)
end)
end)
end)