danvics/openreader
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

test(user): expand cleanup and export coverage for shared docs and TTS segment files

Browse source 
Add tests for user data cleanup with shared document ownership and for TTS segment
variant export scenarios, including cases with duplicate audio keys and storage
disabled. Refactor cleanup logic to use document row types and transactional
ownership removal with last-owner checks. Update TTS segment cache clearing to
report deleted segment count by unique entry. Adjust export logic to always
include all TTS segment variants and conditionally export file buckets based on
storage availability. Update migration to use NOT VALID/VALIDATE for new
cascading constraints.
This commit is contained in:
Richard R 2026-06-06 17:38:30 -06:00
parent 76553023e8
commit 849f4a43e8
8 changed files with 350 additions and 35 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
drizzle/postgres/0010_user-data-cleanup-cascades.sql
View file

@ -1,8 +1,10 @@
ALTER TABLE "user_job_events" ADD CONSTRAINT "user_job_events_user_id_user_id_fk" FOREIGN KEY ("user_id") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE no action NOT VALID;--> statement-breakpoint
ALTER TABLE "user_tts_chars" ADD CONSTRAINT "user_tts_chars_user_id_user_id_fk" FOREIGN KEY ("user_id") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE no action NOT VALID;--> statement-breakpoint
DELETE FROM "user_job_events" WHERE NOT EXISTS (
SELECT 1 FROM "user" WHERE "user"."id" = "user_job_events"."user_id"
);--> statement-breakpoint
DELETE FROM "user_tts_chars" WHERE NOT EXISTS (
SELECT 1 FROM "user" WHERE "user"."id" = "user_tts_chars"."user_id"
);--> statement-breakpoint
ALTER TABLE "user_job_events" ADD CONSTRAINT "user_job_events_user_id_user_id_fk" FOREIGN KEY ("user_id") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint
ALTER TABLE "user_tts_chars" ADD CONSTRAINT "user_tts_chars_user_id_user_id_fk" FOREIGN KEY ("user_id") REFERENCES "public"."user"("id") ON DELETE cascade ON UPDATE no action;
ALTER TABLE "user_job_events" VALIDATE CONSTRAINT "user_job_events_user_id_user_id_fk";--> statement-breakpoint
ALTER TABLE "user_tts_chars" VALIDATE CONSTRAINT "user_tts_chars_user_id_user_id_fk";

1
src/app/api/user/export/route.ts
View file

