diff --git a/server/src/nest/maps/maps.controller.ts b/server/src/nest/maps/maps.controller.ts index 770f3c9c5..888d8d2c6 100644 --- a/server/src/nest/maps/maps.controller.ts +++ b/server/src/nest/maps/maps.controller.ts @@ -171,7 +171,7 @@ export class MapsController { @Get('place-photo/:placeId/bytes') async placePhotoBytes(@Param('placeId') placeId: string, @Res() res: Response): Promise { - const key = this.maps.photoBytesKey(placeId); + const key = await 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 diff --git a/server/src/nest/maps/maps.service.ts b/server/src/nest/maps/maps.service.ts index 057a08160..ecf6dc06b 100644 --- a/server/src/nest/maps/maps.service.ts +++ b/server/src/nest/maps/maps.service.ts @@ -529,7 +529,7 @@ export class MapsService { return this.getPlacePhoto(userId, placeId, lat, lng, name) as Promise; } - photoBytesKey(placeId: string): string | null { + photoBytesKey(placeId: string): Promise { return this.photoCache.serveKey(placeId); } @@ -1802,7 +1802,7 @@ export class MapsService { name?: string, ): Promise<{ photoUrl: string | null; attribution: string | null }> { // Disk cache hit — serve immediately, no Google call - const diskHit = this.photoCache.get(placeId); + const diskHit = await this.photoCache.get(placeId); if (diskHit) return { photoUrl: diskHit.photoUrl, attribution: diskHit.attribution }; // "No photo for this place" is an empty result, not a missing resource: a trip @@ -1828,7 +1828,7 @@ export class MapsService { // or timed out only for a few minutes. let providerFailed = false; - const fetchPromise = (async (): Promise<{ filePath: string; attribution: string | null } | null> => { + const fetchPromise = (async (): Promise<{ attribution: string | null } | null> => { await acquirePhotoFetchSlot(); try { const apiKey = this.getMapsKey(userId); @@ -1837,7 +1837,7 @@ export class MapsService { // (right-click) places and as a fallback when a Google place yields no photo, // so a place added via search still gets a marker image when Google returns // nothing. Returns null (without marking an error) so the caller decides. - const fetchWikimediaFallback = async (): Promise<{ filePath: string; attribution: string | null } | null> => { + const fetchWikimediaFallback = async (): Promise<{ attribution: string | null } | null> => { if (isNaN(lat) || isNaN(lng)) return null; try { const wiki = await this.fetchWikimediaPhoto(lat, lng, name); @@ -1851,7 +1851,7 @@ export class MapsService { } const bytes = Buffer.from(await imgRes.arrayBuffer()); const cached = await this.photoCache.put(placeId, bytes, wiki.attribution); - return { filePath: cached.filePath, attribution: cached.attribution }; + return { attribution: cached.attribution }; } catch { providerFailed = true; return null; @@ -1862,7 +1862,7 @@ export class MapsService { // key, request rejected, no photos, or a failed media download — so the // caller can fall back to Wikimedia; the misses that were Google's fault // flag providerFailed on the way out. - const fetchGooglePhoto = async (): Promise<{ filePath: string; attribution: string | null } | null> => { + const fetchGooglePhoto = async (): Promise<{ attribution: string | null } | null> => { if (!apiKey) return null; // Fetch details to get the photo name @@ -1926,7 +1926,7 @@ export class MapsService { console.error('Failed to persist photo URL to database:', dbErr); } - return { filePath: cached.filePath, attribution }; + return { attribution }; }; // Prefer the Google photo (higher quality); if Google yields nothing, fall diff --git a/server/src/nest/place-enrichment/place-enrichment.controller.ts b/server/src/nest/place-enrichment/place-enrichment.controller.ts index 0e6f7bf76..aeb39e30b 100644 --- a/server/src/nest/place-enrichment/place-enrichment.controller.ts +++ b/server/src/nest/place-enrichment/place-enrichment.controller.ts @@ -60,7 +60,7 @@ export class PlaceEnrichmentController { * local — no provider is contacted, so it needs no rate limit. */ @Get('credit/:key') - credit(@Param('key') key: string): { credit: string | null } { + credit(@Param('key') key: string): Promise<{ credit: string | null }> { return this.enrichment.credit(key); } } diff --git a/server/src/nest/place-enrichment/place-enrichment.service.ts b/server/src/nest/place-enrichment/place-enrichment.service.ts index 4f916679e..7633064f2 100644 --- a/server/src/nest/place-enrichment/place-enrichment.service.ts +++ b/server/src/nest/place-enrichment/place-enrichment.service.ts @@ -269,7 +269,7 @@ export class PlaceEnrichmentService { const placeId = req.placeId?.trim() || `coords:${req.lat}:${req.lng}`; const lang = req.lang; - const cached = this.readCache(placeId, lang); + const cached = await this.readCache(placeId, lang); if (cached) return cached; // One details lookup feeds all three halves: the pictures need its Commons @@ -318,8 +318,8 @@ export class PlaceEnrichmentService { * obligation does not end there — the place inspector reads this so a picture * that was chosen weeks ago still names whoever made it. */ - credit(key: string): { credit: string | null } { - return { credit: this.photoCache.get(key)?.attribution ?? null }; + async credit(key: string): Promise<{ credit: string | null }> { + return { credit: (await this.photoCache.get(key))?.attribution ?? null }; } /** @@ -492,7 +492,7 @@ export class PlaceEnrichmentService { credit: string | null, fetchBytes: () => Promise, ): Promise { - const hit = this.photoCache.get(key); + const hit = await this.photoCache.get(key); if (hit) return hit.photoUrl; const bytes = await withPhotoFetchSlot(fetchBytes); @@ -662,7 +662,7 @@ export class PlaceEnrichmentService { // ── Result cache ─────────────────────────────────────────────────────────── - private readCache(placeId: string, lang: string | undefined): CachedEnrichment | null { + private async readCache(placeId: string, lang: string | undefined): Promise { try { const row = this.database.get<{ payload_json: string; fetched_at: number }>( 'SELECT payload_json, fetched_at FROM place_details_cache WHERE place_id = ? AND lang = ? AND expanded = ?', @@ -679,7 +679,8 @@ export class PlaceEnrichmentService { // them. Treat any loss as a stale entry and rebuild, rather than serving // the survivors: filtering alone would leave the column empty for the rest // of the week for a place that has pictures perfectly available. - const photos = parsed.photos.filter((photo) => this.photoCache.get(photo.key)); + const checks = await Promise.all(parsed.photos.map((photo) => this.photoCache.get(photo.key))); + const photos = parsed.photos.filter((_, i) => checks[i]); if (photos.length !== parsed.photos.length) return null; const value: CachedEnrichment = { diff --git a/server/src/nest/place-photos/place-photo-cache.job.ts b/server/src/nest/place-photos/place-photo-cache.job.ts index 483691068..b15d5123d 100644 --- a/server/src/nest/place-photos/place-photo-cache.job.ts +++ b/server/src/nest/place-photos/place-photo-cache.job.ts @@ -20,13 +20,15 @@ export class PlacePhotoCacheJob implements OnApplicationBootstrap { onApplicationBootstrap(): void { if (!this.registrar.isEnabled()) return; // Run once on startup to reclaim orphans left over from before this sweeper existed. - this.sweep(); - this.registrar.register('place-photo-cache', '30 3 * * *', () => this.sweep()); + void this.sweep(); + this.registrar.register('place-photo-cache', '30 3 * * *', () => { + void this.sweep(); + }); } - sweep(): void { + async sweep(): Promise { try { - const removed = this.cache.sweepOrphans(); + const removed = await this.cache.sweepOrphans(); if (removed > 0) logInfo(`Place-photo cache cleanup: removed ${removed} orphaned file(s)/row(s)`); } catch (err: unknown) { logError(`Place-photo cache cleanup: ${err instanceof Error ? err.message : err}`); diff --git a/server/src/nest/place-photos/place-photo-cache.service.ts b/server/src/nest/place-photos/place-photo-cache.service.ts index de2139489..c228b8f7a 100644 --- a/server/src/nest/place-photos/place-photo-cache.service.ts +++ b/server/src/nest/place-photos/place-photo-cache.service.ts @@ -1,11 +1,9 @@ -import { Injectable, OnModuleInit } from '@nestjs/common'; +import { Injectable } from '@nestjs/common'; import { Jimp, JimpMime } from 'jimp'; import crypto from 'node:crypto'; -import fs from 'node:fs'; -import fsPromises from 'node:fs/promises'; -import path from 'node:path'; +import { Readable } from 'node:stream'; import { DatabaseService } from '../database/database.service'; -import { RuntimeEnvService } from '../app-config/runtime-env.service'; +import { StorageService } from '../storage/storage.service'; // How long a "no photo for this place" answer stays remembered. Nothing about it // changes until a photo appears upstream, so it is worth keeping: without it every @@ -31,7 +29,6 @@ const JPEG_QUALITY = 80; interface CachedPhoto { photoUrl: string; - filePath: string; attribution: string | null; } @@ -44,39 +41,26 @@ interface CachedPhoto { * simultaneous requests for the same uncached place would both call the * provider. That is the reason the nightly sweep (PlacePhotoCacheJob) injects * this container singleton rather than constructing its own. + * + * Bytes live in the 'photos-google' storage category as '.jpg' names. + * The registry's mode-aware prefix reproduces both TREK_PLACE_PHOTO_DIR + * layouts (unset: uploads/photos/google; set: that dir, bare keys), so the + * cache itself is mode-agnostic. */ @Injectable() -export class PlacePhotoCacheService implements OnModuleInit { +export class PlacePhotoCacheService { /** Provider calls that failed recently — in memory only, so a restart retries. */ private readonly recentFailures = new Map(); /** In-flight dedup: stops a stampede when several requests miss the same placeId at once. */ - private readonly inFlight = new Map>(); - /** placeIds whose file was confirmed on disk this session — saves an existsSync per hit. */ + private readonly inFlight = new Map>(); + /** placeIds whose object was confirmed in storage this session — saves a stat per hit. */ private readonly knownOnDisk = new Set(); constructor( private readonly db: DatabaseService, - private readonly env: RuntimeEnvService, + private readonly storage: StorageService, ) {} - /** - * Overridable for tests (mirrors the TREK_DB_FILE seam) so the suite never - * touches the real uploads tree. Read per call rather than frozen at import: - * the tests that set TREK_PLACE_PHOTO_DIR do so after this module loads. - */ - private get photoDir(): string { - return this.env.env().paths.placePhotoDir || path.join(__dirname, '../../../uploads/photos/google'); - } - - /** Ensure the upload dir exists once at startup, so put() never needs a sync FS call. */ - onModuleInit(): void { - try { - fs.mkdirSync(this.photoDir, { recursive: true }); - } catch { - /* already exists */ - } - } - 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 → '_') @@ -84,15 +68,11 @@ export class PlacePhotoCacheService implements OnModuleInit { return `${hash}.jpg`; } - private filePath(placeId: string): string { - return path.join(this.photoDir, this.fileName(placeId)); - } - private proxyUrl(placeId: string): string { return `/api/maps/place-photo/${encodeURIComponent(placeId)}/bytes`; } - get(placeId: string): CachedPhoto | null { + async get(placeId: string): Promise { const row = this.db.get<{ attribution: string | null }>( 'SELECT attribution FROM google_place_photo_meta WHERE place_id = ? AND error_at IS NULL', placeId, @@ -100,19 +80,17 @@ export class PlacePhotoCacheService implements OnModuleInit { if (!row) return null; - const fp = this.filePath(placeId); - if (!this.knownOnDisk.has(placeId)) { - // First time this placeId is checked this session — verify the file exists on disk. + // First time this placeId is checked this session — verify the object exists. // (Guards against volume wipes or manual deletion between server restarts.) - if (!fs.existsSync(fp)) { + if (!(await this.storage.exists('photos-google', this.fileName(placeId)))) { this.db.run('DELETE FROM google_place_photo_meta WHERE place_id = ?', placeId); return null; } this.knownOnDisk.add(placeId); } - return { photoUrl: this.proxyUrl(placeId), filePath: fp, attribution: row.attribution }; + return { photoUrl: this.proxyUrl(placeId), attribution: row.attribution }; } getErrored(placeId: string): boolean { @@ -174,12 +152,8 @@ export class PlacePhotoCacheService implements OnModuleInit { } async put(placeId: string, bytes: Buffer, attribution: string | null): Promise { - const fp = this.filePath(placeId); - const tmp = fp + '.tmp'; - const resized = await this.downscale(bytes); - await fsPromises.writeFile(tmp, resized); - await fsPromises.rename(tmp, fp); + await this.storage.put('photos-google', this.fileName(placeId), Readable.from(resized)); this.knownOnDisk.add(placeId); this.recentFailures.delete(placeId); @@ -189,19 +163,14 @@ export class PlacePhotoCacheService implements OnModuleInit { placeId, attribution, Date.now(), ); - return { photoUrl: this.proxyUrl(placeId), filePath: fp, attribution }; + return { photoUrl: this.proxyUrl(placeId), attribution }; } - getInFlight( - placeId: string, - ): Promise<{ filePath: string; attribution: string | null } | null> | undefined { + getInFlight(placeId: string): Promise<{ attribution: string | null } | null> | undefined { return this.inFlight.get(placeId); } - setInFlight( - placeId: string, - promise: Promise<{ filePath: string; attribution: string | null } | null>, - ): void { + setInFlight(placeId: string, promise: Promise<{ attribution: string | null } | null>): void { this.inFlight.set(placeId, promise); promise .finally(() => this.inFlight.delete(placeId)) @@ -210,22 +179,16 @@ export class PlacePhotoCacheService implements OnModuleInit { }); } - serveFilePath(placeId: string): string | null { - if (this.knownOnDisk.has(placeId)) return this.filePath(placeId); - const fp = this.filePath(placeId); - if (!fs.existsSync(fp)) return null; - this.knownOnDisk.add(placeId); - 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. + * the object is missing. */ - serveKey(placeId: string): string | null { - return this.serveFilePath(placeId) ? this.fileName(placeId) : null; + async serveKey(placeId: string): Promise { + const name = this.fileName(placeId); + if (this.knownOnDisk.has(placeId)) return name; + if (!(await this.storage.exists('photos-google', name))) return null; + this.knownOnDisk.add(placeId); + return name; } /** @@ -249,12 +212,10 @@ export class PlacePhotoCacheService implements OnModuleInit { return !!row; } - private deleteEntry(placeId: string): void { - try { - fs.unlinkSync(this.filePath(placeId)); - } catch { + private async deleteEntry(placeId: string): Promise { + await this.storage.delete('photos-google', this.fileName(placeId)).catch(() => { /* already gone */ - } + }); this.db.run('DELETE FROM google_place_photo_meta WHERE place_id = ?', placeId); this.knownOnDisk.delete(placeId); } @@ -263,41 +224,37 @@ export class PlacePhotoCacheService implements OnModuleInit { * Drop a cache entry if no place references it anymore. Called after a place delete * for prompt reclamation; the nightly sweep is the catch-all for every other path. */ - removeIfUnreferenced(placeId: string): void { + async removeIfUnreferenced(placeId: string): Promise { if (this.isReferenced(placeId)) return; - this.deleteEntry(placeId); + await this.deleteEntry(placeId); } /** * Reclaim orphaned cache files + meta rows. Runs on startup and nightly (scheduler). - * Two passes: (1) meta rows no place references; (2) stray .jpg files with no meta row. + * Two passes: (1) meta rows no place references; (2) stray .jpg objects with no meta row. */ - sweepOrphans(): number { + async sweepOrphans(): Promise { let removed = 0; const rows = this.db.all<{ place_id: string }>('SELECT place_id FROM google_place_photo_meta'); const keepFiles = new Set(); for (const { place_id } of rows) { if (this.isReferenced(place_id)) { - keepFiles.add(`${crypto.createHash('sha1').update(place_id).digest('hex')}.jpg`); + keepFiles.add(this.fileName(place_id)); } else { - this.deleteEntry(place_id); + await this.deleteEntry(place_id); removed++; } } - // Pass 2: files on disk that no surviving meta row maps to (e.g. left over from a - // crash between writeFile and the DB upsert, or a meta row deleted out-of-band). - let entries: string[]; - try { - entries = fs.readdirSync(this.photoDir); - } catch { - entries = []; - } - for (const entry of entries) { - if (!entry.endsWith('.jpg') || keepFiles.has(entry)) continue; + // Pass 2: objects that no surviving meta row maps to (e.g. left over from a + // crash between the object write and the DB upsert, or a meta row deleted + // out-of-band). list() recurses where the old readdirSync didn't — skip + // nested keys so a subdirectory of a relocated cache dir is never swept. + for await (const stat of this.storage.list('photos-google')) { + if (stat.key.includes('/') || !stat.key.endsWith('.jpg') || keepFiles.has(stat.key)) continue; try { - fs.unlinkSync(path.join(this.photoDir, entry)); + await this.storage.delete('photos-google', stat.key); removed++; } catch { /* race */ diff --git a/server/src/nest/place-photos/place-photos.module.ts b/server/src/nest/place-photos/place-photos.module.ts index 90c7385b3..b5a289621 100644 --- a/server/src/nest/place-photos/place-photos.module.ts +++ b/server/src/nest/place-photos/place-photos.module.ts @@ -3,6 +3,7 @@ import { PlacePhotoCacheService } from './place-photo-cache.service'; import { PlacePhotoCacheJob } from './place-photo-cache.job'; import { AppConfigModule } from '../app-config/app-config.module'; import { SchedulingModule } from '../scheduling/scheduling.module'; +import { StorageModule } from '../storage/storage.module'; /** The marker-photo cache. No controller of its own — maps serves the bytes, * places and share read through it, and PlacePhotoCacheJob sweeps it nightly. @@ -11,7 +12,7 @@ import { SchedulingModule } from '../scheduling/scheduling.module'; * imported explicitly because @Global only reaches modules that are in the * graph — which a single-domain e2e TestingModule is not. */ @Module({ - imports: [AppConfigModule, SchedulingModule], + imports: [AppConfigModule, SchedulingModule, StorageModule], providers: [PlacePhotoCacheService, PlacePhotoCacheJob], exports: [PlacePhotoCacheService], }) diff --git a/server/src/nest/places/places.controller.ts b/server/src/nest/places/places.controller.ts index 1d97f3c22..09bef3148 100644 --- a/server/src/nest/places/places.controller.ts +++ b/server/src/nest/places/places.controller.ts @@ -272,7 +272,7 @@ export class PlacesController { @Post('bulk-delete') @HttpCode(200) // Express answers bulk-delete with res.json (200), unlike the 201 imports. - bulkDelete( + async bulkDelete( @CurrentUser() user: User, @Param('tripId') tripId: string, @Body() body: PlaceBulkDeleteDto, @@ -293,7 +293,7 @@ export class PlacesController { for (const id of scoped) this.places.onDeleted(id); // Read the linked expenses before the delete — afterwards the link is gone (#1298). const expenseIds = this.places.linkedExpenseIds(tripId, scoped); - const deleted = this.places.removeMany(tripId, ids); + const deleted = await this.places.removeMany(tripId, ids); for (const id of deleted) { this.places.broadcast(tripId, 'place:deleted', { placeId: id }, socketId); } @@ -452,7 +452,7 @@ export class PlacesController { @Delete(':id') @UseGuards(TripAccessGuard) @RequirePermission('place_edit') - remove(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Headers('x-socket-id') socketId?: string) { + async remove(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string, @Headers('x-socket-id') socketId?: string) { // Scope the id to the trip before the hook (see bulkDelete), then sync the // journey ahead of the actual delete. if (!this.places.get(tripId, id)) { @@ -460,7 +460,7 @@ export class PlacesController { } this.places.onDeleted(Number(id)); const expenseIds = this.places.linkedExpenseIds(tripId, [id]); - if (!this.places.remove(tripId, id)) { + if (!(await this.places.remove(tripId, id))) { throw new HttpException({ error: 'Place not found' }, 404); } this.places.broadcast(tripId, 'place:deleted', { placeId: Number(id) }, socketId); diff --git a/server/src/nest/places/places.helpers.ts b/server/src/nest/places/places.helpers.ts index e55f66124..47ed4bf98 100644 --- a/server/src/nest/places/places.helpers.ts +++ b/server/src/nest/places/places.helpers.ts @@ -101,13 +101,13 @@ export interface KmlImportOptions { // Reclaim a deleted place's cached marker photo if nothing else references it. // The cache key is the Google place_id, or — for coordinate-only places — the // pseudo-id embedded in the stored proxy URL (/api/maps/place-photo/{id}/bytes). -export function reclaimPhotoCache(cache: PlacePhotoCacheService, googlePlaceId: string | null, imageUrl: string | null): void { +export async function reclaimPhotoCache(cache: PlacePhotoCacheService, googlePlaceId: string | null, imageUrl: string | null): Promise { const candidates = new Set(); if (googlePlaceId) candidates.add(googlePlaceId); const m = imageUrl?.match(/^\/api\/maps\/place-photo\/(.+)\/bytes$/); if (m) { try { candidates.add(decodeURIComponent(m[1])); } catch { /* malformed url */ } } for (const id of candidates) { - try { cache.removeIfUnreferenced(id); } catch { /* best-effort */ } + try { await cache.removeIfUnreferenced(id); } catch { /* best-effort */ } } } diff --git a/server/src/nest/places/places.mcp.ts b/server/src/nest/places/places.mcp.ts index 39c88127f..2de67b9a5 100644 --- a/server/src/nest/places/places.mcp.ts +++ b/server/src/nest/places/places.mcp.ts @@ -234,7 +234,7 @@ export class PlacesMcp { try { this.journey.onPlaceDeleted(placeId); } catch { /* non-fatal */ } // sync journeys before the row is gone // The link is gone once the place is, so read it first (#1298). const expenseIds = this.places.linkedExpenseIds(tripId, [placeId]); - const deleted = this.places.remove(String(tripId), String(placeId)); + const deleted = await this.places.remove(String(tripId), String(placeId)); if (!deleted) return { content: [{ type: 'text' as const, text: 'Place not found.' }], isError: true }; this.guards.safeBroadcast(tripId, 'place:deleted', { placeId }); for (const itemId of expenseIds) this.guards.safeBroadcast(tripId, 'budget:deleted', { itemId }); @@ -346,7 +346,7 @@ export class PlacesMcp { } // The link is gone once the places are, so read it first (#1298). const expenseIds = this.places.linkedExpenseIds(tripId, scoped); - const deleted = this.places.removeMany(String(tripId), placeIds); + const deleted = await this.places.removeMany(String(tripId), placeIds); for (const id of deleted) this.guards.safeBroadcast(tripId, 'place:deleted', { placeId: id }); for (const itemId of expenseIds) this.guards.safeBroadcast(tripId, 'budget:deleted', { itemId }); return ok({ deleted, count: deleted.length }); diff --git a/server/src/nest/places/places.service.ts b/server/src/nest/places/places.service.ts index ceef2576d..0e06d6869 100644 --- a/server/src/nest/places/places.service.ts +++ b/server/src/nest/places/places.service.ts @@ -357,7 +357,7 @@ export class PlacesService { return rows.map(r => r.id); } - remove(tripId: string, placeId: string): boolean { + async remove(tripId: string, placeId: string): Promise { const place = this.dbs.get<{ google_place_id: string | null; image_url: string | null }>( 'SELECT google_place_id, image_url FROM places WHERE id = ? AND trip_id = ?', placeId, tripId, ); @@ -369,12 +369,12 @@ export class PlacesService { this.dbs.run('DELETE FROM budget_items WHERE trip_id = ? AND place_id = ?', tripId, placeId); this.dbs.run('DELETE FROM places WHERE id = ?', placeId); }); - reclaimPhotoCache(this.photoCache, place.google_place_id, place.image_url); + await reclaimPhotoCache(this.photoCache, place.google_place_id, place.image_url); reclaimPlaceImage(place.image_url); return true; } - removeMany(tripId: string, ids: number[]): number[] { + async removeMany(tripId: string, ids: number[]): Promise { if (ids.length === 0) return []; const selectStmt = this.dbs.prepare('SELECT google_place_id, image_url FROM places WHERE id = ? AND trip_id = ?'); const deleteStmt = this.dbs.prepare('DELETE FROM places WHERE id = ?'); @@ -393,7 +393,7 @@ export class PlacesService { }); // Reclaim after the transaction commits so isReferenced() sees the final place set. for (const row of reclaimable) { - reclaimPhotoCache(this.photoCache, row.google_place_id, row.image_url); + await reclaimPhotoCache(this.photoCache, row.google_place_id, row.image_url); reclaimPlaceImage(row.image_url); } return deleted; diff --git a/server/src/nest/share/share.controller.ts b/server/src/nest/share/share.controller.ts index dd5ad41c6..842482ef5 100644 --- a/server/src/nest/share/share.controller.ts +++ b/server/src/nest/share/share.controller.ts @@ -97,7 +97,7 @@ export class SharedController { */ @Get(':token/place-photo/:placeId/bytes') async placePhotoBytes(@Param('token') token: string, @Param('placeId') placeId: string, @Res() res: Response): Promise { - const key = this.share.getSharedPlacePhotoKey(token, placeId); + const key = await this.share.getSharedPlacePhotoKey(token, placeId); if (!key) { this.emptyPhoto(res); return; diff --git a/server/src/nest/share/share.service.ts b/server/src/nest/share/share.service.ts index c6cb641bd..e975164a7 100644 --- a/server/src/nest/share/share.service.ts +++ b/server/src/nest/share/share.service.ts @@ -305,7 +305,7 @@ export class ShareService { * Returns null — never throws — so the caller answers a plain miss, * mirroring the authenticated bytes endpoint. */ - getSharedPlacePhotoKey(token: string, placeId: string): string | null { + async getSharedPlacePhotoKey(token: string, placeId: string): Promise { 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, diff --git a/server/tests/e2e/place-enrichment.e2e.test.ts b/server/tests/e2e/place-enrichment.e2e.test.ts index c88dd2f9d..a23305f44 100644 --- a/server/tests/e2e/place-enrichment.e2e.test.ts +++ b/server/tests/e2e/place-enrichment.e2e.test.ts @@ -190,8 +190,8 @@ describe('Place enrichment e2e (real auth guard + real validation pipe)', () => it('200 with the stored credit for a cached picture', async () => { const photoCache = app.get(PlacePhotoCacheService); - vi.spyOn(photoCache, 'get').mockReturnValueOnce({ - photoUrl: '/x', filePath: '/tmp/x', attribution: 'Alice · CC BY-SA 4.0', + vi.spyOn(photoCache, 'get').mockResolvedValueOnce({ + photoUrl: '/x', attribution: 'Alice · CC BY-SA 4.0', }); const res = await request(server) diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index 77a1de006..71eabecba 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -75,6 +75,7 @@ import { AddonsService } from '../../src/nest/addons/addons.service'; import { notificationsStub } from './notifications'; import { EphemeralTokenService } from '../../src/nest/auth/ephemeral-token.service'; import { AllowedFileTypesService } from '../../src/nest/files/allowed-file-types.service'; +import { makeStorageFixture } from './storage-fixture'; /** * Hand-wired counterpart of the boot-time discovery in McpRegistryService, @@ -112,7 +113,7 @@ export function createMcpTestRegistry(): McpRegistry { const collabService = new CollabService(dbService, permissionsService, realtimeService, notificationsStub()); // Exactly one instance, shared by maps, places and share: its stampede guard // and its on-disk set only work if all three readers see the same maps. - const placePhotoCache = new PlacePhotoCacheService(dbService, new RuntimeEnvService()); + const placePhotoCache = new PlacePhotoCacheService(dbService, makeStorageFixture('photos/google/').storage); const mapsService = new MapsService(dbService, placePhotoCache); const journeyDomain = new JourneyDomainService(dbService, realtimeService, new TrekPhotosRepository(dbService)); // The last three were previously omitted, which left them `undefined` at diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index 9cd78c18a..bb11449d8 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -70,6 +70,7 @@ import { UnsplashService } from '../../src/nest/unsplash/unsplash.service'; import { PlacePhotoCacheService } from '../../src/nest/place-photos/place-photo-cache.service'; import { TrekPhotosRepository } from '../../src/nest/photos/trek-photos.repository'; import { RuntimeEnvService } from '../../src/nest/app-config/runtime-env.service'; +import { makeStorageFixture } from './storage-fixture'; /** * Hand-wired counterpart of the PluginsModule DI graph for no-Nest tests @@ -95,7 +96,7 @@ export function createPluginRpcHostFactory(dbs: DatabaseService): PluginRpcHostF const collab = new CollabService(dbs, permissions, realtime, notificationsStub()); const vacay = new VacayService(dbs, realtime, notificationsStub()); const days = new DaysService(dbs, permissions, realtime, queryHelpers); - const photoCache = new PlacePhotoCacheService(dbs, new RuntimeEnvService()); + const photoCache = new PlacePhotoCacheService(dbs, makeStorageFixture('photos/google/').storage); const unsplash = new UnsplashService(dbs, new RuntimeEnvService()); const journey = new JourneyDomainService(dbs, realtime, new TrekPhotosRepository(dbs)); const places = new PlacesService(dbs, permissions, realtime, new MapsService(dbs, photoCache), queryHelpers, unsplash, photoCache, journey); diff --git a/server/tests/helpers/storage-fixture.ts b/server/tests/helpers/storage-fixture.ts new file mode 100644 index 000000000..7123b4fe7 --- /dev/null +++ b/server/tests/helpers/storage-fixture.ts @@ -0,0 +1,41 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { LocalDriver } from '../../src/nest/storage/drivers/local.driver'; +import type { StorageRegistryService, ResolvedCategory } from '../../src/nest/storage/storage-registry.service'; +import { StorageService } from '../../src/nest/storage/storage.service'; + +/** + * A real StorageService facade over a stub registry + a REAL LocalDriver + * rooted in a throwaway mkdtemp dir (the storage.service.test.ts makeFixture + * pattern, shared). Every category resolves to the same driver + keyPrefix — + * exactly what a single-category service test needs, and the two + * TREK_PLACE_PHOTO_DIR modes are just two prefixes ('photos/google/' vs ''). + * The real mode flip lives in the registry and is pinned by its own tests. + */ +export interface StorageFixture { + storage: StorageService; + /** Driver root — objects land at root/. */ + root: string; + cleanup(): void; +} + +export function makeStorageFixture(keyPrefix: string): StorageFixture { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'trek-storage-fx-')); + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'trek-storage-fx-tmp-')); + const driver = new LocalDriver({ id: 'fixture-local', root }); + driver.init({ cleanSpool: true }); + const registry = { + resolve: (): ResolvedCategory => ({ driver, keyPrefix, backendName: 'fixture-local' }), + tempDir: () => tempDir, + replicaFailures: () => [], + } as unknown as StorageRegistryService; + return { + storage: new StorageService(registry), + root, + cleanup: () => { + fs.rmSync(root, { recursive: true, force: true }); + fs.rmSync(tempDir, { recursive: true, force: true }); + }, + }; +} diff --git a/server/tests/integration/share.test.ts b/server/tests/integration/share.test.ts index 69a993214..6cccb1eb6 100644 --- a/server/tests/integration/share.test.ts +++ b/server/tests/integration/share.test.ts @@ -51,13 +51,27 @@ import { createUser, createTrip, addTripMember, createDay, createPlace, createDa import { authCookie } from '../helpers/auth'; import { PlacePhotoCacheService } from '../../src/nest/place-photos/place-photo-cache.service'; import { DatabaseService } from '../../src/nest/database/database.service'; -import { RuntimeEnvService } from '../../src/nest/app-config/runtime-env.service'; import { db as sharedDb } from '../../src/db/database'; +import { LocalDriver } from '../../src/nest/storage/drivers/local.driver'; +import { StorageService } from '../../src/nest/storage/storage.service'; +import type { StorageRegistryService, ResolvedCategory } from '../../src/nest/storage/storage-registry.service'; +import { DEFAULT_UPLOADS_ROOT, GLOBAL_TEMP_DIR } from '../../src/nest/storage/storage-paths'; // A real instance over the same connection the app uses — these cases write a -// cache entry and then read it back through the HTTP route. -const placePhotoCache = new PlacePhotoCacheService(new DatabaseService(sharedDb), new RuntimeEnvService()); +// cache entry and then read it back through the HTTP route, so the stub +// storage must be rooted where the app's registry serves 'photos-google' +// from (mode A: uploads/photos/google). +const uploadsDriver = new LocalDriver({ id: 'share-test-local', root: DEFAULT_UPLOADS_ROOT }); +uploadsDriver.init({ ensurePrefixes: ['photos/google/'], cleanSpool: false }); +const testStorage = new StorageService({ + resolve: (): ResolvedCategory => ({ driver: uploadsDriver, keyPrefix: 'photos/google/', backendName: 'share-test-local' }), + tempDir: () => GLOBAL_TEMP_DIR, + replicaFailures: () => [], +} as unknown as StorageRegistryService); +const placePhotoCache = new PlacePhotoCacheService(new DatabaseService(sharedDb), testStorage); import fs from 'node:fs'; +import path from 'node:path'; +import crypto from 'node:crypto'; let nestApp: INestApplication; let app: Application; @@ -561,8 +575,8 @@ describe('Shared trip — place photos in shared links (issue #1100)', () => { it('SHARE-018 — public proxy streams cached bytes for a valid token + place (no cookie)', async () => { const { token } = await setupSharedPlaceWithPhoto(); - const cached = await placePhotoCache.put(PLACE_ID, photoBytes, null); - cachedFilePath = cached.filePath; + await placePhotoCache.put(PLACE_ID, photoBytes, null); + cachedFilePath = path.join(DEFAULT_UPLOADS_ROOT, 'photos/google', `${crypto.createHash('sha1').update(PLACE_ID).digest('hex')}.jpg`); const res = await request(app).get(`/api/shared/${token}/place-photo/${encodeURIComponent(PLACE_ID)}/bytes`); expect(res.status).toBe(200); @@ -602,8 +616,8 @@ describe('Shared trip — place photos in shared links (issue #1100)', () => { .post(`/api/trips/${trip.id}/share-link`) .set('Cookie', authCookie(user.id)) .send({ share_map: false }); - const cached = await placePhotoCache.put(PLACE_ID, photoBytes, null); - cachedFilePath = cached.filePath; + await placePhotoCache.put(PLACE_ID, photoBytes, null); + cachedFilePath = path.join(DEFAULT_UPLOADS_ROOT, 'photos/google', `${crypto.createHash('sha1').update(PLACE_ID).digest('hex')}.jpg`); const res = await request(app).get(`/api/shared/${token}/place-photo/${encodeURIComponent(PLACE_ID)}/bytes`); expect(res.status).toBe(204); diff --git a/server/tests/unit/nest/cache-sweep-jobs.test.ts b/server/tests/unit/nest/cache-sweep-jobs.test.ts index 00aacfb32..99a3e1b37 100644 --- a/server/tests/unit/nest/cache-sweep-jobs.test.ts +++ b/server/tests/unit/nest/cache-sweep-jobs.test.ts @@ -80,10 +80,10 @@ describe('PlacePhotoCacheJob', () => { expect(registrar.register).toHaveBeenCalledWith('place-photo-cache', '30 3 * * *', expect.any(Function)); }); - it('CSJOB-006 — logs only when something was removed', () => { - make(0).job.sweep(); + it('CSJOB-006 — logs only when something was removed', async () => { + await make(0).job.sweep(); expect(logMock.logInfo).not.toHaveBeenCalled(); - make(3).job.sweep(); + await make(3).job.sweep(); expect(logMock.logInfo).toHaveBeenCalledWith('Place-photo cache cleanup: removed 3 orphaned file(s)/row(s)'); }); diff --git a/server/tests/unit/nest/collections.service.test.ts b/server/tests/unit/nest/collections.service.test.ts index 06d5b17f6..c9be4f58a 100644 --- a/server/tests/unit/nest/collections.service.test.ts +++ b/server/tests/unit/nest/collections.service.test.ts @@ -53,14 +53,14 @@ import { PermissionsService } from '../../../src/nest/permissions/permissions.se import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { CollectionsService } from '../../../src/nest/collections/collections.service'; import { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service'; -import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; +import { makeStorageFixture } from '../../helpers/storage-fixture'; import { PLACE_IMAGES_DIR } from '../../../src/nest/places/place-image'; import { notificationsStub } from '../../helpers/notifications'; const svc = new CollectionsService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService(), notificationsStub(notifSend)); // The real cache: these cases assert what removeIfUnreferenced actually does // about collection_places (#1081), so a stub would assert nothing. -const photoCache = new PlacePhotoCacheService(new DatabaseService(testDb), new RuntimeEnvService()); +const photoCache = new PlacePhotoCacheService(new DatabaseService(testDb), makeStorageFixture('photos/google/').storage); const removeIfUnreferenced = (id: string) => photoCache.removeIfUnreferenced(id); function clearCollections() { @@ -501,22 +501,22 @@ describe('owner_id semantics', () => { // ── Photo-cache guard ──────────────────────────────────────────────────────── describe('photo-cache widening', () => { - it('COLLECTIONS-SVC-042: a collection_places row keeps a photo no places row references', () => { + it('COLLECTIONS-SVC-042: a collection_places row keeps a photo no places row references', async () => { const u = createUser(testDb).user; const col = svc.createCollection(u.id, { name: 'Photos' }); // cache meta for place_id 'gp-x', referenced ONLY by a collection_places row. testDb.prepare('INSERT INTO google_place_photo_meta (place_id, attribution, fetched_at) VALUES (?, ?, ?)').run('gp-x', null, Date.now()); svc.savePlace(u.id, { collection_id: col.id, name: 'Cached', google_place_id: 'gp-x' }); - removeIfUnreferenced('gp-x'); // would evict if isReferenced ignored collection_places + await removeIfUnreferenced('gp-x'); // would evict if isReferenced ignored collection_places const meta = testDb.prepare('SELECT 1 FROM google_place_photo_meta WHERE place_id = ?').get('gp-x'); expect(meta).toBeDefined(); }); - it('COLLECTIONS-SVC-043: an unreferenced photo is still reclaimable', () => { + it('COLLECTIONS-SVC-043: an unreferenced photo is still reclaimable', async () => { testDb.prepare('INSERT INTO google_place_photo_meta (place_id, attribution, fetched_at) VALUES (?, ?, ?)').run('gp-orphan', null, Date.now()); - removeIfUnreferenced('gp-orphan'); + await removeIfUnreferenced('gp-orphan'); const meta = testDb.prepare('SELECT 1 FROM google_place_photo_meta WHERE place_id = ?').get('gp-orphan'); expect(meta).toBeUndefined(); }); diff --git a/server/tests/unit/nest/maps.controller.test.ts b/server/tests/unit/nest/maps.controller.test.ts index 0de4f5996..f5f90535d 100644 --- a/server/tests/unit/nest/maps.controller.test.ts +++ b/server/tests/unit/nest/maps.controller.test.ts @@ -263,7 +263,7 @@ describe('MapsController (parity with the legacy /api/maps route)', () => { // from #1727 — an uncached photo answers 204 with an empty body instead. it('204 without a body when the photo is not cached', async () => { const res = makeRes(); - await makeController({ photoBytesKey: () => null }).placePhotoBytes('p1', res); + await makeController({ photoBytesKey: async () => null }).placePhotoBytes('p1', res); expect(res.status).toHaveBeenCalledWith(204); expect(res.end).toHaveBeenCalled(); expect(res.json).not.toHaveBeenCalled(); @@ -274,7 +274,7 @@ describe('MapsController (parity with the legacy /api/maps route)', () => { const stream = makeStream(); getStream.mockResolvedValue({ stream, stat: { key: 'photos/google/abc.jpg', size: 5, mtimeMs: 0 } }); const res = makeRes(); - await makeController({ photoBytesKey: () => 'abc.jpg' }).placePhotoBytes('p1', res); + await makeController({ photoBytesKey: async () => 'abc.jpg' }).placePhotoBytes('p1', res); expect(res.set).toHaveBeenCalledWith('Cache-Control', 'public, max-age=2592000, immutable'); expect(res.type).toHaveBeenCalledWith('image/jpeg'); expect(getStream).toHaveBeenCalledWith('photos-google', 'abc.jpg'); @@ -284,7 +284,7 @@ describe('MapsController (parity with the legacy /api/maps route)', () => { 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(); - await makeController({ photoBytesKey: () => 'abc.jpg' }).placePhotoBytes('p1', res); + await makeController({ photoBytesKey: async () => '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 @@ -298,7 +298,7 @@ describe('MapsController (parity with the legacy /api/maps route)', () => { 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(); - await makeController({ photoBytesKey: () => 'abc.jpg' }).placePhotoBytes('p1', res); + await makeController({ photoBytesKey: async () => 'abc.jpg' }).placePhotoBytes('p1', res); onError(); expect(res.status).toHaveBeenCalledWith(204); expect(res.end).toHaveBeenCalled(); @@ -311,7 +311,7 @@ describe('MapsController (parity with the legacy /api/maps route)', () => { getStream.mockResolvedValue({ stream, stat: { key: 'photos/google/abc.jpg', size: 5, mtimeMs: 0 } }); const res = makeRes(); (res as { headersSent: boolean }).headersSent = true; - await makeController({ photoBytesKey: () => 'abc.jpg' }).placePhotoBytes('p1', res); + await makeController({ photoBytesKey: async () => 'abc.jpg' }).placePhotoBytes('p1', res); onError(); expect(res.status).not.toHaveBeenCalled(); expect(res.end).not.toHaveBeenCalled(); diff --git a/server/tests/unit/nest/maps.service.test.ts b/server/tests/unit/nest/maps.service.test.ts index 533a3cf61..1142f9444 100644 --- a/server/tests/unit/nest/maps.service.test.ts +++ b/server/tests/unit/nest/maps.service.test.ts @@ -1887,9 +1887,8 @@ describe('getPlacePhoto (fetch stubbed)', () => { it('MAPS-043b: returns cached photo when disk cache returns a hit', async () => { const placeId = `coords:cache-test-${Date.now()}`; const cachedUrl = `/api/maps/place-photo/${encodeURIComponent(placeId)}/bytes`; - mockCacheGet.mockReturnValue({ + mockCacheGet.mockResolvedValue({ photoUrl: cachedUrl, - filePath: `/tmp/${placeId}.jpg`, attribution: null, }); const fetchMock = vi.fn(); diff --git a/server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts b/server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts index bc65ad5c9..9414b21a0 100644 --- a/server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts +++ b/server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts @@ -1,31 +1,28 @@ /** - * Unit tests for placePhotoCache — PPC-001 through PPC-012. + * Unit tests for PlacePhotoCacheService — PPC-001 through PPC-014. * Covers the downscale guard in put(), removeIfUnreferenced(), sweepOrphans(), - * and the two negative-cache windows. - * Uses a real in-memory SQLite DB and a throwaway temp upload dir - * (TREK_PLACE_PHOTO_DIR) so the real uploads tree is never touched. + * the two negative-cache windows, and the serveKey/get miss handling. + * + * Slice 4: the cache is fully async and addresses storage as + * ('photos-google', '.jpg'). The whole suite runs in BOTH registry + * modes — mode A (TREK_PLACE_PHOTO_DIR unset: uploads-local backend, + * 'photos/google/' prefix) and mode B (dir set: place-photos-local backend, + * bare keys) — via the shared stub-registry fixture. The real mode flip is + * pinned by the storage-registry tests; here the two prefixes prove the cache + * itself is mode-agnostic. */ import { describe, it, expect, beforeAll, beforeEach, afterAll, vi } from 'vitest'; import path from 'node:path'; import fs from 'node:fs'; -import os from 'node:os'; import crypto from 'node:crypto'; import { Jimp, JimpMime } from 'jimp'; -import Database from 'better-sqlite3'; -// Throwaway upload dir, hoisted with the DB below: the service is a static -// import now, so vi.mock's factory runs before any top-level statement. -const { TMP_DIR, testDb } = vi.hoisted(() => { - const nodeFs = require('node:fs'); - const nodePath = require('node:path'); - const nodeOs = require('node:os'); +const { testDb } = vi.hoisted(() => { const Db = require('better-sqlite3'); - const dir = nodeFs.mkdtempSync(nodePath.join(nodeOs.tmpdir(), 'ppc-')); - process.env.TREK_PLACE_PHOTO_DIR = dir; - return { TMP_DIR: dir, testDb: new Db(':memory:') }; + return { testDb: new Db(':memory:') }; }); -// Minimal real DB with just the tables the cache touches. isReferenced now +// Minimal real DB with just the tables the cache touches. isReferenced // UNIONs collection_places (#1081 photo-cache fix), so the bare fixture must // declare it too or the reference check would throw "no such table". testDb.exec(` @@ -49,190 +46,238 @@ testDb.exec(` vi.mock('../../../../src/db/database', () => ({ db: testDb })); -function filePathFor(placeId: string): string { - const hash = crypto.createHash('sha1').update(placeId).digest('hex'); - return path.join(TMP_DIR, `${hash}.jpg`); -} +import { PlacePhotoCacheService } from '../../../../src/nest/place-photos/place-photo-cache.service'; +import { DatabaseService } from '../../../../src/nest/database/database.service'; +import { makeStorageFixture, type StorageFixture } from '../../../helpers/storage-fixture'; async function makeJpeg(width: number, height: number): Promise { const img = new Jimp({ width, height, color: 0xff0000ff }); return img.getBuffer(JimpMime.jpeg, { quality: 80 }); } -import { PlacePhotoCacheService } from '../../../../src/nest/place-photos/place-photo-cache.service'; -import { DatabaseService } from '../../../../src/nest/database/database.service'; -import { RuntimeEnvService } from '../../../../src/nest/app-config/runtime-env.service'; +function nameFor(placeId: string): string { + return `${crypto.createHash('sha1').update(placeId).digest('hex')}.jpg`; +} -let cache: PlacePhotoCacheService; +describe.each([ + ['mode A (photos/google/ prefix)', 'photos/google/'], + ['mode B (bare keys)', ''], +])('PlacePhotoCacheService — %s', (_label, keyPrefix) => { + let fx: StorageFixture; + let cache: PlacePhotoCacheService; -beforeAll(() => { - // onModuleInit does what the module-scope mkdirSync used to do on import. - cache = new PlacePhotoCacheService(new DatabaseService(testDb as never), new RuntimeEnvService()); - cache.onModuleInit(); -}); + const prefixDir = () => path.join(fx.root, ...keyPrefix.split('/').filter(Boolean)); + const filePathFor = (placeId: string) => path.join(prefixDir(), nameFor(placeId)); + const writeObject = (placeId: string, bytes: Buffer | string) => { + fs.mkdirSync(prefixDir(), { recursive: true }); + fs.writeFileSync(filePathFor(placeId), bytes); + }; -beforeEach(() => { - testDb.exec('DELETE FROM places; DELETE FROM google_place_photo_meta;'); - for (const f of fs.readdirSync(TMP_DIR)) fs.rmSync(path.join(TMP_DIR, f), { force: true }); -}); - -afterAll(() => { - testDb.close(); - fs.rmSync(TMP_DIR, { recursive: true, force: true }); -}); - -describe('placePhotoCache.put() downscale guard', () => { - it('PPC-001: downscales an oversized image to <= 800px', async () => { - const big = await makeJpeg(1600, 1200); - await cache.put('big-place', big, 'Alice'); - - const written = fs.readFileSync(filePathFor('big-place')); - const decoded = await Jimp.read(written); - expect(Math.max(decoded.bitmap.width, decoded.bitmap.height)).toBeLessThanOrEqual(800); - expect(written.length).toBeLessThan(big.length); + beforeAll(() => { + fx = makeStorageFixture(keyPrefix); + cache = new PlacePhotoCacheService(new DatabaseService(testDb as never), fx.storage); }); - it('PPC-002: passes a small image through unchanged', async () => { - const small = await makeJpeg(100, 100); - await cache.put('small-place', small, null); - - const written = fs.readFileSync(filePathFor('small-place')); - expect(written.equals(small)).toBe(true); - }); - - it('PPC-003: falls back to original bytes when the input is not a decodable image', async () => { - const garbage = Buffer.from('definitely not an image'); - await cache.put('garbage-place', garbage, null); - - const written = fs.readFileSync(filePathFor('garbage-place')); - expect(written.equals(garbage)).toBe(true); - }); -}); - -describe('placePhotoCache.serveKey()', () => { - it('returns the bare .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); - expect(fs.existsSync(filePathFor('orphan'))).toBe(true); - - cache.removeIfUnreferenced('orphan'); - - expect(fs.existsSync(filePathFor('orphan'))).toBe(false); - expect(testDb.prepare('SELECT 1 FROM google_place_photo_meta WHERE place_id = ?').get('orphan')).toBeUndefined(); - }); - - it('PPC-005: keeps an entry still referenced by google_place_id', async () => { - await cache.put('gid-1', await makeJpeg(50, 50), null); - testDb.prepare('INSERT INTO places (google_place_id) VALUES (?)').run('gid-1'); - - cache.removeIfUnreferenced('gid-1'); - - expect(fs.existsSync(filePathFor('gid-1'))).toBe(true); - }); - - it('PPC-006: keeps an entry referenced by a coords proxy URL in image_url', async () => { - const id = 'coords:48.8:2.3'; - await cache.put(id, await makeJpeg(50, 50), null); - const proxy = `/api/maps/place-photo/${encodeURIComponent(id)}/bytes`; - testDb.prepare('INSERT INTO places (image_url) VALUES (?)').run(proxy); - - cache.removeIfUnreferenced(id); - - expect(fs.existsSync(filePathFor(id))).toBe(true); - }); -}); - -describe('placePhotoCache.sweepOrphans()', () => { - it('PPC-007: removes orphaned meta rows + files, keeps referenced ones, deletes stray files', async () => { - await cache.put('keep-gid', await makeJpeg(50, 50), null); - await cache.put('drop-me', await makeJpeg(50, 50), null); - testDb.prepare('INSERT INTO places (google_place_id) VALUES (?)').run('keep-gid'); - - // A stray .jpg on disk with no meta row (e.g. a crash between write and upsert). - const strayPath = path.join(TMP_DIR, 'deadbeef'.padEnd(40, '0') + '.jpg'); - fs.writeFileSync(strayPath, 'stray'); - - const removed = cache.sweepOrphans(); - - expect(fs.existsSync(filePathFor('keep-gid'))).toBe(true); - expect(fs.existsSync(filePathFor('drop-me'))).toBe(false); - expect(fs.existsSync(strayPath)).toBe(false); - expect(testDb.prepare('SELECT 1 FROM google_place_photo_meta WHERE place_id = ?').get('drop-me')).toBeUndefined(); - expect(testDb.prepare('SELECT 1 FROM google_place_photo_meta WHERE place_id = ?').get('keep-gid')).toBeDefined(); - expect(removed).toBe(2); // drop-me (orphan meta+file) + stray file - }); - - it('PPC-008: returns 0 when every entry is referenced', async () => { - await cache.put('ref-a', await makeJpeg(50, 50), null); - testDb.prepare('INSERT INTO places (google_place_id) VALUES (?)').run('ref-a'); - - expect(cache.sweepOrphans()).toBe(0); - expect(fs.existsSync(filePathFor('ref-a'))).toBe(true); - }); -}); - -describe('placePhotoCache negative cache', () => { - function ageError(placeId: string, ms: number): void { - testDb - .prepare('UPDATE google_place_photo_meta SET error_at = ? WHERE place_id = ?') - .run(Date.now() - ms, placeId); - } - - it('PPC-009: a place with no photo stays remembered well past the old five-minute window', () => { - cache.markError('photo-less'); - - ageError('photo-less', 30 * 60 * 1000); - expect(cache.getErrored('photo-less')).toBe(true); - - ageError('photo-less', 5 * 60 * 60 * 1000); - expect(cache.getErrored('photo-less')).toBe(true); - }); - - it('PPC-010: the miss expires once it is a day old', () => { - cache.markError('stale-miss'); - ageError('stale-miss', 24 * 60 * 60 * 1000); - - expect(cache.getErrored('stale-miss')).toBe(false); - }); - - // A failed provider call says nothing about the place, so it must not inherit the - // long window a real "this place has no photo" answer gets. - it('PPC-011: a failed provider call is forgotten after minutes and never persisted', () => { - vi.useFakeTimers(); - try { - cache.markError('flaky', 'provider-error'); - expect(cache.getErrored('flaky')).toBe(true); - // Nothing on disk — a restart retries instead of inheriting someone's outage. - expect(testDb.prepare('SELECT 1 FROM google_place_photo_meta WHERE place_id = ?').get('flaky')).toBeUndefined(); - - vi.advanceTimersByTime(5 * 60 * 1000); - expect(cache.getErrored('flaky')).toBe(false); - - // The long window belongs to the other case: same age, still remembered. - cache.markError('photo-less-too'); - ageError('photo-less-too', 5 * 60 * 1000); - expect(cache.getErrored('photo-less-too')).toBe(true); - } finally { - vi.useRealTimers(); + beforeEach(() => { + testDb.exec('DELETE FROM places; DELETE FROM collection_places; DELETE FROM google_place_photo_meta;'); + for (const f of fs.readdirSync(fx.root)) { + if (f === '.tmp') continue; + fs.rmSync(path.join(fx.root, f), { recursive: true, force: true }); } }); - it('PPC-012: a cached photo clears an earlier failed attempt', async () => { - cache.markError('recovered', 'provider-error'); - expect(cache.getErrored('recovered')).toBe(true); + afterAll(() => { + fx.cleanup(); + }); - await cache.put('recovered', await makeJpeg(40, 40), null); - expect(cache.getErrored('recovered')).toBe(false); + describe('put() downscale guard', () => { + it('PPC-001: downscales an oversized image to <= 800px', async () => { + const big = await makeJpeg(1600, 1200); + await cache.put('big-place', big, 'Alice'); + + const written = fs.readFileSync(filePathFor('big-place')); + const decoded = await Jimp.read(written); + expect(Math.max(decoded.bitmap.width, decoded.bitmap.height)).toBeLessThanOrEqual(800); + expect(written.length).toBeLessThan(big.length); + }); + + it('PPC-002: passes a small image through unchanged', async () => { + const small = await makeJpeg(100, 100); + await cache.put('small-place', small, null); + + const written = fs.readFileSync(filePathFor('small-place')); + expect(written.equals(small)).toBe(true); + }); + + it('PPC-003: falls back to original bytes when the input is not a decodable image', async () => { + const garbage = Buffer.from('definitely not an image'); + await cache.put('garbage-place', garbage, null); + + const written = fs.readFileSync(filePathFor('garbage-place')); + expect(written.equals(garbage)).toBe(true); + }); + }); + + describe('serveKey()', () => { + it('returns the bare .jpg storage name when the photo object exists', async () => { + writeObject('p-key', await makeJpeg(10, 10)); + expect(await cache.serveKey('p-key')).toBe(nameFor('p-key')); + }); + + it('returns null when nothing is cached', async () => { + expect(await cache.serveKey('p-missing')).toBeNull(); + }); + }); + + describe('get()', () => { + it('PPC-013: purges the meta row and returns null when the object vanished', async () => { + testDb + .prepare('INSERT INTO google_place_photo_meta (place_id, attribution, fetched_at) VALUES (?, ?, ?)') + .run('gone-place', 'Bob', Date.now()); + + expect(await cache.get('gone-place')).toBeNull(); + expect(testDb.prepare('SELECT 1 FROM google_place_photo_meta WHERE place_id = ?').get('gone-place')).toBeUndefined(); + }); + + it('PPC-014: returns the proxy URL + attribution for a cached photo', async () => { + await cache.put('hit-place', await makeJpeg(20, 20), 'Carol'); + + const hit = await cache.get('hit-place'); + expect(hit).toEqual({ + photoUrl: `/api/maps/place-photo/${encodeURIComponent('hit-place')}/bytes`, + attribution: 'Carol', + }); + }); + }); + + describe('removeIfUnreferenced()', () => { + it('PPC-004: removes a cache entry that no place references', async () => { + await cache.put('orphan', await makeJpeg(50, 50), null); + expect(fs.existsSync(filePathFor('orphan'))).toBe(true); + + await cache.removeIfUnreferenced('orphan'); + + expect(fs.existsSync(filePathFor('orphan'))).toBe(false); + expect(testDb.prepare('SELECT 1 FROM google_place_photo_meta WHERE place_id = ?').get('orphan')).toBeUndefined(); + }); + + it('PPC-005: keeps an entry still referenced by google_place_id', async () => { + await cache.put('gid-1', await makeJpeg(50, 50), null); + testDb.prepare('INSERT INTO places (google_place_id) VALUES (?)').run('gid-1'); + + await cache.removeIfUnreferenced('gid-1'); + + expect(fs.existsSync(filePathFor('gid-1'))).toBe(true); + }); + + it('PPC-006: keeps an entry referenced by a coords proxy URL in image_url', async () => { + const id = 'coords:48.8:2.3'; + await cache.put(id, await makeJpeg(50, 50), null); + const proxy = `/api/maps/place-photo/${encodeURIComponent(id)}/bytes`; + testDb.prepare('INSERT INTO places (image_url) VALUES (?)').run(proxy); + + await cache.removeIfUnreferenced(id); + + expect(fs.existsSync(filePathFor(id))).toBe(true); + }); + }); + + describe('sweepOrphans()', () => { + it('PPC-007: removes orphaned meta rows + files, keeps referenced ones, deletes stray files', async () => { + await cache.put('keep-gid', await makeJpeg(50, 50), null); + await cache.put('drop-me', await makeJpeg(50, 50), null); + testDb.prepare('INSERT INTO places (google_place_id) VALUES (?)').run('keep-gid'); + + // A stray .jpg on disk with no meta row (e.g. a crash between write and upsert). + const strayPath = path.join(prefixDir(), 'deadbeef'.padEnd(40, '0') + '.jpg'); + fs.writeFileSync(strayPath, 'stray'); + + const removed = await cache.sweepOrphans(); + + expect(fs.existsSync(filePathFor('keep-gid'))).toBe(true); + expect(fs.existsSync(filePathFor('drop-me'))).toBe(false); + expect(fs.existsSync(strayPath)).toBe(false); + expect(testDb.prepare('SELECT 1 FROM google_place_photo_meta WHERE place_id = ?').get('drop-me')).toBeUndefined(); + expect(testDb.prepare('SELECT 1 FROM google_place_photo_meta WHERE place_id = ?').get('keep-gid')).toBeDefined(); + expect(removed).toBe(2); // drop-me (orphan meta+file) + stray file + }); + + it('PPC-008: returns 0 when every entry is referenced', async () => { + await cache.put('ref-a', await makeJpeg(50, 50), null); + testDb.prepare('INSERT INTO places (google_place_id) VALUES (?)').run('ref-a'); + + expect(await cache.sweepOrphans()).toBe(0); + expect(fs.existsSync(filePathFor('ref-a'))).toBe(true); + }); + + it('PPC-015: never touches nested entries or non-jpg files (readdir parity — list() recurses)', async () => { + const nestedDir = path.join(prefixDir(), 'sub'); + fs.mkdirSync(nestedDir, { recursive: true }); + const nested = path.join(nestedDir, 'nested.jpg'); + fs.writeFileSync(nested, 'nested'); + const note = path.join(prefixDir(), 'note.txt'); + fs.writeFileSync(note, 'not a photo'); + + const removed = await cache.sweepOrphans(); + + expect(fs.existsSync(nested)).toBe(true); + expect(fs.existsSync(note)).toBe(true); + expect(removed).toBe(0); + }); + }); + + describe('negative cache', () => { + function ageError(placeId: string, ms: number): void { + testDb + .prepare('UPDATE google_place_photo_meta SET error_at = ? WHERE place_id = ?') + .run(Date.now() - ms, placeId); + } + + it('PPC-009: a place with no photo stays remembered well past the old five-minute window', () => { + cache.markError('photo-less'); + + ageError('photo-less', 30 * 60 * 1000); + expect(cache.getErrored('photo-less')).toBe(true); + + ageError('photo-less', 5 * 60 * 60 * 1000); + expect(cache.getErrored('photo-less')).toBe(true); + }); + + it('PPC-010: the miss expires once it is a day old', () => { + cache.markError('stale-miss'); + ageError('stale-miss', 24 * 60 * 60 * 1000); + + expect(cache.getErrored('stale-miss')).toBe(false); + }); + + // A failed provider call says nothing about the place, so it must not inherit the + // long window a real "this place has no photo" answer gets. + it('PPC-011: a failed provider call is forgotten after minutes and never persisted', () => { + vi.useFakeTimers(); + try { + cache.markError('flaky', 'provider-error'); + expect(cache.getErrored('flaky')).toBe(true); + // Nothing on disk — a restart retries instead of inheriting someone's outage. + expect(testDb.prepare('SELECT 1 FROM google_place_photo_meta WHERE place_id = ?').get('flaky')).toBeUndefined(); + + vi.advanceTimersByTime(5 * 60 * 1000); + expect(cache.getErrored('flaky')).toBe(false); + + // The long window belongs to the other case: same age, still remembered. + cache.markError('photo-less-too'); + ageError('photo-less-too', 5 * 60 * 1000); + expect(cache.getErrored('photo-less-too')).toBe(true); + } finally { + vi.useRealTimers(); + } + }); + + it('PPC-012: a cached photo clears an earlier failed attempt', async () => { + cache.markError('recovered', 'provider-error'); + expect(cache.getErrored('recovered')).toBe(true); + + await cache.put('recovered', await makeJpeg(40, 40), null); + expect(cache.getErrored('recovered')).toBe(false); + }); }); }); diff --git a/server/tests/unit/nest/places.service.test.ts b/server/tests/unit/nest/places.service.test.ts index e98decef5..ec0e493eb 100644 --- a/server/tests/unit/nest/places.service.test.ts +++ b/server/tests/unit/nest/places.service.test.ts @@ -368,32 +368,32 @@ describe('updateMany', () => { // ── remove ──────────────────────────────────────────────────────────────────── describe('remove', () => { - it('PLACE-SVC-017 — deletes a place and returns true', () => { + it('PLACE-SVC-017 — deletes a place and returns true', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const place = createPlace(testDb, trip.id, { name: 'To Delete' }) as any; - expect(svc.remove(String(trip.id), String(place.id))).toBe(true); + expect(await svc.remove(String(trip.id), String(place.id))).toBe(true); expect(svc.get(String(trip.id), String(place.id))).toBeNull(); }); - it('PLACE-SVC-018 — returns false for non-existent place', () => { + it('PLACE-SVC-018 — returns false for non-existent place', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - expect(svc.remove(String(trip.id), '99999')).toBe(false); + expect(await svc.remove(String(trip.id), '99999')).toBe(false); }); - it('PLACE-SVC-019 — deleting one place does not remove others', () => { + it('PLACE-SVC-019 — deleting one place does not remove others', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const p1 = createPlace(testDb, trip.id, { name: 'Keep' }) as any; const p2 = createPlace(testDb, trip.id, { name: 'Remove' }) as any; - svc.remove(String(trip.id), String(p2.id)); + await svc.remove(String(trip.id), String(p2.id)); const remaining = svc.list(String(trip.id), {}) as any[]; expect(remaining).toHaveLength(1); expect(remaining[0].id).toBe(p1.id); }); - it('PLACE-SVC-019c — the linked expense goes with the place (#1298)', () => { + it('PLACE-SVC-019c — the linked expense goes with the place (#1298)', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const place = createPlace(testDb, trip.id, { name: 'Louvre' }) as any; @@ -404,13 +404,13 @@ describe('remove', () => { // Read the link before the delete — that is what the controller broadcasts. expect(svc.linkedExpenseIds(trip.id, [place.id])).toEqual([linked]); - expect(svc.remove(String(trip.id), String(place.id))).toBe(true); + expect(await svc.remove(String(trip.id), String(place.id))).toBe(true); const rows = testDb.prepare('SELECT id FROM budget_items ORDER BY id').all() as { id: number }[]; expect(rows.map(r => r.id)).toEqual([untouched, standalone]); }); - it('PLACE-SVC-019d — removeMany takes the expense of every deleted place with it', () => { + it('PLACE-SVC-019d — removeMany takes the expense of every deleted place with it', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const a = createPlace(testDb, trip.id, { name: 'A' }) as any; @@ -421,7 +421,7 @@ describe('remove', () => { } expect(svc.linkedExpenseIds(trip.id, [a.id, b.id])).toHaveLength(2); - svc.removeMany(String(trip.id), [a.id, b.id]); + await svc.removeMany(String(trip.id), [a.id, b.id]); const rows = testDb.prepare('SELECT place_id FROM budget_items').all() as { place_id: number }[]; expect(rows.map(r => r.place_id)).toEqual([keep.id]); @@ -438,14 +438,14 @@ describe('remove', () => { expect(svc.linkedExpenseIds(trip.id, [])).toEqual([]); }); - it('PLACE-SVC-019b — reclaims the photo cache for the deleted place', () => { + it('PLACE-SVC-019b — reclaims the photo cache for the deleted place', async () => { removeIfUnreferencedSpy.mockClear(); const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const place = createPlace(testDb, trip.id, { name: 'With Photo' }) as any; testDb.prepare('UPDATE places SET google_place_id = ? WHERE id = ?').run('ChIJgid', place.id); - svc.remove(String(trip.id), String(place.id)); + await svc.remove(String(trip.id), String(place.id)); expect(removeIfUnreferencedSpy).toHaveBeenCalledWith('ChIJgid'); }); @@ -454,7 +454,7 @@ describe('remove', () => { // ── removeMany ──────────────────────────────────────────────────────────────── describe('removeMany', () => { - it('PLACE-SVC-056 — deletes the trip-scoped ids in one transaction and reports them', () => { + it('PLACE-SVC-056 — deletes the trip-scoped ids in one transaction and reports them', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const other = createTrip(testDb, user.id); @@ -462,16 +462,16 @@ describe('removeMany', () => { const b = createPlace(testDb, trip.id, { name: 'B' }) as any; const foreign = createPlace(testDb, other.id, { name: 'Foreign' }) as any; - const deleted = svc.removeMany(String(trip.id), [a.id, b.id, foreign.id, 99999]); + const deleted = await svc.removeMany(String(trip.id), [a.id, b.id, foreign.id, 99999]); expect(deleted.sort()).toEqual([a.id, b.id].sort()); expect(svc.get(String(other.id), String(foreign.id))).not.toBeNull(); }); - it('PLACE-SVC-057 — returns [] for an empty id list', () => { + it('PLACE-SVC-057 — returns [] for an empty id list', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); - expect(svc.removeMany(String(trip.id), [])).toEqual([]); + expect(await svc.removeMany(String(trip.id), [])).toEqual([]); }); }); @@ -942,7 +942,7 @@ describe('custom place image reclaim', () => { expect(fs.existsSync(fileA)).toBe(false); }); - it('PLACE-SVC-048 — remove unlinks the uploaded image', () => { + it('PLACE-SVC-048 — remove unlinks the uploaded image', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const place = createPlace(testDb, trip.id, { name: 'Photo' }) as any; @@ -950,11 +950,11 @@ describe('custom place image reclaim', () => { testDb.prepare('UPDATE places SET image_url = ? WHERE id = ?').run('/uploads/places/svc-delete.jpg', place.id); expect(fs.existsSync(fileA)).toBe(true); - svc.remove(String(trip.id), String(place.id)); + await svc.remove(String(trip.id), String(place.id)); expect(fs.existsSync(fileA)).toBe(false); }); - it('PLACE-SVC-049 — a collection_places reference keeps the file when the trip place is deleted', () => { + it('PLACE-SVC-049 — a collection_places reference keeps the file when the trip place is deleted', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const place = createPlace(testDb, trip.id, { name: 'Shared Photo' }) as any; @@ -966,7 +966,7 @@ describe('custom place image reclaim', () => { .run(col.lastInsertRowid, user.id, 'Shared Photo', '/uploads/places/svc-shared.jpg'); expect(fs.existsSync(fileA)).toBe(true); - svc.remove(String(trip.id), String(place.id)); + await svc.remove(String(trip.id), String(place.id)); expect(fs.existsSync(fileA)).toBe(true); // resetTestDb does not clear collections; drop what this test inserted and its file. diff --git a/server/tests/unit/nest/share.service.test.ts b/server/tests/unit/nest/share.service.test.ts index e1e5fee64..168e4e371 100644 --- a/server/tests/unit/nest/share.service.test.ts +++ b/server/tests/unit/nest/share.service.test.ts @@ -380,29 +380,29 @@ describe('getSharedTripData', () => { // ── getSharedPlacePhotoKey ─────────────────────────────────────────────────── describe('getSharedPlacePhotoKey', () => { - it('SHARE-SVC-022: returns null for an unknown or expired token', () => { - expect(svc.getSharedPlacePhotoKey('nope', 'ChIJabc')).toBeNull(); + it('SHARE-SVC-022: returns null for an unknown or expired token', async () => { + expect(await 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.getSharedPlacePhotoKey(token, 'ChIJabc')).toBeNull(); + expect(await svc.getSharedPlacePhotoKey(token, 'ChIJabc')).toBeNull(); expect(serveKey).not.toHaveBeenCalled(); }); - it('SHARE-SVC-023: returns null when the owner disabled the map section', () => { + it('SHARE-SVC-023: returns null when the owner disabled the map section', async () => { 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.getSharedPlacePhotoKey(token, 'ChIJabc')).toBeNull(); + expect(await 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', () => { + it('SHARE-SVC-024: returns null when no place in the trip carries the proxy URL', async () => { const { token } = seedSharedTrip(); - expect(svc.getSharedPlacePhotoKey(token, 'ChIJabc')).toBeNull(); + expect(await svc.getSharedPlacePhotoKey(token, 'ChIJabc')).toBeNull(); expect(serveKey).not.toHaveBeenCalled(); }); - it('SHARE-SVC-025: resolves via serveKey when the encoded placeId matches the stored URL', () => { + it('SHARE-SVC-025: resolves via serveKey when the encoded placeId matches the stored URL', async () => { const { trip, token } = seedSharedTrip(); const place = createPlace(testDb, trip.id); // Wikimedia pseudo-IDs contain characters that must round-trip encoded. @@ -410,7 +410,7 @@ describe('getSharedPlacePhotoKey', () => { testDb.prepare('UPDATE places SET image_url = ? WHERE id = ?') .run(`/api/maps/place-photo/${encodeURIComponent(placeId)}/bytes`, place.id); serveKey.mockReturnValue('abc.jpg'); - expect(svc.getSharedPlacePhotoKey(token, placeId)).toBe('abc.jpg'); + expect(await svc.getSharedPlacePhotoKey(token, placeId)).toBe('abc.jpg'); expect(serveKey).toHaveBeenCalledWith(placeId); }); }); diff --git a/server/tests/unit/nest/trip-read-model.service.test.ts b/server/tests/unit/nest/trip-read-model.service.test.ts index 54ea00bdf..30a84171a 100644 --- a/server/tests/unit/nest/trip-read-model.service.test.ts +++ b/server/tests/unit/nest/trip-read-model.service.test.ts @@ -72,6 +72,7 @@ import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.se import { UnsplashService } from '../../../src/nest/unsplash/unsplash.service'; import { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service'; import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; +import { makeStorageFixture } from '../../helpers/storage-fixture'; import { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service'; import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repository'; @@ -83,7 +84,7 @@ const budgetSvc = new BudgetService(dbs(), new PermissionsService(dbs()), new Ex const daysSvc = new DaysService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new QueryHelpersService(dbs())); // One shared cache instance (the PlacePhotoCacheService rule): the in-flight dedup in // PlacePhotoCacheService only works while both consumers hold the same object. -const photoCache = new PlacePhotoCacheService(dbs(), new RuntimeEnvService()); +const photoCache = new PlacePhotoCacheService(dbs(), makeStorageFixture('photos/google/').storage); const placesSvc = new PlacesService( dbs(), new PermissionsService(dbs()), new RealtimeService(), new MapsService(dbs(), photoCache), new QueryHelpersService(dbs()), diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index d872b6d4e..41ba48993 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -79,6 +79,7 @@ import { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-pho import { JourneyDomainService } from '../../../src/nest/journey/journey-domain.service'; import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repository'; import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; +import { makeStorageFixture } from '../../helpers/storage-fixture'; import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; import fs from 'fs'; import { notificationsStub } from '../../helpers/notifications'; @@ -96,7 +97,7 @@ const daysSvc = new DaysService(dbs(), new PermissionsService(dbs()), new Realti // missing collaborator would look like a pass while swallowing a TypeError. // One PlacePhotoCacheService for both PlacesService and MapsService, matching // production, where the in-flight dedup only works on a shared instance. -const photoCache = new PlacePhotoCacheService(dbs(), new RuntimeEnvService()); +const photoCache = new PlacePhotoCacheService(dbs(), makeStorageFixture('photos/google/').storage); const placesSvc = new PlacesService( dbs(), new PermissionsService(dbs()), diff --git a/server/tests/unit/services/conflictUpdate.test.ts b/server/tests/unit/services/conflictUpdate.test.ts index c318c8975..800124662 100644 --- a/server/tests/unit/services/conflictUpdate.test.ts +++ b/server/tests/unit/services/conflictUpdate.test.ts @@ -52,13 +52,14 @@ import { JourneyDomainService } from '../../../src/nest/journey/journey-domain.s import { TrekPhotosRepository } from '../../../src/nest/photos/trek-photos.repository'; import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; import { notificationsStub } from '../../helpers/notifications'; +import { makeStorageFixture } from '../../helpers/storage-fixture'; const dbs = new DatabaseService(testDb); const realtime = new RealtimeService(); const runtimeEnv = new RuntimeEnvService(); // One cache instance shared by maps and places, the way the container wires it: // the service's stampede guard only works if there is exactly one of them. -const photoCache = new PlacePhotoCacheService(dbs, runtimeEnv); +const photoCache = new PlacePhotoCacheService(dbs, makeStorageFixture('photos/google/').storage); const packing = new PackingService(dbs, new PermissionsService(dbs), realtime, notificationsStub()); const places = new PlacesService(