diff --git a/drizzle/postgres/0010_user-data-cleanup-cascades.sql b/drizzle/postgres/0010_user-data-cleanup-cascades.sql index f0e8dfd..4040691 100644 --- a/drizzle/postgres/0010_user-data-cleanup-cascades.sql +++ b/drizzle/postgres/0010_user-data-cleanup-cascades.sql @@ -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"; diff --git a/src/app/api/user/export/route.ts b/src/app/api/user/export/route.ts index dc7af36..e65b035 100644 --- a/src/app/api/user/export/route.ts +++ b/src/app/api/user/export/route.ts @@ -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); diff --git a/src/lib/server/tts/segments-cache.ts b/src/lib/server/tts/segments-cache.ts index b3d5803..cc63791 100644 --- a/src/lib/server/tts/segments-cache.ts +++ b/src/lib/server/tts/segments-cache.ts @@ -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 } : {}), diff --git a/src/lib/server/user/data-cleanup.ts b/src/lib/server/user/data-cleanup.ts index 0a13601..3787420 100644 --- a/src/lib/server/user/data-cleanup.ts +++ b/src/lib/server/user/data-cleanup.ts @@ -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)`); } } diff --git a/src/lib/server/user/data-export.ts b/src/lib/server/user/data-export.ts index 68b306b..8e173f3 100644 --- a/src/lib/server/user/data-export.ts +++ b/src/lib/server/user/data-export.ts @@ -72,6 +72,7 @@ export type AppendUserExportArchiveInput = { audiobookChapters: ExportAudiobookChapter[]; ttsSegmentEntries: ExportTtsSegmentEntry[]; ttsSegmentVariants: ExportTtsSegmentVariant[]; + storageEnabled: boolean; getDocumentBlobStream: (documentId: string) => Promise; listAudiobookObjects: (bookId: string, userId: string) => Promise; getAudiobookObjectStream: (bookId: string, userId: string, fileName: string) => Promise; @@ -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(); - 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, diff --git a/tests/unit/data-export.vitest.spec.ts b/tests/unit/data-export.vitest.spec.ts index 48fab66..bfc7806 100644 --- a/tests/unit/data-export.vitest.spec.ts +++ b/tests/unit/data-export.vitest.spec.ts @@ -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(); + 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(); + 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(); + }); }); diff --git a/tests/unit/tts-segments-cache.vitest.spec.ts b/tests/unit/tts-segments-cache.vitest.spec.ts new file mode 100644 index 0000000..18ce89b --- /dev/null +++ b/tests/unit/tts-segments-cache.vitest.spec.ts @@ -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, + }); + }); +}); diff --git a/tests/unit/user-data-cleanup.vitest.spec.ts b/tests/unit/user-data-cleanup.vitest.spec.ts index 1d613f9..f862813 100644 --- a/tests/unit/user-data-cleanup.vitest.spec.ts +++ b/tests/unit/user-data-cleanup.vitest.spec.ts @@ -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'); + }); });