Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Onyx Cache #76

Merged
merged 40 commits into from
Jun 7, 2021
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
a343ef1
test: move metrics decoration tests to separate file
kidroca May 25, 2021
a7ee841
test: move Onyx require call to `beforeEach`
kidroca May 25, 2021
d23b8eb
test: fix `withOnyx` test not returning promise
kidroca May 25, 2021
a6df3db
test: add a test suite for Onyx cache
kidroca May 25, 2021
231fe00
feat: Add separate service for cache
kidroca May 25, 2021
2f76d80
test: add OnyxCache tests
kidroca May 25, 2021
1046786
feat: Use OnyxCache to cache storage operations
kidroca May 25, 2021
273afe1
test: update Onyx with Cache to run correctly
kidroca May 25, 2021
47739e4
refactor: cache.merge should throw when a value is not an object or a…
kidroca May 25, 2021
f4a523f
refactor: extract prepareKeyValuePairsForStorage method
kidroca May 25, 2021
6b659d7
refactor: simplify mergeCollection keys partitioning
kidroca May 25, 2021
f7feafc
docs: add todo notes on `withOnyx` test not passing expected props to…
kidroca May 25, 2021
16d42ac
fix: `get` should capture different tasks for different keys
kidroca May 25, 2021
5b11545
fix: clear async storage
kidroca May 27, 2021
0c51923
feat: enhance cache.merge to work with primitives as well as complex …
kidroca May 27, 2021
22ad916
refactor: update `hasCacheForKey` to use `isDefined`
kidroca May 27, 2021
d0c62c5
docs: cache.update try to better explain this method
kidroca May 27, 2021
1b35161
feat: clean up cache when the last key for a subscription is disconne…
kidroca May 27, 2021
b33e696
refactor: encapsulate cache checking and resolving logic for getAllKe…
kidroca May 27, 2021
4af3abf
refactor: encapsulate cache checking and resolving logic for get in t…
kidroca May 27, 2021
ceb4acf
Rename tests/decorateWithMetrics.js to tests/decorateWithMetricsTest.js
kidroca May 27, 2021
5435063
test: check what happens with cache when async storage rejects
kidroca May 27, 2021
dc2a540
docs: update getAllKeys and getValue documentation
kidroca May 27, 2021
34cb2aa
test: Shorten test case names
kidroca May 27, 2021
dfe362d
refactor: handle cache cleanup for collections
kidroca May 28, 2021
f14e5d7
test: fix react warning that items don't have a key
kidroca May 28, 2021
2494a55
refactor: rename `OnyxCache.update` to `OnyxCache.set`
kidroca May 28, 2021
ee177e2
refactor: extract a separate `addKey` method
kidroca May 28, 2021
55086b4
fix: Onyx.multiSet should not use cache.merge, but cache.set to overw…
kidroca May 28, 2021
afd91ca
refactor: Update cache merge to work like AsyncStorage.merge and loda…
kidroca Jun 1, 2021
9231475
Revert "refactor: encapsulate cache checking and resolving logic for …
kidroca Jun 1, 2021
4e57857
Revert "refactor: encapsulate cache checking and resolving logic for …
kidroca Jun 1, 2021
a27841d
refactor: extract separate methods from `resolveTask` so usage in Ony…
kidroca Jun 1, 2021
1ff60fc
test: update tests related to task capturing after changes
kidroca Jun 1, 2021
4eed726
convert async/await tests to promise based chains
kidroca Jun 1, 2021
7b35a24
docs: update comment
kidroca Jun 1, 2021
d6f153c
docs: Apply suggestions from code review
kidroca Jun 2, 2021
27c0ba8
docs: Remove leftover method description
kidroca Jun 2, 2021
5254568
Skip creating an arrow function for AsyncStorage.clear
kidroca Jun 2, 2021
77b07e1
Explicitly convert set to array
kidroca Jun 3, 2021
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
122 changes: 81 additions & 41 deletions lib/Onyx.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import AsyncStorage from '@react-native-community/async-storage';
import Str from 'expensify-common/lib/str';
import lodashMerge from 'lodash/merge';
import {registerLogger, logInfo, logAlert} from './Logger';
import cache from './OnyxCache';

// Keeps track of the last connectionID that was used so we can keep incrementing it
let lastConnectionID = 0;
Expand Down Expand Up @@ -34,17 +35,42 @@ let defaultKeyStates = {};
* @returns {Promise<*>}
*/
function get(key) {
return AsyncStorage.getItem(key)
.then(val => JSON.parse(val))
.catch(err => logInfo(`Unable to get item from persistent storage. Key: ${key} Error: ${err}`));
// When we already have the value in cache - resolve right away
if (cache.hasCacheForKey(key)) {
return Promise.resolve(cache.getValue(key));
}

/* Otherwise setup a task to retrieve it.
This ensures concurrent usages would not start the same task */
return cache.resolveTask(`get:${key}`, () => AsyncStorage.getItem(key)
.then((val) => {
// Add values to cache and return parsed result
const parsed = JSON.parse(val);
cache.update(key, parsed);
return parsed;
})
.catch(err => logInfo(`Unable to get item from persistent storage. Key: ${key} Error: ${err}`)));
}

