fix(kobo): keep metadata file writes off the re-download path

Embedding metadata rewrites the book file, so its hash moves even though the book itself is
unchanged. Kobo reconcile reads a moved hash as new content and re-delivers the book as a
NewEntitlement, which makes the device rebuild its local entry and orphan every annotation
attached to the old one.

Carry the post-write hash into the device snapshots that recorded the pre-write hash, so
reconcile stays on the metadata-only path. The device keeps the bytes it already has and never
receives the rewritten file; a content change from any other source still triggers a download.

Ref #925
This commit is contained in:
neon
2026-08-08 05:59:48 +00:00
parent b09b71dbfe
commit d8105d4820
7 changed files with 272 additions and 19 deletions
+1
View File
@@ -583,6 +583,7 @@ export const E2E_SUITES = Object.freeze({
changedPaths: [
"server/src/modules/kobo/**",
"server/src/modules/book/**",
"server/src/modules/file-write/**",
"server/test/kobo-multi-device-sync.e2e-spec.ts",
"server/test/e2e/reader-state-isolation/**",
"packages/types/src/book.ts",
+3
View File
@@ -144,6 +144,9 @@ export const koboSnapshotBooks = pgTable(
isNew: boolean('is_new').notNull().default(true),
removedByDevice: boolean('removed_by_device').notNull().default(false),
needsLegacyNumericRemoval: boolean('needs_legacy_numeric_removal').notNull().default(false),
// The hash this row has been reconciled against, which is not always the hash of the bytes on
// the device: a metadata write that rewrites the file records its new hash here so reconcile
// does not re-deliver the book as a NewEntitlement and cost the device its annotations.
fileHash: varchar('file_hash', { length: 64 }),
deliveryHash: varchar('delivery_hash', { length: 64 }),
metadataHash: varchar('metadata_hash', { length: 64 }),
@@ -1,5 +1,20 @@
import { FileWriteRepository } from './file-write.repository';
function extractSqlStrings(value: unknown): string[] {
if (!value || typeof value !== 'object') return [];
const record = value as Record<string, unknown>;
if (Array.isArray(record.queryChunks)) return record.queryChunks.flatMap(extractSqlStrings);
if (Array.isArray(record.value)) return record.value.filter((entry): entry is string => typeof entry === 'string');
return [];
}
function extractSqlParams(value: unknown): unknown[] {
if (typeof value === 'number' || typeof value === 'string' || value === null || typeof value === 'boolean') return [value];
if (!value || typeof value !== 'object') return [];
const record = value as Record<string, unknown>;
return Array.isArray(record.queryChunks) ? record.queryChunks.flatMap(extractSqlParams) : [];
}
describe('FileWriteRepository', () => {
function chain<T>(result: T) {
return {
@@ -322,4 +337,80 @@ describe('FileWriteRepository', () => {
// innerJoin is called exactly once per query (books -> libraries only, no bookFiles join)
expect(c1.innerJoin).toHaveBeenCalledTimes(1);
});
it('atomically acknowledges a metadata-only primary file hash transition in matching Kobo snapshots', async () => {
const set = vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue(undefined) });
const execute = vi.fn().mockResolvedValue(undefined);
const tx = { update: vi.fn().mockReturnValue({ set }), execute };
const db = {
transaction: vi.fn().mockImplementation(async (callback: (value: typeof tx) => Promise<unknown>) => callback(tx)),
};
const repo = new FileWriteRepository(db as never);
const mtime = new Date('2026-08-08T00:00:00.000Z');
await repo.updateFileStateAfterMetadataWrite(5, 11, 'oldhash', {
fileHash: 'newhash',
mtime,
sizeBytes: 57,
ino: 99n,
});
expect(db.transaction).toHaveBeenCalledTimes(1);
expect(set).toHaveBeenCalledWith({ fileHash: 'newhash', mtime, sizeBytes: 57, ino: 99n, updatedAt: expect.any(Date) });
expect(execute).toHaveBeenCalledTimes(1);
const statement = execute.mock.calls[0]![0];
const text = extractSqlStrings(statement).join(' ');
expect(text).toContain('book.primary_file_id =');
expect(text).toContain('snapshot.file_hash IS NOT DISTINCT FROM');
expect(extractSqlParams(statement)).toEqual(expect.arrayContaining(['newhash', 'oldhash', 5, 11]));
});
it('leaves snapshots the device no longer holds out of the hash acknowledgement', async () => {
const set = vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue(undefined) });
const execute = vi.fn().mockResolvedValue(undefined);
const tx = { update: vi.fn().mockReturnValue({ set }), execute };
const db = {
transaction: vi.fn().mockImplementation(async (callback: (value: typeof tx) => Promise<unknown>) => callback(tx)),
};
const repo = new FileWriteRepository(db as never);
await repo.updateFileStateAfterMetadataWrite(5, 11, 'oldhash', { fileHash: 'newhash' });
const text = extractSqlStrings(execute.mock.calls[0]![0]).join(' ');
expect(text).toContain('snapshot.pending_delete = false');
expect(text).toContain('snapshot.removed_by_device = false');
});
it('does not acknowledge Kobo snapshots when the post-write hash is unavailable', async () => {
const set = vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue(undefined) });
const db = {
update: vi.fn().mockReturnValue({ set }),
transaction: vi.fn(),
};
const repo = new FileWriteRepository(db as never);
const mtime = new Date('2026-08-08T00:00:00.000Z');
await repo.updateFileStateAfterMetadataWrite(5, 11, 'oldhash', { mtime, sizeBytes: 57, ino: 99n });
expect(db.update).toHaveBeenCalledTimes(1);
expect(set).toHaveBeenCalledWith({ mtime, sizeBytes: 57, ino: 99n, updatedAt: expect.any(Date) });
expect(db.transaction).not.toHaveBeenCalled();
});
it('does not acknowledge Kobo snapshots when the rewrite left the hash unchanged', async () => {
const set = vi.fn().mockReturnValue({ where: vi.fn().mockResolvedValue(undefined) });
const db = {
update: vi.fn().mockReturnValue({ set }),
transaction: vi.fn(),
};
const repo = new FileWriteRepository(db as never);
const mtime = new Date('2026-08-08T00:00:00.000Z');
await repo.updateFileStateAfterMetadataWrite(5, 11, 'samehash', { fileHash: 'samehash', mtime, sizeBytes: 57, ino: 99n });
expect(db.update).toHaveBeenCalledTimes(1);
expect(set).toHaveBeenCalledWith({ fileHash: 'samehash', mtime, sizeBytes: 57, ino: 99n, updatedAt: expect.any(Date) });
expect(db.transaction).not.toHaveBeenCalled();
});
});
@@ -1,5 +1,5 @@
import { Inject, Injectable } from '@nestjs/common';
import { and, asc, desc, eq, isNull, ne } from 'drizzle-orm';
import { and, asc, desc, eq, isNull, ne, sql } from 'drizzle-orm';
import { NodePgDatabase } from 'drizzle-orm/node-postgres';
import type { WriteResult, WriteLogEntry } from '@bookorbit/types';
@@ -116,11 +116,42 @@ export class FileWriteRepository {
.orderBy(asc(books.id));
}
async updateFileStat(bookFileId: number, fields: { fileHash?: string; mtime?: Date; sizeBytes?: number; ino?: bigint }): Promise<void> {
await this.db
.update(bookFiles)
.set({ ...fields, updatedAt: new Date() })
.where(eq(bookFiles.id, bookFileId));
async updateFileStateAfterMetadataWrite(
bookId: number,
bookFileId: number,
previousFileHash: string | null,
fields: { fileHash?: string; mtime?: Date; sizeBytes?: number; ino?: bigint },
): Promise<void> {
const updateFile = (db: Pick<Db, 'update'>) =>
db
.update(bookFiles)
.set({ ...fields, updatedAt: new Date() })
.where(and(eq(bookFiles.id, bookFileId), eq(bookFiles.bookId, bookId)));
if (fields.fileHash === undefined || fields.fileHash === previousFileHash) {
await updateFile(this.db);
return;
}
// Embedding metadata rewrites the file, so its hash moves even though the book itself is
// unchanged. Kobo reconcile reads a moved hash as new content and re-delivers the book as a
// NewEntitlement, which makes the device discard its on-device annotations. Carrying the new
// hash into snapshots that recorded the pre-write one keeps reconcile on the metadata-only
// path. The device keeps the bytes it already has; it never receives the rewritten file.
await this.db.transaction(async (tx) => {
await updateFile(tx);
await tx.execute(sql`
UPDATE ${schema.koboSnapshotBooks} AS snapshot
SET file_hash = ${fields.fileHash}
FROM ${schema.books} AS book
WHERE book.id = ${bookId}
AND book.primary_file_id = ${bookFileId}
AND snapshot.book_id = book.id
AND snapshot.pending_delete = false
AND snapshot.removed_by_device = false
AND snapshot.file_hash IS NOT DISTINCT FROM ${previousFileHash}
`);
});
}
async recordHashHistory(bookFileId: number, fileHash: string, reason: string): Promise<void> {
@@ -65,7 +65,7 @@ describe('FileWriteService', () => {
findLibraryWriteSettingsForBook: vi.fn(),
insertLog: vi.fn().mockResolvedValue(undefined),
setLastWrittenAt: vi.fn().mockResolvedValue(undefined),
updateFileStat: vi.fn().mockResolvedValue(undefined),
updateFileStateAfterMetadataWrite: vi.fn().mockResolvedValue(undefined),
recordHashHistory: vi.fn().mockResolvedValue(undefined),
};
const writer = {
@@ -449,7 +449,7 @@ describe('FileWriteService', () => {
await expect(service.writeToFile(5, 'auto')).resolves.toEqual({ status: 'success', fieldsWritten: ['title'], durationMs: 5 });
expect(fileWriteRepo.updateFileStat).toHaveBeenCalledWith(1, {
expect(fileWriteRepo.updateFileStateAfterMetadataWrite).toHaveBeenCalledWith(5, 1, 'oldhash', {
fileHash: 'newhash',
mtime: POST_WRITE_MTIME,
sizeBytes: 57,
@@ -474,7 +474,7 @@ describe('FileWriteService', () => {
await expect(service.writeToFile(5, 'auto')).resolves.toEqual({ status: 'success', fieldsWritten: ['title'], durationMs: 5 });
expect(fileWriteRepo.updateFileStat).toHaveBeenCalledWith(1, {
expect(fileWriteRepo.updateFileStateAfterMetadataWrite).toHaveBeenCalledWith(5, 1, 'oldhash', {
mtime: POST_WRITE_MTIME,
sizeBytes: 57,
ino: 9_007_199_254_740_993n,
@@ -499,7 +499,7 @@ describe('FileWriteService', () => {
await expect(service.writeToFile(5, 'auto')).resolves.toEqual({ status: 'success', fieldsWritten: ['title'], durationMs: 5 });
expect(mockStat).toHaveBeenCalledTimes(2);
expect(fileWriteRepo.updateFileStat).toHaveBeenCalledTimes(1);
expect(fileWriteRepo.updateFileStateAfterMetadataWrite).toHaveBeenCalledTimes(1);
});
it('retries transient scanner identity persistence failures before reporting success', async () => {
@@ -514,13 +514,13 @@ describe('FileWriteService', () => {
});
fileWriteRepo.loadPayload.mockResolvedValue({ title: 'Book' });
fileWriteRepo.findLibraryFileWriteConfig.mockResolvedValue({ ...DEFAULT_LIB_CONFIG, fileWriteWriteCover: false });
fileWriteRepo.updateFileStat.mockRejectedValueOnce(new Error('temporary database failure'));
fileWriteRepo.updateFileStateAfterMetadataWrite.mockRejectedValueOnce(new Error('temporary database failure'));
writer.write.mockResolvedValue({ status: 'success', fieldsWritten: ['title'], durationMs: 5 });
await expect(service.writeToFile(5, 'auto')).resolves.toEqual({ status: 'success', fieldsWritten: ['title'], durationMs: 5 });
expect(mockStat).toHaveBeenCalledTimes(2);
expect(fileWriteRepo.updateFileStat).toHaveBeenCalledTimes(2);
expect(fileWriteRepo.updateFileStateAfterMetadataWrite).toHaveBeenCalledTimes(2);
expect(fileWriteRepo.setLastWrittenAt).toHaveBeenCalledTimes(1);
});
@@ -541,7 +541,7 @@ describe('FileWriteService', () => {
expect(selfWriteRegistry.isSuppressed(path)).toBe(true);
return Promise.resolve({ status: 'success', fieldsWritten: ['title'], durationMs: 5 });
});
fileWriteRepo.updateFileStat.mockImplementation(() => {
fileWriteRepo.updateFileStateAfterMetadataWrite.mockImplementation(() => {
expect(selfWriteRegistry.isSuppressed(path)).toBe(true);
return Promise.resolve();
});
@@ -574,7 +574,7 @@ describe('FileWriteService', () => {
});
expect(mockStat).toHaveBeenCalledTimes(3);
expect(fileWriteRepo.updateFileStat).not.toHaveBeenCalled();
expect(fileWriteRepo.updateFileStateAfterMetadataWrite).not.toHaveBeenCalled();
expect(fileWriteRepo.setLastWrittenAt).not.toHaveBeenCalled();
expect(fileWriteRepo.insertLog).toHaveBeenCalledWith(
expect.objectContaining({ result: expect.objectContaining({ status: 'failed', reason: 'post-write file state refresh failed' }) }),
@@ -754,8 +754,20 @@ describe('FileWriteService', () => {
);
expect(fileWriteRepo.recordHashHistory).toHaveBeenNthCalledWith(1, 1, 'm4bhash', 'file_write');
expect(fileWriteRepo.recordHashHistory).toHaveBeenNthCalledWith(2, 2, 'mp3hash', 'file_write');
expect(fileWriteRepo.updateFileStat).toHaveBeenNthCalledWith(1, 1, expect.objectContaining({ fileHash: 'new-m4b' }));
expect(fileWriteRepo.updateFileStat).toHaveBeenNthCalledWith(2, 2, expect.objectContaining({ fileHash: 'new-mp3' }));
expect(fileWriteRepo.updateFileStateAfterMetadataWrite).toHaveBeenNthCalledWith(
1,
20,
1,
'm4bhash',
expect.objectContaining({ fileHash: 'new-m4b' }),
);
expect(fileWriteRepo.updateFileStateAfterMetadataWrite).toHaveBeenNthCalledWith(
2,
20,
2,
'mp3hash',
expect.objectContaining({ fileHash: 'new-mp3' }),
);
expect(fileWriteRepo.setLastWrittenAt).toHaveBeenCalledTimes(1);
});
@@ -838,7 +850,7 @@ describe('FileWriteService', () => {
expect(result.status).toBe('failed');
expect(result.reason).toBe('1 of 2 file writes failed');
expect(result.fieldsWritten).toEqual(['coverBytes']);
expect(fileWriteRepo.updateFileStat).toHaveBeenCalledWith(1, expect.objectContaining({ fileHash: 'new-m4b' }));
expect(fileWriteRepo.updateFileStateAfterMetadataWrite).toHaveBeenCalledWith(20, 1, 'm4bhash', expect.objectContaining({ fileHash: 'new-m4b' }));
expect(fileWriteRepo.setLastWrittenAt).not.toHaveBeenCalled();
expect(fileWriteRepo.insertLog).toHaveBeenCalledWith(
expect.objectContaining({
@@ -473,7 +473,7 @@ export class FileWriteService implements OnModuleDestroy {
for (let attempt = 1; attempt <= FILE_STATE_REFRESH_ATTEMPTS; attempt++) {
try {
const stats = await stat(file.absolutePath, { bigint: true });
await this.fileWriteRepo.updateFileStat(file.id, {
await this.fileWriteRepo.updateFileStateAfterMetadataWrite(bookId, file.id, file.fileHash ?? null, {
...(newHash ? { fileHash: newHash } : {}),
mtime: stats.mtime,
sizeBytes: Number(stats.size),
+116 -1
View File
@@ -1,4 +1,4 @@
import { randomUUID } from 'crypto';
import { createHash, randomUUID } from 'crypto';
import { readFile, readdir } from 'fs/promises';
import { dirname, join } from 'path';
import { fileURLToPath } from 'url';
@@ -6,6 +6,7 @@ import { and, eq, inArray } from 'drizzle-orm';
import { Client } from 'pg';
import * as schema from '../src/db/schema';
import { waitForCondition } from './e2e/app-harness';
import { createEpubFixture } from './e2e/reader-state-isolation/reader-state-isolation-fixture-builder';
import {
authHeader,
@@ -64,6 +65,13 @@ function changedReadingStates(entries: unknown[]): Record<string, unknown>[] {
});
}
function changedProductMetadata(entries: unknown[]): Array<{ BookEntitlement: { Id: string }; BookMetadata: { Title: string | null } }> {
return entries.flatMap((entry) => {
const value = entry as { ChangedProductMetadata?: { BookEntitlement: { Id: string }; BookMetadata: { Title: string | null } } };
return value.ChangedProductMetadata ? [value.ChangedProductMetadata] : [];
});
}
function removedEntitlementIds(entries: unknown[]): string[] {
return entries.flatMap((entry) => {
const value = entry as Record<string, Record<string, Record<string, string>>>;
@@ -737,4 +745,111 @@ describe('Kobo multi-device library sync (e2e)', { timeout: 180_000 }, () => {
expect(state).toEqual(expect.objectContaining({ StatusInfo: expect.objectContaining({ Status: 'Finished' }) }));
});
});
describe('metadata edits that rewrite the book file', () => {
let writeBookId!: number;
let writePrimaryFileId!: number;
let writeEntitlementId!: string;
let writeDevice!: KoboDevice;
async function primaryFileHash(): Promise<string | null> {
const [row] = await ctx.db
.select({ fileHash: schema.bookFiles.fileHash })
.from(schema.books)
.innerJoin(schema.bookFiles, eq(schema.bookFiles.id, schema.books.primaryFileId))
.where(eq(schema.books.id, writeBookId));
if (!row) throw new Error(`Book ${writeBookId} has no primary file`);
return row.fileHash;
}
async function snapshotFileHash(deviceId: number): Promise<string | null> {
const [row] = await ctx.db
.select({ fileHash: schema.koboSnapshotBooks.fileHash })
.from(schema.koboSnapshotBooks)
.innerJoin(schema.koboLibrarySnapshots, eq(schema.koboLibrarySnapshots.id, schema.koboSnapshotBooks.snapshotId))
.where(and(eq(schema.koboLibrarySnapshots.deviceId, deviceId), eq(schema.koboSnapshotBooks.bookId, writeBookId)));
if (!row) throw new Error(`Device ${deviceId} has no snapshot row for book ${writeBookId}`);
return row.fileHash;
}
beforeAll(async () => {
const writeLibrary = await createLibraryWithFolder(ctx, { name: `kobo-file-write-${randomUUID()}` });
await ctx.db.update(schema.libraries).set({ fileWriteEnabled: true }).where(eq(schema.libraries.id, writeLibrary.libraryId));
const path = await createEpubFixture(writeLibrary.folderPath, `kobo-file-write-${randomUUID()}.epub`, {
title: 'Kobo File Write Seed',
uid: `urn:uuid:${randomUUID()}`,
});
await triggerAndWaitForLibraryScan(ctx, writeLibrary.libraryId);
writeBookId = (await locateBookByAbsolutePath(ctx, path)).bookId;
const [book] = await ctx.db.select({ primaryFileId: schema.books.primaryFileId }).from(schema.books).where(eq(schema.books.id, writeBookId));
if (!book?.primaryFileId) throw new Error(`Book ${writeBookId} has no primary file`);
writePrimaryFileId = book.primaryFileId;
const addBooksResponse = await ctx.app.inject({
method: 'POST',
url: `/api/v1/collections/${collectionId}/books`,
headers: authHeader(ctx.adminToken),
payload: { bookIds: [writeBookId] },
});
expect([200, 201]).toContain(addBooksResponse.statusCode);
writeDevice = await createDevice('Kobo File Write');
await drainEntries(writeDevice);
const [identity] = await ctx.db
.select({ entitlementId: schema.koboBookEntitlements.entitlementId })
.from(schema.koboBookEntitlements)
.where(and(eq(schema.koboBookEntitlements.userId, userId), eq(schema.koboBookEntitlements.bookId, writeBookId)));
if (!identity) throw new Error(`Book ${writeBookId} has no Kobo entitlement identity`);
writeEntitlementId = identity.entitlementId;
}, 180_000);
it('delivers changed metadata without re-issuing the entitlement after the file is rewritten', async () => {
const hashBeforeEdit = await primaryFileHash();
expect(hashBeforeEdit).toEqual(expect.any(String));
expect(await snapshotFileHash(writeDevice.id)).toBe(hashBeforeEdit);
const patchResponse = await ctx.app.inject({
method: 'PATCH',
url: `/api/v1/books/${writeBookId}/metadata`,
headers: authHeader(ctx.adminToken),
payload: { title: 'Kobo File Write Updated', description: 'Rewritten by the metadata write' },
});
expect(patchResponse.statusCode).toBe(200);
await waitForCondition(async () => {
if ((await primaryFileHash()) === hashBeforeEdit) throw new Error('the metadata write has not rewritten the EPUB yet');
}, 60_000);
const hashAfterEdit = await primaryFileHash();
expect(hashAfterEdit).not.toBe(hashBeforeEdit);
// Read before syncing: reconcile writes the current hash onto the row either way, so after a
// sync this no longer distinguishes an acknowledged rewrite from a re-delivered one.
const acknowledgedHash = await snapshotFileHash(writeDevice.id);
const delivered = await drainEntries(writeDevice);
const changed = changedProductMetadata(delivered).filter((entry) => entry.BookEntitlement.Id === writeEntitlementId);
expect(newEntitlementIds(delivered)).not.toContain(writeEntitlementId);
expect(changed).toHaveLength(1);
expect(changed[0]!.BookMetadata.Title).toBe('Kobo File Write Updated');
expect(acknowledgedHash).toBe(hashAfterEdit);
});
it('re-issues the entitlement when the file changes outside a metadata write', async () => {
await drainEntries(writeDevice);
// A scanner-detected content change moves the hash without the file-write acknowledgement,
// so the device genuinely holds stale bytes and has to download the book again.
await ctx.db
.update(schema.bookFiles)
.set({ fileHash: createHash('sha256').update(randomUUID()).digest('hex') })
.where(eq(schema.bookFiles.id, writePrimaryFileId));
const delivered = await drainEntries(writeDevice);
expect(newEntitlementIds(delivered)).toContain(writeEntitlementId);
});
});
});