Skip to content

Commit f00b48f

Browse files
committed
prevent storage reads for RAM-only keys
1 parent 0c9172b commit f00b48f

3 files changed

Lines changed: 337 additions & 2 deletions

File tree

lib/Onyx.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ function init({
5959

6060
if (shouldSyncMultipleInstances) {
6161
Storage.keepInstancesSync?.((key, value) => {
62+
// RAM-only keys should never sync from storage as they may have stale persisted data
63+
// from before the key was migrated to RAM-only.
64+
if (OnyxUtils.isRamOnlyKey(key)) {
65+
return;
66+
}
67+
6268
cache.set(key, value);
6369

6470
// Check if this is a collection member key to prevent duplicate callbacks

lib/OnyxUtils.ts

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,14 @@ function get<TKey extends OnyxKey, TValue extends OnyxValue<TKey>>(key: TKey): P
265265
return Promise.resolve(cache.get(key) as TValue);
266266
}
267267

268+
// RAM-only keys should never read from storage (they may have stale persisted data
269+
// from before the key was migrated to RAM-only). Mark as nullish so future get() calls
270+
// short-circuit via hasCacheForKey and avoid re-running this branch.
271+
if (isRamOnlyKey(key)) {
272+
cache.addNullishStorageKey(key);
273+
return Promise.resolve(undefined as TValue);
274+
}
275+
268276
const taskName = `${TASK.GET}:${key}` as const;
269277

270278
// When a value retrieving task for this key is still running hook to it
@@ -324,6 +332,15 @@ function multiGet<TKey extends OnyxKey>(keys: CollectionKeyBase[]): Promise<Map<
324332
* These missingKeys will be later used to multiGet the data from the storage.
325333
*/
326334
for (const key of keys) {
335+
// RAM-only keys should never read from storage as they may have stale persisted data
336+
// from before the key was migrated to RAM-only.
337+
if (isRamOnlyKey(key)) {
338+
if (cache.hasCacheForKey(key)) {
339+
dataMap.set(key, cache.get(key) as OnyxValue<TKey>);
340+
}
341+
continue;
342+
}
343+
327344
const cacheValue = cache.get(key) as OnyxValue<TKey>;
328345
if (cacheValue) {
329346
dataMap.set(key, cacheValue);
@@ -441,7 +458,10 @@ function getAllKeys(): Promise<Set<OnyxKey>> {
441458

442459
// Otherwise retrieve the keys from storage and capture a promise to aid concurrent usages
443460
const promise = Storage.getAllKeys().then((keys) => {
444-
cache.setAllKeys(keys);
461+
// Filter out RAM-only keys from storage results as they may be stale entries
462+
// from before the key was migrated to RAM-only.
463+
const filteredKeys = keys.filter((key) => !isRamOnlyKey(key));
464+
cache.setAllKeys(filteredKeys);
445465

446466
// return the updated set of keys
447467
return cache.getAllKeys();
@@ -1101,7 +1121,10 @@ function mergeInternal<TValue extends OnyxInput<OnyxKey> | undefined, TChange ex
11011121
* Merge user provided default key value pairs.
11021122
*/
11031123
function initializeWithDefaultKeyStates(): Promise<void> {
1104-
return Storage.multiGet(Object.keys(defaultKeyStates)).then((pairs) => {
1124+
// Filter out RAM-only keys from storage reads as they may have stale persisted data
1125+
// from before the key was migrated to RAM-only.
1126+
const keysToFetch = Object.keys(defaultKeyStates).filter((key) => !isRamOnlyKey(key));
1127+
return Storage.multiGet(keysToFetch).then((pairs) => {
11051128
const existingDataAsObject = Object.fromEntries(pairs) as Record<string, unknown>;
11061129

11071130
const merged = utils.fastMerge(existingDataAsObject, defaultKeyStates, {

tests/unit/onyxTest.ts

Lines changed: 306 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3112,3 +3112,309 @@ describe('Onyx.init', () => {
31123112
});
31133113
});
31143114
});
3115+
3116+
// Separate describe block to control Onyx.init() per-test so we can pre-seed storage before init.
3117+
describe('RAM-only keys should not read from storage', () => {
3118+
const RAM_ONLY_KEYS = {
3119+
TEST_KEY: 'test',
3120+
OTHER_TEST: 'otherTest',
3121+
COLLECTION: {
3122+
TEST_KEY: 'test_',
3123+
RAM_ONLY_COLLECTION: 'ramOnlyCollection_',
3124+
},
3125+
RAM_ONLY_KEY: 'ramOnlyKey',
3126+
RAM_ONLY_WITH_INITIAL_VALUE: 'ramOnlyWithInitialValue',
3127+
};
3128+
3129+
let cache: typeof OnyxCache;
3130+
3131+
beforeEach(() => {
3132+
Object.assign(OnyxUtils.getDeferredInitTask(), createDeferredTask());
3133+
cache = require('../../lib/OnyxCache').default;
3134+
});
3135+
3136+
afterEach(() => {
3137+
jest.restoreAllMocks();
3138+
return Onyx.clear();
3139+
});
3140+
3141+
it('should not return stale storage data for a RAM-only key via get', async () => {
3142+
// Simulate stale data left in storage from before the key was RAM-only
3143+
await StorageMock.setItem(RAM_ONLY_KEYS.RAM_ONLY_KEY, 'stale_value');
3144+
3145+
Onyx.init({
3146+
keys: RAM_ONLY_KEYS,
3147+
ramOnlyKeys: [RAM_ONLY_KEYS.RAM_ONLY_KEY, RAM_ONLY_KEYS.COLLECTION.RAM_ONLY_COLLECTION, RAM_ONLY_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
3148+
});
3149+
await act(async () => waitForPromisesToResolve());
3150+
3151+
let receivedValue: unknown;
3152+
const connection = Onyx.connect({
3153+
key: RAM_ONLY_KEYS.RAM_ONLY_KEY,
3154+
callback: (value) => {
3155+
receivedValue = value;
3156+
},
3157+
});
3158+
await act(async () => waitForPromisesToResolve());
3159+
3160+
expect(receivedValue).toBeUndefined();
3161+
expect(cache.get(RAM_ONLY_KEYS.RAM_ONLY_KEY)).toBeUndefined();
3162+
3163+
Onyx.disconnect(connection);
3164+
});
3165+
3166+
it('should not return stale storage data for RAM-only collection members via multiGet', async () => {
3167+
const collectionMember1 = `${RAM_ONLY_KEYS.COLLECTION.RAM_ONLY_COLLECTION}1`;
3168+
const collectionMember2 = `${RAM_ONLY_KEYS.COLLECTION.RAM_ONLY_COLLECTION}2`;
3169+
3170+
// Simulate stale collection members in storage
3171+
await StorageMock.setItem(collectionMember1, {name: 'stale_1'});
3172+
await StorageMock.setItem(collectionMember2, {name: 'stale_2'});
3173+
3174+
Onyx.init({
3175+
keys: RAM_ONLY_KEYS,
3176+
ramOnlyKeys: [RAM_ONLY_KEYS.RAM_ONLY_KEY, RAM_ONLY_KEYS.COLLECTION.RAM_ONLY_COLLECTION, RAM_ONLY_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
3177+
});
3178+
await act(async () => waitForPromisesToResolve());
3179+
3180+
let receivedCollection: OnyxCollection<unknown>;
3181+
const connection = Onyx.connect({
3182+
key: RAM_ONLY_KEYS.COLLECTION.RAM_ONLY_COLLECTION,
3183+
callback: (value) => {
3184+
receivedCollection = value;
3185+
},
3186+
waitForCollectionCallback: true,
3187+
});
3188+
await act(async () => waitForPromisesToResolve());
3189+
3190+
expect(receivedCollection!).toBeUndefined();
3191+
expect(cache.get(collectionMember1)).toBeUndefined();
3192+
expect(cache.get(collectionMember2)).toBeUndefined();
3193+
3194+
Onyx.disconnect(connection);
3195+
});
3196+
3197+
it('should not include stale RAM-only keys in getAllKeys results', async () => {
3198+
// Simulate stale data in storage
3199+
await StorageMock.setItem(RAM_ONLY_KEYS.RAM_ONLY_KEY, 'stale_value');
3200+
await StorageMock.setItem(`${RAM_ONLY_KEYS.COLLECTION.RAM_ONLY_COLLECTION}1`, {stale: 'member'});
3201+
await StorageMock.setItem(RAM_ONLY_KEYS.OTHER_TEST, 'normal_value');
3202+
3203+
Onyx.init({
3204+
keys: RAM_ONLY_KEYS,
3205+
ramOnlyKeys: [RAM_ONLY_KEYS.RAM_ONLY_KEY, RAM_ONLY_KEYS.COLLECTION.RAM_ONLY_COLLECTION, RAM_ONLY_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
3206+
});
3207+
await act(async () => waitForPromisesToResolve());
3208+
3209+
const keys = await OnyxUtils.getAllKeys();
3210+
3211+
expect(keys.has(RAM_ONLY_KEYS.RAM_ONLY_KEY)).toBe(false);
3212+
expect(keys.has(`${RAM_ONLY_KEYS.COLLECTION.RAM_ONLY_COLLECTION}1`)).toBe(false);
3213+
// Normal keys should still be present
3214+
expect(keys.has(RAM_ONLY_KEYS.OTHER_TEST)).toBe(true);
3215+
});
3216+
3217+
it('should not read stale storage data for RAM-only keys during initializeWithDefaultKeyStates', async () => {
3218+
// Simulate stale data for a RAM-only key that also has a default key state
3219+
await StorageMock.setItem(RAM_ONLY_KEYS.RAM_ONLY_WITH_INITIAL_VALUE, 'stale_value');
3220+
3221+
Onyx.init({
3222+
keys: RAM_ONLY_KEYS,
3223+
initialKeyStates: {
3224+
[RAM_ONLY_KEYS.RAM_ONLY_WITH_INITIAL_VALUE]: 'default_value',
3225+
},
3226+
ramOnlyKeys: [RAM_ONLY_KEYS.RAM_ONLY_KEY, RAM_ONLY_KEYS.COLLECTION.RAM_ONLY_COLLECTION, RAM_ONLY_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
3227+
});
3228+
await act(async () => waitForPromisesToResolve());
3229+
3230+
// The cache should have the default value, not the stale storage value
3231+
expect(cache.get(RAM_ONLY_KEYS.RAM_ONLY_WITH_INITIAL_VALUE)).toEqual('default_value');
3232+
});
3233+
3234+
it('should not use stale storage data as merge base for RAM-only keys', async () => {
3235+
// Simulate stale data in storage
3236+
await StorageMock.setItem(RAM_ONLY_KEYS.RAM_ONLY_KEY, {name: 'stale', token: 'old_token'});
3237+
3238+
Onyx.init({
3239+
keys: RAM_ONLY_KEYS,
3240+
ramOnlyKeys: [RAM_ONLY_KEYS.RAM_ONLY_KEY, RAM_ONLY_KEYS.COLLECTION.RAM_ONLY_COLLECTION, RAM_ONLY_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
3241+
});
3242+
await act(async () => waitForPromisesToResolve());
3243+
3244+
// Merge new data — should NOT merge with stale storage value
3245+
await Onyx.merge(RAM_ONLY_KEYS.RAM_ONLY_KEY, {name: 'new'});
3246+
3247+
// The result should only contain the merged value, not the stale token
3248+
expect(cache.get(RAM_ONLY_KEYS.RAM_ONLY_KEY)).toEqual({name: 'new'});
3249+
});
3250+
3251+
it('should not read stale storage data when subscribing to individual RAM-only collection members', async () => {
3252+
const collectionMember = `${RAM_ONLY_KEYS.COLLECTION.RAM_ONLY_COLLECTION}1`;
3253+
3254+
// Simulate stale data in storage
3255+
await StorageMock.setItem(collectionMember, {data: 'stale'});
3256+
3257+
Onyx.init({
3258+
keys: RAM_ONLY_KEYS,
3259+
ramOnlyKeys: [RAM_ONLY_KEYS.RAM_ONLY_KEY, RAM_ONLY_KEYS.COLLECTION.RAM_ONLY_COLLECTION, RAM_ONLY_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
3260+
});
3261+
await act(async () => waitForPromisesToResolve());
3262+
3263+
const receivedValues: unknown[] = [];
3264+
const connection = Onyx.connect({
3265+
key: collectionMember,
3266+
callback: (value) => {
3267+
receivedValues.push(value);
3268+
},
3269+
});
3270+
await act(async () => waitForPromisesToResolve());
3271+
3272+
// Should never receive the stale value
3273+
expect(receivedValues.every((v) => v === undefined || v === null)).toBe(true);
3274+
3275+
Onyx.disconnect(connection);
3276+
});
3277+
3278+
it('should still work correctly for normal keys when RAM-only keys have stale storage data', async () => {
3279+
// Simulate both normal and RAM-only stale data in storage
3280+
await StorageMock.setItem(RAM_ONLY_KEYS.TEST_KEY, 'normal_value');
3281+
await StorageMock.setItem(RAM_ONLY_KEYS.RAM_ONLY_KEY, 'stale_ram_value');
3282+
3283+
Onyx.init({
3284+
keys: RAM_ONLY_KEYS,
3285+
ramOnlyKeys: [RAM_ONLY_KEYS.RAM_ONLY_KEY, RAM_ONLY_KEYS.COLLECTION.RAM_ONLY_COLLECTION, RAM_ONLY_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
3286+
});
3287+
await act(async () => waitForPromisesToResolve());
3288+
3289+
let normalValue: unknown;
3290+
let ramOnlyValue: unknown;
3291+
3292+
const connection1 = Onyx.connect({
3293+
key: RAM_ONLY_KEYS.TEST_KEY,
3294+
callback: (value) => {
3295+
normalValue = value;
3296+
},
3297+
});
3298+
const connection2 = Onyx.connect({
3299+
key: RAM_ONLY_KEYS.RAM_ONLY_KEY,
3300+
callback: (value) => {
3301+
ramOnlyValue = value;
3302+
},
3303+
});
3304+
await act(async () => waitForPromisesToResolve());
3305+
3306+
// Normal key should read from storage as expected
3307+
expect(normalValue).toEqual('normal_value');
3308+
// RAM-only key should NOT read stale value from storage
3309+
expect(ramOnlyValue).toBeUndefined();
3310+
3311+
Onyx.disconnect(connection1);
3312+
Onyx.disconnect(connection2);
3313+
});
3314+
3315+
it('should not sync RAM-only keys from other instances via keepInstancesSync', async () => {
3316+
Onyx.init({
3317+
keys: RAM_ONLY_KEYS,
3318+
ramOnlyKeys: [RAM_ONLY_KEYS.RAM_ONLY_KEY, RAM_ONLY_KEYS.COLLECTION.RAM_ONLY_COLLECTION, RAM_ONLY_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
3319+
shouldSyncMultipleInstances: true,
3320+
});
3321+
await act(async () => waitForPromisesToResolve());
3322+
3323+
// Get the callback that was passed to keepInstancesSync
3324+
const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls[0]?.[0];
3325+
expect(syncCallback).toBeDefined();
3326+
3327+
let receivedValue: unknown;
3328+
const connection = Onyx.connect({
3329+
key: RAM_ONLY_KEYS.RAM_ONLY_KEY,
3330+
callback: (value) => {
3331+
receivedValue = value;
3332+
},
3333+
});
3334+
await act(async () => waitForPromisesToResolve());
3335+
3336+
// Simulate another tab syncing a stale RAM-only key value
3337+
syncCallback(RAM_ONLY_KEYS.RAM_ONLY_KEY, 'synced_stale_value');
3338+
await act(async () => waitForPromisesToResolve());
3339+
3340+
// The RAM-only key should NOT have been updated from the sync
3341+
expect(receivedValue).toBeUndefined();
3342+
expect(cache.get(RAM_ONLY_KEYS.RAM_ONLY_KEY)).toBeUndefined();
3343+
3344+
// Verify that normal keys still sync correctly
3345+
let normalValue: unknown;
3346+
const connection2 = Onyx.connect({
3347+
key: RAM_ONLY_KEYS.OTHER_TEST,
3348+
callback: (value) => {
3349+
normalValue = value;
3350+
},
3351+
});
3352+
await act(async () => waitForPromisesToResolve());
3353+
3354+
syncCallback(RAM_ONLY_KEYS.OTHER_TEST, 'synced_normal_value');
3355+
await act(async () => waitForPromisesToResolve());
3356+
3357+
expect(normalValue).toEqual('synced_normal_value');
3358+
3359+
Onyx.disconnect(connection);
3360+
Onyx.disconnect(connection2);
3361+
});
3362+
3363+
it('should serve RAM-only keys from cache and normal keys from storage in multiGet', async () => {
3364+
const ramOnlyMember = `${RAM_ONLY_KEYS.COLLECTION.RAM_ONLY_COLLECTION}1`;
3365+
const normalMember = `${RAM_ONLY_KEYS.COLLECTION.TEST_KEY}1`;
3366+
3367+
// Pre-seed storage with stale data for both normal and RAM-only keys
3368+
await StorageMock.setItem(normalMember, 'normal_from_storage');
3369+
await StorageMock.setItem(ramOnlyMember, {data: 'stale_collection_member'});
3370+
3371+
Onyx.init({
3372+
keys: RAM_ONLY_KEYS,
3373+
ramOnlyKeys: [RAM_ONLY_KEYS.RAM_ONLY_KEY, RAM_ONLY_KEYS.COLLECTION.RAM_ONLY_COLLECTION, RAM_ONLY_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
3374+
});
3375+
await act(async () => waitForPromisesToResolve());
3376+
3377+
// Set a RAM-only collection member via Onyx (goes to cache only)
3378+
await Onyx.set(ramOnlyMember, {data: 'fresh_from_cache'});
3379+
3380+
// multiGet receives individual keys (e.g. collection members), not collection base keys
3381+
const result = await OnyxUtils.multiGet([normalMember, ramOnlyMember]);
3382+
3383+
// Normal key should come from storage
3384+
expect(result.get(normalMember)).toEqual('normal_from_storage');
3385+
// RAM-only collection member should come from cache, not stale storage
3386+
expect(result.get(ramOnlyMember)).toEqual({data: 'fresh_from_cache'});
3387+
});
3388+
3389+
it('should return cached value for RAM-only key after set then connect', async () => {
3390+
await StorageMock.setItem(RAM_ONLY_KEYS.RAM_ONLY_KEY, 'stale_value');
3391+
3392+
Onyx.init({
3393+
keys: RAM_ONLY_KEYS,
3394+
ramOnlyKeys: [RAM_ONLY_KEYS.RAM_ONLY_KEY, RAM_ONLY_KEYS.COLLECTION.RAM_ONLY_COLLECTION, RAM_ONLY_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
3395+
});
3396+
await act(async () => waitForPromisesToResolve());
3397+
3398+
// Write a fresh value to the RAM-only key
3399+
await Onyx.set(RAM_ONLY_KEYS.RAM_ONLY_KEY, 'fresh_value');
3400+
3401+
let receivedValue: unknown;
3402+
const connection = Onyx.connect({
3403+
key: RAM_ONLY_KEYS.RAM_ONLY_KEY,
3404+
callback: (value) => {
3405+
receivedValue = value;
3406+
},
3407+
});
3408+
await act(async () => waitForPromisesToResolve());
3409+
3410+
// Should get the fresh cached value, not the stale storage value
3411+
expect(receivedValue).toEqual('fresh_value');
3412+
expect(cache.get(RAM_ONLY_KEYS.RAM_ONLY_KEY)).toEqual('fresh_value');
3413+
3414+
// Verify storage was NOT written to
3415+
const storageValue = await StorageMock.getItem(RAM_ONLY_KEYS.RAM_ONLY_KEY);
3416+
expect(storageValue).toEqual('stale_value');
3417+
3418+
Onyx.disconnect(connection);
3419+
});
3420+
});

0 commit comments

Comments
 (0)