diff --git a/apps/readest-app/src/__tests__/services/suites/library-tests.ts b/apps/readest-app/src/__tests__/services/suites/library-tests.ts index 08eccb0f..ba274fdc 100644 --- a/apps/readest-app/src/__tests__/services/suites/library-tests.ts +++ b/apps/readest-app/src/__tests__/services/suites/library-tests.ts @@ -90,12 +90,12 @@ export function libraryTests(getService: () => AppService) { expect(loaded).toEqual([]); }); - it('should overwrite previous library on save', async () => { + it('should overwrite (and shrink) the library only when replace is set', async () => { const service = getService(); await service.saveLibraryBooks([makeBook({ hash: 'old' })]); const newBooks = [makeBook({ hash: 'new1' }), makeBook({ hash: 'new2' })]; - await service.saveLibraryBooks(newBooks); + await service.saveLibraryBooks(newBooks, { replace: true }); const loaded = await service.loadLibraryBooks(); expect(loaded).toHaveLength(2); @@ -143,13 +143,70 @@ export function libraryTests(getService: () => AppService) { expect(loaded[0]!.updatedAt).toBe(5000); }); - it('should save empty array', async () => { + it('should clear the library when saving an empty array with replace', async () => { const service = getService(); await service.saveLibraryBooks([makeBook()]); - await service.saveLibraryBooks([]); + await service.saveLibraryBooks([], { replace: true }); const loaded = await service.loadLibraryBooks(); expect(loaded).toEqual([]); }); + + // Merge-floor safebelt: a routine save may ADD or MODIFY rows (including + // setting `deletedAt` tombstones) but must never silently DROP a book that + // exists on disk. Guards against a stale or partially-loaded in-memory + // library wiping library.json (the cold-start "Open with" race). + it('should not drop on-disk books absent from the saved set (merge floor)', async () => { + const service = getService(); + await service.saveLibraryBooks([ + makeBook({ hash: 'a', title: 'A' }), + makeBook({ hash: 'b', title: 'B' }), + makeBook({ hash: 'c', title: 'C' }), + ]); + + // A later save that only knows about 'a' must not erase 'b' and 'c'. + await service.saveLibraryBooks([makeBook({ hash: 'a', title: 'A2' })]); + + const loaded = await service.loadLibraryBooks(); + const byHash = Object.fromEntries(loaded.map((b) => [b.hash, b.title])); + expect(loaded).toHaveLength(3); + expect(byHash).toEqual({ a: 'A2', b: 'B', c: 'C' }); + }); + + it('should preserve the whole library when an empty set is saved (no wipe)', async () => { + const service = getService(); + await service.saveLibraryBooks([makeBook({ hash: 'a' }), makeBook({ hash: 'b' })]); + + await service.saveLibraryBooks([]); + + const loaded = await service.loadLibraryBooks(); + expect(loaded.map((b) => b.hash).sort()).toEqual(['a', 'b']); + }); + + it('should keep tombstones (deletedAt) not present in the incoming set', async () => { + const service = getService(); + await service.saveLibraryBooks([ + makeBook({ hash: 'live' }), + makeBook({ hash: 'gone', deletedAt: 1234 }), + ]); + + // A save that omits the tombstone must neither lose nor resurrect it. + await service.saveLibraryBooks([makeBook({ hash: 'live' })]); + + const loaded = await service.loadLibraryBooks(); + const tomb = loaded.find((b) => b.hash === 'gone'); + expect(loaded).toHaveLength(2); + expect(tomb?.deletedAt).toBe(1234); + }); + + it('should let the incoming row win on a hash conflict (modify in place)', async () => { + const service = getService(); + await service.saveLibraryBooks([makeBook({ hash: 'a', title: 'old' })]); + await service.saveLibraryBooks([makeBook({ hash: 'a', title: 'new' })]); + + const loaded = await service.loadLibraryBooks(); + expect(loaded).toHaveLength(1); + expect(loaded[0]!.title).toBe('new'); + }); }); } diff --git a/apps/readest-app/src/app/library/page.tsx b/apps/readest-app/src/app/library/page.tsx index c467dbc6..7dd172fe 100644 --- a/apps/readest-app/src/app/library/page.tsx +++ b/apps/readest-app/src/app/library/page.tsx @@ -143,6 +143,7 @@ const LibraryPageContent = ({ searchParams }: { searchParams: ReadonlyURLSearchP const { token, user } = useAuth(); const { library: libraryBooks, + libraryLoaded: libraryLoadedFromDisk, isSyncing, syncProgress, updateBook, @@ -545,7 +546,11 @@ const LibraryPageContent = ({ searchParams }: { searchParams: ReadonlyURLSearchP } }; - const hasCachedLibrary = libraryBooks.length > 0; + // Reuse the in-store library only when it was actually loaded from disk. + // Gating on `length > 0` was unsafe: a transient "Open with" entry made the + // store non-empty before any disk load, so this skipped loadLibraryBooks and + // a later save persisted the partial library (wiping library.json). + const hasCachedLibrary = libraryLoadedFromDisk; const loadingTimeout = hasCachedLibrary ? null : setTimeout(() => setLoading(true), 500); const initLibrary = async () => { const appService = await envConfig.getAppService(); diff --git a/apps/readest-app/src/hooks/useOpenWithBooks.ts b/apps/readest-app/src/hooks/useOpenWithBooks.ts index 88d34e81..8c4d4de3 100644 --- a/apps/readest-app/src/hooks/useOpenWithBooks.ts +++ b/apps/readest-app/src/hooks/useOpenWithBooks.ts @@ -83,7 +83,18 @@ export function useOpenWithBooks() { // or uploading to the cloud. We also setLibrary so the reader's // initViewState (which looks the book up via getBookByHash) can find // the ephemeral entry. - const { library, setLibrary, getBookByHash } = useLibraryStore.getState(); + const { setLibrary, getBookByHash, libraryLoaded } = useLibraryStore.getState(); + // Load the real library from disk before building any transient entry. + // On a cold-start "Open with" the store may not be populated yet; importing + // onto an empty in-memory library would (a) miss the hash-match for an + // already-imported book and (b) let the library page's cached-skip persist + // an empty library, wiping library.json. Loading first makes the store + // reflect disk (and marks it loaded) so neither happens. + let library = useLibraryStore.getState().library; + if (!libraryLoaded) { + library = await appService.loadLibraryBooks(); + setLibrary(library); + } const bookIds: string[] = []; let libraryMutated = false; diff --git a/apps/readest-app/src/services/appService.ts b/apps/readest-app/src/services/appService.ts index dd3a85f7..7ac62616 100644 --- a/apps/readest-app/src/services/appService.ts +++ b/apps/readest-app/src/services/appService.ts @@ -9,6 +9,7 @@ import { FileSystem, OsPlatform, ResolvedPath, + SaveLibraryBooksOptions, SelectDirectoryMode, } from '@/types/system'; import { DatabaseOpts, DatabaseService } from '@/types/database'; @@ -421,7 +422,7 @@ export abstract class BaseAppService implements AppService { return LibrarySvc.loadLibraryBooks(this.fs, this.generateCoverImageUrl.bind(this)); } - async saveLibraryBooks(books: Book[]): Promise { - return LibrarySvc.saveLibraryBooks(this.fs, books); + async saveLibraryBooks(books: Book[], options?: SaveLibraryBooksOptions): Promise { + return LibrarySvc.saveLibraryBooks(this.fs, books, options); } } diff --git a/apps/readest-app/src/services/libraryService.ts b/apps/readest-app/src/services/libraryService.ts index 7ca579e0..dbad8fe4 100644 --- a/apps/readest-app/src/services/libraryService.ts +++ b/apps/readest-app/src/services/libraryService.ts @@ -1,4 +1,4 @@ -import { FileSystem } from '@/types/system'; +import { FileSystem, SaveLibraryBooksOptions } from '@/types/system'; import { Book } from '@/types/book'; import { getLibraryFilename } from '@/utils/book'; import { safeLoadJSON, safeSaveJSON } from './persistence'; @@ -35,8 +35,28 @@ export async function loadLibraryBooks( return books; } -export async function saveLibraryBooks(fs: FileSystem, books: Book[]): Promise { +export async function saveLibraryBooks( + fs: FileSystem, + books: Book[], + options?: SaveLibraryBooksOptions, +): Promise { // eslint-disable-next-line @typescript-eslint/no-unused-vars - const libraryBooks = books.map(({ coverImageUrl, ...rest }) => rest); - await safeSaveJSON(fs, getLibraryFilename(), 'Books', libraryBooks); + const incoming = books.map(({ coverImageUrl, ...rest }) => rest); + + if (options?.replace) { + await safeSaveJSON(fs, getLibraryFilename(), 'Books', incoming); + return; + } + + // Merge-floor: treat the on-disk library as a floor. A routine save may add + // new books or modify existing rows (including setting `deletedAt` + // tombstones), but it must never silently drop a book that exists on disk. + // This stops a stale or partially-loaded in-memory library (e.g. the + // cold-start "Open with" race) from wiping library.json. Deliberate removals + // must go through `{ replace: true }`. + const existing = await safeLoadJSON(fs, getLibraryFilename(), 'Books', []); + const merged = new Map(); + for (const book of existing) merged.set(book.hash, book); + for (const book of incoming) merged.set(book.hash, book); // incoming wins per hash + await safeSaveJSON(fs, getLibraryFilename(), 'Books', Array.from(merged.values())); } diff --git a/apps/readest-app/src/types/system.ts b/apps/readest-app/src/types/system.ts index 1863dc6a..198b18e4 100644 --- a/apps/readest-app/src/types/system.ts +++ b/apps/readest-app/src/types/system.ts @@ -68,6 +68,16 @@ export interface FileSystem { getPrefix(base: BaseDir): Promise; } +export interface SaveLibraryBooksOptions { + /** + * Overwrite `library.json` with exactly the given set, allowing it to shrink. + * Reserved for deliberate, authoritative rewrites (tombstone GC, explicit + * "clear library", account reset). Routine saves must NOT set this — the + * default merge-floor protects against silently dropping books on disk. + */ + replace?: boolean; +} + export interface AppService { osPlatform: OsPlatform; appPlatform: AppPlatform; @@ -205,7 +215,7 @@ export interface AppService { loadBookContent(book: Book): Promise; resolveNativeBookFilePath(book: Book): Promise; loadLibraryBooks(): Promise; - saveLibraryBooks(books: Book[]): Promise; + saveLibraryBooks(books: Book[], options?: SaveLibraryBooksOptions): Promise; getCoverImageUrl(book: Book): string; getCoverImageBlobUrl(book: Book): Promise; generateCoverImageUrl(book: Book): Promise;