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

After revert: Defer local updates if there are missing updates and only call GetMissingOnyxMessages once #39683

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
068e624
Revert "Merge pull request #39668 from Expensify/revert-38997-@chrisp…
chrispader Apr 5, 2024
1a8528c
remove queue clearing and add TS type guard for OnyxUpdate
chrispader Apr 5, 2024
e52db00
fix: type check
chrispader Apr 5, 2024
2e13b36
simplify code
chrispader Apr 5, 2024
4fbbd9a
simplify code
chrispader Apr 5, 2024
9eaa455
fix: and simplify
chrispader Apr 5, 2024
c134283
add comment
chrispader Apr 5, 2024
36c9ad6
rename variables
chrispader Apr 5, 2024
c34337f
Merge branch 'main' into @chrispader/GetMissingOnyxMessages-deferred-…
chrispader Apr 15, 2024
48fa52e
remove unnecessary pause
chrispader Apr 15, 2024
beb16a3
check for isLoadingApp and ActiveClientManager first
chrispader Apr 15, 2024
1d396e1
Merge branch 'main' into @chrispader/GetMissingOnyxMessages-deferred-…
chrispader Apr 15, 2024
5f1bb97
Merge branch 'main' into @chrispader/GetMissingOnyxMessages-deferred-…
chrispader Apr 16, 2024
4cdaf5d
Merge branch 'main' into @chrispader/GetMissingOnyxMessages-deferred-…
chrispader Apr 21, 2024
26ded32
minor improvements of OnyxUpdateMangaer
chrispader Apr 21, 2024
e21abf5
implement tests
chrispader Apr 21, 2024
cbe2d9f
add exports for testing
chrispader Apr 21, 2024
2a29079
fix: test implementation
chrispader Apr 21, 2024
28004cb
implement proxy for OnyxUpdateManager
chrispader Apr 21, 2024
70e428e
update tests
chrispader Apr 21, 2024
0890696
extract util logic from OnyxUpdateManager
chrispader Apr 22, 2024
909b5a8
extract createProxyForValue and further split up utils
chrispader Apr 22, 2024
c502513
fix: tests
chrispader Apr 22, 2024
001eb3a
extract mocks from test
chrispader Apr 22, 2024
4eaf83a
apply updaes
chrispader Apr 22, 2024
9910943
remove unnecessary promise trigger code
chrispader Apr 22, 2024
b078cfa
update tests and mocks
chrispader Apr 22, 2024
cf3b7e6
Merge branch 'main' into @chrispader/GetMissingOnyxMessages-deferred-…
chrispader Apr 22, 2024
2c50853
fix: ts
chrispader Apr 22, 2024
3904ef4
fix: tests
chrispader Apr 23, 2024
abff1aa
move specific mocks to mock folders
chrispader Apr 23, 2024
39387f6
fix: update mocks and tests
chrispader Apr 23, 2024
6327968
simplify
chrispader Apr 23, 2024
b0a70ba
fix: update mocks, tests and test utils to allow for broader E2E tests
chrispader Apr 23, 2024
d0e3b68
add more tests and comments
chrispader Apr 23, 2024
52ad15a
remove waitForBatchedChanges
chrispader Apr 23, 2024
a7aac11
rename and move helper function
chrispader Apr 23, 2024
95fc4f6
add isPaused checker function to SequentialQueue
chrispader Apr 23, 2024
3e8bc28
reset more things before each test
chrispader Apr 23, 2024
5e42e99
add another test
chrispader Apr 23, 2024
f57a149
fix: first test
chrispader Apr 23, 2024
0bb6a51
Merge branch 'main' into @chrispader/GetMissingOnyxMessages-deferred-…
chrispader Apr 25, 2024
a738a51
remove unused line
chrispader Apr 25, 2024
23eab2d
Merge branch 'main' into @chrispader/GetMissingOnyxMessages-deferred-…
chrispader Apr 26, 2024
f48d931
Merge branch 'main' into @chrispader/GetMissingOnyxMessages-deferred-…
chrispader Apr 27, 2024
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
6 changes: 5 additions & 1 deletion src/libs/Network/SequentialQueue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ function isRunning(): boolean {
return isSequentialQueueRunning;
}

function isPaused(): boolean {
return isQueuePaused;
}

// Flush the queue when the connection resumes
NetworkStore.onReconnection(flush);

Expand Down Expand Up @@ -191,4 +195,4 @@ function waitForIdle(): Promise<unknown> {
return isReadyPromise;
}

