From c159315245399411333eacea4955b0e510cf83f7 Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Fri, 18 Aug 2017 15:32:08 +0200 Subject: [PATCH 1/3] Split between topology API polling & one time calls. --- client/app/scripts/utils/web-api-utils.js | 39 +++++++++++++++++++---- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/client/app/scripts/utils/web-api-utils.js b/client/app/scripts/utils/web-api-utils.js index f6086f643..b406b6458 100644 --- a/client/app/scripts/utils/web-api-utils.js +++ b/client/app/scripts/utils/web-api-utils.js @@ -103,6 +103,12 @@ export function getApiPath(pathname = window.location.pathname) { return basePath(pathname); } +function topologiesUrl(state) { + const activeTopologyOptions = activeTopologyOptionsSelector(state); + const optionsQuery = buildUrlQuery(activeTopologyOptions, state); + return `${getApiPath()}/api/topology?${optionsQuery}`; +} + export function getWebsocketUrl(host = window.location.host, pathname = window.location.pathname) { const wsProto = location.protocol === 'https:' ? 'wss' : 'ws'; return `${wsProto}://${host}${process.env.SCOPE_API_PREFIX || ''}${basePath(pathname)}`; @@ -235,20 +241,19 @@ export function getResourceViewNodesSnapshot(state, dispatch) { getNodesForTopologies(state, dispatch, topologyIds); } -// NOTE: getState is called every time to make sure the up-to-date state is used. -export function getTopologies(getState, dispatch, initialPoll = false) { +function pollTopologies(getState, dispatch, initialPoll = false) { // Used to resume polling when navigating between pages in Weave Cloud. continuePolling = initialPoll === true ? true : continuePolling; clearTimeout(topologyTimer); - const optionsQuery = buildUrlQuery(activeTopologyOptionsSelector(getState()), getState()); - const url = `${getApiPath()}/api/topology?${optionsQuery}`; + // NOTE: getState is called every time to make sure the up-to-date state is used. + const url = topologiesUrl(getState()); doRequest({ url, success: (res) => { if (continuePolling && !isPausedSelector(getState())) { dispatch(receiveTopologies(res)); topologyTimer = setTimeout(() => { - getTopologies(getState, dispatch); + pollTopologies(getState, dispatch); }, TOPOLOGY_REFRESH_INTERVAL); } }, @@ -258,13 +263,27 @@ export function getTopologies(getState, dispatch, initialPoll = false) { // Only retry in stand-alone mode if (continuePolling && !isPausedSelector(getState())) { topologyTimer = setTimeout(() => { - getTopologies(getState, dispatch); + pollTopologies(getState, dispatch); }, TOPOLOGY_REFRESH_INTERVAL); } } }); } +function getTopologiesOnce(getState, dispatch) { + const url = topologiesUrl(getState()); + doRequest({ + url, + success: (res) => { + dispatch(receiveTopologies(res)); + }, + error: (req) => { + log(`Error in topology request: ${req.responseText}`); + dispatch(receiveError(url)); + } + }); +} + function updateWebsocketChannel(getState, dispatch, forceRequest) { const topologyUrl = getCurrentTopologyUrl(getState()); const topologyOptions = activeTopologyOptionsSelector(getState()); @@ -325,6 +344,14 @@ export function getNodeDetails(getState, dispatch) { } } +export function getTopologies(getState, dispatch, forceRequest) { + if (isPausedSelector(getState())) { + getTopologiesOnce(getState, dispatch); + } else { + pollTopologies(getState, dispatch, forceRequest); + } +} + export function getNodes(getState, dispatch, forceRequest = false) { if (isPausedSelector(getState())) { getNodesOnce(getState, dispatch); From 16e520da5e0e26c53edce01f35b119db84d7d6a5 Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Mon, 21 Aug 2017 15:06:39 +0200 Subject: [PATCH 2/3] Proper handling of namespace selection when time travelling. --- .../scripts/components/topology-options.js | 25 ++++++++++++++++--- client/app/scripts/reducers/root.js | 2 +- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/client/app/scripts/components/topology-options.js b/client/app/scripts/components/topology-options.js index a184fa88a..0fa3cac51 100644 --- a/client/app/scripts/components/topology-options.js +++ b/client/app/scripts/components/topology-options.js @@ -1,6 +1,6 @@ import React from 'react'; import { connect } from 'react-redux'; -import { Map as makeMap } from 'immutable'; +import { Set as makeSet, Map as makeMap } from 'immutable'; import includes from 'lodash/includes'; import { trackMixpanelEvent } from '../utils/tracking-utils'; @@ -73,9 +73,26 @@ class TopologyOptions extends React.Component { renderOption(option) { const { activeOptions, currentTopologyId } = this.props; const optionId = option.get('id'); - const activeValue = activeOptions && activeOptions.has(optionId) - ? activeOptions.get(optionId) - : option.get('defaultValue'); + + // Make the active value be the intersection of the available options + // and the active selection and use the default value if there is no + // overlap. It seems intuitive that active selection would always be a + // subset of available option, but the exception can happen when going + // back in time (making available options change, while not touching + // the selection). + // TODO: This logic should probably be made consistent with how topology + // selection is handled when time travelling, especially when the name- + // spaces are brought under category selection. + let activeValue = option.get('defaultValue'); + if (activeOptions && activeOptions.has(optionId)) { + const activeSelection = makeSet(activeOptions.get(optionId)); + const availableOptions = makeSet(option.get('options').map(o => o.get('value'))); + const intersection = activeSelection.intersect(availableOptions); + if (!intersection.isEmpty()) { + activeValue = intersection.toJS(); + } + } + const noneItem = makeMap({ value: '', label: option.get('noneLabel') diff --git a/client/app/scripts/reducers/root.js b/client/app/scripts/reducers/root.js index fccdae382..bf38e412a 100644 --- a/client/app/scripts/reducers/root.js +++ b/client/app/scripts/reducers/root.js @@ -223,7 +223,7 @@ export function rootReducer(state = initialState, action) { if (topology) { const topologyId = topology.get('parentId') || topology.get('id'); const optionKey = ['topologyOptions', topologyId, action.option]; - const currentOption = state.getIn(['topologyOptions', topologyId, action.option]); + const currentOption = state.getIn(optionKey); if (!isEqual(currentOption, action.value)) { state = clearNodes(state); From f8343c189dab6cb9ac2e7184a4b19d689f90804b Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Thu, 24 Aug 2017 18:14:43 +0200 Subject: [PATCH 3/3] Added one more comment. --- client/app/scripts/components/topology-options.js | 1 + 1 file changed, 1 insertion(+) diff --git a/client/app/scripts/components/topology-options.js b/client/app/scripts/components/topology-options.js index 0fa3cac51..9a8cc7b4f 100644 --- a/client/app/scripts/components/topology-options.js +++ b/client/app/scripts/components/topology-options.js @@ -83,6 +83,7 @@ class TopologyOptions extends React.Component { // TODO: This logic should probably be made consistent with how topology // selection is handled when time travelling, especially when the name- // spaces are brought under category selection. + // TODO: Consider extracting this into a global selector. let activeValue = option.get('defaultValue'); if (activeOptions && activeOptions.has(optionId)) { const activeSelection = makeSet(activeOptions.get(optionId));