From 34065fed475adbc6ef9f0f0ffec74939c89338df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Tue, 30 Jul 2024 21:23:55 +0200 Subject: [PATCH 1/6] fix: reset keys to initial state if they were set to null --- lib/Onyx.ts | 12 ++++++++---- tests/unit/onyxTest.ts | 30 ++++++++++++++++-------------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index da3d12b11..f927f3530 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -554,22 +554,27 @@ function mergeCollection(collectionKey: TK * @param keysToPreserve is a list of ONYXKEYS that should not be cleared with the rest of the data */ function clear(keysToPreserve: OnyxKey[] = []): Promise { + const defaultKeyStates = OnyxUtils.getDefaultKeyStates(); + const initialKeys = Object.keys(defaultKeyStates); + return OnyxUtils.getAllKeys() - .then((keys) => { + .then((cachedKeys) => { cache.clearNullishStorageKeys(); const keysToBeClearedFromStorage: OnyxKey[] = []; const keyValuesToResetAsCollection: Record> = {}; const keyValuesToResetIndividually: KeyValueMapping = {}; + const allKeys = new Set([...cachedKeys, ...initialKeys]); + // The only keys that should not be cleared are: // 1. Anything specifically passed in keysToPreserve (because some keys like language preferences, offline // status, or activeClients need to remain in Onyx even when signed out) // 2. Any keys with a default state (because they need to remain in Onyx as their default, and setting them // to null would cause unknown behavior) - keys.forEach((key) => { + // 2.1 However, if a default key was explicitly set to null, we need to reset it to the default value + allKeys.forEach((key) => { const isKeyToPreserve = keysToPreserve.includes(key); - const defaultKeyStates = OnyxUtils.getDefaultKeyStates(); const isDefaultKey = key in defaultKeyStates; // If the key is being removed or reset to default: @@ -611,7 +616,6 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { updatePromises.push(OnyxUtils.scheduleNotifyCollectionSubscribers(key, value)); }); - const defaultKeyStates = OnyxUtils.getDefaultKeyStates(); const defaultKeyValuePairs = Object.entries( Object.keys(defaultKeyStates) .filter((key) => !keysToPreserve.includes(key)) diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index d4879ee90..5b8ceb8d6 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -51,15 +51,18 @@ describe('Onyx', () => { expect(keys.has(ONYX_KEYS.OTHER_TEST)).toBe(true); return Onyx.set(ONYX_KEYS.OTHER_TEST, null); }) + // Checks if cache value is removed. .then(() => { - // Checks if cache value is removed. - expect(cache.getAllKeys().size).toBe(0); - - // When cache keys length is 0, we fetch the keys from storage. + expect(cache.get(ONYX_KEYS.OTHER_TEST)).toBeUndefined(); return OnyxUtils.getAllKeys(); }) .then((keys) => { expect(keys.has(ONYX_KEYS.OTHER_TEST)).toBe(false); + }) + // Expect to reset to initial key value when calling Onyx.clear() + .then(() => Onyx.clear()) + .then(() => { + expect(cache.get(ONYX_KEYS.OTHER_TEST)).toBe(42); })); it('should set a simple key', () => { @@ -167,32 +170,31 @@ describe('Onyx', () => { }, }); - let otherTestValue: unknown; - const mockCallback = jest.fn((value) => { - otherTestValue = value; - }); + const mockCallback = jest.fn(); const otherTestConnectionID = Onyx.connect({ key: ONYX_KEYS.OTHER_TEST, callback: mockCallback, }); return waitForPromisesToResolve() + .then(() => { + expect(mockCallback).toHaveBeenCalledTimes(1); + expect(mockCallback).toHaveBeenCalledWith(42, ONYX_KEYS.OTHER_TEST); + mockCallback.mockClear(); + }) .then(() => Onyx.set(ONYX_KEYS.TEST_KEY, 'test')) .then(() => { expect(testKeyValue).toBe('test'); - mockCallback.mockReset(); - return Onyx.clear().then(waitForPromisesToResolve); + return Onyx.clear(); }) + .then(() => waitForPromisesToResolve()) .then(() => { // Test key should be cleared expect(testKeyValue).toBeUndefined(); - // Expect that the connection to a key with a default value wasn't cleared + // Expect that the connection to a key with a default value that wasn't changed is not called on clear expect(mockCallback).toHaveBeenCalledTimes(0); - // Other test key should be returned to its default state - expect(otherTestValue).toBe(42); - return Onyx.disconnect(otherTestConnectionID); }); }); From 49629d2177be1b5b2756bd36aa3d93721a5227d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Tue, 30 Jul 2024 18:06:44 +0200 Subject: [PATCH 2/6] add failing test --- tests/unit/onyxTest.ts | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts index 5b8ceb8d6..f5abb204c 100644 --- a/tests/unit/onyxTest.ts +++ b/tests/unit/onyxTest.ts @@ -9,6 +9,8 @@ import type GenericCollection from '../utils/GenericCollection'; const ONYX_KEYS = { TEST_KEY: 'test', OTHER_TEST: 'otherTest', + // Special case: this key is not a collection key, but it has an underscore in its name + KEY_WITH_UNDERSCORE: 'nvp_test', COLLECTION: { TEST_KEY: 'test_', TEST_CONNECT_COLLECTION: 'testConnectCollection_', @@ -25,6 +27,7 @@ Onyx.init({ keys: ONYX_KEYS, initialKeyStates: { [ONYX_KEYS.OTHER_TEST]: 42, + [ONYX_KEYS.KEY_WITH_UNDERSCORE]: 'default', }, }); @@ -199,6 +202,35 @@ describe('Onyx', () => { }); }); + it('should notify key subscribers that use a underscore in their name', () => { + const mockCallback = jest.fn(); + connectionID = Onyx.connect({ + key: ONYX_KEYS.KEY_WITH_UNDERSCORE, + callback: mockCallback, + }); + + return waitForPromisesToResolve() + .then(() => mockCallback.mockReset()) + .then(() => Onyx.set(ONYX_KEYS.KEY_WITH_UNDERSCORE, 'test')) + .then(() => { + expect(mockCallback).toHaveBeenCalledTimes(1); + expect(mockCallback).toHaveBeenLastCalledWith('test', ONYX_KEYS.KEY_WITH_UNDERSCORE); + mockCallback.mockReset(); + return Onyx.clear(); + }) + .then(() => { + expect(mockCallback).toHaveBeenCalledTimes(1); + expect(mockCallback).toHaveBeenCalledWith('default', ONYX_KEYS.KEY_WITH_UNDERSCORE); + }) + .then(() => Onyx.set(ONYX_KEYS.KEY_WITH_UNDERSCORE, 'default')) + .then(() => mockCallback.mockReset()) + .then(() => Onyx.set(ONYX_KEYS.KEY_WITH_UNDERSCORE, 'test')) + .then(() => { + expect(mockCallback).toHaveBeenCalledTimes(1); + expect(mockCallback).toHaveBeenCalledWith('test', ONYX_KEYS.KEY_WITH_UNDERSCORE); + }); + }); + it('should not notify subscribers after they have disconnected', () => { let testKeyValue: unknown; connectionID = Onyx.connect({ From 219b117a9dc80978e4a95659d149a14b2d011b34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Tue, 30 Jul 2024 21:56:04 +0200 Subject: [PATCH 3/6] fix: Onyx.clear handle collections correctly --- lib/Onyx.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index f927f3530..bac648050 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -586,12 +586,14 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { const newValue = defaultKeyStates[key] ?? null; if (newValue !== oldValue) { cache.set(key, newValue); - const collectionKey = key.substring(0, key.indexOf('_') + 1); - if (collectionKey) { + + const potentialCollectionKey = OnyxUtils.getCollectionKey(key); + if (OnyxUtils.isCollectionKey(potentialCollectionKey)) { + const [collectionKey, memberKey] = OnyxUtils.splitCollectionMemberKey(key); if (!keyValuesToResetAsCollection[collectionKey]) { keyValuesToResetAsCollection[collectionKey] = {}; } - keyValuesToResetAsCollection[collectionKey]![key] = newValue ?? undefined; + keyValuesToResetAsCollection[collectionKey]![memberKey] = newValue ?? undefined; } else { keyValuesToResetIndividually[key] = newValue ?? undefined; } @@ -612,6 +614,7 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { Object.entries(keyValuesToResetIndividually).forEach(([key, value]) => { updatePromises.push(OnyxUtils.scheduleSubscriberUpdate(key, value, cache.get(key, false))); }); + console.log(keyValuesToResetAsCollection); Object.entries(keyValuesToResetAsCollection).forEach(([key, value]) => { updatePromises.push(OnyxUtils.scheduleNotifyCollectionSubscribers(key, value)); }); From ff482576d944dd9134450cdb2deb38c4555a0098 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Tue, 30 Jul 2024 21:56:16 +0200 Subject: [PATCH 4/6] cleanup splitCollectionMemberKey --- lib/OnyxUtils.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts index cac8b0b1d..48f422586 100644 --- a/lib/OnyxUtils.ts +++ b/lib/OnyxUtils.ts @@ -403,14 +403,16 @@ function isCollectionMemberKey(collect * @param key - The collection member key to split. * @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] { +function splitCollectionMemberKey(key: TKey): [CollectionKeyType, string] { const underscoreIndex = key.lastIndexOf('_'); if (underscoreIndex === -1) { throw new Error(`Invalid ${key} key provided, only collection keys are allowed.`); } - return [key.substring(0, underscoreIndex + 1) as TKey extends `${infer Prefix}_${string}` ? `${Prefix}_` : never, key.substring(underscoreIndex + 1)]; + const collectionKey = key.substring(0, underscoreIndex + 1) as CollectionKeyType; + const memberKey = key.substring(underscoreIndex + 1); + return [collectionKey, memberKey]; } /** From 63d5a7b7fcd477cd81f28837f0176531163d920f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Tue, 30 Jul 2024 22:01:54 +0200 Subject: [PATCH 5/6] remove debug log --- lib/Onyx.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index bac648050..5e5e55b76 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -614,7 +614,6 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { Object.entries(keyValuesToResetIndividually).forEach(([key, value]) => { updatePromises.push(OnyxUtils.scheduleSubscriberUpdate(key, value, cache.get(key, false))); }); - console.log(keyValuesToResetAsCollection); Object.entries(keyValuesToResetAsCollection).forEach(([key, value]) => { updatePromises.push(OnyxUtils.scheduleNotifyCollectionSubscribers(key, value)); }); From 8de144757e3b30caf9a205d900733f4302a05a5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Tue, 30 Jul 2024 22:08:00 +0200 Subject: [PATCH 6/6] fix use full collection member key as object key --- lib/Onyx.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/Onyx.ts b/lib/Onyx.ts index 5e5e55b76..aef24863c 100644 --- a/lib/Onyx.ts +++ b/lib/Onyx.ts @@ -587,13 +587,12 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise { if (newValue !== oldValue) { cache.set(key, newValue); - const potentialCollectionKey = OnyxUtils.getCollectionKey(key); - if (OnyxUtils.isCollectionKey(potentialCollectionKey)) { - const [collectionKey, memberKey] = OnyxUtils.splitCollectionMemberKey(key); + const collectionKey = OnyxUtils.getCollectionKey(key); + if (OnyxUtils.isCollectionKey(collectionKey)) { if (!keyValuesToResetAsCollection[collectionKey]) { keyValuesToResetAsCollection[collectionKey] = {}; } - keyValuesToResetAsCollection[collectionKey]![memberKey] = newValue ?? undefined; + keyValuesToResetAsCollection[collectionKey]![key] = newValue ?? undefined; } else { keyValuesToResetIndividually[key] = newValue ?? undefined; }