diff --git a/apps/server/src/books/books-service.test.ts b/apps/server/src/books/books-service.test.ts index 26a317f1..b94a7e94 100644 --- a/apps/server/src/books/books-service.test.ts +++ b/apps/server/src/books/books-service.test.ts @@ -72,6 +72,13 @@ describe(BooksService.withData, () => { expect(result.started_reading).toEqual(1720898518000); }); + + it('returns 0 if no stats are available', async () => { + const book1 = await createBook(db, { title: 'Test Book 1' }); + const result = await BooksService.withData(book1); + + expect(result.started_reading).toEqual(0); + }); }); describe('read_per_day', () => { diff --git a/apps/server/src/books/books-service.ts b/apps/server/src/books/books-service.ts index 4532a08c..95480d27 100644 --- a/apps/server/src/books/books-service.ts +++ b/apps/server/src/books/books-service.ts @@ -16,6 +16,7 @@ export class BooksService { } static getStartedReading(stats: PageStat[]): number { + if (stats.length === 0) return 0; return stats.reduce((acc, stat) => Math.min(acc, stat.start_time), Infinity); } diff --git a/apps/server/src/upload/upload-service-annotations.test.ts b/apps/server/src/upload/upload-service-annotations.test.ts index 06a62c38..f585ecda 100644 --- a/apps/server/src/upload/upload-service-annotations.test.ts +++ b/apps/server/src/upload/upload-service-annotations.test.ts @@ -1,8 +1,9 @@ -import { KoReaderAnnotation } from '@koinsight/common/types'; +import { KoReaderAnnotation, KoReaderBook, PageStat } from '@koinsight/common/types'; import { createBook } from '../db/factories/book-factory'; import { createDevice } from '../db/factories/device-factory'; import { db } from '../knex'; import { AnnotationsRepository } from '../annotations/annotations-repository'; +import { UploadService } from './upload-service'; describe('UploadService with annotations', () => { describe('annotation data structure from KoReader', () => { @@ -57,14 +58,14 @@ describe('UploadService with annotations', () => { expect(counts.bookmark).toBe(1); // Verify annotation content - const highlight = importedAnnotations.find(a => a.annotation_type === 'highlight'); + const highlight = importedAnnotations.find((a) => a.annotation_type === 'highlight'); expect(highlight?.text).toBe('First highlight from KoReader'); expect(highlight?.color).toBe('yellow'); - const note = importedAnnotations.find(a => a.annotation_type === 'note'); + const note = importedAnnotations.find((a) => a.annotation_type === 'note'); expect(note?.note).toBe('This is important!'); - const bookmark = importedAnnotations.find(a => a.annotation_type === 'bookmark'); + const bookmark = importedAnnotations.find((a) => a.annotation_type === 'bookmark'); expect(bookmark?.pageno).toBe(50); }); @@ -119,3 +120,97 @@ describe('UploadService with annotations', () => { }); }); }); + +describe('UploadService page stat input hardening', () => { + const makeBook = (md5: string): KoReaderBook => ({ + id: 1, + md5, + title: 'Test Book', + authors: 'Author', + series: '', + language: 'en', + notes: 0, + highlights: 0, + last_open: 0, + pages: 100, + total_read_time: 0, + total_read_pages: 0, + }); + + it('succeeds when stats is a non-array object (annotation-only path)', async () => { + const book = await createBook(db, { md5: 'hardening-test-1' }); + + await expect( + UploadService.uploadStatisticData([makeBook(book.md5)], {} as unknown as PageStat[]) + ).resolves.not.toThrow(); + + const rows = await db('page_stat').where({ book_md5: book.md5 }); + expect(rows).toHaveLength(0); + }); + + it('drops null/undefined/non-object entries in stats array', async () => { + const book = await createBook(db, { md5: 'hardening-test-2' }); + const device = await createDevice(db); + + const statsWithJunk = [ + null, + undefined, + 'not-an-object', + 42, + { + device_id: device.id, + book_md5: book.md5, + page: 1, + start_time: 1000, + duration: 10, + total_pages: 100, + }, + ] as unknown as PageStat[]; + + await expect( + UploadService.uploadStatisticData([makeBook(book.md5)], statsWithJunk) + ).resolves.not.toThrow(); + + const rows = await db('page_stat').where({ book_md5: book.md5 }); + expect(rows).toHaveLength(1); + }); + + it('drops stats with zero or invalid duration/total_pages', async () => { + const book = await createBook(db, { md5: 'hardening-test-3' }); + const device = await createDevice(db); + + const invalidStats: PageStat[] = [ + { + device_id: device.id, + book_md5: book.md5, + page: 1, + start_time: 1000, + duration: 0, + total_pages: 100, + }, + { + device_id: device.id, + book_md5: book.md5, + page: 2, + start_time: 1001, + duration: 10, + total_pages: 0, + }, + { + device_id: device.id, + book_md5: book.md5, + page: 3, + start_time: 1002, + duration: -1, + total_pages: 100, + }, + ]; + + await expect( + UploadService.uploadStatisticData([makeBook(book.md5)], invalidStats) + ).resolves.not.toThrow(); + + const rows = await db('page_stat').where({ book_md5: book.md5 }); + expect(rows).toHaveLength(0); + }); +}); diff --git a/apps/server/src/upload/upload-service.ts b/apps/server/src/upload/upload-service.ts index febf9375..e799e92d 100644 --- a/apps/server/src/upload/upload-service.ts +++ b/apps/server/src/upload/upload-service.ts @@ -45,6 +45,18 @@ export class UploadService { deviceIdOverride?: string // For annotation sync path without stats ) { return db.transaction(async (trx) => { + // Normalize: the plugin sends {} (empty Lua table → JSON object) on the + // annotation-only path, not []. Guard all array operations against this, + // and drop clearly invalid page stat rows. + const safePageStats = (Array.isArray(newPageStats) ? newPageStats : []).filter( + (s) => + s != null && + typeof s === 'object' && + Number.isFinite(s.duration) && + s.duration > 0 && + Number.isFinite(s.total_pages) && + s.total_pages > 0 + ); // Insert books const newBooks: Partial[] = booksToImport.map((book) => ({ id: book.id, @@ -60,10 +72,8 @@ export class UploadService { ); // Determine device ID: from stats, override, or fall back to unknown device - const deviceId = - newPageStats.length > 0 - ? newPageStats[0].device_id - : deviceIdOverride || this.UNKNOWN_DEVICE_ID; + const firstValidStat = safePageStats.find((s) => s.device_id); + const deviceId = firstValidStat?.device_id ?? deviceIdOverride ?? this.UNKNOWN_DEVICE_ID; const hasUnknownDevices = deviceId === this.UNKNOWN_DEVICE_ID; @@ -97,8 +107,14 @@ export class UploadService { const { book_md5, device_id, total_read_time, total_read_pages, ...otherFields } = bookDevice; - // Always merge these fields - const fieldsToMerge: (keyof BookDevice)[] = ['last_open', 'pages', 'notes', 'highlights']; + // Always merge these fields (last_open added conditionally below) + const fieldsToMerge: (keyof BookDevice)[] = ['pages', 'notes', 'highlights']; + + // Only merge last_open if it's a valid positive Unix timestamp (seconds) + const last_open = bookDevice.last_open; + if (Number.isFinite(last_open) && last_open > 0) { + fieldsToMerge.push('last_open'); + } // Only merge statistics fields if they have actual values (if on statistics.db sync path) // This prevents annotation-only syncs from overwriting with zeros @@ -116,10 +132,10 @@ export class UploadService { }) ); - // Insert page stats (only on stats sync path! there are non for annotation sync path) - if (newPageStats.length > 0) { + // Insert page stats (only on stats sync path! there are none for annotation sync path) + if (safePageStats.length > 0) { await Promise.all( - newPageStats.map((pageStat) => + safePageStats.map((pageStat) => trx('page_stat') .insert(pageStat) .onConflict(['device_id', 'book_md5', 'page', 'start_time']) diff --git a/apps/web/src/pages/book-page/book-page.tsx b/apps/web/src/pages/book-page/book-page.tsx index d54f3ea6..5b1b4a3f 100644 --- a/apps/web/src/pages/book-page/book-page.tsx +++ b/apps/web/src/pages/book-page/book-page.tsx @@ -157,7 +157,8 @@ function StatsCard({ book }: { book: BookWithData }): JSX.Element { book?.device_data.reduce((acc, device) => Math.max(acc, device.pages), 0) || 0; - const avgPerDay = book ? book.total_read_time / Object.keys(book.read_per_day).length : 0; + const readingDays = book ? Object.keys(book.read_per_day).length : 0; + const avgPerDay = readingDays > 0 ? (book?.total_read_time ?? 0) / readingDays : 0; return (