Skip to content

Commit 98ddd13

Browse files
committed
multiGet: prefer cache over stale storage on concurrent writes
Mirrors the existing prefer-cache check in get() (lib/OnyxUtils.ts around line 299). Before this change, if a concurrent set/merge populated cache while multiGet's Storage.multiGet was in flight, the trailing cache.merge(temp) would fastMerge the stale storage snapshot on top of the fresher cache value — resurrecting fields the concurrent write meant to drop. Per-key check at the storage-result loop: if cache.hasCacheForKey is true at the time we'd write, take the cached value into dataMap and skip writing to temp. cache.merge(temp) at the end then only touches keys that stayed genuinely cold throughout the read. Adds a regression test under `multiGet cache hit consistency` that sets the cache from inside the Storage.multiGet mock (synchronously, before the promise resolves), then asserts both the returned dataMap and the in-memory cache reflect the concurrent write — not the stale storage snapshot. Fails without the lib change, passes with it.
1 parent 2f9fb8a commit 98ddd13

2 files changed

Lines changed: 28 additions & 0 deletions

File tree

lib/OnyxUtils.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,13 @@ function multiGet<TKey extends OnyxKey>(keys: CollectionKeyBase[]): Promise<Map<
402402
}
403403
}
404404

405+
// Prefer cache over stale storage if a concurrent write populated it during
406+
// the read — otherwise cache.merge(temp) below would resurrect dropped fields.
407+
if (cache.hasCacheForKey(key)) {
408+
dataMap.set(key, cache.get(key) as OnyxValue<TKey>);
409+
continue;
410+
}
411+
405412
dataMap.set(key, value as OnyxValue<TKey>);
406413
temp[key] = value as OnyxValue<TKey>;
407414
}

tests/unit/onyxUtilsTest.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1174,6 +1174,27 @@ describe('OnyxUtils', () => {
11741174
expect(getItemSpy).not.toHaveBeenCalled();
11751175
expect(result.get(falsyKey)).toBe(0);
11761176
});
1177+
1178+
it('prefers cache when a concurrent write lands during the storage read', async () => {
1179+
// Concurrent write during multiGet's storage read must not be overwritten by the
1180+
// stale snapshot via cache.merge.
1181+
const key = `${ONYXKEYS.COLLECTION.TEST_KEY}race`;
1182+
1183+
OnyxCache.drop(key);
1184+
OnyxCache.addKey(key);
1185+
1186+
// Set cache inside the mock so it lands before Storage.multiGet's promise resolves —
1187+
// multiGet's .then() then sees a populated cache and skips writing the stale value.
1188+
StorageMock.multiGet = jest.fn().mockImplementation(() => {
1189+
OnyxCache.set(key, {fresh: 'data'});
1190+
return Promise.resolve([[key, {stale: 'a', alsoStale: 'b'}]]);
1191+
});
1192+
1193+
const result = await OnyxUtils.multiGet([key]);
1194+
1195+
expect(OnyxCache.get(key)).toEqual({fresh: 'data'});
1196+
expect(result.get(key)).toEqual({fresh: 'data'});
1197+
});
11771198
});
11781199

11791200
describe('storage eviction', () => {

0 commit comments

Comments
 (0)