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

Automatically enable memory only keys when payload size of reconnectapp/openapp is too large #23831

Merged
merged 16 commits into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from 15 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
24 changes: 24 additions & 0 deletions src/libs/Middleware/SaveResponseInOnyx.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import Onyx from 'react-native-onyx';
import _ from 'underscore';
import CONST from '../../CONST';
import ONYXKEYS from '../../ONYXKEYS';
import * as QueuedOnyxUpdates from '../actions/QueuedOnyxUpdates';
import * as MemoryOnlyKeys from '../actions/MemoryOnlyKeys/MemoryOnlyKeys';
import * as OnyxUpdates from '../actions/OnyxUpdates';

/**
Expand All @@ -15,6 +18,27 @@ function SaveResponseInOnyx(response, request) {
return;
}

// The data for this response comes in two different formats:
// 1. Original format - this is what was sent before the RELIABLE_UPDATES project and will go away once RELIABLE_UPDATES is fully complete
// - The data is an array of objects, where each object is an onyx update
// Example: [{onyxMethod: 'whatever', key: 'foo', value: 'bar'}]
// 1. Reliable updates format - this is what was sent with the RELIABLE_UPDATES project and will be the format from now on
// - The data is an object, containing updateIDs from the server and an array of onyx updates (this array is the same format as the original format above)
// Example: {lastUpdateID: 1, previousUpdateID: 0, onyxData: [{onyxMethod: 'whatever', key: 'foo', value: 'bar'}]}
// NOTE: This is slightly different than the format of the pusher event data, where pusher has "updates" and HTTPS responses have "onyxData" (long story)

// Supports both the old format and the new format
marcaaron marked this conversation as resolved.
Show resolved Hide resolved
const onyxUpdates = _.isArray(responseData) ? responseData : responseData.onyxData;
// If there is an OnyxUpdate for using memory only keys, enable them
_.find(onyxUpdates, ({key, value}) => {
if (key !== ONYXKEYS.IS_USING_MEMORY_ONLY_KEYS || !value) {
return false;
}

MemoryOnlyKeys.enable();
return true;
});

// Save the update IDs to Onyx so they can be used to fetch incremental updates if the client gets out of sync from the server
OnyxUpdates.saveUpdateIDs(Number(responseData.lastUpdateID || 0), Number(responseData.previousUpdateID || 0));

Expand Down
19 changes: 19 additions & 0 deletions src/libs/actions/MemoryOnlyKeys/MemoryOnlyKeys.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import Onyx from 'react-native-onyx';
import ONYXKEYS from '../../../ONYXKEYS';
import Log from '../../Log';

const memoryOnlyKeys = [ONYXKEYS.COLLECTION.REPORT, ONYXKEYS.COLLECTION.POLICY, ONYXKEYS.PERSONAL_DETAILS_LIST];

const enable = () => {
Log.info('[MemoryOnlyKeys] enabled');
Onyx.set(ONYXKEYS.IS_USING_MEMORY_ONLY_KEYS, true);
Onyx.setMemoryOnlyKeys(memoryOnlyKeys);
};

const disable = () => {
Log.info('[MemoryOnlyKeys] disabled');
Onyx.set(ONYXKEYS.IS_USING_MEMORY_ONLY_KEYS, false);
Onyx.setMemoryOnlyKeys([]);
};

export {disable, enable};
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import * as MemoryOnlyKeys from '../MemoryOnlyKeys';

const exposeGlobalMemoryOnlyKeysMethods = () => {
window.enableMemoryOnlyKeys = () => {
MemoryOnlyKeys.enable();
};
window.disableMemoryOnlyKeys = () => {
MemoryOnlyKeys.disable();
};
Comment on lines +4 to +9
Copy link
Contributor

Choose a reason for hiding this comment

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

What is it for? Hand-debugging via console, or what?

Copy link
Contributor

@cubuspl42 cubuspl42 Dec 14, 2023

Choose a reason for hiding this comment

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

We're just adding typing for this part and I'm wondering if it's actually beneficial to expand the Window interface, or maybe we don't need any typing for this if we don't intend to use it programmatically.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it's a JS console thing. And no, it shouldn't really be used in the code anywhere. So I agree there is no point in getting crazy with the types for this.

};

export default exposeGlobalMemoryOnlyKeysMethods;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* This is a no-op because the global methods will only work for web and desktop
*/
const exposeGlobalMemoryOnlyKeysMethods = () => {};

