fix(webdav): always sync book covers, not just when syncBooks is on (#4445)
Cover uploads were nested inside the `syncBooks` toggle in both the
batch path (`syncLibrary`) and the per-book reader path
(`pushBookFileNow`). With the default `syncBooks: false`, covers
silently never reached the WebDAV server even though `config.json`
did, so receiving devices ended up with progress + notes synced but
no shelf art.
Covers are conceptually metadata, not bytes:
- they're tiny (~30–60 KB after the import-time downscale);
- they cannot be regenerated on a fresh device that doesn't hold
the book bytes (custom covers from metadata services in
particular are completely unrecoverable without sync);
- cloudService already treats them as metadata-grade in its
download path (`downloadBookCovers`, `downloadBook(onlyCover)`).
Two changes:
1. `WebDAVSync.ts::syncLibrary` — moved `pushBookCover` out of the
`if (options.syncBooks)` block; it now runs alongside
`pushBookConfig`, before `pushBookFile`. Step ordering in the
header doc-comment was updated to match.
2. `useWebDAVSync.ts` — extracted a standalone `pushBookCoverNow`
callback (gated only on `allowPush`, with its own
`coverSyncedRef` for per-instance dedupe), and dropped the cover
ride-along that lived at the tail of `pushBookFileNow`. The
open-book effect now fires `pushBookCoverNow` and
`pushBookFileNow` in parallel via `Promise.all` (different remote
paths, no reason to serialize), and the manual-push event handler
triggers both independently.
The WebDAV pull path was already independent of `syncBooks`, so no
changes are needed there — receiving devices will pick up the newly
mirrored covers automatically.
This commit is contained in:
@@ -96,6 +96,14 @@ export const useWebDAVSync = (bookKey: string) => {
|
||||
* a metadata-only operation handled elsewhere.
|
||||
*/
|
||||
const fileSyncedRef = useRef(false);
|
||||
/**
|
||||
* Per-instance lock for the cover uploader. Same shape as
|
||||
* `fileSyncedRef` but tracked separately because covers are gated
|
||||
* differently than book files: cover sync runs whenever we're allowed
|
||||
* to push at all (independent of `syncBooks`), so they need their own
|
||||
* "already done in this hook lifetime" bit.
|
||||
*/
|
||||
const coverSyncedRef = useRef(false);
|
||||
|
||||
// The deviceId is generated lazily on first push so users who never
|
||||
// enable WebDAV don't carry it around.
|
||||
@@ -273,20 +281,6 @@ export const useWebDAVSync = (bookKey: string) => {
|
||||
if (result.uploaded) {
|
||||
await updateLastSyncedAt(Date.now());
|
||||
}
|
||||
// Cover ride-along — best-effort, failures don't unwind the
|
||||
// syncedRef lock or surface a toast. Same HEAD short-circuit as
|
||||
// the book file, so steady-state cost is one HEAD per session.
|
||||
try {
|
||||
await pushBookCover(settings.webdav!, book.hash, async () => {
|
||||
const fp = getCoverFilename(book);
|
||||
if (!(await appService.exists(fp, 'Books'))) return null;
|
||||
const file = await appService.openFile(fp, 'Books');
|
||||
const bytes = await file.arrayBuffer();
|
||||
return { bytes, size: bytes.byteLength };
|
||||
});
|
||||
} catch (e) {
|
||||
console.warn('WD book cover push failed', e);
|
||||
}
|
||||
} catch (e) {
|
||||
// Reset the lock on failure so a manual Sync now or a subsequent
|
||||
// open retries — otherwise a transient hiccup would mark this
|
||||
@@ -303,6 +297,50 @@ export const useWebDAVSync = (bookKey: string) => {
|
||||
}
|
||||
}, [allowPush, settings.webdav, getBookData, bookKey, appService, updateLastSyncedAt, _]);
|
||||
|
||||
/**
|
||||
* Push the local cover image to the remote, independent of
|
||||
* `syncBooks`. Covers are part of the book's metadata: at ~30–60 KB
|
||||
* each (after the import-time downscale) the bandwidth cost is
|
||||
* negligible, but the receiving device cannot regenerate them when
|
||||
* `syncBooks=false` (it has no book bytes to extract from), so a
|
||||
* user who only opts into progress + notes still needs the covers
|
||||
* ride-along to see proper bookshelf art.
|
||||
*
|
||||
* Failures are best-effort warnings: a missing local cover (TXT/MD
|
||||
* imports without metadata, etc.) silently no-ops, and a network
|
||||
* blip is logged but doesn't surface a toast.
|
||||
*/
|
||||
const pushBookCoverNow = useCallback(async () => {
|
||||
if (!allowPush) return;
|
||||
if (coverSyncedRef.current) return;
|
||||
coverSyncedRef.current = true;
|
||||
|
||||
const book = getBookData(bookKey)?.book;
|
||||
if (!book || !appService) return;
|
||||
|
||||
try {
|
||||
await pushBookCover(settings.webdav!, book.hash, async () => {
|
||||
const fp = getCoverFilename(book);
|
||||
if (!(await appService.exists(fp, 'Books'))) return null;
|
||||
const file = await appService.openFile(fp, 'Books');
|
||||
const bytes = await file.arrayBuffer();
|
||||
return { bytes, size: bytes.byteLength };
|
||||
});
|
||||
} catch (e) {
|
||||
// Reset the lock so a manual "Sync now" or a subsequent open
|
||||
// can retry, mirroring `pushBookFileNow`'s recovery model.
|
||||
coverSyncedRef.current = false;
|
||||
if (e instanceof WebDAVRequestError && e.code === 'AUTH_FAILED') {
|
||||
eventDispatcher.dispatch('toast', {
|
||||
type: 'error',
|
||||
message: _('WebDAV authentication failed. Reconnect in Settings.'),
|
||||
});
|
||||
} else {
|
||||
console.warn('WD book cover push failed', e);
|
||||
}
|
||||
}
|
||||
}, [allowPush, settings.webdav, getBookData, bookKey, appService, _]);
|
||||
|
||||
/**
|
||||
* Pull, merge, and persist. Uses the same per-config / per-note merge
|
||||
* semantics as the native cloud sync so a user running both feels the
|
||||
@@ -398,10 +436,10 @@ export const useWebDAVSync = (bookKey: string) => {
|
||||
// Stash the latest pull/push callbacks in a ref so the event-bridge
|
||||
// useEffect below doesn't have to re-bind on every render. Pattern
|
||||
// taken from useKOSync.
|
||||
const syncRefs = useRef({ pushNow, pullNow, pushBookFileNow });
|
||||
const syncRefs = useRef({ pushNow, pullNow, pushBookFileNow, pushBookCoverNow });
|
||||
useEffect(() => {
|
||||
syncRefs.current = { pushNow, pullNow, pushBookFileNow };
|
||||
}, [pushNow, pullNow, pushBookFileNow]);
|
||||
syncRefs.current = { pushNow, pullNow, pushBookFileNow, pushBookCoverNow };
|
||||
}, [pushNow, pullNow, pushBookFileNow, pushBookCoverNow]);
|
||||
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
const debouncedPush = useCallback(
|
||||
@@ -450,10 +488,13 @@ export const useWebDAVSync = (bookKey: string) => {
|
||||
dirtyRef.current = true;
|
||||
await syncRefs.current.pushNow();
|
||||
}
|
||||
// Run the file-binary upload last so the lighter config sync is
|
||||
// already mirrored before we start moving megabytes. The HEAD probe
|
||||
// inside makes this near-free for already-synced books.
|
||||
await syncRefs.current.pushBookFileNow();
|
||||
// Cover sync is independent of `syncBooks`: even users who keep
|
||||
// book bytes off the wire still want their shelf art mirrored.
|
||||
// Run it in parallel with the file upload — they hit different
|
||||
// remote paths and the cover is tiny, so there's no reason to
|
||||
// serialize them. The HEAD probe inside each makes the steady
|
||||
// state near-free for already-mirrored books.
|
||||
await Promise.all([syncRefs.current.pushBookCoverNow(), syncRefs.current.pushBookFileNow()]);
|
||||
})();
|
||||
}, [isReady, progress?.location]);
|
||||
|
||||
@@ -497,12 +538,16 @@ export const useWebDAVSync = (bookKey: string) => {
|
||||
const handlePush = (event: CustomEvent) => {
|
||||
if (event.detail?.bookKey && event.detail.bookKey !== bookKey) return;
|
||||
// User-triggered push is unconditional — flip dirty so the flush
|
||||
// actually does something, and re-run the book-file upload check
|
||||
// so a freshly-toggled "Sync Book Files" picks up the binary.
|
||||
// actually does something, and re-run the book-file + cover
|
||||
// upload checks so a freshly-toggled "Sync Book Files" picks up
|
||||
// the binary, and any cover that wasn't on the wire yet (e.g.
|
||||
// hit a transient failure earlier in the session) gets retried.
|
||||
dirtyRef.current = true;
|
||||
fileSyncedRef.current = false;
|
||||
coverSyncedRef.current = false;
|
||||
debouncedPush.flush();
|
||||
syncRefs.current.pushBookFileNow();
|
||||
syncRefs.current.pushBookCoverNow();
|
||||
};
|
||||
const handlePull = (event: CustomEvent) => {
|
||||
if (event.detail?.bookKey && event.detail.bookKey !== bookKey) return;
|
||||
|
||||
@@ -660,12 +660,16 @@ export interface SyncLibraryOptions {
|
||||
* brand-new entries the user has never opened don't need to sync
|
||||
* anything yet).
|
||||
* 2. `pushBookConfig` — creates `Readest/books/<hash>/config.json`.
|
||||
* 3. `pushBookFile` (only when `syncBooks` is on) — HEAD-probes the
|
||||
* 3. `pushBookCover` (whenever a cover loader was provided) —
|
||||
* HEAD-then-PUT, independent of `syncBooks`. Covers are part of
|
||||
* the book's metadata: they're tiny (~30–60 KB after the import
|
||||
* downscale), they can't be regenerated on a fresh device that
|
||||
* doesn't hold the book bytes, and users who only opt into
|
||||
* progress / notes sync still expect their bookshelf art to
|
||||
* appear on the receiving device. Failures are treated as
|
||||
* warnings, not hard failures.
|
||||
* 4. `pushBookFile` (only when `syncBooks` is on) — HEAD-probes the
|
||||
* friendly file name path, uploads if missing or size-mismatched.
|
||||
* 4. `pushBookCover` (only when `syncBooks` is on AND a cover loader
|
||||
* was provided) — same HEAD-then-PUT pattern. Cover failures are
|
||||
* treated as warnings, not failures, since they don't break the
|
||||
* reading experience on the receiving device.
|
||||
*
|
||||
* Failures on a single book are caught and counted; we keep going so a
|
||||
* single bad apple doesn't abort the rest of the library. The aggregate
|
||||
@@ -938,6 +942,25 @@ export const syncLibrary = async (
|
||||
await pushBookConfig(settings, book, config, options.deviceId);
|
||||
result.configsUploaded += 1;
|
||||
}
|
||||
// Covers ride along with the config-level sync, NOT with
|
||||
// syncBooks. They are conceptually part of the book's metadata
|
||||
// (the receiving device can't regenerate them without the book
|
||||
// bytes, which the user may have chosen not to sync), and at
|
||||
// ~30–60 KB after the import-time downscale the bandwidth cost
|
||||
// is negligible compared to the user benefit of "my shelf art
|
||||
// shows up on every device". Failures here are warnings, not
|
||||
// hard failures, so a flaky cover upload doesn't abort the
|
||||
// rest of the per-book pipeline.
|
||||
if (options.loadBookCover) {
|
||||
try {
|
||||
const coverResult = await pushBookCover(settings, book.hash, () =>
|
||||
options.loadBookCover!(book),
|
||||
);
|
||||
if (coverResult.uploaded) result.coversUploaded += 1;
|
||||
} catch (e) {
|
||||
console.warn('WD library sync: cover failed', book.hash, e);
|
||||
}
|
||||
}
|
||||
if (options.syncBooks) {
|
||||
phase = 'upload-file';
|
||||
const fileResult = await pushBookFile(
|
||||
@@ -951,16 +974,6 @@ export const syncLibrary = async (
|
||||
} else if (fileResult.reason === 'remote-matches') {
|
||||
result.filesAlreadyInSync += 1;
|
||||
}
|
||||
if (options.loadBookCover) {
|
||||
try {
|
||||
const coverResult = await pushBookCover(settings, book.hash, () =>
|
||||
options.loadBookCover!(book),
|
||||
);
|
||||
if (coverResult.uploaded) result.coversUploaded += 1;
|
||||
} catch (e) {
|
||||
console.warn('WD library sync: cover failed', book.hash, e);
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch (e) {
|
||||
result.failures += 1;
|
||||
|
||||
Reference in New Issue
Block a user