[No QA] Fix dependency cycles

src/libs/API.js

@@ -4,15 +4,13 @@ import Onyx from 'react-native-onyx';
 import CONST from '../CONST';
 import CONFIG from '../CONFIG';
 import ONYXKEYS from '../ONYXKEYS';
-// eslint-disable-next-line import/no-cycle
 import redirectToSignIn from './actions/SignInRedirect';
 import isViaExpensifyCashNative from './isViaExpensifyCashNative';
-// eslint-disable-next-line import/no-cycle
+import requireParameters from './requireParameters';
+import Log from './Log';
 import * as Network from './Network';
-// eslint-disable-next-line import/no-cycle
-import LogUtil from './Log';
-// eslint-disable-next-line import/no-cycle
-import * as Session from './actions/Session';
+import updateSessionAuthTokens from './actions/Session/updateSessionAuthTokens';
+import setSessionLoadingAndError from './actions/Session/setSessionLoadingAndError';
 
 let isAuthenticating;
 let credentials;
@@ -89,35 +87,6 @@ function addDefaultValuesToParameters(command, parameters) {
 // Tie into the network layer to add auth token to the parameters of all requests
 Network.registerParameterEnhancer(addDefaultValuesToParameters);
 
-/**
- * @throws {Error} If the "parameters" object has a null or undefined value for any of the given parameterNames
- *
- * @param {String[]} parameterNames Array of the required parameter names
- * @param {Object} parameters A map from available parameter names to their values
- * @param {String} commandName The name of the API command
- */
-function requireParameters(parameterNames, parameters, commandName) {
-    parameterNames.forEach((parameterName) => {
-        if (_(parameters).has(parameterName)
-            && parameters[parameterName] !== null
-            && parameters[parameterName] !== undefined
-        ) {
-            return;
-        }
-
-        const propertiesToRedact = ['authToken', 'password', 'partnerUserSecret', 'twoFactorAuthCode'];
-        const parametersCopy = _.chain(parameters)
-            .clone()
-            .mapObject((val, key) => (_.contains(propertiesToRedact, key) ? '<redacted>' : val))
-            .value();
-        const keys = _(parametersCopy).keys().join(', ') || 'none';
-
-        let error = `Parameter ${parameterName} is required for "${commandName}". `;
-        error += `Supplied parameters: ${keys}`;
-        throw new Error(error);
-    });
-}
-
 /**
  * Function used to handle expired auth tokens. It re-authenticates with the API and
  * then replays the original request
@@ -153,7 +122,34 @@ function handleExpiredAuthToken(originalCommand, originalParameters, originalTyp
         ));
 }
 
+Network.registerRequestHandler((queuedRequest, finalParameters) => {
+    if (queuedRequest.command === 'Log') {
+        return;
+    }
+
+    Log.info('Making API request', false, {
+        command: queuedRequest.command,
+        type: queuedRequest.type,
+        shouldUseSecure: queuedRequest.type,
+        rvl: finalParameters.returnValueList,
+    });
+});
+
+Network.registerRequestSkippedHandler((parameters) => {
+    Log.hmmm('Trying to make a request when Network is not ready', parameters);
+});
+
 Network.registerResponseHandler((queuedRequest, response) => {
+    if (queuedRequest.command !== 'Log') {
+        Log.info('Finished API request', false, {
+            command: queuedRequest.command,
+            type: queuedRequest.type,
+            shouldUseSecure: queuedRequest.shouldUseSecure,
+            jsonCode: response.jsonCode,
+            requestID: response.requestID,
+        });
+    }
+
     if (response.jsonCode === 407) {
         // Credentials haven't been initialized. We will not be able to re-authenticates with the API
         const unableToReauthenticate = (!credentials || !credentials.autoGeneratedLogin
@@ -186,13 +182,13 @@ Network.registerResponseHandler((queuedRequest, response) => {
 
 Network.registerErrorHandler((queuedRequest, error) => {
     if (queuedRequest.command !== 'Log') {
-        LogUtil.hmmm('[API] Handled error when making request', error);
+        Log.hmmm('[API] Handled error when making request', error);
     } else {
         console.debug('[API] There was an error in the Log API command, unable to log to server!', error);
     }
 
     // Set an error state and signify we are done loading
-    Session.setSessionLoadingAndError(false, 'Cannot connect to server');
+    setSessionLoadingAndError(false, 'Cannot connect to server');
 
     // Reject the queued request with an API offline error so that the original caller can handle it.
     queuedRequest.reject(new Error(CONST.ERROR.API_OFFLINE));
@@ -293,7 +289,7 @@ function reauthenticate(command = '') {
 
             // Update authToken in Onyx and in our local variables so that API requests will use the
             // new authToken
-            Session.updateSessionAuthTokens(response.authToken, response.encryptedAuthToken);
+            updateSessionAuthTokens(response.authToken, response.encryptedAuthToken);
             authToken = response.authToken;
 
             // The authentication process is finished so the network can be unpaused to continue
@@ -317,7 +313,7 @@ function reauthenticate(command = '') {
             // If we experience something other than a network error then redirect the user to sign in
             redirectToSignIn(error.message);
 
-            LogUtil.hmmm('Redirecting to Sign In because we failed to reauthenticate', {
+            Log.hmmm('Redirecting to Sign In because we failed to reauthenticate', {
                 command,
                 error: error.message,
             });
@@ -516,21 +512,6 @@ function GetRequestCountryCode() {
     return Network.post(commandName);
 }
 
-/**
- * @param {Object} parameters
- * @param {String} parameters.expensifyCashAppVersion
- * @param {Object[]} parameters.logPacket
- * @returns {Promise}
- */
-function Log(parameters) {
-    const commandName = 'Log';
-    requireParameters(['logPacket', 'expensifyCashAppVersion'],
-        parameters, commandName);
-
-    // Note: We are forcing Log to run since it requires no authToken and should only be queued when we are offline.
-    return Network.post(commandName, {...parameters, forceNetworkRequest: true});
-}
-
 /**
  * @param {Object} parameters
  * @param {String} parameters.name
@@ -1141,7 +1122,6 @@ export {
     GetRequestCountryCode,
     Graphite_Timer,
     Inbox_CallUser,
-    Log,
     PayIOU,
     PayWithWallet,
     PersonalDetails_GetForEmails,

src/libs/HttpUtils.js

@@ -4,9 +4,6 @@ import CONFIG from '../CONFIG';
 import CONST from '../CONST';
 import ONYXKEYS from '../ONYXKEYS';
 
-// To avoid a circular dependency, we can't include Log here, so instead, we define an empty logging method and expose the setLogger method to set the logger from outside this file
-let info = () => {};
-
 let shouldUseSecureStaging = false;
 Onyx.connect({
     key: ONYXKEYS.USER,
@@ -39,14 +36,6 @@ function processHTTPRequest(url, method = 'get', body = null) {
  * @returns {Promise}
  */
 function xhr(command, data, type = CONST.NETWORK.METHOD.POST, shouldUseSecure = false) {
-    if (command !== 'Log') {
-        info('Making API request', false, {
-            command,
-            type,
-            shouldUseSecure,
-            rvl: data.returnValueList,
-        });
-    }
     const formData = new FormData();
     _.each(data, (val, key) => formData.append(key, val));
     let apiRoot = shouldUseSecure ? CONFIG.EXPENSIFY.URL_EXPENSIFY_SECURE : CONFIG.EXPENSIFY.URL_API_ROOT;
@@ -55,19 +44,7 @@ function xhr(command, data, type = CONST.NETWORK.METHOD.POST, shouldUseSecure =
         apiRoot = CONST.STAGING_SECURE_URL;
     }
 
-    return processHTTPRequest(`${apiRoot}api?command=${command}`, type, formData)
-        .then((response) => {
-            if (command !== 'Log') {
-                info('Finished API request', false, {
-                    command,
-                    type,
-                    shouldUseSecure,
-                    jsonCode: response.jsonCode,
-                    requestID: response.requestID,
-                });
-            }
-            return response;
-        });
+    return processHTTPRequest(`${apiRoot}api?command=${command}`, type, formData);
 }
 
 /**
@@ -87,12 +64,7 @@ function download(relativePath) {
     return processHTTPRequest(`${siteRoot}${strippedRelativePath}`);
 }
 
-function setLogger(logger) {
-    info = logger.info;
-}
-
 export default {
-    setLogger,
     download,
     xhr,
 };

src/libs/Log.js

@@ -5,14 +5,26 @@ import Logger from 'expensify-common/lib/Logger';
 import CONFIG from '../CONFIG';
 import getPlatform from './getPlatform';
 import {version} from '../../package.json';
-import NetworkConnection from './NetworkConnection';
-import HttpUtils from './HttpUtils';
-
-// eslint-disable-next-line import/no-cycle
-import * as API from './API';
+import requireParameters from './requireParameters';
+import * as Network from './Network';
 
 let timeout = null;
 
+/**
+ * @param {Object} parameters
+ * @param {String} parameters.expensifyCashAppVersion
+ * @param {Object[]} parameters.logPacket
+ * @returns {Promise}
+ */
+function LogCommand(parameters) {
+    const commandName = 'Log';
+    requireParameters(['logPacket', 'expensifyCashAppVersion'],
+        parameters, commandName);
+
+    // Note: We are forcing Log to run since it requires no authToken and should only be queued when we are offline.
+    return Network.post(commandName, {...parameters, forceNetworkRequest: true});
+}
+
 /**
  * Network interface for logger.
  *
@@ -31,13 +43,11 @@ function serverLoggingCallback(logger, params) {
     }
     clearTimeout(timeout);
     timeout = setTimeout(() => logger.info('Flushing logs older than 10 minutes', true, {}, true), 10 * 60 * 1000);
-    return API.Log(requestParams);
+    return LogCommand(requestParams);
 }
 
-// Note: We are importing Logger from expensify-common because it is
-// used by other platforms. The server and client logging
-// callback methods are passed in here so we can decouple
-// the logging library from the logging methods.
+// Note: We are importing Logger from expensify-common because it is used by other platforms. The server and client logging
+// callback methods are passed in here so we can decouple the logging library from the logging methods.
 const Log = new Logger({
     serverLoggingCallback,
     clientLoggingCallback: (message) => {
@@ -47,7 +57,4 @@ const Log = new Logger({
 });
 timeout = setTimeout(() => Log.info('Flushing logs older than 10 minutes', true, {}, true), 10 * 60 * 1000);
 
-NetworkConnection.registerLogInfoCallback(Log.info);
-HttpUtils.setLogger(Log);
-
 export default Log;

src/libs/Navigation/Navigation.js

@@ -9,7 +9,6 @@ import {
 } from '@react-navigation/native';
 import PropTypes from 'prop-types';
 import Onyx from 'react-native-onyx';
-// eslint-disable-next-line import/no-cycle
 import Log from '../Log';
 import linkTo from './linkTo';
 import ROUTES from '../../ROUTES';

src/libs/Network.js

@@ -5,8 +5,7 @@ import HttpUtils from './HttpUtils';
 import ONYXKEYS from '../ONYXKEYS';
 import * as ActiveClientManager from './ActiveClientManager';
 import CONST from '../CONST';
-// eslint-disable-next-line import/no-cycle
-import LogUtil from './Log';
+import createCallback from './createCallback';
 import * as NetworkRequestQueue from './actions/NetworkRequestQueue';
 
 let isReady = false;
@@ -20,10 +19,12 @@ let networkRequestQueue = [];
 // parameters such as authTokens or CSRF tokens, etc.
 let enhanceParameters;
 
-// These handlers must be registered in order to process the response or network errors returned from the queue.
-// The first argument passed will be the queuedRequest object and the second will be either the response or error.
-let onResponse = () => {};
-let onError = () => {};
+// These handlers must be registered so we can process the request, response, and errors returned from the queue.
+// The first argument passed will be the queuedRequest object and the second will be either the parameters, response, or error.
+const [onRequest, registerRequestHandler] = createCallback();
+const [onResponse, registerResponseHandler] = createCallback();
+const [onError, registerErrorHandler] = createCallback();
+const [onRequestSkipped, registerRequestSkippedHandler] = createCallback();
 
 let didLoadPersistedRequests;
 let isOffline;
@@ -120,7 +121,7 @@ function setIsReady(val) {
  */
 function canMakeRequest(request) {
     if (!isReady) {
-        LogUtil.hmmm('Trying to make a request when Network is not ready', {command: request.command, type: request.type});
+        onRequestSkipped({command: request.command, type: request.type});
         return false;
     }
 
@@ -221,6 +222,7 @@ function processNetworkRequestQueue() {
             ? enhanceParameters(queuedRequest.command, requestData)
             : requestData;
 
+        onRequest(queuedRequest, finalParameters);
         HttpUtils.xhr(queuedRequest.command, finalParameters, queuedRequest.type, queuedRequest.shouldUseSecure)
             .then(response => onResponse(queuedRequest, response))
             .catch(error => onError(queuedRequest, error));
@@ -323,23 +325,6 @@ function clearRequestQueue() {
     networkRequestQueue = [];
 }
 
-/**
- * Register a method to call when the authToken expires
- * @param {Function} callback
- */
-function registerResponseHandler(callback) {
-    onResponse = callback;
-}
-
-/**
- * The error handler will handle fetch() errors. Not used for successful responses that might send expected error codes
- * e.g. jsonCode: 407.
- * @param {Function} callback
- */
-function registerErrorHandler(callback) {
-    onError = callback;
-}
-
 export {
     post,
     pauseRequestQueue,
@@ -349,5 +334,7 @@ export {
     clearRequestQueue,
     registerResponseHandler,
     registerErrorHandler,
+    registerRequestHandler,
     setIsReady,
+    registerRequestSkippedHandler,
 };