diff --git a/server/src/modules/kobo/kobo-reading-progress.repository.ts b/server/src/modules/kobo/kobo-reading-progress.repository.ts index f3f4c787f..b92649d04 100644 --- a/server/src/modules/kobo/kobo-reading-progress.repository.ts +++ b/server/src/modules/kobo/kobo-reading-progress.repository.ts @@ -18,6 +18,15 @@ export class KoboReadingProgressRepository { return file?.fileId ?? null; } + async isReaderFileOfBook(bookId: number, fileId: number): Promise { + const [file] = await this.db + .select({ id: schema.bookFiles.id }) + .from(schema.bookFiles) + .where(and(eq(schema.bookFiles.id, fileId), eq(schema.bookFiles.bookId, bookId), inArray(schema.bookFiles.format, ['epub', 'kepub']))) + .limit(1); + return Boolean(file); + } + async findForFile(userId: number, fileId: number) { const [progress] = await this.db .select({ diff --git a/server/src/modules/kobo/services/kobo-delivery.service.test.ts b/server/src/modules/kobo/services/kobo-delivery.service.test.ts index afd40c2f9..984c6efda 100644 --- a/server/src/modules/kobo/services/kobo-delivery.service.test.ts +++ b/server/src/modules/kobo/services/kobo-delivery.service.test.ts @@ -44,11 +44,12 @@ function makeService() { transition: vi.fn().mockResolvedValue([{}]), adoptForDeviceReadingState: vi.fn().mockResolvedValue(undefined), }; - const service = new KoboDeliveryService(repository as never, {} as never, {} as never, {} as never, {} as never, {} as never, {} as never); + const downloads = { findBook: vi.fn().mockResolvedValue({ id: 3, primaryFileId: 4 }) }; + const service = new KoboDeliveryService(repository as never, downloads as never, {} as never, {} as never, {} as never, {} as never, {} as never); const offer = vi.spyOn(service, 'offer'); const resolve = vi.spyOn(service, 'resolve'); const artifact = (id: string) => ({ path: `/cache/${id}`, release: vi.fn() }) as never; - return { service, repository, offer, resolve, artifact }; + return { service, repository, downloads, offer, resolve, artifact }; } describe('KoboDeliveryService.prepareDownload', () => { @@ -126,6 +127,21 @@ describe('KoboDeliveryService.prepareDownload', () => { }, ); + it('serves a fresh offer of the new primary when the device downloads a copy built from a former primary', async () => { + const { service, repository, downloads, offer, resolve, artifact } = makeService(); + downloads.findBook.mockResolvedValue({ id: 3, primaryFileId: 9 }); + const fresh = record({ id: 'fresh', sourceFileId: 9, state: 'offered' }); + offer.mockResolvedValueOnce(record()).mockResolvedValueOnce(fresh); + const freshArtifact = artifact('fresh'); + resolve.mockResolvedValueOnce(freshArtifact); + + await expect(service.prepareDownload(1, 2, 3)).resolves.toEqual({ record: fresh, artifact: freshArtifact }); + + expect(repository.retire).toHaveBeenCalledWith(1, 2, 'stale'); + expect(resolve).toHaveBeenCalledTimes(1); + expect(resolve).toHaveBeenCalledWith(1, 2, fresh); + }); + it('rejects a delivery id that is no longer current without preparing any bytes', async () => { const { service, offer, resolve } = makeService(); offer.mockResolvedValue(record({ id: 'fresh' })); diff --git a/server/src/modules/kobo/services/kobo-delivery.service.ts b/server/src/modules/kobo/services/kobo-delivery.service.ts index 853862ba3..bf2c91d1a 100644 --- a/server/src/modules/kobo/services/kobo-delivery.service.ts +++ b/server/src/modules/kobo/services/kobo-delivery.service.ts @@ -149,8 +149,9 @@ export class KoboDeliveryService { /** * The bytes a device download is served from. A download asks for the book now, so an offer whose source - * can no longer rebuild it, because the file changed after it was offered, is replaced by a fresh offer of - * the current file instead of failing that book on every attempt. Pending replacements keep their guided flow. + * can no longer rebuild it, because the file changed after it was offered or stopped being the book's + * primary, is replaced by a fresh offer of the current primary instead of serving a stale copy or failing + * that book on every attempt. Pending replacements keep their guided flow. */ async prepareDownload( userId: number, @@ -160,27 +161,43 @@ export class KoboDeliveryService { ): Promise<{ record: KoboDeliveryRecord; artifact: DeliveryArtifact }> { const record = await this.offer(userId, deviceId, bookId); if (deliveryId && record.id !== deliveryId) throw new NotFoundException('Delivery offer is no longer current'); - try { - return { record, artifact: await this.resolve(userId, deviceId, record) }; - } catch (error) { - const code = deliveryErrorCode(error); - if (!record.current || record.options.replacement || !code || !STALE_DELIVERY_CODES.has(code)) throw error; - const startedAt = Date.now(); - await this.repository.retire(userId, deviceId, record.id); - const fresh = await this.offer(userId, deviceId, bookId); - // The device learns the fresh offer's size and URL on its next sync either way. - await this.repository.reoffer(userId, deviceId, bookId); - if (fresh.metadata.format !== record.metadata.format) { - this.logger.warn( - `[kobo.delivery_refresh] [fail] userId=${userId} deviceId=${deviceId} bookId=${bookId} staleId=${record.id} freshId=${fresh.id} reason=${code} durationMs=${Date.now() - startedAt} - fresh offer changes format, the device must sync before downloading`, - ); - throw new ConflictException({ code: 'KOBO_DELIVERY_FORMAT_CHANGED' }); - } - this.logger.log( - `[kobo.delivery_refresh] [end] userId=${userId} deviceId=${deviceId} bookId=${bookId} staleId=${record.id} freshId=${fresh.id} reason=${code} durationMs=${Date.now() - startedAt} - stale offer replaced from the current file`, - ); - return { record: fresh, artifact: await this.resolve(userId, deviceId, fresh) }; + const replaceable = record.current && !record.options.replacement; + let reason: string | null = null; + if (replaceable) { + const book = await this.downloads.findBook(bookId); + if (book?.primaryFileId != null && record.sourceFileId !== book.primaryFileId) reason = 'PRIMARY_FILE_CHANGED'; } + if (!reason) { + try { + return { record, artifact: await this.resolve(userId, deviceId, record) }; + } catch (error) { + const code = deliveryErrorCode(error); + if (!replaceable || !code || !STALE_DELIVERY_CODES.has(code)) throw error; + reason = code; + } + } + const startedAt = Date.now(); + await this.repository.retire(userId, deviceId, record.id); + const fresh = await this.offer(userId, deviceId, bookId); + // The device learns the fresh offer's size and URL on its next sync either way. + await this.repository.reoffer(userId, deviceId, bookId); + if (fresh.metadata.format !== record.metadata.format) { + this.logger.warn( + `[kobo.delivery_refresh] [fail] userId=${userId} deviceId=${deviceId} bookId=${bookId} staleId=${record.id} freshId=${fresh.id} reason=${reason} durationMs=${Date.now() - startedAt} - fresh offer changes format, the device must sync before downloading`, + ); + throw new ConflictException({ code: 'KOBO_DELIVERY_FORMAT_CHANGED' }); + } + this.logger.log( + `[kobo.delivery_refresh] [end] userId=${userId} deviceId=${deviceId} bookId=${bookId} staleId=${record.id} freshId=${fresh.id} reason=${reason} durationMs=${Date.now() - startedAt} - stale offer replaced from the current file`, + ); + return { record: fresh, artifact: await this.resolve(userId, deviceId, fresh) }; + } + + /** The book file a delivery was built from, while it still belongs to the book as an EPUB. */ + async findSourceFile(record: KoboDeliveryRecord) { + if (record.sourceFileId === null) return null; + const file = await this.downloads.findPrimaryFile(record.bookId, record.sourceFileId); + return file && (file.format === 'epub' || file.format === 'kepub') ? file : null; } async resolve(userId: number, deviceId: number, record: KoboDeliveryRecord): Promise { diff --git a/server/src/modules/kobo/services/kobo-kepub-context.service.test.ts b/server/src/modules/kobo/services/kobo-kepub-context.service.test.ts index aae667d03..7c81432ea 100644 --- a/server/src/modules/kobo/services/kobo-kepub-context.service.test.ts +++ b/server/src/modules/kobo/services/kobo-kepub-context.service.test.ts @@ -44,4 +44,46 @@ describe('KoboKepubContextService', () => { expect(await makeContext('pdf').service.resolveForBook(7, 3)).toMatchObject({ ok: false, reason: 'kepub_required' }); expect(await makeContext('epub', false).service.resolveForBook(7, 3)).toMatchObject({ ok: false, reason: 'kepub_required' }); }); + + it('resolves a device copy built from a former primary only for callers that follow its source file', async () => { + const { file, conversion, binary } = makeContext('epub'); + const reader = { findPrimaryReaderFilesByBookIds: vi.fn().mockResolvedValue([file]) }; + const source = { + id: 66, + bookId: 3, + absolutePath: '/library/readalong.epub', + format: 'epub', + fileHash: 'old', + sizeBytes: 900, + mediaOverlayAvailable: true, + }; + const release = vi.fn().mockResolvedValue(undefined); + const deliveries = { + current: vi.fn().mockResolvedValue({ + id: 'copy', + state: 'transfer_observed', + previousId: null, + sourceFileId: 66, + options: { hyphenate: false, converterVersion: 'v1' }, + metadata: { format: 'kepub' }, + }), + resolve: vi.fn().mockResolvedValue({ path: '/cache/copy.kepub.epub', sha256: 'sha', release }), + findSourceFile: vi.fn().mockResolvedValue(source), + }; + const service = new KoboKepubContextService( + reader as never, + { getSettings: vi.fn().mockResolvedValue({ convertToKepub: true }) } as never, + conversion as never, + binary as never, + deliveries as never, + ); + + await expect(service.resolveForBook(7, 3, 11)).resolves.toMatchObject({ ok: false, reason: 'historical_source_unavailable' }); + expect(release).toHaveBeenCalled(); + await expect(service.resolveForBook(7, 3, 11, { followDeliverySource: true })).resolves.toMatchObject({ + ok: true, + file: { id: 66, absolutePath: '/library/readalong.epub' }, + ctx: { kepubPath: '/cache/copy.kepub.epub', fileHash: 'sha' }, + }); + }); }); diff --git a/server/src/modules/kobo/services/kobo-kepub-context.service.ts b/server/src/modules/kobo/services/kobo-kepub-context.service.ts index 399bc5861..6659dd01e 100644 --- a/server/src/modules/kobo/services/kobo-kepub-context.service.ts +++ b/server/src/modules/kobo/services/kobo-kepub-context.service.ts @@ -68,7 +68,17 @@ export class KoboKepubContextService { } } - async resolveForBook(userId: number, bookId: number, deviceId?: number): Promise { + /** + * `followDeliverySource` resolves a device whose copy was built from a file that is no longer the + * book's primary against that file, for callers that keep the device's progress on it. Without it such + * a copy is unavailable, since its positions do not belong to the primary file. + */ + async resolveForBook( + userId: number, + bookId: number, + deviceId?: number, + options: { followDeliverySource?: boolean } = {}, + ): Promise { if (deviceId && this.deliveries) { const settings = await this.koboSettingsService.getSettings(userId); const record = await this.deliveries.current(userId, deviceId, bookId); @@ -79,8 +89,23 @@ export class KoboKepubContextService { if (record.metadata.format !== 'kepub') return { ok: false, reason: 'kepub_required', settings }; try { const artifact = await this.deliveries.resolve(userId, deviceId, record); - const file = await this.resolveReaderFile(bookId); - if (!file || file.id !== record.sourceFileId) { + const primary = await this.resolveReaderFile(bookId); + const source = primary?.id !== record.sourceFileId && options.followDeliverySource ? await this.deliveries.findSourceFile(record) : null; + const file: KepubReaderFile | null = + primary && primary.id === record.sourceFileId + ? primary + : source + ? { + id: source.id, + bookId: source.bookId, + absolutePath: source.absolutePath, + format: source.format, + fileHash: source.fileHash, + sizeBytes: source.sizeBytes, + mediaOverlayAvailable: source.mediaOverlayAvailable, + } + : null; + if (!file) { await artifact.release(); return { ok: false, reason: 'historical_source_unavailable', settings }; } diff --git a/server/src/modules/kobo/services/kobo-progress-bridge.service.test.ts b/server/src/modules/kobo/services/kobo-progress-bridge.service.test.ts index 08c51485b..36131879f 100644 --- a/server/src/modules/kobo/services/kobo-progress-bridge.service.test.ts +++ b/server/src/modules/kobo/services/kobo-progress-bridge.service.test.ts @@ -38,7 +38,7 @@ describe('KoboProgressBridgeService', () => { makeService().narrationToKoboBookmark(1, 44, 55, { fragment: 'OEBPS/xhtml/chapter6.xhtml#chapter6-s161', sectionIndex: 9 }, 7), ).resolves.toEqual({ source: 'OEBPS/xhtml/chapter6.xhtml', value: 'kobo.43.3', contentSourceProgressPercent: 61.9 }); expect(positionConverter.fragmentToPositions).toHaveBeenCalledWith({ bookFileId: 55, chapterIndex: 9, fragment: 'chapter6-s161' }); - expect(kepubContextService.resolveForBook).toHaveBeenCalledWith(1, 44, 7); + expect(kepubContextService.resolveForBook).toHaveBeenCalledWith(1, 44, 7, { followDeliverySource: true }); }); it('degrades to a percent and says why when the narrated sentence cannot be found', async () => { diff --git a/server/src/modules/kobo/services/kobo-progress-bridge.service.ts b/server/src/modules/kobo/services/kobo-progress-bridge.service.ts index c7af5a291..0a4043911 100644 --- a/server/src/modules/kobo/services/kobo-progress-bridge.service.ts +++ b/server/src/modules/kobo/services/kobo-progress-bridge.service.ts @@ -70,7 +70,7 @@ export class KoboProgressBridgeService { deviceId?: number, ): Promise { try { - const kepub = await this.kepubContextService.resolveForBook(userId, bookId, deviceId); + const kepub = await this.kepubContextService.resolveForBook(userId, bookId, deviceId, { followDeliverySource: true }); if (!kepub.ok) return this.logDegraded('bookmark_to_canonical', userId, bookId, kepub.reason); try { @@ -95,7 +95,7 @@ export class KoboProgressBridgeService { async cfiToKoboBookmark(userId: number, bookId: number, cfi: string, deviceId?: number): Promise { try { - const kepub = await this.kepubContextService.resolveForBook(userId, bookId, deviceId); + const kepub = await this.kepubContextService.resolveForBook(userId, bookId, deviceId, { followDeliverySource: true }); if (!kepub.ok) return this.logDegraded('cfi_to_bookmark', userId, bookId, kepub.reason); try { diff --git a/server/src/modules/kobo/services/kobo-reading-state.service.test.ts b/server/src/modules/kobo/services/kobo-reading-state.service.test.ts index 762db723f..f381172be 100644 --- a/server/src/modules/kobo/services/kobo-reading-state.service.test.ts +++ b/server/src/modules/kobo/services/kobo-reading-state.service.test.ts @@ -786,6 +786,17 @@ describe('KoboReadingStateService', () => { expect(insert.values).toHaveBeenCalledWith(expect.objectContaining({ bookFileId: 55, percentage, cfi: null })); expect(userBookStatusService.autoUpdate).toHaveBeenCalled(); }); + it('keeps a device copy built from a file that stopped being primary on that file', async () => { + const { db, insert } = resetDb(); + const queue = [[{ fileId: 55 }], [{ id: 66 }], [{ percentage: 61, cfi: savedCfi, updatedAt: new Date(previousAt) }]]; + db.select.mockImplementation(() => makeSelectChain(queue.shift() ?? [])); + await makeService(db, { + current: vi.fn().mockResolvedValue({ id: 'old', state: 'transfer_observed', options: {}, sourceFileId: 66 }), + }).upsertState(1, 5, { CurrentBookmark: { ProgressPercent: 70, LastModified: incomingAt } }, 1, 99, true, 77); + expect(insert.values).toHaveBeenCalledWith(expect.objectContaining({ bookFileId: 66, percentage: 70 })); + expect(insert.values).not.toHaveBeenCalledWith(expect.objectContaining({ bookFileId: 55 })); + expect(bookService.syncAudioProgressForExternalEbookProgress).toHaveBeenCalledWith(1, 5, 66, 70, expect.anything()); + }); it('keeps the previous copy attributable while replacement is only prepared', async () => { const { db, insert } = resetDb(); await makeService(db, { diff --git a/server/src/modules/kobo/services/kobo-reading-state.service.ts b/server/src/modules/kobo/services/kobo-reading-state.service.ts index 47c0acca6..00da28677 100644 --- a/server/src/modules/kobo/services/kobo-reading-state.service.ts +++ b/server/src/modules/kobo/services/kobo-reading-state.service.ts @@ -17,7 +17,7 @@ import { } from '../kobo-statistics-session.util'; import { KoboAnalyticsResolverService } from './kobo-analytics-resolver.service'; import { KoboReadingProgressRepository } from '../kobo-reading-progress.repository'; -import { KoboDeliveryService, isSettledDelivery, isUnconfirmedReplacement } from './kobo-delivery.service'; +import { KoboDeliveryService, isSettledDelivery, isUnconfirmedReplacement, type KoboDeliveryRecord } from './kobo-delivery.service'; import { KoboBookAccessService } from './kobo-book-access.service'; import { KoboBookIdentityService } from './kobo-book-identity.service'; import { KoboProgressBridgeService, type CanonicalProgressPoint } from './kobo-progress-bridge.service'; @@ -187,7 +187,7 @@ export class KoboReadingStateService { }; let guardBackwards = uncertainDelivery || initialZero || replacedCopy; if (guardBackwards && mergedPercent !== null) { - const fileId = await this.progressRepository.findPrimaryFileId(bookId); + const fileId = await this.progressFileFor(bookId, delivery); const canonical = fileId ? await this.progressRepository.findForFile(userId, fileId) : null; const savedPercent = canonical?.percentage ?? this.extractPercent(this.asJsonObj(shared?.currentBookmark)); protectedRegression = savedPercent !== null && savedPercent > mergedPercent + PROGRESS_EPSILON; @@ -225,6 +225,7 @@ export class KoboReadingStateService { uncertainDelivery ? null : this.extractContentSourceProgressPercent(mergedBookmark), point, guardBackwards, + delivery, )); } @@ -495,7 +496,7 @@ export class KoboReadingStateService { ('deliveryId' in row && row.deliveryId !== delivery.id) || this.extractPercent(this.asJsonObj(row.currentBookmark)) === 0), ); - const refreshed = await this.refreshBookmarkFromHub(userId, bookId, row, deviceId, retainSavedProgress).catch((error: unknown) => { + const refreshed = await this.refreshBookmarkFromHub(userId, bookId, row, deviceId, retainSavedProgress, delivery).catch((error: unknown) => { const err = error instanceof Error ? error : new Error(String(error)); this.logger.warn( `[kobo.reading_state_refresh] [fail] userId=${userId} bookId=${bookId} errorClass=${err.constructor.name} error="${sanitizeLogValue(err.message)}" - hub bookmark refresh failed, serving stored bookmark`, @@ -556,12 +557,13 @@ export class KoboReadingStateService { state: { currentBookmark: unknown; lastModifiedKobo: string | null; priorityTimestamp: string | null }, deviceId?: number, retainSavedProgress = false, + delivery?: KoboDeliveryRecord | null, ): Promise<{ bookmark: JsonObj; lastModifiedKobo: string } | null> { const bookmark = this.asJsonObj(state.currentBookmark); const settings = await this.settingsService.getSettings(userId); if (!settings.twoWayProgressSync && !retainSavedProgress) return null; - const fileId = await this.progressRepository.findPrimaryFileId(bookId); + const fileId = deviceId ? await this.progressFileFor(bookId, delivery) : await this.progressRepository.findPrimaryFileId(bookId); if (!fileId) return null; const progress = await this.progressRepository.findForFile(userId, fileId); if (!progress || (!deviceId && progress.koboLocationValue)) return null; @@ -669,6 +671,18 @@ export class KoboReadingStateService { return Number.isNaN(date.getTime()) ? null : date; } + /** + * The file a device's progress is kept on: the one its copy was built from, which stops being the + * book's primary when the library's format order or the book's files change. The device's positions + * belong to that file, and BookOrbit's sibling sync carries progress from it to the book's other EPUBs. + */ + private async progressFileFor(bookId: number, delivery: KoboDeliveryRecord | null | undefined): Promise { + const primary = await this.progressRepository.findPrimaryFileId(bookId); + const source = delivery?.sourceFileId ?? null; + if (source === null || source === primary) return primary; + return (await this.progressRepository.isReaderFileOfBook(bookId, source)) ? source : primary; + } + private async syncPercentToInternalProgress( userId: number, bookId: number, @@ -680,8 +694,9 @@ export class KoboReadingStateService { koboContentSourceProgressPercent: number | null, precise: { cfi: string; xpointer: string } | null, uncertainDelivery: boolean, + delivery: KoboDeliveryRecord | null | undefined, ): Promise { - const fileId = await this.progressRepository.findPrimaryFileId(bookId); + const fileId = await this.progressFileFor(bookId, delivery); if (!fileId) return true; const existing = await this.progressRepository.findForFile(userId, fileId); if (uncertainDelivery && existing && existing.percentage > percentage + PROGRESS_EPSILON) return false;