Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions __mocks__/localforage-removeitems.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import _ from 'underscore';

function extendPrototype(localforage) {
const newLocalforage = localforage;
newLocalforage.removeItems = keys => new Promise((resolve) => {
_.each(keys, (key) => {
delete newLocalforage.storageMap[key];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused by this. Does this actually delete the stuff from IndexedDB? It looks like it would not...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohhh nvm it's a mock...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well this is a mock. This code has changed a bit since this PR was created, but the mock isn't actually writing anything to IndexedDB. storageMapInternal is just an object so I think delete does work here?

});
resolve();
});
}

// eslint-disable-next-line import/prefer-default-export
export {extendPrototype};
78 changes: 34 additions & 44 deletions lib/Onyx.js
Original file line number Diff line number Diff line change
Expand Up @@ -1047,55 +1047,46 @@ function initializeWithDefaultKeyStates() {
function clear(keysToPreserve = []) {
return getAllKeys()
.then((keys) => {
const keyValuesToReset = [];
const defaultKeys = _.keys(defaultKeyStates);

// Get all the values for the keys that need to be preserved. These key/value pairs will be set
// in Onyx after the database is cleared().
const keyValuesToPreserve = _.map(keysToPreserve, key => [key, cache.getValue(key)]);
const keysToBeClearedFromStorage = [];
const keyValuesToResetAsCollection = {};
const keyValuesToResetIndividually = {};

// 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)
const keysToClear = _.difference(keys, keysToPreserve, defaultKeys);
keyValuesToReset.push(..._.map(keysToClear, key => [key, null]));

// Remove any keysToPreserve from the defaultKeyStates because if they are passed in it has been explicitly
// called out to preserve those values instead of resetting them back
// to the default.
const defaultKeyValuePairs = _.pairs(_.omit(defaultKeyStates, ...keysToPreserve));

// Add the default key value pairs to the keyValuesToReset so that they get set back to their default values
// when we clear Onyx
keyValuesToReset.push(...defaultKeyValuePairs);

// We now have all the key/values that need to be reset, but we're not done yet!
// There will be two groups of key/values and they each need to be updated a little bit differently.
// Collection keys need to be notified differently than non collection keys
const keyValuesToResetAsCollection = {};
const keyValuesToResetIndividually = {};

// Make sure that we also reset the cache values before clearing the values from storage.
// We do this before clearing Storage so that any call to clear() followed by merge() on a key with a
// default state results in the merged value getting saved, since the update from the merge() call would
// happen on the tick after the update from this clear()
_.each(keyValuesToReset, (keyValue) => {
const key = keyValue[0];
const value = keyValue[1];
cache.set(key, value);

const collectionKey = key.substring(0, key.indexOf('_') + 1);
if (collectionKey) {
if (!keyValuesToResetAsCollection[collectionKey]) {
keyValuesToResetAsCollection[collectionKey] = {};
_.each(keys, (key) => {
const isKeyToPreserve = _.contains(keysToPreserve, key);
const isDefaultKey = _.has(defaultKeyStates, key);

// If the key is being removed or reset to default:
// 1. Update it in the cache
// 2. Figure out whether it is a collection key or not,
// since collection key subscribers need to be updated differently
if (!isKeyToPreserve) {
const oldValue = cache.getValue(key);
const newValue = _.get(defaultKeyStates, key, null);
if (newValue !== oldValue) {
cache.set(key, newValue);
const collectionKey = key.substring(0, key.indexOf('_') + 1);
if (collectionKey) {
if (!keyValuesToResetAsCollection[collectionKey]) {
keyValuesToResetAsCollection[collectionKey] = {};
}
keyValuesToResetAsCollection[collectionKey][key] = newValue;
} else {
keyValuesToResetIndividually[key] = newValue;
}
}
keyValuesToResetAsCollection[collectionKey][key] = value;
}

if (isKeyToPreserve || isDefaultKey) {
return;
}

keyValuesToResetIndividually[key] = value;
// If it isn't preserved and doesn't have a default, we'll remove it
keysToBeClearedFromStorage.push(key);
});

// Notify the subscribers for each key/value group so they can receive the new values
Expand All @@ -1106,11 +1097,10 @@ function clear(keysToPreserve = []) {
notifyCollectionSubscribersOnNextTick(key, value);
});

// Call clear() and make sure that the default key/values and the key/values from the parameter
// are preserved in storage. This makes sure to always leave storage in a state that contains
// all the default values and any additional values that we want to remain after the database is cleared.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep a comment here to explain what we're doing and why

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@madmax330 Thanks for pointing that out. I have updated, please help to check again

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated it to only say what and not why...

return Storage.clear()
.then(() => Storage.multiSet([...defaultKeyValuePairs, ...keyValuesToPreserve]));
const defaultKeyValuePairs = _.pairs(_.omit(defaultKeyStates, keysToPreserve));

// Remove only the items that we want cleared from storage, and reset others to default
return Storage.removeItems(keysToBeClearedFromStorage).then(() => Storage.multiSet(defaultKeyValuePairs));
});
}

Expand Down
9 changes: 9 additions & 0 deletions lib/storage/WebStorage.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ function raiseStorageSyncEvent(onyxKey) {
global.localStorage.removeItem(SYNC_ONYX, onyxKey);
}

function raiseStorageSyncManyKeysEvent(onyxKeys) {
_.each(onyxKeys, (onyxKey) => {
raiseStorageSyncEvent(onyxKey);
});
}

const webStorage = {
...Storage,

Expand All @@ -31,6 +37,9 @@ const webStorage = {
this.removeItem = key => Storage.removeItem(key)
.then(() => raiseStorageSyncEvent(key));

this.removeItems = keys => Storage.removeItems(keys)
.then(() => raiseStorageSyncManyKeysEvent(keys));

// If we just call Storage.clear other tabs will have no idea which keys were available previously
// so that they can call keysChanged for them. That's why we iterate over every key and raise a storage sync
// event for each one
Expand Down
7 changes: 7 additions & 0 deletions lib/storage/providers/AsyncStorage.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@ const provider = {
*/
removeItem: AsyncStorage.removeItem,

/**
* Remove given keys and their values from storage
* @param {Array} keys
* @returns {Promise}
*/
removeItems: keys => Promise.all(_.map(keys, key => AsyncStorage.removeItem(key))),
Copy link
Contributor Author

@tienifr tienifr Mar 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is required because we're using AsyncStorage as the mock provider for NativeStorage in unit tests, so removeItems needs to be defined here as well
cc @roryabraham @sobitneupane


/**
* Clear everything from storage
* @returns {Promise<void>}
Expand Down
13 changes: 13 additions & 0 deletions lib/storage/providers/LocalForage.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@

import localforage from 'localforage';
import _ from 'underscore';
import {extendPrototype} from 'localforage-removeitems';
import SyncQueue from '../../SyncQueue';
import fastMerge from '../../fastMerge';

extendPrototype(localforage);

localforage.config({
name: 'OnyxDB',
});
Expand Down Expand Up @@ -103,6 +106,16 @@ const provider = {
*/
removeItem: localforage.removeItem,

/**
* Remove given keys and their values from storage
*
* @param {Array} keys
* @returns {Promise}
*/
removeItems(keys) {
return localforage.removeItems(keys);
},

/**
* Sets the value for a given key. The only requirement is that the value should be serializable to JSON string
* @param {String} key
Expand Down
12 changes: 12 additions & 0 deletions lib/storage/providers/SQLiteStorage.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,18 @@ const provider = {
*/
removeItem: key => db.executeAsync('DELETE FROM keyvaluepairs WHERE record_key = ?;', [key]),

/**
* Removes given keys and their values from storage
*
* @param {Array<String>} keys
* @returns {Promise<void>}
*/
removeItems: (keys) => {
const placeholders = _.map(keys, () => '?').join(',');
const query = `DELETE FROM keyvaluepairs WHERE record_key IN (${placeholders});`;
return db.executeAsync(query, keys);
},

/**
* Clears absolutely everything from storage
* @returns {Promise<void>}
Expand Down
36 changes: 30 additions & 6 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
"@react-native-async-storage/async-storage": "^1.17.11",
"expensify-common": ">=1",
"localforage": "^1.10.0",
"localforage-removeitems": "^1.4.0",
"react": ">=18.1.0",
"react-native-performance": "^4.0.0",
"react-native-quick-sqlite": "^8.0.0-beta.2"
Expand All @@ -95,6 +96,9 @@
},
"localforage": {
"optional": true
},
"localforage-removeitems": {
"optional": true
}
},
"engines": {
Expand Down
3 changes: 1 addition & 2 deletions tests/unit/onyxTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,11 @@ describe('Onyx', () => {
.then(() => Onyx.set(ONYX_KEYS.TEST_KEY, 'test'))
.then(() => {
expect(testKeyValue).toBe('test');
expect(otherTestValue).toBe(42);
return Onyx.clear().then(waitForPromisesToResolve);
})
.then(() => {
expect(testKeyValue).toBeNull();
expect(otherTestValue).toBe(42);
expect(otherTestValue).toBe(null);
return Onyx.disconnect(otherTestConnectionID);
});
});
Expand Down