Merge pull request #354 from Hornwitser/improve-config

Improve config

docs/configuration.md

@@ -5,6 +5,14 @@ Configuration
 Master Configuration
 --------------------
 
+### master.name
+
+Name of the cluster.  Used to distinguish it from other clusters and is
+prepended to the name of servers in the server list.
+
+Defaluts to "Your Cluster".
+
+
 ### master.database_directory
 
 Directory used to store cluster data to.  Needs to be writeable by the

docs/writing-plugins.md

@@ -381,8 +381,10 @@ the event, for example the following could be defined in `info.js`:
             type: "foo_frobber:start_frobnication",
             links: ["master-slave", "slave-instance"],
             forwardTo: "instance",
+            eventRequired: ["frobnication_type"],
             eventProperties: {
                 "frobnication_type": { type: "string" },
+                "urgent": { type: "boolean" },
             },
         }),
     },
@@ -390,8 +392,8 @@ the event, for example the following could be defined in `info.js`:
 This specifies an event that can be sent from the master to a slave,
 and from a slave to an instance.  It also specifies that the event must
 contain the property `frobnication_type`, with a string value in the
-data payload.  It will also be forwarded by slaves to a specific
-instance.
+data payload and that it may optionally contain a boolean `urgent`
+property.  It will also be forwarded by slaves to a specific instance.
 
 The following properties are recognized by the Event constructor:
 
@@ -434,12 +436,19 @@ it's sent to, but not back from where it came from. Currently, only
 that slave except for the instance it came from.  from an instance will
 cause it to be broadcast to all instances of
 
+#### eventRequired
+
+By default all properties defined in `eventProperties` are required to
+be present in the payload for the event.  This can be overriden by
+specifying an array of properties which are required here.  This is
+equivalent to using the `required` keyword in JSON schema.
+
 #### eventProperties
 
 Object with properties mapping to a JSON schema of that property that
 specifies what's valid to send in the event.  This is equivalent to
 using the `properties` keyword in JSON schema, except that the
-properties specified are implicitly required and additional properties
+properties specified are by default required and additional properties
 are not allowed.  See [this guide][guide] for an introduction to writing
 JSON schemas.
 
@@ -466,8 +475,10 @@ the event. For example, the following could be defined in `info.js`:
             type: "foo_frobber:report_frobnication",
             links: ["master-slave", "slave-instance"],
             forwardTo: "instance",
