From d7cfb1262433dcf02cb5a58a7c31e6c032cc9abe Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Wed, 28 Dec 2022 12:36:40 -0700 Subject: [PATCH 01/10] Add multiRemove to both storage providers --- lib/storage/providers/AsyncStorage.js | 1 + lib/storage/providers/LocalForage.js | 10 ++++++++++ lib/storage/providers/SQLiteStorage.js | 10 ++++++++++ 3 files changed, 21 insertions(+) diff --git a/lib/storage/providers/AsyncStorage.js b/lib/storage/providers/AsyncStorage.js index 2398916c0..6ced2ee4f 100644 --- a/lib/storage/providers/AsyncStorage.js +++ b/lib/storage/providers/AsyncStorage.js @@ -78,6 +78,7 @@ const provider = { * @returns {Promise} */ clear: AsyncStorage.clear, + multiRemove: AsyncStorage.multiRemove, }; export default provider; diff --git a/lib/storage/providers/LocalForage.js b/lib/storage/providers/LocalForage.js index ea85a9153..9cd9c49b6 100644 --- a/lib/storage/providers/LocalForage.js +++ b/lib/storage/providers/LocalForage.js @@ -74,6 +74,16 @@ const provider = { return Promise.all(tasks).then(() => Promise.resolve()); }, + /** + * Removes all the keys specified in the array + * @param {Array} keys + * @return {Promise} + */ + multiRemove(keys) { + const tasks = _.map(keys, key => this.removeItem(key)); + return Promise.all(tasks).then(() => Promise.resolve()); + }, + /** * Clear absolutely everything from storage * @returns {Promise} diff --git a/lib/storage/providers/SQLiteStorage.js b/lib/storage/providers/SQLiteStorage.js index 5a1c3b485..7a6fc46de 100644 --- a/lib/storage/providers/SQLiteStorage.js +++ b/lib/storage/providers/SQLiteStorage.js @@ -108,6 +108,16 @@ const provider = { */ removeItem: key => db.executeAsync('DELETE FROM keyvaluepairs WHERE record_key = ?;', [key]), + /** + * Removes all of the given keys + * @param {Array} keys + * @returns {Promise} + */ + multiRemove: (keys) => { + const placeholders = _.map(keys, () => '?').join(','); + return db.executeAsync(`DELETE FROM keyvaluepairs WHERE record_key IN (${placeholders});`, keys); + }, + /** * Clears absolutely everything from storage * @returns {Promise} From 02a600422c6f7b22d78457cc0dcf577ff5ae8038 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Wed, 28 Dec 2022 12:47:22 -0700 Subject: [PATCH 02/10] Add some basic unit tests --- .../storage/providers/AsyncStorageProviderTest.js | 14 ++++++++++++++ .../storage/providers/LocalForageProviderTest.js | 13 +++++++++++++ 2 files changed, 27 insertions(+) diff --git a/tests/unit/storage/providers/AsyncStorageProviderTest.js b/tests/unit/storage/providers/AsyncStorageProviderTest.js index 3c45ef1ad..4ae8f173e 100644 --- a/tests/unit/storage/providers/AsyncStorageProviderTest.js +++ b/tests/unit/storage/providers/AsyncStorageProviderTest.js @@ -108,4 +108,18 @@ describe('storage/providers/AsyncStorage', () => { .then(values => expect(values).toEqual(items)); }); }); + + describe('multiRemove', () => { + it('Call multiRemove() with the list of keys', () => { + // Given sample content of all basic types + const pairs = SAMPLE_ITEMS.slice(); + const keys = _.map(pairs, pair => pair[0]); + + // When multiple keys are removed + StorageProvider.multiRemove(keys); + + // multiRemove should be called + expect(AsyncStorageMock.multiRemove).toHaveBeenCalledWith(keys); + }); + }); }); diff --git a/tests/unit/storage/providers/LocalForageProviderTest.js b/tests/unit/storage/providers/LocalForageProviderTest.js index e2c3b57c4..004eb20f0 100644 --- a/tests/unit/storage/providers/LocalForageProviderTest.js +++ b/tests/unit/storage/providers/LocalForageProviderTest.js @@ -104,4 +104,17 @@ describe('storage/providers/LocalForage', () => { }); }); }); + + it('multiRemove', () => { + // Given multiple pairs to be saved in storage + const pairs = SAMPLE_ITEMS.slice(); + const keys = _.map(pairs, pair => pair[0]); + + // When they are saved + return StorageProvider.multiRemove(keys) + .then(() => { + // We expect a call to localForage.removeItem for each pair + _.each(keys, key => expect(localforage.removeItem).toHaveBeenCalledWith(key)); + }); + }); }); From 842be4292a774a8f33806cd381d19cb35340b204 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Wed, 28 Dec 2022 12:47:42 -0700 Subject: [PATCH 03/10] Implement the multiRemove method --- lib/Onyx.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Onyx.js b/lib/Onyx.js index 4a4196da1..3906f8b66 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -1102,7 +1102,7 @@ function clear(keysToPreserve = []) { notifyCollectionSubscribersOnNextTick(key, value); }); - return Storage.multiSet(keyValuesToReset); + return Storage.multiRemove(keyValuesToReset); }); } From b98d3c589b5b332200c562eb3ce40c7424dffe51 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 29 Dec 2022 09:11:42 -0700 Subject: [PATCH 04/10] Fix the keys passed to multiRemove --- lib/Onyx.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/Onyx.js b/lib/Onyx.js index 3906f8b66..c401bbf8b 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -1102,7 +1102,9 @@ function clear(keysToPreserve = []) { notifyCollectionSubscribersOnNextTick(key, value); }); - return Storage.multiRemove(keyValuesToReset); + // Extract all the keys and clear them out + const keysToReset = _.map(keyValuesToReset, keyValueToReset => keyValueToReset[0]); + return Storage.multiRemove(keysToReset); }); } From c57082390894bfef323e619816c49fad7c95d52d Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 29 Dec 2022 11:13:38 -0700 Subject: [PATCH 05/10] Fix all the tests --- lib/Onyx.js | 6 +++--- tests/unit/onyxClearNativeStorageTest.js | 5 ++++- tests/unit/onyxMultiMergeWebStorageTest.js | 6 +----- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/Onyx.js b/lib/Onyx.js index c401bbf8b..282d5ea2e 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -1102,9 +1102,9 @@ function clear(keysToPreserve = []) { notifyCollectionSubscribersOnNextTick(key, value); }); - // Extract all the keys and clear them out - const keysToReset = _.map(keyValuesToReset, keyValueToReset => keyValueToReset[0]); - return Storage.multiRemove(keysToReset); + return Storage.multiSet(defaultKeyValuePairs).then(() => { + return Storage.multiRemove(keysToClear); + }); }); } diff --git a/tests/unit/onyxClearNativeStorageTest.js b/tests/unit/onyxClearNativeStorageTest.js index 4af1cdeb9..2704ee334 100644 --- a/tests/unit/onyxClearNativeStorageTest.js +++ b/tests/unit/onyxClearNativeStorageTest.js @@ -79,7 +79,10 @@ describe('Set data while storage is clearing', () => { const cachedValue = cache.getValue(ONYX_KEYS.DEFAULT_KEY); expect(cachedValue).toBe(DEFAULT_VALUE); return Storage.getItem(ONYX_KEYS.DEFAULT_KEY) - .then(storedValue => expect(parseInt(storedValue, 10)).toBe(DEFAULT_VALUE)); + .then(storedValue => { + console.log(storedValue) + expect(parseInt(storedValue, 10)).toBe(DEFAULT_VALUE) + }); }); }); diff --git a/tests/unit/onyxMultiMergeWebStorageTest.js b/tests/unit/onyxMultiMergeWebStorageTest.js index f60e24d81..cb9a0e1af 100644 --- a/tests/unit/onyxMultiMergeWebStorageTest.js +++ b/tests/unit/onyxMultiMergeWebStorageTest.js @@ -142,11 +142,7 @@ describe('Onyx.mergeCollection() amd WebStorage', () => { it('setItem() and multiMerge()', () => { // When Onyx is cleared, since it uses multiSet() to clear out all the values, the keys will remain with null for all the values - expect(localforageMock.storageMap).toEqual({ - test_1: null, - test_2: null, - test_3: null, - }); + expect(localforageMock.storageMap).toEqual({}); // Given no previous data and several calls to setItem and call to mergeCollection to update a given key From 88a165ce10e8026f86b33b5670608e0defa699c4 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Thu, 29 Dec 2022 11:32:07 -0700 Subject: [PATCH 06/10] Fix lint --- lib/Onyx.js | 5 ++--- tests/unit/onyxClearNativeStorageTest.js | 5 +---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/Onyx.js b/lib/Onyx.js index 282d5ea2e..70c421dee 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -1102,9 +1102,8 @@ function clear(keysToPreserve = []) { notifyCollectionSubscribersOnNextTick(key, value); }); - return Storage.multiSet(defaultKeyValuePairs).then(() => { - return Storage.multiRemove(keysToClear); - }); + return Storage.multiSet(defaultKeyValuePairs) + .then(() => Storage.multiRemove(keysToClear)); }); } diff --git a/tests/unit/onyxClearNativeStorageTest.js b/tests/unit/onyxClearNativeStorageTest.js index 2704ee334..4af1cdeb9 100644 --- a/tests/unit/onyxClearNativeStorageTest.js +++ b/tests/unit/onyxClearNativeStorageTest.js @@ -79,10 +79,7 @@ describe('Set data while storage is clearing', () => { const cachedValue = cache.getValue(ONYX_KEYS.DEFAULT_KEY); expect(cachedValue).toBe(DEFAULT_VALUE); return Storage.getItem(ONYX_KEYS.DEFAULT_KEY) - .then(storedValue => { - console.log(storedValue) - expect(parseInt(storedValue, 10)).toBe(DEFAULT_VALUE) - }); + .then(storedValue => expect(parseInt(storedValue, 10)).toBe(DEFAULT_VALUE)); }); }); From cd4568382e2587866970f2665191b971ffce6075 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 3 Jan 2023 15:40:53 -0700 Subject: [PATCH 07/10] Update lib/storage/providers/AsyncStorage.js Co-authored-by: Yuwen Memon --- lib/storage/providers/AsyncStorage.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/storage/providers/AsyncStorage.js b/lib/storage/providers/AsyncStorage.js index 6ced2ee4f..3793d1ca8 100644 --- a/lib/storage/providers/AsyncStorage.js +++ b/lib/storage/providers/AsyncStorage.js @@ -78,6 +78,12 @@ const provider = { * @returns {Promise} */ clear: AsyncStorage.clear, + + /** + * Removes all the keys specified in the array + * @param {Array} keys + * @returns {Promise} + */ multiRemove: AsyncStorage.multiRemove, }; From 829798c1a8ec3b8f7489a1d9f894df09490c89b3 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 3 Jan 2023 15:41:00 -0700 Subject: [PATCH 08/10] Update lib/storage/providers/LocalForage.js Co-authored-by: Yuwen Memon --- lib/storage/providers/LocalForage.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/storage/providers/LocalForage.js b/lib/storage/providers/LocalForage.js index 9cd9c49b6..b83d5bced 100644 --- a/lib/storage/providers/LocalForage.js +++ b/lib/storage/providers/LocalForage.js @@ -77,7 +77,7 @@ const provider = { /** * Removes all the keys specified in the array * @param {Array} keys - * @return {Promise} + * @returns {Promise} */ multiRemove(keys) { const tasks = _.map(keys, key => this.removeItem(key)); From 88a19be0f139ab1d59572f20053b031eeb755ff4 Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 3 Jan 2023 15:41:10 -0700 Subject: [PATCH 09/10] Update lib/storage/providers/LocalForage.js Co-authored-by: Yuwen Memon --- lib/storage/providers/LocalForage.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/storage/providers/LocalForage.js b/lib/storage/providers/LocalForage.js index b83d5bced..fb8fc9839 100644 --- a/lib/storage/providers/LocalForage.js +++ b/lib/storage/providers/LocalForage.js @@ -81,7 +81,9 @@ const provider = { */ multiRemove(keys) { const tasks = _.map(keys, key => this.removeItem(key)); - return Promise.all(tasks).then(() => Promise.resolve()); + + // We're returning Promise.resolve, otherwise the array of task results will be returned to the caller + return Promise.all(tasks).then(() => Promise.resolve()); }, /** From e0142f7b1550e91d374e08cd6a1cda54a4947ebf Mon Sep 17 00:00:00 2001 From: Tim Golen Date: Tue, 3 Jan 2023 17:34:56 -0800 Subject: [PATCH 10/10] Lint --- lib/storage/providers/AsyncStorage.js | 2 +- lib/storage/providers/LocalForage.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/storage/providers/AsyncStorage.js b/lib/storage/providers/AsyncStorage.js index 3793d1ca8..0e62094d7 100644 --- a/lib/storage/providers/AsyncStorage.js +++ b/lib/storage/providers/AsyncStorage.js @@ -78,7 +78,7 @@ const provider = { * @returns {Promise} */ clear: AsyncStorage.clear, - + /** * Removes all the keys specified in the array * @param {Array} keys diff --git a/lib/storage/providers/LocalForage.js b/lib/storage/providers/LocalForage.js index fb8fc9839..11749d386 100644 --- a/lib/storage/providers/LocalForage.js +++ b/lib/storage/providers/LocalForage.js @@ -82,8 +82,8 @@ const provider = { multiRemove(keys) { const tasks = _.map(keys, key => this.removeItem(key)); - // We're returning Promise.resolve, otherwise the array of task results will be returned to the caller - return Promise.all(tasks).then(() => Promise.resolve()); + // We're returning Promise.resolve, otherwise the array of task results will be returned to the caller + return Promise.all(tasks).then(() => Promise.resolve()); }, /**