From b1abfd2110fe9d3202a9dc354f1124d3f8fed033 Mon Sep 17 00:00:00 2001 From: jubnl Date: Sun, 16 Aug 2026 09:34:48 +0200 Subject: [PATCH] refactor(journey): spool journey media uploads through storage --- server/src/nest/journey/journey.controller.ts | 175 ++++++++++------ server/src/nest/journey/journey.module.ts | 20 +- server/tests/integration/journey.test.ts | 151 ++++++++++++++ .../unit/nest/journey.controller.test.ts | 194 +++++++++--------- 4 files changed, 376 insertions(+), 164 deletions(-) diff --git a/server/src/nest/journey/journey.controller.ts b/server/src/nest/journey/journey.controller.ts index b14c35169..c1f16ef1b 100644 --- a/server/src/nest/journey/journey.controller.ts +++ b/server/src/nest/journey/journey.controller.ts @@ -16,11 +16,13 @@ import { UseInterceptors, } from '@nestjs/common'; import { FileInterceptor, FilesInterceptor, FileFieldsInterceptor } from '@nestjs/platform-express'; -import { diskStorage } from 'multer'; +import type { Options } from 'multer'; +import type { Request } from 'express'; import path from 'node:path'; import fs from 'node:fs'; import crypto from 'node:crypto'; import type { User } from '../../types'; +import { StorageService } from '../storage/storage.service'; import { JourneyService } from './journey.service'; import { AddonGuard } from '../addons/addon.guard'; import { RequireAddon } from '../addons/require-addon.decorator'; @@ -36,73 +38,73 @@ import { import { isVideoMime, isVideoExtension, MAX_VIDEO_SIZE } from '../files/files.constants'; import { AllowedFileTypesService } from '../files/allowed-file-types.service'; -const uploadsBase = path.join(__dirname, '../../../uploads/journey'); /** - * Journey image upload options, built from the container. - * - * Same reason as the trip-file factory: the fileFilter reads the operator's - * allowed-extension list at request time. Note this config carries NO - * defParamCharset, unlike the trip-file one; that difference is deliberate and - * predates the move, so the two are not folded into one shared object. + * One filename hook for all four journey upload routes (consumed by + * journey.module.ts's storage-upload factory). Field routing is exact: the + * image routes only ever admit 'photos'/'cover' fields and the video route + * only 'video'/'poster' — any other part dies in multer with + * LIMIT_UNEXPECTED_FILE before this hook runs. */ -export function buildJourneyImageUploadOptions(allowedTypes: AllowedFileTypesService) { - return { - storage: diskStorage({ - destination: (_req, _file, cb) => { if (!fs.existsSync(uploadsBase)) fs.mkdirSync(uploadsBase, { recursive: true }); cb(null, uploadsBase); }, - filename: (_req, file, cb) => cb(null, `${crypto.randomUUID()}${path.extname(file.originalname).toLowerCase() || '.jpg'}`), - }), - limits: { fileSize: 20 * 1024 * 1024 }, - fileFilter: (_req: unknown, file: Express.Multer.File, cb: (err: Error | null, accept: boolean) => void) => { - if (!file.mimetype.startsWith('image/') || file.mimetype.includes('svg')) { - const err: Error & { statusCode?: number } = new Error('Only image files are allowed'); - err.statusCode = 400; - return cb(err, false); - } - const ext = path.extname(file.originalname).toLowerCase().replace('.', ''); - const allowed = allowedTypes.get().split(',').map((e) => e.trim().toLowerCase()); - if (!allowed.includes('*') && !allowed.includes(ext)) { - const err: Error & { statusCode?: number } = new Error(`File type .${ext} is not allowed`); - err.statusCode = 400; - return cb(err, false); - } - cb(null, true); - }, +export const journeyUploadFilename = (_req: Request, file: Express.Multer.File): string => { + // The poster is ALWAYS stored as .jpg, never the client-supplied extension: + // otherwise a poster declared image/* but named x.html / x.js would land on + // disk with that extension and be served inline same-origin (stored XSS, + // reachable via the public share proxy). The video extension is validated by + // the fileFilter, so it is safe to keep. + const ext = + file.fieldname === 'poster' ? '.jpg' + : file.fieldname === 'video' ? (path.extname(file.originalname).toLowerCase() || '.mp4') + : (path.extname(file.originalname).toLowerCase() || '.jpg'); + return `${crypto.randomUUID()}${ext}`; +}; + +/** + * Journey image upload filter, built from the container. + * + * Same reason as the trip-file factory: it reads the operator's + * allowed-extension list at request time. The module's multer options carry NO + * defParamCharset, unlike the trip-file ones; that difference is deliberate + * and predates the move. + */ +export function journeyImageFileFilter(allowedTypes: AllowedFileTypesService): Options['fileFilter'] { + return (_req, file, cb) => { + if (!file.mimetype.startsWith('image/') || file.mimetype.includes('svg')) { + const err: Error & { statusCode?: number } = new Error('Only image files are allowed'); + err.statusCode = 400; + return cb(err); + } + const ext = path.extname(file.originalname).toLowerCase().replace('.', ''); + const allowed = allowedTypes.get().split(',').map((e) => e.trim().toLowerCase()); + if (!allowed.includes('*') && !allowed.includes(ext)) { + const err: Error & { statusCode?: number } = new Error(`File type .${ext} is not allowed`); + err.statusCode = 400; + return cb(err); + } + cb(null, true); }; } // Gallery video upload (#823): one video plus an optional client-captured poster // image, written to the same uploads/journey store. Larger cap than images since // phone clips are big; videos are stored as-is and streamed with HTTP Range. -const VIDEO_UPLOAD = { - storage: diskStorage({ - destination: (_req, _file, cb) => { if (!fs.existsSync(uploadsBase)) fs.mkdirSync(uploadsBase, { recursive: true }); cb(null, uploadsBase); }, - filename: (_req, file, cb) => { - // The poster is ALWAYS stored as .jpg, never the client-supplied extension: - // otherwise a poster declared image/* but named x.html / x.js would land on - // disk with that extension and be served inline same-origin (stored XSS, - // reachable via the public share proxy). The video extension is validated by - // the fileFilter, so it is safe to keep. - const ext = file.fieldname === 'poster' ? '.jpg' : (path.extname(file.originalname).toLowerCase() || '.mp4'); - cb(null, `${crypto.randomUUID()}${ext}`); - }, - }), - limits: { fileSize: MAX_VIDEO_SIZE }, - fileFilter: (_req: unknown, file: Express.Multer.File, cb: (err: Error | null, accept: boolean) => void) => { - const reject = (msg: string) => { - const err: Error & { statusCode?: number } = new Error(msg); - err.statusCode = 400; - cb(err, false); - }; - if (file.fieldname === 'poster') { - if (!file.mimetype.startsWith('image/') || file.mimetype.includes('svg')) return reject('Poster must be an image'); - return cb(null, true); - } - // 'video' field - const ext = path.extname(file.originalname).toLowerCase().replace('.', ''); - if (!isVideoMime(file.mimetype)) return reject('Only video files are allowed'); - if (!isVideoExtension(ext)) return reject(`Video type .${ext} is not allowed`); - cb(null, true); - }, +// Passed inline on the video route; the storage engine (spool destination + +// journeyUploadFilename) is inherited from the module options via the +// interceptor's shallow merge. +const VIDEO_FILE_FILTER: Options['fileFilter'] = (_req, file, cb) => { + const reject = (msg: string) => { + const err: Error & { statusCode?: number } = new Error(msg); + err.statusCode = 400; + cb(err); + }; + if (file.fieldname === 'poster') { + if (!file.mimetype.startsWith('image/') || file.mimetype.includes('svg')) return reject('Poster must be an image'); + return cb(null, true); + } + // 'video' field + const ext = path.extname(file.originalname).toLowerCase().replace('.', ''); + if (!isVideoMime(file.mimetype)) return reject('Only video files are allowed'); + if (!isVideoExtension(ext)) return reject(`Video type .${ext} is not allowed`); + cb(null, true); }; /** @@ -120,7 +122,29 @@ const VIDEO_UPLOAD = { @UseGuards(AddonGuard, JwtAuthGuard) @RequireAddon(ADDON_IDS.JOURNEY, 'Journey') export class JourneyController { - constructor(private readonly journey: JourneyService) {} + constructor( + private readonly journey: JourneyService, + private readonly storage: StorageService, + ) {} + + /** + * Commit spooled multer files to their final uploads/journey location. On any + * failure: best-effort removal of both spool leftovers and already-committed + * names, then rethrow (500) — so a DB row never references a missing file. + */ + private async commitJourneyUploads(files: Express.Multer.File[]): Promise { + try { + for (const f of files) await this.storage.put('journey', f.filename, { tmpPath: f.path }); + } catch (err) { + for (const f of files) { + if (f.path) { try { fs.unlinkSync(f.path); } catch { /* best-effort */ } } + } + for (const f of files) { + await this.storage.delete('journey', f.filename).catch(() => {}); + } + throw err; + } + } // ── Static prefix routes (before /:id) ────────────────────────────────── @Get() @@ -176,6 +200,9 @@ export class JourneyController { if (!files?.length) { throw new HttpException({ error: 'No files uploaded' }, 400); } + // Commit to final storage BEFORE the addPhoto loop: the Immich mirror below + // reads each file at its final uploads/journey path. + await this.commitJourneyUploads(files); const results: unknown[] = []; for (const file of files) { const relativePath = `journey/${file.filename}`; @@ -268,10 +295,11 @@ export class JourneyController { // ── Gallery (prefix /:id/gallery — before /:id) ───────────────────────── @Post(':id/gallery/photos') @UseInterceptors(FilesInterceptor('photos')) - uploadGalleryPhotos(@CurrentUser() user: User, @Param('id') id: string, @UploadedFiles() files: Express.Multer.File[] | undefined) { + async uploadGalleryPhotos(@CurrentUser() user: User, @Param('id') id: string, @UploadedFiles() files: Express.Multer.File[] | undefined) { if (!files?.length) { throw new HttpException({ error: 'No files uploaded' }, 400); } + await this.commitJourneyUploads(files); const filePaths = files.map((f) => ({ path: `journey/${f.filename}` })); const photos = this.journey.uploadGalleryPhotos(Number(id), user.id, filePaths); if (!photos.length) { @@ -281,8 +309,13 @@ export class JourneyController { } @Post(':id/gallery/video') - @UseInterceptors(FileFieldsInterceptor([{ name: 'video', maxCount: 1 }, { name: 'poster', maxCount: 1 }], VIDEO_UPLOAD)) - uploadGalleryVideo( + @UseInterceptors(FileFieldsInterceptor( + [{ name: 'video', maxCount: 1 }, { name: 'poster', maxCount: 1 }], + // Inherits the module's storage engine (spool + journeyUploadFilename) via + // the interceptor's shallow options merge; overrides only cap and filter. + { limits: { fileSize: MAX_VIDEO_SIZE }, fileFilter: VIDEO_FILE_FILTER }, + )) + async uploadGalleryVideo( @CurrentUser() user: User, @Param('id') id: string, @UploadedFiles() files: { video?: Express.Multer.File[]; poster?: Express.Multer.File[] } | undefined, @@ -290,8 +323,8 @@ export class JourneyController { ) { const video = files?.video?.[0]; const poster = files?.poster?.[0]; - // multer already wrote both parts; clean them up on any rejection so a POST to - // a journey the user can't edit doesn't orphan a 500 MB clip on disk (#823). + // multer already spooled both parts; clean them up on a pre-commit rejection + // so a bad POST doesn't orphan a 500 MB clip (#823). const cleanup = () => { for (const f of [video, poster]) { if (f?.path) { try { fs.unlinkSync(f.path); } catch { /* best-effort */ } } @@ -301,6 +334,7 @@ export class JourneyController { cleanup(); throw new HttpException({ error: 'No video uploaded' }, 400); } + await this.commitJourneyUploads(poster ? [video, poster] : [video]); const durationMs = body?.duration_ms != null ? Number(body.duration_ms) : null; const photos = this.journey.uploadGalleryPhotos(Number(id), user.id, [{ path: `journey/${video.filename}`, @@ -309,7 +343,11 @@ export class JourneyController { durationMs: durationMs != null && Number.isFinite(durationMs) ? durationMs : null, }]); if (!photos.length) { - cleanup(); + // The parts are already committed (file.path is consumed), so the + // pre-commit unlink cleanup would silently orphan them — delete the + // final objects instead. Same observable outcome as before: bytes gone. + await this.storage.delete('journey', video.filename).catch(() => {}); + if (poster) await this.storage.delete('journey', poster.filename).catch(() => {}); throw new HttpException({ error: 'Not allowed' }, 403); } return { photos }; @@ -371,10 +409,11 @@ export class JourneyController { @Post(':id/cover') @HttpCode(200) // Express answers cover with res.json (200). @UseInterceptors(FileInterceptor('cover')) - cover(@CurrentUser() user: User, @Param('id') id: string, @UploadedFile() file: Express.Multer.File | undefined) { + async cover(@CurrentUser() user: User, @Param('id') id: string, @UploadedFile() file: Express.Multer.File | undefined) { if (!file) { throw new HttpException({ error: 'No file uploaded' }, 400); } + await this.commitJourneyUploads([file]); const result = this.journey.updateJourney(Number(id), user.id, { cover_image: `journey/${file.filename}` }); if (!result) { throw new HttpException({ error: 'Journey not found' }, 404); diff --git a/server/src/nest/journey/journey.module.ts b/server/src/nest/journey/journey.module.ts index e6b7ab86e..08155c8ab 100644 --- a/server/src/nest/journey/journey.module.ts +++ b/server/src/nest/journey/journey.module.ts @@ -10,16 +10,28 @@ import { AuthModule } from '../auth/auth.module'; import { MulterModule } from '@nestjs/platform-express'; import { AllowedFileTypesModule } from '../files/allowed-file-types.module'; import { AllowedFileTypesService } from '../files/allowed-file-types.service'; -import { buildJourneyImageUploadOptions } from './journey.controller'; +import { StorageModule } from '../storage/storage.module'; +import { StorageService } from '../storage/storage.service'; +import { buildStorageUploadOptions } from '../storage/storage-upload.factory'; +import { journeyImageFileFilter, journeyUploadFilename } from './journey.controller'; @Module({ // MemoriesModule: the journey gallery streams provider assets and uploads to Immich. imports: [ MulterModule.registerAsync({ - imports: [AllowedFileTypesModule], - inject: [AllowedFileTypesService], - useFactory: (allowedTypes: AllowedFileTypesService) => buildJourneyImageUploadOptions(allowedTypes), + imports: [StorageModule, AllowedFileTypesModule], + inject: [StorageService, AllowedFileTypesService], + // NO defParamCharset here — deliberate, documented asymmetry with the + // trip-file options (see journey.controller.ts). + useFactory: (storage: StorageService, allowedTypes: AllowedFileTypesService) => + buildStorageUploadOptions(storage, { + category: 'journey', + maxSize: 20 * 1024 * 1024, + fileFilter: journeyImageFileFilter(allowedTypes), + filename: journeyUploadFilename, + }), }), + StorageModule, AuthModule, AddonsModule, MemoriesModule, JourneyDomainModule], controllers: [JourneyController, JourneyPublicController], providers: [JourneyService, JourneyMcp], diff --git a/server/tests/integration/journey.test.ts b/server/tests/integration/journey.test.ts index af2c9f11c..421b07751 100644 --- a/server/tests/integration/journey.test.ts +++ b/server/tests/integration/journey.test.ts @@ -1008,3 +1008,154 @@ describe('Photo upload validation', () => { expect(res.body.error).toBe('No files uploaded'); }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// Upload parity (JOURNEY-P01…P09) — entry photos, gallery photos, video+poster, +// journey cover. Written BEFORE the storage-upload swap to pin behavior. +// ───────────────────────────────────────────────────────────────────────────── + +describe('Journey upload parity', () => { + const fsMod = require('fs') as typeof import('fs'); + const pathMod = require('path') as typeof import('path'); + const FIXTURE_IMG = pathMod.join(__dirname, '../fixtures/small-image.jpg'); + const journeyDir = pathMod.join(__dirname, '../../uploads/journey'); + + afterAll(() => { + fsMod.rmSync(journeyDir, { recursive: true, force: true }); + }); + + it('JOURNEY-P01 — entry photo upload stores journey/ with a lowercased extension', async () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id); + const entry = createJourneyEntry(testDb, journey.id, user.id, { entry_date: '2026-04-01' }); + + const res = await request(app) + .post(`/api/journeys/entries/${entry.id}/photos`) + .set('Cookie', authCookie(user.id)) + .attach('photos', FIXTURE_IMG, 'PHOTO.JPG'); + expect(res.status).toBe(201); + expect(res.body.photos).toHaveLength(1); + expect(res.body.photos[0].file_path).toMatch(/^journey\/[0-9a-f-]{36}\.jpg$/); + const diskName = res.body.photos[0].file_path.replace(/^journey\//, ''); + expect(fsMod.existsSync(pathMod.join(journeyDir, diskName))).toBe(true); + }); + + it('JOURNEY-P02 — extensionless originalname falls back to .jpg (wildcard allowlist)', async () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id); + const entry = createJourneyEntry(testDb, journey.id, user.id, { entry_date: '2026-04-01' }); + // The admin allowlist rejects the empty extension before the filename + // fallback can run; the wildcard makes the fallback reachable so it is + // actually pinned here. + testDb.prepare("INSERT OR REPLACE INTO app_settings (key, value) VALUES ('allowed_file_types', '*')").run(); + + const res = await request(app) + .post(`/api/journeys/entries/${entry.id}/photos`) + .set('Cookie', authCookie(user.id)) + .attach('photos', fsMod.readFileSync(FIXTURE_IMG), { filename: 'noext', contentType: 'image/jpeg' }); + expect(res.status).toBe(201); + expect(res.body.photos[0].file_path).toMatch(/^journey\/[0-9a-f-]{36}\.jpg$/); + }); + + it('JOURNEY-P03 — non-image entry upload is rejected 400', async () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id); + const entry = createJourneyEntry(testDb, journey.id, user.id, { entry_date: '2026-04-01' }); + + const res = await request(app) + .post(`/api/journeys/entries/${entry.id}/photos`) + .set('Cookie', authCookie(user.id)) + .attach('photos', Buffer.from('plain text'), { filename: 'doc.txt', contentType: 'text/plain' }); + expect(res.status).toBe(400); + expect(res.body.error).toBe('Only image files are allowed'); + }); + + it('JOURNEY-P04 — gallery photo upload stores journey/ paths', async () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id); + + const res = await request(app) + .post(`/api/journeys/${journey.id}/gallery/photos`) + .set('Cookie', authCookie(user.id)) + .attach('photos', FIXTURE_IMG); + expect(res.status).toBe(201); + expect(res.body.photos.length).toBeGreaterThan(0); + }); + + it('JOURNEY-P05 — non-contributor gallery upload is 403 and files remain on disk (no cleanup today)', async () => { + const { user: owner } = createUser(testDb); + const { user: other } = createUser(testDb); + const journey = createJourney(testDb, owner.id); + + const before = fsMod.existsSync(journeyDir) ? fsMod.readdirSync(journeyDir).length : 0; + const res = await request(app) + .post(`/api/journeys/${journey.id}/gallery/photos`) + .set('Cookie', authCookie(other.id)) + .attach('photos', FIXTURE_IMG); + expect(res.status).toBe(403); + expect(res.body.error).toBe('Not allowed'); + const after = fsMod.existsSync(journeyDir) ? fsMod.readdirSync(journeyDir).length : 0; + expect(after).toBe(before + 1); + }); + + it('JOURNEY-P06 — video+poster upload: poster is ALWAYS stored as .jpg (stored-XSS pin)', async () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id); + + const res = await request(app) + .post(`/api/journeys/${journey.id}/gallery/video`) + .set('Cookie', authCookie(user.id)) + .attach('video', Buffer.from('fake video bytes'), { filename: 'clip.MP4', contentType: 'video/mp4' }) + .attach('poster', fsMod.readFileSync(FIXTURE_IMG), { filename: 'poster.png', contentType: 'image/png' }); + expect(res.status).toBe(201); + expect(res.body.photos).toHaveLength(1); + expect(res.body.photos[0].file_path).toMatch(/^journey\/[0-9a-f-]{36}\.mp4$/); + expect(res.body.photos[0].thumbnail_path).toMatch(/^journey\/[0-9a-f-]{36}\.jpg$/); + }); + + it('JOURNEY-P07 — poster-only video upload is 400 with no files left anywhere', async () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id); + + const before = fsMod.existsSync(journeyDir) ? fsMod.readdirSync(journeyDir).length : 0; + const res = await request(app) + .post(`/api/journeys/${journey.id}/gallery/video`) + .set('Cookie', authCookie(user.id)) + .attach('poster', fsMod.readFileSync(FIXTURE_IMG), { filename: 'poster.png', contentType: 'image/png' }); + expect(res.status).toBe(400); + expect(res.body.error).toBe('No video uploaded'); + const after = fsMod.existsSync(journeyDir) ? fsMod.readdirSync(journeyDir).length : 0; + expect(after).toBe(before); + }); + + it('JOURNEY-P08 — non-contributor video upload is 403 and BOTH files are removed', async () => { + const { user: owner } = createUser(testDb); + const { user: other } = createUser(testDb); + const journey = createJourney(testDb, owner.id); + + const before = fsMod.existsSync(journeyDir) ? fsMod.readdirSync(journeyDir).length : 0; + const res = await request(app) + .post(`/api/journeys/${journey.id}/gallery/video`) + .set('Cookie', authCookie(other.id)) + .attach('video', Buffer.from('fake video bytes'), { filename: 'clip.mp4', contentType: 'video/mp4' }) + .attach('poster', fsMod.readFileSync(FIXTURE_IMG), { filename: 'poster.png', contentType: 'image/png' }); + expect(res.status).toBe(403); + expect(res.body.error).toBe('Not allowed'); + const after = fsMod.existsSync(journeyDir) ? fsMod.readdirSync(journeyDir).length : 0; + expect(after).toBe(before); + }); + + it('JOURNEY-P09 — cover upload stores journey/ into cover_image', async () => { + const { user } = createUser(testDb); + const journey = createJourney(testDb, user.id); + + const res = await request(app) + .post(`/api/journeys/${journey.id}/cover`) + .set('Cookie', authCookie(user.id)) + .attach('cover', FIXTURE_IMG); + expect(res.status).toBe(200); + expect(res.body.cover_image).toMatch(/^journey\/[0-9a-f-]{36}\.jpg$/); + const diskName = res.body.cover_image.replace(/^journey\//, ''); + expect(fsMod.existsSync(pathMod.join(journeyDir, diskName))).toBe(true); + }); +}); diff --git a/server/tests/unit/nest/journey.controller.test.ts b/server/tests/unit/nest/journey.controller.test.ts index 194179dc4..9563cba61 100644 --- a/server/tests/unit/nest/journey.controller.test.ts +++ b/server/tests/unit/nest/journey.controller.test.ts @@ -7,6 +7,7 @@ import fs from 'node:fs'; import { JourneyController } from '../../../src/nest/journey/journey.controller'; import { JourneyPublicController } from '../../../src/nest/journey/journey-public.controller'; import type { JourneyService } from '../../../src/nest/journey/journey.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; @@ -15,6 +16,11 @@ function svc(o: Partial = {}): JourneyService { return { journeyAddonEnabled: vi.fn().mockReturnValue(true), ...o } as unknown as JourneyService; } +const storageStub = { + put: vi.fn().mockResolvedValue(undefined), + delete: vi.fn().mockResolvedValue(undefined), +} as unknown as StorageService; + function thrown(fn: () => unknown): { status: number; body: unknown } { try { fn(); } catch (err) { expect(err).toBeInstanceOf(HttpException); @@ -36,127 +42,130 @@ beforeEach(() => vi.clearAllMocks()); describe('JourneyController', () => { it('GET / lists; POST / 400 without title, else creates', () => { - expect(new JourneyController(svc({ listJourneys: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial)).list(user)).toEqual({ journeys: [{ id: 1 }] }); - expect(thrown(() => new JourneyController(svc()).create(user, { title: ' ' }))).toEqual({ status: 400, body: { error: 'Title is required' } }); + expect(new JourneyController(svc({ listJourneys: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial), storageStub).list(user)).toEqual({ journeys: [{ id: 1 }] }); + expect(thrown(() => new JourneyController(svc(), storageStub).create(user, { title: ' ' }))).toEqual({ status: 400, body: { error: 'Title is required' } }); const createJourney = vi.fn().mockReturnValue({ id: 9 }); - expect(new JourneyController(svc({ createJourney } as Partial)).create(user, { title: ' Trip ', trip_ids: [1, '2'] })).toEqual({ id: 9 }); + expect(new JourneyController(svc({ createJourney } as Partial), storageStub).create(user, { title: ' Trip ', trip_ids: [1, '2'] })).toEqual({ id: 9 }); expect(createJourney).toHaveBeenCalledWith(1, { title: 'Trip', subtitle: undefined, trip_ids: [1, 2] }); }); it('GET /suggestions + /available-trips', () => { - expect(new JourneyController(svc({ getSuggestions: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial)).suggestions(user)).toEqual({ trips: [{ id: 1 }] }); - expect(new JourneyController(svc({ listUserTrips: vi.fn().mockReturnValue([{ id: 2 }]) } as Partial)).availableTrips(user)).toEqual({ trips: [{ id: 2 }] }); + expect(new JourneyController(svc({ getSuggestions: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial), storageStub).suggestions(user)).toEqual({ trips: [{ id: 1 }] }); + expect(new JourneyController(svc({ listUserTrips: vi.fn().mockReturnValue([{ id: 2 }]) } as Partial), storageStub).availableTrips(user)).toEqual({ trips: [{ id: 2 }] }); }); it('PATCH/DELETE entries map 404', () => { - expect(thrown(() => new JourneyController(svc({ updateEntry: vi.fn().mockReturnValue(null) } as Partial)).updateEntry(user, '3', {}))).toEqual({ status: 404, body: { error: 'Entry not found' } }); - expect(new JourneyController(svc({ updateEntry: vi.fn().mockReturnValue({ id: 3 }) } as Partial)).updateEntry(user, '3', { title: 'x' })).toEqual({ id: 3 }); - expect(thrown(() => new JourneyController(svc({ deleteEntry: vi.fn().mockReturnValue(false) } as Partial)).deleteEntry(user, '3'))).toEqual({ status: 404, body: { error: 'Entry not found' } }); - expect(new JourneyController(svc({ deleteEntry: vi.fn().mockReturnValue(true) } as Partial)).deleteEntry(user, '3')).toEqual({ success: true }); + expect(thrown(() => new JourneyController(svc({ updateEntry: vi.fn().mockReturnValue(null) } as Partial), storageStub).updateEntry(user, '3', {}))).toEqual({ status: 404, body: { error: 'Entry not found' } }); + expect(new JourneyController(svc({ updateEntry: vi.fn().mockReturnValue({ id: 3 }) } as Partial), storageStub).updateEntry(user, '3', { title: 'x' })).toEqual({ id: 3 }); + expect(thrown(() => new JourneyController(svc({ deleteEntry: vi.fn().mockReturnValue(false) } as Partial), storageStub).deleteEntry(user, '3'))).toEqual({ status: 404, body: { error: 'Entry not found' } }); + expect(new JourneyController(svc({ deleteEntry: vi.fn().mockReturnValue(true) } as Partial), storageStub).deleteEntry(user, '3')).toEqual({ success: true }); }); it('provider-photos: batch, single 400/403, success', () => { const batch = svc({ addProviderPhoto: vi.fn().mockReturnValue({ id: 1 }) } as Partial); - expect(new JourneyController(batch).providerPhotos(user, '3', { provider: 'immich', asset_ids: ['a', 'b'] })).toEqual({ photos: [{ id: 1 }, { id: 1 }], added: 2 }); - expect(thrown(() => new JourneyController(svc()).providerPhotos(user, '3', { provider: 'immich' }))).toEqual({ status: 400, body: { error: 'provider and asset_id required' } }); - expect(thrown(() => new JourneyController(svc({ addProviderPhoto: vi.fn().mockReturnValue(null) } as Partial)).providerPhotos(user, '3', { provider: 'immich', asset_id: 'a' }))).toEqual({ status: 403, body: { error: 'Not allowed or duplicate' } }); + expect(new JourneyController(batch, storageStub).providerPhotos(user, '3', { provider: 'immich', asset_ids: ['a', 'b'] })).toEqual({ photos: [{ id: 1 }, { id: 1 }], added: 2 }); + expect(thrown(() => new JourneyController(svc(), storageStub).providerPhotos(user, '3', { provider: 'immich' }))).toEqual({ status: 400, body: { error: 'provider and asset_id required' } }); + expect(thrown(() => new JourneyController(svc({ addProviderPhoto: vi.fn().mockReturnValue(null) } as Partial), storageStub).providerPhotos(user, '3', { provider: 'immich', asset_id: 'a' }))).toEqual({ status: 403, body: { error: 'Not allowed or duplicate' } }); }); it('link-photo: 400 without id (accepts legacy photo_id), 403, success', () => { - expect(thrown(() => new JourneyController(svc()).linkPhoto(user, '3', {}))).toEqual({ status: 400, body: { error: 'journey_photo_id required' } }); + expect(thrown(() => new JourneyController(svc(), storageStub).linkPhoto(user, '3', {}))).toEqual({ status: 400, body: { error: 'journey_photo_id required' } }); const linkPhotoToEntry = vi.fn().mockReturnValue({ id: 5 }); - const c = new JourneyController(svc({ linkPhotoToEntry } as Partial)); + const c = new JourneyController(svc({ linkPhotoToEntry } as Partial), storageStub); expect(c.linkPhoto(user, '3', { photo_id: 5 })).toEqual({ id: 5 }); expect(linkPhotoToEntry).toHaveBeenCalledWith(3, 5, 1); // accepts the canonical journey_photo_id, 403 when the service refuses - expect(thrown(() => new JourneyController(svc({ linkPhotoToEntry: vi.fn().mockReturnValue(null) } as Partial)).linkPhoto(user, '3', { journey_photo_id: 9 }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(thrown(() => new JourneyController(svc({ linkPhotoToEntry: vi.fn().mockReturnValue(null) } as Partial), storageStub).linkPhoto(user, '3', { journey_photo_id: 9 }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); }); it('unlink photo (204) maps 404; delete photo 404 then unlinks file', () => { - expect(thrown(() => new JourneyController(svc({ unlinkPhotoFromEntry: vi.fn().mockReturnValue(false) } as Partial)).unlinkPhoto(user, '3', '7'))).toEqual({ status: 404, body: { error: 'Not found or not allowed' } }); - expect(new JourneyController(svc({ unlinkPhotoFromEntry: vi.fn().mockReturnValue(true) } as Partial)).unlinkPhoto(user, '3', '7')).toBeUndefined(); - expect(thrown(() => new JourneyController(svc({ deletePhoto: vi.fn().mockReturnValue(null) } as Partial)).deletePhoto(user, '7'))).toEqual({ status: 404, body: { error: 'Photo not found' } }); - expect(new JourneyController(svc({ deletePhoto: vi.fn().mockReturnValue({ id: 7, file_path: null }) } as Partial)).deletePhoto(user, '7')).toEqual({ success: true }); + expect(thrown(() => new JourneyController(svc({ unlinkPhotoFromEntry: vi.fn().mockReturnValue(false) } as Partial), storageStub).unlinkPhoto(user, '3', '7'))).toEqual({ status: 404, body: { error: 'Not found or not allowed' } }); + expect(new JourneyController(svc({ unlinkPhotoFromEntry: vi.fn().mockReturnValue(true) } as Partial), storageStub).unlinkPhoto(user, '3', '7')).toBeUndefined(); + expect(thrown(() => new JourneyController(svc({ deletePhoto: vi.fn().mockReturnValue(null) } as Partial), storageStub).deletePhoto(user, '7'))).toEqual({ status: 404, body: { error: 'Photo not found' } }); + expect(new JourneyController(svc({ deletePhoto: vi.fn().mockReturnValue({ id: 7, file_path: null }) } as Partial), storageStub).deletePhoto(user, '7')).toEqual({ success: true }); }); - it('gallery upload 400 no files / 403 not allowed, else returns photos', () => { - expect(thrown(() => new JourneyController(svc()).uploadGalleryPhotos(user, '3', undefined))).toEqual({ status: 400, body: { error: 'No files uploaded' } }); - expect(thrown(() => new JourneyController(svc({ uploadGalleryPhotos: vi.fn().mockReturnValue([]) } as Partial)).uploadGalleryPhotos(user, '3', [{ filename: 'a.jpg' } as Express.Multer.File]))).toEqual({ status: 403, body: { error: 'Not allowed' } }); - expect(new JourneyController(svc({ uploadGalleryPhotos: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial)).uploadGalleryPhotos(user, '3', [{ filename: 'a.jpg' } as Express.Multer.File])).toEqual({ photos: [{ id: 1 }] }); + it('gallery upload 400 no files / 403 not allowed, else commits + returns photos', async () => { + expect(await thrownAsync(() => new JourneyController(svc(), storageStub).uploadGalleryPhotos(user, '3', undefined))).toEqual({ status: 400, body: { error: 'No files uploaded' } }); + expect(await thrownAsync(() => new JourneyController(svc({ uploadGalleryPhotos: vi.fn().mockReturnValue([]) } as Partial), storageStub).uploadGalleryPhotos(user, '3', [{ filename: 'a.jpg' } as Express.Multer.File]))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(await new JourneyController(svc({ uploadGalleryPhotos: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial), storageStub).uploadGalleryPhotos(user, '3', [{ filename: 'a.jpg' } as Express.Multer.File])).toEqual({ photos: [{ id: 1 }] }); + expect(storageStub.put).toHaveBeenCalledWith('journey', 'a.jpg', { tmpPath: undefined }); }); - it('gallery video: 400 no video, 403 not allowed, else stores the clip + poster (#823)', () => { + it('gallery video: 400 no video, 403 not allowed, else stores the clip + poster (#823)', async () => { const files = { video: [{ filename: 'v.mp4' } as Express.Multer.File], poster: [{ filename: 'p.jpg' } as Express.Multer.File] }; - expect(thrown(() => new JourneyController(svc()).uploadGalleryVideo(user, '3', {}, {}))).toEqual({ status: 400, body: { error: 'No video uploaded' } }); - // Rejected with real paths → the cleanup unlinks the orphaned bytes (the files - // don't exist, so the best-effort catch swallows it). + expect(await thrownAsync(() => new JourneyController(svc(), storageStub).uploadGalleryVideo(user, '3', {}, {}))).toEqual({ status: 400, body: { error: 'No video uploaded' } }); + // Not-allowed after commit → the final objects are deleted through storage + // (the pre-commit unlink would miss them: file.path is already consumed). const withPaths = { video: [{ filename: 'v.mp4', path: '/nonexistent/v.mp4' } as Express.Multer.File], poster: [{ filename: 'p.jpg', path: '/nonexistent/p.jpg' } as Express.Multer.File] }; - expect(thrown(() => new JourneyController(svc({ uploadGalleryPhotos: vi.fn().mockReturnValue([]) } as Partial)).uploadGalleryVideo(user, '3', withPaths, { duration_ms: 'abc' }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(await thrownAsync(() => new JourneyController(svc({ uploadGalleryPhotos: vi.fn().mockReturnValue([]) } as Partial), storageStub).uploadGalleryVideo(user, '3', withPaths, { duration_ms: 'abc' }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(storageStub.delete).toHaveBeenCalledWith('journey', 'v.mp4'); + expect(storageStub.delete).toHaveBeenCalledWith('journey', 'p.jpg'); const up = vi.fn().mockReturnValue([{ id: 7 }]); - expect(new JourneyController(svc({ uploadGalleryPhotos: up } as Partial)).uploadGalleryVideo(user, '3', files, { duration_ms: '4200' })).toEqual({ photos: [{ id: 7 }] }); + expect(await new JourneyController(svc({ uploadGalleryPhotos: up } as Partial), storageStub).uploadGalleryVideo(user, '3', files, { duration_ms: '4200' })).toEqual({ photos: [{ id: 7 }] }); expect(up).toHaveBeenCalledWith(3, 1, [{ path: 'journey/v.mp4', thumbnail: 'journey/p.jpg', mediaType: 'video', durationMs: 4200 }]); // No poster + no duration → thumbnail undefined, durationMs null. const up2 = vi.fn().mockReturnValue([{ id: 8 }]); - new JourneyController(svc({ uploadGalleryPhotos: up2 } as Partial)).uploadGalleryVideo(user, '3', { video: [{ filename: 'v2.mp4' } as Express.Multer.File] }, {}); + await new JourneyController(svc({ uploadGalleryPhotos: up2 } as Partial), storageStub).uploadGalleryVideo(user, '3', { video: [{ filename: 'v2.mp4' } as Express.Multer.File] }, {}); expect(up2).toHaveBeenCalledWith(3, 1, [{ path: 'journey/v2.mp4', thumbnail: undefined, mediaType: 'video', durationMs: null }]); }); it('provider-photos forwards per-asset media_types for gallery and entries (#823)', () => { const add = vi.fn().mockReturnValue({ id: 1 }); - new JourneyController(svc({ addProviderPhotoToGallery: add } as Partial)).galleryProviderPhotos(user, '9', { provider: 'immich', asset_ids: ['a', 'b'], media_types: ['video', 'image'] }); + new JourneyController(svc({ addProviderPhotoToGallery: add } as Partial), storageStub).galleryProviderPhotos(user, '9', { provider: 'immich', asset_ids: ['a', 'b'], media_types: ['video', 'image'] }); expect(add).toHaveBeenNthCalledWith(1, 9, 1, 'immich', 'a', undefined, undefined, 'video'); expect(add).toHaveBeenNthCalledWith(2, 9, 1, 'immich', 'b', undefined, undefined, 'image'); const addOne = vi.fn().mockReturnValue({ id: 2 }); - new JourneyController(svc({ addProviderPhotoToGallery: addOne } as Partial)).galleryProviderPhotos(user, '9', { provider: 'immich', asset_id: 'c', media_type: 'video' }); + new JourneyController(svc({ addProviderPhotoToGallery: addOne } as Partial), storageStub).galleryProviderPhotos(user, '9', { provider: 'immich', asset_id: 'c', media_type: 'video' }); expect(addOne).toHaveBeenCalledWith(9, 1, 'immich', 'c', undefined, undefined, 'video'); // Entry path mirrors the gallery path. const eAdd = vi.fn().mockReturnValue({ id: 3 }); - new JourneyController(svc({ addProviderPhoto: eAdd } as Partial)).providerPhotos(user, '4', { provider: 'immich', asset_ids: ['x'], media_types: ['video'], caption: 'c' }); + new JourneyController(svc({ addProviderPhoto: eAdd } as Partial), storageStub).providerPhotos(user, '4', { provider: 'immich', asset_ids: ['x'], media_types: ['video'], caption: 'c' }); expect(eAdd).toHaveBeenNthCalledWith(1, 4, 1, 'immich', 'x', 'c', undefined, 'video'); const eOne = vi.fn().mockReturnValue({ id: 4 }); - new JourneyController(svc({ addProviderPhoto: eOne } as Partial)).providerPhotos(user, '4', { provider: 'immich', asset_id: 'y', media_type: 'video' }); + new JourneyController(svc({ addProviderPhoto: eOne } as Partial), storageStub).providerPhotos(user, '4', { provider: 'immich', asset_id: 'y', media_type: 'video' }); expect(eOne).toHaveBeenCalledWith(4, 1, 'immich', 'y', undefined, undefined, 'video'); }); it('GET/PATCH/DELETE /:id map 404', () => { - expect(thrown(() => new JourneyController(svc({ getJourneyFull: vi.fn().mockReturnValue(null) } as Partial)).get(user, '9'))).toEqual({ status: 404, body: { error: 'Journey not found' } }); - expect(new JourneyController(svc({ getJourneyFull: vi.fn().mockReturnValue({ id: 9 }) } as Partial)).get(user, '9')).toEqual({ id: 9 }); - expect(thrown(() => new JourneyController(svc({ updateJourney: vi.fn().mockReturnValue(null) } as Partial)).update(user, '9', {}))).toEqual({ status: 404, body: { error: 'Journey not found' } }); - expect(thrown(() => new JourneyController(svc({ deleteJourney: vi.fn().mockReturnValue(false) } as Partial)).remove(user, '9'))).toEqual({ status: 404, body: { error: 'Journey not found' } }); + expect(thrown(() => new JourneyController(svc({ getJourneyFull: vi.fn().mockReturnValue(null) } as Partial), storageStub).get(user, '9'))).toEqual({ status: 404, body: { error: 'Journey not found' } }); + expect(new JourneyController(svc({ getJourneyFull: vi.fn().mockReturnValue({ id: 9 }) } as Partial), storageStub).get(user, '9')).toEqual({ id: 9 }); + expect(thrown(() => new JourneyController(svc({ updateJourney: vi.fn().mockReturnValue(null) } as Partial), storageStub).update(user, '9', {}))).toEqual({ status: 404, body: { error: 'Journey not found' } }); + expect(thrown(() => new JourneyController(svc({ deleteJourney: vi.fn().mockReturnValue(false) } as Partial), storageStub).remove(user, '9'))).toEqual({ status: 404, body: { error: 'Journey not found' } }); }); it('trips: POST 400 without trip_id / 403, DELETE 403', () => { - expect(thrown(() => new JourneyController(svc()).addTrip(user, '9', {}))).toEqual({ status: 400, body: { error: 'trip_id required' } }); - expect(thrown(() => new JourneyController(svc({ addTripToJourney: vi.fn().mockReturnValue(false) } as Partial)).addTrip(user, '9', { trip_id: 2 }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); - expect(new JourneyController(svc({ addTripToJourney: vi.fn().mockReturnValue(true) } as Partial)).addTrip(user, '9', { trip_id: 2 })).toEqual({ success: true }); - expect(thrown(() => new JourneyController(svc({ removeTripFromJourney: vi.fn().mockReturnValue(false) } as Partial)).removeTrip(user, '9', '2'))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(thrown(() => new JourneyController(svc(), storageStub).addTrip(user, '9', {}))).toEqual({ status: 400, body: { error: 'trip_id required' } }); + expect(thrown(() => new JourneyController(svc({ addTripToJourney: vi.fn().mockReturnValue(false) } as Partial), storageStub).addTrip(user, '9', { trip_id: 2 }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(new JourneyController(svc({ addTripToJourney: vi.fn().mockReturnValue(true) } as Partial), storageStub).addTrip(user, '9', { trip_id: 2 })).toEqual({ success: true }); + expect(thrown(() => new JourneyController(svc({ removeTripFromJourney: vi.fn().mockReturnValue(false) } as Partial), storageStub).removeTrip(user, '9', '2'))).toEqual({ status: 403, body: { error: 'Not allowed' } }); }); it('entries under journey: list 404, create 400/404, reorder 400/403', () => { - expect(thrown(() => new JourneyController(svc({ listEntries: vi.fn().mockReturnValue(null) } as Partial)).listEntries(user, '9'))).toEqual({ status: 404, body: { error: 'Journey not found' } }); - expect(new JourneyController(svc({ listEntries: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial)).listEntries(user, '9')).toEqual({ entries: [{ id: 1 }] }); - expect(thrown(() => new JourneyController(svc()).createEntry(user, '9', {}))).toEqual({ status: 400, body: { error: 'entry_date is required' } }); - expect(thrown(() => new JourneyController(svc({ createEntry: vi.fn().mockReturnValue(null) } as Partial)).createEntry(user, '9', { entry_date: '2026-01-01' }))).toEqual({ status: 404, body: { error: 'Journey not found' } }); - expect(thrown(() => new JourneyController(svc()).reorderEntries(user, '9', { orderedIds: 'no' }))).toEqual({ status: 400, body: { error: 'orderedIds must be an array of numbers' } }); - expect(thrown(() => new JourneyController(svc({ reorderEntries: vi.fn().mockReturnValue(false) } as Partial)).reorderEntries(user, '9', { orderedIds: [1, 2] }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(thrown(() => new JourneyController(svc({ listEntries: vi.fn().mockReturnValue(null) } as Partial), storageStub).listEntries(user, '9'))).toEqual({ status: 404, body: { error: 'Journey not found' } }); + expect(new JourneyController(svc({ listEntries: vi.fn().mockReturnValue([{ id: 1 }]) } as Partial), storageStub).listEntries(user, '9')).toEqual({ entries: [{ id: 1 }] }); + expect(thrown(() => new JourneyController(svc(), storageStub).createEntry(user, '9', {}))).toEqual({ status: 400, body: { error: 'entry_date is required' } }); + expect(thrown(() => new JourneyController(svc({ createEntry: vi.fn().mockReturnValue(null) } as Partial), storageStub).createEntry(user, '9', { entry_date: '2026-01-01' }))).toEqual({ status: 404, body: { error: 'Journey not found' } }); + expect(thrown(() => new JourneyController(svc(), storageStub).reorderEntries(user, '9', { orderedIds: 'no' }))).toEqual({ status: 400, body: { error: 'orderedIds must be an array of numbers' } }); + expect(thrown(() => new JourneyController(svc({ reorderEntries: vi.fn().mockReturnValue(false) } as Partial), storageStub).reorderEntries(user, '9', { orderedIds: [1, 2] }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); }); it('contributors: add 400/403, update 403, remove 403', () => { - expect(thrown(() => new JourneyController(svc()).addContributor(user, '9', {}))).toEqual({ status: 400, body: { error: 'user_id required' } }); - expect(thrown(() => new JourneyController(svc({ addContributor: vi.fn().mockReturnValue(false) } as Partial)).addContributor(user, '9', { user_id: 2 }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); - expect(new JourneyController(svc({ addContributor: vi.fn().mockReturnValue(true) } as Partial)).addContributor(user, '9', { user_id: 2 })).toEqual({ success: true }); - expect(thrown(() => new JourneyController(svc({ updateContributorRole: vi.fn().mockReturnValue(false) } as Partial)).updateContributor(user, '9', '2', { role: 'editor' }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); - expect(thrown(() => new JourneyController(svc({ removeContributor: vi.fn().mockReturnValue(false) } as Partial)).removeContributor(user, '9', '2'))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(thrown(() => new JourneyController(svc(), storageStub).addContributor(user, '9', {}))).toEqual({ status: 400, body: { error: 'user_id required' } }); + expect(thrown(() => new JourneyController(svc({ addContributor: vi.fn().mockReturnValue(false) } as Partial), storageStub).addContributor(user, '9', { user_id: 2 }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(new JourneyController(svc({ addContributor: vi.fn().mockReturnValue(true) } as Partial), storageStub).addContributor(user, '9', { user_id: 2 })).toEqual({ success: true }); + expect(thrown(() => new JourneyController(svc({ updateContributorRole: vi.fn().mockReturnValue(false) } as Partial), storageStub).updateContributor(user, '9', '2', { role: 'editor' }))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(thrown(() => new JourneyController(svc({ removeContributor: vi.fn().mockReturnValue(false) } as Partial), storageStub).removeContributor(user, '9', '2'))).toEqual({ status: 403, body: { error: 'Not allowed' } }); }); it('preferences 403, share-link get/set/delete', () => { - expect(thrown(() => new JourneyController(svc({ updateJourneyPreferences: vi.fn().mockReturnValue(null) } as Partial)).preferences(user, '9', {}))).toEqual({ status: 403, body: { error: 'Not allowed' } }); - expect(new JourneyController(svc({ getJourneyShareLink: vi.fn().mockReturnValue({ token: 'abc' }) } as Partial)).getShareLink(user, '9')).toEqual({ link: { token: 'abc' } }); - expect(thrown(() => new JourneyController(svc({ createOrUpdateJourneyShareLink: vi.fn().mockReturnValue(null) } as Partial)).setShareLink(user, '9', {}))).toEqual({ status: 403, body: { error: 'Not allowed' } }); - expect(new JourneyController(svc({ createOrUpdateJourneyShareLink: vi.fn().mockReturnValue({ token: 'abc' }) } as Partial)).setShareLink(user, '9', { share_timeline: true })).toEqual({ token: 'abc' }); - expect(thrown(() => new JourneyController(svc({ deleteJourneyShareLink: vi.fn().mockReturnValue(false) } as Partial)).deleteShareLink(user, '9'))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(thrown(() => new JourneyController(svc({ updateJourneyPreferences: vi.fn().mockReturnValue(null) } as Partial), storageStub).preferences(user, '9', {}))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(new JourneyController(svc({ getJourneyShareLink: vi.fn().mockReturnValue({ token: 'abc' }) } as Partial), storageStub).getShareLink(user, '9')).toEqual({ link: { token: 'abc' } }); + expect(thrown(() => new JourneyController(svc({ createOrUpdateJourneyShareLink: vi.fn().mockReturnValue(null) } as Partial), storageStub).setShareLink(user, '9', {}))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(new JourneyController(svc({ createOrUpdateJourneyShareLink: vi.fn().mockReturnValue({ token: 'abc' }) } as Partial), storageStub).setShareLink(user, '9', { share_timeline: true })).toEqual({ token: 'abc' }); + expect(thrown(() => new JourneyController(svc({ deleteJourneyShareLink: vi.fn().mockReturnValue(false) } as Partial), storageStub).deleteShareLink(user, '9'))).toEqual({ status: 403, body: { error: 'Not allowed' } }); }); it('entry photo upload mirrors to Immich only when opted in', async () => { @@ -164,51 +173,51 @@ describe('JourneyController', () => { const uploadToImmich = vi.fn().mockResolvedValue('immich-1'); const setPhotoProvider = vi.fn(); const s = svc({ addPhoto, immichAutoUploadEnabled: vi.fn().mockReturnValue(true), uploadToImmich, setPhotoProvider } as Partial); - const res = await new JourneyController(s).uploadEntryPhotos(user, '3', [{ filename: 'a.jpg', originalname: 'a.jpg' } as Express.Multer.File], {}); + const res = await new JourneyController(s, storageStub).uploadEntryPhotos(user, '3', [{ filename: 'a.jpg', originalname: 'a.jpg' } as Express.Multer.File], {}); expect(setPhotoProvider).toHaveBeenCalledWith(5, 'immich', 'immich-1', 1); expect(res).toEqual({ photos: [{ id: 5, provider: 'immich', asset_id: 'immich-1', owner_id: 1 }] }); const noOptIn = svc({ addPhoto: vi.fn().mockReturnValue({ id: 6 }), immichAutoUploadEnabled: vi.fn().mockReturnValue(false), uploadToImmich } as Partial); - await new JourneyController(noOptIn).uploadEntryPhotos(user, '3', [{ filename: 'b.jpg', originalname: 'b.jpg' } as Express.Multer.File], {}); + await new JourneyController(noOptIn, storageStub).uploadEntryPhotos(user, '3', [{ filename: 'b.jpg', originalname: 'b.jpg' } as Express.Multer.File], {}); expect(uploadToImmich).toHaveBeenCalledTimes(1); // only the opted-in upload above }); it('entry photo upload: 400 no files, 403 when nothing added, swallows immich errors and empty ids', async () => { - expect(await thrownAsync(() => new JourneyController(svc()).uploadEntryPhotos(user, '3', undefined, {}))).toEqual({ status: 400, body: { error: 'No files uploaded' } }); - expect(await thrownAsync(() => new JourneyController(svc({ addPhoto: vi.fn().mockReturnValue(null) } as Partial)).uploadEntryPhotos(user, '3', [{ filename: 'a.jpg', originalname: 'a.jpg' } as Express.Multer.File], {}))).toEqual({ status: 403, body: { error: 'Not allowed' } }); + expect(await thrownAsync(() => new JourneyController(svc(), storageStub).uploadEntryPhotos(user, '3', undefined, {}))).toEqual({ status: 400, body: { error: 'No files uploaded' } }); + expect(await thrownAsync(() => new JourneyController(svc({ addPhoto: vi.fn().mockReturnValue(null) } as Partial), storageStub).uploadEntryPhotos(user, '3', [{ filename: 'a.jpg', originalname: 'a.jpg' } as Express.Multer.File], {}))).toEqual({ status: 403, body: { error: 'Not allowed' } }); // opted in but the immich upload throws → best-effort, the local photo still wins const setPhotoProvider = vi.fn(); const blowsUp = svc({ addPhoto: vi.fn().mockReturnValue({ id: 8 }), immichAutoUploadEnabled: vi.fn().mockReturnValue(true), uploadToImmich: vi.fn().mockRejectedValue(new Error('immich down')), setPhotoProvider } as Partial); - expect(await new JourneyController(blowsUp).uploadEntryPhotos(user, '3', [{ filename: 'a.jpg', originalname: 'a.jpg' } as Express.Multer.File], { caption: 'c' })).toEqual({ photos: [{ id: 8 }] }); + expect(await new JourneyController(blowsUp, storageStub).uploadEntryPhotos(user, '3', [{ filename: 'a.jpg', originalname: 'a.jpg' } as Express.Multer.File], { caption: 'c' })).toEqual({ photos: [{ id: 8 }] }); expect(setPhotoProvider).not.toHaveBeenCalled(); // opted in but immich returns a falsy id → no provider stamping const noId = svc({ addPhoto: vi.fn().mockReturnValue({ id: 9 }), immichAutoUploadEnabled: vi.fn().mockReturnValue(true), uploadToImmich: vi.fn().mockResolvedValue(''), setPhotoProvider } as Partial); - expect(await new JourneyController(noId).uploadEntryPhotos(user, '3', [{ filename: 'a.jpg', originalname: 'a.jpg' } as Express.Multer.File], {})).toEqual({ photos: [{ id: 9 }] }); + expect(await new JourneyController(noId, storageStub).uploadEntryPhotos(user, '3', [{ filename: 'a.jpg', originalname: 'a.jpg' } as Express.Multer.File], {})).toEqual({ photos: [{ id: 9 }] }); }); it('provider-photos batch passes the passphrase through when present', () => { const addProviderPhoto = vi.fn().mockReturnValue({ id: 1 }); - new JourneyController(svc({ addProviderPhoto } as Partial)).providerPhotos(user, '3', { provider: 'immich', asset_ids: ['a'], caption: 'cap', passphrase: 'secret' }); + new JourneyController(svc({ addProviderPhoto } as Partial), storageStub).providerPhotos(user, '3', { provider: 'immich', asset_ids: ['a'], caption: 'cap', passphrase: 'secret' }); expect(addProviderPhoto).toHaveBeenCalledWith(3, 1, 'immich', 'a', 'cap', 'secret', 'image'); // single-photo success path - expect(new JourneyController(svc({ addProviderPhoto: vi.fn().mockReturnValue({ id: 2 }) } as Partial)).providerPhotos(user, '3', { provider: 'immich', asset_id: 'a' })).toEqual({ id: 2 }); + expect(new JourneyController(svc({ addProviderPhoto: vi.fn().mockReturnValue({ id: 2 }) } as Partial), storageStub).providerPhotos(user, '3', { provider: 'immich', asset_id: 'a' })).toEqual({ id: 2 }); }); it('PATCH photos: 404 then returns the updated photo', () => { - expect(thrown(() => new JourneyController(svc({ updatePhoto: vi.fn().mockReturnValue(null) } as Partial)).updatePhoto(user, '7', { caption: 'x' }))).toEqual({ status: 404, body: { error: 'Photo not found' } }); - expect(new JourneyController(svc({ updatePhoto: vi.fn().mockReturnValue({ id: 7 }) } as Partial)).updatePhoto(user, '7', { caption: 'x' })).toEqual({ id: 7 }); + expect(thrown(() => new JourneyController(svc({ updatePhoto: vi.fn().mockReturnValue(null) } as Partial), storageStub).updatePhoto(user, '7', { caption: 'x' }))).toEqual({ status: 404, body: { error: 'Photo not found' } }); + expect(new JourneyController(svc({ updatePhoto: vi.fn().mockReturnValue({ id: 7 }) } as Partial), storageStub).updatePhoto(user, '7', { caption: 'x' })).toEqual({ id: 7 }); }); it('DELETE photo unlinks the file when a path exists', () => { const unlinkSpy = vi.spyOn(fs, 'unlinkSync').mockImplementation(() => undefined); try { - expect(new JourneyController(svc({ deletePhoto: vi.fn().mockReturnValue({ id: 7, file_path: 'journey/a.jpg' }) } as Partial)).deletePhoto(user, '7')).toEqual({ success: true }); + expect(new JourneyController(svc({ deletePhoto: vi.fn().mockReturnValue({ id: 7, file_path: 'journey/a.jpg' }) } as Partial), storageStub).deletePhoto(user, '7')).toEqual({ success: true }); expect(unlinkSpy).toHaveBeenCalledTimes(1); // a vanished file is swallowed unlinkSpy.mockImplementationOnce(() => { throw new Error('ENOENT'); }); - expect(new JourneyController(svc({ deletePhoto: vi.fn().mockReturnValue({ id: 8, file_path: 'journey/b.jpg' }) } as Partial)).deletePhoto(user, '8')).toEqual({ success: true }); + expect(new JourneyController(svc({ deletePhoto: vi.fn().mockReturnValue({ id: 8, file_path: 'journey/b.jpg' }) } as Partial), storageStub).deletePhoto(user, '8')).toEqual({ success: true }); } finally { unlinkSpy.mockRestore(); } @@ -216,68 +225,69 @@ describe('JourneyController', () => { it('gallery provider-photos: batch (with passphrase), single 400/403, success', () => { const addProviderPhotoToGallery = vi.fn().mockReturnValue({ id: 1 }); - const batch = new JourneyController(svc({ addProviderPhotoToGallery } as Partial)); + const batch = new JourneyController(svc({ addProviderPhotoToGallery } as Partial), storageStub); expect(batch.galleryProviderPhotos(user, '9', { provider: 'immich', asset_ids: ['a', 'b'], passphrase: 'pw' })).toEqual({ photos: [{ id: 1 }, { id: 1 }], added: 2 }); expect(addProviderPhotoToGallery).toHaveBeenCalledWith(9, 1, 'immich', 'a', undefined, 'pw', 'image'); - expect(thrown(() => new JourneyController(svc()).galleryProviderPhotos(user, '9', { provider: 'immich' }))).toEqual({ status: 400, body: { error: 'provider and asset_id required' } }); - expect(thrown(() => new JourneyController(svc({ addProviderPhotoToGallery: vi.fn().mockReturnValue(null) } as Partial)).galleryProviderPhotos(user, '9', { provider: 'immich', asset_id: 'a' }))).toEqual({ status: 403, body: { error: 'Not allowed or duplicate' } }); - expect(new JourneyController(svc({ addProviderPhotoToGallery: vi.fn().mockReturnValue({ id: 3 }) } as Partial)).galleryProviderPhotos(user, '9', { provider: 'immich', asset_id: 'a' })).toEqual({ id: 3 }); + expect(thrown(() => new JourneyController(svc(), storageStub).galleryProviderPhotos(user, '9', { provider: 'immich' }))).toEqual({ status: 400, body: { error: 'provider and asset_id required' } }); + expect(thrown(() => new JourneyController(svc({ addProviderPhotoToGallery: vi.fn().mockReturnValue(null) } as Partial), storageStub).galleryProviderPhotos(user, '9', { provider: 'immich', asset_id: 'a' }))).toEqual({ status: 403, body: { error: 'Not allowed or duplicate' } }); + expect(new JourneyController(svc({ addProviderPhotoToGallery: vi.fn().mockReturnValue({ id: 3 }) } as Partial), storageStub).galleryProviderPhotos(user, '9', { provider: 'immich', asset_id: 'a' })).toEqual({ id: 3 }); }); it('DELETE gallery photo: 404, then unlinks the file when present', () => { - expect(thrown(() => new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue(null) } as Partial)).deleteGalleryPhoto(user, '7'))).toEqual({ status: 404, body: { error: 'Photo not found or not allowed' } }); + expect(thrown(() => new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue(null) } as Partial), storageStub).deleteGalleryPhoto(user, '7'))).toEqual({ status: 404, body: { error: 'Photo not found or not allowed' } }); // no file_path → nothing to unlink, returns void - expect(new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue({ id: 7, file_path: null }) } as Partial)).deleteGalleryPhoto(user, '7')).toBeUndefined(); + expect(new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue({ id: 7, file_path: null }) } as Partial), storageStub).deleteGalleryPhoto(user, '7')).toBeUndefined(); const unlinkSpy = vi.spyOn(fs, 'unlinkSync').mockImplementation(() => undefined); try { - new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue({ id: 8, file_path: 'journey/g.jpg' }) } as Partial)).deleteGalleryPhoto(user, '8'); + new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue({ id: 8, file_path: 'journey/g.jpg' }) } as Partial), storageStub).deleteGalleryPhoto(user, '8'); expect(unlinkSpy).toHaveBeenCalledTimes(1); unlinkSpy.mockImplementationOnce(() => { throw new Error('ENOENT'); }); - expect(new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue({ id: 9, file_path: 'journey/h.jpg' }) } as Partial)).deleteGalleryPhoto(user, '9')).toBeUndefined(); + expect(new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue({ id: 9, file_path: 'journey/h.jpg' }) } as Partial), storageStub).deleteGalleryPhoto(user, '9')).toBeUndefined(); } finally { unlinkSpy.mockRestore(); } }); it('PATCH /:id returns the updated journey on success', () => { - expect(new JourneyController(svc({ updateJourney: vi.fn().mockReturnValue({ id: 9 }) } as Partial)).update(user, '9', { title: 'x' })).toEqual({ id: 9 }); + expect(new JourneyController(svc({ updateJourney: vi.fn().mockReturnValue({ id: 9 }) } as Partial), storageStub).update(user, '9', { title: 'x' })).toEqual({ id: 9 }); }); - it('cover upload: 400 without file, 404 when the journey is gone, else returns the journey', () => { - expect(thrown(() => new JourneyController(svc()).cover(user, '9', undefined))).toEqual({ status: 400, body: { error: 'No file uploaded' } }); - expect(thrown(() => new JourneyController(svc({ updateJourney: vi.fn().mockReturnValue(null) } as Partial)).cover(user, '9', { filename: 'c.jpg' } as Express.Multer.File))).toEqual({ status: 404, body: { error: 'Journey not found' } }); + it('cover upload: 400 without file, 404 when the journey is gone, else commits + returns the journey', async () => { + expect(await thrownAsync(() => new JourneyController(svc(), storageStub).cover(user, '9', undefined))).toEqual({ status: 400, body: { error: 'No file uploaded' } }); + expect(await thrownAsync(() => new JourneyController(svc({ updateJourney: vi.fn().mockReturnValue(null) } as Partial), storageStub).cover(user, '9', { filename: 'c.jpg' } as Express.Multer.File))).toEqual({ status: 404, body: { error: 'Journey not found' } }); const updateJourney = vi.fn().mockReturnValue({ id: 9, cover_image: 'journey/c.jpg' }); - expect(new JourneyController(svc({ updateJourney } as Partial)).cover(user, '9', { filename: 'c.jpg' } as Express.Multer.File)).toEqual({ id: 9, cover_image: 'journey/c.jpg' }); + expect(await new JourneyController(svc({ updateJourney } as Partial), storageStub).cover(user, '9', { filename: 'c.jpg' } as Express.Multer.File)).toEqual({ id: 9, cover_image: 'journey/c.jpg' }); + expect(storageStub.put).toHaveBeenCalledWith('journey', 'c.jpg', { tmpPath: undefined }); expect(updateJourney).toHaveBeenCalledWith(9, 1, { cover_image: 'journey/c.jpg' }); }); it('DELETE /:id and trips/contributors success paths', () => { - expect(new JourneyController(svc({ deleteJourney: vi.fn().mockReturnValue(true) } as Partial)).remove(user, '9')).toEqual({ success: true }); - expect(new JourneyController(svc({ removeTripFromJourney: vi.fn().mockReturnValue(true) } as Partial)).removeTrip(user, '9', '2')).toEqual({ success: true }); - expect(new JourneyController(svc({ updateContributorRole: vi.fn().mockReturnValue(true) } as Partial)).updateContributor(user, '9', '2', { role: 'editor' })).toEqual({ success: true }); - expect(new JourneyController(svc({ removeContributor: vi.fn().mockReturnValue(true) } as Partial)).removeContributor(user, '9', '2')).toEqual({ success: true }); + expect(new JourneyController(svc({ deleteJourney: vi.fn().mockReturnValue(true) } as Partial), storageStub).remove(user, '9')).toEqual({ success: true }); + expect(new JourneyController(svc({ removeTripFromJourney: vi.fn().mockReturnValue(true) } as Partial), storageStub).removeTrip(user, '9', '2')).toEqual({ success: true }); + expect(new JourneyController(svc({ updateContributorRole: vi.fn().mockReturnValue(true) } as Partial), storageStub).updateContributor(user, '9', '2', { role: 'editor' })).toEqual({ success: true }); + expect(new JourneyController(svc({ removeContributor: vi.fn().mockReturnValue(true) } as Partial), storageStub).removeContributor(user, '9', '2')).toEqual({ success: true }); }); it('addContributor defaults the role to viewer when omitted', () => { const addContributor = vi.fn().mockReturnValue(true); - new JourneyController(svc({ addContributor } as Partial)).addContributor(user, '9', { user_id: 2 }); + new JourneyController(svc({ addContributor } as Partial), storageStub).addContributor(user, '9', { user_id: 2 }); expect(addContributor).toHaveBeenCalledWith(9, 1, 2, 'viewer'); }); it('createEntry returns the entry when the journey exists', () => { - expect(new JourneyController(svc({ createEntry: vi.fn().mockReturnValue({ id: 4 }) } as Partial)).createEntry(user, '9', { entry_date: '2026-01-01' })).toEqual({ id: 4 }); + expect(new JourneyController(svc({ createEntry: vi.fn().mockReturnValue({ id: 4 }) } as Partial), storageStub).createEntry(user, '9', { entry_date: '2026-01-01' })).toEqual({ id: 4 }); }); it('reorderEntries succeeds for a numeric array', () => { - expect(new JourneyController(svc({ reorderEntries: vi.fn().mockReturnValue(true) } as Partial)).reorderEntries(user, '9', { orderedIds: [3, 1, 2] })).toEqual({ success: true }); + expect(new JourneyController(svc({ reorderEntries: vi.fn().mockReturnValue(true) } as Partial), storageStub).reorderEntries(user, '9', { orderedIds: [3, 1, 2] })).toEqual({ success: true }); }); it('preferences returns the result on success', () => { - expect(new JourneyController(svc({ updateJourneyPreferences: vi.fn().mockReturnValue({ ok: true }) } as Partial)).preferences(user, '9', { theme: 'dark' })).toEqual({ ok: true }); + expect(new JourneyController(svc({ updateJourneyPreferences: vi.fn().mockReturnValue({ ok: true }) } as Partial), storageStub).preferences(user, '9', { theme: 'dark' })).toEqual({ ok: true }); }); it('deleteShareLink returns success when removed', () => { - expect(new JourneyController(svc({ deleteJourneyShareLink: vi.fn().mockReturnValue(true) } as Partial)).deleteShareLink(user, '9')).toEqual({ success: true }); + expect(new JourneyController(svc({ deleteJourneyShareLink: vi.fn().mockReturnValue(true) } as Partial), storageStub).deleteShareLink(user, '9')).toEqual({ success: true }); }); });