+            requestRequired: ["verbosity"],
             requestProperties: {
                 "verbosity": { type: "integer" },
+                "special": { type: "boolean" },
             },
             responseProperties: {
                 "report": {
@@ -481,7 +492,8 @@ the event. For example, the following could be defined in `info.js`:
 This specifies a request that can be sent from the master to a slave,
 and from a slave to an instance.  The request data must contain the
 property `verbosity` with an integer number as the value, as well as the
-`instance_id` property (implied by `forwardTo: "instance"`), and the
+`instance_id` property (implied by `forwardTo: "instance"`) and it may
+also contain a boolean `special` property.  It also defines that the
 response sent must contain a `report` property mapping to an array of
 strings.  When received by a slave, it will also be forwarded to the
 instance specified by `instance_id`.
@@ -520,15 +532,27 @@ instance, or `"instance"` to indicate it should be forwarded to the
 instances specified by the `instance_id` request property.  This works
 by using a default handler for the request by the links that forward it.
 
+#### requestRequired
+
+By default all properties defined in `requestProperties` are required to
+be present in the payload for the request.  This can be overriden by
+specifying an array of properties which are required here.  This is
+equivalent to using the `required` keyword in JSON schema.
+
 #### requestProperties
 
 Object with properties mapping to a JSON schema of that property that
 specifies what's valid to send in the request.  This is equivalent to
 using the `properties` keyword in JSON schema, except that the
-properties specified are implicitly required and additional properties
+properties specified are by default required and additional properties
 are not allowed.  See [this guide][guide] for an introduction to writing
 JSON schemas
 
+#### responseRequired
+
+Same as the requestRequired only for the response sent back by the
+target.
+
 #### responseProperties
 
 Same as the requestProperties only for the response sent back by the

packages/ctl/ctl.js

@@ -23,6 +23,7 @@ const libCommand = require("@clusterio/lib/command");
 const libSharedCommands = require("@clusterio/lib/shared_commands");
 const { ConsoleTransport, levels, logger } = require("@clusterio/lib/logging");
 const libLoggingUtils = require("@clusterio/lib/logging_utils");
+const libHelpers = require("@clusterio/lib/helpers");
 
 
 function print(...content) {
@@ -134,13 +135,21 @@ instanceConfigCommands.add(new libCommand.Command({
 }));
 
 instanceConfigCommands.add(new libCommand.Command({
-	definition: ["set <instance> <field> <value>", "Set field in instance config", (yargs) => {
+	definition: ["set <instance> <field> [value]", "Set field in instance config", (yargs) => {
 		yargs.positional("instance", { describe: "Instance to set config on", type: "string" });
 		yargs.positional("field", { describe: "Field to set", type: "string" });
 		yargs.positional("value", { describe: "Value to set", type: "string" });
+		yargs.options({
+			"stdin": { describe: "read value from stdin", nargs: 0, type: "boolean" },
+		});
 	}],
 	handler: async function(args, control) {
 		let instanceId = await libCommand.resolveInstance(control, args.instance);
+		if (args.stdin) {
+			args.value = (await libHelpers.readStream(process.stdin)).toString().replace(/\r?\n$/, "");
+		} else if (args.value === undefined) {
+			args.value = "";
+		}
 		await libLink.messages.setInstanceConfigField.send(control, {
 			instance_id: instanceId,
 			field: args.field,
@@ -150,20 +159,49 @@ instanceConfigCommands.add(new libCommand.Command({
 }));
 
 instanceConfigCommands.add(new libCommand.Command({
-	definition: ["set-prop <instance> <field> <prop> <value>", "Set property of field in instance config", (yargs) => {
+	definition: ["set-prop <instance> <field> <prop> [value]", "Set property of field in instance config", (yargs) => {
 		yargs.positional("instance", { describe: "Instance to set config on", type: "string" });
 		yargs.positional("field", { describe: "Field to set", type: "string" });
 		yargs.positional("prop", { describe: "Property to set", type: "string" });
 		yargs.positional("value", { describe: "JSON parsed value to set", type: "string" });
+		yargs.options({
+			"stdin": { describe: "read value from stdin", nargs: 0, type: "boolean" },
+		});
 	}],
 	handler: async function(args, control) {
 		let instanceId = await libCommand.resolveInstance(control, args.instance);
-		await libLink.messages.setInstanceConfigProp.send(control, {
+		if (args.stdin) {
+			args.value = (await libHelpers.readStream(process.stdin)).toString().replace(/\r?\n$/, "");
+		}
+		let request = {
 			instance_id: instanceId,
 			field: args.field,
 			prop: args.prop,
-			value: JSON.parse(args.value),
-		});
+		};
+		try {
+			if (args.value !== undefined) {
+				request.value = JSON.parse(args.value);
+			}
+		} catch (err) {
+			// If this is from stdin or looks like an array, object or string
+			// literal throw the parse error, otherwise assume this is a string.
+			// The resoning behind this is that correctly quoting the string
+			// with the all the layers of quote removal at play is difficult.
+			// See the following table for how to pass "That's a \" quote" in
+			// different environments:
+			// cmd              : """""That's a \\"" quote"""""
+			// cmd + npx        : """""""""""That's a \\\\"""" quote"""""""""""
+			// PowerShell       : '"""""That''s a \\"" quote"""""'
+			// PowerShell + npx : '"""""""""""That''s a \\\\"""" quote"""""""""""'
+			// bash             : '""That'\''s a \" quote""'
+			// bash + npx       : '""That'\''s a \" quote""'
+			// bash + npx -s sh : "'\"\"That'\\''s a \\\" quote\"\"'"
+			if (args.stdin || /^(\[.*]|{.*}|".*")$/.test(args.value)) {
+				throw new libErrors.CommandError(`In parsing value '${args.value}': ${err.message}`);
+			}
+			request.value = args.value;
+		}
+		await libLink.messages.setInstanceConfigProp.send(control, request);
 	},
 }));
 instanceCommands.add(instanceConfigCommands);

packages/lib/config/classes.js

@@ -448,7 +448,11 @@ class ConfigGroup {
 		let prev = this._fields.get(name);
 		let updated = {...prev || {}};
 
-		updated[prop] = value;
+		if (value !== undefined) {
+			updated[prop] = value;
+		} else {
+			delete updated[prop];
+		}
 		this._fields.set(name, updated);
 		if (this._config && !isDeepStrictEqual(updated, prev)) {
 			this._config.emit("fieldChanged", this, name, prev);

packages/lib/config/definitions.js

@@ -11,6 +11,13 @@ const classes = require("./classes");
  */
 class MasterGroup extends classes.ConfigGroup { }
 MasterGroup.groupName = "master";
+MasterGroup.define({
+	name: "name",
+	title: "Name",
+	description: "Name of the cluster",
+	type: "string",
+	initial_value: "Your Cluster",
+});
 MasterGroup.define({
 	name: "database_directory",
 	title: "Database directory",
@@ -262,10 +269,7 @@ FactorioGroup.define({
 	name: "settings",
 	description: "Settings overridden in server-settings.json",
 	type: "object",
-	initial_value: {
-		"tags": ["clusterio"],
-		"auto_pause": false,
-	},
+	initial_value: {}, // See create instance handler in master.
 });
 FactorioGroup.define({
 	name: "verbose_logging",

packages/lib/helpers.js

@@ -52,8 +52,32 @@ async function timeout(promise, time, timeoutResult) {
 	}
 }
 
+
+/**
+ * Read stream to the end and return its content
+ *
+ * Reads the stream given asynchronously until the end is reached and
+ * returns all the data which was read from the stream.
+ *
+ * @param {Readable} stream - byte stream to read to the end.
+ * @returns {Buffer} content of the stream.
+ */
+async function readStream(stream) {
+	let chunks = [];
+	for await (let chunk of stream) {
+		// Support using ^Z to end input on Windows
+		if (process.platform === "win32" && stream.isTTY && chunk.toString() === "\x1a\r\n") {
+			break;
+		}
+		chunks.push(chunk);
+	}
+	return Buffer.concat(chunks);
+}
+
+
 module.exports = {
 	basicType,
 	wait,
 	timeout,
+	readStream,
 };

packages/lib/link/messages.js

@@ -70,16 +70,24 @@ class Request extends Message {
 	 *     Optional target to forward this request to.  'instance' add
 	 *     instance_id into the requestProperties and forward to the given
 	 *     instance.  'master' forwards it to the master server.
+	 * @param {?Array<string>} requestRequired -
+	 *     List of properties that are required to be present in the data
+	 *     payload of this request.  Defaults to all.
 	 * @param {Object<string, Object>} requestProperties -
 	 *     Mapping of property values to JSON schema specifications for the
 	 *     properties that are valid in the data payload of this requst.
+	 * @param {?Array<string>} responseRequired -
+	 *     List of properties that are required to be present in the data
+	 *     payload of the repsonse to this request.  Defaults to all.
 	 * @param {Object<string, Object>} responseProperties -
 	 *     Mapping of property values to JSON schema specifications for the
 	 *     properties that are valid in the data payload of the response to
 	 *     this requst.
 	 */
 	constructor({
-		type, permission, links, forwardTo = null, requestProperties = {}, responseProperties = {},
+		type, permission, links, forwardTo = null,
+		requestRequired = null, requestProperties = {},
+		responseRequired = null, responseProperties = {},
 	}) {
 		super();
 		this.type = type;
@@ -98,7 +106,12 @@ class Request extends Message {
 			throw new Error(`permission is not allowed on ${this.type} request as it is not over control-master link`);
 		}
 
+		if (!requestRequired) {
+			requestRequired = Object.keys(requestProperties);
+		}
+
 		if (forwardTo === "instance") {
+			requestRequired = ["instance_id", ...requestRequired];
 			requestProperties = {
 				"instance_id": { type: "integer" },
 				...requestProperties,
@@ -114,12 +127,16 @@ class Request extends Message {
 				"type": { const: this.requestType },
 				"data": {
 					additionalProperties: false,
-					required: Object.keys(requestProperties),
+					required: requestRequired,
 					properties: requestProperties,
 				},
 			},
 		});
 
+		if (!responseRequired) {
+			responseRequired = Object.keys(responseProperties);
+		}
+
 		this._responseValidator = libSchema.compile({
 			$schema: "http://json-schema.org/draft-07/schema#",
 			properties: {
@@ -128,7 +145,7 @@ class Request extends Message {
 					anyOf: [
 						{
 							additionalProperties: false,
-							required: ["seq", ...Object.keys(responseProperties)],
+							required: ["seq", ...responseRequired],
 							properties: {
 								"seq": { type: "integer" },
 								...responseProperties,
@@ -390,6 +407,7 @@ messages.setInstanceConfigProp = new Request({
 	type: "set_instance_config_prop",
 	links: ["control-master"],
 	permission: "core.instance.update_config",
+	requestRequired: ["instance_id", "field", "prop"],
 	requestProperties: {
 		"instance_id": { type: "integer" },
 		"field": { type: "string" },
@@ -760,11 +778,14 @@ class Event extends Message {
 	 *     links that are downstream, so sending an event set to instance
 	 *     broadcast that's sent to a slave will only be broadcast to that
 	 *     slave's instances.
+	 * @param {?Array<string>} eventRequired -
+	 *     List of properties required to be present in the event payload.
+	 *     Defaults to all.
 	 * @param {Object<string, Object>} eventProperties -
 	 *     Mapping of property values to JSON schema specifications for the
 	 *     properties that are valid in the data payload of this event.
 	 */
-	constructor({ type, links, forwardTo = null, broadcastTo = null, eventProperties = {} }) {
+	constructor({ type, links, forwardTo = null, broadcastTo = null, eventRequired = null, eventProperties = {} }) {
 		super();
 		this.type = type;
 		this.links = links;
@@ -772,7 +793,12 @@ class Event extends Message {
 		this.broadcastTo = broadcastTo;
 		this.handlerSuffix = "EventHandler";
 
+		if (!eventRequired) {
+			eventRequired = Object.keys(eventProperties);
+		}
+
 		if (forwardTo === "instance") {
+			eventRequired = ["instance_id", ...eventRequired];
 			eventProperties = {
 				"instance_id": { type: "integer" },
 				...eventProperties,
@@ -793,7 +819,7 @@ class Event extends Message {
 				"type": { const: this.eventType },
 				"data": {
 					additionalProperties: false,
-					required: Object.keys(eventProperties),
+					required: eventRequired,
 					properties: eventProperties,
 				},
 			},

packages/lib/shared_commands.js

@@ -8,6 +8,7 @@ const path = require("path");
 
 const libConfig = require("@clusterio/lib/config");
 const { logger } = require("@clusterio/lib/logging");
+const libHelpers = require("@clusterio/lib/helpers");
 
 
 function print(...content) {
@@ -100,7 +101,11 @@ async function handlePluginCommand(args, pluginList, pluginListPath) {
  */
 function configCommand(yargs) {
 	yargs
-		.command("set <field> [value]", "Set config field")
+		.command("set <field> [value]", "Set config field", yargs => {
+			yargs.options({
+				"stdin": { describe: "read value from stdin", nargs: 0, type: "boolean" },
+			});
+		})
 		.command("show <field>", "Show value of the given config field")
 		.command("list", "List all configuration fields and their values")
 		.demandCommand(1, "You need to specify a command to run")
@@ -141,7 +146,10 @@ async function handleConfigCommand(args, instance, configPath) {
 		}
 
 	} else if (command === "set") {
-		if (args.value === undefined) {
+		if (args.stdin) {
+			args.value = (await libHelpers.readStream(process.stdin)).toString().replace(/\r?\n$/, "");
+
+		} else if (args.value === undefined) {
 			args.value = null;
 		}
 

packages/master/master.js

@@ -741,6 +741,33 @@ class ControlConnection extends BaseConnection {
 		if (db.instances.has(instanceId)) {
 			throw new libErrors.RequestError(`Instance with ID ${instanceId} already exists`);
 		}
+
+		// Add common settings for the Factorio server
+		let settings = {
+			"name": `${masterConfig.get("master.name")} - ${instanceConfig.get("instance.name")}`,
+			"description": `Clusterio instance for ${masterConfig.get("master.name")}`,
+			"tags": ["clusterio"],
+			"max_players": 0,
+			"visibility": { "public": true, "lan": true },
+			"username": "",
+			"token": "",
+			"game_password": "",
+			"require_user_verification": true,
+			"max_upload_in_kilobytes_per_second": 0,
+			"max_upload_slots": 5,
+			"ignore_player_limit_for_returning_players": false,
+			"allow_commands": "admins-only",
+			"autosave_interval": 10,
+			"autosave_slots": 5,
+			"afk_autokick_interval": 0,
+			"auto_pause": false,
+			"only_admins_can_pause_the_game": true,
+			"autosave_only_on_server": true,
+
+			...instanceConfig.get("factorio.settings"),
+		};
+		instanceConfig.set("factorio.settings", settings);
+
 		db.instances.set(instanceId, { config: instanceConfig, status: "unassigned" });
 	}
 

packages/web_ui/src/components/InstanceViewPage.jsx

@@ -359,6 +359,23 @@ function InstanceConfigTree(props) {
 	}
 
 	let initialValues = {};
+	function onValuesChange(changedValues) {
+		let newChangedFields = new Set(changedFields);
+		let changed = false;
+		for (let [key, value] of Object.entries(changedValues)) {
+			if (initialValues[key] === value) {
+				changed = newChangedFields.delete(key) || changed;
+			} else {
+				changed = !newChangedFields.has(key) || changed;
+				newChangedFields.add(key);
+			}
+		}
+
+		if (changed) {
+			setChangedFields(newChangedFields);
+		}
+	}
+
 	let treeData = [];
 	let propsMap = new Map();
 	for (let [name, GroupClass] of libConfig.InstanceConfig.groups) {
@@ -387,6 +404,9 @@ function InstanceConfigTree(props) {
 							name={propPath}
 							label={`"${prop}"`}
 							rules={[{ validator: (_, fieldValue) => {
+								if (!fieldValue.length) {
+									return Promise.reject(new Error("Will be removed"));
+								}
 								try {
 									JSON.parse(fieldValue);
 								} catch (err) {
@@ -403,6 +423,49 @@ function InstanceConfigTree(props) {
 					propsMap.set(propPath, [fieldName, prop]);
 				}
 
+				let newPropPath = `${group.name}.${def.name}:add`;
+				childNode.children.push({
+					key: newPropPath,
+					title: <Space>
+						<Form.Item
+							name={`${newPropPath}.name`}
+							noStyle={true}
+						>
+							<Input />
+						</Form.Item>:
+						<Form.Item
+							name={`${newPropPath}.value`}
+							noStyle={true}
+						>
+							<Input className="json-input" />
+						</Form.Item>
+						<Button
+							size="small"
+							onClick={async () => {
+								let propName = form.getFieldValue(`${newPropPath}.name`);
+								let propValue = form.getFieldValue(`${newPropPath}.value`);
+								if (!Object.prototype.hasOwnProperty.call(value, propName)) {
+									let newConfig = new libConfig.InstanceConfig();
+									await newConfig.load(config.serialize());
+									newConfig.setProp(fieldName, propName, null);
+									setConfig(newConfig);
+								}
+
+								let propPath = `${group.name}.${def.name}.${propName}`;
+								form.setFieldsValue({
+									[`${newPropPath}.name`]: "",
+									[`${newPropPath}.value`]: "",
+									[propPath]: propValue,
+								});
+								form.validateFields([propPath]);
+								onValuesChange({ [propPath]: propValue });
+							}}
+						>
+							Add
+						</Button>
+					</Space>,
+				});
+
 			} else {
 				childNode.title = <Form.Item
 					name={fieldName}
@@ -423,23 +486,6 @@ function InstanceConfigTree(props) {
 		treeData.push(treeNode);
 	}
 
-	function onValuesChange(changedValues) {
-		let newChangedFields = new Set(changedFields);
-		let changed = false;
-		for (let [key, value] of Object.entries(changedValues)) {
-			if (initialValues[key] === value) {
-				changed = newChangedFields.delete(key) || changed;
-			} else {
-				changed = !newChangedFields.has(key) || changed;
-				newChangedFields.add(key);
-			}
-		}
-
-		if (changed) {
-			setChangedFields(newChangedFields);
-		}
-	}
-
 	return <>
 		<Title level={5} style={{ marginTop: 16 }}>
 			Config
@@ -455,18 +501,20 @@ function InstanceConfigTree(props) {
 							let request;
 							if (propsMap.has(field)) {
 								let [fieldName, prop] = propsMap.get(field);
-								try {
-									value = JSON.parse(value);
-								} catch (err) {
-									continue;
-								}
-
-								request = libLink.messages.setInstanceConfigProp.send(control, {
+								let data = {
 									instance_id: props.id,
 									field: fieldName,
 									prop,
-									value,
-								});
+								};
+								if (value) {
+									try {
+										data.value = JSON.parse(value);
+									} catch (err) {
+										continue;
+									}
+								}
+
+								request = libLink.messages.setInstanceConfigProp.send(control, data);
 
 							} else {
 								request = libLink.messages.setInstanceConfigField.send(control, {

test/lib/config/classes.js

@@ -397,6 +397,12 @@ describe("lib/config/classes", function() {
 				testInstance.setProp("json", "test", true);
 				assert.deepEqual(testInstance.get("json"), { test: true });
 			});
+
+			it("should unset field if passed undefined", function() {
+				testInstance.set("json", { test: true, extra: "yes" });
+				testInstance.setProp("json", "extra", undefined);
+				assert.deepEqual(testInstance.get("json"), { test: true });
+			});
 		});
 	});
 

test/lib/link/messages.js

@@ -17,11 +17,15 @@ describe("lib/link/messages", function() {
 		let testRequest = new libLink.Request({
 			type: "test",
 			links: ["source-target"],
+			requestRequired: ["test"],
 			requestProperties: {
 				"test": { type: "string" },
+				"optional": { type: "number" },
 			},
+			responseRequired: ["test"],
 			responseProperties: {
 				"test": { type: "string" },
+				"optional": { type: "number" },
 			},
 		});
 
@@ -127,8 +131,10 @@ describe("lib/link/messages", function() {
 		let testEvent = new libLink.Event({
 			type: "test",
 			links: ["source-target"],
+			eventRequired: ["test"],
 			eventProperties: {
 				"test": { type: "string" },
+				"optional": { type: "number" },
 			},
 		});