refactor(maps): stream place-photo bytes through storage

server/src/nest/maps/maps.controller.ts

@@ -11,7 +11,7 @@ import {
   UseGuards,
 } from '@nestjs/common';
 import type { Response } from 'express';
-import { createReadStream } from 'node:fs';
+import type { Readable } from 'node:stream';
 import type {
   MapsAutocompleteResult,
   MapsPlaceDetailsResult,
@@ -22,6 +22,7 @@ import type {
 } from '@trek/shared';
 import type { User } from '../../types';
 import { MapsService } from './maps.service';
+import { StorageService } from '../storage/storage.service';
 import { JwtAuthGuard } from '../auth/jwt-auth.guard';
 import { CurrentUser } from '../auth/current-user.decorator';
 import { MapsSearchDto, MapsAutocompleteDto, MapsResolveUrlDto } from './maps.dto';
@@ -60,7 +61,10 @@ function toHttpException(err: unknown, fallbackMessage: string, defaultStatus: n
 @Controller('api/maps')
 @UseGuards(JwtAuthGuard)
 export class MapsController {
-  constructor(private readonly maps: MapsService) {}
+  constructor(
+    private readonly maps: MapsService,
+    private readonly storage: StorageService,
+  ) {}
 
   @Post('search')
   @HttpCode(200) // Express answers with res.json (200); Nest would otherwise default POST to 201.
@@ -166,9 +170,9 @@ export class MapsController {
   }
 
   @Get('place-photo/:placeId/bytes')
-  placePhotoBytes(@Param('placeId') placeId: string, @Res() res: Response): void {
-    const fp = this.maps.photoBytesPath(placeId);
-    if (!fp) {
+  async placePhotoBytes(@Param('placeId') placeId: string, @Res() res: Response): Promise<void> {
+    const key = this.maps.photoBytesKey(placeId);
+    if (!key) {
       // Same reasoning as the JSON endpoint above, and the bigger half of #1727:
       // places keep this URL in image_url, so a trip render asks for one photo per
       // place and every entry the cache sweep dropped answered 404 — a whole map
@@ -177,15 +181,21 @@ export class MapsController {
       this.emptyPhoto(res);
       return;
     }
-    // Stream the cached file directly instead of res.sendFile(): the send library
-    // bundled under @nestjs/platform-express rejects absolute Windows paths (drive
-    // letter, no `root`) with a NotFoundError that surfaced as an unhandled 500,
-    // even though the file exists. A plain read stream serves the bytes
-    // cross-platform. Cached photos are always JPEG (placePhotoCache writes
-    // `<hash>.jpg`).
+    // Bytes are stream-piped (spec §Serving pins this route's stream contract);
+    // headers go on before the stream attempt, matching the old createReadStream
+    // ordering — an early failure overrides them via emptyPhoto exactly as the
+    // old error event did. Cached photos are always JPEG (placePhotoCache
+    // writes `<hash>.jpg`).
     res.set('Cache-Control', 'public, max-age=2592000, immutable');
     res.type('image/jpeg');
-    const stream = createReadStream(fp);
+    let stream: Readable;
+    try {
+      ({ stream } = await this.storage.getStream('photos-google', key));
+    } catch {
+      // Cache-delete race — same terminal state as the old stream error path.
+      if (!res.headersSent) this.emptyPhoto(res);
+      return;
+    }
     stream.on('error', () => {
       if (!res.headersSent) this.emptyPhoto(res);
     });

server/src/nest/maps/maps.module.ts

@@ -3,6 +3,7 @@ import { MapsController } from './maps.controller';
 import { MapsService } from './maps.service';
 import { MapsMcp } from './maps.mcp';
 import { PlacePhotosModule } from '../place-photos/place-photos.module';
+import { StorageModule } from '../storage/storage.module';
 
 /**
  * Maps / geo domain (L3 leaf module). Registered in AppModule. Exports
@@ -11,7 +12,7 @@ import { PlacePhotosModule } from '../place-photos/place-photos.module';
  * Nothing outside the container consumes this domain, so there is no bridge.
  */
 @Module({
-  imports: [PlacePhotosModule],
+  imports: [PlacePhotosModule, StorageModule],
   controllers: [MapsController],
   providers: [MapsService, MapsMcp],
   exports: [MapsService],

server/src/nest/maps/maps.service.ts

@@ -529,8 +529,8 @@ export class MapsService {
     return this.getPlacePhoto(userId, placeId, lat, lng, name) as Promise<MapsPlacePhotoResult>;
   }
 
-  photoBytesPath(placeId: string): string | null {
-    return this.photoCache.serveFilePath(placeId);
+  photoBytesKey(placeId: string): string | null {
+    return this.photoCache.serveKey(placeId);
   }
 
   reverse(lat: string, lng: string, lang?: string): Promise<MapsReverseResult> {

server/src/nest/place-photos/place-photo-cache.service.ts

@@ -77,11 +77,15 @@ export class PlacePhotoCacheService implements OnModuleInit {
     }
   }
 
-  private filePath(placeId: string): string {
+  private fileName(placeId: string): string {
     // Hash to avoid filename collisions — coords:lat:lng pseudo-IDs contain characters that
     // collapse identically under sanitization (e.g. ':' and '.' both → '_')
     const hash = crypto.createHash('sha1').update(placeId).digest('hex');
-    return path.join(this.photoDir, `${hash}.jpg`);
+    return `${hash}.jpg`;
+  }
+
+  private filePath(placeId: string): string {
+    return path.join(this.photoDir, this.fileName(placeId));
   }
 
   private proxyUrl(placeId: string): string {
@@ -214,6 +218,16 @@ export class PlacePhotoCacheService implements OnModuleInit {
     return fp;
   }
 
+  /**
+   * Storage name (category 'photos-google') for a cached photo, or null when
+   * not on disk. The registry's mode-aware prefix reproduces photoDir in both
+   * TREK_PLACE_PHOTO_DIR modes; the cache's own disk internals move onto
+   * storage in slice 4.
+   */
+  serveKey(placeId: string): string | null {
+    return this.serveFilePath(placeId) ? this.fileName(placeId) : null;
+  }
+
   /**
    * A cache entry is "referenced" while any place still points at it — either by the
    * Google place_id (the dedup key) or by the stable proxy URL stored in image_url

server/tests/unit/nest/maps.controller.test.ts

@@ -2,17 +2,19 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { HttpException } from '@nestjs/common';
 import type { Response } from 'express';
 
-const { createReadStream } = vi.hoisted(() => ({ createReadStream: vi.fn() }));
-vi.mock('node:fs', () => ({ createReadStream }));
-
 import { MapsController } from '../../../src/nest/maps/maps.controller';
 import type { MapsService } from '../../../src/nest/maps/maps.service';
+import type { StorageService } from '../../../src/nest/storage/storage.service';
 import type { User } from '../../../src/types';
 
 const user = { id: 3 } as User;
 
+// The bytes route is the only storage consumer; everything else ignores it.
+const getStream = vi.fn();
+const storageStub = { getStream } as unknown as StorageService;
+
 function makeController(svc: Partial<MapsService>) {
-  return new MapsController(svc as MapsService);
+  return new MapsController(svc as MapsService, storageStub);
 }
 
 /** Run an async handler, expecting an HttpException; return its { status, body }. */
@@ -243,38 +245,46 @@ describe('MapsController (parity with the legacy /api/maps route)', () => {
       return res as unknown as Response & { status: ReturnType<typeof vi.fn>; json: ReturnType<typeof vi.fn>; set: ReturnType<typeof vi.fn>; type: ReturnType<typeof vi.fn>; end: ReturnType<typeof vi.fn> };
     }
 
-    beforeEach(() => createReadStream.mockReset());
+    // Braced body on purpose: mockReset() returns the mock, and a function
+    // returned from beforeEach is invoked as a teardown callback — which would
+    // call getStream() bare and turn a mockRejectedValue impl into an
+    // unhandled rejection.
+    beforeEach(() => {
+      getStream.mockReset();
+    });
+
+    function makeStream() {
+      const stream = { on: vi.fn().mockReturnThis(), pipe: vi.fn() };
+      return stream;
+    }
 
     // Places persist this URL in image_url, so an evicted cache entry means one
     // request per place on a trip render. 404 for each of them is the ban vector
     // from #1727 — an uncached photo answers 204 with an empty body instead.
-    it('204 without a body when the photo is not cached', () => {
+    it('204 without a body when the photo is not cached', async () => {
       const res = makeRes();
-      makeController({ photoBytesPath: () => null }).placePhotoBytes('p1', res);
+      await makeController({ photoBytesKey: () => null }).placePhotoBytes('p1', res);
       expect(res.status).toHaveBeenCalledWith(204);
       expect(res.end).toHaveBeenCalled();
       expect(res.json).not.toHaveBeenCalled();
-      expect(createReadStream).not.toHaveBeenCalled();
+      expect(getStream).not.toHaveBeenCalled();
     });
 
-    it('streams the cached file with image/jpeg + an immutable cache header on a hit', () => {
-      const stream = { on: vi.fn().mockReturnThis(), pipe: vi.fn() };
-      createReadStream.mockReturnValue(stream);
+    it('streams the cached bytes with image/jpeg + an immutable cache header on a hit', async () => {
+      const stream = makeStream();
+      getStream.mockResolvedValue({ stream, stat: { key: 'photos/google/abc.jpg', size: 5, mtimeMs: 0 } });
       const res = makeRes();
-      makeController({ photoBytesPath: () => '/cache/p1.jpg' }).placePhotoBytes('p1', res);
+      await makeController({ photoBytesKey: () => 'abc.jpg' }).placePhotoBytes('p1', res);
       expect(res.set).toHaveBeenCalledWith('Cache-Control', 'public, max-age=2592000, immutable');
       expect(res.type).toHaveBeenCalledWith('image/jpeg');
-      expect(createReadStream).toHaveBeenCalledWith('/cache/p1.jpg');
+      expect(getStream).toHaveBeenCalledWith('photos-google', 'abc.jpg');
       expect(stream.pipe).toHaveBeenCalledWith(res);
     });
 
-    it('falls back to an empty 204 when the read stream errors', () => {
-      let onError: () => void = () => {};
-      const stream = { on: vi.fn((ev: string, cb: () => void) => { if (ev === 'error') onError = cb; return stream; }), pipe: vi.fn() };
-      createReadStream.mockReturnValue(stream);
+    it('falls back to an empty 204 when the stream cannot be opened (cache-delete race)', async () => {
+      getStream.mockRejectedValue(new Error('storage object not found'));
       const res = makeRes();
-      makeController({ photoBytesPath: () => '/cache/p1.jpg' }).placePhotoBytes('p1', res);
-      onError();
+      await makeController({ photoBytesKey: () => 'abc.jpg' }).placePhotoBytes('p1', res);
       expect(res.status).toHaveBeenCalledWith(204);
       expect(res.end).toHaveBeenCalled();
       // The hit path already asked for a month of immutable caching — that header
@@ -283,13 +293,25 @@ describe('MapsController (parity with the legacy /api/maps route)', () => {
       expect(res.set).toHaveBeenLastCalledWith('Cache-Control', 'no-store');
     });
 
-    it('does not re-send a 204 when the stream errors after headers were flushed', () => {
+    it('falls back to an empty 204 when the stream errors before headers were flushed', async () => {
       let onError: () => void = () => {};
       const stream = { on: vi.fn((ev: string, cb: () => void) => { if (ev === 'error') onError = cb; return stream; }), pipe: vi.fn() };
-      createReadStream.mockReturnValue(stream);
+      getStream.mockResolvedValue({ stream, stat: { key: 'photos/google/abc.jpg', size: 5, mtimeMs: 0 } });
+      const res = makeRes();
+      await makeController({ photoBytesKey: () => 'abc.jpg' }).placePhotoBytes('p1', res);
+      onError();
+      expect(res.status).toHaveBeenCalledWith(204);
+      expect(res.end).toHaveBeenCalled();
+      expect(res.set).toHaveBeenLastCalledWith('Cache-Control', 'no-store');
+    });
+
+    it('does not re-send a 204 when the stream errors after headers were flushed', async () => {
+      let onError: () => void = () => {};
+      const stream = { on: vi.fn((ev: string, cb: () => void) => { if (ev === 'error') onError = cb; return stream; }), pipe: vi.fn() };
+      getStream.mockResolvedValue({ stream, stat: { key: 'photos/google/abc.jpg', size: 5, mtimeMs: 0 } });
       const res = makeRes();
       (res as { headersSent: boolean }).headersSent = true;
-      makeController({ photoBytesPath: () => '/cache/p1.jpg' }).placePhotoBytes('p1', res);
+      await makeController({ photoBytesKey: () => 'abc.jpg' }).placePhotoBytes('p1', res);
       onError();
       expect(res.status).not.toHaveBeenCalled();
       expect(res.end).not.toHaveBeenCalled();

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

@@ -112,7 +112,7 @@ const photoCacheStub = {
   markError: (placeId: string, kind?: string) => mockCacheMarkError(placeId, kind),
   getInFlight: (placeId: string) => mockCacheGetInFlight(placeId),
   setInFlight: (placeId: string, p: Promise<any>) => mockCacheSetInFlight(placeId, p),
-  serveFilePath: (placeId: string) => mockServeFilePath(placeId),
+  serveKey: (placeId: string) => mockServeFilePath(placeId),
 } as unknown as PlacePhotoCacheService;
 
 import { db } from '../../../src/db/database';
@@ -2329,16 +2329,16 @@ describe('kill-switch settings reads', () => {
   });
 });
 
-describe('photoBytesPath', () => {
-  it('returns the cached file path from placePhotoCache', () => {
-    mockServeFilePath.mockReturnValue('/cache/p1.jpg');
-    expect(svc.photoBytesPath('p1')).toBe('/cache/p1.jpg');
+describe('photoBytesKey', () => {
+  it('returns the cached storage name from placePhotoCache', () => {
+    mockServeFilePath.mockReturnValue('abc.jpg');
+    expect(svc.photoBytesKey('p1')).toBe('abc.jpg');
     expect(mockServeFilePath).toHaveBeenCalledWith('p1');
   });
 
   it('returns null when nothing is cached', () => {
     mockServeFilePath.mockReturnValue(null);
-    expect(svc.photoBytesPath('p1')).toBeNull();
+    expect(svc.photoBytesKey('p1')).toBeNull();
   });
 });
 

server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts

@@ -109,6 +109,18 @@ describe('placePhotoCache.put() downscale guard', () => {
   });
 });
 
+describe('placePhotoCache.serveKey()', () => {
+  it('returns the bare <sha1>.jpg storage name when the photo is on disk', async () => {
+    fs.writeFileSync(filePathFor('p-key'), await makeJpeg(10, 10));
+    const hash = crypto.createHash('sha1').update('p-key').digest('hex');
+    expect(cache.serveKey('p-key')).toBe(`${hash}.jpg`);
+  });
+
+  it('returns null when nothing is cached', () => {
+    expect(cache.serveKey('p-missing')).toBeNull();
+  });
+});
+
 describe('placePhotoCache.removeIfUnreferenced()', () => {
   it('PPC-004: removes a cache entry that no place references', async () => {
     await cache.put('orphan', await makeJpeg(50, 50), null);