mirror of
https://github.com/bookorbit/bookorbit.git
synced 2026-11-03 00:11:37 -06:00
fix(annotations): resolve html entities when indexing chapter text
Chapters are parsed in XML mode, which resolves only the five predefined XML entities and numeric character references. EPUB2 XHTML may use any named HTML entity through its DTD, so ` ` and its kin were indexed as literal text while kepubify wrote the decoded character. The two text indexes then never matched, which disabled structural resolution for every annotation in the chapter and failed each one with kepub_text_mismatch, even though the Kobo position was exact. Resolve the references on the raw markup before parsing, where an escaped ` ` is still distinguishable from a real reference, and read each one back through an attribute value so an unknown reference is never partially matched against a legacy entity. Rebuild canonical positions from the stored kobo_span position on position retry as well. Retry only ran the cfi backfill, which derives cfi from an xpointer and therefore had no candidate when the xpointer failed too, leaving an affected annotation pending forever with no way to recover from the UI. Route the rebuild through a registry that the Kobo module registers into, since it already imports the annotation module and the dependency cannot run the other way. Fixes #944
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
import { ConflictException, NotFoundException } from '@nestjs/common';
|
||||
|
||||
import { AnnotationHubService } from './annotation-hub.service';
|
||||
import { DevicePositionRebuilderRegistry } from './device-position-rebuilder';
|
||||
|
||||
function makeHubRow(overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
@@ -30,7 +31,7 @@ function makeHubRow(overrides: Record<string, unknown> = {}) {
|
||||
};
|
||||
}
|
||||
|
||||
function makeService() {
|
||||
function makeService(rebuilderRegistered = false) {
|
||||
const annotationRepo = {
|
||||
findHubPaginated: vi.fn().mockResolvedValue({ items: [makeHubRow()], total: 1 }),
|
||||
getHubStats: vi.fn().mockResolvedValue({ books: 1, withNotes: 1, web: 1, koreader: 0, kobo: 0 }),
|
||||
@@ -46,7 +47,7 @@ function makeService() {
|
||||
};
|
||||
const exportService = { export: vi.fn().mockResolvedValue({ contentType: 'text/markdown', filename: 'x.md', content: '#' }) };
|
||||
const syncRepo = {
|
||||
findAnnotationById: vi.fn().mockResolvedValue({ id: 1, bookId: 5, origin: 'web', version: 3, deletedAt: null }),
|
||||
findAnnotationById: vi.fn().mockResolvedValue({ id: 1, bookId: 5, origin: 'web', version: 3, deletedAt: null, text: 'selected text' }),
|
||||
findPositionsByAnnotationIds: vi
|
||||
.fn()
|
||||
.mockResolvedValue([
|
||||
@@ -61,8 +62,17 @@ function makeService() {
|
||||
updatePosition: vi.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
const conversionService = { ensureCfiPositionsForBook: vi.fn().mockResolvedValue(1) };
|
||||
const service = new AnnotationHubService(annotationRepo as never, exportService as never, syncRepo as never, conversionService as never);
|
||||
return { service, annotationRepo, exportService, syncRepo, conversionService };
|
||||
const rebuilder = { rebuildCanonicalPositions: vi.fn().mockResolvedValue({ rebuilt: true }) };
|
||||
const rebuilderRegistry = new DevicePositionRebuilderRegistry();
|
||||
if (rebuilderRegistered) rebuilderRegistry.register(rebuilder);
|
||||
const service = new AnnotationHubService(
|
||||
annotationRepo as never,
|
||||
exportService as never,
|
||||
syncRepo as never,
|
||||
conversionService as never,
|
||||
rebuilderRegistry,
|
||||
);
|
||||
return { service, annotationRepo, exportService, syncRepo, conversionService, rebuilder, rebuilderRegistry };
|
||||
}
|
||||
|
||||
describe('AnnotationHubService', () => {
|
||||
@@ -354,7 +364,7 @@ describe('AnnotationHubService', () => {
|
||||
expect(conversionService.ensureCfiPositionsForBook).toHaveBeenCalledWith(10, 5);
|
||||
});
|
||||
|
||||
it('does not run cfi conversion for device formats', async () => {
|
||||
it('does not run the cfi backfill for an xpointer retry', async () => {
|
||||
const { service, conversionService } = makeService();
|
||||
|
||||
await service.retryPosition(10, 1, 'xpointer');
|
||||
@@ -362,6 +372,42 @@ describe('AnnotationHubService', () => {
|
||||
expect(conversionService.ensureCfiPositionsForBook).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rebuilds from the device position instead of the cfi backfill when one is available', async () => {
|
||||
const { service, rebuilder, conversionService } = makeService(true);
|
||||
|
||||
await service.retryPosition(10, 1, 'cfi');
|
||||
|
||||
expect(rebuilder.rebuildCanonicalPositions).toHaveBeenCalledWith(10, { id: 1, bookId: 5, text: 'selected text' });
|
||||
expect(conversionService.ensureCfiPositionsForBook).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('falls back to the cfi backfill when the device rebuild has no source position', async () => {
|
||||
const { service, rebuilder, conversionService } = makeService(true);
|
||||
rebuilder.rebuildCanonicalPositions.mockResolvedValueOnce({ rebuilt: false, reason: 'no_kobo_span_position' });
|
||||
|
||||
await service.retryPosition(10, 1, 'cfi');
|
||||
|
||||
expect(conversionService.ensureCfiPositionsForBook).toHaveBeenCalledWith(10, 5);
|
||||
});
|
||||
|
||||
it('rebuilds an xpointer position, which the cfi backfill can never produce', async () => {
|
||||
const { service, rebuilder, syncRepo } = makeService(true);
|
||||
|
||||
await service.retryPosition(10, 1, 'xpointer');
|
||||
|
||||
expect(syncRepo.updatePosition).toHaveBeenCalledWith(1, 'xpointer', { status: 'pending', converterVersion: null });
|
||||
expect(rebuilder.rebuildCanonicalPositions).toHaveBeenCalledWith(10, { id: 1, bookId: 5, text: 'selected text' });
|
||||
});
|
||||
|
||||
it('leaves a device-native format to its own next sync', async () => {
|
||||
const { service, rebuilder, conversionService } = makeService(true);
|
||||
|
||||
await service.retryPosition(10, 1, 'kobo_span');
|
||||
|
||||
expect(rebuilder.rebuildCanonicalPositions).not.toHaveBeenCalled();
|
||||
expect(conversionService.ensureCfiPositionsForBook).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('throws NotFoundException for an unknown annotation', async () => {
|
||||
const { service, syncRepo } = makeService();
|
||||
syncRepo.findAnnotationById.mockResolvedValueOnce(null);
|
||||
|
||||
@@ -15,6 +15,7 @@ import { AnnotationConversionService } from './annotation-conversion.service';
|
||||
import { AnnotationExportService, type AnnotationExportFormat, type AnnotationExportResult } from './annotation-export.service';
|
||||
import { AnnotationSyncRepository } from './annotation-sync.repository';
|
||||
import { AnnotationRepository, type HubAnnotationRow, type HubFilters, type HubSort } from './annotation.repository';
|
||||
import { DevicePositionRebuilderRegistry } from './device-position-rebuilder';
|
||||
import type { AnnotationBulkDto, AnnotationExportQueryDto, AnnotationHubQueryDto } from './dto/annotation-hub.dto';
|
||||
|
||||
const BULK_EVENT = 'annotation.bulk';
|
||||
@@ -34,6 +35,7 @@ export class AnnotationHubService {
|
||||
private readonly exportService: AnnotationExportService,
|
||||
private readonly syncRepo: AnnotationSyncRepository,
|
||||
private readonly conversionService: AnnotationConversionService,
|
||||
private readonly rebuilderRegistry: DevicePositionRebuilderRegistry,
|
||||
) {}
|
||||
|
||||
async list(userId: number, query: AnnotationHubQueryDto): Promise<AnnotationHubResponse> {
|
||||
@@ -164,20 +166,27 @@ export class AnnotationHubService {
|
||||
}
|
||||
|
||||
/**
|
||||
* Resets a position to pending so converters recompute it. cfi recomputes
|
||||
* immediately; device formats recompute on that device type's next sync.
|
||||
* Resets a position to pending so converters recompute it. A canonical format is
|
||||
* rebuilt from the device position first, since the cfi backfill can only derive
|
||||
* cfi from an xpointer and has nothing to work from when that failed too. Formats
|
||||
* with no usable source recompute on that device type's next sync.
|
||||
*/
|
||||
async retryPosition(userId: number, annotationId: number, format: AnnotationPositionFormat): Promise<AnnotationSyncDetail> {
|
||||
const annotation = await this.syncRepo.findAnnotationById(annotationId, userId);
|
||||
if (!annotation) throw new NotFoundException(`Annotation ${annotationId} not found`);
|
||||
|
||||
await this.syncRepo.updatePosition(annotationId, format, { status: 'pending', converterVersion: null });
|
||||
|
||||
let rebuilt = false;
|
||||
let converted = 0;
|
||||
if (format === 'cfi') {
|
||||
converted = await this.conversionService.ensureCfiPositionsForBook(userId, annotation.bookId);
|
||||
if (format === 'cfi' || format === 'xpointer') {
|
||||
const target = { id: annotationId, bookId: annotation.bookId, text: annotation.text };
|
||||
rebuilt = (await this.rebuilderRegistry.get()?.rebuildCanonicalPositions(userId, target))?.rebuilt ?? false;
|
||||
if (!rebuilt && format === 'cfi') converted = await this.conversionService.ensureCfiPositionsForBook(userId, annotation.bookId);
|
||||
}
|
||||
|
||||
this.logger.log(
|
||||
`[${RETRY_EVENT}] [end] userId=${userId} annotationId=${annotationId} format=${format} converted=${converted} - position retry applied`,
|
||||
`[${RETRY_EVENT}] [end] userId=${userId} annotationId=${annotationId} format=${format} rebuilt=${rebuilt} converted=${converted} - position retry applied`,
|
||||
);
|
||||
return this.syncDetail(userId, annotationId);
|
||||
}
|
||||
|
||||
@@ -13,6 +13,7 @@ import { AnnotationRepository } from './annotation.repository';
|
||||
import { AnnotationService } from './annotation.service';
|
||||
import { AnnotationSyncRepository } from './annotation-sync.repository';
|
||||
import { AnnotationSyncService } from './annotation-sync.service';
|
||||
import { DevicePositionRebuilderRegistry } from './device-position-rebuilder';
|
||||
|
||||
@Module({
|
||||
imports: [BookModule, AchievementModule, PositionConverterModule],
|
||||
@@ -26,7 +27,8 @@ import { AnnotationSyncService } from './annotation-sync.service';
|
||||
AnnotationConversionService,
|
||||
AnnotationExportService,
|
||||
AnnotationHubService,
|
||||
DevicePositionRebuilderRegistry,
|
||||
],
|
||||
exports: [AnnotationSyncService, AnnotationHubService],
|
||||
exports: [AnnotationSyncService, AnnotationHubService, DevicePositionRebuilderRegistry],
|
||||
})
|
||||
export class AnnotationModule {}
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { DevicePositionRebuilderRegistry, type DevicePositionRebuilder } from './device-position-rebuilder';
|
||||
|
||||
function makeRebuilder(): DevicePositionRebuilder {
|
||||
return { rebuildCanonicalPositions: vi.fn().mockResolvedValue({ rebuilt: true }) };
|
||||
}
|
||||
|
||||
describe('DevicePositionRebuilderRegistry', () => {
|
||||
it('reports no rebuilder until one registers', () => {
|
||||
expect(new DevicePositionRebuilderRegistry().get()).toBeNull();
|
||||
});
|
||||
|
||||
it('returns the registered rebuilder', () => {
|
||||
const registry = new DevicePositionRebuilderRegistry();
|
||||
const rebuilder = makeRebuilder();
|
||||
|
||||
registry.register(rebuilder);
|
||||
|
||||
expect(registry.get()).toBe(rebuilder);
|
||||
});
|
||||
|
||||
it('keeps the most recently registered rebuilder', () => {
|
||||
const registry = new DevicePositionRebuilderRegistry();
|
||||
const second = makeRebuilder();
|
||||
|
||||
registry.register(makeRebuilder());
|
||||
registry.register(second);
|
||||
|
||||
expect(registry.get()).toBe(second);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,39 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
|
||||
export interface DevicePositionRebuildTarget {
|
||||
id: number;
|
||||
bookId: number;
|
||||
text: string | null;
|
||||
}
|
||||
|
||||
export interface DevicePositionRebuildResult {
|
||||
rebuilt: boolean;
|
||||
reason?: string;
|
||||
}
|
||||
|
||||
/** Recomputes canonical positions from a device's own stored position format. */
|
||||
export interface DevicePositionRebuilder {
|
||||
rebuildCanonicalPositions(userId: number, annotation: DevicePositionRebuildTarget): Promise<DevicePositionRebuildResult>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds the device-side rebuilder for the annotation hub's position retry, which
|
||||
* has to recompute cfi and xpointer from a device position when neither canonical
|
||||
* format has a usable source of its own.
|
||||
*
|
||||
* Device modules import AnnotationModule, so the dependency can only run in this
|
||||
* direction: the implementation registers itself here at startup instead of being
|
||||
* injected into the hub, which would close a module cycle.
|
||||
*/
|
||||
@Injectable()
|
||||
export class DevicePositionRebuilderRegistry {
|
||||
private rebuilder: DevicePositionRebuilder | null = null;
|
||||
|
||||
register(rebuilder: DevicePositionRebuilder): void {
|
||||
this.rebuilder = rebuilder;
|
||||
}
|
||||
|
||||
get(): DevicePositionRebuilder | null {
|
||||
return this.rebuilder;
|
||||
}
|
||||
}
|
||||
@@ -18,6 +18,7 @@ import { KoboBookIdentityService } from './services/kobo-book-identity.service';
|
||||
import { KoboDeviceService } from './services/kobo-device.service';
|
||||
import { KoboDownloadService } from './services/kobo-download.service';
|
||||
import { KoboKepubContextService } from './services/kobo-kepub-context.service';
|
||||
import { KoboPositionRebuildService } from './services/kobo-position-rebuild.service';
|
||||
import { KoboProgressBridgeService } from './services/kobo-progress-bridge.service';
|
||||
import { KoboProxyService } from './services/kobo-proxy.service';
|
||||
import { KoboReadingStateService } from './services/kobo-reading-state.service';
|
||||
@@ -55,6 +56,7 @@ describe('KoboModule', () => {
|
||||
KoboKepubContextService,
|
||||
KoboProgressBridgeService,
|
||||
KoboAnnotationMaterializerService,
|
||||
KoboPositionRebuildService,
|
||||
KoboAnnotationExchangeService,
|
||||
KoboReadingStateService,
|
||||
KoboSyncHistoryService,
|
||||
|
||||
@@ -23,6 +23,7 @@ import { KoboAnnotationMaterializerService } from './services/kobo-annotation-ma
|
||||
import { KoboDeviceService } from './services/kobo-device.service';
|
||||
import { KoboDownloadService } from './services/kobo-download.service';
|
||||
import { KoboKepubContextService } from './services/kobo-kepub-context.service';
|
||||
import { KoboPositionRebuildService } from './services/kobo-position-rebuild.service';
|
||||
import { KoboProgressBridgeService } from './services/kobo-progress-bridge.service';
|
||||
import { KoboProxyService } from './services/kobo-proxy.service';
|
||||
import { KoboBookIdentityService } from './services/kobo-book-identity.service';
|
||||
@@ -59,6 +60,7 @@ import { KoboAnalyticsService } from './services/kobo-analytics.service';
|
||||
KoboKepubContextService,
|
||||
KoboProgressBridgeService,
|
||||
KoboAnnotationMaterializerService,
|
||||
KoboPositionRebuildService,
|
||||
KoboAnnotationExchangeService,
|
||||
KoboReadingStateService,
|
||||
KoboSyncHistoryService,
|
||||
|
||||
@@ -109,61 +109,9 @@ export class KoboAnnotationMaterializerService {
|
||||
if (!koboSpan?.pos0) continue;
|
||||
if (!this.needsCanonical(formats?.cfi) && !this.needsCanonical(formats?.xpointer)) continue;
|
||||
|
||||
const location = (koboSpan.extras as { koboLocation?: unknown } | null)?.koboLocation;
|
||||
const outcome = await this.koboSpanConverter.koboSpanToCanonical({
|
||||
bookFileId: file.id,
|
||||
ctx,
|
||||
location,
|
||||
text: annotation.text || null,
|
||||
});
|
||||
|
||||
// The device's own position belongs to the current rendition by definition;
|
||||
// stamping the hash keeps the materializer from re-synthesizing the verbatim
|
||||
// location the device uploaded.
|
||||
if (outcome.renditionHash) {
|
||||
await this.annotationSync.upsertGeneratedPosition({
|
||||
annotationId: annotation.id,
|
||||
userId,
|
||||
bookFileId: file.id,
|
||||
format: 'kobo_span',
|
||||
pos0: koboSpan.pos0,
|
||||
pos1: koboSpan.pos1,
|
||||
status: koboSpan.status as 'exact' | 'repaired' | 'failed' | 'pending',
|
||||
converterVersion: this.koboSpanConverter.version,
|
||||
extras: { ...(koboSpan.extras ?? {}), chapterIndex: outcome.chapterIndex, renditionHash: outcome.renditionHash },
|
||||
});
|
||||
}
|
||||
|
||||
if (outcome.status === 'failed' || !outcome.cfi || !outcome.xpointerPos0) {
|
||||
failed += 1;
|
||||
await this.storeFailure(userId, annotation.id, file.id, 'cfi', outcome.chapterIndex ?? null, outcome.reason ?? 'conversion_failed');
|
||||
await this.storeFailure(userId, annotation.id, file.id, 'xpointer', outcome.chapterIndex ?? null, outcome.reason ?? 'conversion_failed');
|
||||
continue;
|
||||
}
|
||||
|
||||
converted += 1;
|
||||
await this.annotationSync.upsertGeneratedPosition({
|
||||
annotationId: annotation.id,
|
||||
userId,
|
||||
bookFileId: file.id,
|
||||
format: 'cfi',
|
||||
pos0: outcome.cfi,
|
||||
pos1: null,
|
||||
status: outcome.status,
|
||||
converterVersion: this.positionConverter.version,
|
||||
extras: { chapterIndex: outcome.chapterIndex },
|
||||
});
|
||||
await this.annotationSync.upsertGeneratedPosition({
|
||||
annotationId: annotation.id,
|
||||
userId,
|
||||
bookFileId: file.id,
|
||||
format: 'xpointer',
|
||||
pos0: outcome.xpointerPos0,
|
||||
pos1: outcome.xpointerPos1 ?? null,
|
||||
status: outcome.status,
|
||||
converterVersion: this.positionConverter.version,
|
||||
extras: { chapterIndex: outcome.chapterIndex },
|
||||
});
|
||||
const outcome = await this.convertFromKoboSpan(userId, annotation, koboSpan, file, ctx);
|
||||
if (outcome.converted) converted += 1;
|
||||
else failed += 1;
|
||||
}
|
||||
|
||||
if (converted > 0 || failed > 0) {
|
||||
@@ -179,6 +127,75 @@ export class KoboAnnotationMaterializerService {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Converts one stored kobo_span position to cfi + xpointer, recording a failure on
|
||||
* both canonical formats when it cannot resolve. Shared by device sync and by hub
|
||||
* position retry, which is the only way an already-failed conversion is redone.
|
||||
*/
|
||||
async convertFromKoboSpan(
|
||||
userId: number,
|
||||
annotation: { id: number; text: string | null },
|
||||
koboSpan: AnnotationPosition,
|
||||
file: KepubReaderFile,
|
||||
ctx: KepubContext,
|
||||
): Promise<{ converted: boolean; reason?: string }> {
|
||||
const location = (koboSpan.extras as { koboLocation?: unknown } | null)?.koboLocation;
|
||||
const outcome = await this.koboSpanConverter.koboSpanToCanonical({
|
||||
bookFileId: file.id,
|
||||
ctx,
|
||||
location,
|
||||
text: annotation.text || null,
|
||||
});
|
||||
|
||||
// The device's own position belongs to the current rendition by definition;
|
||||
// stamping the hash keeps the materializer from re-synthesizing the verbatim
|
||||
// location the device uploaded.
|
||||
if (outcome.renditionHash && koboSpan.pos0) {
|
||||
await this.annotationSync.upsertGeneratedPosition({
|
||||
annotationId: annotation.id,
|
||||
userId,
|
||||
bookFileId: file.id,
|
||||
format: 'kobo_span',
|
||||
pos0: koboSpan.pos0,
|
||||
pos1: koboSpan.pos1,
|
||||
status: koboSpan.status as 'exact' | 'repaired' | 'failed' | 'pending',
|
||||
converterVersion: this.koboSpanConverter.version,
|
||||
extras: { ...(koboSpan.extras ?? {}), chapterIndex: outcome.chapterIndex, renditionHash: outcome.renditionHash },
|
||||
});
|
||||
}
|
||||
|
||||
if (outcome.status === 'failed' || !outcome.cfi || !outcome.xpointerPos0) {
|
||||
const reason = outcome.reason ?? 'conversion_failed';
|
||||
await this.storeFailure(userId, annotation.id, file.id, 'cfi', outcome.chapterIndex ?? null, reason);
|
||||
await this.storeFailure(userId, annotation.id, file.id, 'xpointer', outcome.chapterIndex ?? null, reason);
|
||||
return { converted: false, reason };
|
||||
}
|
||||
|
||||
await this.annotationSync.upsertGeneratedPosition({
|
||||
annotationId: annotation.id,
|
||||
userId,
|
||||
bookFileId: file.id,
|
||||
format: 'cfi',
|
||||
pos0: outcome.cfi,
|
||||
pos1: null,
|
||||
status: outcome.status,
|
||||
converterVersion: this.positionConverter.version,
|
||||
extras: { chapterIndex: outcome.chapterIndex },
|
||||
});
|
||||
await this.annotationSync.upsertGeneratedPosition({
|
||||
annotationId: annotation.id,
|
||||
userId,
|
||||
bookFileId: file.id,
|
||||
format: 'xpointer',
|
||||
pos0: outcome.xpointerPos0,
|
||||
pos1: outcome.xpointerPos1 ?? null,
|
||||
status: outcome.status,
|
||||
converterVersion: this.positionConverter.version,
|
||||
extras: { chapterIndex: outcome.chapterIndex },
|
||||
});
|
||||
return { converted: true };
|
||||
}
|
||||
|
||||
private async materializeOne(
|
||||
userId: number,
|
||||
annotation: AnnotationRow,
|
||||
|
||||
@@ -0,0 +1,58 @@
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { DevicePositionRebuilderRegistry } from '../../annotation/device-position-rebuilder';
|
||||
import { KoboPositionRebuildService } from './kobo-position-rebuild.service';
|
||||
|
||||
const TARGET = { id: 1, bookId: 5, text: 'selected text' };
|
||||
const KOBO_SPAN_ROW = { annotationId: 1, format: 'kobo_span', pos0: 'kobo.6.2:0', pos1: 'kobo.6.2:28', extras: {} };
|
||||
const CONTEXT = { ok: true as const, file: { id: 9 }, ctx: { kepubPath: '/tmp/b.kepub.epub' }, settings: {} };
|
||||
|
||||
function makeService(overrides: { positions?: unknown[]; context?: unknown; converted?: boolean; reason?: string } = {}) {
|
||||
const registry = new DevicePositionRebuilderRegistry();
|
||||
const annotationSync = { findPositions: vi.fn().mockResolvedValue(overrides.positions ?? [KOBO_SPAN_ROW]) };
|
||||
const kepubContext = { resolveForBook: vi.fn().mockResolvedValue(overrides.context ?? CONTEXT) };
|
||||
const materializer = {
|
||||
convertFromKoboSpan: vi.fn().mockResolvedValue({ converted: overrides.converted ?? true, reason: overrides.reason }),
|
||||
};
|
||||
const service = new KoboPositionRebuildService(registry, annotationSync as never, kepubContext as never, materializer as never);
|
||||
return { service, registry, annotationSync, kepubContext, materializer };
|
||||
}
|
||||
|
||||
describe('KoboPositionRebuildService', () => {
|
||||
it('registers itself as the device rebuilder on init', () => {
|
||||
const { service, registry } = makeService();
|
||||
|
||||
service.onModuleInit();
|
||||
|
||||
expect(registry.get()).toBe(service);
|
||||
});
|
||||
|
||||
it('converts the stored kobo_span position through the shared materializer path', async () => {
|
||||
const { service, annotationSync, materializer } = makeService();
|
||||
|
||||
await expect(service.rebuildCanonicalPositions(10, TARGET)).resolves.toEqual({ rebuilt: true, reason: undefined });
|
||||
|
||||
expect(annotationSync.findPositions).toHaveBeenCalledWith([1], ['kobo_span']);
|
||||
expect(materializer.convertFromKoboSpan).toHaveBeenCalledWith(10, TARGET, KOBO_SPAN_ROW, CONTEXT.file, CONTEXT.ctx);
|
||||
});
|
||||
|
||||
it('reports the conversion reason when the codec rejects the position', async () => {
|
||||
const { service } = makeService({ converted: false, reason: 'kepub_text_mismatch' });
|
||||
|
||||
await expect(service.rebuildCanonicalPositions(10, TARGET)).resolves.toEqual({ rebuilt: false, reason: 'kepub_text_mismatch' });
|
||||
});
|
||||
|
||||
it('does nothing without a kobo_span position to rebuild from', async () => {
|
||||
const { service, kepubContext } = makeService({ positions: [] });
|
||||
|
||||
await expect(service.rebuildCanonicalPositions(10, TARGET)).resolves.toEqual({ rebuilt: false, reason: 'no_kobo_span_position' });
|
||||
expect(kepubContext.resolveForBook).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does nothing when the kepub artifact is unavailable', async () => {
|
||||
const { service, materializer } = makeService({ context: { ok: false, reason: 'kepub_required', settings: null } });
|
||||
|
||||
await expect(service.rebuildCanonicalPositions(10, TARGET)).resolves.toEqual({ rebuilt: false, reason: 'kepub_required' });
|
||||
expect(materializer.convertFromKoboSpan).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,49 @@
|
||||
import { Injectable, Logger, OnModuleInit } from '@nestjs/common';
|
||||
|
||||
import { sanitizeLogValue } from '../../../common/utils/log-sanitize.utils';
|
||||
import { AnnotationSyncService } from '../../annotation/annotation-sync.service';
|
||||
import {
|
||||
type DevicePositionRebuildResult,
|
||||
type DevicePositionRebuildTarget,
|
||||
type DevicePositionRebuilder,
|
||||
DevicePositionRebuilderRegistry,
|
||||
} from '../../annotation/device-position-rebuilder';
|
||||
import { KoboAnnotationMaterializerService } from './kobo-annotation-materializer.service';
|
||||
import { KoboKepubContextService } from './kobo-kepub-context.service';
|
||||
|
||||
const EVENT = 'kobo.position_rebuild';
|
||||
|
||||
/**
|
||||
* Rebuilds canonical positions from a stored kobo_span position on demand, so a
|
||||
* conversion that failed during sync can be retried from the annotation hub instead
|
||||
* of waiting for the device to upload the annotation again.
|
||||
*/
|
||||
@Injectable()
|
||||
export class KoboPositionRebuildService implements DevicePositionRebuilder, OnModuleInit {
|
||||
private readonly logger = new Logger(KoboPositionRebuildService.name);
|
||||
|
||||
constructor(
|
||||
private readonly registry: DevicePositionRebuilderRegistry,
|
||||
private readonly annotationSync: AnnotationSyncService,
|
||||
private readonly kepubContext: KoboKepubContextService,
|
||||
private readonly materializer: KoboAnnotationMaterializerService,
|
||||
) {}
|
||||
|
||||
onModuleInit(): void {
|
||||
this.registry.register(this);
|
||||
}
|
||||
|
||||
async rebuildCanonicalPositions(userId: number, annotation: DevicePositionRebuildTarget): Promise<DevicePositionRebuildResult> {
|
||||
const [koboSpan] = await this.annotationSync.findPositions([annotation.id], ['kobo_span']);
|
||||
if (!koboSpan?.pos0) return { rebuilt: false, reason: 'no_kobo_span_position' };
|
||||
|
||||
const resolved = await this.kepubContext.resolveForBook(userId, annotation.bookId);
|
||||
if (!resolved.ok) return { rebuilt: false, reason: resolved.reason };
|
||||
|
||||
const outcome = await this.materializer.convertFromKoboSpan(userId, annotation, koboSpan, resolved.file, resolved.ctx);
|
||||
this.logger.log(
|
||||
`[${EVENT}] [end] userId=${userId} annotationId=${annotation.id} converted=${outcome.converted} reason="${sanitizeLogValue(outcome.reason ?? 'none')}" - kobo span rebuild finished`,
|
||||
);
|
||||
return { rebuilt: outcome.converted, reason: outcome.reason };
|
||||
}
|
||||
}
|
||||
@@ -62,6 +62,33 @@ const IMG_EPUB = `<?xml version="1.0" encoding="utf-8"?>
|
||||
</body>
|
||||
</html>`;
|
||||
|
||||
// Named HTML entities are legal in EPUB2 XHTML through its DTD. kepubify resolves
|
||||
// them when it converts, so the epub side has to resolve them too or the two text
|
||||
// indexes never align. The fixtures below mirror real kepubify input and output:
|
||||
// it re-emits nbsp numerically and writes the other characters literally.
|
||||
const NBSP = '\u00a0';
|
||||
const SHY = '\u00ad';
|
||||
const THINSP = '\u2009';
|
||||
|
||||
const ENTITY_EPUB = `<?xml version="1.0" encoding="utf-8"?>
|
||||
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
|
||||
<html xmlns="http://www.w3.org/1999/xhtml">
|
||||
<head><title>T</title></head>
|
||||
<body><p>Rumata piktai dėbtelėjo. — Malonėk pa­sakyti tiksliau. Gerai?</p></body>
|
||||
</html>`;
|
||||
|
||||
const ENTITY_KEPUB = `<?xml version="1.0" encoding="utf-8"?>
|
||||
<html xmlns="http://www.w3.org/1999/xhtml">
|
||||
<head><title>T</title></head>
|
||||
<body><div id="book-columns"><div id="book-inner"><p><span class="koboSpan" id="kobo.1.1">Rumata piktai dėbtelėjo. </span><span class="koboSpan" id="kobo.1.2">— Malonėk pa${SHY}sakyti tiksliau.${THINSP}Gerai?</span></p></div></div></body>
|
||||
</html>`;
|
||||
|
||||
/** What the device reports: the rendered characters, invisible ones included. */
|
||||
const ENTITY_SPAN_TEXT = `\u2014${NBSP}Malonėk pa${SHY}sakyti tiksliau.${THINSP}Gerai?`;
|
||||
|
||||
/** What the collapsed index holds: whitespace collapses, the soft hyphen survives. */
|
||||
const ENTITY_SPAN_COLLAPSED = `\u2014 Malonėk pa${SHY}sakyti tiksliau. Gerai?`;
|
||||
|
||||
function docs(epubXhtml: string, kepubXhtml: string) {
|
||||
const epubDoc = parseChapterDocument(epubXhtml);
|
||||
const kepubDoc = parseChapterDocument(kepubXhtml);
|
||||
@@ -235,6 +262,46 @@ describe('koboSpanRangeToCanonical', () => {
|
||||
expect(result).toEqual({ status: 'failed', reason: 'span_not_found' });
|
||||
});
|
||||
|
||||
it('aligns a chapter whose epub side uses named html entities', () => {
|
||||
const { epubDoc, kepubDoc } = docs(ENTITY_EPUB, ENTITY_KEPUB);
|
||||
expect(epubDoc.index.collapsed).toBe(kepubDoc.index.collapsed);
|
||||
expect(epubDoc.index.collapsed).toContain(`dėbtelėjo. ${ENTITY_SPAN_COLLAPSED}`);
|
||||
});
|
||||
|
||||
it('converts a kobo highlight in a chapter that uses named html entities', () => {
|
||||
const { epubDoc, kepubDoc, spanIndex } = docs(ENTITY_EPUB, ENTITY_KEPUB);
|
||||
const result = koboSpanRangeToCanonical(
|
||||
epubDoc,
|
||||
kepubDoc,
|
||||
spanIndex,
|
||||
6,
|
||||
{ startId: 'kobo.1.2', startChar: 0, endId: 'kobo.1.2', endChar: ENTITY_SPAN_TEXT.length },
|
||||
ENTITY_SPAN_TEXT,
|
||||
);
|
||||
expect(result.status).toBe('exact');
|
||||
if (result.status === 'failed') return;
|
||||
expect(result.aligned).toBe(true);
|
||||
expect(epubDoc.index.extractCollapsed(result.startCp, result.endCp)).toBe(ENTITY_SPAN_COLLAPSED);
|
||||
expect(result.cfi).toContain('epubcfi(');
|
||||
expect(result.xpointerPos0).toContain('DocFragment[7]');
|
||||
});
|
||||
|
||||
it('converts a highlight that starts inside an entity-bearing run', () => {
|
||||
const { epubDoc, kepubDoc, spanIndex } = docs(ENTITY_EPUB, ENTITY_KEPUB);
|
||||
const startChar = ENTITY_SPAN_TEXT.indexOf('Malonėk');
|
||||
const result = koboSpanRangeToCanonical(
|
||||
epubDoc,
|
||||
kepubDoc,
|
||||
spanIndex,
|
||||
6,
|
||||
{ startId: 'kobo.1.2', startChar, endId: 'kobo.1.2', endChar: ENTITY_SPAN_TEXT.length },
|
||||
ENTITY_SPAN_TEXT.slice(startChar),
|
||||
);
|
||||
expect(result.status).toBe('exact');
|
||||
if (result.status === 'failed') return;
|
||||
expect(epubDoc.index.extractCollapsed(result.startCp, result.endCp)).toBe(ENTITY_SPAN_COLLAPSED.slice(startChar));
|
||||
});
|
||||
|
||||
it('fails with text_not_found when spans resolve but the text is absent', () => {
|
||||
const { epubDoc, kepubDoc, spanIndex } = docs(EPUB_CHAPTER, KEPUB_CHAPTER);
|
||||
const result = koboSpanRangeToCanonical(
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import { chapterIndexFromSpineStep, parseCfi } from './cfi.utils';
|
||||
import { cfiRangeToXPointer, parseChapterDocument, xpointerRangeToCfi } from './position-converter.core';
|
||||
import { cfiRangeToXPointer, collapsedRangeToCfi, parseChapterDocument, xpointerRangeToCfi } from './position-converter.core';
|
||||
import { parseXPointer } from './xpointer.utils';
|
||||
|
||||
const SIMPLE_CHAPTER = `<?xml version="1.0" encoding="utf-8"?>
|
||||
@@ -98,6 +98,27 @@ describe('parseCfi', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('parseChapterDocument', () => {
|
||||
const chapter = (body: string) => `<?xml version="1.0" encoding="utf-8"?>
|
||||
<html xmlns="http://www.w3.org/1999/xhtml"><head><title>T</title></head><body>${body}</body></html>`;
|
||||
|
||||
it('indexes named entities as the characters a renderer would show', () => {
|
||||
const doc = parseChapterDocument(chapter('<p>a b pa­sakyti c—d</p>'));
|
||||
expect(doc.index.collapsed).toBe('a b pasakyti c—d');
|
||||
});
|
||||
|
||||
it('keeps escaped entity text literal instead of resolving it twice', () => {
|
||||
const doc = parseChapterDocument(chapter('<p>write &nbsp; for a space</p>'));
|
||||
expect(doc.index.collapsed).toBe('write for a space');
|
||||
});
|
||||
|
||||
it('builds cfi offsets against resolved characters', () => {
|
||||
const doc = parseChapterDocument(chapter('<p>a bc</p>'));
|
||||
// The nbsp occupies one UTF-16 unit, so "bc" starts at offset 2, not 7.
|
||||
expect(collapsedRangeToCfi(doc, 0, 2, 4)).toBe('epubcfi(/6/2!/4/2,/1:2,/1:4)');
|
||||
});
|
||||
});
|
||||
|
||||
describe('xpointerRangeToCfi', () => {
|
||||
it('converts a simple in-paragraph range exactly', () => {
|
||||
const doc = parseChapterDocument(SIMPLE_CHAPTER);
|
||||
|
||||
@@ -19,6 +19,7 @@ import {
|
||||
resolveCfiParts,
|
||||
spineCfiForChapterIndex,
|
||||
} from './cfi.utils';
|
||||
import { decodeNamedEntities } from './xhtml-entities';
|
||||
import { buildXPointer, parseXPointer, resolveXPointerElement } from './xpointer.utils';
|
||||
|
||||
export const CONVERTER_VERSION = 1;
|
||||
@@ -46,7 +47,7 @@ export interface ConversionFailure {
|
||||
export type ConversionResult = ConversionSuccess | ConversionFailure;
|
||||
|
||||
export function parseChapterDocument(xhtml: string): ChapterDocument {
|
||||
const $ = cheerio.load(xhtml, { xml: true });
|
||||
const $ = cheerio.load(decodeNamedEntities(xhtml), { xml: true });
|
||||
const root = $.root()[0] as unknown as CfiNode;
|
||||
return { root, index: new ChapterTextIndex(root) };
|
||||
}
|
||||
|
||||
@@ -0,0 +1,71 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import { decodeNamedEntities } from './xhtml-entities';
|
||||
|
||||
describe('decodeNamedEntities', () => {
|
||||
it('resolves named entities an XML parser would leave literal', () => {
|
||||
expect(decodeNamedEntities('a b')).toBe('a b');
|
||||
expect(decodeNamedEntities('pa­sakyti')).toBe('pasakyti');
|
||||
expect(decodeNamedEntities('a b')).toBe('a b');
|
||||
expect(decodeNamedEntities('a—b…')).toBe('a—b…');
|
||||
expect(decodeNamedEntities('café')).toBe('café');
|
||||
});
|
||||
|
||||
it('leaves the predefined XML entities to the parser', () => {
|
||||
const markup = '&<>"'';
|
||||
expect(decodeNamedEntities(markup)).toBe(markup);
|
||||
});
|
||||
|
||||
it('leaves aliases that would resolve to markup-significant characters', () => {
|
||||
// < and & are HTML-only aliases; resolving them here would inject markup.
|
||||
const markup = '<p> & "';
|
||||
expect(decodeNamedEntities(markup)).toBe(markup);
|
||||
});
|
||||
|
||||
it('does not turn escaped entity text into a character', () => {
|
||||
expect(decodeNamedEntities('&nbsp;')).toBe('&nbsp;');
|
||||
expect(decodeNamedEntities('&lt;')).toBe('&lt;');
|
||||
});
|
||||
|
||||
it('leaves numeric character references to the parser', () => {
|
||||
expect(decodeNamedEntities('a b—c')).toBe('a b—c');
|
||||
});
|
||||
|
||||
it('leaves unknown and unterminated references untouched', () => {
|
||||
expect(decodeNamedEntities('¬anentity;   x&y')).toBe('¬anentity;   x&y');
|
||||
});
|
||||
|
||||
it('never partially matches a legacy entity inside a longer reference', () => {
|
||||
// HTML text decoding would read the `¬` prefix and leave the rest behind.
|
||||
expect(decodeNamedEntities('¬a;')).toBe('¬a;');
|
||||
expect(decodeNamedEntities('¬ify;')).toBe('¬ify;');
|
||||
expect(decodeNamedEntities('∉')).toBe('∉');
|
||||
});
|
||||
|
||||
it('resolves references that expand to two code points', () => {
|
||||
expect(decodeNamedEntities('fj')).toBe('fj');
|
||||
});
|
||||
|
||||
it('leaves references longer than the named character table untouched', () => {
|
||||
const overlong = `&${'a'.repeat(200)};`;
|
||||
expect(decodeNamedEntities(overlong)).toBe(overlong);
|
||||
});
|
||||
|
||||
it('resolves the longest reference in the named character table', () => {
|
||||
expect(decodeNamedEntities('∳')).toBe('∳');
|
||||
});
|
||||
|
||||
it('keeps entity references inside CDATA sections literal', () => {
|
||||
const markup = '<style><![CDATA[ .a::after { content: " " } ]]></style><p>a b</p>';
|
||||
expect(decodeNamedEntities(markup)).toBe('<style><![CDATA[ .a::after { content: " " } ]]></style><p>a b</p>');
|
||||
});
|
||||
|
||||
it('resolves references in attribute values', () => {
|
||||
expect(decodeNamedEntities('<img alt="café"/>')).toBe('<img alt="café"/>');
|
||||
});
|
||||
|
||||
it('is a no-op for markup without named references', () => {
|
||||
const markup = '<p>plain text with numbers 1 & 2</p>';
|
||||
expect(decodeNamedEntities(markup)).toBe(markup);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,56 @@
|
||||
/**
|
||||
* EPUB chapters are parsed in XML mode, which resolves only the five predefined
|
||||
* XML entities and numeric character references. EPUB2 XHTML may use any named
|
||||
* HTML entity through its DTD (` `, `­`, `—`, ...), and every real
|
||||
* renderer resolves them: browsers, crengine, and kepubify (which re-serializes
|
||||
* them as literal characters). Leaving them as literal text would make a chapter's
|
||||
* text index disagree with its own kepub and with the rendered document, so they
|
||||
* are resolved on the raw markup before parsing.
|
||||
*
|
||||
* Resolving must happen before parsing, not on the parsed text nodes: ` ` and
|
||||
* `&nbsp;` both parse to the text ` `, so afterwards the two are
|
||||
* indistinguishable and escaped markup would be corrupted.
|
||||
*/
|
||||
|
||||
import * as cheerio from 'cheerio';
|
||||
|
||||
/** CDATA sections keep entity references literal, so they are passed through whole. */
|
||||
const CDATA_OR_NAMED_ENTITY_RE = /<!\[CDATA\[[\s\S]*?\]\]>|&[a-zA-Z][a-zA-Z0-9]*;/g;
|
||||
|
||||
const MARKUP_SIGNIFICANT_RE = /[<>&"']/;
|
||||
|
||||
/** Longest reference in the HTML named character table is `∳`. */
|
||||
const MAX_REFERENCE_LENGTH = 34;
|
||||
|
||||
/** Bounded by the named character table, since longer candidates are never looked up. */
|
||||
const resolvedByReference = new Map<string, string | null>();
|
||||
|
||||
/** Resolves one `&name;` reference, or null when it must be left to the XML parser. */
|
||||
function resolveReference(reference: string): string | null {
|
||||
const cached = resolvedByReference.get(reference);
|
||||
if (cached !== undefined) return cached;
|
||||
|
||||
// Cheerio's default HTML mode carries the full named character table, and reading
|
||||
// the reference back from an attribute value applies HTML's strict matching: an
|
||||
// unknown reference stays intact rather than partially matching a legacy entity,
|
||||
// so `¬anentity;` does not become `¬anentity;`. The pattern above admits only
|
||||
// ASCII alphanumerics and a trailing semicolon, so inlining it here cannot escape
|
||||
// the attribute.
|
||||
const text = cheerio.load(`<i data-reference="${reference}"></i>`)('i').attr('data-reference') ?? reference;
|
||||
const resolved = text === reference || text.length === 0 || MARKUP_SIGNIFICANT_RE.test(text) ? null : text;
|
||||
resolvedByReference.set(reference, resolved);
|
||||
return resolved;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves named HTML entity references in XHTML markup to their characters.
|
||||
* References the XML parser already handles are left untouched, as are references
|
||||
* whose character is markup-significant (`&`, `<`, and case aliases such as
|
||||
* `<`), so resolving can never introduce markup or break an escaped sequence.
|
||||
*/
|
||||
export function decodeNamedEntities(markup: string): string {
|
||||
return markup.replace(CDATA_OR_NAMED_ENTITY_RE, (match) => {
|
||||
if (match.startsWith('<') || match.length > MAX_REFERENCE_LENGTH) return match;
|
||||
return resolveReference(match) ?? match;
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user