Add test for user merging

packages/controller/src/UserManager.ts

@@ -58,7 +58,7 @@ export default class UserManager {
 				lib.logger.warn(
 					`A total of ${duplicates} users were merged, a backup was written to: ${backupPath}`
 				);
-				await lib.safeOutputFile(backupPath, content);
+				await lib.safeOutputFile(backupPath, JSON.stringify(content, null, "\t"));
 			}
 
 		} catch (err: any) {

packages/lib/src/data/User.ts

@@ -165,7 +165,7 @@ export default class User {
 		this.isAdmin = this.isAdmin && otherUser.isAdmin; // More secure to use && rather ||
 		this.isBanned = this.isBanned || otherUser.isBanned;
 		this.isBanned = this.isWhitelisted || otherUser.isWhitelisted;
-		this.banReason = this.banReason.length > otherUser.banReason.length ? this.banReason : otherUser.banReason;
+		this.banReason = this.updatedAtMs > otherUser.updatedAtMs ? this.banReason : otherUser.banReason;
 		this.updatedAtMs = Date.now();
 
 		// Merge instance stats

test/controller/UserManager.js

@@ -1,4 +1,5 @@
 "use strict";
+const fs = require("fs-extra");
 const assert = require("assert").strict;
 const { PlayerStats } = require("@clusterio/lib");
 const { ControllerUser, UserManager } = require("@clusterio/controller");
@@ -36,8 +37,8 @@ describe("controller/src/UserManager", function() {
 			it("should remove the instance stats from existing users", function() {
 				let user1 = ControllerUser.fromJSON({ name: "admin", roles: [1] });
 				let user2 = ControllerUser.fromJSON({ name: "player", roles: [1] });
-				userManager.users.set(user1.name, user1);
-				userManager.users.set(user2.name, user2);
+				userManager.users.set(user1.id, user1);
+				userManager.users.set(user2.id, user2);
 				user1.instanceStats.set(10, new PlayerStats({ join_count: 1 }));
 				user1.instanceStats.set(11, new PlayerStats({ join_count: 6 }));
 				user1.recalculatePlayerStats();
@@ -55,5 +56,66 @@ describe("controller/src/UserManager", function() {
 				assert.equal(user2.playerStats.joinCount, 3);
 			});
 		});
+		describe(".load()", function() {
+			it("should merge users on load", async function() {
+				// Create a user db file containing duplicate user ids
+				const names = ["user1", "User1", "user3"];
+				names.forEach(name => {
+					const user = ControllerUser.fromJSON({ name: name, roles: [1] });
+					userManager.users.set(user.name, user);
+				});
+
+				// Setup the player stats
+				const user1Before = userManager.users.get("user1");
+				user1Before.instanceStats.set(9, new PlayerStats({ join_count: 3 }));
+				user1Before.instanceStats.set(10, new PlayerStats({ join_count: 5 }));
+				user1Before.banReason = "ban reason here";
+				user1Before.isWhitelisted = true;
+				user1Before.isBanned = true;
+				user1Before.isAdmin = true;
+				user1Before.updatedAtMs = 100;
+
+				const user2Before = userManager.users.get("User1");
+				user2Before.instanceStats.set(10, new PlayerStats({ join_count: 7 }));
+				user2Before.instanceStats.set(11, new PlayerStats({ join_count: 9 }));
+				user2Before.isWhitelisted = false;
+				user2Before.isBanned = false;
+				user2Before.isAdmin = false;
+				user1Before.updatedAtMs = 200;
+
+				const user3Before = userManager.users.get("user3");
+				user3Before.instanceStats.set(10, new PlayerStats({ join_count: 11 }));
+				user3Before.instanceStats.set(11, new PlayerStats({ join_count: 13 }));
+				user3Before.isWhitelisted = false;
+				user3Before.isAdmin = true;
+
+				// Save and reload the user data
+				await fs.emptyDir("./temp/test/user_manager");
+				await userManager.save("./temp/test/user_manager/load.json");
+				names.map(name => userManager.users.delete(name));
+				await userManager.load("./temp/test/user_manager/load.json");
+
+				// Check it was loaded correctly
+				const user1After = userManager.users.get("user1");
+				const user2After = userManager.users.get("User1");
+				const user3After = userManager.users.get("user3");
+				assert.notEqual(user1After, undefined, "'user1' was not loaded");
+				assert.equal(user2After, undefined, "'User1' was loaded");
+				assert.notEqual(user3After, undefined, "'user3' was not loaded");
+
+				assert.equal(user1After.isAdmin, false, "User 1 is admin");
+				assert.equal(user1After.isBanned, true, "User 1 is not banned");
+				assert.equal(user1After.isWhitelisted, true, "User 1 is not whitelisted");
+				assert.equal(user1After.banReason, "ban reason here", "User 1 ban reason is wrong");
+				assert.equal(user1After.instanceStats.get(9)?.joinCount, 3, "User 1 instance 9 stats are wrong");
+				assert.equal(user1After.instanceStats.get(10)?.joinCount, 12, "User 1 instance 10 stats are wrong");
+				assert.equal(user1After.instanceStats.get(11)?.joinCount, 9, "User 1 instance 11 stats are wrong");
+
+				assert.equal(user3After.isAdmin, true, "User 3 is not admin");
+				assert.equal(user3After.isWhitelisted, false, "User 3 is whitelisted");
+				assert.equal(user3After.instanceStats.get(10)?.joinCount, 11, "User 3 instance 10 stats are wrong");
+				assert.equal(user3After.instanceStats.get(11)?.joinCount, 13, "User 3 instance 11 stats are wrong");
+			});
+		});
 	});
 });