/**
* Returns current key names stored in persisted storage
* @returns {Promise<string[]>}
*/
function getAllKeys() {
return AsyncStorage.getAllKeys();
// When we've already read stored keys, resolve right away
const storedKeys = cache.getAllKeys();
if (storedKeys.length > 0) {
return Promise.resolve(storedKeys);
}

/* Otherwise setup a task to retrieve them.
This ensures concurrent usages would not start the same task */
return cache.resolveTask('getAllKeys', () => AsyncStorage.getAllKeys()
.then((keys) => {
// Add values to cache and return original result
_.each(keys, key => cache.update(key));
return keys;
}));
}

/**
Expand Down Expand Up @@ -377,8 +403,13 @@ function disconnect(connectionID, keyToRemoveFromEvictionBlocklist) {
* @return {Promise}
*/
function remove(key) {
return AsyncStorage.removeItem(key)
.then(() => keyChanged(key, null));
// Remove from cache
cache.remove(key);

// Optimistically inform subscribers
keyChanged(key, null);

return AsyncStorage.removeItem(key);
}

/**
Expand Down Expand Up @@ -414,12 +445,28 @@ function evictStorageAndRetry(error, ionMethod, ...args) {
* @returns {Promise}
*/
function set(key, val) {
// Adds the key to cache when it's not available
cache.update(key, val);

// Optimistically inform subscribers
keyChanged(key, val);

// Write the thing to persistent storage, which will trigger a storage event for any other tabs open on this domain
return AsyncStorage.setItem(key, JSON.stringify(val))
.then(() => keyChanged(key, val))
.catch(error => evictStorageAndRetry(error, set, key, val));
}

/**
* AsyncStorage expects array like: [["@MyApp_user", "value_1"], ["@MyApp_key", "value_2"]]
* This method transforms an object like {'@MyApp_user': 'myUserValue', '@MyApp_key': 'myKeyValue'}
* to an array of key-value pairs in the above format
* @param {Record<string, *>} data
* @return {Array<[string, string]>} an array of stringified key - value pairs
*/
function prepareKeyValuePairsForStorage(data) {
return _.keys(data).map(key => [key, JSON.stringify(data[key])]);
}

/**
* Sets multiple keys and values. Example
* Onyx.multiSet({'key1': 'a', 'key2': 'b'});
Expand All @@ -428,17 +475,15 @@ function set(key, val) {
* @returns {Promise}
*/
function multiSet(data) {
// AsyncStorage expenses the data in an array like:
// [["@MyApp_user", "value_1"], ["@MyApp_key", "value_2"]]
// This method will transform the params from a better JSON format like:
// {'@MyApp_user': 'myUserValue', '@MyApp_key': 'myKeyValue'}
const keyValuePairs = _.reduce(data, (finalArray, val, key) => ([
...finalArray,
[key, JSON.stringify(val)],
]), []);
const keyValuePairs = prepareKeyValuePairsForStorage(data);

// Capture (non-stringified) changes to cache
cache.merge(_.pairs(data));
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this basically doing multiMerge not multiSet()?

// previous value {loading: true}
Onyx.multiSet(['someKey', {data: 'test'}]); // -> {loading: true, data: 'test'}

and the value should be {data: 'test'} ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, cache.merge is doing a multi merge. So this works incorrect here and data in cache isn't overwritten but merged. Will address this tomorrow


// Optimistically inform subscribers
_.each(data, (val, key) => keyChanged(key, val));

return AsyncStorage.multiSet(keyValuePairs)
.then(() => _.each(data, (val, key) => keyChanged(key, val)))
.catch(error => evictStorageAndRetry(error, multiSet, data));
}

