k1nq/TREK
1
0
Fork 0
mirror of https://github.com/mauriceboe/TREK.git synced 2026-10-19 21:46:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(share): stream shared place-photo bytes through storage

Browse Source
This commit is contained in:
jubnl
2026-08-16 11:07:04 +02:00
parent 786b1f5c07
commit 1bf6517b75
5 changed files with 91 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/nest/share/share.controller.ts
+20 -6
View File
@@ -1,8 +1,9 @@
import { Body, Controller, Delete, Get, HttpException, Param, Post, Res, UseGuards } from '@nestjs/common';
import type { Response } from 'express';
import { createReadStream } from 'node:fs';
import type { Readable } from 'node:stream';
import type { User } from '../../types';
import { ShareService } from './share.service';
import { StorageService } from '../storage/storage.service';
import { ShareLinkDto } from './share.dto';
import { JwtAuthGuard } from '../auth/jwt-auth.guard';
import { CurrentUser } from '../auth/current-user.decorator';
@@ -78,7 +79,10 @@ export class TripShareController {
@Public('share-token validated: a shared trip link has to work for somebody without an account')
@Controller('api/shared')
export class SharedController {
constructor(private readonly share: ShareService) {}
constructor(
private readonly share: ShareService,
private readonly storage: StorageService,
) {}
/**
* Public, token-scoped place-photo proxy. The shared payload rewrites place
@@ -89,15 +93,25 @@ export class SharedController {
* MapsController.placePhotoBytes (cached photos are always JPEG).
*/
@Get(':token/place-photo/:placeId/bytes')
placePhotoBytes(@Param('token') token: string, @Param('placeId') placeId: string, @Res() res: Response): void {
const fp = this.share.getSharedPlacePhotoPath(token, placeId);
if (!fp) {
async placePhotoBytes(@Param('token') token: string, @Param('placeId') placeId: string, @Res() res: Response): Promise<void> {
const key = this.share.getSharedPlacePhotoKey(token, placeId);
if (!key) {
res.status(404).json({ error: 'Photo not cached' });
return;
}
// Bytes are stream-piped like the maps handler; headers go on before the
// stream attempt so an early failure overrides them exactly as the old
// createReadStream error event did.
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) res.status(404).json({ error: 'Photo not cached' });
return;
}
stream.on('error', () => {
if (!res.headersSent) res.status(404).json({ error: 'Photo not cached' });
});
server/src/nest/share/share.module.ts
+2 -1
View File
@@ -6,11 +6,12 @@ import { SettingsModule } from '../settings/settings.module';
import { PermissionsModule } from '../permissions/permissions.module';
import { QueryHelpersModule } from '../query-helpers/query-helpers.module';
import { PlacePhotosModule } from '../place-photos/place-photos.module';
import { StorageModule } from '../storage/storage.module';
import { AuthModule } from '../auth/auth.module';
import { McpSharedModule } from '../mcp-shared/mcp-shared.module';
@Module({
imports: [McpSharedModule, SettingsModule, PermissionsModule, QueryHelpersModule, AuthModule, PlacePhotosModule],
imports: [McpSharedModule, SettingsModule, PermissionsModule, QueryHelpersModule, AuthModule, PlacePhotosModule, StorageModule],
controllers: [TripShareController, SharedController],
providers: [ShareService, ShareMcp],
})
server/src/nest/share/share.service.ts
+9 -8
View File
@@ -297,14 +297,15 @@ export class ShareService {
}
/**
* Resolves the on-disk path for a cached place photo requested through a public
* share link. Validates that the token is valid + unexpired and that the place
* actually belongs to that token's trip (matched via the stored proxy URL, which
* covers both Google `placeId` and Wikimedia `coords:` pseudo-IDs without
* depending on google_place_id). Returns null — never throws — so the caller
* answers a plain 404, mirroring the authenticated bytes endpoint.
* Resolves the storage name (category 'photos-google') for a cached place
* photo requested through a public share link. Validates that the token is
* valid + unexpired and that the place actually belongs to that token's trip
* (matched via the stored proxy URL, which covers both Google `placeId` and
* Wikimedia `coords:` pseudo-IDs without depending on google_place_id).
* Returns null — never throws — so the caller answers a plain miss,
* mirroring the authenticated bytes endpoint.
*/
getSharedPlacePhotoPath(token: string, placeId: string): string | null {
getSharedPlacePhotoKey(token: string, placeId: string): string | null {
const shareRow = this.dbs.get<{ trip_id: string; share_map: number }>(
"SELECT trip_id, share_map FROM share_tokens WHERE token = ? AND (expires_at IS NULL OR expires_at > datetime('now'))",
token,
@@ -319,6 +320,6 @@ export class ShareService {
const place = this.dbs.get('SELECT 1 FROM places WHERE trip_id = ? AND image_url = ?', shareRow.trip_id, expectedUrl);
if (!place) return null;
return this.photoCache.serveFilePath(placeId);
return this.photoCache.serveKey(placeId);
}
}
server/tests/unit/nest/share.controller.test.ts
+44 -28
View File
@@ -2,13 +2,15 @@ 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 { TripShareController, SharedController } from '../../../src/nest/share/share.controller';
import type { ShareService } from '../../../src/nest/share/share.service';
import type { StorageService } from '../../../src/nest/storage/storage.service';
import type { User } from '../../../src/types';
// Only the shared place-photo proxy consumes storage.
const getStream = vi.fn();
const storageStub = { getStream } as unknown as StorageService;
const user = { id: 1, role: 'user', email: 'u@example.test' } as User;
function svc(o: Partial<ShareService> = {}): ShareService {
@@ -74,8 +76,8 @@ describe('TripShareController', () => {
describe('SharedController', () => {
it('404 for an invalid token, else returns the snapshot', () => {
expect(thrown(() => new SharedController(svc({ getSharedTripData: vi.fn().mockReturnValue(null) } as Partial<ShareService>)).read('bad'))).toEqual({ status: 404, body: { error: 'Invalid or expired link' } });
expect(new SharedController(svc({ getSharedTripData: vi.fn().mockReturnValue({ trip: { id: 9 } }) } as Partial<ShareService>)).read('tok')).toEqual({ trip: { id: 9 } });
expect(thrown(() => new SharedController(svc({ getSharedTripData: vi.fn().mockReturnValue(null) } as Partial<ShareService>), storageStub).read('bad'))).toEqual({ status: 404, body: { error: 'Invalid or expired link' } });
expect(new SharedController(svc({ getSharedTripData: vi.fn().mockReturnValue({ trip: { id: 9 } }) } as Partial<ShareService>), storageStub).read('tok')).toEqual({ trip: { id: 9 } });
});
describe('place-photo proxy', () => {
@@ -91,49 +93,63 @@ describe('SharedController', () => {
return r as unknown as Response & { status: ReturnType<typeof vi.fn>; json: ReturnType<typeof vi.fn>; set: ReturnType<typeof vi.fn>; type: ReturnType<typeof vi.fn> };
}
beforeEach(() => createReadStream.mockReset());
function controller(path: string | null) {
return new SharedController(svc({ getSharedPlacePhotoPath: vi.fn().mockReturnValue(path) } as Partial<ShareService>));
}
it('404 without streaming when the photo is not cached for the token', () => {
const res = photoRes();
controller(null).placePhotoBytes('tok', 'p1', res);
expect(res.status).toHaveBeenCalledWith(404);
expect(res.json).toHaveBeenCalledWith({ error: 'Photo not cached' });
expect(createReadStream).not.toHaveBeenCalled();
// 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 rejecting impl into an unhandled
// rejection.
beforeEach(() => {
getStream.mockReset();
});
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);
function controller(key: string | null) {
return new SharedController(svc({ getSharedPlacePhotoKey: vi.fn().mockReturnValue(key) } as Partial<ShareService>), storageStub);
}
it('404 without streaming when the photo is not cached for the token', async () => {
const res = photoRes();
controller('/cache/p1.jpg').placePhotoBytes('tok', 'p1', res);
await controller(null).placePhotoBytes('tok', 'p1', res);
expect(res.status).toHaveBeenCalledWith(404);
expect(res.json).toHaveBeenCalledWith({ error: 'Photo not cached' });
expect(getStream).not.toHaveBeenCalled();
});
it('streams the cached bytes with image/jpeg + an immutable cache header on a hit', async () => {
const stream = { on: vi.fn().mockReturnThis(), pipe: vi.fn() };
getStream.mockResolvedValue({ stream, stat: { key: 'photos/google/abc.jpg', size: 5, mtimeMs: 0 } });
const res = photoRes();
await controller('abc.jpg').placePhotoBytes('tok', '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 404 when the read stream errors before headers were sent', () => {
it('falls back to 404 when the stream cannot be opened (cache-delete race)', async () => {
getStream.mockRejectedValue(new Error('storage object not found'));
const res = photoRes();
await controller('abc.jpg').placePhotoBytes('tok', 'p1', res);
expect(res.status).toHaveBeenCalledWith(404);
expect(res.json).toHaveBeenCalledWith({ error: 'Photo not cached' });
});
it('falls back to 404 when the stream errors before headers were sent', 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 = photoRes();
controller('/cache/p1.jpg').placePhotoBytes('tok', 'p1', res);
await controller('abc.jpg').placePhotoBytes('tok', 'p1', res);
onError();
expect(res.status).toHaveBeenCalledWith(404);
expect(res.json).toHaveBeenCalledWith({ error: 'Photo not cached' });
});
it('does not re-send a 404 when the stream errors after headers were flushed', () => {
it('does not re-send a 404 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() };
createReadStream.mockReturnValue(stream);
getStream.mockResolvedValue({ stream, stat: { key: 'photos/google/abc.jpg', size: 5, mtimeMs: 0 } });
const res = photoRes();
(res as { headersSent: boolean }).headersSent = true;
controller('/cache/p1.jpg').placePhotoBytes('tok', 'p1', res);
await controller('abc.jpg').placePhotoBytes('tok', 'p1', res);
onError();
expect(res.status).not.toHaveBeenCalled();
expect(res.json).not.toHaveBeenCalled();
server/tests/unit/nest/share.service.test.ts
+16 -16
View File
@@ -45,8 +45,8 @@ const checkPermission = vi.fn();
const permissionsStub = { checkPermission } as unknown as PermissionsService;
// Injected stub since the photo-cache fold (was a path mock of the module).
const serveFilePath = vi.fn();
const photoCacheStub = { serveFilePath } as unknown as PlacePhotoCacheService;
const serveKey = vi.fn();
const photoCacheStub = { serveKey } as unknown as PlacePhotoCacheService;
import { createTables } from '../../../src/db/schema';
import { runMigrations } from '../../../src/db/migrations';
@@ -71,7 +71,7 @@ beforeAll(() => {
beforeEach(() => {
resetTestDb(testDb);
checkPermission.mockReset();
serveFilePath.mockReset();
serveKey.mockReset();
});
afterAll(() => {
@@ -377,41 +377,41 @@ describe('getSharedTripData', () => {
});
});
// ── getSharedPlacePhotoPath ───────────────────────────────────────────────────
// ── getSharedPlacePhotoKey ───────────────────────────────────────────────────
describe('getSharedPlacePhotoPath', () => {
describe('getSharedPlacePhotoKey', () => {
it('SHARE-SVC-022: returns null for an unknown or expired token', () => {
expect(svc.getSharedPlacePhotoPath('nope', 'ChIJabc')).toBeNull();
expect(svc.getSharedPlacePhotoKey('nope', 'ChIJabc')).toBeNull();
const { trip, token } = seedSharedTrip();
testDb.prepare('UPDATE share_tokens SET expires_at = ? WHERE trip_id = ?').run('2020-01-01T00:00:00.000Z', trip.id);
expect(svc.getSharedPlacePhotoPath(token, 'ChIJabc')).toBeNull();
expect(serveFilePath).not.toHaveBeenCalled();
expect(svc.getSharedPlacePhotoKey(token, 'ChIJabc')).toBeNull();
expect(serveKey).not.toHaveBeenCalled();
});
it('SHARE-SVC-023: returns null when the owner disabled the map section', () => {
const { trip, token } = seedSharedTrip({ share_map: false });
const place = createPlace(testDb, trip.id);
testDb.prepare('UPDATE places SET image_url = ? WHERE id = ?').run('/api/maps/place-photo/ChIJabc/bytes', place.id);
expect(svc.getSharedPlacePhotoPath(token, 'ChIJabc')).toBeNull();
expect(serveFilePath).not.toHaveBeenCalled();
expect(svc.getSharedPlacePhotoKey(token, 'ChIJabc')).toBeNull();
expect(serveKey).not.toHaveBeenCalled();
});
it('SHARE-SVC-024: returns null when no place in the trip carries the proxy URL', () => {
const { token } = seedSharedTrip();
expect(svc.getSharedPlacePhotoPath(token, 'ChIJabc')).toBeNull();
expect(serveFilePath).not.toHaveBeenCalled();
expect(svc.getSharedPlacePhotoKey(token, 'ChIJabc')).toBeNull();
expect(serveKey).not.toHaveBeenCalled();
});
it('SHARE-SVC-025: resolves via serveFilePath when the encoded placeId matches the stored URL', () => {
it('SHARE-SVC-025: resolves via serveKey when the encoded placeId matches the stored URL', () => {
const { trip, token } = seedSharedTrip();
const place = createPlace(testDb, trip.id);
// Wikimedia pseudo-IDs contain characters that must round-trip encoded.
const placeId = 'coords:48.8,2.3';
testDb.prepare('UPDATE places SET image_url = ? WHERE id = ?')
.run(`/api/maps/place-photo/${encodeURIComponent(placeId)}/bytes`, place.id);
serveFilePath.mockReturnValue('/cache/abc.jpg');
expect(svc.getSharedPlacePhotoPath(token, placeId)).toBe('/cache/abc.jpg');
expect(serveFilePath).toHaveBeenCalledWith(placeId);
serveKey.mockReturnValue('abc.jpg');
expect(svc.getSharedPlacePhotoKey(token, placeId)).toBe('abc.jpg');
expect(serveKey).toHaveBeenCalledWith(placeId);
});
});