mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-20 14:06:27 +00:00
refactor(memories): serve photo bytes through storage
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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<void> {
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<boolean> {
|
||||
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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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<Buffer | null>(null);
|
||||
svc.setInFlight('shared-key', promise);
|
||||
expect(other.getInFlight('shared-key')).toBe(promise);
|
||||
|
||||
Reference in New Issue
Block a user