From 592305df74fa203ba4cb5cba2869b49636e44404 Mon Sep 17 00:00:00 2001 From: Lyubo Marinov Date: Wed, 23 May 2018 16:10:04 -0500 Subject: [PATCH] [RN] Fix processing outdated loadConfig requests (continued) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Lyubo Marinov Co-authored-by: Paweł Domas --- react/features/app/actions.js | 52 ++++++++++--------- react/features/base/config/actionTypes.js | 12 ++--- react/features/base/config/actions.js | 28 +++++----- react/features/base/config/reducer.js | 41 ++++++++++----- .../mobile/external-api/middleware.js | 18 ++++--- 5 files changed, 87 insertions(+), 64 deletions(-) diff --git a/react/features/app/actions.js b/react/features/app/actions.js index cccbfa56d..3455b1224 100644 --- a/react/features/app/actions.js +++ b/react/features/app/actions.js @@ -1,4 +1,4 @@ -/* @flow */ +// @flow import type { Dispatch } from 'redux'; @@ -51,8 +51,9 @@ function _appNavigateToMandatoryLocation( newLocation: Object ): Promise { const { room } = newLocation; + const locationURL = new URL(newLocation.toString()); - dispatch(configWillLoad(newLocation)); + dispatch(configWillLoad(locationURL)); return ( _loadConfig(dispatch, getState, newLocation) @@ -73,33 +74,32 @@ function _appNavigateToMandatoryLocation( * @returns {void} */ function loadConfigSettled(error, config) { - // Due to the asynchronous nature of the loading, the specified - // config may or may not be required by the time the notification - // arrives. If we receive the config for a location we are no longer - // interested in, just dump it. + let promise; - const { locationURL: currentLocationURL } - = getState()['features/base/config']; - - if (currentLocationURL !== newLocation) { - throw new Error('Config no longer needed'); + // Due to the asynchronous nature of the loading, the specified config + // may or may not be required by the time the notification arrives. + // If we receive the config for a location we are no longer interested + // in, "ignore" it - deliver it to the external API, for example, but do + // not proceed with the appNavigate procedure/process. + if (getState()['features/base/config'].locationURL === locationURL) { + promise = dispatch(setLocationURL(locationURL)); + } else { + // eslint-disable-next-line no-param-reassign + error || (error = new Error('Config no longer needed!')); + promise = Promise.resolve(); } - const promise - = dispatch(setLocationURL(new URL(newLocation.toString()))); - - if (error) { - // XXX The failure could be, for example, because of a - // certificate-related error. In which case the connection will - // fail later in Strophe anyway. - return promise.then(() => { - dispatch(loadConfigError(error, newLocation)); + return promise.then(() => { + if (error) { + // XXX The failure could be, for example, because of a + // certificate-related error. In which case the connection will + // fail later in Strophe anyway. + dispatch(loadConfigError(error, locationURL)); throw error; - }); - } + } - return promise.then(() => - dispatch(setConfig(config))); + return dispatch(setConfig(config)); + }); } } @@ -227,6 +227,10 @@ function _loadConfig( return loadConfig(url).then( /* onFulfilled */ config => { + // FIXME If the config is no longer needed (in the terms of + // _loadConfig) and that happened because of an intervening + // _loadConfig for the same baseURL, then the unneeded config may be + // stored after the needed config. Anyway. dispatch(storeConfig(baseURL, config)); return config; diff --git a/react/features/base/config/actionTypes.js b/react/features/base/config/actionTypes.js index 74846d614..0f15f9e8e 100644 --- a/react/features/base/config/actionTypes.js +++ b/react/features/base/config/actionTypes.js @@ -1,22 +1,22 @@ /** - * The redux action which signals that a configuration will be loaded for a - * specific locationURL. + * The redux action which signals that a configuration (commonly known in Jitsi + * Meet as config.js) will be loaded for a specific locationURL. * * { * type: CONFIG_WILL_LOAD, - * locationURL: string | URL + * locationURL: URL * } */ export const CONFIG_WILL_LOAD = Symbol('CONFIG_WILL_LOAD'); /** - * The redux action which signals that a configuration could not be loaded due - * to a specific error. + * The redux action which signals that a configuration (commonly known in Jitsi + * Meet as config.js) could not be loaded due to a specific error. * * { * type: LOAD_CONFIG_ERROR, * error: Error, - * locationURL: string | URL + * locationURL: URL * } */ export const LOAD_CONFIG_ERROR = Symbol('LOAD_CONFIG_ERROR'); diff --git a/react/features/base/config/actions.js b/react/features/base/config/actions.js index 2b7c1fe0a..0d4dc01f1 100644 --- a/react/features/base/config/actions.js +++ b/react/features/base/config/actions.js @@ -5,25 +5,22 @@ import type { Dispatch } from 'redux'; import { addKnownDomains } from '../known-domains'; import { parseURIString } from '../util'; -import { - CONFIG_WILL_LOAD, - LOAD_CONFIG_ERROR, - SET_CONFIG -} from './actionTypes'; +import { CONFIG_WILL_LOAD, LOAD_CONFIG_ERROR, SET_CONFIG } from './actionTypes'; import { _CONFIG_STORE_PREFIX } from './constants'; import { setConfigFromURLParams } from './functions'; /** - * Signals that the configuration for a specific locationURL will be loaded now. + * Signals that the configuration (commonly known in Jitsi Meet as config.js) + * for a specific locationURL will be loaded now. * - * @param {string|URL} locationURL - The URL of the location which necessitated - * the loading of a configuration. + * @param {URL} locationURL - The URL of the location which necessitated the + * loading of a configuration. * @returns {{ * type: CONFIG_WILL_LOAD, - * locationURL + * locationURL: URL * }} */ -export function configWillLoad(locationURL: string | URL) { +export function configWillLoad(locationURL: URL) { return { type: CONFIG_WILL_LOAD, locationURL @@ -31,19 +28,20 @@ export function configWillLoad(locationURL: string | URL) { } /** - * Signals that a configuration could not be loaded due to a specific error. + * Signals that a configuration (commonly known in Jitsi Meet as config.js) + * could not be loaded due to a specific error. * * @param {Error} error - The {@code Error} which prevented the successful * loading of a configuration. - * @param {string|URL} locationURL - The URL of the location which necessitated - * the loading of a configuration. + * @param {URL} locationURL - The URL of the location which necessitated the + * loading of a configuration. * @returns {{ * type: LOAD_CONFIG_ERROR, * error: Error, - * locationURL + * locationURL: URL * }} */ -export function loadConfigError(error: Error, locationURL: string | URL) { +export function loadConfigError(error: Error, locationURL: URL) { return { type: LOAD_CONFIG_ERROR, error, diff --git a/react/features/base/config/reducer.js b/react/features/base/config/reducer.js index 5c7b203af..a586927d9 100644 --- a/react/features/base/config/reducer.js +++ b/react/features/base/config/reducer.js @@ -1,14 +1,10 @@ -/* @flow */ +// @flow import _ from 'lodash'; import { equals, ReducerRegistry, set } from '../redux'; -import { - CONFIG_WILL_LOAD, - LOAD_CONFIG_ERROR, - SET_CONFIG -} from './actionTypes'; +import { CONFIG_WILL_LOAD, LOAD_CONFIG_ERROR, SET_CONFIG } from './actionTypes'; /** * The initial state of the feature base/config when executing in a @@ -54,20 +50,41 @@ ReducerRegistry.register( case CONFIG_WILL_LOAD: return { error: undefined, + + /** + * The URL of the location associated with/configured by this + * configuration. + * + * @type URL + */ locationURL: action.locationURL }; case LOAD_CONFIG_ERROR: - return { - error: action.error - }; + // XXX LOAD_CONFIG_ERROR is one of the settlement execution paths of + // the asynchronous "loadConfig procedure/process" started with + // CONFIG_WILL_LOAD. Due to the asynchronous nature of it, whoever + // is settling the process needs to provide proof that they have + // started it and that the iteration of the process being completed + // now is still of interest to the app. + if (state.locationURL === action.locationURL) { + return { + /** + * The {@link Error} which prevented the loading of the + * configuration of the associated {@code locationURL}. + * + * @type Error + */ + error: action.error + }; + } + break; case SET_CONFIG: return _setConfig(state, action); - - default: - return state; } + + return state; }); /** diff --git a/react/features/mobile/external-api/middleware.js b/react/features/mobile/external-api/middleware.js index de8b5d299..89e807635 100644 --- a/react/features/mobile/external-api/middleware.js +++ b/react/features/mobile/external-api/middleware.js @@ -28,8 +28,9 @@ import { ENTER_PICTURE_IN_PICTURE } from '../picture-in-picture'; */ MiddlewareRegistry.register(store => next => action => { const result = next(action); + const { type } = action; - switch (action.type) { + switch (type) { case CONFERENCE_FAILED: { const { error, ...data } = action; @@ -64,16 +65,19 @@ MiddlewareRegistry.register(store => next => action => { break; case ENTER_PICTURE_IN_PICTURE: - _sendEvent(store, _getSymbolDescription(action.type), /* data */ {}); + _sendEvent(store, _getSymbolDescription(type), /* data */ {}); break; case LOAD_CONFIG_ERROR: { - const { error, locationURL, type } = action; + const { error, locationURL } = action; - _sendEvent(store, _getSymbolDescription(type), /* data */ { - error: _toErrorString(error), - url: toURLString(locationURL) - }); + _sendEvent( + store, + _getSymbolDescription(type), + /* data */ { + error: _toErrorString(error), + url: toURLString(locationURL) + }); break; }