From 515577021335d4968051daea919849c681028f1f Mon Sep 17 00:00:00 2001 From: Nick O'Leary Date: Mon, 30 Jul 2018 10:08:39 +0100 Subject: [PATCH] Ensure add/remove modules are handled sequentially --- red/runtime/nodes/registry/installer.js | 233 ++++++++++-------- .../runtime/nodes/registry/installer_spec.js | 18 +- 2 files changed, 133 insertions(+), 118 deletions(-) diff --git a/red/runtime/nodes/registry/installer.js b/red/runtime/nodes/registry/installer.js index 4e185c257..46a0c5a4b 100644 --- a/red/runtime/nodes/registry/installer.js +++ b/red/runtime/nodes/registry/installer.js @@ -15,7 +15,6 @@ **/ -var when = require("when"); var path = require("path"); var fs = require("fs"); @@ -36,6 +35,8 @@ function init(_settings) { settings = _settings; } +var activePromise = Promise.resolve(); + function checkModulePath(folder) { var moduleName; var err; @@ -71,79 +72,86 @@ function checkExistingModule(module,version) { return false; } function installModule(module,version) { - //TODO: ensure module is 'safe' - return when.promise(function(resolve,reject) { - var installName = module; - var isUpgrade = false; - try { - if (moduleRe.test(module)) { - // Simple module name - assume it can be npm installed - if (version) { - installName += "@"+version; + activePromise = activePromise.then(() => { + //TODO: ensure module is 'safe' + return new Promise((resolve,reject) => { + var installName = module; + var isUpgrade = false; + try { + if (moduleRe.test(module)) { + // Simple module name - assume it can be npm installed + if (version) { + installName += "@"+version; + } + } else if (slashRe.test(module)) { + // A path - check if there's a valid package.json + installName = module; + module = checkModulePath(module); } - } else if (slashRe.test(module)) { - // A path - check if there's a valid package.json - installName = module; - module = checkModulePath(module); + isUpgrade = checkExistingModule(module,version); + } catch(err) { + return reject(err); } - isUpgrade = checkExistingModule(module,version); - } catch(err) { - return reject(err); - } - if (!isUpgrade) { - log.info(log._("server.install.installing",{name: module,version: version||"latest"})); - } else { - log.info(log._("server.install.upgrading",{name: module,version: version||"latest"})); - } - - var installDir = settings.userDir || process.env.NODE_RED_HOME || "."; - var args = ['install','--save','--save-prefix="~"','--production',installName]; - log.trace(npmCommand + JSON.stringify(args)); - var child = child_process.spawn(npmCommand,args,{ - cwd: installDir, - shell: true - }); - var output = ""; - child.stdout.on('data', (data) => { - output += data; - }); - child.stderr.on('data', (data) => { - output += data; - }); - child.on('close', (code) => { - if (code !== 0) { - var e; - var lookFor404 = new RegExp(" 404 .*"+module,"m"); - var lookForVersionNotFound = new RegExp("version not found: "+module+"@"+version,"m"); - if (lookFor404.test(output)) { - log.warn(log._("server.install.install-failed-not-found",{name:module})); - e = new Error("Module not found"); - e.code = 404; - reject(e); - } else if (isUpgrade && lookForVersionNotFound.test(output)) { - log.warn(log._("server.install.upgrade-failed-not-found",{name:module})); - e = new Error("Module not found"); - e.code = 404; - reject(e); - } else { - log.warn(log._("server.install.install-failed-long",{name:module})); - log.warn("------------------------------------------"); - log.warn(output); - log.warn("------------------------------------------"); - reject(new Error(log._("server.install.install-failed"))); - } + if (!isUpgrade) { + log.info(log._("server.install.installing",{name: module,version: version||"latest"})); } else { - if (!isUpgrade) { - log.info(log._("server.install.installed",{name:module})); - resolve(require("./index").addModule(module).then(reportAddedModules)); - } else { - log.info(log._("server.install.upgraded",{name:module, version:version})); - events.emit("runtime-event",{id:"restart-required",payload:{type:"warning",text:"notification.warnings.restartRequired"},retain:true}); - resolve(require("./registry").setModulePendingUpdated(module,version)); - } + log.info(log._("server.install.upgrading",{name: module,version: version||"latest"})); } + + var installDir = settings.userDir || process.env.NODE_RED_HOME || "."; + var args = ['install','--save','--save-prefix="~"','--production',installName]; + log.trace(npmCommand + JSON.stringify(args)); + var child = child_process.spawn(npmCommand,args,{ + cwd: installDir, + shell: true + }); + var output = ""; + child.stdout.on('data', (data) => { + output += data; + }); + child.stderr.on('data', (data) => { + output += data; + }); + child.on('close', (code) => { + if (code !== 0) { + var e; + var lookFor404 = new RegExp(" 404 .*"+module,"m"); + var lookForVersionNotFound = new RegExp("version not found: "+module+"@"+version,"m"); + if (lookFor404.test(output)) { + log.warn(log._("server.install.install-failed-not-found",{name:module})); + e = new Error("Module not found"); + e.code = 404; + reject(e); + } else if (isUpgrade && lookForVersionNotFound.test(output)) { + log.warn(log._("server.install.upgrade-failed-not-found",{name:module})); + e = new Error("Module not found"); + e.code = 404; + reject(e); + } else { + log.warn(log._("server.install.install-failed-long",{name:module})); + log.warn("------------------------------------------"); + log.warn(output); + log.warn("------------------------------------------"); + reject(new Error(log._("server.install.install-failed"))); + } + } else { + if (!isUpgrade) { + log.info(log._("server.install.installed",{name:module})); + resolve(require("./index").addModule(module).then(reportAddedModules)); + } else { + log.info(log._("server.install.upgraded",{name:module, version:version})); + events.emit("runtime-event",{id:"restart-required",payload:{type:"warning",text:"notification.warnings.restartRequired"},retain:true}); + resolve(require("./registry").setModulePendingUpdated(module,version)); + } + } + }); }); + }).catch(err => { + // In case of error, reset activePromise to be resolvable + activePromise = Promise.resolve(); + throw err; }); + return activePromise; } @@ -176,47 +184,54 @@ function reportRemovedModules(removedNodes) { } function uninstallModule(module) { - return when.promise(function(resolve,reject) { - if (/[\s;]/.test(module)) { - reject(new Error(log._("server.install.invalid"))); - return; - } - var installDir = settings.userDir || process.env.NODE_RED_HOME || "."; - var moduleDir = path.join(installDir,"node_modules",module); - - try { - fs.statSync(moduleDir); - } catch(err) { - return reject(new Error(log._("server.install.uninstall-failed",{name:module}))); - } - - var list = registry.removeModule(module); - log.info(log._("server.install.uninstalling",{name:module})); - - var args = ['remove','--save',module]; - log.trace(npmCommand + JSON.stringify(args)); - - var child = child_process.execFile(npmCommand,args, - { - cwd: installDir - }, - function(err, stdin, stdout) { - if (err) { - log.warn(log._("server.install.uninstall-failed-long",{name:module})); - log.warn("------------------------------------------"); - log.warn(err.toString()); - log.warn("------------------------------------------"); - reject(new Error(log._("server.install.uninstall-failed",{name:module}))); - } else { - log.info(log._("server.install.uninstalled",{name:module})); - reportRemovedModules(list); - // TODO: tidy up internal event names - events.emit("node-module-uninstalled",module) - resolve(list); - } + activePromise = activePromise.then(() => { + return new Promise((resolve,reject) => { + if (/[\s;]/.test(module)) { + reject(new Error(log._("server.install.invalid"))); + return; } - ); + var installDir = settings.userDir || process.env.NODE_RED_HOME || "."; + var moduleDir = path.join(installDir,"node_modules",module); + + try { + fs.statSync(moduleDir); + } catch(err) { + return reject(new Error(log._("server.install.uninstall-failed",{name:module}))); + } + + var list = registry.removeModule(module); + log.info(log._("server.install.uninstalling",{name:module})); + + var args = ['remove','--save',module]; + log.trace(npmCommand + JSON.stringify(args)); + + var child = child_process.execFile(npmCommand,args, + { + cwd: installDir + }, + function(err, stdin, stdout) { + if (err) { + log.warn(log._("server.install.uninstall-failed-long",{name:module})); + log.warn("------------------------------------------"); + log.warn(err.toString()); + log.warn("------------------------------------------"); + reject(new Error(log._("server.install.uninstall-failed",{name:module}))); + } else { + log.info(log._("server.install.uninstalled",{name:module})); + reportRemovedModules(list); + // TODO: tidy up internal event names + events.emit("node-module-uninstalled",module) + resolve(list); + } + } + ); + }); + }).catch(err => { + // In case of error, reset activePromise to be resolvable + activePromise = Promise.resolve(); + throw err; }); + return activePromise; } function checkPrereq() { @@ -227,9 +242,9 @@ function checkPrereq() { ) { log.info(log._("server.palette-editor.disabled")); paletteEditorEnabled = false; - return when.resolve(); + return Promise.resolve(); } else { - return when.promise(function(resolve) { + return new Promise(resolve => { child_process.execFile(npmCommand,['-v'],function(err) { if (err) { log.info(log._("server.palette-editor.npm-not-found")); diff --git a/test/red/runtime/nodes/registry/installer_spec.js b/test/red/runtime/nodes/registry/installer_spec.js index 8bd685f3c..bb44dbbbe 100644 --- a/test/red/runtime/nodes/registry/installer_spec.js +++ b/test/red/runtime/nodes/registry/installer_spec.js @@ -73,7 +73,7 @@ describe('nodes/registry/installer', function() { return ee; }); - installer.installModule("this_wont_exist").otherwise(function(err) { + installer.installModule("this_wont_exist").catch(function(err) { err.code.should.be.eql(404); done(); }); @@ -95,7 +95,7 @@ describe('nodes/registry/installer', function() { } }); - installer.installModule("this_wont_exist","0.1.2").otherwise(function(err) { + installer.installModule("this_wont_exist","0.1.2").catch(function(err) { err.code.should.be.eql(404); done(); }); @@ -106,7 +106,7 @@ describe('nodes/registry/installer', function() { version: "0.1.1" } }); - installer.installModule("this_wont_exist","0.1.1").otherwise(function(err) { + installer.installModule("this_wont_exist","0.1.1").catch(function(err) { err.code.should.be.eql('module_already_loaded'); done(); }); @@ -125,7 +125,7 @@ describe('nodes/registry/installer', function() { installer.installModule("this_wont_exist").then(function() { done(new Error("Unexpected success")); - }).otherwise(function(err) { + }).catch(function(err) { done(); }); }); @@ -150,7 +150,7 @@ describe('nodes/registry/installer', function() { // commsMessages[0].topic.should.equal("node/added"); // commsMessages[0].msg.should.eql(nodeInfo.nodes); done(); - }).otherwise(function(err) { + }).catch(function(err) { done(err); }); }); @@ -159,7 +159,7 @@ describe('nodes/registry/installer', function() { var resourcesDir = path.resolve(path.join(__dirname,"..","resources","local","TestNodeModule","node_modules","NonExistant")); installer.installModule(resourcesDir).then(function() { done(new Error("Unexpected success")); - }).otherwise(function(err) { + }).catch(function(err) { if (err.hasOwnProperty("code")) { err.code.should.eql(404); done(); @@ -189,7 +189,7 @@ describe('nodes/registry/installer', function() { installer.installModule(resourcesDir).then(function(info) { info.should.eql(nodeInfo); done(); - }).otherwise(function(err) { + }).catch(function(err) { done(err); }); }); @@ -218,7 +218,7 @@ describe('nodes/registry/installer', function() { installer.uninstallModule("this_wont_exist").then(function() { done(new Error("Unexpected success")); - }).otherwise(function(err) { + }).catch(function(err) { done(); }); }); @@ -242,7 +242,7 @@ describe('nodes/registry/installer', function() { // commsMessages[0].topic.should.equal("node/removed"); // commsMessages[0].msg.should.eql(nodeInfo); done(); - }).otherwise(function(err) { + }).catch(function(err) { done(err); }); });