Merge pull request #3967 from bluewave-labs/fix/cache-db-settings

fix: cache DB settings

server/src/domain/app-settings/app-settings.service.ts

@@ -23,16 +23,27 @@ export interface ISettingsService {
 	getSettings(): EnvConfig;
 	areStatusPageThemesEnabled(): boolean;
 	getDBSettings(): Promise<Settings>;
+	getCachedDBSettings(): Promise<Settings>;
 	updateDbSettings(newSettings: SettingsUpdate): Promise<Settings>;
 	removeEgressNotification(notificationId: string): Promise<void>;
 }
 
+const DB_SETTINGS_CACHE_TTL_MS = 60_000;
+
+type CachedSettings = {
+	value: Promise<Settings>;
+	expiresAt: number;
+};
 export class SettingsService implements ISettingsService {
 	static SERVICE_NAME = SERVICE_NAME;
 	private settings: EnvConfig;
 	private settingsRepository: ISettingsRepository | null = null;
+	private dbSettingsCache: CachedSettings | null = null;
 
-	constructor(env: ValidatedEnv) {
+	constructor(
+		env: ValidatedEnv,
+		private dbSettingsTTLMs: number = DB_SETTINGS_CACHE_TTL_MS
+	) {
 		this.settings = {
 			jwtSecret: env.JWT_SECRET,
 			jwtTTL: env.TOKEN_TTL as StringValue,
@@ -76,11 +87,14 @@ export class SettingsService implements ISettingsService {
 	}
 
 	updateDbSettings = async (newSettings: SettingsUpdate) => {
-		return await this.getRepository().update(newSettings);
+		const updated = await this.getRepository().update(newSettings);
+		this.dbSettingsCache = null;
+		return updated;
 	};
 
 	removeEgressNotification = async (notificationId: string) => {
 		await this.getRepository().removeEgressNotification(notificationId);
+		this.dbSettingsCache = null;
 	};
 
 	getDBSettings = async () => {
@@ -100,4 +114,17 @@ export class SettingsService implements ISettingsService {
 
 		return settings;
 	};
+
+	getCachedDBSettings = async () => {
+		const hit = this.dbSettingsCache;
+		if (hit && hit.expiresAt > Date.now()) {
+			return hit.value;
+		}
+		const entry: CachedSettings = { value: this.getDBSettings(), expiresAt: Date.now() + this.dbSettingsTTLMs };
+		this.dbSettingsCache = entry;
+		entry.value.catch(() => {
+			if (this.dbSettingsCache === entry) this.dbSettingsCache = null;
+		});
+		return entry.value;
+	};
 }

server/src/domain/egress/egress.service.ts

@@ -173,7 +173,7 @@ export class EgressService implements IEgressService {
 
 	private assess = async (): Promise<EgressAssessment | null> => {
 		try {
-			const settings = await this.settingsService.getDBSettings();
+			const settings = await this.settingsService.getCachedDBSettings();
 			if (!settings.egressCheckEnabled) {
 				return "disabled";
 			}
@@ -250,7 +250,7 @@ export class EgressService implements IEgressService {
 	// Runs on the queue at the configured interval while degraded. Errors propagate so the queue records
 	// the failure and retries with its usual backoff.
 	checkRecovery = async (job: Job): Promise<void> => {
-		const settings = await this.settingsService.getDBSettings();
+		const settings = await this.settingsService.getCachedDBSettings();
 		const state = await this.egressStateRepository.findSingleton();
 		if (!settings.egressCheckEnabled || state.status !== "degraded") {
 			// Feature switched off, recovery recorded by another worker, or state reset: nothing left to poll for.

server/src/service/network/PageSpeedProvider.ts

@@ -22,7 +22,7 @@ export class PageSpeedProvider implements IStatusProvider<PageSpeedStatusPayload
 		const { url, strategy } = monitor;
 		try {
 			if (!url) throw new Error("URL is required for PageSpeed monitor");
-			const dbSettings = await this.settingsService.getDBSettings();
+			const dbSettings = await this.settingsService.getCachedDBSettings();
 			const apiKey = dbSettings?.pagespeedApiKey;
 			const resolvedStrategy = strategy ?? DefaultPageSpeedStrategy;
 			let pageSpeedUrl = `https://pagespeedonline.googleapis.com/pagespeedonline/v5/runPagespeed?url=${encodeURIComponent(

server/src/service/network/ProxyResolver.ts

@@ -73,7 +73,7 @@ export class ProxyResolver implements IProxyResolver {
 	};
 
 	private resolveInherit = async (monitor: Monitor): Promise<string | undefined> => {
-		const settings = await this.getCached("settings", () => this.settingsService.getDBSettings());
+		const settings = await this.settingsService.getCachedDBSettings();
 		if (!settings.globalProxyEnabled) {
 			return undefined;
 		}

server/test/unit/providers/network/pageSpeedProvider.test.ts

@@ -33,7 +33,7 @@ const createMockHttpProvider = () =>
 
 const createMockSettingsService = (apiKey?: string) =>
 	({
-		getDBSettings: jest.fn().mockResolvedValue({ pagespeedApiKey: apiKey }),
+		getCachedDBSettings: jest.fn().mockResolvedValue({ pagespeedApiKey: apiKey }),
 	}) as unknown as jest.Mocked<ISettingsService>;
 
 const createProvider = (opts?: { apiKey?: string }) => {
@@ -154,7 +154,7 @@ describe("PageSpeedProvider", () => {
 		const logger = createMockLogger();
 		const httpProvider = createMockHttpProvider();
 		const settingsService = {
-			getDBSettings: jest.fn().mockResolvedValue(null),
+			getCachedDBSettings: jest.fn().mockResolvedValue(null),
 		} as unknown as jest.Mocked<ISettingsService>;
 		const provider = new PageSpeedProvider(httpProvider, settingsService, logger as any);
 

server/test/unit/services/egressService.test.ts

@@ -66,7 +66,7 @@ const degradedRepository = (overrides?: Record<string, any>) => ({
 
 const createService = (overrides?: Record<string, any>) => {
 	const defaults = {
-		settingsService: { getDBSettings: jest.fn().mockResolvedValue(makeSettings()) },
+		settingsService: { getCachedDBSettings: jest.fn().mockResolvedValue(makeSettings()) },
 		egressStateRepository: {
 			findSingleton: jest.fn().mockResolvedValue(makeState()),
 			recordProbe: jest.fn().mockResolvedValue(makeState()),
@@ -201,7 +201,7 @@ describe("EgressService", () => {
 	describe("assessAfterFailure", () => {
 		it("returns null and does not probe or record when the feature is disabled", async () => {
 			const { service, defaults } = createService({
-				settingsService: { getDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressCheckEnabled: false })) },
+				settingsService: { getCachedDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressCheckEnabled: false })) },
 			});
 
 			const result = await service.assessAfterFailure();
@@ -299,7 +299,7 @@ describe("EgressService", () => {
 
 		it("uses the default poll interval for the recovery job when the setting is missing or invalid", async () => {
 			const { service, defaults } = createService({
-				settingsService: { getDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressPollIntervalSeconds: undefined })) },
+				settingsService: { getCachedDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressPollIntervalSeconds: undefined })) },
 				networkService: { requestStatus: statusFor([]) },
 			});
 
@@ -310,7 +310,7 @@ describe("EgressService", () => {
 
 		it("falls back to the default targets when the configured list is empty", async () => {
 			const { service, defaults } = createService({
-				settingsService: { getDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressCheckTargets: [] })) },
+				settingsService: { getCachedDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressCheckTargets: [] })) },
 			});
 
 			await service.assessAfterFailure();
@@ -320,7 +320,7 @@ describe("EgressService", () => {
 
 		it("returns null and logs when the assessment itself fails", async () => {
 			const { service, defaults } = createService({
-				settingsService: { getDBSettings: jest.fn().mockRejectedValue(new Error("db down")) },
+				settingsService: { getCachedDBSettings: jest.fn().mockRejectedValue(new Error("db down")) },
 			});
 
 			const result = await service.assessAfterFailure();
@@ -339,7 +339,7 @@ describe("EgressService", () => {
 			const results = await Promise.all([service.assessAfterFailure(), service.assessAfterFailure(), service.assessAfterFailure()]);
 
 			expect(results).toEqual(["ok", "ok", "ok"]);
-			expect(defaults.settingsService.getDBSettings).toHaveBeenCalledTimes(1);
+			expect(defaults.settingsService.getCachedDBSettings).toHaveBeenCalledTimes(1);
 			expect(defaults.networkService.requestStatus).toHaveBeenCalledTimes(2); // one call per target, once
 			expect(defaults.egressStateRepository.recordProbe).toHaveBeenCalledTimes(1);
 		});
@@ -372,18 +372,18 @@ describe("EgressService", () => {
 		});
 
 		it("reuses a disabled result instead of re-reading settings for every failure", async () => {
-			const getDBSettings = jest.fn().mockResolvedValue(makeSettings({ egressCheckEnabled: false }));
-			const { service } = createService({ settingsService: { getDBSettings } });
+			const getCachedDBSettings = jest.fn().mockResolvedValue(makeSettings({ egressCheckEnabled: false }));
+			const { service } = createService({ settingsService: { getCachedDBSettings } });
 
 			expect(await service.assessAfterFailure()).toBeNull();
 			expect(await service.assessAfterFailure()).toBeNull();
 
-			expect(getDBSettings).toHaveBeenCalledTimes(1);
+			expect(getCachedDBSettings).toHaveBeenCalledTimes(1);
 		});
 
 		it("does not cache an internal failure", async () => {
-			const getDBSettings = jest.fn().mockRejectedValueOnce(new Error("db down")).mockResolvedValue(makeSettings());
-			const { service, defaults } = createService({ settingsService: { getDBSettings } });
+			const getCachedDBSettings = jest.fn().mockRejectedValueOnce(new Error("db down")).mockResolvedValue(makeSettings());
+			const { service, defaults } = createService({ settingsService: { getCachedDBSettings } });
 
 			expect(await service.assessAfterFailure()).toBeNull();
 			expect(await service.assessAfterFailure()).toBe("ok");
@@ -451,7 +451,7 @@ describe("EgressService", () => {
 
 		it("releases the job without probing when the feature has been disabled mid-episode", async () => {
 			const { service, defaults } = createService({
-				settingsService: { getDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressCheckEnabled: false })) },
+				settingsService: { getCachedDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressCheckEnabled: false })) },
 				egressStateRepository: degradedRepository(),
 			});
 

server/test/unit/services/proxyResolver.test.ts

@@ -27,7 +27,7 @@ const makeProxy = (overrides?: Partial<Proxy>): Proxy =>
 const createResolver = (overrides?: Record<string, any>) => {
 	const defaults = {
 		proxiesRepository: { findByIdOrNull: jest.fn<() => Promise<Proxy | null>>().mockResolvedValue(makeProxy()) },
-		settingsService: { getDBSettings: jest.fn<() => Promise<any>>().mockResolvedValue({ globalProxyEnabled: true, globalProxyId: "p1" }) },
+		settingsService: { getCachedDBSettings: jest.fn<() => Promise<any>>().mockResolvedValue({ globalProxyEnabled: true, globalProxyId: "p1" }) },
 		logger: createMockLogger(),
 		ttlMs: 60_000,
 		...overrides,
@@ -46,7 +46,7 @@ describe("ProxyResolver", () => {
 
 		expect(result).toBeUndefined();
 		expect(defaults.proxiesRepository.findByIdOrNull).not.toHaveBeenCalled();
-		expect(defaults.settingsService.getDBSettings).not.toHaveBeenCalled();
+		expect(defaults.settingsService.getCachedDBSettings).not.toHaveBeenCalled();
 	});
 
 	it("returns undefined for proxyMode 'none' without calling settings", async () => {
@@ -55,7 +55,7 @@ describe("ProxyResolver", () => {
 		const result = await resolver.resolve(makeMonitor({ proxyMode: "none" }));
 
 		expect(result).toBeUndefined();
-		expect(defaults.settingsService.getDBSettings).not.toHaveBeenCalled();
+		expect(defaults.settingsService.getCachedDBSettings).not.toHaveBeenCalled();
 	});
 
 	// ── custom mode ───────────────────────────────────────────────────────────
@@ -94,7 +94,7 @@ describe("ProxyResolver", () => {
 
 	it("returns undefined when the global proxy is disabled, without a repo call", async () => {
 		const { resolver, defaults } = createResolver({
-			settingsService: { getDBSettings: jest.fn<() => Promise<any>>().mockResolvedValue({ globalProxyEnabled: false, globalProxyId: "p1" }) },
+			settingsService: { getCachedDBSettings: jest.fn<() => Promise<any>>().mockResolvedValue({ globalProxyEnabled: false, globalProxyId: "p1" }) },
 		});
 
 		const result = await resolver.resolve(makeMonitor({ proxyMode: "inherit" }));
@@ -114,7 +114,7 @@ describe("ProxyResolver", () => {
 
 	it("returns undefined when the global proxy is enabled but globalProxyId is null, without a repo call", async () => {
 		const { resolver, defaults } = createResolver({
-			settingsService: { getDBSettings: jest.fn<() => Promise<any>>().mockResolvedValue({ globalProxyEnabled: true, globalProxyId: null }) },
+			settingsService: { getCachedDBSettings: jest.fn<() => Promise<any>>().mockResolvedValue({ globalProxyEnabled: true, globalProxyId: null }) },
 		});
 
 		const result = await resolver.resolve(makeMonitor({ proxyMode: "inherit" }));
@@ -140,7 +140,7 @@ describe("ProxyResolver", () => {
 		const result = await resolver.resolve(makeMonitor({ proxyMode: "garbage" as Monitor["proxyMode"] }));
 
 		expect(result).toBe("http://proxy.example.com:8080");
-		expect(defaults.settingsService.getDBSettings).toHaveBeenCalled();
+		expect(defaults.settingsService.getCachedDBSettings).toHaveBeenCalled();
 	});
 
 	// ── never throws ──────────────────────────────────────────────────────────
@@ -158,7 +158,7 @@ describe("ProxyResolver", () => {
 
 	it("returns undefined and warns when the settings service throws", async () => {
 		const { resolver, defaults } = createResolver({
-			settingsService: { getDBSettings: jest.fn<() => Promise<any>>().mockRejectedValue(new Error("db down")) },
+			settingsService: { getCachedDBSettings: jest.fn<() => Promise<any>>().mockRejectedValue(new Error("db down")) },
 		});
 
 		const result = await resolver.resolve(makeMonitor({ proxyMode: "inherit" }));
@@ -175,7 +175,6 @@ describe("ProxyResolver", () => {
 		await resolver.resolve(makeMonitor({ proxyMode: "inherit" }));
 		await resolver.resolve(makeMonitor({ proxyMode: "inherit" }));
 
-		expect(defaults.settingsService.getDBSettings).toHaveBeenCalledTimes(1);
 		expect(defaults.proxiesRepository.findByIdOrNull).toHaveBeenCalledTimes(1);
 	});
 
@@ -185,7 +184,6 @@ describe("ProxyResolver", () => {
 		await resolver.resolve(makeMonitor({ proxyMode: "inherit" }));
 		await resolver.resolve(makeMonitor({ proxyMode: "inherit" }));
 
-		expect(defaults.settingsService.getDBSettings).toHaveBeenCalledTimes(2);
 		expect(defaults.proxiesRepository.findByIdOrNull).toHaveBeenCalledTimes(2);
 	});
 
@@ -209,7 +207,6 @@ describe("ProxyResolver", () => {
 			resolver.resolve(makeMonitor({ proxyMode: "inherit" })),
 		]);
 
-		expect(defaults.settingsService.getDBSettings).toHaveBeenCalledTimes(1);
 		expect(defaults.proxiesRepository.findByIdOrNull).toHaveBeenCalledTimes(1);
 	});
 

server/test/unit/services/settingsService.test.ts

@@ -47,9 +47,9 @@ const createSettingsRepo = () =>
 		deleteLegacy: jest.fn().mockResolvedValue(true),
 	}) as unknown as jest.Mocked<ISettingsRepository>;
 
-const createService = (envOverrides?: Partial<ValidatedEnv>) => {
+const createService = (envOverrides?: Partial<ValidatedEnv>, dbSettingsTTLMs?: number) => {
 	const env = makeEnv(envOverrides);
-	const service = new SettingsService(env);
+	const service = new SettingsService(env, dbSettingsTTLMs);
 	const settingsRepository = createSettingsRepo();
 	service.setRepository(settingsRepository);
 	return { service, settingsRepository, env };
@@ -172,6 +172,66 @@ describe("SettingsService", () => {
 		});
 	});
 
+	// ── getCachedDBSettings ─────────────────────────────────────────────────
+
+	describe("getCachedDBSettings", () => {
+		it("serves a second call within the TTL from cache", async () => {
+			const { service, settingsRepository } = createService();
+
+			const first = await service.getCachedDBSettings();
+			const second = await service.getCachedDBSettings();
+
+			expect(second).toBe(first);
+			expect(settingsRepository.findSingleton).toHaveBeenCalledTimes(1);
+		});
+
+		it("refetches after the TTL expires", async () => {
+			const { service, settingsRepository } = createService(undefined, 0);
+
+			await service.getCachedDBSettings();
+			await service.getCachedDBSettings();
+
+			expect(settingsRepository.findSingleton).toHaveBeenCalledTimes(2);
+		});
+
+		it("shares one in-flight fetch between concurrent calls", async () => {
+			const { service, settingsRepository } = createService();
+
+			await Promise.all([service.getCachedDBSettings(), service.getCachedDBSettings(), service.getCachedDBSettings()]);
+
+			expect(settingsRepository.findSingleton).toHaveBeenCalledTimes(1);
+		});
+
+		it("does not cache a rejected fetch, the next call retries", async () => {
+			const { service, settingsRepository } = createService();
+			(settingsRepository.findSingleton as jest.Mock).mockRejectedValueOnce(new Error("db down")).mockResolvedValueOnce(makeSettings());
+
+			await expect(service.getCachedDBSettings()).rejects.toThrow("db down");
+			await expect(service.getCachedDBSettings()).resolves.toEqual(makeSettings());
+			expect(settingsRepository.findSingleton).toHaveBeenCalledTimes(2);
+		});
+
+		it("refetches after updateDbSettings so the write is visible immediately", async () => {
+			const { service, settingsRepository } = createService();
+
+			await service.getCachedDBSettings();
+			await service.updateDbSettings({ checkTTL: 60 });
+			await service.getCachedDBSettings();
+
+			expect(settingsRepository.findSingleton).toHaveBeenCalledTimes(2);
+		});
+
+		it("refetches after removeEgressNotification", async () => {
+			const { service, settingsRepository } = createService();
+
+			await service.getCachedDBSettings();
+			await service.removeEgressNotification("notif-1");
+			await service.getCachedDBSettings();
+
+			expect(settingsRepository.findSingleton).toHaveBeenCalledTimes(2);
+		});
+	});
+
 	// ── updateDbSettings ────────────────────────────────────────────────────
 
 	describe("updateDbSettings", () => {