From 4eeed74cdd85cb1c02d6af95aef56d02c65075eb Mon Sep 17 00:00:00 2001 From: loveheaven Date: Fri, 5 Jun 2026 00:23:53 +0800 Subject: [PATCH] fix(webdav): always sync book covers, not just when syncBooks is on (#4445) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../src/app/reader/hooks/useWebDAVSync.ts | 91 ++++++++++++++----- .../src/services/webdav/WebDAVSync.ts | 43 ++++++--- 2 files changed, 96 insertions(+), 38 deletions(-) diff --git a/apps/readest-app/src/app/reader/hooks/useWebDAVSync.ts b/apps/readest-app/src/app/reader/hooks/useWebDAVSync.ts index f5ec766a..314bed89 100644 --- a/apps/readest-app/src/app/reader/hooks/useWebDAVSync.ts +++ b/apps/readest-app/src/app/reader/hooks/useWebDAVSync.ts @@ -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; diff --git a/apps/readest-app/src/services/webdav/WebDAVSync.ts b/apps/readest-app/src/services/webdav/WebDAVSync.ts index 8ab7f2d5..043389b3 100644 --- a/apps/readest-app/src/services/webdav/WebDAVSync.ts +++ b/apps/readest-app/src/services/webdav/WebDAVSync.ts @@ -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//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;