export {flush, getCurrentRequest, isRunning, push, waitForIdle, pause, unpause};
export {flush, getCurrentRequest, isRunning, isPaused, push, waitForIdle, pause, unpause};
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ import Onyx from 'react-native-onyx';
import * as ActiveClientManager from '@libs/ActiveClientManager';
import Log from '@libs/Log';
import * as SequentialQueue from '@libs/Network/SequentialQueue';
import CONST from '@src/CONST';
import * as App from '@userActions/App';
import ONYXKEYS from '@src/ONYXKEYS';
import type {OnyxUpdatesFromServer} from '@src/types/onyx';
import * as App from './App';
import * as OnyxUpdates from './OnyxUpdates';
import type {OnyxUpdatesFromServer, Response} from '@src/types/onyx';
import {isValidOnyxUpdateFromServer} from '@src/types/onyx/OnyxUpdatesFromServer';
import * as OnyxUpdateManagerUtils from './utils';
import deferredUpdatesProxy from './utils/deferredUpdates';

// This file is in charge of looking at the updateIDs coming from the server and comparing them to the last updateID that the client has.
// If the client is behind the server, then we need to
Expand All @@ -19,15 +20,15 @@ import * as OnyxUpdates from './OnyxUpdates';
// 6. Restart the sequential queue
// 7. Restart the Onyx updates from Pusher
// This will ensure that the client is up-to-date with the server and all the updates have been applied in the correct order.
// It's important that this file is separate and not imported by OnyxUpdates.js, so that there are no circular dependencies. Onyx
// is used as a pub/sub mechanism to break out of the circular dependency.
// The circular dependency happens because this file calls API.GetMissingOnyxUpdates() which uses the SaveResponseInOnyx.js file
// (as a middleware). Therefore, SaveResponseInOnyx.js can't import and use this file directly.
// It's important that this file is separate and not imported by OnyxUpdates.js, so that there are no circular dependencies.
// Onyx is used as a pub/sub mechanism to break out of the circular dependency.
// The circular dependency happens because this file calls API.GetMissingOnyxUpdates() which uses the SaveResponseInOnyx.js file (as a middleware).
// Therefore, SaveResponseInOnyx.js can't import and use this file directly.

let lastUpdateIDAppliedToClient: number | null = 0;
let lastUpdateIDAppliedToClient = 0;
Onyx.connect({
key: ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT,
callback: (value) => (lastUpdateIDAppliedToClient = value),
callback: (value) => (lastUpdateIDAppliedToClient = value ?? 0),
});

let isLoadingApp = false;
Expand All @@ -38,17 +39,40 @@ Onyx.connect({
},
});

let queryPromise: Promise<Response | Response[] | void> | undefined;

let resolveQueryPromiseWrapper: () => void;
const createQueryPromiseWrapper = () =>
new Promise<void>((resolve) => {
resolveQueryPromiseWrapper = resolve;
});
// eslint-disable-next-line import/no-mutable-exports
let queryPromiseWrapper = createQueryPromiseWrapper();

const resetDeferralLogicVariables = () => {
queryPromise = undefined;
deferredUpdatesProxy.deferredUpdates = {};

This comment was marked as resolved.

};

// This function will reset the query variables, unpause the SequentialQueue and log an info to the user.
function finalizeUpdatesAndResumeQueue() {
console.debug('[OnyxUpdateManager] Done applying all updates');

resolveQueryPromiseWrapper();
queryPromiseWrapper = createQueryPromiseWrapper();

resetDeferralLogicVariables();
Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, null);
SequentialQueue.unpause();
}

