Skip to content

Commit 58e345e

Browse files
elirangoshenclaude
andcommitted
test: cover mergeCollectionWithPatches multiGet pre-warm fast/slow paths
Adds 5 tests to the mergeCollection pre-warm describe block: - Fast path: warm cache skips Storage.multiGet/getItem entirely. - Slow path: cold existing keys batch into one Storage.multiGet with no individual getItem calls. - Slow path correctness: cold-cache merge layers delta on top of persisted storage value (no field drops). - Onyx.update timing: warm cache broadcasts a single merged value (no transient undefined) — guards the promise-chain depth invariant. - Equivalence: warm and cold paths converge on identical cache state. Helper notes: - OnyxCache.drop removes the key from storageKeys; re-register via addKey so getAllKeys still sees the key as persisted when other keys remain in cache (the slow-path scenario). - Capture pristine StorageMock methods at file load and restore in beforeEach because the retryOperation describe block mutates setItem and never restores it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 3295637 commit 58e345e

1 file changed

Lines changed: 220 additions & 13 deletions

File tree

tests/unit/onyxUtilsTest.ts

Lines changed: 220 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,10 @@ describe('OnyxUtils', () => {
158158
[routeA]: {name: 'Route A'},
159159
} as GenericCollection);
160160

161-
await OnyxUtils.partialSetCollection({collectionKey: ONYXKEYS.COLLECTION.ROUTES, collection: {} as GenericCollection});
161+
await OnyxUtils.partialSetCollection({
162+
collectionKey: ONYXKEYS.COLLECTION.ROUTES,
163+
collection: {} as GenericCollection,
164+
});
162165

