From d80385c947f9a597d9bda3e240e879f656fb822f Mon Sep 17 00:00:00 2001 From: Cooldude2606 <25043174+Cooldude2606@users.noreply.github.com> Date: Mon, 11 Nov 2024 23:15:53 +0000 Subject: [PATCH] Add test for user merging --- packages/controller/src/UserManager.ts | 2 +- packages/lib/src/data/User.ts | 2 +- test/controller/UserManager.js | 66 +++++++++++++++++++++++++- 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/packages/controller/src/UserManager.ts b/packages/controller/src/UserManager.ts index 10d5787f..9d05b538 100644 --- a/packages/controller/src/UserManager.ts +++ b/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) { diff --git a/packages/lib/src/data/User.ts b/packages/lib/src/data/User.ts index 2aa63766..641c3cab 100644 --- a/packages/lib/src/data/User.ts +++ b/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 diff --git a/test/controller/UserManager.js b/test/controller/UserManager.js index 392b3669..da01ef92 100644 --- a/test/controller/UserManager.js +++ b/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"); + }); + }); }); });