From da96c85d325bce196ba5b947643b470bbaa27618 Mon Sep 17 00:00:00 2001 From: Nick O'Leary Date: Wed, 25 Nov 2020 19:07:30 +0000 Subject: [PATCH] Handle subflow modules with their own npm dependencies --- .../@node-red/editor-api/lib/admin/nodes.js | 1 + .../@node-red/editor-client/src/js/red.js | 1 + .../@node-red/registry/lib/installer.js | 44 ++++--- .../@node-red/registry/lib/loader.js | 28 ++++- .../@node-red/registry/lib/localfilesystem.js | 109 +++++++++++++----- .../@node-red/registry/lib/registry.js | 80 +++++++++++-- .../@node-red/runtime/lib/api/nodes.js | 2 +- .../@node-red/runtime/lib/nodes/index.js | 7 +- test/resources/subflow/package/README.md | 3 + test/resources/subflow/package/package.json | 18 +++ test/resources/subflow/package/subflow.js | 4 + test/resources/subflow/package/subflow.json | 4 + .../subflow/test-subflow-mod-1.0.1.tgz | Bin 0 -> 1449 bytes .../@node-red/registry/lib/installer_spec.js | 2 + .../@node-red/registry/lib/loader_spec.js | 4 +- 15 files changed, 237 insertions(+), 70 deletions(-) create mode 100644 test/resources/subflow/package/README.md create mode 100644 test/resources/subflow/package/package.json create mode 100644 test/resources/subflow/package/subflow.js create mode 100644 test/resources/subflow/package/subflow.json create mode 100644 test/resources/subflow/test-subflow-mod-1.0.1.tgz diff --git a/packages/node_modules/@node-red/editor-api/lib/admin/nodes.js b/packages/node_modules/@node-red/editor-api/lib/admin/nodes.js index 187bd823f..058053a29 100644 --- a/packages/node_modules/@node-red/editor-api/lib/admin/nodes.js +++ b/packages/node_modules/@node-red/editor-api/lib/admin/nodes.js @@ -60,6 +60,7 @@ module.exports = { runtimeAPI.nodes.addModule(opts).then(function(info) { res.json(info); }).catch(function(err) { + console.log(err.stack); apiUtils.rejectHandler(req,res,err); }) }, diff --git a/packages/node_modules/@node-red/editor-client/src/js/red.js b/packages/node_modules/@node-red/editor-client/src/js/red.js index 97159e6d2..d692315a1 100644 --- a/packages/node_modules/@node-red/editor-client/src/js/red.js +++ b/packages/node_modules/@node-red/editor-client/src/js/red.js @@ -386,6 +386,7 @@ var RED = (function() { } }); RED.comms.subscribe("notification/node/#",function(topic,msg) { + console.log(topic,msg); var i,m; var typeList; var info; diff --git a/packages/node_modules/@node-red/registry/lib/installer.js b/packages/node_modules/@node-red/registry/lib/installer.js index c7cd0d5e0..459220774 100644 --- a/packages/node_modules/@node-red/registry/lib/installer.js +++ b/packages/node_modules/@node-red/registry/lib/installer.js @@ -94,20 +94,6 @@ function checkModulePath(folder) { version: moduleVersion }; } - -function checkExistingModule(module,version) { - var info = registry.getModuleInfo(module); - if (info) { - if (!version || info.version === version) { - var err = new Error("Module already loaded"); - err.code = "module_already_loaded"; - throw err; - } - return true; - } - return false; -} - async function installModule(module,version,url) { if (Buffer.isBuffer(module)) { return installTarball(module) @@ -118,6 +104,7 @@ async function installModule(module,version,url) { var installName = module; let isRegistryPackage = true; var isUpgrade = false; + var isExisting = false; if (url) { if (pkgurlRe.test(url) || localtgzRe.test(url)) { // Git remote url or Tarball url - check the valid package url @@ -158,7 +145,21 @@ async function installModule(module,version,url) { throw e; } } - isUpgrade = checkExistingModule(module,version); + + var info = registry.getModuleInfo(module); + if (info) { + if (!info.user) { + log.debug(`Installing existing module: ${module}`) + isExisting = true; + } else if (!version || info.version === version) { + var err = new Error("Module already loaded"); + err.code = "module_already_loaded"; + throw err; + } + isUpgrade = true; + } else { + isUpgrade = false; + } if (!isUpgrade) { log.info(log._("server.install.installing",{name: module,version: version||"latest"})); @@ -172,7 +173,17 @@ async function installModule(module,version,url) { return exec.run(npmCommand,args,{ cwd: installDir }, true).then(result => { - if (!isUpgrade) { + if (isExisting) { + // This is a module we already have installed as a non-user module. + // That means it was discovered when loading, but was not listed + // in package.json and has been hidden from the editor. + // The user has requested to install this module. Having run + // the npm install above, it will now be listed in package.json. + // Update the registry to mark it as a user module so it will + // be available to the editor. + log.info(log._("server.install.installed",{name:module})); + return require("./registry").setUserInstalled(module,true).then(reportAddedModules); + } else if (!isUpgrade) { log.info(log._("server.install.installed",{name:module})); return require("./index").addModule(module).then(reportAddedModules); } else { @@ -212,7 +223,6 @@ async function installModule(module,version,url) { } function reportAddedModules(info) { - //comms.publish("node/added",info.nodes,false); if (info.nodes.length > 0) { log.info(log._("server.added-types")); for (var i=0;i 0) { + var moduleToLoad = moduleStack.shift(); + var files = localfilesystem.getModuleFiles(moduleToLoad); + if (files[moduleToLoad]) { + moduleFiles[moduleToLoad] = files[moduleToLoad]; + if (moduleFiles[moduleToLoad].dependencies) { + log.debug(`Loading dependencies for ${module}`) + for (var i=0; i module) } catch(err) { return Promise.reject(err); } diff --git a/packages/node_modules/@node-red/registry/lib/localfilesystem.js b/packages/node_modules/@node-red/registry/lib/localfilesystem.js index 25e423571..ed4b81d8e 100644 --- a/packages/node_modules/@node-red/registry/lib/localfilesystem.js +++ b/packages/node_modules/@node-red/registry/lib/localfilesystem.js @@ -28,6 +28,7 @@ let loadDenyList = []; var settings; var disableNodePathScan = false; var iconFileExtensions = [".png", ".gif", ".svg"]; +var packageList = {}; function init(_settings) { settings = _settings; @@ -187,9 +188,17 @@ function scanTreeForNodesModules(moduleName) { var userDir; if (settings.userDir) { + packageList = getPackageList(); userDir = path.join(settings.userDir,"node_modules"); results = scanDirForNodesModules(userDir,moduleName); - results.forEach(function(r) { r.local = true; }); + results.forEach(function(r) { + // If it was found in /node_modules then it is considered + // a local module. + // Also check to see if it is listed in the package.json file as a user-installed + // module. This distinguishes modules installed as a dependency + r.local = true; + r.user = !!packageList[r.package.name]; + }); } if (dir) { @@ -288,20 +297,19 @@ function getNodeFiles(disableNodePathScan) { } } - var nodeList = { - "node-red": { - name: "node-red", - version: settings.version, - nodes: {}, - icons: iconList - } + var nodeList = {}; + var coreNodeEntry = { + name: "node-red", + version: settings.version, + nodes: {}, + icons: iconList } nodeFiles.forEach(function(node) { - nodeList["node-red"].nodes[node.name] = node; + coreNodeEntry.nodes[node.name] = node; }); if (settings.coreNodesDir) { var examplesDir = path.join(settings.coreNodesDir,"examples"); - nodeList["node-red"].examples = {path: examplesDir}; + coreNodeEntry.examples = {path: examplesDir}; } if (!disableNodePathScan) { @@ -310,7 +318,6 @@ function getNodeFiles(disableNodePathScan) { // Filter the module list to ignore global modules // that have also been installed locally - allowing the user to // update a module they may not otherwise be able to touch - moduleFiles.sort(function(A,B) { if (A.local && !B.local) { return -1 @@ -323,7 +330,7 @@ function getNodeFiles(disableNodePathScan) { moduleFiles = moduleFiles.filter(function(mod) { var result; if (!knownModules[mod.package.name]) { - knownModules[mod.package.name] = true; + knownModules[mod.package.name] = mod; result = true; } else { result = false; @@ -332,48 +339,62 @@ function getNodeFiles(disableNodePathScan) { return result; }); - moduleFiles.forEach(function(moduleFile) { - var nodeModuleFiles = getModuleNodeFiles(moduleFile); - nodeList[moduleFile.package.name] = { - name: moduleFile.package.name, - version: moduleFile.package.version, - path: moduleFile.dir, - local: moduleFile.local||false, - nodes: {}, - icons: nodeModuleFiles.icons, - examples: nodeModuleFiles.examples - }; - if (moduleFile.package['node-red'].version) { - nodeList[moduleFile.package.name].redVersion = moduleFile.package['node-red'].version; + // Do a second pass to check we have all the declared node dependencies + // As this is only done as part of the initial palette load, `knownModules` will + // contain a list of everything discovered during this phase. This means + // we can check for missing dependencies here. + moduleFiles = moduleFiles.filter(function(mod) { + if (Array.isArray(mod.package["node-red"].dependencies)) { + const deps = mod.package["node-red"].dependencies; + const missingDeps = mod.package["node-red"].dependencies.filter(dep => { + if (knownModules[dep]) { + knownModules[dep].usedBy = knownModules[dep].usedBy || []; + knownModules[dep].usedBy.push(mod.package.name) + } else { + return true; + } + }) + if (missingDeps.length > 0) { + log.error(`Module: ${mod.package.name} missing dependencies:`); + missingDeps.forEach(m => { log.error(` - ${m}`)}); + return false; + } } - nodeModuleFiles.files.forEach(function(node) { - node.local = moduleFile.local||false; - nodeList[moduleFile.package.name].nodes[node.name] = node; - }); - nodeFiles = nodeFiles.concat(nodeModuleFiles.files); + return true; }); + nodeList = convertModuleFileListToObject(moduleFiles); } else { // console.log("node path scan disabled"); } + nodeList["node-red"] = coreNodeEntry; return nodeList; } function getModuleFiles(module) { - var nodeList = {}; - + // Update the package list var moduleFiles = scanTreeForNodesModules(module); if (moduleFiles.length === 0) { var err = new Error(log._("nodes.registry.localfilesystem.module-not-found", {module:module})); err.code = 'MODULE_NOT_FOUND'; throw err; } + // Unlike when doing the initial palette load, this call cannot verify the + // dependencies of the new module as it doesn't have visiblity of what + // is in the registry. That will have to be done be the caller in loader.js + return convertModuleFileListToObject(moduleFiles); +} +function convertModuleFileListToObject(moduleFiles) { + const nodeList = {}; moduleFiles.forEach(function(moduleFile) { + var nodeModuleFiles = getModuleNodeFiles(moduleFile); nodeList[moduleFile.package.name] = { name: moduleFile.package.name, version: moduleFile.package.version, path: moduleFile.dir, + local: moduleFile.local||false, + user: moduleFile.user||false, nodes: {}, icons: nodeModuleFiles.icons, examples: nodeModuleFiles.examples @@ -381,7 +402,14 @@ function getModuleFiles(module) { if (moduleFile.package['node-red'].version) { nodeList[moduleFile.package.name].redVersion = moduleFile.package['node-red'].version; } + if (moduleFile.package['node-red'].dependencies) { + nodeList[moduleFile.package.name].dependencies = moduleFile.package['node-red'].dependencies; + } + if (moduleFile.usedBy) { + nodeList[moduleFile.package.name].usedBy = moduleFile.usedBy; + } nodeModuleFiles.files.forEach(function(node) { + node.local = moduleFile.local||false; nodeList[moduleFile.package.name].nodes[node.name] = node; nodeList[moduleFile.package.name].nodes[node.name].local = moduleFile.local || false; }); @@ -412,6 +440,23 @@ function scanIconDir(dir) { }) return iconList; } +/** + * Gets the list of modules installed in this runtime as reported by package.json + * Note: these may include non-Node-RED modules + */ +function getPackageList() { + var list = {}; + if (settings.userDir) { + try { + var userPackage = path.join(settings.userDir,"package.json"); + var pkg = JSON.parse(fs.readFileSync(userPackage,"utf-8")); + return pkg.dependencies; + } catch(err) { + log.error(err); + } + } + return list; +} module.exports = { init: init, diff --git a/packages/node_modules/@node-red/registry/lib/registry.js b/packages/node_modules/@node-red/registry/lib/registry.js index 159ec96da..730280d72 100644 --- a/packages/node_modules/@node-red/registry/lib/registry.js +++ b/packages/node_modules/@node-red/registry/lib/registry.js @@ -58,7 +58,8 @@ function filterNodeInfo(n) { name: n.name, types: n.types, enabled: n.enabled, - local: n.local||false + local: n.local||false, + user: n.user || false }; if (n.hasOwnProperty("module")) { r.module = n.module; @@ -94,6 +95,7 @@ function saveNodeList() { name: module, version: moduleConfigs[module].version, local: moduleConfigs[module].local||false, + user: moduleConfigs[module].user||false, nodes: {} }; if (moduleConfigs[module].hasOwnProperty('pending_version')) { @@ -179,6 +181,7 @@ function loadNodeConfigs() { function addModule(module) { moduleNodes[module.name] = []; moduleConfigs[module.name] = module; + // console.log("registry.js.addModule",module.name,"user?",module.user,"usedBy",module.usedBy,"dependencies",module.dependencies) for (var setName in module.nodes) { if (module.nodes.hasOwnProperty(setName)) { var set = module.nodes[setName]; @@ -240,21 +243,47 @@ function removeNode(id) { return filterNodeInfo(config); } -function removeModule(module) { +function removeModule(name,skipSave) { if (!settings.available()) { throw new Error("Settings unavailable"); } - var nodes = moduleNodes[module]; - if (!nodes) { - throw new Error("Unrecognised module: "+module); - } var infoList = []; - for (var i=0;i 0) { + // We are removing a module that is used by other modules... so whilst + // this module should be removed from the editor palette, it needs to + // stay in the runtime... for now. + module.user = false; + for (var i=0;i m !== name); + if (moduleConfigs[dep].usedBy.length === 0) { + // Remove the dependency + removeModule(dep,true); + } + } + }); + } + for (var i=0;i 0)) { + continue; + } var nodes = moduleConfigs[module].nodes; for (var node in nodes) { /* istanbul ignore else */ @@ -346,9 +378,13 @@ function getModuleInfo(module) { name: module, version: moduleConfigs[module].version, local: moduleConfigs[module].local, + user: moduleConfigs[module].user, path: moduleConfigs[module].path, nodes: [] }; + if (moduleConfigs[module].dependencies) { + m.dependencies = moduleConfigs[module].dependencies; + } if (moduleConfigs[module] && moduleConfigs[module].pending_version) { m.pending_version = moduleConfigs[module].pending_version; } @@ -425,7 +461,13 @@ function getAllNodeConfigs(lang) { var script = ""; for (var i=0;i 0)) { + continue; + } + + var config = module.nodes[getNode(id)]; if (config.enabled && !config.err) { result += "\n\n"; result += config.config; @@ -587,6 +629,17 @@ function setModulePendingUpdated(module,version) { }); } +function setUserInstalled(module,userInstalled) { + moduleConfigs[module].user = userInstalled; + return saveNodeList().then(function() { + return getModuleInfo(module); + }); +} +function addModuleDependency(module,usedBy) { + moduleConfigs[module].usedBy = moduleConfigs[module].usedBy || []; + moduleConfigs[module].usedBy.push(usedBy); +} + var icon_paths = { }; var iconCache = {}; @@ -648,6 +701,9 @@ var registry = module.exports = { disableNodeSet: disableNodeSet, setModulePendingUpdated: setModulePendingUpdated, + setUserInstalled: setUserInstalled, + addModuleDependency:addModuleDependency, + removeModule: removeModule, getNodeInfo: getNodeInfo, diff --git a/packages/node_modules/@node-red/runtime/lib/api/nodes.js b/packages/node_modules/@node-red/runtime/lib/api/nodes.js index dfaee4e54..556e57df9 100644 --- a/packages/node_modules/@node-red/runtime/lib/api/nodes.js +++ b/packages/node_modules/@node-red/runtime/lib/api/nodes.js @@ -194,7 +194,7 @@ var api = module.exports = { } if (opts.module) { var existingModule = runtime.nodes.getModuleInfo(opts.module); - if (existingModule) { + if (existingModule && existingModule.user) { if (!opts.version || existingModule.version === opts.version) { runtime.log.audit({event: "nodes.install",module:opts.module, version:opts.version, error:"module_already_loaded"}, opts.req); var err = new Error("Module already loaded"); diff --git a/packages/node_modules/@node-red/runtime/lib/nodes/index.js b/packages/node_modules/@node-red/runtime/lib/nodes/index.js index 2bc7fd61e..73e7baee8 100644 --- a/packages/node_modules/@node-red/runtime/lib/nodes/index.js +++ b/packages/node_modules/@node-red/runtime/lib/nodes/index.js @@ -181,11 +181,12 @@ function installModule(module,version,url) { function uninstallModule(module) { var info = registry.getModuleInfo(module); - if (!info) { + if (!info || !info.user) { throw new Error(log._("nodes.index.unrecognised-module", {module:module})); } else { - for (var i=0;i `${module}/${n.name}`); + for (var i=0;ic;W?p+j07R8~9-04m|k(`l_r$DJ15g%9qNiY%v!9KM8cEEGlzik?VqS zDU9EZxTK-X7~MZTesN&jb_@U*T9idZSezu8kc5w`c-jAReDar7U@pi$C;yfiC;NM` z*gF759KFnPGQOeVf`kezQ*>Z`U>V(i|31n%#Z%Hn%jjbi9RPIZKiV<>6DaEZumAN4 zeC<6R$p1j){GI#{{9u>=-vd;BBb9_`WFSd|H0xY6p)jDXqJAV?Ks`_lj%8a;g@s}e z@~n_Gr9xhjn;GMg0L=M?#^hN@sL;}1jh|Oss*;$TM1ZWyON|pPghsaYQ4*GA0@ZBu zZX+Nf1)&k4VW#Drx8*b&n$-Z)h)o*_7oA$lT?s3yt*r)aepMpooq6X+pW&B4vDW{; zcmDf>fxVXhntzx7-yUvJ26>1=9J|3V3@x9yp+7`jByS4H+9u@yQ56Yv203Yr7ubx( z46rZ?u_OuOH%c0tPN6I)DjJ>NqH8pAT)T^a=GgE*gS&vx6ur3{J5hh&#}>BT$Q`2j z#r&d+SSg#u5F1+;JO1Io_4>rJ2eIQq$wKNPgGM$;>FU*9z1rQdI`%!!kAv9q5A7ga zSqv%=`cbpiroTKnSvn?E1+0t842?imsHz}}8I>lU5y7UU7h_YVgim1%_a)OtbEQu$ z>ed^-jSeG>@B{(L|%SSd3O3bNX({c*$7a;*`83pW`0CbzUh6 zuP)j3$}#-&>h+tWB@Zp9?SruI-|CW0FIy%R8Ks1;h{v;JoA8IMz)=K&Dok*|mB}93 zCf@4WW}D9p5=!og4>@D^goaExAzR}DuPA%p@!ofC;MnT-Si7mVs3k!VoCxyh^=THV z^-vQaurLt7zL4dN1MQci{q+4xFpH zRwgr?XP+96WVAJdGjsh7{pftH6bKG8=Bnt-3V%Fv=&>7e8V&$k5Gx1c^#`#=aZ@`1&rz#P%}jUPb=Z-lAxo zPO~I!x4UAgv9eKKgI!G z(e}Cy1lV=ZU)i;*HojPXUTN1_sRIofO&)^Hui22NPwv8y%mIu z9%Xqig!0>KUd_vadd>GB4q3@VBAORx@9swjQ+;HxEt!0RK(4+{sua8XX=1)-q*4~6 zUN2`M&Qm7j$Qcf8y`SiGD#I2^zc_wUaGPcCdpTJOuv%u%7=D02}}S DM(WP+ literal 0 HcmV?d00001 diff --git a/test/unit/@node-red/registry/lib/installer_spec.js b/test/unit/@node-red/registry/lib/installer_spec.js index e0e7380f6..c394956f6 100644 --- a/test/unit/@node-red/registry/lib/installer_spec.js +++ b/test/unit/@node-red/registry/lib/installer_spec.js @@ -131,6 +131,7 @@ describe('nodes/registry/installer', function() { it("rejects when update requested to existing version", function(done) { sinon.stub(typeRegistry,"getModuleInfo", function() { return { + user: true, version: "0.1.1" } }); @@ -142,6 +143,7 @@ describe('nodes/registry/installer', function() { it("rejects when update requested to existing version and url", function(done) { sinon.stub(typeRegistry,"getModuleInfo", function() { return { + user: true, version: "0.1.1" } }); diff --git a/test/unit/@node-red/registry/lib/loader_spec.js b/test/unit/@node-red/registry/lib/loader_spec.js index 274a20d44..55f435069 100644 --- a/test/unit/@node-red/registry/lib/loader_spec.js +++ b/test/unit/@node-red/registry/lib/loader_spec.js @@ -528,7 +528,7 @@ describe("red/nodes/registry/loader",function() { stubs.push(sinon.stub(nodes,"registerType")); loader.init({nodes:nodes,log:{info:function(){},_:function(){}},settings:{available:function(){return true;}}}); loader.addModule("TestNodeModule").then(function(result) { - result.should.eql("a node list"); + result.should.eql("TestNodeModule"); registry.addModule.called.should.be.true(); var module = registry.addModule.lastCall.args[0]; @@ -585,7 +585,7 @@ describe("red/nodes/registry/loader",function() { stubs.push(sinon.stub(nodes,"registerType")); loader.init({log:{"_":function(){},warn:function(){}},nodes:nodes,version: function() { return "0.12.0"}, settings:{available:function(){return true;}}}); loader.addModule("TestNodeModule").then(function(result) { - result.should.eql("a node list"); + result.should.eql("TestNodeModule"); registry.addModule.called.should.be.false(); nodes.registerType.called.should.be.false(); done();