Skip to content

Commit

Permalink
Merge pull request #206 from Expensify/marcaaron-onyxUpdatePromise
Browse files Browse the repository at this point in the history 
Make `Onyx.update()` return a promise
  • Loading branch information
@arosiclair
arosiclair authored Oct 28, 2022
2 parents d4f6e5a + 3170288 commit d3dfe61
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 31 deletions.
34 changes: 24 additions & 10 deletions lib/Onyx.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ import createDeferredTask from './createDeferredTask';
import fastMerge from './fastMerge';
import * as PerformanceUtils from './metrics/PerformanceUtils';

// Method constants
const METHOD = {
SET: 'set',
MERGE: 'merge',
MERGE_COLLECTION: 'mergecollection',
CLEAR: 'clear',
};

// Keeps track of the last connectionID that was used so we can keep incrementing it
let lastConnectionID = 0;

Expand Down Expand Up @@ -1101,36 +1109,41 @@ function mergeCollection(collectionKey, collection) {
* Insert API responses and lifecycle data into Onyx
*
* @param {Array} data An array of objects with shape {onyxMethod: oneOf('set', 'merge', 'mergeCollection'), key: string, value: *}
* @returns {Promise} resolves when all operations are complete
*/
function update(data) {
// First, validate the Onyx object is in the format we expect
_.each(data, ({onyxMethod, key}) => {
if (!_.contains(['clear', 'set', 'merge', 'mergecollection'], onyxMethod)) {
if (!_.contains([METHOD.CLEAR, METHOD.SET, METHOD.MERGE, METHOD.MERGE_COLLECTION], onyxMethod)) {
throw new Error(`Invalid onyxMethod ${onyxMethod} in Onyx update.`);
}
if (onyxMethod !== 'clear' && !_.isString(key)) {
if (onyxMethod !== METHOD.CLEAR && !_.isString(key)) {
throw new Error(`Invalid ${typeof key} key provided in Onyx update. Onyx key must be of type string.`);
}
});

const promises = [];

_.each(data, ({onyxMethod, key, value}) => {
switch (onyxMethod) {
case 'set':
set(key, value);
case METHOD.SET:
promises.push(set(key, value));
break;
case 'merge':
merge(key, value);
case METHOD.MERGE:
promises.push(merge(key, value));
break;
case 'mergecollection':
mergeCollection(key, value);
case METHOD.MERGE_COLLECTION:
promises.push(mergeCollection(key, value));
break;
case 'clear':
clear();
case METHOD.CLEAR:
promises.push(clear());
break;
default:
break;
}
});

return Promise.all(promises);
}

/**
Expand Down Expand Up @@ -1219,6 +1232,7 @@ const Onyx = {
addToEvictionBlockList,
removeFromEvictionBlockList,
isSafeEvictionKey,
METHOD,
};

/**
Expand Down
67 changes: 46 additions & 21 deletions tests/unit/onyxTest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ const ONYX_KEYS = {
OTHER_TEST: 'otherTest',
COLLECTION: {
TEST_KEY: 'test_',
TEST_CONNECT_COLLECTION: 'test_connect_collection_',
TEST_POLICY: 'test_policy_',
TEST_CONNECT_COLLECTION: 'testConnectCollection_',
TEST_POLICY: 'testPolicy_',
TEST_UPDATE: 'testUpdate_',
},
};

Expand Down Expand Up @@ -487,15 +488,15 @@ describe('Onyx', () => {

// Given some initial collection data
const initialCollectionData = {
test_connect_collection_1: {
testConnectCollection_1: {
ID: 123,
value: 'one',
},
test_connect_collection_2: {
testConnectCollection_2: {
ID: 234,
value: 'two',
},
test_connect_collection_3: {
testConnectCollection_3: {
ID: 345,
value: 'three',
},
Expand All @@ -522,8 +523,8 @@ describe('Onyx', () => {
it('should return all collection keys as a single object when updating a collection key with waitForCollectionCallback = true', () => {
const mockCallback = jest.fn();
const collectionUpdate = {
test_policy_1: {ID: 234, value: 'one'},
test_policy_2: {ID: 123, value: 'two'},
testPolicy_1: {ID: 234, value: 'one'},
testPolicy_2: {ID: 123, value: 'two'},
};

// Given an Onyx.connect call with waitForCollectionCallback=true
Expand Down Expand Up @@ -551,8 +552,8 @@ describe('Onyx', () => {
it('should send a value to Onyx.connect() when subscribing to a specific collection member key and keysChanged() is called', () => {
const mockCallback = jest.fn();
const collectionUpdate = {
test_policy_1: {ID: 234, value: 'one'},
test_policy_2: {ID: 123, value: 'two'},
testPolicy_1: {ID: 234, value: 'one'},
testPolicy_2: {ID: 123, value: 'two'},
};

// Given an Onyx.connect call with waitForCollectionCallback=true
Expand All @@ -572,17 +573,17 @@ describe('Onyx', () => {
expect(mockCallback).toHaveBeenCalledTimes(2);

// AND the value for the first call should be null since the collection was not initialized at that point
expect(mockCallback).toHaveBeenNthCalledWith(1, null, 'test_policy_1');
expect(mockCallback).toHaveBeenNthCalledWith(1, null, 'testPolicy_1');

// AND the value for the second call should be collectionUpdate since the collection was updated
expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate.test_policy_1, 'test_policy_1');
expect(mockCallback).toHaveBeenNthCalledWith(2, collectionUpdate.testPolicy_1, 'testPolicy_1');
});
});

it('should return all collection keys as a single object for subscriber using waitForCollectionCallback when a single collection member key is updated', () => {
const mockCallback = jest.fn();
const collectionUpdate = {
test_policy_1: {ID: 234, value: 'one'},
testPolicy_1: {ID: 234, value: 'one'},
};

// Given an Onyx.connect call with waitForCollectionCallback=true
Expand All @@ -594,7 +595,7 @@ describe('Onyx', () => {
return waitForPromisesToResolve()

// When mergeCollection is called with an updated collection
.then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1))
.then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.testPolicy_1))
.then(() => {
// Then we expect the callback to have called twice, once for the initial connect call + once for the collection update
expect(mockCallback).toHaveBeenCalledTimes(2);
Expand All @@ -617,7 +618,7 @@ describe('Onyx', () => {
it('should not update a subscriber if the value in the cache has not changed at all', () => {
const mockCallback = jest.fn();
const collectionUpdate = {
test_policy_1: {ID: 234, value: 'one'},
testPolicy_1: {ID: 234, value: 'one'},
};

// Given an Onyx.connect call with waitForCollectionCallback=true
Expand All @@ -629,7 +630,7 @@ describe('Onyx', () => {
return waitForPromisesToResolve()

// When merge is called with an updated collection
.then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1))
.then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.testPolicy_1))
.then(() => {
// Then we expect the callback to have called twice, once for the initial connect call + once for the collection update
expect(mockCallback).toHaveBeenCalledTimes(2);
Expand All @@ -639,14 +640,14 @@ describe('Onyx', () => {
})

// When merge is called again with the same collection not modified
.then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1))
.then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.testPolicy_1))
.then(() => {
// Then we should not expect another invocation of the callback
expect(mockCallback).toHaveBeenCalledTimes(2);
})

// When merge is called again with an object of equivalent value but not the same reference
.then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, _.clone(collectionUpdate.test_policy_1)))
.then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, _.clone(collectionUpdate.testPolicy_1)))
.then(() => {
// Then we should not expect another invocation of the callback
expect(mockCallback).toHaveBeenCalledTimes(2);
Expand All @@ -656,7 +657,7 @@ describe('Onyx', () => {
it('should update subscriber if the value in the cache has not changed at all but initWithStoredValues === false', () => {
const mockCallback = jest.fn();
const collectionUpdate = {
test_policy_1: {ID: 234, value: 'one'},
testPolicy_1: {ID: 234, value: 'one'},
};

// Given an Onyx.connect call with waitForCollectionCallback=true
Expand All @@ -669,7 +670,7 @@ describe('Onyx', () => {
return waitForPromisesToResolve()

// When merge is called with an updated collection
.then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1))
.then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.testPolicy_1))
.then(() => {
// Then we expect the callback to have called once. 0 times the initial connect call + 1 time for the merge()
expect(mockCallback).toHaveBeenCalledTimes(1);
Expand All @@ -679,17 +680,41 @@ describe('Onyx', () => {
})

// When merge is called again with the same collection not modified
.then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.test_policy_1))
.then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, collectionUpdate.testPolicy_1))
.then(() => {
// Then we should expect another invocation of the callback because initWithStoredValues = false
expect(mockCallback).toHaveBeenCalledTimes(2);
})

// When merge is called again with an object of equivalent value but not the same reference
.then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, _.clone(collectionUpdate.test_policy_1)))
.then(() => Onyx.merge(`${ONYX_KEYS.COLLECTION.TEST_POLICY}${1}`, _.clone(collectionUpdate.testPolicy_1)))
.then(() => {
// Then we should expect another invocation of the callback because initWithStoredValues = false
expect(mockCallback).toHaveBeenCalledTimes(3);
});
});

it('should return a promise that completes when all update() operations are done', () => {
const connectionIDs = [];

const testCallback = jest.fn();
const otherTestCallback = jest.fn();
const collectionCallback = jest.fn();
const itemKey = `${ONYX_KEYS.COLLECTION.TEST_UPDATE}1`;

connectionIDs.push(Onyx.connect({key: ONYX_KEYS.TEST_KEY, callback: testCallback}));
connectionIDs.push(Onyx.connect({key: ONYX_KEYS.OTHER_TEST, callback: otherTestCallback}));
connectionIDs.push(Onyx.connect({key: ONYX_KEYS.COLLECTION.TEST_UPDATE, callback: collectionCallback, waitForCollectionCallback: true}));
return Onyx.update([
{onyxMethod: Onyx.METHOD.SET, key: ONYX_KEYS.TEST_KEY, value: 'taco'},
{onyxMethod: Onyx.METHOD.MERGE, key: ONYX_KEYS.OTHER_TEST, value: 'pizza'},
{onyxMethod: Onyx.METHOD.MERGE_COLLECTION, key: ONYX_KEYS.COLLECTION.TEST_UPDATE, value: {[itemKey]: {a: 'a'}}},
])
.then(() => {
expect(collectionCallback).toHaveBeenNthCalledWith(1, {[itemKey]: {a: 'a'}});
expect(testCallback).toHaveBeenNthCalledWith(1, 'taco', ONYX_KEYS.TEST_KEY);
expect(otherTestCallback).toHaveBeenNthCalledWith(1, 'pizza', ONYX_KEYS.OTHER_TEST);
Onyx.disconnect(connectionIDs);
});
});
});

0 comments on commit d3dfe61

Please sign in to comment.