unify TimeTracker code, explicitly mark nullable playerCounts as unsafe

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())
     }
 

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
+        this._app.timeTracker.loadGraphPoints(startTime, timestamps)
-        // 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)
       }
 
       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()
   }
 }

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
       }))

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)
     }
   }
 

lib/time.js

@@ -3,14 +3,14 @@ const config = require('../config.json')
 class TimeTracker {
   constructor (app) {
     this._app = app
-    this._points = []
+    this._serverGraphPoints = []
-    this._historicalTimestamps = []
+    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) {
+  loadGraphPoints (startTime, timestamps) {
-    this._historicalTimestamps = 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 () {
+  getGraphPointAt (i) {
-    return this._historicalTimestamps.map(value => Math.floor(value / 1000))
+    return TimeTracker.toSeconds(this._graphPoints[i])
   }
 
-  getHistoricalPointSeconds (index) {
+  getServerGraphPoints () {
-    return Math.floor(this._historicalTimestamps[index] / 1000)
+    return this._serverGraphPoints.map(TimeTracker.toSeconds)
   }
 
-  getServerPointsSeconds () {
+  getGraphPoints () {
-    return this._points.map(value => Math.floor(value / 1000))
+    return this._graphPoints.map(TimeTracker.toSeconds)
+  }
+
+  static toSeconds = (timestamp) => {
+    return Math.floor(timestamp / 1000)
   }
 
   static getMaxServerGraphDataLength () {

lib/util.js

@@ -0,0 +1,11 @@
+function getPlayerCountOrNull (resp) {
+  if (resp) {
+    return resp.players.online
+  } else {
+    return null
+  }
+}
+
+module.exports = {
+  getPlayerCountOrNull
+}