163166
expect(result).toEqual({
164167
[routeA]: {name: 'Route A'},
@@ -230,9 +233,18 @@ describe('OnyxUtils', () => {
230233
const spy2 = jest.fn();
231234
const spy3 = jest.fn();
232235

233-
const conn1 = Onyx.connect({key: `${ONYXKEYS.COLLECTION.TEST_KEY}1`, callback: spy1});
234-
const conn2 = Onyx.connect({key: `${ONYXKEYS.COLLECTION.TEST_KEY}2`, callback: spy2});
235-
const conn3 = Onyx.connect({key: `${ONYXKEYS.COLLECTION.TEST_KEY}3`, callback: spy3});
236+
const conn1 = Onyx.connect({
237+
key: `${ONYXKEYS.COLLECTION.TEST_KEY}1`,
238+
callback: spy1,
239+
});
240+
const conn2 = Onyx.connect({
241+
key: `${ONYXKEYS.COLLECTION.TEST_KEY}2`,
242+
callback: spy2,
243+
});
244+
const conn3 = Onyx.connect({
245+
key: `${ONYXKEYS.COLLECTION.TEST_KEY}3`,
246+
callback: spy3,
247+
});
236248
await waitForPromisesToResolve();
237249
spy1.mockClear();
238250
spy2.mockClear();
@@ -335,8 +347,14 @@ describe('OnyxUtils', () => {
335347

336348
const spy1 = jest.fn();
337349
const spy2 = jest.fn();
338-
const conn1 = Onyx.connect({key: `${ONYXKEYS.COLLECTION.TEST_KEY}1`, callback: spy1});
339-
const conn2 = Onyx.connect({key: `${ONYXKEYS.COLLECTION.TEST_KEY}2`, callback: spy2});
350+
const conn1 = Onyx.connect({
351+
key: `${ONYXKEYS.COLLECTION.TEST_KEY}1`,
352+
callback: spy1,
353+
});
354+
const conn2 = Onyx.connect({
355+
key: `${ONYXKEYS.COLLECTION.TEST_KEY}2`,
356+
callback: spy2,
357+
});
340358
await waitForPromisesToResolve();
341359
spy1.mockClear();
342360
spy2.mockClear();
@@ -389,7 +407,10 @@ describe('OnyxUtils', () => {
389407
// A subscriber for keyA synchronously calls Onyx.set() on keyB during its callback.
390408
// After multiSet completes, the cache must reflect the multiSet's value for keyB
391409
// (multiSet wins), and the keyB subscriber's last seen value must equal the cache.
392-
await Onyx.multiSet({[ONYXKEYS.TEST_KEY]: 'initialA', [ONYXKEYS.TEST_KEY_2]: 'initialB'});
410+
await Onyx.multiSet({
411+
[ONYXKEYS.TEST_KEY]: 'initialA',
412+
[ONYXKEYS.TEST_KEY_2]: 'initialB',
413+
});
393414

394415
const callbackA = jest.fn((value: unknown) => {
395416
if (value !== 'newA') {
@@ -613,8 +634,16 @@ describe('OnyxUtils', () => {
613634
const failingCallback = jest.fn();
614635
const workingCallback = jest.fn();
615636

616-
const connFailing = Onyx.connect({key: entryKey, callback: failingCallback, reuseConnection: false});
617-
const connWorking = Onyx.connect({key: entryKey, callback: workingCallback, reuseConnection: false});
637+
const connFailing = Onyx.connect({
638+
key: entryKey,
639+
callback: failingCallback,
640+
reuseConnection: false,
641+
});
642+
const connWorking = Onyx.connect({
643+
key: entryKey,
644+
callback: workingCallback,
645+
reuseConnection: false,
646+
});
618647
await waitForPromisesToResolve();
619648
failingCallback.mockReset();
620649
failingCallback.mockImplementation(() => {
@@ -869,7 +898,9 @@ describe('OnyxUtils', () => {
869898
// Cache must reflect the merge regardless of the multiMerge rejection. This is the
870899
// cache-first / storage-second invariant that mergeCollectionWithPatches must honor.
871900
const cachedCollection = OnyxCache.getCollectionData(collectionKey);
872-
expect(cachedCollection?.[existingMemberKey]).toEqual({value: 'merged'});
901+
expect(cachedCollection?.[existingMemberKey]).toEqual({
902+
value: 'merged',
903+
});
873904
expect(cachedCollection?.[newMemberKey]).toEqual({value: 'new'});
874905

875906
// Subscribers must have been notified with the merged values.
@@ -920,6 +951,176 @@ describe('OnyxUtils', () => {
920951
});
921952
});
922953

954+
describe('mergeCollection pre-warm', () => {
955+
// The retryOperation describe block above mutates StorageMock.setItem (and never restores it),
956+
// so by the time we run, setItem may be a rejecting mock from a prior test. Capture pristine
957+
// references at file-load time and restore them in beforeEach so our seeding via Onyx.set
958+
// actually reaches the in-memory storage provider.
959+
const pristineSetItem = StorageMock.setItem;
960+
const pristineMultiSet = StorageMock.multiSet;
961+
const pristineMultiGet = StorageMock.multiGet;
962+
const pristineGetItem = StorageMock.getItem;
963+
const pristineMultiMerge = StorageMock.multiMerge;
964+
965+
beforeEach(() => {
966+
StorageMock.setItem = pristineSetItem;
967+
StorageMock.multiSet = pristineMultiSet;
968+
StorageMock.multiGet = pristineMultiGet;
969+
StorageMock.getItem = pristineGetItem;
970+
StorageMock.multiMerge = pristineMultiMerge;
971+
});
972+
973+
// Make a key "cold" — value evicted from cache but the key is still tracked as persisted.
974+
// OnyxCache.drop also removes the key from `storageKeys`, which would cause getAllKeys() to
975+
// miss it (unless the entire storageKeys set is empty, in which case the function falls back
976+
// to Storage.getAllKeys). To reliably hit the cold-but-persisted state regardless of how many
977+
// other keys remain in cache, we re-register the key as known after dropping its value.
978+
const evictFromCache = (...keys: string[]) =>
979+
keys.forEach((key) => {
980+
OnyxCache.drop(key);
981+
OnyxCache.addKey(key);
982+
});
983+
984+
it('fast path: skips storage reads entirely when every existing key is warm in cache', async () => {
985+
const collectionKey = ONYXKEYS.COLLECTION.TEST_KEY;
986+
const existingKey1 = `${collectionKey}1`;
987+
const existingKey2 = `${collectionKey}2`;
988+
989+
// Seed both members so they are both warm in cache and present in storage.
990+
await Onyx.set(existingKey1, {value: 'initial-1'});
991+
await Onyx.set(existingKey2, {value: 'initial-2'});
992+
993+
const multiGetSpy = jest.spyOn(StorageMock, 'multiGet');
994+
const getItemSpy = jest.spyOn(StorageMock, 'getItem');
995+
996+
await Onyx.mergeCollection(collectionKey, {
997+
[existingKey1]: {value: 'merged-1'},
998+
[existingKey2]: {value: 'merged-2'},
999+
} as GenericCollection);
1000+
1001+
// With every existingKey warm, the diff swaps Promise.all(get) for Promise.resolve(),
1002+
// so no storage reads should happen during the pre-warm.
1003+
expect(multiGetSpy).not.toHaveBeenCalled();
1004+
expect(getItemSpy).not.toHaveBeenCalled();
1005+
1006+
// Cache still reflects the merge.
1007+
const cached = OnyxCache.getCollectionData(collectionKey);
1008+
expect(cached?.[existingKey1]).toEqual({value: 'merged-1'});
1009+
expect(cached?.[existingKey2]).toEqual({value: 'merged-2'});
1010+
});
1011+
1012+
it('slow path: batches cold existing keys into a single Storage.multiGet, with no individual getItem calls', async () => {
1013+
const collectionKey = ONYXKEYS.COLLECTION.TEST_KEY;
1014+
const coldKey1 = `${collectionKey}1`;
1015+
const coldKey2 = `${collectionKey}2`;
1016+
const warmKey = `${collectionKey}3`;
1017+
1018+
// Seed all three in storage, then evict two from cache so they are cold-but-persisted.
1019+
await Onyx.set(coldKey1, {value: 'persisted-1'});
1020+
await Onyx.set(coldKey2, {value: 'persisted-2'});
1021+
await Onyx.set(warmKey, {value: 'persisted-3'});
1022+
evictFromCache(coldKey1, coldKey2);
1023+
1024+
// Reset spies AFTER seeding so we only count calls made during mergeCollection itself.
1025+
const multiGetSpy = jest.spyOn(StorageMock, 'multiGet').mockClear();
1026+
const getItemSpy = jest.spyOn(StorageMock, 'getItem').mockClear();
1027+
1028+
await Onyx.mergeCollection(collectionKey, {
1029+
[coldKey1]: {value: 'merged-1'},
1030+
[coldKey2]: {value: 'merged-2'},
1031+
[warmKey]: {value: 'merged-3'},
1032+
} as GenericCollection);
1033+
1034+
// OnyxUtils.multiGet filters to cache-missing keys before issuing Storage.multiGet, so we
1035+
// expect exactly one batched read containing only the cold keys (the warm key is skipped).
1036+
expect(multiGetSpy).toHaveBeenCalledTimes(1);
1037+
const requestedKeys = multiGetSpy.mock.calls[0][0] as string[];
1038+
expect(requestedKeys.sort()).toEqual([coldKey1, coldKey2].sort());
1039+
1040+
// No individual Storage.getItem calls during pre-warm. Old code path would have fired one
1041+
// get() per existing key, each potentially landing in Storage.getItem on cache miss.
1042+
expect(getItemSpy).not.toHaveBeenCalled();
1043+
});
1044+
1045+
it('slow path: cold-cache merge layers the new delta on top of existing storage data (no field drops)', async () => {
1046+
const collectionKey = ONYXKEYS.COLLECTION.TEST_KEY;
1047+
const coldKey = `${collectionKey}1`;
1048+
1049+
// Seed an object with multiple fields in storage, then evict from cache so the merge base
1050+
// must come from a storage read — not from `undefined`.
1051+
await Onyx.set(coldKey, {a: 1, b: 2});
1052+
evictFromCache(coldKey);
1053+
1054+
await Onyx.mergeCollection(collectionKey, {
1055+
[coldKey]: {c: 3},
1056+
} as GenericCollection);
1057+
1058+
// If the pre-warm did NOT populate the cache from storage, fastMerge would treat the
1059+
// previous value as undefined and the result would drop {a:1, b:2}. With the pre-warm
1060+
// running multiGet on the cold key, the merge layers {c:3} on top of {a:1, b:2}.
1061+
const cached = OnyxCache.getCollectionData(collectionKey);
1062+
expect(cached?.[coldKey]).toEqual({a: 1, b: 2, c: 3});
1063+
});
1064+
1065+
it('warm cache: subscriber receives a single merged broadcast for an Onyx.update batch (no transient undefined)', async () => {
1066+
const collectionKey = ONYXKEYS.COLLECTION.TEST_KEY;
1067+
const existingKey = `${collectionKey}1`;
1068+
1069+
await Onyx.set(existingKey, {value: 'initial'});
1070+
1071+
const collectionCallback = jest.fn();
1072+
Onyx.connect({
1073+
key: collectionKey,
1074+
waitForCollectionCallback: true,
1075+
callback: collectionCallback,
1076+
});
1077+
await waitForPromisesToResolve();
1078+
collectionCallback.mockClear();
1079+
1080+
await Onyx.update([
1081+
{
1082+
onyxMethod: Onyx.METHOD.MERGE_COLLECTION,
1083+
key: collectionKey,
1084+
value: {
1085+
[existingKey]: {value: 'merged'},
1086+
} as GenericCollection,
1087+
},
1088+
]);
1089+
1090+
// The fast path resolves the pre-warm synchronously (Promise.resolve()), preserving the
1091+
// original promise-chain depth. The Onyx.update batch must therefore broadcast exactly
1092+
// one merged value — not undefined first and the merged value on a later microtask.
1093+
const broadcasts = collectionCallback.mock.calls.map((c) => c[0]);
1094+
expect(broadcasts).toHaveLength(1);
1095+
expect(broadcasts[0]?.[existingKey]).toEqual({value: 'merged'});
1096+
});
1097+
1098+
it('equivalence: warm-path and cold-path produce the same final cache state for the same merge', async () => {
1099+
const collectionKey = ONYXKEYS.COLLECTION.TEST_KEY;
1100+
const memberKey = `${collectionKey}1`;
1101+
const delta = {value: 'after'} as const;
1102+
1103+
// Warm-path run.
1104+
await Onyx.set(memberKey, {value: 'before', extra: 'kept'});
1105+
await Onyx.mergeCollection(collectionKey, {
1106+
[memberKey]: delta,
1107+
} as GenericCollection);
1108+
const warmResult = OnyxCache.getCollectionData(collectionKey)?.[memberKey];
1109+
1110+
// Reset and replay with a cold cache before the merge.
1111+
await Onyx.clear();
1112+
await Onyx.set(memberKey, {value: 'before', extra: 'kept'});
1113+
evictFromCache(memberKey);
1114+
await Onyx.mergeCollection(collectionKey, {
1115+
[memberKey]: delta,
1116+
} as GenericCollection);
1117+
const coldResult = OnyxCache.getCollectionData(collectionKey)?.[memberKey];
1118+
1119+
expect(warmResult).toEqual(coldResult);
1120+
expect(coldResult).toEqual({value: 'after', extra: 'kept'});
1121+
});
1122+
});
1123+
9231124
describe('storage eviction', () => {
9241125
const diskFullError = new Error('database or disk is full');
9251126

@@ -1004,7 +1205,9 @@ describe('OnyxUtils', () => {
10041205
// No more evictable candidates
10051206
expect(LocalOnyxCache.getKeyForEviction()).toBeUndefined();
10061207
// Non-evictable key should still be in cache
1007-
expect(LocalOnyxCache.get(ONYXKEYS.TEST_KEY)).toEqual({test: 'not evictable'});
1208+
expect(LocalOnyxCache.get(ONYXKEYS.TEST_KEY)).toEqual({
1209+
test: 'not evictable',
1210+
});
10081211
});
10091212

10101213
it('should not add collection keys to eviction candidates, only their members', async () => {
@@ -1030,8 +1233,12 @@ describe('OnyxUtils', () => {
10301233
LocalOnyxCache = require('../../lib/OnyxCache').default;
10311234
const storage = require('../../lib/storage').default;
10321235

1033-
await storage.setItem(`${ONYXKEYS.COLLECTION.TEST_KEY}pre1`, {id: 'pre1'});
1034-
await storage.setItem(`${ONYXKEYS.COLLECTION.TEST_KEY}pre2`, {id: 'pre2'});
1236+
await storage.setItem(`${ONYXKEYS.COLLECTION.TEST_KEY}pre1`, {
1237+
id: 'pre1',
1238+
});
1239+
await storage.setItem(`${ONYXKEYS.COLLECTION.TEST_KEY}pre2`, {
1240+
id: 'pre2',
1241+
});
10351242

10361243
// Init — addEvictableKeysToRecentlyAccessedList should seed them
10371244
LocalOnyx.init({

0 commit comments

Comments
 (0)