From b9ba83ffcab03416dc22b15f33b826bc11bbc44f Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Fri, 10 Feb 2017 17:37:37 +0100 Subject: [PATCH] Moved the node networks computation to selectors. --- client/app/scripts/actions/app-actions.js | 4 ++ .../app/scripts/charts/nodes-chart-edges.js | 10 +-- .../app/scripts/charts/nodes-chart-nodes.js | 62 +++++++++++-------- client/app/scripts/charts/nodes-chart.js | 4 +- client/app/scripts/charts/nodes-grid.js | 2 +- client/app/scripts/components/app.js | 3 +- .../scripts/components/networks-selector.js | 3 +- client/app/scripts/reducers/root.js | 21 +------ client/app/scripts/selectors/node-filters.js | 9 +++ .../selectors/{metrics.js => node-metric.js} | 2 +- client/app/scripts/selectors/node-networks.js | 45 ++++++++++++++ client/app/scripts/selectors/nodes.js | 28 --------- client/app/scripts/selectors/search.js | 2 +- .../app/scripts/utils/network-view-utils.js | 31 ++++------ 14 files changed, 122 insertions(+), 104 deletions(-) create mode 100644 client/app/scripts/selectors/node-filters.js rename client/app/scripts/selectors/{metrics.js => node-metric.js} (93%) create mode 100644 client/app/scripts/selectors/node-networks.js delete mode 100644 client/app/scripts/selectors/nodes.js diff --git a/client/app/scripts/actions/app-actions.js b/client/app/scripts/actions/app-actions.js index 300c4bbfe..78a972f13 100644 --- a/client/app/scripts/actions/app-actions.js +++ b/client/app/scripts/actions/app-actions.js @@ -404,6 +404,10 @@ export function focusSearch() { dispatch({ type: ActionTypes.FOCUS_SEARCH }); // update nodes cache to allow search across all topologies, // wait a second until animation is over + // NOTE: This will cause matching recalculation (and rerendering) + // of all the nodes in the topology, instead applying it only on + // the nodes delta. The solution would be to implement deeper + // search selectors with per-node caching instead of per-topology. setTimeout(() => { getAllNodes(getState, dispatch); }, 1200); diff --git a/client/app/scripts/charts/nodes-chart-edges.js b/client/app/scripts/charts/nodes-chart-edges.js index 245edfac1..3603c1341 100644 --- a/client/app/scripts/charts/nodes-chart-edges.js +++ b/client/app/scripts/charts/nodes-chart-edges.js @@ -1,7 +1,7 @@ import React from 'react'; import { connect } from 'react-redux'; -import { List as makeList } from 'immutable'; +import { selectedNetworkNodesIdsSelector } from '../selectors/node-networks'; import { currentTopologySearchNodeMatchesSelector } from '../selectors/search'; import { hasSelectedNode as hasSelectedNodeFn } from '../utils/topology-utils'; import EdgeContainer from './edge-container'; @@ -9,7 +9,7 @@ import EdgeContainer from './edge-container'; class NodesChartEdges extends React.Component { render() { const { hasSelectedNode, highlightedEdgeIds, layoutEdges, searchQuery, - isAnimated, selectedScale, selectedNodeId, selectedNetwork, selectedNetworkNodes, + isAnimated, selectedScale, selectedNodeId, selectedNetwork, selectedNetworkNodesIds, searchNodeMatches } = this.props; return ( @@ -24,8 +24,8 @@ class NodesChartEdges extends React.Component { !(searchNodeMatches.has(edge.get('source')) && searchNodeMatches.has(edge.get('target'))); const noSelectedNetworks = selectedNetwork && - !(selectedNetworkNodes.contains(edge.get('source')) && - selectedNetworkNodes.contains(edge.get('target'))); + !(selectedNetworkNodesIds.contains(edge.get('source')) && + selectedNetworkNodesIds.contains(edge.get('target'))); const blurred = !highlighted && (otherNodesSelected || (!focused && noMatches) || (!focused && noSelectedNetworks)); @@ -57,7 +57,7 @@ export default connect( searchNodeMatches: currentTopologySearchNodeMatchesSelector(state), searchQuery: state.get('searchQuery'), selectedNetwork: state.get('selectedNetwork'), - selectedNetworkNodes: state.getIn(['networkNodes', state.get('selectedNetwork')], makeList()), + selectedNetworkNodesIds: selectedNetworkNodesIdsSelector(state), selectedNodeId: state.get('selectedNodeId'), }) )(NodesChartEdges); diff --git a/client/app/scripts/charts/nodes-chart-nodes.js b/client/app/scripts/charts/nodes-chart-nodes.js index caed5a42f..1a0a57a88 100644 --- a/client/app/scripts/charts/nodes-chart-nodes.js +++ b/client/app/scripts/charts/nodes-chart-nodes.js @@ -1,8 +1,8 @@ import React from 'react'; import { connect } from 'react-redux'; -import { List as makeList } from 'immutable'; -import { nodeMetricsSelector } from '../selectors/metrics'; +import { nodeNetworksSelector, selectedNetworkNodesIdsSelector } from '../selectors/node-networks'; +import { nodeMetricSelector } from '../selectors/node-metric'; import { currentTopologySearchNodeMatchesSelector } from '../selectors/search'; import { getAdjacentNodes } from '../utils/topology-utils'; import NodeContainer from './node-container'; @@ -10,8 +10,8 @@ import NodeContainer from './node-container'; class NodesChartNodes extends React.Component { render() { const { adjacentNodes, highlightedNodeIds, layoutNodes, isAnimated, - mouseOverNodeId, nodeMetrics, selectedScale, searchQuery, selectedNetwork, - selectedNodeId, searchNodeMatches } = this.props; + mouseOverNodeId, nodeMetric, selectedScale, searchQuery, selectedNetwork, + selectedNodeId, searchNodeMatches, nodeNetworks, selectedNetworkNodesIds } = this.props; // highlighter functions const setHighlighted = node => node.set('highlighted', @@ -22,7 +22,7 @@ class NodesChartNodes extends React.Component { const setBlurred = node => node.set('blurred', (selectedNodeId && !node.get('focused')) || (searchQuery && !searchNodeMatches.has(node.get('id')) && !node.get('highlighted')) - || (selectedNetwork && !(node.get('networks') || makeList()).find(n => n.get('id') === selectedNetwork))); + || (selectedNetwork && !selectedNetworkNodesIds.contains(node.get('id')))); // make sure blurred nodes are in the background const sortNodes = (node) => { @@ -46,26 +46,32 @@ class NodesChartNodes extends React.Component { return ( - {nodesToRender.map(node => )} + {nodesToRender.map((node) => { + const nodeScale = node.get('focused') ? selectedScale : 1; + const nodeId = node.get('id'); + return ( + + ); + })} ); } @@ -74,12 +80,14 @@ class NodesChartNodes extends React.Component { function mapStateToProps(state) { return { adjacentNodes: getAdjacentNodes(state), - nodeMetrics: nodeMetricsSelector(state), + nodeMetric: nodeMetricSelector(state), + nodeNetworks: nodeNetworksSelector(state), + searchNodeMatches: currentTopologySearchNodeMatchesSelector(state), + selectedNetworkNodesIds: selectedNetworkNodesIdsSelector(state), highlightedNodeIds: state.get('highlightedNodeIds'), mouseOverNodeId: state.get('mouseOverNodeId'), selectedNetwork: state.get('selectedNetwork'), selectedNodeId: state.get('selectedNodeId'), - searchNodeMatches: currentTopologySearchNodeMatchesSelector(state), searchQuery: state.get('searchQuery'), }; } diff --git a/client/app/scripts/charts/nodes-chart.js b/client/app/scripts/charts/nodes-chart.js index c0d3263ad..2c9288b0c 100644 --- a/client/app/scripts/charts/nodes-chart.js +++ b/client/app/scripts/charts/nodes-chart.js @@ -6,7 +6,7 @@ import { Map as makeMap } from 'immutable'; import { event as d3Event, select } from 'd3-selection'; import { zoom, zoomIdentity } from 'd3-zoom'; -import { nodeAdjacenciesSelector } from '../selectors/nodes'; +import { shownNodesSelector } from '../selectors/node-filters'; import { clickBackground } from '../actions/app-actions'; import Logo from '../components/logo'; import NodesChartElements from './nodes-chart-elements'; @@ -172,7 +172,7 @@ class NodesChart extends React.Component { function mapStateToProps(state) { return { - nodes: nodeAdjacenciesSelector(state), + nodes: shownNodesSelector(state), forceRelayout: state.get('forceRelayout'), selectedNodeId: state.get('selectedNodeId'), topologyId: state.get('currentTopologyId'), diff --git a/client/app/scripts/charts/nodes-grid.js b/client/app/scripts/charts/nodes-grid.js index 7bfad8956..158fa4b30 100644 --- a/client/app/scripts/charts/nodes-grid.js +++ b/client/app/scripts/charts/nodes-grid.js @@ -5,7 +5,7 @@ import { connect } from 'react-redux'; import { List as makeList, Map as makeMap } from 'immutable'; import NodeDetailsTable from '../components/node-details/node-details-table'; import { clickNode, sortOrderChanged } from '../actions/app-actions'; -import { shownNodesSelector } from '../selectors/nodes'; +import { shownNodesSelector } from '../selectors/node-filters'; import { currentTopologySearchNodeMatchesSelector } from '../selectors/search'; import { getNodeColor } from '../utils/color-utils'; diff --git a/client/app/scripts/components/app.js b/client/app/scripts/components/app.js index 05ac9bf59..4a68db76b 100644 --- a/client/app/scripts/components/app.js +++ b/client/app/scripts/components/app.js @@ -22,6 +22,7 @@ import NetworkSelector from './networks-selector'; import DebugToolbar, { showingDebugToolbar, toggleDebugToolbar } from './debug-toolbar'; import { getRouter, getUrlState } from '../utils/router-utils'; import { getActiveTopologyOptions } from '../utils/topology-utils'; +import { availableNetworksSelector } from '../selectors/node-networks'; const BACKSPACE_KEY_CODE = 8; const ENTER_KEY_CODE = 13; @@ -151,7 +152,7 @@ function mapStateToProps(state) { showingHelp: state.get('showingHelp'), showingTroubleshootingMenu: state.get('showingTroubleshootingMenu'), showingMetricsSelector: state.get('availableCanvasMetrics').count() > 0, - showingNetworkSelector: state.get('availableNetworks').count() > 0, + showingNetworkSelector: availableNetworksSelector(state).count() > 0, showingTerminal: state.get('controlPipes').size > 0, urlState: getUrlState(state) }; diff --git a/client/app/scripts/components/networks-selector.js b/client/app/scripts/components/networks-selector.js index 2a8d55525..aebe06f87 100644 --- a/client/app/scripts/components/networks-selector.js +++ b/client/app/scripts/components/networks-selector.js @@ -3,6 +3,7 @@ import { connect } from 'react-redux'; import classNames from 'classnames'; import { selectNetwork, showNetworks } from '../actions/app-actions'; +import { availableNetworksSelector } from '../selectors/node-networks'; import NetworkSelectorItem from './network-selector-item'; class NetworkSelector extends React.Component { @@ -51,7 +52,7 @@ class NetworkSelector extends React.Component { function mapStateToProps(state) { return { - availableNetworks: state.get('availableNetworks'), + availableNetworks: availableNetworksSelector(state), showingNetworks: state.get('showingNetworks'), pinnedNetwork: state.get('pinnedNetwork') }; diff --git a/client/app/scripts/reducers/root.js b/client/app/scripts/reducers/root.js index 2474ba97e..be34fc7be 100644 --- a/client/app/scripts/reducers/root.js +++ b/client/app/scripts/reducers/root.js @@ -6,7 +6,7 @@ import { fromJS, is as isDeepEqual, List as makeList, Map as makeMap, import ActionTypes from '../constants/action-types'; import { EDGE_ID_SEPARATOR } from '../constants/naming'; import { applyPinnedSearches } from '../utils/search-utils'; -import { getNetworkNodes, getAvailableNetworks } from '../utils/network-view-utils'; +import { getNetworkNodes } from '../utils/network-view-utils'; import { findTopologyById, getAdjacentNodes, @@ -29,7 +29,6 @@ const topologySorter = topology => topology.get('rank'); export const initialState = makeMap({ availableCanvasMetrics: makeList(), - availableNetworks: makeList(), controlPipes: makeOrderedMap(), // pipeId -> controlPipe controlStatus: makeMap(), currentTopology: null, @@ -572,23 +571,7 @@ export function rootReducer(state = initialState, action) { // apply pinned searches, filters nodes that dont match state = applyPinnedSearches(state); - - // TODO move this setting of networks as toplevel node field to backend, - // to not rely on field IDs here. should be determined by topology implementer - state = state.update('nodes', nodes => nodes.map((node) => { - if (node.has('metadata')) { - const networks = node.get('metadata') - .find(field => field.get('id') === 'docker_container_networks'); - if (networks) { - return node.set('networks', fromJS( - networks.get('value').split(', ').map(n => ({id: n, label: n, colorKey: n})))); - } - } - return node; - })); - - state = state.set('networkNodes', getNetworkNodes(state.get('nodes'))); - state = state.set('availableNetworks', getAvailableNetworks(state.get('nodes'))); + state = state.set('networkNodes', getNetworkNodes(state)); state = state.set('availableCanvasMetrics', state.get('nodes') .valueSeq() diff --git a/client/app/scripts/selectors/node-filters.js b/client/app/scripts/selectors/node-filters.js new file mode 100644 index 000000000..0d1d0c7d3 --- /dev/null +++ b/client/app/scripts/selectors/node-filters.js @@ -0,0 +1,9 @@ +import { createSelector } from 'reselect'; + + +export const shownNodesSelector = createSelector( + [ + state => state.get('nodes'), + ], + nodes => nodes.filter(node => !node.get('filtered')) +); diff --git a/client/app/scripts/selectors/metrics.js b/client/app/scripts/selectors/node-metric.js similarity index 93% rename from client/app/scripts/selectors/metrics.js rename to client/app/scripts/selectors/node-metric.js index 54803663c..eb1445aba 100644 --- a/client/app/scripts/selectors/metrics.js +++ b/client/app/scripts/selectors/node-metric.js @@ -10,7 +10,7 @@ const topCardNodeSelector = createSelector( nodeDetails => nodeDetails.last() ); -export const nodeMetricsSelector = createMapSelector( +export const nodeMetricSelector = createMapSelector( [ state => state.get('nodes'), state => state.get('selectedMetric'), diff --git a/client/app/scripts/selectors/node-networks.js b/client/app/scripts/selectors/node-networks.js new file mode 100644 index 000000000..27f9fe6dd --- /dev/null +++ b/client/app/scripts/selectors/node-networks.js @@ -0,0 +1,45 @@ +import { createSelector } from 'reselect'; +import { createMapSelector } from 'reselect-map'; +import { fromJS, List as makeList } from 'immutable'; + + +const extractNodeNetworksValue = (node) => { + if (node.has('metadata')) { + const networks = node.get('metadata') + .find(field => field.get('id') === 'docker_container_networks'); + return networks && networks.get('value'); + } + return null; +}; + +// TODO: Move this setting of networks as toplevel node field to backend, +// to not rely on field IDs here. should be determined by topology implementer. +export const nodeNetworksSelector = createMapSelector( + [ + state => state.get('nodes').map(extractNodeNetworksValue), + ], + (networksValue) => { + if (!networksValue) { + return makeList(); + } + return fromJS(networksValue.split(', ').map(network => ({ + id: network, label: network, colorKey: network + }))); + } +); + +export const availableNetworksSelector = createSelector( + [ + nodeNetworksSelector + ], + networksMap => networksMap.toList().flatten(true).toSet().toList() + .sortBy(m => m.get('label')) +); + +export const selectedNetworkNodesIdsSelector = createSelector( + [ + state => state.get('networkNodes'), + state => state.get('selectedNetwork'), + ], + (networkNodes, selectedNetwork) => networkNodes.get(selectedNetwork) +); diff --git a/client/app/scripts/selectors/nodes.js b/client/app/scripts/selectors/nodes.js deleted file mode 100644 index f6470b768..000000000 --- a/client/app/scripts/selectors/nodes.js +++ /dev/null @@ -1,28 +0,0 @@ -import { createSelector } from 'reselect'; -import { Map as makeMap } from 'immutable'; - - -export const shownNodesSelector = createSelector( - [ - state => state.get('nodes'), - ], - nodes => nodes.filter(node => !node.get('filtered')) -); - -export const nodeAdjacenciesSelector = createSelector( - [ - shownNodesSelector, - ], - nodes => nodes.map(node => makeMap({ - id: node.get('id'), - adjacency: node.get('adjacency'), - label: node.get('label'), - pseudo: node.get('pseudo'), - subLabel: node.get('labelMinor'), - metrics: node.get('metrics'), - rank: node.get('rank'), - shape: node.get('shape'), - stack: node.get('stack'), - networks: node.get('networks'), - })) -); diff --git a/client/app/scripts/selectors/search.js b/client/app/scripts/selectors/search.js index f5297ec20..354bc6677 100644 --- a/client/app/scripts/selectors/search.js +++ b/client/app/scripts/selectors/search.js @@ -18,7 +18,7 @@ export const searchNodeMatchesSelector = createMapSelector( parsedSearchQuerySelector, ], // TODO: Bring map selectors one level deeper here so that `searchTopology` is - // not executed against all the topology nodes every time a small change occurs. + // not executed against all the topology nodes when the nodes delta is small. (nodes, parsed) => (parsed ? searchTopology(nodes, parsed) : makeMap()) ); diff --git a/client/app/scripts/utils/network-view-utils.js b/client/app/scripts/utils/network-view-utils.js index 343cc4112..08a66ceb6 100644 --- a/client/app/scripts/utils/network-view-utils.js +++ b/client/app/scripts/utils/network-view-utils.js @@ -1,20 +1,15 @@ -import { fromJS, List as makeList } from 'immutable'; +import { fromJS } from 'immutable'; -export function getNetworkNodes(nodes) { - const networks = {}; - nodes.forEach(node => (node.get('networks') || makeList()).forEach((n) => { - const networkId = n.get('id'); - networks[networkId] = (networks[networkId] || []).concat([node.get('id')]); - })); - return fromJS(networks); -} - - -export function getAvailableNetworks(nodes) { - return nodes - .valueSeq() - .flatMap(node => node.get('networks') || makeList()) - .toSet() - .toList() - .sortBy(m => m.get('label')); +import { nodeNetworksSelector } from '../selectors/node-networks'; + +export function getNetworkNodes(state) { + const networksMap = {}; + nodeNetworksSelector(state).forEach((networks, nodeId) => { + networks.forEach((network) => { + const networkId = network.get('id'); + networksMap[networkId] = networksMap[networkId] || []; + networksMap[networkId].push(nodeId); + }); + }); + return fromJS(networksMap); }