@ -201,6 +201,7 @@ export async function GET(req: NextRequest) {
audiobookChapters: allChapters,
ttsSegmentEntries: segmentEntries,
ttsSegmentVariants: segmentVariants,
storageEnabled,
getDocumentBlobStream: async (documentId: string) => {
requireStorage();
return getDocumentBlobStream(documentId, testNamespace);

7
src/lib/server/tts/segments-cache.ts
View file

@ -40,6 +40,11 @@ export async function clearTtsSegmentCache(
conditions.push(eq(ttsSegmentEntries.readerType, input.readerType));
}
const entryRows = (await db
.select({ segmentEntryId: ttsSegmentEntries.segmentEntryId })
.from(ttsSegmentEntries)
.where(and(...conditions))) as Array<{ segmentEntryId: string }>;
const rows = (await db
.select({
segmentId: ttsSegmentVariants.segmentId,
@ -90,7 +95,7 @@ export async function clearTtsSegmentCache(
}
return {
deletedSegments: warning ? 0 : rows.length,
deletedSegments: warning ? 0 : new Set(entryRows.map((row) => row.segmentEntryId)).size,
requestedAudioObjects: uniqueAudioKeys.length,
deletedAudioObjects,
...(warning ? { warning } : {}),

88
src/lib/server/user/data-cleanup.ts
View file

@ -22,7 +22,7 @@ import { deleteTtsSegmentPrefix } from '@/lib/server/tts/segments-blobstore';
import { hashForLog, serverLogger } from '@/lib/server/logger';
import { logDegraded } from '@/lib/server/errors/logging';
type DocumentRow = { id: string };
type DocumentRow = typeof documents.$inferSelect;
type AudiobookRow = { id: string };
/**
@ -48,21 +48,75 @@ export async function deleteUserStorageData(
// --- Documents & previews ---
const userDocs: DocumentRow[] = await database
.select({ id: documents.id })
.select()
.from(documents)
.where(eq(documents.userId, userId));
let docsDeleted = 0;
const removedDocs: DocumentRow[] = [];
const restoreRemovedDocs = async () => {
if (removedDocs.length === 0) return;
const docsToRestore = [...removedDocs];
await database.insert(documents).values(docsToRestore).onConflictDoNothing();
removedDocs.length = 0;
};
const removeOwnershipAndCheckLastOwner = async (doc: DocumentRow) => {
const run = async (tx: typeof database) => {
// Lock every ownership row for this document so concurrent account
// deletions cannot both observe the other owner's uncommitted row.
await tx
.select({ id: documents.id })
.from(documents)
.where(eq(documents.id, doc.id))
.for('update');
const [removedDoc] = await tx
.delete(documents)
.where(and(eq(documents.id, doc.id), eq(documents.userId, userId)))
.returning();
if (!removedDoc) return { removedDoc: null, isLastOwner: false };
const otherOwners = await tx
.select({ id: documents.id })
.from(documents)
.where(and(
eq(documents.id, doc.id),
ne(documents.userId, userId),
))
.limit(1);
return { removedDoc, isLastOwner: otherOwners.length === 0 };
};
if (process.env.POSTGRES_URL) {
return database.transaction(run);
}
return database.transaction((tx: typeof database) => {
const removedRows = tx
.delete(documents)
.where(and(eq(documents.id, doc.id), eq(documents.userId, userId)))
.returning()
.all();
const removedDoc = removedRows[0] ?? null;
if (!removedDoc) return { removedDoc: null, isLastOwner: false };
const otherOwners = tx
.select({ id: documents.id })
.from(documents)
.where(and(
eq(documents.id, doc.id),
ne(documents.userId, userId),
))
.limit(1)
.all();
return { removedDoc, isLastOwner: otherOwners.length === 0 };
}, { behavior: 'immediate' });
};
for (const doc of userDocs) {
const otherOwners = await database
.select({ id: documents.id })
.from(documents)
.where(and(
eq(documents.id, doc.id),
ne(documents.userId, userId),
))
.limit(1);
const isLastOwner = otherOwners.length === 0;
const { removedDoc, isLastOwner } = await removeOwnershipAndCheckLastOwner(doc);
if (!removedDoc) continue;
removedDocs.push(removedDoc);
if (s3Enabled && isLastOwner) {
try {
@ -99,9 +153,9 @@ export async function deleteUserStorageData(
}
}
// Preview rows/artifacts are shared by document id, so only the final
// owner may remove them.
if (isLastOwner) {
// Preview metadata is global, so only the canonical final-owner pass may
// remove it.
if (namespace === null && isLastOwner) {
try {
await deleteDocumentPreviewRows(doc.id, namespace);
} catch (error) {
@ -118,6 +172,10 @@ export async function deleteUserStorageData(
});
}
}
if (namespace !== null) {
await restoreRemovedDocs();
}
}
// --- Audiobooks ---
@ -185,6 +243,7 @@ export async function deleteUserStorageData(
}
if (failures.length > 0) {
await restoreRemovedDocs();
throw new AggregateError(failures, `User storage cleanup failed in ${failures.length} operation(s)`);
}
@ -225,6 +284,7 @@ export async function deleteUserStorageData(
}
if (failures.length > 0) {
await restoreRemovedDocs();
throw new AggregateError(failures, `User database cleanup failed in ${failures.length} operation(s)`);
}
}

18
src/lib/server/user/data-export.ts
View file

@ -72,6 +72,7 @@ export type AppendUserExportArchiveInput = {
audiobookChapters: ExportAudiobookChapter[];
ttsSegmentEntries: ExportTtsSegmentEntry[];
ttsSegmentVariants: ExportTtsSegmentVariant[];
storageEnabled: boolean;
getDocumentBlobStream: (documentId: string) => Promise<ExportBlobBody>;
listAudiobookObjects: (bookId: string, userId: string) => Promise<ExportAudiobookObject[]>;
getAudiobookObjectStream: (bookId: string, userId: string, fileName: string) => Promise<ExportBlobBody>;
@ -154,6 +155,7 @@ export async function appendUserExportArchive(input: AppendUserExportArchiveInpu
audiobookChapters,
ttsSegmentEntries,
ttsSegmentVariants,
storageEnabled,
getDocumentBlobStream,
listAudiobookObjects,
getAudiobookObjectStream,
@ -192,7 +194,7 @@ export async function appendUserExportArchive(input: AppendUserExportArchiveInpu
}));
appendJson(archive, 'library_audiobooks.json', audiobooksWithChapters);
for (const doc of documents) {
for (const doc of storageEnabled ? documents : []) {
const documentId = toSafePathSegment(doc.id, 'document');
const fileName = toSafePathSegment(doc.name || `${doc.id}.bin`, `${documentId}.bin`);
const entryName = `files/documents/${documentId}/${fileName}`;
@ -212,7 +214,7 @@ export async function appendUserExportArchive(input: AppendUserExportArchiveInpu
}
}
for (const book of audiobooks) {
for (const book of storageEnabled ? audiobooks : []) {
let objects: ExportAudiobookObject[] = [];
try {
objects = await listAudiobookObjects(book.id, userId);
@ -253,11 +255,9 @@ export async function appendUserExportArchive(input: AppendUserExportArchiveInpu
const documentIdByEntryId = new Map(
ttsSegmentEntries.map((entry) => [entry.segmentEntryId, entry.documentId]),
);
const exportedAudioKeys = new Set<string>();
for (const variant of ttsSegmentVariants) {
for (const variant of storageEnabled ? ttsSegmentVariants : []) {
const audioKey = typeof variant.audioKey === 'string' ? variant.audioKey : '';
if (!audioKey || exportedAudioKeys.has(audioKey)) continue;
exportedAudioKeys.add(audioKey);
if (!audioKey) continue;
const documentId = toSafePathSegment(
documentIdByEntryId.get(variant.segmentEntryId) ?? 'unknown-document',
@ -304,9 +304,9 @@ export async function appendUserExportArchive(input: AppendUserExportArchiveInpu
},
includes: {
metadata: true,
documentFiles: true,
audiobookFiles: true,
ttsSegmentFiles: true,
documentFiles: storageEnabled,
audiobookFiles: storageEnabled,
ttsSegmentFiles: storageEnabled,
credentialSecrets: false,
temporaryUploads: false,
derivedDocumentPreviews: false,

93
tests/unit/data-export.vitest.spec.ts
View file

@ -1,4 +1,4 @@
import { describe, expect, test } from 'vitest';
import { describe, expect, test, vi } from 'vitest';
import type { Archiver } from 'archiver';
import { appendUserExportArchive } from '../../src/lib/server/user/data-export';
@ -34,6 +34,7 @@ describe('user data export archive', () => {
audioKey: 'private/audio/key',
audioFormat: 'mp3',
}],
storageEnabled: true,
getDocumentBlobStream: async () => new Uint8Array(),
listAudiobookObjects: async () => [],
getAudiobookObjectStream: async () => new Uint8Array(),
@ -60,4 +61,94 @@ describe('user data export archive', () => {
},
});
});
test('exports every TTS variant even when variants share an audio key', async () => {
const entries = new Map<string, unknown>();
const archive = {
append(value: unknown, options: { name: string }) {
entries.set(options.name, value);
return archive;
},
} as unknown as Archiver;
const getTtsSegmentAudioStream = vi.fn(async () => new Uint8Array([1]));
await appendUserExportArchive({
archive,
userId: 'user-1',
exportedAtMs: 123,
profileData: {},
preferences: null,
readingHistory: [],
ttsUsage: [],
jobEvents: [],
documentSettings: [],
authSessions: [],
linkedAccounts: [],
documents: [],
audiobooks: [],
audiobookChapters: [],
ttsSegmentEntries: [{ segmentEntryId: 'entry-1', documentId: 'doc-1' }],
ttsSegmentVariants: [
{ segmentId: 'segment-1', segmentEntryId: 'entry-1', audioKey: 'shared-key', audioFormat: 'mp3' },
{ segmentId: 'segment-2', segmentEntryId: 'entry-1', audioKey: 'shared-key', audioFormat: 'mp3' },
],
storageEnabled: true,
getDocumentBlobStream: async () => new Uint8Array(),
listAudiobookObjects: async () => [],
getAudiobookObjectStream: async () => new Uint8Array(),
getTtsSegmentAudioStream,
});
expect(entries.has('files/tts_segments/doc-1/segment-1.mp3')).toBe(true);
expect(entries.has('files/tts_segments/doc-1/segment-2.mp3')).toBe(true);
expect(getTtsSegmentAudioStream).toHaveBeenCalledTimes(2);
});
test('reports file buckets as excluded when storage is disabled', async () => {
const entries = new Map<string, unknown>();
const archive = {
append(value: unknown, options: { name: string }) {
entries.set(options.name, value);
return archive;
},
} as unknown as Archiver;
const getDocumentBlobStream = vi.fn(async () => new Uint8Array());
await appendUserExportArchive({
archive,
userId: 'user-1',
exportedAtMs: 123,
profileData: {},
preferences: null,
readingHistory: [],
ttsUsage: [],
jobEvents: [],
documentSettings: [],
authSessions: [],
linkedAccounts: [],
documents: [{ id: 'doc-1', name: 'doc.pdf' }],
audiobooks: [],
audiobookChapters: [],
ttsSegmentEntries: [],
ttsSegmentVariants: [],
storageEnabled: false,
getDocumentBlobStream,
listAudiobookObjects: async () => [],
getAudiobookObjectStream: async () => new Uint8Array(),
getTtsSegmentAudioStream: async () => new Uint8Array(),
});
const manifest = JSON.parse(String(entries.get('export_manifest.json')));
expect(manifest.includes).toMatchObject({
documentFiles: false,
audiobookFiles: false,
ttsSegmentFiles: false,
});
expect(manifest.counts).toMatchObject({
documentFiles: 0,
audiobookFiles: 0,
ttsSegmentFiles: 0,
});
expect(getDocumentBlobStream).not.toHaveBeenCalled();
});
});

78
tests/unit/tts-segments-cache.vitest.spec.ts Normal file
View file

@ -0,0 +1,78 @@
import { beforeEach, describe, expect, test, vi } from 'vitest';
const mocks = vi.hoisted(() => ({
selectResults: [] as unknown[][],
deleteWhere: vi.fn(async () => undefined),
deleteTtsSegmentAudioObjects: vi.fn(async () => 0),
}));
function resultBuilder(result: unknown[]) {
return {
innerJoin: vi.fn(() => ({
where: vi.fn(async () => result),
})),
where: vi.fn(async () => result),
};
}
vi.mock('@/db', () => ({
db: {
select: vi.fn(() => ({
from: vi.fn(() => resultBuilder(mocks.selectResults.shift() ?? [])),
})),
delete: vi.fn(() => ({
where: mocks.deleteWhere,
})),
},
}));
vi.mock('@/lib/server/tts/segments-blobstore', () => ({
deleteTtsSegmentAudioObjects: mocks.deleteTtsSegmentAudioObjects,
deleteTtsSegmentPrefix: vi.fn(async () => 0),
}));
vi.mock('@/lib/server/storage/s3', () => ({
getS3Config: () => ({ prefix: 'openreader-test' }),
}));
vi.mock('@/lib/server/logger', () => ({
serverLogger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
}));
vi.mock('@/lib/server/errors/logging', () => ({
logDegraded: vi.fn(),
}));
import { clearTtsSegmentCache } from '../../src/lib/server/tts/segments-cache';
describe('TTS segment cache cleanup', () => {
beforeEach(() => {
mocks.selectResults = [];
mocks.deleteWhere.mockReset();
mocks.deleteWhere.mockResolvedValue(undefined);
mocks.deleteTtsSegmentAudioObjects.mockReset();
mocks.deleteTtsSegmentAudioObjects.mockResolvedValue(2);
});
test('counts deleted entries rather than joined variants', async () => {
mocks.selectResults = [
[{ segmentEntryId: 'entry-1' }, { segmentEntryId: 'entry-2' }],
[
{ segmentId: 'variant-1', audioKey: 'audio-1' },
{ segmentId: 'variant-2', audioKey: 'audio-2' },
{ segmentId: 'variant-3', audioKey: 'audio-2' },
],
];
const result = await clearTtsSegmentCache({
userId: 'user-1',
documentId: 'doc-1',
});
expect(result).toMatchObject({
deletedSegments: 2,
requestedAudioObjects: 2,
deletedAudioObjects: 2,
});
});
});

94
tests/unit/user-data-cleanup.vitest.spec.ts
View file

@ -1,8 +1,11 @@
import { beforeEach, describe, expect, test, vi } from 'vitest';
import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest';
const mocks = vi.hoisted(() => ({
selectResults: [] as unknown[][],
documentDeleteResults: [] as unknown[][],
deleteWhere: vi.fn(async () => undefined),
insertValues: vi.fn(() => ({ onConflictDoNothing: vi.fn(async () => undefined) })),
forUpdate: vi.fn(),
deleteDocumentBlob: vi.fn(async () => undefined),
deleteDocumentPrefix: vi.fn(async () => 0),
deleteDocumentPreviewArtifacts: vi.fn(async () => 0),
@ -12,25 +15,47 @@ const mocks = vi.hoisted(() => ({
}));
function resultBuilder(result: unknown[]) {
const limitedResult = {
all: () => result,
then: (resolve: (value: unknown[]) => unknown, reject: (error: unknown) => unknown) =>
Promise.resolve(result).then(resolve, reject),
};
return {
limit: async () => result,
all: () => result,
for: vi.fn((mode: string) => {
mocks.forUpdate(mode);
return Promise.resolve(result);
}),
limit: () => limitedResult,
then: (resolve: (value: unknown[]) => unknown, reject: (error: unknown) => unknown) =>
Promise.resolve(result).then(resolve, reject),
};
}
vi.mock('@/db', () => ({
db: {
vi.mock('@/db', () => {
const database = {
select: vi.fn(() => ({
from: vi.fn(() => ({
where: vi.fn(() => resultBuilder(mocks.selectResults.shift() ?? [])),
})),
})),
delete: vi.fn(() => ({
where: mocks.deleteWhere,
where: vi.fn(() => {
const promise = mocks.deleteWhere();
return {
then: promise.then.bind(promise),
catch: promise.catch.bind(promise),
returning: () => resultBuilder(mocks.documentDeleteResults.shift() ?? []),
};
}),
})),
},
}));
insert: vi.fn(() => ({
values: mocks.insertValues,
})),
transaction: vi.fn((callback: (tx: unknown) => unknown) => callback(database)),
};
return { db: database };
});
vi.mock('@/lib/server/storage/s3', () => ({
isS3Configured: () => true,
@ -72,14 +97,21 @@ vi.mock('@/lib/server/errors/logging', () => ({
import { deleteUserStorageData } from '../../src/lib/server/user/data-cleanup';
describe('user data cleanup', () => {
afterEach(() => {
vi.unstubAllEnvs();
});
beforeEach(() => {
mocks.selectResults = [];
mocks.documentDeleteResults = [];
for (const mock of Object.values(mocks)) {
if (typeof mock === 'function' && 'mockReset' in mock) {
mock.mockReset();
}
}
mocks.deleteWhere.mockResolvedValue(undefined);
mocks.insertValues.mockReturnValue({ onConflictDoNothing: vi.fn(async () => undefined) });
mocks.forUpdate.mockReset();
mocks.deleteDocumentBlob.mockResolvedValue(undefined);
mocks.deleteDocumentPrefix.mockResolvedValue(0);
mocks.deleteDocumentPreviewArtifacts.mockResolvedValue(0);
@ -94,13 +126,14 @@ describe('user data cleanup', () => {
[{ id: 'shared-doc' }],
[],
];
mocks.documentDeleteResults = [[{ id: 'shared-doc' }]];
await deleteUserStorageData('user-1', null);
expect(mocks.deleteDocumentBlob).not.toHaveBeenCalled();
expect(mocks.deleteDocumentPreviewArtifacts).not.toHaveBeenCalled();
expect(mocks.deleteDocumentPreviewRows).not.toHaveBeenCalled();
expect(mocks.deleteWhere).toHaveBeenCalledTimes(3);
expect(mocks.deleteWhere).toHaveBeenCalledTimes(4);
});
test('blocks database cleanup when storage cleanup fails', async () => {
@ -112,4 +145,49 @@ describe('user data cleanup', () => {
);
expect(mocks.deleteWhere).not.toHaveBeenCalled();
});
test('restores document ownership when document storage cleanup fails', async () => {
mocks.selectResults = [
[{ id: 'doc-1' }],
[],
[],
];
mocks.documentDeleteResults = [[{ id: 'doc-1' }]];
mocks.deleteDocumentBlob.mockRejectedValueOnce(new Error('storage unavailable'));
await expect(deleteUserStorageData('user-1', null)).rejects.toThrow(
'User storage cleanup failed',
);
expect(mocks.insertValues).toHaveBeenCalledWith([{ id: 'doc-1' }]);
});
test('does not delete global preview rows during namespaced cleanup', async () => {
mocks.selectResults = [
[{ id: 'doc-1' }],
[],
[],
];
mocks.documentDeleteResults = [[{ id: 'doc-1' }]];
await deleteUserStorageData('user-1', 'test-ns');
expect(mocks.deleteDocumentPreviewRows).not.toHaveBeenCalled();
expect(mocks.insertValues).toHaveBeenCalledWith([{ id: 'doc-1' }]);
});
test('locks all document owners before the Postgres ownership decision', async () => {
vi.stubEnv('POSTGRES_URL', 'postgres://test');
mocks.selectResults = [
[{ id: 'doc-1' }],
[{ id: 'doc-1' }],
[],
[],
];
mocks.documentDeleteResults = [[{ id: 'doc-1' }]];
await deleteUserStorageData('user-1', null);
expect(mocks.forUpdate).toHaveBeenCalledWith('update');
});
});