Cleanup Network.js code. Fix persisted request handling. #8306
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| import lodashGet from 'lodash/get'; | ||
| import Onyx from 'react-native-onyx'; | ||
| import _ from 'underscore'; | ||
| import ONYXKEYS from '../../ONYXKEYS'; | ||
|
|
||
| let credentials; | ||
| let authToken; | ||
| let currentUserEmail; | ||
| let networkReady = false; | ||
|
|
||
| /** | ||
| * @param {Boolean} ready | ||
| */ | ||
| function setIsReady(ready) { | ||
| networkReady = ready; | ||
| } | ||
|
|
||
| /** | ||
| * 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. | ||
| * If the values are undefined we haven't read them yet. If they are null or have a value then we have and the network is "ready". | ||
| */ | ||
| function checkRequiredDataAndSetNetworkReady() { | ||
| if (_.isUndefined(authToken) || _.isUndefined(credentials)) { | ||
marcaaron marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return; | ||
roryabraham marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| setIsReady(true); | ||
| } | ||
|
|
||
| Onyx.connect({ | ||
| key: ONYXKEYS.SESSION, | ||
| callback: (val) => { | ||
| authToken = lodashGet(val, 'authToken', null); | ||
| currentUserEmail = lodashGet(val, 'email', null); | ||
| checkRequiredDataAndSetNetworkReady(); | ||
| }, | ||
| }); | ||
|
|
||
| Onyx.connect({ | ||
| key: ONYXKEYS.CREDENTIALS, | ||
| callback: (val) => { | ||
| credentials = val || null; | ||
| checkRequiredDataAndSetNetworkReady(); | ||
| }, | ||
| }); | ||
|
|
||
| /** | ||
| * @returns {String} | ||
| */ | ||
| function getAuthToken() { | ||
| return authToken; | ||
| } | ||
|
|
||
| /** | ||
| * @param {String} newAuthToken | ||
| */ | ||
| function setAuthToken(newAuthToken) { | ||
|
There was a problem hiding this comment. Why expose this function to set the There was a problem hiding this comment. Good eye. It's not quite a hack, but close. Setting values in Onyx always takes at least one tick of the event loop and updates subscribers late. If you look at where this is used it might make more sense. Lines 236 to 237 in 9c6c2a9 Inside I'll leave a comment here. Basically, Onyx being Onyx though. There was a problem hiding this comment. Thanks, makes sense.
Is there a way we could fix this? Maybe we could create an issue in the Onyx repo? There was a problem hiding this comment. Yeah, I think we even tried once, but it broke things in unexpected ways and there is a high likelihood that it may break stuff again. Anyone using There was a problem hiding this comment. Hmmm seems to me like things might be a bit easier to reason about if one could safely assume than any value written in Onyx will be updated immediately, but I'll take your word for it. There was a problem hiding this comment. They would 1000% be easier. And we could still do it. But whoever does it will need to check everything carefully and probably remove a lot of There was a problem hiding this comment. Created an issue: Expensify/react-native-onyx#123 It's not very detail-rich, but will hopefully explore more when I have some spare cycles. |
||
| authToken = newAuthToken; | ||
| } | ||
|
|
||
| /** | ||
| * @returns {Object} | ||
| */ | ||
| function getCredentials() { | ||
| return credentials; | ||
| } | ||
|
|
||
| /** | ||
| * @returns {String} | ||
| */ | ||
| function getCurrentUserEmail() { | ||
| return currentUserEmail; | ||
| } | ||
|
|
||
| /** | ||
| * @returns {Boolean} | ||
| */ | ||
| function isReady() { | ||
| return networkReady; | ||
| } | ||
|
|
||
| export { | ||
| getAuthToken, | ||
| setAuthToken, | ||
| getCredentials, | ||
| getCurrentUserEmail, | ||
| isReady, | ||
| setIsReady, | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import lodashGet from 'lodash/get'; | ||
| import _ from 'underscore'; | ||
| import CONFIG from '../../CONFIG'; | ||
| import * as NetworkStore from './NetworkStore'; | ||
|
|
||
| /** | ||
| * Does this command require an authToken? | ||
| * | ||
| * @param {String} command | ||
| * @return {Boolean} | ||
| */ | ||
| function isAuthTokenRequired(command) { | ||
| return !_.contains([ | ||
| 'Log', | ||
|
There was a problem hiding this comment. NAB alphabetize this list |
||
| 'Graphite_Timer', | ||
| 'Authenticate', | ||
| 'GetAccountStatus', | ||
| 'SetPassword', | ||
| 'User_SignUp', | ||
| 'ResendValidateCode', | ||
| 'ResetPassword', | ||
| 'User_ReopenAccount', | ||
| 'ValidateEmail', | ||
| ], command); | ||
| } | ||
|
|
||
| /** | ||
| * Adds default values to our request data | ||
| * | ||
| * @param {String} command | ||
| * @param {Object} parameters | ||
| * @returns {Object} | ||
| */ | ||
| export default function enhanceParameters(command, parameters) { | ||
| const finalParameters = {...parameters}; | ||
|
|
||
| if (isAuthTokenRequired(command) && !parameters.authToken) { | ||
| finalParameters.authToken = NetworkStore.getAuthToken(); | ||
| } | ||
|
|
||
| finalParameters.referer = CONFIG.EXPENSIFY.EXPENSIFY_CASH_REFERER; | ||
roryabraham marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // This application does not save its authToken in cookies like the classic Expensify app. | ||
| // Setting api_setCookie to false will ensure that the Expensify API doesn't set any cookies | ||
| // and prevents interfering with the cookie authToken that Expensify classic uses. | ||
| finalParameters.api_setCookie = false; | ||
|
|
||
| // Include current user's email in every request and the server logs | ||
| finalParameters.email = lodashGet(parameters, 'email', NetworkStore.getCurrentUserEmail()); | ||
|
|
||
| return finalParameters; | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.