Merge pull request #156 from Cryptkeeper/use-serverids

release 5.2.0

assets/js/app.js

@@ -99,7 +99,7 @@ export class App {
     // error = defined with "Waiting" description
     // info = safely defined with configured data
     const latestPing = pings[pings.length - 1]
-    const serverRegistration = this.serverRegistry.createServerRegistration(latestPing.info.name)
+    const serverRegistration = this.serverRegistry.createServerRegistration(latestPing.serverId)
 
     serverRegistration.initServerStatus(latestPing)
 

assets/js/graph.js

@@ -148,11 +148,7 @@ export class GraphDisplayManager {
       this.loadLocalStorage()
     }
 
-    // Remap the incoming data from being string (serverName) keyed into serverId keys
+    this._graphData = graphData
-    for (const serverName of Object.keys(graphData)) {
-      const serverRegistration = this._app.serverRegistry.getServerRegistration(serverName)
-      this._graphData[serverRegistration.serverId] = graphData[serverName]
-    }
 
     // Explicitly define a height so flot.js can rescale the Y axis
     document.getElementById('big-graph').style.height = '400px'

assets/js/main.js

@@ -34,7 +34,10 @@ document.addEventListener('DOMContentLoaded', function () {
     let lastRowCounter = 0
     let controlsHTML = ''
 
-    Object.keys(data).sort().forEach(function (serverName) {
+    app.serverRegistry.getServerRegistrations()
+      .map(serverRegistration => serverRegistration.data.name)
+      .sort()
+      .forEach(serverName => {
         const serverRegistration = app.serverRegistry.getServerRegistration(serverName)
 
         controlsHTML += '<td>' +
@@ -59,25 +62,6 @@ document.addEventListener('DOMContentLoaded', function () {
     app.graphDisplayManager.initEventListeners()
   })
 
-  socket.on('updateHistoryGraph', function (data) {
-    // Skip any incoming updates if the graph is disabled
-    // The backend shouldn't send these anyways
-    if (!app.graphDisplayManager.isVisible) {
-      return
-    }
-
-    const serverRegistration = app.serverRegistry.getServerRegistration(data.name)
-
-    if (serverRegistration) {
-      app.graphDisplayManager.addGraphPoint(serverRegistration.serverId, data.timestamp, data.playerCount)
-
-      // Only redraw the graph if not mutating hidden data
-      if (serverRegistration.isVisible) {
-        app.graphDisplayManager.requestRedraw()
-      }
-    }
-  })
-
   socket.on('add', function (data) {
     data.forEach(app.addServer)
   })
@@ -86,11 +70,25 @@ document.addEventListener('DOMContentLoaded', function () {
     // The backend may send "update" events prior to receiving all "add" events
     // A server has only been added once it's ServerRegistration is defined
     // Checking undefined protects from this race condition
-    const serverRegistration = app.serverRegistry.getServerRegistration(data.info.name)
+    const serverRegistration = app.serverRegistry.getServerRegistration(data.serverId)
 
     if (serverRegistration) {
       serverRegistration.updateServerStatus(data, false, app.publicConfig.minecraftVersions)
     }
+
+    // Use update payloads to conditionally append data to graph
+    // Skip any incoming updates if the graph is disabled
+    if (data.updateHistoryGraph && app.graphDisplayManager.isVisible) {
+      // Update may not be successful, safely append 0 points
+      const playerCount = data.result ? data.result.players.online : 0
+
+      app.graphDisplayManager.addGraphPoint(serverRegistration.serverId, data.timestamp, playerCount)
+
+      // Only redraw the graph if not mutating hidden data
+      if (serverRegistration.isVisible) {
+        app.graphDisplayManager.requestRedraw()
+      }
+    }
   })
 
   socket.on('updateMojangServices', function (data) {
@@ -121,26 +119,6 @@ document.addEventListener('DOMContentLoaded', function () {
     app.handleSyncComplete()
   })
 
-  socket.on('updatePeak', function (data) {
-    const serverRegistration = app.serverRegistry.getServerRegistration(data.name)
-
-    if (serverRegistration) {
-      serverRegistration.updateServerPeak(data.timestamp, data.playerCount)
-    }
-  })
-
-  socket.on('peaks', function (data) {
-    Object.keys(data).forEach(function (serverName) {
-      const serverRegistration = app.serverRegistry.getServerRegistration(serverName)
-
-      if (serverRegistration) {
-        const graphPeak = data[serverName]
-
-        serverRegistration.updateServerPeak(graphPeak.timestamp, graphPeak.playerCount)
-      }
-    })
-  })
-
   window.addEventListener('resize', function () {
     app.percentageBar.redraw()
 

assets/js/servers.js

@@ -1,4 +1,4 @@
-import { formatNumber, formatTimestamp, formatDate, formatMinecraftServerAddress, formatMinecraftVersions, isArrayEqual, isObjectEqual } from './util'
+import { formatNumber, formatTimestamp, formatDate, formatMinecraftServerAddress, formatMinecraftVersions } from './util'
 
 import MISSING_FAVICON from '../images/missing_favicon.svg'
 
@@ -48,8 +48,7 @@ export class ServerRegistry {
     }
   }
 
-  createServerRegistration (serverName) {
+  createServerRegistration (serverId) {
-    const serverId = this._serverIdsByName[serverName]
     const serverData = this._serverDataById[serverId]
     const serverRegistration = new ServerRegistration(this._app, serverId, serverData)
     this._registeredServers[serverId] = serverRegistration
@@ -65,23 +64,6 @@ export class ServerRegistry {
     }
   }
 
-  getServerRankBy (serverRegistration, x, sort) {
-    const records = Object.values(this._registeredServers)
-      .map(x)
-      .filter(val => val !== undefined)
-
-    // Invalidate any results that do not account for all serverRegistrations
-    if (records.length === this._registeredServers.length) {
-      records.sort(sort)
-
-      // Pull matching data from target serverRegistration
-      // Assume indexOf cannot be -1 or val undefined since they have been pre-tested in the map call above
-      const val = x(serverRegistration)
-      const indexOf = records.indexOf(val)
-      return indexOf + 1
-    }
-  }
-
   getServerRegistrations = () => Object.values(this._registeredServers)
 
   reset () {
@@ -102,7 +84,6 @@ export class ServerRegistration {
   isFavorite = false
   rankIndex
   lastRecordData
-  lastVersions = []
   lastPeakData
 
   constructor (app, serverId, data) {
@@ -139,7 +120,7 @@ export class ServerRegistration {
       if (pushToGraph) {
         // Only update graph for successful pings
         // This intentionally pauses the server graph when pings begin to fail
-        this._graphData.push([payload.info.timestamp, this.playerCount])
+        this._graphData.push([payload.timestamp, this.playerCount])
 
         // Trim graphData to within the max length by shifting out the leading elements
         if (this._graphData.length > SERVER_GRAPH_DATA_MAX_LENGTH) {
@@ -174,7 +155,7 @@ export class ServerRegistration {
     document.getElementById('ranking_' + this.serverId).innerText = '#' + (rankIndex + 1)
   }
 
-  updateServerPeak (time, playerCount) {
+  updateServerPeak (data) {
     const peakLabelElement = document.getElementById('peak_' + this.serverId)
 
     // Always set label once any peak data has been received
@@ -182,13 +163,10 @@ export class ServerRegistration {
 
     const peakValueElement = document.getElementById('peak-value_' + this.serverId)
 
-    peakValueElement.innerText = formatNumber(playerCount)
+    peakValueElement.innerText = formatNumber(data.playerCount)
-    peakLabelElement.title = 'At ' + formatTimestamp(time)
+    peakLabelElement.title = 'At ' + formatTimestamp(data.timestamp)
 
-    this.lastPeakData = {
+    this.lastPeakData = data
-      timestamp: time,
-      playerCount: playerCount
-    }
   }
 
   updateServerStatus (ping, isInitialUpdate, minecraftVersions) {
@@ -196,21 +174,14 @@ export class ServerRegistration {
     // Otherwise the ping value is pushed into the graphData when already present
     this.handlePing(ping, !isInitialUpdate)
 
-    // Compare against a cached value to avoid empty updates
+    if (ping.versions) {
-    // Allow undefined ping.versions inside the if statement for text reset handling
-    if (ping.versions && !isArrayEqual(ping.versions, this.lastVersions)) {
-      this.lastVersions = ping.versions
-
       const versionsElement = document.getElementById('version_' + this.serverId)
 
       versionsElement.style.display = 'block'
       versionsElement.innerText = formatMinecraftVersions(ping.versions, minecraftVersions[this.data.type]) || ''
     }
 
-    // Compare against a cached value to avoid empty updates
+    if (ping.recordData) {
-    if (ping.recordData !== undefined && !isObjectEqual(ping.recordData, this.lastRecordData, ['playerCount', 'timestamp'])) {
-      this.lastRecordData = ping.recordData
-
       // Always set label once any record data has been received
       const recordLabelElement = document.getElementById('record_' + this.serverId)
 
@@ -227,6 +198,12 @@ export class ServerRegistration {
       } else {
         recordValueElement.innerText = formatNumber(recordData.playerCount)
       }
+
+      this.lastRecordData = recordData
+    }
+
+    if (ping.graphPeakData) {
+      this.updateServerPeak(ping.graphPeakData)
     }
 
     const playerCountLabelElement = document.getElementById('player-count_' + this.serverId)

assets/js/util.js

@@ -135,20 +135,6 @@ export function isArrayEqual (a, b) {
   return true
 }
 
-export function isObjectEqual (a, b, props) {
-  if (typeof a === 'undefined' || typeof a !== typeof b) {
-    return false
-  }
-  for (let i = 0; i < props.length; i++) {
-    const prop = props[i]
-
-    if (typeof a[prop] === 'undefined' || typeof a[prop] !== typeof b[prop] || a[prop] !== b[prop]) {
-      return false
-    }
-  }
-  return true
-}
-
 // From http://detectmobilebrowsers.com/
 export function isMobileBrowser () {
   var check = false;

docs/CHANGELOG.md

@@ -1,3 +1,8 @@
+**5.2.0** *(Apr 29 2020)*
+- Updated protocol to use serverIds instead of string names. This will reduce wasted bandwidth when pushing updates.
+- Removed "updatePeak", "peaks" and "updateHistoryGraph" socket events. Their behavior has been optimized and merged into "update".
+- Removed various legacy code.
+
 **5.1.2** *(Apr 22 2020)*
 - Fixes the historical graph overflowing the maximum graphDuration value.
 

lib/app.js

@@ -39,24 +39,11 @@ class App {
       client.on('requestHistoryGraph', () => {
         // Send historical graphData built from all serverRegistrations
         const graphData = {}
-        const graphPeaks = {}
 
         this.serverRegistrations.forEach((serverRegistration) => {
-          graphData[serverRegistration.data.name] = serverRegistration.graphData
+          graphData[serverRegistration.serverId] = serverRegistration.graphData
-
-          // Send current peak, if any
-          const graphPeak = serverRegistration.getGraphPeak()
-          if (graphPeak) {
-            graphPeaks[serverRegistration.data.name] = graphPeak
-          }
         })
 
-        // Send current peaks, if any
-        // Emit peaks first since graphData may take a while to receive
-        if (Object.keys(graphPeaks).length > 0) {
-          client.emit('peaks', graphPeaks)
-        }
-
         client.emit('historyGraph', graphData)
       })
     }

lib/ping.js

@@ -107,10 +107,10 @@ class PingController {
   handlePing (serverRegistration, resp, err, version) {
     const timestamp = new Date().getTime()
 
-    this._app.server.broadcast('update', serverRegistration.getUpdate(timestamp, resp, err, version))
-
     serverRegistration.addPing(timestamp, resp)
 
+    let updateHistoryGraph = false
+
     if (config.logToDatabase) {
       const playerCount = resp ? resp.players.online : 0
 
@@ -118,25 +118,14 @@ class PingController {
       this._app.database.insertPing(serverRegistration.data.ip, timestamp, playerCount)
 
       if (serverRegistration.addGraphPoint(resp !== undefined, playerCount, timestamp)) {
-        this._app.server.broadcast('updateHistoryGraph', {
+        updateHistoryGraph = true
-          name: serverRegistration.data.name,
+      }
-          playerCount: playerCount,
-          timestamp: timestamp
-        })
     }
 
-      // Update calculated graph peak regardless if the graph is being updated
+    // Generate a combined update payload
-      // This can cause a (harmless) desync between live and stored data, but it allows it to be more accurate for long surviving processes
+    // This includes any modified fields and flags used by the frontend
-      if (serverRegistration.findNewGraphPeak()) {
+    // This will not be cached and can contain live metadata
-        const graphPeak = serverRegistration.getGraphPeak()
+    this._app.server.broadcast('update', serverRegistration.getUpdate(timestamp, resp, err, version, updateHistoryGraph))
-
-        this._app.server.broadcast('updatePeak', {
-          name: serverRegistration.data.name,
-          playerCount: graphPeak.playerCount,
-          timestamp: graphPeak.timestamp
-        })
-      }
-    }
   }
 }
 

lib/servers.js

@@ -2,23 +2,23 @@ const config = require('../config')
 const minecraftVersions = require('../minecraft_versions')
 
 class ServerRegistration {
+  serverId
   lastFavicon
   versions = []
   recordData
   graphData = []
 
-  constructor (data) {
+  constructor (serverId, data) {
+    this.serverId = serverId
     this.data = data
     this._pingHistory = []
   }
 
-  getUpdate (timestamp, resp, err, version) {
+  getUpdate (timestamp, resp, err, version, updateHistoryGraph) {
     const update = {
-      info: {
+      serverId: this.serverId,
-        name: this.data.name,
       timestamp: timestamp
     }
-    }
 
     if (resp) {
       if (resp.version && this.updateProtocolVersionCompat(resp.version, version.protocolId, version.protocolIndex)) {
@@ -48,6 +48,20 @@ class ServerRegistration {
       update.result = {
         players: resp.players
       }
+
+      if (config.logToDatabase) {
+        // Update calculated graph peak regardless if the graph is being updated
+        // This can cause a (harmless) desync between live and stored data, but it allows it to be more accurate for long surviving processes
+        if (this.findNewGraphPeak()) {
+          update.graphPeakData = this.getGraphPeak()
+        }
+
+        // Handled inside logToDatabase to validate logic from #getUpdate call
+        // Only append when true since an undefined value == false
+        if (updateHistoryGraph) {
+          update.updateHistoryGraph = true
+        }
+      }
     } else if (err) {
       // Append a filtered copy of err
       // This ensures any unintended data is not leaked
@@ -94,15 +108,21 @@ class ServerRegistration {
       const lastPing = this._pingHistory[this._pingHistory.length - 1]
 
       const payload = {
-        info: {
+        serverId: this.serverId,
-          name: this.data.name
-        },
         timestamp: lastPing.timestamp,
         versions: this.versions,
         recordData: this.recordData,
         favicon: this.lastFavicon
       }
 
+      // Only append graphPeakData if defined
+      // The value is lazy computed and conditional that config->logToDatabase == true
+      const graphPeakData = this.getGraphPeak()
+
+      if (graphPeakData) {
+        payload.graphPeakData = graphPeakData
+      }
+
       // Conditionally append to avoid defining fields with undefined values
       if (lastPing.result) {
         payload.result = lastPing.result
@@ -118,15 +138,14 @@ class ServerRegistration {
     }
 
     return [{
+      serverId: this.serverId,
+      timestamp: new Date().getTime(),
       error: {
         message: 'Waiting...',
         placeholder: true
       },
-      timestamp: new Date().getTime(),
+      recordData: this.recordData,
-      info: {
+      graphPeakData: this.getGraphPeak()
-        name: this.data.name
-      },
-      recordData: this.recordData
     }]
   }
 

main.js

@@ -8,7 +8,7 @@ const servers = require('./servers')
 
 const app = new App()
 
-servers.forEach(server => {
+servers.forEach((server, serverId) => {
   // Assign a generated color for each servers.json entry if not manually defined
   // These will be passed to the frontend for use in rendering
   if (!server.color) {
@@ -22,7 +22,7 @@ servers.forEach(server => {
   }
 
   // Init a ServerRegistration instance of each entry in servers.json
-  app.serverRegistrations.push(new ServerRegistration(server))
+  app.serverRegistrations.push(new ServerRegistration(serverId, server))
 })
 
 if (!config.logToDatabase) {

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "minetrack",
-  "version": "5.1.2",
+  "version": "5.2.0",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {

package.json

@@ -1,6 +1,6 @@
 {
   "name": "minetrack",
-  "version": "5.1.2",
+  "version": "5.2.0",
   "description": "A Minecraft server tracker that lets you focus on the basics.",
   "main": "main.js",
   "dependencies": {