From 3f8a26d55155d9f788470453a513240cbcf5e1e9 Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Mon, 14 Mar 2016 18:28:35 +0100 Subject: [PATCH] Make app-store's topologies object immutable * refactoring, functionality should be the same * adapted tests --- .../charts/__tests__/node-layout-test.js | 2 +- client/app/scripts/charts/nodes-layout.js | 2 +- client/app/scripts/charts/topology-utils.js | 10 --- client/app/scripts/components/status.js | 8 +- client/app/scripts/components/topologies.js | 30 +++---- .../scripts/components/topology-options.js | 30 +++---- .../stores/__tests__/app-store-test.js | 19 +++-- client/app/scripts/stores/app-store.js | 82 +++++++++---------- .../__tests__/topology-utils-test.js | 0 client/app/scripts/utils/topology-utils.js | 56 +++++++++++++ 10 files changed, 134 insertions(+), 105 deletions(-) delete mode 100644 client/app/scripts/charts/topology-utils.js rename client/app/scripts/{charts => utils}/__tests__/topology-utils-test.js (100%) create mode 100644 client/app/scripts/utils/topology-utils.js diff --git a/client/app/scripts/charts/__tests__/node-layout-test.js b/client/app/scripts/charts/__tests__/node-layout-test.js index c370c6e45..088dd43f6 100644 --- a/client/app/scripts/charts/__tests__/node-layout-test.js +++ b/client/app/scripts/charts/__tests__/node-layout-test.js @@ -1,5 +1,5 @@ jest.dontMock('../nodes-layout'); -jest.dontMock('../topology-utils'); +jest.dontMock('../../utils/topology-utils'); jest.dontMock('../../constants/naming'); // edge naming: 'source-target' import { fromJS, Map } from 'immutable'; diff --git a/client/app/scripts/charts/nodes-layout.js b/client/app/scripts/charts/nodes-layout.js index 05daf643c..c6543a8a3 100644 --- a/client/app/scripts/charts/nodes-layout.js +++ b/client/app/scripts/charts/nodes-layout.js @@ -3,7 +3,7 @@ import debug from 'debug'; import { Map as makeMap, Set as ImmSet } from 'immutable'; import { EDGE_ID_SEPARATOR } from '../constants/naming'; -import { updateNodeDegrees } from './topology-utils'; +import { updateNodeDegrees } from '../utils/topology-utils'; const log = debug('scope:nodes-layout'); diff --git a/client/app/scripts/charts/topology-utils.js b/client/app/scripts/charts/topology-utils.js deleted file mode 100644 index ab5c57b4f..000000000 --- a/client/app/scripts/charts/topology-utils.js +++ /dev/null @@ -1,10 +0,0 @@ - -export function updateNodeDegrees(nodes, edges) { - return nodes.map(node => { - const nodeId = node.get('id'); - const degree = edges.count(edge => { - return edge.get('source') === nodeId || edge.get('target') === nodeId; - }); - return node.set('degree', degree); - }); -} diff --git a/client/app/scripts/components/status.js b/client/app/scripts/components/status.js index 017b45a1d..4b9cb3dcf 100644 --- a/client/app/scripts/components/status.js +++ b/client/app/scripts/components/status.js @@ -19,10 +19,10 @@ export default class Status extends React.Component { classNames += ' status-loading'; showWarningIcon = true; } else if (this.props.topology) { - const stats = this.props.topology.stats; - text = `${stats.node_count} nodes`; - if (stats.filtered_nodes) { - text = `${text} (${stats.filtered_nodes} filtered)`; + const stats = this.props.topology.get('stats'); + text = `${stats.get('node_count')} nodes`; + if (stats.get('filtered_nodes')) { + text = `${text} (${stats.get('filtered_nodes')} filtered)`; } classNames += ' status-stats'; showWarningIcon = false; diff --git a/client/app/scripts/components/topologies.js b/client/app/scripts/components/topologies.js index cc99cc198..4e18428ce 100644 --- a/client/app/scripts/components/topologies.js +++ b/client/app/scripts/components/topologies.js @@ -1,9 +1,9 @@ import React from 'react'; -import _ from 'lodash'; import { clickTopology } from '../actions/app-actions'; export default class Topologies extends React.Component { + constructor(props, context) { super(props, context); this.onTopologyClick = this.onTopologyClick.bind(this); @@ -16,8 +16,8 @@ export default class Topologies extends React.Component { } renderSubTopology(subTopology) { - const isActive = subTopology.name === this.props.currentTopology.name; - const topologyId = subTopology.id; + const isActive = subTopology === this.props.currentTopology; + const topologyId = subTopology.get('id'); const title = this.renderTitle(subTopology); const className = isActive ? 'topologies-sub-item topologies-sub-item-active' : 'topologies-sub-item'; @@ -25,47 +25,43 @@ export default class Topologies extends React.Component {
- {subTopology.name} + {subTopology.get('name')}
); } renderTitle(topology) { - return ['Nodes: ' + topology.stats.node_count, - 'Connections: ' + topology.stats.node_count].join('\n'); + return ['Nodes: ' + topology.getIn(['stats', 'node_count']), + 'Connections: ' + topology.getIn(['stats', 'node_count'])].join('\n'); } renderTopology(topology) { - const isActive = topology.name === this.props.currentTopology.name; + const isActive = topology === this.props.currentTopology; const className = isActive ? 'topologies-item-main topologies-item-main-active' : 'topologies-item-main'; - const topologyId = topology.id; + const topologyId = topology.get('id'); const title = this.renderTitle(topology); return (
- {topology.name} + {topology.get('name')}
- {topology.sub_topologies && topology.sub_topologies.map(this.renderSubTopology)} + {topology.has('sub_topologies') && topology.get('sub_topologies').map(this.renderSubTopology)}
); } render() { - const topologies = _.sortBy(this.props.topologies, function(topology) { - return topology.rank; - }); - return (
- {this.props.currentTopology && topologies.map(function(topology) { - return this.renderTopology(topology); - }, this)} + {this.props.currentTopology && this.props.topologies.map( + topology => this.renderTopology(topology) + )}
); } diff --git a/client/app/scripts/components/topology-options.js b/client/app/scripts/components/topology-options.js index 9c598a3b3..4c021581b 100644 --- a/client/app/scripts/components/topology-options.js +++ b/client/app/scripts/components/topology-options.js @@ -1,5 +1,4 @@ import React from 'react'; -import _ from 'lodash'; import TopologyOptionAction from './topology-option-action'; @@ -21,7 +20,7 @@ export default class TopologyOptions extends React.Component { const actions = []; const activeOptions = this.props.activeOptions; const topologyId = this.props.topologyId; - const option = items[0].option; + const option = items.first().get('option'); // find active option value if (activeOptions && activeOptions.has(option)) { @@ -29,18 +28,18 @@ export default class TopologyOptions extends React.Component { } else { // get default value items.forEach(function(item) { - if (item.default) { - activeValue = item.value; + if (item.get('default')) { + activeValue = item.get('value'); } }); } // render active option as text, add other options as actions items.forEach(function(item) { - if (item.value === activeValue) { - activeText = item.display; + if (item.get('value') === activeValue) { + activeText = item.get('display'); } else { - actions.push(this.renderAction(item.value, item.option, topologyId)); + actions.push(this.renderAction(item.get('value'), item.get('option'), topologyId)); } }, this); @@ -55,20 +54,15 @@ export default class TopologyOptions extends React.Component { } render() { - const options = _.sortBy( - _.map(this.props.options, function(items, optionId) { - _.each(items, function(item) { - item.option = optionId; - }); - items.option = optionId; - return items; - }), - 'option' - ); + const options = this.props.options.map((items, optionId) => { + let itemsMap = items.map(item => item.set('option', optionId)); + itemsMap = itemsMap.set('option', optionId); + return itemsMap; + }); return (
- {options.map(function(items) { + {options.toIndexedSeq().map(function(items) { return this.renderOption(items); }, this)}
diff --git a/client/app/scripts/stores/__tests__/app-store-test.js b/client/app/scripts/stores/__tests__/app-store-test.js index ca31a2fdc..e90356603 100644 --- a/client/app/scripts/stores/__tests__/app-store-test.js +++ b/client/app/scripts/stores/__tests__/app-store-test.js @@ -1,3 +1,4 @@ +jest.dontMock('../../utils/topology-utils'); jest.dontMock('../../constants/action-types'); jest.dontMock('../app-store'); @@ -160,7 +161,7 @@ describe('AppStore', function() { it('init with no topologies', function() { const topos = AppStore.getTopologies(); - expect(topos.length).toBe(0); + expect(topos.size).toBe(0); expect(AppStore.getCurrentTopology()).toBeUndefined(); }); @@ -168,20 +169,20 @@ describe('AppStore', function() { registeredCallback(ClickTopologyAction); registeredCallback(ReceiveTopologiesAction); - expect(AppStore.getTopologies().length).toBe(2); - expect(AppStore.getCurrentTopology().name).toBe('Topo1'); + expect(AppStore.getTopologies().size).toBe(2); + expect(AppStore.getCurrentTopology().get('name')).toBe('Topo1'); expect(AppStore.getCurrentTopologyUrl()).toBe('/topo1'); - expect(AppStore.getCurrentTopologyOptions().option1).toBeDefined(); + expect(AppStore.getCurrentTopologyOptions().get('option1')).toBeDefined(); }); it('get sub-topology', function() { registeredCallback(ReceiveTopologiesAction); registeredCallback(ClickSubTopologyAction); - expect(AppStore.getTopologies().length).toBe(2); - expect(AppStore.getCurrentTopology().name).toBe('topo 1 grouped'); + expect(AppStore.getTopologies().size).toBe(2); + expect(AppStore.getCurrentTopology().get('name')).toBe('topo 1 grouped'); expect(AppStore.getCurrentTopologyUrl()).toBe('/topo1-grouped'); - expect(AppStore.getCurrentTopologyOptions()).toBeUndefined(); + expect(AppStore.getCurrentTopologyOptions().size).toBe(0); }); // topology options @@ -402,7 +403,7 @@ describe('AppStore', function() { it('selectes relatives topology while keeping node selected', function() { registeredCallback(ClickTopologyAction); registeredCallback(ReceiveTopologiesAction); - expect(AppStore.getCurrentTopology().name).toBe('Topo1'); + expect(AppStore.getCurrentTopology().get('name')).toBe('Topo1'); registeredCallback(ClickNodeAction); expect(AppStore.getSelectedNodeId()).toBe('n1'); @@ -422,6 +423,6 @@ describe('AppStore', function() { expect(AppStore.getSelectedNodeId()).toBe('rel1'); expect(AppStore.getNodeDetails().keySeq().last()).toEqual('rel1'); expect(AppStore.getNodeDetails().size).toEqual(1); - expect(AppStore.getCurrentTopology().name).toBe('Topo2'); + expect(AppStore.getCurrentTopology().get('name')).toBe('Topo2'); }); }); diff --git a/client/app/scripts/stores/app-store.js b/client/app/scripts/stores/app-store.js index c6d6d14e7..0bfea9c21 100644 --- a/client/app/scripts/stores/app-store.js +++ b/client/app/scripts/stores/app-store.js @@ -1,39 +1,23 @@ import _ from 'lodash'; import debug from 'debug'; -import Immutable from 'immutable'; +import { fromJS, List, Map, OrderedMap, Set } from 'immutable'; import { Store } from 'flux/utils'; import AppDispatcher from '../dispatcher/app-dispatcher'; import ActionTypes from '../constants/action-types'; import { EDGE_ID_SEPARATOR } from '../constants/naming'; +import { findTopologyById, setTopologyUrlsById, updateTopologyIds } from '../utils/topology-utils'; -const makeMap = Immutable.Map; -const makeOrderedMap = Immutable.OrderedMap; -const makeSet = Immutable.Set; +const makeList = List; +const makeMap = Map; +const makeOrderedMap = OrderedMap; +const makeSet = Set; const log = debug('scope:app-store'); const error = debug('scope:error'); // Helpers -function findTopologyById(subTree, topologyId) { - let foundTopology; - - _.each(subTree, function(topology) { - if (_.endsWith(topology.url, topologyId)) { - foundTopology = topology; - } - if (!foundTopology) { - foundTopology = findTopologyById(topology.sub_topologies, topologyId); - } - if (foundTopology) { - return false; - } - }); - - return foundTopology; -} - function makeNode(node) { return { id: node.id, @@ -64,7 +48,7 @@ let mouseOverNodeId = null; let nodeDetails = makeOrderedMap(); // nodeId -> details let nodes = makeOrderedMap(); // nodeId -> node let selectedNodeId = null; -let topologies = []; +let topologies = makeList(); let topologiesLoaded = false; let topologyUrlsById = makeOrderedMap(); // topologyId -> topologyUrl let routeSet = false; @@ -72,15 +56,19 @@ let controlPipes = makeOrderedMap(); // pipeId -> controlPipe let updatePausedAt = null; // Date let websocketClosed = true; +const topologySorter = topology => topology.get('rank'); + // adds ID field to topology (based on last part of URL path) and save urls in // map for easy lookup -function processTopologies(topologyList) { - _.each(topologyList, function(topology) { - topology.id = topology.url.split('/').pop(); - topologyUrlsById = topologyUrlsById.set(topology.id, topology.url); - processTopologies(topology.sub_topologies); - }); - return topologyList; +function processTopologies(nextTopologies) { + // add IDs to topology objects in-place + updateTopologyIds(nextTopologies); + + // cache URLs by ID + topologyUrlsById = setTopologyUrlsById(topologyUrlsById, nextTopologies); + + const immNextTopologies = fromJS(nextTopologies).sortBy(topologySorter); + topologies = topologies.mergeDeep(immNextTopologies); } function setTopology(topologyId) { @@ -89,24 +77,28 @@ function setTopology(topologyId) { } function setDefaultTopologyOptions(topologyList) { - _.each(topologyList, function(topology) { + topologyList.forEach(topology => { let defaultOptions = makeOrderedMap(); - _.each(topology.options, function(items, option) { - _.each(items, function(item) { - if (item.default === true) { - defaultOptions = defaultOptions.set(option, item.value); - } + if (topology.has('options')) { + topology.get('options').forEach(function(items, option) { + items.forEach(item => { + if (item.get('default') === true) { + defaultOptions = defaultOptions.set(option, item.get('value')); + } + }); }); - }); + } if (defaultOptions.size) { topologyOptions = topologyOptions.set( - topology.id, + topology.get('id'), defaultOptions ); } - setDefaultTopologyOptions(topology.sub_topologies); + if (topology.has('sub_topologies')) { + setDefaultTopologyOptions(topology.get('sub_topologies')); + } }); } @@ -191,11 +183,11 @@ export class AppStore extends Store { } getCurrentTopologyOptions() { - return currentTopology && currentTopology.options; + return currentTopology && currentTopology.get('options') || makeOrderedMap(); } getCurrentTopologyUrl() { - return currentTopology && currentTopology.url; + return currentTopology && currentTopology.get('url'); } getErrorUrl() { @@ -289,7 +281,7 @@ export class AppStore extends Store { } isTopologyEmpty() { - return currentTopology && currentTopology.stats && currentTopology.stats.node_count === 0 && nodes.size === 0; + return currentTopology && currentTopology.get('stats') && currentTopology.get('stats').get('node_count') === 0 && nodes.size === 0; } isUpdatePaused() { @@ -560,7 +552,7 @@ export class AppStore extends Store { // add new nodes _.each(payload.delta.add, function(node) { - nodes = nodes.set(node.id, Immutable.fromJS(makeNode(node))); + nodes = nodes.set(node.id, fromJS(makeNode(node))); }); if (emitChange) { @@ -581,7 +573,7 @@ export class AppStore extends Store { case ActionTypes.RECEIVE_TOPOLOGIES: errorUrl = null; topologyUrlsById = topologyUrlsById.clear(); - topologies = processTopologies(payload.topologies); + processTopologies(payload.topologies); setTopology(currentTopologyId); // only set on first load, if options are not already set via route if (!topologiesLoaded && topologyOptions.size === 0) { @@ -619,7 +611,7 @@ export class AppStore extends Store { } else { nodeDetails = nodeDetails.clear(); } - topologyOptions = Immutable.fromJS(payload.state.topologyOptions) + topologyOptions = fromJS(payload.state.topologyOptions) || topologyOptions; this.__emitChange(); break; diff --git a/client/app/scripts/charts/__tests__/topology-utils-test.js b/client/app/scripts/utils/__tests__/topology-utils-test.js similarity index 100% rename from client/app/scripts/charts/__tests__/topology-utils-test.js rename to client/app/scripts/utils/__tests__/topology-utils-test.js diff --git a/client/app/scripts/utils/topology-utils.js b/client/app/scripts/utils/topology-utils.js new file mode 100644 index 000000000..db0897592 --- /dev/null +++ b/client/app/scripts/utils/topology-utils.js @@ -0,0 +1,56 @@ +import _ from 'lodash'; + +export function findTopologyById(subTree, topologyId) { + let foundTopology; + + subTree.forEach(topology => { + if (_.endsWith(topology.get('url'), topologyId)) { + foundTopology = topology; + } + if (!foundTopology && topology.has('sub_topologies')) { + foundTopology = findTopologyById(topology.get('sub_topologies'), topologyId); + } + if (foundTopology) { + return false; + } + }); + + return foundTopology; +} + + +export function updateNodeDegrees(nodes, edges) { + return nodes.map(node => { + const nodeId = node.get('id'); + const degree = edges.count(edge => { + return edge.get('source') === nodeId || edge.get('target') === nodeId; + }); + return node.set('degree', degree); + }); +} + +/* set topology.id in place on each topology */ +export function updateTopologyIds(topologies) { + topologies.forEach(topology => { + topology.id = topology.url.split('/').pop(); + if (topology.sub_topologies) { + updateTopologyIds(topology.sub_topologies); + } + }); + return topologies; +} + +// adds ID field to topology (based on last part of URL path) and save urls in +// map for easy lookup +export function setTopologyUrlsById(topologyUrlsById, topologies) { + let urlMap = topologyUrlsById; + topologies.forEach(topology => { + urlMap = urlMap.set(topology.id, topology.url); + if (topology.sub_topologies) { + topology.sub_topologies.forEach(subTopology => { + urlMap = urlMap.set(subTopology.id, subTopology.url); + }); + } + }); + return urlMap; +}