Skip to content

Commit

Permalink
Update getCollectionKey() to throw error if key is not a collection one
Browse files Browse the repository at this point in the history
  • Loading branch information
fabioh8010 committed Sep 16, 2024
1 parent 5c84ef0 commit 08e795b
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 55 deletions.
8 changes: 5 additions & 3 deletions API-INTERNAL.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,9 @@ or if the provided key is a collection member key (in case our configured key is
<p>For example:</p>
<ul>
<li><code>getCollectionKey(&quot;report_123&quot;)</code> would return &quot;report_&quot;</li>
<li><code>getCollectionKey(&quot;report&quot;)</code> would return &quot;report&quot;</li>
<li><code>getCollectionKey(&quot;report_&quot;)</code> would return &quot;report_&quot;</li>
<li><code>getCollectionKey(&quot;report_-1_something&quot;)</code> would return &quot;report_&quot;</li>
<li><code>getCollectionKey(&quot;sharedNVP_user_-1_something&quot;)</code> would return &quot;sharedNVP_user_&quot;</li>
</ul>
</dd>
<dt><a href="#tryGetCachedValue">tryGetCachedValue()</a></dt>
Expand Down Expand Up @@ -297,11 +298,12 @@ It extracts the non-numeric collection identifier of a given key.

For example:
- `getCollectionKey("report_123")` would return "report_"
- `getCollectionKey("report")` would return "report"
- `getCollectionKey("report_")` would return "report_"
- `getCollectionKey("report_-1_something")` would return "report_"
- `getCollectionKey("sharedNVP_user_-1_something")` would return "sharedNVP_user_"

**Kind**: global function
**Returns**: <code>string</code> - The pure key without any numeric
**Returns**: <code>string</code> - The plain collection key.

| Param | Type | Description |
| --- | --- | --- |
Expand Down
11 changes: 9 additions & 2 deletions lib/Onyx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -479,8 +479,15 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise<void> {
if (newValue !== oldValue) {
cache.set(key, newValue);

const collectionKey = OnyxUtils.getCollectionKey(key);
if (OnyxUtils.isCollectionKey(collectionKey)) {
let collectionKey: string | undefined;
try {
collectionKey = OnyxUtils.getCollectionKey(key);
} catch (e) {
// If getCollectionKey() throws an error it means the key is not a collection key.
collectionKey = undefined;
}

if (collectionKey) {
if (!keyValuesToResetAsCollection[collectionKey]) {
keyValuesToResetAsCollection[collectionKey] = {};
}
Expand Down
23 changes: 11 additions & 12 deletions lib/OnyxUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -422,11 +422,6 @@ function isCollectionMemberKey<TCollectionKey extends CollectionKeyBase>(collect
*/
function splitCollectionMemberKey<TKey extends CollectionKey, CollectionKeyType = TKey extends `${infer Prefix}_${string}` ? `${Prefix}_` : never>(key: TKey): [CollectionKeyType, string] {
const collectionKey = getCollectionKey(key);

if (key === collectionKey && !isCollectionKey(key)) {
throw new Error(`Invalid '${key}' key provided, only collection keys are allowed.`);
}

return [collectionKey as CollectionKeyType, key.slice(collectionKey.length)];
}

Expand All @@ -448,13 +443,12 @@ function isSafeEvictionKey(testKey: OnyxKey): boolean {
*
* For example:
* - `getCollectionKey("report_123")` would return "report_"
* - `getCollectionKey("report")` would return "report"
* - `getCollectionKey("report_")` would return "report_"
* - `getCollectionKey("report_-1_something")` would return "report_"
* - `getCollectionKey("sharedNVP_user_-1_something")` would return "sharedNVP_user_"
*
* @param {OnyxKey} key - The key to process.
* @return {string} The pure key without any numeric
* @return {string} The plain collection key.
*/
function getCollectionKey(key: OnyxKey): string {
// Start by finding the position of the last underscore in the string
Expand All @@ -474,7 +468,7 @@ function getCollectionKey(key: OnyxKey): string {
lastUnderscoreIndex = key.lastIndexOf('_', lastUnderscoreIndex - 1);
}

return key;
throw new Error(`Invalid '${key}' key provided, only collection keys are allowed.`);
}

/**
Expand Down Expand Up @@ -803,12 +797,17 @@ function keyChanged<TKey extends OnyxKey>(
// 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 = onyxKeyToSubscriptionIDs.get(key) ?? [];
const collectionKey = getCollectionKey(key);
const plainCollectionKey = collectionKey.lastIndexOf('_') !== -1 ? collectionKey : undefined;
let collectionKey: string | undefined;
try {
collectionKey = getCollectionKey(key);
} catch (e) {
// If getCollectionKey() throws an error it means the key is not a collection key.
collectionKey = undefined;
}

if (plainCollectionKey) {
if (collectionKey) {
// Getting the collection key from the specific key because only collection keys were stored in the mapping.
stateMappingKeys = [...stateMappingKeys, ...(onyxKeyToSubscriptionIDs.get(plainCollectionKey) ?? [])];
stateMappingKeys = [...stateMappingKeys, ...(onyxKeyToSubscriptionIDs.get(collectionKey) ?? [])];
if (stateMappingKeys.length === 0) {
return;
}
Expand Down
88 changes: 50 additions & 38 deletions tests/unit/onyxUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,50 +17,62 @@ Onyx.init({
beforeEach(() => Onyx.clear());

describe('OnyxUtils', () => {
describe('splitCollectionMemberKey should return correct values', () => {
const dataResult: Record<string, [string, string]> = {
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'],
test___FAKE__: ['test_', '__FAKE__'],
'test_-1_something': ['test_', '-1_something'],
'test_level_-1_something': ['test_level_', '-1_something'],
};
describe('splitCollectionMemberKey', () => {
describe('should return correct values', () => {
const dataResult: Record<string, [string, string]> = {
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'],
test___FAKE__: ['test_', '__FAKE__'],
'test_-1_something': ['test_', '-1_something'],
'test_level_-1_something': ['test_level_', '-1_something'],
};

it.each(Object.keys(dataResult))('%s', (key) => {
const [collectionKey, id] = OnyxUtils.splitCollectionMemberKey(key);
expect(collectionKey).toEqual(dataResult[key][0]);
expect(id).toEqual(dataResult[key][1]);
it.each(Object.keys(dataResult))('%s', (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(ONYXKEYS.TEST_KEY);
}).toThrowError("Invalid 'test' key provided, only collection keys are allowed.");
expect(() => {
OnyxUtils.splitCollectionMemberKey('');
}).toThrowError("Invalid '' key provided, only collection keys are allowed.");
it('should throw error if key does not contain underscore', () => {
expect(() => {
OnyxUtils.splitCollectionMemberKey(ONYXKEYS.TEST_KEY);
}).toThrowError("Invalid 'test' key provided, only collection keys are allowed.");
expect(() => {
OnyxUtils.splitCollectionMemberKey('');
}).toThrowError("Invalid '' key provided, only collection keys are allowed.");
});
});

describe('getCollectionKey should return correct values', () => {
const dataResult: Record<string, string> = {
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_',
test___FAKE__: 'test_',
'test_-1_something': 'test_',
'test_level_-1_something': 'test_level_',
};
describe('getCollectionKey', () => {
describe('should return correct values', () => {
const dataResult: Record<string, string> = {
test_: 'test_',
test_level_: 'test_level_',
test_level_1: 'test_level_',
test_level_2: 'test_level_',
test_level_last_3: 'test_level_last_',
test___FAKE__: 'test_',
'test_-1_something': 'test_',
'test_level_-1_something': 'test_level_',
};

it.each(Object.keys(dataResult))('%s', (key) => {
const collectionKey = OnyxUtils.getCollectionKey(key);
expect(collectionKey).toEqual(dataResult[key]);
});
});

it.each(Object.keys(dataResult))('%s', (key) => {
const collectionKey = OnyxUtils.getCollectionKey(key);
expect(collectionKey).toEqual(dataResult[key]);
it('should throw error if key does not contain underscore', () => {
expect(() => {
OnyxUtils.getCollectionKey(ONYXKEYS.TEST_KEY);
}).toThrowError("Invalid 'test' key provided, only collection keys are allowed.");
expect(() => {
OnyxUtils.getCollectionKey('');
}).toThrowError("Invalid '' key provided, only collection keys are allowed.");
});
});
});

0 comments on commit 08e795b

Please sign in to comment.