k1nq/TREK
1
0
Fork 0
mirror of https://github.com/mauriceboe/TREK.git synced 2026-10-19 13:36:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(collab): note-file deletion through storage

Browse Source
This commit is contained in:
jubnl
2026-08-17 05:51:44 +02:00
parent dfb8f8a8a4
commit f382a7635e
9 changed files with 36 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/nest/collab/collab.controller.ts
+4 -4
View File
@@ -135,8 +135,8 @@ export class CollabController {
@UseGuards(TripAccessGuard)
@RequirePermission('collab_edit')
@Delete('notes/:id')
deleteNote(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Headers('x-socket-id') socketId?: string) {
if (!this.collab.deleteNote(tripId, id)) {
async deleteNote(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Headers('x-socket-id') socketId?: string) {
if (!(await this.collab.deleteNote(tripId, id))) {
throw new HttpException({ error: 'Note not found' }, 404);
}
this.collab.broadcast(tripId, 'collab:note:deleted', { noteId: Number(id) }, socketId);
@@ -167,8 +167,8 @@ export class CollabController {
@UseGuards(TripAccessGuard)
@RequirePermission('collab_edit')
@Delete('notes/:id/files/:fileId')
deleteNoteFile(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Param('fileId') fileId: string, @Headers('x-socket-id') socketId?: string) {
if (!this.collab.deleteNoteFile(tripId, id, fileId)) {
async deleteNoteFile(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Param('fileId') fileId: string, @Headers('x-socket-id') socketId?: string) {
if (!(await this.collab.deleteNoteFile(tripId, id, fileId))) {
throw new HttpException({ error: 'File not found' }, 404);
}
this.collab.broadcast(tripId, 'collab:note:updated', { note: this.collab.getFormattedNoteById(tripId, id) }, socketId);
server/src/nest/collab/collab.mcp.ts
+1 -1
View File
@@ -145,7 +145,7 @@ export class CollabMcp {
if (this.auth.isDemoUser(ctx.userId)) return demoDenied();
if (!this.collab.verifyTripAccess(tripId, ctx.userId)) return noAccess();
if (!this.guards.hasTripPermission('collab_edit', tripId, ctx.userId)) return permissionDenied();
const deleted = this.collab.deleteNote(tripId, noteId);
const deleted = await this.collab.deleteNote(tripId, noteId);
if (!deleted) return errorResult('Note not found.');
this.guards.safeBroadcast(tripId, 'collab:note:deleted', { noteId });
return ok({ success: true });
server/src/nest/collab/collab.service.ts
+10 -12
View File
@@ -1,15 +1,14 @@
import path from 'path';
import fs from 'fs';
import { Injectable } from '@nestjs/common';
import { DatabaseService, type TripAccess } from '../database/database.service';
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';
import { StorageService } from '../storage/storage.service';
type Trip = TripAccess;
@@ -70,6 +69,7 @@ export class CollabService {
private readonly permissions: PermissionsService,
private readonly realtime: RealtimeService,
private readonly notifications: NotificationsService,
private readonly storage: StorageService,
) {}
verifyTripAccess(tripId: string | number, userId: number) {
@@ -194,18 +194,17 @@ export class CollabService {
return this.formatNote(note);
}
deleteNote(tripId: string | number, noteId: string | number): boolean {
async deleteNote(tripId: string | number, noteId: string | number): Promise<boolean> {
const existing = this.db.get('SELECT id FROM collab_notes WHERE id = ? AND trip_id = ?', noteId, tripId);
if (!existing) return false;
// 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.
// Clean up attached objects first (delete-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.
const noteFiles = this.db.all<NoteFileRow>('SELECT id, filename FROM trip_files WHERE note_id = ?', noteId);
for (const f of noteFiles) {
const filePath = path.join(filesDir, path.basename(f.filename));
try { fs.unlinkSync(filePath); } catch { /* ignore */ }
await this.storage.delete('files', path.basename(f.filename)).catch(() => { /* ignore */ });
}
this.db.transaction(() => {
this.db.run('DELETE FROM trip_files WHERE note_id = ?', noteId);
@@ -237,7 +236,7 @@ export class CollabService {
return this.formatNote(note);
}
deleteNoteFile(tripId: string | number, noteId: string | number, fileId: string | number): boolean {
async deleteNoteFile(tripId: string | number, noteId: string | number, fileId: string | number): Promise<boolean> {
// Scope to the trip — like every sibling collab op — so a caller authorized for THEIR
// trip can't delete a note-file that belongs to someone else's trip (IDOR). trip_files
// carries trip_id, so this ties the deleted object to the URL's :tripId the controller
@@ -245,8 +244,7 @@ export class CollabService {
const file = this.db.get<TripFile>('SELECT * FROM trip_files WHERE id = ? AND note_id = ? AND trip_id = ?', fileId, noteId, tripId);
if (!file) return false;
const filePath = path.join(filesDir, path.basename(file.filename));
try { fs.unlinkSync(filePath); } catch { /* ignore */ }
await this.storage.delete('files', path.basename(file.filename)).catch(() => { /* ignore */ });
this.db.run('DELETE FROM trip_files WHERE id = ?', fileId);
return true;
server/tests/helpers/mcp-test-controllers.ts
+2 -2
View File
@@ -86,6 +86,7 @@ import { makeStorageFixture } from './storage-fixture';
*/
export function createMcpTestRegistry(): McpRegistry {
const dbService = new DatabaseService(db);
const generalStorage = makeStorageFixture('').storage;
const permissionsService = new PermissionsService(dbService);
// Same argument list as auth.bridge.ts. AtlasService used to sit in third
// place; when getTravelStats moved onto AtlasService itself the edge was
@@ -110,11 +111,10 @@ export function createMcpTestRegistry(): McpRegistry {
const daysService = new DaysService(dbService, permissionsService, realtimeService, queryHelpersService);
const todoService = new TodoService(dbService, permissionsService, realtimeService);
const packingService = new PackingService(dbService, permissionsService, realtimeService, notificationsStub());
const collabService = new CollabService(dbService, permissionsService, realtimeService, notificationsStub());
const collabService = new CollabService(dbService, permissionsService, realtimeService, notificationsStub(), generalStorage);
// Exactly one instance, shared by maps, places and share: its stampede guard
// and its on-disk set only work if all three readers see the same maps.
const placePhotoCache = new PlacePhotoCacheService(dbService, makeStorageFixture('photos/google/').storage);
const generalStorage = makeStorageFixture('').storage;
const mapsService = new MapsService(dbService, placePhotoCache);
const journeyDomain = new JourneyDomainService(dbService, realtimeService, new TrekPhotosRepository(dbService));
// The last three were previously omitted, which left them `undefined` at
server/tests/helpers/plugin-host.ts
+2 -2
View File
@@ -83,6 +83,7 @@ import { makeStorageFixture } from './storage-fixture';
* container would discover, handed to the host factory as a registry.
*/
export function createPluginRpcHostFactory(dbs: DatabaseService): PluginRpcHostFactory {
const generalStorage = makeStorageFixture('').storage;
const permissions = new PermissionsService(dbs);
const exchangeRates = new ExchangeRatesService();
const realtime = new RealtimeService();
@@ -93,11 +94,10 @@ export function createPluginRpcHostFactory(dbs: DatabaseService): PluginRpcHostF
const packing = new PackingService(dbs, permissions, realtime, notificationsStub());
const files = new FilesService(dbs, permissions, realtime, new EphemeralTokenService());
const reservations = new ReservationsService(dbs, permissions, budget, realtime, notificationsStub(), new ReservationsReadRepository(dbs));
const collab = new CollabService(dbs, permissions, realtime, notificationsStub());
const collab = new CollabService(dbs, permissions, realtime, notificationsStub(), generalStorage);
const vacay = new VacayService(dbs, realtime, notificationsStub());
const days = new DaysService(dbs, permissions, realtime, queryHelpers);
const photoCache = new PlacePhotoCacheService(dbs, makeStorageFixture('photos/google/').storage);
const generalStorage = makeStorageFixture('').storage;
const unsplash = new UnsplashService(dbs, new RuntimeEnvService(), generalStorage);
const journey = new JourneyDomainService(dbs, realtime, new TrekPhotosRepository(dbs));
const places = new PlacesService(dbs, permissions, realtime, new MapsService(dbs, photoCache), queryHelpers, unsplash, photoCache, journey, generalStorage);
server/tests/unit/nest/collab.controller.test.ts
+8 -8
View File
@@ -67,11 +67,11 @@ describe('CollabController (parity with the legacy /api/trips/:tripId/collab rou
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<CollabService>), storageStub).deleteNote(user, '5', '9'))).toEqual({ status: 404, body: { error: 'Note not found' } });
it('DELETE 404 when missing, else success + broadcasts', async () => {
expect(await thrownAsync(() => new CollabController(svc({ deleteNote: vi.fn().mockResolvedValue(false) } as Partial<CollabService>), 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<CollabService>);
expect(new CollabController(s, storageStub).deleteNote(user, '5', '9', 'sock')).toEqual({ success: true });
const s = svc({ deleteNote: vi.fn().mockResolvedValue(true), broadcast } as Partial<CollabService>);
expect(await new CollabController(s, storageStub).deleteNote(user, '5', '9', 'sock')).toEqual({ success: true });
expect(broadcast).toHaveBeenCalledWith('5', 'collab:note:deleted', { noteId: 9 }, 'sock');
});
});
@@ -89,10 +89,10 @@ describe('CollabController (parity with the legacy /api/trips/:tripId/collab rou
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<CollabService>), 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<CollabService>);
expect(new CollabController(s, storageStub).deleteNoteFile(user, '5', '9', '3')).toEqual({ success: true });
it('DELETE file 404 when missing, else success', async () => {
expect(await thrownAsync(() => new CollabController(svc({ deleteNoteFile: vi.fn().mockResolvedValue(false) } as Partial<CollabService>), storageStub).deleteNoteFile(user, '5', '9', '3'))).toEqual({ status: 404, body: { error: 'File not found' } });
const s = svc({ deleteNoteFile: vi.fn().mockResolvedValue(true), getFormattedNoteById: vi.fn().mockReturnValue({ id: 9 }), broadcast: vi.fn() } as Partial<CollabService>);
expect(await new CollabController(s, storageStub).deleteNoteFile(user, '5', '9', '3')).toEqual({ success: true });
});
});
server/tests/unit/nest/collab.service.test.ts
+7 -5
View File
@@ -67,8 +67,10 @@ import { PermissionsService } from '../../../src/nest/permissions/permissions.se
import { CollabService } from '../../../src/nest/collab/collab.service';
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
import { notificationsStub } from '../../helpers/notifications';
import { makeStorageFixture } from '../../helpers/storage-fixture';
const svc = new CollabService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService(), notificationsStub());
const collabFx = makeStorageFixture('files/');
const svc = new CollabService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService(), notificationsStub(), collabFx.storage);
beforeAll(() => {
createTables(testDb);
@@ -421,7 +423,7 @@ describe('hardening', () => {
it('COLLAB-SVC-034: votePoll switch is atomic — prior vote survives a failed INSERT', () => {
const { user1, trip } = setup();
const dbs = new DatabaseService(testDb);
const failing = new CollabService(dbs, new PermissionsService(dbs), new RealtimeService(), notificationsStub());
const failing = new CollabService(dbs, new PermissionsService(dbs), new RealtimeService(), notificationsStub(), collabFx.storage);
const poll = failing.createPoll(trip.id, user1.id, { question: 'Q?', options: ['A', 'B'] });
failing.votePoll(trip.id, poll!.id, user1.id, 0);
@@ -439,10 +441,10 @@ describe('hardening', () => {
expect(votes).toEqual([{ option_index: 0 }]);
});
it('COLLAB-SVC-035: deleteNote is atomic — trip_files rows survive a failed note DELETE', () => {
it('COLLAB-SVC-035: deleteNote is atomic — trip_files rows survive a failed note DELETE', async () => {
const { user1, trip } = setup();
const dbs = new DatabaseService(testDb);
const failing = new CollabService(dbs, new PermissionsService(dbs), new RealtimeService(), notificationsStub());
const failing = new CollabService(dbs, new PermissionsService(dbs), new RealtimeService(), notificationsStub(), collabFx.storage);
const note = failing.createNote(trip.id, user1.id, { title: 'With file' });
testDb.prepare('INSERT INTO trip_files (trip_id, note_id, filename, original_name) VALUES (?, ?, ?, ?)')
.run(trip.id, note.id, 'files/a.pdf', 'a.pdf');
@@ -452,7 +454,7 @@ describe('hardening', () => {
if (sql.includes('DELETE FROM collab_notes')) throw new Error('boom');
return realRun(sql, ...params);
});
expect(() => failing.deleteNote(trip.id, note.id)).toThrow('boom');
await expect(failing.deleteNote(trip.id, note.id)).rejects.toThrow('boom');
spy.mockRestore();
expect(testDb.prepare('SELECT COUNT(*) as c FROM trip_files WHERE note_id = ?').get(note.id)).toEqual({ c: 1 });
server/tests/unit/nest/trip-read-model.service.test.ts
+1 -1
View File
@@ -100,7 +100,7 @@ const buildReadModel = (database: DatabaseService, roster: TripMembersService =
database, roster, daysSvc, accommodationsSvc, budgetSvc,
new PackingService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub()),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs())),
new CollabService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub()),
new CollabService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub(), makeStorageFixture('').storage),
placesSvc,
new TodoService(dbs(), new PermissionsService(dbs()), new RealtimeService()),
new FilesService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new EphemeralTokenService()),
server/tests/unit/nest/trips.service.test.ts
+1 -1
View File
@@ -130,7 +130,7 @@ const readModelSvc = new TripReadModelService(
dbs(), membersSvc, daysSvc, accommodationsSvc, budgetSvc,
new PackingService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub()),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs())),
new CollabService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub()),
new CollabService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub(), coversFx.storage),
placesSvc,
new TodoService(dbs(), new PermissionsService(dbs()), new RealtimeService()),
new FilesService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new EphemeralTokenService()),