Make both main and sequential queue wait for credentials

src/libs/ActiveClientManager/index.js

@@ -12,7 +12,14 @@ let activeClients;
 
 // Keeps track of the ActiveClientManager's readiness in one place
 // so that multiple calls of isReady resolve the same promise
-const [isReady, setIsReady] = createOnReadyTask();
+const activeClientsReadyTask = createOnReadyTask();
+
+/**
+ * @returns {Promise}
+ */
+function isReady() {
+    return activeClientsReadyTask.isReady();
+}
 
 Onyx.connect({
     key: ONYXKEYS.ACTIVE_CLIENTS,
@@ -30,7 +37,7 @@ Onyx.connect({
  */
 function init() {
     ActiveClients.addClient(clientID)
-        .then(setIsReady);
+        .then(activeClientsReadyTask.setIsReady);
 }
 
 /**

src/libs/Growl.js

@@ -3,7 +3,11 @@ import CONST from '../CONST';
 import createOnReadyTask from './createOnReadyTask';
 
 const growlRef = React.createRef();
-const [isGrowlReady, setIsReady] = createOnReadyTask();
+const growlReadyTask = createOnReadyTask();
+
+function setIsReady() {
+    growlReadyTask.setIsReady();
+}
 
 /**
  * Show the growl notification
@@ -13,7 +17,7 @@ const [isGrowlReady, setIsReady] = createOnReadyTask();
  * @param {Number} [duration]
 */
 function show(bodyText, type, duration = CONST.GROWL.DURATION) {
-    isGrowlReady().then(() => growlRef.current.show(bodyText, type, duration));
+    growlReadyTask.isReady().then(() => growlRef.current.show(bodyText, type, duration));
 }
 
 /**

src/libs/Middleware/Reauthentication.js

@@ -24,19 +24,6 @@ function Reauthentication(response, request, isFromSequentialQueue) {
             }
 
             if (data.jsonCode === CONST.JSON_CODE.NOT_AUTHENTICATED) {
-                const credentials = NetworkStore.getCredentials();
-
-                // Credentials haven't been initialized. We will not be able to re-authenticate with the API.
-                const unableToReauthenticate = (!credentials || !credentials.autoGeneratedLogin || !credentials.autoGeneratedPassword);
-                if (unableToReauthenticate) {
-                    if (isFromSequentialQueue) {
-                        throw new Error('Missing credentials required for authentication');
-                    }
-
-                    MainQueue.replay(request);
-                    return;
-                }
-
                 // There are some API requests that should not be retried when there is an auth failure like
                 // creating and deleting logins. In those cases, they should handle the original response instead
                 // of the new response created by handleExpiredAuthToken.

src/libs/Network/MainQueue.js

@@ -20,12 +20,6 @@ let networkRequestQueue = [];
  * @return {Boolean}
  */
 function canMakeRequest(request) {
-    // We must attempt to read authToken and credentials from storage before allowing any requests to happen so that any requests that
-    // require authToken or trigger reauthentication will succeed.
-    if (!NetworkStore.hasReadRequiredDataFromStorage()) {
-        return false;
-    }
-
     // Some requests are always made even when we are in the process of authenticating (typically because they require no authToken e.g. Log, GetAccountStatus)
     // However, if we are in the process of authenticating we always want to queue requests until we are no longer authenticating.
     return request.data.forceNetworkRequest === true || (!NetworkStore.isAuthenticating() && !SequentialQueue.isRunning());

src/libs/Network/NetworkStore.js

@@ -3,22 +3,16 @@ import Onyx from 'react-native-onyx';
 import _ from 'underscore';
 import ONYXKEYS from '../../ONYXKEYS';
 import createCallback from '../createCallback';
+import createOnReadyTask from '../createOnReadyTask';
 
 let credentials;
 let authToken;
 let currentUserEmail;
-let hasReadRequiredData = false;
 let offline = false;
 let authenticating = false;
 
 const [triggerConnectivityResumed, onConnectivityResumed] = createCallback();
-
-/**
- * @param {Boolean} val
- */
-function setHasReadRequiredDataFromStorage(val) {
-    hasReadRequiredData = val;
-}
+const requiredDataReadyTask = createOnReadyTask();
 
 /**
  * This is a hack to workaround the fact that Onyx may not yet have read these values from storage by the time Network starts processing requests.
@@ -29,7 +23,11 @@ function checkRequiredData() {
         return;
     }
 
-    setHasReadRequiredDataFromStorage(true);
+    requiredDataReadyTask.setIsReady();
+}
+
+function resetHasReadRequiredDataFromStorage() {
+    requiredDataReadyTask.reset();
 }
 
 Onyx.connect({
@@ -103,10 +101,10 @@ function getCurrentUserEmail() {
 }
 
 /**
- * @returns {Boolean}
+ * @returns {Promise}
  */
 function hasReadRequiredDataFromStorage() {
-    return hasReadRequiredData;
+    return requiredDataReadyTask.isReady();
 }
 
 /**
@@ -128,7 +126,7 @@ export {
     setAuthToken,
     getCurrentUserEmail,
     hasReadRequiredDataFromStorage,
-    setHasReadRequiredDataFromStorage,
+    resetHasReadRequiredDataFromStorage,
     isOffline,
     onConnectivityResumed,
     isAuthenticating,

src/libs/Request.js

@@ -1,6 +1,7 @@
 import _ from 'underscore';
 import HttpUtils from './HttpUtils';
 import enhanceParameters from './Network/enhanceParameters';
+import * as NetworkStore from './Network/NetworkStore';
 
 let middlewares = [];
 
@@ -14,7 +15,8 @@ let middlewares = [];
  */
 function makeXHR(request) {
     const finalParameters = enhanceParameters(request.command, request.data);
-    return HttpUtils.xhr(request.command, finalParameters, request.type, request.shouldUseSecure);
+    return NetworkStore.hasReadRequiredDataFromStorage()
+        .then(() => HttpUtils.xhr(request.command, finalParameters, request.type, request.shouldUseSecure));
 }
 
 /**

src/libs/createOnReadyTask.js

@@ -3,16 +3,24 @@
  *
  * @example
  *
- *     const [isSomethingReady, setIsReady] = createOnReadyTask();
- *     isSomethingReady().then(() => doIt());
- *     setIsReady(); // -> doIt() will now execute
- *
- * @returns {Array<Promise, Function>}
+ *     const task = createOnReadyTask();
+ *     task.isReady().then(() => doIt());
+ *     task.setIsReady(); // -> doIt() will now execute
+ *     task.reset() // -> will let us reset the task (useful for testing)
+ * @returns {Object}
  */
 export default function createOnReadyTask() {
     let resolveIsReadyPromise;
-    const isReadyPromise = (new Promise((resolve) => {
-        resolveIsReadyPromise = resolve;
-    }));
-    return [() => isReadyPromise, resolveIsReadyPromise];
+    let isReadyPromise;
+    function reset() {
+        isReadyPromise = (new Promise((resolve) => {
+            resolveIsReadyPromise = resolve;
+        }));
+    }
+    reset();
+    return {
+        isReady: () => isReadyPromise,
+        setIsReady: () => resolveIsReadyPromise(),
+        reset,
+    };
 }

tests/actions/ReimbursementAccountTest.js

@@ -6,7 +6,6 @@ import HttpUtils from '../../src/libs/HttpUtils';
 import waitForPromisesToResolve from '../utils/waitForPromisesToResolve';
 import CONST from '../../src/CONST';
 import BankAccount from '../../src/libs/models/BankAccount';
-import * as NetworkStore from '../../src/libs/Network/NetworkStore';
 
 const TEST_BANK_ACCOUNT_ID = 1;
 const TEST_BANK_ACCOUNT_CITY = 'Opa-locka';
@@ -37,7 +36,6 @@ beforeAll(() => Onyx.init());
 
 beforeEach(() => Onyx.clear()
     .then(() => {
-        NetworkStore.setHasReadRequiredDataFromStorage(true);
         TestHelper.signInWithTestUser();
         return waitForPromisesToResolve();
     }));

tests/unit/NetworkTest.js

@@ -17,6 +17,7 @@ import * as PersistedRequests from '../../src/libs/actions/PersistedRequests';
 import Log from '../../src/libs/Log';
 import * as SequentialQueue from '../../src/libs/Network/SequentialQueue';
 import * as MainQueue from '../../src/libs/Network/MainQueue';
+import * as Request from '../../src/libs/Request';
 
 // Set up manual mocks for methods used in the actions so our test does not fail.
 jest.mock('../../src/libs/Notification/PushNotification', () => ({
@@ -34,14 +35,18 @@ Onyx.init({
 const originalXHR = HttpUtils.xhr;
 
 beforeEach(() => {
+    global.fetch = jest.fn().mockResolvedValue({
+        ok: true,
+        json: () => Promise.resolve({jsonCode: 200}),
+    });
     HttpUtils.xhr = originalXHR;
     PersistedRequests.clear();
     MainQueue.clear();
     return Onyx.clear().then(waitForPromisesToResolve);
 });
 
 afterEach(() => {
-    NetworkStore.setHasReadRequiredDataFromStorage(false);
+    NetworkStore.resetHasReadRequiredDataFromStorage();
     Onyx.addDelayToConnectCallback(0);
     jest.clearAllMocks();
 });
@@ -234,9 +239,9 @@ test('consecutive API calls eventually succeed when authToken is expired', () =>
         });
 });
 
-test('retry network request if auth and credentials are not read from Onyx yet', () => {
-    // In order to test an scenario where the auth token and credentials hasn't been read from storage we set
-    // NetworkStore.setHasReadRequiredDataFromStorage(false) and set the session and credentials to "ready" the Network
+test('Request will not run until credentials are read from Onyx', () => {
+    // In order to test an scenario where the auth token and credentials hasn't been read from storage we reset hasReadRequiredDataFromStorage
+    // and set the session and credentials to "ready" the Network
 
     // Given a test user login and account ID
     const TEST_USER_LOGIN = 'test@testguy.com';
@@ -246,37 +251,24 @@ test('retry network request if auth and credentials are not read from Onyx yet',
     Onyx.addDelayToConnectCallback(ONYX_DELAY_MS);
 
     // Given initial state to Network
-    NetworkStore.setHasReadRequiredDataFromStorage(false);
-
-    // Given an initial value to trigger an update
-    Onyx.merge(ONYXKEYS.CREDENTIALS, {login: 'test-login'});
-    Onyx.merge(ONYXKEYS.SESSION, {authToken: 'test-auth-token'});
+    NetworkStore.resetHasReadRequiredDataFromStorage();
 
     // Given some mock functions to track the isReady
     // flag in Network and the http requests made
     const spyHttpUtilsXhr = jest.spyOn(HttpUtils, 'xhr').mockImplementation(() => Promise.resolve({}));
 
-    // When we make an arbitrary request that can be retried
-    // And we wait for the Onyx.callbacks to be set
+    // When we make a request
     Session.fetchAccountDetails(TEST_USER_LOGIN);
-    return waitForPromisesToResolve().then(() => {
-        // Then we expect not having the Network ready and not making an http request
-        expect(NetworkStore.hasReadRequiredDataFromStorage()).toBe(false);
-        expect(spyHttpUtilsXhr).not.toHaveBeenCalled();
 
-        // When we resolve Onyx.connect callbacks
-        jest.advanceTimersByTime(ONYX_DELAY_MS);
+    // Then we should expect that no requests have been made yet
+    expect(spyHttpUtilsXhr).not.toHaveBeenCalled();
 
-        // Then we should expect call Network.setIsReady(true)
-        // And We should expect not making an http request yet
-        expect(NetworkStore.hasReadRequiredDataFromStorage()).toBe(true);
+    // Once credentials are set and we wait for promises to resolve
+    Onyx.merge(ONYXKEYS.CREDENTIALS, {login: 'test-login'});
+    Onyx.merge(ONYXKEYS.SESSION, {authToken: 'test-auth-token'});
+    return waitForPromisesToResolve().then(() => {
+        // Then we should expect the request to have been made since the network is now ready
         expect(spyHttpUtilsXhr).not.toHaveBeenCalled();
-
-        // When we run MainQueue.process() in the setInterval of Network.js
-        jest.advanceTimersByTime(CONST.NETWORK.PROCESS_REQUEST_DELAY_MS);
-
-        // Then we should expect a retry of the network request
-        expect(spyHttpUtilsXhr).toHaveBeenCalled();
     });
 });
 
@@ -534,7 +526,11 @@ test('several actions made while offline will get added in the order they are cr
     const xhr = jest.spyOn(HttpUtils, 'xhr')
         .mockResolvedValue({jsonCode: CONST.JSON_CODE.SUCCESS});
 
-    return Onyx.set(ONYXKEYS.NETWORK, {isOffline: true})
+    return Onyx.multiSet({
+        [ONYXKEYS.SESSION]: {authToken: 'anyToken'},
+        [ONYXKEYS.NETWORK]: {isOffline: true},
+        [ONYXKEYS.CREDENTIALS]: {autoGeneratedLogin: 'test_user', autoGeneratedPassword: 'psswd'},
+    })
         .then(() => {
             // When we queue 6 persistable commands
             Network.post('MockCommand', {content: 'value1', persist: true});
@@ -562,6 +558,9 @@ test('several actions made while offline will get added in the order they are cr
 
             // Move main queue forward so it processes the "read" request
             jest.advanceTimersByTime(CONST.NETWORK.PROCESS_REQUEST_DELAY_MS);
+            return waitForPromisesToResolve();
+        })
+        .then(() => {
             expect(xhr.mock.calls[6][1].content).toBe('not-persisted');
         });
 });
@@ -677,3 +676,46 @@ test('Sequential queue will succeed if triggered while reauthentication via main
             expect(NetworkStore.isAuthenticating()).toBe(false);
         });
 });
+
+test('Sequential queue will not run until credentials are read', () => {
+    const xhr = jest.spyOn(HttpUtils, 'xhr');
+    const processWithMiddleware = jest.spyOn(Request, 'processWithMiddleware');
+
+    // Given a simulated a condition where the credentials have not yet been read from storage and we are offline
+    return Onyx.multiSet({
+        [ONYXKEYS.NETWORK]: {isOffline: true},
+        [ONYXKEYS.CREDENTIALS]: null,
+        [ONYXKEYS.SESSION]: null,
+    })
+        .then(() => {
+            expect(NetworkStore.isOffline()).toBe(true);
+
+            NetworkStore.resetHasReadRequiredDataFromStorage();
+
+            // And queue a request while offline
+            Network.post('MockCommand', {content: 'value1', persist: true});
+
+            // Then we should expect the request to get persisted
+            expect(PersistedRequests.getAll().length).toBe(1);
+
+            // When we go online and wait for promises to resolve
+            return Onyx.set(ONYXKEYS.NETWORK, {isOffline: false});
+        })
+        .then(() => {
+            expect(processWithMiddleware).toHaveBeenCalled();
+
+            // Then we should not expect XHR to run
+            expect(xhr).not.toHaveBeenCalled();
+
+            // When we set our credentials and authToken
+            return Onyx.multiSet({
+                [ONYXKEYS.CREDENTIALS]: {autoGeneratedLogin: 'test_user', autoGeneratedPassword: 'psswd'},
+                [ONYXKEYS.SESSION]: {authToken: 'oldToken'},
+            });
+        })
+        .then(waitForPromisesToResolve)
+        .then(() => {
+            // Then we should expect XHR to run
+            expect(xhr).toHaveBeenCalled();
+        });
+});