From 0bc36aa58c18ad3868a98dc2506674c7374a9fa8 Mon Sep 17 00:00:00 2001 From: Claude Fable 5 Date: Thu, 30 Jul 2026 13:19:58 +0200 Subject: [PATCH] #194: orphan-file sweep, drop the unused Attachment.deletedAt Nightly sweep with two directions: attachments still unclaimed (pageId null) after a 24 h grace period - claimed by no collab persist, page upload, or import - are reclaimed (row, file, quota released); files on the uploads volume without a database row (drift after a crashed upload) are removed once older than the grace period. The grace period protects the paste-then-insert window. Deliberate deviation from the issue's content-reference idea, documented in schema comment and operations.md: claimed attachments whose page content no longer embeds them are NOT auto-deleted. The page attachments panel lists claimed files as user-managed objects (inserting into the document is optional there), so 'not embedded' is not 'unused' - an auto-delete would destroy panel assets. Humans clean those up in the panel or the pond file manager, which flags orphans already. Attachment.deletedAt is removed by migration - deletion is hard everywhere (sweep, purge, manual), there is no soft-delete state; the never-true deletedAt:null filters in files/export queries went with it. Refs #194 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0168Ph5uBmHm8X28CSVpbpnJ --- .../migration.sql | 4 + apps/api/prisma/schema.prisma | 40 ++-- apps/api/src/files/file-storage.service.ts | 35 +++- apps/api/src/files/files.e2e.db.test.ts | 1 - apps/api/src/files/files.module.ts | 30 ++- apps/api/src/files/files.service.ts | 4 +- .../api/src/files/orphan-sweep.e2e.db.test.ts | 179 ++++++++++++++++++ apps/api/src/files/orphan-sweep.service.ts | 81 ++++++++ apps/api/src/import-export/export.service.ts | 4 +- docs/architecture/operations.md | 12 ++ docs/vs-nfd/20-massnahmenplan.md | 2 +- 11 files changed, 362 insertions(+), 30 deletions(-) create mode 100644 apps/api/prisma/migrations/20260730120000_drop_attachment_deleted_at/migration.sql create mode 100644 apps/api/src/files/orphan-sweep.e2e.db.test.ts create mode 100644 apps/api/src/files/orphan-sweep.service.ts diff --git a/apps/api/prisma/migrations/20260730120000_drop_attachment_deleted_at/migration.sql b/apps/api/prisma/migrations/20260730120000_drop_attachment_deleted_at/migration.sql new file mode 100644 index 0000000..978b98f --- /dev/null +++ b/apps/api/prisma/migrations/20260730120000_drop_attachment_deleted_at/migration.sql @@ -0,0 +1,4 @@ +-- Issue #194: attachments have exactly one deletion semantics (hard delete +-- by sweep, purge, or manual removal) — the never-written soft-delete +-- marker goes away. +ALTER TABLE "attachments" DROP COLUMN "deleted_at"; diff --git a/apps/api/prisma/schema.prisma b/apps/api/prisma/schema.prisma index 227080c..1cf856e 100644 --- a/apps/api/prisma/schema.prisma +++ b/apps/api/prisma/schema.prisma @@ -529,26 +529,28 @@ model PondUsage { /// `//` (FileStorageService); this row carries the /// metadata needed to serve and account for it. `pageId` starts unset — /// images are uploaded before the page referencing them is known -/// (paste-then-insert, issue #28) — and is set on every page state save to -/// whichever page's document currently embeds the file (issue #31, -/// `PagesService.saveState`); the trash-purge job uses that link to delete -/// a purged page's files. Not touched when an image is later removed from -/// its page's content — an orphan-file sweep to reclaim those is a -/// separate future maintenance job (operations.md), not this one. -/// `deletedAt` stays unused for now — purge hard-deletes attachments -/// directly rather than soft-deleting them first — reserved for that same -/// future orphan-sweep job. +/// (paste-then-insert, issue #28) — and is claimed on every collab persist +/// by whichever page's document embeds the file (issue #31), or at upload +/// for the page attachments panel (#61); the trash-purge job uses that +/// link to delete a purged page's files. A row whose `pageId` is STILL +/// null after a grace period was claimed by nothing and is reclaimed by +/// the nightly orphan-file sweep (issue #194, OrphanSweepService). +/// Claimed files are deliberately NOT auto-reclaimed when the content +/// stops referencing them: the page attachments panel lists them as +/// user-managed objects (insert is optional there), so "not embedded" is +/// not "unused" — the pond file manager is the human cleanup path. +/// Deletion is hard everywhere (sweep, purge, manual) — there is no +/// soft-delete state on attachments (issue #194 removed `deletedAt`). model Attachment { - id String @id @default(uuid()) - pondId String @map("pond_id") - pageId String? @map("page_id") - fileName String @map("file_name") - mimeType String @map("mime_type") - sizeBytes Int @map("size_bytes") - storagePath String @map("storage_path") - uploadedBy String @map("uploaded_by") - createdAt DateTime @default(now()) @map("created_at") - deletedAt DateTime? @map("deleted_at") + id String @id @default(uuid()) + pondId String @map("pond_id") + pageId String? @map("page_id") + fileName String @map("file_name") + mimeType String @map("mime_type") + sizeBytes Int @map("size_bytes") + storagePath String @map("storage_path") + uploadedBy String @map("uploaded_by") + createdAt DateTime @default(now()) @map("created_at") pond Pond @relation(fields: [pondId], references: [id]) page Page? @relation(fields: [pageId], references: [id]) diff --git a/apps/api/src/files/file-storage.service.ts b/apps/api/src/files/file-storage.service.ts index 3a42655..3a7cba7 100644 --- a/apps/api/src/files/file-storage.service.ts +++ b/apps/api/src/files/file-storage.service.ts @@ -1,5 +1,5 @@ import { createReadStream } from 'node:fs'; -import { access, mkdir, rm, writeFile } from 'node:fs/promises'; +import { access, mkdir, readdir, rm, stat, writeFile } from 'node:fs/promises'; import { join } from 'node:path'; import type { Readable } from 'node:stream'; @@ -46,4 +46,37 @@ export class FileStorageService { async delete(pondId: string, fileId: string): Promise { await rm(this.pathFor(pondId, fileId), { force: true }); } + + /** + * Every stored file with its modification time, for the orphan sweep's + * volume↔database direction (issue #194, ADR 0011). A missing uploads + * directory is an empty volume, not an error. + */ + async listStored(): Promise<{ pondId: string; fileId: string; mtimeMs: number }[]> { + const root = this.config.env.UPLOADS_DIR; + const result: { pondId: string; fileId: string; mtimeMs: number }[] = []; + let pondDirs: string[]; + try { + pondDirs = await readdir(root); + } catch { + return result; + } + for (const pondId of pondDirs) { + let files: string[]; + try { + files = await readdir(join(root, pondId)); + } catch { + continue; // not a directory or vanished mid-walk + } + for (const fileId of files) { + try { + const info = await stat(join(root, pondId, fileId)); + if (info.isFile()) result.push({ pondId, fileId, mtimeMs: info.mtimeMs }); + } catch { + // vanished mid-walk — the next sweep sees the truth + } + } + } + return result; + } } diff --git a/apps/api/src/files/files.e2e.db.test.ts b/apps/api/src/files/files.e2e.db.test.ts index 8501d61..1e5fe07 100644 --- a/apps/api/src/files/files.e2e.db.test.ts +++ b/apps/api/src/files/files.e2e.db.test.ts @@ -301,7 +301,6 @@ describe.skipIf(!hasTestDb)('files (e2e, issue #27)', () => { await api().get(`/api/v1/media/${uploaded.body.id}`).set('Cookie', ownerCookie).expect(200); const stillThere = await prisma.attachment.findUnique({ where: { id: uploaded.body.id } }); expect(stillThere).not.toBeNull(); - expect(stillThere?.deletedAt).toBeNull(); }); it('lists a page attachment for the page and links it (#61)', async () => { diff --git a/apps/api/src/files/files.module.ts b/apps/api/src/files/files.module.ts index 1fa378a..fcddffe 100644 --- a/apps/api/src/files/files.module.ts +++ b/apps/api/src/files/files.module.ts @@ -1,16 +1,38 @@ -import { Module } from '@nestjs/common'; +import { Module, OnModuleInit } from '@nestjs/common'; +import { CommonModule } from '../common/common.module'; import { PondsModule } from '../ponds/ponds.module'; import { QuotasModule } from '../quotas/quotas.module'; +import { SchedulerModule } from '../scheduler/scheduler.module'; +import { SchedulerService } from '../scheduler/scheduler.service'; import { FileStorageService } from './file-storage.service'; import { FilesController } from './files.controller'; import { FilesService } from './files.service'; +import { OrphanSweepService } from './orphan-sweep.service'; + +/** Nightly, per operations.md's maintenance-jobs table (issue #194). */ +const ORPHAN_SWEEP_CADENCE_SECONDS = 24 * 60 * 60; @Module({ - imports: [PondsModule, QuotasModule], + imports: [CommonModule, PondsModule, QuotasModule, SchedulerModule], controllers: [FilesController], - providers: [FilesService, FileStorageService], + providers: [FilesService, FileStorageService, OrphanSweepService], exports: [FileStorageService, FilesService], }) -export class FilesModule {} +export class FilesModule implements OnModuleInit { + constructor( + private readonly scheduler: SchedulerService, + private readonly sweep: OrphanSweepService, + ) {} + + onModuleInit(): void { + this.scheduler.register({ + name: 'orphan-file-sweep', + cadenceSeconds: ORPHAN_SWEEP_CADENCE_SECONDS, + run: async () => { + await this.sweep.sweep(); + }, + }); + } +} diff --git a/apps/api/src/files/files.service.ts b/apps/api/src/files/files.service.ts index da943be..444f945 100644 --- a/apps/api/src/files/files.service.ts +++ b/apps/api/src/files/files.service.ts @@ -208,7 +208,7 @@ export class FilesService { /** Attachments linked to a page, for its attachments section (#61). */ async listForPage(pageId: string): Promise { const rows = await this.prisma.attachment.findMany({ - where: { pageId, deletedAt: null }, + where: { pageId }, orderBy: { createdAt: 'desc' }, include: { uploader: true, page: true }, }); @@ -219,7 +219,7 @@ export class FilesService { async listForPond(pondId: string): Promise { const [rows, usage, storageBytesLimit] = await Promise.all([ this.prisma.attachment.findMany({ - where: { pondId, deletedAt: null }, + where: { pondId }, orderBy: { createdAt: 'desc' }, include: { uploader: true, page: true }, }), diff --git a/apps/api/src/files/orphan-sweep.e2e.db.test.ts b/apps/api/src/files/orphan-sweep.e2e.db.test.ts new file mode 100644 index 0000000..825b042 --- /dev/null +++ b/apps/api/src/files/orphan-sweep.e2e.db.test.ts @@ -0,0 +1,179 @@ +import { existsSync } from 'node:fs'; +import { utimes, writeFile, mkdir } from 'node:fs/promises'; +import { join } from 'node:path'; + +import { INestApplication } from '@nestjs/common'; +import { PrismaClient } from '@prisma/client'; +import request from 'supertest'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +import { createTestApp, sessionCookieOf } from '../testing/test-app'; +import { createTestPrisma, hasTestDb, uniqueSuffix } from '../testing/test-db'; +import { UsersService } from '../users/users.service'; +import { OrphanSweepService } from './orphan-sweep.service'; + +const HOUR = 60 * 60 * 1000; + +/** + * Orphan-file sweep (issue #194): unclaimed attachments past the grace + * period are reclaimed (row, file, quota), fresh ones are protected + * (paste-then-insert), claimed ones are never touched — the page + * attachments panel is a legitimate reference — and stray files without a + * database row disappear once old enough. + */ +describe.skipIf(!hasTestDb)('orphan file sweep (e2e, issue #194)', () => { + let app: INestApplication; + let prisma: PrismaClient; + const suffix = uniqueSuffix(); + const password = 'orphan sweep pass 1'; + const ids: Record = {}; + const cookies: Record = {}; + let pondId: string; + let pageId: string; + + const api = () => request(app.getHttpServer()); + const fileOnDisk = (fondId: string, fileId: string) => + join(process.env.UPLOADS_DIR!, fondId, fileId); + + async function makeUser(handle: string, siteAdmin = false): Promise { + const users = app.get(UsersService); + const username = `os-${handle}-${suffix}`; + const user = await users.createUser({ + username, + email: `${username}@example.org`, + displayName: `Sweep ${handle}`, + password, + locale: 'en', + }); + ids[handle] = user.id; + await users.markEmailVerified(user.id); + if (siteAdmin) { + await prisma.user.update({ where: { id: user.id }, data: { isSiteAdmin: true } }); + } + cookies[handle] = sessionCookieOf( + await api() + .post('/api/v1/auth/login') + .send({ usernameOrEmail: username, password }) + .expect(200), + ); + } + + /** A real upload via the pond route (pageId stays null = unclaimed). */ + async function uploadUnclaimed(name: string): Promise { + const res = await api() + .post(`/api/v1/ponds/${pondId}/files`) + .set('Cookie', cookies.owner!) + .attach('file', Buffer.from(`bytes of ${name}`), name) + .expect(201); + return res.body.id as string; + } + + function backdate(attachmentId: string, ageMs: number): Promise { + return prisma.attachment.update({ + where: { id: attachmentId }, + data: { createdAt: new Date(Date.now() - ageMs) }, + }); + } + + beforeAll(async () => { + prisma = createTestPrisma(); + await prisma.rateLimit.deleteMany({}); + app = await createTestApp(); + await makeUser('owner'); + await makeUser('admin', true); + await api() + .put(`/api/v1/admin/quotas/user/${ids.owner!}/additional_ponds`) + .set('Cookie', cookies.admin!) + .send({ value: 5 }) + .expect(200); + const pond = await api() + .post('/api/v1/ponds') + .set('Cookie', cookies.owner!) + .send({ name: `Sweep Pond ${suffix}` }) + .expect(201); + pondId = pond.body.id; + const page = await api() + .post(`/api/v1/ponds/${pondId}/pages`) + .set('Cookie', cookies.owner!) + .send({ title: `Sweep Page ${suffix}` }) + .expect(201); + pageId = page.body.id; + }); + + afterAll(async () => { + const all = Object.values(ids); + await prisma.quotaOverride.deleteMany({ where: { subjectId: { in: all } } }); + await prisma.auditEntry.deleteMany({ where: { actorId: { in: all } } }); + const ponds = await prisma.pond.findMany({ + where: { ownerId: { in: all } }, + select: { id: true }, + }); + const pondIds = ponds.map((p) => p.id); + await prisma.attachment.deleteMany({ where: { pondId: { in: pondIds } } }); + await prisma.page.deleteMany({ where: { pondId: { in: pondIds } } }); + await prisma.pond.deleteMany({ where: { id: { in: pondIds } } }); + await prisma.watch.deleteMany({ where: { userId: { in: all } } }); + await prisma.session.deleteMany({ where: { userId: { in: all } } }); + await prisma.userIdentity.deleteMany({ where: { userId: { in: all } } }); + await prisma.user.deleteMany({ where: { id: { in: all } } }); + await prisma.$disconnect(); + await app.close(); + }); + + it('reclaims unclaimed attachments past the grace period, protects fresh and claimed ones', async () => { + const oldUnclaimed = await uploadUnclaimed('old-unclaimed.txt'); + const freshUnclaimed = await uploadUnclaimed('fresh-unclaimed.txt'); + const oldClaimed = await api() + .post(`/api/v1/pages/${pageId}/files`) + .set('Cookie', cookies.owner!) + .attach('file', Buffer.from('panel asset'), 'panel-asset.txt') + .expect(201); + await backdate(oldUnclaimed, 25 * HOUR); + await backdate(oldClaimed.body.id, 25 * HOUR); + + const usageBefore = await prisma.pondUsage.findUnique({ where: { pondId } }); + const reclaimedBytes = ( + await prisma.attachment.findUniqueOrThrow({ + where: { id: oldUnclaimed }, + }) + ).sizeBytes; + + const result = await app.get(OrphanSweepService).sweep(); + expect(result.reclaimed).toBeGreaterThanOrEqual(1); + + // The old unclaimed upload is gone: row, file, quota. + expect(await prisma.attachment.findUnique({ where: { id: oldUnclaimed } })).toBeNull(); + expect(existsSync(fileOnDisk(pondId, oldUnclaimed))).toBe(false); + const usageAfter = await prisma.pondUsage.findUnique({ where: { pondId } }); + expect(Number(usageBefore!.storageBytesUsed) - Number(usageAfter!.storageBytesUsed)).toBe( + reclaimedBytes, + ); + + // The fresh unclaimed upload survives (paste-then-insert grace). + expect(await prisma.attachment.findUnique({ where: { id: freshUnclaimed } })).not.toBeNull(); + expect(existsSync(fileOnDisk(pondId, freshUnclaimed))).toBe(true); + + // The claimed panel asset survives despite its age — never swept. + expect( + await prisma.attachment.findUnique({ where: { id: oldClaimed.body.id } }), + ).not.toBeNull(); + expect(existsSync(fileOnDisk(pondId, oldClaimed.body.id))).toBe(true); + }); + + it('removes stray files without a database row once they are old enough', async () => { + const dir = join(process.env.UPLOADS_DIR!, pondId); + await mkdir(dir, { recursive: true }); + const oldStray = join(dir, `stray-old-${suffix}`); + const freshStray = join(dir, `stray-fresh-${suffix}`); + await writeFile(oldStray, 'stray bytes'); + await writeFile(freshStray, 'stray bytes'); + const past = new Date(Date.now() - 25 * HOUR); + await utimes(oldStray, past, past); + + const result = await app.get(OrphanSweepService).sweep(); + + expect(existsSync(oldStray)).toBe(false); + expect(existsSync(freshStray)).toBe(true); + expect(result.strays).toBeGreaterThanOrEqual(1); + }); +}); diff --git a/apps/api/src/files/orphan-sweep.service.ts b/apps/api/src/files/orphan-sweep.service.ts new file mode 100644 index 0000000..1bddac5 --- /dev/null +++ b/apps/api/src/files/orphan-sweep.service.ts @@ -0,0 +1,81 @@ +import { Injectable } from '@nestjs/common'; +import { PinoLogger } from 'nestjs-pino'; + +import { ClockService } from '../common/clock.service'; +import { PrismaService } from '../prisma/prisma.service'; +import { QuotaService } from '../quotas/quota.service'; + +import { FileStorageService } from './file-storage.service'; + +/** + * Nightly orphan-file sweep (issue #194, ADR 0011) with two directions: + * + * 1. UNCLAIMED ROWS: an attachment whose `pageId` is still null after the + * grace period was claimed by nothing — not by a collab persist (which + * claims every embedded image, apps/collab persistence), not by a page + * upload (#61), not by an import (`linkAttachmentsToPage`). The pond + * file manager flags exactly these as orphans; the sweep reclaims them + * (row + file + quota). The grace period protects paste-then-insert: + * an upload is unclaimed until the ~2 s-debounced persist runs. + * + * 2. STRAY FILES: bytes on the uploads volume without a database row + * (volume↔DB drift, e.g. a crash between file write and row insert). + * Removed once older than the grace period; no quota to correct — the + * reservation was rolled back with the failed upload. + * + * Deliberately NOT swept: claimed attachments whose page content no longer + * references them. The page attachments panel lists claimed files as + * user-managed objects — inserting into the document is optional there — + * so "not embedded" is not "unused"; auto-deleting would destroy panel + * assets. Humans clean those up in the panel or the pond file manager. + */ +const GRACE_MS = 24 * 60 * 60 * 1000; + +@Injectable() +export class OrphanSweepService { + constructor( + private readonly prisma: PrismaService, + private readonly storage: FileStorageService, + private readonly quotas: QuotaService, + private readonly clock: ClockService, + private readonly logger: PinoLogger, + ) { + this.logger.setContext(OrphanSweepService.name); + } + + async sweep(): Promise<{ reclaimed: number; strays: number }> { + const cutoff = this.clock.now().getTime() - GRACE_MS; + + const unclaimed = await this.prisma.attachment.findMany({ + where: { pageId: null, createdAt: { lte: new Date(cutoff) } }, + }); + for (const attachment of unclaimed) { + // File first (idempotent), then row + quota — a crash in between + // leaves a row the next sweep finishes, never untracked bytes. + await this.storage.delete(attachment.pondId, attachment.id); + await this.prisma.attachment.deleteMany({ where: { id: attachment.id } }); + await this.quotas.release(attachment.pondId, attachment.sizeBytes); + this.logger.info( + { attachmentId: attachment.id, pondId: attachment.pondId }, + 'audit: orphaned attachment reclaimed', + ); + } + + let strays = 0; + const stored = await this.storage.listStored(); + const ids = new Set( + (await this.prisma.attachment.findMany({ select: { id: true } })).map((a) => a.id), + ); + for (const file of stored) { + if (ids.has(file.fileId) || file.mtimeMs > cutoff) continue; + await this.storage.delete(file.pondId, file.fileId); + strays += 1; + this.logger.info( + { fileId: file.fileId, pondId: file.pondId }, + 'audit: stray file without database row removed', + ); + } + + return { reclaimed: unclaimed.length, strays }; + } +} diff --git a/apps/api/src/import-export/export.service.ts b/apps/api/src/import-export/export.service.ts index d18d375..eb52662 100644 --- a/apps/api/src/import-export/export.service.ts +++ b/apps/api/src/import-export/export.service.ts @@ -117,7 +117,7 @@ export class ExportService { const attachmentRows = referenced.size > 0 ? await this.prisma.attachment.findMany({ - where: { id: { in: [...referenced] }, pondId, deletedAt: null }, + where: { id: { in: [...referenced] }, pondId }, select: { id: true, mimeType: true }, }) : []; @@ -293,7 +293,7 @@ export class ExportService { const dataUriById = new Map(); if (ids.length === 0) return dataUriById; const attachments = await this.prisma.attachment.findMany({ - where: { id: { in: ids }, pondId, deletedAt: null }, + where: { id: { in: ids }, pondId }, select: { id: true, mimeType: true }, }); for (const attachment of attachments) { diff --git a/docs/architecture/operations.md b/docs/architecture/operations.md index 807c2c3..5541b7f 100644 --- a/docs/architecture/operations.md +++ b/docs/architecture/operations.md @@ -110,6 +110,18 @@ monitoring, structured logs, backup alerting — no dedicated metrics stack. Job outcomes are visible in the Site Admin UI (last run, status) — that panel is the operator's single glance for instance health. +Orphan file sweep (issue #194): nightly, two directions. Attachments +still unclaimed (`pageId` null) after a 24 h grace period — claimed by +no collab persist, page upload, or import — are reclaimed (row, file, +quota); the grace period protects the paste-then-insert window. Files on +the uploads volume without a database row (drift after a crashed upload) +are removed once older than the grace period. Claimed attachments whose +page content no longer embeds them are deliberately NOT auto-deleted: +the page attachments panel lists them as user-managed objects (insert is +optional there), so cleanup of those is a human decision in the panel or +the pond file manager. Attachment deletion is hard everywhere — the +unused `deleted_at` column was removed with #194. + Pond purge (issue #193): a trashed pond past the trash retention is removed with everything it holds — pages (cascading versions, comments, content cache incl. the search vector, update log, mentions, label diff --git a/docs/vs-nfd/20-massnahmenplan.md b/docs/vs-nfd/20-massnahmenplan.md index 37306db..d9f8179 100644 --- a/docs/vs-nfd/20-massnahmenplan.md +++ b/docs/vs-nfd/20-massnahmenplan.md @@ -100,7 +100,7 @@ chain`_ - [x] **Backup-Ziele einschränkbar** — Allowlist, WebDAV/rsync per Deploy vollständig deaktivierbar · 2 AT · #192 - [x] **Pond-Purge implementieren** — getrashte Ponds bleiben ewig liegen · 3 AT · #193 -- [ ] **Orphan-File-Sweep** implementieren, `Attachment.deletedAt` nutzen +- [x] **Orphan-File-Sweep** implementieren, `Attachment.deletedAt` nutzen oder entfernen · 2 AT · #194 - [ ] **Papierkorb aus dem Suchindex** entfernen statt query-seitig filtern · 2 AT · #195 - [ ] **Retention-Job für `audit_log`** · 1 AT · #196