mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(kobo): stop a replacement with no earlier copy from blocking delivery
A replacement prepared before the device ever received the book had no earlier delivery to fall back to, so every sync failed the book. It now stops standing for the book and the book gets an ordinary first offer. Cancelling a replacement the device has not fetched now clears it, hands the book back to the earlier delivery, and re-announces it. Update the download E2E suite for the stale-offer replacement from #1816. Ref #1786
This commit is contained in:
@@ -61,7 +61,12 @@ export class KoboDeliveryRepository {
|
||||
});
|
||||
}
|
||||
|
||||
async cancel(userId: number, deviceId: number, id: string) {
|
||||
/**
|
||||
* Cancels a pending replacement and returns whether it was cancelled. One the device has not fetched
|
||||
* hands the book back to the delivery it replaced, or, when it replaced nothing, stops standing for the
|
||||
* book so the next offer starts fresh.
|
||||
*/
|
||||
async cancel(userId: number, deviceId: number, id: string): Promise<boolean> {
|
||||
return this.db.transaction(async (tx) => {
|
||||
const record = await tx.query.koboDeviceDeliveries.findFirst({
|
||||
where: and(
|
||||
@@ -76,11 +81,12 @@ export class KoboDeliveryRepository {
|
||||
!record.options.replacement ||
|
||||
!['prepared', 'offered', 'transfer_observed', 'cancelled'].includes(record.state)
|
||||
)
|
||||
return;
|
||||
return false;
|
||||
await tx.execute(sql`select pg_advisory_xact_lock(${deviceId}, ${record.bookId})`);
|
||||
const unfetched = record.state === 'prepared' || record.state === 'offered';
|
||||
const cancelled = await tx
|
||||
.update(schema.koboDeviceDeliveries)
|
||||
.set({ state: 'cancelled', current: record.state === 'prepared' && record.previousId ? false : record.current, updatedAt: new Date() })
|
||||
.set({ state: 'cancelled', current: unfetched ? false : record.current, updatedAt: new Date() })
|
||||
.where(
|
||||
and(
|
||||
eq(schema.koboDeviceDeliveries.id, id),
|
||||
@@ -89,8 +95,10 @@ export class KoboDeliveryRepository {
|
||||
),
|
||||
)
|
||||
.returning();
|
||||
if (cancelled.length && record.state === 'prepared' && record.previousId)
|
||||
if (!cancelled.length) return false;
|
||||
if (unfetched && record.previousId)
|
||||
await tx.update(schema.koboDeviceDeliveries).set({ current: true }).where(eq(schema.koboDeviceDeliveries.id, record.previousId));
|
||||
return true;
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -149,3 +149,93 @@ describe('KoboDeliveryService.transferObserved', () => {
|
||||
expect(repository.adoptForDeviceReadingState).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
function makeOfferService(current: KoboDeliveryRecord | null, previous: KoboDeliveryRecord | null = null) {
|
||||
const fresh = record({ id: 'fresh', state: 'offered', metadata: { ...record().metadata, format: 'pdf' } });
|
||||
const repository = {
|
||||
assertDevice: vi.fn().mockResolvedValue(undefined),
|
||||
findCurrent: vi.fn().mockResolvedValue(current),
|
||||
findById: vi.fn().mockResolvedValue(previous),
|
||||
retire: vi.fn().mockResolvedValue(undefined),
|
||||
offer: vi.fn().mockResolvedValue(fresh),
|
||||
cancel: vi.fn().mockResolvedValue(true),
|
||||
reoffer: vi.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
const downloads = {
|
||||
findBook: vi.fn().mockResolvedValue({ id: 3, primaryFileId: 4 }),
|
||||
findPrimaryFile: vi.fn().mockResolvedValue({ id: 4, absolutePath: '/library/book.pdf', format: 'pdf', mediaOverlayAvailable: false }),
|
||||
};
|
||||
const settings = {
|
||||
getSettings: vi
|
||||
.fn()
|
||||
.mockResolvedValue({ readAloudDelivery: null, convertToKepub: false, forceEnableHyphenation: false, kepubConversionLimitMb: 100 }),
|
||||
};
|
||||
const artifacts = {
|
||||
prepare: vi.fn().mockResolvedValue({ ...fresh.metadata, path: '/library/book.pdf', release: vi.fn().mockResolvedValue(undefined) }),
|
||||
};
|
||||
const service = new KoboDeliveryService(
|
||||
repository as never,
|
||||
downloads as never,
|
||||
{ assertBookAccessible: vi.fn().mockResolvedValue(undefined) } as never,
|
||||
settings as never,
|
||||
artifacts as never,
|
||||
{} as never,
|
||||
{} as never,
|
||||
);
|
||||
return { service, repository, fresh };
|
||||
}
|
||||
|
||||
const replacement = (overrides: Partial<KoboDeliveryRecord> = {}) =>
|
||||
record({ id: 'replacement', state: 'prepared', options: { hyphenate: false, converterVersion: 'v1', replacement: true }, ...overrides });
|
||||
|
||||
describe('KoboDeliveryService.offer', () => {
|
||||
it.each(['prepared', 'cancelled'])('gives a book a first offer when a %s replacement has no earlier copy to stand back for', async (state) => {
|
||||
const { service, repository, fresh } = makeOfferService(replacement({ state }));
|
||||
|
||||
await expect(service.offer(1, 2, 3)).resolves.toBe(fresh);
|
||||
|
||||
expect(repository.retire).toHaveBeenCalledWith(1, 2, 'replacement');
|
||||
expect(repository.offer).toHaveBeenCalledWith(expect.objectContaining({ state: 'offered', sourceFileId: 4 }), false);
|
||||
});
|
||||
|
||||
it('keeps serving the earlier copy while a replacement is only prepared', async () => {
|
||||
const previous = record({ id: 'previous', current: false });
|
||||
const { service, repository } = makeOfferService(replacement({ previousId: 'previous' }), previous);
|
||||
|
||||
await expect(service.offer(1, 2, 3)).resolves.toBe(previous);
|
||||
|
||||
expect(repository.retire).not.toHaveBeenCalled();
|
||||
expect(repository.offer).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('returns a current delivery unchanged', async () => {
|
||||
const current = record();
|
||||
const { service, repository } = makeOfferService(current);
|
||||
|
||||
await expect(service.offer(1, 2, 3)).resolves.toBe(current);
|
||||
|
||||
expect(repository.retire).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('KoboDeliveryService.cancelReplacement', () => {
|
||||
it('tells the device again which copy stands once an offered replacement is cancelled', async () => {
|
||||
const { service, repository } = makeOfferService(null, replacement({ state: 'offered' }));
|
||||
|
||||
await service.cancelReplacement(1, 2, 'replacement');
|
||||
|
||||
expect(repository.cancel).toHaveBeenCalledWith(1, 2, 'replacement');
|
||||
expect(repository.reoffer).toHaveBeenCalledWith(1, 2, 3);
|
||||
});
|
||||
|
||||
it('does not re-announce a replacement the device was never told about, or one that was not cancelled', async () => {
|
||||
const prepared = makeOfferService(null, replacement());
|
||||
await prepared.service.cancelReplacement(1, 2, 'replacement');
|
||||
expect(prepared.repository.reoffer).not.toHaveBeenCalled();
|
||||
|
||||
const confirmed = makeOfferService(null, replacement({ state: 'offered' }));
|
||||
confirmed.repository.cancel.mockResolvedValue(false);
|
||||
await confirmed.service.cancelReplacement(1, 2, 'replacement');
|
||||
expect(confirmed.repository.reoffer).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -46,12 +46,12 @@ export class KoboDeliveryService {
|
||||
const startedAt = Date.now();
|
||||
const current = await this.current(userId, deviceId, bookId);
|
||||
if (current && replacementVariant === undefined) {
|
||||
if (['prepared', 'cancelled'].includes(current.state)) {
|
||||
const previous = current.previousId ? await this.repository.findById(userId, deviceId, current.previousId) : null;
|
||||
if (previous) return previous;
|
||||
throw new ConflictException({ code: 'KOBO_REPLACEMENT_NOT_OFFERED' });
|
||||
}
|
||||
return current;
|
||||
if (!['prepared', 'cancelled'].includes(current.state)) return current;
|
||||
const previous = current.previousId ? await this.repository.findById(userId, deviceId, current.previousId) : null;
|
||||
if (previous) return previous;
|
||||
// A replacement prepared before this device ever received the book has no copy to stand back
|
||||
// for, so it stops standing for the book and the book gets an ordinary first offer.
|
||||
await this.repository.retire(userId, deviceId, current.id);
|
||||
}
|
||||
const book = await this.downloads.findBook(bookId);
|
||||
if (!book) throw new NotFoundException();
|
||||
@@ -252,7 +252,9 @@ export class KoboDeliveryService {
|
||||
async cancelReplacement(userId: number, deviceId: number, id: string) {
|
||||
const record = await this.byId(userId, deviceId, id);
|
||||
if (!record) throw new NotFoundException();
|
||||
await this.repository.cancel(userId, deviceId, id);
|
||||
const cancelled = await this.repository.cancel(userId, deviceId, id);
|
||||
// The device was told about an offered replacement, so it must hear which copy stands for the book now.
|
||||
if (cancelled && record.state === 'offered') await this.repository.reoffer(userId, deviceId, record.bookId);
|
||||
return this.byId(userId, deviceId, id);
|
||||
}
|
||||
|
||||
|
||||
@@ -222,7 +222,6 @@ describe('Kobo download identity in KOReader (e2e)', { timeout: 180_000 }, () =>
|
||||
expect(Date.now() - duplicateStart).toBeLessThan(1000);
|
||||
vi.spyOn(ctx.app.get(KepubConversionService), 'getKepubPath').mockResolvedValue(path);
|
||||
const startedAt = Date.now();
|
||||
expect((await download()).statusCode).toBe(409);
|
||||
const response = await download(freshDevice.token);
|
||||
const durationMs = Date.now() - startedAt;
|
||||
expect(response.statusCode).toBe(200);
|
||||
@@ -537,4 +536,31 @@ describe('Kobo download identity in KOReader (e2e)', { timeout: 180_000 }, () =>
|
||||
expect(after.libraryVersion).not.toBe(before);
|
||||
expect(after.matches).toEqual([]);
|
||||
});
|
||||
|
||||
it('replaces an offer its source can no longer rebuild instead of failing every download of the book', async () => {
|
||||
const created = await ctx.app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/v1/kobo/devices',
|
||||
headers: authHeader(user.accessToken),
|
||||
payload: { name: 'Stale offer device' },
|
||||
});
|
||||
const device = created.json() as { id: number; token: string };
|
||||
const deliveries = ctx.app.get(KoboDeliveryService);
|
||||
const conversion = vi.spyOn(ctx.app.get(KepubConversionService), 'getKepubPath').mockResolvedValue(downloadPath);
|
||||
const stale = await deliveries.offer(user.userId, device.id, source.bookId);
|
||||
const rebuilt = await createEpubFixture(ctx.fixture.rootPath, 'rebuilt.epub', { title: 'Rebuilt conversion', uid: `urn:uuid:${randomUUID()}` });
|
||||
conversion.mockResolvedValue(rebuilt);
|
||||
try {
|
||||
const response = await download(device.token);
|
||||
expect(response.statusCode).toBe(200);
|
||||
expect(response.rawPayload).toEqual(await readFile(rebuilt));
|
||||
const fresh = await deliveries.current(user.userId, device.id, source.bookId);
|
||||
expect(fresh!.id).not.toBe(stale.id);
|
||||
expect(fresh!.previousId).toBeNull();
|
||||
expect((await deliveries.byId(user.userId, device.id, stale.id))!.current).toBe(false);
|
||||
expect((await download(device.token)).rawPayload).toEqual(await readFile(rebuilt));
|
||||
} finally {
|
||||
conversion.mockResolvedValue(downloadPath);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -413,6 +413,32 @@ describe('Read-aloud delivery contracts (e2e)', { timeout: 180_000 }, () => {
|
||||
expect((await action('cancel')).json().state).toBe('user_confirmed');
|
||||
});
|
||||
|
||||
it('delivers a book whose replacement was prepared before any copy, and an unfetched cancel hands back the earlier copy', async () => {
|
||||
const device = (await request('POST', '/api/v1/kobo/devices', { name: 'Replacement before delivery' })).json();
|
||||
await request('PATCH', '/api/v1/kobo/settings', { readAloudDelivery: 'original' });
|
||||
const base = `/api/v1/kobo/devices/${device.id}`;
|
||||
const download = () => ctx.app.inject({ method: 'GET', url: `/api/v1/kobo/${device.token}/v1/books/${book.bookId}/download` });
|
||||
|
||||
const early = (await request('POST', `${base}/books/${book.bookId}/replacement`, { variant: 'audioless_epub' })).json();
|
||||
expect(early).toMatchObject({ state: 'prepared', previousId: null });
|
||||
const first = await download();
|
||||
expect(first.statusCode).toBe(200);
|
||||
expect(first.rawPayload).toEqual(sourceBytes);
|
||||
const delivered = (await request('GET', `${base}/books/${book.bookId}/delivery`)).json();
|
||||
expect(delivered.id).not.toBe(early.id);
|
||||
expect(delivered.options.replacement).toBe(false);
|
||||
await waitForCondition(async () => {
|
||||
const row = await ctx.db.query.koboDeviceDeliveries.findFirst({ where: eq(schema.koboDeviceDeliveries.id, delivered.id) });
|
||||
expect(row?.state).toBe('transfer_observed');
|
||||
});
|
||||
|
||||
const pending = (await request('POST', `${base}/books/${book.bookId}/replacement`, { variant: 'audioless_epub' })).json();
|
||||
expect((await request('POST', `${base}/replacements/${pending.id}/offer`)).json().state).toBe('offered');
|
||||
expect((await request('POST', `${base}/replacements/${pending.id}/cancel`)).json()).toMatchObject({ state: 'cancelled', current: false });
|
||||
expect((await request('GET', `${base}/books/${book.bookId}/delivery`)).json()).toMatchObject({ id: delivered.id, state: 'transfer_observed' });
|
||||
expect((await download()).rawPayload).toEqual(sourceBytes);
|
||||
});
|
||||
|
||||
describe('Storyteller progress-reset regressions over live HTTP', () => {
|
||||
let origin: string;
|
||||
const cfi = 'epubcfi(/6/2!/4/2/2/1:3)';
|
||||
|
||||
Reference in New Issue
Block a user