diff --git a/src/ONYXKEYS.ts b/src/ONYXKEYS.ts index cb8bf2fdb5d..b9964d59b4b 100755 --- a/src/ONYXKEYS.ts +++ b/src/ONYXKEYS.ts @@ -32,6 +32,7 @@ const ONYXKEYS = { /** Note: These are Persisted Requests - not all requests in the main queue as the key name might lead one to believe */ PERSISTED_REQUESTS: 'networkRequestQueue', + PERSISTED_ONGOING_REQUESTS: 'networkOngoingRequestQueue', /** Stores current date */ CURRENT_DATE: 'currentDate', @@ -855,6 +856,7 @@ type OnyxValuesMapping = { [ONYXKEYS.DEVICE_ID]: string; [ONYXKEYS.IS_SIDEBAR_LOADED]: boolean; [ONYXKEYS.PERSISTED_REQUESTS]: OnyxTypes.Request[]; + [ONYXKEYS.PERSISTED_ONGOING_REQUESTS]: OnyxTypes.Request; [ONYXKEYS.CURRENT_DATE]: string; [ONYXKEYS.CREDENTIALS]: OnyxTypes.Credentials; [ONYXKEYS.STASHED_CREDENTIALS]: OnyxTypes.Credentials; diff --git a/src/libs/API/index.ts b/src/libs/API/index.ts index be1706886b1..960410d1d32 100644 --- a/src/libs/API/index.ts +++ b/src/libs/API/index.ts @@ -9,7 +9,7 @@ import * as Request from '@libs/Request'; import * as PersistedRequests from '@userActions/PersistedRequests'; import CONST from '@src/CONST'; import type OnyxRequest from '@src/types/onyx/Request'; -import type {PaginatedRequest, PaginationConfig} from '@src/types/onyx/Request'; +import type {PaginatedRequest, PaginationConfig, RequestConflictResolver} from '@src/types/onyx/Request'; import type Response from '@src/types/onyx/Response'; import type {ApiCommand, ApiRequestCommandParameters, ApiRequestType, CommandOfType, ReadCommand, SideEffectRequestCommand, WriteCommand} from './types'; @@ -45,7 +45,13 @@ type OnyxData = { /** * Prepare the request to be sent. Bind data together with request metadata and apply optimistic Onyx data. */ -function prepareRequest(command: TCommand, type: ApiRequestType, params: ApiRequestCommandParameters[TCommand], onyxData: OnyxData = {}): OnyxRequest { +function prepareRequest( + command: TCommand, + type: ApiRequestType, + params: ApiRequestCommandParameters[TCommand], + onyxData: OnyxData = {}, + conflictResolver: RequestConflictResolver = {}, +): OnyxRequest { Log.info('[API] Preparing request', false, {command, type}); const {optimisticData, ...onyxDataWithoutOptimisticData} = onyxData; @@ -71,6 +77,7 @@ function prepareRequest(command: TCommand, type: Ap command, data, ...onyxDataWithoutOptimisticData, + ...conflictResolver, }; if (isWriteRequest) { @@ -116,9 +123,15 @@ function processRequest(request: OnyxRequest, type: ApiRequestType): Promise(command: TCommand, apiCommandParameters: ApiRequestCommandParameters[TCommand], onyxData: OnyxData = {}): Promise { + +function write( + command: TCommand, + apiCommandParameters: ApiRequestCommandParameters[TCommand], + onyxData: OnyxData = {}, + conflictResolver: RequestConflictResolver = {}, +): Promise { Log.info('[API] Called API write', false, {command, ...apiCommandParameters}); - const request = prepareRequest(command, CONST.API_REQUEST_TYPE.WRITE, apiCommandParameters, onyxData); + const request = prepareRequest(command, CONST.API_REQUEST_TYPE.WRITE, apiCommandParameters, onyxData, conflictResolver); return processRequest(request, CONST.API_REQUEST_TYPE.WRITE); } diff --git a/src/libs/Middleware/HandleUnusedOptimisticID.ts b/src/libs/Middleware/HandleUnusedOptimisticID.ts index ad010880bdf..5e557dd86cf 100644 --- a/src/libs/Middleware/HandleUnusedOptimisticID.ts +++ b/src/libs/Middleware/HandleUnusedOptimisticID.ts @@ -34,14 +34,21 @@ const handleUnusedOptimisticID: Middleware = (requestResponse, request, isFromSe return; } const oldReportID = request.data?.reportID; - const offset = isFromSequentialQueue ? 1 : 0; - PersistedRequests.getAll() - .slice(offset) - .forEach((persistedRequest, index) => { - const persistedRequestClone = clone(persistedRequest); - persistedRequestClone.data = deepReplaceKeysAndValues(persistedRequest.data, oldReportID as string, preexistingReportID); - PersistedRequests.update(index + offset, persistedRequestClone); - }); + + if (isFromSequentialQueue) { + const ongoingRequest = PersistedRequests.getOngoingRequest(); + if (ongoingRequest && ongoingRequest.data?.reportID === oldReportID) { + const ongoingRequestClone = clone(ongoingRequest); + ongoingRequestClone.data = deepReplaceKeysAndValues(ongoingRequest.data, oldReportID as string, preexistingReportID); + PersistedRequests.updateOngoingRequest(ongoingRequestClone); + } + } + + PersistedRequests.getAll().forEach((persistedRequest, index) => { + const persistedRequestClone = clone(persistedRequest); + persistedRequestClone.data = deepReplaceKeysAndValues(persistedRequest.data, oldReportID as string, preexistingReportID); + PersistedRequests.update(index, persistedRequestClone); + }); }); return response; }); diff --git a/src/libs/Network/SequentialQueue.ts b/src/libs/Network/SequentialQueue.ts index 9442ebda3cf..35c7b2bf779 100644 --- a/src/libs/Network/SequentialQueue.ts +++ b/src/libs/Network/SequentialQueue.ts @@ -25,7 +25,7 @@ let isReadyPromise = new Promise((resolve) => { resolveIsReadyPromise?.(); let isSequentialQueueRunning = false; -let currentRequest: Promise | null = null; +let currentRequestPromise: Promise | null = null; let isQueuePaused = false; /** @@ -80,10 +80,14 @@ function process(): Promise { return Promise.resolve(); } - const requestToProcess = persistedRequests[0]; + const requestToProcess = PersistedRequests.processNextRequest(); + if (!requestToProcess) { + Log.info('[SequentialQueue] Unable to process. No next request to handle.'); + return Promise.resolve(); + } // Set the current request to a promise awaiting its processing so that getCurrentRequest can be used to take some action after the current request has processed. - currentRequest = Request.processWithMiddleware(requestToProcess, true) + currentRequestPromise = Request.processWithMiddleware(requestToProcess, true) .then((response) => { // A response might indicate that the queue should be paused. This happens when a gap in onyx updates is detected between the client and the server and // that gap needs resolved before the queue can continue. @@ -91,6 +95,7 @@ function process(): Promise { Log.info("[SequentialQueue] Handled 'shouldPauseQueue' in response. Pausing the queue."); pause(); } + PersistedRequests.remove(requestToProcess); RequestThrottle.clear(); return process(); @@ -103,6 +108,7 @@ function process(): Promise { RequestThrottle.clear(); return process(); } + PersistedRequests.rollbackOngoingRequest(); return RequestThrottle.sleep(error, requestToProcess.command) .then(process) .catch(() => { @@ -113,7 +119,7 @@ function process(): Promise { }); }); - return currentRequest; + return currentRequestPromise; } function flush() { @@ -161,7 +167,8 @@ function flush() { if (NetworkStore.isOffline() || PersistedRequests.getAll().length === 0) { resolveIsReadyPromise?.(); } - currentRequest = null; + currentRequestPromise = null; + // The queue can be paused when we sync the data with backend so we should only update the Onyx data when the queue is empty if (PersistedRequests.getAll().length === 0) { flushOnyxUpdatesQueue(); @@ -181,7 +188,7 @@ function unpause() { } const numberOfPersistedRequests = PersistedRequests.getAll().length || 0; - console.debug(`[SequentialQueue] Unpausing the queue and flushing ${numberOfPersistedRequests} requests`); + Log.info(`[SequentialQueue] Unpausing the queue and flushing ${numberOfPersistedRequests} requests`); isQueuePaused = false; flush(); } @@ -197,9 +204,29 @@ function isPaused(): boolean { // Flush the queue when the connection resumes NetworkStore.onReconnection(flush); -function push(request: OnyxRequest) { - // Add request to Persisted Requests so that it can be retried if it fails - PersistedRequests.save(request); +function push(newRequest: OnyxRequest) { + const {checkAndFixConflictingRequest} = newRequest; + + if (checkAndFixConflictingRequest) { + const requests = PersistedRequests.getAll(); + const {conflictAction} = checkAndFixConflictingRequest(requests); + Log.info(`[SequentialQueue] Conflict action for command ${newRequest.command} - ${conflictAction.type}:`); + + // don't try to serialize a function. + // eslint-disable-next-line no-param-reassign + delete newRequest.checkAndFixConflictingRequest; + + if (conflictAction.type === 'push') { + PersistedRequests.save(newRequest); + } else if (conflictAction.type === 'replace') { + PersistedRequests.update(conflictAction.index, newRequest); + } else { + Log.info(`[SequentialQueue] No action performed to command ${newRequest.command} and it will be ignored.`); + } + } else { + // Add request to Persisted Requests so that it can be retried if it fails + PersistedRequests.save(newRequest); + } // If we are offline we don't need to trigger the queue to empty as it will happen when we come back online if (NetworkStore.isOffline()) { @@ -216,10 +243,10 @@ function push(request: OnyxRequest) { } function getCurrentRequest(): Promise { - if (currentRequest === null) { + if (currentRequestPromise === null) { return Promise.resolve(); } - return currentRequest; + return currentRequestPromise; } /** @@ -229,5 +256,5 @@ function waitForIdle(): Promise { return isReadyPromise; } -export {flush, getCurrentRequest, isRunning, isPaused, push, waitForIdle, pause, unpause}; +export {flush, getCurrentRequest, isRunning, isPaused, push, waitForIdle, pause, unpause, process}; export type {RequestError}; diff --git a/src/libs/ReportActionsUtils.ts b/src/libs/ReportActionsUtils.ts index 15d7728ba35..6b176edb691 100644 --- a/src/libs/ReportActionsUtils.ts +++ b/src/libs/ReportActionsUtils.ts @@ -58,7 +58,6 @@ Onyx.connect({ if (!actions) { return; } - allReportActions = actions; }, }); diff --git a/src/libs/actions/App.ts b/src/libs/actions/App.ts index 6b6f1a5f6dc..8a66a9702ac 100644 --- a/src/libs/actions/App.ts +++ b/src/libs/actions/App.ts @@ -273,7 +273,25 @@ function reconnectApp(updateIDFrom: OnyxEntry = 0) { params.updateIDFrom = updateIDFrom; } - API.write(WRITE_COMMANDS.RECONNECT_APP, params, getOnyxDataForOpenOrReconnect()); + API.write(WRITE_COMMANDS.RECONNECT_APP, params, getOnyxDataForOpenOrReconnect(), { + checkAndFixConflictingRequest: (persistedRequests) => { + const index = persistedRequests.findIndex((request) => request.command === WRITE_COMMANDS.RECONNECT_APP); + if (index === -1) { + return { + conflictAction: { + type: 'push', + }, + }; + } + + return { + conflictAction: { + type: 'replace', + index, + }, + }; + }, + }); }); } diff --git a/src/libs/actions/PersistedRequests.ts b/src/libs/actions/PersistedRequests.ts index 851e5387650..77d6e388438 100644 --- a/src/libs/actions/PersistedRequests.ts +++ b/src/libs/actions/PersistedRequests.ts @@ -5,24 +5,46 @@ import ONYXKEYS from '@src/ONYXKEYS'; import type {Request} from '@src/types/onyx'; let persistedRequests: Request[] = []; +let ongoingRequest: Request | null = null; Onyx.connect({ key: ONYXKEYS.PERSISTED_REQUESTS, - callback: (val) => (persistedRequests = val ?? []), + callback: (val) => { + persistedRequests = val ?? []; + + if (ongoingRequest && persistedRequests.length > 0) { + const nextRequestToProcess = persistedRequests[0]; + + // We try to remove the next request from the persistedRequests if it is the same as ongoingRequest + // so we don't process it twice. + if (isEqual(nextRequestToProcess, ongoingRequest)) { + persistedRequests = persistedRequests.slice(1); + } + } + }, +}); +Onyx.connect({ + key: ONYXKEYS.PERSISTED_ONGOING_REQUESTS, + callback: (val) => { + ongoingRequest = val ?? null; + }, }); /** * This promise is only used by tests. DO NOT USE THIS PROMISE IN THE APPLICATION CODE */ function clear() { + ongoingRequest = null; return Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, []); } function getLength(): number { - return persistedRequests.length; + // Making it backwards compatible with the old implementation + return persistedRequests.length + (ongoingRequest ? 1 : 0); } function save(requestToPersist: Request) { + // If the command is not in the keepLastInstance array, add the new request as usual const requests = [...persistedRequests, requestToPersist]; persistedRequests = requests; Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, requests).then(() => { @@ -31,18 +53,24 @@ function save(requestToPersist: Request) { } function remove(requestToRemove: Request) { + ongoingRequest = null; /** * We only remove the first matching request because the order of requests matters. * If we were to remove all matching requests, we can end up with a final state that is different than what the user intended. */ const requests = [...persistedRequests]; const index = requests.findIndex((persistedRequest) => isEqual(persistedRequest, requestToRemove)); - if (index === -1) { - return; + + if (index !== -1) { + requests.splice(index, 1); } - requests.splice(index, 1); + persistedRequests = requests; - Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, requests).then(() => { + + Onyx.multiSet({ + [ONYXKEYS.PERSISTED_REQUESTS]: persistedRequests, + [ONYXKEYS.PERSISTED_ONGOING_REQUESTS]: null, + }).then(() => { Log.info(`[SequentialQueue] '${requestToRemove.command}' removed from the queue. Queue length is ${getLength()}`); }); } @@ -54,8 +82,52 @@ function update(oldRequestIndex: number, newRequest: Request) { Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, requests); } +function updateOngoingRequest(newRequest: Request) { + ongoingRequest = newRequest; + + if (newRequest.persistWhenOngoing) { + Onyx.set(ONYXKEYS.PERSISTED_ONGOING_REQUESTS, newRequest); + } +} + +function processNextRequest(): Request | null { + if (ongoingRequest) { + Log.info(`Ongoing Request already set returning same one ${ongoingRequest.commandName}`); + return ongoingRequest; + } + + // You must handle the case where there are no requests to process + if (persistedRequests.length === 0) { + throw new Error('No requests to process'); + } + + ongoingRequest = persistedRequests.shift() ?? null; + + if (ongoingRequest && ongoingRequest.persistWhenOngoing) { + Onyx.set(ONYXKEYS.PERSISTED_ONGOING_REQUESTS, ongoingRequest); + } + + return ongoingRequest; +} + +function rollbackOngoingRequest() { + if (!ongoingRequest) { + return; + } + + // Prepend ongoingRequest to persistedRequests + persistedRequests.unshift(ongoingRequest); + + // Clear the ongoingRequest + ongoingRequest = null; +} + function getAll(): Request[] { return persistedRequests; } -export {clear, save, getAll, remove, update, getLength}; +function getOngoingRequest(): Request | null { + return ongoingRequest; +} + +export {clear, save, getAll, remove, update, getLength, getOngoingRequest, processNextRequest, updateOngoingRequest, rollbackOngoingRequest}; diff --git a/src/types/onyx/Request.ts b/src/types/onyx/Request.ts index 1f3f788a841..238e3a8c6a8 100644 --- a/src/types/onyx/Request.ts +++ b/src/types/onyx/Request.ts @@ -55,8 +55,70 @@ type RequestData = { shouldSkipWebProxy?: boolean; }; +/** + * Model of a conflict request that has to be replaced in the request queue. + */ +type ConflictRequestReplace = { + /** + * The action to take in case of a conflict. + */ + type: 'replace'; + + /** + * The index of the request in the queue to update. + */ + index: number; +}; + +/** + * Model of a conflict request that has to be enqueued at the end of request queue. + */ +type ConflictRequestPush = { + /** + * The action to take in case of a conflict. + */ + type: 'push'; +}; + +/** + * Model of a conflict request that does not need to be updated or saved in the request queue. + */ +type ConflictRequestNoAction = { + /** + * The action to take in case of a conflict. + */ + type: 'noAction'; +}; + +/** + * An object that has the request and the action to take in case of a conflict. + */ +type ConflictActionData = { + /** + * The action to take in case of a conflict. + */ + conflictAction: ConflictRequestReplace | ConflictRequestPush | ConflictRequestNoAction; +}; + +/** + * An object that describes how a new write request can identify any queued requests that may conflict with or be undone by the new request, + * and how to resolve those conflicts. + */ +type RequestConflictResolver = { + /** + * A function that checks if a new request conflicts with any existing requests in the queue. + */ + checkAndFixConflictingRequest?: (persistedRequest: Request[]) => ConflictActionData; + + /** + * A boolean flag to mark a request as persisting into Onyx, if set to true it means when Onyx loads + * the ongoing request, it will be removed from the persisted request queue. + */ + persistWhenOngoing?: boolean; +}; + /** Model of requests sent to the API */ -type Request = RequestData & OnyxData; +type Request = RequestData & OnyxData & RequestConflictResolver; /** * An object used to describe how a request can be paginated. @@ -85,4 +147,4 @@ type PaginatedRequest = Request & }; export default Request; -export type {OnyxData, RequestType, PaginationConfig, PaginatedRequest}; +export type {OnyxData, RequestType, PaginationConfig, PaginatedRequest, RequestConflictResolver, ConflictActionData}; diff --git a/tests/actions/SessionTest.ts b/tests/actions/SessionTest.ts index 62d6a54b20b..e46806bef99 100644 --- a/tests/actions/SessionTest.ts +++ b/tests/actions/SessionTest.ts @@ -3,10 +3,12 @@ import Onyx from 'react-native-onyx'; import type {OnyxEntry} from 'react-native-onyx'; import * as App from '@libs/actions/App'; import OnyxUpdateManager from '@libs/actions/OnyxUpdateManager'; +import {WRITE_COMMANDS} from '@libs/API/types'; import HttpUtils from '@libs/HttpUtils'; import PushNotification from '@libs/Notification/PushNotification'; // This lib needs to be imported, but it has nothing to export since all it contains is an Onyx connection import '@libs/Notification/PushNotification/subscribePushNotification'; +import * as PersistedRequests from '@userActions/PersistedRequests'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Credentials, Session} from '@src/types/onyx'; @@ -28,7 +30,7 @@ OnyxUpdateManager(); beforeEach(() => Onyx.clear().then(waitForBatchedUpdates)); describe('Session', () => { - test('Authenticate is called with saved credentials when a session expires', () => { + test('Authenticate is called with saved credentials when a session expires', async () => { // Given a test user and set of authToken with subscriptions to session and credentials const TEST_USER_LOGIN = 'test@testguy.com'; const TEST_USER_ACCOUNT_ID = 1; @@ -48,61 +50,100 @@ describe('Session', () => { }); // When we sign in with the test user - return TestHelper.signInWithTestUser(TEST_USER_ACCOUNT_ID, TEST_USER_LOGIN, 'Password1', TEST_INITIAL_AUTH_TOKEN) - .then(waitForBatchedUpdates) - .then(() => { - // Then our re-authentication credentials should be generated and our session data - // have the correct information + initial authToken. - expect(credentials?.login).toBe(TEST_USER_LOGIN); - expect(credentials?.autoGeneratedLogin).not.toBeUndefined(); - expect(credentials?.autoGeneratedPassword).not.toBeUndefined(); - expect(session?.authToken).toBe(TEST_INITIAL_AUTH_TOKEN); - expect(session?.accountID).toBe(TEST_USER_ACCOUNT_ID); - expect(session?.email).toBe(TEST_USER_LOGIN); - - // At this point we have an authToken. To simulate it expiring we'll just make another - // request and mock the response so it returns 407. Once this happens we should attempt - // to Re-Authenticate with the stored credentials. Our next call will be to Authenticate - // so we will mock that response with a new authToken and then verify that Onyx has our - // data. - (HttpUtils.xhr as jest.MockedFunction) - - // This will make the call to OpenApp below return with an expired session code - .mockImplementationOnce(() => - Promise.resolve({ - jsonCode: CONST.JSON_CODE.NOT_AUTHENTICATED, - }), - ) - - // The next call should be Authenticate since we are reauthenticating - .mockImplementationOnce(() => - Promise.resolve({ - jsonCode: CONST.JSON_CODE.SUCCESS, - accountID: TEST_USER_ACCOUNT_ID, - authToken: TEST_REFRESHED_AUTH_TOKEN, - email: TEST_USER_LOGIN, - }), - ); - - // When we attempt to fetch the initial app data via the API - App.confirmReadyToOpenApp(); - App.openApp(); - return waitForBatchedUpdates(); - }) - .then(() => { - // Then it should fail and reauthenticate the user adding the new authToken to the session - // data in Onyx - expect(session?.authToken).toBe(TEST_REFRESHED_AUTH_TOKEN); - }); + await TestHelper.signInWithTestUser(TEST_USER_ACCOUNT_ID, TEST_USER_LOGIN, 'Password1', TEST_INITIAL_AUTH_TOKEN); + await waitForBatchedUpdates(); + + // Then our re-authentication credentials should be generated and our session data + // have the correct information + initial authToken. + expect(credentials?.login).toBe(TEST_USER_LOGIN); + expect(credentials?.autoGeneratedLogin).not.toBeUndefined(); + expect(credentials?.autoGeneratedPassword).not.toBeUndefined(); + expect(session?.authToken).toBe(TEST_INITIAL_AUTH_TOKEN); + expect(session?.accountID).toBe(TEST_USER_ACCOUNT_ID); + expect(session?.email).toBe(TEST_USER_LOGIN); + + // At this point we have an authToken. To simulate it expiring we'll just make another + // request and mock the response so it returns 407. Once this happens we should attempt + // to Re-Authenticate with the stored credentials. Our next call will be to Authenticate + // so we will mock that response with a new authToken and then verify that Onyx has our + // data. + (HttpUtils.xhr as jest.MockedFunction) + + // This will make the call to OpenApp below return with an expired session code + .mockImplementationOnce(() => + Promise.resolve({ + jsonCode: CONST.JSON_CODE.NOT_AUTHENTICATED, + }), + ) + + // The next call should be Authenticate since we are reauthenticating + .mockImplementationOnce(() => + Promise.resolve({ + jsonCode: CONST.JSON_CODE.SUCCESS, + accountID: TEST_USER_ACCOUNT_ID, + authToken: TEST_REFRESHED_AUTH_TOKEN, + email: TEST_USER_LOGIN, + }), + ); + + // When we attempt to fetch the initial app data via the API + App.confirmReadyToOpenApp(); + App.openApp(); + await waitForBatchedUpdates(); + + // Then it should fail and reauthenticate the user adding the new authToken to the session + // data in Onyx + expect(session?.authToken).toBe(TEST_REFRESHED_AUTH_TOKEN); }); - test('Push notifications are subscribed after signing in', () => - TestHelper.signInWithTestUser() - .then(waitForBatchedUpdates) - .then(() => expect(PushNotification.register).toBeCalled())); + test('Push notifications are subscribed after signing in', async () => { + await TestHelper.signInWithTestUser(); + await waitForBatchedUpdates(); + expect(PushNotification.register).toBeCalled(); + }); + + test('Push notifications are unsubscribed after signing out', async () => { + await TestHelper.signInWithTestUser(); + await TestHelper.signOutTestUser(); + expect(PushNotification.deregister).toBeCalled(); + }); + + test('ReconnectApp should push request to the queue', async () => { + await TestHelper.signInWithTestUser(); + await Onyx.set(ONYXKEYS.NETWORK, {isOffline: true}); + + App.confirmReadyToOpenApp(); + App.reconnectApp(); + + await waitForBatchedUpdates(); - test('Push notifications are unsubscribed after signing out', () => - TestHelper.signInWithTestUser() - .then(TestHelper.signOutTestUser) - .then(() => expect(PushNotification.deregister).toBeCalled())); + expect(PersistedRequests.getAll().length).toBe(1); + expect(PersistedRequests.getAll()[0].command).toBe(WRITE_COMMANDS.RECONNECT_APP); + + await Onyx.set(ONYXKEYS.NETWORK, {isOffline: false}); + + await waitForBatchedUpdates(); + + expect(PersistedRequests.getAll().length).toBe(0); + }); + + test('ReconnectApp should replace same requests from the queue', async () => { + await TestHelper.signInWithTestUser(); + await Onyx.set(ONYXKEYS.NETWORK, {isOffline: true}); + + App.confirmReadyToOpenApp(); + App.reconnectApp(); + App.reconnectApp(); + App.reconnectApp(); + App.reconnectApp(); + + await waitForBatchedUpdates(); + + expect(PersistedRequests.getAll().length).toBe(1); + expect(PersistedRequests.getAll()[0].command).toBe(WRITE_COMMANDS.RECONNECT_APP); + + await Onyx.set(ONYXKEYS.NETWORK, {isOffline: false}); + + expect(PersistedRequests.getAll().length).toBe(0); + }); }); diff --git a/tests/unit/APITest.ts b/tests/unit/APITest.ts index 74be6c742f5..63d38fd7079 100644 --- a/tests/unit/APITest.ts +++ b/tests/unit/APITest.ts @@ -43,6 +43,7 @@ const originalXHR = HttpUtils.xhr; beforeEach(() => { global.fetch = TestHelper.getGlobalFetchMock(); HttpUtils.xhr = originalXHR; + MainQueue.clear(); HttpUtils.cancelPendingRequests(); PersistedRequests.clear(); @@ -166,36 +167,38 @@ describe('APITests', () => { .then(waitForBatchedUpdates) .then(() => { // Then requests should remain persisted until the xhr call is resolved - expect(PersistedRequests.getAll().length).toEqual(2); + expect(PersistedRequests.getAll().length).toEqual(1); xhrCalls[0].resolve({jsonCode: CONST.JSON_CODE.SUCCESS}); return waitForBatchedUpdates(); }) .then(waitForBatchedUpdates) .then(() => { - expect(PersistedRequests.getAll().length).toEqual(1); - expect(PersistedRequests.getAll()).toEqual([expect.objectContaining({command: 'mock command', data: expect.objectContaining({param2: 'value2'})})]); + expect(PersistedRequests.getAll().length).toEqual(0); + expect(PersistedRequests.getOngoingRequest()).toEqual(expect.objectContaining({command: 'mock command', data: expect.objectContaining({param2: 'value2'})})); // When a request fails it should be retried xhrCalls[1].reject(new Error(CONST.ERROR.FAILED_TO_FETCH)); return waitForBatchedUpdates(); }) .then(() => { + // The ongoingRequest it is moving back to the persistedRequests queue expect(PersistedRequests.getAll().length).toEqual(1); expect(PersistedRequests.getAll()).toEqual([expect.objectContaining({command: 'mock command', data: expect.objectContaining({param2: 'value2'})})]); - // We need to advance past the request throttle back off timer because the request won't be retried until then return new Promise((resolve) => { setTimeout(resolve, CONST.NETWORK.MAX_RANDOM_RETRY_WAIT_TIME_MS); }).then(waitForBatchedUpdates); }) .then(() => { + // A new promise is created after the back off timer // Finally, after it succeeds the queue should be empty xhrCalls[2].resolve({jsonCode: CONST.JSON_CODE.SUCCESS}); return waitForBatchedUpdates(); }) .then(() => { expect(PersistedRequests.getAll().length).toEqual(0); + expect(PersistedRequests.getOngoingRequest()).toBeNull(); }) ); }); @@ -551,7 +554,6 @@ describe('APITests', () => { // THEN the queue should be stopped and there should be no more requests to run expect(SequentialQueue.isRunning()).toBe(false); expect(PersistedRequests.getAll().length).toBe(0); - // And our Write request should run before our non persistable one in a blocking way const firstRequest = xhr.mock.calls[0]; const [firstRequestCommandName] = firstRequest; diff --git a/tests/unit/PersistedRequests.ts b/tests/unit/PersistedRequests.ts index 670625f65f9..476b3f96395 100644 --- a/tests/unit/PersistedRequests.ts +++ b/tests/unit/PersistedRequests.ts @@ -1,5 +1,9 @@ +import Onyx from 'react-native-onyx'; import * as PersistedRequests from '../../src/libs/actions/PersistedRequests'; +import ONYXKEYS from '../../src/ONYXKEYS'; import type Request from '../../src/types/onyx/Request'; +import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; +import wrapOnyxWithWaitForBatchedUpdates from '../utils/wrapOnyxWithWaitForBatchedUpdates'; const request: Request = { command: 'OpenReport', @@ -7,13 +11,22 @@ const request: Request = { failureData: [{key: 'reportMetadata_2', onyxMethod: 'merge', value: {}}], }; +beforeAll(() => + Onyx.init({ + keys: ONYXKEYS, + safeEvictionKeys: [ONYXKEYS.COLLECTION.REPORT_ACTIONS], + }), +); + beforeEach(() => { + wrapOnyxWithWaitForBatchedUpdates(Onyx); PersistedRequests.clear(); PersistedRequests.save(request); }); afterEach(() => { PersistedRequests.clear(); + Onyx.clear(); }); describe('PersistedRequests', () => { @@ -26,4 +39,53 @@ describe('PersistedRequests', () => { PersistedRequests.remove(request); expect(PersistedRequests.getAll().length).toBe(0); }); + + it('when process the next request, queue should be empty', () => { + const nextRequest = PersistedRequests.processNextRequest(); + expect(PersistedRequests.getAll().length).toBe(0); + expect(nextRequest).toEqual(request); + }); + + it('when onyx persist the request, it should remove from the list the ongoing request', () => { + expect(PersistedRequests.getAll().length).toBe(1); + const request2: Request = { + command: 'AddComment', + successData: [{key: 'reportMetadata_3', onyxMethod: 'merge', value: {}}], + failureData: [{key: 'reportMetadata_4', onyxMethod: 'merge', value: {}}], + }; + PersistedRequests.save(request2); + PersistedRequests.processNextRequest(); + return waitForBatchedUpdates().then(() => { + expect(PersistedRequests.getAll().length).toBe(1); + expect(PersistedRequests.getAll()[0]).toEqual(request2); + }); + }); + + it('update the request at the given index with new data', () => { + const newRequest: Request = { + command: 'OpenReport', + successData: [{key: 'reportMetadata_1', onyxMethod: 'set', value: {}}], + failureData: [{key: 'reportMetadata_2', onyxMethod: 'set', value: {}}], + }; + PersistedRequests.update(0, newRequest); + expect(PersistedRequests.getAll()[0]).toEqual(newRequest); + }); + + it('update the ongoing request with new data', () => { + const newRequest: Request = { + command: 'OpenReport', + successData: [{key: 'reportMetadata_1', onyxMethod: 'set', value: {}}], + failureData: [{key: 'reportMetadata_2', onyxMethod: 'set', value: {}}], + }; + PersistedRequests.updateOngoingRequest(newRequest); + expect(PersistedRequests.getOngoingRequest()).toEqual(newRequest); + }); + + it('when removing a request should update the persistedRequests queue and clear the ongoing request', () => { + PersistedRequests.processNextRequest(); + expect(PersistedRequests.getOngoingRequest()).toEqual(request); + PersistedRequests.remove(request); + expect(PersistedRequests.getOngoingRequest()).toBeNull(); + expect(PersistedRequests.getAll().length).toBe(0); + }); }); diff --git a/tests/unit/SequentialQueueTest.ts b/tests/unit/SequentialQueueTest.ts new file mode 100644 index 00000000000..8651d7e95e3 --- /dev/null +++ b/tests/unit/SequentialQueueTest.ts @@ -0,0 +1,260 @@ +import Onyx from 'react-native-onyx'; +import {waitForActiveRequestsToBeEmpty} from '@libs/E2E/utils/NetworkInterceptor'; +import * as PersistedRequests from '@userActions/PersistedRequests'; +import ONYXKEYS from '@src/ONYXKEYS'; +import * as SequentialQueue from '../../src/libs/Network/SequentialQueue'; +import type Request from '../../src/types/onyx/Request'; +import type {ConflictActionData} from '../../src/types/onyx/Request'; +import * as TestHelper from '../utils/TestHelper'; +import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; + +const request: Request = { + command: 'ReconnectApp', + successData: [{key: 'userMetadata', onyxMethod: 'set', value: {accountID: 1234}}], + failureData: [{key: 'userMetadata', onyxMethod: 'set', value: {}}], +}; + +describe('SequentialQueue', () => { + beforeAll(() => { + Onyx.init({ + keys: ONYXKEYS, + }); + }); + beforeEach(() => { + global.fetch = TestHelper.getGlobalFetchMock(); + return Onyx.clear().then(waitForBatchedUpdates); + }); + + it('should push one request and persist one', () => { + SequentialQueue.push(request); + expect(PersistedRequests.getLength()).toBe(1); + }); + + it('should push two requests and persist two', () => { + SequentialQueue.push(request); + SequentialQueue.push(request); + expect(PersistedRequests.getLength()).toBe(2); + }); + + it('should push two requests with conflict resolution and replace', () => { + SequentialQueue.push(request); + const requestWithConflictResolution: Request = { + command: 'ReconnectApp', + data: {accountID: 56789}, + checkAndFixConflictingRequest: (persistedRequests) => { + // should be one instance of ReconnectApp, get the index to replace it later + const index = persistedRequests.findIndex((r) => r.command === 'ReconnectApp'); + if (index === -1) { + return {conflictAction: {type: 'push'}}; + } + + return { + conflictAction: {type: 'replace', index}, + }; + }, + }; + SequentialQueue.push(requestWithConflictResolution); + expect(PersistedRequests.getLength()).toBe(1); + // We know there is only one request in the queue, so we can get the first one and verify + // that the persisted request is the second one. + const persistedRequest = PersistedRequests.getAll()[0]; + expect(persistedRequest?.data?.accountID).toBe(56789); + }); + + it('should push two requests with conflict resolution and push', () => { + SequentialQueue.push(request); + const requestWithConflictResolution: Request = { + command: 'ReconnectApp', + data: {accountID: 56789}, + checkAndFixConflictingRequest: () => { + return {conflictAction: {type: 'push'}}; + }, + }; + SequentialQueue.push(requestWithConflictResolution); + expect(PersistedRequests.getLength()).toBe(2); + }); + + it('should push two requests with conflict resolution and noAction', () => { + SequentialQueue.push(request); + const requestWithConflictResolution: Request = { + command: 'ReconnectApp', + data: {accountID: 56789}, + checkAndFixConflictingRequest: () => { + return {conflictAction: {type: 'noAction'}}; + }, + }; + SequentialQueue.push(requestWithConflictResolution); + expect(PersistedRequests.getLength()).toBe(1); + }); + + it('should add a new request even if a similar one is ongoing', async () => { + // .push at the end flush the queue + SequentialQueue.push(request); + + // wait for Onyx.connect execute the callback and start processing the queue + await Promise.resolve(); + + const requestWithConflictResolution: Request = { + command: 'ReconnectApp', + data: {accountID: 56789}, + checkAndFixConflictingRequest: (persistedRequests) => { + // should be one instance of ReconnectApp, get the index to replace it later + const index = persistedRequests.findIndex((r) => r.command === 'ReconnectApp'); + if (index === -1) { + return {conflictAction: {type: 'push'}}; + } + + return { + conflictAction: {type: 'replace', index}, + }; + }, + }; + + SequentialQueue.push(requestWithConflictResolution); + expect(PersistedRequests.getLength()).toBe(2); + }); + + it('should replace request request in queue while a similar one is ongoing', async () => { + // .push at the end flush the queue + SequentialQueue.push(request); + + // wait for Onyx.connect execute the callback and start processing the queue + await Promise.resolve(); + + const conflictResolver = (persistedRequests: Request[]): ConflictActionData => { + // should be one instance of ReconnectApp, get the index to replace it later + const index = persistedRequests.findIndex((r) => r.command === 'ReconnectApp'); + if (index === -1) { + return {conflictAction: {type: 'push'}}; + } + + return { + conflictAction: {type: 'replace', index}, + }; + }; + + const requestWithConflictResolution: Request = { + command: 'ReconnectApp', + data: {accountID: 56789}, + checkAndFixConflictingRequest: conflictResolver, + }; + + const requestWithConflictResolution2: Request = { + command: 'ReconnectApp', + data: {accountID: 56789}, + checkAndFixConflictingRequest: conflictResolver, + }; + + SequentialQueue.push(requestWithConflictResolution); + SequentialQueue.push(requestWithConflictResolution2); + + expect(PersistedRequests.getLength()).toBe(2); + }); + + it('should replace request request in queue while a similar one is ongoing and keep the same index', () => { + SequentialQueue.push({command: 'OpenReport'}); + SequentialQueue.push(request); + + const requestWithConflictResolution: Request = { + command: 'ReconnectApp', + data: {accountID: 56789}, + checkAndFixConflictingRequest: (persistedRequests) => { + // should be one instance of ReconnectApp, get the index to replace it later + const index = persistedRequests.findIndex((r) => r.command === 'ReconnectApp'); + if (index === -1) { + return {conflictAction: {type: 'push'}}; + } + + return { + conflictAction: {type: 'replace', index}, + }; + }, + }; + + SequentialQueue.push(requestWithConflictResolution); + SequentialQueue.push({command: 'AddComment'}); + SequentialQueue.push({command: 'OpenReport'}); + + expect(PersistedRequests.getLength()).toBe(4); + const persistedRequests = PersistedRequests.getAll(); + // We know ReconnectApp is at index 1 in the queue, so we can get it to verify + // that was replaced by the new request. + expect(persistedRequests[1]?.data?.accountID).toBe(56789); + }); + + // need to test a rance condition between processing the next request and then pushing a new request with conflict resolver + it('should resolve the conflict and replace the correct request in the queue while a new request is picked up after unpausing', async () => { + SequentialQueue.pause(); + for (let i = 0; i < 5; i++) { + SequentialQueue.push({command: `OpenReport${i}`}); + SequentialQueue.push({command: `AddComment${i}`}); + } + SequentialQueue.push(request); + SequentialQueue.push({command: 'AddComment6'}); + SequentialQueue.push({command: 'OpenReport6'}); + // wait for Onyx.connect execute the callback and start processing the queue + await Promise.resolve(); + const requestWithConflictResolution: Request = { + command: 'ReconnectApp-replaced', + data: {accountID: 56789}, + checkAndFixConflictingRequest: (persistedRequests) => { + // should be one instance of ReconnectApp, get the index to replace it later + const index = persistedRequests.findIndex((r) => r.command === 'ReconnectApp'); + if (index === -1) { + return {conflictAction: {type: 'push'}}; + } + + return { + conflictAction: {type: 'replace', index}, + }; + }, + }; + + Promise.resolve().then(() => { + SequentialQueue.unpause(); + }); + Promise.resolve().then(() => { + SequentialQueue.push(requestWithConflictResolution); + }); + + await Promise.resolve(); + await waitForActiveRequestsToBeEmpty(); + const persistedRequests = PersistedRequests.getAll(); + + // We know ReconnectApp is at index 9 in the queue, so we can get it to verify + // that was replaced by the new request. + expect(persistedRequests[9]?.command).toBe('ReconnectApp-replaced'); + expect(persistedRequests[9]?.data?.accountID).toBe(56789); + }); + + // I need to test now when moving the request from the queue to the ongoing request the PERSISTED_REQUESTS is decreased and PERSISTED_ONGOING_REQUESTS has the new request + it('should move the request from the queue to the ongoing request and save it into Onyx', () => { + const persistedRequest = {...request, persistWhenOngoing: true}; + SequentialQueue.push(persistedRequest); + + const connectionId = Onyx.connect({ + key: ONYXKEYS.PERSISTED_ONGOING_REQUESTS, + callback: (ongoingRequest) => { + if (!ongoingRequest) { + return; + } + + Onyx.disconnect(connectionId); + expect(ongoingRequest).toEqual(persistedRequest); + expect(ongoingRequest).toEqual(PersistedRequests.getOngoingRequest()); + expect(PersistedRequests.getAll().length).toBe(0); + }, + }); + }); + + it('should get the ongoing request from onyx and start processing it', async () => { + const persistedRequest = {...request, persistWhenOngoing: true}; + Onyx.set(ONYXKEYS.PERSISTED_ONGOING_REQUESTS, persistedRequest); + SequentialQueue.push({command: 'OpenReport'}); + + await Promise.resolve(); + + expect(persistedRequest).toEqual(PersistedRequests.getOngoingRequest()); + expect(PersistedRequests.getAll().length).toBe(1); + }); +});