diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 42a3cd57c..20fc3d1cc 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -82,10 +82,13 @@ function init({ OnyxUtils.initStoreValues(keys, initialKeyStates, evictableKeys); - // Initialize all of our keys with data provided then give green light to any pending connections - Promise.all([cache.addEvictableKeysToRecentlyAccessedList(OnyxUtils.isCollectionKey, OnyxUtils.getAllKeys), OnyxUtils.initializeWithDefaultKeyStates()]).then( - OnyxUtils.getDeferredInitTask().resolve, - ); + // Initialize all of our keys with data provided then give green light to any pending connections. + // addEvictableKeysToRecentlyAccessedList must run after initializeWithDefaultKeyStates because + // eager cache loading populates the key index (cache.setAllKeys) inside initializeWithDefaultKeyStates, + // and the evictable keys list depends on that index being populated. + OnyxUtils.initializeWithDefaultKeyStates() + .then(() => cache.addEvictableKeysToRecentlyAccessedList(OnyxUtils.isCollectionKey, OnyxUtils.getAllKeys)) + .then(OnyxUtils.getDeferredInitTask().resolve); } /** diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index f203870da..1b31e83e2 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -100,7 +100,9 @@ let lastSubscriptionID = 0; // Connections can be made before `Onyx.init`. They would wait for this task before resolving const deferredInitTask = createDeferredTask(); -// Holds a set of collection member IDs which updates will be ignored when using Onyx methods. +// Collection member IDs that Onyx should silently ignore across all operations — reads, writes, cache, and subscriber +// notifications. This is used to filter out keys formed from invalid/default IDs (e.g. "-1", "0", +// "undefined", "null", "NaN") that can appear when an ID variable is accidentally coerced to string. let skippableCollectionMemberIDs = new Set(); // Holds a set of keys that should always be merged into snapshot entries. let snapshotMergeKeys = new Set(); @@ -1111,19 +1113,76 @@ function mergeInternal | undefined, TChange ex * Merge user provided default key value pairs. */ function initializeWithDefaultKeyStates(): Promise { - // Filter out RAM-only keys from storage reads as they may have stale persisted data - // from before the key was migrated to RAM-only. - const keysToFetch = Object.keys(defaultKeyStates).filter((key) => !isRamOnlyKey(key)); - return Storage.multiGet(keysToFetch).then((pairs) => { - const existingDataAsObject = Object.fromEntries(pairs) as Record; - - const merged = utils.fastMerge(existingDataAsObject, defaultKeyStates, { - shouldRemoveNestedNulls: true, - }).result; - cache.merge(merged ?? {}); - - for (const [key, value] of Object.entries(merged ?? {})) keyChanged(key, value); - }); + // Eagerly load the entire database into cache in a single batch read. + // This is faster than lazy-loading individual keys because: + // 1. One DB transaction instead of hundreds + // 2. All subsequent reads are synchronous cache hits + return Storage.getAll() + .then((pairs) => { + const allDataFromStorage: Record = {}; + for (const [key, value] of pairs) { + // RAM-only keys should not be cached from storage as they may have stale persisted data + // from before the key was migrated to RAM-only. + if (isRamOnlyKey(key)) { + continue; + } + + // Skip collection members that are marked as skippable + if (skippableCollectionMemberIDs.size && getCollectionKey(key)) { + const [, collectionMemberID] = splitCollectionMemberKey(key); + + if (skippableCollectionMemberIDs.has(collectionMemberID)) { + continue; + } + } + + allDataFromStorage[key] = value; + } + + // Load all storage data into cache silently (no subscriber notifications) + cache.setAllKeys(Object.keys(allDataFromStorage)); + cache.merge(allDataFromStorage); + + // For keys that have a developer-defined default (via `initialKeyStates`), merge the + // persisted value with the default so new properties added in code updates are applied + // without wiping user data that already exists in storage. + const defaultKeysFromStorage = Object.keys(defaultKeyStates).reduce((obj: Record, key) => { + if (key in allDataFromStorage) { + // eslint-disable-next-line no-param-reassign + obj[key] = allDataFromStorage[key]; + } + return obj; + }, {}); + + const merged = utils.fastMerge(defaultKeysFromStorage, defaultKeyStates, { + shouldRemoveNestedNulls: true, + }).result; + cache.merge(merged ?? {}); + + // Notify subscribers about default key states so that any subscriber that connected + // before init (e.g. during module load) receives the merged default values immediately + for (const [key, value] of Object.entries(merged ?? {})) { + keyChanged(key, value); + } + }) + .catch((error) => { + Logger.logAlert(`Failed to load data from storage during init. The app will boot with default key states only. Error: ${error}`); + + // Populate the key index so getAllKeys() returns correct results for default keys. + // Without this, subscribers that check getAllKeys() would see an empty set even + // though we have default values in cache. + cache.setAllKeys(Object.keys(defaultKeyStates)); + + // Boot with defaults so the app renders instead of deadlocking. + // Users will get a fresh-install experience but the app won't be bricked. + cache.merge(defaultKeyStates); + + // Notify subscribers about default key states so that any subscriber that connected + // before init (e.g. during module load) receives the merged default values immediately + for (const [key, value] of Object.entries(defaultKeyStates)) { + keyChanged(key, value); + } + }); } /** diff --git a/lib/storage/__mocks__/index.ts b/lib/storage/__mocks__/index.ts index b4fcc31bd..a4456a7e7 100644 --- a/lib/storage/__mocks__/index.ts +++ b/lib/storage/__mocks__/index.ts @@ -16,6 +16,7 @@ const StorageMock = { removeItems: jest.fn(MemoryOnlyProvider.removeItems), clear: jest.fn(MemoryOnlyProvider.clear), getAllKeys: jest.fn(MemoryOnlyProvider.getAllKeys), + getAll: jest.fn(MemoryOnlyProvider.getAll), getDatabaseSize: jest.fn(MemoryOnlyProvider.getDatabaseSize), keepInstancesSync: jest.fn(), diff --git a/lib/storage/index.ts b/lib/storage/index.ts index 07e7f7536..be45bc41a 100644 --- a/lib/storage/index.ts +++ b/lib/storage/index.ts @@ -187,6 +187,11 @@ const storage: Storage = { */ getAllKeys: () => tryOrDegradePerformance(() => provider.getAllKeys()), + /** + * Returns all key-value pairs from storage in a single batch operation + */ + getAll: () => tryOrDegradePerformance(() => provider.getAll()), + /** * Gets the total bytes of the store */ @@ -220,6 +225,7 @@ GlobalSettings.addGlobalSettingsChangeListener(({enablePerformanceMetrics}) => { storage.removeItems = decorateWithMetrics(storage.removeItems, 'Storage.removeItems'); storage.clear = decorateWithMetrics(storage.clear, 'Storage.clear'); storage.getAllKeys = decorateWithMetrics(storage.getAllKeys, 'Storage.getAllKeys'); + storage.getAll = decorateWithMetrics(storage.getAll, 'Storage.getAll'); }); export default storage; diff --git a/lib/storage/providers/IDBKeyValProvider/index.ts b/lib/storage/providers/IDBKeyValProvider/index.ts index c140ed763..89b5077b8 100644 --- a/lib/storage/providers/IDBKeyValProvider/index.ts +++ b/lib/storage/providers/IDBKeyValProvider/index.ts @@ -4,6 +4,7 @@ import utils from '../../../utils'; import type StorageProvider from '../types'; import type {OnyxKey, OnyxValue} from '../../../types'; import createStore from './createStore'; +import type {StorageKeyValuePair} from '../types'; const DB_NAME = 'OnyxDB'; const STORE_NAME = 'keyvaluepairs'; @@ -109,6 +110,13 @@ const provider: StorageProvider = { return IDB.keys(provider.store); }, + getAll() { + if (!provider.store) { + throw new Error('Store not initialized!'); + } + + return IDB.entries(provider.store) as Promise; + }, getItem(key) { if (!provider.store) { throw new Error('Store not initialized!'); diff --git a/lib/storage/providers/MemoryOnlyProvider.ts b/lib/storage/providers/MemoryOnlyProvider.ts index 15e9c264d..1f92425b6 100644 --- a/lib/storage/providers/MemoryOnlyProvider.ts +++ b/lib/storage/providers/MemoryOnlyProvider.ts @@ -135,6 +135,13 @@ const provider: StorageProvider = { return Promise.resolve(_.keys(provider.store)); }, + /** + * Returns all key-value pairs from memory + */ + getAll() { + return Promise.resolve(Object.entries(provider.store) as StorageKeyValuePair[]); + }, + /** * Gets the total bytes of the store. * `bytesRemaining` will always be `Number.POSITIVE_INFINITY` since we don't have a hard limit on memory. diff --git a/lib/storage/providers/NoopProvider.ts b/lib/storage/providers/NoopProvider.ts index dc3bcb2af..677826cfb 100644 --- a/lib/storage/providers/NoopProvider.ts +++ b/lib/storage/providers/NoopProvider.ts @@ -87,6 +87,13 @@ const provider: StorageProvider = { return Promise.resolve([]); }, + /** + * Returns all key-value pairs from storage + */ + getAll() { + return Promise.resolve([]); + }, + /** * Gets the total bytes of the store. * `bytesRemaining` will always be `Number.POSITIVE_INFINITY` since we don't have a hard limit on memory. diff --git a/lib/storage/providers/SQLiteProvider.ts b/lib/storage/providers/SQLiteProvider.ts index afbc0f405..61de2cf7d 100644 --- a/lib/storage/providers/SQLiteProvider.ts +++ b/lib/storage/providers/SQLiteProvider.ts @@ -198,6 +198,17 @@ const provider: StorageProvider = { return (result ?? []) as StorageKeyList; }); }, + getAll() { + if (!provider.store) { + throw new Error('Store is not initialized!'); + } + + return provider.store.executeAsync('SELECT record_key, valueJSON FROM keyvaluepairs;').then(({rows}) => { + // eslint-disable-next-line no-underscore-dangle + const result = rows?._array.map((row) => [row.record_key, JSON.parse(row.valueJSON)]); + return (result ?? []) as StorageKeyValuePair[]; + }); + }, removeItem(key) { if (!provider.store) { throw new Error('Store is not initialized!'); diff --git a/lib/storage/providers/types.ts b/lib/storage/providers/types.ts index ab275e39c..8bd4e17e2 100644 --- a/lib/storage/providers/types.ts +++ b/lib/storage/providers/types.ts @@ -60,6 +60,12 @@ type StorageProvider = { */ getAllKeys: () => Promise; + /** + * Returns all key-value pairs from storage in a single batch operation. + * More efficient than getAllKeys + multiGet for loading the entire database. + */ + getAll: () => Promise; + /** * Removes given key and its value from storage */ diff --git a/lib/types.ts b/lib/types.ts index 92396fffe..18cfa1a34 100644 --- a/lib/types.ts +++ b/lib/types.ts @@ -421,8 +421,9 @@ type InitOptions = { enableDevTools?: boolean; /** - * Array of collection member IDs which updates will be ignored when using Onyx methods. - * Additionally, any subscribers from these keys to won't receive any data from Onyx. + * Array of collection member IDs that Onyx should silently ignore across all operations. + * This prevents keys formed from invalid or default IDs (e.g. "-1", "0", "undefined") from + * polluting cache or triggering subscriber notifications. */ skippableCollectionMemberIDs?: string[]; diff --git a/tests/unit/onyxCacheTest.tsx b/tests/unit/onyxCacheTest.tsx index 5f2154287..1d1656dca 100644 --- a/tests/unit/onyxCacheTest.tsx +++ b/tests/unit/onyxCacheTest.tsx @@ -662,6 +662,88 @@ describe('Onyx', () => { }); }); + describe('eager loading during initialisation', () => { + beforeEach(() => { + StorageMock = require('../../lib/storage').default; + }); + + it('should load all storage data into cache during init', async () => { + await StorageMock.setItem(ONYX_KEYS.TEST_KEY, 'storageValue'); + await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1, name: 'Item 1'}); + await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}2`, {id: 2, name: 'Item 2'}); + await initOnyx(); + + expect(cache.getAllKeys().size).toBe(3); + expect(cache.get(ONYX_KEYS.TEST_KEY)).toBe('storageValue'); + expect(cache.get(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`)).toEqual({id: 1, name: 'Item 1'}); + expect(cache.get(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}2`)).toEqual({id: 2, name: 'Item 2'}); + }); + + it('should not load RAM-only keys from storage during init', async () => { + const testKeys = { + ...ONYX_KEYS, + RAM_ONLY_KEY: 'ramOnlyKey', + }; + + await StorageMock.setItem(testKeys.RAM_ONLY_KEY, 'staleValue'); + await StorageMock.setItem(ONYX_KEYS.TEST_KEY, 'normalValue'); + await initOnyx({keys: testKeys, ramOnlyKeys: [testKeys.RAM_ONLY_KEY]}); + + expect(cache.getAllKeys().size).toBe(1); + expect(cache.get(testKeys.RAM_ONLY_KEY)).toBeUndefined(); + expect(cache.get(ONYX_KEYS.TEST_KEY)).toBe('normalValue'); + }); + + it('should merge default key states with storage data during init', async () => { + await StorageMock.setItem(ONYX_KEYS.OTHER_TEST, {fromStorage: true}); + await initOnyx({ + initialKeyStates: { + [ONYX_KEYS.OTHER_TEST]: {fromDefault: true}, + }, + }); + + // Default key states are merged on top of storage data. + expect(cache.get(ONYX_KEYS.OTHER_TEST)).toEqual({fromStorage: true, fromDefault: true}); + }); + + it('should use default key states when storage data is not available for a key', async () => { + await StorageMock.clear(); + await initOnyx({ + initialKeyStates: { + [ONYX_KEYS.OTHER_TEST]: 42, + }, + }); + + expect(cache.get(ONYX_KEYS.OTHER_TEST)).toBe(42); + }); + + it('should gracefully handle Storage.getAll() failure and boot with defaults', async () => { + (StorageMock.getAll as jest.Mock).mockImplementationOnce(() => Promise.reject(new Error('Database corrupted'))); + + await initOnyx({ + initialKeyStates: { + [ONYX_KEYS.OTHER_TEST]: 42, + }, + }); + + expect(cache.getAllKeys().size).toBe(1); + expect(cache.get(ONYX_KEYS.OTHER_TEST)).toBe(42); + }); + + it('should populate cache key index with all storage keys during init', async () => { + await StorageMock.setItem(ONYX_KEYS.TEST_KEY, 'value1'); + await StorageMock.setItem(ONYX_KEYS.OTHER_TEST, 'value2'); + await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`, {id: 1}); + await initOnyx(); + + const allKeys = cache.getAllKeys(); + expect(allKeys.size).toBe(3); + expect(allKeys.has(ONYX_KEYS.TEST_KEY)).toBe(true); + expect(allKeys.has(ONYX_KEYS.OTHER_TEST)).toBe(true); + expect(allKeys.has(`${ONYX_KEYS.COLLECTION.MOCK_COLLECTION}1`)).toBe(true); + }); + }); + it('should save RAM-only keys', () => { const testKeys = { ...ONYX_KEYS,