Try to synchronize operations within one update

lib/Onyx.js

@@ -31,7 +31,7 @@
 const callbackToStateMapping = {};
 
 // Keeps a copy of the values of the onyx collection keys as a map for faster lookups
-let onyxCollectionKeyMap = new Map();
+let onyxCollectionKeySet = new Set();
 
 // Holds a list of keys that have been directly subscribed to or recently modified from least to most recent
 let recentlyAccessedKeys = [];
@@ -164,6 +164,10 @@
     return cache.captureTask(taskName, promise);
 }
 
+function multiGet(keys) {
+    return Promise.all(_.map(keys, (key) => get(key).then((value) => [key, value])));
+}
+
 /**
  * Returns current key names stored in persisted storage
  * @private
@@ -201,7 +205,7 @@
  * @returns {Boolean}
  */
 function isCollectionKey(key) {
-    return onyxCollectionKeyMap.has(key);
+    return onyxCollectionKeySet.has(key);
 }
 
 /**
@@ -400,10 +404,10 @@
  * @private
  * @param {String} collectionKey
  * @param {Object} partialCollection - a partial collection of grouped member keys
- * @param {boolean} [notifyRegularSubscibers=true]
+ * @param {boolean} [notifyRegularSubscribers=true]
  * @param {boolean} [notifyWithOnyxSubscibers=true]
  */
