fix(book-dock): refine duplicate detection for books without isbn

The duplicate detection logic in BookDockFinalizeService previously identified books as duplicates if they had the same title, even if their authors differed, when ISBNs were unavailable. This has been updated to include author names in the duplicate check for books missing ISBNs to prevent incorrect duplicate identifications. Closes #175
2026-11-03 08:21:39 -06:00 · 2026-06-01 23:20:34 +00:00
parent 9fd61d327a
commit f20efdeedd
3 changed files with 197 additions and 34 deletions
@@ -730,7 +730,7 @@ describe('BookDockFinalizeService', () => {
    await expect((service as any).findFolderOrFail(9, 5)).rejects.toBeInstanceOf(BadRequestException);
  });

-  it('buildDuplicateLookup collects isbn and title keys per library', async () => {
+  it('buildDuplicateLookup collects isbn and title+author keys per library', async () => {
    const { service, db } = makeService();
    const selectChain = {
      from: vi.fn(),
@@ -742,14 +742,14 @@ describe('BookDockFinalizeService', () => {
    selectChain.where
      .mockResolvedValueOnce([{ bookId: 11, isbn13: '9780306406157' }])
      .mockResolvedValueOnce([{ bookId: 12, isbn10: '0306406152' }])
-      .mockResolvedValueOnce([{ bookId: 13, normalizedTitle: 'dune' }]);
+      .mockResolvedValueOnce([{ bookId: 13, normalizedTitle: 'dune', normalizedAuthor: 'frank herbert' }]);
    db.select.mockReturnValue(selectChain);

    const lookup = await (service as any).buildDuplicateLookup(
      [
        makeRow({ id: 1, targetLibraryId: 5, selectedMetadata: { isbn13: '9780306406157' } }),
        makeRow({ id: 2, targetLibraryId: 5, selectedMetadata: { isbn10: '0306406152' } }),
-        makeRow({ id: 3, targetLibraryId: 5, selectedMetadata: { title: 'Dune' } }),
+        makeRow({ id: 3, targetLibraryId: 5, selectedMetadata: { title: 'Dune', authors: ['Frank Herbert'] } }),
      ],
      undefined,
      new Map(),
@@ -757,10 +757,38 @@ describe('BookDockFinalizeService', () => {

    expect(lookup.get('library:5|isbn13:9780306406157')).toBe(11);
    expect(lookup.get('library:5|isbn10:0306406152')).toBe(12);
-    expect(lookup.get('library:5|title:dune')).toBe(13);
+    expect(lookup.get('library:5|title:dune|author:frank herbert')).toBe(13);
  });

-  it('findDuplicate queries by isbn and title when prebuilt lookup misses', async () => {
+  it('buildDuplicateLookup keeps only requested title+author pairs', async () => {
+    const { service, db } = makeService();
+    const selectChain = {
+      from: vi.fn(),
+      innerJoin: vi.fn(),
+      where: vi.fn(),
+    };
+    selectChain.from.mockReturnValue(selectChain);
+    selectChain.innerJoin.mockReturnValue(selectChain);
+    selectChain.where.mockResolvedValueOnce([
+      { bookId: 13, normalizedTitle: 'dune', normalizedAuthor: 'frank herbert' },
+      { bookId: 14, normalizedTitle: 'dune', normalizedAuthor: 'isaac asimov' },
+    ]);
+    db.select.mockReturnValue(selectChain);
+
+    const lookup = await (service as any).buildDuplicateLookup(
+      [
+        makeRow({ id: 1, targetLibraryId: 5, selectedMetadata: { title: 'Dune', authors: ['Frank Herbert'] } }),
+        makeRow({ id: 2, targetLibraryId: 5, selectedMetadata: { title: 'Foundation', authors: ['Isaac Asimov'] } }),
+      ],
+      undefined,
+      new Map(),
+    );
+
+    expect(lookup.get('library:5|title:dune|author:frank herbert')).toBe(13);
+    expect(lookup.get('library:5|title:dune|author:isaac asimov')).toBeUndefined();
+  });
+
+  it('findDuplicate queries by isbn and title+author when prebuilt lookup misses', async () => {
    const { service, db } = makeService();
    const selectChain = {
      from: vi.fn(),
@@ -771,15 +799,32 @@ describe('BookDockFinalizeService', () => {
    selectChain.from.mockReturnValue(selectChain);
    selectChain.innerJoin.mockReturnValue(selectChain);
    selectChain.where.mockReturnValue(selectChain);
-    selectChain.limit
-      .mockResolvedValueOnce([{ bookId: 91 }])
-      .mockResolvedValueOnce([{ bookId: 92 }])
-      .mockResolvedValueOnce([]);
+    selectChain.limit.mockResolvedValueOnce([{ bookId: 91 }]).mockResolvedValueOnce([{ bookId: 92 }]);
    db.select.mockReturnValue(selectChain);

-    await expect((service as any).findDuplicate(4, { isbn13: '9780306406157', isbn10: null, title: null })).resolves.toBe(91);
-    await expect((service as any).findDuplicate(4, { isbn13: null, isbn10: null, title: 'Dune' })).resolves.toBe(92);
-    await expect((service as any).findDuplicate(4, { isbn13: null, isbn10: null, title: null })).resolves.toBeNull();
+    await expect((service as any).findDuplicate(4, { isbn13: '9780306406157', isbn10: null, title: null, authors: [] })).resolves.toBe(91);
+    await expect((service as any).findDuplicate(4, { isbn13: null, isbn10: null, title: 'Dune', authors: ['Frank Herbert'] })).resolves.toBe(92);
+    await expect((service as any).findDuplicate(4, { isbn13: null, isbn10: null, title: 'Dune', authors: [] })).resolves.toBeNull();
+  });
+
+  it('findDuplicate does not treat title-only prebuilt entries as duplicates when author differs', async () => {
+    const { service, db } = makeService();
+    const selectChain = {
+      from: vi.fn(),
+      innerJoin: vi.fn(),
+      where: vi.fn(),
+      limit: vi.fn(),
+    };
+    selectChain.from.mockReturnValue(selectChain);
+    selectChain.innerJoin.mockReturnValue(selectChain);
+    selectChain.where.mockReturnValue(selectChain);
+    selectChain.limit.mockResolvedValueOnce([]);
+    db.select.mockReturnValue(selectChain);
+
+    const duplicateLookup = new Map([['library:4|title:dune', 77]]);
+    await expect(
+      (service as any).findDuplicate(4, { isbn13: null, isbn10: null, title: 'Dune', authors: ['Brian Herbert'] }, duplicateLookup),
+    ).resolves.toBeNull();
  });

  it('resolveDestination builds names from patterns and falls back to original filename', async () => {
@@ -845,10 +890,21 @@ describe('BookDockFinalizeService', () => {
  });

  it('findDuplicate resolves from prebuilt lookup before querying database', async () => {
-    const { service } = makeService();
+    const { service, db } = makeService();
    const duplicateLookup = new Map([['library:4|isbn13:9780306406157', 88]]);

    await expect((service as any).findDuplicate(4, { isbn13: '9780306406157', isbn10: null, title: null }, duplicateLookup)).resolves.toBe(88);
+    expect(db.select).not.toHaveBeenCalled();
+  });
+
+  it('findDuplicate resolves title+author from prebuilt lookup before querying database', async () => {
+    const { service, db } = makeService();
+    const duplicateLookup = new Map([['library:4|title:dune|author:frank herbert', 89]]);
+
+    await expect(
+      (service as any).findDuplicate(4, { isbn13: null, isbn10: null, title: 'Dune', authors: ['Frank Herbert'] }, duplicateLookup),
+    ).resolves.toBe(89);
+    expect(db.select).not.toHaveBeenCalled();
  });

  it('triggerAutoFinalize skips when auto-finalize is disabled or destination is incomplete', async () => {
@@ -9,7 +9,7 @@ import { NotificationType, resolveUploadPath } from '@bookorbit/types';
 import { NotificationService } from '../notification/notification.service';
 import { DB } from '../../db';
 import * as schema from '../../db/schema';
-import { bookMetadata, books, libraries, libraryFolders } from '../../db/schema';
+import { authors, bookAuthors, bookMetadata, books, libraries, libraryFolders } from '../../db/schema';
 import { AppSettingsService } from '../app-settings/app-settings.service';
 import { LibraryService } from '../library/library.service';
 import { MetadataService } from '../metadata/metadata.service';
@@ -321,7 +321,10 @@ export class BookDockFinalizeService implements OnModuleInit {
    defaultLibraryId: number | undefined,
    overrideMap: Map<number, { libraryId?: number; folderId?: number }>,
  ): Promise<Map<string, number>> {
-    const needsByLibrary = new Map<number, { isbn13: Set<string>; isbn10: Set<string>; titles: Set<string> }>();
+    const needsByLibrary = new Map<
+      number,
+      { isbn13: Set<string>; isbn10: Set<string>; titles: Set<string>; authors: Set<string>; titleAuthorPairs: Set<string> }
+    >();

    for (const row of rows) {
      const override = overrideMap.get(row.id);
@@ -331,7 +334,7 @@ export class BookDockFinalizeService implements OnModuleInit {
      const meta = normalizeFinalizeMetadata(row.selectedMetadata ?? row.embeddedMetadata ?? {});
      let bucket = needsByLibrary.get(libraryId);
      if (!bucket) {
-        bucket = { isbn13: new Set(), isbn10: new Set(), titles: new Set() };
+        bucket = { isbn13: new Set(), isbn10: new Set(), titles: new Set(), authors: new Set(), titleAuthorPairs: new Set() };
        needsByLibrary.set(libraryId, bucket);
      }

@@ -340,7 +343,13 @@ export class BookDockFinalizeService implements OnModuleInit {
      } else if (meta.isbn10) {
        bucket.isbn10.add(meta.isbn10);
      } else if (meta.title) {
+        const normalizedAuthors = normalizeDuplicateAuthors(meta.authors);
+        if (normalizedAuthors.length === 0) continue;
        bucket.titles.add(meta.title.toLowerCase());
+        for (const authorName of normalizedAuthors) {
+          bucket.authors.add(authorName);
+          bucket.titleAuthorPairs.add(`${meta.title.toLowerCase()}|${authorName}`);
+        }
      }
    }

@@ -376,18 +385,34 @@ export class BookDockFinalizeService implements OnModuleInit {
        }
      }

-      if (values.titles.size > 0) {
-        const rowsByTitle = await this.db
+      if (values.titles.size > 0 && values.authors.size > 0) {
+        const rowsByTitleAuthor = await this.db
          .select({
            bookId: bookMetadata.bookId,
            normalizedTitle: sql<string>`lower(${bookMetadata.title})`,
+            normalizedAuthor: sql<string>`lower(${authors.name})`,
          })
          .from(bookMetadata)
          .innerJoin(books, eq(books.id, bookMetadata.bookId))
-          .where(and(eq(books.libraryId, libraryId), inArray(sql<string>`lower(${bookMetadata.title})`, [...values.titles])));
-        for (const row of rowsByTitle) {
-          if (!row.normalizedTitle) continue;
-          const key = this.buildDuplicateLookupKey(libraryId, { isbn13: null, isbn10: null, title: row.normalizedTitle });
+          .innerJoin(bookAuthors, eq(bookAuthors.bookId, bookMetadata.bookId))
+          .innerJoin(authors, eq(authors.id, bookAuthors.authorId))
+          .where(
+            and(
+              eq(books.libraryId, libraryId),
+              inArray(sql<string>`lower(${bookMetadata.title})`, [...values.titles]),
+              inArray(sql<string>`lower(${authors.name})`, [...values.authors]),
+            ),
+          );
+        for (const row of rowsByTitleAuthor) {
+          if (!row.normalizedTitle || !row.normalizedAuthor) continue;
+          const pairKey = `${row.normalizedTitle}|${row.normalizedAuthor}`;
+          if (!values.titleAuthorPairs.has(pairKey)) continue;
+          const key = this.buildDuplicateLookupKey(libraryId, {
+            isbn13: null,
+            isbn10: null,
+            title: row.normalizedTitle,
+            author: row.normalizedAuthor,
+          });
          if (key && !lookup.has(key)) {
            lookup.set(key, row.bookId);
          }
@@ -398,31 +423,34 @@ export class BookDockFinalizeService implements OnModuleInit {
    return lookup;
  }

-  private buildDuplicateLookupKey(libraryId: number, meta: Pick<NormalizedFinalizeMetadata, 'isbn13' | 'isbn10' | 'title'>): string | null {
+  private buildDuplicateLookupKey(
+    libraryId: number,
+    meta: { isbn13: string | null; isbn10: string | null; title: string | null; author?: string | null },
+  ): string | null {
    const isbn = meta.isbn13 ?? meta.isbn10;
    if (isbn) {
      const isbnKind = meta.isbn13 ? 'isbn13' : 'isbn10';
      return `library:${libraryId}|${isbnKind}:${isbn}`;
    }
-    if (meta.title) {
-      return `library:${libraryId}|title:${meta.title.toLowerCase()}`;
+    if (meta.title && meta.author) {
+      return `library:${libraryId}|title:${meta.title.toLowerCase()}|author:${meta.author.toLowerCase()}`;
    }
    return null;
  }

  private async findDuplicate(
    libraryId: number,
-    meta: Pick<NormalizedFinalizeMetadata, 'isbn13' | 'isbn10' | 'title'>,
+    meta: Pick<NormalizedFinalizeMetadata, 'isbn13' | 'isbn10' | 'title' | 'authors'>,
    duplicateLookup?: Map<string, number>,
  ): Promise<number | null> {
-    const lookupKey = this.buildDuplicateLookupKey(libraryId, meta);
-    if (lookupKey && duplicateLookup?.has(lookupKey)) {
-      return duplicateLookup.get(lookupKey) ?? null;
-    }
-
    const isbn = meta.isbn13 ?? meta.isbn10;

    if (isbn) {
+      const lookupKey = this.buildDuplicateLookupKey(libraryId, { isbn13: meta.isbn13, isbn10: meta.isbn10, title: null, author: null });
+      if (lookupKey && duplicateLookup?.has(lookupKey)) {
+        return duplicateLookup.get(lookupKey) ?? null;
+      }
+
      const conditions = meta.isbn13 ? [eq(bookMetadata.isbn13, meta.isbn13)] : [eq(bookMetadata.isbn10, meta.isbn10!)];

      const [existing] = await this.db
@@ -436,11 +464,34 @@ export class BookDockFinalizeService implements OnModuleInit {
    }

    if (!isbn && meta.title) {
+      const normalizedAuthors = normalizeDuplicateAuthors(meta.authors);
+      if (normalizedAuthors.length === 0) return null;
+
+      for (const authorName of normalizedAuthors) {
+        const lookupKey = this.buildDuplicateLookupKey(libraryId, {
+          isbn13: null,
+          isbn10: null,
+          title: meta.title,
+          author: authorName,
+        });
+        if (lookupKey && duplicateLookup?.has(lookupKey)) {
+          return duplicateLookup.get(lookupKey) ?? null;
+        }
+      }
+
      const [existing] = await this.db
        .select({ bookId: bookMetadata.bookId })
        .from(bookMetadata)
        .innerJoin(books, eq(books.id, bookMetadata.bookId))
-        .where(and(eq(books.libraryId, libraryId), sql`lower(${bookMetadata.title}) = lower(${meta.title})`))
+        .innerJoin(bookAuthors, eq(bookAuthors.bookId, bookMetadata.bookId))
+        .innerJoin(authors, eq(authors.id, bookAuthors.authorId))
+        .where(
+          and(
+            eq(books.libraryId, libraryId),
+            sql`lower(${bookMetadata.title}) = lower(${meta.title})`,
+            inArray(sql<string>`lower(${authors.name})`, normalizedAuthors),
+          ),
+        )
        .limit(1);

      if (existing) return existing.bookId;
@@ -638,6 +689,18 @@ function normalizeStringArray(value: unknown, maxLength: number): string[] {
  return normalized;
 }

+function normalizeDuplicateAuthors(value: string[] | null | undefined): string[] {
+  if (!Array.isArray(value)) return [];
+  const normalized = new Set<string>();
+  for (const author of value) {
+    if (typeof author !== 'string') continue;
+    const trimmed = author.trim().toLowerCase();
+    if (!trimmed) continue;
+    normalized.add(trimmed);
+  }
+  return [...normalized];
+}
+
 function normalizeFinalizeMetadata(meta: BookDockMetadata | null | undefined): NormalizedFinalizeMetadata {
  return {
    title: normalizeText(meta?.title, 1000),
@@ -365,7 +365,7 @@ describe('Book Dock ingest + finalize (e2e)', () => {

    const seedRow = await createBookDockRow(context, {
      fileName: 'seed-duplicate.fb2',
-      selectedMetadata: { title: 'Duplicate Title', authors: ['Duplicate Author'] },
+      selectedMetadata: { title: 'Duplicate Seed Title', authors: ['Duplicate Author'], isbn13: '9780306406157' },
      targetLibraryId: destination.libraryId,
      targetFolderId: destination.libraryFolderId,
    });
@@ -380,7 +380,7 @@ describe('Book Dock ingest + finalize (e2e)', () => {

    const duplicateRow = await createBookDockRow(context, {
      fileName: 'duplicate-input.fb2',
-      selectedMetadata: { title: 'Duplicate Title', authors: ['Another Author'] },
+      selectedMetadata: { title: 'Duplicate Candidate Title', authors: ['Another Author'], isbn13: '9780306406157' },
      targetLibraryId: destination.libraryId,
      targetFolderId: destination.libraryFolderId,
    });
@@ -447,6 +447,50 @@ describe('Book Dock ingest + finalize (e2e)', () => {
    expect(await getBookDockRow(context, successRow.id)).toBeUndefined();
  });

+  it('finalize allows same title with different author when isbn is missing', async () => {
+    const destination = await createLibraryWithFolder(context);
+
+    const seedRow = await createBookDockRow(context, {
+      fileName: 'seed-same-title.fb2',
+      selectedMetadata: { title: 'Shared Title', authors: ['Author One'] },
+      targetLibraryId: destination.libraryId,
+      targetFolderId: destination.libraryFolderId,
+    });
+    const seedResponse = await context.app.inject({
+      method: 'POST',
+      url: '/api/v1/book-dock/finalize',
+      headers: authHeader(context.adminToken),
+      payload: { fileIds: [seedRow.id] },
+    });
+    expect(seedResponse.statusCode).toBe(201);
+    const seedBody = seedResponse.json() as BookDockFinalizeResult;
+    expect(seedBody.results[0]?.success).toBe(true);
+    const existingBookId = seedBody.results[0]!.bookId!;
+
+    const candidateRow = await createBookDockRow(context, {
+      fileName: 'candidate-same-title.fb2',
+      selectedMetadata: { title: 'Shared Title', authors: ['Author Two'] },
+      targetLibraryId: destination.libraryId,
+      targetFolderId: destination.libraryFolderId,
+    });
+
+    const response = await context.app.inject({
+      method: 'POST',
+      url: '/api/v1/book-dock/finalize',
+      headers: authHeader(context.adminToken),
+      payload: { fileIds: [candidateRow.id] },
+    });
+
+    expect(response.statusCode).toBe(201);
+    const body = response.json() as BookDockFinalizeResult;
+    expect(body).toMatchObject({ total: 1, succeeded: 1, failed: 0 });
+    expect(body.results[0]).toMatchObject({ fileId: candidateRow.id, success: true });
+    expect(body.results[0]?.isDuplicate).toBeUndefined();
+    expect(body.results[0]?.bookId).toEqual(expect.any(Number));
+    expect(body.results[0]?.bookId).not.toBe(existingBookId);
+    expect(await getBookDockRow(context, candidateRow.id)).toBeUndefined();
+  });
+
  it('finalize stores a folder path even when the naming pattern is flat', async () => {
    const destination = await createLibraryWithFolder(context);
    await context.db.update(schema.libraries).set({ fileNamingPattern: '{title}' }).where(eq(schema.libraries.id, destination.libraryId));