refactor(memories): thumbnail service category-addressed

server/src/nest/memories/photo-resolver.service.ts

@@ -3,7 +3,6 @@ import { Response } from 'express';
 import type { TrekPhoto } from '../../types';
 import { decrypt_api_key } from '../common/crypto/apiKeyCrypto';
 import { TrekPhotosRepository } from '../photos/trek-photos.repository';
-import { UPLOADS_ROOT } from './uploads-root';
 import { ThumbnailService } from './thumbnail.service';
 import { TrekPhotoCacheService } from './trek-photo-cache.service';
 import { fail, success, type AssetInfo, type ServiceResult } from './memories.helpers';
@@ -90,9 +89,7 @@ export class PhotoResolverService {
         // 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) {
-          // 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);
+          const result = await this.thumbnails.ensureLocalThumbnail(photo.file_path);
           if (result) {
             thumbRel = result.thumbnailRelPath;
             this.photos.recordLocalThumbnail(photo.id, thumbRel, result.width, result.height);

server/src/nest/memories/thumbnail.service.ts

@@ -1,55 +1,73 @@
 import { Jimp } from 'jimp'
-import path from 'path'
-import fs from 'fs/promises'
-import crypto from 'crypto'
+import path from 'node:path'
+import crypto from 'node:crypto'
 import { Injectable } from '@nestjs/common'
 import { ADDON_IDS } from '../../addons'
 import { AddonsService } from '../addons/addons.service'
+import { StorageService } from '../storage/storage.service'
 
 const THUMB_MAX = 800
 const THUMB_QUALITY = 80
 
+/**
+ * Storage name (category 'journey') of the derived thumbnail for an
+ * uploads-relative journey photo path ('journey/<file>'). The hash input is
+ * deliberately the uploads-relative path — the scheme predates the storage
+ * layer, and changing it would orphan every existing thumbnail.
+ */
+export function journeyThumbName(originalRelPath: string): string {
+  const hash = crypto.createHash('sha1').update(originalRelPath).digest('hex').slice(0, 16)
+  return `thumbs/${hash}.jpg`
+}
+
 /**
  * Downscaled JPEGs for locally uploaded journey photos. Gated on the journey
- * addon, because that is the only surface that uploads them.
+ * addon, because that is the only surface that uploads them. Category-addressed
+ * since storage slice 4: originals and thumbs are ('journey', <name>) objects.
  */
 @Injectable()
 export class ThumbnailService {
-  constructor(private readonly addons: AddonsService) {}
+  constructor(
+    private readonly addons: AddonsService,
+    private readonly storage: StorageService,
+  ) {}
 
   async ensureLocalThumbnail(
-    uploadsRoot: string,
     originalRelPath: string,
   ): Promise<{ thumbnailRelPath: string; width: number; height: number } | null> {
     if (!this.addons.isAddonEnabled(ADDON_IDS.JOURNEY)) return null
 
-    const originalAbs = path.join(uploadsRoot, originalRelPath)
-    try { await fs.access(originalAbs) } catch { return null }
+    // The DB stores uploads-relative paths ('journey/<file>'); anything else
+    // is not a local journey photo and has no thumbnail to derive.
+    const origName = originalRelPath.startsWith('journey/') ? originalRelPath.slice('journey/'.length) : null
+    if (!origName) return null
 
     // Deterministic name so concurrent requests don't race on the same photo.
-    const hash = crypto.createHash('sha1').update(originalRelPath).digest('hex').slice(0, 16)
-    const thumbRel = `journey/thumbs/${hash}.jpg`
-    const thumbAbs = path.join(uploadsRoot, thumbRel)
+    const thumbName = journeyThumbName(originalRelPath)
+    const thumbRel = `journey/${thumbName}`
 
     try {
       const [srcStat, dstStat] = await Promise.all([
-        fs.stat(originalAbs),
-        fs.stat(thumbAbs).catch(() => null),
+        this.storage.stat('journey', origName),
+        this.storage.stat('journey', thumbName),
       ])
+      if (!srcStat) return null
       if (dstStat && dstStat.mtimeMs >= srcStat.mtimeMs) {
-        const img = await Jimp.read(thumbAbs)
+        const img = await this.storage.withLocalFile('journey', thumbName, (p) => Jimp.read(p))
         return { thumbnailRelPath: thumbRel, width: img.bitmap.width, height: img.bitmap.height }
       }
 
-      await fs.mkdir(path.dirname(thumbAbs), { recursive: true })
-
       // Jimp auto-applies EXIF orientation on read, matching sharp's .rotate() behavior.
-      const img = await Jimp.read(originalAbs)
+      const img = await this.storage.withLocalFile('journey', origName, (p) => Jimp.read(p))
       const { width: w, height: h } = img.bitmap
       if (w > THUMB_MAX || h > THUMB_MAX) {
         img.scaleToFit({ w: THUMB_MAX, h: THUMB_MAX })
       }
-      await img.write(thumbAbs as `${string}.jpg`, { quality: THUMB_QUALITY })
+      // Jimp writes straight to a path, so spool on the category's volume and
+      // commit through storage for the atomic rename.
+      const spool = path.join(this.storage.spoolDirFor('journey'), `thumb-${crypto.randomUUID()}.jpg`)
+      await img.write(spool as `${string}.jpg`, { quality: THUMB_QUALITY })
+      await this.storage.put('journey', thumbName, { tmpPath: spool })
 
       return { thumbnailRelPath: thumbRel, width: img.bitmap.width, height: img.bitmap.height }
     } catch {

server/tests/unit/nest/thumbnail.service.test.ts

@@ -11,14 +11,18 @@ import { describe, it, expect, vi, beforeEach, afterAll } from 'vitest';
 const { isAddonEnabled } = vi.hoisted(() => ({ isAddonEnabled: vi.fn(() => true) }));
 
 import fs from 'node:fs';
-import os from 'node:os';
 import path from 'node:path';
 import { Jimp } from 'jimp';
 import { ThumbnailService } from '../../../src/nest/memories/thumbnail.service';
 import type { AddonsService } from '../../../src/nest/addons/addons.service';
+import { makeStorageFixture } from '../../helpers/storage-fixture';
 
-const svc = new ThumbnailService({ isAddonEnabled } as unknown as AddonsService);
-const root = fs.mkdtempSync(path.join(os.tmpdir(), 'trek-thumbs-'));
+// Category-addressed since slice 4: originals + thumbs are ('journey', <name>)
+// 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);
+const root = fx.root;
 
 /** A real 1200x900 JPEG — Jimp has to be able to decode it for the happy path. */
 async function writeSourceImage(rel: string): Promise<void> {
@@ -34,23 +38,27 @@ beforeEach(() => {
 });
 
 afterAll(() => {
-  fs.rmSync(root, { recursive: true, force: true });
+  fx.cleanup();
 });
 
 describe('ensureLocalThumbnail', () => {
   it('THUMB-001: returns null when the journey addon is off, without touching the disk', async () => {
     isAddonEnabled.mockReturnValue(false);
-    expect(await svc.ensureLocalThumbnail(root, 'anything.jpg')).toBeNull();
+    expect(await svc.ensureLocalThumbnail('anything.jpg')).toBeNull();
   });
 
-  it('THUMB-002: returns null when the source file does not exist', async () => {
-    expect(await svc.ensureLocalThumbnail(root, 'missing/nope.jpg')).toBeNull();
+  it('THUMB-002: returns null when the source object does not exist', async () => {
+    expect(await svc.ensureLocalThumbnail('journey/missing.jpg')).toBeNull();
+  });
+
+  it('THUMB-002b: returns null for a path outside the journey category', async () => {
+    expect(await svc.ensureLocalThumbnail('missing/nope.jpg')).toBeNull();
   });
 
   it('THUMB-003: downscales an oversized image and reports the resulting size', async () => {
     await writeSourceImage('journey/big.jpg');
 
-    const result = await svc.ensureLocalThumbnail(root, 'journey/big.jpg');
+    const result = await svc.ensureLocalThumbnail('journey/big.jpg');
 
     expect(result).not.toBeNull();
     expect(result!.thumbnailRelPath).toMatch(/^journey\/thumbs\/[0-9a-f]{16}\.jpg$/);
@@ -62,19 +70,19 @@ describe('ensureLocalThumbnail', () => {
   it('THUMB-004: the path is deterministic, so concurrent requests cannot race on two names', async () => {
     await writeSourceImage('journey/stable.jpg');
 
-    const first = await svc.ensureLocalThumbnail(root, 'journey/stable.jpg');
-    const second = await svc.ensureLocalThumbnail(root, 'journey/stable.jpg');
+    const first = await svc.ensureLocalThumbnail('journey/stable.jpg');
+    const second = await svc.ensureLocalThumbnail('journey/stable.jpg');
 
     expect(second!.thumbnailRelPath).toBe(first!.thumbnailRelPath);
   });
 
   it('THUMB-005: reuses an existing thumbnail that is newer than the source', async () => {
     await writeSourceImage('journey/cached.jpg');
-    const first = await svc.ensureLocalThumbnail(root, 'journey/cached.jpg');
+    const first = await svc.ensureLocalThumbnail('journey/cached.jpg');
     const thumbAbs = path.join(root, first!.thumbnailRelPath);
     const mtimeBefore = fs.statSync(thumbAbs).mtimeMs;
 
-    const second = await svc.ensureLocalThumbnail(root, 'journey/cached.jpg');
+    const second = await svc.ensureLocalThumbnail('journey/cached.jpg');
 
     expect(second).toEqual(first);
     expect(fs.statSync(thumbAbs).mtimeMs).toBe(mtimeBefore);
@@ -82,13 +90,13 @@ describe('ensureLocalThumbnail', () => {
 
   it('THUMB-006: regenerates when the source is newer than the thumbnail', async () => {
     await writeSourceImage('journey/changed.jpg');
-    const first = await svc.ensureLocalThumbnail(root, 'journey/changed.jpg');
+    const first = await svc.ensureLocalThumbnail('journey/changed.jpg');
     const thumbAbs = path.join(root, first!.thumbnailRelPath);
     // Age the thumbnail past the source.
     const old = new Date(Date.now() - 60_000);
     fs.utimesSync(thumbAbs, old, old);
 
-    const second = await svc.ensureLocalThumbnail(root, 'journey/changed.jpg');
+    const second = await svc.ensureLocalThumbnail('journey/changed.jpg');
 
     expect(second!.thumbnailRelPath).toBe(first!.thumbnailRelPath);
     expect(fs.statSync(thumbAbs).mtimeMs).toBeGreaterThan(old.getTime());
@@ -99,6 +107,6 @@ describe('ensureLocalThumbnail', () => {
     fs.mkdirSync(path.dirname(abs), { recursive: true });
     fs.writeFileSync(abs, 'not an image at all');
 
-    expect(await svc.ensureLocalThumbnail(root, 'journey/corrupt.jpg')).toBeNull();
+    expect(await svc.ensureLocalThumbnail('journey/corrupt.jpg')).toBeNull();
   });
 });