Merge pull request #1042 from bbassie/fix/plugin-install-restart-required

Mark restart required after installing a new plugin

packages/controller/src/ControlConnection.ts

@@ -1315,7 +1315,11 @@ export default class ControlConnection extends BaseConnection {
 		if (!this._controller.config.get("controller.allow_plugin_install")) {
 			throw new lib.RequestError("Plugin installs are disabled on this machine");
 		}
-		return await lib.handlePluginInstall(request.pluginPackage);
+		await lib.handlePluginInstall(request.pluginPackage);
+		// A new plugin is not in pluginInfos, so checkRestartRequired cannot notice it
+		if (!this._controller.pluginInfos.some(pluginInfo => pluginInfo.npmPackage === request.pluginPackage)) {
+			this._controller.config.restartRequired = true;
+		}
 	}
 
 	async handleUpdateAllRequest(request: lib.UpdateAllRequest) {

packages/host/src/Host.ts

@@ -1143,7 +1143,11 @@ export default class Host extends lib.Link {
 		if (!this.config.get("host.allow_plugin_install")) {
 			throw new lib.RequestError("Plugin installs are disabled on this machine");
 		}
-		return await lib.handlePluginInstall(request.pluginPackage);
+		await lib.handlePluginInstall(request.pluginPackage);
+		// A new plugin is not in pluginInfos, so checkRestartRequired cannot notice it
+		if (!this.pluginInfos.some(pluginInfo => pluginInfo.npmPackage === request.pluginPackage)) {
+			this.config.restartRequired = true;
+		}
 	}
 
 	async handleUpdateAllRequest(request: lib.UpdateAllRequest) {

test/controller/ControlConnection.test.js

@@ -76,6 +76,46 @@ describe("controller/src/ControlConnection", function() {
 		});
 	});
 
+	describe(".handlePluginInstallRequest()", function() {
+		let mockController;
+		let _fetch;
+
+		beforeEach(function() {
+			mockController = {
+				config: new lib.ControllerConfig("controller", { "controller.allow_plugin_install": true }),
+				pluginInfos: [{ name: "restart_test", npmPackage: "restart_test", version: "1.0.0" }],
+			};
+			_fetch = global.fetch;
+			global.fetch = async () => ({ ok: true });
+		});
+		afterEach(function() {
+			global.fetch = _fetch;
+		});
+
+		async function install(name) {
+			await ControlConnection.prototype.handlePluginInstallRequest.call(
+				{ _controller: mockController },
+				new lib.PluginInstallRequest(name),
+			);
+		}
+
+		it("marks a restart as required for a new plugin", async function() {
+			await install("new_plugin");
+			assert.equal(mockController.config.restartRequired, true);
+		});
+		it("leaves the flag alone for an already loaded plugin", async function() {
+			await install("restart_test");
+			assert.equal(mockController.config.restartRequired, false);
+		});
+		it("rejects when installs are disabled", async function() {
+			mockController.config = new lib.ControllerConfig(
+				"controller", { "controller.allow_plugin_install": false }
+			);
+			await assert.rejects(install("new_plugin"), /Plugin installs are disabled/);
+			assert.equal(mockController.config.restartRequired, false);
+		});
+	});
+
 	describe(".handleInstanceCreateRequest()", function() {
 		let mockController;
 

test/host/Host.test.js

@@ -179,6 +179,38 @@ describe("host/src/Host", function() {
 				assert.equal(host.config.restartRequired, true);
 			});
 		});
+		describe(".handlePluginInstallRequest()", function() {
+			let host;
+			let _fetch;
+			beforeEach(function() {
+				const pluginInfos = [{ name: "restart_test", npmPackage: "restart_test", version: "1.0.0" }];
+				const hostConfig = new lib.HostConfig("host", { "host.allow_plugin_install": true });
+				const hostConnector = new HostConnector(hostConfig, pluginInfos);
+				host = new Host(hostConnector, hostConfig, pluginInfos);
+				_fetch = global.fetch;
+				global.fetch = async () => ({ ok: true });
+			});
+			afterEach(function() {
+				global.fetch = _fetch;
+			});
+			it("marks a restart as required for a new plugin", async function() {
+				await host.handlePluginInstallRequest(new lib.PluginInstallRequest("new_plugin"));
+				assert.equal(host.config.restartRequired, true);
+			});
+			it("leaves the flag alone for an already loaded plugin", async function() {
+				await host.handlePluginInstallRequest(new lib.PluginInstallRequest("restart_test"));
+				assert.equal(host.config.restartRequired, false);
+			});
+			it("rejects when installs are disabled", async function() {
+				host.config.set("host.allow_plugin_install", false);
+				host.config.restartRequired = false; // Setting allow_plugin_install requires a restart
+				await assert.rejects(
+					host.handlePluginInstallRequest(new lib.PluginInstallRequest("new_plugin")),
+					/Plugin installs are disabled/
+				);
+				assert.equal(host.config.restartRequired, false);
+			});
+		});
 		describe(".checkRestartDowngrade()", async function() {
 			let host;
 			const hostVersion = (