Skip to content

Commit

Permalink
Refactor Onyx cache tasks
Browse files Browse the repository at this point in the history
  • Loading branch information
fabioh8010 committed Oct 14, 2024
1 parent 209e445 commit fcda312
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 22 deletions.
4 changes: 2 additions & 2 deletions lib/Onyx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import _ from 'underscore';
import lodashPick from 'lodash/pick';
import * as Logger from './Logger';
import cache from './OnyxCache';
import cache, {TASK} from './OnyxCache';
import * as PerformanceUtils from './PerformanceUtils';
import Storage from './storage';
import utils from './utils';
Expand Down Expand Up @@ -538,7 +538,7 @@ function clear(keysToPreserve: OnyxKey[] = []): Promise<void> {
})
.then(() => undefined);

return cache.captureTask('clear', promise) as Promise<void>;
return cache.captureTask(TASK.CLEAR, promise) as Promise<void>;
}

function updateSnapshots(data: OnyxUpdate[]) {
Expand Down
18 changes: 15 additions & 3 deletions lib/OnyxCache.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,18 @@
import {deepEqual} from 'fast-equals';
import bindAll from 'lodash/bindAll';
import type {ValueOf} from 'type-fest';
import utils from './utils';
import type {OnyxKey, OnyxValue} from './types';

// Task constants
const TASK = {
GET: 'get',
GET_ALL_KEYS: 'getAllKeys',
CLEAR: 'clear',
} as const;

type CacheTask = ValueOf<typeof TASK> | `${ValueOf<typeof TASK>}:${string}`;

/**
* In memory cache providing data by reference
* Encapsulates Onyx cache related functionality
Expand Down Expand Up @@ -172,7 +182,7 @@ class OnyxCache {
* Check whether the given task is already running
* @param taskName - unique name given for the task
*/
hasPendingTask(taskName: string): boolean {
hasPendingTask(taskName: CacheTask): boolean {
return this.pendingPromises.get(taskName) !== undefined;
}

Expand All @@ -182,7 +192,7 @@ class OnyxCache {
* provided from this function
* @param taskName - unique name given for the task
*/
getTaskPromise(taskName: string): Promise<OnyxValue<OnyxKey> | OnyxKey[]> | undefined {
getTaskPromise(taskName: CacheTask): Promise<OnyxValue<OnyxKey> | OnyxKey[]> | undefined {
return this.pendingPromises.get(taskName);
}

Expand All @@ -191,7 +201,7 @@ class OnyxCache {
* hook up to the promise if it's still pending
* @param taskName - unique name for the task
*/
captureTask(taskName: string, promise: Promise<OnyxValue<OnyxKey>>): Promise<OnyxValue<OnyxKey>> {
captureTask(taskName: CacheTask, promise: Promise<OnyxValue<OnyxKey>>): Promise<OnyxValue<OnyxKey>> {
const returnPromise = promise.finally(() => {
this.pendingPromises.delete(taskName);
});
Expand Down Expand Up @@ -242,3 +252,5 @@ class OnyxCache {
const instance = new OnyxCache();

export default instance;
export {TASK};
export type {CacheTask};
14 changes: 6 additions & 8 deletions lib/OnyxUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type {ValueOf} from 'type-fest';
import DevTools from './DevTools';
import * as Logger from './Logger';
import type Onyx from './Onyx';
import cache from './OnyxCache';
import cache, {TASK} from './OnyxCache';
import * as PerformanceUtils from './PerformanceUtils';
import * as Str from './Str';
import unstable_batchedUpdates from './batch';
Expand Down Expand Up @@ -244,7 +244,7 @@ function get<TKey extends OnyxKey, TValue extends OnyxValue<TKey>>(key: TKey): P
return Promise.resolve(cache.get(key) as TValue);
}

const taskName = `get:${key}`;
const taskName = `${TASK.GET}:${key}` as const;

// When a value retrieving task for this key is still running hook to it
if (cache.hasPendingTask(taskName)) {
Expand Down Expand Up @@ -296,7 +296,7 @@ function multiGet<TKey extends OnyxKey>(keys: CollectionKeyBase[]): Promise<Map<
return;
}

const pendingKey = `get:${key}`;
const pendingKey = `${TASK.GET}:${key}` as const;
if (cache.hasPendingTask(pendingKey)) {
pendingTasks.push(cache.getTaskPromise(pendingKey) as Promise<OnyxValue<TKey>>);
pendingKeys.push(key);
Expand Down Expand Up @@ -378,11 +378,9 @@ function getAllKeys(): Promise<Set<OnyxKey>> {
return Promise.resolve(cachedKeys);
}

const taskName = 'getAllKeys';

// When a value retrieving task for all keys is still running hook to it
if (cache.hasPendingTask(taskName)) {
return cache.getTaskPromise(taskName) as Promise<Set<OnyxKey>>;
if (cache.hasPendingTask(TASK.GET_ALL_KEYS)) {
return cache.getTaskPromise(TASK.GET_ALL_KEYS) as Promise<Set<OnyxKey>>;
}

// Otherwise retrieve the keys from storage and capture a promise to aid concurrent usages
Expand All @@ -393,7 +391,7 @@ function getAllKeys(): Promise<Set<OnyxKey>> {
return cache.getAllKeys();
});

return cache.captureTask(taskName, promise) as Promise<Set<OnyxKey>>;
return cache.captureTask(TASK.GET_ALL_KEYS, promise) as Promise<Set<OnyxKey>>;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions lib/useOnyx.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {deepEqual, shallowEqual} from 'fast-equals';
import {useCallback, useEffect, useRef, useSyncExternalStore} from 'react';
import OnyxCache from './OnyxCache';
import OnyxCache, {TASK} from './OnyxCache';
import type {Connection} from './OnyxConnectionManager';
import connectionManager from './OnyxConnectionManager';
import OnyxUtils from './OnyxUtils';
Expand Down Expand Up @@ -239,7 +239,7 @@ function useOnyx<TKey extends OnyxKey, TReturnValue = OnyxValue<TKey>>(key: TKey
// If the previously cached value is different from the new value, we update both cached value
// and the result to be returned by the hook.
// If the cache was set for the first time or we have a pending Onyx.clear() task, we also update the cached value and the result.
const isCacheSetFirstTime = previousValueRef.current === null && (hasCacheForKey || OnyxCache.hasPendingTask('clear'));
const isCacheSetFirstTime = previousValueRef.current === null && (hasCacheForKey || OnyxCache.hasPendingTask(TASK.CLEAR));
if (isCacheSetFirstTime || !areValuesEqual) {
previousValueRef.current = newValueRef.current;

Expand Down
17 changes: 10 additions & 7 deletions tests/unit/onyxCacheTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import type withOnyxType from '../../lib/withOnyx';
import type {InitOptions} from '../../lib/types';
import generateRange from '../utils/generateRange';
import type {Connection} from '../../lib/OnyxConnectionManager';
import type {CacheTask} from '../../lib/OnyxCache';

const MOCK_TASK = 'mockTask' as CacheTask;

describe('Onyx', () => {
describe('Cache Service', () => {
Expand Down Expand Up @@ -374,22 +377,22 @@ describe('Onyx', () => {
// Given empty cache with no started tasks
// When a task has not been started
// Then it should return false
expect(cache.hasPendingTask('mockTask')).toBe(false);
expect(cache.hasPendingTask(MOCK_TASK)).toBe(false);
});

it('Should return true when a task is running', () => {
// Given empty cache with no started tasks
// When a unique task is started
const promise = Promise.resolve();
cache.captureTask('mockTask', promise);
cache.captureTask(MOCK_TASK, promise);

// Then `hasPendingTask` should return true
expect(cache.hasPendingTask('mockTask')).toBe(true);
expect(cache.hasPendingTask(MOCK_TASK)).toBe(true);

// When the promise is completed
return waitForPromisesToResolve().then(() => {
// Then `hasPendingTask` should return false
expect(cache.hasPendingTask('mockTask')).toBe(false);
expect(cache.hasPendingTask(MOCK_TASK)).toBe(false);
});
});
});
Expand All @@ -399,7 +402,7 @@ describe('Onyx', () => {
// Given empty cache with no started tasks

// When a task is retrieved
const task = cache.getTaskPromise('mockTask');
const task = cache.getTaskPromise(MOCK_TASK);

// Then it should be undefined
expect(task).not.toBeDefined();
Expand All @@ -409,10 +412,10 @@ describe('Onyx', () => {
// Given empty cache with no started tasks
// When a unique task is started
const promise = Promise.resolve({mockResult: true});
cache.captureTask('mockTask', promise);
cache.captureTask(MOCK_TASK, promise);

// When a task is retrieved
const taskPromise = cache.getTaskPromise('mockTask');
const taskPromise = cache.getTaskPromise(MOCK_TASK);

// Then it should resolve with the same result as the captured task
return taskPromise!.then((result) => {
Expand Down

0 comments on commit fcda312

Please sign in to comment.