-function keysChanged(collectionKey, partialCollection, notifyRegularSubscibers = true, notifyWithOnyxSubscibers = true) {
+function keysChanged(collectionKey, partialCollection, notifyRegularSubscribers = true, notifyWithOnyxSubscibers = true) {
     // We are iterating over all subscribers similar to keyChanged(). However, we are looking for subscribers who are subscribing to either a collection key or
     // individual collection key member for the collection that is being updated. It is important to note that the collection parameter cane be a PARTIAL collection
     // and does not represent all of the combined keys and values for a collection key. It is just the "new" data that was merged in via mergeCollection().
@@ -435,7 +439,7 @@
 
         // Regular Onyx.connect() subscriber found.
         if (_.isFunction(subscriber.callback)) {
-            if (!notifyRegularSubscibers) {
+            if (!notifyRegularSubscribers) {
                 continue;
             }
 
@@ -1080,7 +1084,7 @@
 }
 
 /**
- * Notifys subscribers and writes current value to cache
+ * Notifies subscribers and writes current value to cache
  *
  * @param {String} key
  * @param {*} value
@@ -1437,24 +1441,10 @@
     });
 }
 
-/**
- * Merges a collection based on their keys
- *
- * @example
- *
- * Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, {
- *     [`${ONYXKEYS.COLLECTION.REPORT}1`]: report1,
- *     [`${ONYXKEYS.COLLECTION.REPORT}2`]: report2,
- * });
- *
- * @param {String} collectionKey e.g. `ONYXKEYS.COLLECTION.REPORT`
- * @param {Object} collection Object collection keyed by individual collection member keys and values
- * @returns {Promise}
- */
-function mergeCollection(collectionKey, collection) {
+function isValidMergeCollection(collectionKey, collection) {
     if (!_.isObject(collection) || _.isArray(collection) || _.isEmpty(collection)) {
         Logger.logInfo('mergeCollection() called with invalid or empty value. Skipping this update.');
-        return Promise.resolve();
+        return false;
     }
 
     // Confirm all the collection keys belong to the same parent
@@ -1471,9 +1461,25 @@
         hasCollectionKeyCheckFailed = true;
         Logger.logAlert(`Provided collection doesn't have all its data belonging to the same parent. CollectionKey: ${collectionKey}, DataKey: ${dataKey}`);
     });
+    return !hasCollectionKeyCheckFailed;
+}
 
-    // Gracefully handle bad mergeCollection updates so it doesn't block the merge queue
-    if (hasCollectionKeyCheckFailed) {
+/**
+ * Merges a collection based on their keys
+ *
+ * @example
+ *
+ * Onyx.mergeCollection(ONYXKEYS.COLLECTION.REPORT, {
+ *     [`${ONYXKEYS.COLLECTION.REPORT}1`]: report1,
+ *     [`${ONYXKEYS.COLLECTION.REPORT}2`]: report2,
+ * });
+ *
+ * @param {String} collectionKey e.g. `ONYXKEYS.COLLECTION.REPORT`
+ * @param {Object} collection Object collection keyed by individual collection member keys and values
+ * @returns {Promise}
+ */
+function mergeCollection(collectionKey, collection) {
+    if (!isValidMergeCollection(collectionKey, collection)) {
         return Promise.resolve();
     }
 
@@ -1546,22 +1552,38 @@
         }
     });
 
-    const promises = [];
-    let clearPromise = Promise.resolve();
+    const updateQueue = {};
+    const enqueueSetOperation = (key, value) => {
+        // If a `set` operation is enqueued, we should clear the whole queue.
+        // Since the `set` operation replaces the value entirely, there's no need to perform any previous operations.
+        updateQueue[key] = [{onyxMethod: METHOD.SET, value}];
+    };
+    const enqueueMergeOperation = (key, value) => {
+        const operation = {onyxMethod: METHOD.MERGE, value};
+        if (!updateQueue[key]) {
+            updateQueue[key] = [operation];
+        } else {
+            updateQueue[key].push(operation);
+        }
+    };
 
+    let clearPromise = Promise.resolve();
     _.each(data, ({onyxMethod, key, value}) => {
         switch (onyxMethod) {
             case METHOD.SET:
-                promises.push(() => set(key, value));
+                enqueueSetOperation(key, value);
                 break;
-            case METHOD.MERGE:
-                promises.push(() => merge(key, value));
+            case METHOD.MERGE: {
+                enqueueMergeOperation(key, value);
                 break;
+            }
             case METHOD.MERGE_COLLECTION:
-                promises.push(() => mergeCollection(key, value));
+                if (isValidMergeCollection(key, value)) {
+                    _.each(value, (_value, _key) => enqueueMergeOperation(_key, _value));
+                }
                 break;
             case METHOD.MULTI_SET:
-                promises.push(() => multiSet(value));
+                _.each(value, (_value, _key) => enqueueSetOperation(_key, _value));
                 break;
             case METHOD.CLEAR:
                 clearPromise = clear();
@@ -1571,7 +1593,99 @@
         }
     });
 
-    return clearPromise.then(() => Promise.all(_.map(promises, (p) => p())));
+    return clearPromise.then(() =>
+        multiGet(_.keys(updateQueue)).then((pairs) => {
+            const existingSnapshot = _.reduce(
+                pairs,
+                (acc, [key, value]) => {
+                    acc[key] = value;
+                    return acc;
+                },
+                {},
+            );
+
+            const updatedSnapshot = _.reduce(
+                updateQueue,
+                (acc, operations, key) => {
+                    // If the first operation is a set, we want to use that as the existing value.
+                    // Only the first operation can be a `set`, since when enqueuing the operations, we clear the whole queue each time a `set` is enqueued.
+                    const existingValue = operations[0].onyxMethod === METHOD.SET ? operations[0].value : existingSnapshot[key];
+
+                    // Remove the first operation if it was a `set`, since we already used it to get the existing value.
+                    const mergeOperations = _.filter(operations, ({onyxMethod}) => onyxMethod === METHOD.MERGE);
+
+                    // If there are no merge operations, it means only a single `set` operation was enqueued.
+                    // In this case, we should use the existing value.
+                    // Otherwise, apply the merge operations to the existing value.
+                    acc[key] = _.isEmpty(mergeOperations)
+                        ? existingValue
+                        : applyMerge(
+                              existingValue,
+                              _.map(mergeOperations, ({value}) => value),
+                              true,
+                          );
+                    return acc;
+                },
+                {},
+            );
+
+            const pairsForStorage = prepareKeyValuePairsForStorage(updatedSnapshot);
+
+            console.log('Onyx: data', data);
+            console.log('Onyx: updateQueue', updateQueue);
+            console.log('Onyx: existingSnapshot', existingSnapshot);
+            console.log('Onyx: updatedSnapshot', updatedSnapshot);
+
+            const updatesToBroadcast = [];
+            onyxCollectionKeySet.forEach((collectionKey) => {
+                const updatedCollection = _.reduce(
+                    _.keys(updatedSnapshot),
+                    (acc, key) => {
+                        if (isKeyMatch(collectionKey, key)) {
+                            acc[key] = updatedSnapshot[key];
+                            delete updatedSnapshot[key];
+                        }
+                        return acc;
+                    },
+                    {},
+                );
+
+                if (_.isEmpty(updatedCollection)) {
+                    return;
+                }
+
+                updatesToBroadcast.push(
+                    Promise.resolve().then(() => {
+                        _.each(updatedCollection, (value, key) => {
+                            cache.set(key, value);
+                        });
+                        return keysChanged(collectionKey, updatedCollection);
+                    }),
+                );
+            });
+
+            _.mapObject(updatedSnapshot, (value, key) => {
+                updatesToBroadcast.push(
+                    Promise.resolve().then(() => {
+                        const hasChanged = cache.hasValueChanged(key, value);
+                        const wasRemoved = value === null;
+
+                        // Update subscribers if the cached value has changed, or when the subscriber specifically requires
+                        // all updates regardless of value changes (indicated by initWithStoredValues set to false).
+                        if (hasChanged && !wasRemoved) {
+                            cache.set(key, value);
+                        } else {
+                            cache.addToAccessedKeys(key);
+                        }
+
+                        return keyChanged(key, value, existingSnapshot[key], (subscriber) => hasChanged || subscriber.initWithStoredValues === false);
+                    }),
+                );
+            });
+
+            return Storage.multiSet(pairsForStorage).then(() => Promise.all(updatesToBroadcast));
+        }),
+    );
 }
 
 /**
@@ -1598,7 +1712,6 @@
  * @param {Number} [options.maxCachedKeysCount=55] Sets how many recent keys should we try to keep in cache
  * Setting this to 0 would practically mean no cache
  * We try to free cache when we connect to a safe eviction key
- * @param {Boolean} [options.captureMetrics] Enables Onyx benchmarking and exposes the get/print/reset functions
  * @param {Boolean} [options.shouldSyncMultipleInstances] Auto synchronize storage events between multiple instances
  * of Onyx running in different tabs/windows. Defaults to true for platforms that support local storage (web/desktop)
  * @param {Boolean} [options.debugSetState] Enables debugging setState() calls to connected components.
@@ -1622,13 +1735,13 @@
     // We need the value of the collection keys later for checking if a
     // key is a collection. We store it in a map for faster lookup.
     const collectionValues = _.values(keys.COLLECTION);
-    onyxCollectionKeyMap = _.reduce(
+    onyxCollectionKeySet = _.reduce(
         collectionValues,
         (acc, val) => {
-            acc.set(val, true);
+            acc.add(val);
             return acc;
         },
-        new Map(),
+        new Set(),
     );
 
     // Set our default key states to use when initializing and clearing Onyx data

tests/unit/onyxTest.js

@@ -10,6 +10,8 @@ const ONYX_KEYS = {
         TEST_CONNECT_COLLECTION: 'testConnectCollection_',
         TEST_POLICY: 'testPolicy_',
         TEST_UPDATE: 'testUpdate_',
+        PEOPLE: 'people_',
+        ANIMALS: 'animals_',
     },
 };
 
@@ -908,22 +910,67 @@ describe('Onyx', () => {
     it('should return a promise that completes when all update() operations are done', () => {
         const connectionIDs = [];
 
+        const bob = `${ONYX_KEYS.COLLECTION.PEOPLE}bob`;
+        const lisa = `${ONYX_KEYS.COLLECTION.PEOPLE}lisa`;
+
+        const cat = `${ONYX_KEYS.COLLECTION.ANIMALS}cat`;
+        const dog = `${ONYX_KEYS.COLLECTION.ANIMALS}dog`;
+
         const testCallback = jest.fn();
         const otherTestCallback = jest.fn();
-        const collectionCallback = jest.fn();
-        const itemKey = `${ONYX_KEYS.COLLECTION.TEST_UPDATE}1`;
+        const peopleCollectionCallback = jest.fn();
+        const animalsCollectionCallback = jest.fn();
+        const catCallback = jest.fn();
 
         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}));
+        connectionIDs.push(Onyx.connect({key: ONYX_KEYS.COLLECTION.ANIMALS, callback: animalsCollectionCallback, waitForCollectionCallback: true}));
+        connectionIDs.push(Onyx.connect({key: ONYX_KEYS.COLLECTION.PEOPLE, callback: peopleCollectionCallback, waitForCollectionCallback: true}));
+        connectionIDs.push(Onyx.connect({key: cat, callback: catCallback}));
+
         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'}}},
+            {onyxMethod: Onyx.METHOD.MERGE, key: ONYX_KEYS.OTHER_TEST, value: {food: 'pizza'}},
+            {onyxMethod: Onyx.METHOD.MERGE, key: ONYX_KEYS.OTHER_TEST, value: {drink: 'water'}},
+            {onyxMethod: Onyx.METHOD.MERGE, key: dog, value: {sound: 'woof'}},
+            {
+                onyxMethod: Onyx.METHOD.MERGE_COLLECTION,
+                key: ONYX_KEYS.COLLECTION.ANIMALS,
+                value: {
+                    [cat]: {sound: 'meow'},
+                    [dog]: {size: 'M'},
+                },
+            },
+            {onyxMethod: Onyx.METHOD.MERGE, key: cat, value: {size: 'S'}},
+            {onyxMethod: Onyx.METHOD.MERGE, key: bob, value: {car: 'sedan'}},
+            {onyxMethod: Onyx.METHOD.MERGE, key: lisa, value: {car: 'SUV', age: 21}},
+            {onyxMethod: Onyx.METHOD.MERGE, key: bob, value: {age: 25}},
         ]).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);
+            expect(animalsCollectionCallback).toHaveBeenCalledTimes(2);
+            expect(animalsCollectionCallback).toHaveBeenNthCalledWith(1, null, undefined);
+            expect(animalsCollectionCallback).toHaveBeenNthCalledWith(2, {
+                [cat]: {size: 'S', sound: 'meow'},
+                [dog]: {size: 'M', sound: 'woof'},
+            });
+
+            expect(catCallback).toHaveBeenCalledTimes(2);
+            expect(catCallback).toHaveBeenNthCalledWith(1, null, undefined);
+            expect(catCallback).toHaveBeenNthCalledWith(2, {size: 'S', sound: 'meow'}, cat);
+
+            expect(peopleCollectionCallback).toHaveBeenCalledTimes(2);
+            expect(peopleCollectionCallback).toHaveBeenNthCalledWith(1, null, undefined);
+            expect(peopleCollectionCallback).toHaveBeenNthCalledWith(2, {
+                [bob]: {age: 25, car: 'sedan'},
+                [lisa]: {age: 21, car: 'SUV'},
+            });
+
+            expect(testCallback).toHaveBeenCalledTimes(2);
+            expect(testCallback).toHaveBeenNthCalledWith(1, null, undefined);
+            expect(testCallback).toHaveBeenNthCalledWith(2, 'taco', ONYX_KEYS.TEST_KEY);
+
+            expect(otherTestCallback).toHaveBeenCalledTimes(2);
+            expect(otherTestCallback).toHaveBeenNthCalledWith(1, 42, ONYX_KEYS.OTHER_TEST);
+            expect(otherTestCallback).toHaveBeenNthCalledWith(2, {food: 'pizza', drink: 'water'}, ONYX_KEYS.OTHER_TEST);
             Onyx.disconnect(connectionIDs);
         });
     });