Expand Down Expand Up @@ -522,13 +567,11 @@ function initializeWithDefaultKeyStates() {
* @returns {Promise<void>}
*/
function clear() {
let allKeys;
return getAllKeys()
.then(keys => allKeys = keys)
.then(() => AsyncStorage.clear())
.then(() => {
_.each(allKeys, (key) => {
.then((keys) => {
_.each(keys, (key) => {
keyChanged(key, null);
cache.remove(key);
});

initializeWithDefaultKeyStates();
Expand All @@ -551,34 +594,31 @@ function mergeCollection(collectionKey, collection) {
}
});

const existingKeyCollection = {};
const newCollection = {};
return getAllKeys()
.then((keys) => {
_.each(collection, (data, dataKey) => {
if (keys.includes(dataKey)) {
existingKeyCollection[dataKey] = data;
} else {
newCollection[dataKey] = data;
}
});

const keyValuePairsForExistingCollection = _.reduce(existingKeyCollection, (finalArray, val, key) => ([
...finalArray,
[key, JSON.stringify(val)],
]), []);
const keyValuePairsForNewCollection = _.reduce(newCollection, (finalArray, val, key) => ([
...finalArray,
[key, JSON.stringify(val)],
]), []);
.then((persistedKeys) => {
// Split to keys that exist in storage and keys that don't
const [existingKeys, newKeys] = _.chain(collection)
.keys()
.partition(key => persistedKeys.includes(key))
.value();

const existingKeyCollection = _.pick(collection, existingKeys);
const newCollection = _.pick(collection, newKeys);
const keyValuePairsForExistingCollection = prepareKeyValuePairsForStorage(existingKeyCollection);
const keyValuePairsForNewCollection = prepareKeyValuePairsForStorage(newCollection);

// New keys will be added via multiSet while existing keys will be updated using multiMerge
// This is because setting a key that doesn't exist yet with multiMerge will throw errors
const existingCollectionPromise = AsyncStorage.multiMerge(keyValuePairsForExistingCollection);
const newCollectionPromise = AsyncStorage.multiSet(keyValuePairsForNewCollection);

// Capture (non-stringified) changes to cache
cache.merge(_.pairs(collection));

// Optimistically inform collection subscribers
keysChanged(collectionKey, collection);

return Promise.all([existingCollectionPromise, newCollectionPromise])
.then(() => keysChanged(collectionKey, collection))
.catch(error => evictStorageAndRetry(error, mergeCollection, collection));
});
}
Expand Down
137 changes: 137 additions & 0 deletions lib/OnyxCache.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
import _ from 'underscore';

const isDefined = _.negate(_.isUndefined);

/**
* In memory cache providing data by reference
* Encapsulates Onyx cache related functionality
*/
class OnyxCache {
constructor() {
/**
* @private
* Cache of all the storage keys available in persistent storage
* @type {Set<string>}
*/
this.storageKeys = new Set();

/**
* @private
* A map of cached values
* @type {Record<string, *>}
*/
this.storageMap = {};

/**
* @private
* Captured pending tasks for already running storage methods
* @type {Record<string, Promise>}
*/
this.pendingPromises = {};

// bind all methods to prevent problems with `this`
_.bindAll(
this,
'getAllKeys', 'getValue', 'hasCacheForKey', 'update', 'remove', 'merge', 'resolveTask',
);
}

/**
* Get all the storage keys
* @returns {string[]}
*/
getAllKeys() {
return [...this.storageKeys];
}

/**
* Get a cached value from storage
* @param {string} key
* @returns {*}
*/
getValue(key) {
return this.storageMap[key];
}

/**
* Check whether cache has data for the given key
* @param {string} key
* @returns {boolean}
*/
hasCacheForKey(key) {
return key in this.storageMap;
}

/**
* Update or add data to cache
* @param {string} key
* @param {*} [value]
*/
update(key, value) {
// Update all storage keys
this.storageKeys.add(key);

// When value is provided update general cache as well
if (isDefined(value)) {
this.storageMap[key] = value;
}
}

/**
* Remove data from cache
* @param {string} key
*/
remove(key) {
this.storageKeys.delete(key);
delete this.storageMap[key];
}

/**
* Merge data to cache, any non existing keys will be crated
* @param {Array<[string, Object]>} pairs - array of key value pairs
*/
merge(pairs) {
_.forEach(pairs, ([key, value]) => {
const currentValue = _.result(this.storageMap, key, {});

if (_.isObject(value) || _.isArray(value)) {
const merged = {...currentValue, ...value};
this.update(key, merged);
return;
}

throw new Error(`The provided merge value is invalid: ${value}`);
});
}

/**
* Use this method to prevents additional calls from the same thing
* When a promise for a given call already exists the caller would
* be hooked to it. Otherwise the task is started and captured so
* that other callers don't have to start over
* Particularly useful for data retrieving methods
* @param {string} taskName
* @param {function(): Promise} startTask - Provide a promise returning function that
* will be invoked if there is no pending promise for this task
* @returns {Promise}
*/
resolveTask(taskName, startTask) {
// When a task is already running return it right away
if (isDefined(this.pendingPromises[taskName])) {
return this.pendingPromises[taskName];
}

// Otherwise start the task and store a reference
this.pendingPromises[taskName] = startTask()
.finally(() => {
// Cleanup after the task is over
delete this.pendingPromises[taskName];
});

return this.pendingPromises[taskName];
}
}

const instance = new OnyxCache();

export default instance;
Loading