Skip to content

Commit 6a80cab

Browse files
committed
test(graphile-cache): add regression test for double-disposal prevention
Adds unit tests that verify: - pgl.release() is called exactly once per entry during closeAllCaches() - closeAllCaches() does not throw when LRU dispose callback fires after manual disposal (the original bug scenario) - Empty cache is handled gracefully - Single LRU eviction disposes exactly once
1 parent ef30583 commit 6a80cab

1 file changed

Lines changed: 133 additions & 0 deletions

File tree

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
import { createServer } from 'http';
2+
import express from 'express';
3+
import { LRUCache } from 'lru-cache';
4+
5+
/**
6+
* Regression test for double-disposal of PostGraphile instances.
7+
*
8+
* The bug: closeAllCaches() manually disposed each entry via disposeEntry(),
9+
* which tracked disposal by key string in a Set and deleted the key in a
10+
* `finally` block. Then graphileCache.clear() triggered the LRU dispose
11+
* callback for the same entries — but the guard key was already gone, so
12+
* pgl.release() was called a second time on an already-released instance.
13+
*
14+
* The fix: track disposal by entry object reference (WeakSet) so the guard
15+
* persists across the manual-dispose → clear() sequence.
16+
*/
17+
18+
// ── Helpers ──────────────────────────────────────────────────────────
19+
20+
/** Minimal mock that records release() calls and throws on double-release */
21+
function createMockPgl() {
22+
let released = false;
23+
return {
24+
release: jest.fn(async () => {
25+
if (released) {
26+
throw new Error('PostGraphile instance has been released');
27+
}
28+
released = true;
29+
}),
30+
get isReleased() {
31+
return released;
32+
}
33+
};
34+
}
35+
36+
function createMockEntry(key: string) {
37+
const pgl = createMockPgl();
38+
const handler = express();
39+
const httpServer = createServer(handler);
40+
41+
return {
42+
entry: {
43+
pgl: pgl as any,
44+
serv: {} as any,
45+
handler,
46+
httpServer,
47+
cacheKey: key,
48+
createdAt: Date.now()
49+
},
50+
pgl
51+
};
52+
}
53+
54+
// ── Tests ────────────────────────────────────────────────────────────
55+
56+
// We import the real module so the WeakSet-based guard is exercised.
57+
// pg-cache is a workspace dependency that may not resolve in isolation,
58+
// so we mock it along with @pgpmjs/logger.
59+
jest.mock('pg-cache', () => ({
60+
pgCache: {
61+
registerCleanupCallback: jest.fn(() => jest.fn()),
62+
close: jest.fn(async () => {})
63+
}
64+
}));
65+
66+
jest.mock('@pgpmjs/logger', () => ({
67+
Logger: class {
68+
info = jest.fn();
69+
warn = jest.fn();
70+
error = jest.fn();
71+
debug = jest.fn();
72+
success = jest.fn();
73+
}
74+
}));
75+
76+
import {
77+
graphileCache,
78+
closeAllCaches,
79+
type GraphileCacheEntry
80+
} from '../src/graphile-cache';
81+
82+
describe('graphile-cache', () => {
83+
afterEach(async () => {
84+
// Ensure the cache is clean between tests.
85+
// closeAllCaches resets the singleton promise, so we can call it again.
86+
graphileCache.clear();
87+
});
88+
89+
describe('closeAllCaches – double-disposal regression', () => {
90+
it('should call pgl.release() exactly once per entry', async () => {
91+
const { entry: entry1, pgl: pgl1 } = createMockEntry('api-one');
92+
const { entry: entry2, pgl: pgl2 } = createMockEntry('api-two');
93+
94+
graphileCache.set('api-one', entry1 as GraphileCacheEntry);
95+
graphileCache.set('api-two', entry2 as GraphileCacheEntry);
96+
97+
await closeAllCaches();
98+
99+
// Each release should be called exactly once — not twice.
100+
expect(pgl1.release).toHaveBeenCalledTimes(1);
101+
expect(pgl2.release).toHaveBeenCalledTimes(1);
102+
});
103+
104+
it('should not throw when closeAllCaches triggers LRU dispose after manual disposal', async () => {
105+
const { entry, pgl } = createMockEntry('api-disposable');
106+
graphileCache.set('api-disposable', entry as GraphileCacheEntry);
107+
108+
// This should complete without the mock throwing
109+
// "PostGraphile instance has been released"
110+
await expect(closeAllCaches()).resolves.toBeUndefined();
111+
expect(pgl.release).toHaveBeenCalledTimes(1);
112+
});
113+
114+
it('should handle empty cache gracefully', async () => {
115+
await expect(closeAllCaches()).resolves.toBeUndefined();
116+
});
117+
});
118+
119+
describe('LRU eviction – single disposal', () => {
120+
it('should dispose entry once on LRU eviction', async () => {
121+
const { entry, pgl } = createMockEntry('api-evict');
122+
graphileCache.set('api-evict', entry as GraphileCacheEntry);
123+
124+
// Delete triggers the LRU dispose callback
125+
graphileCache.delete('api-evict');
126+
127+
// Give the async fire-and-forget disposal time to complete
128+
await new Promise((r) => setTimeout(r, 50));
129+
130+
expect(pgl.release).toHaveBeenCalledTimes(1);
131+
});
132+
});
133+
});

0 commit comments

Comments
 (0)