From 566009cb409c5b1c58fbcf74866d4b78859c5765 Mon Sep 17 00:00:00 2001 From: eliran goshen Date: Mon, 26 Oct 2026 15:36:18 +0200 Subject: [PATCH 1/2] Keep values written while Onyx.clear is in progress Onyx.clear resets the cache right away but writes defaults to storage and notifies subscribers only after storage finishes clearing, using the values it captured at the start. A write that lands in between, like a new sign-in saving its session while the previous sign-out is still clearing, was then overwritten in storage and subscribers were told the key was reset. Skip the default write and the stale notification for keys written during the clear, and save those keys again after removeItems. --- lib/Onyx.ts | 40 ++++++- tests/unit/onyxClearConcurrentWriteTest.ts | 118 +++++++++++++++++++++ 2 files changed, 156 insertions(+), 2 deletions(-) create mode 100644 tests/unit/onyxClearConcurrentWriteTest.ts diff --git a/lib/Onyx.ts b/lib/Onyx.ts index e5df45366..b417f5d3b 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -1,3 +1,4 @@ +import {deepEqual} from 'fast-equals'; import * as Logger from './Logger'; import cache, {TASK} from './OnyxCache'; import Storage from './storage'; @@ -449,6 +450,21 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { const allKeys = new Set([...cachedKeys, ...initialKeys]); + // Writes can land while storage clears (e.g. a new sign-in's session), so track them to avoid clobbering them. + const valuesBeforeClear: KeyValueMapping = {}; + const valuesSetByClear: KeyValueMapping = {}; + const wasWrittenDuringClear = (key: OnyxKey): boolean => { + if (!(key in valuesSetByClear)) { + return false; + } + const currentValue = cache.get(key); + if (currentValue === undefined || currentValue === valuesSetByClear[key]) { + return false; + } + // A read can refill the cache with the old value from storage before it is removed, which is not a new write + return !deepEqual(currentValue, valuesBeforeClear[key]); + }; + OnyxUtils.cancelPendingMerges(pendingMergeEntries); // The only keys that should not be cleared are: @@ -468,6 +484,8 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { if (!isKeyToPreserve) { const oldValue = cache.get(key); const newValue = defaultKeyStates[key] ?? null; + valuesBeforeClear[key] = oldValue; + valuesSetByClear[key] = newValue; if (newValue !== oldValue) { cache.set(key, newValue); @@ -507,17 +525,35 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { // Remove only the items that we want cleared from storage, and reset others to default for (const key of keysToBeClearedFromStorage) cache.drop(key); return Storage.removeItems(keysToBeClearedFromStorage) - .then(() => Storage.multiSet(defaultKeyValuePairs)) + .then(() => { + // Save written keys again in case `removeItems` deleted them after they were stored. + const writtenKeyValuePairs = Object.keys(valuesSetByClear) + .filter((key) => !OnyxKeys.isRamOnlyKey(key) && wasWrittenDuringClear(key)) + .map((key): [OnyxKey, OnyxValue] => [key, cache.get(key)]); + return Storage.multiSet([...defaultKeyValuePairs.filter(([key]) => !wasWrittenDuringClear(key)), ...writtenKeyValuePairs]); + }) .then(() => { DevTools.clearState(keysToPreserve); onyxSubscriptionManager.resetCacheMissReports(); - // Notify the subscribers for each key/value group so they can receive the new values + // Notify the subscribers for each key/value group so they can receive the new values. + // Keys written during the clear are skipped, their subscribers already have the newer value. for (const [key, value] of Object.entries(keyValuesToResetIndividually)) { + if (wasWrittenDuringClear(key)) { + continue; + } OnyxUtils.notifyKey(key, value); } for (const [key, value] of Object.entries(keyValuesToResetAsCollection)) { + const writtenMemberKeys = Object.keys(value.newValues).filter(wasWrittenDuringClear); + for (const memberKey of writtenMemberKeys) { + delete value.newValues[memberKey]; + delete value.oldValues[memberKey]; + } + if (Object.keys(value.newValues).length === 0) { + continue; + } OnyxUtils.notifyCollection(key, value.newValues, value.oldValues); } }); diff --git a/tests/unit/onyxClearConcurrentWriteTest.ts b/tests/unit/onyxClearConcurrentWriteTest.ts new file mode 100644 index 000000000..481b7f506 --- /dev/null +++ b/tests/unit/onyxClearConcurrentWriteTest.ts @@ -0,0 +1,118 @@ +import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; +import StorageMock from '../../lib/storage'; +import Onyx from '../../lib/Onyx'; +import type OnyxCache from '../../lib/OnyxCache'; +import type GenericCollection from '../utils/GenericCollection'; +import type {Connection} from '../../lib/Onyx'; + +const ONYX_KEYS = { + DEFAULT_KEY: 'defaultKey', + REGULAR_KEY: 'regularKey', + COLLECTION: { + TEST: 'test_', + }, +}; +const OLD_VALUE = 'old'; +const NEW_VALUE = 'new'; +const DEFAULT_VALUE = 'default'; + +describe('Write data while Onyx.clear is in progress', () => { + let connections: Connection[] = []; + let defaultKeyValue: unknown; + let regularKeyValue: unknown; + let collectionValue: GenericCollection | undefined; + let cache: typeof OnyxCache; + let finishRemoveItems: () => void; + + beforeEach(() => { + cache = require('../../lib/OnyxCache').default; + Onyx.init({ + keys: ONYX_KEYS, + initialKeyStates: { + [ONYX_KEYS.DEFAULT_KEY]: DEFAULT_VALUE, + }, + }); + connections = [ + Onyx.connect({key: ONYX_KEYS.DEFAULT_KEY, callback: (value) => (defaultKeyValue = value)}), + Onyx.connect({key: ONYX_KEYS.REGULAR_KEY, callback: (value) => (regularKeyValue = value)}), + Onyx.connect({key: ONYX_KEYS.COLLECTION.TEST, callback: (value) => (collectionValue = value as GenericCollection)}), + ]; + return waitForPromisesToResolve(); + }); + + afterEach(() => { + for (const connection of connections) { + connection.unsubscribe(); + } + return Onyx.clear(); + }); + + // Holds Onyx.clear() in its storage step, so the test can write while the clear is still running. + function holdNextRemoveItems() { + const removeItemsMock = StorageMock.removeItems as jest.Mock; + const originalImplementation = removeItemsMock.getMockImplementation(); + removeItemsMock.mockImplementationOnce( + (keys: string[]) => + new Promise((resolve) => { + finishRemoveItems = () => resolve(originalImplementation?.(keys)); + }), + ); + } + + it('should keep values written to keys after the clear reset them', async () => { + // Given keys that hold values from a previous session + await Onyx.multiSet({ + [ONYX_KEYS.DEFAULT_KEY]: OLD_VALUE, + [ONYX_KEYS.REGULAR_KEY]: OLD_VALUE, + [`${ONYX_KEYS.COLLECTION.TEST}1`]: OLD_VALUE, + }); + + // When a clear starts and new values are written before its storage step finishes, like a new sign-in + // saving its session while the previous sign-out is still wiping storage + holdNextRemoveItems(); + const clearPromise = Onyx.clear(); + await waitForPromisesToResolve(); + Onyx.set(ONYX_KEYS.DEFAULT_KEY, NEW_VALUE); + Onyx.merge(ONYX_KEYS.REGULAR_KEY, NEW_VALUE); + Onyx.set(`${ONYX_KEYS.COLLECTION.TEST}1`, NEW_VALUE); + await waitForPromisesToResolve(); + finishRemoveItems(); + await clearPromise; + await waitForPromisesToResolve(); + + // Then the clear does not overwrite the new values in subscribers, the cache or storage + expect(defaultKeyValue).toBe(NEW_VALUE); + expect(regularKeyValue).toBe(NEW_VALUE); + expect(collectionValue).toEqual({[`${ONYX_KEYS.COLLECTION.TEST}1`]: NEW_VALUE}); + expect(cache.get(ONYX_KEYS.DEFAULT_KEY)).toBe(NEW_VALUE); + expect(cache.get(ONYX_KEYS.REGULAR_KEY)).toBe(NEW_VALUE); + expect(cache.get(`${ONYX_KEYS.COLLECTION.TEST}1`)).toBe(NEW_VALUE); + await expect(StorageMock.getItem(ONYX_KEYS.DEFAULT_KEY)).resolves.toBe(NEW_VALUE); + await expect(StorageMock.getItem(ONYX_KEYS.REGULAR_KEY)).resolves.toBe(NEW_VALUE); + await expect(StorageMock.getItem(`${ONYX_KEYS.COLLECTION.TEST}1`)).resolves.toBe(NEW_VALUE); + }); + + it('should still reset keys that were not written during the clear', async () => { + // Given keys that hold values from a previous session + await Onyx.multiSet({ + [ONYX_KEYS.DEFAULT_KEY]: OLD_VALUE, + [ONYX_KEYS.REGULAR_KEY]: OLD_VALUE, + }); + + // When a clear runs and only one key gets a new value while it is in progress + holdNextRemoveItems(); + const clearPromise = Onyx.clear(); + await waitForPromisesToResolve(); + Onyx.set(ONYX_KEYS.REGULAR_KEY, NEW_VALUE); + await waitForPromisesToResolve(); + finishRemoveItems(); + await clearPromise; + await waitForPromisesToResolve(); + + // Then the written key keeps its new value and the other key is reset to its default + expect(regularKeyValue).toBe(NEW_VALUE); + expect(defaultKeyValue).toBe(DEFAULT_VALUE); + expect(cache.get(ONYX_KEYS.DEFAULT_KEY)).toBe(DEFAULT_VALUE); + await expect(StorageMock.getItem(ONYX_KEYS.DEFAULT_KEY)).resolves.toBe(DEFAULT_VALUE); + }); +}); From a4ae39fa94893a5056440bc8acd7842542349834 Mon Sep 17 00:00:00 2001 From: eliran goshen Date: Tue, 27 Oct 2026 14:23:45 +0200 Subject: [PATCH 2/2] Track writes during Onyx.clear instead of comparing values Comparing the cache value with the value from before the clear treated a write that repeats that value (e.g. a re-auth restoring unchanged session data) as a read refilling the cache, so clear still wiped it. Record the keys of writes issued while clear runs through PendingWrites instead, since reads never register there. --- lib/Onyx.ts | 27 +++++++++++++--------- lib/PendingWrites.ts | 16 +++++++++++++ tests/unit/onyxClearConcurrentWriteTest.ts | 26 +++++++++++++++++++++ 3 files changed, 58 insertions(+), 11 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index b417f5d3b..3a05db78a 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -1,4 +1,3 @@ -import {deepEqual} from 'fast-equals'; import * as Logger from './Logger'; import cache, {TASK} from './OnyxCache'; import Storage from './storage'; @@ -429,7 +428,16 @@ function mergeCollection(collectionKey: TKey, co * @param keysToPreserve is a list of ONYXKEYS that should not be cleared with the rest of the data */ function clear(keysToPreserve: OnyxKey[] = []): Promise { - return PendingWrites.trackPendingGlobalWrite( + // Writes can land while storage clears (e.g. a new sign-in's session), so record them to avoid clobbering them. + // Matching on values instead would confuse them with reads refilling the cache from storage. + const keysWrittenDuringClear = new Set(); + const removeWriteListener = PendingWrites.addWriteListener((keys) => { + for (const key of keys) { + keysWrittenDuringClear.add(key); + } + }); + + const clearPromise = PendingWrites.trackPendingGlobalWrite( OnyxUtils.afterInit(() => { const pendingMergeEntries = OnyxUtils.getPendingMergeEntries(keysToPreserve); const defaultKeyStates = OnyxUtils.getDefaultKeyStates(); @@ -450,19 +458,15 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { const allKeys = new Set([...cachedKeys, ...initialKeys]); - // Writes can land while storage clears (e.g. a new sign-in's session), so track them to avoid clobbering them. - const valuesBeforeClear: KeyValueMapping = {}; + // A write issued before the reset below is overwritten by it, so only a cache value that differs from + // what clear set counts as a write that landed after the reset. const valuesSetByClear: KeyValueMapping = {}; const wasWrittenDuringClear = (key: OnyxKey): boolean => { - if (!(key in valuesSetByClear)) { + if (!keysWrittenDuringClear.has(key) || !(key in valuesSetByClear)) { return false; } const currentValue = cache.get(key); - if (currentValue === undefined || currentValue === valuesSetByClear[key]) { - return false; - } - // A read can refill the cache with the old value from storage before it is removed, which is not a new write - return !deepEqual(currentValue, valuesBeforeClear[key]); + return currentValue !== undefined && currentValue !== valuesSetByClear[key]; }; OnyxUtils.cancelPendingMerges(pendingMergeEntries); @@ -484,7 +488,6 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { if (!isKeyToPreserve) { const oldValue = cache.get(key); const newValue = defaultKeyStates[key] ?? null; - valuesBeforeClear[key] = oldValue; valuesSetByClear[key] = newValue; if (newValue !== oldValue) { cache.set(key, newValue); @@ -563,6 +566,8 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { return cache.captureTask(TASK.CLEAR, promise) as Promise; }), ); + clearPromise.then(removeWriteListener, removeWriteListener); + return clearPromise; } /** diff --git a/lib/PendingWrites.ts b/lib/PendingWrites.ts index 9664fefb0..9aac33f4b 100644 --- a/lib/PendingWrites.ts +++ b/lib/PendingWrites.ts @@ -8,6 +8,9 @@ const pendingWritesByKey = new Map>>(); // In-flight writes that affect every key (Onyx.clear). Any initial notification waits for these. const pendingGlobalWrites = new Set>(); +// Told about every write as it is issued, so Onyx.clear can tell writes apart from reads refilling the cache. +const writeListeners = new Set<(keys: OnyxKey[]) => void>(); + /** * Registers an in-flight write under each key it can change, so `scheduleInitialSubscriberNotification` * waits only for the writes relevant to a connecting key. Returns the same promise so callers can wrap a @@ -16,6 +19,9 @@ const pendingGlobalWrites = new Set>(); function trackPendingWrite(keys: OnyxKey | OnyxKey[], pendingWrite: Promise): Promise { // Drop nullish keys (e.g. a keyless `clear` item) so they never reach `getPendingWritesForKey`'s scan. const keyList = (Array.isArray(keys) ? keys : [keys]).filter((key) => typeof key === 'string'); + for (const listener of writeListeners) { + listener(keyList); + } for (const key of keyList) { let pendingWritesForKey = pendingWritesByKey.get(key); if (!pendingWritesForKey) { @@ -50,6 +56,14 @@ function trackPendingGlobalWrite(pendingGlobalWrite: Promise): Promise return pendingGlobalWrite; } +/** + * Calls `listener` with the keys of every write issued from now on, until the returned function is called. + */ +function addWriteListener(listener: (keys: OnyxKey[]) => void): () => void { + writeListeners.add(listener); + return () => writeListeners.delete(listener); +} + /** * In-flight writes that can change the value delivered to a subscriber of `key`: writes to the key * itself, writes to any member when `key` is a collection root, and global writes (clear). @@ -96,11 +110,13 @@ function scheduleInitialSubscriberNotification(key: OnyxKey, notify: () => void) function clearPendingWrites(): void { pendingWritesByKey.clear(); pendingGlobalWrites.clear(); + writeListeners.clear(); } export default { trackPendingWrite, trackPendingGlobalWrite, + addWriteListener, getPendingWritesForKey, scheduleInitialSubscriberNotification, clearPendingWrites, diff --git a/tests/unit/onyxClearConcurrentWriteTest.ts b/tests/unit/onyxClearConcurrentWriteTest.ts index 481b7f506..dd23c067c 100644 --- a/tests/unit/onyxClearConcurrentWriteTest.ts +++ b/tests/unit/onyxClearConcurrentWriteTest.ts @@ -92,6 +92,32 @@ describe('Write data while Onyx.clear is in progress', () => { await expect(StorageMock.getItem(`${ONYX_KEYS.COLLECTION.TEST}1`)).resolves.toBe(NEW_VALUE); }); + it('should keep a value written during the clear even when it equals the value from before the clear', async () => { + // Given keys that hold values from a previous session + await Onyx.multiSet({ + [ONYX_KEYS.DEFAULT_KEY]: OLD_VALUE, + [ONYX_KEYS.REGULAR_KEY]: OLD_VALUE, + }); + + // When the same values are written again while the clear is running, like a re-auth restoring unchanged session data + holdNextRemoveItems(); + const clearPromise = Onyx.clear(); + await waitForPromisesToResolve(); + Onyx.set(ONYX_KEYS.DEFAULT_KEY, OLD_VALUE); + Onyx.set(ONYX_KEYS.REGULAR_KEY, OLD_VALUE); + await waitForPromisesToResolve(); + finishRemoveItems(); + await clearPromise; + await waitForPromisesToResolve(); + + // Then the written values are kept, because they are new writes and not leftovers from before the clear + expect(defaultKeyValue).toBe(OLD_VALUE); + expect(regularKeyValue).toBe(OLD_VALUE); + expect(cache.get(ONYX_KEYS.DEFAULT_KEY)).toBe(OLD_VALUE); + await expect(StorageMock.getItem(ONYX_KEYS.DEFAULT_KEY)).resolves.toBe(OLD_VALUE); + await expect(StorageMock.getItem(ONYX_KEYS.REGULAR_KEY)).resolves.toBe(OLD_VALUE); + }); + it('should still reset keys that were not written during the clear', async () => { // Given keys that hold values from a previous session await Onyx.multiSet({