fix(cache): dedupe cache key handling and harden cache lifecycle safety (#1570)

* fix(cache): dedupe cache key handling and harden cache lifecycle safety

* fix(cache): address review findings in circuit-breaker hardening PR

- src/utils/index.ts: remove duplicate re-export of storage-quota symbols;
  single import at bottom of file now serves both local use (saveToStorage)
  and re-export to consumers
- src/services/market/index.ts: document first-wins semantics on
  uppercaseMetaMap so the intent is clear to future readers
- src/services/persistent-cache.ts: replace double-negation regex predicate
  with explicit suffix.length === 0 form for readability
- src-tauri/src/main.rs: replace !any(ch != ':') with is_empty() ||
  all(ch == ':') — same logic, no double negation
- tests/tech-readiness-circuit-breakers.test.mjs: note that the typescript
  import is the devDep already required by tsc

---------

Co-authored-by: Elie Habib <elie.habib@gmail.com>
This commit is contained in:
Fayez Bast
2026-03-19 09:08:04 +02:00
committed by GitHub
parent 29ed8b0fb3
commit 8c6177b927
8 changed files with 270 additions and 79 deletions

View File

@@ -435,6 +435,12 @@ fn delete_cache_entry(webview: Webview, app: AppHandle, cache: tauri::State<'_,
#[tauri::command]
fn delete_cache_entries_by_prefix(webview: Webview, app: AppHandle, cache: tauri::State<'_, PersistentCache>, prefix: String) -> Result<(), String> {
require_trusted_window(webview.label())?;
let suffix = prefix
.strip_prefix("breaker:")
.ok_or_else(|| "delete_cache_entries_by_prefix only accepts breaker: prefixes".to_string())?;
if suffix.is_empty() || suffix.chars().all(|ch| ch == ':') {
return Err("delete_cache_entries_by_prefix requires a specific breaker: prefix".to_string());
}
let removed_any = {
let mut data = cache.data.lock().unwrap_or_else(|e| e.into_inner());
let before = data.len();

View File

@@ -68,12 +68,8 @@ export interface MarketFetchResult {
const lastSuccessfulByKey = new Map<string, MarketData[]>();
function trimSymbol(symbol: string): string {
return symbol.trim();
}
function symbolSetKey(symbols: string[]): string {
return [...new Set(symbols.map(trimSymbol))].sort().join(',');
return [...new Set(symbols.map((symbol) => symbol.trim()))].sort().join(',');
}
export async function fetchMultipleStocks(
@@ -83,17 +79,19 @@ export async function fetchMultipleStocks(
// Preserve exact requested symbols for cache keys and request payloads so
// case-distinct instruments do not collapse into one cache entry.
const symbolMetaMap = new Map<string, { symbol: string; name: string; display: string }>();
const uppercaseMetaMap = new Map<string, { symbol: string; name: string; display: string } | null>();
// Case-insensitive fallback: maps UPPER(symbol) → first requested candidate.
// "First wins" is intentional — assumes case-variants are the same instrument
// (e.g. btc-usd / BTC-USD both refer to the same asset). When the backend
// normalizes casing (e.g. returns "Btc-Usd"), we still recover metadata
// rather than silently dropping it as the old null-sentinel approach did.
const uppercaseMetaMap = new Map<string, { symbol: string; name: string; display: string }>();
for (const s of symbols) {
const trimmed = trimSymbol(s.symbol);
const trimmed = s.symbol.trim();
if (!symbolMetaMap.has(trimmed)) symbolMetaMap.set(trimmed, s);
const upper = trimmed.toUpperCase();
const existingUpper = uppercaseMetaMap.get(upper);
if (existingUpper === undefined) {
if (!uppercaseMetaMap.has(upper)) {
uppercaseMetaMap.set(upper, s);
} else if (existingUpper !== null && existingUpper.symbol !== s.symbol) {
uppercaseMetaMap.set(upper, null);
}
}
const allSymbolStrings = [...symbolMetaMap.keys()];
@@ -108,7 +106,7 @@ export async function fetchMultipleStocks(
});
const results = resp.quotes.map((q) => {
const trimmed = trimSymbol(q.symbol);
const trimmed = q.symbol.trim();
const meta = symbolMetaMap.get(trimmed) ?? uppercaseMetaMap.get(trimmed.toUpperCase()) ?? undefined;
return toMarketData(q, meta);
});

View File

@@ -1,6 +1,6 @@
import { isDesktopRuntime } from './runtime';
import { invokeTauri } from './tauri-bridge';
import { isStorageQuotaExceeded, isQuotaError, markStorageQuotaExceeded } from '@/utils';
import { isStorageQuotaExceeded, isQuotaError, markStorageQuotaExceeded } from '@/utils/storage-quota';
type CacheEnvelope<T> = {
key: string;
@@ -107,6 +107,14 @@ function deleteFromLocalStorageByPrefix(prefix: string): void {
}
}
function validateBreakerPrefix(prefix: string): void {
const trimmed = prefix.trim();
const suffix = trimmed.slice('breaker:'.length);
if (!trimmed.startsWith('breaker:') || suffix.length === 0 || !/\w/.test(suffix)) {
throw new Error('deletePersistentCacheByPrefix requires a specific breaker: prefix');
}
}
export async function getPersistentCache<T>(key: string): Promise<CacheEnvelope<T> | null> {
if (isDesktopRuntime()) {
try {
@@ -200,6 +208,8 @@ export async function deletePersistentCache(key: string): Promise<void> {
}
export async function deletePersistentCacheByPrefix(prefix: string): Promise<void> {
validateBreakerPrefix(prefix);
if (isDesktopRuntime()) {
try {
await invokeTauri<void>('delete_cache_entries_by_prefix', { prefix });

View File

@@ -111,7 +111,7 @@ export function getApiBaseUrl(): string {
return '';
}
const configuredBaseUrl = ENV.VITE_TAURI_API_BASE_URL;
const configuredBaseUrl = ENV.VITE_TAURI_API_BASE_URL;
if (configuredBaseUrl) {
return normalizeBaseUrl(configuredBaseUrl);
}
@@ -151,7 +151,7 @@ export function getCanonicalApiOrigin(): string {
}
export function getRemoteApiBaseUrl(): string {
const configuredRemoteBase = ENV.VITE_TAURI_REMOTE_API_BASE_URL;
const configuredRemoteBase = ENV.VITE_TAURI_REMOTE_API_BASE_URL;
if (configuredRemoteBase) {
return normalizeBaseUrl(configuredRemoteBase);
}
@@ -215,7 +215,7 @@ const APP_HOSTS = new Set([
'api.worldmonitor.app',
'localhost',
'127.0.0.1',
...extractHostnames(WS_API_URL, ENV.VITE_WS_RELAY_URL),
...extractHostnames(WS_API_URL, ENV.VITE_WS_RELAY_URL),
]);
function isAppOriginUrl(urlStr: string): boolean {

View File

@@ -121,25 +121,8 @@ export function loadFromStorage<T>(key: string, defaultValue: T): T {
return defaultValue;
}
let _storageQuotaExceeded = false;
export function isStorageQuotaExceeded(): boolean {
return _storageQuotaExceeded;
}
export function isQuotaError(e: unknown): boolean {
return e instanceof DOMException && (e.name === 'QuotaExceededError' || e.code === 22);
}
export function markStorageQuotaExceeded(): void {
if (!_storageQuotaExceeded) {
_storageQuotaExceeded = true;
console.warn('[Storage] Quota exceeded — disabling further writes');
}
}
export function saveToStorage<T>(key: string, value: T): void {
if (_storageQuotaExceeded) return;
if (isStorageQuotaExceeded()) return;
try {
localStorage.setItem(key, JSON.stringify(value));
} catch (e) {
@@ -193,3 +176,5 @@ export type { Theme, ThemePreference } from './theme-manager';
export { toFlagEmoji } from './country-flag';
import { getCurrentLanguage } from '../services/i18n';
import { isStorageQuotaExceeded, isQuotaError, markStorageQuotaExceeded } from './storage-quota';
export { isStorageQuotaExceeded, isQuotaError, markStorageQuotaExceeded };

View File

@@ -0,0 +1,16 @@
let storageQuotaExceeded = false;
export function isStorageQuotaExceeded(): boolean {
return storageQuotaExceeded;
}
export function isQuotaError(error: unknown): boolean {
return error instanceof DOMException && (error.name === 'QuotaExceededError' || error.code === 22);
}
export function markStorageQuotaExceeded(): void {
if (!storageQuotaExceeded) {
storageQuotaExceeded = true;
console.warn('[Storage] Quota exceeded — disabling further writes');
}
}

View File

@@ -12,6 +12,40 @@ function freshImportUrl(url) {
return `${url}?t=${Date.now()}-${Math.random().toString(36).slice(2)}`;
}
function overrideGlobal(name, value) {
const original = Object.getOwnPropertyDescriptor(globalThis, name);
Object.defineProperty(globalThis, name, {
configurable: true,
writable: true,
value,
});
return () => {
if (original) Object.defineProperty(globalThis, name, original);
else delete globalThis[name];
};
}
function installBrowserEnv() {
const location = {
hostname: 'worldmonitor.app',
protocol: 'https:',
host: 'worldmonitor.app',
origin: 'https://worldmonitor.app',
};
const navigator = { userAgent: 'node-test', onLine: true };
const window = { location, navigator };
const restoreWindow = overrideGlobal('window', window);
const restoreLocation = overrideGlobal('location', location);
const restoreNavigator = overrideGlobal('navigator', navigator);
return () => {
restoreNavigator();
restoreLocation();
restoreWindow();
};
}
function getRequestUrl(input) {
if (typeof input === 'string') return new URL(input, 'http://localhost');
if (input instanceof URL) return new URL(input.toString());
@@ -40,6 +74,7 @@ function marketResponse(quotes) {
describe('market service symbol casing', () => {
it('preserves distinct-case symbols in the batched request and response mapping', async () => {
const restoreBrowserEnv = installBrowserEnv();
const { clearAllCircuitBreakers } = await import(freshImportUrl(CIRCUIT_BREAKER_URL));
clearAllCircuitBreakers();
@@ -77,10 +112,12 @@ describe('market service symbol casing', () => {
} finally {
globalThis.fetch = originalFetch;
clearAllCircuitBreakers();
restoreBrowserEnv();
}
});
it('keeps per-request cache keys isolated when symbols differ only by case', async () => {
const restoreBrowserEnv = installBrowserEnv();
const { clearAllCircuitBreakers } = await import(freshImportUrl(CIRCUIT_BREAKER_URL));
clearAllCircuitBreakers();
@@ -116,6 +153,38 @@ describe('market service symbol casing', () => {
} finally {
globalThis.fetch = originalFetch;
clearAllCircuitBreakers();
restoreBrowserEnv();
}
});
it('keeps requested metadata when the backend normalizes symbol casing', async () => {
const restoreBrowserEnv = installBrowserEnv();
const { clearAllCircuitBreakers } = await import(freshImportUrl(CIRCUIT_BREAKER_URL));
clearAllCircuitBreakers();
const originalFetch = globalThis.fetch;
globalThis.fetch = async () => new Response(JSON.stringify(marketResponse([
quote('Btc-Usd', 101),
])), {
status: 200,
headers: { 'Content-Type': 'application/json' },
});
try {
const { fetchMultipleStocks } = await import(freshImportUrl(MARKET_SERVICE_URL));
const result = await fetchMultipleStocks([
{ symbol: 'btc-usd', name: 'Lower BTC', display: 'btc lower' },
{ symbol: 'BTC-USD', name: 'Upper BTC', display: 'BTC upper' },
]);
assert.equal(result.data[0]?.symbol, 'Btc-Usd');
assert.equal(result.data[0]?.name, 'Lower BTC');
assert.equal(result.data[0]?.display, 'btc lower');
} finally {
globalThis.fetch = originalFetch;
clearAllCircuitBreakers();
restoreBrowserEnv();
}
});
});

View File

@@ -24,69 +24,164 @@ import assert from 'node:assert/strict';
import { readFileSync } from 'node:fs';
import { dirname, resolve } from 'node:path';
import { fileURLToPath, pathToFileURL } from 'node:url';
import * as ts from 'typescript'; // TypeScript compiler API — available via the typescript devDep used by tsc
const __dirname = dirname(fileURLToPath(import.meta.url));
const root = resolve(__dirname, '..');
const economicPath = resolve(root, 'src/services/economic/index.ts');
const readSrc = (relPath) => readFileSync(resolve(root, relPath), 'utf-8');
function loadEconomicSourceFile() {
return ts.createSourceFile(
economicPath,
readFileSync(economicPath, 'utf-8'),
ts.ScriptTarget.Latest,
true,
ts.ScriptKind.TS,
);
}
function walk(node, visit) {
visit(node);
ts.forEachChild(node, (child) => walk(child, visit));
}
function findVariableDeclaration(sourceFile, name) {
for (const stmt of sourceFile.statements) {
if (!ts.isVariableStatement(stmt)) continue;
for (const decl of stmt.declarationList.declarations) {
if (ts.isIdentifier(decl.name) && decl.name.text === name) {
return decl;
}
}
}
return undefined;
}
function findFunctionDeclaration(sourceFile, name) {
return sourceFile.statements.find(
(stmt) => ts.isFunctionDeclaration(stmt) && stmt.name?.text === name,
);
}
function collectCallExpressions(node) {
const calls = [];
walk(node, (current) => {
if (ts.isCallExpression(current)) calls.push(current);
});
return calls;
}
function findPropertyAssignment(node, name) {
if (!ts.isObjectLiteralExpression(node)) return undefined;
return node.properties.find(
(prop) => ts.isPropertyAssignment(prop)
&& ((ts.isIdentifier(prop.name) && prop.name.text === name)
|| (ts.isStringLiteral(prop.name) && prop.name.text === name)),
);
}
function isIdentifierNamed(node, name) {
return ts.isIdentifier(node) && node.text === name;
}
function isStringLiteralValue(node, value) {
return (ts.isStringLiteral(node) || ts.isNoSubstitutionTemplateLiteral(node)) && node.text === value;
}
function getTechIndicatorKeys(sourceFile) {
const decl = findVariableDeclaration(sourceFile, 'TECH_INDICATORS');
assert.ok(decl?.initializer && ts.isObjectLiteralExpression(decl.initializer), 'TECH_INDICATORS object must exist');
const keys = new Set();
for (const prop of decl.initializer.properties) {
if (!ts.isPropertyAssignment(prop)) continue;
if (ts.isStringLiteral(prop.name) || ts.isIdentifier(prop.name)) {
keys.add(prop.name.text);
}
}
return keys;
}
function getCreateCircuitBreakerNameInitializer(fn) {
const createCall = collectCallExpressions(fn).find((call) => isIdentifierNamed(call.expression, 'createCircuitBreaker'));
assert.ok(createCall, 'getWbBreaker must call createCircuitBreaker');
const optionsArg = createCall.arguments[0];
assert.ok(optionsArg && ts.isObjectLiteralExpression(optionsArg), 'createCircuitBreaker must receive an options object');
const nameProp = findPropertyAssignment(optionsArg, 'name');
assert.ok(nameProp, 'createCircuitBreaker options must include a name');
return nameProp.initializer;
}
// ============================================================
// 1. Static analysis: source structure guarantees
// ============================================================
describe('economic/index.ts — per-indicator World Bank circuit breakers', () => {
const src = readSrc('src/services/economic/index.ts');
const sourceFile = loadEconomicSourceFile();
it('does NOT have a single shared wbBreaker', () => {
// The old bug: `const wbBreaker = createCircuitBreaker<...>({ name: 'World Bank', ... })`
assert.doesNotMatch(
src,
/\bconst\s+wbBreaker\s*=/,
assert.equal(
findVariableDeclaration(sourceFile, 'wbBreaker'),
undefined,
'Single shared wbBreaker must not exist — use getWbBreaker(indicatorCode) instead',
);
});
it('has a wbBreakers Map for per-indicator instances', () => {
assert.match(
src,
/\bwbBreakers\s*=\s*new\s+Map/,
'wbBreakers Map must exist to store per-indicator circuit breakers',
);
const decl = findVariableDeclaration(sourceFile, 'wbBreakers');
assert.ok(decl?.initializer && ts.isNewExpression(decl.initializer), 'wbBreakers declaration must exist');
assert.ok(isIdentifierNamed(decl.initializer.expression, 'Map'), 'wbBreakers must be initialized with new Map(...)');
});
it('has a getWbBreaker(indicatorCode) factory function', () => {
assert.match(
src,
/function\s+getWbBreaker\s*\(\s*indicatorCode/,
'getWbBreaker(indicatorCode) factory function must exist',
const fn = findFunctionDeclaration(sourceFile, 'getWbBreaker');
assert.ok(fn, 'getWbBreaker function must exist');
assert.equal(fn.parameters[0]?.name.getText(sourceFile), 'indicatorCode');
assert.ok(
collectCallExpressions(fn).some((call) => isIdentifierNamed(call.expression, 'createCircuitBreaker')),
'getWbBreaker must create circuit breakers lazily',
);
});
it('getIndicatorData calls getWbBreaker(indicator) not a shared breaker', () => {
assert.match(
src,
/getWbBreaker\s*\(\s*indicator\s*\)\s*\.execute/,
it('getIndicatorData calls getWbBreaker(indicator).execute, not a shared breaker', () => {
const fn = findFunctionDeclaration(sourceFile, 'getIndicatorData');
assert.ok(fn?.body, 'getIndicatorData must exist');
const executeCall = collectCallExpressions(fn.body).find((call) => {
if (!ts.isPropertyAccessExpression(call.expression) || call.expression.name.text !== 'execute') return false;
const receiver = call.expression.expression;
return ts.isCallExpression(receiver)
&& isIdentifierNamed(receiver.expression, 'getWbBreaker')
&& isIdentifierNamed(receiver.arguments[0], 'indicator');
});
assert.ok(
executeCall,
'getIndicatorData must use getWbBreaker(indicator).execute, not a shared wbBreaker',
);
});
it('per-indicator breaker names include the indicator code', () => {
// name: `WB:${indicatorCode}` — ensures distinct IndexedDB keys per indicator
assert.match(
src,
/name\s*:\s*`WB:\$\{indicatorCode\}`/,
'Breaker name must embed indicatorCode (e.g. "WB:IT.NET.USER.ZS") for unique IndexedDB persistence',
const fn = findFunctionDeclaration(sourceFile, 'getWbBreaker');
assert.ok(fn, 'getWbBreaker function must exist');
const nameInitializer = getCreateCircuitBreakerNameInitializer(fn);
assert.ok(
ts.isTemplateExpression(nameInitializer),
'Breaker name should be a template string scoped to the indicator code',
);
assert.equal(nameInitializer.head.text, 'WB:');
assert.equal(nameInitializer.templateSpans.length, 1);
assert.ok(isIdentifierNamed(nameInitializer.templateSpans[0]?.expression, 'indicatorCode'));
});
it('mirrors fredBatchBreaker pattern (consistency check)', () => {
// fredBatchBreaker uses the same circuit breaker pattern
assert.match(src, /fredBatchBreaker\s*=/, 'fredBatchBreaker must exist as reference');
assert.match(src, /getWbBreaker\s*\(/, 'getWbBreaker implementation should be present');
// Both should use circuit breakers
assert.match(src, /fredBatchBreaker\s*=\s*createCircuitBreaker/, 'fredBatchBreaker uses createCircuitBreaker');
assert.match(src, /wbBreakers\s*=\s*new\s+Map/, 'wbBreakers uses Map for per-indicator breakers');
const fredDecl = findVariableDeclaration(sourceFile, 'fredBatchBreaker');
assert.ok(fredDecl?.initializer && ts.isCallExpression(fredDecl.initializer), 'fredBatchBreaker must exist');
assert.ok(isIdentifierNamed(fredDecl.initializer.expression, 'createCircuitBreaker'));
assert.ok(findFunctionDeclaration(sourceFile, 'getWbBreaker'), 'getWbBreaker implementation should be present');
});
});
@@ -194,25 +289,37 @@ describe('CircuitBreaker isolation — independent per-indicator instances', ()
// ============================================================
describe('getTechReadinessRankings — bootstrap-only data flow', () => {
const src = readSrc('src/services/economic/index.ts');
const sourceFile = loadEconomicSourceFile();
const fn = findFunctionDeclaration(sourceFile, 'getTechReadinessRankings');
it('reads from bootstrap hydration or endpoint, never calls WB API directly', () => {
const fnStart = src.indexOf('export async function getTechReadinessRankings');
const fnEnd = src.indexOf('\nexport ', fnStart + 1);
const fnBody = src.slice(fnStart, fnEnd !== -1 ? fnEnd : fnStart + 3000);
assert.ok(fn?.body, 'getTechReadinessRankings must exist');
const calls = collectCallExpressions(fn.body);
assert.match(fnBody, /getHydratedData\s*\(\s*'techReadiness'\s*\)/,
'Must try bootstrap hydration cache first');
assert.match(fnBody, /\/api\/bootstrap\?keys=techReadiness/,
'Must fallback to bootstrap endpoint');
assert.doesNotMatch(fnBody, /getIndicatorData\s*\(/,
'Must NOT call getIndicatorData (WB API) from frontend');
const hydratedCall = calls.find((call) =>
isIdentifierNamed(call.expression, 'getHydratedData')
&& isStringLiteralValue(call.arguments[0], 'techReadiness'),
);
assert.ok(hydratedCall, 'Must try bootstrap hydration cache first');
const bootstrapFetch = calls.find((call) => {
if (!isIdentifierNamed(call.expression, 'fetch')) return false;
const firstArg = call.arguments[0];
return ts.isCallExpression(firstArg)
&& isIdentifierNamed(firstArg.expression, 'toApiUrl')
&& isStringLiteralValue(firstArg.arguments[0], '/api/bootstrap?keys=techReadiness');
});
assert.ok(bootstrapFetch, 'Must fallback to bootstrap endpoint');
const wbCalls = calls.filter((call) => isIdentifierNamed(call.expression, 'getIndicatorData'));
assert.equal(wbCalls.length, 0, 'Must NOT call getIndicatorData (WB API) from frontend');
});
it('indicator codes exist in TECH_INDICATORS for seed script parity', () => {
assert.match(src, /'IT\.NET\.USER\.ZS'/, 'Internet Users indicator must be present');
assert.match(src, /'IT\.CEL\.SETS\.P2'/, 'Mobile Subscriptions indicator must be present');
assert.match(src, /'IT\.NET\.BBND\.P2'/, 'Fixed Broadband indicator must be present');
assert.match(src, /'GB\.XPD\.RSDV\.GD\.ZS'/, 'R&D Expenditure indicator must be present');
const keys = getTechIndicatorKeys(sourceFile);
assert.ok(keys.has('IT.NET.USER.ZS'), 'Internet Users indicator must be present');
assert.ok(keys.has('IT.CEL.SETS.P2'), 'Mobile Subscriptions indicator must be present');
assert.ok(keys.has('IT.NET.BBND.P2'), 'Fixed Broadband indicator must be present');
assert.ok(keys.has('GB.XPD.RSDV.GD.ZS'), 'R&D Expenditure indicator must be present');
});
});