Skip to content

Commit

Permalink
Merge pull request #272 from Expensify/cmartins-fixOnyx
Browse files Browse the repository at this point in the history
Gracefully handle bad mergeCollection
  • Loading branch information
mountiny authored Jul 12, 2023
2 parents f2cc880 + ad1a66d commit ed5dc52
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 8 deletions.
10 changes: 8 additions & 2 deletions lib/Onyx.js
Original file line number Diff line number Diff line change
Expand Up @@ -1129,14 +1129,20 @@ function mergeCollection(collectionKey, collection) {
}

// Confirm all the collection keys belong to the same parent
let hasCollectionKeyCheckFailed = false;
_.each(collection, (_data, dataKey) => {
if (isKeyMatch(collectionKey, dataKey)) {
return;
}

throw new Error(`Provided collection doesn't have all its data belonging to the same parent. CollectionKey: ${collectionKey}, DataKey: ${dataKey}`);
hasCollectionKeyCheckFailed = true;
Logger.logAlert(`Provided collection doesn't have all its data belonging to the same parent. CollectionKey: ${collectionKey}, DataKey: ${dataKey}`);
});

// Gracefully handle bad mergeCollection updates so it doesn't block the merge queue
if (hasCollectionKeyCheckFailed) {
return Promise.resolve();
}

return getAllKeys()
.then((persistedKeys) => {
// Split to keys that exist in storage and keys that don't
Expand Down
18 changes: 12 additions & 6 deletions tests/unit/onyxTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -306,12 +306,18 @@ describe('Onyx', () => {
});
});

it('should throw error when a key not belonging to collection key is present in mergeCollection', () => {
try {
Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {test_1: {ID: 123}, notMyTest: {beep: 'boop'}});
} catch (error) {
expect(error.message).toEqual(`Provided collection doesn't have all its data belonging to the same parent. CollectionKey: ${ONYX_KEYS.COLLECTION.TEST_KEY}, DataKey: notMyTest`);
}
it('should skip the update when a key not belonging to collection key is present in mergeCollection', () => {
const valuesReceived = {};
connectionID = Onyx.connect({
key: ONYX_KEYS.COLLECTION.TEST_KEY,
initWithStoredValues: false,
callback: (data, key) => valuesReceived[key] = data,
});

return Onyx.mergeCollection(ONYX_KEYS.COLLECTION.TEST_KEY, {test_1: {ID: 123}, notMyTest: {beep: 'boop'}})
.then(() => {
expect(valuesReceived).toEqual({});
});
});

it('should return full object to callback when calling mergeCollection()', () => {
Expand Down

0 comments on commit ed5dc52

Please sign in to comment.