mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(kobo): keep a Kobo's progress on the file its copy was built from
When a book's primary file changed, the Kobo kept the copy built from the old file but its positions no longer converted, so it was sent a bare percentage, reset to the start, and that reset was saved over real progress. Sync each device against its copy's source file instead, which BookOrbit's sibling sync carries to the other EPUBs, and serve the new primary when the device downloads the book again. Fixes #1786
This commit is contained in:
@@ -18,6 +18,15 @@ export class KoboReadingProgressRepository {
|
||||
return file?.fileId ?? null;
|
||||
}
|
||||
|
||||
async isReaderFileOfBook(bookId: number, fileId: number): Promise<boolean> {
|
||||
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({
|
||||
|
||||
@@ -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' }));
|
||||
|
||||
@@ -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<DeliveryArtifact> {
|
||||
|
||||
@@ -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' },
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -68,7 +68,17 @@ export class KoboKepubContextService {
|
||||
}
|
||||
}
|
||||
|
||||
async resolveForBook(userId: number, bookId: number, deviceId?: number): Promise<KepubContextResult> {
|
||||
/**
|
||||
* `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<KepubContextResult> {
|
||||
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 };
|
||||
}
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -70,7 +70,7 @@ export class KoboProgressBridgeService {
|
||||
deviceId?: number,
|
||||
): Promise<CanonicalProgressPoint | null> {
|
||||
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<KoboBookmarkPoint | null> {
|
||||
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 {
|
||||
|
||||
@@ -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, {
|
||||
|
||||
@@ -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<number | null> {
|
||||
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<boolean> {
|
||||
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;
|
||||
|
||||
Reference in New Issue
Block a user