From dc4855ba56a1532757f07e7666b1eac6551a45eb Mon Sep 17 00:00:00 2001 From: Filip Barl Date: Wed, 7 Jun 2017 12:45:40 +0200 Subject: [PATCH] Polishing & refactoring. --- client/app/scripts/actions/app-actions.js | 30 +++---- client/app/scripts/components/pause-button.js | 18 ++-- .../scripts/components/timeline-control.js | 89 +++++++------------ .../components/topology-timestamp-button.js | 48 ++++------ client/app/scripts/constants/limits.js | 1 + client/app/scripts/constants/timer.js | 3 + client/app/scripts/reducers/root.js | 12 +-- client/app/scripts/selectors/timeline.js | 9 ++ ...e-buffer-utils.js => nodes-delta-utils.js} | 28 +++--- 9 files changed, 99 insertions(+), 139 deletions(-) create mode 100644 client/app/scripts/selectors/timeline.js rename client/app/scripts/utils/{update-buffer-utils.js => nodes-delta-utils.js} (68%) diff --git a/client/app/scripts/actions/app-actions.js b/client/app/scripts/actions/app-actions.js index a3c98cd650..d829ebffc3 100644 --- a/client/app/scripts/actions/app-actions.js +++ b/client/app/scripts/actions/app-actions.js @@ -4,10 +4,6 @@ import { find } from 'lodash'; import ActionTypes from '../constants/action-types'; import { saveGraph } from '../utils/file-utils'; import { updateRoute } from '../utils/router-utils'; -import { - isNodesDeltaPaused, - getUpdateBufferSize, -} from '../utils/update-buffer-utils'; import { doControlRequest, getAllNodes, @@ -21,6 +17,7 @@ import { } from '../utils/web-api-utils'; import { storageSet } from '../utils/storage-utils'; import { loadTheme } from '../utils/contrast-utils'; +import { isPausedSelector } from '../selectors/timeline'; import { availableMetricTypesSelector, nextPinnedMetricTypeSelector, @@ -32,11 +29,14 @@ import { isResourceViewModeSelector, resourceViewAvailableSelector, } from '../selectors/topology'; + +import { NODES_DELTA_BUFFER_SIZE_LIMIT } from '../constants/limits'; +import { NODES_DELTA_BUFFER_FEED_INTERVAL } from '../constants/timer'; import { GRAPH_VIEW_MODE, TABLE_VIEW_MODE, RESOURCE_VIEW_MODE, - } from '../constants/naming'; +} from '../constants/naming'; const log = debug('scope:app-actions'); @@ -88,8 +88,7 @@ function bufferDeltaUpdate(delta) { return; } - const bufferLength = 100; - if (getUpdateBufferSize(getState()) >= bufferLength) { + if (getState().get('nodesDeltaBuffer').size >= NODES_DELTA_BUFFER_SIZE_LIMIT) { dispatch({ type: ActionTypes.CONSOLIDATE_NODES_DELTA_BUFFER }); } @@ -97,7 +96,7 @@ function bufferDeltaUpdate(delta) { type: ActionTypes.ADD_TO_NODES_DELTA_BUFFER, delta, }); - log('Buffering node delta, new size', getUpdateBufferSize(getState())); + log('Buffering node delta, new size', getState().get('nodesDeltaBuffer').size); }; } @@ -626,7 +625,7 @@ export function receiveNodesDelta(delta) { const hasChanges = delta.add || delta.update || delta.remove; if (hasChanges || movingInTime) { - if (state.get('updatePausedAt') !== null) { + if (isPausedSelector(state)) { dispatch(bufferDeltaUpdate(delta)); } else { dispatch({ @@ -640,20 +639,19 @@ export function receiveNodesDelta(delta) { function maybeUpdateFromNodesDeltaBuffer() { return (dispatch, getState) => { - const state = getState(); - if (isNodesDeltaPaused(state)) { + if (isPausedSelector(getState())) { dispatch(resetNodesDeltaBuffer()); } else { - if (getUpdateBufferSize(state) > 0) { - const delta = state.get('nodesDeltaBuffer').first(); + if (!getState().get('nodesDeltaBuffer').isEmpty()) { + const delta = getState().get('nodesDeltaBuffer').first(); dispatch({ type: ActionTypes.POP_NODES_DELTA_BUFFER }); dispatch(receiveNodesDelta(delta)); } - if (getUpdateBufferSize(state) > 0) { - const feedInterval = 1000; + if (!getState().get('nodesDeltaBuffer').isEmpty()) { nodesDeltaBufferUpdateTimer = setTimeout( () => dispatch(maybeUpdateFromNodesDeltaBuffer()), - feedInterval); + NODES_DELTA_BUFFER_FEED_INTERVAL, + ); } } }; diff --git a/client/app/scripts/components/pause-button.js b/client/app/scripts/components/pause-button.js index e363c5db57..328334d19e 100644 --- a/client/app/scripts/components/pause-button.js +++ b/client/app/scripts/components/pause-button.js @@ -3,20 +3,20 @@ import moment from 'moment'; import classNames from 'classnames'; import { connect } from 'react-redux'; -import { getUpdateBufferSize } from '../utils/update-buffer-utils'; +import { isPausedSelector } from '../selectors/timeline'; import { clickPauseUpdate, clickResumeUpdate } from '../actions/app-actions'; class PauseButton extends React.Component { render() { - const isPaused = this.props.updatePausedAt !== null; - const updateCount = this.props.updateCount; - const hasUpdates = updateCount > 0; - const title = isPaused ? - `Paused ${moment(this.props.updatePausedAt).fromNow()}` : - 'Pause updates (freezes the nodes in their current layout)'; + const { isPaused, hasUpdates, updateCount, updatePausedAt } = this.props; const action = isPaused ? this.props.clickResumeUpdate : this.props.clickPauseUpdate; const className = classNames('button pause-button', { active: isPaused }); + + const title = isPaused ? + `Paused ${moment(updatePausedAt).fromNow()}` : + 'Pause updates (freezes the nodes in their current layout)'; + let label = ''; if (hasUpdates && isPaused) { label = `Paused +${updateCount}`; @@ -37,8 +37,10 @@ class PauseButton extends React.Component { function mapStateToProps(state) { return { - updateCount: getUpdateBufferSize(state), + hasUpdates: !state.get('nodesDeltaBuffer').isEmpty(), + updateCount: state.get('nodesDeltaBuffer').size, updatePausedAt: state.get('updatePausedAt'), + isPaused: isPausedSelector(state), }; } diff --git a/client/app/scripts/components/timeline-control.js b/client/app/scripts/components/timeline-control.js index 0e1e7b3860..813c33559f 100644 --- a/client/app/scripts/components/timeline-control.js +++ b/client/app/scripts/components/timeline-control.js @@ -20,83 +20,51 @@ const sliderRanges = { last15Minutes: { label: 'Last 15 minutes', getStart: () => moment().utc().subtract(15, 'minutes'), - getEnd: () => moment().utc(), }, last1Hour: { label: 'Last 1 hour', getStart: () => moment().utc().subtract(1, 'hour'), - getEnd: () => moment().utc(), }, last6Hours: { label: 'Last 6 hours', getStart: () => moment().utc().subtract(6, 'hours'), - getEnd: () => moment().utc(), }, last24Hours: { label: 'Last 24 hours', getStart: () => moment().utc().subtract(24, 'hours'), - getEnd: () => moment().utc(), }, last7Days: { label: 'Last 7 days', getStart: () => moment().utc().subtract(7, 'days'), - getEnd: () => moment().utc(), }, last30Days: { label: 'Last 30 days', getStart: () => moment().utc().subtract(30, 'days'), - getEnd: () => moment().utc(), }, last90Days: { label: 'Last 90 days', getStart: () => moment().utc().subtract(90, 'days'), - getEnd: () => moment().utc(), }, last1Year: { label: 'Last 1 year', getStart: () => moment().subtract(1, 'year'), - getEnd: () => moment().utc(), }, todaySoFar: { label: 'Today so far', getStart: () => moment().utc().startOf('day'), - getEnd: () => moment().utc(), }, thisWeekSoFar: { label: 'This week so far', getStart: () => moment().utc().startOf('week'), - getEnd: () => moment().utc(), }, thisMonthSoFar: { label: 'This month so far', getStart: () => moment().utc().startOf('month'), - getEnd: () => moment().utc(), }, thisYearSoFar: { label: 'This year so far', getStart: () => moment().utc().startOf('year'), - getEnd: () => moment().utc(), }, - // yesterday: { - // label: 'Yesterday', - // getStart: () => moment().utc().subtract(1, 'day').startOf('day'), - // getEnd: () => moment().utc().subtract(1, 'day').endOf('day'), - // }, - // previousWeek: { - // label: 'Previous week', - // getStart: () => moment().utc().subtract(1, 'week').startOf('week'), - // getEnd: () => moment().utc().subtract(1, 'week').endOf('week'), - // }, - // previousMonth: { - // label: 'Previous month', - // getStart: () => moment().utc().subtract(1, 'month').startOf('month'), - // getEnd: () => moment().utc().subtract(1, 'month').endOf('month'), - // }, - // previousYear: { - // label: 'Previous year', - // getStart: () => moment().utc().subtract(1, 'year').startOf('year'), - // getEnd: () => moment().utc().subtract(1, 'year').endOf('year'), - // }, }; class TimelineControl extends React.Component { @@ -105,7 +73,7 @@ class TimelineControl extends React.Component { this.state = { showTimelinePanel: false, - offsetMilliseconds: 0, + millisecondsInPast: 0, rangeOptionSelected: sliderRanges.last1Hour, }; @@ -130,42 +98,46 @@ class TimelineControl extends React.Component { this.setState({ showTimelinePanel: !this.state.showTimelinePanel }); } - handleSliderChange(value) { - const offsetMilliseconds = this.getRangeMilliseconds() - value; + handleSliderChange(sliderValue) { + const millisecondsInPast = this.getRangeMilliseconds() - sliderValue; this.props.startMovingInTime(); - this.debouncedUpdateTimestamp(offsetMilliseconds); - this.setState({ offsetMilliseconds }); + this.debouncedUpdateTimestamp(millisecondsInPast); + this.setState({ millisecondsInPast }); } - getRangeMilliseconds() { - const range = this.state.rangeOptionSelected; - return range.getEnd().diff(range.getStart()); + handleRangeOptionClick(rangeOption) { + this.setState({ rangeOptionSelected: rangeOption }); + + const rangeMilliseconds = this.getRangeMilliseconds(rangeOption); + if (this.state.millisecondsInPast > rangeMilliseconds) { + this.updateTimestamp(rangeMilliseconds); + this.setState({ millisecondsInPast: rangeMilliseconds }); + } + } + + getRangeMilliseconds(rangeOption) { + rangeOption = rangeOption || this.state.rangeOptionSelected; + return moment().diff(rangeOption.getStart()); } jumpToNow() { this.setState({ showTimelinePanel: false, - offsetMilliseconds: 0, + millisecondsInPast: 0, rangeOptionSelected: sliderRanges.last1Hour, }); this.props.startMovingInTime(); this.updateTimestamp(null); } - getTotalOffset() { - const { rangeOptionSelected, offsetMilliseconds } = this.state; - const rangeBehindMilliseconds = moment().utc().diff(rangeOptionSelected.getEnd()); - return offsetMilliseconds + rangeBehindMilliseconds; - } - - renderRangeOption(option) { - const handleClick = () => { this.setState({ rangeOptionSelected: option }); }; - const selected = (this.state.rangeOptionSelected.label === option.label); + renderRangeOption(rangeOption) { + const handleClick = () => { this.handleRangeOptionClick(rangeOption); }; + const selected = (this.state.rangeOptionSelected.label === rangeOption.label); const className = classNames('option', { selected }); return ( - - {option.label} + + {rangeOption.label} ); } @@ -179,13 +151,13 @@ class TimelineControl extends React.Component { } renderTimelineSlider() { - const { offsetMilliseconds } = this.state; + const { millisecondsInPast } = this.state; const rangeMilliseconds = this.getRangeMilliseconds(); return ( ); @@ -193,9 +165,8 @@ class TimelineControl extends React.Component { render() { const { movingInTime } = this.props; - const { showTimelinePanel, offsetMilliseconds } = this.state; - - const showingCurrent = (this.getTotalOffset() === 0); + const { showTimelinePanel, millisecondsInPast } = this.state; + const isCurrent = (millisecondsInPast === 0); return (
@@ -230,10 +201,10 @@ class TimelineControl extends React.Component {
} - {!showingCurrent && this.renderJumpToNowButton()} + {!isCurrent && this.renderJumpToNowButton()} diff --git a/client/app/scripts/components/topology-timestamp-button.js b/client/app/scripts/components/topology-timestamp-button.js index 727012d72c..46d6f91fbc 100644 --- a/client/app/scripts/components/topology-timestamp-button.js +++ b/client/app/scripts/components/topology-timestamp-button.js @@ -3,63 +3,44 @@ import moment from 'moment'; import classNames from 'classnames'; import { connect } from 'react-redux'; +import { isPausedSelector } from '../selectors/timeline'; +import { TIMELINE_TICK_INTERVAL } from '../constants/timer'; -const TIMESTAMP_TICK_INTERVAL = 500; class TopologyTimestampButton extends React.PureComponent { - constructor(props, context) { - super(props, context); - - this.state = this.getFreshState(); - } - componentDidMount() { this.timer = setInterval(() => { - if (!this.props.paused) { - this.setState(this.getFreshState()); + if (!this.props.isPaused) { + this.forceUpdate(); } - }, TIMESTAMP_TICK_INTERVAL); + }, TIMELINE_TICK_INTERVAL); } componentWillUnmount() { clearInterval(this.timer); } - getFreshState() { - const { updatePausedAt, offset } = this.props; - - let timestamp = updatePausedAt; - let showingCurrentState = false; - - if (!updatePausedAt) { - timestamp = moment().utc(); - showingCurrentState = true; - - if (offset >= 1000) { - timestamp = timestamp.subtract(offset); - showingCurrentState = false; - } - } - return { timestamp, showingCurrentState }; - } - renderTimestamp() { + const { isPaused, updatePausedAt, millisecondsInPast } = this.props; + const timestamp = isPaused ? updatePausedAt : moment().utc().subtract(millisecondsInPast); + return ( - + ); } render() { - const { selected, onClick } = this.props; - const { showingCurrentState } = this.state; + const { selected, onClick, millisecondsInPast } = this.props; + const isCurrent = (millisecondsInPast === 0); + const className = classNames('button topology-timestamp-button', { - selected, current: showingCurrentState, + selected, current: isCurrent }); return ( - {showingCurrentState ? 'now' : this.renderTimestamp()} + {isCurrent ? 'now' : this.renderTimestamp()} @@ -69,6 +50,7 @@ class TopologyTimestampButton extends React.PureComponent { function mapStateToProps(state) { return { + isPaused: isPausedSelector(state), updatePausedAt: state.get('updatePausedAt'), }; } diff --git a/client/app/scripts/constants/limits.js b/client/app/scripts/constants/limits.js index afb2c79382..56e80aa409 100644 --- a/client/app/scripts/constants/limits.js +++ b/client/app/scripts/constants/limits.js @@ -1,2 +1,3 @@ export const NODE_DETAILS_DATA_ROWS_DEFAULT_LIMIT = 5; +export const NODES_DELTA_BUFFER_SIZE_LIMIT = 100; diff --git a/client/app/scripts/constants/timer.js b/client/app/scripts/constants/timer.js index 83234b6181..e4b365b72d 100644 --- a/client/app/scripts/constants/timer.js +++ b/client/app/scripts/constants/timer.js @@ -1,6 +1,7 @@ /* Intervals in ms */ export const API_REFRESH_INTERVAL = 30000; export const TOPOLOGY_REFRESH_INTERVAL = 5000; +export const NODES_DELTA_BUFFER_FEED_INTERVAL = 1000; export const TOPOLOGY_LOADER_DELAY = 100; @@ -8,3 +9,5 @@ export const TABLE_ROW_FOCUS_DEBOUNCE_INTERVAL = 10; export const VIEWPORT_RESIZE_DEBOUNCE_INTERVAL = 200; export const ZOOM_CACHE_DEBOUNCE_INTERVAL = 500; export const TIMELINE_DEBOUNCE_INTERVAL = 500; + +export const TIMELINE_TICK_INTERVAL = 100; diff --git a/client/app/scripts/reducers/root.js b/client/app/scripts/reducers/root.js index 1689ceda50..d2152c343c 100644 --- a/client/app/scripts/reducers/root.js +++ b/client/app/scripts/reducers/root.js @@ -22,7 +22,7 @@ import { isResourceViewModeSelector, } from '../selectors/topology'; import { activeTopologyZoomCacheKeyPathSelector } from '../selectors/zooming'; -import { consolidatedBeginningOfNodesDeltaBuffer } from '../utils/update-buffer-utils'; +import { consolidateNodesDeltas } from '../utils/nodes-delta-utils'; import { getWebsocketQueryTimestamp } from '../utils/web-api-utils'; import { applyPinnedSearches } from '../utils/search-utils'; import { @@ -88,7 +88,7 @@ export const initialState = makeMap({ topologyOptions: makeOrderedMap(), // topologyId -> options topologyUrlsById: makeOrderedMap(), // topologyId -> topologyUrl topologyViewMode: GRAPH_VIEW_MODE, - updatePausedAt: null, // Date + updatePausedAt: null, // moment.js timestamp version: '...', versionUpdate: null, viewport: makeMap(), @@ -375,10 +375,10 @@ export function rootReducer(state = initialState, action) { } case ActionTypes.CONSOLIDATE_NODES_DELTA_BUFFER: { - const deltaUnion = consolidatedBeginningOfNodesDeltaBuffer(state); - state = state.setIn(['nodesDeltaBuffer', 0], deltaUnion); - state = state.deleteIn(['nodesDeltaBuffer', 1]); - return state; + const firstDelta = state.getIn(['nodesDeltaBuffer', 0]); + const secondDelta = state.getIn(['nodesDeltaBuffer', 1]); + const deltaUnion = consolidateNodesDeltas(firstDelta, secondDelta); + return state.update('nodesDeltaBuffer', buffer => buffer.shift().set(0, deltaUnion)); } case ActionTypes.POP_NODES_DELTA_BUFFER: { diff --git a/client/app/scripts/selectors/timeline.js b/client/app/scripts/selectors/timeline.js new file mode 100644 index 0000000000..1bb7b801e3 --- /dev/null +++ b/client/app/scripts/selectors/timeline.js @@ -0,0 +1,9 @@ +import { createSelector } from 'reselect'; + + +export const isPausedSelector = createSelector( + [ + state => state.get('updatePausedAt') + ], + updatePausedAt => updatePausedAt !== null +); diff --git a/client/app/scripts/utils/update-buffer-utils.js b/client/app/scripts/utils/nodes-delta-utils.js similarity index 68% rename from client/app/scripts/utils/update-buffer-utils.js rename to client/app/scripts/utils/nodes-delta-utils.js index 665a77480e..173a54504f 100644 --- a/client/app/scripts/utils/update-buffer-utils.js +++ b/client/app/scripts/utils/nodes-delta-utils.js @@ -1,22 +1,18 @@ import debug from 'debug'; import { union, size, map, find, reject, each } from 'lodash'; -const log = debug('scope:update-buffer-utils'); +const log = debug('scope:nodes-delta-utils'); -export function isNodesDeltaPaused(state) { - return state.get('updatePausedAt') !== null; -} - -// consolidate first buffer entry with second -export function consolidatedBeginningOfNodesDeltaBuffer(state) { - const first = state.getIn(['nodesDeltaBuffer', 0]); - const second = state.getIn(['nodesDeltaBuffer', 1]); +// TODO: Would be nice to have a unit test for this function. +export function consolidateNodesDeltas(first, second) { let toAdd = union(first.add, second.add); let toUpdate = union(first.update, second.update); let toRemove = union(first.remove, second.remove); - log('Consolidating delta buffer', 'add', size(toAdd), 'update', - size(toUpdate), 'remove', size(toRemove)); + log('Consolidating delta buffer', + 'add', size(toAdd), + 'update', size(toUpdate), + 'remove', size(toRemove)); // check if an added node in first was updated in second -> add second update toAdd = map(toAdd, (node) => { @@ -51,8 +47,10 @@ export function consolidatedBeginningOfNodesDeltaBuffer(state) { // check if an removed node in first was added in second -> update // remove -> add is fine for the store - log('Consolidated delta buffer', 'add', size(toAdd), 'update', - size(toUpdate), 'remove', size(toRemove)); + log('Consolidated delta buffer', + 'add', size(toAdd), + 'update', size(toUpdate), + 'remove', size(toRemove)); return { add: toAdd.length > 0 ? toAdd : null, @@ -60,7 +58,3 @@ export function consolidatedBeginningOfNodesDeltaBuffer(state) { remove: toRemove.length > 0 ? toRemove : null }; } - -export function getUpdateBufferSize(state) { - return state.get('nodesDeltaBuffer').size; -}