From f37c387d52577455edee2d13950bb3f1d78015c8 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Wed, 9 Jun 2021 13:10:17 +0200 Subject: [PATCH 1/4] Fix instance deleted status not being sent out --- packages/lib/link/messages.js | 2 +- packages/master/src/ControlConnection.js | 2 +- test/integration/clusterio.js | 7 ++++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/lib/link/messages.js b/packages/lib/link/messages.js index 72dbe3a8..6ba0a8a4 100644 --- a/packages/lib/link/messages.js +++ b/packages/lib/link/messages.js @@ -374,7 +374,7 @@ let instanceProperties = { "assigned_slave": { type: ["null", "integer"] }, "status": { enum: [ "unknown", "unassigned", "stopped", "starting", "running", "stopping", - "creating_save", "exporting_data", + "creating_save", "exporting_data", "deleted", ]}, }; diff --git a/packages/master/src/ControlConnection.js b/packages/master/src/ControlConnection.js index ad53cb0b..ddf42e83 100644 --- a/packages/master/src/ControlConnection.js +++ b/packages/master/src/ControlConnection.js @@ -252,7 +252,7 @@ class ControlConnection extends BaseConnection { let prev = instance.status; instance.status = "deleted"; - this.instanceUpdated(instance); + this._master.instanceUpdated(instance); await libPlugin.invokeHook(this._master.plugins, "onInstanceStatusChanged", instance, prev); } diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js index 474c86e4..990c52ec 100644 --- a/test/integration/clusterio.js +++ b/test/integration/clusterio.js @@ -501,9 +501,10 @@ describe("Integration of Clusterio", function() { describe("instanceUpdateEventHandler()", function() { it("should have triggered for the previous instance status updates", function() { - let statusesToCheck = new Set( - ["unassigned", "stopped", "creating_save", "exporting_data", "starting", "running", "stopping"] - ); + let statusesToCheck = new Set([ + "unassigned", "stopped", "creating_save", "exporting_data", + "starting", "running", "stopping", "deleted", + ]); let statusesNotSeen = new Set(statusesToCheck); for (let update of getControl().instanceUpdates) { From 0e9893fcb685627cc692ca5f789db9fbad89d055 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Wed, 9 Jun 2021 13:12:56 +0200 Subject: [PATCH 2/4] Fix create save modal Fix the wrong variable being passed as mapSettings and clear the form after successfully creating the save. --- packages/web_ui/src/components/CreateSaveModal.jsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/web_ui/src/components/CreateSaveModal.jsx b/packages/web_ui/src/components/CreateSaveModal.jsx index f067635b..10907779 100644 --- a/packages/web_ui/src/components/CreateSaveModal.jsx +++ b/packages/web_ui/src/components/CreateSaveModal.jsx @@ -43,7 +43,7 @@ export default function CreateSaveModal(props) { } if (values.mapSettings && values.mapSettings.trim()) { try { - mapSettings = JSON.parse(values.mapGenSettings); + mapSettings = JSON.parse(values.mapSettings); } catch (err) { form.setFields([{ name: "mapSettings", errors: [err.message] }]); return; @@ -59,6 +59,7 @@ export default function CreateSaveModal(props) { map_gen_settings: mapGenSettings, map_settings: mapSettings, }).then(() => { + form.resetFields(); setVisible(false); }).catch( notifyErrorHandler("Error creating save") From 82c3d27bff591774e4b8225b93c66b05942ed0d5 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Wed, 9 Jun 2021 13:15:16 +0200 Subject: [PATCH 3/4] Rewrite instances page Render the table in the instances page using an antd Table component directly and feed it the data via an automatically updating list of instances. --- .../web_ui/src/components/InstancesPage.jsx | 114 +++++++++++++----- packages/web_ui/src/index.css | 4 + packages/web_ui/src/model/instance.jsx | 41 +++++++ packages/web_ui/src/util/websocket.jsx | 13 +- 4 files changed, 136 insertions(+), 36 deletions(-) diff --git a/packages/web_ui/src/components/InstancesPage.jsx b/packages/web_ui/src/components/InstancesPage.jsx index 0d4f9031..bab926ba 100644 --- a/packages/web_ui/src/components/InstancesPage.jsx +++ b/packages/web_ui/src/components/InstancesPage.jsx @@ -1,52 +1,104 @@ -import React, { useContext } from "react"; +import React, { useContext, useState } from "react"; import { useHistory } from "react-router-dom"; +import { Button, Form, Input, Modal, PageHeader, Table } from "antd"; import libLink from "@clusterio/lib/link"; import libConfig from "@clusterio/lib/config"; -import DataTable from "./data-table"; import ControlContext from "./ControlContext"; import PageLayout from "./PageLayout"; +import { useInstanceList } from "../model/instance"; +import { notifyErrorHandler } from "../util/notify"; +import { useSlaveList } from "../model/slave"; -export default function InstancesPage() { +function CreateInstanceButton(props) { let control = useContext(ControlContext); let history = useHistory(); + let [visible, setVisible] = useState(false); + let [form] = Form.useForm(); - async function listInstances() { - let result = await libLink.messages.listInstances.send(control); - return result["list"].map(item => ({ - key: item["id"], - "Name": item["name"], - "Assigned Slave": item["assigned_slave"], - "Status": item["status"], - })); + async function createInstance() { + let values = form.getFieldsValue(); + if (!values.instanceName) { + form.setFields([{ name: "mapGenSettings", errors: ["Name is required"] }]); + } + + let instanceConfig = new libConfig.InstanceConfig("control"); + await instanceConfig.init(); + instanceConfig.set("instance.name", values.instanceName); + let serialized_config = instanceConfig.serialize("master"); + let result = await libLink.messages.createInstance.send(control, { serialized_config }); + setVisible(false); + history.push(`/instances/${instanceConfig.get("instance.id")}/view`); } + return <> + + { createInstance().catch(notifyErrorHandler("Error creating instance")); }} + onCancel={() => { setVisible(false); }} + destroyOnClose + > +
+ + + +
+ + ; +} + +export default function InstancesPage() { + let history = useHistory(); + let [slaveList] = useSlaveList(); + let [instanceList] = useInstanceList(); + return -

Instances

- + /> + + { - let instanceConfig = new libConfig.InstanceConfig("control"); - await instanceConfig.init(); - instanceConfig.set("instance.name", args.name); - let serialized_config = instanceConfig.serialize("master"); - let response = await libLink.messages.createInstance.send(control, { serialized_config }); + dataIndex: "name", }, - }} - TableProps={{ - onRow: (record, rowIndex) => ({ - onClick: event => { - history.push(`/instances/${record.key}/view`); + { + title: "Assigned Slave", + key: "assigned_slave", + render: instance => { + let slave = slaveList.find(s => s.id === instance.assigned_slave); + if (slave) { + return slave.name; + } + return instance.assigned_slave; }, - }), - }} + }, + { + title: "Status", + dataIndex: "status", + }, + ]} + dataSource={instanceList} + rowKey={instance => instance["id"]} + pagination={false} + onRow={(record, rowIndex) => ({ + onClick: event => { + history.push(`/instances/${record.id}/view`); + }, + })} /> ; } diff --git a/packages/web_ui/src/index.css b/packages/web_ui/src/index.css index 97d8a4a6..1129e846 100644 --- a/packages/web_ui/src/index.css +++ b/packages/web_ui/src/index.css @@ -67,6 +67,10 @@ body { padding-right: 64px; } +.site-page-header { + padding: 0 0 16px; +} + .section-header { display: flex; align-items: center; diff --git a/packages/web_ui/src/model/instance.jsx b/packages/web_ui/src/model/instance.jsx index 4048a135..5b027d3a 100644 --- a/packages/web_ui/src/model/instance.jsx +++ b/packages/web_ui/src/model/instance.jsx @@ -39,3 +39,44 @@ export function useInstance(id) { return [instance, updateInstance]; } + +export function useInstanceList() { + let control = useContext(ControlContext); + let [instanceList, setInstanceList] = useState([]); + + function updateInstanceList() { + libLink.messages.listInstances.send(control).then(result => { + setInstanceList(result.list); + }).catch(err => { + logger.error(`Failed to list instances:\n${err}`); + }); + } + + useEffect(() => { + updateInstanceList(); + + function updateHandler(newInstance) { + setInstanceList(oldList => { + let newList = oldList.concat(); + let index = newList.findIndex(s => s.id === newInstance.id); + if (newInstance.status !== "deleted") { + if (index !== -1) { + newList[index] = newInstance; + } else { + newList.push(newInstance); + } + } else if (index !== -1) { + newList.splice(index, 1); + } + return newList; + }); + } + + control.onInstanceUpdate(null, updateHandler); + return () => { + control.offInstanceUpdate(null, updateHandler); + }; + }, []); + + return [instanceList]; +} diff --git a/packages/web_ui/src/util/websocket.jsx b/packages/web_ui/src/util/websocket.jsx index 6b07fc65..6b3b983f 100644 --- a/packages/web_ui/src/util/websocket.jsx +++ b/packages/web_ui/src/util/websocket.jsx @@ -154,14 +154,17 @@ export class Control extends libLink.Link { } async instanceUpdateEventHandler(message) { - let handlers = this.instanceUpdateHandlers.get(message.data.id); - for (let handler of handlers || []) { + let handlers = [].concat( + this.instanceUpdateHandlers.get(null) || [], + this.instanceUpdateHandlers.get(message.data.id) || [], + ); + for (let handler of handlers) { handler(message.data); } }; async onInstanceUpdate(id, handler) { - if (!Number.isInteger(id)) { + if (id !== null && !Number.isInteger(id)) { throw new Error("Invalid instance id"); } @@ -202,8 +205,8 @@ export class Control extends libLink.Link { } await libLink.messages.setInstanceSubscriptions.send(this, { - all: false, - instance_ids: [...this.instanceUpdateHandlers.keys()], + all: this.instanceUpdateHandlers.has(null), + instance_ids: [...this.instanceUpdateHandlers.keys()].filter(e => e !== null), }); } From 9b34b949a62f90bb04127e94330eeeea5e0fb1e8 Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Wed, 9 Jun 2021 13:18:24 +0200 Subject: [PATCH 4/4] Accept invalid IDs in useSlave and useInstance Gracefully handle invalid IDs in useSlave and useInstance as this is necessary when when the ID isn't known by the component using the hook, since it can't conditionally call the hook. --- packages/web_ui/src/model/instance.jsx | 11 +++++------ packages/web_ui/src/model/slave.jsx | 9 ++++----- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/packages/web_ui/src/model/instance.jsx b/packages/web_ui/src/model/instance.jsx index 5b027d3a..ef4f27a2 100644 --- a/packages/web_ui/src/model/instance.jsx +++ b/packages/web_ui/src/model/instance.jsx @@ -10,21 +10,20 @@ export function useInstance(id) { let [instance, setInstance] = useState({ loading: true }); function updateInstance() { - if (!Number.isInteger(id)) { - setInstance({ missing: true }); - return; - } - libLink.messages.getInstance.send(control, { id }).then(result => { setInstance({ ...result, present: true }); }).catch(err => { - logger.log(`Failed to get instance: ${err}`); + logger.error(`Failed to get instance: ${err}`); setInstance({ missing: true }); }); } useEffect(() => { + if (!Number.isInteger(id)) { + setInstance({ missing: true }); + return undefined; + } updateInstance(); function updateHandler(newInstance) { diff --git a/packages/web_ui/src/model/slave.jsx b/packages/web_ui/src/model/slave.jsx index c29e7405..729e671a 100644 --- a/packages/web_ui/src/model/slave.jsx +++ b/packages/web_ui/src/model/slave.jsx @@ -9,11 +9,6 @@ export function useSlave(id) { let [slave, setSlave] = useState({ loading: true }); function updateSlave() { - if (!Number.isInteger(id)) { - setSlave({ missing: true }); - return; - } - // XXX optimize by requesting only the slave in question libLink.messages.listSlaves.send(control).then(result => { let match = result.list.find(i => i.id === id); @@ -26,6 +21,10 @@ export function useSlave(id) { } useEffect(() => { + if (!Number.isInteger(id)) { + setSlave({ missing: true }); + return undefined; + } updateSlave(); function updateHandler(newSlave) {