diff --git a/server/src/nest/memories/memories.module.ts b/server/src/nest/memories/memories.module.ts index 3e7e39d8f..f7909a2f3 100644 --- a/server/src/nest/memories/memories.module.ts +++ b/server/src/nest/memories/memories.module.ts @@ -21,6 +21,7 @@ import { PhotoProviderRegistry } from './photo-provider.registry'; import { ImmichPhotoProvider } from './providers/immich.provider'; import { SynologyPhotoProvider } from './providers/synology.provider'; import { NotificationsModule } from '../notifications/notifications.module'; +import { StorageModule } from '../storage/storage.module'; /** * Memories (photo-providers) domain — mounted at /api/integrations/memories. @@ -43,7 +44,7 @@ import { NotificationsModule } from '../notifications/notifications.module'; * adapters themselves so the set is readable in one place. */ @Module({ - imports: [NotificationsModule, AddonsModule, AuditModule, TrekPhotosModule, RealtimeModule, SchedulingModule], + imports: [NotificationsModule, AddonsModule, AuditModule, TrekPhotosModule, RealtimeModule, SchedulingModule, StorageModule], controllers: [UnifiedMemoriesController, ImmichMemoriesController, SynologyMemoriesController], providers: [ MemoriesService, diff --git a/server/src/nest/memories/photo-resolver.service.ts b/server/src/nest/memories/photo-resolver.service.ts index bd3828c90..c1d039f39 100644 --- a/server/src/nest/memories/photo-resolver.service.ts +++ b/server/src/nest/memories/photo-resolver.service.ts @@ -1,7 +1,5 @@ import { Injectable } from '@nestjs/common'; import { Response } from 'express'; -import path from 'path'; -import fs from 'fs'; import type { TrekPhoto } from '../../types'; import { decrypt_api_key } from '../common/crypto/apiKeyCrypto'; import { TrekPhotosRepository } from '../photos/trek-photos.repository'; @@ -11,6 +9,7 @@ import { TrekPhotoCacheService } from './trek-photo-cache.service'; import { fail, success, type AssetInfo, type ServiceResult } from './memories.helpers'; import { PhotoProviderRegistry } from './photo-provider.registry'; import type { PhotoAssetRef } from './photo-provider'; +import { StorageService } from '../storage/storage.service'; /** * Resolves a stored trek_photo to bytes or metadata by asking whichever provider @@ -29,8 +28,16 @@ export class PhotoResolverService { private readonly thumbnails: ThumbnailService, private readonly cache: TrekPhotoCacheService, private readonly providers: PhotoProviderRegistry, + private readonly storage: StorageService, ) {} + /** photos.file_path / thumbnail_path are uploads-relative 'journey/…' by + * every writer; anything else reads as a local miss, matching the old + * existsSync(join(uploadsRoot, file_path)) === false path. */ + private storageName(relPath: string): string | null { + return relPath.startsWith('journey/') ? relPath.slice('journey/'.length) : null; + } + // ── Streaming ──────────────────────────────────────────────────────────── private async streamCachedThumbnail( @@ -41,12 +48,12 @@ export class PhotoResolverService { ): Promise { const key = this.cache.cacheKey(photo.provider!, photo.asset_id!, 'thumbnail', photo.owner_id!); - if (this.cache.serveFresh(res, key)) return; + if (await this.cache.serveFresh(res, key)) return; const existing = this.cache.getInFlight(key); if (existing) { const bytes = await existing; - if (bytes && this.cache.serveFresh(res, key)) return; + if (bytes && (await this.cache.serveFresh(res, key))) return; await fallback(); return; } @@ -59,7 +66,7 @@ export class PhotoResolverService { this.cache.setInFlight(key, promise); const bytes = await promise; - if (bytes && this.cache.serveFresh(res, key)) return; + if (bytes && (await this.cache.serveFresh(res, key))) return; await fallback(); } @@ -77,26 +84,26 @@ export class PhotoResolverService { } if (photo.file_path) { - const uploadsRoot = UPLOADS_ROOT; - if (kind === 'thumbnail') { const isVideo = photo.media_type === 'video'; let thumbRel = photo.thumbnail_path ?? null; // Only raster images get a lazily-generated Jimp thumbnail; Jimp can't decode // video, so a video relies on the poster captured at upload (#823). if (!thumbRel && !isVideo) { - const result = await this.thumbnails.ensureLocalThumbnail(uploadsRoot, photo.file_path); + // Generation stays root-addressed until ThumbnailService becomes + // category-addressed in slice 4; serving below is already storage-backed. + const result = await this.thumbnails.ensureLocalThumbnail(UPLOADS_ROOT, photo.file_path); if (result) { thumbRel = result.thumbnailRelPath; this.photos.recordLocalThumbnail(photo.id, thumbRel, result.width, result.height); } } if (thumbRel) { - const thumbAbs = path.join(uploadsRoot, thumbRel); - if (fs.existsSync(thumbAbs)) { + const thumbName = this.storageName(thumbRel); + if (thumbName && (await this.storage.exists('journey', thumbName).catch(() => false))) { res.set('Cache-Control', 'public, max-age=86400, immutable'); res.set('X-Content-Type-Options', 'nosniff'); - res.sendFile(thumbAbs); + await this.storage.sendToResponse('journey', thumbName, res); return; } } @@ -109,11 +116,11 @@ export class PhotoResolverService { // Images fall through to original if the thumbnail is unavailable. } - const localPath = path.join(uploadsRoot, photo.file_path); - if (fs.existsSync(localPath)) { + const name = this.storageName(photo.file_path); + if (name && (await this.storage.exists('journey', name).catch(() => false))) { res.set('Cache-Control', 'public, max-age=86400'); res.set('X-Content-Type-Options', 'nosniff'); - res.sendFile(localPath); + await this.storage.sendToResponse('journey', name, res); return; } } diff --git a/server/src/nest/memories/trek-photo-cache.service.ts b/server/src/nest/memories/trek-photo-cache.service.ts index d0c264a5b..57fbc3312 100644 --- a/server/src/nest/memories/trek-photo-cache.service.ts +++ b/server/src/nest/memories/trek-photo-cache.service.ts @@ -5,6 +5,8 @@ import crypto from 'node:crypto'; import { Injectable } from '@nestjs/common'; import { Response } from 'express'; import { DatabaseService } from '../database/database.service'; +import { StorageService } from '../storage/storage.service'; +import { StorageNotFoundError } from '../storage/storage.types'; import { UPLOADS_ROOT } from './uploads-root'; const TREK_PHOTO_DIR = path.join(UPLOADS_ROOT, 'photos/trek'); @@ -36,7 +38,10 @@ function cachedFilePath(key: string): string { /** Disk + metadata cache for provider thumbnails and originals. */ @Injectable() export class TrekPhotoCacheService { - constructor(private readonly db: DatabaseService) {} + constructor( + private readonly db: DatabaseService, + private readonly storage: StorageService, + ) {} cacheKey(provider: string, assetId: string, kind: string, ownerId: number): string { return crypto.createHash('sha1').update(`${provider}:${assetId}:${kind}:${ownerId}`).digest('hex'); @@ -79,13 +84,21 @@ export class TrekPhotoCacheService { ); } - serveFresh(res: Response, key: string): boolean { + async serveFresh(res: Response, key: string): Promise { const entry = this.getFresh(key); if (!entry) return false; res.set('Content-Type', entry.contentType); res.set('Cache-Control', 'public, max-age=3600'); - res.sendFile(entry.filePath); + try { + // send() keeps a pre-set Content-Type, so entry.contentType survives the + // .bin extension. + await this.storage.sendToResponse('photos-trek', `${key}.bin`, res); + } catch (err) { + // getFresh→send delete race: fall back like a cache miss. + if (err instanceof StorageNotFoundError && !res.headersSent) return false; + throw err; + } return true; } diff --git a/server/tests/unit/nest/photo-resolver.service.test.ts b/server/tests/unit/nest/photo-resolver.service.test.ts index 9b443c413..1a659b81c 100644 --- a/server/tests/unit/nest/photo-resolver.service.test.ts +++ b/server/tests/unit/nest/photo-resolver.service.test.ts @@ -63,6 +63,9 @@ const cache = { setInFlight: vi.fn(), put: vi.fn(), }; +// Local bytes go through the storage facade (slice 3); the resolver only ever +// addresses the 'journey' category with the 'journey/' prefix stripped. +const storage = { exists: vi.fn(), sendToResponse: vi.fn() }; const dbs = new DatabaseService(testDb); const repo = new TrekPhotosRepository(dbs); @@ -80,6 +83,7 @@ const svc = new PhotoResolverService( thumbnails as unknown as ThumbnailService, cache as unknown as TrekPhotoCacheService, providers, + storage as unknown as import('../../../src/nest/storage/storage.service').StorageService, ); function makeRes() { @@ -111,6 +115,8 @@ beforeEach(() => { cache.serveFresh.mockReturnValue(false); cache.getInFlight.mockReturnValue(undefined); cache.cacheKey.mockReturnValue('k'); + storage.exists.mockResolvedValue(false); + storage.sendToResponse.mockResolvedValue(undefined); }); afterAll(() => testDb.close()); @@ -168,6 +174,64 @@ describe('streamPhoto — dispatch', () => { expect(row).toEqual({ thumbnail_path: 'journey/thumbs/abc.jpg', width: 800, height: 600 }); }); + it('RESOLVE-014: a local thumbnail hit streams through storage with the immutable headers', async () => { + const id = insertPhoto({ provider: 'local', file_path: 'journey/x.jpg', thumbnail_path: 'journey/thumbs/h.jpg' }); + storage.exists.mockResolvedValue(true); + const res = makeRes(); + + await svc.streamPhoto(res as never, 1, id, 'thumbnail'); + + expect(storage.exists).toHaveBeenCalledWith('journey', 'thumbs/h.jpg'); + expect(res.set).toHaveBeenCalledWith('Cache-Control', 'public, max-age=86400, immutable'); + expect(res.set).toHaveBeenCalledWith('X-Content-Type-Options', 'nosniff'); + expect(storage.sendToResponse).toHaveBeenCalledWith('journey', 'thumbs/h.jpg', res); + }); + + it('RESOLVE-015: a local original hit streams through storage with the day-long cache header', async () => { + const id = insertPhoto({ provider: 'local', file_path: 'journey/x.jpg' }); + storage.exists.mockResolvedValue(true); + const res = makeRes(); + + await svc.streamPhoto(res as never, 1, id, 'original'); + + expect(res.set).toHaveBeenCalledWith('Cache-Control', 'public, max-age=86400'); + expect(res.set).toHaveBeenCalledWith('X-Content-Type-Options', 'nosniff'); + expect(storage.sendToResponse).toHaveBeenCalledWith('journey', 'x.jpg', res); + }); + + it('RESOLVE-016: a missing thumbnail falls through to the original for images', async () => { + const id = insertPhoto({ provider: 'local', file_path: 'journey/x.jpg', thumbnail_path: 'journey/thumbs/h.jpg' }); + storage.exists.mockResolvedValueOnce(false).mockResolvedValueOnce(true); + const res = makeRes(); + + await svc.streamPhoto(res as never, 1, id, 'thumbnail'); + + expect(storage.sendToResponse).toHaveBeenCalledWith('journey', 'x.jpg', res); + }); + + it('RESOLVE-017: a video whose recorded poster is gone 404s instead of falling through (#823)', async () => { + const id = insertPhoto({ provider: 'local', file_path: 'journey/clip.mp4', thumbnail_path: 'journey/thumbs/t.jpg', media_type: 'video' }); + storage.exists.mockResolvedValue(false); + const res = makeRes(); + + await svc.streamPhoto(res as never, 1, id, 'thumbnail'); + + expect(res.status).toHaveBeenCalledWith(404); + expect(res.json).toHaveBeenCalledWith({ error: 'No poster available' }); + expect(storage.sendToResponse).not.toHaveBeenCalled(); + }); + + it('RESOLVE-018: a file_path outside journey/ reads as a local miss without touching storage', async () => { + const id = insertPhoto({ provider: 'local', file_path: 'nope/other.jpg' }); + const res = makeRes(); + + await svc.streamPhoto(res as never, 1, id, 'original'); + + expect(res.status).toHaveBeenCalledWith(404); + expect(res.json).toHaveBeenCalledWith({ error: 'File not found' }); + expect(storage.exists).not.toHaveBeenCalled(); + }); + it('RESOLVE-006: an immich original goes straight to the provider with the range header', async () => { const id = insertPhoto({ provider: 'immich', asset_id: 'a1', owner_id: 5 }); const res = makeRes(); diff --git a/server/tests/unit/nest/trek-photo-cache.service.test.ts b/server/tests/unit/nest/trek-photo-cache.service.test.ts index 747bc2fa9..89a7aa658 100644 --- a/server/tests/unit/nest/trek-photo-cache.service.test.ts +++ b/server/tests/unit/nest/trek-photo-cache.service.test.ts @@ -27,8 +27,13 @@ import { runMigrations } from '../../../src/db/migrations'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { TrekPhotoCacheService, CACHE_TTL } from '../../../src/nest/memories/trek-photo-cache.service'; import { UPLOADS_ROOT } from '../../../src/nest/memories/uploads-root'; +import { StorageNotFoundError } from '../../../src/nest/storage/storage.types'; +import type { StorageService } from '../../../src/nest/storage/storage.service'; -const svc = new TrekPhotoCacheService(new DatabaseService(testDb)); +// serveFresh is the one byte-serving method; the cache's disk internals +// (getFresh/put/sweep) stay raw-fs until slice 4. +const storage = { sendToResponse: vi.fn().mockResolvedValue(undefined) }; +const svc = new TrekPhotoCacheService(new DatabaseService(testDb), storage as unknown as StorageService); const CACHE_DIR = path.join(UPLOADS_ROOT, 'photos/trek'); const written: string[] = []; @@ -112,22 +117,32 @@ describe('put / getFresh', () => { }); describe('serveFresh', () => { - it('CACHE-007: sends the cached file with a one-hour cache header', async () => { + it('CACHE-007: sends the cached bytes through storage with a one-hour cache header', async () => { const key = freshKey('serve'); await svc.put(key, Buffer.from('bytes'), 'image/webp'); - const res = { set: vi.fn(), sendFile: vi.fn() }; + const res = { set: vi.fn() }; - expect(svc.serveFresh(res as never, key)).toBe(true); + expect(await svc.serveFresh(res as never, key)).toBe(true); expect(res.set).toHaveBeenCalledWith('Content-Type', 'image/webp'); expect(res.set).toHaveBeenCalledWith('Cache-Control', 'public, max-age=3600'); - expect(res.sendFile).toHaveBeenCalledOnce(); + expect(storage.sendToResponse).toHaveBeenCalledWith('photos-trek', `${key}.bin`, res); }); - it('CACHE-008: reports a miss without touching the response', () => { - const res = { set: vi.fn(), sendFile: vi.fn() }; - expect(svc.serveFresh(res as never, 'nothing-cached')).toBe(false); + it('CACHE-008: reports a miss without touching the response or storage', async () => { + storage.sendToResponse.mockClear(); + const res = { set: vi.fn() }; + expect(await svc.serveFresh(res as never, 'nothing-cached')).toBe(false); expect(res.set).not.toHaveBeenCalled(); - expect(res.sendFile).not.toHaveBeenCalled(); + expect(storage.sendToResponse).not.toHaveBeenCalled(); + }); + + it('CACHE-013: a getFresh→send delete race reads as a miss, not a crash', async () => { + const key = freshKey('race'); + await svc.put(key, Buffer.from('bytes'), 'image/webp'); + storage.sendToResponse.mockRejectedValueOnce(new StorageNotFoundError(`photos/trek/${key}.bin`)); + const res = { set: vi.fn(), headersSent: false }; + + expect(await svc.serveFresh(res as never, key)).toBe(false); }); }); @@ -151,7 +166,7 @@ describe('the stampede guard', () => { // Load-bearing: a per-instance map would hand any second instance a private // guard and the dedup would be silently gone. The sweep cron injects the // container singleton now, but the invariant stays pinned. - const other = new TrekPhotoCacheService(new DatabaseService(testDb)); + const other = new TrekPhotoCacheService(new DatabaseService(testDb), storage as unknown as StorageService); const promise = Promise.resolve(null); svc.setInFlight('shared-key', promise); expect(other.getInFlight('shared-key')).toBe(promise);