From b7c7e79a86b86236e2e0235b6204490728baef04 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Tue, 26 Jan 2021 11:42:20 +0100
Subject: [PATCH 1/9] Move distributed static/ folder to dist/web/

Move the target folder for the built web interface to dist/web/ instead
of static/.  This better follows the norm for npm packages to include
built files in the dist/ folder and also allows putting other built
files into that folder in the future.
---
 .eslintignore                                 | 3 +--
 .gitignore                                    | 4 ----
 packages/master/master.js                     | 6 +++---
 packages/master/webpack.config.js             | 4 ++--
 plugins/global_chat/webpack.config.js         | 2 +-
 plugins/research_sync/webpack.config.js       | 2 +-
 plugins/statistics_exporter/webpack.config.js | 2 +-
 plugins/subspace_storage/webpack.config.js    | 2 +-
 8 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/.eslintignore b/.eslintignore
index d3cfadef..849ddff3 100644
--- a/.eslintignore
+++ b/.eslintignore
@@ -1,2 +1 @@
-packages/master/static
-plugins/*/static
+dist/
diff --git a/.gitignore b/.gitignore
index 08f2a543..d93623fb 100644
--- a/.gitignore
+++ b/.gitignore
@@ -46,10 +46,6 @@ static/export
 saves
 /temp/
 
-# Web build
-/packages/master/static
-/plugins/*/static
-
 # IDE
 .idea/
 .vs/
diff --git a/packages/master/master.js b/packages/master/master.js
index dbc1ed1a..02e05286 100755
--- a/packages/master/master.js
+++ b/packages/master/master.js
@@ -115,7 +115,7 @@ function serveWeb(route) {
 }
 
 // Set folder to serve static content from (the website)
-app.use(express.static(path.join(__dirname, "static")));
+app.use(express.static(path.join(__dirname, "dist", "web")));
 app.use(express.static("static")); // Used for data export files
 
 const slaveMappingGauge = new libPrometheus.Gauge(
@@ -2025,8 +2025,8 @@ async function startServer(args) {
 		}
 
 		let pluginPackagePath = require.resolve(path.posix.join(pluginInfo.requirePath, "package.json"));
-		let staticPath = path.join(path.dirname(pluginPackagePath), "static");
-		app.use(`/plugin/${pluginInfo.name}`, express.static(staticPath));
+		let webPath = path.join(path.dirname(pluginPackagePath), "dist", "web");
+		app.use(`/plugin/${pluginInfo.name}`, express.static(webPath));
 	}
 
 	// Load plugins
