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

refactor(files): trash deletion through storage

Browse Source
This commit is contained in:
jubnl
2026-08-17 06:01:28 +02:00
parent f382a7635e
commit f8faef77c4
6 changed files with 31 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/nest/files/files.service.ts
+14 -10
View File
@@ -1,6 +1,5 @@
import { Injectable } from '@nestjs/common';
import path from 'path';
import fs from 'fs';
import type { Request } from 'express';
import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared';
import { RealtimeService } from '../realtime/realtime.service';
@@ -11,6 +10,7 @@ import { verifyJwtAndLoadUser } from '../auth/jwt-verify';
import type { User, TripFile } from '../../types';
import { DatabaseService, type TripAccess } from '../database/database.service';
import { DEFAULT_ALLOWED_EXTENSIONS, filesDir } from './files.constants';
import { StorageService } from '../storage/storage.service';
type Trip = TripAccess;
type FilePermission = 'file_upload' | 'file_edit' | 'file_delete';
@@ -60,6 +60,7 @@ export class FilesService {
private readonly permissions: PermissionsService,
private readonly realtime: RealtimeService,
private readonly tokens: EphemeralTokenService,
private readonly storage: StorageService,
) {}
verifyTripAccess(tripId: string | number, userId: number) {
@@ -105,6 +106,12 @@ export class FilesService {
return { error: 'Authentication required', status: 401 };
}
/**
* Absolute-path resolution for the one remaining raw-fs consumer, the plugin
* RPC read (files.rpc.ts). Deferred beyond slice 4 by user decision
* 2026-08-16; everything else in this domain addresses storage as
* ('files', <bare name>).
*/
resolveFilePath(filename: string): { resolved: string; safe: boolean } {
const safeName = path.basename(filename);
const filePath = path.join(filesDir, safeName);
@@ -244,14 +251,12 @@ export class FilesService {
}
async permanentDeleteFile(file: TripFile): Promise<void> {
const { resolved } = this.resolveFilePath(file.filename);
// `force: true` swallows ENOENT, replacing the prior existsSync+unlink
// double-call that blocked the event loop twice per deletion. Only
// drop the DB row when the on-disk unlink either succeeded or the
// file was already gone — otherwise a permission / ENOSPC failure
// would orphan the bytes on disk with no DB pointer left to clean it.
// storage.delete is idempotent on a missing object (the old rm force:true
// contract). Only drop the DB row when the delete either succeeded or the
// object was already gone — otherwise a permission / ENOSPC failure
// would orphan the bytes with no DB pointer left to clean them.
try {
await fs.promises.rm(resolved, { force: true });
await this.storage.delete('files', path.basename(file.filename));
} catch (e) {
console.error(`[files] unlink failed for ${file.filename}, keeping DB row:`, e);
throw e;
@@ -266,9 +271,8 @@ export class FilesService {
// and a retry via the single-file delete path can try again.
const successfullyUnlinked: number[] = [];
await Promise.all(trashed.map(async (file) => {
const { resolved } = this.resolveFilePath(file.filename);
try {
await fs.promises.rm(resolved, { force: true });
await this.storage.delete('files', path.basename(file.filename));
successfullyUnlinked.push(Number(file.id));
} catch (e) {
console.error(`[files] unlink failed for ${file.filename}, keeping DB row:`, e);
server/tests/helpers/mcp-test-controllers.ts
+1 -1
View File
@@ -145,7 +145,7 @@ export function createMcpTestRegistry(): McpRegistry {
const readModelService = new TripReadModelService(
dbService, membersService, daysService, accommodationsService, budgetService,
packingService, reservationsService, collabService, placesService, todoService,
new FilesService(dbService, permissionsService, realtimeService, new EphemeralTokenService()),
new FilesService(dbService, permissionsService, realtimeService, new EphemeralTokenService(), generalStorage),
);
const calendarService = new CalendarService(dbService, reservationsService);
// The nine addon-gated surfaces read their toggle off an injected service now
server/tests/helpers/plugin-host.ts
+1 -1
View File
@@ -92,7 +92,7 @@ export function createPluginRpcHostFactory(dbs: DatabaseService): PluginRpcHostF
const queryHelpers = new QueryHelpersService(dbs);
const todos = new TodoService(dbs, permissions, realtime);
const packing = new PackingService(dbs, permissions, realtime, notificationsStub());
const files = new FilesService(dbs, permissions, realtime, new EphemeralTokenService());
const files = new FilesService(dbs, permissions, realtime, new EphemeralTokenService(), generalStorage);
const reservations = new ReservationsService(dbs, permissions, budget, realtime, notificationsStub(), new ReservationsReadRepository(dbs));
const collab = new CollabService(dbs, permissions, realtime, notificationsStub(), generalStorage);
const vacay = new VacayService(dbs, realtime, notificationsStub());
server/tests/unit/nest/files.service.test.ts
+13 -13
View File
@@ -8,7 +8,6 @@
*/
import { describe, it, expect, vi, beforeAll, beforeEach, afterAll, afterEach } from 'vitest';
import path from 'path';
import fs from 'fs';
// ── DB setup ──────────────────────────────────────────────────────────────────
@@ -76,7 +75,9 @@ import type { TripFile, User } from '../../../src/types';
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service';
const svc = new FilesService(new DatabaseService(testDb), permissionsStub, new RealtimeService(), new EphemeralTokenService());
const storageDelete = vi.fn();
const storageStub = { delete: storageDelete } as unknown as import('../../../src/nest/storage/storage.service').StorageService;
const svc = new FilesService(new DatabaseService(testDb), permissionsStub, new RealtimeService(), new EphemeralTokenService(), storageStub);
beforeAll(() => {
createTables(testDb);
@@ -347,20 +348,20 @@ describe('toggleStarred / softDeleteFile / restoreFile', () => {
// ── permanentDeleteFile / emptyTrash (unlink-first semantics) ─────────────────
describe('permanentDeleteFile', () => {
it('FILE-SVC-022: removes the resolved path with force:true, then the DB row', async () => {
it('FILE-SVC-022: deletes the storage object (idempotent on missing), then the DB row', async () => {
const { user, trip } = seedTrip();
const file = makeFile(trip.id, user.id, { filename: 'on-disk.pdf' });
const rm = vi.spyOn(fs.promises, 'rm').mockResolvedValue(undefined);
storageDelete.mockResolvedValue(undefined);
await svc.permanentDeleteFile(svc.getFileById(file.id, trip.id) as TripFile);
expect(rm).toHaveBeenCalledWith(svc.resolveFilePath('on-disk.pdf').resolved, { force: true });
expect(storageDelete).toHaveBeenCalledWith('files', 'on-disk.pdf');
expect(svc.getFileById(file.id, trip.id)).toBeUndefined();
});
it('FILE-SVC-023: an unlink failure logs [files], rethrows and keeps the DB row', async () => {
it('FILE-SVC-023: a delete failure logs [files], rethrows and keeps the DB row', async () => {
const { user, trip } = seedTrip();
const file = makeFile(trip.id, user.id, { filename: 'stuck.pdf' });
const boom = new Error('EACCES');
vi.spyOn(fs.promises, 'rm').mockRejectedValue(boom);
storageDelete.mockRejectedValue(boom);
const err = vi.spyOn(console, 'error').mockImplementation(() => {});
await expect(svc.permanentDeleteFile(svc.getFileById(file.id, trip.id) as TripFile)).rejects.toThrow('EACCES');
expect(err).toHaveBeenCalledWith('[files] unlink failed for stuck.pdf, keeping DB row:', boom);
@@ -376,7 +377,7 @@ describe('emptyTrash', () => {
const kept = makeFile(trip.id, user.id, { filename: 'kept.pdf' });
svc.softDeleteFile(a.id);
svc.softDeleteFile(b.id);
vi.spyOn(fs.promises, 'rm').mockResolvedValue(undefined);
storageDelete.mockResolvedValue(undefined);
await expect(svc.emptyTrash(trip.id)).resolves.toBe(2);
expect(svc.listFiles(trip.id, true)).toEqual([]);
@@ -390,8 +391,8 @@ describe('emptyTrash', () => {
svc.softDeleteFile(good.id);
svc.softDeleteFile(bad.id);
const boom = new Error('EBUSY');
vi.spyOn(fs.promises, 'rm').mockImplementation((p) =>
String(p).includes('bad.pdf') ? Promise.reject(boom) : Promise.resolve()
storageDelete.mockImplementation((_category: string, name: string) =>
name.includes('bad.pdf') ? Promise.reject(boom) : Promise.resolve()
);
const err = vi.spyOn(console, 'error').mockImplementation(() => {});
@@ -401,11 +402,10 @@ describe('emptyTrash', () => {
expect(svc.getDeletedFile(bad.id, trip.id)).toBeDefined();
});
it('FILE-SVC-026: an empty trash resolves to 0 without touching the disk', async () => {
it('FILE-SVC-026: an empty trash resolves to 0 without touching storage', async () => {
const { trip } = seedTrip();
const rm = vi.spyOn(fs.promises, 'rm');
await expect(svc.emptyTrash(trip.id)).resolves.toBe(0);
expect(rm).not.toHaveBeenCalled();
expect(storageDelete).not.toHaveBeenCalled();
});
});
server/tests/unit/nest/trip-read-model.service.test.ts
+1 -1
View File
@@ -103,7 +103,7 @@ const buildReadModel = (database: DatabaseService, roster: TripMembersService =
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()),
new FilesService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new EphemeralTokenService(), makeStorageFixture('').storage),
);
const svc = buildReadModel(dbs());
server/tests/unit/nest/trips.service.test.ts
+1 -1
View File
@@ -133,7 +133,7 @@ const readModelSvc = new TripReadModelService(
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()),
new FilesService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new EphemeralTokenService(), coversFx.storage),
);