From 2e5333af29384a0572d636e6f0f2be15065dc517 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 7 Dec 2020 18:49:25 +0100 Subject: [PATCH] Fix banlist sync being broken Fix the logic for determining which entries in the banlist changed being incorrect and causing the banlist sync to be broken. Add unit tests for the syncUserListsEventHandler to ensure it functions as exxpected. --- packages/slave/slave.js | 11 ++-- plugins/statistics_exporter/instance.js | 15 ++++- test/slave.js | 81 +++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 9 deletions(-) diff --git a/packages/slave/slave.js b/packages/slave/slave.js index 47cfb3bb..b717228e 100755 --- a/packages/slave/slave.js +++ b/packages/slave/slave.js @@ -1059,8 +1059,8 @@ class Slave extends libLink.Link { let updateList = (list, updatedList, prop, event) => { let added = new Set(updatedList); let removed = new Set(list); - list.forEach(Set.prototype.delete.bind(added)); - updatedList.forEach(Set.prototype.delete.bind(removed)); + list.forEach(el => added.delete(el)); + updatedList.forEach(el => removed.delete(el)); for (let name of added) { list.add(name); @@ -1080,15 +1080,14 @@ class Slave extends libLink.Link { updateList(this.adminlist, message.data.adminlist, "admin", libLink.messages.adminlistUpdate); updateList(this.whitelist, message.data.whitelist, "whitelisted", libLink.messages.whitelistUpdate); - let updatedBanlist = new Map(message.data.banlist); - let addedOrChanged = new Map(updatedBanlist); + let addedOrChanged = new Map(message.data.banlist); let removed = new Set(this.banlist.keys()); - removed.forEach((name, reason) => { + addedOrChanged.forEach((_, name) => removed.delete(name)); + this.banlist.forEach((reason, name) => { if (addedOrChanged.get(name) === reason) { addedOrChanged.delete(name); } }); - updatedBanlist.forEach(Set.prototype.delete.bind(removed)); for (let [name, reason] of addedOrChanged) { this.banlist.set(name, reason); diff --git a/plugins/statistics_exporter/instance.js b/plugins/statistics_exporter/instance.js index 002e51f6..001d96b6 100644 --- a/plugins/statistics_exporter/instance.js +++ b/plugins/statistics_exporter/instance.js @@ -21,6 +21,17 @@ const instanceGameFlowStatistics = new Gauge( { labels: ["instance_id", "statistic", "direction", "name"] }, ); +function setForceFlowStatistic(instanceId, forceName, statisticName, direction, item, value) { + instanceForceFlowStatistics.labels(String(instanceId), forceName, statisticName, direction, item).set(value); + + // For item and fluid statistics it's useful to compare the input flow with the + // output flow, to simplify the comparison ensure both directions have a value. + if (["item_production_statistics", "fluid_production_statistic"].includes(statisticName)) { + let reverseDirection = direction === "input" ? "output" : "input"; + instanceForceFlowStatistics.labels(String(instanceId), forceName, statisticName, reverseDirection, item); + } +} + class InstancePlugin extends libPlugin.BaseInstancePlugin { async init() { @@ -46,9 +57,7 @@ class InstancePlugin extends libPlugin.BaseInstancePlugin { for (let [statisticName, statistic] of Object.entries(flowStatistics)) { for (let [direction, counts] of Object.entries(statistic)) { for (let [item, value] of Object.entries(counts)) { - instanceForceFlowStatistics.labels( - String(instanceId), forceName, statisticName, direction, item - ).set(value); + setForceFlowStatistic(instanceId, forceName, statisticName, direction, item, value); } } } diff --git a/test/slave.js b/test/slave.js index 7e0801e1..f436b312 100644 --- a/test/slave.js +++ b/test/slave.js @@ -162,4 +162,85 @@ describe("Slave testing", function() { ])); }); }); + + describe("class Slave", function() { + describe(".syncUserListsEventHandler()", function() { + let mockSlave; + beforeEach(function() { + mockSlave = { + adminlist: new Set(), + whitelist: new Set(), + banlist: new Map(), + broadcasts: [], + broadcastEventToInstance(message, event) { + this.broadcasts.push(message["data"]); + }, + syncUserListsEventHandler: slave._Slave.prototype.syncUserListsEventHandler, + syncLists(adminlist, banlist, whitelist) { + return this.syncUserListsEventHandler({ "data": { + "adminlist": adminlist, + "banlist": banlist, + "whitelist": whitelist, + }}); + }, + }; + }); + + it("should broadcast new entries to adminlist", async function() { + await mockSlave.syncLists(["admin1"], [], []); + await mockSlave.syncLists(["admin1", "admin2"], [], []); + + assert.deepEqual(mockSlave.broadcasts, [ + { "name": "admin1", "admin": true }, + { "name": "admin2", "admin": true }, + ]); + }); + + it("should broadcast removals from adminlist", async function() { + mockSlave.adminlist.add("admin1").add("admin2"); + await mockSlave.syncLists(["admin1"], [], []); + assert.deepEqual(mockSlave.broadcasts, [ + { "name": "admin2", "admin": false }, + ]); + }); + + it("should broadcast new entries to whitelist", async function() { + await mockSlave.syncLists([], [], ["player1"]); + await mockSlave.syncLists([], [], ["player1", "player2"]); + + assert.deepEqual(mockSlave.broadcasts, [ + { "name": "player1", "whitelisted": true }, + { "name": "player2", "whitelisted": true }, + ]); + }); + + it("should broadcast removals from whitelist", async function() { + mockSlave.whitelist.add("player1").add("player2"); + await mockSlave.syncLists([], [], ["player1"]); + + assert.deepEqual(mockSlave.broadcasts, [ + { "name": "player2", "whitelisted": false }, + ]); + }); + + it("should broadcast new entries to banlist", async function() { + await mockSlave.syncLists([], [["badie1", "greifing"]], []); + await mockSlave.syncLists([], [["badie1", "greifing"], ["badie2", "annoying"]], []); + + assert.deepEqual(mockSlave.broadcasts, [ + { "name": "badie1", "banned": true, "reason": "greifing" }, + { "name": "badie2", "banned": true, "reason": "annoying" }, + ]); + }); + + it("should broadcast removals to banlist", async function() { + mockSlave.banlist.set("badie1", "greifing").set("badie2", "annoying"); + await mockSlave.syncLists([], [["badie1", "greifing"]], []); + + assert.deepEqual(mockSlave.broadcasts, [ + { "name": "badie2", "banned": false, "reason": "" }, + ]); + }); + }); + }); });