fix(stats): route KOReader hash collisions safely

Track colliding KOReader statistics rows independently and require verified explicit targets before upload. Keep unresolved history pending with a visible recovery path.

Fixes #1457

server/src/modules/koreader/dto/koreader-plugin.dto.ts

@@ -119,6 +119,11 @@ export class PageStatsBookDto {
   @Matches(MD5_HEX)
   hash!: string;
 
+  @IsOptional()
+  @IsInt()
+  @Min(1)
+  bookFileId?: number;
+
   @IsArray()
   @ArrayMinSize(1)
   @ValidateNested({ each: true })

server/src/modules/koreader/koreader-plugin-source.test.ts

@@ -321,22 +321,26 @@ describe('KOReader plugin update source wiring', () => {
     expect(statsReader).toContain('function BookOrbitStatsReader.primeIdentity(md5)');
     expect(statsReader).toContain('local book = BookOrbitStatsReader.getBook(md5)');
     expect(bookSync).toContain('local current_stats_id = stats and tonumber(stats.id_curr_book) or nil');
-    expect(bookSync).toContain('local stats_ambiguous = current_stats_id == nil and metadata.metadata_ambiguous == true');
+    expect(bookSync).toContain('local stats_ambiguous = current_stats_id == nil and metadata.stats_ambiguous == true');
     expect(bookSync).toContain('table.insert(stats_ids, current_stats_id)');
     expect(bookSync).toContain('title = stats_ambiguous and titleFromFile(file) or (metadata.title or titleFromFile(file))');
     expect(bookSync).toContain('authors = stats_ambiguous and nil or metadata.authors');
     expect(bookSync).toContain('last_open = metadata.last_open or ts');
     expect(bookSync).toContain('metadata_ambiguous = false');
     expect(bookSync).toContain('stats_metadata_ambiguous = stats_ambiguous');
+    expect(bookSync).toContain('stats_row_ambiguous = current_stats_row ~= nil and metadata.stats_ambiguous == true');
+    expect(bookSync).toContain('and not ctx.snap.stats_row_ambiguous');
     expect(bookSync).toContain('local body, err = ctx.client:matchCheck({ ctx.snap.digest }, {');
     expect(bookSync).toContain('title = ctx.snap.title');
     expect(bookSync).toContain('authors = ctx.snap.authors');
     expect(bookSync).toContain('last_open = ctx.snap.last_open');
     expect(bookSync).toContain('source = "current_file"');
-    expect(bookSync).toContain('book_file_id = book and book.fileId or nil');
+    expect(bookSync).toContain('book_file_id = matchedFileCandidate(ctx, book)');
+    expect(bookSync).toContain('if not ctx.snap.stats_row_ambiguous then return book and book.fileId or nil end');
     expect(bookSync).toContain('metadata_ambiguous = ctx.snap.metadata_ambiguous');
     expect(bookSync).toContain('if ctx.snap.stats_metadata_ambiguous then');
     expect(bookSync).toContain('ctx.state:setMatched(match.hash, match.bookFileId, match.bookId, ctx.snap.file)');
+    expect(bookSync).toContain('if ctx.snap.stats_row_ambiguous then upload.bookFileId = stats_target.bookFileId end');
     expect(bookSync).not.toContain(
       'if book then\n        if ctx.snap.file and not book.file then\n            book.file = ctx.snap.file\n        end\n        return step(ctx, stepStats)\n    end',
     );
@@ -360,12 +364,15 @@ describe('KOReader plugin update source wiring', () => {
     expect(api).toContain('source = validMatchSource(cand.source)');
     expect(api).toContain('metadataAmbiguous = optionalBoolean(cand.metadata_ambiguous)');
     expect(statsReader).toContain('entry.metadata_ambiguous = (entry._variant_count or 0) > 1');
+    expect(statsReader).toContain('entry.stats_ambiguous = #entry.ids > 1');
     expect(sweep).toContain('source = "statistics"');
     expect(sweep).toContain('metadata_ambiguous = entry.metadata_ambiguous');
-    expect(sweep).toContain('stats_metadata_ambiguous = entry.metadata_ambiguous');
+    expect(sweep).toContain('stats_metadata_ambiguous = entry.stats_ambiguous');
     expect(sweep).toContain('local file_exists = lfs.attributes(file, "mode") == "file"');
     expect(sweep).toContain('cand.source = "file"');
     expect(sweep).toContain('cand.metadata_ambiguous = false');
-    expect(sweep).toContain('if cand.stat_ids and not cand.stats_metadata_ambiguous then');
+    expect(sweep).toContain('for _, row in ipairs(cand.stats_rows or {}) do');
+    expect(sweep).toContain('if item.book_file_id then upload.bookFileId = item.book_file_id end');
+    expect(sweep).toContain('%1 books need to be opened and synced once to recover their reading history.');
   });
 });

server/src/modules/koreader/koreader-stats.service.test.ts

@@ -1,4 +1,4 @@
-import { BadRequestException, Logger } from '@nestjs/common';
+import { BadRequestException, Logger, NotFoundException } from '@nestjs/common';
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 
 import type { RequestUser } from '../../common/types/request-user';
@@ -10,6 +10,7 @@ import {
 import type { PageStatsUploadDto } from './dto';
 import type { KoreaderPluginRepository } from './koreader-plugin.repository';
 import type { KoreaderRepository } from './koreader.repository';
+import type { BookService } from '../book/book.service';
 import { KoreaderStatsService } from './koreader-stats.service';
 import type { DerivedKoreaderSession } from './koreader-stats.util';
 
@@ -44,6 +45,7 @@ describe('KoreaderStatsService', () => {
   };
   let pluginRepo: { ingestAndDeriveForBook: ReturnType<typeof vi.fn> };
   let achievementEvents: { emit: ReturnType<typeof vi.fn> };
+  let bookService: { verifyFileAccess: ReturnType<typeof vi.fn> };
   let service: KoreaderStatsService;
 
   beforeEach(() => {
@@ -62,11 +64,13 @@ describe('KoreaderStatsService', () => {
         .mockResolvedValue({ accepted: 2, duplicates: 0, insertedSessions: [], updatedSessions: [], deletedSessions: 0 }),
     };
     achievementEvents = { emit: vi.fn() };
+    bookService = { verifyFileAccess: vi.fn() };
 
     service = new KoreaderStatsService(
       koreaderRepo as unknown as KoreaderRepository,
       pluginRepo as unknown as KoreaderPluginRepository,
       achievementEvents as unknown as AchievementEventsService,
+      bookService as unknown as BookService,
     );
   });
 
@@ -121,6 +125,108 @@ describe('KoreaderStatsService', () => {
     expect(result.results[0]).toEqual({ hash: HASH_A, accepted: 0, duplicates: 2, watermark: 9000 });
   });
 
+  it('routes a colliding statistics row to its explicit accessible file when the hash matches', async () => {
+    koreaderRepo.resolveBookFilesByHashes.mockResolvedValue(new Map());
+    bookService.verifyFileAccess.mockResolvedValue({
+      id: 11,
+      bookId: 21,
+      libraryId: 1,
+      format: 'epub',
+      role: 'content',
+      fileHash: HASH_A,
+    });
+    const dto = makeDto([
+      {
+        hash: HASH_A,
+        bookFileId: 11,
+        events: [{ page: 1, startTime: 1000, durationSeconds: 30, totalPages: 100 }],
+      },
+    ]);
+
+    const result = await service.uploadPageStats(makeUser(), dto);
+
+    expect(bookService.verifyFileAccess).toHaveBeenCalledWith(11, makeUser());
+    expect(pluginRepo.ingestAndDeriveForBook).toHaveBeenCalledWith(expect.objectContaining({ bookFileId: 11, bookId: 21, libraryId: 1 }));
+    expect(result.unmatched).toEqual([]);
+  });
+
+  it('refuses an explicit statistics target whose server hash does not match', async () => {
+    bookService.verifyFileAccess.mockResolvedValue({
+      id: 11,
+      bookId: 21,
+      libraryId: 1,
+      format: 'epub',
+      role: 'content',
+      fileHash: HASH_B,
+    });
+    const dto = makeDto([
+      {
+        hash: HASH_A,
+        bookFileId: 11,
+        events: [{ page: 1, startTime: 1000, durationSeconds: 30, totalPages: 100 }],
+      },
+    ]);
+
+    const result = await service.uploadPageStats(makeUser(), dto);
+
+    expect(pluginRepo.ingestAndDeriveForBook).not.toHaveBeenCalled();
+    expect(result.unmatched).toEqual([HASH_A]);
+  });
+
+  it('refuses an inaccessible explicit statistics target instead of falling back to the hash match', async () => {
+    bookService.verifyFileAccess.mockRejectedValue(new NotFoundException());
+    const dto = makeDto([
+      {
+        hash: HASH_A,
+        bookFileId: 11,
+        events: [{ page: 1, startTime: 1000, durationSeconds: 30, totalPages: 100 }],
+      },
+    ]);
+
+    const result = await service.uploadPageStats(makeUser(), dto);
+
+    expect(pluginRepo.ingestAndDeriveForBook).not.toHaveBeenCalled();
+    expect(result.unmatched).toEqual([HASH_A]);
+  });
+
+  it('refuses a non-content file as an explicit statistics target', async () => {
+    bookService.verifyFileAccess.mockResolvedValue({
+      id: 11,
+      bookId: 21,
+      libraryId: 1,
+      format: 'jpg',
+      role: 'cover',
+      fileHash: HASH_A,
+    });
+    const dto = makeDto([
+      {
+        hash: HASH_A,
+        bookFileId: 11,
+        events: [{ page: 1, startTime: 1000, durationSeconds: 30, totalPages: 100 }],
+      },
+    ]);
+
+    const result = await service.uploadPageStats(makeUser(), dto);
+
+    expect(pluginRepo.ingestAndDeriveForBook).not.toHaveBeenCalled();
+    expect(result.unmatched).toEqual([HASH_A]);
+  });
+
+  it('does not disguise an unexpected explicit-target lookup failure as an unmatched book', async () => {
+    const failure = new Error('database unavailable');
+    bookService.verifyFileAccess.mockRejectedValue(failure);
+    const dto = makeDto([
+      {
+        hash: HASH_A,
+        bookFileId: 11,
+        events: [{ page: 1, startTime: 1000, durationSeconds: 30, totalPages: 100 }],
+      },
+    ]);
+
+    await expect(service.uploadPageStats(makeUser(), dto)).rejects.toBe(failure);
+    expect(pluginRepo.ingestAndDeriveForBook).not.toHaveBeenCalled();
+  });
+
   it('emits one reading-session event per inserted session below the backfill threshold', async () => {
     pluginRepo.ingestAndDeriveForBook.mockResolvedValue({
       accepted: 2,

server/src/modules/koreader/koreader-stats.service.ts

@@ -1,4 +1,4 @@
-import { BadRequestException, Injectable, Logger } from '@nestjs/common';
+import { BadRequestException, HttpException, Injectable, Logger } from '@nestjs/common';
 
 import type { UserSettings } from '@bookorbit/types';
 import type { RequestUser } from '../../common/types/request-user';
@@ -9,6 +9,7 @@ import {
   ACHIEVEMENT_EVENT_READING_SESSION_SAVED,
   AchievementEventsService,
 } from '../achievement/achievement-events.service';
+import { BookService } from '../book/book.service';
 import type { PageStatsUploadDto } from './dto';
 import { KoreaderPluginRepository } from './koreader-plugin.repository';
 import { KoreaderRepository } from './koreader.repository';
@@ -37,6 +38,7 @@ export class KoreaderStatsService {
     private readonly koreaderRepo: KoreaderRepository,
     private readonly pluginRepo: KoreaderPluginRepository,
     private readonly achievementEvents: AchievementEventsService,
+    private readonly bookService: BookService,
   ) {}
 
   async uploadPageStats(user: RequestUser, dto: PageStatsUploadDto): Promise<PageStatsUploadResult> {
@@ -65,7 +67,23 @@ export class KoreaderStatsService {
 
       for (const book of dto.books) {
         const hash = book.hash.toLowerCase();
-        const match = matches.get(hash);
+        let match = matches.get(hash);
+        if (book.bookFileId) {
+          const explicitFile = await this.bookService.verifyFileAccess(book.bookFileId, user).catch((error: unknown) => {
+            if (error instanceof HttpException) return null;
+            throw error;
+          });
+          if (explicitFile?.role === 'content' && explicitFile.fileHash?.toLowerCase() === hash) {
+            match = {
+              bookFileId: explicitFile.id,
+              bookId: explicitFile.bookId,
+              libraryId: explicitFile.libraryId,
+              format: explicitFile.format,
+            };
+          } else {
+            match = undefined;
+          }
+        }
         if (!match) {
           unmatched.push(hash);
           continue;

server/test/koreader-page-stats.e2e-spec.ts

@@ -53,7 +53,7 @@ describe('KOReader page stats derivation (e2e)', { timeout: 180_000 }, () => {
     return { 'x-auth-user': KOREADER_USERNAME, 'x-auth-key': KOREADER_PASSWORD };
   }
 
-  async function uploadPageStats(events: KoreaderPageEvent[]) {
+  async function uploadPageStats(events: KoreaderPageEvent[], bookFileId?: number) {
     const response = await ctx.app.inject({
       method: 'POST',
       url: '/api/v1/koreader/plugin/page-stats',
@@ -62,7 +62,7 @@ describe('KOReader page stats derivation (e2e)', { timeout: 180_000 }, () => {
         deviceId: DEVICE_ID,
         deviceModel: 'E2E',
         pluginVersion: '0.4.0',
-        books: [{ hash: fileHash, events }],
+        books: [{ hash: fileHash, bookFileId, events }],
       },
     });
     expect(response.statusCode).toBe(201);
@@ -310,10 +310,22 @@ describe('KOReader page stats derivation (e2e)', { timeout: 180_000 }, () => {
     await uploadPageStats([{ page: 1, startTime: firstStart, durationSeconds: 55, totalPages: TOTAL_PAGES }]);
     await uploadPageStats([{ page: 2, startTime: firstStart + 60, durationSeconds: 55, totalPages: TOTAL_PAGES }]);
 
+    const explicitStart = firstStart + 120;
+    const explicit = await uploadPageStats([{ page: 3, startTime: explicitStart, durationSeconds: 55, totalPages: TOTAL_PAGES }], sharing[1]!.id);
+    expect(explicit.results).toEqual([{ hash: fileHash, accepted: 1, duplicates: 0, watermark: explicitStart }]);
+
+    const [explicitRow] = await ctx.db
+      .select({ bookFileId: schema.koreaderPageStats.bookFileId })
+      .from(schema.koreaderPageStats)
+      .where(and(eq(schema.koreaderPageStats.deviceId, DEVICE_ID), eq(schema.koreaderPageStats.startTime, explicitStart)));
+    expect(explicitRow?.bookFileId).toBe(sharing[1]!.id);
+
     const targets = await ctx.db
       .selectDistinct({ bookFileId: schema.koreaderPageStats.bookFileId })
       .from(schema.koreaderPageStats)
       .where(and(eq(schema.koreaderPageStats.deviceId, DEVICE_ID), gte(schema.koreaderPageStats.startTime, firstStart)));
-    expect(targets.map((row) => row.bookFileId)).toEqual([epub.bookFileId]);
+    expect(targets.map((row) => row.bookFileId).sort((left, right) => left - right)).toEqual(
+      [epub.bookFileId, sharing[1]!.id].sort((left, right) => left - right),
+    );
   });
 });