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 cd77d35d..8ce1f155 100644 --- a/apps/readest-app/src/__tests__/services/import-metahash.test.ts +++ b/apps/readest-app/src/__tests__/services/import-metahash.test.ts @@ -648,4 +648,23 @@ describe('importBook with BookLookupIndex', () => { // Should reuse the existing book object via lookup index expect(result).toBe(existingBook); }); + + it('buildBookLookupIndex skips deleted and url-backed books in byFilePath', async () => { + const inPlaceBook = makeBook({ + hash: 'a', + filePath: '/lib/a.epub', + }); + const deletedBook = makeBook({ + hash: 'b', + filePath: '/lib/b.epub', + deletedAt: Date.now(), + }); + const urlBook = makeBook({ hash: 'c', filePath: 'https://example.com/c.epub' }); + + const lookupIndex = buildBookLookupIndex([inPlaceBook, deletedBook, urlBook], 'linux'); + + expect(lookupIndex.byFilePath.get('/lib/a.epub')).toBe(inPlaceBook); + expect(lookupIndex.byFilePath.has('/lib/b.epub')).toBe(false); + expect(lookupIndex.byFilePath.has('https://example.com/c.epub')).toBe(false); + }); }); diff --git a/apps/readest-app/src/__tests__/services/ingest-service.test.ts b/apps/readest-app/src/__tests__/services/ingest-service.test.ts index 03a09262..8ca7da87 100644 --- a/apps/readest-app/src/__tests__/services/ingest-service.test.ts +++ b/apps/readest-app/src/__tests__/services/ingest-service.test.ts @@ -543,6 +543,87 @@ describe('ingestFile', () => { expect(importBook.mock.calls[0]?.[2]).toMatchObject({ inPlace: false }); }); + // ------ in-place + byFilePath fast path ------ + // ingestFile probes the byFilePath index before delegating to importBook + // so a re-scan of an already-imported external folder skips file I/O, + // parsing, hashing, AND every downstream side effect (group / tag / upload + // decisions). The fast path returning early is what guarantees a manual + // GroupingModal assignment survives a folder re-import — without it, a + // path-derived empty groupId would clobber the existing group. + + test('byFilePath hit short-circuits importBook entirely on in-place re-import', async () => { + const { appService, settings, isLoggedIn, importBook } = makeDeps({ + externalLibraryFolders: ['/Users/me/Library'], + osPlatform: 'macos', + }); + const sourcePath = '/Users/me/Library/sample.epub'; + const existing: Book = { + hash: 'previously-hashed', + format: 'EPUB', + title: 'Existing', + author: 'Author', + filePath: sourcePath, + createdAt: 1000, + updatedAt: 2000, + groupId: 'manual', + groupName: 'Manual/Group', + }; + // macOS is case-insensitive, so the index key is lowercased to match + // what `normalizeFilePathForIndex` produces in production. + const lookupIndex = { + byHash: new Map(), + byMetaKey: new Map(), + byFilePath: new Map([[sourcePath.toLowerCase(), existing]]), + } as unknown as Parameters[0]['lookupIndex']; + const book = await ingestFile( + { + file: sourcePath, + books: [existing], + lookupIndex, + groupId: '', + groupName: undefined, + }, + { appService, settings, isLoggedIn }, + ); + expect(book).toBe(existing); + // importBook never ran, so no file I/O, no parser, no partialMD5. + expect(importBook).not.toHaveBeenCalled(); + // Existing fields are untouched: createdAt / updatedAt / group all stay. + expect(existing.createdAt).toBe(1000); + expect(existing.updatedAt).toBe(2000); + expect(existing.groupId).toBe('manual'); + expect(existing.groupName).toBe('Manual/Group'); + }); + + test('without inPlace the byFilePath index is ignored and importBook runs', async () => { + // Fast path is gated on `inPlace`. A classic copy-mode import (no + // external library folder configured) that happens to point at a path + // already in the library must still go through importBook so dedup + // falls back to byHash. + const { appService, settings, isLoggedIn, importBook } = makeDeps(); + const sourcePath = '/Users/me/Downloads/sample.epub'; + const existing: Book = { + hash: 'previously-hashed', + format: 'EPUB', + title: 'Existing', + author: 'Author', + filePath: sourcePath, + createdAt: 1000, + updatedAt: 2000, + }; + const lookupIndex = { + byHash: new Map(), + byMetaKey: new Map(), + byFilePath: new Map([[sourcePath, existing]]), + } as unknown as Parameters[0]['lookupIndex']; + await ingestFile( + { file: sourcePath, books: [existing], lookupIndex }, + { appService, settings, isLoggedIn }, + ); + expect(importBook).toHaveBeenCalledTimes(1); + expect(importBook.mock.calls[0]?.[2]).toMatchObject({ inPlace: false }); + }); + // ------ in-place + cloud upload ------ // In-place imports are still uploaded so the user gets backup / cross-device // sync. Only transient imports opt out of upload entirely. The on-the-wire diff --git a/apps/readest-app/src/app/library/page.tsx b/apps/readest-app/src/app/library/page.tsx index fbb21a21..fd5526c0 100644 --- a/apps/readest-app/src/app/library/page.tsx +++ b/apps/readest-app/src/app/library/page.tsx @@ -678,7 +678,12 @@ const LibraryPageContent = ({ searchParams }: { searchParams: ReadonlyURLSearchP // O(1) instead of O(n) over the existing library. importBook also keeps // the index updated as new books are appended, so subsequent files in // the same batch see the additions. - const lookupIndex = buildBookLookupIndex(library); + // + // `osPlatform` is required for the `byFilePath` arm: on case-insensitive + // filesystems (macOS / iOS / Windows) two paths that differ only in + // casing must hash to the same key, so the in-place fast path in + // importBook can recognize a re-import of the same file. + const lookupIndex = buildBookLookupIndex(library, appService?.osPlatform); const failedImports: Array<{ filename: string; errorMessage: string }> = []; const successfulImports: string[] = []; diff --git a/apps/readest-app/src/services/appService.ts b/apps/readest-app/src/services/appService.ts index 7f9bb9e0..dd3a85f7 100644 --- a/apps/readest-app/src/services/appService.ts +++ b/apps/readest-app/src/services/appService.ts @@ -261,6 +261,10 @@ export abstract class BaseAppService implements AppService { return BookSvc.importBook(this.fs, file, books, { saveBookConfig: this.saveBookConfig.bind(this), generateCoverImageUrl: this.generateCoverImageUrl.bind(this), + // Pass the host platform through so the in-place fast path and the + // lookup index can normalize source paths consistently on + // case-insensitive filesystems (macOS / iOS / Windows). + osPlatform: this.osPlatform, ...options, }); } diff --git a/apps/readest-app/src/services/bookService.ts b/apps/readest-app/src/services/bookService.ts index 7fdec13d..019138e0 100644 --- a/apps/readest-app/src/services/bookService.ts +++ b/apps/readest-app/src/services/bookService.ts @@ -1,5 +1,5 @@ import { SystemSettings } from '@/types/settings'; -import { FileSystem, AppPlatform, BaseDir } from '@/types/system'; +import { FileSystem, AppPlatform, BaseDir, OsPlatform } from '@/types/system'; import { Book, BookConfig, @@ -44,9 +44,10 @@ import { type BookFileContentSource, } from './bookContent'; -export function buildBookLookupIndex(books: Book[]): BookLookupIndex { +export function buildBookLookupIndex(books: Book[], osPlatform?: OsPlatform): BookLookupIndex { const byHash = new Map(); const byMetaKey = new Map(); + const byFilePath = new Map(); for (const book of books) { byHash.set(book.hash, book); if (book.metaHash && !book.deletedAt) { @@ -55,8 +56,38 @@ export function buildBookLookupIndex(books: Book[]): BookLookupIndex { if (list) list.push(book); else byMetaKey.set(key, [book]); } + // In-place books carry the absolute source path on `filePath` (set by + // importBook below). Indexing them here lets a re-import of the exact + // same file short-circuit before touching disk or computing partialMD5. + // Skip URL-backed entries (remote books) and tombstoned ones. + if (book.filePath && !isValidURL(book.filePath) && !book.deletedAt) { + const key = normalizeFilePathForIndex(book.filePath, osPlatform); + if (key) byFilePath.set(key, book); + } } - return { byHash, byMetaKey }; + return { byHash, byMetaKey, byFilePath }; +} + +/** + * Normalize an absolute file path into a stable map key for `byFilePath`. + * + * Mirrors the same rules `ingestService.shouldImportInPlace` uses to compare + * paths against the user's in-place roots so both sides agree on whether a + * given source file matches a previously-indexed book: + * - Backslashes are normalized to `/`. + * - Trailing slashes are stripped. + * - On case-insensitive filesystems (macOS / iOS / Windows) the key is + * lowercased. Linux / Android keep the original casing. + * + * Returns an empty string for non-string / falsy input so callers can do a + * `if (key) map.set(key, …)` guard without an extra null check. + */ +export function normalizeFilePathForIndex(path: string, osPlatform?: OsPlatform): string { + if (!path) return ''; + const caseInsensitive = + osPlatform === 'macos' || osPlatform === 'ios' || osPlatform === 'windows'; + const n = path.replace(/\\/g, '/').replace(/\/+$/, ''); + return caseInsensitive ? n.toLowerCase() : n; } export interface CoverContext { @@ -223,6 +254,14 @@ export async function mergeBooks( export interface ImportBookInternalOptions extends ImportBookOptions { saveBookConfig: (book: Book, config: BookConfig) => Promise; generateCoverImageUrl: (book: Book) => Promise; + /** + * Used by the in-place fast path to normalize the source path the same way + * `buildBookLookupIndex` does. Optional: when omitted the fast path falls + * back to a case-sensitive comparison, which is still safe (it will simply + * miss matches that differ only in casing on macOS / iOS / Windows and the + * import will proceed down the slow path as before). + */ + osPlatform?: OsPlatform; } export async function importBook( @@ -246,8 +285,10 @@ export async function importBook( transient = false, inPlace = false, lookupIndex, + osPlatform, } = options; const isPseStream = typeof file === 'string' && isPseStreamFileName(file); + try { let loadedBook: BookDoc; let format: BookFormat; @@ -530,6 +571,18 @@ export async function importBook( if (existingBook) existingBook.filePath = file; } } + // Now that `filePath` is set, keep the path index in sync so later files + // in the same batch (and the next call into importBook) can hit the + // in-place fast path. Only persistent in-place imports go into the + // index — transient previews are short-lived and never persisted to + // the library so indexing them would just leak references. + if (lookupIndex && inPlace && !transient && typeof file === 'string') { + const indexedBook = existingBook || book; + if (indexedBook.filePath) { + const key = normalizeFilePathForIndex(indexedBook.filePath, osPlatform); + if (key) lookupIndex.byFilePath.set(key, indexedBook); + } + } book.coverImageUrl = await generateCoverImageUrlFn(book); const f = file as ClosableFile; if (f && f.close) { diff --git a/apps/readest-app/src/services/ingestService.ts b/apps/readest-app/src/services/ingestService.ts index 7dce9cbf..82f438a8 100644 --- a/apps/readest-app/src/services/ingestService.ts +++ b/apps/readest-app/src/services/ingestService.ts @@ -2,6 +2,9 @@ import type { Book, BookLookupIndex } from '@/types/book'; import type { AppService, OsPlatform } from '@/types/system'; import type { SystemSettings } from '@/types/settings'; import { transferManager } from '@/services/transferManager'; +import { normalizeFilePathForIndex } from '@/services/bookService'; +import { isContentURI, isValidURL } from '@/utils/misc'; +import { isPseStreamFileName } from '@/services/opds/pseStream'; export interface IngestFileDeps { appService: AppService; @@ -102,12 +105,13 @@ function shouldImportInPlace( // case-sensitive and stay strict. We do not attempt unicode normalization // (NFC/NFD) — APFS handles that at the FS layer and `toLocaleLowerCase` // with the wrong locale would introduce its own bugs (e.g. Turkish `İ`). - const caseInsensitive = - osPlatform === 'macos' || osPlatform === 'ios' || osPlatform === 'windows'; - const norm = (p: string) => { - const n = p.replace(/\\/g, '/').replace(/\/+$/, ''); - return caseInsensitive ? n.toLowerCase() : n; - }; + // + // Defer the actual canonicalization to `normalizeFilePathForIndex` so the + // path index (`BookLookupIndex.byFilePath`) and this in-place decision + // agree on what counts as the same path — otherwise a re-import could + // hit the in-place branch here but miss the fast-path dedup in + // importBook (or vice versa). + const norm = (p: string) => normalizeFilePathForIndex(p, osPlatform); const target = norm(file); // If the file already lives inside Readest's own managed books directory @@ -160,6 +164,43 @@ export async function ingestFile( appBooksPrefix ?? null, ); + // In-place re-import fast path. When the source file lives under one of + // the user's registered external library folders and the byFilePath index + // already knows about it, skip importBook entirely and return the existing + // library entry verbatim. No fs.openFile, no native parser, no partialMD5, + // no timestamp / cover / config writes — and crucially no downstream group + // / tag / upload logic, so a re-scan can't silently rewrite library sort + // order or clobber a manual GroupingModal assignment via a path-derived + // group string. + // + // This is intentionally separate from importBook's byHash / byMetaKey + // dedup: a byHash hit means a *different* source path resolves to a known + // book (drop a copy from elsewhere, or revive a soft-deleted entry), which + // correctly clears `deletedAt` and refreshes timestamps. A byFilePath hit + // is the same on-disk file at the same path — there is nothing to refresh, + // and refreshing would silently rewrite library sort order on every + // re-scan. Soft-deleted books are excluded from `byFilePath` at index + // build time so they fall through to byHash and get resurrected. + // + // Reject URLs, content URIs and PSE streams defensively. The byFilePath + // index only carries real on-disk paths, but `inPlace` could in principle + // be set on a non-path source by a buggy caller. + if ( + inPlace && + !opts.transient && + opts.lookupIndex && + typeof opts.file === 'string' && + !isPseStreamFileName(opts.file) && + !isValidURL(opts.file) && + !isContentURI(opts.file) + ) { + const key = normalizeFilePathForIndex(opts.file, appService.osPlatform); + const existing = key ? opts.lookupIndex.byFilePath.get(key) : undefined; + if (existing) { + return existing; + } + } + const book = await appService.importBook(opts.file, opts.books, { lookupIndex: opts.lookupIndex, transient: opts.transient, diff --git a/apps/readest-app/src/types/book.ts b/apps/readest-app/src/types/book.ts index 35cd3f28..30c89b49 100644 --- a/apps/readest-app/src/types/book.ts +++ b/apps/readest-app/src/types/book.ts @@ -45,6 +45,13 @@ export const FIXED_LAYOUT_FORMATS: Set = new Set(['PDF', 'CBZ']); export interface BookLookupIndex { byHash: Map; byMetaKey: Map; // key = `${metaHash}:${format}` + // Maps normalized absolute source path -> Book for in-place imports. + // Lets the importer recognize "I already have this exact file" without + // having to open, parse, and hash it again. Only books with a non-empty + // `filePath` and `!deletedAt` are indexed. The key is produced by + // `normalizeFilePathForIndex` so callers must use the same helper to + // probe; importBook handles that internally. + byFilePath: Map; } /**