diff --git a/server/src/nest/journey/journey-domain.service.ts b/server/src/nest/journey/journey-domain.service.ts index 5658ca2b1..dce34e1d4 100644 --- a/server/src/nest/journey/journey-domain.service.ts +++ b/server/src/nest/journey/journey-domain.service.ts @@ -1234,22 +1234,22 @@ export class JourneyDomainService { deleteGalleryPhoto( journeyPhotoId: number, userId: number, - ): { photo_id: number; file_path?: string | null } | null { + ): { photo_id: number; file_path?: string | null; thumbnail_path?: string | null } | null { const row = this.db.prepare('SELECT * FROM journey_photos WHERE id = ?').get(journeyPhotoId) as | { id: number; journey_id: number; photo_id: number } | undefined; if (!row) return null; if (!this.canEdit(row.journey_id, userId)) return null; - const trekRow = this.db.prepare('SELECT file_path, provider FROM trek_photos WHERE id = ?').get(row.photo_id) as - | { file_path?: string; provider?: string } + const trekRow = this.db.prepare('SELECT file_path, thumbnail_path, provider FROM trek_photos WHERE id = ?').get(row.photo_id) as + | { file_path?: string; thumbnail_path?: string; provider?: string } | undefined; // cascade on journey_entry_photos.journey_photo_id handles junction cleanup this.db.prepare('DELETE FROM journey_photos WHERE id = ?').run(journeyPhotoId); this.photos.deleteIfOrphan(row.photo_id); - return { photo_id: row.photo_id, file_path: trekRow?.file_path ?? null }; + return { photo_id: row.photo_id, file_path: trekRow?.file_path ?? null, thumbnail_path: trekRow?.thumbnail_path ?? null }; } setPhotoProvider(photoId: number, provider: string, assetId: string, ownerId: number) { @@ -1299,21 +1299,21 @@ export class JourneyDomainService { deletePhoto( photoId: number, userId: number, - ): { id: number; photo_id: number; file_path?: string | null; journey_id: number } | null { + ): { id: number; photo_id: number; file_path?: string | null; thumbnail_path?: string | null; journey_id: number } | null { const row = this.db.prepare('SELECT id, journey_id, photo_id FROM journey_photos WHERE id = ?').get(photoId) as | { id: number; journey_id: number; photo_id: number } | undefined; if (!row) return null; if (!this.canEdit(row.journey_id, userId)) return null; - const trekRow = this.db.prepare('SELECT file_path, provider FROM trek_photos WHERE id = ?').get(row.photo_id) as - | { file_path?: string; provider?: string } + const trekRow = this.db.prepare('SELECT file_path, thumbnail_path, provider FROM trek_photos WHERE id = ?').get(row.photo_id) as + | { file_path?: string; thumbnail_path?: string; provider?: string } | undefined; this.db.prepare('DELETE FROM journey_photos WHERE id = ?').run(photoId); this.photos.deleteIfOrphan(row.photo_id); - return { id: row.id, photo_id: row.photo_id, file_path: trekRow?.file_path ?? null, journey_id: row.journey_id }; + return { id: row.id, photo_id: row.photo_id, file_path: trekRow?.file_path ?? null, thumbnail_path: trekRow?.thumbnail_path ?? null, journey_id: row.journey_id }; } // ── Contributors ───────────────────────────────────────────────────────── diff --git a/server/src/nest/journey/journey.controller.ts b/server/src/nest/journey/journey.controller.ts index 306fbacec..a05633bd3 100644 --- a/server/src/nest/journey/journey.controller.ts +++ b/server/src/nest/journey/journey.controller.ts @@ -23,6 +23,7 @@ import fs from 'node:fs'; import crypto from 'node:crypto'; import type { User } from '../../types'; import { StorageService } from '../storage/storage.service'; +import { journeyThumbName } from '../memories/thumbnail.service'; import { JourneyService } from './journey.service'; import { AddonGuard } from '../addons/addon.guard'; import { RequireAddon } from '../addons/require-addon.decorator'; @@ -287,6 +288,7 @@ export class JourneyController { throw new HttpException({ error: 'Photo not found' }, 404); } await this.deletePhotoObject(photo.file_path); + await this.reclaimDerived(photo.file_path, photo.thumbnail_path); return { success: true }; } @@ -302,6 +304,21 @@ export class JourneyController { await this.storage.delete('journey', name).catch(() => { /* file already gone */ }); } + /** + * Delete a photo's derived artifacts with it (spec In-scope fix #2): the + * hash-named thumbnail derivable from file_path, and whatever + * thumbnail_path recorded — a generated thumb (usually the same object) or + * a video's uploaded poster. Both best-effort, both idempotent. + */ + private async reclaimDerived(filePath: string | null | undefined, thumbnailPath: string | null | undefined): Promise { + if (filePath?.startsWith('journey/')) { + await this.storage.delete('journey', journeyThumbName(filePath)).catch(() => { /* already gone */ }); + } + if (thumbnailPath?.startsWith('journey/')) { + await this.storage.delete('journey', thumbnailPath.slice('journey/'.length)).catch(() => { /* already gone */ }); + } + } + // ── Gallery (prefix /:id/gallery — before /:id) ───────────────────────── @Post(':id/gallery/photos') @UseInterceptors(FilesInterceptor('photos')) @@ -393,6 +410,7 @@ export class JourneyController { throw new HttpException({ error: 'Photo not found or not allowed' }, 404); } await this.deletePhotoObject(photo.file_path); + await this.reclaimDerived(photo.file_path, photo.thumbnail_path); } // ── Journeys /:id ─────────────────────────────────────────────────────── diff --git a/server/src/nest/memories/journey-thumbs.job.ts b/server/src/nest/memories/journey-thumbs.job.ts new file mode 100644 index 000000000..c354facda --- /dev/null +++ b/server/src/nest/memories/journey-thumbs.job.ts @@ -0,0 +1,37 @@ +import { Injectable, type OnApplicationBootstrap } from '@nestjs/common'; +import { logInfo, logError } from '../audit/audit-log.logger'; +import { CronRegistrarService } from '../scheduling/cron-registrar.service'; +import { ThumbnailService } from './thumbnail.service'; + +/** + * Journey thumbnail cleanup: daily — reclaim journey/thumbs/ objects whose + * photo row is gone (spec In-scope fix #2: delete sites only started removing + * the derived thumb with this slice, so historical orphans need a sweeper). + * The Journey addon gate lives inside sweepOrphanThumbs, so toggling the + * addon takes effect without a restart. + */ +@Injectable() +export class JourneyThumbsJob implements OnApplicationBootstrap { + constructor( + private readonly thumbnails: ThumbnailService, + private readonly registrar: CronRegistrarService, + ) {} + + onApplicationBootstrap(): void { + if (!this.registrar.isEnabled()) return; + // Run once on startup to reclaim orphans left over from before this sweeper existed. + void this.sweep(); + this.registrar.register('journey-thumbs', '0 4 * * *', () => { + void this.sweep(); + }); + } + + async sweep(): Promise { + try { + const removed = await this.thumbnails.sweepOrphanThumbs(); + if (removed > 0) logInfo(`Journey thumbnail cleanup: removed ${removed} orphaned thumbnail(s)`); + } catch (err: unknown) { + logError(`Journey thumbnail cleanup: ${err instanceof Error ? err.message : err}`); + } + } +} diff --git a/server/src/nest/memories/memories.module.ts b/server/src/nest/memories/memories.module.ts index f7909a2f3..2ed13a6e2 100644 --- a/server/src/nest/memories/memories.module.ts +++ b/server/src/nest/memories/memories.module.ts @@ -8,6 +8,7 @@ import { PhotoResolverService } from './photo-resolver.service'; import { ThumbnailService } from './thumbnail.service'; import { TrekPhotoCacheService } from './trek-photo-cache.service'; import { TrekPhotoCacheJob } from './trek-photo-cache.job'; +import { JourneyThumbsJob } from './journey-thumbs.job'; import { SchedulingModule } from '../scheduling/scheduling.module'; import { UnifiedMemoriesController } from './unified.controller'; import { ImmichMemoriesController } from './immich.controller'; @@ -56,6 +57,7 @@ import { StorageModule } from '../storage/storage.module'; ThumbnailService, TrekPhotoCacheService, TrekPhotoCacheJob, + JourneyThumbsJob, ImmichPhotoProvider, SynologyPhotoProvider, PhotoProviderRegistry, diff --git a/server/src/nest/memories/thumbnail.service.ts b/server/src/nest/memories/thumbnail.service.ts index f32abc30d..e70443ba6 100644 --- a/server/src/nest/memories/thumbnail.service.ts +++ b/server/src/nest/memories/thumbnail.service.ts @@ -4,6 +4,7 @@ import crypto from 'node:crypto' import { Injectable } from '@nestjs/common' import { ADDON_IDS } from '../../addons' import { AddonsService } from '../addons/addons.service' +import { DatabaseService } from '../database/database.service' import { StorageService } from '../storage/storage.service' const THUMB_MAX = 800 @@ -30,6 +31,7 @@ export class ThumbnailService { constructor( private readonly addons: AddonsService, private readonly storage: StorageService, + private readonly db: DatabaseService, ) {} async ensureLocalThumbnail( @@ -75,4 +77,39 @@ export class ThumbnailService { return null } } + + /** + * Reclaim journey/thumbs/ objects no live trek_photos row can account for + * (spec In-scope fix #2). The live set is every hash derivable from a + * journey file_path plus every recorded thumbnail_path under thumbs/ — + * video posters live in journey/ proper and are reclaimed at the delete + * sites, not here. Gated on the Journey addon like the generator, inside + * the method so an admin toggle takes effect without a restart + * (airtrail-sync.job.ts precedent). + */ + async sweepOrphanThumbs(): Promise { + if (!this.addons.isAddonEnabled(ADDON_IDS.JOURNEY)) return 0 + + const live = new Set() + for (const row of this.db.all<{ file_path: string }>( + "SELECT file_path FROM trek_photos WHERE file_path LIKE 'journey/%'", + )) { + live.add(journeyThumbName(row.file_path)) + } + for (const row of this.db.all<{ thumbnail_path: string }>( + "SELECT thumbnail_path FROM trek_photos WHERE thumbnail_path LIKE 'journey/thumbs/%'", + )) { + live.add(row.thumbnail_path.slice('journey/'.length)) + } + + let removed = 0 + for await (const stat of this.storage.list('journey', 'thumbs/')) { + if (!stat.key.endsWith('.jpg') || live.has(stat.key)) continue + try { + await this.storage.delete('journey', stat.key) + removed++ + } catch { /* race */ } + } + return removed + } } diff --git a/server/tests/unit/nest/cache-sweep-jobs.test.ts b/server/tests/unit/nest/cache-sweep-jobs.test.ts index 99a3e1b37..de41f7d46 100644 --- a/server/tests/unit/nest/cache-sweep-jobs.test.ts +++ b/server/tests/unit/nest/cache-sweep-jobs.test.ts @@ -13,8 +13,10 @@ vi.mock('../../../src/nest/audit/audit-log.logger', () => logMock); import { TrekPhotoCacheJob } from '../../../src/nest/memories/trek-photo-cache.job'; import { PlacePhotoCacheJob } from '../../../src/nest/place-photos/place-photo-cache.job'; import { AirtrailSyncJob } from '../../../src/nest/integrations/airtrail-sync.job'; +import { JourneyThumbsJob } from '../../../src/nest/memories/journey-thumbs.job'; import type { TrekPhotoCacheService } from '../../../src/nest/memories/trek-photo-cache.service'; import type { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service'; +import type { ThumbnailService } from '../../../src/nest/memories/thumbnail.service'; import type { AirtrailSyncService } from '../../../src/nest/integrations/airtrail-sync.service'; import type { DatabaseService } from '../../../src/nest/database/database.service'; import type { CronRegistrarService } from '../../../src/nest/scheduling/cron-registrar.service'; @@ -99,6 +101,40 @@ describe('PlacePhotoCacheJob', () => { }); }); +describe('JourneyThumbsJob', () => { + function make(removed: number | (() => number) = 0, enabled = true) { + const registrar = registrarStub(enabled); + const thumbnails = { sweepOrphanThumbs: vi.fn(typeof removed === 'function' ? removed : () => Promise.resolve(removed)) }; + const job = new JourneyThumbsJob(thumbnails as unknown as ThumbnailService, registrar as unknown as CronRegistrarService); + return { job, registrar, thumbnails }; + } + + it('CSJOB-010 — boot sweep runs immediately, then the daily app-tz cron registers', () => { + const { job, registrar, thumbnails } = make(); + job.onApplicationBootstrap(); + expect(thumbnails.sweepOrphanThumbs).toHaveBeenCalledTimes(1); + expect(registrar.register).toHaveBeenCalledWith('journey-thumbs', '0 4 * * *', expect.any(Function)); + }); + + it('CSJOB-011 — logs only when something was removed', async () => { + await make(0).job.sweep(); + expect(logMock.logInfo).not.toHaveBeenCalled(); + await make(2).job.sweep(); + expect(logMock.logInfo).toHaveBeenCalledWith('Journey thumbnail cleanup: removed 2 orphaned thumbnail(s)'); + }); + + it('CSJOB-012 — a throwing sweep is contained to the cleanup log line, and the gate skips the boot sweep', async () => { + const { job } = make(() => { throw new Error('storage down'); }); + await job.sweep(); + expect(logMock.logError).toHaveBeenCalledWith('Journey thumbnail cleanup: storage down'); + + const gated = make(0, false); + gated.job.onApplicationBootstrap(); + expect(gated.thumbnails.sweepOrphanThumbs).not.toHaveBeenCalled(); + expect(gated.registrar.register).not.toHaveBeenCalled(); + }); +}); + describe('AirtrailSyncJob', () => { function make(intervalSetting: string | undefined, enabled = true) { const registrar = registrarStub(enabled); diff --git a/server/tests/unit/nest/journey.controller.test.ts b/server/tests/unit/nest/journey.controller.test.ts index 0d93583f0..65e6191f0 100644 --- a/server/tests/unit/nest/journey.controller.test.ts +++ b/server/tests/unit/nest/journey.controller.test.ts @@ -3,6 +3,7 @@ import { HttpException } from '@nestjs/common'; import type { Response } from 'express'; import { JourneyController } from '../../../src/nest/journey/journey.controller'; +import { journeyThumbName } from '../../../src/nest/memories/thumbnail.service'; 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'; @@ -215,6 +216,8 @@ describe('JourneyController', () => { it('DELETE photo removes the storage object when a path exists', async () => { expect(await new JourneyController(svc({ deletePhoto: vi.fn().mockReturnValue({ id: 7, file_path: 'journey/a.jpg' }) } as Partial), storageStub).deletePhoto(user, '7')).toEqual({ success: true }); expect(storageStub.delete).toHaveBeenCalledWith('journey', 'a.jpg'); + // ...and the derived thumb goes with it (spec fix #2). + expect(storageStub.delete).toHaveBeenCalledWith('journey', journeyThumbName('journey/a.jpg')); // a vanished object is swallowed (storageStub.delete as ReturnType).mockRejectedValueOnce(new Error('ENOENT')); expect(await new JourneyController(svc({ deletePhoto: vi.fn().mockReturnValue({ id: 8, file_path: 'journey/b.jpg' }) } as Partial), storageStub).deletePhoto(user, '8')).toEqual({ success: true }); @@ -240,10 +243,22 @@ describe('JourneyController', () => { expect(await new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue({ id: 7, file_path: null }) } as Partial), storageStub).deleteGalleryPhoto(user, '7')).toBeUndefined(); await new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue({ id: 8, file_path: 'journey/g.jpg' }) } as Partial), storageStub).deleteGalleryPhoto(user, '8'); expect(storageStub.delete).toHaveBeenCalledWith('journey', 'g.jpg'); + expect(storageStub.delete).toHaveBeenCalledWith('journey', journeyThumbName('journey/g.jpg')); (storageStub.delete as ReturnType).mockRejectedValueOnce(new Error('ENOENT')); expect(await new JourneyController(svc({ deleteGalleryPhoto: vi.fn().mockReturnValue({ id: 9, file_path: 'journey/h.jpg' }) } as Partial), storageStub).deleteGalleryPhoto(user, '9')).toBeUndefined(); }); + it('DELETE gallery video also reclaims its uploaded poster (spec fix #2 extension)', async () => { + (storageStub.delete as ReturnType).mockClear(); + await new JourneyController( + svc({ deleteGalleryPhoto: vi.fn().mockReturnValue({ id: 10, file_path: 'journey/clip.mp4', thumbnail_path: 'journey/poster.jpg' }) } as Partial), + storageStub, + ).deleteGalleryPhoto(user, '10'); + expect(storageStub.delete).toHaveBeenCalledWith('journey', 'clip.mp4'); + expect(storageStub.delete).toHaveBeenCalledWith('journey', journeyThumbName('journey/clip.mp4')); + expect(storageStub.delete).toHaveBeenCalledWith('journey', 'poster.jpg'); + }); + it('PATCH /:id returns the updated journey on success', () => { expect(new JourneyController(svc({ updateJourney: vi.fn().mockReturnValue({ id: 9 }) } as Partial), storageStub).update(user, '9', { title: 'x' })).toEqual({ id: 9 }); }); diff --git a/server/tests/unit/nest/thumbnail.service.test.ts b/server/tests/unit/nest/thumbnail.service.test.ts index 303ac7e82..4cb2aeb6b 100644 --- a/server/tests/unit/nest/thumbnail.service.test.ts +++ b/server/tests/unit/nest/thumbnail.service.test.ts @@ -13,15 +13,20 @@ const { isAddonEnabled } = vi.hoisted(() => ({ isAddonEnabled: vi.fn(() => true) import fs from 'node:fs'; import path from 'node:path'; import { Jimp } from 'jimp'; -import { ThumbnailService } from '../../../src/nest/memories/thumbnail.service'; +import { ThumbnailService, journeyThumbName } from '../../../src/nest/memories/thumbnail.service'; import type { AddonsService } from '../../../src/nest/addons/addons.service'; +import Database from 'better-sqlite3'; +import { DatabaseService } from '../../../src/nest/database/database.service'; import { makeStorageFixture } from '../../helpers/storage-fixture'; // Category-addressed since slice 4: originals + thumbs are ('journey', ) // objects; the fixture's 'journey/' prefix reproduces the real layout, so the // on-disk paths below look exactly like the old uploads-root ones. const fx = makeStorageFixture('journey/'); -const svc = new ThumbnailService({ isAddonEnabled } as unknown as AddonsService, fx.storage); +// Minimal real DB — the orphan sweep only SELECTs these two columns. +const thumbsDb = new Database(':memory:'); +thumbsDb.exec('CREATE TABLE trek_photos (id INTEGER PRIMARY KEY AUTOINCREMENT, file_path TEXT, thumbnail_path TEXT)'); +const svc = new ThumbnailService({ isAddonEnabled } as unknown as AddonsService, fx.storage, new DatabaseService(thumbsDb as never)); const root = fx.root; /** A real 1200x900 JPEG — Jimp has to be able to decode it for the happy path. */ @@ -38,6 +43,7 @@ beforeEach(() => { }); afterAll(() => { + thumbsDb.close(); fx.cleanup(); }); @@ -110,3 +116,59 @@ describe('ensureLocalThumbnail', () => { expect(await svc.ensureLocalThumbnail('journey/corrupt.jpg')).toBeNull(); }); }); + +describe('sweepOrphanThumbs (spec fix #2)', () => { + const thumbsDir = path.join(root, 'journey', 'thumbs'); + const writeThumb = (name: string): string => { + fs.mkdirSync(thumbsDir, { recursive: true }); + const fp = path.join(thumbsDir, name); + fs.writeFileSync(fp, 'jpeg'); + return fp; + }; + + beforeEach(() => { + thumbsDb.prepare('DELETE FROM trek_photos').run(); + fs.rmSync(thumbsDir, { recursive: true, force: true }); + }); + + it('THUMB-SWEEP-001: deletes strays, spares thumbs derivable from live journey rows', async () => { + thumbsDb.prepare('INSERT INTO trek_photos (file_path) VALUES (?)').run('journey/live.jpg'); + const liveName = journeyThumbName('journey/live.jpg'); // 'thumbs/.jpg' + const livePath = writeThumb(path.basename(liveName)); + const strayPath = writeThumb('deadbeefdeadbeef.jpg'); + + const removed = await svc.sweepOrphanThumbs(); + + expect(fs.existsSync(livePath)).toBe(true); + expect(fs.existsSync(strayPath)).toBe(false); + expect(removed).toBe(1); + }); + + it('THUMB-SWEEP-002: spares a recorded thumbnail_path even without a matching file_path hash', async () => { + thumbsDb.prepare('INSERT INTO trek_photos (file_path, thumbnail_path) VALUES (?, ?)') + .run('elsewhere/x.jpg', 'journey/thumbs/recorded00000000.jpg'); + const recorded = writeThumb('recorded00000000.jpg'); + + const removed = await svc.sweepOrphanThumbs(); + + expect(fs.existsSync(recorded)).toBe(true); + expect(removed).toBe(0); + }); + + it('THUMB-SWEEP-003: the addon gate answers 0 without listing or deleting', async () => { + isAddonEnabled.mockReturnValue(false); + const stray = writeThumb('gatedstray000000.jpg'); + + expect(await svc.sweepOrphanThumbs()).toBe(0); + expect(fs.existsSync(stray)).toBe(true); + }); + + it('THUMB-SWEEP-004: non-jpg entries under thumbs/ are never touched', async () => { + fs.mkdirSync(thumbsDir, { recursive: true }); + const note = path.join(thumbsDir, 'note.txt'); + fs.writeFileSync(note, 'not a thumb'); + + expect(await svc.sweepOrphanThumbs()).toBe(0); + expect(fs.existsSync(note)).toBe(true); + }); +});