From 192ac17c758edc79adc1a576e4a2e24dcb2748a1 Mon Sep 17 00:00:00 2001 From: Edu Date: Tue, 26 Mar 2024 12:23:13 +0100 Subject: [PATCH 01/10] Storing key/connectionID for faster lookup --- lib/Onyx.ts | 4 ++++ lib/OnyxUtils.d.ts | 10 ++++++++++ lib/OnyxUtils.js | 45 +++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 3d3992532..1e6e8bbc1 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -94,6 +94,8 @@ function connect(mapping: ConnectOptions): number { callbackToStateMapping[connectionID] = mapping; callbackToStateMapping[connectionID].connectionID = connectionID; + OnyxUtils.storeKeyByConnections(mapping.key, callbackToStateMapping[connectionID].connectionID); + if (mapping.initWithStoredValues === false) { return connectionID; } @@ -194,6 +196,8 @@ function disconnect(connectionID: number, keyToRemoveFromEvictionBlocklist?: Ony OnyxUtils.removeFromEvictionBlockList(keyToRemoveFromEvictionBlocklist, connectionID); } + OnyxUtils.deleteKeyByConnections(lastConnectionID); + delete callbackToStateMapping[connectionID]; } diff --git a/lib/OnyxUtils.d.ts b/lib/OnyxUtils.d.ts index e3dc63162..06a594c13 100644 --- a/lib/OnyxUtils.d.ts +++ b/lib/OnyxUtils.d.ts @@ -51,6 +51,14 @@ declare function getCallbackToStateMapping(): Record>; /** Getter - returns the default key states. */ declare function getDefaultKeyStates(): Record>; +/** + * + * @param connectionID connectionID of the subscriber + * @param key key that the subscriber is connected to + */ +declare function storeKeyByConnections(connectionID: string, key: OnyxKey): void; + +declare function deleteKeyByConnections(connectionID: number): void; /** * Sets the initial values for the Onyx store * @@ -315,6 +323,8 @@ const OnyxUtils = { prepareKeyValuePairsForStorage, applyMerge, initializeWithDefaultKeyStates, + storeKeyByConnections, + deleteKeyByConnections, } as const; export default OnyxUtils; diff --git a/lib/OnyxUtils.js b/lib/OnyxUtils.js index bf559ed85..ad644f4c9 100644 --- a/lib/OnyxUtils.js +++ b/lib/OnyxUtils.js @@ -29,6 +29,9 @@ const callbackToStateMapping = {}; // Keeps a copy of the values of the onyx collection keys as a map for faster lookups let onyxCollectionKeyMap = new Map(); +// Holds a mapping of the connected key to the connectionID for faster lookups +const onyxKeyToConnectionIDs = new Map(); + // Holds a list of keys that have been directly subscribed to or recently modified from least to most recent let recentlyAccessedKeys = []; @@ -311,6 +314,21 @@ function isSafeEvictionKey(testKey) { return _.some(evictionAllowList, (key) => isKeyMatch(key, testKey)); } +function storeKeyByConnections(key, connectionID) { + if (!onyxKeyToConnectionIDs.has(key)) { + onyxKeyToConnectionIDs.set(key, []); + } + onyxKeyToConnectionIDs.get(key).push(connectionID); +} + +function deleteKeyByConnections(connectionID) { + const subscriber = callbackToStateMapping[connectionID]; + + if (subscriber && onyxKeyToConnectionIDs.has(subscriber.key)) { + onyxKeyToConnectionIDs.set(subscriber.key, _.without(onyxKeyToConnectionIDs.get(subscriber.key), connectionID)); + } +} + /** * Tries to get a value from the cache. If the value is not present in cache it will return the default value or undefined. * If the requested key is a collection, it will return an object with all the collection members. @@ -429,6 +447,13 @@ function addToEvictionBlockList(key, connectionID) { evictionBlocklist[key].push(connectionID); } +function getPureKey(key) { + if (!key) { + return ''; + } + return key.replace(/\d+/g, ''); +} + /** * Take all the keys that are safe to evict and add them to * the recently accessed list when initializing the app. This @@ -493,7 +518,12 @@ function keysChanged(collectionKey, partialCollection, notifyRegularSubscibers = // 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(). - const stateMappingKeys = _.keys(callbackToStateMapping); + const stateMappingKeys = onyxKeyToConnectionIDs.get(collectionKey); + + // If the key was not found in the mapping then we can skip notifying subscribers + if (!stateMappingKeys) { + return; + } for (let i = 0; i < stateMappingKeys.length; i++) { const subscriber = callbackToStateMapping[stateMappingKeys[i]]; if (!subscriber) { @@ -670,7 +700,16 @@ function keyChanged(key, data, prevData, canUpdateSubscriber = () => true, notif // We are iterating over all subscribers to see if they are interested in the key that has just changed. If the subscriber's key is a collection key then we will // notify them if the key that changed is a collection member. Or if it is a regular key notify them when there is an exact match. Depending on whether the subscriber // was connected via withOnyx we will call setState() directly on the withOnyx instance. If it is a regular connection we will pass the data to the provided callback. - const stateMappingKeys = _.keys(callbackToStateMapping); + let stateMappingKeys = onyxKeyToConnectionIDs.get(key); + + if (!stateMappingKeys) { + // Getting the collection key from the specific key 'cos only collection keys were stored in the mapping. + stateMappingKeys = onyxKeyToConnectionIDs.get(getPureKey(key)); + if (!stateMappingKeys) { + return; + } + } + for (let i = 0; i < stateMappingKeys.length; i++) { const subscriber = callbackToStateMapping[stateMappingKeys[i]]; if (!subscriber || !isKeyMatch(subscriber.key, key) || !canUpdateSubscriber(subscriber)) { @@ -1187,6 +1226,8 @@ const OnyxUtils = { prepareKeyValuePairsForStorage, applyMerge, initializeWithDefaultKeyStates, + storeKeyByConnections, + deleteKeyByConnections, }; export default OnyxUtils; From 7207211e24a09fc4311d9e91e47973dd6f356e12 Mon Sep 17 00:00:00 2001 From: Edu Date: Wed, 3 Apr 2024 11:46:42 +0200 Subject: [PATCH 02/10] Documented new functions --- lib/OnyxUtils.d.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/OnyxUtils.d.ts b/lib/OnyxUtils.d.ts index 06a594c13..67dfb4e93 100644 --- a/lib/OnyxUtils.d.ts +++ b/lib/OnyxUtils.d.ts @@ -52,13 +52,20 @@ declare function getCallbackToStateMapping(): Record>; declare function getDefaultKeyStates(): Record>; /** + * Stores a connection ID associated with a given key. * * @param connectionID connectionID of the subscriber * @param key key that the subscriber is connected to */ declare function storeKeyByConnections(connectionID: string, key: OnyxKey): void; +/** + * Deletes a connection ID associated with its corresponding key. + * + * @param {number} connectionID - The connection ID to be deleted. + */ declare function deleteKeyByConnections(connectionID: number): void; + /** * Sets the initial values for the Onyx store * From f69045f55e353583971b42b24c2da905ebb2b55c Mon Sep 17 00:00:00 2001 From: Edu Date: Thu, 9 May 2024 17:16:37 +0200 Subject: [PATCH 03/10] Updated JDocs --- lib/Onyx.ts | 3 +++ lib/OnyxUtils.ts | 23 ++++++++++++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index a3a3d150a..20a9e16a1 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -99,6 +99,9 @@ function connect(connectOptions: ConnectOptions): nu callbackToStateMapping[connectionID] = mapping as Mapping; callbackToStateMapping[connectionID].connectionID = connectionID; + // When keyChanged is called, the key is passed and looks through all the callbackToStateMapping the right key + // to avoid having to loop through all the keys all the time (even when just one connection belongs to one key), + // we store the key by connectionID, to access the specific list of connectionIDs OnyxUtils.storeKeyByConnections(mapping.key, callbackToStateMapping[connectionID].connectionID); if (mapping.initWithStoredValues === false) { diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 12a1986a9..a5a20520c 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -301,8 +301,8 @@ function isSafeEvictionKey(testKey: OnyxKey): boolean { /** * Stores a connection ID associated with a given key. * - * @param connectionID connectionID of the subscriber - * @param key key that the subscriber is connected to + * @param connectionID - a connection ID of the subscriber + * @param key - a key that the subscriber is connected to */ function storeKeyByConnections(key: OnyxKey, connectionID: number) { if (!onyxKeyToConnectionIDs.has(key)) { @@ -410,7 +410,18 @@ function addToEvictionBlockList(key: OnyxKey, connectionID: number): void { evictionBlocklist[key].push(connectionID); } - +/** + * It extracts the pure, non-numeric part of a given key. + * + * For example: + * - `getPureKey("report_123")` would return "report_" + * - `getPureKey("report")` would return "report" + * - `getPureKey("report_")` would return "report_" + * - `getPureKey(null)` would return "" + * + * @param {OnyxKey} key - The key to process. + * @return {string} The pure key without any numeric + */ function getPureKey(key: OnyxKey): string { if (!key) { return ''; @@ -663,13 +674,15 @@ function keyChanged( removeLastAccessedKey(key); } - // We are iterating over all subscribers to see if they are interested in the key that has just changed. If the subscriber's key is a collection key then we will + // We get the subscribers interested in the key that has just changed. If the subscriber's key is a collection key then we will // notify them if the key that changed is a collection member. Or if it is a regular key notify them when there is an exact match. Depending on whether the subscriber // was connected via withOnyx we will call setState() directly on the withOnyx instance. If it is a regular connection we will pass the data to the provided callback. + // Given the amount of times this function is called we need to make sure we are not iterating over all subscribers every time. On the other hand, we don't need to + // do the same in keysChanged, because we only call that function when a collection key changes, and it doesn't happen that often. let stateMappingKeys = onyxKeyToConnectionIDs.get(key); if (!stateMappingKeys) { - // Getting the collection key from the specific key 'cos only collection keys were stored in the mapping. + // Getting the collection key from the specific key because only collection keys were stored in the mapping. stateMappingKeys = onyxKeyToConnectionIDs.get(getPureKey(key)); if (!stateMappingKeys) { return; From fba03310b5875cc8b39c34f90924d070bb422520 Mon Sep 17 00:00:00 2001 From: Edu Date: Thu, 9 May 2024 20:32:13 +0200 Subject: [PATCH 04/10] add clarification to a comment --- lib/Onyx.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 20a9e16a1..73a8dbef1 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -99,9 +99,9 @@ function connect(connectOptions: ConnectOptions): nu callbackToStateMapping[connectionID] = mapping as Mapping; callbackToStateMapping[connectionID].connectionID = connectionID; - // When keyChanged is called, the key is passed and looks through all the callbackToStateMapping the right key - // to avoid having to loop through all the keys all the time (even when just one connection belongs to one key), - // we store the key by connectionID, to access the specific list of connectionIDs + // When keyChanged is called, a key is passed and the method looks through all the Subscribers in callbackToStateMapping for the matching key to get the connectionID + // to avoid having to loop through all the Subscribers all the time (even when just one connection belongs to one key), + // We create a mapping from key to lists of connectionIDs to access the specific list of connectionIDs. OnyxUtils.storeKeyByConnections(mapping.key, callbackToStateMapping[connectionID].connectionID); if (mapping.initWithStoredValues === false) { From 3fb68996dd9731145f5cbe0e797af08187b37b3d Mon Sep 17 00:00:00 2001 From: Edu Date: Fri, 10 May 2024 13:15:38 +0200 Subject: [PATCH 05/10] Make it more readable and performant not using _.without --- lib/OnyxUtils.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index a5a20520c..8a296d3e5 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -320,7 +320,8 @@ function deleteKeyByConnections(connectionID: number) { const subscriber = callbackToStateMapping[connectionID]; if (subscriber && onyxKeyToConnectionIDs.has(subscriber.key)) { - onyxKeyToConnectionIDs.set(subscriber.key, _.without(onyxKeyToConnectionIDs.get(subscriber.key), connectionID)); + const updatesConnectionIDs = onyxKeyToConnectionIDs.get(subscriber.key).filter((id: number) => id !== connectionID); + onyxKeyToConnectionIDs.set(subscriber.key, updatesConnectionIDs); } } From fe17400e7c646e24678fca3244c9c8292ebe814c Mon Sep 17 00:00:00 2001 From: Edu Date: Fri, 10 May 2024 13:25:24 +0200 Subject: [PATCH 06/10] fixed typo --- lib/OnyxUtils.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 8a296d3e5..68a33c61e 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -320,8 +320,8 @@ function deleteKeyByConnections(connectionID: number) { const subscriber = callbackToStateMapping[connectionID]; if (subscriber && onyxKeyToConnectionIDs.has(subscriber.key)) { - const updatesConnectionIDs = onyxKeyToConnectionIDs.get(subscriber.key).filter((id: number) => id !== connectionID); - onyxKeyToConnectionIDs.set(subscriber.key, updatesConnectionIDs); + const updatedConnectionIDs = onyxKeyToConnectionIDs.get(subscriber.key).filter((id: number) => id !== connectionID); + onyxKeyToConnectionIDs.set(subscriber.key, updatedConnectionIDs); } } @@ -680,6 +680,7 @@ function keyChanged( // was connected via withOnyx we will call setState() directly on the withOnyx instance. If it is a regular connection we will pass the data to the provided callback. // Given the amount of times this function is called we need to make sure we are not iterating over all subscribers every time. On the other hand, we don't need to // do the same in keysChanged, because we only call that function when a collection key changes, and it doesn't happen that often. + // For performance reason, we look for the given key and later if don't find it we look for the collection key, instead of checking if it is a collection key first. let stateMappingKeys = onyxKeyToConnectionIDs.get(key); if (!stateMappingKeys) { From 238cced4f7c79cf49620090d42efd11ba346a3de Mon Sep 17 00:00:00 2001 From: Edu Date: Fri, 10 May 2024 17:21:22 +0200 Subject: [PATCH 07/10] adding support to numbers and text --- lib/OnyxUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 68a33c61e..98e69b8b9 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -427,7 +427,7 @@ function getPureKey(key: OnyxKey): string { if (!key) { return ''; } - return key.replace(/\d+/g, ''); + return key.replace(/_\w+/g, '_'); } /** From 5af3812cac5c2521a45e2172f6d14782b4eced9b Mon Sep 17 00:00:00 2001 From: Edu Date: Mon, 13 May 2024 18:00:54 +0200 Subject: [PATCH 08/10] Now getting connections from key and keys_collection --- lib/OnyxUtils.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index 98e69b8b9..836fe8e33 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -423,7 +423,7 @@ function addToEvictionBlockList(key: OnyxKey, connectionID: number): void { * @param {OnyxKey} key - The key to process. * @return {string} The pure key without any numeric */ -function getPureKey(key: OnyxKey): string { +function getCollectionKey(key: OnyxKey): string { if (!key) { return ''; } @@ -681,12 +681,14 @@ function keyChanged( // Given the amount of times this function is called we need to make sure we are not iterating over all subscribers every time. On the other hand, we don't need to // do the same in keysChanged, because we only call that function when a collection key changes, and it doesn't happen that often. // For performance reason, we look for the given key and later if don't find it we look for the collection key, instead of checking if it is a collection key first. - let stateMappingKeys = onyxKeyToConnectionIDs.get(key); + let stateMappingKeys = onyxKeyToConnectionIDs.get(key) ?? []; + const collectionKey = getCollectionKey(key); + const plainCollectionKey = collectionKey.lastIndexOf('_') !== -1 ? collectionKey : undefined; - if (!stateMappingKeys) { + if (plainCollectionKey) { // Getting the collection key from the specific key because only collection keys were stored in the mapping. - stateMappingKeys = onyxKeyToConnectionIDs.get(getPureKey(key)); - if (!stateMappingKeys) { + stateMappingKeys = [...stateMappingKeys, ...(onyxKeyToConnectionIDs.get(plainCollectionKey) ?? [])]; + if (stateMappingKeys.length === 0) { return; } } From d55dcd1dbea0eff0b911aeed422471eb24253b8a Mon Sep 17 00:00:00 2001 From: Edu Date: Fri, 21 Jun 2024 12:43:50 +0200 Subject: [PATCH 09/10] Updated JSDoc --- lib/OnyxUtils.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index eecc0bf63..f359402fe 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -428,13 +428,13 @@ function addToEvictionBlockList(key: OnyxKey, connectionID: number): void { evictionBlocklist[key].push(connectionID); } /** - * It extracts the pure, non-numeric part of a given key. + * It extracts the non-numeric collection identifier of a given key. * * For example: - * - `getPureKey("report_123")` would return "report_" - * - `getPureKey("report")` would return "report" - * - `getPureKey("report_")` would return "report_" - * - `getPureKey(null)` would return "" + * - `getCollectionKey("report_123")` would return "report_" + * - `getCollectionKey("report")` would return "report" + * - `getCollectionKey("report_")` would return "report_" + * - `getCollectionKey(null)` would return "" * * @param {OnyxKey} key - The key to process. * @return {string} The pure key without any numeric From d4569ddd2b9ddd8733fb5b830f52e8ce9608dbbc Mon Sep 17 00:00:00 2001 From: Edu Date: Wed, 26 Jun 2024 12:21:46 +0200 Subject: [PATCH 10/10] Updated collection keys to handle multiple underscores --- lib/OnyxUtils.ts | 13 ++++++----- tests/unit/onyxUtilsTest.ts | 44 +++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 5 deletions(-) create mode 100644 tests/unit/onyxUtilsTest.ts diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index f359402fe..530f4ec68 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -292,7 +292,7 @@ function isCollectionMemberKey(collect * @returns A tuple where the first element is the collection part and the second element is the ID part. */ function splitCollectionMemberKey(key: TKey): [TKey extends `${infer Prefix}_${string}` ? `${Prefix}_` : never, string] { - const underscoreIndex = key.indexOf('_'); + const underscoreIndex = key.lastIndexOf('_'); if (underscoreIndex === -1) { throw new Error(`Invalid ${key} key provided, only collection keys are allowed.`); @@ -434,16 +434,18 @@ function addToEvictionBlockList(key: OnyxKey, connectionID: number): void { * - `getCollectionKey("report_123")` would return "report_" * - `getCollectionKey("report")` would return "report" * - `getCollectionKey("report_")` would return "report_" - * - `getCollectionKey(null)` would return "" * * @param {OnyxKey} key - The key to process. * @return {string} The pure key without any numeric */ function getCollectionKey(key: OnyxKey): string { - if (!key) { - return ''; + const underscoreIndex = key.lastIndexOf('_'); + + if (underscoreIndex === -1) { + return key; } - return key.replace(/_\w+/g, '_'); + + return key.substring(0, underscoreIndex + 1); } /** @@ -1203,6 +1205,7 @@ const OnyxUtils = { keyChanged, sendDataToConnection, addKeyToRecentlyAccessedIfNeeded, + getCollectionKey, getCollectionDataAndSendAsObject, scheduleSubscriberUpdate, scheduleNotifyCollectionSubscribers, diff --git a/tests/unit/onyxUtilsTest.ts b/tests/unit/onyxUtilsTest.ts new file mode 100644 index 000000000..d9b675142 --- /dev/null +++ b/tests/unit/onyxUtilsTest.ts @@ -0,0 +1,44 @@ +import OnyxUtils from '../../lib/OnyxUtils'; + +describe('OnyxUtils', () => { + it('splitCollectionMemberKey should return correct values', () => { + const dataResult: Record = { + test_: ['test_', ''], + test_level_: ['test_level_', ''], + test_level_1: ['test_level_', '1'], + test_level_2: ['test_level_', '2'], + test_level_last_3: ['test_level_last_', '3'], + }; + + Object.keys(dataResult).forEach((key) => { + const [collectionKey, id] = OnyxUtils.splitCollectionMemberKey(key); + expect(collectionKey).toEqual(dataResult[key][0]); + expect(id).toEqual(dataResult[key][1]); + }); + }); + + it('splitCollectionMemberKey should throw error if key does not contain underscore', () => { + expect(() => { + OnyxUtils.splitCollectionMemberKey('test'); + }).toThrowError('Invalid test key provided, only collection keys are allowed.'); + expect(() => { + OnyxUtils.splitCollectionMemberKey(''); + }).toThrowError('Invalid key provided, only collection keys are allowed.'); + }); + + it('getCollectionKey should return correct values', () => { + const dataResult: Record = { + test: 'test', + test_: 'test_', + test_level_: 'test_level_', + test_level_1: 'test_level_', + test_level_2: 'test_level_', + test_level_last_3: 'test_level_last_', + }; + + Object.keys(dataResult).forEach((key) => { + const collectionKey = OnyxUtils.getCollectionKey(key); + expect(collectionKey).toEqual(dataResult[key]); + }); + }); +});