From 082edc204b83b8f5f648715a719e620184a16a04 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Mon, 22 Jun 2026 23:31:03 +0800 Subject: [PATCH] fix(sync): sync updated book covers across devices (#4544) (#4731) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * docs: design for syncing updated book data (cover + file) (#4544) Cover-change sync via a content hash (coverHash = partial MD5 of cover.png) plus a cover_updated_at field-level merge timestamp; file updates ride the existing re-import / metaHash dedupe. Co-Authored-By: Claude Opus 4.8 (1M context) * fix(sync): sync updated book covers across devices (#4544) Editing a book's cover wrote cover.png locally but changed no hash (the cover is keyed by the file hash), so peers had no signal to re-download it and the change never propagated. Give the cover its own content-addressed version: - coverHash = partial MD5 of cover.png; a peer re-downloads the cover iff the synced hash differs from the local one (idempotent, no churn on identical/re-extracted covers — compatible with the metaHash dedupe). - coverUpdatedAt = field-level LWW timestamp so a page-turn that wins whole-row LWW on updated_at can't clobber a cover edit (mirrors the reading_status_updated_at fix for #4634). Editing a cover recomputes the hash, bumps coverUpdatedAt, and re-uploads only the cover; the server merges cover fields independently; peers re-download on a hash diff. File updates continue to ride the existing re-import / metaHash dedupe (changed file -> changed hash -> re-key). Migration 016 adds cover_hash / cover_updated_at to books. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- ...026-06-22-sync-updated-book-data-design.md | 300 ++++++++++++++++++ .../app/library/cover-sync-helpers.test.ts | 109 +++++++ .../pages/api/sync-cover-merge.test.ts | 48 +++ .../src/__tests__/services/cover-sync.test.ts | 129 ++++++++ .../services/import-metahash.test.ts | 25 ++ .../src/__tests__/utils/transform.test.ts | 55 ++++ .../src/app/library/hooks/useBooksSync.ts | 24 +- apps/readest-app/src/app/library/page.tsx | 26 ++ .../src/app/library/utils/libraryUtils.ts | 41 +++ apps/readest-app/src/pages/api/sync.ts | 60 +++- apps/readest-app/src/services/appService.ts | 8 + apps/readest-app/src/services/bookService.ts | 20 ++ apps/readest-app/src/services/cloudService.ts | 19 ++ apps/readest-app/src/types/book.ts | 7 + apps/readest-app/src/types/records.ts | 2 + apps/readest-app/src/types/system.ts | 2 + apps/readest-app/src/utils/transform.ts | 8 + docker/volumes/db/init/schema.sql | 2 + .../migrations/017_add_book_cover_version.sql | 23 ++ 19 files changed, 892 insertions(+), 16 deletions(-) create mode 100644 apps/readest-app/.claude/plans/2026-06-22-sync-updated-book-data-design.md create mode 100644 apps/readest-app/src/__tests__/app/library/cover-sync-helpers.test.ts create mode 100644 apps/readest-app/src/__tests__/pages/api/sync-cover-merge.test.ts create mode 100644 apps/readest-app/src/__tests__/services/cover-sync.test.ts create mode 100644 docker/volumes/db/migrations/017_add_book_cover_version.sql diff --git a/apps/readest-app/.claude/plans/2026-06-22-sync-updated-book-data-design.md b/apps/readest-app/.claude/plans/2026-06-22-sync-updated-book-data-design.md new file mode 100644 index 00000000..67c3e5f4 --- /dev/null +++ b/apps/readest-app/.claude/plans/2026-06-22-sync-updated-book-data-design.md @@ -0,0 +1,300 @@ +# Sync Updated Book Data (Cover + File) — Design + +**Issue:** [#4544 — Customized Book Cover Not Synced](https://github.com/readest/readest/issues/4544) +**Date:** 2026-06-22 +**Status:** Approved design, pending implementation plan + +## Problem + +A user changed a book's cover via Readest's metadata editor on macOS. The new +cover appeared on the Mac but never propagated to their iPad or other devices. +More generally, Readest cannot re-sync a book's **cover** after the book has +been uploaded — the cover is uploaded once at first sync and downloaded once per +peer, with no way to detect or propagate a later change. + +### Root cause (verified) + +1. A cover lives in cloud storage at `books//cover.png`, uploaded **once** + by `cloudService.uploadBook()` (which stamps `book.uploadedAt`). + — `src/services/cloudService.ts:195-200,216-218` +2. Peers download the cover **once**, gated one-shot on + `!book.deletedAt && book.uploadedAt && !book.coverDownloadedAt`. Once + `coverDownloadedAt` is set it never refetches. + — `src/app/library/hooks/useBooksSync.ts:119-128` +3. Editing the cover (`handleUpdateMetadata` → `appService.updateCoverImage`) + writes the new `cover.png` **locally only** and bumps `book.updatedAt`. It + never re-uploads the cover and emits no synced signal that the cover changed. + The cover is keyed by the **file** hash, so a cover-only edit changes **no + hash** — there is nothing for peers to detect. + — `src/app/library/page.tsx:934-962`, `src/services/bookService.ts:158-170` + +## Design principle: content hashes + the existing dedupe + +Readest already identifies content by **partial MD5**: + +- `book.hash` = `partialMD5` of the **file bytes** (sampled ranges) — the unique + identifier. `src/utils/md5.ts:11-30` +- `book.metaHash` = MD5 of `title|authors|identifiers` — the "logical book" + across versions. `src/utils/book.ts:355-372` + +We extend the same content-addressed philosophy to the cover, and we reuse the +existing dedupe for the file, rather than inventing a parallel versioning system. + +### File updates ride the existing re-import / dedupe (no new field) + +A changed file already changes its `partialMD5` (`book.hash`). The importer's +Tier-3 **metaHash match** (`bookService.ts:402-458`, `mergeBooks` +`bookService.ts:183-245`) already handles re-import of an edited file: + +- overwrites `book.hash` with the new file's hash, overwrites metadata, +- migrates `config.json` / booknotes to the new `Books//` dir, +- **soft-deletes** the old entry (`deletedAt`), and sets `uploadedAt=null` to + force re-upload of the new file + cover. + +Cross-device, this converges through existing sync: peers pull the **old-hash +row with `deleted_at`** (remove old) and the **new-hash row with `uploaded_at`** +(download new), and progress/notes follow the `book_hash OR meta_hash` pull +query (`sync.ts:142`, `useNotesSync.ts:183`). + +> Implication: there is **no** `fileUpdatedAt`, no stable-hash-vs-content split, +> and no in-place file replacement. "Use the partial MD5 to detect a file +> update" is already true — the hash *is* the file's partial MD5, and changing +> the file changes the hash. The book's identity for progress/notes is preserved +> by `metaHash`, exactly the dedupe mechanism. + +This part of the work is **verify + fix gaps**, not new architecture (see §E). + +### Cover updates use a cover content hash (new) + +The cover is keyed by the *file* hash, so a cover-only edit changes no hash and +emits no signal. Fix: give the cover its own content hash. + +- **`coverHash`** = `partialMD5` of the local `cover.png`, synced. +- **Invariant:** on every device, `book.coverHash === partialMD5(cover.png)`. +- A peer re-downloads the cover **iff** `synced.coverHash !== local.coverHash`. + Content-addressed ⇒ re-extracting/re-importing a byte-identical cover yields + the same hash ⇒ **no churn** (the dedupe-compatible property). +- **`coverUpdatedAt`** (timestamp) accompanies it purely for **merge ordering**: + the cover edit shares the `books` row with page-turn progress, so without a + per-field timestamp a concurrent page-turn would clobber a just-edited + `coverHash` under whole-row last-writer-wins — the #4634 bug class fixed by + `reading_status_updated_at`. `coverHash` answers *"did it change?"*; + `coverUpdatedAt` answers *"whose change wins?"*. + +## Scope + +In scope: + +- **Cover update sync** (build): `coverHash` + `coverUpdatedAt`, re-upload on + edit, peer re-download on hash diff. Fixes #4544 and covers both triggers — a + metadata-editor cover edit, and a re-extracted cover from a re-imported file. +- **File update sync** (leverage + verify): rely on the existing re-import / + metaHash dedupe; verify cross-device convergence end-to-end and fix any gaps. + +Out of scope: + +- Dedicated "Replace file…" UI. Users update a file by re-importing it normally + (drag-drop / open-with / file picker); the dedupe path handles it. +- Remapping reading position (CFI) across a structurally-changed file (inherent + to changing the bytes; best-effort via the metaHash config carry-over). +- Preserving a **custom** cover across a file re-import. Tier-3 re-import + re-extracts the cover from the new file, overwriting a custom one. Pre-existing + behavior; noted as a future consideration, not solved here (see §E). + +## A. Data model & schema + +**Migration** `docker/volumes/db/migrations/016_add_book_cover_version.sql` +(mirrors `015_add_reading_status_updated_at.sql`): + +```sql +-- Migration 016: Add cover_hash / cover_updated_at to books +-- +-- Cover-change sync. cover_hash = partial MD5 of cover.png (content-addressed +-- change detection: identical cover ⇒ identical hash ⇒ no re-sync churn). +-- cover_updated_at = field-level LWW timestamp so a page-turn that wins +-- whole-row LWW on updated_at cannot clobber a cover edit (same hazard the 015 +-- reading_status_updated_at fix addressed for #4634). Both additive + nullable; +-- NULL cover_updated_at = epoch 0 (oldest) in the merge. +ALTER TABLE public.books + ADD COLUMN IF NOT EXISTS cover_hash text NULL, + ADD COLUMN IF NOT EXISTS cover_updated_at timestamp with time zone NULL; +``` + +Mirror both columns into `docker/volumes/db/init/schema.sql` (the `books` table). + +Types & transform: + +- `DBBook` (`src/types/records.ts`): add `cover_hash?: string | null`, + `cover_updated_at?: string | null`. +- `Book` (`src/types/book.ts`): add `coverHash?: string | null`, + `coverUpdatedAt?: number | null`. +- `src/utils/transform.ts`: map both directions (`cover_hash` ↔ `coverHash` + verbatim; `cover_updated_at` ms ↔ ISO), like `reading_status_updated_at` + (`transformBookToDB` ~99-107, `transformBookFromDB` ~143-151). + +New helper (e.g. `src/services/bookService.ts` or `src/utils/book.ts`): + +```ts +// Open Books//cover.png as a File and partial-MD5 it. Returns null if +// the cover is absent. Keeps book.coverHash === partialMD5(cover.png). +computeCoverHash(fs, book): Promise +``` + +## B. Local change → recompute hash + re-upload + signal + +### Cover edit (extend the existing path) + +In `handleUpdateMetadata` (`src/app/library/page.tsx:934-962`), after +`appService.updateCoverImage()` writes `cover.png`: + +1. `const newHash = await appService.computeCoverHash(book);` +2. **Idempotency gate:** if `newHash === book.coverHash`, do nothing further (no + timestamp bump, no re-upload, no churn). +3. Else: `book.coverHash = newHash; book.coverUpdatedAt = Date.now();` and bump + `book.updatedAt` (already happens via `getBookWithUpdatedMetadata`; keep the + value consistent). +4. If `book.uploadedAt || settings.autoUpload`, re-upload **only** the cover via + a new `appService.uploadBookCover(book)` (it must **not** touch `uploadedAt`, + which means "the file is in cloud as of T"). +5. `updateBook(envConfig, book)` persists; the existing push (`getNewBooks` keys + off `updatedAt` — `useBooksSync.ts:26-32`) carries the row incl. the new + `coverHash` / `coverUpdatedAt`. + +New `cloudService.uploadBookCover(fs, resolveFilePath, book, onProgress?)`: a +small sibling of `uploadBook` that uploads only `cover.png` to +`books//cover.png`. + +### Cover on import / re-import (maintain the invariant) + +In the cover-extract path (`bookService.ts:500-512`), after writing `cover.png`, +set `book.coverHash = partialMD5(cover.png)` (and leave `coverUpdatedAt` unset on +first import — the cover version is established when first synced/uploaded; the +diff machinery handles first download via the existing `!coverDownloadedAt` +gate). On a Tier-3 re-import the new file's extracted cover yields a fresh +`coverHash` for the new `book.hash`; peers receive it via the new-book download. + +## C. Server merge (`src/pages/api/sync.ts`) + +Add `resolveCoverMerge(client, server)` next to `resolveReadingStatusMerge` +(`sync.ts:60-74`): pick `{cover_hash, cover_updated_at}` from whichever side has +the greater `cover_updated_at` (NULL = epoch 0). In the `books` branch of +`upsertRecords` (`sync.ts:417-455`), alongside the existing reading-status graft: + +- **Client wins the row** (`clientIsNewer`): graft the resolved cover fields onto + the client row before `toUpdate.push`. +- **Server wins the row but the client's cover is newer** + (`cover_updated_at` greater **and** `cover_hash` differs): write the server row + with the client's cover fields and bump `updated_at = now()` so peers re-pull + (mirrors the existing `statusChanged` re-propagation branch). +- Else: server row authoritative. + +This guarantees a cover edit survives even when a concurrent page-turn dominates +whole-row LWW on `updated_at`. + +## D. Peer pull → re-download on hash diff (`src/app/library/hooks/useBooksSync.ts`) + +Extend `updateLibrary` / `processOldBook` (`useBooksSync.ts:119-147`). Keep the +existing first-download path (`!oldBook.coverDownloadedAt`) for new/never-fetched +books — and have it **adopt** the synced hash (`oldBook.coverHash = +matchingBook.coverHash ?? (await computeCoverHash(oldBook))`) so the invariant +holds and the change path below sees equality afterwards. Then add a **change** +path: + +For a synced book with `!deletedAt && uploadedAt && matchingBook.coverHash`: + +1. If `oldBook.coverHash == null`, lazily compute it from the **local** + `cover.png` (`oldBook.coverHash = await computeCoverHash(oldBook)`). This runs + **only** for books that carry a synced `coverHash` (i.e. someone edited the + cover) and lack a local hash — a bounded set, not every book. +2. If `oldBook.coverHash !== matchingBook.coverHash`: re-download the cover + **forcing overwrite**, set `oldBook.coverHash = matchingBook.coverHash` and + `oldBook.coverImageUrl = await appService.generateCoverImageUrl(oldBook)`. + Optionally recompute the downloaded cover's hash and warn if it doesn't match + `matchingBook.coverHash` (cloud lagging a concurrent overwrite — best-effort). + +`cloudService.downloadBookCovers` / `downloadBook` get a force/`redownload` flag +so a changed cover overwrites the existing local file (today they skip when the +file already exists — `cloudService.ts:233-272,287-289`). + +The merged book carries the resolved `coverHash` / `coverUpdatedAt` forward. + +## E. File updates via re-import (verify + fix gaps) + +No new architecture. Implementation tasks: + +1. **Verify cross-device convergence** of an edited-file re-import end-to-end: + device A re-imports an edited file (same title/author) → A re-keys hash, + soft-deletes old, re-uploads new → device B removes the old version, + downloads the new, and carries progress/notes via `metaHash`. +2. **Fix gaps** found, e.g.: peer cleanup of the **old** local `Books//` + dir and its cloud objects (orphan avoidance); ensuring the soft-deleted + old-hash row and the new-hash row are both pushed in the same sync; ensuring a + re-uploaded new file actually triggers the peer download path. +3. **Custom cover note:** a Tier-3 re-import re-extracts the cover, overwriting a + custom one. If we later want to preserve a custom cover across re-import, we + can compare the pre-import `coverHash` against the freshly-extracted hash and + keep the custom cover when it differs — explicitly deferred. + +## Edge cases + +- **Idempotent re-extract / re-import:** identical cover bytes ⇒ identical + `coverHash` ⇒ no re-upload, no peer re-download (the dedupe-compatible win). +- **Legacy books (no `coverHash`):** no diff is computed until some device edits + the cover and syncs a `coverHash`; peers then lazily compute their local hash + to compare (§D). No mass re-download. +- **Not-yet-uploaded book:** `coverHash`/`coverUpdatedAt` bump locally and ride + the first `uploadBook`; peers gate on `uploadedAt`. +- **Concurrent cover edits across devices:** resolved by `coverUpdatedAt` + field-level max-merge (§C); the losing device re-downloads the winner's cover. +- **Web / OPFS:** `cover.png` lives in OPFS via `fs`; `computeCoverHash` opens it + as a File the same way `uploadFileToCloud` does. +- **Old clients:** ignore `cover_hash` / `cover_updated_at` → they neither read + nor write them, so they don't propagate cover edits (today's behavior) and + never break. +- **Delete:** unchanged. `deleted_at` tombstone + the `!deletedAt` gate take + precedence; cloud delete still clears `uploadedAt` and removes `cover.png`. + +## Testing (test-first) + +Unit (vitest): + +1. `transform.ts` — round-trips `coverHash` (verbatim) and `coverUpdatedAt` + (ms ↔ ISO), incl. null. +2. `resolveCoverMerge` — picks the side with greater `cover_updated_at`; graft + onto client-wins row; graft + `updated_at` re-propagation when server wins the + row but client cover is newer **and** hash differs; NULL = epoch 0; equal + hash ⇒ no re-propagation churn. +3. `computeCoverHash` — stable per content; differs after a cover edit; null when + no cover. +4. Cover-edit path (`handleUpdateMetadata`): identical new cover ⇒ no bump / no + `uploadBookCover`; changed cover ⇒ bumps `coverHash`+`coverUpdatedAt` and + calls `uploadBookCover` when uploaded/autoUpload (skips upload otherwise). +5. `useBooksSync` re-download decisions: synced `coverHash` differs ⇒ force + re-download + regenerate URL + adopt synced hash; equal ⇒ no-op; legacy + (no synced hash) ⇒ no-op; lazy local-hash compute only for books with a + synced hash; deleted ⇒ no-op. +6. **File re-import** (covers §E verification): an edited-file re-import re-keys + `hash`, soft-deletes the old entry, migrates config/notes, and the resulting + push set contains both the deleted old-hash row and the uploaded new-hash row. + +Verification gates (per `.agents/rules/verification.md`): `pnpm test`, +`pnpm lint`. No `src-tauri/` or koplugin changes expected. + +## Affected files (summary) + +- `docker/volumes/db/migrations/016_add_book_cover_version.sql` (new) +- `docker/volumes/db/init/schema.sql` +- `src/types/records.ts`, `src/types/book.ts` +- `src/utils/transform.ts` +- `src/utils/book.ts` / `src/services/bookService.ts` (`computeCoverHash`; + set `coverHash` in the import cover-extract path) +- `src/pages/api/sync.ts` (`resolveCoverMerge` + books-branch graft) +- `src/services/cloudService.ts` (`uploadBookCover`; force re-download in + `downloadBookCovers`/`downloadBook`) +- `src/services/appService.ts` + `src/types/system.ts` (new app-service methods) +- `src/app/library/page.tsx` (cover-edit: idempotent hash, marker bump, cover + re-upload) +- `src/app/library/hooks/useBooksSync.ts` (hash-diff cover re-download) +- (§E) re-import convergence verification + any gap fixes in + `src/services/bookService.ts` / `src/services/ingestService.ts` / + `src/services/cloudService.ts` diff --git a/apps/readest-app/src/__tests__/app/library/cover-sync-helpers.test.ts b/apps/readest-app/src/__tests__/app/library/cover-sync-helpers.test.ts new file mode 100644 index 00000000..b5acd290 --- /dev/null +++ b/apps/readest-app/src/__tests__/app/library/cover-sync-helpers.test.ts @@ -0,0 +1,109 @@ +import { describe, expect, it } from 'vitest'; +import { needsCoverRefresh, pickFresherCover } from '@/app/library/utils/libraryUtils'; +import type { Book } from '@/types/book'; + +const base: Book = { + hash: 'h1', + format: 'EPUB', + title: 'T', + author: 'A', + createdAt: 1, + updatedAt: 2, +}; + +const local = (over: Partial): Book => ({ ...base, ...over }); +const synced = (over: Partial): Book => ({ ...base, ...over }); + +describe('needsCoverRefresh (issue #4544)', () => { + it('first download: synced is in cloud and local never fetched the cover', () => { + expect( + needsCoverRefresh( + local({ coverDownloadedAt: null }), + synced({ uploadedAt: 1000, coverHash: 'hA', coverUpdatedAt: 1000 }), + ), + ).toBe(true); + }); + + it('changed cover: newer coverUpdatedAt and a different hash → refresh', () => { + expect( + needsCoverRefresh( + local({ coverDownloadedAt: 1000, coverHash: 'hOLD', coverUpdatedAt: 1000 }), + synced({ uploadedAt: 1000, coverHash: 'hNEW', coverUpdatedAt: 2000 }), + ), + ).toBe(true); + }); + + it('same hash even if timestamp is newer → no refresh (idempotent, no churn)', () => { + expect( + needsCoverRefresh( + local({ coverDownloadedAt: 1000, coverHash: 'hSAME', coverUpdatedAt: 1000 }), + synced({ uploadedAt: 1000, coverHash: 'hSAME', coverUpdatedAt: 2000 }), + ), + ).toBe(false); + }); + + it('not newer (synced older/equal) → no refresh (unpushed-local-edit race)', () => { + expect( + needsCoverRefresh( + local({ coverDownloadedAt: 1000, coverHash: 'hLOCAL', coverUpdatedAt: 3000 }), + synced({ uploadedAt: 1000, coverHash: 'hSTALE', coverUpdatedAt: 2000 }), + ), + ).toBe(false); + }); + + it('legacy synced book without a coverHash → no refresh', () => { + expect( + needsCoverRefresh( + local({ coverDownloadedAt: 1000, coverHash: null, coverUpdatedAt: null }), + synced({ uploadedAt: 1000, coverHash: null, coverUpdatedAt: null }), + ), + ).toBe(false); + }); + + it('not uploaded to cloud → no refresh', () => { + expect( + needsCoverRefresh( + local({ coverDownloadedAt: null }), + synced({ uploadedAt: null, coverHash: 'hA', coverUpdatedAt: 1000 }), + ), + ).toBe(false); + }); + + it('deleted synced book → no refresh', () => { + expect( + needsCoverRefresh( + local({ coverDownloadedAt: 1000, coverHash: 'hOLD', coverUpdatedAt: 1000 }), + synced({ deletedAt: 5000, uploadedAt: 1000, coverHash: 'hNEW', coverUpdatedAt: 2000 }), + ), + ).toBe(false); + }); +}); + +describe('pickFresherCover (issue #4544)', () => { + it('keeps the cover whose coverUpdatedAt is newer', () => { + expect( + pickFresherCover( + { coverHash: 'hLOCAL', coverUpdatedAt: 1000 }, + { coverHash: 'hSYNCED', coverUpdatedAt: 2000 }, + ), + ).toEqual({ coverHash: 'hSYNCED', coverUpdatedAt: 2000 }); + }); + + it('ties go to local (it already holds the file)', () => { + expect( + pickFresherCover( + { coverHash: 'hLOCAL', coverUpdatedAt: 1500 }, + { coverHash: 'hSYNCED', coverUpdatedAt: 1500 }, + ), + ).toEqual({ coverHash: 'hLOCAL', coverUpdatedAt: 1500 }); + }); + + it('treats a missing timestamp as oldest', () => { + expect( + pickFresherCover( + { coverHash: 'hLOCAL', coverUpdatedAt: null }, + { coverHash: 'hSYNCED', coverUpdatedAt: 1 }, + ), + ).toEqual({ coverHash: 'hSYNCED', coverUpdatedAt: 1 }); + }); +}); diff --git a/apps/readest-app/src/__tests__/pages/api/sync-cover-merge.test.ts b/apps/readest-app/src/__tests__/pages/api/sync-cover-merge.test.ts new file mode 100644 index 00000000..32bf3da8 --- /dev/null +++ b/apps/readest-app/src/__tests__/pages/api/sync-cover-merge.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from 'vitest'; +import { resolveCoverMerge } from '@/pages/api/sync'; + +const iso = (ms: number) => new Date(ms).toISOString(); + +describe('resolveCoverMerge (issue #4544)', () => { + it('keeps the client cover when its cover_updated_at is newer', () => { + const out = resolveCoverMerge( + { cover_hash: 'newhash', cover_updated_at: iso(200) }, + { cover_hash: 'oldhash', cover_updated_at: iso(100) }, + ); + expect(out).toEqual({ cover_hash: 'newhash', cover_updated_at: iso(200) }); + }); + + it('keeps the server cover when its cover_updated_at is newer', () => { + const out = resolveCoverMerge( + { cover_hash: 'clienthash', cover_updated_at: iso(100) }, + { cover_hash: 'serverhash', cover_updated_at: iso(300) }, + ); + expect(out).toEqual({ cover_hash: 'serverhash', cover_updated_at: iso(300) }); + }); + + it('ties go to the client', () => { + const out = resolveCoverMerge( + { cover_hash: 'clienthash', cover_updated_at: iso(150) }, + { cover_hash: 'serverhash', cover_updated_at: iso(150) }, + ); + expect(out).toEqual({ cover_hash: 'clienthash', cover_updated_at: iso(150) }); + }); + + it('treats a missing/null timestamp as oldest', () => { + // Server has a real cover edit; an unstamped client (legacy / page-turn + // push carrying no cover_updated_at) must not win and clobber it. + const out = resolveCoverMerge( + { cover_hash: 'staleclient', cover_updated_at: null }, + { cover_hash: 'realserver', cover_updated_at: iso(1) }, + ); + expect(out).toEqual({ cover_hash: 'realserver', cover_updated_at: iso(1) }); + }); + + it('both unset → null cover (no spurious change)', () => { + const out = resolveCoverMerge( + { cover_hash: null, cover_updated_at: null }, + { cover_hash: null, cover_updated_at: null }, + ); + expect(out).toEqual({ cover_hash: null, cover_updated_at: null }); + }); +}); diff --git a/apps/readest-app/src/__tests__/services/cover-sync.test.ts b/apps/readest-app/src/__tests__/services/cover-sync.test.ts new file mode 100644 index 00000000..7fc2f8e5 --- /dev/null +++ b/apps/readest-app/src/__tests__/services/cover-sync.test.ts @@ -0,0 +1,129 @@ +import { describe, test, expect, vi, beforeEach } from 'vitest'; +import { Book, BookFormat } from '@/types/book'; +import { FileSystem } from '@/types/system'; + +// uploadBookCover uses the storage layer; mock it so we can assert the cloud +// path it uploads to without touching the network. computeCoverHash uses the +// REAL @/utils/book + @/utils/md5 (content hashing must be exercised for real). +vi.mock('@/libs/storage', () => ({ + downloadFile: vi.fn().mockResolvedValue(undefined), + uploadFile: vi.fn().mockResolvedValue(undefined), + uploadReplicaFile: vi.fn().mockResolvedValue(undefined), + deleteFile: vi.fn(), + createProgressHandler: vi.fn().mockReturnValue(vi.fn()), + batchGetDownloadUrls: vi.fn().mockResolvedValue([]), +})); + +import { uploadBookCover } from '@/services/cloudService'; +import { computeCoverHash } from '@/services/bookService'; +import { uploadFile } from '@/libs/storage'; + +function createMockBook(overrides: Partial = {}): Book { + return { + hash: 'abc123', + format: 'EPUB' as BookFormat, + title: 'Test Book', + author: 'Author', + createdAt: 1, + updatedAt: 2, + deletedAt: null, + uploadedAt: 1000, + downloadedAt: 1000, + coverDownloadedAt: 1000, + ...overrides, + }; +} + +function createMockFs(overrides: Partial = {}): FileSystem { + return { + resolvePath: vi + .fn() + .mockReturnValue({ baseDir: 0, basePrefix: async () => '', fp: 't', base: 'Books' }), + getURL: vi.fn().mockReturnValue('url'), + getBlobURL: vi.fn().mockResolvedValue('blob:url'), + getImageURL: vi.fn().mockResolvedValue('image:url'), + openFile: vi.fn().mockResolvedValue(new File(['cover-bytes'], 'cover.png')), + copyFile: vi.fn().mockResolvedValue(undefined), + readFile: vi.fn().mockResolvedValue('content'), + writeFile: vi.fn().mockResolvedValue(undefined), + removeFile: vi.fn().mockResolvedValue(undefined), + readDir: vi.fn().mockResolvedValue([]), + createDir: vi.fn().mockResolvedValue(undefined), + removeDir: vi.fn().mockResolvedValue(undefined), + exists: vi.fn().mockResolvedValue(true), + stats: vi.fn().mockResolvedValue({ + isFile: true, + isDirectory: false, + size: 100, + mtime: null, + atime: null, + birthtime: null, + }), + getPrefix: vi.fn().mockResolvedValue('Readest/Books'), + ...overrides, + } as FileSystem; +} + +describe('computeCoverHash (issue #4544)', () => { + test('returns null when the cover does not exist', async () => { + const fs = createMockFs({ exists: vi.fn().mockResolvedValue(false) }); + const hash = await computeCoverHash(fs, createMockBook()); + expect(hash).toBeNull(); + expect(fs.openFile).not.toHaveBeenCalled(); + }); + + test('is stable for identical cover bytes and differs for different bytes', async () => { + const fsA = createMockFs({ + openFile: vi.fn().mockResolvedValue(new File(['cover-A'], 'cover.png')), + }); + const fsA2 = createMockFs({ + openFile: vi.fn().mockResolvedValue(new File(['cover-A'], 'cover.png')), + }); + const fsB = createMockFs({ + openFile: vi.fn().mockResolvedValue(new File(['cover-B-different'], 'cover.png')), + }); + + const hashA = await computeCoverHash(fsA, createMockBook()); + const hashA2 = await computeCoverHash(fsA2, createMockBook()); + const hashB = await computeCoverHash(fsB, createMockBook()); + + expect(hashA).toBeTruthy(); + expect(hashA).toBe(hashA2); // idempotent: identical content ⇒ identical hash + expect(hashA).not.toBe(hashB); // changed content ⇒ changed hash + }); + + test('reads cover.png under the book hash dir', async () => { + const fs = createMockFs(); + await computeCoverHash(fs, createMockBook({ hash: 'deadbeef' })); + expect(fs.openFile).toHaveBeenCalledWith('deadbeef/cover.png', 'Books'); + }); +}); + +describe('uploadBookCover (issue #4544)', () => { + const resolveFilePath = vi.fn().mockResolvedValue('/abs/abc123/cover.png'); + + beforeEach(() => { + vi.clearAllMocks(); + }); + + test('uploads only the cover to books//cover.png and does not touch uploadedAt', async () => { + const fs = createMockFs(); + const book = createMockBook({ uploadedAt: 1000 }); + await uploadBookCover(fs, resolveFilePath, book); + expect(uploadFile).toHaveBeenCalledTimes(1); + // The cloud key is built inside uploadFileToCloud via + // fs.openFile(lfp, base, cfp). Assert the cover key was passed through. + expect(fs.openFile).toHaveBeenCalledWith( + 'abc123/cover.png', + 'Books', + 'Readest/Books/abc123/cover.png', + ); + expect(book.uploadedAt).toBe(1000); // unchanged + }); + + test('no-ops when the cover is absent', async () => { + const fs = createMockFs({ exists: vi.fn().mockResolvedValue(false) }); + await uploadBookCover(fs, resolveFilePath, createMockBook()); + expect(uploadFile).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/readest-app/src/__tests__/services/import-metahash.test.ts b/apps/readest-app/src/__tests__/services/import-metahash.test.ts index 673567a5..102e41d4 100644 --- a/apps/readest-app/src/__tests__/services/import-metahash.test.ts +++ b/apps/readest-app/src/__tests__/services/import-metahash.test.ts @@ -158,6 +158,31 @@ describe('importBook metaHash deduplication', () => { expect(existingBook.metaHash).toBe(metaHash); }); + // Cross-device file-update convergence (issue #4544 §E): re-importing an + // edited file re-keys the hash and clears uploadedAt so the new bytes get + // re-uploaded; the old entry is soft-deleted. Peers then pull the deleted + // old-hash row (remove old) + the uploaded new-hash row (download new). + it('clears uploadedAt on a metaHash re-import so the new file re-uploads', async () => { + const metaHash = getMetadataHash(TEST_METADATA); + const existingBook = makeBook({ + hash: 'old-hash-123', + metaHash, + uploadedAt: Date.now() - 5000, + }); + const books: Book[] = [existingBook]; + + mockPartialMD5.mockResolvedValue('new-hash-456'); + setupMockBookDoc(); + + const mockFile = new File(['new content'], 'test.epub', { type: 'application/epub+zip' }); + const result = await service.importBook(mockFile, books); + + expect(result).toBe(existingBook); + expect(existingBook.hash).toBe('new-hash-456'); + // uploadedAt cleared → autoUpload / manual upload re-pushes the new file. + expect(existingBook.uploadedAt).toBeNull(); + }); + it('should not match metaHash for deleted books', async () => { const metaHash = getMetadataHash(TEST_METADATA); diff --git a/apps/readest-app/src/__tests__/utils/transform.test.ts b/apps/readest-app/src/__tests__/utils/transform.test.ts index dd2679ad..3f94c5a7 100644 --- a/apps/readest-app/src/__tests__/utils/transform.test.ts +++ b/apps/readest-app/src/__tests__/utils/transform.test.ts @@ -339,3 +339,58 @@ describe('transformBook readingStatus + readingStatusUpdatedAt', () => { expect(back.readingStatusUpdatedAt).toBeUndefined(); }); }); + +describe('transformBook coverHash + coverUpdatedAt (issue #4544)', () => { + const userId = 'user-1'; + const baseBook: Book = { + hash: 'h1', + format: 'EPUB', + title: 'T', + author: 'A', + createdAt: 1, + updatedAt: 2, + }; + + it('serializes coverHash verbatim and coverUpdatedAt to ISO', () => { + const ts = Date.UTC(2026, 5, 22, 12, 0, 0); + const db = transformBookToDB( + { ...baseBook, coverHash: 'abcdef0123456789', coverUpdatedAt: ts }, + userId, + ); + expect(db.cover_hash).toBe('abcdef0123456789'); + expect(db.cover_updated_at).toBe(new Date(ts).toISOString()); + }); + + it('leaves cover columns null when unset', () => { + const db = transformBookToDB(baseBook, userId); + expect(db.cover_hash).toBeNull(); + expect(db.cover_updated_at).toBeNull(); + }); + + it('round-trips coverHash + coverUpdatedAt back to the client shape', () => { + const ts = Date.UTC(2026, 5, 22, 12, 0, 0); + const db = transformBookToDB( + { ...baseBook, coverHash: 'deadbeef', coverUpdatedAt: ts }, + userId, + ); + const back = transformBookFromDB(db); + expect(back.coverHash).toBe('deadbeef'); + expect(back.coverUpdatedAt).toBe(ts); + }); + + it('reads null cover fields when the DB columns are null', () => { + const back = transformBookFromDB({ + user_id: userId, + book_hash: 'h1', + format: 'EPUB', + title: 'T', + author: 'A', + cover_hash: null, + cover_updated_at: null, + created_at: new Date(1).toISOString(), + updated_at: new Date(2).toISOString(), + }); + expect(back.coverHash).toBeNull(); + expect(back.coverUpdatedAt).toBeNull(); + }); +}); diff --git a/apps/readest-app/src/app/library/hooks/useBooksSync.ts b/apps/readest-app/src/app/library/hooks/useBooksSync.ts index 7fce895d..fb909142 100644 --- a/apps/readest-app/src/app/library/hooks/useBooksSync.ts +++ b/apps/readest-app/src/app/library/hooks/useBooksSync.ts @@ -9,7 +9,11 @@ import { SYNC_BOOKS_INTERVAL_SEC } from '@/services/constants'; import { throttle } from '@/utils/throttle'; import { debounce } from '@/utils/debounce'; import { eventDispatcher } from '@/utils/event'; -import { pickFresherReadingStatus } from '@/app/library/utils/libraryUtils'; +import { + pickFresherReadingStatus, + needsCoverRefresh, + pickFresherCover, +} from '@/app/library/utils/libraryUtils'; export const useBooksSync = () => { const _ = useTranslation(); @@ -114,16 +118,23 @@ export const useBooksSync = () => { // Process old books first so that when we update the library the order is preserved syncedBooks.sort((a, b) => a.updatedAt - b.updatedAt); const bookHashesInSynced = new Set(syncedBooks.map((book) => book.hash)); + const syncedByHash = new Map(syncedBooks.map((book) => [book.hash, book])); const liveLibrary = useLibraryStore.getState().library; const oldBooks = liveLibrary.filter((book) => bookHashesInSynced.has(book.hash)); + // Books whose cover must be (re)fetched: never-downloaded, or a newer cover + // edit arrived from another device (issue #4544). Captured before the + // download loop so the post-download merge can still tell which covers were + // refreshed (the loop mutates coverDownloadedAt). const oldBooksNeedsDownload = oldBooks.filter((book) => { - return !book.deletedAt && book.uploadedAt && !book.coverDownloadedAt; + const matchingBook = syncedByHash.get(book.hash); + return !!matchingBook && needsCoverRefresh(book, matchingBook); }); + const coverRefreshHashes = new Set(oldBooksNeedsDownload.map((book) => book.hash)); const processOldBook = async (oldBook: Book) => { - const matchingBook = syncedBooks.find((newBook) => newBook.hash === oldBook.hash); + const matchingBook = syncedByHash.get(oldBook.hash); if (matchingBook) { - if (!matchingBook.deletedAt && matchingBook.uploadedAt && !oldBook.coverDownloadedAt) { + if (coverRefreshHashes.has(oldBook.hash)) { oldBook.coverImageUrl = await appService?.generateCoverImageUrl(oldBook); } const mergedBook = @@ -135,6 +146,11 @@ export const useBooksSync = () => { const status = pickFresherReadingStatus(oldBook, matchingBook); mergedBook.readingStatus = status.readingStatus; mergedBook.readingStatusUpdatedAt = status.readingStatusUpdatedAt; + // Cover is likewise resolved by its own coverUpdatedAt, independent of + // the row's updatedAt — issue #4544. + const cover = pickFresherCover(oldBook, matchingBook); + mergedBook.coverHash = cover.coverHash; + mergedBook.coverUpdatedAt = cover.coverUpdatedAt; return mergedBook; } return oldBook; diff --git a/apps/readest-app/src/app/library/page.tsx b/apps/readest-app/src/app/library/page.tsx index feef5c56..8cda2c67 100644 --- a/apps/readest-app/src/app/library/page.tsx +++ b/apps/readest-app/src/app/library/page.tsx @@ -944,6 +944,32 @@ const LibraryPageContent = ({ searchParams }: { searchParams: ReadonlyURLSearchP metadata.coverImageBlobUrl || metadata.coverImageUrl, metadata.coverImageFile, ); + // Cover-change sync (issue #4544): recompute the cover's content hash. + // If it actually changed, bump coverHash + coverUpdatedAt so peers + // re-download it (the book row already syncs via updatedAt). + // computeCoverHash returns null for a '_blank' deletion — we skip the + // bump there (cover deletion is intentionally not synced; peers keep + // their cover until a new one is set). + const newCoverHash = (await appService?.computeCoverHash(updatedBook)) ?? null; + if (newCoverHash && newCoverHash !== book.coverHash) { + // For a book already in the cloud, re-upload the cover FIRST and only + // advertise the new version if it succeeded — otherwise peers would + // try to fetch a cover that isn't there. A not-yet-uploaded book + // carries the new cover on its first full upload, so the bump is safe. + let coverUploaded = true; + if (user && updatedBook.uploadedAt) { + try { + await appService?.uploadBookCover(updatedBook); + } catch (uploadError) { + console.warn('Failed to upload updated cover:', uploadError); + coverUploaded = false; + } + } + if (coverUploaded) { + updatedBook.coverHash = newCoverHash; + updatedBook.coverUpdatedAt = Date.now(); + } + } } catch (error) { console.warn('Failed to update cover image:', error); } diff --git a/apps/readest-app/src/app/library/utils/libraryUtils.ts b/apps/readest-app/src/app/library/utils/libraryUtils.ts index 3b78d1f2..1d24868c 100644 --- a/apps/readest-app/src/app/library/utils/libraryUtils.ts +++ b/apps/readest-app/src/app/library/utils/libraryUtils.ts @@ -638,6 +638,47 @@ export const pickFresherReadingStatus = ( }; }; +type CoverFields = Pick; +type CoverSyncFields = Pick< + Book, + 'coverHash' | 'coverUpdatedAt' | 'coverDownloadedAt' | 'deletedAt' | 'uploadedAt' +>; + +const coverMs = (t?: number | null) => t ?? 0; + +/** + * Decide whether a peer should (re)download a book's cover from the cloud + * (issue #4544). True when the synced book is in the cloud AND either: + * - this device has never fetched the cover (first download), or + * - a newer cover edit exists (synced `coverUpdatedAt` strictly newer) whose + * content hash differs from the local one. + * + * Gating on `coverUpdatedAt` (not just the hash) prevents two failure modes: + * - churn: once a device adopts the synced `coverUpdatedAt` after downloading, + * the comparison stops firing on every subsequent sync; + * - the unpushed-local-edit race: a device that just edited its cover (newer + * local `coverUpdatedAt`) is not made to overwrite it with the stale cloud + * copy before its own push lands. + */ +export const needsCoverRefresh = (local: CoverSyncFields, synced: CoverSyncFields): boolean => { + if (synced.deletedAt || !synced.uploadedAt) return false; + if (!local.coverDownloadedAt) return true; // first download + if (!synced.coverHash) return false; // nothing to compare (legacy book) + if (coverMs(synced.coverUpdatedAt) <= coverMs(local.coverUpdatedAt)) return false; + return synced.coverHash !== local.coverHash; +}; + +/** + * Field-level last-writer-wins for the cover, by `coverUpdatedAt` (ties → + * `local`, which already holds the file). Mirrors {@link pickFresherReadingStatus}: + * the row's `updatedAt` is dominated by page-turn progress, so the cover must be + * resolved by its own timestamp or progress would clobber a cover edit. + */ +export const pickFresherCover = (local: CoverFields, synced: CoverFields): CoverFields => + coverMs(synced.coverUpdatedAt) > coverMs(local.coverUpdatedAt) + ? { coverHash: synced.coverHash, coverUpdatedAt: synced.coverUpdatedAt } + : { coverHash: local.coverHash, coverUpdatedAt: local.coverUpdatedAt }; + /** * Resolve the ordered list of context-menu item ids for a book from its state. * diff --git a/apps/readest-app/src/pages/api/sync.ts b/apps/readest-app/src/pages/api/sync.ts index 7a1fbb11..82c63d61 100644 --- a/apps/readest-app/src/pages/api/sync.ts +++ b/apps/readest-app/src/pages/api/sync.ts @@ -95,6 +95,24 @@ export function buildStatusPropagationRow( }; } +/** + * Field-level last-writer-wins for a books row's cover: return the + * {cover_hash, cover_updated_at} with the newer cover_updated_at (ties → + * client). NULL timestamp = epoch 0. A cover edit shares the row with + * page-turn progress, so this lets the cover survive even when the whole row + * is decided the other way by updated_at — the same #4634 hazard the + * reading_status merge addresses (issue #4544). + */ +export function resolveCoverMerge( + client: Pick, + server: Pick, +): Pick { + const ms = (s?: string | null) => (s ? new Date(s).getTime() : 0); + return ms(client.cover_updated_at) >= ms(server.cover_updated_at) + ? { cover_hash: client.cover_hash, cover_updated_at: client.cover_updated_at } + : { cover_hash: server.cover_hash, cover_updated_at: server.cover_updated_at }; +} + const transformsToDB = { books: transformBookToDB, book_notes: transformBookNoteToDB, @@ -450,34 +468,52 @@ export async function POST(req: NextRequest) { if (table === 'books') { // `dbRec` is DBBook | DBBookConfig; in the 'books' branch it is always DBBook. const clientBook = dbRec as DBBook; - // `serverData` is BookDataRecord but the DB row carries the status columns at - // runtime — widen the type without going through `unknown`. + // `serverData` is BookDataRecord but the DB row carries the status + + // cover columns at runtime — widen the type without going through `unknown`. const serverBook = serverData as BookDataRecord & - Partial>; + Partial< + Pick< + DBBook, + 'reading_status' | 'reading_status_updated_at' | 'cover_hash' | 'cover_updated_at' + > + >; const status = resolveReadingStatusMerge(clientBook, serverBook); + // Cover has its own field-level LWW so a page-turn can't clobber a + // cover edit (issue #4544; mirrors reading_status / #4634). + const cover = resolveCoverMerge(clientBook, serverBook); if (clientIsNewer) { - // Client wins the row; graft the fresher status onto it (server's - // status may be the newer one even though the row is older). + // Client wins the row; graft the fresher status + cover onto it + // (server's may be the newer one even though the row is older). clientBook.reading_status = status.reading_status; clientBook.reading_status_updated_at = status.reading_status_updated_at; + clientBook.cover_hash = cover.cover_hash; + clientBook.cover_updated_at = cover.cover_updated_at; toUpdate.push(clientBook); } else { - // Only rewrite when the resolved status VALUE differs from the + // Only rewrite when a resolved field VALUE differs from the // server's — a timestamp-only difference on the same value is a // no-op, and rewriting it would churn updated_at + re-propagate. const statusChanged = readingStatusChanged( status.reading_status, serverBook.reading_status, ); - if (statusChanged) { - // Server wins the row, but the client's status is newer. Write - // server's row with the fresher status; the books_set_synced_at - // trigger advances synced_at so peers re-pull the change, while - // updated_at (the date-read sort key) stays put. See #4678. + const coverChanged = (cover.cover_hash ?? null) !== (serverBook.cover_hash ?? null); + if (statusChanged || coverChanged) { + // Server wins the row, but the client's status and/or cover is + // the fresher one. Graft the fresher fields onto the server row + // and leave updated_at untouched; the books_set_synced_at + // trigger advances synced_at so peers re-pull via the synced_at + // cursor without reordering the date-read library (#4678, #4544). // The runtime DB row carries all DBBook columns; the static type // of `serverBook` is a narrower intersection so `unknown` is // required to bridge the gap at this one construction site. - toUpdate.push(buildStatusPropagationRow(serverBook as unknown as DBBook, status)); + const propagated = buildStatusPropagationRow( + serverBook as unknown as DBBook, + status, + ); + propagated.cover_hash = cover.cover_hash; + propagated.cover_updated_at = cover.cover_updated_at; + toUpdate.push(propagated); } else { batchAuthoritativeRecords.push(serverData); } diff --git a/apps/readest-app/src/services/appService.ts b/apps/readest-app/src/services/appService.ts index 2834ff76..be2a4993 100644 --- a/apps/readest-app/src/services/appService.ts +++ b/apps/readest-app/src/services/appService.ts @@ -238,6 +238,10 @@ export abstract class BaseAppService implements AppService { return BookSvc.updateCoverImage(this.coverCtx, book, imageUrl, imageFile); } + async computeCoverHash(book: Book): Promise { + return BookSvc.computeCoverHash(this.fs, book); + } + async importFont(file?: string | File): Promise { return FontSvc.importFont(this.fs, file); } @@ -354,6 +358,10 @@ export abstract class BaseAppService implements AppService { return CloudSvc.uploadBook(this.fs, this.resolveFilePath.bind(this), book, onProgress); } + async uploadBookCover(book: Book, onProgress?: ProgressHandler): Promise { + return CloudSvc.uploadBookCover(this.fs, this.resolveFilePath.bind(this), book, onProgress); + } + async downloadCloudFile(lfp: string, cfp: string, onProgress: ProgressHandler) { return CloudSvc.downloadCloudFile(this, this.localBooksDir, lfp, cfp, onProgress); } diff --git a/apps/readest-app/src/services/bookService.ts b/apps/readest-app/src/services/bookService.ts index 019138e0..0a10b520 100644 --- a/apps/readest-app/src/services/bookService.ts +++ b/apps/readest-app/src/services/bookService.ts @@ -169,6 +169,20 @@ export async function updateCoverImage( } } +/** + * Partial MD5 of the local cover.png, or null when the cover is absent. This is + * the content-addressed cover-change signal for cross-device sync (issue + * #4544): keeping `book.coverHash === computeCoverHash(book)` lets a peer + * re-download the cover iff the synced hash differs from the local one. An + * identical (re-extracted / re-imported) cover yields the same hash, so there + * is no re-sync churn. + */ +export async function computeCoverHash(fs: FileSystem, book: Book): Promise { + if (!(await fs.exists(getCoverFilename(book), 'Books'))) return null; + const coverFile = await fs.openFile(getCoverFilename(book), 'Books'); + return partialMD5(coverFile); +} + // --- Book Merge --- /** @@ -510,6 +524,12 @@ export async function importBook( await fs.writeFile(getCoverFilename(book), 'Books', coverBytes); } } + // Maintain coverHash === partialMD5(cover.png) so cross-device cover sync + // can detect changes (issue #4544). Read from disk regardless of whether we + // just wrote it — a hash-match reimport may reuse an existing cover. + const coverHash = await computeCoverHash(fs, book); + book.coverHash = coverHash; + if (existingBook) existingBook.coverHash = coverHash; // Never overwrite the config file only when it's not existed if (!existingBook) { await saveBookConfigFn(book, INIT_BOOK_CONFIG); diff --git a/apps/readest-app/src/services/cloudService.ts b/apps/readest-app/src/services/cloudService.ts index 6791a23b..69bc7dbf 100644 --- a/apps/readest-app/src/services/cloudService.ts +++ b/apps/readest-app/src/services/cloudService.ts @@ -218,6 +218,25 @@ export async function uploadBook( book.coverDownloadedAt = Date.now(); } +// Re-upload only the cover (books//cover.png), overwriting the cloud +// copy. Used after a cover edit (issue #4544) so peers can re-download it. +// Deliberately does NOT touch book.uploadedAt — that marker means "the book +// file is in cloud as of T"; a cover-only change must not trigger a file +// re-download on peers. +export async function uploadBookCover( + fs: FileSystem, + resolveFilePath: (path: string, base: BaseDir) => Promise, + book: Book, + onProgress?: ProgressHandler, +): Promise { + if (!(await fs.exists(getCoverFilename(book), 'Books'))) return; + const completedFiles = { count: 0 }; + const handleProgress = createProgressHandler(1, completedFiles, onProgress); + const lfp = getCoverFilename(book); + const cfp = `${CLOUD_BOOKS_SUBDIR}/${getCoverFilename(book)}`; + await uploadFileToCloud(fs, resolveFilePath, lfp, cfp, 'Books', handleProgress, book.hash); +} + export async function downloadCloudFile( appService: AppService, localBooksDir: string, diff --git a/apps/readest-app/src/types/book.ts b/apps/readest-app/src/types/book.ts index 40a16725..100add06 100644 --- a/apps/readest-app/src/types/book.ts +++ b/apps/readest-app/src/types/book.ts @@ -99,6 +99,10 @@ export interface Book { groupName?: string; tags?: string[]; coverImageUrl?: string | null; + // Partial MD5 of the local cover.png. Content-addressed cover-change signal: + // a peer re-downloads the cover iff its synced value differs from the local + // one (issue #4544). Invariant: coverHash === partialMD5(cover.png). + coverHash?: string | null; createdAt: number; updatedAt: number; @@ -107,6 +111,9 @@ export interface Book { uploadedAt?: number | null; downloadedAt?: number | null; coverDownloadedAt?: number | null; + // Field-level LWW timestamp for the cover, so a page-turn that wins whole-row + // LWW on updatedAt cannot clobber a cover edit (mirrors readingStatusUpdatedAt). + coverUpdatedAt?: number | null; syncedAt?: number | null; lastUpdated?: number; // deprecated in favor of updatedAt diff --git a/apps/readest-app/src/types/records.ts b/apps/readest-app/src/types/records.ts index 96ed1dc0..497bd8e9 100644 --- a/apps/readest-app/src/types/records.ts +++ b/apps/readest-app/src/types/records.ts @@ -12,6 +12,8 @@ export interface DBBook { progress?: [number, number]; reading_status?: string; reading_status_updated_at?: string | null; + cover_hash?: string | null; + cover_updated_at?: string | null; metadata?: string | null; created_at?: string; diff --git a/apps/readest-app/src/types/system.ts b/apps/readest-app/src/types/system.ts index 8ef76be6..22506d15 100644 --- a/apps/readest-app/src/types/system.ts +++ b/apps/readest-app/src/types/system.ts @@ -175,6 +175,7 @@ export interface AppService { refreshBookMetadata(book: Book): Promise; deleteBook(book: Book, deleteAction: DeleteAction): Promise; uploadBook(book: Book, onProgress?: ProgressHandler): Promise; + uploadBookCover(book: Book, onProgress?: ProgressHandler): Promise; downloadBook( book: Book, onlyCover?: boolean, @@ -223,6 +224,7 @@ export interface AppService { getCoverImageBlobUrl(book: Book): Promise; generateCoverImageUrl(book: Book): Promise; updateCoverImage(book: Book, imageUrl?: string, imageFile?: string): Promise; + computeCoverHash(book: Book): Promise; ask(message: string): Promise; openDatabase( schema: SchemaType, diff --git a/apps/readest-app/src/utils/transform.ts b/apps/readest-app/src/utils/transform.ts index 3436fb7b..3a55aa04 100644 --- a/apps/readest-app/src/utils/transform.ts +++ b/apps/readest-app/src/utils/transform.ts @@ -77,6 +77,8 @@ export const transformBookToDB = (book: unknown, userId: string): DBBook => { progress, readingStatus, readingStatusUpdatedAt, + coverHash, + coverUpdatedAt, metadata, createdAt, updatedAt, @@ -99,6 +101,8 @@ export const transformBookToDB = (book: unknown, userId: string): DBBook => { reading_status_updated_at: readingStatusUpdatedAt ? new Date(readingStatusUpdatedAt).toISOString() : null, + cover_hash: coverHash ?? null, + cover_updated_at: coverUpdatedAt ? new Date(coverUpdatedAt).toISOString() : null, source_title: sanitizeString(sourceTitle), metadata: metadata ? sanitizeString(JSON.stringify(metadata)) : null, created_at: new Date(createdAt ?? Date.now()).toISOString(), @@ -121,6 +125,8 @@ export const transformBookFromDB = (dbBook: DBBook): Book => { progress, reading_status, reading_status_updated_at, + cover_hash, + cover_updated_at, source_title, metadata, created_at, @@ -143,6 +149,8 @@ export const transformBookFromDB = (dbBook: DBBook): Book => { readingStatusUpdatedAt: reading_status_updated_at ? new Date(reading_status_updated_at).getTime() : undefined, + coverHash: cover_hash ?? null, + coverUpdatedAt: cover_updated_at ? new Date(cover_updated_at).getTime() : null, sourceTitle: source_title, metadata: metadata ? JSON.parse(metadata) : null, createdAt: new Date(created_at!).getTime(), diff --git a/docker/volumes/db/init/schema.sql b/docker/volumes/db/init/schema.sql index 81613496..4fc4e42f 100644 --- a/docker/volumes/db/init/schema.sql +++ b/docker/volumes/db/init/schema.sql @@ -19,6 +19,8 @@ CREATE TABLE public.books ( progress integer[] NULL, reading_status text NULL, reading_status_updated_at timestamp with time zone NULL, + cover_hash text NULL, + cover_updated_at timestamp with time zone NULL, group_id text NULL, group_name text NULL, metadata json NULL, diff --git a/docker/volumes/db/migrations/017_add_book_cover_version.sql b/docker/volumes/db/migrations/017_add_book_cover_version.sql new file mode 100644 index 00000000..fc1ae2af --- /dev/null +++ b/docker/volumes/db/migrations/017_add_book_cover_version.sql @@ -0,0 +1,23 @@ +-- Migration 017: Add `cover_hash` / `cover_updated_at` to books +-- +-- Cover-change sync (issue #4544). Editing a book's cover writes cover.png +-- locally but changes no hash (the cover is keyed by the file hash), so peers +-- had no signal to re-download it. Give the cover its own content-addressed +-- version: +-- +-- cover_hash = partial MD5 of cover.png. A peer re-downloads the cover +-- iff its synced cover_hash differs from the local one. +-- Content-addressed ⇒ a byte-identical (re-extracted / +-- re-imported) cover yields the same hash ⇒ no re-sync +-- churn (compatible with the metaHash dedupe mechanism). +-- cover_updated_at = field-level last-writer-wins timestamp so a page-turn +-- that wins whole-row LWW on updated_at cannot clobber a +-- cover edit — the same hazard the 015 +-- reading_status_updated_at fix addressed for #4634. +-- +-- Both additive + nullable; NULL cover_updated_at is treated as epoch 0 +-- (oldest) by the merge. Old clients ignore the columns, so they never break. + +ALTER TABLE public.books + ADD COLUMN IF NOT EXISTS cover_hash text NULL, + ADD COLUMN IF NOT EXISTS cover_updated_at timestamp with time zone NULL;