Skip to content

Commit e5d620e

Browse files
committed
fix(config-cache): has() no longer inflates hit/miss stats
has() delegated to get(), which increments this.hits or this.misses. The common has()+get() pattern double-counted every lookup. Re-implement has() with its own TTL check so cache statistics stay accurate. 35 unit tests added including 3 regression tests for this fix. Closes #497
1 parent fcfb757 commit e5d620e

2 files changed

Lines changed: 323 additions & 1 deletion

File tree

.aiox-core/core/config/config-cache.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,17 @@ class ConfigCache {
7272
* @returns {boolean} True if key exists and is valid
7373
*/
7474
has(key) {
75-
return this.get(key) !== null;
75+
// Check directly instead of delegating to get(), which would
76+
// increment hits/misses and inflate cache statistics (fixes #497)
77+
if (!this.cache.has(key)) return false;
78+
79+
const timestamp = this.timestamps.get(key);
80+
if (Date.now() - timestamp > this.ttl) {
81+
this.cache.delete(key);
82+
this.timestamps.delete(key);
83+
return false;
84+
}
85+
return true;
7686
}
7787

7888
/**
Lines changed: 312 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,312 @@
1+
'use strict';
2+
3+
/**
4+
* Unit tests for config-cache
5+
*
6+
* Tests ConfigCache class: TTL expiration, get/set/has, invalidation,
7+
* clearExpired, statistics, entries, serialization, and global singleton.
8+
*
9+
* Fixes #497 — has() was inflating hit/miss stats by calling get()
10+
*/
11+
12+
jest.useFakeTimers();
13+
14+
const { ConfigCache, globalConfigCache } = require('../../../.aios-core/core/config/config-cache');
15+
16+
describe('config-cache', () => {
17+
let cache;
18+
19+
beforeEach(() => {
20+
cache = new ConfigCache(1000); // 1 second TTL for tests
21+
});
22+
23+
// ── Constructor ─────────────────────────────────────────────────
24+
25+
describe('constructor', () => {
26+
test('uses default TTL of 5 minutes', () => {
27+
const defaultCache = new ConfigCache();
28+
expect(defaultCache.ttl).toBe(5 * 60 * 1000);
29+
});
30+
31+
test('accepts custom TTL', () => {
32+
expect(cache.ttl).toBe(1000);
33+
});
34+
35+
test('initializes empty stats', () => {
36+
expect(cache.hits).toBe(0);
37+
expect(cache.misses).toBe(0);
38+
expect(cache.size).toBe(0);
39+
});
40+
});
41+
42+
// ── set / get ───────────────────────────────────────────────────
43+
44+
describe('set and get', () => {
45+
test('stores and retrieves a value', () => {
46+
cache.set('key1', 'value1');
47+
expect(cache.get('key1')).toBe('value1');
48+
});
49+
50+
test('stores objects', () => {
51+
const obj = { nested: { data: true } };
52+
cache.set('obj', obj);
53+
expect(cache.get('obj')).toBe(obj);
54+
});
55+
56+
test('returns null for missing key', () => {
57+
expect(cache.get('missing')).toBeNull();
58+
});
59+
60+
test('returns null for expired key', () => {
61+
cache.set('key1', 'value1');
62+
jest.advanceTimersByTime(1500); // past 1s TTL
63+
expect(cache.get('key1')).toBeNull();
64+
});
65+
66+
test('removes expired entry from cache on get', () => {
67+
cache.set('key1', 'value1');
68+
jest.advanceTimersByTime(1500);
69+
cache.get('key1');
70+
expect(cache.size).toBe(0);
71+
});
72+
73+
test('overwrites existing key', () => {
74+
cache.set('key1', 'v1');
75+
cache.set('key1', 'v2');
76+
expect(cache.get('key1')).toBe('v2');
77+
});
78+
});
79+
80+
// ── has ─────────────────────────────────────────────────────────
81+
82+
describe('has', () => {
83+
test('returns true for valid entry', () => {
84+
cache.set('key1', 'value1');
85+
expect(cache.has('key1')).toBe(true);
86+
});
87+
88+
test('returns false for missing entry', () => {
89+
expect(cache.has('missing')).toBe(false);
90+
});
91+
92+
test('returns false for expired entry', () => {
93+
cache.set('key1', 'value1');
94+
jest.advanceTimersByTime(1500);
95+
expect(cache.has('key1')).toBe(false);
96+
});
97+
98+
test('cleans up expired entry on check', () => {
99+
cache.set('key1', 'value1');
100+
jest.advanceTimersByTime(1500);
101+
cache.has('key1');
102+
expect(cache.size).toBe(0);
103+
});
104+
105+
// ── BUG #497 regression — has() must NOT affect stats ─────
106+
107+
test('does not increment hits counter (fix #497)', () => {
108+
cache.set('key1', 'value1');
109+
cache.has('key1'); // should NOT touch stats
110+
expect(cache.hits).toBe(0);
111+
expect(cache.misses).toBe(0);
112+
});
113+
114+
test('does not increment misses counter (fix #497)', () => {
115+
cache.has('missing'); // should NOT touch stats
116+
expect(cache.hits).toBe(0);
117+
expect(cache.misses).toBe(0);
118+
});
119+
120+
test('has() + get() counts exactly one hit, not two (fix #497)', () => {
121+
cache.set('key1', 'value1');
122+
cache.has('key1'); // no stat change
123+
const val = cache.get('key1'); // exactly 1 hit
124+
expect(val).toBe('value1');
125+
expect(cache.hits).toBe(1);
126+
expect(cache.misses).toBe(0);
127+
});
128+
});
129+
130+
// ── invalidate ─────────────────────────────────────────────────
131+
132+
describe('invalidate', () => {
133+
test('removes specific entry', () => {
134+
cache.set('key1', 'v1');
135+
cache.set('key2', 'v2');
136+
const deleted = cache.invalidate('key1');
137+
expect(deleted).toBe(true);
138+
expect(cache.get('key1')).toBeNull();
139+
expect(cache.get('key2')).toBe('v2');
140+
});
141+
142+
test('returns false for non-existent key', () => {
143+
expect(cache.invalidate('missing')).toBe(false);
144+
});
145+
});
146+
147+
// ── clear ──────────────────────────────────────────────────────
148+
149+
describe('clear', () => {
150+
test('removes all entries and resets stats', () => {
151+
cache.set('a', 1);
152+
cache.set('b', 2);
153+
cache.get('a'); // hit
154+
cache.get('missing'); // miss
155+
cache.clear();
156+
expect(cache.size).toBe(0);
157+
expect(cache.hits).toBe(0);
158+
expect(cache.misses).toBe(0);
159+
});
160+
});
161+
162+
// ── clearExpired ───────────────────────────────────────────────
163+
164+
describe('clearExpired', () => {
165+
test('removes only expired entries', () => {
166+
cache.set('old', 'data');
167+
jest.advanceTimersByTime(800);
168+
cache.set('new', 'data');
169+
jest.advanceTimersByTime(300); // old is 1100ms, new is 300ms
170+
const cleared = cache.clearExpired();
171+
expect(cleared).toBe(1);
172+
expect(cache.get('new')).toBe('data');
173+
});
174+
175+
test('returns 0 when nothing expired', () => {
176+
cache.set('fresh', 'data');
177+
expect(cache.clearExpired()).toBe(0);
178+
});
179+
180+
test('returns 0 on empty cache', () => {
181+
expect(cache.clearExpired()).toBe(0);
182+
});
183+
});
184+
185+
// ── size ───────────────────────────────────────────────────────
186+
187+
describe('size', () => {
188+
test('returns number of entries', () => {
189+
expect(cache.size).toBe(0);
190+
cache.set('a', 1);
191+
expect(cache.size).toBe(1);
192+
cache.set('b', 2);
193+
expect(cache.size).toBe(2);
194+
});
195+
});
196+
197+
// ── statistics ─────────────────────────────────────────────────
198+
199+
describe('statistics', () => {
200+
test('tracks hits and misses', () => {
201+
cache.set('key1', 'v1');
202+
cache.get('key1'); // hit
203+
cache.get('key1'); // hit
204+
cache.get('missing'); // miss
205+
expect(cache.hits).toBe(2);
206+
expect(cache.misses).toBe(1);
207+
});
208+
209+
test('getStats returns correct statistics', () => {
210+
cache.set('key1', 'v1');
211+
cache.get('key1'); // hit
212+
cache.get('missing'); // miss
213+
const stats = cache.getStats();
214+
expect(stats.size).toBe(1);
215+
expect(stats.hits).toBe(1);
216+
expect(stats.misses).toBe(1);
217+
expect(stats.total).toBe(2);
218+
expect(stats.hitRate).toBe('50.0%');
219+
expect(stats.ttl).toBe(1000);
220+
expect(stats.ttlMinutes).toBe('0.0');
221+
});
222+
223+
test('getStats returns 0.0% hit rate when no requests', () => {
224+
const stats = cache.getStats();
225+
expect(stats.hitRate).toBe('0.0%');
226+
});
227+
228+
test('resetStats clears counters but keeps cache', () => {
229+
cache.set('key1', 'v1');
230+
cache.get('key1');
231+
cache.resetStats();
232+
expect(cache.hits).toBe(0);
233+
expect(cache.misses).toBe(0);
234+
expect(cache.get('key1')).toBe('v1');
235+
});
236+
});
237+
238+
// ── keys ───────────────────────────────────────────────────────
239+
240+
describe('keys', () => {
241+
test('returns array of cache keys', () => {
242+
cache.set('a', 1);
243+
cache.set('b', 2);
244+
expect(cache.keys()).toEqual(['a', 'b']);
245+
});
246+
247+
test('returns empty array for empty cache', () => {
248+
expect(cache.keys()).toEqual([]);
249+
});
250+
});
251+
252+
// ── entries ────────────────────────────────────────────────────
253+
254+
describe('entries', () => {
255+
test('returns entries with age and expiry info', () => {
256+
cache.set('key1', 'value1');
257+
jest.advanceTimersByTime(200);
258+
const entries = cache.entries();
259+
expect(entries).toHaveLength(1);
260+
expect(entries[0].key).toBe('key1');
261+
expect(entries[0].value).toBe('value1');
262+
expect(entries[0].age).toBe(200);
263+
expect(entries[0].ageSeconds).toBe('0.2');
264+
expect(entries[0].expires).toBe(800);
265+
expect(entries[0].expiresSeconds).toBe('0.8');
266+
});
267+
});
268+
269+
// ── setTTL ─────────────────────────────────────────────────────
270+
271+
describe('setTTL', () => {
272+
test('updates TTL for future expiration checks', () => {
273+
cache.set('key1', 'v1');
274+
cache.setTTL(500);
275+
jest.advanceTimersByTime(600);
276+
expect(cache.get('key1')).toBeNull();
277+
});
278+
});
279+
280+
// ── toJSON ─────────────────────────────────────────────────────
281+
282+
describe('toJSON', () => {
283+
test('serializes cache state to JSON string', () => {
284+
cache.set('key1', 'v1');
285+
cache.get('key1'); // hit
286+
const json = cache.toJSON();
287+
const parsed = JSON.parse(json);
288+
expect(parsed.size).toBe(1);
289+
expect(parsed.stats).toBeDefined();
290+
expect(parsed.entries).toHaveLength(1);
291+
expect(parsed.entries[0].key).toBe('key1');
292+
});
293+
294+
test('produces valid JSON for empty cache', () => {
295+
const parsed = JSON.parse(cache.toJSON());
296+
expect(parsed.size).toBe(0);
297+
expect(parsed.entries).toEqual([]);
298+
});
299+
});
300+
301+
// ── globalConfigCache singleton ────────────────────────────────
302+
303+
describe('globalConfigCache', () => {
304+
test('is a ConfigCache instance', () => {
305+
expect(globalConfigCache).toBeInstanceOf(ConfigCache);
306+
});
307+
308+
test('has default 5 minute TTL', () => {
309+
expect(globalConfigCache.ttl).toBe(5 * 60 * 1000);
310+
});
311+
});
312+
});

0 commit comments

Comments
 (0)