From db562ce6a05bdcb04794117eb837ff3e14d433ed Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Wed, 6 Jul 2022 15:07:10 +0200 Subject: [PATCH 1/8] Provide reason for player leaving Expose the reason for why a player is leaving a server in playerEvent. --- packages/lib/link/messages.js | 2 ++ packages/lib/plugin.js | 6 ++++++ packages/slave/modules/clusterio/impl.lua | 11 ++++++++++- packages/slave/src/Instance.js | 18 ++++++++++++++---- test/slave/Instance.js | 10 +++++----- 5 files changed, 37 insertions(+), 10 deletions(-) diff --git a/packages/lib/link/messages.js b/packages/lib/link/messages.js index dd187266..01f75da0 100644 --- a/packages/lib/link/messages.js +++ b/packages/lib/link/messages.js @@ -1284,10 +1284,12 @@ messages.playerEvent = new Event({ type: "player_event", links: ["instance-slave", "slave-master"], forwardTo: "master", + eventRequired: ["instance_id", "type", "name"], eventProperties: { "instance_id": { type: "integer" }, "type": { type: "string", enum: ["join", "leave"] }, "name": { type: "string" }, + "reason": { type: "string" }, }, }); diff --git a/packages/lib/plugin.js b/packages/lib/plugin.js index b9810254..23499e4b 100644 --- a/packages/lib/plugin.js +++ b/packages/lib/plugin.js @@ -208,6 +208,9 @@ class BaseInstancePlugin { * @param {Object} event - Information about the event. * @param {string} event.type - Either "join" or "leave". * @param {string} event.name - Name of the player that joined/left. + * @param {string=} event.reason - + * Only present for type "leave". Reason for player leaving the + * game, one of the possible reasons in defines.disconnect_reason. */ async onPlayerEvent(event) { } @@ -500,6 +503,9 @@ class BaseMasterPlugin { * @param {Object} event - Information about the event. * @param {string} event.type - Either "join" or "leave". * @param {string} event.name - Name of the player that joined/left. + * @param {string=} event.reason - + * Only present for type "leave". Reason for player leaving the + * game, one of the possible reasons in defines.disconnect_reason. */ async onPlayerEvent(instance, event) { } diff --git a/packages/slave/modules/clusterio/impl.lua b/packages/slave/modules/clusterio/impl.lua index cdb1d68e..acce50ee 100644 --- a/packages/slave/modules/clusterio/impl.lua +++ b/packages/slave/modules/clusterio/impl.lua @@ -28,9 +28,18 @@ impl.events[defines.events.on_player_joined_game] = function(event) api.send_json("player_event", { type = "join", name = player.name }) end +local disconnect_reason_name = {} +for name, i in pairs(defines.disconnect_reason) do + disconnect_reason_name[i] = name +end + impl.events[defines.events.on_player_left_game] = function(event) local player = game.players[event.player_index] - api.send_json("player_event", { type = "leave", name = player.name }) + api.send_json("player_event", { + type = "leave", + name = player.name, + reason = disconnect_reason_name[event.reason], + }) end -- Internal API diff --git a/packages/slave/src/Instance.js b/packages/slave/src/Instance.js index 062a7017..9207cad9 100644 --- a/packages/slave/src/Instance.js +++ b/packages/slave/src/Instance.js @@ -250,7 +250,7 @@ class Instance extends libLink.Link { if (event.type === "join") { this._recordPlayerJoin(event.name); } else if (event.type === "leave") { - this._recordPlayerLeave(event.name); + this._recordPlayerLeave(event.name, event.reason); } }); @@ -264,6 +264,9 @@ class Instance extends libLink.Link { * Unix timestamp in ms the player was last seen leaving this * instance. Not present if player has yet to leave after first * time joining. + * @property {string=} lastLeaveReason - + * Reason the player was last seen leaving with. Not present if + * player has yet to leave after first time joining. * @property {number} joinCount - * Count of the number of times this player has been seen * joining this server. @@ -297,7 +300,12 @@ class Instance extends libLink.Link { if (parsed.action === "JOIN") { this._recordPlayerJoin(name); } else if (["LEAVE", "KICK", "BAN"].includes(parsed.action)) { - this._recordPlayerLeave(name); + let reason = { + "LEAVE": "quit", + "KICK": "kicked", + "BAN": "banned", + }[parsed.action]; + this._recordPlayerLeave(name, reason); } }); @@ -322,7 +330,7 @@ class Instance extends libLink.Link { this.playersOnline.forEach(player => joined.delete(player)); for (let player of left) { - this._recordPlayerLeave(player); + this._recordPlayerLeave(player, "quit"); } // Missing join messages is not supposed to happen. @@ -358,13 +366,14 @@ class Instance extends libLink.Link { libPlugin.invokeHook(this.plugins, "onPlayerEvent", event); } - _recordPlayerLeave(name) { + _recordPlayerLeave(name, reason) { if (!this.playersOnline.delete(name)) { return; } let stats = this.playerStats.get(name); stats.lastLeaveAt = Date.now(); + stats.lastLeaveReason = reason; stats.onlineTimeMs += stats.lastLeaveAt - stats.lastJoinAt; this._hadPlayersOnline = true; @@ -372,6 +381,7 @@ class Instance extends libLink.Link { instance_id: this.id, type: "leave", name, + reason, }; libLink.messages.playerEvent.send(this, event); libPlugin.invokeHook(this.plugins, "onPlayerEvent", event); diff --git a/test/slave/Instance.js b/test/slave/Instance.js index 0008489f..142e1cd0 100644 --- a/test/slave/Instance.js +++ b/test/slave/Instance.js @@ -89,21 +89,21 @@ describe("class Instance", function() { it("should send player_event", function() { instance._recordPlayerJoin("player"); - instance._recordPlayerLeave("player"); + instance._recordPlayerLeave("player", "quit"); assert.deepEqual( connector.sentMessages[1], { seq: 2, type: "player_event_event", - data: { type: "leave", instance_id: instance.id, name: "player" }, + data: { type: "leave", instance_id: instance.id, name: "player", reason: "quit" }, }, ); }); it("should be idempotent", function() { instance._recordPlayerJoin("player"); - instance._recordPlayerLeave("player"); - instance._recordPlayerLeave("player"); + instance._recordPlayerLeave("player", "quit"); + instance._recordPlayerLeave("player", "quit"); assert(!instance.playersOnline.has("player"), "player was not removed"); assert.equal(connector.sentMessages.length, 2); }); @@ -148,7 +148,7 @@ describe("class Instance", function() { { seq: 2, type: "player_event_event", - data: { type: "leave", instance_id: instance.id, name: "player" }, + data: { type: "leave", instance_id: instance.id, name: "player", reason: "quit" }, }, ); }); From 096f3382774336e4938e514d2179144701ae496e Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Fri, 8 Jul 2022 00:30:35 +0200 Subject: [PATCH 2/8] Add live updating of users Provied a subscription for user updates and send updates whenever the details of a user account is changed. --- packages/ctl/ctl.js | 2 + packages/lib/link/messages.js | 53 +++++++++--- packages/lib/users.js | 16 ++++ packages/master/src/ControlConnection.js | 55 ++++++++++++- packages/master/src/Master.js | 10 +++ packages/master/src/SlaveConnection.js | 1 + .../web_ui/src/components/UserViewPage.jsx | 24 +----- packages/web_ui/src/components/UsersPage.jsx | 8 +- packages/web_ui/src/model/user.jsx | 81 +++++++++++++++++++ packages/web_ui/src/util/websocket.jsx | 61 ++++++++++++++ test/integration/clusterio.js | 12 +++ test/integration/index.js | 8 ++ 12 files changed, 292 insertions(+), 39 deletions(-) create mode 100644 packages/web_ui/src/model/user.jsx diff --git a/packages/ctl/ctl.js b/packages/ctl/ctl.js index 41325876..9211543c 100755 --- a/packages/ctl/ctl.js +++ b/packages/ctl/ctl.js @@ -89,6 +89,8 @@ class Control extends libLink.Link { async saveListUpdateEventHandler() { } + async userUpdateEventHandler() { } + async setLogSubscriptions({ all = false, master = false, diff --git a/packages/lib/link/messages.js b/packages/lib/link/messages.js index 01f75da0..1e7050cf 100644 --- a/packages/lib/link/messages.js +++ b/packages/lib/link/messages.js @@ -786,6 +786,28 @@ messages.deleteRole = new Request({ }, }); +const userRequired = ["name", "roles", "instances"]; +const userProperties = { + "name": { type: "string" }, + "roles": { type: "array", items: { type: "integer" }}, + "is_admin": { type: "boolean" }, + "is_banned": { type: "boolean" }, + "is_whitelisted": { type: "boolean" }, + "instances": { type: "array", items: { type: "integer" }}, + "is_deleted": { type: "boolean" }, +}; + +messages.getUser = new Request({ + type: "get_user", + links: ["control-master"], + permission: "core.user.get", + requestProperties: { + "name": { type: "string" }, + }, + responseRequired: userRequired, + responseProperties: userProperties, +}); + messages.listUsers = new Request({ type: "list_users", links: ["control-master"], @@ -795,20 +817,26 @@ messages.listUsers = new Request({ type: "array", items: { additionalProperties: false, - required: ["name", "roles", "instances"], - properties: { - "name": { type: "string" }, - "roles": { type: "array", items: { type: "integer" }}, - "is_admin": { type: "boolean" }, - "is_banned": { type: "boolean" }, - "is_whitelisted": { type: "boolean" }, - "instances": { type: "array", items: { type: "integer" }}, - }, + required: userRequired, + properties: userProperties, }, }, }, }); +messages.setUserSubscriptions = new Request({ + type: "set_user_subscriptions", + links: ["control-master"], + permission: "core.user.subscribe", + requestProperties: { + "all": { type: "boolean" }, + "names": { + type: "array", + items: { type: "string" }, + }, + }, +}); + messages.createUser = new Request({ type: "create_user", links: ["control-master"], @@ -1218,6 +1246,13 @@ messages.saveListUpdate = new Event({ }, }); +messages.userUpdate = new Event({ + type: "user_update", + links: ["master-control"], + eventRequired: userRequired, + eventProperties: userProperties, +}); + messages.masterConnectionEvent = new Event({ type: "master_connection_event", links: ["slave-instance"], diff --git a/packages/lib/users.js b/packages/lib/users.js index 8432191b..8171a499 100644 --- a/packages/lib/users.js +++ b/packages/lib/users.js @@ -195,6 +195,12 @@ class User { } } } + + /** + * True if this user object has been removed from the cluster. + * @type {boolean} + */ + this.isDeleted = false; } serialize() { @@ -487,11 +493,21 @@ definePermission({ description: "Permanently delete roles.", }); +definePermission({ + name: "core.user.get", + title: "Get user", + description: "Get the details of a user in the cluster.", +}); definePermission({ name: "core.user.list", title: "List users", description: "Get the full list of users in the cluster.", }); +definePermission({ + name: "core.user.subscribe", + title: "Subscribe to user updates", + description: "Subscribe to be notified on updates on the details and status of users.", +}); definePermission({ name: "core.user.create", title: "Create users", diff --git a/packages/master/src/ControlConnection.js b/packages/master/src/ControlConnection.js index 05a31320..3b0b4f33 100644 --- a/packages/master/src/ControlConnection.js +++ b/packages/master/src/ControlConnection.js @@ -53,6 +53,11 @@ class ControlConnection extends BaseConnection { instance_ids: [], }; + this.userSubscriptions = { + all: false, + names: [], + }; + this.logTransport = null; this.logSubscriptions = { all: false, @@ -559,6 +564,23 @@ class ControlConnection extends BaseConnection { } } + async getUserRequestHandler(message) { + let name = message.data.name; + let user = this._master.userManager.users.get(name); + if (!user) { + throw new libErrors.RequestError(`User ${name} does not exist`); + } + + return { + name: user.name, + roles: [...user.roles].map(role => role.id), + is_admin: user.isAdmin, + is_banned: user.isBanned, + is_whitelisted: user.isWhitelisted, + instances: [...user.instances], + }; + } + async listUsersRequestHandler(message) { let list = []; for (let user of this._master.userManager.users.values()) { @@ -574,8 +596,30 @@ class ControlConnection extends BaseConnection { return { list }; } + async setUserSubscriptionsRequestHandler(message) { + this.userSubscriptions = message.data; + } + + userUpdated(user) { + if ( + this.userSubscriptions.all + || this.userSubscriptions.names.includes(user.name) + ) { + libLink.messages.userUpdate.send(this, { + name: user.name, + roles: [...user.roles].map(role => role.id), + is_admin: user.isAdmin, + is_banned: user.isBanned, + is_whitelisted: user.isWhitelisted, + instances: [...user.instances], + is_deleted: user.isDeleted, + }); + } + } + async createUserRequestHandler(message) { - this._master.userManager.createUser(message.data.name); + let user = this._master.userManager.createUser(message.data.name); + this._master.userUpdated(user); } async updateUserRolesRequestHandler(message) { @@ -596,6 +640,7 @@ class ControlConnection extends BaseConnection { user.roles = resolvedRoles; this._master.userPermissionsUpdated(user); + this._master.userUpdated(user); } async setUserAdminRequestHandler(message) { @@ -611,6 +656,7 @@ class ControlConnection extends BaseConnection { } user.isAdmin = admin; + this._master.userUpdated(user); this.broadcastEventToSlaves({ data: { name, admin }}, libLink.messages.adminlistUpdate); } @@ -628,6 +674,7 @@ class ControlConnection extends BaseConnection { user.isBanned = banned; user.banReason = reason; + this._master.userUpdated(user); this.broadcastEventToSlaves({ data: { name, banned, reason }}, libLink.messages.banlistUpdate); } @@ -644,6 +691,7 @@ class ControlConnection extends BaseConnection { } user.isWhitelisted = whitelisted; + this._master.userUpdated(user); this.broadcastEventToSlaves({ data: { name, whitelisted }}, libLink.messages.whitelistUpdate); } @@ -653,6 +701,10 @@ class ControlConnection extends BaseConnection { throw new libErrors.RequestError(`User '${message.data.name}' does not exist`); } + user.isDeleted = true; + this._master.userManager.users.delete(message.data.name); + this._master.userUpdated(user); + if (user.is_admin) { this.broadcastEventToSlaves({ data: { name, admin: false }}, libLink.messages.adminlistUpdate); } @@ -662,7 +714,6 @@ class ControlConnection extends BaseConnection { if (user.is_banned) { this.broadcastEventToSlaves({ data: { name, banned: false, reason: "" }}, libLink.messages.banlistUpdate); } - this._master.userManager.users.delete(message.data.name); } async debugDumpWsRequestHandler(message) { diff --git a/packages/master/src/Master.js b/packages/master/src/Master.js index eb292e62..8268021c 100644 --- a/packages/master/src/Master.js +++ b/packages/master/src/Master.js @@ -503,6 +503,16 @@ class Master { } } + userUpdated(user) { + for (let controlConnection of this.wsServer.controlConnections) { + if (controlConnection.connector.closing) { + continue; + } + + controlConnection.userUpdated(user); + } + } + /** * Notify connected control clients under the given user that the * permissions for this user may have changed. diff --git a/packages/master/src/SlaveConnection.js b/packages/master/src/SlaveConnection.js index a8e1e456..c2788adb 100644 --- a/packages/master/src/SlaveConnection.js +++ b/packages/master/src/SlaveConnection.js @@ -186,6 +186,7 @@ class SlaveConnection extends BaseConnection { } else if (type === "leave") { user.notifyLeave(instance_id); } + this._master.userUpdated(user); let instance = this._master.instances.get(instance_id); await libPlugin.invokeHook(this._master.plugins, "onPlayerEvent", instance, message.data); diff --git a/packages/web_ui/src/components/UserViewPage.jsx b/packages/web_ui/src/components/UserViewPage.jsx index f2730fa4..07858a95 100644 --- a/packages/web_ui/src/components/UserViewPage.jsx +++ b/packages/web_ui/src/components/UserViewPage.jsx @@ -10,31 +10,9 @@ import ControlContext from "./ControlContext"; import PageLayout from "./PageLayout"; import PluginExtra from "./PluginExtra"; import { notifyErrorHandler } from "../util/notify"; +import { useUser } from "../model/user"; -function useUser(name) { - let control = useContext(ControlContext); - let [user, setUser] = useState({ loading: true }); - - function updateUser() { - // XXX optimize by requesting only the user in question - libLink.messages.listUsers.send(control).then(result => { - let match = result.list.find(u => u.name === name); - if (!match) { - setUser({ missing: true }); - } else { - setUser({ ...match, present: true }); - } - }); - } - - useEffect(() => { - updateUser(); - }, [name]); - - return [user, updateUser]; -} - export default function UserViewPage() { let params = useParams(); let userName = params.name; diff --git a/packages/web_ui/src/components/UsersPage.jsx b/packages/web_ui/src/components/UsersPage.jsx index 397db3d9..9405b8cd 100644 --- a/packages/web_ui/src/components/UsersPage.jsx +++ b/packages/web_ui/src/components/UsersPage.jsx @@ -9,6 +9,7 @@ import ControlContext from "./ControlContext"; import { notifyErrorHandler } from "../util/notify"; import PageLayout from "./PageLayout"; import PluginExtra from "./PluginExtra"; +import { useUserList } from "../model/user"; const strcmp = new Intl.Collator(undefined, { numerice: "true", sensitivity: "base" }).compare; @@ -57,14 +58,11 @@ export default function UsersPage() { let account = useAccount(); let control = useContext(ControlContext); let history = useHistory(); + let [userList] = useUserList(); - let [users, setUsers] = useState([]); let [roles, setRoles] = useState(new Map()); useEffect(() => { - libLink.messages.listUsers.send(control).then(result => { - setUsers(result["list"]); - }).catch(notifyErrorHandler("Error fetching user list")); libLink.messages.listRoles.send(control).then(result => { setRoles(new Map(result["list"].map(role => [role["id"], role]))); }).catch(() => { @@ -113,7 +111,7 @@ export default function UsersPage() { responsive: ["lg"], }, ]} - dataSource={users} + dataSource={userList} pagination={false} rowKey={user => user["name"]} onRow={(user, rowIndex) => ({ diff --git a/packages/web_ui/src/model/user.jsx b/packages/web_ui/src/model/user.jsx new file mode 100644 index 00000000..6741430a --- /dev/null +++ b/packages/web_ui/src/model/user.jsx @@ -0,0 +1,81 @@ +import { useEffect, useContext, useState } from "react"; +import ControlContext from "../components/ControlContext"; + +import { libLink, libLogging } from "@clusterio/lib"; +const { logger } = libLogging; + + +export function useUser(name) { + let control = useContext(ControlContext); + let [user, setUser] = useState({ loading: true }); + + function updateUser() { + libLink.messages.getUser.send(control, { name }).then(result => { + setUser({ ...result, present: true }); + }).catch(err => { + logger.error(`Failed to get user: ${err}`); + setUser({ missing: true }); + }); + + } + + useEffect(() => { + if (typeof name !== "string") { + setUser({ missing: true }); + return undefined; + } + updateUser(); + + function updateHandler(newUser) { + setUser({ ...newUser, present: true }); + } + + control.onUserUpdate(name, updateHandler); + return () => { + control.offUserUpdate(name, updateHandler); + }; + }, [name]); + + return [user, updateUser]; +} + +export function useUserList() { + let control = useContext(ControlContext); + let [userList, setUserList] = useState([]); + + function updateUserList() { + libLink.messages.listUsers.send(control).then(result => { + setUserList(result.list); + }).catch(err => { + logger.error(`Failed to list users:\n${err}`); + }); + } + + useEffect(() => { + updateUserList(); + + function updateHandler(newUser) { + setUserList(oldList => { + let newList = oldList.concat(); + let index = newList.findIndex(u => u.name === newUser.name); + if (!newUser.is_deleted) { + if (index !== -1) { + newList[index] = newUser; + } else { + newList.push(newUser); + } + } else if (index !== -1) { + newList.splice(index, 1); + } + return newList; + }); + } + + control.onUserUpdate(null, updateHandler); + return () => { + control.offUserUpdate(null, updateHandler); + }; + }, []); + + return [userList]; +} diff --git a/packages/web_ui/src/util/websocket.jsx b/packages/web_ui/src/util/websocket.jsx index 5473ad49..12d8eacd 100644 --- a/packages/web_ui/src/util/websocket.jsx +++ b/packages/web_ui/src/util/websocket.jsx @@ -86,6 +86,7 @@ export class Control extends libLink.Link { this.slaveUpdateHandlers = new Map(); this.instanceUpdateHandlers = new Map(); this.saveListUpdateHandlers = new Map(); + this.userUpdateHandlers = new Map(); this.logHandlers = new Map(); this.connector.on("connect", data => { @@ -101,6 +102,9 @@ export class Control extends libLink.Link { this.updateSaveListSubscriptions().catch(err => logger.error( `Unexpected error updating save list subscriptions:\n${err.stack}` )); + this.updateUserSubscriptions().catch(err => logger.error( + `Unexpected error updating user subscriptions:\n${err.stack}` + )); this.updateLogSubscriptions().catch(err => logger.error( `Unexpected error updating log subscriptions:\n${err.stack}` )); @@ -315,6 +319,63 @@ export class Control extends libLink.Link { }); } + async userUpdateEventHandler(message) { + let handlers = [].concat( + this.userUpdateHandlers.get(null) || [], + this.userUpdateHandlers.get(message.data.name) || [], + ); + for (let handler of handlers) { + handler(message.data); + } + }; + + async onUserUpdate(name, handler) { + if (name !== null && typeof name !== "string") { + throw new Error("Invalid user name"); + } + + let handlers = this.userUpdateHandlers.get(name); + if (!handlers) { + handlers = []; + this.userUpdateHandlers.set(name, handlers); + } + + handlers.push(handler); + + if (handlers.length === 1) { + await this.updateUserSubscriptions(); + } + } + + async offUserUpdate(name, handler) { + let handlers = this.userUpdateHandlers.get(name); + if (!handlers || !handlers.length) { + throw new Error(`No handlers for user ${name} exist`); + } + + let index = handlers.lastIndexOf(handler); + if (index === -1) { + throw new Error(`Given handler is not registered for user ${name}`); + } + + handlers.splice(index, 1); + if (!handlers.length) { + this.userUpdateHandlers.delete(name); + await this.updateUserSubscriptions(); + } + } + + async updateUserSubscriptions() { + if (!this.connector.connected) { + return; + } + + await libLink.messages.setUserSubscriptions.send(this, { + all: this.userUpdateHandlers.has(null), + names: [...this.userUpdateHandlers.keys()].filter(e => e !== null), + }); + } + async logMessageEventHandler(message) { let info = message.data.info; diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js index d4051043..aab1af97 100644 --- a/test/integration/clusterio.js +++ b/test/integration/clusterio.js @@ -483,6 +483,7 @@ describe("Integration of Clusterio", function() { let lists = ["admin", "whitelisted", "banned"]; it("should add and remove the given user to the list", async function() { slowTest(this); + getControl().userUpdates = []; await libLink.messages.createUser.send(getControl(), { name: "list_test" }); let user = (await getUsers()).get("list_test"); for (let list of lists) { @@ -499,6 +500,7 @@ describe("Integration of Clusterio", function() { for (let list of lists) { assert.equal(user[`is_${list}`], false, `unexpected ${list} status`); } + assert.equal(getControl().userUpdates.length, 7); }); it("should not create the user if not instructed to", async function() { slowTest(this); @@ -512,11 +514,13 @@ describe("Integration of Clusterio", function() { }); it("should create the user if instructed to", async function() { slowTest(this); + getControl().userUpdates = []; for (let list of lists) { await execCtl(`user set-${list} --create test_create_${list}`); let user = (await getUsers()).get(`test_create_${list}`); assert.equal(user && user[`is_${list}`], true, `user not created and added to ${list}`); } + assert.equal(getControl().userUpdates.length, 3); }); }); @@ -868,28 +872,36 @@ describe("Integration of Clusterio", function() { describe("user create", function() { it("should create the given user", async function() { + getControl().userUpdates = []; await execCtl("user create temp"); let result = await libLink.messages.listUsers.send(getControl()); let tempUser = result.list.find(user => user.name === "temp"); assert(tempUser, "user was not created"); + assert.equal(getControl().userUpdates.length, 1); + assert.equal(getControl().userUpdates[0].name, "temp"); }); }); describe("user set-roles", function() { it("should set the roles on the user", async function() { + getControl().userUpdates = []; await execCtl('user set-roles temp "Cluster Admin"'); let result = await libLink.messages.listUsers.send(getControl()); let tempUser = result.list.find(user => user.name === "temp"); assert.deepEqual(tempUser.roles, [0]); + assert.equal(getControl().userUpdates.length, 1); }); }); describe("user delete", function() { it("should delete the user", async function() { + getControl().userUpdates = []; await execCtl("user delete temp"); let result = await libLink.messages.listUsers.send(getControl()); let tempUser = result.list.find(user => user.name === "temp"); assert(!tempUser, "user was note deleted"); + assert.equal(getControl().userUpdates.length, 1); + assert.equal(getControl().userUpdates[0].is_deleted, true); }); }); }); diff --git a/test/integration/index.js b/test/integration/index.js index f8c40f46..1084570e 100644 --- a/test/integration/index.js +++ b/test/integration/index.js @@ -28,6 +28,7 @@ class TestControl extends libLink.Link { this.slaveUpdates = []; this.instanceUpdates = []; this.saveListUpdates = []; + this.userUpdates = []; this.connector.on("connect", () => { libLink.messages.setSlaveSubscriptions.send( @@ -39,6 +40,9 @@ class TestControl extends libLink.Link { libLink.messages.setSaveListSubscriptions.send( this, { all: true, instance_ids: [] } ).catch(err => logger.error(`Error setting save list subscriptions:\n${err.stack}`)); + libLink.messages.setUserSubscriptions.send( + this, { all: true, names: [] } + ).catch(err => logger.error(`Error setting user subscriptions:\n${err.stack}`)); }); } @@ -63,6 +67,10 @@ class TestControl extends libLink.Link { this.saveListUpdates.push(message.data); } + async userUpdateEventHandler(message) { + this.userUpdates.push(message.data); + } + async logMessageEventHandler() { } } From 9a3edc8c119a0842650b9ca346d30145f823f4a2 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Fri, 8 Jul 2022 00:32:53 +0200 Subject: [PATCH 3/8] Fix web build broken by assert and buffer imports Not sure what made it break, but it seems like it shouldn't have worked to begin with. --- packages/web_ui/webpack.common.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/web_ui/webpack.common.js b/packages/web_ui/webpack.common.js index 14f0696f..8b871783 100644 --- a/packages/web_ui/webpack.common.js +++ b/packages/web_ui/webpack.common.js @@ -98,9 +98,10 @@ module.exports = (env = {}) => ({ // Required for winston "util": require.resolve("util/"), "os": require.resolve("os-browserify/browser"), + "buffer": require.resolve("buffer/"), // Required for zlib - "assert": require.resolve("assert"), + "assert": require.resolve("assert/"), "stream": require.resolve("stream-browserify"), "events": require.resolve("events/"), From 097827b5154895aaf746981ae3d73584a6da415c Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Tue, 12 Jul 2022 17:29:22 +0200 Subject: [PATCH 4/8] Update linter for ES2022 syntax Allow modern JavaScript syntax including class property declarations since Node.js v12 supports this. --- .eslintrc.js | 5 +---- package.json | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index a7b7a962..8adba765 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -6,10 +6,7 @@ module.exports = { "env": { "node": true, "commonjs": true, - "es6": true, - }, - "parserOptions": { - "ecmaVersion": 11, + "es2022": true, }, "overrides": [ diff --git a/package.json b/package.json index 655d9336..2fbb199f 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ "@clusterio/lib": "workspace:*", "@clusterio/master": "workspace:*", "@clusterio/slave": "workspace:*", - "eslint": "^7.25.0", + "eslint": "^8.18.0", "eslint-plugin-node": "^11.1.0", "express": "^4.16.2", "form-data": "^4.0.0", From ac62f66aa1bd856ee5d71aabaa53c601c2835416 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Tue, 12 Jul 2022 22:21:58 +0200 Subject: [PATCH 5/8] Move PlayerStats to separate data class --- packages/lib/PlayerStats.js | 84 ++++++++++++++++++++++++++++++++++ packages/slave/src/Instance.js | 39 ++++------------ 2 files changed, 94 insertions(+), 29 deletions(-) create mode 100644 packages/lib/PlayerStats.js diff --git a/packages/lib/PlayerStats.js b/packages/lib/PlayerStats.js new file mode 100644 index 00000000..19e36818 --- /dev/null +++ b/packages/lib/PlayerStats.js @@ -0,0 +1,84 @@ +"use strict"; + +/** + * Stastics collected about a player. + * @alias module:lib/PlayerStats + */ +class PlayerStats { + /** + * Count of the number of times this player has been seen joining this server. + * @type {number} + */ + joinCount = 0; + + /** + * Time in ms this player has been seen online on this instance. + * @type {number} + */ + onlineTimeMs = 0; + + /** + * Timestamp the player was last seen joining. + * @type {Date=} + */ + lastJoinAt; + + /** + * Timestamp the player was last seen leaving. + * @type {Date=} + */ + lastLeaveAt; + + /** + * Reason the player was last seen leaving with. + * @type {string=} + */ + lastLeaveReason; + + static jsonSchema = { + type: "object", + properties: { + "join_count": { type: "integer" }, + "online_time_ms": { type: "number" }, + "last_join_at": { type: "number" }, + "last_leave_at": { type: "number" }, + "last_leave_reason": { type: "string" }, + }, + }; + + constructor(json = {}) { + if (json["join_count"]) { + this.joinCount = json["join_count"]; + } + if (json["online_time_ms"]) { + this.onlineTimeMs = json["online_time_ms"]; + } + if (json["last_join_at"]) { + this.lastJoinAt = new Date(json["last_join_at"]); + } + if (json["last_leave_at"]) { + this.lastLeaveAt = new Date(json["last_leave_at"]); + this.lastLeaveReason = json["last_leave_reason"] || "quit"; + } + } + + toJSON() { + let json = {}; + if (this.joinCount) { + json["join_count"] = this.joinCount; + } + if (this.onlineTimeMs) { + json["online_time_ms"] = this.onlineTimeMs; + } + if (this.lastJoinAt) { + json["last_join_at"] = this.lastJoinAt.getTime(); + } + if (this.lastLeaveAt) { + json["last_leave_at"] = this.lastLeaveAt.getTime(); + json["last_leave_reason"] = this.lastLeaveReason; + } + return json; + } +} + +module.exports = PlayerStats; diff --git a/packages/slave/src/Instance.js b/packages/slave/src/Instance.js index 9207cad9..1fa1543c 100644 --- a/packages/slave/src/Instance.js +++ b/packages/slave/src/Instance.js @@ -9,6 +9,7 @@ const util = require("util"); const libFileOps = require("@clusterio/lib/file_ops"); const libFactorio = require("@clusterio/lib/factorio"); const libLink = require("@clusterio/lib/link"); +const PlayerStats = require("@clusterio/lib/PlayerStats"); const libPlugin = require("@clusterio/lib/plugin"); const libPluginLoader = require("@clusterio/lib/plugin_loader"); const libErrors = require("@clusterio/lib/errors"); @@ -254,29 +255,9 @@ class Instance extends libLink.Link { } }); - /** - * @typedef module:slave/src/Instance~PlayerStat - * @type {object} - * @property {number} lastJoinAt - - * Unix timestamp in ms the player was last seen joining this - * instance. - * @property {number=} lastLeaveAt - - * Unix timestamp in ms the player was last seen leaving this - * instance. Not present if player has yet to leave after first - * time joining. - * @property {string=} lastLeaveReason - - * Reason the player was last seen leaving with. Not present if - * player has yet to leave after first time joining. - * @property {number} joinCount - - * Count of the number of times this player has been seen - * joining this server. - * @property {number} onlineTimeMs - - * Time in ms this player has been seen online on this instance. - */ - /** * Per player statistics recorded by this instance. - * @type {Map} + * @type {Map} */ this.playerStats = new Map(); @@ -351,10 +332,10 @@ class Instance extends libLink.Link { let stats = this.playerStats.get(name); if (!stats) { - stats = { joinCount: 0, onlineTimeMs: 0 }; + stats = new PlayerStats(); this.playerStats.set(name, stats); } - stats.lastJoinAt = Date.now(); + stats.lastJoinAt = new Date(); stats.joinCount += 1; let event = { @@ -372,9 +353,9 @@ class Instance extends libLink.Link { } let stats = this.playerStats.get(name); - stats.lastLeaveAt = Date.now(); + stats.lastLeaveAt = new Date(); stats.lastLeaveReason = reason; - stats.onlineTimeMs += stats.lastLeaveAt - stats.lastJoinAt; + stats.onlineTimeMs += stats.lastLeaveAt.getTime() - stats.lastJoinAt.getTime(); this._hadPlayersOnline = true; let event = { @@ -517,17 +498,17 @@ class Instance extends libLink.Link { } async _loadStats() { - let stats; + let instanceStats; try { - stats = JSON.parse(await fs.readFile(this.path("instance-stats.json"))); + instanceStats = JSON.parse(await fs.readFile(this.path("instance-stats.json"))); } catch (err) { if (err.code === "ENOENT") { return; } throw err; } - this.playerStats = new Map(stats["players"]); - this._playerAutosaveSlot = stats["player_autosave_slot"] || 1; + this.playerStats = new Map(instanceStats["players"].map(([id, stats]) => [id, new PlayerStats(stats)])); + this._playerAutosaveSlot = instanceStats["player_autosave_slot"] || 1; } async _saveStats() { From deb6842a4db07c65a090df0e1ad822048f53439c Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Tue, 12 Jul 2022 22:22:58 +0200 Subject: [PATCH 6/8] Record all players as leaving on server exit In the case of shutdown the Factorio server will not notify the currently online players as leaving the game. Notify the master server that all remaining online players quit the game in this case. --- packages/lib/plugin.js | 6 ++++-- packages/slave/src/Instance.js | 3 +++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/lib/plugin.js b/packages/lib/plugin.js index 23499e4b..162794d4 100644 --- a/packages/lib/plugin.js +++ b/packages/lib/plugin.js @@ -210,7 +210,8 @@ class BaseInstancePlugin { * @param {string} event.name - Name of the player that joined/left. * @param {string=} event.reason - * Only present for type "leave". Reason for player leaving the - * game, one of the possible reasons in defines.disconnect_reason. + * game, one of the possible reasons in defines.disconnect_reason + * or "server_quit" if the server exits while the player is online. */ async onPlayerEvent(event) { } @@ -505,7 +506,8 @@ class BaseMasterPlugin { * @param {string} event.name - Name of the player that joined/left. * @param {string=} event.reason - * Only present for type "leave". Reason for player leaving the - * game, one of the possible reasons in defines.disconnect_reason. + * game, one of the possible reasons in defines.disconnect_reason + * or "server_quit" if the server exits while the player is online. */ async onPlayerEvent(instance, event) { } diff --git a/packages/slave/src/Instance.js b/packages/slave/src/Instance.js index 1fa1543c..58f15554 100644 --- a/packages/slave/src/Instance.js +++ b/packages/slave/src/Instance.js @@ -483,6 +483,9 @@ class Instance extends libLink.Link { pluginInstance.onExit(); } + for (let player of left) { + this._recordPlayerLeave(player, "server_quit"); + } this._saveStats().catch(err => this.logger.error(`Error saving stats:\n${err.stack}`)); } From 0245ccd16c4f6edc5d3feb97d3df72cc416de242 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Tue, 12 Jul 2022 23:01:26 +0200 Subject: [PATCH 7/8] Share player stats with master server Send the per player stats with the player event updates to the master server and expose it to the Web UI and ctl interfaces. --- packages/ctl/src/commands.js | 52 +++++++++- packages/lib/link/messages.js | 10 ++ packages/lib/users.js | 42 ++++++++ packages/master/src/ControlConnection.js | 5 + packages/master/src/SlaveConnection.js | 6 +- packages/slave/src/Instance.js | 4 +- .../web_ui/src/components/UserViewPage.jsx | 97 ++++++++++++++++++- packages/web_ui/src/components/UsersPage.jsx | 40 ++++---- packages/web_ui/src/model/user.jsx | 49 +++++++++- packages/web_ui/src/util/time_format.jsx | 33 +++++++ test/lib/users.js | 30 ++++++ test/slave/Instance.js | 66 +++++++++++-- 12 files changed, 400 insertions(+), 34 deletions(-) create mode 100644 packages/web_ui/src/util/time_format.jsx diff --git a/packages/ctl/src/commands.js b/packages/ctl/src/commands.js index cfa79189..7c7fa270 100644 --- a/packages/ctl/src/commands.js +++ b/packages/ctl/src/commands.js @@ -941,9 +941,59 @@ roleCommands.add(new libCommand.Command({ const userCommands = new libCommand.CommandTree({ name: "user", alias: ["u"], description: "User management" }); userCommands.add(new libCommand.Command({ - definition: [["list", "l"], "List user in the cluster"], + definition: ["show ", "Show details for one user", (yargs) => { + yargs.positional("name", { decribe: "Name of user to show", type: "string" }); + yargs.options({ + "instance-stats": { describe: "include per-instance stats", nargs: 0, type: "boolean", default: false }, + }); + }], + handler: async function(args, control) { + let response = await libLink.messages.getUser.send(control, { name: args.name }); + delete response["seq"]; + Object.assign(response, response["player_stats"]); + delete response["player_stats"]; + let instanceStats = response["instance_stats"]; + delete response["instance_stats"]; + print(asTable(Object.entries(response).map(([property, value]) => ({ property, value })))); + + if (args.instanceStats) { + let instances = (await libLink.messages.listInstances.send(control)).list; + function instanceName(id) { + let instance = instances.find(i => i.id === id); + if (instance) { + return instance.name; + } + return ""; + } + for (let [id, playerInstanceStats] of instanceStats) { + print(); + print(`Instance ${instanceName(id)} (${id}):`); + print(asTable(Object.entries(playerInstanceStats).map(([property, value]) => ({ property, value })))); + } + } + }, +})); + +userCommands.add(new libCommand.Command({ + definition: [["list", "l"], "List user in the cluster", (yargs) => { + yargs.options({ + "stats": { describe: "include user stats", nargs: 0, type: "boolean", default: false }, + "attributes": { describe: "include admin/whitelisted/banned", nargs: 0, type: "boolean", default: false }, + }); + }], handler: async function(args, control) { let response = await libLink.messages.listUsers.send(control); + for (let user of response.list) { + if (args.stats) { + Object.assign(user, user["player_stats"]); + } + delete user["player_stats"]; + if (!args.attributes) { + delete user["is_admin"]; + delete user["is_whitelisted"]; + delete user["is_banned"]; + } + } print(asTable(response.list)); }, })); diff --git a/packages/lib/link/messages.js b/packages/lib/link/messages.js index 1e7050cf..6028c31e 100644 --- a/packages/lib/link/messages.js +++ b/packages/lib/link/messages.js @@ -3,6 +3,7 @@ const libSchema = require("../schema"); const libErrors = require("../errors"); const { logger } = require("../logging"); +const PlayerStats = require("../PlayerStats"); class MissingLinkHandlerError extends Error { constructor(type, source, target) { @@ -795,6 +796,14 @@ const userProperties = { "is_whitelisted": { type: "boolean" }, "instances": { type: "array", items: { type: "integer" }}, "is_deleted": { type: "boolean" }, + "player_stats": PlayerStats.jsonSchema, + "instance_stats": { + type: "array", + items: { + type: "array", + items: [{ type: "integer" }, PlayerStats.jsonSchema], + }, + }, }; messages.getUser = new Request({ @@ -1325,6 +1334,7 @@ messages.playerEvent = new Event({ "type": { type: "string", enum: ["join", "leave"] }, "name": { type: "string" }, "reason": { type: "string" }, + "stats": PlayerStats.jsonSchema, }, }); diff --git a/packages/lib/users.js b/packages/lib/users.js index 8171a499..c1f72afd 100644 --- a/packages/lib/users.js +++ b/packages/lib/users.js @@ -11,6 +11,8 @@ const jwt = require("jsonwebtoken"); const libErrors = require("./errors"); +const PlayerStats = require("./PlayerStats"); + /** * Represents a permission that can be granted @@ -196,6 +198,22 @@ class User { } } + /** + * Per instance statistics for the player this user account is tied to. + * @type {Map} + */ + this.instanceStats = new Map( + (serializedUser.instance_stats ? serializedUser.instance_stats : []).map( + ([id, stats]) => [id, new PlayerStats(stats)] + ) + ); + + /** + * Combined statistics for the player this user account is tied to. + * @type {module:lib/PlayerStats} + */ + this.playerStats = this._calculatePlayerStats(); + /** * True if this user object has been removed from the cluster. * @type {boolean} @@ -232,6 +250,10 @@ class User { serialized.ban_reason = this.banReason; } + if (this.instanceStats.size) { + serialized.instance_stats = [...this.instanceStats].map(([id, stats]) => [id, stats.toJSON()]); + } + return serialized; } @@ -291,6 +313,26 @@ class User { User.onlineUsers.delete(this); } } + + recalculatePlayerStats() { + this.playerStats = this._calculatePlayerStats(); + } + + _calculatePlayerStats() { + let playerStats = new PlayerStats(); + for (let instanceStats of this.instanceStats.values()) { + if (!playerStats.lastJoinAt || instanceStats.lastJoinAt > playerStats.lastJoinAt) { + playerStats.lastJoinAt = instanceStats.lastJoinAt; + } + if (!playerStats.lastLeaveAt || instanceStats.lastLeaveAt > playerStats.lastLeaveAt) { + playerStats.lastLeaveAt = instanceStats.lastLeaveAt; + playerStats.lastLeaveReason = instanceStats.lastLeaveReason; + } + playerStats.joinCount += instanceStats.joinCount; + playerStats.onlineTimeMs += instanceStats.onlineTimeMs; + } + return playerStats; + } } /** * Set of users currently online in the cluster. diff --git a/packages/master/src/ControlConnection.js b/packages/master/src/ControlConnection.js index 3b0b4f33..37cc1b6c 100644 --- a/packages/master/src/ControlConnection.js +++ b/packages/master/src/ControlConnection.js @@ -578,6 +578,8 @@ class ControlConnection extends BaseConnection { is_banned: user.isBanned, is_whitelisted: user.isWhitelisted, instances: [...user.instances], + player_stats: user.playerStats, + instance_stats: [...user.instanceStats], }; } @@ -591,6 +593,7 @@ class ControlConnection extends BaseConnection { is_banned: user.isBanned, is_whitelisted: user.isWhitelisted, instances: [...user.instances], + player_stats: user.playerStats, }); } return { list }; @@ -612,6 +615,8 @@ class ControlConnection extends BaseConnection { is_banned: user.isBanned, is_whitelisted: user.isWhitelisted, instances: [...user.instances], + player_stats: user.playerStats, + instance_stats: [...user.instanceStats], is_deleted: user.isDeleted, }); } diff --git a/packages/master/src/SlaveConnection.js b/packages/master/src/SlaveConnection.js index c2788adb..35564264 100644 --- a/packages/master/src/SlaveConnection.js +++ b/packages/master/src/SlaveConnection.js @@ -3,6 +3,7 @@ const libConfig = require("@clusterio/lib/config"); const libLink = require("@clusterio/lib/link"); const { logger } = require("@clusterio/lib/logging"); const libPlugin = require("@clusterio/lib/plugin"); +const PlayerStats = require("@clusterio/lib/PlayerStats"); const BaseConnection = require("./BaseConnection"); @@ -175,7 +176,7 @@ class SlaveConnection extends BaseConnection { } async playerEventEventHandler(message) { - let { instance_id, name, type } = message.data; + let { instance_id, name, type, stats } = message.data; let user = this._master.userManager.users.get(name); if (!user) { user = this._master.userManager.createUser(name); @@ -186,8 +187,11 @@ class SlaveConnection extends BaseConnection { } else if (type === "leave") { user.notifyLeave(instance_id); } + user.instanceStats.set(instance_id, new PlayerStats(stats)); + user.recalculatePlayerStats(); this._master.userUpdated(user); + delete message.data.stats; let instance = this._master.instances.get(instance_id); await libPlugin.invokeHook(this._master.plugins, "onPlayerEvent", instance, message.data); } diff --git a/packages/slave/src/Instance.js b/packages/slave/src/Instance.js index 58f15554..a3b6fb6a 100644 --- a/packages/slave/src/Instance.js +++ b/packages/slave/src/Instance.js @@ -342,6 +342,7 @@ class Instance extends libLink.Link { instance_id: this.id, type: "join", name, + stats: stats.toJSON(), }; libLink.messages.playerEvent.send(this, event); libPlugin.invokeHook(this.plugins, "onPlayerEvent", event); @@ -363,6 +364,7 @@ class Instance extends libLink.Link { type: "leave", name, reason, + stats: stats.toJSON(), }; libLink.messages.playerEvent.send(this, event); libPlugin.invokeHook(this.plugins, "onPlayerEvent", event); @@ -483,7 +485,7 @@ class Instance extends libLink.Link { pluginInstance.onExit(); } - for (let player of left) { + for (let player of this.playersOnline) { this._recordPlayerLeave(player, "server_quit"); } this._saveStats().catch(err => this.logger.error(`Error saving stats:\n${err.stack}`)); diff --git a/packages/web_ui/src/components/UserViewPage.jsx b/packages/web_ui/src/components/UserViewPage.jsx index 07858a95..81a11e00 100644 --- a/packages/web_ui/src/components/UserViewPage.jsx +++ b/packages/web_ui/src/components/UserViewPage.jsx @@ -1,16 +1,24 @@ import React, { useEffect, useContext, useState } from "react"; import { useHistory, useParams } from "react-router-dom"; -import { Button, Col, Form, Input, PageHeader, Popconfirm, Popover, Row, Tag, Select, Space, Spin, Switch } from "antd"; +import { + Button, Col, Descriptions, Form, Input, PageHeader, Popconfirm, + Popover, Row, Table, Tag, Select, Space, Spin, Switch, +} from "antd"; import DeleteOutlined from "@ant-design/icons/DeleteOutlined"; import { libLink } from "@clusterio/lib"; import { useAccount } from "../model/account"; +import { useInstanceList } from "../model/instance"; import ControlContext from "./ControlContext"; import PageLayout from "./PageLayout"; import PluginExtra from "./PluginExtra"; +import SectionHeader from "./SectionHeader"; import { notifyErrorHandler } from "../util/notify"; -import { useUser } from "../model/user"; +import { formatDuration } from "../util/time_format"; +import { formatLastSeen, sortLastSeen, useUser } from "../model/user"; + +const strcmp = new Intl.Collator(undefined, { numerice: "true", sensitivity: "base" }).compare; export default function UserViewPage() { @@ -21,6 +29,7 @@ export default function UserViewPage() { let account = useAccount(); let control = useContext(ControlContext); + let [instanceList] = useInstanceList(); let [user, updateUser] = useUser(userName); let [roles, setRoles] = useState(null); let [form] = Form.useForm(); @@ -83,10 +92,22 @@ export default function UserViewPage() { ; + function instanceName(id) { + let instance = instanceList.find(i => i.id === id); + return instance ? instance["name"] : id; + } + return + {userName} + + {user["is_admin"] && Admin} + {user["is_whitelisted"] && Whitelisted} + {user["is_banned"] && Banned} + + } extra={ account.hasPermission("core.user.delete") && @@ -110,7 +131,23 @@ export default function UserViewPage() { } /> -
+ @@ -237,6 +274,58 @@ export default function UserViewPage() {
+ + + + {user["player_stats"]["online_time_ms"] && formatDuration(user["player_stats"]["online_time_ms"])} + + + {user["player_stats"]["join_count"]} + + + {formatLastSeen(user)} + + + + instanceName(id), + defaultSortOrder: "ascend", + sorter: (a, b) => strcmp(a[0], b[0]), + }, + { + title: "Play time", + key: "playTime", + render: ([, stats]) => formatDuration(stats["online_time_ms"] || 0), + sorter: (a, b) => (a[1]["online_time_ms"] || 0) - (b[1]["online_time_ms"] || 0), + }, + { + title: "Join count", + key: "joinCoint", + render: ([, stats]) => stats["join_count"] || 0, + sorter: (a, b) => (a[1]["join_count"] || 0) - (b[1]["join_count"] || 0), + responsive: ["sm"], + }, + { + title: "Last seen", + key: "lastSeen", + render: ([id]) => formatLastSeen(user, id), + sorter: (a, b) => sortLastSeen(user, user, a[0], b[0]), + }, + ]} + dataSource={user["instance_stats"]} + pagination={false} + rowKey={([id]) => id} + onRow={([id], rowIndex) => ({ + onClick: event => { + history.push(`/instances/${id}/view`); + }, + })} + /> ; } diff --git a/packages/web_ui/src/components/UsersPage.jsx b/packages/web_ui/src/components/UsersPage.jsx index 9405b8cd..4b32f147 100644 --- a/packages/web_ui/src/components/UsersPage.jsx +++ b/packages/web_ui/src/components/UsersPage.jsx @@ -1,15 +1,16 @@ import React, { useEffect, useContext, useState } from "react"; import { useHistory } from "react-router-dom"; -import { Button, Form, Input, Modal, PageHeader, Table, Tag } from "antd"; +import { Button, Form, Input, Modal, PageHeader, Space, Table, Tag } from "antd"; import { libLink } from "@clusterio/lib"; import { useAccount } from "../model/account"; import ControlContext from "./ControlContext"; import { notifyErrorHandler } from "../util/notify"; +import { formatDuration } from "../util/time_format"; import PageLayout from "./PageLayout"; import PluginExtra from "./PluginExtra"; -import { useUserList } from "../model/user"; +import { formatLastSeen, sortLastSeen, useUserList } from "../model/user"; const strcmp = new Intl.Collator(undefined, { numerice: "true", sensitivity: "base" }).compare; @@ -80,7 +81,15 @@ export default function UsersPage() { columns={[ { title: "Name", - dataIndex: "name", + key: "name", + render: user => + {user["name"]} + + {user["is_admin"] && Admin} + {user["is_whitelisted"] && Whitelisted} + {user["is_banned"] && Banned} + + , defaultSortOrder: "ascend", sorter: (a, b) => strcmp(a["name"], b["name"]), }, @@ -90,24 +99,19 @@ export default function UsersPage() { render: user => user.roles.map(id => {(roles.get(id) || { name: id })["name"]}), }, { - title: "Admin", - key: "admin", - render: user => user["is_admin"] && "yes", - sorter: (a, b) => a["is_admin"] - b["is_admin"], + title: "Play time", + key: "playTime", + render: user => user["player_stats"]["online_time_ms"] + && formatDuration(user["player_stats"]["online_time_ms"]), + sorter: (a, b) => (a["player_stats"]["online_time_ms"] || 0) - + (b["player_stats"]["online_time_ms"] || 0), responsive: ["lg"], }, { - title: "Whitelisted", - key: "whitelisted", - render: user => user["is_whitelisted"] && "yes", - sorter: (a, b) => a["is_whitelisted"] - b["is_whitelisted"], - responsive: ["lg"], - }, - { - title: "Banned", - key: "banned", - render: user => user["is_banned"] && "yes", - sorter: (a, b) => a["is_banned"] - b["is_banned"], + title: "Last seen", + key: "lastSeen", + render: user => formatLastSeen(user), + sorter: sortLastSeen, responsive: ["lg"], }, ]} diff --git a/packages/web_ui/src/model/user.jsx b/packages/web_ui/src/model/user.jsx index 6741430a..5dad3ae9 100644 --- a/packages/web_ui/src/model/user.jsx +++ b/packages/web_ui/src/model/user.jsx @@ -1,9 +1,56 @@ -import { useEffect, useContext, useState } from "react"; +import React, { useEffect, useContext, useState } from "react"; +import { Tag } from "antd"; import ControlContext from "../components/ControlContext"; import { libLink, libLogging } from "@clusterio/lib"; const { logger } = libLogging; +function calculateLastSeen(user, instanceId) { + let stats; + if (instanceId === undefined) { + stats = user["player_stats"]; + } else { + let entry = (user["instance_stats"] || []).find(([id]) => id === instanceId); + if (!entry) { + return undefined; + } + stats = entry[1]; + } + if (stats["last_leave_at"] > stats["last_join_at"]) { + return stats["last_leave_at"]; + } + if (stats["last_join_at"]) { + return stats["last_leave_at"]; + } + return undefined; +} + +export function formatLastSeen(user, instanceId = undefined) { + if (user["instances"].some(id => instanceId === undefined || id === instanceId)) { + return Online; + } + let lastSeen = calculateLastSeen(user, instanceId); + if (lastSeen === undefined) { + return undefined; + } + return new Date(lastSeen).toLocaleTimeString(); +} + +export function sortLastSeen(userA, userB, instanceIdA = undefined, instanceIdB = undefined) { + function epoch(user, instanceId) { + return user["instances"].some(id => instanceId === undefined || id === instanceId); + } + + let epochA = epoch(userA, instanceIdA); + let epochB = epoch(userB, instanceIdB); + if (epochA !== epochB) { + return epochA - epochB; + } + + let lastSeenA = calculateLastSeen(userA, instanceIdA) || 0; + let lastSeenB = calculateLastSeen(userB, instanceIdB) || 0; + return lastSeenA - lastSeenB; +} export function useUser(name) { let control = useContext(ControlContext); diff --git a/packages/web_ui/src/util/time_format.jsx b/packages/web_ui/src/util/time_format.jsx new file mode 100644 index 00000000..4f77dac9 --- /dev/null +++ b/packages/web_ui/src/util/time_format.jsx @@ -0,0 +1,33 @@ +const formatSecond = new Intl.NumberFormat(undefined, { style: "unit", unit: "second", unitDisplay: "narrow" }).format; +const formatMinute = new Intl.NumberFormat(undefined, { style: "unit", unit: "minute", unitDisplay: "narrow" }).format; +const formatHour = new Intl.NumberFormat(undefined, { style: "unit", unit: "hour", unitDisplay: "narrow" }).format; +const formatDay = new Intl.NumberFormat(undefined, { style: "unit", unit: "day", unitDisplay: "narrow" }).format; + +/** + * Formats a duration of time in milliseconds to something human readable. + * @param {number} ms - Duration to formatDuration + * @param {object=} options - Options to control the formatting. + * @returns {string} human readable representation of the duration. + */ +export function formatDuration(ms, options = {}) { + let result = ""; + if (ms < 0) { + result += "-"; + ms = -ms; + } + + if (ms > 86400e3) { + result += formatDay(Math.floor(ms / 86400e3)); + ms %= 86400e3; + } + if (ms > 3600e3) { + result += formatHour(Math.floor(ms / 3600e3)); + ms %= 3600e3; + } + if (ms > 60e3) { + result += formatMinute(Math.floor(ms / 60e3)); + ms %= 60e3; + } + result += formatSecond(Math.round(ms / 1e3)); + return result; +} diff --git a/test/lib/users.js b/test/lib/users.js index f9619f91..6dd95a1f 100644 --- a/test/lib/users.js +++ b/test/lib/users.js @@ -85,6 +85,7 @@ describe("lib/users", function() { test_roundtrip({ name: "user", roles: [2], token_valid_after: 12345 }); test_roundtrip({ name: "user", is_admin: true, is_whitelisted: true }); test_roundtrip({ name: "user", is_banned: true, ban_reason: "Bad user" }); + test_roundtrip({ name: "user", instance_stats: [[1, { join_count: 1 }]]}); }); it("should ignore invalid roles", function() { let user = new libUsers.User({ name: "test", roles: [1, 4, 55] }, roles); @@ -115,6 +116,35 @@ describe("lib/users", function() { assert(!libUsers.User.onlineUsers.has(user)); assert.deepEqual(user.instances, new Set()); }); + it("should calculate playerStats", function() { + let user = new libUsers.User({ name: "test", roles: [1], instance_stats: [ + [1, { + join_count: 1, + online_time_ms: 60e3, + last_join_at: new Date("2020-05T12:00Z").getTime(), + last_leave_at: new Date("2020-05T12:01Z").getTime(), + last_leave_reason: "quit", + }], + [2, { + join_count: 1, + online_time_ms: 0, + last_join_at: new Date("2020-05T12:02Z").getTime(), + }], + [3, { + join_count: 2, + online_time_ms: 120e3, + last_join_at: new Date("2020-05T11:00Z").getTime(), + last_leave_at: new Date("2020-05T11:02Z").getTime(), + last_leave_reason: "afk", + }], + ]}, roles); + + assert.equal(user.playerStats.onlineTimeMs, 180e3); + assert.equal(user.playerStats.joinCount, 4); + assert.deepEqual(user.playerStats.lastJoinAt, new Date("2020-05T12:02Z")); + assert.deepEqual(user.playerStats.lastLeaveAt, new Date("2020-05T12:01Z")); + assert.equal(user.playerStats.lastLeaveReason, "quit"); + }); describe(".checkPermission()", function() { it("should correctly resolve permissions", function() { diff --git a/test/slave/Instance.js b/test/slave/Instance.js index 142e1cd0..1c3b5808 100644 --- a/test/slave/Instance.js +++ b/test/slave/Instance.js @@ -54,18 +54,28 @@ describe("class Instance", function() { it("should send player_event", function() { instance._recordPlayerJoin("player"); + let stats = instance.playerStats.get("player"); assert.deepEqual( connector.sentMessages[0], { seq: 1, type: "player_event_event", - data: { type: "join", instance_id: instance.id, name: "player" }, + data: { + type: "join", + instance_id: instance.id, + name: "player", + stats: { + join_count: 1, + last_join_at: stats.lastJoinAt.getTime(), + }, + }, }, ); }); - it("should be idempotent", function() { + it("should be idempotent", async function() { instance._recordPlayerJoin("player"); + await wait(10); instance._recordPlayerJoin("player"); assert(instance.playersOnline.has("player"), "player was not added"); assert.equal(connector.sentMessages.length, 1); @@ -73,8 +83,9 @@ describe("class Instance", function() { }); describe("._recordPlayerLeave()", function() { - it("should remove player to playersOnline", function() { + it("should remove player to playersOnline", async function() { instance._recordPlayerJoin("player"); + await wait(10); instance._recordPlayerLeave("player"); assert(!instance.playersOnline.has("player"), "player was not removed"); }); @@ -87,22 +98,38 @@ describe("class Instance", function() { assert(instance.playerStats.get("player").onlineTimeMs > 0, "no onlineTimeMs recorded"); }); - it("should send player_event", function() { + it("should send player_event", async function() { instance._recordPlayerJoin("player"); + await wait(10); instance._recordPlayerLeave("player", "quit"); + let stats = instance.playerStats.get("player"); assert.deepEqual( connector.sentMessages[1], { seq: 2, type: "player_event_event", - data: { type: "leave", instance_id: instance.id, name: "player", reason: "quit" }, + data: { + type: "leave", + instance_id: instance.id, + name: "player", + reason: "quit", + stats: { + join_count: 1, + online_time_ms: stats.onlineTimeMs, + last_join_at: stats.lastJoinAt.getTime(), + last_leave_at: stats.lastLeaveAt.getTime(), + last_leave_reason: "quit", + }, + }, }, ); }); - it("should be idempotent", function() { + it("should be idempotent", async function() { instance._recordPlayerJoin("player"); + await wait(10); instance._recordPlayerLeave("player", "quit"); + await wait(10); instance._recordPlayerLeave("player", "quit"); assert(!instance.playersOnline.has("player"), "player was not removed"); assert.equal(connector.sentMessages.length, 2); @@ -129,12 +156,21 @@ describe("class Instance", function() { ); instance._recordPlayerJoin("player"); await instance._checkOnlinePlayers(); + let stats = instance.playerStats.get("foo"); assert.deepEqual( connector.sentMessages[1], { seq: 2, type: "player_event_event", - data: { type: "join", instance_id: instance.id, name: "foo" }, + data: { + type: "join", + instance_id: instance.id, + name: "foo", + stats: { + join_count: 1, + last_join_at: stats.lastJoinAt.getTime(), + }, + }, }, ); }); @@ -142,13 +178,27 @@ describe("class Instance", function() { it("should remove extra players", async function() { instance.server.rconCommandResults.set("/players online", "Online Players (0):\n"); instance._recordPlayerJoin("player"); + await wait(10); await instance._checkOnlinePlayers(); + let stats = instance.playerStats.get("player"); assert.deepEqual( connector.sentMessages[1], { seq: 2, type: "player_event_event", - data: { type: "leave", instance_id: instance.id, name: "player", reason: "quit" }, + data: { + type: "leave", + instance_id: instance.id, + name: "player", + reason: "quit", + stats: { + join_count: 1, + online_time_ms: stats.onlineTimeMs, + last_join_at: stats.lastJoinAt.getTime(), + last_leave_at: stats.lastLeaveAt.getTime(), + last_leave_reason: "quit", + }, + }, }, ); }); From 08e7434d247c0e5ef1644076591f19bca800c986 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Wed, 13 Jul 2022 00:28:25 +0200 Subject: [PATCH 8/8] Add command to extract play time from save Add command that extracts play time (and players) from the currently running save to the recorded instance statistics. This is useful for getting correct play time statistics when an existing save game is imported. --- docs/managing-a-cluster.md | 10 +++++++ packages/ctl/src/commands.js | 13 +++++++++ packages/lib/link/messages.js | 9 +++++- packages/lib/plugin.js | 4 +-- packages/lib/users.js | 5 ++++ packages/slave/src/Instance.js | 28 +++++++++++++++++++ .../src/components/InstanceViewPage.jsx | 13 +++++++++ test/integration/clusterio.js | 7 +++++ 8 files changed, 86 insertions(+), 3 deletions(-) diff --git a/docs/managing-a-cluster.md b/docs/managing-a-cluster.md index dca36cff..28c03a39 100644 --- a/docs/managing-a-cluster.md +++ b/docs/managing-a-cluster.md @@ -50,6 +50,16 @@ Terminates the running Factorio server without giving it a chance to save or cle Note: This may cause loss of data. +### Extract players from instance + + ctl> instance extract-players + +Creates a user account in the cluster for each player that has been online on the currently running save of the instance and sets the play time of that account on this instance to the online time recorded in the save. + +Useful when importing a save to a cluster. +Note that the online time in a save is recorded differntly from how Clusterio records online time. +Most notably Clusterio records wall clock time while the save time is recorded in ticks and this extraction of player time assumes the game runs at 60 ticks a second. + ### Config management diff --git a/packages/ctl/src/commands.js b/packages/ctl/src/commands.js index 7c7fa270..06f83956 100644 --- a/packages/ctl/src/commands.js +++ b/packages/ctl/src/commands.js @@ -748,6 +748,19 @@ instanceCommands.add(new libCommand.Command({ }, })); +instanceCommands.add(new libCommand.Command({ + definition: ["extract-players ", "Extract players from running save into the cluster.", (yargs) => { + yargs.positional("instance", { describe: "Instance to extract players and play time from", type: "string" }); + }], + handler: async function(args, control) { + let instanceId = await libCommand.resolveInstance(control, args.instance); + await control.setLogSubscriptions({ instance_ids: [instanceId] }); + await libLink.messages.extractPlayers.send(control, { + instance_id: instanceId, + }); + }, +})); + instanceCommands.add(new libCommand.Command({ definition: ["start ", "Start instance", (yargs) => { yargs.positional("instance", { describe: "Instance to start", type: "string" }); diff --git a/packages/lib/link/messages.js b/packages/lib/link/messages.js index 6028c31e..cd0a00c8 100644 --- a/packages/lib/link/messages.js +++ b/packages/lib/link/messages.js @@ -668,6 +668,13 @@ messages.exportData = new Request({ forwardTo: "instance", }); +messages.extractPlayers = new Request({ + type: "extract_players", + links: ["control-master", "master-slave", "slave-instance"], + permission: "core.instance.extract_players", + forwardTo: "instance", +}); + messages.stopInstance = new Request({ type: "stop_instance", links: ["control-master", "master-slave", "slave-instance"], @@ -1331,7 +1338,7 @@ messages.playerEvent = new Event({ eventRequired: ["instance_id", "type", "name"], eventProperties: { "instance_id": { type: "integer" }, - "type": { type: "string", enum: ["join", "leave"] }, + "type": { type: "string", enum: ["join", "leave", "import"] }, "name": { type: "string" }, "reason": { type: "string" }, "stats": PlayerStats.jsonSchema, diff --git a/packages/lib/plugin.js b/packages/lib/plugin.js index 162794d4..9a1cc25f 100644 --- a/packages/lib/plugin.js +++ b/packages/lib/plugin.js @@ -206,7 +206,7 @@ class BaseInstancePlugin { * Invoked when a player either joins or leaves the instance. * * @param {Object} event - Information about the event. - * @param {string} event.type - Either "join" or "leave". + * @param {string} event.type - Either "join", "leave" or "import". * @param {string} event.name - Name of the player that joined/left. * @param {string=} event.reason - * Only present for type "leave". Reason for player leaving the @@ -502,7 +502,7 @@ class BaseMasterPlugin { * * @param {Object} instance - the instance it occured on. * @param {Object} event - Information about the event. - * @param {string} event.type - Either "join" or "leave". + * @param {string} event.type - Either "join", "leave" or "import". * @param {string} event.name - Name of the player that joined/left. * @param {string=} event.reason - * Only present for type "leave". Reason for player leaving the diff --git a/packages/lib/users.js b/packages/lib/users.js index c1f72afd..2ccc702b 100644 --- a/packages/lib/users.js +++ b/packages/lib/users.js @@ -475,6 +475,11 @@ definePermission({ title: "Export locale and icons", description: "Export the the locale and icons from an instance and upload it to the master.", }); +definePermission({ + name: "core.instance.extract_players", + title: "Extract player stats from running save", + description: "Run extraction to create a user for each player in the save and set the play time from the save.", +}); definePermission({ name: "core.instance.start", title: "Start instance", diff --git a/packages/slave/src/Instance.js b/packages/slave/src/Instance.js index a3b6fb6a..a5a717a6 100644 --- a/packages/slave/src/Instance.js +++ b/packages/slave/src/Instance.js @@ -370,6 +370,34 @@ class Instance extends libLink.Link { libPlugin.invokeHook(this.plugins, "onPlayerEvent", event); } + async extractPlayersRequestHandler() { + const exportPlayerTimes = `/sc +local players = {} +for _, p in pairs(game.players) do + players[p.name] = p.online_time +end +rcon.print(game.table_to_json(players))`.replace(/\r?\n/g, " "); + let playerTimes = JSON.parse(await this.sendRcon(exportPlayerTimes)); + + for (let [name, onlineTimeTicks] of Object.entries(playerTimes)) { + let stats = this.playerStats.get(name); + if (!stats) { + stats = new PlayerStats(); + this.playerStats.set(name, stats); + } + stats.onlineTimeMs = onlineTimeTicks * 1000 / 60; + + let event = { + instance_id: this.id, + type: "import", + name, + stats: stats.toJSON(), + }; + libLink.messages.playerEvent.send(this, event); + libPlugin.invokeHook(this.plugins, "onPlayerEvent", event); + } + } + async sendRcon(message, expectEmpty, plugin = "") { let instanceId = String(this.id); let observeDuration = instanceRconCommandDuration.labels(instanceId).startTimer(); diff --git a/packages/web_ui/src/components/InstanceViewPage.jsx b/packages/web_ui/src/components/InstanceViewPage.jsx index e544bb60..e0eb3ff3 100644 --- a/packages/web_ui/src/components/InstanceViewPage.jsx +++ b/packages/web_ui/src/components/InstanceViewPage.jsx @@ -100,6 +100,13 @@ export default function InstanceViewPage(props) { label: "Export data", }); } + if (account.hasPermission("core.instance.extract_players")) { + instanceButtonMenuItems.push({ + disabled: instance["status"] !== "running", + key: "extract", + label: "Extract players from save", + }); + } if (account.hasPermission("core.instance.kill")) { instanceButtonMenuItems.push({ disabled: ["unknown", "unassigned", "stopped"].includes(instance["status"]), @@ -132,6 +139,11 @@ export default function InstanceViewPage(props) { setExportingData(false); }); + } else if (key === "extract") { + libLink.messages.extractPlayers.send( + control, { instance_id: instanceId } + ).catch(notifyErrorHandler("Error extracting player data")); + } else if (key === "kill") { libLink.messages.killInstance.send( control, { instance_id: instanceId } @@ -162,6 +174,7 @@ export default function InstanceViewPage(props) { {account.hasPermission("core.instance.load_scenario") && } {account.hasAnyPermission( "core.instance.export_data", + "core.instance.extract_players", "core.instance.kill", "core.instance.delete", ) && diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js index aab1af97..375c1eca 100644 --- a/test/integration/clusterio.js +++ b/test/integration/clusterio.js @@ -524,6 +524,13 @@ describe("Integration of Clusterio", function() { }); }); + describe("instance extract-players", function() { + it("runs", async function() { + slowTest(this); + await execCtl("instance extract-players test"); + }); + }); + describe("instance stop", function() { it("stops the instance", async function() { slowTest(this);