Merge pull request #583 from fabioh8010/bugfix/49609

Disable connection reuse for collection keys with waitForCollectionCallback set to `undefined`/`false`
Expensify · Sep 26, 2024 · a3e7b93 · a3e7b93
2 parents 5ed6cae + 58ebfd9
commit a3e7b93
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 6 deletions.
diff --git a/lib/OnyxConnectionManager.ts b/lib/OnyxConnectionManager.ts
@@ -88,19 +88,25 @@ class OnyxConnectionManager {
      * according to their purpose and effect they produce in the Onyx connection.
      */
     private generateConnectionID<TKey extends OnyxKey>(connectOptions: ConnectOptions<TKey>): string {
+        const {key, initWithStoredValues, reuseConnection, waitForCollectionCallback} = connectOptions;
         let suffix = '';
 
         // We will generate a unique ID in any of the following situations:
-        // - `connectOptions.reuseConnection` is `false`. That means the subscriber explicitly wants the connection to not be reused.
-        // - `connectOptions.initWithStoredValues` is `false`. This flag changes the subscription flow when set to `false`, so the connection can't be reused.
+        // - `reuseConnection` is `false`. That means the subscriber explicitly wants the connection to not be reused.
+        // - `initWithStoredValues` is `false`. This flag changes the subscription flow when set to `false`, so the connection can't be reused.
+        // - `key` is a collection key AND `waitForCollectionCallback` is `undefined/false`. This combination needs a new connection at every subscription
+        //   in order to send all the collection entries, so the connection can't be reused.
         // - `withOnyxInstance` is defined inside `connectOptions`. That means the subscriber is a `withOnyx` HOC and therefore doesn't support connection reuse.
-        if (connectOptions.reuseConnection === false || connectOptions.initWithStoredValues === false || utils.hasWithOnyxInstance(connectOptions)) {
+        if (
+            reuseConnection === false ||
+            initWithStoredValues === false ||
+            (!utils.hasWithOnyxInstance(connectOptions) && OnyxUtils.isCollectionKey(key) && (waitForCollectionCallback === undefined || waitForCollectionCallback === false)) ||
+            utils.hasWithOnyxInstance(connectOptions)
+        ) {
             suffix += `,uniqueID=${Str.guid()}`;
         }
 
-        return `onyxKey=${connectOptions.key},initWithStoredValues=${connectOptions.initWithStoredValues ?? true},waitForCollectionCallback=${
-            connectOptions.waitForCollectionCallback ?? false
-        }${suffix}`;
+        return `onyxKey=${key},initWithStoredValues=${initWithStoredValues ?? true},waitForCollectionCallback=${waitForCollectionCallback ?? false}${suffix}`;
     }
 
     /**

diff --git a/tests/unit/OnyxConnectionManagerTest.ts b/tests/unit/OnyxConnectionManagerTest.ts
@@ -271,6 +271,43 @@ describe('OnyxConnectionManager', () => {
             expect(connectionsMap.size).toEqual(0);
         });
 
+        it("should create a separate connection to the same key when it's a collection one and waitForCollectionCallback is undefined/false", async () => {
+            const collection = {
+                [`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`]: {id: 'entry1_id', name: 'entry1_name'},
+                [`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`]: {id: 'entry2_id', name: 'entry2_name'},
+                [`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`]: {id: 'entry3_id', name: 'entry3_name'},
+            };
+
+            Onyx.mergeCollection(ONYXKEYS.COLLECTION.TEST_KEY, collection as GenericCollection);
+
+            await act(async () => waitForPromisesToResolve());
+
+            const callback1 = jest.fn();
+            const connection1 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, waitForCollectionCallback: undefined, callback: callback1});
+
+            await act(async () => waitForPromisesToResolve());
+
+            expect(callback1).toHaveBeenCalledTimes(3);
+            expect(callback1).toHaveBeenNthCalledWith(1, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry1`);
+            expect(callback1).toHaveBeenNthCalledWith(2, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry2`);
+            expect(callback1).toHaveBeenNthCalledWith(3, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry3`);
+
+            const callback2 = jest.fn();
+            const connection2 = connectionManager.connect({key: ONYXKEYS.COLLECTION.TEST_KEY, waitForCollectionCallback: false, callback: callback2});
+
+            expect(connection1.id).not.toEqual(connection2.id);
+            expect(connectionsMap.size).toEqual(2);
+            expect(connectionsMap.has(connection1.id)).toBeTruthy();
+            expect(connectionsMap.has(connection2.id)).toBeTruthy();
+
+            await act(async () => waitForPromisesToResolve());
+
+            expect(callback2).toHaveBeenCalledTimes(3);
+            expect(callback2).toHaveBeenNthCalledWith(1, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry1`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry1`);
+            expect(callback2).toHaveBeenNthCalledWith(2, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry2`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry2`);
+            expect(callback2).toHaveBeenNthCalledWith(3, collection[`${ONYXKEYS.COLLECTION.TEST_KEY}entry3`], `${ONYXKEYS.COLLECTION.TEST_KEY}entry3`);
+        });
+
         it('should not throw any errors when passing an undefined connection or trying to access an inexistent one inside disconnect()', () => {
             expect(connectionsMap.size).toEqual(0);