From ecacaf52b0defcbc679be8e195f434d5820ec82f Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Wed, 28 Feb 2024 16:33:51 +0100 Subject: [PATCH 01/15] feat: fallback to InMemoryProvider --- lib/storage/providers/MemoryOnlyProvider.ts | 144 ++++++++++++++++++++ package-lock.json | 59 +++++++- package.json | 1 + 3 files changed, 198 insertions(+), 6 deletions(-) create mode 100644 lib/storage/providers/MemoryOnlyProvider.ts diff --git a/lib/storage/providers/MemoryOnlyProvider.ts b/lib/storage/providers/MemoryOnlyProvider.ts new file mode 100644 index 000000000..c319ee3b0 --- /dev/null +++ b/lib/storage/providers/MemoryOnlyProvider.ts @@ -0,0 +1,144 @@ +import _ from 'underscore'; +import sizeof from 'object-sizeof'; +import utils from '../../utils'; +import type StorageProvider from './types'; +import type {Key, KeyValuePair, Value} from './types'; + +type Store = Record; + +// eslint-disable-next-line import/no-mutable-exports +let store: Store = {}; + +const setInternal = (key: Key, value: Value) => { + store[key] = value; + return Promise.resolve(value); +}; + +const isJestRunning = typeof jest !== 'undefined'; +const set = isJestRunning ? jest.fn(setInternal) : setInternal; + +const provider: StorageProvider = { + /** + * The name of the provider that can be printed to the logs + */ + name: 'MemoryOnlyProvider', + + /** + * Initializes the storage provider + */ + init() { + // do nothing + }, + + /** + * Get the value of a given key or return `null` if it's not available in memory + */ + getItem(key) { + const value = store[key]; + + return Promise.resolve(value === undefined ? null : value); + }, + + /** + * Get multiple key-value pairs for the give array of keys in a batch. + */ + multiGet(keys) { + const getPromises = _.map(keys, (key) => new Promise((resolve) => this.getItem(key).then((value) => resolve([key, value])))) as Array>; + return Promise.all(getPromises); + }, + + /** + * Sets the value for a given key. The only requirement is that the value should be serializable to JSON string + */ + setItem(key, value) { + set(key, value); + + return Promise.resolve(); + }, + + /** + * Stores multiple key-value pairs in a batch + */ + multiSet(pairs) { + const setPromises = _.map(pairs, ([key, value]) => this.setItem(key, value)); + return new Promise((resolve) => Promise.all(setPromises).then(() => resolve())); + }, + + /** + * Merging an existing value with a new one + */ + mergeItem(key, _changes, modifiedData) { + // Since Onyx already merged the existing value with the changes, we can just set the value directly + return this.setItem(key, modifiedData); + }, + + /** + * Multiple merging of existing and new values in a batch + * This function also removes all nested null values from an object. + */ + multiMerge(pairs) { + _.forEach(pairs, ([key, value]) => { + const existingValue = store[key] as unknown as Record; + const newValue = utils.fastMerge(existingValue, value as unknown as Record) as unknown as Value; + + set(key, newValue); + }); + + return Promise.resolve([]); + }, + + /** + * Remove given key and it's value from memory + */ + removeItem(key) { + delete store[key]; + return Promise.resolve(); + }, + + /** + * Remove given keys and their values from memory + */ + removeItems(keys) { + _.each(keys, (key) => { + delete store[key]; + }); + return Promise.resolve(); + }, + + /** + * Clear everything from memory + */ + clear() { + store = {}; + return Promise.resolve(); + }, + + // This is a noop for now in order to keep clients from crashing see https://github.com/Expensify/Expensify/issues/312438 + setMemoryOnlyKeys() { + // do nothing + }, + + /** + * Returns all keys available in memory + */ + getAllKeys() { + return Promise.resolve(_.keys(store)); + }, + + /** + * Gets the total bytes of the store. + * `bytesRemaining` will always be `Number.POSITIVE_INFINITY` since we don't have a hard limit on memory. + */ + getDatabaseSize() { + const storeSize = sizeof(store); + + return Promise.resolve({bytesRemaining: Number.POSITIVE_INFINITY, bytesUsed: storeSize}); + }, +}; + +const setMockStore = (data: Store) => { + store = data; +}; + +export default provider; +export {store as mockStore, set as mockSet, setMockStore}; diff --git a/package-lock.json b/package-lock.json index de81e94af..54711b458 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,6 +11,7 @@ "dependencies": { "ascii-table": "0.0.9", "fast-equals": "^4.0.3", + "object-sizeof": "^2.6.4", "underscore": "^1.13.6" }, "devDependencies": { @@ -5268,7 +5269,6 @@ "version": "1.5.1", "resolved": "https://registry.npmjs.org/base64-js/-/base64-js-1.5.1.tgz", "integrity": "sha512-AKpaYlHn8t4SVbOHCy+b5+KKgvR4vrsD8vbvrbiQJps7fKDTkjkDry6ji0rUJjC0kzbNePLwzxq8iypo41qeWA==", - "dev": true, "funding": [ { "type": "github", @@ -9204,7 +9204,6 @@ "version": "1.2.1", "resolved": "https://registry.npmjs.org/ieee754/-/ieee754-1.2.1.tgz", "integrity": "sha512-dcyqhDvX1C46lXZcVqCpK+FtMRQVdIMN6/Df5js2zouUsqG7I6sFxitIC+7KYK29KdXOLHdu9zL4sFnoVQnqaA==", - "dev": true, "funding": [ { "type": "github", @@ -13405,6 +13404,37 @@ "node": ">= 0.4" } }, + "node_modules/object-sizeof": { + "version": "2.6.4", + "resolved": "https://registry.npmjs.org/object-sizeof/-/object-sizeof-2.6.4.tgz", + "integrity": "sha512-YuJAf7Bi61KROcYmXm8RCeBrBw8UOaJDzTm1gp0eU7RjYi1xEte3/Nmg/VyPaHcJZ3sNojs1Y0xvSrgwkLmcFw==", + "dependencies": { + "buffer": "^6.0.3" + } + }, + "node_modules/object-sizeof/node_modules/buffer": { + "version": "6.0.3", + "resolved": "https://registry.npmjs.org/buffer/-/buffer-6.0.3.tgz", + "integrity": "sha512-FTiCpNxtwiZZHEZbcbTIcZjERVICn9yq/pDFkTl95/AxzD1naBctN7YO68riM/gLSDY7sdrMby8hofADYuuqOA==", + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/feross" + }, + { + "type": "patreon", + "url": "https://www.patreon.com/feross" + }, + { + "type": "consulting", + "url": "https://feross.org/support" + } + ], + "dependencies": { + "base64-js": "^1.3.1", + "ieee754": "^1.2.1" + } + }, "node_modules/object-to-spawn-args": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/object-to-spawn-args/-/object-to-spawn-args-2.0.1.tgz", @@ -21694,8 +21724,7 @@ "base64-js": { "version": "1.5.1", "resolved": "https://registry.npmjs.org/base64-js/-/base64-js-1.5.1.tgz", - "integrity": "sha512-AKpaYlHn8t4SVbOHCy+b5+KKgvR4vrsD8vbvrbiQJps7fKDTkjkDry6ji0rUJjC0kzbNePLwzxq8iypo41qeWA==", - "dev": true + "integrity": "sha512-AKpaYlHn8t4SVbOHCy+b5+KKgvR4vrsD8vbvrbiQJps7fKDTkjkDry6ji0rUJjC0kzbNePLwzxq8iypo41qeWA==" }, "binary-extensions": { "version": "2.2.0", @@ -24741,8 +24770,7 @@ "ieee754": { "version": "1.2.1", "resolved": "https://registry.npmjs.org/ieee754/-/ieee754-1.2.1.tgz", - "integrity": "sha512-dcyqhDvX1C46lXZcVqCpK+FtMRQVdIMN6/Df5js2zouUsqG7I6sFxitIC+7KYK29KdXOLHdu9zL4sFnoVQnqaA==", - "dev": true + "integrity": "sha512-dcyqhDvX1C46lXZcVqCpK+FtMRQVdIMN6/Df5js2zouUsqG7I6sFxitIC+7KYK29KdXOLHdu9zL4sFnoVQnqaA==" }, "ignore": { "version": "4.0.6", @@ -28067,6 +28095,25 @@ "integrity": "sha512-NuAESUOUMrlIXOfHKzD6bpPu3tYt3xvjNdRIQ+FeT0lNb4K8WR70CaDxhuNguS2XG+GjkyMwOzsN5ZktImfhLA==", "dev": true }, + "object-sizeof": { + "version": "2.6.4", + "resolved": "https://registry.npmjs.org/object-sizeof/-/object-sizeof-2.6.4.tgz", + "integrity": "sha512-YuJAf7Bi61KROcYmXm8RCeBrBw8UOaJDzTm1gp0eU7RjYi1xEte3/Nmg/VyPaHcJZ3sNojs1Y0xvSrgwkLmcFw==", + "requires": { + "buffer": "^6.0.3" + }, + "dependencies": { + "buffer": { + "version": "6.0.3", + "resolved": "https://registry.npmjs.org/buffer/-/buffer-6.0.3.tgz", + "integrity": "sha512-FTiCpNxtwiZZHEZbcbTIcZjERVICn9yq/pDFkTl95/AxzD1naBctN7YO68riM/gLSDY7sdrMby8hofADYuuqOA==", + "requires": { + "base64-js": "^1.3.1", + "ieee754": "^1.2.1" + } + } + } + }, "object-to-spawn-args": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/object-to-spawn-args/-/object-to-spawn-args-2.0.1.tgz", diff --git a/package.json b/package.json index 273f19846..72e863412 100644 --- a/package.json +++ b/package.json @@ -40,6 +40,7 @@ "dependencies": { "ascii-table": "0.0.9", "fast-equals": "^4.0.3", + "object-sizeof": "^2.6.4", "underscore": "^1.13.6" }, "devDependencies": { From 865e1a8fe245eaf59f0ea310907984ad5ae0d406 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Thu, 29 Feb 2024 13:36:10 +0100 Subject: [PATCH 02/15] feat: use MemoryProvider in unit-tests --- lib/storage/__mocks__/index.ts | 106 ++++-------------- tests/unit/onyxMultiMergeWebStorageTest.js | 36 +++--- .../providers/IDBKeyvalProviderTest.js | 8 +- 3 files changed, 44 insertions(+), 106 deletions(-) diff --git a/lib/storage/__mocks__/index.ts b/lib/storage/__mocks__/index.ts index a88e6f953..8d279abb5 100644 --- a/lib/storage/__mocks__/index.ts +++ b/lib/storage/__mocks__/index.ts @@ -1,89 +1,27 @@ -import type {OnyxKey, OnyxValue} from '../../types'; -import utils from '../../utils'; -import type {KeyValuePairList} from '../providers/types'; -import type StorageProvider from '../providers/types'; +import MemoryOnlyProvider, {mockStore, mockSet, setMockStore} from '../providers/MemoryOnlyProvider'; -let storageMapInternal: Record> = {}; +const init = jest.fn(MemoryOnlyProvider.init); -const set = jest.fn((key, value) => { - storageMapInternal[key] = value; - return Promise.resolve(value); -}); +init(); -const idbKeyvalMock: StorageProvider = { - setItem(key, value) { - return set(key, value); - }, - multiSet(pairs) { - const setPromises = pairs.map(([key, value]) => this.setItem(key, value)); - return new Promise((resolve) => { - Promise.all(setPromises).then(() => resolve(storageMapInternal)); - }); - }, - getItem(key: TKey) { - return Promise.resolve(storageMapInternal[key] as OnyxValue); - }, - multiGet(keys) { - const getPromises = keys.map( - (key) => - new Promise((resolve) => { - this.getItem(key).then((value) => resolve([key, value])); - }), - ); - return Promise.all(getPromises) as Promise; - }, - multiMerge(pairs) { - pairs.forEach(([key, value]) => { - const existingValue = storageMapInternal[key]; - const newValue = utils.fastMerge(existingValue as Record, value as Record); - - set(key, newValue); - }); - - return Promise.resolve(storageMapInternal); - }, - mergeItem(key, _changes, modifiedData) { - return this.setItem(key, modifiedData); - }, - removeItem(key) { - delete storageMapInternal[key]; - return Promise.resolve(); - }, - removeItems(keys) { - keys.forEach((key) => { - delete storageMapInternal[key]; - }); - return Promise.resolve(); - }, - clear() { - storageMapInternal = {}; - return Promise.resolve(); - }, - getAllKeys() { - return Promise.resolve(Object.keys(storageMapInternal)); - }, - getDatabaseSize() { - return Promise.resolve({bytesRemaining: 0, bytesUsed: 99999}); - }, -}; - -const idbKeyvalMockSpy = { - idbKeyvalSet: set, - setItem: jest.fn(idbKeyvalMock.setItem), - getItem: jest.fn(idbKeyvalMock.getItem), - removeItem: jest.fn(idbKeyvalMock.removeItem), - removeItems: jest.fn(idbKeyvalMock.removeItems), - clear: jest.fn(idbKeyvalMock.clear), - getAllKeys: jest.fn(idbKeyvalMock.getAllKeys), - multiGet: jest.fn(idbKeyvalMock.multiGet), - multiSet: jest.fn(idbKeyvalMock.multiSet), - multiMerge: jest.fn(idbKeyvalMock.multiMerge), - mergeItem: jest.fn(idbKeyvalMock.mergeItem), - getStorageMap: jest.fn(() => storageMapInternal), - setInitialMockData: jest.fn((data) => { - storageMapInternal = data; - }), - getDatabaseSize: jest.fn(idbKeyvalMock.getDatabaseSize), +const StorageMock = { + init, + getItem: jest.fn(MemoryOnlyProvider.getItem), + multiGet: jest.fn(MemoryOnlyProvider.multiGet), + setItem: jest.fn(MemoryOnlyProvider.setItem), + multiSet: jest.fn(MemoryOnlyProvider.multiSet), + mergeItem: jest.fn(MemoryOnlyProvider.mergeItem), + multiMerge: jest.fn(MemoryOnlyProvider.multiMerge), + removeItem: jest.fn(MemoryOnlyProvider.removeItem), + removeItems: jest.fn(MemoryOnlyProvider.removeItems), + clear: jest.fn(MemoryOnlyProvider.clear), + setMemoryOnlyKeys: jest.fn(MemoryOnlyProvider.setMemoryOnlyKeys), + getAllKeys: jest.fn(MemoryOnlyProvider.getAllKeys), + getDatabaseSize: jest.fn(MemoryOnlyProvider.getDatabaseSize), + keepInstancesSync: jest.fn(), + mockSet, + getMockStore: jest.fn(() => mockStore), + setMockStore: jest.fn((data) => setMockStore(data)), }; -export default idbKeyvalMockSpy; +export default StorageMock; diff --git a/tests/unit/onyxMultiMergeWebStorageTest.js b/tests/unit/onyxMultiMergeWebStorageTest.js index 618b68d00..f1091bc6b 100644 --- a/tests/unit/onyxMultiMergeWebStorageTest.js +++ b/tests/unit/onyxMultiMergeWebStorageTest.js @@ -32,12 +32,12 @@ describe('Onyx.mergeCollection() and WebStorage', () => { afterEach(() => Onyx.clear()); it('merges two sets of data consecutively', () => { - StorageMock.setInitialMockData(initialData); + StorageMock.setMockStore(initialData); // Given initial data in storage - expect(StorageMock.getStorageMap().test_1).toEqual(initialTestObject); - expect(StorageMock.getStorageMap().test_2).toEqual(initialTestObject); - expect(StorageMock.getStorageMap().test_3).toEqual(initialTestObject); + expect(StorageMock.getMockStore().test_1).toEqual(initialTestObject); + expect(StorageMock.getMockStore().test_2).toEqual(initialTestObject); + expect(StorageMock.getMockStore().test_3).toEqual(initialTestObject); // And an empty cache values for the collection keys expect(OnyxCache.getValue('test_1')).not.toBeDefined(); @@ -75,17 +75,17 @@ describe('Onyx.mergeCollection() and WebStorage', () => { expect(OnyxCache.getValue('test_3')).toEqual(finalObject); // And the storage should reflect the same state - expect(StorageMock.getStorageMap().test_1).toEqual(finalObject); - expect(StorageMock.getStorageMap().test_2).toEqual(finalObject); - expect(StorageMock.getStorageMap().test_3).toEqual(finalObject); + expect(StorageMock.getMockStore().test_1).toEqual(finalObject); + expect(StorageMock.getMockStore().test_2).toEqual(finalObject); + expect(StorageMock.getMockStore().test_3).toEqual(finalObject); }); }); it('cache updates correctly when accessed again if keys are removed or evicted', () => { // Given empty storage - expect(StorageMock.getStorageMap().test_1).toBeFalsy(); - expect(StorageMock.getStorageMap().test_2).toBeFalsy(); - expect(StorageMock.getStorageMap().test_3).toBeFalsy(); + expect(StorageMock.getMockStore().test_1).toBeFalsy(); + expect(StorageMock.getMockStore().test_2).toBeFalsy(); + expect(StorageMock.getMockStore().test_3).toBeFalsy(); // And an empty cache values for the collection keys expect(OnyxCache.getValue('test_1')).toBeFalsy(); @@ -106,9 +106,9 @@ describe('Onyx.mergeCollection() and WebStorage', () => { expect(OnyxCache.getValue('test_1')).toEqual(data); expect(OnyxCache.getValue('test_2')).toEqual(data); expect(OnyxCache.getValue('test_3')).toEqual(data); - expect(StorageMock.getStorageMap().test_1).toEqual(data); - expect(StorageMock.getStorageMap().test_2).toEqual(data); - expect(StorageMock.getStorageMap().test_3).toEqual(data); + expect(StorageMock.getMockStore().test_1).toEqual(data); + expect(StorageMock.getMockStore().test_2).toEqual(data); + expect(StorageMock.getMockStore().test_3).toEqual(data); // When we drop all the cache keys (but do not modify the underlying storage) and merge another object OnyxCache.drop('test_1'); @@ -137,15 +137,15 @@ describe('Onyx.mergeCollection() and WebStorage', () => { expect(OnyxCache.getValue('test_3')).toEqual(finalObject); // And the storage should reflect the same state - expect(StorageMock.getStorageMap().test_1).toEqual(finalObject); - expect(StorageMock.getStorageMap().test_2).toEqual(finalObject); - expect(StorageMock.getStorageMap().test_3).toEqual(finalObject); + expect(StorageMock.getMockStore().test_1).toEqual(finalObject); + expect(StorageMock.getMockStore().test_2).toEqual(finalObject); + expect(StorageMock.getMockStore().test_3).toEqual(finalObject); }); }); it('setItem() and multiMerge()', () => { // Onyx should be empty after clear() is called - expect(StorageMock.getStorageMap()).toEqual({}); + expect(StorageMock.getMockStore()).toEqual({}); // Given no previous data and several calls to setItem and call to mergeCollection to update a given key @@ -174,7 +174,7 @@ describe('Onyx.mergeCollection() and WebStorage', () => { }; expect(OnyxCache.getValue('test_1')).toEqual(finalObject); - expect(StorageMock.getStorageMap().test_1).toEqual(finalObject); + expect(StorageMock.getMockStore().test_1).toEqual(finalObject); }); }); }); diff --git a/tests/unit/storage/providers/IDBKeyvalProviderTest.js b/tests/unit/storage/providers/IDBKeyvalProviderTest.js index ea4f5601a..50e053e5e 100644 --- a/tests/unit/storage/providers/IDBKeyvalProviderTest.js +++ b/tests/unit/storage/providers/IDBKeyvalProviderTest.js @@ -64,7 +64,7 @@ describe('storage/providers/IDBKeyVal', () => { ]); return waitForPromisesToResolve().then(() => { - IDBKeyValProviderMock.idbKeyvalSet.mockClear(); + IDBKeyValProviderMock.mockSet.mockClear(); // Given deltas matching existing structure const USER_1_DELTA = { @@ -83,7 +83,7 @@ describe('storage/providers/IDBKeyVal', () => { ['@USER_2', USER_2_DELTA], ]).then(() => { // Then each existing item should be set with the merged content - expect(IDBKeyValProviderMock.idbKeyvalSet).toHaveBeenNthCalledWith(1, '@USER_1', { + expect(IDBKeyValProviderMock.mockSet).toHaveBeenNthCalledWith(1, '@USER_1', { name: 'Tom', age: 31, traits: { @@ -92,7 +92,7 @@ describe('storage/providers/IDBKeyVal', () => { }, }); - expect(IDBKeyValProviderMock.idbKeyvalSet).toHaveBeenNthCalledWith(2, '@USER_2', { + expect(IDBKeyValProviderMock.mockSet).toHaveBeenNthCalledWith(2, '@USER_2', { name: 'Sarah', age: 26, traits: { @@ -131,7 +131,7 @@ describe('storage/providers/IDBKeyVal', () => { // If StorageProvider.clear() does not abort the queue, more idbKeyval.setItem calls would be executed because they would // be sitting in the setItemQueue return waitForPromisesToResolve().then(() => { - expect(IDBKeyValProviderMock.idbKeyvalSet).toHaveBeenCalledTimes(0); + expect(IDBKeyValProviderMock.mockSet).toHaveBeenCalledTimes(0); expect(IDBKeyValProviderMock.clear).toHaveBeenCalledTimes(1); }); }); From 4721c7a2ef412b6a193fef7d918d86f1eabad8ad Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Tue, 5 Mar 2024 13:23:57 +0100 Subject: [PATCH 03/15] docs: re-worded comment --- lib/storage/providers/MemoryOnlyProvider.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/storage/providers/MemoryOnlyProvider.ts b/lib/storage/providers/MemoryOnlyProvider.ts index c319ee3b0..04f317c0a 100644 --- a/lib/storage/providers/MemoryOnlyProvider.ts +++ b/lib/storage/providers/MemoryOnlyProvider.ts @@ -113,7 +113,9 @@ const provider: StorageProvider = { return Promise.resolve(); }, - // This is a noop for now in order to keep clients from crashing see https://github.com/Expensify/Expensify/issues/312438 + /* + * Since this is an in-memory only provider, nothing special needs to happen here and it can just be a noop + */ setMemoryOnlyKeys() { // do nothing }, From bee53ad1813a7cddd2188bddac8008ab2c29a3e2 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Wed, 6 Mar 2024 17:44:58 +0100 Subject: [PATCH 04/15] fix: removed sizeof --- lib/storage/providers/MemoryOnlyProvider.ts | 5 +- package-lock.json | 59 +++------------------ package.json | 1 - 3 files changed, 7 insertions(+), 58 deletions(-) diff --git a/lib/storage/providers/MemoryOnlyProvider.ts b/lib/storage/providers/MemoryOnlyProvider.ts index 04f317c0a..b62cefd7c 100644 --- a/lib/storage/providers/MemoryOnlyProvider.ts +++ b/lib/storage/providers/MemoryOnlyProvider.ts @@ -1,5 +1,4 @@ import _ from 'underscore'; -import sizeof from 'object-sizeof'; import utils from '../../utils'; import type StorageProvider from './types'; import type {Key, KeyValuePair, Value} from './types'; @@ -132,9 +131,7 @@ const provider: StorageProvider = { * `bytesRemaining` will always be `Number.POSITIVE_INFINITY` since we don't have a hard limit on memory. */ getDatabaseSize() { - const storeSize = sizeof(store); - - return Promise.resolve({bytesRemaining: Number.POSITIVE_INFINITY, bytesUsed: storeSize}); + return Promise.resolve({bytesRemaining: Number.POSITIVE_INFINITY, bytesUsed: 0}); }, }; diff --git a/package-lock.json b/package-lock.json index 54711b458..de81e94af 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,7 +11,6 @@ "dependencies": { "ascii-table": "0.0.9", "fast-equals": "^4.0.3", - "object-sizeof": "^2.6.4", "underscore": "^1.13.6" }, "devDependencies": { @@ -5269,6 +5268,7 @@ "version": "1.5.1", "resolved": "https://registry.npmjs.org/base64-js/-/base64-js-1.5.1.tgz", "integrity": "sha512-AKpaYlHn8t4SVbOHCy+b5+KKgvR4vrsD8vbvrbiQJps7fKDTkjkDry6ji0rUJjC0kzbNePLwzxq8iypo41qeWA==", + "dev": true, "funding": [ { "type": "github", @@ -9204,6 +9204,7 @@ "version": "1.2.1", "resolved": "https://registry.npmjs.org/ieee754/-/ieee754-1.2.1.tgz", "integrity": "sha512-dcyqhDvX1C46lXZcVqCpK+FtMRQVdIMN6/Df5js2zouUsqG7I6sFxitIC+7KYK29KdXOLHdu9zL4sFnoVQnqaA==", + "dev": true, "funding": [ { "type": "github", @@ -13404,37 +13405,6 @@ "node": ">= 0.4" } }, - "node_modules/object-sizeof": { - "version": "2.6.4", - "resolved": "https://registry.npmjs.org/object-sizeof/-/object-sizeof-2.6.4.tgz", - "integrity": "sha512-YuJAf7Bi61KROcYmXm8RCeBrBw8UOaJDzTm1gp0eU7RjYi1xEte3/Nmg/VyPaHcJZ3sNojs1Y0xvSrgwkLmcFw==", - "dependencies": { - "buffer": "^6.0.3" - } - }, - "node_modules/object-sizeof/node_modules/buffer": { - "version": "6.0.3", - "resolved": "https://registry.npmjs.org/buffer/-/buffer-6.0.3.tgz", - "integrity": "sha512-FTiCpNxtwiZZHEZbcbTIcZjERVICn9yq/pDFkTl95/AxzD1naBctN7YO68riM/gLSDY7sdrMby8hofADYuuqOA==", - "funding": [ - { - "type": "github", - "url": "https://github.com/sponsors/feross" - }, - { - "type": "patreon", - "url": "https://www.patreon.com/feross" - }, - { - "type": "consulting", - "url": "https://feross.org/support" - } - ], - "dependencies": { - "base64-js": "^1.3.1", - "ieee754": "^1.2.1" - } - }, "node_modules/object-to-spawn-args": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/object-to-spawn-args/-/object-to-spawn-args-2.0.1.tgz", @@ -21724,7 +21694,8 @@ "base64-js": { "version": "1.5.1", "resolved": "https://registry.npmjs.org/base64-js/-/base64-js-1.5.1.tgz", - "integrity": "sha512-AKpaYlHn8t4SVbOHCy+b5+KKgvR4vrsD8vbvrbiQJps7fKDTkjkDry6ji0rUJjC0kzbNePLwzxq8iypo41qeWA==" + "integrity": "sha512-AKpaYlHn8t4SVbOHCy+b5+KKgvR4vrsD8vbvrbiQJps7fKDTkjkDry6ji0rUJjC0kzbNePLwzxq8iypo41qeWA==", + "dev": true }, "binary-extensions": { "version": "2.2.0", @@ -24770,7 +24741,8 @@ "ieee754": { "version": "1.2.1", "resolved": "https://registry.npmjs.org/ieee754/-/ieee754-1.2.1.tgz", - "integrity": "sha512-dcyqhDvX1C46lXZcVqCpK+FtMRQVdIMN6/Df5js2zouUsqG7I6sFxitIC+7KYK29KdXOLHdu9zL4sFnoVQnqaA==" + "integrity": "sha512-dcyqhDvX1C46lXZcVqCpK+FtMRQVdIMN6/Df5js2zouUsqG7I6sFxitIC+7KYK29KdXOLHdu9zL4sFnoVQnqaA==", + "dev": true }, "ignore": { "version": "4.0.6", @@ -28095,25 +28067,6 @@ "integrity": "sha512-NuAESUOUMrlIXOfHKzD6bpPu3tYt3xvjNdRIQ+FeT0lNb4K8WR70CaDxhuNguS2XG+GjkyMwOzsN5ZktImfhLA==", "dev": true }, - "object-sizeof": { - "version": "2.6.4", - "resolved": "https://registry.npmjs.org/object-sizeof/-/object-sizeof-2.6.4.tgz", - "integrity": "sha512-YuJAf7Bi61KROcYmXm8RCeBrBw8UOaJDzTm1gp0eU7RjYi1xEte3/Nmg/VyPaHcJZ3sNojs1Y0xvSrgwkLmcFw==", - "requires": { - "buffer": "^6.0.3" - }, - "dependencies": { - "buffer": { - "version": "6.0.3", - "resolved": "https://registry.npmjs.org/buffer/-/buffer-6.0.3.tgz", - "integrity": "sha512-FTiCpNxtwiZZHEZbcbTIcZjERVICn9yq/pDFkTl95/AxzD1naBctN7YO68riM/gLSDY7sdrMby8hofADYuuqOA==", - "requires": { - "base64-js": "^1.3.1", - "ieee754": "^1.2.1" - } - } - } - }, "object-to-spawn-args": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/object-to-spawn-args/-/object-to-spawn-args-2.0.1.tgz", diff --git a/package.json b/package.json index 72e863412..273f19846 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,6 @@ "dependencies": { "ascii-table": "0.0.9", "fast-equals": "^4.0.3", - "object-sizeof": "^2.6.4", "underscore": "^1.13.6" }, "devDependencies": { From b85946538b66c37a8461528ec328ec3269d647fb Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Wed, 6 Mar 2024 17:48:33 +0100 Subject: [PATCH 05/15] Revert "Revert #475" This reverts commit 577e611d6b48c00abfe07e68837d373cf0a897f3. --- jestSetup.js | 6 +- lib/Onyx.ts | 18 ++- lib/storage/InstanceSync/index.ts | 16 ++ lib/storage/InstanceSync/index.web.ts | 66 +++++++++ lib/storage/NativeStorage.ts | 3 - lib/storage/WebStorage.ts | 74 ---------- lib/storage/index.native.ts | 3 - lib/storage/index.ts | 138 +++++++++++++++++- lib/storage/platforms/index.native.ts | 3 + lib/storage/platforms/index.ts | 3 + .../{IDBKeyVal.ts => IDBKeyValProvider.ts} | 38 +++-- .../{SQLiteStorage.ts => SQLiteProvider.ts} | 26 ++-- lib/storage/providers/types.ts | 6 +- .../providers/IDBKeyvalProviderTest.js | 2 +- .../storage/providers/StorageProviderTest.js | 10 +- 15 files changed, 286 insertions(+), 126 deletions(-) create mode 100644 lib/storage/InstanceSync/index.ts create mode 100644 lib/storage/InstanceSync/index.web.ts delete mode 100644 lib/storage/NativeStorage.ts delete mode 100644 lib/storage/WebStorage.ts delete mode 100644 lib/storage/index.native.ts create mode 100644 lib/storage/platforms/index.native.ts create mode 100644 lib/storage/platforms/index.ts rename lib/storage/providers/{IDBKeyVal.ts => IDBKeyValProvider.ts} (70%) rename lib/storage/providers/{SQLiteStorage.ts => SQLiteProvider.ts} (84%) diff --git a/jestSetup.js b/jestSetup.js index 2288af999..0a5ef85fb 100644 --- a/jestSetup.js +++ b/jestSetup.js @@ -1,7 +1,7 @@ jest.mock('./lib/storage'); -jest.mock('./lib/storage/NativeStorage', () => require('./lib/storage/__mocks__')); -jest.mock('./lib/storage/WebStorage', () => require('./lib/storage/__mocks__')); -jest.mock('./lib/storage/providers/IDBKeyVal', () => require('./lib/storage/__mocks__')); +jest.mock('./lib/storage/platforms/index.native', () => require('./lib/storage/__mocks__')); +jest.mock('./lib/storage/platforms/index', () => require('./lib/storage/__mocks__')); +jest.mock('./lib/storage/providers/IDBKeyValProvider', () => require('./lib/storage/__mocks__')); jest.mock('react-native-device-info', () => ({getFreeDiskStorage: () => {}})); jest.mock('react-native-quick-sqlite', () => ({ diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 3d3992532..507de756d 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -38,6 +38,16 @@ function init({ shouldSyncMultipleInstances = Boolean(global.localStorage), debugSetState = false, }: InitOptions): void { + Storage.init(); + + if (shouldSyncMultipleInstances) { + Storage.keepInstancesSync?.((key, value) => { + const prevValue = cache.getValue(key, false); + cache.set(key, value); + OnyxUtils.keyChanged(key, value, prevValue); + }); + } + if (debugSetState) { PerformanceUtils.setShouldDebugSetState(true); } @@ -50,14 +60,6 @@ function init({ // Initialize all of our keys with data provided then give green light to any pending connections Promise.all([OnyxUtils.addAllSafeEvictionKeysToRecentlyAccessedList(), OnyxUtils.initializeWithDefaultKeyStates()]).then(deferredInitTask.resolve); - - if (shouldSyncMultipleInstances) { - Storage.keepInstancesSync?.((key, value) => { - const prevValue = cache.getValue(key, false); - cache.set(key, value); - OnyxUtils.keyChanged(key, value, prevValue); - }); - } } /** diff --git a/lib/storage/InstanceSync/index.ts b/lib/storage/InstanceSync/index.ts new file mode 100644 index 000000000..327fbefdb --- /dev/null +++ b/lib/storage/InstanceSync/index.ts @@ -0,0 +1,16 @@ +import NOOP from 'lodash/noop'; + +/** + * This is used to keep multiple browser tabs in sync, therefore only needed on web + * On native platforms, we omit this syncing logic by setting this to mock implementation. + */ +const InstanceSync = { + init: NOOP, + setItem: NOOP, + removeItem: NOOP, + removeItems: NOOP, + mergeItem: NOOP, + clear: void>(callback: T) => Promise.resolve(callback()), +}; + +export default InstanceSync; diff --git a/lib/storage/InstanceSync/index.web.ts b/lib/storage/InstanceSync/index.web.ts new file mode 100644 index 000000000..a89cb54dc --- /dev/null +++ b/lib/storage/InstanceSync/index.web.ts @@ -0,0 +1,66 @@ +/* eslint-disable no-invalid-this */ +/** + * The InstancesSync object provides data-changed events like the ones that exist + * when using LocalStorage APIs in the browser. These events are great because multiple tabs can listen for when + * data changes and then stay up-to-date with everything happening in Onyx. + */ +import type {OnyxKey, OnyxValue} from '../../types'; +import type {KeyList, OnStorageKeyChanged} from '../providers/types'; + +const SYNC_ONYX = 'SYNC_ONYX'; + +/** + * Raise an event through `localStorage` to let other tabs know a value changed + * @param {String} onyxKey + */ +function raiseStorageSyncEvent(onyxKey: OnyxKey) { + global.localStorage.setItem(SYNC_ONYX, onyxKey); + global.localStorage.removeItem(SYNC_ONYX); +} + +function raiseStorageSyncManyKeysEvent(onyxKeys: KeyList) { + onyxKeys.forEach((onyxKey) => { + raiseStorageSyncEvent(onyxKey); + }); +} + +const InstanceSync = { + /** + * @param {Function} onStorageKeyChanged Storage synchronization mechanism keeping all opened tabs in sync + */ + init: (onStorageKeyChanged: OnStorageKeyChanged) => { + // This listener will only be triggered by events coming from other tabs + global.addEventListener('storage', (event) => { + // Ignore events that don't originate from the SYNC_ONYX logic + if (event.key !== SYNC_ONYX || !event.newValue) { + return; + } + + const onyxKey = event.newValue; + // @ts-expect-error `this` will be substituted later in actual function call + this.getItem(onyxKey).then((value: OnyxValue) => onStorageKeyChanged(onyxKey, value)); + }); + }, + setItem: raiseStorageSyncEvent, + removeItem: raiseStorageSyncEvent, + removeItems: raiseStorageSyncManyKeysEvent, + mergeItem: raiseStorageSyncEvent, + clear: (clearImplementation: () => void) => { + let allKeys: KeyList; + + // The keys must be retrieved before storage is cleared or else the list of keys would be empty + // @ts-expect-error `this` will be substituted later in actual function call + return this.getAllKeys() + .then((keys: KeyList) => { + allKeys = keys; + }) + .then(() => clearImplementation()) + .then(() => { + // Now that storage is cleared, the storage sync event can happen which is a more atomic action + // for other browser tabs + raiseStorageSyncManyKeysEvent(allKeys); + }); + }, +}; + +export default InstanceSync; diff --git a/lib/storage/NativeStorage.ts b/lib/storage/NativeStorage.ts deleted file mode 100644 index 1473613fa..000000000 --- a/lib/storage/NativeStorage.ts +++ /dev/null @@ -1,3 +0,0 @@ -import SQLiteStorage from './providers/SQLiteStorage'; - -export default SQLiteStorage; diff --git a/lib/storage/WebStorage.ts b/lib/storage/WebStorage.ts deleted file mode 100644 index 6621a084b..000000000 --- a/lib/storage/WebStorage.ts +++ /dev/null @@ -1,74 +0,0 @@ -/** - * This file is here to wrap IDBKeyVal with a layer that provides data-changed events like the ones that exist - * when using LocalStorage APIs in the browser. These events are great because multiple tabs can listen for when - * data changes and then stay up-to-date with everything happening in Onyx. - */ -import type {OnyxKey} from '../types'; -import Storage from './providers/IDBKeyVal'; -import type {KeyList} from './providers/types'; -import type StorageProvider from './providers/types'; - -const SYNC_ONYX = 'SYNC_ONYX'; - -/** - * Raise an event thorough `localStorage` to let other tabs know a value changed - */ -function raiseStorageSyncEvent(onyxKey: OnyxKey) { - global.localStorage.setItem(SYNC_ONYX, onyxKey); - global.localStorage.removeItem(SYNC_ONYX); -} - -function raiseStorageSyncManyKeysEvent(onyxKeys: KeyList) { - onyxKeys.forEach((onyxKey) => { - raiseStorageSyncEvent(onyxKey); - }); -} - -const webStorage: StorageProvider = { - ...Storage, - /** - * @param onStorageKeyChanged Storage synchronization mechanism keeping all opened tabs in sync - */ - keepInstancesSync(onStorageKeyChanged) { - // Override set, remove and clear to raise storage events that we intercept in other tabs - this.setItem = (key, value) => Storage.setItem(key, value).then(() => raiseStorageSyncEvent(key)); - - this.removeItem = (key) => Storage.removeItem(key).then(() => raiseStorageSyncEvent(key)); - - this.removeItems = (keys) => Storage.removeItems(keys).then(() => raiseStorageSyncManyKeysEvent(keys)); - - this.mergeItem = (key, batchedChanges, modifiedData) => Storage.mergeItem(key, batchedChanges, modifiedData).then(() => raiseStorageSyncEvent(key)); - - // If we just call Storage.clear other tabs will have no idea which keys were available previously - // so that they can call keysChanged for them. That's why we iterate over every key and raise a storage sync - // event for each one - this.clear = () => { - let allKeys: KeyList; - - // The keys must be retrieved before storage is cleared or else the list of keys would be empty - return Storage.getAllKeys() - .then((keys) => { - allKeys = keys; - }) - .then(() => Storage.clear()) - .then(() => { - // Now that storage is cleared, the storage sync event can happen which is a more atomic action - // for other browser tabs - allKeys.forEach(raiseStorageSyncEvent); - }); - }; - - // This listener will only be triggered by events coming from other tabs - global.addEventListener('storage', (event) => { - // Ignore events that don't originate from the SYNC_ONYX logic - if (event.key !== SYNC_ONYX || !event.newValue) { - return; - } - - const onyxKey = event.newValue; - Storage.getItem(onyxKey).then((value) => onStorageKeyChanged(onyxKey, value)); - }); - }, -}; - -export default webStorage; diff --git a/lib/storage/index.native.ts b/lib/storage/index.native.ts deleted file mode 100644 index 51b21ca5a..000000000 --- a/lib/storage/index.native.ts +++ /dev/null @@ -1,3 +0,0 @@ -import NativeStorage from './NativeStorage'; - -export default NativeStorage; diff --git a/lib/storage/index.ts b/lib/storage/index.ts index 4ee520d20..a38348fc2 100644 --- a/lib/storage/index.ts +++ b/lib/storage/index.ts @@ -1,3 +1,137 @@ -import WebStorage from './WebStorage'; +import PlatformStorage from './platforms'; +import InstanceSync from './InstanceSync'; +import type StorageProvider from './providers/types'; -export default WebStorage; +const provider = PlatformStorage; +let shouldKeepInstancesSync = false; + +type Storage = { + getStorageProvider: () => StorageProvider; +} & StorageProvider; + +const Storage: Storage = { + /** + * Returns the storage provider currently in use + */ + getStorageProvider() { + return provider; + }, + + /** + * Initializes all providers in the list of storage providers + * and enables fallback providers if necessary + */ + init() { + provider.init(); + }, + + /** + * Get the value of a given key or return `null` if it's not available + */ + getItem: (key) => provider.getItem(key), + + /** + * Get multiple key-value pairs for the give array of keys in a batch + */ + multiGet: (keys) => provider.multiGet(keys), + + /** + * Sets the value for a given key. The only requirement is that the value should be serializable to JSON string + */ + setItem: (key, value) => { + const promise = provider.setItem(key, value); + + if (shouldKeepInstancesSync) { + return promise.then(() => InstanceSync.setItem(key)); + } + + return promise; + }, + + /** + * Stores multiple key-value pairs in a batch + */ + multiSet: (pairs) => provider.multiSet(pairs), + + /** + * Merging an existing value with a new one + */ + mergeItem: (key, changes, modifiedData) => { + const promise = provider.mergeItem(key, changes, modifiedData); + + if (shouldKeepInstancesSync) { + return promise.then(() => InstanceSync.mergeItem(key)); + } + + return promise; + }, + + /** + * Multiple merging of existing and new values in a batch + * This function also removes all nested null values from an object. + */ + multiMerge: (pairs) => provider.multiMerge(pairs), + + /** + * Removes given key and its value + */ + removeItem: (key) => { + const promise = provider.removeItem(key); + + if (shouldKeepInstancesSync) { + return promise.then(() => InstanceSync.removeItem(key)); + } + + return promise; + }, + + /** + * Remove given keys and their values + */ + removeItems: (keys) => { + const promise = provider.removeItems(keys); + + if (shouldKeepInstancesSync) { + return promise.then(() => InstanceSync.removeItems(keys)); + } + + return promise; + }, + + /** + * Clears everything + */ + clear: () => { + if (shouldKeepInstancesSync) { + return InstanceSync.clear(() => provider.clear()); + } + + return provider.clear(); + }, + + // This is a noop for now in order to keep clients from crashing see https://github.com/Expensify/Expensify/issues/312438 + setMemoryOnlyKeys: () => provider.setMemoryOnlyKeys(), + + /** + * Returns all available keys + */ + getAllKeys: () => provider.getAllKeys(), + + /** + * Gets the total bytes of the store + */ + getDatabaseSize: () => provider.getDatabaseSize(), + + /** + * @param onStorageKeyChanged - Storage synchronization mechanism keeping all opened tabs in sync (web only) + */ + keepInstancesSync(onStorageKeyChanged) { + // If InstanceSync is null, it means we're on a native platform and we don't need to keep instances in sync + if (InstanceSync == null) return; + + shouldKeepInstancesSync = true; + InstanceSync.init(onStorageKeyChanged); + }, +}; + +export default Storage; diff --git a/lib/storage/platforms/index.native.ts b/lib/storage/platforms/index.native.ts new file mode 100644 index 000000000..95822c4a5 --- /dev/null +++ b/lib/storage/platforms/index.native.ts @@ -0,0 +1,3 @@ +import NativeStorage from '../providers/SQLiteProvider'; + +export default NativeStorage; diff --git a/lib/storage/platforms/index.ts b/lib/storage/platforms/index.ts new file mode 100644 index 000000000..0b95dc97d --- /dev/null +++ b/lib/storage/platforms/index.ts @@ -0,0 +1,3 @@ +import WebStorage from '../providers/IDBKeyValProvider'; + +export default WebStorage; diff --git a/lib/storage/providers/IDBKeyVal.ts b/lib/storage/providers/IDBKeyValProvider.ts similarity index 70% rename from lib/storage/providers/IDBKeyVal.ts rename to lib/storage/providers/IDBKeyValProvider.ts index 6312d0e3e..340e1ad04 100644 --- a/lib/storage/providers/IDBKeyVal.ts +++ b/lib/storage/providers/IDBKeyValProvider.ts @@ -6,19 +6,23 @@ import type {OnyxKey, OnyxValue} from '../../types'; // We don't want to initialize the store while the JS bundle loads as idb-keyval will try to use global.indexedDB // which might not be available in certain environments that load the bundle (e.g. electron main process). -let customStoreInstance: UseStore; -function getCustomStore(): UseStore { - if (!customStoreInstance) { - customStoreInstance = createStore('OnyxDB', 'keyvaluepairs'); - } - return customStoreInstance; -} +let idbKeyValStore: UseStore; const provider: StorageProvider = { - setItem: (key, value) => set(key, value, getCustomStore()), - multiGet: (keysParam) => getMany(keysParam, getCustomStore()).then((values) => values.map((value, index) => [keysParam[index], value])), + /** + * Initializes the storage provider + */ + init() { + const newIdbKeyValStore = createStore('OnyxDB', 'keyvaluepairs'); + + if (newIdbKeyValStore == null) throw Error('IDBKeyVal store could not be created'); + + idbKeyValStore = newIdbKeyValStore; + }, + setItem: (key, value) => set(key, value, idbKeyValStore), + multiGet: (keysParam) => getMany(keysParam, idbKeyValStore).then((values) => values.map((value, index) => [keysParam[index], value])), multiMerge: (pairs) => - getCustomStore()('readwrite', (store) => { + idbKeyValStore('readwrite', (store) => { // Note: we are using the manual store transaction here, to fit the read and update // of the items in one transaction to achieve best performance. const getValues = Promise.all(pairs.map(([key]) => promisifyRequest>(store.get(key)))); @@ -36,15 +40,17 @@ const provider: StorageProvider = { // Since Onyx also merged the existing value with the changes, we can just set the value directly return provider.setItem(key, modifiedData); }, - multiSet: (pairs) => setMany(pairs, getCustomStore()), - clear: () => clear(getCustomStore()), - getAllKeys: () => keys(getCustomStore()), + multiSet: (pairs) => setMany(pairs, idbKeyValStore), + clear: () => clear(idbKeyValStore), + // eslint-disable-next-line @typescript-eslint/no-empty-function + setMemoryOnlyKeys: () => {}, + getAllKeys: () => keys(idbKeyValStore), getItem: (key) => - get(key, getCustomStore()) + get(key, idbKeyValStore) // idb-keyval returns undefined for missing items, but this needs to return null so that idb-keyval does the same thing as SQLiteStorage. .then((val) => (val === undefined ? null : val)), - removeItem: (key) => del(key, getCustomStore()), - removeItems: (keysParam) => delMany(keysParam, getCustomStore()), + removeItem: (key) => del(key, idbKeyValStore), + removeItems: (keysParam) => delMany(keysParam, idbKeyValStore), getDatabaseSize() { if (!window.navigator || !window.navigator.storage) { throw new Error('StorageManager browser API unavailable'); diff --git a/lib/storage/providers/SQLiteStorage.ts b/lib/storage/providers/SQLiteProvider.ts similarity index 84% rename from lib/storage/providers/SQLiteStorage.ts rename to lib/storage/providers/SQLiteProvider.ts index 36eb36227..4b93e821c 100644 --- a/lib/storage/providers/SQLiteStorage.ts +++ b/lib/storage/providers/SQLiteProvider.ts @@ -2,7 +2,7 @@ * The SQLiteStorage provider stores everything in a key/value store by * converting the value to a JSON string */ -import type {BatchQueryResult} from 'react-native-quick-sqlite'; +import type {BatchQueryResult, QuickSQLiteConnection} from 'react-native-quick-sqlite'; import {open} from 'react-native-quick-sqlite'; import {getFreeDiskStorage} from 'react-native-device-info'; import type StorageProvider from './types'; @@ -10,17 +10,23 @@ import utils from '../../utils'; import type {KeyList, KeyValuePairList} from './types'; const DB_NAME = 'OnyxDB'; -const db = open({name: DB_NAME}); +let db: QuickSQLiteConnection; -db.execute('CREATE TABLE IF NOT EXISTS keyvaluepairs (record_key TEXT NOT NULL PRIMARY KEY , valueJSON JSON NOT NULL) WITHOUT ROWID;'); +const provider: StorageProvider = { + /** + * Initializes the storage provider + */ + init() { + db = open({name: DB_NAME}); -// All of the 3 pragmas below were suggested by SQLite team. -// You can find more info about them here: https://www.sqlite.org/pragma.html -db.execute('PRAGMA CACHE_SIZE=-20000;'); -db.execute('PRAGMA synchronous=NORMAL;'); -db.execute('PRAGMA journal_mode=WAL;'); + db.execute('CREATE TABLE IF NOT EXISTS keyvaluepairs (record_key TEXT NOT NULL PRIMARY KEY , valueJSON JSON NOT NULL) WITHOUT ROWID;'); -const provider: StorageProvider = { + // All of the 3 pragmas below were suggested by SQLite team. + // You can find more info about them here: https://www.sqlite.org/pragma.html + db.execute('PRAGMA CACHE_SIZE=-20000;'); + db.execute('PRAGMA synchronous=NORMAL;'); + db.execute('PRAGMA journal_mode=WAL;'); + }, getItem(key) { return db.executeAsync('SELECT record_key, valueJSON FROM keyvaluepairs WHERE record_key = ?;', [key]).then(({rows}) => { if (!rows || rows?.length === 0) { @@ -94,7 +100,7 @@ const provider: StorageProvider = { }, // eslint-disable-next-line @typescript-eslint/no-empty-function - keepInstancesSync: () => {}, + setMemoryOnlyKeys: () => {}, }; export default provider; diff --git a/lib/storage/providers/types.ts b/lib/storage/providers/types.ts index 353190f09..801f2185f 100644 --- a/lib/storage/providers/types.ts +++ b/lib/storage/providers/types.ts @@ -8,6 +8,10 @@ type KeyValuePairList = KeyValuePair[]; type OnStorageKeyChanged = (key: TKey, value: OnyxValue | null) => void; type StorageProvider = { + /** + * Initializes the storage provider + */ + init: () => void; /** * Gets the value of a given key or return `null` if it's not available in storage */ @@ -72,4 +76,4 @@ type StorageProvider = { }; export default StorageProvider; -export type {KeyList, KeyValuePair, KeyValuePairList}; +export type {KeyList, KeyValuePair, KeyValuePairList, OnStorageKeyChanged}; diff --git a/tests/unit/storage/providers/IDBKeyvalProviderTest.js b/tests/unit/storage/providers/IDBKeyvalProviderTest.js index 50e053e5e..c511cd0a3 100644 --- a/tests/unit/storage/providers/IDBKeyvalProviderTest.js +++ b/tests/unit/storage/providers/IDBKeyvalProviderTest.js @@ -1,6 +1,6 @@ import _ from 'underscore'; -import IDBKeyValProviderMock from '../../../../lib/storage/providers/IDBKeyVal'; +import IDBKeyValProviderMock from '../../../../lib/storage/providers/IDBKeyValProvider'; import createDeferredTask from '../../../../lib/createDeferredTask'; import waitForPromisesToResolve from '../../../utils/waitForPromisesToResolve'; diff --git a/tests/unit/storage/providers/StorageProviderTest.js b/tests/unit/storage/providers/StorageProviderTest.js index 82aca46b5..5ce43cfc9 100644 --- a/tests/unit/storage/providers/StorageProviderTest.js +++ b/tests/unit/storage/providers/StorageProviderTest.js @@ -1,11 +1,11 @@ /* eslint-disable import/first */ -jest.unmock('../../../../lib/storage/NativeStorage'); -jest.unmock('../../../../lib/storage/WebStorage'); -jest.unmock('../../../../lib/storage/providers/IDBKeyVal'); +jest.unmock('../../../../lib/storage/platforms/index.native'); +jest.unmock('../../../../lib/storage/platforms/index'); +jest.unmock('../../../../lib/storage/providers/IDBKeyValProvider'); import _ from 'underscore'; -import NativeStorage from '../../../../lib/storage/NativeStorage'; -import WebStorage from '../../../../lib/storage/WebStorage'; +import NativeStorage from '../../../../lib/storage/platforms/index.native'; +import WebStorage from '../../../../lib/storage/platforms/index'; it('storage providers have same methods implemented', () => { const nativeMethods = _.keys(NativeStorage); From b681f58419d3ab78ea5e5c477dd7f11ebec6021c Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Wed, 6 Mar 2024 17:48:54 +0100 Subject: [PATCH 06/15] Revert "Revert "feat: fallback to NoopProvider if we run into OOM [2/3]"" This reverts commit 651540ba1d603ac8d537c37cb4e63fbb4a188697. --- lib/storage/index.ts | 136 ++++++++++++++------- lib/storage/providers/IDBKeyValProvider.ts | 4 + lib/storage/providers/NoopProvider.ts | 103 ++++++++++++++++ lib/storage/providers/SQLiteProvider.ts | 4 + lib/storage/providers/types.ts | 4 + 5 files changed, 206 insertions(+), 45 deletions(-) create mode 100644 lib/storage/providers/NoopProvider.ts diff --git a/lib/storage/index.ts b/lib/storage/index.ts index a38348fc2..7090cd6ce 100644 --- a/lib/storage/index.ts +++ b/lib/storage/index.ts @@ -1,13 +1,52 @@ +import * as Logger from '../Logger'; + import PlatformStorage from './platforms'; import InstanceSync from './InstanceSync'; +import NoopProvider from './providers/NoopProvider'; import type StorageProvider from './providers/types'; -const provider = PlatformStorage; +let provider = PlatformStorage; let shouldKeepInstancesSync = false; +let finishInitalization: (value?: unknown) => void; +const initPromise = new Promise((resolve) => { + finishInitalization = resolve; +}); type Storage = { getStorageProvider: () => StorageProvider; -} & StorageProvider; +} & Omit; + +/** + * Degrade performance by removing the storage provider and only using cache + */ +function degradePerformance(error: Error) { + Logger.logAlert(`Error while using ${provider.name}. Falling back to only using cache and dropping storage.`); + console.error(error); + provider = NoopProvider; +} + +/** + * Runs a piece of code and degrades performance if certain errors are thrown + */ +function tryOrDegradePerformance(fn: () => Promise | T): Promise { + return new Promise((resolve, reject) => { + initPromise.then(() => { + try { + resolve(fn()); + } catch (error) { + // Test for known critical errors that the storage provider throws, e.g. when storage is full + if (error instanceof Error) { + // IndexedDB error when storage is full (https://github.com/Expensify/App/issues/29403) + if (error.message.includes('Internal error opening backing store for indexedDB.open')) { + degradePerformance(error); + } + } + + reject(error); + } + }); + }); +} const Storage: Storage = { /** @@ -22,112 +61,119 @@ const Storage: Storage = { * and enables fallback providers if necessary */ init() { - provider.init(); + tryOrDegradePerformance(provider.init).finally(() => { + finishInitalization(); + }); }, /** * Get the value of a given key or return `null` if it's not available */ - getItem: (key) => provider.getItem(key), + getItem: (key) => tryOrDegradePerformance(() => provider.getItem(key)), /** * Get multiple key-value pairs for the give array of keys in a batch */ - multiGet: (keys) => provider.multiGet(keys), + multiGet: (keys) => tryOrDegradePerformance(() => provider.multiGet(keys)), /** * Sets the value for a given key. The only requirement is that the value should be serializable to JSON string */ - setItem: (key, value) => { - const promise = provider.setItem(key, value); + setItem: (key, value) => + tryOrDegradePerformance(() => { + const promise = provider.setItem(key, value); - if (shouldKeepInstancesSync) { - return promise.then(() => InstanceSync.setItem(key)); - } + if (shouldKeepInstancesSync) { + return promise.then(() => InstanceSync.setItem(key)); + } - return promise; - }, + return promise; + }), /** * Stores multiple key-value pairs in a batch */ - multiSet: (pairs) => provider.multiSet(pairs), + multiSet: (pairs) => tryOrDegradePerformance(() => provider.multiSet(pairs)), /** * Merging an existing value with a new one */ - mergeItem: (key, changes, modifiedData) => { - const promise = provider.mergeItem(key, changes, modifiedData); + mergeItem: (key, changes, modifiedData) => + tryOrDegradePerformance(() => { + const promise = provider.mergeItem(key, changes, modifiedData); - if (shouldKeepInstancesSync) { - return promise.then(() => InstanceSync.mergeItem(key)); - } + if (shouldKeepInstancesSync) { + return promise.then(() => InstanceSync.mergeItem(key)); + } - return promise; - }, + return promise; + }), /** * Multiple merging of existing and new values in a batch * This function also removes all nested null values from an object. */ - multiMerge: (pairs) => provider.multiMerge(pairs), + multiMerge: (pairs) => tryOrDegradePerformance(() => provider.multiMerge(pairs)), /** * Removes given key and its value */ - removeItem: (key) => { - const promise = provider.removeItem(key); + removeItem: (key) => + tryOrDegradePerformance(() => { + const promise = provider.removeItem(key); - if (shouldKeepInstancesSync) { - return promise.then(() => InstanceSync.removeItem(key)); - } + if (shouldKeepInstancesSync) { + return promise.then(() => InstanceSync.removeItem(key)); + } - return promise; - }, + return promise; + }), /** * Remove given keys and their values */ - removeItems: (keys) => { - const promise = provider.removeItems(keys); + removeItems: (keys) => + tryOrDegradePerformance(() => { + const promise = provider.removeItems(keys); - if (shouldKeepInstancesSync) { - return promise.then(() => InstanceSync.removeItems(keys)); - } + if (shouldKeepInstancesSync) { + return promise.then(() => InstanceSync.removeItems(keys)); + } - return promise; - }, + return promise; + }), /** * Clears everything */ - clear: () => { - if (shouldKeepInstancesSync) { - return InstanceSync.clear(() => provider.clear()); - } + clear: () => + tryOrDegradePerformance(() => { + if (shouldKeepInstancesSync) { + return InstanceSync.clear(() => provider.clear()); + } - return provider.clear(); - }, + return provider.clear(); + }), // This is a noop for now in order to keep clients from crashing see https://github.com/Expensify/Expensify/issues/312438 - setMemoryOnlyKeys: () => provider.setMemoryOnlyKeys(), + setMemoryOnlyKeys: () => tryOrDegradePerformance(() => provider.setMemoryOnlyKeys()), /** * Returns all available keys */ - getAllKeys: () => provider.getAllKeys(), + getAllKeys: () => tryOrDegradePerformance(() => provider.getAllKeys()), /** * Gets the total bytes of the store */ - getDatabaseSize: () => provider.getDatabaseSize(), + getDatabaseSize: () => tryOrDegradePerformance(() => provider.getDatabaseSize()), /** * @param onStorageKeyChanged - Storage synchronization mechanism keeping all opened tabs in sync (web only) */ keepInstancesSync(onStorageKeyChanged) { // If InstanceSync is null, it means we're on a native platform and we don't need to keep instances in sync - if (InstanceSync == null) return; + if (InstanceSync === null) return; shouldKeepInstancesSync = true; InstanceSync.init(onStorageKeyChanged); diff --git a/lib/storage/providers/IDBKeyValProvider.ts b/lib/storage/providers/IDBKeyValProvider.ts index 340e1ad04..1d2e8ce39 100644 --- a/lib/storage/providers/IDBKeyValProvider.ts +++ b/lib/storage/providers/IDBKeyValProvider.ts @@ -9,6 +9,10 @@ import type {OnyxKey, OnyxValue} from '../../types'; let idbKeyValStore: UseStore; const provider: StorageProvider = { + /** + * The name of the provider that can be printed to the logs + */ + name: 'IDBKeyValProvider', /** * Initializes the storage provider */ diff --git a/lib/storage/providers/NoopProvider.ts b/lib/storage/providers/NoopProvider.ts new file mode 100644 index 000000000..06d05e65d --- /dev/null +++ b/lib/storage/providers/NoopProvider.ts @@ -0,0 +1,103 @@ +import type StorageProvider from './types'; + +const provider: StorageProvider = { + /** + * The name of the provider that can be printed to the logs + */ + name: 'NoopProvider', + + /** + * Initializes the storage provider + */ + init() { + // do nothing + }, + + /** + * Get the value of a given key or return `null` if it's not available in memory + * @param {String} key + * @return {Promise<*>} + */ + getItem() { + return Promise.resolve(null); + }, + + /** + * Get multiple key-value pairs for the give array of keys in a batch. + */ + multiGet() { + return Promise.resolve([]); + }, + + /** + * Sets the value for a given key. The only requirement is that the value should be serializable to JSON string + */ + setItem() { + return Promise.resolve(); + }, + + /** + * Stores multiple key-value pairs in a batch + */ + multiSet() { + return Promise.resolve(); + }, + + /** + * Merging an existing value with a new one + */ + mergeItem() { + return Promise.resolve(); + }, + + /** + * Multiple merging of existing and new values in a batch + * This function also removes all nested null values from an object. + */ + multiMerge() { + return Promise.resolve([]); + }, + + /** + * Remove given key and it's value from memory + */ + removeItem() { + return Promise.resolve(); + }, + + /** + * Remove given keys and their values from memory + */ + removeItems() { + return Promise.resolve(); + }, + + /** + * Clear everything from memory + */ + clear() { + return Promise.resolve(); + }, + + // This is a noop for now in order to keep clients from crashing see https://github.com/Expensify/Expensify/issues/312438 + setMemoryOnlyKeys() { + // do nothing + }, + + /** + * Returns all keys available in memory + */ + getAllKeys() { + 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. + */ + getDatabaseSize() { + return Promise.resolve({bytesRemaining: Number.POSITIVE_INFINITY, bytesUsed: 0}); + }, +}; + +export default provider; diff --git a/lib/storage/providers/SQLiteProvider.ts b/lib/storage/providers/SQLiteProvider.ts index 4b93e821c..1a86088bf 100644 --- a/lib/storage/providers/SQLiteProvider.ts +++ b/lib/storage/providers/SQLiteProvider.ts @@ -13,6 +13,10 @@ const DB_NAME = 'OnyxDB'; let db: QuickSQLiteConnection; const provider: StorageProvider = { + /** + * The name of the provider that can be printed to the logs + */ + name: 'SQLiteProvider', /** * Initializes the storage provider */ diff --git a/lib/storage/providers/types.ts b/lib/storage/providers/types.ts index 801f2185f..0472c4ff5 100644 --- a/lib/storage/providers/types.ts +++ b/lib/storage/providers/types.ts @@ -8,6 +8,10 @@ type KeyValuePairList = KeyValuePair[]; type OnStorageKeyChanged = (key: TKey, value: OnyxValue | null) => void; type StorageProvider = { + /** + * The name of the provider that can be printed to the logs + */ + name: string; /** * Initializes the storage provider */ From 7aa57c27bccca6e150449997624bae60757473b6 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Wed, 6 Mar 2024 17:59:58 +0100 Subject: [PATCH 07/15] fix: crash on storage synchronization attempt --- lib/storage/InstanceSync/index.web.ts | 19 ++++++++++++------- lib/storage/index.ts | 2 +- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/storage/InstanceSync/index.web.ts b/lib/storage/InstanceSync/index.web.ts index a89cb54dc..8327ef110 100644 --- a/lib/storage/InstanceSync/index.web.ts +++ b/lib/storage/InstanceSync/index.web.ts @@ -1,11 +1,12 @@ -/* eslint-disable no-invalid-this */ /** * The InstancesSync object provides data-changed events like the ones that exist * when using LocalStorage APIs in the browser. These events are great because multiple tabs can listen for when * data changes and then stay up-to-date with everything happening in Onyx. */ -import type {OnyxKey, OnyxValue} from '../../types'; +import type {OnyxKey} from '../../types'; +import NoopProvider from '../providers/NoopProvider'; import type {KeyList, OnStorageKeyChanged} from '../providers/types'; +import StorageProvider from '../providers/types'; const SYNC_ONYX = 'SYNC_ONYX'; @@ -24,11 +25,15 @@ function raiseStorageSyncManyKeysEvent(onyxKeys: KeyList) { }); } +let storage = NoopProvider; + const InstanceSync = { /** * @param {Function} onStorageKeyChanged Storage synchronization mechanism keeping all opened tabs in sync */ - init: (onStorageKeyChanged: OnStorageKeyChanged) => { + init: (onStorageKeyChanged: OnStorageKeyChanged, store: StorageProvider) => { + storage = store; + // This listener will only be triggered by events coming from other tabs global.addEventListener('storage', (event) => { // Ignore events that don't originate from the SYNC_ONYX logic @@ -37,8 +42,8 @@ const InstanceSync = { } const onyxKey = event.newValue; - // @ts-expect-error `this` will be substituted later in actual function call - this.getItem(onyxKey).then((value: OnyxValue) => onStorageKeyChanged(onyxKey, value)); + + storage.getItem(onyxKey).then((value) => onStorageKeyChanged(onyxKey, value)); }); }, setItem: raiseStorageSyncEvent, @@ -49,8 +54,8 @@ const InstanceSync = { let allKeys: KeyList; // The keys must be retrieved before storage is cleared or else the list of keys would be empty - // @ts-expect-error `this` will be substituted later in actual function call - return this.getAllKeys() + return storage + .getAllKeys() .then((keys: KeyList) => { allKeys = keys; }) diff --git a/lib/storage/index.ts b/lib/storage/index.ts index 7090cd6ce..836d3efba 100644 --- a/lib/storage/index.ts +++ b/lib/storage/index.ts @@ -176,7 +176,7 @@ const Storage: Storage = { if (InstanceSync === null) return; shouldKeepInstancesSync = true; - InstanceSync.init(onStorageKeyChanged); + InstanceSync.init(onStorageKeyChanged, this); }, }; From 62aa6be6014f970557f1fd9cf0833bbd97355a23 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Wed, 6 Mar 2024 20:04:00 +0100 Subject: [PATCH 08/15] fix: infinite initialization on web --- lib/storage/index.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/storage/index.ts b/lib/storage/index.ts index 836d3efba..ba3db4409 100644 --- a/lib/storage/index.ts +++ b/lib/storage/index.ts @@ -28,9 +28,11 @@ function degradePerformance(error: Error) { /** * Runs a piece of code and degrades performance if certain errors are thrown */ -function tryOrDegradePerformance(fn: () => Promise | T): Promise { +function tryOrDegradePerformance(fn: () => Promise | T, waitForInitialization = true): Promise { return new Promise((resolve, reject) => { - initPromise.then(() => { + const promise = waitForInitialization ? initPromise : Promise.resolve(); + + promise.then(() => { try { resolve(fn()); } catch (error) { @@ -61,7 +63,7 @@ const Storage: Storage = { * and enables fallback providers if necessary */ init() { - tryOrDegradePerformance(provider.init).finally(() => { + tryOrDegradePerformance(provider.init, false).finally(() => { finishInitalization(); }); }, From 7d405022b4b283965c93964e5e2fff426a1d91b3 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Thu, 7 Mar 2024 12:11:00 +0100 Subject: [PATCH 09/15] fix: eslint --- lib/storage/InstanceSync/index.web.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/storage/InstanceSync/index.web.ts b/lib/storage/InstanceSync/index.web.ts index 8327ef110..afa8805d8 100644 --- a/lib/storage/InstanceSync/index.web.ts +++ b/lib/storage/InstanceSync/index.web.ts @@ -6,7 +6,7 @@ import type {OnyxKey} from '../../types'; import NoopProvider from '../providers/NoopProvider'; import type {KeyList, OnStorageKeyChanged} from '../providers/types'; -import StorageProvider from '../providers/types'; +import type StorageProvider from '../providers/types'; const SYNC_ONYX = 'SYNC_ONYX'; From 69bf646807f688130b86b9e460dee3f6423fa7b1 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Thu, 7 Mar 2024 21:04:30 +0100 Subject: [PATCH 10/15] feat: cacth db initialization error --- lib/storage/index.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/storage/index.ts b/lib/storage/index.ts index ba3db4409..0a55e7a89 100644 --- a/lib/storage/index.ts +++ b/lib/storage/index.ts @@ -42,6 +42,11 @@ function tryOrDegradePerformance(fn: () => Promise | T, waitForInitializat if (error.message.includes('Internal error opening backing store for indexedDB.open')) { degradePerformance(error); } + + // catch the error if DB connection can not be established/DB can not be created + if (error.message.includes('IDBKeyVal store could not be created')) { + degradePerformance(error); + } } reject(error); From 77e9982800aa967b64de8752b67d454de6f229b6 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Fri, 8 Mar 2024 12:18:36 +0100 Subject: [PATCH 11/15] feat: use if DB is not available for some reasons --- lib/storage/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/storage/index.ts b/lib/storage/index.ts index 0a55e7a89..7b37c8456 100644 --- a/lib/storage/index.ts +++ b/lib/storage/index.ts @@ -2,7 +2,7 @@ import * as Logger from '../Logger'; import PlatformStorage from './platforms'; import InstanceSync from './InstanceSync'; -import NoopProvider from './providers/NoopProvider'; +import MemoryOnlyProvider from './providers/MemoryOnlyProvider'; import type StorageProvider from './providers/types'; let provider = PlatformStorage; @@ -22,7 +22,7 @@ type Storage = { function degradePerformance(error: Error) { Logger.logAlert(`Error while using ${provider.name}. Falling back to only using cache and dropping storage.`); console.error(error); - provider = NoopProvider; + provider = MemoryOnlyProvider; } /** From 18eca4920ee793504405c552503da4ee6d4a775e Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Tue, 12 Mar 2024 18:27:41 +0100 Subject: [PATCH 12/15] chore: remove `setMemoryOnlyKeys` --- lib/storage/__mocks__/index.ts | 1 - lib/storage/index.ts | 3 --- lib/storage/providers/IDBKeyValProvider.ts | 2 -- lib/storage/providers/MemoryOnlyProvider.ts | 7 ------- lib/storage/providers/NoopProvider.ts | 5 ----- lib/storage/providers/SQLiteProvider.ts | 3 --- 6 files changed, 21 deletions(-) diff --git a/lib/storage/__mocks__/index.ts b/lib/storage/__mocks__/index.ts index 8d279abb5..3a15fecf1 100644 --- a/lib/storage/__mocks__/index.ts +++ b/lib/storage/__mocks__/index.ts @@ -15,7 +15,6 @@ const StorageMock = { removeItem: jest.fn(MemoryOnlyProvider.removeItem), removeItems: jest.fn(MemoryOnlyProvider.removeItems), clear: jest.fn(MemoryOnlyProvider.clear), - setMemoryOnlyKeys: jest.fn(MemoryOnlyProvider.setMemoryOnlyKeys), getAllKeys: jest.fn(MemoryOnlyProvider.getAllKeys), getDatabaseSize: jest.fn(MemoryOnlyProvider.getDatabaseSize), keepInstancesSync: jest.fn(), diff --git a/lib/storage/index.ts b/lib/storage/index.ts index 7b37c8456..a229c97e2 100644 --- a/lib/storage/index.ts +++ b/lib/storage/index.ts @@ -162,9 +162,6 @@ const Storage: Storage = { return provider.clear(); }), - // This is a noop for now in order to keep clients from crashing see https://github.com/Expensify/Expensify/issues/312438 - setMemoryOnlyKeys: () => tryOrDegradePerformance(() => provider.setMemoryOnlyKeys()), - /** * Returns all available keys */ diff --git a/lib/storage/providers/IDBKeyValProvider.ts b/lib/storage/providers/IDBKeyValProvider.ts index 1d2e8ce39..3a385d228 100644 --- a/lib/storage/providers/IDBKeyValProvider.ts +++ b/lib/storage/providers/IDBKeyValProvider.ts @@ -46,8 +46,6 @@ const provider: StorageProvider = { }, multiSet: (pairs) => setMany(pairs, idbKeyValStore), clear: () => clear(idbKeyValStore), - // eslint-disable-next-line @typescript-eslint/no-empty-function - setMemoryOnlyKeys: () => {}, getAllKeys: () => keys(idbKeyValStore), getItem: (key) => get(key, idbKeyValStore) diff --git a/lib/storage/providers/MemoryOnlyProvider.ts b/lib/storage/providers/MemoryOnlyProvider.ts index b62cefd7c..bcb247830 100644 --- a/lib/storage/providers/MemoryOnlyProvider.ts +++ b/lib/storage/providers/MemoryOnlyProvider.ts @@ -112,13 +112,6 @@ const provider: StorageProvider = { return Promise.resolve(); }, - /* - * Since this is an in-memory only provider, nothing special needs to happen here and it can just be a noop - */ - setMemoryOnlyKeys() { - // do nothing - }, - /** * Returns all keys available in memory */ diff --git a/lib/storage/providers/NoopProvider.ts b/lib/storage/providers/NoopProvider.ts index 06d05e65d..07af0a496 100644 --- a/lib/storage/providers/NoopProvider.ts +++ b/lib/storage/providers/NoopProvider.ts @@ -79,11 +79,6 @@ const provider: StorageProvider = { return Promise.resolve(); }, - // This is a noop for now in order to keep clients from crashing see https://github.com/Expensify/Expensify/issues/312438 - setMemoryOnlyKeys() { - // do nothing - }, - /** * Returns all keys available in memory */ diff --git a/lib/storage/providers/SQLiteProvider.ts b/lib/storage/providers/SQLiteProvider.ts index 1a86088bf..ad5324ea9 100644 --- a/lib/storage/providers/SQLiteProvider.ts +++ b/lib/storage/providers/SQLiteProvider.ts @@ -102,9 +102,6 @@ const provider: StorageProvider = { }; }); }, - - // eslint-disable-next-line @typescript-eslint/no-empty-function - setMemoryOnlyKeys: () => {}, }; export default provider; From 946cf22333cdacfb6426289c6ff70e444d956c59 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Tue, 12 Mar 2024 18:29:35 +0100 Subject: [PATCH 13/15] fix: correct usage for InstanceSync --- lib/storage/InstanceSync/index.ts | 1 + lib/storage/InstanceSync/index.web.ts | 1 + lib/storage/index.ts | 4 ++-- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/storage/InstanceSync/index.ts b/lib/storage/InstanceSync/index.ts index 327fbefdb..543cc0409 100644 --- a/lib/storage/InstanceSync/index.ts +++ b/lib/storage/InstanceSync/index.ts @@ -5,6 +5,7 @@ import NOOP from 'lodash/noop'; * On native platforms, we omit this syncing logic by setting this to mock implementation. */ const InstanceSync = { + shouldBeUsed: false, init: NOOP, setItem: NOOP, removeItem: NOOP, diff --git a/lib/storage/InstanceSync/index.web.ts b/lib/storage/InstanceSync/index.web.ts index afa8805d8..6be21b2e7 100644 --- a/lib/storage/InstanceSync/index.web.ts +++ b/lib/storage/InstanceSync/index.web.ts @@ -28,6 +28,7 @@ function raiseStorageSyncManyKeysEvent(onyxKeys: KeyList) { let storage = NoopProvider; const InstanceSync = { + shouldBeUsed: true, /** * @param {Function} onStorageKeyChanged Storage synchronization mechanism keeping all opened tabs in sync */ diff --git a/lib/storage/index.ts b/lib/storage/index.ts index a229c97e2..5871e7fb0 100644 --- a/lib/storage/index.ts +++ b/lib/storage/index.ts @@ -176,8 +176,8 @@ const Storage: Storage = { * @param onStorageKeyChanged - Storage synchronization mechanism keeping all opened tabs in sync (web only) */ keepInstancesSync(onStorageKeyChanged) { - // If InstanceSync is null, it means we're on a native platform and we don't need to keep instances in sync - if (InstanceSync === null) return; + // If InstanceSync shouldn't be used, it means we're on a native platform and we don't need to keep instances in sync + if (!InstanceSync.shouldBeUsed) return; shouldKeepInstancesSync = true; InstanceSync.init(onStorageKeyChanged, this); From fe2780ed4cf6ec1f8f0a6171567dd4053e361a63 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Tue, 12 Mar 2024 18:40:04 +0100 Subject: [PATCH 14/15] fix: eslint --- lib/storage/providers/MemoryOnlyProvider.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/storage/providers/MemoryOnlyProvider.ts b/lib/storage/providers/MemoryOnlyProvider.ts index bcb247830..b96d38ec0 100644 --- a/lib/storage/providers/MemoryOnlyProvider.ts +++ b/lib/storage/providers/MemoryOnlyProvider.ts @@ -42,7 +42,13 @@ const provider: StorageProvider = { * Get multiple key-value pairs for the give array of keys in a batch. */ multiGet(keys) { - const getPromises = _.map(keys, (key) => new Promise((resolve) => this.getItem(key).then((value) => resolve([key, value])))) as Array>; + const getPromises = _.map( + keys, + (key) => + new Promise((resolve) => { + this.getItem(key).then((value) => resolve([key, value])); + }), + ) as Array>; return Promise.all(getPromises); }, @@ -60,7 +66,12 @@ const provider: StorageProvider = { */ multiSet(pairs) { const setPromises = _.map(pairs, ([key, value]) => this.setItem(key, value)); - return new Promise((resolve) => Promise.all(setPromises).then(() => resolve())); + + return new Promise((resolve) => { + Promise.all(setPromises).then(() => { + resolve(undefined); + }); + }); }, /** From 8254d590ff6c07fcff1e4c60177aad95a0994e91 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Fri, 15 Mar 2024 19:32:30 +0100 Subject: [PATCH 15/15] fix: types after rebase --- lib/storage/providers/MemoryOnlyProvider.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/storage/providers/MemoryOnlyProvider.ts b/lib/storage/providers/MemoryOnlyProvider.ts index b96d38ec0..74d9e3a84 100644 --- a/lib/storage/providers/MemoryOnlyProvider.ts +++ b/lib/storage/providers/MemoryOnlyProvider.ts @@ -1,14 +1,15 @@ import _ from 'underscore'; import utils from '../../utils'; import type StorageProvider from './types'; -import type {Key, KeyValuePair, Value} from './types'; +import type {KeyValuePair} from './types'; +import type {OnyxKey, OnyxValue} from '../../types'; -type Store = Record; +type Store = Record>; // eslint-disable-next-line import/no-mutable-exports let store: Store = {}; -const setInternal = (key: Key, value: Value) => { +const setInternal = (key: OnyxKey, value: OnyxValue) => { store[key] = value; return Promise.resolve(value); }; @@ -33,7 +34,7 @@ const provider: StorageProvider = { * Get the value of a given key or return `null` if it's not available in memory */ getItem(key) { - const value = store[key]; + const value = store[key] as OnyxValue; return Promise.resolve(value === undefined ? null : value); }, @@ -88,8 +89,8 @@ const provider: StorageProvider = { */ multiMerge(pairs) { _.forEach(pairs, ([key, value]) => { - const existingValue = store[key] as unknown as Record; - const newValue = utils.fastMerge(existingValue, value as unknown as Record) as unknown as Value; + const existingValue = store[key] as Record; + const newValue = utils.fastMerge(existingValue, value as Record) as OnyxValue; set(key, newValue); });