From fd81ee00f104f97f9ca116ef673ea6357f4d14fb Mon Sep 17 00:00:00 2001 From: jubnl Date: Sun, 16 Aug 2026 09:29:46 +0200 Subject: [PATCH] refactor(collab): spool note attachments through storage; normalize files/ prefix --- server/src/db/migrations.ts | 10 +++ server/src/nest/collab/collab.controller.ts | 47 ++++++------ server/src/nest/collab/collab.module.ts | 21 +++++- server/src/nest/collab/collab.service.ts | 9 ++- server/tests/integration/collab.test.ts | 75 +++++++++++++++++++ .../db/trip-files-prefix-migration.test.ts | 43 +++++++++++ .../tests/unit/nest/collab.controller.test.ts | 72 +++++++++--------- 7 files changed, 214 insertions(+), 63 deletions(-) create mode 100644 server/tests/unit/db/trip-files-prefix-migration.test.ts diff --git a/server/src/db/migrations.ts b/server/src/db/migrations.ts index dfa0b72c9..df95ecacb 100644 --- a/server/src/db/migrations.ts +++ b/server/src/db/migrations.ts @@ -3891,6 +3891,16 @@ function runMigrations(db: Database.Database): void { db.exec('ALTER TABLE budget_items ADD COLUMN place_id INTEGER REFERENCES places(id) ON DELETE SET NULL DEFAULT NULL'); } }, + // Storage slice 2 — collab note attachments historically stored 'files/' + // in trip_files.filename while the file manager stored bare names in the same + // column; the storage layer addresses objects as category + bare name, so + // normalize the legacy rows. substr is 1-indexed: 7 drops the six chars of + // 'files/'. The LIKE guard makes it a no-op on already-bare rows. + // Appended LAST: the array is index-addressed against schema_version, so a + // slot inserted above this line never runs on an existing database. + () => { + db.exec("UPDATE trip_files SET filename = substr(filename, 7) WHERE filename LIKE 'files/%'"); + }, ]; if (currentVersion < migrations.length) { diff --git a/server/src/nest/collab/collab.controller.ts b/server/src/nest/collab/collab.controller.ts index 526cab011..04c7ffe4f 100644 --- a/server/src/nest/collab/collab.controller.ts +++ b/server/src/nest/collab/collab.controller.ts @@ -15,12 +15,11 @@ import { UseInterceptors, } from '@nestjs/common'; import { FileInterceptor } from '@nestjs/platform-express'; -import { diskStorage } from 'multer'; +import type { Options } from 'multer'; import path from 'path'; -import fs from 'fs'; -import { v4 as uuidv4 } from 'uuid'; import type { User } from '../../types'; import { CollabService } from './collab.service'; +import { StorageService } from '../storage/storage.service'; import { CollabNoteCreateDto, CollabNoteUpdateDto, @@ -34,24 +33,18 @@ import { CurrentUser } from '../auth/current-user.decorator'; import { RequirePermission, TripAccessGuard } from '../permissions/trip-access.guard'; import { BLOCKED_EXTENSIONS } from '../files/files.constants'; -const MAX_NOTE_FILE_SIZE = 50 * 1024 * 1024; -const filesDir = path.join(__dirname, '../../../uploads/files'); -const NOTE_UPLOAD = { - storage: diskStorage({ - destination: (_req, _file, cb) => { if (!fs.existsSync(filesDir)) fs.mkdirSync(filesDir, { recursive: true }); cb(null, filesDir); }, - filename: (_req, file, cb) => cb(null, `${uuidv4()}${path.extname(file.originalname)}`), - }), - limits: { fileSize: MAX_NOTE_FILE_SIZE }, - defParamCharset: 'utf8', // parity with legacy routes/collab.ts — preserve non-ASCII original filenames - fileFilter: (_req: unknown, file: Express.Multer.File, cb: (err: Error | null, accept: boolean) => void) => { - const ext = path.extname(file.originalname).toLowerCase(); - if (BLOCKED_EXTENSIONS.includes(ext) || file.mimetype.includes('svg') || file.mimetype.includes('html') || file.mimetype.includes('javascript')) { - const err: Error & { statusCode?: number } = new Error('File type not allowed'); - err.statusCode = 400; - return cb(err, false); - } - cb(null, true); - }, +export const MAX_NOTE_FILE_SIZE = 50 * 1024 * 1024; +// Consumed by collab.module.ts's MulterModule factory; the rest of the multer +// options (spool destination, filename, limits) come from the storage upload +// factory. +export const collabNoteFileFilter: Options['fileFilter'] = (_req, file, cb) => { + const ext = path.extname(file.originalname).toLowerCase(); + if (BLOCKED_EXTENSIONS.includes(ext) || file.mimetype.includes('svg') || file.mimetype.includes('html') || file.mimetype.includes('javascript')) { + const err: Error & { statusCode?: number } = new Error('File type not allowed'); + err.statusCode = 400; + return cb(err); + } + cb(null, true); }; /** @@ -78,7 +71,10 @@ const NOTE_UPLOAD = { // caller then sees ECONNRESET instead of the 404). Same as files.controller.ts. @UseGuards(JwtAuthGuard) export class CollabController { - constructor(private readonly collab: CollabService) {} + constructor( + private readonly collab: CollabService, + private readonly storage: StorageService, + ) {} private requireTrip(tripId: string, user: User) { const trip = this.collab.verifyTripAccess(tripId, user.id); @@ -148,8 +144,8 @@ export class CollabController { } @Post('notes/:id/files') - @UseInterceptors(FileInterceptor('file', NOTE_UPLOAD)) - addNoteFile(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @UploadedFile() file: Express.Multer.File | undefined, @Headers('x-socket-id') socketId?: string) { + @UseInterceptors(FileInterceptor('file')) + async addNoteFile(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @UploadedFile() file: Express.Multer.File | undefined, @Headers('x-socket-id') socketId?: string) { const trip = this.requireTrip(tripId, user); if (!this.collab.canUploadFiles(trip, user)) { throw new HttpException({ error: 'No permission to upload files' }, 403); @@ -157,6 +153,9 @@ export class CollabController { if (!file) { throw new HttpException({ error: 'No file uploaded' }, 400); } + // Commit the spooled upload to its final storage location (atomic + // same-volume rename) before the DB row references the final path. + await this.storage.put('files', file.filename, { tmpPath: file.path }); const result = this.collab.addNoteFile(tripId, id, file); if (!result) { throw new HttpException({ error: 'Note not found' }, 404); diff --git a/server/src/nest/collab/collab.module.ts b/server/src/nest/collab/collab.module.ts index a8f3b440d..dcb148297 100644 --- a/server/src/nest/collab/collab.module.ts +++ b/server/src/nest/collab/collab.module.ts @@ -1,5 +1,6 @@ import { Module } from '@nestjs/common'; -import { CollabController } from './collab.controller'; +import { MulterModule } from '@nestjs/platform-express'; +import { CollabController, collabNoteFileFilter, MAX_NOTE_FILE_SIZE } from './collab.controller'; import { CollabService } from './collab.service'; import { CollabRpc } from './collab.rpc'; import { PluginGuardsModule } from '../plugins/host/plugin-guards.module'; @@ -10,9 +11,25 @@ import { AuthModule } from '../auth/auth.module'; import { AddonsModule } from '../addons/addons.module'; import { NotificationsModule } from '../notifications/notifications.module'; import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; +import { StorageModule } from '../storage/storage.module'; +import { StorageService } from '../storage/storage.service'; +import { buildStorageUploadOptions } from '../storage/storage-upload.factory'; @Module({ - imports: [McpSharedModule, NotificationsModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule], + imports: [ + MulterModule.registerAsync({ + imports: [StorageModule], + inject: [StorageService], + useFactory: (storage: StorageService) => + buildStorageUploadOptions(storage, { + category: 'files', + maxSize: MAX_NOTE_FILE_SIZE, + defParamCharset: 'utf8', // parity with legacy routes/collab.ts — preserve non-ASCII original filenames + fileFilter: collabNoteFileFilter, + }), + }), + StorageModule, + McpSharedModule, NotificationsModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule], controllers: [CollabController], providers: [CollabService, CollabMcp, CollabRpc], // For in-container consumers (CollabRpc). diff --git a/server/src/nest/collab/collab.service.ts b/server/src/nest/collab/collab.service.ts index 5b55f9e5b..e6e94e6ac 100644 --- a/server/src/nest/collab/collab.service.ts +++ b/server/src/nest/collab/collab.service.ts @@ -6,6 +6,7 @@ import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; import { RealtimeService } from '../realtime/realtime.service'; import { PermissionsService } from '../permissions/permissions.service'; import { avatarUrl } from '../common/avatarUrl'; +import { filesDir } from '../files/files.constants'; import { checkSsrf, createPinnedDispatcher } from '../../utils/ssrfGuard'; import type { CollabNote, CollabPoll, CollabMessage, TripFile, User } from '../../types'; import { NotificationsService } from '../notifications/notifications.service'; @@ -199,9 +200,11 @@ export class CollabService { // Clean up attached files from disk (unlink-first is intentional — a // failed row delete leaves dangling rows, never orphaned files). + // basename() tolerates any legacy 'files/'-prefixed row the boot migration + // has not seen; the raw unlink moves to storage.delete in the serving slice. const noteFiles = this.db.all('SELECT id, filename FROM trip_files WHERE note_id = ?', noteId); for (const f of noteFiles) { - const filePath = path.join(__dirname, '../../../uploads', f.filename); + const filePath = path.join(filesDir, path.basename(f.filename)); try { fs.unlinkSync(filePath); } catch { /* ignore */ } } this.db.transaction(() => { @@ -221,7 +224,7 @@ export class CollabService { const result = this.db.run( 'INSERT INTO trip_files (trip_id, note_id, filename, original_name, file_size, mime_type) VALUES (?, ?, ?, ?, ?, ?)', - tripId, noteId, `files/${file.filename}`, file.originalname, file.size, file.mimetype + tripId, noteId, file.filename, file.originalname, file.size, file.mimetype ); const saved = this.db.get('SELECT * FROM trip_files WHERE id = ?', result.lastInsertRowid)!; @@ -242,7 +245,7 @@ export class CollabService { const file = this.db.get('SELECT * FROM trip_files WHERE id = ? AND note_id = ? AND trip_id = ?', fileId, noteId, tripId); if (!file) return false; - const filePath = path.join(__dirname, '../../../uploads', file.filename); + const filePath = path.join(filesDir, path.basename(file.filename)); try { fs.unlinkSync(filePath); } catch { /* ignore */ } this.db.run('DELETE FROM trip_files WHERE id = ?', fileId); diff --git a/server/tests/integration/collab.test.ts b/server/tests/integration/collab.test.ts index bf3a9de67..44a72066f 100644 --- a/server/tests/integration/collab.test.ts +++ b/server/tests/integration/collab.test.ts @@ -264,6 +264,81 @@ describe('Collab notes', () => { expect(del.body.success).toBe(true); }); + it('COLLAB-P01 — attachment filename is stored bare (category + name; no files/ prefix)', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + + const create = await request(app) + .post(`/api/trips/${trip.id}/collab/notes`) + .set('Cookie', authCookie(user.id)) + .send({ title: 'Bare name' }); + const noteId = create.body.note.id; + + const upload = await request(app) + .post(`/api/trips/${trip.id}/collab/notes/${noteId}/files`) + .set('Cookie', authCookie(user.id)) + .attach('file', FIXTURE_PDF); + expect(upload.status).toBe(201); + + const list = await request(app) + .get(`/api/trips/${trip.id}/collab/notes`) + .set('Cookie', authCookie(user.id)); + const note = list.body.notes.find((n: any) => n.id === noteId); + expect(note.attachments[0].filename).toMatch(/^[0-9a-f-]{36}\.pdf$/); + expect(note.attachments[0].url).toMatch(/^\/api\/trips\/\d+\/files\/\d+\/download$/); + // The bytes live in uploads/files under the bare name. + expect(fs.existsSync(path.join(uploadsDir, note.attachments[0].filename))).toBe(true); + }); + + it('COLLAB-P02 — deleting a note file removes its bytes from uploads/files', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + + const create = await request(app) + .post(`/api/trips/${trip.id}/collab/notes`) + .set('Cookie', authCookie(user.id)) + .send({ title: 'Delete file bytes' }); + const noteId = create.body.note.id; + + const upload = await request(app) + .post(`/api/trips/${trip.id}/collab/notes/${noteId}/files`) + .set('Cookie', authCookie(user.id)) + .attach('file', FIXTURE_PDF); + const fileId = upload.body.file.id; + const diskName = path.basename(upload.body.file.filename); + expect(fs.existsSync(path.join(uploadsDir, diskName))).toBe(true); + + const del = await request(app) + .delete(`/api/trips/${trip.id}/collab/notes/${noteId}/files/${fileId}`) + .set('Cookie', authCookie(user.id)); + expect(del.status).toBe(200); + expect(fs.existsSync(path.join(uploadsDir, diskName))).toBe(false); + }); + + it('COLLAB-P03 — deleting a note removes its attachments’ bytes from uploads/files', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + + const create = await request(app) + .post(`/api/trips/${trip.id}/collab/notes`) + .set('Cookie', authCookie(user.id)) + .send({ title: 'Delete note bytes' }); + const noteId = create.body.note.id; + + const upload = await request(app) + .post(`/api/trips/${trip.id}/collab/notes/${noteId}/files`) + .set('Cookie', authCookie(user.id)) + .attach('file', FIXTURE_PDF); + const diskName = path.basename(upload.body.file.filename); + expect(fs.existsSync(path.join(uploadsDir, diskName))).toBe(true); + + const del = await request(app) + .delete(`/api/trips/${trip.id}/collab/notes/${noteId}`) + .set('Cookie', authCookie(user.id)); + expect(del.status).toBe(200); + expect(fs.existsSync(path.join(uploadsDir, diskName))).toBe(false); + }); + it('COLLAB-028 — uploaded note file URL uses authenticated download path, not /uploads/', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); diff --git a/server/tests/unit/db/trip-files-prefix-migration.test.ts b/server/tests/unit/db/trip-files-prefix-migration.test.ts new file mode 100644 index 000000000..e47e4317b --- /dev/null +++ b/server/tests/unit/db/trip-files-prefix-migration.test.ts @@ -0,0 +1,43 @@ +/** + * Boot migration: strip the legacy 'files/' prefix from trip_files.filename. + * + * Collab note attachments historically stored 'files/' while the file + * manager stored bare names in the same column; the storage layer addresses + * objects as category + bare name, so existing prefixed rows are normalized + * once at boot (storage slice 2). + */ +import { describe, it, expect } from 'vitest'; +import Database from 'better-sqlite3'; +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; + +function makeDbWithRows(): Database.Database { + const db = new Database(':memory:'); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA busy_timeout = 5000'); + db.exec('PRAGMA foreign_keys = ON'); + createTables(db); + // Minimal FK chain: user → trip → trip_files rows. + db.prepare("INSERT INTO users (id, username, email, password_hash) VALUES (1, 'u', 'u@example.test', 'x')").run(); + db.prepare("INSERT INTO trips (id, user_id, title) VALUES (1, 1, 'T')").run(); + db.prepare( + "INSERT INTO trip_files (trip_id, filename, original_name) VALUES (1, 'files/aaa.pdf', 'a.pdf'), (1, 'bbb.pdf', 'b.pdf')", + ).run(); + return db; +} + +describe('trip_files files/-prefix migration', () => { + it('strips the files/ prefix from legacy collab rows and leaves bare rows alone', () => { + const db = makeDbWithRows(); + try { + runMigrations(db); + const names = db + .prepare('SELECT filename FROM trip_files ORDER BY id') + .all() + .map((r) => (r as { filename: string }).filename); + expect(names).toEqual(['aaa.pdf', 'bbb.pdf']); + } finally { + db.close(); + } + }); +}); diff --git a/server/tests/unit/nest/collab.controller.test.ts b/server/tests/unit/nest/collab.controller.test.ts index 76426a06f..8fe14df45 100644 --- a/server/tests/unit/nest/collab.controller.test.ts +++ b/server/tests/unit/nest/collab.controller.test.ts @@ -3,6 +3,7 @@ import { HttpException } from '@nestjs/common'; import { CollabController } from '../../../src/nest/collab/collab.controller'; import type { CollabService } from '../../../src/nest/collab/collab.service'; +import type { StorageService } from '../../../src/nest/storage/storage.service'; import type { User } from '../../../src/types'; const user = { id: 1, username: 'u', role: 'user', email: 'u@example.test' } as User; @@ -18,6 +19,8 @@ function svc(o: Partial = {}): CollabService { } as unknown as CollabService; } +const storageStub = { put: vi.fn().mockResolvedValue(undefined) } as unknown as StorageService; + function thrown(fn: () => unknown): { status: number; body: unknown } { try { fn(); } catch (err) { expect(err).toBeInstanceOf(HttpException); @@ -42,7 +45,7 @@ describe('CollabController (parity with the legacy /api/trips/:tripId/collab rou describe('notes', () => { it('GET lists', () => { const s = svc({ listNotes: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial); - expect(new CollabController(s).listNotes(user, '5')).toEqual({ notes: [{ id: 1 }] }); + expect(new CollabController(s, storageStub).listNotes(user, '5')).toEqual({ notes: [{ id: 1 }] }); }); it('POST creates + broadcasts + notifies (empty title now 400s in the Zod pipe)', () => { @@ -50,118 +53,119 @@ describe('CollabController (parity with the legacy /api/trips/:tripId/collab rou const broadcast = vi.fn(); const notifyCollab = vi.fn(); const s = svc({ createNote, broadcast, notifyCollab } as Partial); - expect(new CollabController(s).createNote(user, '5', { title: 'T', content: 'c' }, 'sock')).toEqual({ note: { id: 9 } }); + expect(new CollabController(s, storageStub).createNote(user, '5', { title: 'T', content: 'c' }, 'sock')).toEqual({ note: { id: 9 } }); expect(createNote).toHaveBeenCalledWith('5', 1, { title: 'T', content: 'c', category: undefined, color: undefined, website: undefined }); expect(broadcast).toHaveBeenCalledWith('5', 'collab:note:created', { note: { id: 9 } }, 'sock'); expect(notifyCollab).toHaveBeenCalledWith('5', user); }); it('PUT 404 when missing, else updates + broadcasts', () => { - expect(thrown(() => new CollabController(svc({ updateNote: vi.fn().mockReturnValue(null) } as Partial)).updateNote(user, '5', '9', {}))).toEqual({ status: 404, body: { error: 'Note not found' } }); + expect(thrown(() => new CollabController(svc({ updateNote: vi.fn().mockReturnValue(null) } as Partial), storageStub).updateNote(user, '5', '9', {}))).toEqual({ status: 404, body: { error: 'Note not found' } }); const broadcast = vi.fn(); const s = svc({ updateNote: vi.fn().mockReturnValue({ id: 9 }), broadcast } as Partial); - expect(new CollabController(s).updateNote(user, '5', '9', { title: 'b' }, 'sock')).toEqual({ note: { id: 9 } }); + expect(new CollabController(s, storageStub).updateNote(user, '5', '9', { title: 'b' }, 'sock')).toEqual({ note: { id: 9 } }); expect(broadcast).toHaveBeenCalledWith('5', 'collab:note:updated', { note: { id: 9 } }, 'sock'); }); it('DELETE 404 when missing, else success + broadcasts', () => { - expect(thrown(() => new CollabController(svc({ deleteNote: vi.fn().mockReturnValue(false) } as Partial)).deleteNote(user, '5', '9'))).toEqual({ status: 404, body: { error: 'Note not found' } }); + expect(thrown(() => new CollabController(svc({ deleteNote: vi.fn().mockReturnValue(false) } as Partial), storageStub).deleteNote(user, '5', '9'))).toEqual({ status: 404, body: { error: 'Note not found' } }); const broadcast = vi.fn(); const s = svc({ deleteNote: vi.fn().mockReturnValue(true), broadcast } as Partial); - expect(new CollabController(s).deleteNote(user, '5', '9', 'sock')).toEqual({ success: true }); + expect(new CollabController(s, storageStub).deleteNote(user, '5', '9', 'sock')).toEqual({ success: true }); expect(broadcast).toHaveBeenCalledWith('5', 'collab:note:deleted', { noteId: 9 }, 'sock'); }); }); describe('note files', () => { const file = { filename: 'a.pdf' } as Express.Multer.File; - it('403 without file_upload, 400 without file, 404 unknown note, else returns result', () => { - expect(thrown(() => new CollabController(svc({ canUploadFiles: vi.fn().mockReturnValue(false) })).addNoteFile(user, '5', '9', file))).toEqual({ status: 403, body: { error: 'No permission to upload files' } }); - expect(thrown(() => new CollabController(svc()).addNoteFile(user, '5', '9', undefined))).toEqual({ status: 400, body: { error: 'No file uploaded' } }); - expect(thrown(() => new CollabController(svc({ addNoteFile: vi.fn().mockReturnValue(null) } as Partial)).addNoteFile(user, '5', '9', file))).toEqual({ status: 404, body: { error: 'Note not found' } }); + it('403 without file_upload, 400 without file, 404 unknown note, else commits + returns result', async () => { + expect(await thrownAsync(() => new CollabController(svc({ canUploadFiles: vi.fn().mockReturnValue(false) }), storageStub).addNoteFile(user, '5', '9', file))).toEqual({ status: 403, body: { error: 'No permission to upload files' } }); + expect(await thrownAsync(() => new CollabController(svc(), storageStub).addNoteFile(user, '5', '9', undefined))).toEqual({ status: 400, body: { error: 'No file uploaded' } }); + expect(await thrownAsync(() => new CollabController(svc({ addNoteFile: vi.fn().mockReturnValue(null) } as Partial), storageStub).addNoteFile(user, '5', '9', file))).toEqual({ status: 404, body: { error: 'Note not found' } }); const broadcast = vi.fn(); const s = svc({ addNoteFile: vi.fn().mockReturnValue({ file: { id: 3 } }), getFormattedNoteById: vi.fn().mockReturnValue({ id: 9 }), broadcast } as Partial); - expect(new CollabController(s).addNoteFile(user, '5', '9', file, 'sock')).toEqual({ file: { id: 3 } }); + expect(await new CollabController(s, storageStub).addNoteFile(user, '5', '9', file, 'sock')).toEqual({ file: { id: 3 } }); + expect(storageStub.put).toHaveBeenCalledWith('files', 'a.pdf', { tmpPath: undefined }); expect(broadcast).toHaveBeenCalledWith('5', 'collab:note:updated', { note: { id: 9 } }, 'sock'); }); it('DELETE file 404 when missing, else success', () => { - expect(thrown(() => new CollabController(svc({ deleteNoteFile: vi.fn().mockReturnValue(false) } as Partial)).deleteNoteFile(user, '5', '9', '3'))).toEqual({ status: 404, body: { error: 'File not found' } }); + expect(thrown(() => new CollabController(svc({ deleteNoteFile: vi.fn().mockReturnValue(false) } as Partial), storageStub).deleteNoteFile(user, '5', '9', '3'))).toEqual({ status: 404, body: { error: 'File not found' } }); const s = svc({ deleteNoteFile: vi.fn().mockReturnValue(true), getFormattedNoteById: vi.fn().mockReturnValue({ id: 9 }), broadcast: vi.fn() } as Partial); - expect(new CollabController(s).deleteNoteFile(user, '5', '9', '3')).toEqual({ success: true }); + expect(new CollabController(s, storageStub).deleteNoteFile(user, '5', '9', '3')).toEqual({ success: true }); }); }); describe('polls', () => { it('POST creates (missing question / <2 options now 400 in the Zod pipe)', () => { const s = svc({ createPoll: vi.fn().mockReturnValue({ id: 7 }), broadcast: vi.fn() } as Partial); - expect(new CollabController(s).createPoll(user, '5', { question: 'q', options: ['a', 'b'] })).toEqual({ poll: { id: 7 } }); + expect(new CollabController(s, storageStub).createPoll(user, '5', { question: 'q', options: ['a', 'b'] })).toEqual({ poll: { id: 7 } }); }); it('vote maps not_found/closed/invalid_index, else broadcasts the poll', () => { - expect(thrown(() => new CollabController(svc({ votePoll: vi.fn().mockReturnValue({ error: 'not_found' }) } as Partial)).votePoll(user, '5', '7', { option_index: 0 }))).toEqual({ status: 404, body: { error: 'Poll not found' } }); - expect(thrown(() => new CollabController(svc({ votePoll: vi.fn().mockReturnValue({ error: 'closed' }) } as Partial)).votePoll(user, '5', '7', { option_index: 0 }))).toEqual({ status: 400, body: { error: 'Poll is closed' } }); - expect(thrown(() => new CollabController(svc({ votePoll: vi.fn().mockReturnValue({ error: 'invalid_index' }) } as Partial)).votePoll(user, '5', '7', { option_index: 9 }))).toEqual({ status: 400, body: { error: 'Invalid option index' } }); + expect(thrown(() => new CollabController(svc({ votePoll: vi.fn().mockReturnValue({ error: 'not_found' }) } as Partial), storageStub).votePoll(user, '5', '7', { option_index: 0 }))).toEqual({ status: 404, body: { error: 'Poll not found' } }); + expect(thrown(() => new CollabController(svc({ votePoll: vi.fn().mockReturnValue({ error: 'closed' }) } as Partial), storageStub).votePoll(user, '5', '7', { option_index: 0 }))).toEqual({ status: 400, body: { error: 'Poll is closed' } }); + expect(thrown(() => new CollabController(svc({ votePoll: vi.fn().mockReturnValue({ error: 'invalid_index' }) } as Partial), storageStub).votePoll(user, '5', '7', { option_index: 9 }))).toEqual({ status: 400, body: { error: 'Invalid option index' } }); const broadcast = vi.fn(); const s = svc({ votePoll: vi.fn().mockReturnValue({ poll: { id: 7 } }), broadcast } as Partial); - expect(new CollabController(s).votePoll(user, '5', '7', { option_index: 0 }, 'sock')).toEqual({ poll: { id: 7 } }); + expect(new CollabController(s, storageStub).votePoll(user, '5', '7', { option_index: 0 }, 'sock')).toEqual({ poll: { id: 7 } }); expect(broadcast).toHaveBeenCalledWith('5', 'collab:poll:voted', { poll: { id: 7 } }, 'sock'); }); it('close 404 when missing, else broadcasts', () => { - expect(thrown(() => new CollabController(svc({ closePoll: vi.fn().mockReturnValue(null) } as Partial)).closePoll(user, '5', '7'))).toEqual({ status: 404, body: { error: 'Poll not found' } }); + expect(thrown(() => new CollabController(svc({ closePoll: vi.fn().mockReturnValue(null) } as Partial), storageStub).closePoll(user, '5', '7'))).toEqual({ status: 404, body: { error: 'Poll not found' } }); const s = svc({ closePoll: vi.fn().mockReturnValue({ id: 7 }), broadcast: vi.fn() } as Partial); - expect(new CollabController(s).closePoll(user, '5', '7')).toEqual({ poll: { id: 7 } }); + expect(new CollabController(s, storageStub).closePoll(user, '5', '7')).toEqual({ poll: { id: 7 } }); }); it('delete 404 when missing, else success', () => { - expect(thrown(() => new CollabController(svc({ deletePoll: vi.fn().mockReturnValue(false) } as Partial)).deletePoll(user, '5', '7'))).toEqual({ status: 404, body: { error: 'Poll not found' } }); + expect(thrown(() => new CollabController(svc({ deletePoll: vi.fn().mockReturnValue(false) } as Partial), storageStub).deletePoll(user, '5', '7'))).toEqual({ status: 404, body: { error: 'Poll not found' } }); const s = svc({ deletePoll: vi.fn().mockReturnValue(true), broadcast: vi.fn() } as Partial); - expect(new CollabController(s).deletePoll(user, '5', '7')).toEqual({ success: true }); + expect(new CollabController(s, storageStub).deletePoll(user, '5', '7')).toEqual({ success: true }); }); }); describe('messages', () => { it('POST 400 whitespace-only, 400 reply_not_found, else creates + notifies (length checks now in the Zod pipe)', () => { - expect(thrown(() => new CollabController(svc()).createMessage(user, '5', { text: ' ' }))).toEqual({ status: 400, body: { error: 'Message text is required' } }); - expect(thrown(() => new CollabController(svc({ createMessage: vi.fn().mockReturnValue({ error: 'reply_not_found' }) } as Partial)).createMessage(user, '5', { text: 'hi', reply_to: 99 }))).toEqual({ status: 400, body: { error: 'Reply target message not found' } }); + expect(thrown(() => new CollabController(svc(), storageStub).createMessage(user, '5', { text: ' ' }))).toEqual({ status: 400, body: { error: 'Message text is required' } }); + expect(thrown(() => new CollabController(svc({ createMessage: vi.fn().mockReturnValue({ error: 'reply_not_found' }) } as Partial), storageStub).createMessage(user, '5', { text: 'hi', reply_to: 99 }))).toEqual({ status: 400, body: { error: 'Reply target message not found' } }); const broadcast = vi.fn(); const notifyCollab = vi.fn(); const s = svc({ createMessage: vi.fn().mockReturnValue({ message: { id: 3 } }), broadcast, notifyCollab } as Partial); - expect(new CollabController(s).createMessage(user, '5', { text: 'hello' }, 'sock')).toEqual({ message: { id: 3 } }); + expect(new CollabController(s, storageStub).createMessage(user, '5', { text: 'hello' }, 'sock')).toEqual({ message: { id: 3 } }); expect(broadcast).toHaveBeenCalledWith('5', 'collab:message:created', { message: { id: 3 } }, 'sock'); expect(notifyCollab).toHaveBeenCalledWith('5', user, 'hello'); }); it('react 404 unknown, else broadcasts reactions (empty emoji now 400s in the Zod pipe)', () => { - expect(thrown(() => new CollabController(svc({ reactMessage: vi.fn().mockReturnValue({ found: false, reactions: [] }) } as Partial)).react(user, '5', '3', { emoji: '👍' }))).toEqual({ status: 404, body: { error: 'Message not found' } }); + expect(thrown(() => new CollabController(svc({ reactMessage: vi.fn().mockReturnValue({ found: false, reactions: [] }) } as Partial), storageStub).react(user, '5', '3', { emoji: '👍' }))).toEqual({ status: 404, body: { error: 'Message not found' } }); const broadcast = vi.fn(); const s = svc({ reactMessage: vi.fn().mockReturnValue({ found: true, reactions: [{ emoji: '👍', count: 1 }] }), broadcast } as Partial); - expect(new CollabController(s).react(user, '5', '3', { emoji: '👍' }, 'sock')).toEqual({ reactions: [{ emoji: '👍', count: 1 }] }); + expect(new CollabController(s, storageStub).react(user, '5', '3', { emoji: '👍' }, 'sock')).toEqual({ reactions: [{ emoji: '👍', count: 1 }] }); expect(broadcast).toHaveBeenCalledWith('5', 'collab:message:reacted', { messageId: 3, reactions: [{ emoji: '👍', count: 1 }] }, 'sock'); }); it('delete maps not_found (404) / not_owner (403), else success with username', () => { - expect(thrown(() => new CollabController(svc({ deleteMessage: vi.fn().mockReturnValue({ error: 'not_found' }) } as Partial)).deleteMessage(user, '5', '3'))).toEqual({ status: 404, body: { error: 'Message not found' } }); - expect(thrown(() => new CollabController(svc({ deleteMessage: vi.fn().mockReturnValue({ error: 'not_owner' }) } as Partial)).deleteMessage(user, '5', '3'))).toEqual({ status: 403, body: { error: 'You can only delete your own messages' } }); + expect(thrown(() => new CollabController(svc({ deleteMessage: vi.fn().mockReturnValue({ error: 'not_found' }) } as Partial), storageStub).deleteMessage(user, '5', '3'))).toEqual({ status: 404, body: { error: 'Message not found' } }); + expect(thrown(() => new CollabController(svc({ deleteMessage: vi.fn().mockReturnValue({ error: 'not_owner' }) } as Partial), storageStub).deleteMessage(user, '5', '3'))).toEqual({ status: 403, body: { error: 'You can only delete your own messages' } }); const broadcast = vi.fn(); const s = svc({ deleteMessage: vi.fn().mockReturnValue({ username: 'bob' }), broadcast } as Partial); - expect(new CollabController(s).deleteMessage(user, '5', '3', 'sock')).toEqual({ success: true }); + expect(new CollabController(s, storageStub).deleteMessage(user, '5', '3', 'sock')).toEqual({ success: true }); expect(broadcast).toHaveBeenCalledWith('5', 'collab:message:deleted', { messageId: 3, username: 'bob' }, 'sock'); }); }); describe('link preview', () => { it('400 without url, maps an error result to 400, else returns the preview', async () => { - expect(await thrownAsync(() => new CollabController(svc()).linkPreview(user, '5', undefined))).toEqual({ status: 400, body: { error: 'URL is required' } }); - expect(await thrownAsync(() => new CollabController(svc({ linkPreview: vi.fn().mockResolvedValue({ error: 'bad url' }) } as Partial)).linkPreview(user, '5', 'http://x'))).toEqual({ status: 400, body: { error: 'bad url' } }); + expect(await thrownAsync(() => new CollabController(svc(), storageStub).linkPreview(user, '5', undefined))).toEqual({ status: 400, body: { error: 'URL is required' } }); + expect(await thrownAsync(() => new CollabController(svc({ linkPreview: vi.fn().mockResolvedValue({ error: 'bad url' }) } as Partial), storageStub).linkPreview(user, '5', 'http://x'))).toEqual({ status: 400, body: { error: 'bad url' } }); const s = svc({ linkPreview: vi.fn().mockResolvedValue({ title: 'T', description: null, image: null, url: 'http://x' }) } as Partial); - expect(await new CollabController(s).linkPreview(user, '5', 'http://x')).toEqual({ title: 'T', description: null, image: null, url: 'http://x' }); + expect(await new CollabController(s, storageStub).linkPreview(user, '5', 'http://x')).toEqual({ title: 'T', description: null, image: null, url: 'http://x' }); }); it('falls back to a null preview when the service throws', async () => { const s = svc({ linkPreview: vi.fn().mockRejectedValue(new Error('network')) } as Partial); - expect(await new CollabController(s).linkPreview(user, '5', 'http://x')).toEqual({ title: null, description: null, image: null, url: 'http://x' }); + expect(await new CollabController(s, storageStub).linkPreview(user, '5', 'http://x')).toEqual({ title: null, description: null, image: null, url: 'http://x' }); }); }); });