From 7537744a834b722c259bea8f2d0c8245a58bd8e0 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 9 Mar 2022 15:25:41 -1000 Subject: [PATCH 1/9] add multi merge test against webstorage --- __mocks__/localforage.js | 34 ++++++++++++++++++++-- lib/storage/index.js | 2 +- tests/unit/onyxMultiMerge.js | 56 ++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+), 3 deletions(-) create mode 100644 tests/unit/onyxMultiMerge.js diff --git a/__mocks__/localforage.js b/__mocks__/localforage.js index c552895f..73a3e516 100644 --- a/__mocks__/localforage.js +++ b/__mocks__/localforage.js @@ -1,7 +1,37 @@ +import _ from 'underscore'; + +let storageMap = {}; +const DELAY_MS = 500; + const localforageMock = { + keys() { + return new Promise((resolve) => { + resolve(_.keys(storageMap)); + }); + }, + setInitialMockData: (data) => { + storageMap = data; + }, config: jest.fn(), - getItem: jest.fn(), - setItem: jest.fn(() => Promise.resolve()), + getItem(key) { + return new Promise((resolve) => { + setTimeout(() => { + resolve(storageMap[key]); + }, DELAY_MS); + }); + }, + setItem: jest.fn((key, value) => new Promise((resolve) => { + setTimeout(() => { + storageMap[key] = value; + resolve(); + }, DELAY_MS); + })), + removeItem: jest.fn(key => new Promise((resolve) => { + setTimeout(() => { + delete storageMap[key]; + resolve(); + }, DELAY_MS); + })), }; export default localforageMock; diff --git a/lib/storage/index.js b/lib/storage/index.js index f5a621f4..bd218a59 100644 --- a/lib/storage/index.js +++ b/lib/storage/index.js @@ -2,7 +2,7 @@ import {Platform} from 'react-native'; const Storage = Platform.select({ default: () => require('./WebStorage').default, - native: () => require('./NativeStorage').default, + // native: () => require('./NativeStorage').default, })(); export default Storage; diff --git a/tests/unit/onyxMultiMerge.js b/tests/unit/onyxMultiMerge.js new file mode 100644 index 00000000..2a3791a0 --- /dev/null +++ b/tests/unit/onyxMultiMerge.js @@ -0,0 +1,56 @@ +import Onyx from '../../index'; +import OnyxCache from '../../lib/OnyxCache'; +import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; +import localforageMock from '../../__mocks__/localforage'; + +const ONYX_KEYS = { + COLLECTION: { + TEST_KEY: 'test_', + }, +}; + +const initialTestObject = {a: 'a'}; +const initialData = { + test_1: initialTestObject, + test_2: initialTestObject, + test_3: initialTestObject, +}; + +localforageMock.setInitialMockData(initialData); + +Onyx.init({ + keys: ONYX_KEYS, + registerStorageEventListener: () => {}, + maxCachedKeysCount: 1, + initialKeyStates: {}, +}); + +describe('Onyx.mergeCollection()', () => { + it('mergeCollection', () => { + jest.useFakeTimers(); + + const additionalDataOne = {b: 'b', c: 'c'}; + Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { + test_1: additionalDataOne, + test_2: additionalDataOne, + test_3: additionalDataOne, + }); + + const additionalDataTwo = {d: 'd'}; + Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { + test_1: additionalDataTwo, + test_2: additionalDataTwo, + test_3: additionalDataTwo, + }); + + jest.runAllTimers(); + + return waitForPromisesToResolve() + .then(() => { + // Expect that our new data has merged with the existing data in storage + expect(OnyxCache.getValue('test_1')).toEqual({ + a: 'a', b: 'b', c: 'c', d: 'd', + }); + }); + }); +}); From ca0f4a5561ec6826e59875a6404be6614d003eee Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 9 Mar 2022 16:03:55 -1000 Subject: [PATCH 2/9] Create failing test --- __mocks__/localforage.js | 5 ++++- lib/Onyx.js | 2 +- tests/unit/onyxMultiMerge.js | 20 +++++++++++++------- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/__mocks__/localforage.js b/__mocks__/localforage.js index 73a3e516..095dafa3 100644 --- a/__mocks__/localforage.js +++ b/__mocks__/localforage.js @@ -1,9 +1,12 @@ import _ from 'underscore'; let storageMap = {}; -const DELAY_MS = 500; +const DELAY_MS = 5000; const localforageMock = { + get storageMap() { + return storageMap; + }, keys() { return new Promise((resolve) => { resolve(_.keys(storageMap)); diff --git a/lib/Onyx.js b/lib/Onyx.js index 38fc32bc..d992abad 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -808,7 +808,7 @@ function init({ keys = {}, initialKeyStates = {}, safeEvictionKeys = [], - maxCachedKeysCount = 55, + maxCachedKeysCount = 1000, captureMetrics = false, shouldSyncMultipleInstances = Boolean(global.localStorage), } = {}) { diff --git a/tests/unit/onyxMultiMerge.js b/tests/unit/onyxMultiMerge.js index 2a3791a0..c7ba6f3a 100644 --- a/tests/unit/onyxMultiMerge.js +++ b/tests/unit/onyxMultiMerge.js @@ -26,9 +26,8 @@ Onyx.init({ }); describe('Onyx.mergeCollection()', () => { + beforeEach(() => jest.useRealTimers()); it('mergeCollection', () => { - jest.useFakeTimers(); - const additionalDataOne = {b: 'b', c: 'c'}; Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { test_1: additionalDataOne, @@ -43,14 +42,21 @@ describe('Onyx.mergeCollection()', () => { test_3: additionalDataTwo, }); - jest.runAllTimers(); - return waitForPromisesToResolve() .then(() => { - // Expect that our new data has merged with the existing data in storage - expect(OnyxCache.getValue('test_1')).toEqual({ + const finalObject = { a: 'a', b: 'b', c: 'c', d: 'd', - }); + }; + + // Expect that our new data has merged with the existing data in the cache + expect(OnyxCache.getValue('test_1')).toEqual(finalObject); + expect(OnyxCache.getValue('test_2')).toEqual(finalObject); + expect(OnyxCache.getValue('test_3')).toEqual(finalObject); + + // Check the storage to make sure our data has been saved + expect(localforageMock.storageMap.test_1).toEqual(finalObject); + expect(localforageMock.storageMap.test_2).toEqual(finalObject); + expect(localforageMock.storageMap.test_3).toEqual(finalObject); }); }); }); From 7a2e72219b1cf268d43f893d02ce98789f5486bf Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 9 Mar 2022 16:59:38 -1000 Subject: [PATCH 3/9] fix WebStorage mock --- __mocks__/lib/storage/index.js | 3 --- __mocks__/localforage.js | 12 ++++----- lib/storage/NativeStorage.js | 2 ++ lib/storage/WebStorage.js | 1 + lib/storage/__mocks__/index.js | 3 +++ lib/storage/index.js | 2 +- lib/storage/providers/AsyncStorage.js | 2 ++ ...nyxMultiMerge.js => onyxMultiMergeTest.js} | 26 ++++++++++++------- 8 files changed, 31 insertions(+), 20 deletions(-) delete mode 100644 __mocks__/lib/storage/index.js create mode 100644 lib/storage/__mocks__/index.js rename tests/unit/{onyxMultiMerge.js => onyxMultiMergeTest.js} (81%) diff --git a/__mocks__/lib/storage/index.js b/__mocks__/lib/storage/index.js deleted file mode 100644 index 97de83c8..00000000 --- a/__mocks__/lib/storage/index.js +++ /dev/null @@ -1,3 +0,0 @@ -import WebStorage from '../../../lib/storage/WebStorage'; - -export default WebStorage; diff --git a/__mocks__/localforage.js b/__mocks__/localforage.js index 095dafa3..7c7631a8 100644 --- a/__mocks__/localforage.js +++ b/__mocks__/localforage.js @@ -16,13 +16,11 @@ const localforageMock = { storageMap = data; }, config: jest.fn(), - getItem(key) { - return new Promise((resolve) => { - setTimeout(() => { - resolve(storageMap[key]); - }, DELAY_MS); - }); - }, + getItem: jest.fn(key => new Promise((resolve) => { + setTimeout(() => { + resolve(storageMap[key]); + }, DELAY_MS); + })), setItem: jest.fn((key, value) => new Promise((resolve) => { setTimeout(() => { storageMap[key] = value; diff --git a/lib/storage/NativeStorage.js b/lib/storage/NativeStorage.js index a4aaade0..e3781a0c 100644 --- a/lib/storage/NativeStorage.js +++ b/lib/storage/NativeStorage.js @@ -1,3 +1,5 @@ import Storage from './providers/AsyncStorage'; +console.log(Storage); + export default Storage; diff --git a/lib/storage/WebStorage.js b/lib/storage/WebStorage.js index 0d2d9d12..32691508 100644 --- a/lib/storage/WebStorage.js +++ b/lib/storage/WebStorage.js @@ -13,6 +13,7 @@ function raiseStorageSyncEvent(onyxKey) { } const webStorage = { + name: 'WebStorage', ...Storage, /** diff --git a/lib/storage/__mocks__/index.js b/lib/storage/__mocks__/index.js new file mode 100644 index 00000000..c7d08295 --- /dev/null +++ b/lib/storage/__mocks__/index.js @@ -0,0 +1,3 @@ +import WebStorage from '../WebStorage'; + +export default WebStorage; diff --git a/lib/storage/index.js b/lib/storage/index.js index bd218a59..f5a621f4 100644 --- a/lib/storage/index.js +++ b/lib/storage/index.js @@ -2,7 +2,7 @@ import {Platform} from 'react-native'; const Storage = Platform.select({ default: () => require('./WebStorage').default, - // native: () => require('./NativeStorage').default, + native: () => require('./NativeStorage').default, })(); export default Storage; diff --git a/lib/storage/providers/AsyncStorage.js b/lib/storage/providers/AsyncStorage.js index 2398916c..8d311202 100644 --- a/lib/storage/providers/AsyncStorage.js +++ b/lib/storage/providers/AsyncStorage.js @@ -7,6 +7,8 @@ import _ from 'underscore'; import AsyncStorage from '@react-native-async-storage/async-storage'; const provider = { + name: 'AsyncStorage', + /** * Get the value of a given key or return `null` if it's not available in storage * @param {String} key diff --git a/tests/unit/onyxMultiMerge.js b/tests/unit/onyxMultiMergeTest.js similarity index 81% rename from tests/unit/onyxMultiMerge.js rename to tests/unit/onyxMultiMergeTest.js index c7ba6f3a..5331d296 100644 --- a/tests/unit/onyxMultiMerge.js +++ b/tests/unit/onyxMultiMergeTest.js @@ -1,4 +1,3 @@ -import Onyx from '../../index'; import OnyxCache from '../../lib/OnyxCache'; import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; import localforageMock from '../../__mocks__/localforage'; @@ -18,16 +17,25 @@ const initialData = { localforageMock.setInitialMockData(initialData); -Onyx.init({ - keys: ONYX_KEYS, - registerStorageEventListener: () => {}, - maxCachedKeysCount: 1, - initialKeyStates: {}, -}); - describe('Onyx.mergeCollection()', () => { - beforeEach(() => jest.useRealTimers()); + let Onyx; + let Storage; + beforeEach(() => { + jest.mock('../../lib/storage'); + Onyx = require('../../index').default; + Storage = require('../../lib/storage').default; + jest.useRealTimers(); + }); + it('mergeCollection', () => { + expect(Storage.name).toBe('WebStorage'); + + Onyx.init({ + keys: ONYX_KEYS, + registerStorageEventListener: () => {}, + initialKeyStates: {}, + }); + const additionalDataOne = {b: 'b', c: 'c'}; Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { test_1: additionalDataOne, From d4e0d10fd711142cdbe407d95d1dd4d84d7bfcad Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Wed, 9 Mar 2022 18:54:13 -1000 Subject: [PATCH 4/9] Create SyncQueue and fix multiMerge issues --- __mocks__/localforage.js | 17 ++--- lib/Onyx.js | 11 +-- lib/SyncQueue.js | 35 ++++++++++ lib/storage/NativeStorage.js | 2 - lib/storage/providers/LocalForage.js | 68 +++++++------------ ...est.js => onyxMultiMergeWebStorageTest.js} | 2 +- 6 files changed, 70 insertions(+), 65 deletions(-) create mode 100644 lib/SyncQueue.js rename tests/unit/{onyxMultiMergeTest.js => onyxMultiMergeWebStorageTest.js} (98%) diff --git a/__mocks__/localforage.js b/__mocks__/localforage.js index 7c7631a8..a0e14e75 100644 --- a/__mocks__/localforage.js +++ b/__mocks__/localforage.js @@ -1,7 +1,6 @@ import _ from 'underscore'; let storageMap = {}; -const DELAY_MS = 5000; const localforageMock = { get storageMap() { @@ -17,21 +16,15 @@ const localforageMock = { }, config: jest.fn(), getItem: jest.fn(key => new Promise((resolve) => { - setTimeout(() => { - resolve(storageMap[key]); - }, DELAY_MS); + resolve(storageMap[key]); })), setItem: jest.fn((key, value) => new Promise((resolve) => { - setTimeout(() => { - storageMap[key] = value; - resolve(); - }, DELAY_MS); + storageMap[key] = value; + resolve(); })), removeItem: jest.fn(key => new Promise((resolve) => { - setTimeout(() => { - delete storageMap[key]; - resolve(); - }, DELAY_MS); + delete storageMap[key]; + resolve(); })), }; diff --git a/lib/Onyx.js b/lib/Onyx.js index d992abad..5d731213 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -768,11 +768,12 @@ function mergeCollection(collectionKey, collection) { promises.push(Storage.multiSet(keyValuePairsForNewCollection)); } - // Merge original data to cache - cache.merge(collection); - - // Optimistically inform collection subscribers - keysChanged(collectionKey, collection); + // Prefill cache if necessary by calling get() on any existing keys and then merge original data to cache + // and update all subscribers + Promise.all(_.map(existingKeys, get)).then(() => { + cache.merge(collection); + keysChanged(collectionKey, collection); + }); return Promise.all(promises) .catch(error => evictStorageAndRetry(error, mergeCollection, collection)); diff --git a/lib/SyncQueue.js b/lib/SyncQueue.js new file mode 100644 index 00000000..0db2ac79 --- /dev/null +++ b/lib/SyncQueue.js @@ -0,0 +1,35 @@ +export default class SyncQueue { + constructor(run) { + this.queue = []; + this.isProcessing = false; + this.run = run; + } + + process() { + if (this.isProcessing || this.queue.length === 0) { + return; + } + + this.isProcessing = true; + + const {data, resolve, reject} = this.queue.shift(); + this.run(data) + .then(resolve) + .catch(reject) + .finally(() => { + this.isProcessing = false; + this.process(); + }); + } + + /** + * @param {*} data + * @returns {Promise} + */ + push(data) { + return new Promise((resolve, reject) => { + this.queue.push({resolve, reject, data}); + this.process(); + }); + } +} diff --git a/lib/storage/NativeStorage.js b/lib/storage/NativeStorage.js index e3781a0c..a4aaade0 100644 --- a/lib/storage/NativeStorage.js +++ b/lib/storage/NativeStorage.js @@ -1,5 +1,3 @@ import Storage from './providers/AsyncStorage'; -console.log(Storage); - export default Storage; diff --git a/lib/storage/providers/LocalForage.js b/lib/storage/providers/LocalForage.js index 7c254be8..e194cc04 100644 --- a/lib/storage/providers/LocalForage.js +++ b/lib/storage/providers/LocalForage.js @@ -7,39 +7,32 @@ import localforage from 'localforage'; import _ from 'underscore'; import lodashMerge from 'lodash/merge'; +import SyncQueue from '../../SyncQueue'; localforage.config({ name: 'OnyxDB' }); -const writeQueue = []; -let isQueueProcessing = false; - -/** - * Writing very quickly to IndexedDB causes performance issues and can lock up the page and lead to jank. - * So, we are slowing this process down by waiting until one write is complete before moving on - * to the next. - */ -function processNextWrite() { - if (isQueueProcessing || writeQueue.length === 0) { - return; - } - - isQueueProcessing = true; +const provider = { + /** + * Writing very quickly to IndexedDB causes performance issues and can lock up the page and lead to jank. + * So, we are slowing this process down by waiting until one write is complete before moving on + * to the next. + */ + setItemQueue: new SyncQueue(({key, value}) => localforage.setItem(key, value)), + multiMergeQueue: new SyncQueue((pairs) => { + const tasks = _.map(pairs, ([key, partialValue]) => this.default.getItem(key) + .then((existingValue) => { + const newValue = _.isObject(existingValue) + ? lodashMerge({}, existingValue, partialValue) + : partialValue; + return this.default.setItem(key, newValue); + })); - const { - key, value, resolve, reject, - } = writeQueue.shift(); - localforage.setItem(key, value) - .then(resolve) - .catch(reject) - .finally(() => { - isQueueProcessing = false; - processNextWrite(); - }); -} + // We're returning Promise.resolve, otherwise the array of task results will be returned to the caller + return Promise.all(tasks).then(() => Promise.resolve()); + }), -const provider = { /** * Get multiple key-value pairs for the give array of keys in a batch * @param {String[]} keys @@ -61,17 +54,7 @@ const provider = { * @return {Promise} */ multiMerge(pairs) { - const tasks = _.map(pairs, ([key, partialValue]) => this.getItem(key) - .then((existingValue) => { - const newValue = _.isObject(existingValue) - ? lodashMerge(existingValue, partialValue) - : partialValue; - - return this.setItem(key, newValue); - })); - - // We're returning Promise.resolve, otherwise the array of task results will be returned to the caller - return Promise.all(tasks).then(() => Promise.resolve()); + return this.multiMergeQueue.push(pairs); }, /** @@ -117,14 +100,9 @@ const provider = { * @param {*} value * @return {Promise} */ - setItem: (key, value) => ( - new Promise((resolve, reject) => { - writeQueue.push({ - key, value, resolve, reject, - }); - processNextWrite(); - }) - ), + setItem(key, value) { + return this.setItemQueue.push({key, value}); + }, }; export default provider; diff --git a/tests/unit/onyxMultiMergeTest.js b/tests/unit/onyxMultiMergeWebStorageTest.js similarity index 98% rename from tests/unit/onyxMultiMergeTest.js rename to tests/unit/onyxMultiMergeWebStorageTest.js index 5331d296..d1955c04 100644 --- a/tests/unit/onyxMultiMergeTest.js +++ b/tests/unit/onyxMultiMergeWebStorageTest.js @@ -20,7 +20,7 @@ localforageMock.setInitialMockData(initialData); describe('Onyx.mergeCollection()', () => { let Onyx; let Storage; - beforeEach(() => { + beforeAll(() => { jest.mock('../../lib/storage'); Onyx = require('../../index').default; Storage = require('../../lib/storage').default; From 3fa2aea31a5f139fcfeefb904bb15021e4d708a9 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 10 Mar 2022 08:45:15 -1000 Subject: [PATCH 5/9] Fix test --- __mocks__/localforage.js | 6 +++ lib/storage/providers/LocalForage.js | 27 ++++++------ tests/unit/onyxMultiMergeWebStorageTest.js | 43 ++++++++++++++++--- .../providers/LocalForageProviderTest.js | 16 +++---- 4 files changed, 66 insertions(+), 26 deletions(-) diff --git a/__mocks__/localforage.js b/__mocks__/localforage.js index a0e14e75..3e5fb940 100644 --- a/__mocks__/localforage.js +++ b/__mocks__/localforage.js @@ -26,6 +26,12 @@ const localforageMock = { delete storageMap[key]; resolve(); })), + clear() { + return new Promise((resolve) => { + storageMap = {}; + resolve(); + }); + }, }; export default localforageMock; diff --git a/lib/storage/providers/LocalForage.js b/lib/storage/providers/LocalForage.js index e194cc04..3ac44b7c 100644 --- a/lib/storage/providers/LocalForage.js +++ b/lib/storage/providers/LocalForage.js @@ -19,18 +19,18 @@ const provider = { * So, we are slowing this process down by waiting until one write is complete before moving on * to the next. */ - setItemQueue: new SyncQueue(({key, value}) => localforage.setItem(key, value)), - multiMergeQueue: new SyncQueue((pairs) => { - const tasks = _.map(pairs, ([key, partialValue]) => this.default.getItem(key) - .then((existingValue) => { - const newValue = _.isObject(existingValue) - ? lodashMerge({}, existingValue, partialValue) - : partialValue; - return this.default.setItem(key, newValue); - })); + setItemQueue: new SyncQueue(({key, value, shouldMerge}) => { + if (shouldMerge) { + return localforage.getItem(key) + .then((existingValue) => { + const newValue = _.isObject(existingValue) + ? lodashMerge({}, existingValue, value) + : value; + return localforage.setItem(key, newValue); + }); + } - // We're returning Promise.resolve, otherwise the array of task results will be returned to the caller - return Promise.all(tasks).then(() => Promise.resolve()); + return localforage.setItem(key, value); }), /** @@ -54,7 +54,10 @@ const provider = { * @return {Promise} */ multiMerge(pairs) { - return this.multiMergeQueue.push(pairs); + const tasks = _.map(pairs, ([key, value]) => this.setItemQueue.push({key, value, shouldMerge: true})); + + // We're returning Promise.resolve, otherwise the array of task results will be returned to the caller + return Promise.all(tasks).then(() => Promise.resolve()); }, /** diff --git a/tests/unit/onyxMultiMergeWebStorageTest.js b/tests/unit/onyxMultiMergeWebStorageTest.js index d1955c04..a9c03425 100644 --- a/tests/unit/onyxMultiMergeWebStorageTest.js +++ b/tests/unit/onyxMultiMergeWebStorageTest.js @@ -17,24 +17,27 @@ const initialData = { localforageMock.setInitialMockData(initialData); -describe('Onyx.mergeCollection()', () => { +describe('Onyx.mergeCollection() amd WebStorage', () => { let Onyx; let Storage; + beforeAll(() => { jest.mock('../../lib/storage'); Onyx = require('../../index').default; Storage = require('../../lib/storage').default; jest.useRealTimers(); - }); - - it('mergeCollection', () => { - expect(Storage.name).toBe('WebStorage'); Onyx.init({ keys: ONYX_KEYS, registerStorageEventListener: () => {}, initialKeyStates: {}, }); + }); + + afterEach(() => Onyx.clear()); + + it('merges two sets of data consecutively', () => { + expect(Storage.name).toBe('WebStorage'); const additionalDataOne = {b: 'b', c: 'c'}; Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { @@ -67,4 +70,34 @@ describe('Onyx.mergeCollection()', () => { expect(localforageMock.storageMap.test_3).toEqual(finalObject); }); }); + + it('setItem() and multiMerge()', () => { + expect(localforageMock.storageMap).toEqual({}); + + // Given no previous data and several calls to setItem and call to mergeCollection to update a given key + + // 1st call + Onyx.set('test_1', {a: 'a'}); + + // These merges will all queue together + Onyx.merge('test_1', {b: 'b'}); + Onyx.merge('test_1', {c: 'c'}); + + // 2nd call + Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { + test_1: {d: 'd', e: 'e'}, + }); + + // Last call + Onyx.merge('test_1', {f: 'f'}); + return waitForPromisesToResolve() + .then(() => { + const finalObject = { + a: 'a', b: 'b', c: 'c', f: 'f', d: 'd', e: 'e', + }; + + expect(OnyxCache.getValue('test_1')).toEqual(finalObject); + expect(localforageMock.storageMap.test_1).toEqual(finalObject); + }); + }); }); diff --git a/tests/unit/storage/providers/LocalForageProviderTest.js b/tests/unit/storage/providers/LocalForageProviderTest.js index 1940a09a..c776a56d 100644 --- a/tests/unit/storage/providers/LocalForageProviderTest.js +++ b/tests/unit/storage/providers/LocalForageProviderTest.js @@ -78,32 +78,30 @@ describe('storage/providers/LocalForage', () => { const setItemSpy = localforage.setItem.mockImplementation(() => Promise.resolve()); // When data is merged to storage - StorageProvider.multiMerge([ + return StorageProvider.multiMerge([ ['@USER_1', USER_1_DELTA], ['@USER_2', USER_2_DELTA] ]) .then(() => { // Then each existing item should be set with the merged content - expect(setItemSpy).toHaveBeenCalledWith( + expect(setItemSpy).toHaveBeenNthCalledWith(1, '@USER_1', { name: 'Tom', - age: 30, + age: 31, traits: { hair: 'brown', eyes: 'blue' } - } - ); + }); - expect(setItemSpy).toHaveBeenCalledWith( + expect(setItemSpy).toHaveBeenNthCalledWith(2, '@USER_2', { name: 'Sarah', - age: 31, + age: 26, traits: { hair: 'green', } - } - ); + }); }); }); }); From 1ac6096bbc2d41fb25b6c885a189e4b789e1d4b7 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 10 Mar 2022 08:49:46 -1000 Subject: [PATCH 6/9] add some comments to the sync queue --- lib/SyncQueue.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/lib/SyncQueue.js b/lib/SyncQueue.js index 0db2ac79..5775911e 100644 --- a/lib/SyncQueue.js +++ b/lib/SyncQueue.js @@ -1,4 +1,20 @@ +/** + * Synchronous queue that can be used to ensure promise based tasks are run in sequence. + * Pass to the constructor a function that returns a promise to run the task then add data. + * + * @example + * + * const queue = new SyncQueue(({key, val}) => { + * return someAsyncProcess(key, val); + * }); + * + * queue.push({key: 1, val: '1'}); + * queue.push({key: 2, val: '2'}); + */ export default class SyncQueue { + /** + * @param {Function} run - must return a promise + */ constructor(run) { this.queue = []; this.isProcessing = false; From 29fd8a34d2c40639c9e651aa399c264d5d704fa3 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 10 Mar 2022 08:53:46 -1000 Subject: [PATCH 7/9] remove check for correct storage --- lib/storage/WebStorage.js | 1 - lib/storage/providers/AsyncStorage.js | 2 -- tests/unit/onyxMultiMergeWebStorageTest.js | 4 ---- 3 files changed, 7 deletions(-) diff --git a/lib/storage/WebStorage.js b/lib/storage/WebStorage.js index 32691508..0d2d9d12 100644 --- a/lib/storage/WebStorage.js +++ b/lib/storage/WebStorage.js @@ -13,7 +13,6 @@ function raiseStorageSyncEvent(onyxKey) { } const webStorage = { - name: 'WebStorage', ...Storage, /** diff --git a/lib/storage/providers/AsyncStorage.js b/lib/storage/providers/AsyncStorage.js index 8d311202..2398916c 100644 --- a/lib/storage/providers/AsyncStorage.js +++ b/lib/storage/providers/AsyncStorage.js @@ -7,8 +7,6 @@ import _ from 'underscore'; import AsyncStorage from '@react-native-async-storage/async-storage'; const provider = { - name: 'AsyncStorage', - /** * Get the value of a given key or return `null` if it's not available in storage * @param {String} key diff --git a/tests/unit/onyxMultiMergeWebStorageTest.js b/tests/unit/onyxMultiMergeWebStorageTest.js index a9c03425..0d886044 100644 --- a/tests/unit/onyxMultiMergeWebStorageTest.js +++ b/tests/unit/onyxMultiMergeWebStorageTest.js @@ -19,12 +19,10 @@ localforageMock.setInitialMockData(initialData); describe('Onyx.mergeCollection() amd WebStorage', () => { let Onyx; - let Storage; beforeAll(() => { jest.mock('../../lib/storage'); Onyx = require('../../index').default; - Storage = require('../../lib/storage').default; jest.useRealTimers(); Onyx.init({ @@ -37,8 +35,6 @@ describe('Onyx.mergeCollection() amd WebStorage', () => { afterEach(() => Onyx.clear()); it('merges two sets of data consecutively', () => { - expect(Storage.name).toBe('WebStorage'); - const additionalDataOne = {b: 'b', c: 'c'}; Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { test_1: additionalDataOne, From 451b8c1b38cd141b9c962c611497fa97fbed512f Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 10 Mar 2022 08:54:30 -1000 Subject: [PATCH 8/9] add comment to explain mock --- tests/unit/onyxMultiMergeWebStorageTest.js | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/onyxMultiMergeWebStorageTest.js b/tests/unit/onyxMultiMergeWebStorageTest.js index 0d886044..ff38f972 100644 --- a/tests/unit/onyxMultiMergeWebStorageTest.js +++ b/tests/unit/onyxMultiMergeWebStorageTest.js @@ -21,6 +21,7 @@ describe('Onyx.mergeCollection() amd WebStorage', () => { let Onyx; beforeAll(() => { + // Force using WebStorage provider for these tests jest.mock('../../lib/storage'); Onyx = require('../../index').default; jest.useRealTimers(); From f6bad97f5e8e33fd4f4958e023fa0fe766127567 Mon Sep 17 00:00:00 2001 From: Marc Glasser Date: Thu, 10 Mar 2022 13:44:48 -1000 Subject: [PATCH 9/9] Add another test --- tests/unit/onyxMultiMergeWebStorageTest.js | 76 +++++++++++++++++++++- 1 file changed, 74 insertions(+), 2 deletions(-) diff --git a/tests/unit/onyxMultiMergeWebStorageTest.js b/tests/unit/onyxMultiMergeWebStorageTest.js index ff38f972..c2d48b83 100644 --- a/tests/unit/onyxMultiMergeWebStorageTest.js +++ b/tests/unit/onyxMultiMergeWebStorageTest.js @@ -36,6 +36,17 @@ describe('Onyx.mergeCollection() amd WebStorage', () => { afterEach(() => Onyx.clear()); it('merges two sets of data consecutively', () => { + // Given initial data in storage + expect(localforageMock.storageMap.test_1).toEqual(initialTestObject); + expect(localforageMock.storageMap.test_2).toEqual(initialTestObject); + expect(localforageMock.storageMap.test_3).toEqual(initialTestObject); + + // And an empty cache values for the collection keys + expect(OnyxCache.getValue('test_1')).not.toBeDefined(); + expect(OnyxCache.getValue('test_2')).not.toBeDefined(); + expect(OnyxCache.getValue('test_3')).not.toBeDefined(); + + // When we merge additional data const additionalDataOne = {b: 'b', c: 'c'}; Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { test_1: additionalDataOne, @@ -43,6 +54,7 @@ describe('Onyx.mergeCollection() amd WebStorage', () => { test_3: additionalDataOne, }); + // And call again consecutively with different data const additionalDataTwo = {d: 'd'}; Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { test_1: additionalDataTwo, @@ -56,12 +68,72 @@ describe('Onyx.mergeCollection() amd WebStorage', () => { a: 'a', b: 'b', c: 'c', d: 'd', }; - // Expect that our new data has merged with the existing data in the cache + // Then our new data should merge with the existing data in the cache + expect(OnyxCache.getValue('test_1')).toEqual(finalObject); + expect(OnyxCache.getValue('test_2')).toEqual(finalObject); + expect(OnyxCache.getValue('test_3')).toEqual(finalObject); + + // And the storage should reflect the same state + expect(localforageMock.storageMap.test_1).toEqual(finalObject); + expect(localforageMock.storageMap.test_2).toEqual(finalObject); + expect(localforageMock.storageMap.test_3).toEqual(finalObject); + }); + }); + + it('cache updates correctly when accessed again if keys are removed or evicted', () => { + // Given empty storage + expect(localforageMock.storageMap.test_1).toBeFalsy(); + expect(localforageMock.storageMap.test_2).toBeFalsy(); + expect(localforageMock.storageMap.test_3).toBeFalsy(); + + // And an empty cache values for the collection keys + expect(OnyxCache.getValue('test_1')).toBeFalsy(); + expect(OnyxCache.getValue('test_2')).toBeFalsy(); + expect(OnyxCache.getValue('test_3')).toBeFalsy(); + + // When we merge additional data and wait for the change + const data = {a: 'a', b: 'b'}; + Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { + test_1: data, + test_2: data, + test_3: data, + }); + + return waitForPromisesToResolve() + .then(() => { + // Then the cache and storage should match + expect(OnyxCache.getValue('test_1')).toEqual(data); + expect(OnyxCache.getValue('test_2')).toEqual(data); + expect(OnyxCache.getValue('test_3')).toEqual(data); + expect(localforageMock.storageMap.test_1).toEqual(data); + expect(localforageMock.storageMap.test_2).toEqual(data); + expect(localforageMock.storageMap.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'); + OnyxCache.drop('test_2'); + OnyxCache.drop('test_3'); + + const additionalData = {c: 'c'}; + Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, { + test_1: additionalData, + test_2: additionalData, + test_3: additionalData, + }); + + return waitForPromisesToResolve(); + }) + .then(() => { + const finalObject = { + a: 'a', b: 'b', c: 'c', + }; + + // Then our new data should merge with the existing data in the cache expect(OnyxCache.getValue('test_1')).toEqual(finalObject); expect(OnyxCache.getValue('test_2')).toEqual(finalObject); expect(OnyxCache.getValue('test_3')).toEqual(finalObject); - // Check the storage to make sure our data has been saved + // And the storage should reflect the same state expect(localforageMock.storageMap.test_1).toEqual(finalObject); expect(localforageMock.storageMap.test_2).toEqual(finalObject); expect(localforageMock.storageMap.test_3).toEqual(finalObject);