refactor(place-photos): route place-photo cache through storage

This commit is contained in:
jubnl
2026-08-17 05:20:09 +02:00
parent d27e54952a
commit 4eca78b53e
28 changed files with 430 additions and 365 deletions
+1 -1
View File
@@ -171,7 +171,7 @@ export class MapsController {
@Get('place-photo/:placeId/bytes')
async placePhotoBytes(@Param('placeId') placeId: string, @Res() res: Response): Promise<void> {
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
+7 -7
View File
@@ -529,7 +529,7 @@ export class MapsService {
return this.getPlacePhoto(userId, placeId, lat, lng, name) as Promise<MapsPlacePhotoResult>;
}
photoBytesKey(placeId: string): string | null {
photoBytesKey(placeId: string): Promise<string | null> {
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
@@ -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);
}
}
@@ -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<Buffer | null>,
): Promise<string | null> {
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<CachedEnrichment | null> {
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 = {
@@ -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<void> {
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}`);
@@ -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 '<sha1>.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<string, number>();
/** In-flight dedup: stops a stampede when several requests miss the same placeId at once. */
private readonly inFlight = new Map<string, Promise<{ filePath: string; attribution: string | null } | null>>();
/** placeIds whose file was confirmed on disk this session — saves an existsSync per hit. */
private readonly inFlight = new Map<string, Promise<{ attribution: string | null } | null>>();
/** placeIds whose object was confirmed in storage this session — saves a stat per hit. */
private readonly knownOnDisk = new Set<string>();
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<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,
@@ -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<CachedPhoto> {
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<string | null> {
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<void> {
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<void> {
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<number> {
let removed = 0;
const rows = this.db.all<{ place_id: string }>('SELECT place_id FROM google_place_photo_meta');
const keepFiles = new Set<string>();
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 */
@@ -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],
})
+4 -4
View File
@@ -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);
+2 -2
View File
@@ -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<void> {
const candidates = new Set<string>();
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 */ }
}
}
+2 -2
View File
@@ -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 });
+4 -4
View File
@@ -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<boolean> {
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<number[]> {
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;
+1 -1
View File
@@ -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<void> {
const key = this.share.getSharedPlacePhotoKey(token, placeId);
const key = await this.share.getSharedPlacePhotoKey(token, placeId);
if (!key) {
this.emptyPhoto(res);
return;
+1 -1
View File
@@ -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<string | null> {
const shareRow = this.dbs.get<{ trip_id: string; share_map: number }>(
"SELECT trip_id, share_map FROM share_tokens WHERE token = ? AND (expires_at IS NULL OR expires_at > datetime('now'))",
token,
@@ -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)
+2 -1
View File
@@ -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
+2 -1
View File
@@ -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);
+41
View File
@@ -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/<keyPrefix><name>. */
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 });
},
};
}
+21 -7
View File
@@ -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);
@@ -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)');
});
@@ -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();
});
@@ -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();
+1 -2
View File
@@ -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();
@@ -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', '<sha1>.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<Buffer> {
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 <sha1>.jpg storage name when the photo is on disk', async () => {
fs.writeFileSync(filePathFor('p-key'), await makeJpeg(10, 10));
const hash = crypto.createHash('sha1').update('p-key').digest('hex');
expect(cache.serveKey('p-key')).toBe(`${hash}.jpg`);
});
it('returns null when nothing is cached', () => {
expect(cache.serveKey('p-missing')).toBeNull();
});
});
describe('placePhotoCache.removeIfUnreferenced()', () => {
it('PPC-004: removes a cache entry that no place references', async () => {
await cache.put('orphan', await makeJpeg(50, 50), null);
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 <sha1>.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);
});
});
});
+20 -20
View File
@@ -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.
+9 -9
View File
@@ -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);
});
});
@@ -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()),
+2 -1
View File
@@ -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()),
@@ -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(