mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-24 16:03:02 +00:00
refactor(server): make the place-photo cache a provider (#1833)
The last file in be-places-helpers-fold, and the one that had to be a provider rather than a move: it carries three in-memory structures that only do their job if the process has exactly one of them. recentFailures stops a screenful of markers hammering an API that is down inFlight stops a stampede when several requests miss the same placeId knownOnDisk saves an existsSync per cache hit A second instance gives the stampede guard its own inFlight map, so two simultaneous requests for the same uncached place both call the provider. That is why the scheduler takes this one from the container instead of constructing it: SchedulerDeps gains a placePhotos entry alongside backups, filled from app.get() in index.ts — the shape PR #1827 was built for. The module-scope mkdirSync becomes onModuleInit, and GOOGLE_PHOTO_DIR is read per call through RuntimeEnvService rather than frozen at import. That is not cosmetic: TREK_PLACE_PHOTO_DIR is the seam the tests use to keep off the real uploads tree, and its own test used to have to import the module dynamically to set the variable in time. The __dirname fallback moved a level with the file and is corrected accordingly. Consumers: maps, places and share inject it; places.helpers.reclaimPhotoCache takes it as a parameter, since it is a free function. Six test seams follow. Four path mocks become injected stubs with the same mock functions behind them; the collections case keeps the REAL implementation, because what it asserts is that removeIfUnreferenced counts collection_places as a reference (#1081) — a stub there would assert nothing. The share e2e overrides the provider in its container. services/ is down to 14 top-level files, from 44 when this started.
This commit is contained in:
@@ -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(`
|
||||
|
||||
@@ -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<any>) => 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();
|
||||
|
||||
+22
-10
@@ -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<Buffer> {
|
||||
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(() => {
|
||||
@@ -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<typeof import('../../../src/services/placePhotoCache')>()),
|
||||
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<MapsService>) {
|
||||
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 () => {
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user