From 903343fbdf197f52d373a9777ba0d605dffdc18a Mon Sep 17 00:00:00 2001 From: Nick Krecklow Date: Mon, 11 May 2020 18:29:26 -0500 Subject: [PATCH] unify TimeTracker code, explicitly mark nullable playerCounts as unsafe --- lib/app.js | 4 ++-- lib/database.js | 26 ++++++-------------------- lib/ping.js | 11 +++++++---- lib/servers.js | 12 +++++++----- lib/time.js | 42 +++++++++++++++++++++++++++++------------- lib/util.js | 11 +++++++++++ 6 files changed, 62 insertions(+), 44 deletions(-) create mode 100644 lib/util.js diff --git a/lib/app.js b/lib/app.js index de640cd..8f8b924 100644 --- a/lib/app.js +++ b/lib/app.js @@ -48,7 +48,7 @@ class App { // any header data being appended by #MessageOf since the graph data is fed // directly into the graphing system client.send(MessageOf('historyGraph', { - timestamps: this.timeTracker.getHistoricalPointsSeconds(), + timestamps: this.timeTracker.getGraphPoints(), graphData })) } @@ -74,7 +74,7 @@ class App { } })(), mojangServices: this.mojangUpdater.getLastUpdate(), - timestampPoints: this.timeTracker.getServerPointsSeconds(), + timestampPoints: this.timeTracker.getServerGraphPoints(), servers: this.serverRegistrations.map(serverRegistration => serverRegistration.getPingHistory()) } diff --git a/lib/database.js b/lib/database.js index e3b3b49..c54bfb7 100644 --- a/lib/database.js +++ b/lib/database.js @@ -1,5 +1,7 @@ const sqlite = require('sqlite3') +const TimeTracker = require('./time') + class Database { constructor (app) { this._app = app @@ -58,23 +60,7 @@ class Database { const serverIp = Object.keys(relativeGraphData)[0] const timestamps = relativeGraphData[serverIp][0] - // This is a copy of ServerRegistration#loadGraphPoints - // relativeGraphData contains original timestamp data and needs to be filtered into minutes - const sharedTimestamps = [] - - let lastTimestamp = startTime - - for (let i = 0; i < timestamps.length; i++) { - const timestamp = timestamps[i] - - if (timestamp - lastTimestamp >= 60 * 1000) { - lastTimestamp = timestamp - - sharedTimestamps.push(timestamp) - } - } - - this._app.timeTracker.loadHistoricalTimestamps(sharedTimestamps) + this._app.timeTracker.loadGraphPoints(startTime, timestamps) } callback() @@ -94,7 +80,7 @@ class Database { this.getRecord(serverRegistration.data.ip, (playerCount, timestamp) => { serverRegistration.recordData = { playerCount, - timestamp: Math.floor(timestamp / 1000) + timestamp: TimeTracker.toSeconds(timestamp) } // Check if completedTasks hit the finish value @@ -119,9 +105,9 @@ class Database { ], (_, data) => callback(data[0]['MAX(playerCount)'], data[0].timestamp)) } - insertPing (ip, timestamp, playerCount) { + insertPing (ip, timestamp, unsafePlayerCount) { const statement = this._sql.prepare('INSERT INTO pings (timestamp, ip, playerCount) VALUES (?, ?, ?)') - statement.run(timestamp, ip, playerCount) + statement.run(timestamp, ip, unsafePlayerCount) statement.finalize() } } diff --git a/lib/ping.js b/lib/ping.js index 00fca63..a91e92a 100644 --- a/lib/ping.js +++ b/lib/ping.js @@ -3,6 +3,9 @@ const minecraftBedrockPing = require('mcpe-ping-fixed') const logger = require('./logger') const MessageOf = require('./message') +const TimeTracker = require('./time') + +const { getPlayerCountOrNull } = require('./util') const config = require('../config') @@ -83,7 +86,7 @@ class PingController { } pingAll = () => { - const { timestamp, updateHistoryGraph } = this._app.timeTracker.newPingTimestamp() + const { timestamp, updateHistoryGraph } = this._app.timeTracker.newPointTimestamp() this.startPingTasks(results => { const updates = [] @@ -94,9 +97,9 @@ class PingController { // Log to database if enabled // Use null to represent a failed ping if (config.logToDatabase) { - const playerCount = result.resp ? result.resp.players.online : null + const unsafePlayerCount = getPlayerCountOrNull(result.resp) - this._app.database.insertPing(serverRegistration.data.ip, timestamp, playerCount) + this._app.database.insertPing(serverRegistration.data.ip, timestamp, unsafePlayerCount) } // Generate a combined update payload @@ -110,7 +113,7 @@ class PingController { // Send object since updates uses serverIds as keys // Send a single timestamp entry since it is shared this._app.server.broadcast(MessageOf('updateServers', { - timestamp: Math.floor(timestamp / 1000), + timestamp: TimeTracker.toSeconds(timestamp), updateHistoryGraph, updates })) diff --git a/lib/servers.js b/lib/servers.js index 58e6b8d..f400368 100644 --- a/lib/servers.js +++ b/lib/servers.js @@ -4,6 +4,8 @@ const dns = require('dns') const TimeTracker = require('./time') const Server = require('./server') +const { getPlayerCountOrNull } = require('./util') + const config = require('../config') const minecraftVersions = require('../minecraft_versions') @@ -23,16 +25,16 @@ class ServerRegistration { handlePing (timestamp, resp, err, version, updateHistoryGraph) { // Use null to represent a failed ping - const playerCount = resp ? resp.players.online : null + const unsafePlayerCount = getPlayerCountOrNull(resp) // Store into in-memory ping data - TimeTracker.pushAndShift(this._pingHistory, playerCount, TimeTracker.getMaxServerGraphDataLength()) + TimeTracker.pushAndShift(this._pingHistory, unsafePlayerCount, TimeTracker.getMaxServerGraphDataLength()) // Only notify the frontend to append to the historical graph // if both the graphing behavior is enabled and the backend agrees // that the ping is eligible for addition if (updateHistoryGraph) { - TimeTracker.pushAndShift(this.graphData, playerCount, TimeTracker.getMaxGraphDataLength()) + TimeTracker.pushAndShift(this.graphData, unsafePlayerCount, TimeTracker.getMaxGraphDataLength()) } // Delegate out update payload generation @@ -51,7 +53,7 @@ class ServerRegistration { if (config.logToDatabase && (!this.recordData || resp.players.online > this.recordData.playerCount)) { this.recordData = { playerCount: resp.players.online, - timestamp: Math.floor(timestamp / 1000) + timestamp: TimeTracker.toSeconds(timestamp) } // Append an updated recordData @@ -160,7 +162,7 @@ class ServerRegistration { } return { playerCount: this.graphData[this._graphPeakIndex], - timestamp: this._app.timeTracker.getHistoricalPointSeconds(this._graphPeakIndex) + timestamp: this._app.timeTracker.getGraphPointAt(this._graphPeakIndex) } } diff --git a/lib/time.js b/lib/time.js index 34c8afd..b515eca 100644 --- a/lib/time.js +++ b/lib/time.js @@ -3,14 +3,14 @@ const config = require('../config.json') class TimeTracker { constructor (app) { this._app = app - this._points = [] - this._historicalTimestamps = [] + this._serverGraphPoints = [] + this._graphPoints = [] } - newPingTimestamp () { + newPointTimestamp () { const timestamp = new Date().getTime() - TimeTracker.pushAndShift(this._points, timestamp, TimeTracker.getMaxServerGraphDataLength()) + TimeTracker.pushAndShift(this._serverGraphPoints, timestamp, TimeTracker.getMaxServerGraphDataLength()) // Flag each group as history graph additions each minute // This is sent to the frontend for graph updates @@ -20,7 +20,7 @@ class TimeTracker { this._lastHistoryGraphUpdate = timestamp // Push into timestamps array to update backend state - TimeTracker.pushAndShift(this._historicalTimestamps, timestamp, TimeTracker.getMaxGraphDataLength()) + TimeTracker.pushAndShift(this._graphPoints, timestamp, TimeTracker.getMaxGraphDataLength()) } return { @@ -29,20 +29,36 @@ class TimeTracker { } } - loadHistoricalTimestamps (timestamps) { - this._historicalTimestamps = timestamps + loadGraphPoints (startTime, timestamps) { + // This is a copy of ServerRegistration#loadGraphPoints + // relativeGraphData contains original timestamp data and needs to be filtered into minutes + let lastTimestamp = startTime + + for (let i = 0; i < timestamps.length; i++) { + const timestamp = timestamps[i] + + if (timestamp - lastTimestamp >= 60 * 1000) { + lastTimestamp = timestamp + + this._graphPoints.push(timestamp) + } + } } - getHistoricalPointsSeconds () { - return this._historicalTimestamps.map(value => Math.floor(value / 1000)) + getGraphPointAt (i) { + return TimeTracker.toSeconds(this._graphPoints[i]) } - getHistoricalPointSeconds (index) { - return Math.floor(this._historicalTimestamps[index] / 1000) + getServerGraphPoints () { + return this._serverGraphPoints.map(TimeTracker.toSeconds) } - getServerPointsSeconds () { - return this._points.map(value => Math.floor(value / 1000)) + getGraphPoints () { + return this._graphPoints.map(TimeTracker.toSeconds) + } + + static toSeconds = (timestamp) => { + return Math.floor(timestamp / 1000) } static getMaxServerGraphDataLength () { diff --git a/lib/util.js b/lib/util.js new file mode 100644 index 0000000..d603481 --- /dev/null +++ b/lib/util.js @@ -0,0 +1,11 @@ +function getPlayerCountOrNull (resp) { + if (resp) { + return resp.players.online + } else { + return null + } +} + +module.exports = { + getPlayerCountOrNull +}