/**
*
* @param onyxUpdatesFromServer
* @param clientLastUpdateID an optional override for the lastUpdateIDAppliedToClient
* @returns
*/
function handleOnyxUpdateGap(onyxUpdatesFromServer: OnyxEntry<OnyxUpdatesFromServer>, clientLastUpdateID = 0) {
// When there's no value, there's nothing to process, so let's return early.
if (!onyxUpdatesFromServer) {
return;
}
function handleOnyxUpdateGap(onyxUpdatesFromServer: OnyxEntry<OnyxUpdatesFromServer>, clientLastUpdateID?: number) {
// If isLoadingApp is positive it means that OpenApp command hasn't finished yet, and in that case
// we don't have base state of the app (reports, policies, etc) setup. If we apply this update,
// we'll only have them overriten by the openApp response. So let's skip it and return.
Expand All @@ -66,24 +90,15 @@ function handleOnyxUpdateGap(onyxUpdatesFromServer: OnyxEntry<OnyxUpdatesFromSer
return;
}

// Since we used the same key that used to store another object, let's confirm that the current object is
// following the new format before we proceed. If it isn't, then let's clear the object in Onyx.
if (
!(typeof onyxUpdatesFromServer === 'object' && !!onyxUpdatesFromServer) ||
!('type' in onyxUpdatesFromServer) ||
(!(onyxUpdatesFromServer.type === CONST.ONYX_UPDATE_TYPES.HTTPS && onyxUpdatesFromServer.request && onyxUpdatesFromServer.response) &&
!((onyxUpdatesFromServer.type === CONST.ONYX_UPDATE_TYPES.PUSHER || onyxUpdatesFromServer.type === CONST.ONYX_UPDATE_TYPES.AIRSHIP) && onyxUpdatesFromServer.updates))
) {
console.debug('[OnyxUpdateManager] Invalid format found for updates, cleaning and unpausing the queue');
Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, null);
SequentialQueue.unpause();
// When there is no value or an invalid value, there's nothing to process, so let's return early.
if (!isValidOnyxUpdateFromServer(onyxUpdatesFromServer)) {
return;
}

const updateParams = onyxUpdatesFromServer;
const lastUpdateIDFromServer = onyxUpdatesFromServer.lastUpdateID;
const previousUpdateIDFromServer = onyxUpdatesFromServer.previousUpdateID;
const lastUpdateIDFromClient = clientLastUpdateID || lastUpdateIDAppliedToClient;
const lastUpdateIDFromClient = clientLastUpdateID ?? lastUpdateIDAppliedToClient ?? 0;

// In cases where we received a previousUpdateID and it doesn't match our lastUpdateIDAppliedToClient
// we need to perform one of the 2 possible cases:
Expand All @@ -92,32 +107,45 @@ function handleOnyxUpdateGap(onyxUpdatesFromServer: OnyxEntry<OnyxUpdatesFromSer
// fully migrating to the reliable updates mode.
// 2. This client already has the reliable updates mode enabled, but it's missing some updates and it
// needs to fetch those.
let canUnpauseQueuePromise;

// The flow below is setting the promise to a reconnect app to address flow (1) explained above.
if (!lastUpdateIDFromClient) {
// If there is a ReconnectApp query in progress, we should not start another one.
if (queryPromise) {
return;
}

Log.info('Client has not gotten reliable updates before so reconnecting the app to start the process');

// Since this is a full reconnectApp, we'll not apply the updates we received - those will come in the reconnect app request.
canUnpauseQueuePromise = App.finalReconnectAppAfterActivatingReliableUpdates();
queryPromise = App.finalReconnectAppAfterActivatingReliableUpdates();
} else {
// The flow below is setting the promise to a getMissingOnyxUpdates to address flow (2) explained above.

// Get the number of deferred updates before adding the new one
const existingDeferredUpdatesCount = Object.keys(deferredUpdatesProxy.deferredUpdates).length;

// Add the new update to the deferred updates
deferredUpdatesProxy.deferredUpdates[Number(updateParams.lastUpdateID)] = updateParams;

// If there are deferred updates already, we don't need to fetch the missing updates again.
if (existingDeferredUpdatesCount > 0) {
return;
}

console.debug(`[OnyxUpdateManager] Client is behind the server by ${Number(previousUpdateIDFromServer) - lastUpdateIDFromClient} so fetching incremental updates`);
Log.info('Gap detected in update IDs from server so fetching incremental updates', true, {
lastUpdateIDFromServer,
previousUpdateIDFromServer,
lastUpdateIDFromClient,
});
canUnpauseQueuePromise = App.getMissingOnyxUpdates(lastUpdateIDFromClient, previousUpdateIDFromServer);

// Get the missing Onyx updates from the server and afterwards validate and apply the deferred updates.
// This will trigger recursive calls to "validateAndApplyDeferredUpdates" if there are gaps in the deferred updates.
queryPromise = App.getMissingOnyxUpdates(lastUpdateIDFromClient, previousUpdateIDFromServer).then(() => OnyxUpdateManagerUtils.validateAndApplyDeferredUpdates(clientLastUpdateID));
}

canUnpauseQueuePromise.finally(() => {
OnyxUpdates.apply(updateParams).finally(() => {
console.debug('[OnyxUpdateManager] Done applying all updates');
Onyx.set(ONYXKEYS.ONYX_UPDATES_FROM_SERVER, null);
SequentialQueue.unpause();
});
});
queryPromise.finally(finalizeUpdatesAndResumeQueue);
}

export default () => {
Expand All @@ -128,4 +156,4 @@ export default () => {
});
};

export {handleOnyxUpdateGap};
export {handleOnyxUpdateGap, queryPromiseWrapper as queryPromise, resetDeferralLogicVariables};
7 changes: 7 additions & 0 deletions src/libs/actions/OnyxUpdateManager/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import type {OnyxUpdatesFromServer} from '@src/types/onyx';

type DeferredUpdatesDictionary = Record<number, OnyxUpdatesFromServer>;

type DetectGapAndSplitResult = {applicableUpdates: DeferredUpdatesDictionary; updatesAfterGaps: DeferredUpdatesDictionary; latestMissingUpdateID: number | undefined};

export type {DeferredUpdatesDictionary, DetectGapAndSplitResult};
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import Onyx from 'react-native-onyx';
import type {DeferredUpdatesDictionary} from '@libs/actions/OnyxUpdateManager/types';
import ONYXKEYS from '@src/ONYXKEYS';
import createProxyForObject from '@src/utils/createProxyForObject';

let lastUpdateIDAppliedToClient = 0;
Onyx.connect({
key: ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT,
callback: (value) => (lastUpdateIDAppliedToClient = value ?? 0),
});

type ApplyUpdatesMockValues = {
onApplyUpdates: ((updates: DeferredUpdatesDictionary) => Promise<void>) | undefined;
};

type ApplyUpdatesMock = {
applyUpdates: jest.Mock<Promise<[]>, [updates: DeferredUpdatesDictionary]>;
mockValues: ApplyUpdatesMockValues;
};

const mockValues: ApplyUpdatesMockValues = {
onApplyUpdates: undefined,
};
const mockValuesProxy = createProxyForObject(mockValues);

const applyUpdates = jest.fn((updates: DeferredUpdatesDictionary) => {
const lastUpdateIdFromUpdates = Math.max(...Object.keys(updates).map(Number));
return (mockValuesProxy.onApplyUpdates === undefined ? Promise.resolve() : mockValuesProxy.onApplyUpdates(updates)).then(() =>
Onyx.set(ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT, Math.max(lastUpdateIDAppliedToClient, lastUpdateIdFromUpdates)),
);
});

export {applyUpdates, mockValuesProxy as mockValues};
export type {ApplyUpdatesMock};
32 changes: 32 additions & 0 deletions src/libs/actions/OnyxUpdateManager/utils/__mocks__/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import type {DeferredUpdatesDictionary, DetectGapAndSplitResult} from '@libs/actions/OnyxUpdateManager/types';
import createProxyForObject from '@src/utils/createProxyForObject';
import type * as OnyxUpdateManagerUtilsImport from '..';
import {applyUpdates} from './applyUpdates';

const UtilsImplementation: typeof OnyxUpdateManagerUtilsImport = jest.requireActual('@libs/actions/OnyxUpdateManager/utils');

type OnyxUpdateManagerUtilsMockValues = {
onValidateAndApplyDeferredUpdates: ((clientLastUpdateID?: number) => Promise<void>) | undefined;
};

type OnyxUpdateManagerUtilsMock = typeof UtilsImplementation & {
detectGapsAndSplit: jest.Mock<Promise<DetectGapAndSplitResult>, [updates: DeferredUpdatesDictionary, clientLastUpdateID?: number]>;
validateAndApplyDeferredUpdates: jest.Mock<Promise<void>, [clientLastUpdateID?: number]>;
mockValues: OnyxUpdateManagerUtilsMockValues;
};

const mockValues: OnyxUpdateManagerUtilsMockValues = {
onValidateAndApplyDeferredUpdates: undefined,
};
const mockValuesProxy = createProxyForObject(mockValues);

const detectGapsAndSplit = jest.fn(UtilsImplementation.detectGapsAndSplit);

const validateAndApplyDeferredUpdates = jest.fn((clientLastUpdateID?: number) =>
(mockValuesProxy.onValidateAndApplyDeferredUpdates === undefined ? Promise.resolve() : mockValuesProxy.onValidateAndApplyDeferredUpdates(clientLastUpdateID)).then(() =>
UtilsImplementation.validateAndApplyDeferredUpdates(clientLastUpdateID),
),
);

export {applyUpdates, detectGapsAndSplit, validateAndApplyDeferredUpdates, mockValuesProxy as mockValues};
export type {OnyxUpdateManagerUtilsMock};
9 changes: 9 additions & 0 deletions src/libs/actions/OnyxUpdateManager/utils/applyUpdates.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// We need to keep this in a separate file, so that we can mock this function in tests.
import type {DeferredUpdatesDictionary} from '@libs/actions/OnyxUpdateManager/types';
import * as OnyxUpdates from '@userActions/OnyxUpdates';

// This function applies a list of updates to Onyx in order and resolves when all updates have been applied
const applyUpdates = (updates: DeferredUpdatesDictionary) => Promise.all(Object.values(updates).map((update) => OnyxUpdates.apply(update)));

// eslint-disable-next-line import/prefer-default-export
export {applyUpdates};
8 changes: 8 additions & 0 deletions src/libs/actions/OnyxUpdateManager/utils/deferredUpdates.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import type {DeferredUpdatesDictionary} from '@libs/actions/OnyxUpdateManager/types';
import createProxyForObject from '@src/utils/createProxyForObject';

const deferredUpdatesValue = {deferredUpdates: {} as DeferredUpdatesDictionary};

const deferredUpdatesProxy = createProxyForObject(deferredUpdatesValue);

export default deferredUpdatesProxy;
Loading
Loading