diff --git a/packages/master/webpack.config.js b/packages/master/webpack.config.js
index 756d809d..8fca09c6 100644
--- a/packages/master/webpack.config.js
+++ b/packages/master/webpack.config.js
@@ -9,11 +9,11 @@ module.exports = (env = {}) => merge(common(env), {
 	context: __dirname,
 	entry: "./web/index.jsx",
 	devServer: {
-		contentBase: "./static",
+		contentBase: "./dist/web",
 	},
 	output: {
 		filename: "bundle.js",
-		path: path.resolve(__dirname, "static"),
+		path: path.resolve(__dirname, "dist", "web"),
 	},
 	plugins: [
 		new webpack.container.ModuleFederationPlugin({
diff --git a/plugins/global_chat/webpack.config.js b/plugins/global_chat/webpack.config.js
index 042295a4..60b98d7b 100644
--- a/plugins/global_chat/webpack.config.js
+++ b/plugins/global_chat/webpack.config.js
@@ -10,7 +10,7 @@ module.exports = (env = {}) => merge(common(env), {
 	entry: "./web/index.jsx",
 	output: {
 		filename: "bundle.js",
-		path: path.resolve(__dirname, "static"),
+		path: path.resolve(__dirname, "dist", "web"),
 	},
 	plugins: [
 		new webpack.container.ModuleFederationPlugin({
diff --git a/plugins/research_sync/webpack.config.js b/plugins/research_sync/webpack.config.js
index 4a519c63..484dfafe 100644
--- a/plugins/research_sync/webpack.config.js
+++ b/plugins/research_sync/webpack.config.js
@@ -10,7 +10,7 @@ module.exports = (env = {}) => merge(common(env), {
 	entry: "./web/index.jsx",
 	output: {
 		filename: "bundle.js",
-		path: path.resolve(__dirname, "static"),
+		path: path.resolve(__dirname, "dist", "web"),
 	},
 	plugins: [
 		new webpack.container.ModuleFederationPlugin({
diff --git a/plugins/statistics_exporter/webpack.config.js b/plugins/statistics_exporter/webpack.config.js
index 3198c71e..0da4c121 100644
--- a/plugins/statistics_exporter/webpack.config.js
+++ b/plugins/statistics_exporter/webpack.config.js
@@ -10,7 +10,7 @@ module.exports = (env = {}) => merge(common(env), {
 	entry: "./web/index.jsx",
 	output: {
 		filename: "bundle.js",
-		path: path.resolve(__dirname, "static"),
+		path: path.resolve(__dirname, "dist", "web"),
 	},
 	plugins: [
 		new webpack.container.ModuleFederationPlugin({
diff --git a/plugins/subspace_storage/webpack.config.js b/plugins/subspace_storage/webpack.config.js
index 4a7b4901..cfd034a8 100644
--- a/plugins/subspace_storage/webpack.config.js
+++ b/plugins/subspace_storage/webpack.config.js
@@ -10,7 +10,7 @@ module.exports = (env = {}) => merge(common(env), {
 	entry: "./web/index.jsx",
 	output: {
 		filename: "bundle.js",
-		path: path.resolve(__dirname, "static"),
+		path: path.resolve(__dirname, "dist", "web"),
 	},
 	plugins: [
 		new webpack.container.ModuleFederationPlugin({

From f8153bd7843bf83ce843e5b89ec8b5f112f688a8 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Tue, 26 Jan 2021 12:14:51 +0100
Subject: [PATCH 2/9] Remove redundant parameter from attachPluginMessages

Remove the pluginInfo parameter from lib/plugin attachPluginMessages as
the plugin info object is available in the `.info` property of the
plugin.  While a plugin instance doesn't exist for the slave side and
was the reason for these to be separate arguments, I expect slave
plugins to be implemented eventually.
---
 packages/ctl/ctl.js       |  2 +-
 packages/lib/plugin.js    | 17 ++++++++---------
 packages/master/master.js |  2 +-
 packages/slave/slave.js   |  6 +++---
 test/lib/plugin.js        |  8 ++++----
 5 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/packages/ctl/ctl.js b/packages/ctl/ctl.js
index 0c6dd755..74351e37 100755
--- a/packages/ctl/ctl.js
+++ b/packages/ctl/ctl.js
@@ -621,7 +621,7 @@ class Control extends libLink.Link {
 		 */
 		this.plugins = controlPlugins;
 		for (let controlPlugin of controlPlugins.values()) {
-			libPlugin.attachPluginMessages(this, controlPlugin.info, controlPlugin);
+			libPlugin.attachPluginMessages(this, controlPlugin);
 		}
 
 		/**
diff --git a/packages/lib/plugin.js b/packages/lib/plugin.js
index 08795c84..8e9aaafa 100644
--- a/packages/lib/plugin.js
+++ b/packages/lib/plugin.js
@@ -418,19 +418,18 @@ class BaseControlPlugin {
 /**
  * Attach plugin messages
  *
- * Attaches all messages defined in the `.message` property of the
- * `pluginInfo` passed with handlers taken from `pluin`.
+ * Attaches all messages defined in the `.info.message` property of the
+ * `plugin` passed with handlers taken from the `pluin`.
  *
  * @param {module:lib/link.Link} link - Link to attach handlers to.
- * @param {object} pluginInfo - Plugin info object.
  * @param {module:lib/plugin.BasePlugin} plugin -
- *     The instance of the plugin to use handlers from.
+ *     The instance of the plugin attach messages and handlers from.
  */
-function attachPluginMessages(link, pluginInfo, plugin) {
-	let messageDefinitions = pluginInfo.messages || [];
+function attachPluginMessages(link, plugin) {
+	let messageDefinitions = plugin.info.messages || [];
 	for (let [name, messageFormat] of Object.entries(messageDefinitions)) {
-		if (messageFormat.plugin !== pluginInfo.name) {
-			throw new Error(`Type of ${name} message must start with "${pluginInfo.name}:"`);
+		if (messageFormat.plugin !== plugin.info.name) {
+			throw new Error(`Type of ${name} message must start with "${plugin.info.name}:"`);
 		}
 
 		let handler = name + messageFormat.handlerSuffix;
@@ -444,7 +443,7 @@ function attachPluginMessages(link, pluginInfo, plugin) {
 
 		} catch (err) {
 			if (err.code === "MISSING_LINK_HANDLER") {
-				err.plugin = pluginInfo.name;
+				err.plugin = plugin.info.name;
 				err.handler = handler;
 			}
 
diff --git a/packages/master/master.js b/packages/master/master.js
index 02e05286..048a661b 100755
--- a/packages/master/master.js
+++ b/packages/master/master.js
@@ -562,7 +562,7 @@ class BaseConnection extends libLink.Link {
 		super("master", target, connector);
 		libLink.attachAllMessages(this);
 		for (let masterPlugin of masterPlugins.values()) {
-			libPlugin.attachPluginMessages(this, masterPlugin.info, masterPlugin);
+			libPlugin.attachPluginMessages(this, masterPlugin);
 		}
 	}
 
diff --git a/packages/slave/slave.js b/packages/slave/slave.js
index 7e474879..e220d6b6 100755
--- a/packages/slave/slave.js
+++ b/packages/slave/slave.js
@@ -254,7 +254,7 @@ class Instance extends libLink.Link {
 		let instancePlugin = new InstancePluginClass(pluginInfo, this, slave);
 		this.plugins.set(pluginInfo.name, instancePlugin);
 		await instancePlugin.init();
-		libPlugin.attachPluginMessages(this, pluginInfo, instancePlugin);
+		libPlugin.attachPluginMessages(this, instancePlugin);
 
 		this.logger.info(`Loaded plugin ${pluginInfo.name} in ${Date.now() - pluginLoadStarted}ms`);
 	}
@@ -830,7 +830,7 @@ class InstanceConnection extends libLink.Link {
 		libLink.attachAllMessages(this);
 
 		for (let pluginInfo of slave.pluginInfos) {
-			libPlugin.attachPluginMessages(this, pluginInfo, null);
+			libPlugin.attachPluginMessages(this, { info: pluginInfo });
 		}
 	}
 
@@ -977,7 +977,7 @@ class Slave extends libLink.Link {
 
 		this.pluginInfos = pluginInfos;
 		for (let pluginInfo of pluginInfos) {
-			libPlugin.attachPluginMessages(this, pluginInfo, null);
+			libPlugin.attachPluginMessages(this, { info: pluginInfo });
 		}
 
 		this.config = slaveConfig;
diff --git a/test/lib/plugin.js b/test/lib/plugin.js
index 6d5fc169..6ea1d9ac 100644
--- a/test/lib/plugin.js
+++ b/test/lib/plugin.js
@@ -43,24 +43,24 @@ describe("lib/plugin", function() {
 		let mockLink = new libLink.Link("source", "target", new mock.MockConnector());
 		let mockEvent = new libLink.Event({ type: "test:test", links: ["target-source"] });
 		it("should accept pluginInfo without messages", function() {
-			libPlugin.attachPluginMessages(mockLink, {}, null);
+			libPlugin.attachPluginMessages(mockLink, { info: {} });
 		});
 		it("should attach handler for the given message", function() {
 			function mockEventEventHandler() { };
 			libPlugin.attachPluginMessages(
-				mockLink, { name: "test", messages: { mockEvent }}, { mockEventEventHandler }
+				mockLink, { mockEventEventHandler, info: { name: "test", messages: { mockEvent }}}
 			);
 			assert(mockLink._handlers.get("test:test_event"), "handler was not registered");
 		});
 		it("should throw if missing handler for the given message", function() {
 			assert.throws(
-				() => libPlugin.attachPluginMessages(mockLink, { name: "test", messages: { mockEvent }}, {}),
+				() => libPlugin.attachPluginMessages(mockLink, { info: { name: "test", messages: { mockEvent }}}),
 				new Error("Missing mockEventEventHandler on plugin test for test:test_event on source-target link")
 			);
 		});
 		it("should throw if message starts with the wrong prefix", function() {
 			assert.throws(
-				() => libPlugin.attachPluginMessages(mockLink, { name: "foo", messages: { mockEvent }}, {}),
+				() => libPlugin.attachPluginMessages(mockLink, { info: { name: "foo", messages: { mockEvent }}}),
 				new Error('Type of mockEvent message must start with "foo:"')
 			);
 		});

From 75dfc66e9a36b1e5db9b1a8e056de5de72baabc3 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Thu, 28 Jan 2021 12:37:58 +0100
Subject: [PATCH 3/9] Document event_handler requiremenet for modules

Add note in the docs for modules that the event_handler interface has to
be used.
---
 docs/developing-for-clusterio.md | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/docs/developing-for-clusterio.md b/docs/developing-for-clusterio.md
index cfbcfe4c..76fd682b 100644
--- a/docs/developing-for-clusterio.md
+++ b/docs/developing-for-clusterio.md
@@ -177,7 +177,9 @@ modules that are loaded into the game if you don't need the capabilites
 of the plugin system.  The save patcher puts modules into the `modules`
 folder of the Factorio save and adds code to `control.lua` to load the
 module according to the `load` and `require` options to the module.json
-file.
+file.  Like with scenarios for Clusterio the [event_handler
+interface](#event_handler-interface) has to be used for any event
+subscriptions in modules.
 
 Stand alone modules are placed into the modules folder of Clusterio,
 plugin modules are located in the module folder of the plugin.  In

From 01e442c42e806f75f0289b1ed5e85a3498a586ab Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Thu, 28 Jan 2021 12:40:27 +0100
Subject: [PATCH 4/9] Mention WebSocket interface in docs

Replace old paragraph in the developing for clusterio document about the
removed HTTP interface with a notice of the new protocol running over
WebSocket.
---
 docs/developing-for-clusterio.md | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/docs/developing-for-clusterio.md b/docs/developing-for-clusterio.md
index 76fd682b..c6c4271e 100644
--- a/docs/developing-for-clusterio.md
+++ b/docs/developing-for-clusterio.md
@@ -161,11 +161,13 @@ which accepts a table and calls `add_lib` for each value in the table.
 Communicating with Clusterio
 ----------------------------
 
-There's a poorly to virtuarly undocumented HTTP interface that can be
-used to communicate with Clusterio to do things like get the status of
-instances, grab items from the store, run commands on instances, etc,
-etc.  For now you can read the source code, and maybe ask on the Discord
-for some pointers if you get stuck.
+Clusterio uses a homebrew protocol based on sending JSON payloads over
+a WebSocket connection.  See [socket.md](socket.md) for the
+implementation details of it.
+
+It's also possible to write a plugin for Clusterio that exposes a custom
+interface over HTTP, WebSocket or any other technology supported by
+Node.js.
 
 
 Clusterio Modules

From 09c3f82226fcc51f1a480557fc0b105c1fab9922 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Wed, 17 Feb 2021 19:40:10 +0100
Subject: [PATCH 5/9] Fix test failures on Windows

Fix broken regex, too tight time check, and lack of shell expansion on
Windows causing test cases to fail or not run at all.
---
 package.json                    |  2 +-
 packages/lib/factorio/server.js | 17 +----------------
 packages/lib/helpers.js         | 17 +++++++++++++++++
 test/lib/helpers.js             |  3 ++-
 test/lib/plugin_loader.js       |  3 ++-
 5 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/package.json b/package.json
index 8d33925a..541a855a 100644
--- a/package.json
+++ b/package.json
@@ -2,7 +2,7 @@
   "name": "root",
   "private": true,
   "scripts": {
-    "test": "mocha --exit --check-leaks -R spec --recursive test plugins/*/test",
+    "test": "mocha --exit --check-leaks -R spec --recursive test plugins/global_chat/test plugins/research_sync/test plugins/statistics_exporter/test plugins/subspace_storage/test",
     "fast-test": "FAST_TEST=y npm test",
     "cover": "nyc npm test",
     "fast-cover": "FAST_TEST=y nyc npm test",
diff --git a/packages/lib/factorio/server.js b/packages/lib/factorio/server.js
index d2c764a6..896860b1 100644
--- a/packages/lib/factorio/server.js
+++ b/packages/lib/factorio/server.js
@@ -11,24 +11,9 @@ const ini = require("ini");
 const rconClient = require("rcon-client");
 const libErrors = require("@clusterio/lib/errors");
 const { logger } = require("@clusterio/lib/logging");
+const { escapeRegExp } = require("@clusterio/lib/helpers");
 
 
-/**
- * Escapes text for inclusion in a RegExp
- *
- * Adds \ character in front of special meta characters in the passsed in
- * text so that it can be embedded into a RegExp and only match the text.
- *
- * See https://stackoverflow.com/a/9310752
- *
- * @param {string} text - Text to escape RegExp meta chars in.
- * @returns {string} escaped text.
- * @inner
- */
-function escapeRegExp(text) {
-	return text.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, "\\$&");
-}
-
 /**
  * Determines the version of Factorio the datadir is pointing to by
  * reading the changelog.txt in it.
diff --git a/packages/lib/helpers.js b/packages/lib/helpers.js
index cbbed42e..17464ec5 100644
--- a/packages/lib/helpers.js
+++ b/packages/lib/helpers.js
@@ -75,9 +75,26 @@ async function readStream(stream) {
 }
 
 
+/**
+ * Escapes text for inclusion in a RegExp
+ *
+ * Adds \ character in front of special meta characters in the passsed in
+ * text so that it can be embedded into a RegExp and only match the text.
+ *
+ * See https://stackoverflow.com/a/9310752
+ *
+ * @param {string} text - Text to escape RegExp meta chars in.
+ * @returns {string} escaped text.
+ */
+function escapeRegExp(text) {
+	return text.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, "\\$&");
+}
+
+
 module.exports = {
 	basicType,
 	wait,
 	timeout,
 	readStream,
+	escapeRegExp,
 };
diff --git a/test/lib/helpers.js b/test/lib/helpers.js
index b9671a93..5b5159f7 100644
--- a/test/lib/helpers.js
+++ b/test/lib/helpers.js
@@ -23,7 +23,8 @@ describe("lib/helpers", function() {
 			await libHelpers.wait(100);
 			let duration = Number(hrtime() - startNs) / 1e6;
 
-			assert(duration > 90 && duration < 110, `duration waited (${duration}ms) 10% of expected (100ms)`);
+			// On windows the time waited is normally up to 15 ms late with outliers in the 20ms range.
+			assert(duration > 90 && duration < 130, `duration (${duration}ms) not within (90-130)`);
 		});
 	});
 
diff --git a/test/lib/plugin_loader.js b/test/lib/plugin_loader.js
index c5ce7332..b5098955 100644
--- a/test/lib/plugin_loader.js
+++ b/test/lib/plugin_loader.js
@@ -4,6 +4,7 @@ const fs = require("fs-extra");
 const path = require("path");
 
 const libPluginLoader = require("@clusterio/lib/plugin_loader");
+const { escapeRegExp } = require("@clusterio/lib/helpers");
 
 
 describe("lib/plugin_loader", function() {
@@ -34,7 +35,7 @@ describe("lib/plugin_loader", function() {
 		it("should throw on missing plugin", async function() {
 			await assert.rejects(
 				libPluginLoader.loadPluginInfos(new Map([["missing", missingPlugin]]), []),
-				new RegExp(`^Error: PluginError: Cannot find module '${missingPlugin}/info'`)
+				new RegExp(`^Error: PluginError: Cannot find module '${escapeRegExp(missingPlugin)}/info'`)
 			);
 		});
 		it("should load test plugin", async function() {

From b4cbe2355d3b89551067bd782c0a5607700ada2e Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Wed, 17 Feb 2021 21:19:17 +0100
Subject: [PATCH 6/9] Add add/remove perms option to ctl role edit

Implement --add-perms and --remove-perms to add and remove permissions
from the role without having to list up all of the permissions in the
role again.

Rename --permissions to --set-perms to be consistent with the two new
options added.
---
 packages/ctl/ctl.js           | 20 +++++++++++++++++---
 test/integration/clusterio.js | 29 +++++++++++++++++++++++++++--
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/packages/ctl/ctl.js b/packages/ctl/ctl.js
index 74351e37..beb4159a 100755
--- a/packages/ctl/ctl.js
+++ b/packages/ctl/ctl.js
@@ -368,7 +368,11 @@ roleCommands.add(new libCommand.Command({
 		yargs.options({
 			"name": { describe: "New name for role", nargs: 1, type: "string" },
 			"description": { describe: "New description for role", nargs: 1, type: "string" },
-			"permissions": { describe: "New permissions for role", array: true, type: "string" },
+			"set-perms": { describe: "Set permissions for role", array: true, type: "string" },
+			"add-perms": { describe: "Add permissions to role", array: true, type: "string", conflicts: "set-perms" },
+			"remove-perms": {
+				describe: "Remove permissions from role", array: true, type: "string", conflicts: "set-perms",
+			},
 			"grant-default": { describe: "Add default permissions to role", nargs: 0, type: "boolean" },
 		});
 	}],
@@ -381,8 +385,18 @@ roleCommands.add(new libCommand.Command({
 		if (args.description !== undefined) {
 			role.description = args.description;
 		}
-		if (args.permissions !== undefined) {
-			role.permissions = args.permissions;
+		if (args.addPerms) {
+			role.permissions = role.permissions.concat(args.addPerms);
+		}
+		if (args.removePerms) {
+			let perms = new Set(role.permissions);
+			for (let perm of args.removePerms) {
+				perms.delete(perm);
+			}
+			role.permissions = [...perms];
+		}
+		if (args.setPerms !== undefined) {
+			role.permissions = args.setPerms;
 		}
 		await libLink.messages.updateRole.send(control, role);
 
diff --git a/test/integration/clusterio.js b/test/integration/clusterio.js
index 550c8fd7..3ab0ffb4 100644
--- a/test/integration/clusterio.js
+++ b/test/integration/clusterio.js
@@ -4,6 +4,7 @@ const fs = require("fs-extra");
 const path = require("path");
 
 const libLink = require("@clusterio/lib/link");
+const libUsers = require("@clusterio/lib/users");
 
 const { slowTest, get, exec, execCtl, sendRcon, getControl, instancesDir } = require("./index");
 
@@ -282,16 +283,40 @@ describe("Integration of Clusterio", function() {
 
 		describe("role edit", function() {
 			it("should modify the given role", async function() {
-				let args = "--name new --description \"A new role\" --permissions";
+				let args = "--name new --description \"A new role\" --set-perms";
 				await execCtl(`role edit temp ${args}`);
 				let result = await libLink.messages.listRoles.send(getControl());
 				let newRole = result.list.find(role => role.name === "new");
 				assert.deepEqual(newRole, { id: 5, name: "new", description: "A new role", permissions: [] });
 			});
+			it("should add permissions with --add-perms", async function() {
+				let args = "--name new --add-perms core.slave.list core.instance.list";
+				await execCtl(`role edit new ${args}`);
+				let result = await libLink.messages.listRoles.send(getControl());
+				let newRole = result.list.find(role => role.name === "new");
+				assert.deepEqual(newRole.permissions, ["core.slave.list", "core.instance.list"]);
+			});
+			it("should remove permissions with --remove-perms", async function() {
+				let args = "--name new --remove-perms core.slave.list";
+				await execCtl(`role edit new ${args}`);
+				let result = await libLink.messages.listRoles.send(getControl());
+				let newRole = result.list.find(role => role.name === "new");
+				assert.deepEqual(newRole.permissions, ["core.instance.list"]);
+			});
+			it("should grant default permissions with --grant-default", async function() {
+				await execCtl("role edit new --grant-default");
+				let result = await libLink.messages.listRoles.send(getControl());
+				let newRole = result.list.find(role => role.name === "new");
+				let defaultPermissions = [...libUsers.permissions.values()]
+					.filter(p => p.grantByDefault)
+					.map(p => p.name)
+				;
+				assert.deepEqual(new Set(newRole.permissions), new Set(defaultPermissions));
+			});
 		});
 
 		describe("role delete", function() {
-			it("should modify the given role", async function() {
+			it("should delete the given role", async function() {
 				await execCtl("role delete new");
 				let result = await libLink.messages.listRoles.send(getControl());
 				let newRole = result.list.find(role => role.name === "new");

From 1de8b76303c811c796d2e3233cef79c97654889a Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Fri, 19 Feb 2021 14:28:19 +0100
Subject: [PATCH 7/9] Emit resume instead of connect on resumed connections

Raise "resume" instead of "connect" when a connection is re-established
after a connection drop.  This prevents logic meant to resynchronize
state after a full reconnection from being invoked whenever the
connection was temporarily dropped.
---
 docs/writing-plugins.md         |  2 +-
 packages/lib/link/connectors.js |  2 +-
 packages/lib/plugin.js          | 28 ++++++++++++++++++----------
 packages/master/master.js       |  4 ++--
 packages/slave/slave.js         |  2 +-
 test/integration/link.js        |  6 +++---
 6 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/docs/writing-plugins.md b/docs/writing-plugins.md
index ae5ab831..5e007b35 100644
--- a/docs/writing-plugins.md
+++ b/docs/writing-plugins.md
@@ -621,7 +621,7 @@ until the connection is re-established.  This means that if your plugin
 sends a lot of events or requests, they can end up being queued up in a
 buffer for a long time and sent out all at once.  To avoid this you
 should be throtteling and/or stopping your requests/events after `drop`
-has been raised, and continue back as normal when `connect` is raised.
+has been raised, and continue back as normal when `resume` is raised.
 
 
 Collecting Statistics
diff --git a/packages/lib/link/connectors.js b/packages/lib/link/connectors.js
index ccf24904..f3881d26 100644
--- a/packages/lib/link/connectors.js
+++ b/packages/lib/link/connectors.js
@@ -404,7 +404,7 @@ class WebSocketClientConnector extends WebSocketBaseConnector {
 			}
 			this._startedReconnect = null;
 			this._connected = true;
-			this.emit("connect");
+			this.emit("resume");
 
 		} else if (type === "invalidate") {
 			this._heartbeatInterval = null;
diff --git a/packages/lib/plugin.js b/packages/lib/plugin.js
index 8e9aaafa..ddaf50e0 100644
--- a/packages/lib/plugin.js
+++ b/packages/lib/plugin.js
@@ -148,13 +148,12 @@ class BaseInstancePlugin {
 	/**
 	 * Called when an event on the master connection happens
 	 *
-	 * The event param may be one of connect, drop and close and has the
-	 * following meaning:
+	 * The event param may be one of connect, drop, resume and close and has
+	 * the following meaning:
 	 *
 	 * ##### connect
 	 *
-	 * Invoked when a new connection to the master has been established, or
-	 * the existing one that have previously been dropped is re-established.
+	 * Invoked when a new connection to the master has been established.
 	 *
 	 * ##### drop
 	 *
@@ -166,6 +165,11 @@ class BaseInstancePlugin {
 	 * memory on the slave and sent all in one go when the connection is
 	 * re-established again.
 	 *
+	 * ##### resume
+	 *
+	 * Invoked when the connection that had previously dropped is
+	 * re-established.
+	 *
 	 * ##### close
 	 *
 	 * Invoked when the connection to the master has been closed.  This
@@ -173,7 +177,7 @@ class BaseInstancePlugin {
 	 * send any messages that goes to or via the master server after the
 	 * connection has been closed and before a new one is established.
 	 *
-	 * @param {string} event - one of connect, drop, and close
+	 * @param {string} event - one of connect, drop, resumeand close
 	 */
 	onMasterConnectionEvent(event) { }
 
@@ -293,13 +297,12 @@ class BaseMasterPlugin {
 	/**
 	 * Called when an event on a slave connection happens
 	 *
-	 * The event param may be one of connect, drop and close and has the
-	 * following meaning:
+	 * The event param may be one of connect, drop, resume and close and has
+	 * the following meaning:
 	 *
 	 * ##### connect
 	 *
-	 * Invoked when a new connection to the slave has been established, or
-	 * an existing one that had previously been dropped is re-established.
+	 * Invoked when a new connection to the slave has been established.
 	 *
 	 * ##### drop
 	 *
@@ -311,13 +314,18 @@ class BaseMasterPlugin {
 	 * memory on the master and sent all in one go when the connection is
 	 * re-established again.
 	 *
+	 * ##### resume
+	 *
+	 * Invoked when a connection that had previously been dropped is
+	 * re-established
+	 *
 	 * ##### close
 	 *
 	 * Invoked when the connection to the slave has been closed.
 	 *
 	 * @param {module:master/master~SlaveConnection} connection -
 	 *     The connection the event occured on.
-	 * @param {string} event - one of connect, drop, and close
+	 * @param {string} event - one of connect, drop, resume and close
 	 */
 	onSlaveConnectionEvent(connection, event) { }
 
diff --git a/packages/master/master.js b/packages/master/master.js
index 048a661b..e5a2a1aa 100755
--- a/packages/master/master.js
+++ b/packages/master/master.js
@@ -1168,7 +1168,7 @@ class SlaveConnection extends BaseConnection {
 			}
 		});
 
-		for (let event of ["connect", "drop", "close"]) {
+		for (let event of ["connect", "drop", "resume", "close"]) {
 			// eslint-disable-next-line no-loop-func
 			this.connector.on(event, () => {
 				for (let masterPlugin of masterPlugins.values()) {
@@ -1397,7 +1397,7 @@ class WebSocketServerConnector extends libLink.WebSocketBaseConnector {
 		for (let message of this._sendBuffer) {
 			this._socket.send(JSON.stringify(message));
 		}
-		this.emit("connect");
+		this.emit("resume");
 	}
 
 	setTimeout(timeout) {
diff --git a/packages/slave/slave.js b/packages/slave/slave.js
index e220d6b6..6c0dfc30 100755
--- a/packages/slave/slave.js
+++ b/packages/slave/slave.js
@@ -1028,7 +1028,7 @@ class Slave extends libLink.Link {
 			}
 		});
 
-		for (let event of ["connect", "drop", "close"]) {
+		for (let event of ["connect", "drop", "resume", "close"]) {
 			this.connector.on(event, () => {
 				for (let instanceConnection of this.instanceConnections.values()) {
 					libLink.messages.masterConnectionEvent.send(instanceConnection, { event });
diff --git a/test/integration/link.js b/test/integration/link.js
index 731fef97..39848e16 100644
--- a/test/integration/link.js
+++ b/test/integration/link.js
@@ -34,17 +34,17 @@ describe("Integration of lib/link", function() {
 
 	it("should reconnect on connection lost", async function() {
 		controlConnector._socket.close(1008, "Test");
-		await events.once(controlConnector, "connect");
+		await events.once(controlConnector, "resume");
 	});
 
 	it("should reconnect on connection terminated", async function() {
 		controlConnector._socket.terminate();
-		await events.once(controlConnector, "connect");
+		await events.once(controlConnector, "resume");
 	});
 
 	it("should reconnect on connection heartbeat timeout", async function() {
 		controlConnector.stopHeartbeat();
-		await events.once(controlConnector, "connect");
+		await events.once(controlConnector, "resume");
 	});
 
 	it("should invalidate on reconnection with bad session token", async function() {

From 7e9f1562e43361655efb7ece071d9da3dcbf31a7 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Fri, 19 Feb 2021 14:54:14 +0100
Subject: [PATCH 8/9] Add Instance.id shortcut

Add shortcut for instance.config.get("instance.id") similar to the
already existing shortcut for its name.
---
 docs/writing-plugins.md                 |  2 +-
 packages/slave/slave.js                 | 33 ++++++++++++++-----------
 plugins/statistics_exporter/instance.js |  2 +-
 plugins/subspace_storage/instance.js    |  4 +--
 test/mock.js                            |  1 +
 5 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/docs/writing-plugins.md b/docs/writing-plugins.md
index 5e007b35..62b8c854 100644
--- a/docs/writing-plugins.md
+++ b/docs/writing-plugins.md
@@ -660,7 +660,7 @@ set it accordingly, for example:
     );
 
     // Somewhere in the instance plugin code
-    barMetric.labels(String(this.instance.config.get("instance.id"))).set(someValue);
+    barMetric.labels(String(this.instance.id)).set(someValue);
 
 Metrics are automatically registered to the default registry, and this
 default registry is automatically polled by the master server on slaves.
diff --git a/packages/slave/slave.js b/packages/slave/slave.js
index 6c0dfc30..4e2ad729 100755
--- a/packages/slave/slave.js
+++ b/packages/slave/slave.js
@@ -128,9 +128,15 @@ class Instance extends libLink.Link {
 		this.plugins = new Map();
 		this.config = instanceConfig;
 
+		/**
+		 * ID of this instance, equivalenet to `instance.config.get("instance.id")`.
+		 * @constant {number}
+		 */
+		this.id = this.config.get("instance.id");
+
 		this.logger = logger.child({
-			instance_id: this.config.get("instance.id"),
-			instance_name: this.config.get("instance.name"),
+			instance_id: this.id,
+			instance_name: this.name,
 		});
 
 		this._configFieldChanged = (group, field, prev) => {
@@ -168,7 +174,7 @@ class Instance extends libLink.Link {
 		);
 
 		this.server.on("output", (parsed, line) => {
-			this.logger.log("server", { message: line, instance_id: this.config.get("instance.id"), parsed });
+			this.logger.log("server", { message: line, instance_id: this.id, parsed });
 
 			libPlugin.invokeHook(this.plugins, "onOutput", parsed, line);
 		});
@@ -185,7 +191,7 @@ class Instance extends libLink.Link {
 
 		this.server.on("ipc-player_event", event => {
 			libLink.messages.playerEvent.send(this, {
-				instance_id: this.config.get("instance.id"),
+				instance_id: this.id,
 				...event,
 			});
 			libPlugin.invokeHook(this.plugins, "onPlayerEvent", event);
@@ -193,7 +199,7 @@ class Instance extends libLink.Link {
 	}
 
 	async sendRcon(message, expectEmpty, plugin = "") {
-		let instanceId = String(this.config.get("instance.id"));
+		let instanceId = String(this.id);
 		let observeDuration = instanceRconCommandDuration.labels(instanceId).startTimer();
 		try {
 			return await this.server.sendRcon(message, expectEmpty);
@@ -205,13 +211,13 @@ class Instance extends libLink.Link {
 
 	async _autosave(name) {
 		let stat = await fs.stat(this.path("saves", `${name}.zip`));
-		instanceFactorioAutosaveSize.labels(String(this.config.get("instance.id"))).set(stat.size);
+		instanceFactorioAutosaveSize.labels(String(this.id)).set(stat.size);
 	}
 
 	notifyStatus(status) {
 		this._status = status;
 		libLink.messages.instanceStatusChanged.send(this, {
-			instance_id: this.config.get("instance.id"), status,
+			instance_id: this.id, status,
 		});
 	}
 
@@ -238,7 +244,7 @@ class Instance extends libLink.Link {
 				collector instanceof libPrometheus.ValueCollector
 				&& collector.metric.labels.includes("instance_id")
 			) {
-				collector.removeAll({ instance_id: String(this.config.get("instance.id")) });
+				collector.removeAll({ instance_id: String(this.id) });
 			}
 		}
 
@@ -285,7 +291,7 @@ class Instance extends libLink.Link {
 		for (let [name, plugin] of this.plugins) {
 			plugins[name] = plugin.info.version;
 		}
-		libLink.messages.instanceInitialized.send(this, { instance_id: this.config.get("instance.id"), plugins });
+		libLink.messages.instanceInitialized.send(this, { instance_id: this.id, plugins });
 	}
 
 	/**
@@ -543,9 +549,8 @@ class Instance extends libLink.Link {
 	 * Update instance information on the Factorio side
 	 */
 	async updateInstanceData() {
-		let name = libLuaTools.escapeString(this.config.get("instance.name"));
-		let id = this.config.get("instance.id");
-		await this.sendRcon(`/sc clusterio_private.update_instance(${id}, "${name}")`, true);
+		let name = libLuaTools.escapeString(this.name);
+		await this.sendRcon(`/sc clusterio_private.update_instance(${this.id}, "${name}")`, true);
 	}
 
 	async updateFactorioSettings(current, previous) {
@@ -648,8 +653,8 @@ class Instance extends libLink.Link {
 		let pid = this.server.pid;
 		if (pid) {
 			let stats = await pidusage(pid);
-			instanceFactorioCpuTime.labels(String(this.config.get("instance.id"))).set(stats.ctime / 1000);
-			instanceFactorioMemoryUsage.labels(String(this.config.get("instance.id"))).set(stats.memory);
+			instanceFactorioCpuTime.labels(String(this.id)).set(stats.ctime / 1000);
+			instanceFactorioMemoryUsage.labels(String(this.id)).set(stats.memory);
 		}
 
 		return { results };
diff --git a/plugins/statistics_exporter/instance.js b/plugins/statistics_exporter/instance.js
index e7204d7a..db40dcbd 100644
--- a/plugins/statistics_exporter/instance.js
+++ b/plugins/statistics_exporter/instance.js
@@ -53,7 +53,7 @@ class InstancePlugin extends libPlugin.BaseInstancePlugin {
 			throw new Error(`Error parsing statistics JSON: ${err.message}, content "${string}"`);
 		}
 
-		let instanceId = this.instance.config.get("instance.id");
+		let instanceId = this.instance.id;
 		instanceGameTicksTotal.labels(String(instanceId)).set(stats.game_tick);
 		instancePlayerCount.labels(String(instanceId)).set(stats.player_count);
 
diff --git a/plugins/subspace_storage/instance.js b/plugins/subspace_storage/instance.js
index 668f0b69..1f2529ea 100644
--- a/plugins/subspace_storage/instance.js
+++ b/plugins/subspace_storage/instance.js
@@ -39,7 +39,7 @@ class InstancePlugin extends libPlugin.BaseInstancePlugin {
 	async provideItems(items) {
 		this.info.messages.place.send(this.instance, {
 			items,
-			instance_id: this.instance.config.get("instance.id"),
+			instance_id: this.instance.id,
 		});
 
 		if (this.instance.config.get("subspace_storage.log_item_transfers")) {
@@ -52,7 +52,7 @@ class InstancePlugin extends libPlugin.BaseInstancePlugin {
 	async requestItems(items) {
 		// Request the items all at once
 		let response = await this.info.messages.remove.send(this.instance, {
-			instance_id: this.instance.config.get("instance.id"),
+			instance_id: this.instance.id,
 			items,
 		});
 
diff --git a/test/mock.js b/test/mock.js
index 89ada241..6ca83e96 100644
--- a/test/mock.js
+++ b/test/mock.js
@@ -103,6 +103,7 @@ class MockInstance extends libLink.Link {
 		this.logger = new MockLogger();
 		this.server = new MockServer();
 		this.name = "test";
+		this.id = 7357;
 		this.mockConfigEntries = new Map([
 			["instance.id", 7357],
 			["factorio.enable_save_patching", true],

From c6a04357512711777f0bf84c5c38c0f5dc924489 Mon Sep 17 00:00:00 2001
From: Hornwitser <github@hornwitser.no>
Date: Sat, 20 Feb 2021 18:10:46 +0100
Subject: [PATCH 9/9] Fix missing space

---
 packages/lib/plugin.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/lib/plugin.js b/packages/lib/plugin.js
index ddaf50e0..4dce8257 100644
--- a/packages/lib/plugin.js
+++ b/packages/lib/plugin.js
@@ -177,7 +177,7 @@ class BaseInstancePlugin {
 	 * send any messages that goes to or via the master server after the
 	 * connection has been closed and before a new one is established.
 	 *
-	 * @param {string} event - one of connect, drop, resumeand close
+	 * @param {string} event - one of connect, drop, resume and close
 	 */
 	onMasterConnectionEvent(event) { }