Skip to content

Commit d02382f

Browse files
elirangoshenclaude
andcommitted
test: cover mergeCollectionWithPatches multiGet pre-warm fast/slow paths
Adds a new describe('mergeCollection pre-warm', ...) block with 5 tests: 1. Fast path: skips storage reads entirely when every existing key is warm in cache (spies on StorageMock.multiGet and StorageMock.getItem, asserts both are called 0 times). 2. Slow path: batches cold existing keys into a single Storage.multiGet, with no individual getItem calls. 3. Slow path: cold-cache merge layers the new delta on top of existing storage data (no field drops) — guards the correctness invariant the in-code comment specifically calls out. 4. Warm cache: subscriber receives a single merged broadcast for an Onyx.update batch (no transient undefined) — guards promise-chain depth. 5. Equivalence: warm-path and cold-path produce the same final cache state for the same merge. Includes a suite-pollution fix (capture pristine StorageMock refs and restore them in beforeEach so seeding via Onyx.set isn't intercepted by mocks leaking from the earlier retryOperation describe block), and an evictFromCache helper that uses for…of to satisfy unicorn/no-array-for-each. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 3295637 commit d02382f

1 file changed

Lines changed: 171 additions & 0 deletions

File tree

tests/unit/onyxUtilsTest.ts

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -920,6 +920,177 @@ describe('OnyxUtils', () => {
920920
});
921921
});
922922

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

0 commit comments

Comments
 (0)