* 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) <noreply@anthropic.com> * 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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/<hash>/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/<newhash>/` 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/<hash>/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<string | null>
|
||||
```
|
||||
|
||||
## 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/<hash>/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/<oldhash>/`
|
||||
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`
|
||||
@@ -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>): Book => ({ ...base, ...over });
|
||||
const synced = (over: Partial<Book>): 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 });
|
||||
});
|
||||
});
|
||||
@@ -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 });
|
||||
});
|
||||
});
|
||||
@@ -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> = {}): 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> = {}): 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/<hash>/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();
|
||||
});
|
||||
});
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -638,6 +638,47 @@ export const pickFresherReadingStatus = (
|
||||
};
|
||||
};
|
||||
|
||||
type CoverFields = Pick<Book, 'coverHash' | 'coverUpdatedAt'>;
|
||||
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.
|
||||
*
|
||||
|
||||
@@ -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<DBBook, 'cover_hash' | 'cover_updated_at'>,
|
||||
server: Pick<DBBook, 'cover_hash' | 'cover_updated_at'>,
|
||||
): Pick<DBBook, 'cover_hash' | 'cover_updated_at'> {
|
||||
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<Pick<DBBook, 'reading_status' | 'reading_status_updated_at'>>;
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -238,6 +238,10 @@ export abstract class BaseAppService implements AppService {
|
||||
return BookSvc.updateCoverImage(this.coverCtx, book, imageUrl, imageFile);
|
||||
}
|
||||
|
||||
async computeCoverHash(book: Book): Promise<string | null> {
|
||||
return BookSvc.computeCoverHash(this.fs, book);
|
||||
}
|
||||
|
||||
async importFont(file?: string | File): Promise<CustomFontInfo | null> {
|
||||
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<void> {
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -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<string | null> {
|
||||
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);
|
||||
|
||||
@@ -218,6 +218,25 @@ export async function uploadBook(
|
||||
book.coverDownloadedAt = Date.now();
|
||||
}
|
||||
|
||||
// Re-upload only the cover (books/<hash>/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<string>,
|
||||
book: Book,
|
||||
onProgress?: ProgressHandler,
|
||||
): Promise<void> {
|
||||
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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -175,6 +175,7 @@ export interface AppService {
|
||||
refreshBookMetadata(book: Book): Promise<boolean>;
|
||||
deleteBook(book: Book, deleteAction: DeleteAction): Promise<void>;
|
||||
uploadBook(book: Book, onProgress?: ProgressHandler): Promise<void>;
|
||||
uploadBookCover(book: Book, onProgress?: ProgressHandler): Promise<void>;
|
||||
downloadBook(
|
||||
book: Book,
|
||||
onlyCover?: boolean,
|
||||
@@ -223,6 +224,7 @@ export interface AppService {
|
||||
getCoverImageBlobUrl(book: Book): Promise<string>;
|
||||
generateCoverImageUrl(book: Book): Promise<string>;
|
||||
updateCoverImage(book: Book, imageUrl?: string, imageFile?: string): Promise<void>;
|
||||
computeCoverHash(book: Book): Promise<string | null>;
|
||||
ask(message: string): Promise<boolean>;
|
||||
openDatabase(
|
||||
schema: SchemaType,
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
Reference in New Issue
Block a user