export default exposeGlobalMemoryOnlyKeysMethods;
11 changes: 9 additions & 2 deletions src/libs/actions/User.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import * as ErrorUtils from '../ErrorUtils';
import * as Session from './Session';
import * as PersonalDetails from './PersonalDetails';
import * as OnyxUpdates from './OnyxUpdates';
import * as MemoryOnlyKeys from './MemoryOnlyKeys/MemoryOnlyKeys';
tgolen marked this conversation as resolved.
Show resolved Hide resolved

let currentUserAccountID = '';
let currentEmail = '';
Expand Down Expand Up @@ -548,8 +549,13 @@ function subscribeToUserEvents() {
PusherUtils.subscribeToPrivateUserChannelEvent(Pusher.TYPE.MULTIPLE_EVENTS, currentUserAccountID, (pushJSON) => {
let updates;

// This is the old format where each update was just an array, with the new changes it's an object with
// lastUpdateID, previousUpdateID and updates
// The data for this push event comes in two different formats:
// 1. Original format - this is what was sent before the RELIABLE_UPDATES project and will go away once RELIABLE_UPDATES is fully complete
// - The data is an array of objects, where each object is an onyx update
// Example: [{onyxMethod: 'whatever', key: 'foo', value: 'bar'}]
// 1. Reliable updates format - this is what was sent with the RELIABLE_UPDATES project and will be the format from now on
// - The data is an object, containing updateIDs from the server and an array of onyx updates (this array is the same format as the original format above)
// Example: {lastUpdateID: 1, previousUpdateID: 0, updates: [{onyxMethod: 'whatever', key: 'foo', value: 'bar'}]}
if (_.isArray(pushJSON)) {
updates = pushJSON;
} else {
Expand All @@ -568,6 +574,7 @@ function subscribeToUserEvents() {
if (!currentUserAccountID) {
return;
}

Onyx.update(pushJSON);
triggerNotifications(pushJSON);
});
Expand Down
10 changes: 2 additions & 8 deletions src/pages/signin/LoginForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import * as PolicyUtils from '../../libs/PolicyUtils';
import Log from '../../libs/Log';
import withNavigationFocus, {withNavigationFocusPropTypes} from '../../components/withNavigationFocus';
import usePrevious from '../../hooks/usePrevious';
import * as MemoryOnlyKeys from '../../libs/actions/MemoryOnlyKeys/MemoryOnlyKeys';

const propTypes = {
/** Should we dismiss the keyboard when transitioning away from the page? */
Expand Down Expand Up @@ -74,13 +75,6 @@ const defaultProps = {
blurOnSubmit: false,
};

/**
* Enables experimental "memory only keys" mode in Onyx
*/
const setEnableMemoryOnlyKeys = () => {
window.enableMemoryOnlyKeys();
};

function LoginForm(props) {
const input = useRef();
const [login, setLogin] = useState('');
Expand Down Expand Up @@ -149,7 +143,7 @@ function LoginForm(props) {
// If the user has entered a guide email, then we are going to enable an experimental Onyx mode to help with performance
if (PolicyUtils.isExpensifyGuideTeam(loginTrim)) {
Log.info('Detected guide email in login field, setting memory only keys.');
setEnableMemoryOnlyKeys();
MemoryOnlyKeys.enable();
}

setFormError(null);
Expand Down
14 changes: 2 additions & 12 deletions src/setup/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import platformSetup from './platformSetup';
import * as Metrics from '../libs/Metrics';
import * as Device from '../libs/actions/Device';
import intlPolyfill from '../libs/IntlPolyfill';
import exposeGlobalMemoryOnlyKeysMethods from '../libs/actions/MemoryOnlyKeys/exposeGlobalMemoryOnlyKeysMethods';

export default function () {
/*
Expand Down Expand Up @@ -42,18 +43,7 @@ export default function () {
},
});

// When enabled we will skip persisting to disk any server-side downloaded objects (e.g. workspaces, chats, etc) that can hog up a user's resources.
window.enableMemoryOnlyKeys = () => {
// eslint-disable-next-line rulesdir/prefer-actions-set-data
Onyx.set(ONYXKEYS.IS_USING_MEMORY_ONLY_KEYS, true);
Onyx.setMemoryOnlyKeys([ONYXKEYS.COLLECTION.REPORT, ONYXKEYS.COLLECTION.POLICY, ONYXKEYS.PERSONAL_DETAILS_LIST]);
};

window.disableMemoryOnlyKeys = () => {
// eslint-disable-next-line rulesdir/prefer-actions-set-data
Onyx.set(ONYXKEYS.IS_USING_MEMORY_ONLY_KEYS, false);
Onyx.setMemoryOnlyKeys([]);
};
exposeGlobalMemoryOnlyKeysMethods();

Device.setDeviceID();

Expand Down