diff --git a/server/src/index.ts b/server/src/index.ts index 91a7764af..bca2c2284 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -10,6 +10,7 @@ import http from 'node:http'; import type { INestApplication } from '@nestjs/common'; import { buildApp } from './bootstrap'; import { BackupService } from './nest/backup/backup.service'; +import { PlacePhotoCacheService } from './nest/place-photos/place-photo-cache.service'; // Create upload and data directories on startup. // Every uploads subdir the app writes to must be listed here (#1762): a dir @@ -91,7 +92,12 @@ const onListen = () => { // The scheduler runs outside the container but needs things inside it. It is // handed them here, from the app buildApp() already initialised, rather than // reaching for src/services at call time. - scheduler.setSchedulerDeps({ backups: nestApp.get(BackupService) }); + scheduler.setSchedulerDeps({ + backups: nestApp.get(BackupService), + // The container singleton, not a fresh instance: the in-flight dedup and the + // known-on-disk set only work if the whole process shares one. + placePhotos: nestApp.get(PlacePhotoCacheService), + }); scheduler.start(); scheduler.startTripReminders(); scheduler.startTodoReminders(); diff --git a/server/src/nest/maps/maps.module.ts b/server/src/nest/maps/maps.module.ts index 9daaff76a..8a26d0e3b 100644 --- a/server/src/nest/maps/maps.module.ts +++ b/server/src/nest/maps/maps.module.ts @@ -2,6 +2,7 @@ import { Module } from '@nestjs/common'; import { MapsController } from './maps.controller'; import { MapsService } from './maps.service'; import { MapsMcp } from './maps.mcp'; +import { PlacePhotosModule } from '../place-photos/place-photos.module'; /** * Maps / geo domain (L3 leaf module). Registered in AppModule. Exports @@ -10,6 +11,7 @@ import { MapsMcp } from './maps.mcp'; * Nothing outside the container consumes this domain, so there is no bridge. */ @Module({ + imports: [PlacePhotosModule], controllers: [MapsController], providers: [MapsService, MapsMcp], exports: [MapsService], diff --git a/server/src/nest/maps/maps.service.ts b/server/src/nest/maps/maps.service.ts index a91f9bdd0..0da190ae5 100644 --- a/server/src/nest/maps/maps.service.ts +++ b/server/src/nest/maps/maps.service.ts @@ -11,7 +11,7 @@ import { readEnv, getAppUrl } from '../../app-config'; import { safeFetchFollow, SsrfBlockedError } from '../../utils/ssrfGuard'; import { decrypt_api_key } from '../common/crypto/apiKeyCrypto'; // ── Photo cache (disk-backed) ──────────────────────────────────────────────── -import * as placePhotoCache from '../../services/placePhotoCache'; +import { PlacePhotoCacheService } from '../place-photos/place-photo-cache.service'; import { DatabaseService } from '../database/database.service'; import { UA, @@ -238,7 +238,10 @@ type LocationBias = { low: { lat: number; lng: number }; high: { lat: number; ln */ @Injectable() export class MapsService { - constructor(private readonly database: DatabaseService) {} + constructor( + private readonly database: DatabaseService, + private readonly photoCache: PlacePhotoCacheService, + ) {} private isSettingDisabled(key: string): boolean { const row = this.database.get<{ value: string }>( @@ -283,7 +286,7 @@ export class MapsService { } photoBytesPath(placeId: string): string | null { - return placePhotoCache.serveFilePath(placeId); + return this.photoCache.serveFilePath(placeId); } reverse(lat: string, lng: string, lang?: string): Promise { @@ -949,7 +952,7 @@ export class MapsService { name?: string, ): Promise<{ photoUrl: string | null; attribution: string | null }> { // Disk cache hit — serve immediately, no Google call - const diskHit = placePhotoCache.get(placeId); + const diskHit = 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 @@ -960,10 +963,10 @@ export class MapsService { const noPhoto = { photoUrl: null, attribution: null }; // Recent miss — don't hammer the API - if (placePhotoCache.getErrored(placeId)) return noPhoto; + if (this.photoCache.getErrored(placeId)) return noPhoto; // Deduplicate concurrent requests for the same placeId - const existing = placePhotoCache.getInFlight(placeId); + const existing = this.photoCache.getInFlight(placeId); if (existing) { const result = await existing; if (!result) return noPhoto; @@ -997,7 +1000,7 @@ export class MapsService { return null; } const bytes = Buffer.from(await imgRes.arrayBuffer()); - const cached = await placePhotoCache.put(placeId, bytes, wiki.attribution); + const cached = await this.photoCache.put(placeId, bytes, wiki.attribution); return { filePath: cached.filePath, attribution: cached.attribution }; } catch { providerFailed = true; @@ -1060,7 +1063,7 @@ export class MapsService { return null; } - const cached = await placePhotoCache.put(placeId, bytes, attribution); + const cached = await this.photoCache.put(placeId, bytes, attribution); // Persist stable proxy URL to database try { @@ -1087,14 +1090,14 @@ export class MapsService { const fallback = await fetchWikimediaFallback(); if (fallback) return fallback; - placePhotoCache.markError(placeId, providerFailed ? 'provider-error' : 'no-photo'); + this.photoCache.markError(placeId, providerFailed ? 'provider-error' : 'no-photo'); return null; } finally { releasePhotoFetchSlot(); } })(); - placePhotoCache.setInFlight(placeId, fetchPromise); + this.photoCache.setInFlight(placeId, fetchPromise); const result = await fetchPromise; if (!result) return noPhoto; diff --git a/server/src/nest/place-photos/place-photo-cache.service.ts b/server/src/nest/place-photos/place-photo-cache.service.ts new file mode 100644 index 000000000..57b83439e --- /dev/null +++ b/server/src/nest/place-photos/place-photo-cache.service.ts @@ -0,0 +1,295 @@ +import { Injectable, OnModuleInit } 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 { DatabaseService } from '../database/database.service'; +import { RuntimeEnvService } from '../app-config/runtime-env.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 +// trip open replays the whole provider fan-out, one lookup per place. A day is +// still short enough that a newly configured API key takes effect overnight. +// Entries are dropped outright when the place goes away (removeIfUnreferenced / +// sweepOrphans). +const MISSING_TTL = 24 * 60 * 60 * 1000; +// A provider call that failed says nothing about the place — only that the API was +// unreachable or unhappy just now. It is remembered just long enough to stop a +// screenful of markers from hammering an API that is down, and only in memory, so +// a restart retries right away. +const FAILURE_TTL = 5 * 60 * 1000; +// Sweep expired failures once the map grows past this. Whatever survives the sweep +// is younger than FAILURE_TTL and expires on its own shortly after. +const FAILURE_SWEEP_AT = 500; + +// Marker photos are displayed tiny — cap stored images so an oversized source +// (e.g. a Wikimedia Commons full-res original) can't bloat the cache. Matches +// THUMB_MAX/THUMB_QUALITY in memories/thumbnailService.ts. +const MAX_DIM = 800; +const JPEG_QUALITY = 80; + +interface CachedPhoto { + photoUrl: string; + filePath: string; + attribution: string | null; +} + +/** + * Cache for the marker photos the map providers hand back. + * + * Its own module because maps, places and share all read it — and because its + * three in-memory structures only do their job if there is exactly ONE of them. + * A second instance would give the stampede guard its own inFlight map, so two + * simultaneous requests for the same uncached place would both call the + * provider. That is the reason the scheduler takes this from the container + * rather than constructing it: see SchedulerDeps. + */ +@Injectable() +export class PlacePhotoCacheService implements OnModuleInit { + /** 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 knownOnDisk = new Set(); + + constructor( + private readonly db: DatabaseService, + private readonly env: RuntimeEnvService, + ) {} + + /** + * 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 filePath(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`); + } + + private proxyUrl(placeId: string): string { + return `/api/maps/place-photo/${encodeURIComponent(placeId)}/bytes`; + } + + get(placeId: string): CachedPhoto | null { + const row = this.db.get<{ attribution: string | null }>( + 'SELECT attribution FROM google_place_photo_meta WHERE place_id = ? AND error_at IS NULL', + placeId, + ); + + 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. + // (Guards against volume wipes or manual deletion between server restarts.) + if (!fs.existsSync(fp)) { + 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 }; + } + + getErrored(placeId: string): boolean { + const failedAt = this.recentFailures.get(placeId); + if (failedAt !== undefined) { + if (Date.now() - failedAt < FAILURE_TTL) return true; + this.recentFailures.delete(placeId); + } + + const row = this.db.get<{ error_at: number }>( + 'SELECT error_at FROM google_place_photo_meta WHERE place_id = ? AND error_at IS NOT NULL', + placeId, + ); + + if (!row) return false; + return Date.now() - row.error_at < MISSING_TTL; + } + + /** + * Remember that a lookup came back empty. 'no-photo' is the lasting answer — no + * provider has an image for this place — and is persisted; 'provider-error' is a + * failed attempt and is only held in memory for a few minutes. + */ + markError(placeId: string, kind: 'no-photo' | 'provider-error' = 'no-photo'): void { + if (kind === 'provider-error') { + if (this.recentFailures.size >= FAILURE_SWEEP_AT) { + const cutoff = Date.now() - FAILURE_TTL; + for (const [id, at] of this.recentFailures) { + if (at <= cutoff) this.recentFailures.delete(id); + } + } + this.recentFailures.set(placeId, Date.now()); + return; + } + + this.recentFailures.delete(placeId); + this.knownOnDisk.delete(placeId); + this.db.run( + 'INSERT OR REPLACE INTO google_place_photo_meta (place_id, attribution, fetched_at, error_at) VALUES (?, NULL, ?, ?)', + placeId, Date.now(), Date.now(), + ); + } + + /** + * Downscale oversized images to MAX_DIM before caching, re-encoding to JPEG. + * Defense-in-depth: keeps the cache small regardless of what the fetch path hands + * us. Jimp auto-applies EXIF orientation on read. Falls back to the original bytes + * on any failure (corrupt/unsupported format) so behaviour is never worse than before. + */ + private async downscale(bytes: Buffer): Promise { + try { + const img = await Jimp.read(bytes); + if (img.bitmap.width <= MAX_DIM && img.bitmap.height <= MAX_DIM) return bytes; + img.scaleToFit({ w: MAX_DIM, h: MAX_DIM }); + return await img.getBuffer(JimpMime.jpeg, { quality: JPEG_QUALITY }); + } catch { + return bytes; + } + } + + 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); + + this.knownOnDisk.add(placeId); + this.recentFailures.delete(placeId); + + this.db.run( + 'INSERT OR REPLACE INTO google_place_photo_meta (place_id, attribution, fetched_at, error_at) VALUES (?, ?, ?, NULL)', + placeId, attribution, Date.now(), + ); + + return { photoUrl: this.proxyUrl(placeId), filePath: fp, attribution }; + } + + getInFlight( + placeId: string, + ): Promise<{ filePath: string; attribution: string | null } | null> | undefined { + return this.inFlight.get(placeId); + } + + setInFlight( + placeId: string, + promise: Promise<{ filePath: string; attribution: string | null } | null>, + ): void { + this.inFlight.set(placeId, promise); + promise + .finally(() => this.inFlight.delete(placeId)) + .catch(() => { + /* awaiter logs; this .catch only prevents unhandledRejection */ + }); + } + + 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; + } + + /** + * 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 + * (covers coords: pseudo-ids, which never have a google_place_id). + * + * A collection-saved place copies image_url = proxyUrl(google_place_id) and/or + * the google_place_id itself, so collection_places must count as a referencing + * table — otherwise the nightly sweep + trip-place delete would evict a photo + * still shown on a collection thumbnail (#1081 photo-cache pitfall). + */ + private isReferenced(placeId: string): boolean { + const row = this.db.get( + `SELECT 1 FROM places WHERE google_place_id = ? OR image_url = ? + UNION ALL + SELECT 1 FROM collection_places WHERE google_place_id = ? OR image_url = ? + LIMIT 1`, + placeId, this.proxyUrl(placeId), placeId, this.proxyUrl(placeId), + ); + return !!row; + } + + private deleteEntry(placeId: string): void { + try { + fs.unlinkSync(this.filePath(placeId)); + } catch { + /* already gone */ + } + this.db.run('DELETE FROM google_place_photo_meta WHERE place_id = ?', placeId); + this.knownOnDisk.delete(placeId); + } + + /** + * 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 { + if (this.isReferenced(placeId)) return; + 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. + */ + sweepOrphans(): number { + 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`); + } else { + 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; + try { + fs.unlinkSync(path.join(this.photoDir, entry)); + removed++; + } catch { + /* race */ + } + } + + return removed; + } +} diff --git a/server/src/nest/place-photos/place-photos.module.ts b/server/src/nest/place-photos/place-photos.module.ts new file mode 100644 index 000000000..b026a1c07 --- /dev/null +++ b/server/src/nest/place-photos/place-photos.module.ts @@ -0,0 +1,16 @@ +import { Module } from '@nestjs/common'; +import { PlacePhotoCacheService } from './place-photo-cache.service'; +import { AppConfigModule } from '../app-config/app-config.module'; + +/** The marker-photo cache. No controller of its own — maps serves the bytes, + * places and share read through it, and the scheduler sweeps it nightly. + * + * Deliberately NOT @Global (permissions precedent), and AppConfigModule is + * imported explicitly because @Global only reaches modules that are in the + * graph — which a single-domain e2e TestingModule is not. */ +@Module({ + imports: [AppConfigModule], + providers: [PlacePhotoCacheService], + exports: [PlacePhotoCacheService], +}) +export class PlacePhotosModule {} diff --git a/server/src/nest/places/places.helpers.ts b/server/src/nest/places/places.helpers.ts index 5eae0c2c6..2f33fe3c8 100644 --- a/server/src/nest/places/places.helpers.ts +++ b/server/src/nest/places/places.helpers.ts @@ -3,7 +3,7 @@ import unzipper from 'unzipper'; import type { Place } from '../../types'; import type { PlaceWithTags } from '../database/database.service'; import type { KmlImportSummary } from './kml-import.helpers'; -import * as placePhotoCache from '../../services/placePhotoCache'; +import type { PlacePhotoCacheService } from '../place-photos/place-photo-cache.service'; /** * Pure helpers and module-scope constants of the places domain, moved verbatim @@ -100,13 +100,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(googlePlaceId: string | null, imageUrl: string | null): void { +export function reclaimPhotoCache(cache: PlacePhotoCacheService, googlePlaceId: string | null, imageUrl: string | null): void { 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 { placePhotoCache.removeIfUnreferenced(id); } catch { /* best-effort */ } + try { cache.removeIfUnreferenced(id); } catch { /* best-effort */ } } } diff --git a/server/src/nest/places/places.module.ts b/server/src/nest/places/places.module.ts index e791511e3..62b87aab1 100644 --- a/server/src/nest/places/places.module.ts +++ b/server/src/nest/places/places.module.ts @@ -5,6 +5,7 @@ import { PlacesMcp } from './places.mcp'; import { PermissionsModule } from '../permissions/permissions.module'; import { AppConfigModule } from '../app-config/app-config.module'; import { UnsplashModule } from '../unsplash/unsplash.module'; +import { PlacePhotosModule } from '../place-photos/place-photos.module'; import { QueryHelpersModule } from '../query-helpers/query-helpers.module'; import { MapsModule } from '../maps/maps.module'; import { AuthModule } from '../auth/auth.module'; @@ -18,7 +19,7 @@ import { AuthModule } from '../auth/auth.module'; * is no places.bridge.ts: nothing outside the container consumes this domain. */ @Module({ - imports: [PermissionsModule, QueryHelpersModule, MapsModule, AuthModule, AppConfigModule, UnsplashModule], + imports: [PermissionsModule, QueryHelpersModule, MapsModule, AuthModule, AppConfigModule, UnsplashModule, PlacePhotosModule], controllers: [PlacesController], providers: [PlacesService, PlacesMcp], exports: [PlacesService], diff --git a/server/src/nest/places/places.service.ts b/server/src/nest/places/places.service.ts index c209090a7..7edd5f270 100644 --- a/server/src/nest/places/places.service.ts +++ b/server/src/nest/places/places.service.ts @@ -20,6 +20,7 @@ import { resolveCategoryIdForFolder, } from './kml-import.helpers'; import { UnsplashService } from '../unsplash/unsplash.service'; +import { PlacePhotoCacheService } from '../place-photos/place-photo-cache.service'; import { type UpdateConflict, isUpdateConflict } from '../common/conflictResult'; import { reclaimPlaceImage } from './place-image'; import { onPlaceCreated, onPlaceUpdated, onPlaceDeleted } from '../../services/journeyService'; @@ -106,6 +107,7 @@ export class PlacesService { private readonly maps: MapsService, private readonly queryHelpers: QueryHelpersService, private readonly unsplash: UnsplashService, + private readonly photoCache: PlacePhotoCacheService, ) {} verifyTripAccess(tripId: string, userId: number) { @@ -343,7 +345,7 @@ export class PlacesService { ); if (!place) return false; this.dbs.run('DELETE FROM places WHERE id = ?', placeId); - reclaimPhotoCache(place.google_place_id, place.image_url); + reclaimPhotoCache(this.photoCache, place.google_place_id, place.image_url); reclaimPlaceImage(place.image_url); return true; } @@ -365,7 +367,7 @@ export class PlacesService { }); // Reclaim after the transaction commits so isReferenced() sees the final place set. for (const row of reclaimable) { - reclaimPhotoCache(row.google_place_id, row.image_url); + reclaimPhotoCache(this.photoCache, row.google_place_id, row.image_url); reclaimPlaceImage(row.image_url); } return deleted; diff --git a/server/src/nest/share/share.module.ts b/server/src/nest/share/share.module.ts index d35c52f95..9b8d0e01a 100644 --- a/server/src/nest/share/share.module.ts +++ b/server/src/nest/share/share.module.ts @@ -5,10 +5,11 @@ import { ShareMcp } from './share.mcp'; 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 { AuthModule } from '../auth/auth.module'; @Module({ - imports: [SettingsModule, PermissionsModule, QueryHelpersModule, AuthModule], + imports: [SettingsModule, PermissionsModule, QueryHelpersModule, AuthModule, PlacePhotosModule], controllers: [TripShareController, SharedController], providers: [ShareService, ShareMcp], }) diff --git a/server/src/nest/share/share.service.ts b/server/src/nest/share/share.service.ts index c80bdb45b..11aec174b 100644 --- a/server/src/nest/share/share.service.ts +++ b/server/src/nest/share/share.service.ts @@ -4,7 +4,7 @@ import { DatabaseService } from '../database/database.service'; import type { TripAccess } from '../database/database.service'; import { PermissionsService } from '../permissions/permissions.service'; import { QueryHelpersService } from '../query-helpers/query-helpers.service'; -import { serveFilePath } from '../../services/placePhotoCache'; +import { PlacePhotoCacheService } from '../place-photos/place-photo-cache.service'; import { SettingsService } from '../settings/settings.service'; import type { User } from '../../types'; @@ -57,6 +57,7 @@ export class ShareService { private readonly settings: SettingsService, private readonly permissions: PermissionsService, private readonly queryHelpers: QueryHelpersService, + private readonly photoCache: PlacePhotoCacheService, ) {} verifyTripAccess(tripId: string, userId: number) { @@ -318,6 +319,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 serveFilePath(placeId); + return this.photoCache.serveFilePath(placeId); } } diff --git a/server/src/nest/trips/trips.bridge.ts b/server/src/nest/trips/trips.bridge.ts index 851202bec..254cc1730 100644 --- a/server/src/nest/trips/trips.bridge.ts +++ b/server/src/nest/trips/trips.bridge.ts @@ -4,6 +4,7 @@ import { RealtimeService } from '../realtime/realtime.service'; import { PermissionsService } from '../permissions/permissions.service'; import { QueryHelpersService } from '../query-helpers/query-helpers.service'; import { UnsplashService } from '../unsplash/unsplash.service'; +import { PlacePhotoCacheService } from '../place-photos/place-photo-cache.service'; import { RuntimeEnvService } from '../app-config/runtime-env.service'; import { TripsService } from './trips.service'; import { TodoService } from '../todo/todo.service'; @@ -34,6 +35,9 @@ import { MapsService } from '../maps/maps.service'; * the shared better-sqlite3 singleton. */ const dbs = () => new DatabaseService(db); +// One instance, not one per consumer: the in-flight dedup only works if the +// stampede guard is shared (see PlacePhotoCacheService). +const photoCache = new PlacePhotoCacheService(dbs(), new RuntimeEnvService()); const budget = new BudgetService(dbs(), new PermissionsService(dbs()), new ExchangeRatesService(), new RealtimeService()); const trips = new TripsService( dbs(), @@ -47,7 +51,7 @@ const trips = new TripsService( new CollabService(dbs(), new PermissionsService(dbs()), new RealtimeService()), new VacayService(dbs(), new RealtimeService()), new RealtimeService(), - new PlacesService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new MapsService(dbs()), new QueryHelpersService(dbs()), new UnsplashService(dbs(), new RuntimeEnvService())), + new PlacesService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new MapsService(dbs(), photoCache), new QueryHelpersService(dbs()), new UnsplashService(dbs(), new RuntimeEnvService()), photoCache), new UnsplashService(dbs(), new RuntimeEnvService()), ); diff --git a/server/src/scheduler.ts b/server/src/scheduler.ts index 4b05cb5b2..6ddee2355 100644 --- a/server/src/scheduler.ts +++ b/server/src/scheduler.ts @@ -73,6 +73,7 @@ function saveSettings(settings: BackupSettings): void { */ export interface SchedulerDeps { backups: { createBackup(prefix?: 'backup' | 'auto-backup'): Promise<{ filename: string }> }; + placePhotos: { sweepOrphans(): number }; } let deps: SchedulerDeps | undefined; @@ -381,8 +382,11 @@ function startPlacePhotoCacheCleanup(): void { const sweep = () => { try { - const { sweepOrphans } = require('./services/placePhotoCache'); - const removed = sweepOrphans(); + if (!deps) { + logError('Place-photo cache cleanup: skipped, the scheduler was started without its container dependencies'); + return; + } + const removed = deps.placePhotos.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/services/placePhotoCache.ts b/server/src/services/placePhotoCache.ts deleted file mode 100644 index 93e7f21ec..000000000 --- a/server/src/services/placePhotoCache.ts +++ /dev/null @@ -1,256 +0,0 @@ -import { readEnv } from '../app-config'; -import { db } from '../db/database'; - -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'; - -// Overridable for tests (mirrors the TREK_DB_FILE seam) so the suite never touches -// the real uploads tree. -const GOOGLE_PHOTO_DIR = readEnv().paths.placePhotoDir || path.join(__dirname, '../../uploads/photos/google'); -// 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 -// trip open replays the whole provider fan-out, one lookup per place. A day is -// still short enough that a newly configured API key takes effect overnight. -// Entries are dropped outright when the place goes away (removeIfUnreferenced / -// sweepOrphans). -const MISSING_TTL = 24 * 60 * 60 * 1000; -// A provider call that failed says nothing about the place — only that the API was -// unreachable or unhappy just now. It is remembered just long enough to stop a -// screenful of markers from hammering an API that is down, and only in memory, so -// a restart retries right away. -const FAILURE_TTL = 5 * 60 * 1000; -const recentFailures = new Map(); -// Sweep expired failures once the map grows past this. Whatever survives the sweep -// is younger than FAILURE_TTL and expires on its own shortly after. -const FAILURE_SWEEP_AT = 500; - -// Marker photos are displayed tiny — cap stored images so an oversized source -// (e.g. a Wikimedia Commons full-res original) can't bloat the cache. Matches -// THUMB_MAX/THUMB_QUALITY in memories/thumbnailService.ts. -const MAX_DIM = 800; -const JPEG_QUALITY = 80; - -// In-flight dedup — prevents stampedes when multiple requests hit the same uncached placeId simultaneously -const inFlight = new Map>(); - -// In-memory set of placeIds whose file is confirmed on disk this session. -// Avoids a synchronous fs.existsSync() call on every cache hit after the first verification. -const knownOnDisk = new Set(); - -// Ensure upload dir exists once at startup — avoids sync FS calls inside put() on every write. -try { - fs.mkdirSync(GOOGLE_PHOTO_DIR, { recursive: true }); -} catch { - /* already exists */ -} - -function filePath(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(GOOGLE_PHOTO_DIR, `${hash}.jpg`); -} - -function proxyUrl(placeId: string): string { - return `/api/maps/place-photo/${encodeURIComponent(placeId)}/bytes`; -} - -interface CachedPhoto { - photoUrl: string; - filePath: string; - attribution: string | null; -} - -export function get(placeId: string): CachedPhoto | null { - const row = db - .prepare('SELECT attribution FROM google_place_photo_meta WHERE place_id = ? AND error_at IS NULL') - .get(placeId) as { attribution: string | null } | undefined; - - if (!row) return null; - - const fp = filePath(placeId); - - if (!knownOnDisk.has(placeId)) { - // First time this placeId is checked this session — verify the file exists on disk. - // (Guards against volume wipes or manual deletion between server restarts.) - if (!fs.existsSync(fp)) { - db.prepare('DELETE FROM google_place_photo_meta WHERE place_id = ?').run(placeId); - return null; - } - knownOnDisk.add(placeId); - } - - return { photoUrl: proxyUrl(placeId), filePath: fp, attribution: row.attribution }; -} - -export function getErrored(placeId: string): boolean { - const failedAt = recentFailures.get(placeId); - if (failedAt !== undefined) { - if (Date.now() - failedAt < FAILURE_TTL) return true; - recentFailures.delete(placeId); - } - - const row = db - .prepare('SELECT error_at FROM google_place_photo_meta WHERE place_id = ? AND error_at IS NOT NULL') - .get(placeId) as { error_at: number } | undefined; - - if (!row) return false; - return Date.now() - row.error_at < MISSING_TTL; -} - -// Remember that a lookup came back empty. 'no-photo' is the lasting answer — no -// provider has an image for this place — and is persisted; 'provider-error' is a -// failed attempt and is only held in memory for a few minutes. -export function markError(placeId: string, kind: 'no-photo' | 'provider-error' = 'no-photo'): void { - if (kind === 'provider-error') { - if (recentFailures.size >= FAILURE_SWEEP_AT) { - const cutoff = Date.now() - FAILURE_TTL; - for (const [id, at] of recentFailures) { - if (at <= cutoff) recentFailures.delete(id); - } - } - recentFailures.set(placeId, Date.now()); - return; - } - - recentFailures.delete(placeId); - knownOnDisk.delete(placeId); - db.prepare( - 'INSERT OR REPLACE INTO google_place_photo_meta (place_id, attribution, fetched_at, error_at) VALUES (?, NULL, ?, ?)', - ).run(placeId, Date.now(), Date.now()); -} - -// Downscale oversized images to MAX_DIM before caching, re-encoding to JPEG. -// Defense-in-depth: keeps the cache small regardless of what the fetch path hands -// us. Jimp auto-applies EXIF orientation on read. Falls back to the original bytes -// on any failure (corrupt/unsupported format) so behaviour is never worse than before. -async function downscale(bytes: Buffer): Promise { - try { - const img = await Jimp.read(bytes); - if (img.bitmap.width <= MAX_DIM && img.bitmap.height <= MAX_DIM) return bytes; - img.scaleToFit({ w: MAX_DIM, h: MAX_DIM }); - return await img.getBuffer(JimpMime.jpeg, { quality: JPEG_QUALITY }); - } catch { - return bytes; - } -} - -export async function put(placeId: string, bytes: Buffer, attribution: string | null): Promise { - const fp = filePath(placeId); - const tmp = fp + '.tmp'; - - const resized = await downscale(bytes); - await fsPromises.writeFile(tmp, resized); - await fsPromises.rename(tmp, fp); - - knownOnDisk.add(placeId); - recentFailures.delete(placeId); - - db.prepare( - 'INSERT OR REPLACE INTO google_place_photo_meta (place_id, attribution, fetched_at, error_at) VALUES (?, ?, ?, NULL)', - ).run(placeId, attribution, Date.now()); - - return { photoUrl: proxyUrl(placeId), filePath: fp, attribution }; -} - -export function getInFlight( - placeId: string, -): Promise<{ filePath: string; attribution: string | null } | null> | undefined { - return inFlight.get(placeId); -} - -export function setInFlight( - placeId: string, - promise: Promise<{ filePath: string; attribution: string | null } | null>, -): void { - inFlight.set(placeId, promise); - promise - .finally(() => inFlight.delete(placeId)) - .catch(() => { - /* awaiter logs; this .catch only prevents unhandledRejection */ - }); -} - -export function serveFilePath(placeId: string): string | null { - if (knownOnDisk.has(placeId)) return filePath(placeId); - const fp = filePath(placeId); - if (!fs.existsSync(fp)) return null; - knownOnDisk.add(placeId); - return fp; -} - -// 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 -// (covers coords: pseudo-ids, which never have a google_place_id). -function isReferenced(placeId: string): boolean { - // A collection-saved place copies image_url = proxyUrl(google_place_id) and/or - // the google_place_id itself, so collection_places must count as a referencing - // table — otherwise the nightly sweep + trip-place delete would evict a photo - // still shown on a collection thumbnail (#1081 photo-cache pitfall). - const row = db - .prepare( - `SELECT 1 FROM places WHERE google_place_id = ? OR image_url = ? - UNION ALL - SELECT 1 FROM collection_places WHERE google_place_id = ? OR image_url = ? - LIMIT 1`, - ) - .get(placeId, proxyUrl(placeId), placeId, proxyUrl(placeId)); - return !!row; -} - -function deleteEntry(placeId: string): void { - try { - fs.unlinkSync(filePath(placeId)); - } catch { - /* already gone */ - } - db.prepare('DELETE FROM google_place_photo_meta WHERE place_id = ?').run(placeId); - knownOnDisk.delete(placeId); -} - -// 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. -export function removeIfUnreferenced(placeId: string): void { - if (isReferenced(placeId)) return; - 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. -export function sweepOrphans(): number { - let removed = 0; - - const rows = db.prepare('SELECT place_id FROM google_place_photo_meta').all() as { place_id: string }[]; - const keepFiles = new Set(); - for (const { place_id } of rows) { - if (isReferenced(place_id)) { - keepFiles.add(`${crypto.createHash('sha1').update(place_id).digest('hex')}.jpg`); - } else { - 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(GOOGLE_PHOTO_DIR); - } catch { - entries = []; - } - for (const entry of entries) { - if (!entry.endsWith('.jpg') || keepFiles.has(entry)) continue; - try { - fs.unlinkSync(path.join(GOOGLE_PHOTO_DIR, entry)); - removed++; - } catch { - /* race */ - } - } - - return removed; -} diff --git a/server/tests/e2e/share.e2e.test.ts b/server/tests/e2e/share.e2e.test.ts index a9eea5b60..4131bfc33 100644 --- a/server/tests/e2e/share.e2e.test.ts +++ b/server/tests/e2e/share.e2e.test.ts @@ -45,8 +45,8 @@ import { PermissionsService } from '../../src/nest/permissions/permissions.servi // PermissionsService singleton (created in beforeAll, after build()). let checkPermission: MockInstance; -const { serveFilePath } = vi.hoisted(() => ({ serveFilePath: vi.fn() })); -vi.mock('../../src/services/placePhotoCache', () => ({ serveFilePath })); +// Overridden in the container rather than path-mocked: the cache is a provider now. +const serveFilePath = vi.fn(); import os from 'node:os'; import path from 'node:path'; @@ -54,6 +54,7 @@ import fs from 'node:fs'; import { createTables } from '../../src/db/schema'; import { runMigrations } from '../../src/db/migrations'; import { ShareModule } from '../../src/nest/share/share.module'; +import { PlacePhotoCacheService } from '../../src/nest/place-photos/place-photo-cache.service'; import { DatabaseModule } from '../../src/nest/database/database.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe'; @@ -64,7 +65,9 @@ describe('Share-link e2e (real auth guard + real SQL over temp SQLite)', () => { let tripId: number; async function build() { - const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, ShareModule] }).compile(); + const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, ShareModule] }) + .overrideProvider(PlacePhotoCacheService).useValue({ serveFilePath }) + .compile(); const nest = moduleRef.createNestApplication(); nest.use(cookieParser()); nest.useGlobalFilters(new TrekExceptionFilter()); diff --git a/server/tests/integration/share.test.ts b/server/tests/integration/share.test.ts index 40ef707a5..220102e8a 100644 --- a/server/tests/integration/share.test.ts +++ b/server/tests/integration/share.test.ts @@ -49,7 +49,14 @@ import { runMigrations } from '../../src/db/migrations'; import { resetTestDb, resetRateLimits } from '../helpers/test-db'; import { createUser, createTrip, addTripMember, createDay, createPlace, createDayAssignment, createDayNote } from '../helpers/factories'; import { authCookie } from '../helpers/auth'; -import * as placePhotoCache from '../../src/services/placePhotoCache'; +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'; + +// 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()); import fs from 'node:fs'; let nestApp: INestApplication; diff --git a/server/tests/unit/nest/collections.service.test.ts b/server/tests/unit/nest/collections.service.test.ts index 76d4ab9ab..a25f2fff2 100644 --- a/server/tests/unit/nest/collections.service.test.ts +++ b/server/tests/unit/nest/collections.service.test.ts @@ -53,10 +53,15 @@ import { DatabaseService } from '../../../src/nest/database/database.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { CollectionsService } from '../../../src/nest/collections/collections.service'; -import { removeIfUnreferenced } from '../../../src/services/placePhotoCache'; +import { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service'; +import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; import { PLACE_IMAGES_DIR } from '../../../src/nest/places/place-image'; const svc = new CollectionsService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService()); +// 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 removeIfUnreferenced = (id: string) => photoCache.removeIfUnreferenced(id); function clearCollections() { testDb.exec(` diff --git a/server/tests/unit/nest/maps.service.test.ts b/server/tests/unit/nest/maps.service.test.ts index 266098f8c..462e840f7 100644 --- a/server/tests/unit/nest/maps.service.test.ts +++ b/server/tests/unit/nest/maps.service.test.ts @@ -88,7 +88,9 @@ vi.mock('../../../src/config', () => ({ ENCRYPTION_KEY: '0'.repeat(64), })); -vi.mock('../../../src/services/placePhotoCache', () => ({ +// Injected stub since the photo-cache fold (was a path mock of the module). +// Same seven seams, same mock functions behind them. +const photoCacheStub = { get: (placeId: string) => mockCacheGet(placeId), getErrored: (placeId: string) => mockCacheGetErrored(placeId), put: (placeId: string, bytes: Buffer, attribution: string | null) => mockCachePut(placeId, bytes, attribution), @@ -96,16 +98,17 @@ vi.mock('../../../src/services/placePhotoCache', () => ({ getInFlight: (placeId: string) => mockCacheGetInFlight(placeId), setInFlight: (placeId: string, p: Promise) => mockCacheSetInFlight(placeId, p), serveFilePath: (placeId: string) => mockServeFilePath(placeId), -})); +} as unknown as PlacePhotoCacheService; import { db } from '../../../src/db/database'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { MapsService } from '../../../src/nest/maps/maps.service'; +import type { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service'; // The service under test, constructed over the mocked db stub — DatabaseService // routes get/run through the stubbed prepare(), so mockDbGet/mockDbRun keep // flowing exactly as they did for the legacy module. -const svc = new MapsService(new DatabaseService(db as never)); +const svc = new MapsService(new DatabaseService(db as never), photoCacheStub); afterEach(() => { vi.unstubAllGlobals(); @@ -2155,7 +2158,7 @@ function makeSettingsDb(row?: { value: string }) { } function settingsSvc(row?: { value: string }) { - return new MapsService(makeSettingsDb(row).db); + return new MapsService(makeSettingsDb(row).db, photoCacheStub); } describe('kill-switch settings reads', () => { @@ -2179,7 +2182,7 @@ describe('kill-switch settings reads', () => { it('queries the matching app_settings key', () => { const { db: settingsDb, get } = makeSettingsDb({ value: 'true' }); - const s = new MapsService(settingsDb); + const s = new MapsService(settingsDb, photoCacheStub); s.autocompleteDisabled(); expect(get).toHaveBeenCalledWith(expect.stringContaining('app_settings'), 'places_autocomplete_enabled'); s.detailsDisabled(); diff --git a/server/tests/unit/services/placePhotoCache.test.ts b/server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts similarity index 86% rename from server/tests/unit/services/placePhotoCache.test.ts rename to server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts index 196859f40..850a58b95 100644 --- a/server/tests/unit/services/placePhotoCache.test.ts +++ b/server/tests/unit/nest/place-photos/place-photo-cache.service.test.ts @@ -13,15 +13,21 @@ import crypto from 'node:crypto'; import { Jimp, JimpMime } from 'jimp'; import Database from 'better-sqlite3'; -// Throwaway upload dir — set before importing the module under test (it reads the -// env at load time and mkdirs the dir). -const TMP_DIR = fs.mkdtempSync(path.join(os.tmpdir(), 'ppc-')); -process.env.TREK_PLACE_PHOTO_DIR = TMP_DIR; +// 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 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:') }; +}); -// Minimal real DB with just the tables placePhotoCache touches. isReferenced now +// Minimal real DB with just the tables the cache touches. isReferenced now // UNIONs collection_places (#1081 photo-cache fix), so the bare fixture must // declare it too or the reference check would throw "no such table". -const testDb = new Database(':memory:'); testDb.exec(` CREATE TABLE places ( id INTEGER PRIMARY KEY AUTOINCREMENT, @@ -41,7 +47,7 @@ testDb.exec(` ); `); -vi.mock('../../../src/db/database', () => ({ db: testDb })); +vi.mock('../../../../src/db/database', () => ({ db: testDb })); function filePathFor(placeId: string): string { const hash = crypto.createHash('sha1').update(placeId).digest('hex'); @@ -53,10 +59,16 @@ async function makeJpeg(width: number, height: number): Promise { return img.getBuffer(JimpMime.jpeg, { quality: 80 }); } -let cache: typeof import('../../../src/services/placePhotoCache'); +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'; -beforeAll(async () => { - cache = await import('../../../src/services/placePhotoCache'); +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(); }); beforeEach(() => { diff --git a/server/tests/unit/nest/places.service.test.ts b/server/tests/unit/nest/places.service.test.ts index 794ff5099..1ff3a0769 100644 --- a/server/tests/unit/nest/places.service.test.ts +++ b/server/tests/unit/nest/places.service.test.ts @@ -10,6 +10,7 @@ * are mocked where needed. */ import { describe, it, expect, vi, beforeAll, beforeEach, afterEach, afterAll } from 'vitest'; +import type { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service'; import { UnsplashService } from '../../../src/nest/unsplash/unsplash.service'; import { RuntimeEnvService } from '../../../src/nest/app-config/runtime-env.service'; import { TRACK_COLORS } from '@trek/shared'; @@ -52,11 +53,9 @@ vi.mock('../../../src/config', () => ({ // Spy on the photo-cache reclaim hook so delete tests assert the wiring without // touching disk. The removal logic itself is covered in placePhotoCache.test.ts. -const { removeIfUnreferencedSpy } = vi.hoisted(() => ({ removeIfUnreferencedSpy: vi.fn() })); -vi.mock('../../../src/services/placePhotoCache', async (importOriginal) => ({ - ...(await importOriginal()), - removeIfUnreferenced: removeIfUnreferencedSpy, -})); +// Injected stub since the photo-cache fold (was a partial path mock). +const removeIfUnreferencedSpy = vi.fn(); +const photoCacheStub = { removeIfUnreferenced: removeIfUnreferencedSpy } as unknown as PlacePhotoCacheService; import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; @@ -76,7 +75,7 @@ const GPX_FIXTURE = path.join(__dirname, '../../fixtures/test.gpx'); const KML_FIXTURE = path.join(__dirname, '../../fixtures/test.kml'); const dbs = new DatabaseService(testDb); -const svc = new PlacesService(dbs, new PermissionsService(dbs), new RealtimeService(), new MapsService(dbs), new QueryHelpersService(dbs), new UnsplashService(dbs, new RuntimeEnvService())); +const svc = new PlacesService(dbs, new PermissionsService(dbs), new RealtimeService(), new MapsService(dbs), new QueryHelpersService(dbs), new UnsplashService(dbs, new RuntimeEnvService()), photoCacheStub); beforeAll(() => { createTables(testDb); @@ -1014,7 +1013,7 @@ describe('PlacesService — automatic track colours (#776)', () => { describe('enrichImportedPlaces', () => { function enrichSvc(maps: Partial) { - return new PlacesService(dbs, new PermissionsService(dbs), new RealtimeService(), maps as MapsService, new QueryHelpersService(dbs), new UnsplashService(dbs, new RuntimeEnvService())); + return new PlacesService(dbs, new PermissionsService(dbs), new RealtimeService(), maps as MapsService, new QueryHelpersService(dbs), new UnsplashService(dbs, new RuntimeEnvService()), photoCacheStub); } it('PLACE-SVC-058 — no-ops when no Google Maps key is configured', async () => { diff --git a/server/tests/unit/nest/share.service.test.ts b/server/tests/unit/nest/share.service.test.ts index 01a460476..eca53a484 100644 --- a/server/tests/unit/nest/share.service.test.ts +++ b/server/tests/unit/nest/share.service.test.ts @@ -7,6 +7,7 @@ * SQL logic is exercised faithfully. */ import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; +import type { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service'; // ── DB setup ────────────────────────────────────────────────────────────────── @@ -43,8 +44,9 @@ vi.mock('../../../src/config', () => ({ const checkPermission = vi.fn(); const permissionsStub = { checkPermission } as unknown as PermissionsService; -const { serveFilePath } = vi.hoisted(() => ({ serveFilePath: vi.fn() })); -vi.mock('../../../src/services/placePhotoCache', () => ({ serveFilePath })); +// 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; import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; @@ -59,7 +61,7 @@ import { SettingsService } from '../../../src/nest/settings/settings.service'; import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; import type { User } from '../../../src/types'; -const svc = new ShareService(new DatabaseService(testDb), new SettingsService(new DatabaseService(testDb)), permissionsStub, new QueryHelpersService(new DatabaseService(testDb))); +const svc = new ShareService(new DatabaseService(testDb), new SettingsService(new DatabaseService(testDb)), permissionsStub, new QueryHelpersService(new DatabaseService(testDb)), photoCacheStub); beforeAll(() => { createTables(testDb);