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

@@ -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);

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

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": "" },
+				]);
+			});
+		});
+	});
 });