Skip to content

Commit 47aff5b

Browse files
committed
Refactored to return undefined instead of null on cache miss
This will be more straightforward and enable e.g. setting value to null. It's also algined to how other caching tool apis looks like
1 parent 21d7ebc commit 47aff5b

12 files changed

Lines changed: 94 additions & 73 deletions

src/packages/pongo/src/core/cache/cacheWrapper.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import type { CacheHooks, PongoCache } from './types';
1+
import type { PongoDocument } from '../typing';
2+
import type { CacheHooks, PongoCache, PongoDocumentCacheKey } from './types';
23

34
export const pongoCacheWrapper = (options: {
45
provider: PongoCache;
@@ -14,10 +15,12 @@ export const pongoCacheWrapper = (options: {
1415

1516
return {
1617
cacheType: provider.cacheType,
17-
async get(key) {
18+
async get<Doc extends PongoDocument = PongoDocument>(
19+
key: PongoDocumentCacheKey,
20+
) {
1821
try {
19-
const result = await provider.get(key);
20-
if (result != null) {
22+
const result = await provider.get<Doc>(key);
23+
if (result !== undefined) {
2124
hooks?.onHit?.(key);
2225
} else {
2326
hooks?.onMiss?.(key);
@@ -26,7 +29,7 @@ export const pongoCacheWrapper = (options: {
2629
} catch (error) {
2730
onError(error, 'get');
2831
hooks?.onMiss?.(key);
29-
return null;
32+
return undefined;
3033
}
3134
},
3235

src/packages/pongo/src/core/cache/cacheWrapper.unit.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ describe('pongoCacheWrapper — error swallowing', () => {
7676
throw new Error('boom');
7777
});
7878
const wrapper = pongoCacheWrapper({ provider });
79-
expect(await wrapper.get(`${dbName}:${collectionName}:x`)).toBeNull();
79+
expect(await wrapper.get(`${dbName}:${collectionName}:x`)).toBeUndefined();
8080
});
8181

8282
it('set silently succeeds when provider throws', async () => {
@@ -150,7 +150,7 @@ describe('pongoCacheWrapper — event hooks', () => {
150150
expect(onHit).toHaveBeenCalledWith(`${dbName}:${collectionName}:x`);
151151
});
152152

153-
it('onMiss called when get returns null/undefined', async () => {
153+
it('onMiss called when get returns undefined', async () => {
154154
const provider = identityMapCache();
155155
const onMiss = vi.fn();
156156
const wrapper = pongoCacheWrapper({ provider, hooks: { onMiss } });

src/packages/pongo/src/core/cache/pongoCache.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import type { CacheConfig, CacheSettings, PongoCache } from './types';
55
const DEFAULT_CONFIG: CacheSettings = { type: 'in-memory' };
66

77
export const pongoCache = (
8-
options: CacheConfig | 'disabled' | PongoCache | undefined,
8+
options?: CacheConfig | 'disabled' | PongoCache | undefined,
99
): PongoCache => {
1010
if (options === undefined || options === 'disabled') return noopCacheProvider;
1111

src/packages/pongo/src/core/cache/pongoCache.unit.spec.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,11 @@ describe('pongoCache factory', () => {
3636
});
3737

3838
it('defaults to disabled when no config provided', async () => {
39-
const cache = pongoCache(undefined);
39+
const cache = pongoCache();
4040

4141
await cache.set('db:col:1', { _id: '1', name: 'Bob' });
4242
const result = await cache.get('db:col:1');
43-
expect(result).toBeNull();
43+
expect(result).toBeUndefined();
4444
});
4545

4646
it('in-memory LRU evicts when over max', async () => {
@@ -52,7 +52,7 @@ describe('pongoCache factory', () => {
5252

5353
// Oldest entry should be evicted
5454
const first = await cache.get('db:col:0');
55-
expect(first).toBeNull();
55+
expect(first).toBeUndefined();
5656
});
5757
});
5858

@@ -62,7 +62,7 @@ describe('pongoCache factory', () => {
6262

6363
await cache.set('db:col:1', { _id: '1', name: 'Carol' });
6464
const result = await cache.get('db:col:1');
65-
expect(result).toBeNull();
65+
expect(result).toBeUndefined();
6666
});
6767
});
6868

src/packages/pongo/src/core/cache/providers/identityMapCache.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
import type { PongoDocument } from '../../typing';
2-
import type { PongoCache } from '../types';
2+
import type { PongoCache, PongoDocumentCacheKey } from '../types';
33

44
export const identityMapCache = (): PongoCache => {
55
const store = new Map<string, PongoDocument>();
66

77
return {
88
cacheType: 'pongo:cache:identity-map',
9-
get: (key) => Promise.resolve(store.get(key) ?? null),
9+
get: <Doc extends PongoDocument = PongoDocument>(
10+
key: PongoDocumentCacheKey,
11+
): Promise<Doc | undefined> =>
12+
Promise.resolve(store.get(key) as Doc | undefined),
1013
set: (key, value) => {
1114
store.set(key, value);
1215
},
@@ -21,8 +24,9 @@ export const identityMapCache = (): PongoCache => {
2124
delete: (key) => {
2225
store.delete(key);
2326
},
24-
getMany: (keys) =>
25-
keys.map((k) => store.get(k)).filter((v) => v !== undefined),
27+
getMany: <Doc extends PongoDocument = PongoDocument>(
28+
keys: PongoDocumentCacheKey[],
29+
): (Doc | undefined)[] => keys.map((k) => store.get(k) as Doc | undefined),
2630
setMany: (entries) => {
2731
for (const { key, value } of entries) store.set(key, value);
2832
},

src/packages/pongo/src/core/cache/providers/lruCache.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { LRUCache } from 'lru-cache';
2-
import type { PongoDocument } from '../../typing';
3-
import type { PongoCache } from '../types';
2+
import type { MaybePromise, PongoDocument } from '../../typing';
3+
import type { PongoCache, PongoDocumentCacheKey } from '../types';
44

55
export type LRUCacheOptions = Omit<
66
LRUCache.Options<string, PongoDocument, unknown>,
@@ -20,7 +20,10 @@ export const lruCache = (options?: LRUCacheOptions): PongoCache => {
2020

2121
return {
2222
cacheType: 'pongo:cache:lru',
23-
get: (key) => cache.get(key) ?? null,
23+
get: <Doc extends PongoDocument = PongoDocument>(
24+
key: PongoDocumentCacheKey,
25+
): MaybePromise<Doc | undefined> =>
26+
cache.get(key) as MaybePromise<Doc | undefined>,
2427
set: (key, value, opts) => {
2528
cache.set(
2629
key,
@@ -40,8 +43,9 @@ export const lruCache = (options?: LRUCacheOptions): PongoCache => {
4043
// cache.set(key, updated, opts?.ttl !== undefined ? { ttl: opts.ttl } : undefined);
4144
// }
4245
},
43-
getMany: (keys) =>
44-
keys.map((k) => cache.get(k)).filter((v) => v !== undefined),
46+
getMany: <Doc extends PongoDocument = PongoDocument>(
47+
keys: PongoDocumentCacheKey[],
48+
): (Doc | undefined)[] => keys.map((k) => cache.get(k) as Doc | undefined),
4549
setMany: (entries) => {
4650
for (const { key, value, ttl: entryTtl } of entries) {
4751
cache.set(

src/packages/pongo/src/core/cache/providers/lruCache.unit.spec.ts

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ import { describe, expect, it } from 'vitest';
22
import { lruCache } from './lruCache';
33

44
describe('inMemoryCacheProvider', () => {
5-
it('returns null for missing keys', () => {
5+
it('returns undefined for missing keys', () => {
66
const cache = lruCache();
7-
expect(cache.get('db:collection:missing')).toBeNull();
7+
expect(cache.get('db:collection:missing')).toBeUndefined();
88
});
99

1010
it('set then get returns the stored document', () => {
@@ -19,28 +19,29 @@ describe('inMemoryCacheProvider', () => {
1919
cache.set('db:collection:x', { _id: 'x' });
2020
expect(cache.get('db:collection:x')).toBeDefined();
2121
await new Promise((r) => setTimeout(r, 100));
22-
expect(cache.get('db:collection:x')).toBeNull();
22+
expect(cache.get('db:collection:x')).toBeUndefined();
2323
});
2424

2525
it('delete removes a cached entry', () => {
2626
const cache = lruCache();
2727
cache.set('db:collection:a', { _id: 'a' });
2828
cache.delete('db:collection:a');
29-
expect(cache.get('db:collection:a')).toBeNull();
29+
expect(cache.get('db:collection:a')).toBeUndefined();
3030
});
3131

32-
it('getMany returns documents for found keys and nothing for missing', () => {
32+
it('getMany returns documents for found keys and undefined for missing', () => {
3333
const cache = lruCache();
3434
cache.set('db:collection:k1', { _id: 'k1' });
3535
cache.set('db:collection:k2', { _id: 'k2' });
3636
const results = cache.getMany([
3737
'db:collection:k1',
3838
'db:collection:missing',
3939
'db:collection:k2',
40-
]) as (Record<string, unknown> | null | undefined)[];
41-
expect(results).toHaveLength(2); // Only returns found entries
40+
]) as (Record<string, unknown> | undefined)[];
41+
expect(results).toHaveLength(3);
4242
expect(results[0]).toEqual({ _id: 'k1' });
43-
expect(results[1]).toEqual({ _id: 'k2' });
43+
expect(results[1]).toBeUndefined();
44+
expect(results[2]).toEqual({ _id: 'k2' });
4445
});
4546

4647
it('setMany stores multiple documents retrievable via get', () => {
@@ -61,8 +62,8 @@ describe('inMemoryCacheProvider', () => {
6162
{ key: 'db:collection:c', value: { _id: 'c' } },
6263
]);
6364
cache.deleteMany(['db:collection:a', 'db:collection:b']);
64-
expect(cache.get('db:collection:a')).toBeNull();
65-
expect(cache.get('db:collection:b')).toBeNull();
65+
expect(cache.get('db:collection:a')).toBeUndefined();
66+
expect(cache.get('db:collection:b')).toBeUndefined();
6667
expect(cache.get('db:collection:c')).toEqual({ _id: 'c' });
6768
});
6869

@@ -73,8 +74,8 @@ describe('inMemoryCacheProvider', () => {
7374
{ key: 'db:collection:b', value: { _id: 'b' } },
7475
]);
7576
cache.clear();
76-
expect(cache.get('db:collection:a')).toBeNull();
77-
expect(cache.get('db:collection:b')).toBeNull();
77+
expect(cache.get('db:collection:a')).toBeUndefined();
78+
expect(cache.get('db:collection:b')).toBeUndefined();
7879
});
7980

8081
it('respects max — LRU entry is evicted when full', () => {
@@ -85,7 +86,7 @@ describe('inMemoryCacheProvider', () => {
8586
cache.set('db:collection:c', { _id: 'c' });
8687
expect(cache.get('db:collection:a')).toBeDefined();
8788
expect(cache.get('db:collection:c')).toBeDefined();
88-
expect(cache.get('db:collection:b')).toBeNull();
89+
expect(cache.get('db:collection:b')).toBeUndefined();
8990
});
9091

9192
it('returns values synchronously (not wrapped in Promises)', () => {

src/packages/pongo/src/core/cache/providers/noopCache.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { PongoCache } from '../types';
22

33
export const noopCacheProvider: PongoCache = {
44
cacheType: 'pongo:cache:no-op',
5-
get: async () => await Promise.resolve(null),
5+
get: () => undefined,
66
set: () => {},
77
update: () => {},
88
delete: () => {},

0 commit comments

Comments
 (0)