From 38c9b1afa388671f6f053b222dc7bba69532257f Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Tue, 23 Jan 2018 22:17:24 +0000 Subject: [PATCH 01/23] removes the sync logic, we don't use it --- architect.js | 93 +++------------------------------------------------- 1 file changed, 4 insertions(+), 89 deletions(-) diff --git a/architect.js b/architect.js index 090b564..8669bce 100644 --- a/architect.js +++ b/architect.js @@ -13,13 +13,12 @@ var DEBUG = typeof location != "undefined" && location.href.match(/debug=[123]/) if (typeof module === "object") (function () { var dirname = require('path').dirname; var resolve = require('path').resolve; - var existsSync = require('fs').existsSync || require('path').existsSync; - var realpathSync = require('fs').realpathSync; var exists = require('fs').exists || require('path').exists; var realpath = require('fs').realpath; var packagePathCache = {}; var basePath; + exports.loadConfig = loadConfig; exports.resolveConfig = resolveConfig; @@ -41,31 +40,7 @@ if (typeof module === "object") (function () { basePath = base; } - if (!callback) - return resolveConfigSync(config, base); - else - resolveConfigAsync(config, base, callback); - } - - function resolveConfigSync(config, base) { - config.forEach(function (plugin, index) { - // Shortcut where string is used for plugin without any options. - if (typeof plugin === "string") { - plugin = config[index] = { packagePath: plugin }; - } - // The plugin is a package on the disk. We need to load it. - if (plugin.hasOwnProperty("packagePath") && !plugin.hasOwnProperty("setup")) { - var defaults = resolveModuleSync(base, plugin.packagePath); - Object.keys(defaults).forEach(function (key) { - if (!plugin.hasOwnProperty(key)) { - plugin[key] = defaults[key]; - } - }); - plugin.packagePath = defaults.packagePath; - plugin.setup = require(plugin.packagePath); - } - }); - return config; + resolveConfigAsync(config, base, callback); } function resolveConfigAsync(config, base, callback) { @@ -108,29 +83,6 @@ if (typeof module === "object") (function () { resolveNext(0); } - // Loads a module, getting metadata from either it's package.json or export - // object. - function resolveModuleSync(base, modulePath) { - var packagePath; - try { - packagePath = resolvePackageSync(base, modulePath + "/package.json"); - } - catch (err) { - if (err.code !== "ENOENT") throw err; - } - var metadata = packagePath && require(packagePath).plugin || {}; - if (packagePath) { - modulePath = dirname(packagePath); - } else { - modulePath = resolvePackageSync(base, modulePath); - } - var module = require(modulePath); - metadata.provides = metadata.provides || module.provides || []; - metadata.consumes = metadata.consumes || module.consumes || []; - metadata.packagePath = modulePath; - return metadata; - } - // Loads a module, getting metadata from either it's package.json or export // object. function resolveModule(base, modulePath, callback) { @@ -175,45 +127,6 @@ if (typeof module === "object") (function () { }); } - // Node style package resolving so that plugins' package.json can be found relative to the config file - // It's not the full node require system algorithm, but it's the 99% case - // This throws, make sure to wrap in try..catch - function resolvePackageSync(base, packagePath) { - var originalBase = base; - if (!(base in packagePathCache)) { - packagePathCache[base] = {}; - } - var cache = packagePathCache[base]; - if (packagePath in cache) { - return cache[packagePath]; - } - var newPath; - if (packagePath[0] === "." || packagePath[0] === "/") { - newPath = resolve(base, packagePath); - if (!existsSync(newPath)) { - newPath = newPath + ".js"; - } - if (existsSync(newPath)) { - newPath = realpathSync(newPath); - cache[packagePath] = newPath; - return newPath; - } - } - else { - while (base) { - newPath = resolve(base, "node_modules", packagePath); - if (existsSync(newPath)) { - newPath = realpathSync(newPath); - cache[packagePath] = newPath; - return newPath; - } - base = resolve(base, '..'); - } - } - var err = new Error("Can't find '" + packagePath + "' relative to '" + originalBase + "'"); - err.code = "ENOENT"; - throw err; - } function resolvePackage(base, packagePath, callback) { var originalBase = base; @@ -224,8 +137,10 @@ if (typeof module === "object") (function () { if (cache.hasOwnProperty(packagePath)) { return callback(null, cache[packagePath]); } + if (packagePath[0] === "." || packagePath[0] === "/") { var newPath = resolve(base, packagePath); + exists(newPath, function(exists) { if (exists) { realpath(newPath, function(err, newPath) { From 061a95f196409f82f167036d653fed6b41a65523 Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Tue, 23 Jan 2018 22:18:17 +0000 Subject: [PATCH 02/23] adds a promise style handler --- architect.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/architect.js b/architect.js index 8669bce..f4bd6f7 100644 --- a/architect.js +++ b/architect.js @@ -40,6 +40,15 @@ if (typeof module === "object") (function () { basePath = base; } + if (!callback) { + return new Promise(function (resolve, reject) { + resolveConfigAsync(config, base, (err, config) =>{ + if (err) return reject(err); + resolve(config); + }); + }); + } + resolveConfigAsync(config, base, callback); } From 368f8c943c2563429db0d5633b83fdd8a97fb829 Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Tue, 23 Jan 2018 22:18:32 +0000 Subject: [PATCH 03/23] adds a simple test --- architect_test.js | 94 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 architect_test.js diff --git a/architect_test.js b/architect_test.js new file mode 100644 index 0000000..0a08614 --- /dev/null +++ b/architect_test.js @@ -0,0 +1,94 @@ +const architect = require("./architect"); +const test = require("tape"); +const fs = require("fs"); +const promisify = require("util").promisify; +const path = require("path"); + +const readFile = promisify(fs.readFile); +const unlink = promisify(fs.unlink); +const writeFile = promisify(fs.writeFile); +const mkdirp = promisify(require("mkdirp")); + +test("resolve config resolved", assert => { + const config = [ + { + setup: function(){ + // noop + }, + provides: ["foo"], + consumes: ["foo"] + } + ]; + + architect.resolveConfig(config, "", (err, resolvedConfig) => { + assert.ok(!err, "no error"); + assert.deepEqual(resolvedConfig, config); + assert.end(); + }); +}); + +test("resolve config from basepath + node_modules", async (assert) => { + const fakePlugin = ` + module.exports = { + setup: function(){ + // noop + }, + provides: ["foo"], + consumes: ["foo"] + } + `; + + let packagePath = "_fake/plugin_" + Date.now(); + let packageDir = "/tmp/_architect_test_fixtures/node_modules"; + let fullPath = packageDir + "/" + packagePath + ".js"; + + let config = [ + packagePath, + ]; + + await mkdirp(path.dirname(fullPath)); + await writeFile(fullPath, fakePlugin.toString()); + + architect.resolveConfig(config, path.dirname(packageDir), async (err, resolvedConfig) => { + assert.ok(!err); + + assert.equal(resolvedConfig[0].packagePath, fullPath); + assert.deepEqual(resolvedConfig[0].consumes, ["foo"]); + assert.deepEqual(resolvedConfig[0].provides, ["foo"]); + + await unlink(fullPath); + assert.end(); + }); +}); + +test("resolve config from basepath + node_modules, async", async (assert) => { + const fakePlugin = ` + module.exports = { + setup: function(){ + // noop + }, + provides: ["foo"], + consumes: ["foo"] + } + `; + + let packagePath = "_fake/plugin_" + Date.now(); + let packageDir = "/tmp/_architect_test_fixtures/node_modules"; + let fullPath = packageDir + "/" + packagePath + ".js"; + + let config = [ + packagePath, + ]; + + await mkdirp(path.dirname(fullPath)); + await writeFile(fullPath, fakePlugin.toString()); + + let resolvedConfig = await architect.resolveConfig(config, path.dirname(packageDir)); + + assert.equal(resolvedConfig[0].packagePath, fullPath); + assert.deepEqual(resolvedConfig[0].consumes, ["foo"]); + assert.deepEqual(resolvedConfig[0].provides, ["foo"]); + + await unlink(fullPath); + assert.end(); +}); From e54faec6db27746fbc89ecc91a7282b8c827fbe6 Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Tue, 23 Jan 2018 22:19:03 +0000 Subject: [PATCH 04/23] adds a bunch of test dependencies --- package.json | 65 +++++++++++++++++++++++++++------------------------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/package.json b/package.json index d440745..3f75f4b 100644 --- a/package.json +++ b/package.json @@ -1,34 +1,37 @@ { - "name": "architect", - "description": "A Simple yet powerful plugin system for node applications", - "version": "0.1.13", - "author": "ajax.org B.V. ", - "contributors": [ - { - "name": "Tim Caswell", - "email": "tim@c9.io" - }, - { - "name": "Fabian Jakobs", - "email": "fabian@c9.io" - }, - { - "name": "Christoph Dorn", - "email": "christoph@christophdorn.com" - } - ], - "main": "architect.js", - "repository": { - "type": "git", - "url": "http://github.com/c9/architect.git" + "name": "architect", + "description": "A Simple yet powerful plugin system for node applications", + "version": "0.1.13", + "author": "ajax.org B.V. ", + "contributors": [ + { + "name": "Tim Caswell", + "email": "tim@c9.io" }, - "dependencies": {}, - "devDependencies": {}, - "optionalDependencies": {}, - "licenses": [ - { - "type": "MIT", - "url": "http://github.com/c9/architect/raw/master/LICENSE" - } - ] + { + "name": "Fabian Jakobs", + "email": "fabian@c9.io" + }, + { + "name": "Christoph Dorn", + "email": "christoph@christophdorn.com" + } + ], + "main": "architect.js", + "repository": { + "type": "git", + "url": "http://github.com/c9/architect.git" + }, + "dependencies": {}, + "devDependencies": { + "mkdirp": "^0.5.1", + "tape": "^4.8.0" + }, + "optionalDependencies": {}, + "licenses": [ + { + "type": "MIT", + "url": "http://github.com/c9/architect/raw/master/LICENSE" + } + ] } From 96108ec02913ec77e2bbd450f6ee8499257a32b4 Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Thu, 25 Jan 2018 22:47:09 +0000 Subject: [PATCH 05/23] simplify resolve --- architect.js | 138 +++++++++++++++------------------------------------ 1 file changed, 41 insertions(+), 97 deletions(-) diff --git a/architect.js b/architect.js index f4bd6f7..e28c383 100644 --- a/architect.js +++ b/architect.js @@ -17,6 +17,28 @@ if (typeof module === "object") (function () { var realpath = require('fs').realpath; var packagePathCache = {}; var basePath; + + var fs = require("fs"); + var path = require("path"); + + function findPackagePath(packagePath, paths) { + paths = paths.reduce((paths, basePath) => { + while(basePath != "/") { + paths.push(path.resolve(basePath, "node_modules", packagePath, "package.json")); + paths.push(path.resolve(basePath, packagePath, "package.json")); + paths.push(path.resolve(basePath, "node_modules", packagePath + ".js")); + paths.push(path.resolve(basePath, packagePath + ".js")); + basePath = path.resolve(basePath, ".."); + } + + return paths; + }, []); + + for (let packagePath of paths) { + if (fs.existsSync(packagePath)) + return packagePath; + } + } exports.loadConfig = loadConfig; @@ -95,108 +117,30 @@ if (typeof module === "object") (function () { // Loads a module, getting metadata from either it's package.json or export // object. function resolveModule(base, modulePath, callback) { - resolvePackage(base, modulePath + "/package.json", function(err, packagePath) { - //if (err && err.code !== "ENOENT") return callback(err); - - var metadata = {}; - if (!err) { - try { - metadata = packagePath && require(packagePath).plugin || {}; - } catch(e) { - return callback(e); - } - } - - (function(next) { - if (err) { - //@todo Fabian what is a better way? - resolvePackage(base, modulePath + ".js", next); - } - else if (packagePath) { - next(null, dirname(packagePath)); - } - else { - resolvePackage(base, modulePath, next); - } - })(function(err, modulePath) { - if (err) return callback(err); - - var module; - try { - module = require(modulePath); - } catch(e) { - return callback(e); - } - - metadata.provides = metadata.provides || module.provides || []; - metadata.consumes = metadata.consumes || module.consumes || []; - metadata.packagePath = modulePath; - callback(null, metadata); - }); - }); - } - - - function resolvePackage(base, packagePath, callback) { - var originalBase = base; - if (!packagePathCache.hasOwnProperty(base)) { - packagePathCache[base] = {}; - } - var cache = packagePathCache[base]; - if (cache.hasOwnProperty(packagePath)) { - return callback(null, cache[packagePath]); - } + var packagePath = findPackagePath(modulePath, [base]); - if (packagePath[0] === "." || packagePath[0] === "/") { - var newPath = resolve(base, packagePath); - - exists(newPath, function(exists) { - if (exists) { - realpath(newPath, function(err, newPath) { - if (err) return callback(err); - - cache[packagePath] = newPath; - return callback(null, newPath); - }); - } else { - var err = new Error("Can't find '" + packagePath + "' relative to '" + originalBase + "'"); - err.code = "ENOENT"; - return callback(err); - } - }); + if (!packagePath) { + var err = new Error("Can't find '" + packagePath + "' relative to '" + base + "'"); + err.code = "ENOENT"; + return callback(err); } - else { - tryNext(base); - } - - function tryNext(base) { - if (base == "/") { - var err = new Error("Can't find '" + packagePath + "' relative to '" + originalBase + "'"); - err.code = "ENOENT"; - return callback(err); - } - - var newPath = resolve(base, "node_modules", packagePath); - exists(newPath, function(exists) { - if (exists) { - realpath(newPath, function(err, newPath) { - if (err) return callback(err); - - cache[packagePath] = newPath; - return callback(null, newPath); - }); - } else { - var nextBase = resolve(base, '..'); - if (nextBase === base) - tryNext("/"); // for windows - else - tryNext(nextBase); - } - }); + + + var metadata = require(packagePath); + metadata.packagePath = packagePath; + + if (/package[.].json$/.test(packagePath)) { + metadata = metadata.module; + modulePath = require.resolve(path.dirname(packagePath)); + var module = require(modulePath); + metadata.provides = metadata.provides || module.provides || []; + metadata.consumes = metadata.consumes || module.consumes || []; + metadata.packagePath = modulePath; } + + return callback(null, metadata); } - }()); // Otherwise use amd to load modules. From 6b5f1af70c487485314e41c55a2de447c173708c Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Thu, 25 Jan 2018 23:16:41 +0000 Subject: [PATCH 06/23] use more async/await --- architect.js | 107 ++++++++++++++++++--------------------------------- 1 file changed, 38 insertions(+), 69 deletions(-) diff --git a/architect.js b/architect.js index e28c383..449f567 100644 --- a/architect.js +++ b/architect.js @@ -12,12 +12,6 @@ var DEBUG = typeof location != "undefined" && location.href.match(/debug=[123]/) // Only define Node-style usage using sync I/O if in node. if (typeof module === "object") (function () { var dirname = require('path').dirname; - var resolve = require('path').resolve; - var exists = require('fs').exists || require('path').exists; - var realpath = require('fs').realpath; - var packagePathCache = {}; - var basePath; - var fs = require("fs"); var path = require("path"); @@ -40,12 +34,9 @@ if (typeof module === "object") (function () { } } - exports.loadConfig = loadConfig; exports.resolveConfig = resolveConfig; - // This is assumed to be used at startup and uses sync I/O as well as can - // throw exceptions. It loads and parses a config file. function loadConfig(configPath, callback) { var config = require(configPath); var base = dirname(configPath); @@ -53,79 +44,57 @@ if (typeof module === "object") (function () { return resolveConfig(config, base, callback); } - function resolveConfig(config, base, callback) { - if(typeof base === 'function') { - // probably being called from loadAdditionalConfig, use saved base - callback = base; - base = basePath; - } else { - basePath = base; - } + async function resolveConfig(config, base, callback) { + config = config.map((plugin) => { + if (typeof plugin === "string") + return { packagePath: plugin }; + return plugin; + }); - if (!callback) { - return new Promise(function (resolve, reject) { - resolveConfigAsync(config, base, (err, config) =>{ - if (err) return reject(err); - resolve(config); - }); - }); + let err = null; + + try { + config = await resolveConfigAsync(config, base); + } + catch (_err) { + err = _err; + if (!callback) throw err; } - resolveConfigAsync(config, base, callback); - } - - function resolveConfigAsync(config, base, callback) { - function resolveNext(i) { - if (i >= config.length) { - return callback(null, config); - } - - var plugin = config[i]; + if (callback) + return callback(err, config); - // Shortcut where string is used for plugin without any options. - if (typeof plugin === "string") { - plugin = config[i] = { packagePath: plugin }; - } - // The plugin is a package on the disk. We need to load it. - if (plugin.hasOwnProperty("packagePath") && !plugin.hasOwnProperty("setup")) { - resolveModule(base, plugin.packagePath, function(err, defaults) { - if (err) return callback(err); - - Object.keys(defaults).forEach(function (key) { - if (!plugin.hasOwnProperty(key)) { - plugin[key] = defaults[key]; - } - }); - plugin.packagePath = defaults.packagePath; - try { - plugin.setup = require(plugin.packagePath); - } catch(e) { - return callback(e); - } - - return resolveNext(++i); - }); - return; - } + return config; + } - return resolveNext(++i); + async function resolveConfigAsync(config, base, callback) { + for (let plugin of config) { + if (plugin.hasOwnProperty("setup")) + continue; + + let defaults = await resolveModule(base, plugin.packagePath); + + Object.keys(defaults).forEach(function (key) { + if (!plugin.hasOwnProperty(key)) { + plugin[key] = defaults[key]; + } + }); + plugin.packagePath = defaults.packagePath; + plugin.setup = defaults.setup; } - - resolveNext(0); + + return config; } - // Loads a module, getting metadata from either it's package.json or export - // object. - function resolveModule(base, modulePath, callback) { + function resolveModule(base, modulePath) { var packagePath = findPackagePath(modulePath, [base]); if (!packagePath) { var err = new Error("Can't find '" + packagePath + "' relative to '" + base + "'"); err.code = "ENOENT"; - return callback(err); + throw err; } - - + var metadata = require(packagePath); metadata.packagePath = packagePath; @@ -138,7 +107,7 @@ if (typeof module === "object") (function () { metadata.packagePath = modulePath; } - return callback(null, metadata); + return metadata; } }()); From 2918343342da6ac50df9f3820a095f9a5cbbc024 Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Thu, 25 Jan 2018 23:23:56 +0000 Subject: [PATCH 07/23] cleanup; split async code path using promises --- architect.js | 41 +++++++++++++++-------------------------- architect_test.js | 2 +- 2 files changed, 16 insertions(+), 27 deletions(-) diff --git a/architect.js b/architect.js index 449f567..88859a2 100644 --- a/architect.js +++ b/architect.js @@ -11,7 +11,6 @@ var DEBUG = typeof location != "undefined" && location.href.match(/debug=[123]/) // Only define Node-style usage using sync I/O if in node. if (typeof module === "object") (function () { - var dirname = require('path').dirname; var fs = require("fs"); var path = require("path"); @@ -34,40 +33,30 @@ if (typeof module === "object") (function () { } } - exports.loadConfig = loadConfig; exports.resolveConfig = resolveConfig; - - function loadConfig(configPath, callback) { - var config = require(configPath); - var base = dirname(configPath); - - return resolveConfig(config, base, callback); + + function resolveConfigAsync(config, base, callback) { + loadPlugin(config, base) + .then(config => callback(null, config)) + .catch(err => callback(err)); + } + + function normalize(plugin) { + if (typeof plugin === "string") + return { packagePath: plugin }; + return plugin; } async function resolveConfig(config, base, callback) { - config = config.map((plugin) => { - if (typeof plugin === "string") - return { packagePath: plugin }; - return plugin; - }); - - let err = null; + config = config.map(normalize); - try { - config = await resolveConfigAsync(config, base); - } - catch (_err) { - err = _err; - if (!callback) throw err; - } - if (callback) - return callback(err, config); + return resolveConfigAsync(config, base, callback); - return config; + return loadPlugin(config, base); } - async function resolveConfigAsync(config, base, callback) { + async function loadPlugin(config, base, callback) { for (let plugin of config) { if (plugin.hasOwnProperty("setup")) continue; diff --git a/architect_test.js b/architect_test.js index 0a08614..781ec6c 100644 --- a/architect_test.js +++ b/architect_test.js @@ -75,7 +75,7 @@ test("resolve config from basepath + node_modules, async", async (assert) => { let packagePath = "_fake/plugin_" + Date.now(); let packageDir = "/tmp/_architect_test_fixtures/node_modules"; let fullPath = packageDir + "/" + packagePath + ".js"; - + let config = [ packagePath, ]; From c362ecabdf0c78a85442b0111d1a379ce5a0d6dc Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Thu, 25 Jan 2018 23:42:59 +0000 Subject: [PATCH 08/23] simplify a bit --- architect.js | 57 ++++++++++++++++++++++------------------------------ 1 file changed, 24 insertions(+), 33 deletions(-) diff --git a/architect.js b/architect.js index 88859a2..73f1fc7 100644 --- a/architect.js +++ b/architect.js @@ -11,8 +11,8 @@ var DEBUG = typeof location != "undefined" && location.href.match(/debug=[123]/) // Only define Node-style usage using sync I/O if in node. if (typeof module === "object") (function () { - var fs = require("fs"); - var path = require("path"); + const fs = require("fs"); + const path = require("path"); function findPackagePath(packagePath, paths) { paths = paths.reduce((paths, basePath) => { @@ -61,42 +61,33 @@ if (typeof module === "object") (function () { if (plugin.hasOwnProperty("setup")) continue; - let defaults = await resolveModule(base, plugin.packagePath); + var packagePath = findPackagePath(plugin.packagePath, [].concat(base)); - Object.keys(defaults).forEach(function (key) { - if (!plugin.hasOwnProperty(key)) { - plugin[key] = defaults[key]; - } - }); - plugin.packagePath = defaults.packagePath; - plugin.setup = defaults.setup; - } - - return config; - } + if (!packagePath) + throw packageNotFoundError(plugin.packagePath, base); - function resolveModule(base, modulePath) { - var packagePath = findPackagePath(modulePath, [base]); - - if (!packagePath) { - var err = new Error("Can't find '" + packagePath + "' relative to '" + base + "'"); - err.code = "ENOENT"; - throw err; - } + var metadata = require(packagePath); + metadata.packagePath = packagePath; + + if (/package[.].json$/.test(packagePath)) { + metadata = metadata.module; + let modulePath = require.resolve(path.dirname(packagePath)); + let module = require(modulePath); + metadata.provides = metadata.provides || module.provides || []; + metadata.consumes = metadata.consumes || module.consumes || []; + metadata.packagePath = modulePath; + } - var metadata = require(packagePath); - metadata.packagePath = packagePath; - - if (/package[.].json$/.test(packagePath)) { - metadata = metadata.module; - modulePath = require.resolve(path.dirname(packagePath)); - var module = require(modulePath); - metadata.provides = metadata.provides || module.provides || []; - metadata.consumes = metadata.consumes || module.consumes || []; - metadata.packagePath = modulePath; + Object.assign(plugin, metadata); } - return metadata; + return config; + } + + function packageNotFoundError(packagePath, base) { + var err = new Error(`Can't find ${packagePath} relative to ${base}`); + err.code = "ENOENT"; + return err; } }()); From e040ac941801ff7c40750299ee470dea4a90b253 Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Thu, 25 Jan 2018 23:43:45 +0000 Subject: [PATCH 09/23] let is the better var --- architect.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/architect.js b/architect.js index 73f1fc7..f827429 100644 --- a/architect.js +++ b/architect.js @@ -61,12 +61,12 @@ if (typeof module === "object") (function () { if (plugin.hasOwnProperty("setup")) continue; - var packagePath = findPackagePath(plugin.packagePath, [].concat(base)); + let packagePath = findPackagePath(plugin.packagePath, [].concat(base)); if (!packagePath) throw packageNotFoundError(plugin.packagePath, base); - var metadata = require(packagePath); + let metadata = require(packagePath); metadata.packagePath = packagePath; if (/package[.].json$/.test(packagePath)) { @@ -85,7 +85,7 @@ if (typeof module === "object") (function () { } function packageNotFoundError(packagePath, base) { - var err = new Error(`Can't find ${packagePath} relative to ${base}`); + let err = new Error(`Can't find ${packagePath} relative to ${base}`); err.code = "ENOENT"; return err; } From b72992f1a00be001dfc171dd9f7905ebe7b4864c Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Mon, 29 Jan 2018 22:16:09 +0000 Subject: [PATCH 10/23] use class, not function --- architect.js | 761 ++++++++++++++++++++++++++------------------------- 1 file changed, 389 insertions(+), 372 deletions(-) diff --git a/architect.js b/architect.js index f827429..d02ef7b 100644 --- a/architect.js +++ b/architect.js @@ -1,421 +1,438 @@ ( // Module boilerplate to support node.js and AMD. - (typeof module !== "undefined" && function (m) { module.exports = m(require('events')); }) || - (typeof define === "function" && function (m) { define(["events"], m); }) -)(function (events) { -"use strict"; -var EventEmitter = events.EventEmitter; - -var exports = {}; - -var DEBUG = typeof location != "undefined" && location.href.match(/debug=[123]/) ? true : false; - -// Only define Node-style usage using sync I/O if in node. -if (typeof module === "object") (function () { - const fs = require("fs"); - const path = require("path"); - - function findPackagePath(packagePath, paths) { - paths = paths.reduce((paths, basePath) => { - while(basePath != "/") { - paths.push(path.resolve(basePath, "node_modules", packagePath, "package.json")); - paths.push(path.resolve(basePath, packagePath, "package.json")); - paths.push(path.resolve(basePath, "node_modules", packagePath + ".js")); - paths.push(path.resolve(basePath, packagePath + ".js")); - basePath = path.resolve(basePath, ".."); + (typeof module !== "undefined" && function(m) { module.exports = m(require('events')); }) || + (typeof define === "function" && function(m) { define(["events"], m); }) +)(function(events) { + "use strict"; + var EventEmitter = events.EventEmitter; + + var exports = {}; + + var DEBUG = typeof location != "undefined" && location.href.match(/debug=[123]/) ? true : false; + + // Only define Node-style usage using sync I/O if in node. + if (typeof module === "object")(function() { + const fs = require("fs"); + const path = require("path"); + + function findPackagePath(packagePath, paths) { + paths = paths.reduce((paths, basePath) => { + while (basePath != "/") { + paths.push(path.resolve(basePath, "node_modules", packagePath, "package.json")); + paths.push(path.resolve(basePath, packagePath, "package.json")); + paths.push(path.resolve(basePath, "node_modules", packagePath + ".js")); + paths.push(path.resolve(basePath, packagePath + ".js")); + basePath = path.resolve(basePath, ".."); + } + + return paths; + }, []); + + for (let packagePath of paths) { + if (fs.existsSync(packagePath)) + return packagePath; } - - return paths; - }, []); - - for (let packagePath of paths) { - if (fs.existsSync(packagePath)) - return packagePath; } - } - exports.resolveConfig = resolveConfig; - - function resolveConfigAsync(config, base, callback) { - loadPlugin(config, base) - .then(config => callback(null, config)) - .catch(err => callback(err)); - } - - function normalize(plugin) { - if (typeof plugin === "string") - return { packagePath: plugin }; - return plugin; - } + exports.resolveConfig = resolveConfig; - async function resolveConfig(config, base, callback) { - config = config.map(normalize); - - if (callback) - return resolveConfigAsync(config, base, callback); + function resolveConfigAsync(config, base, callback) { + loadPlugin(config, base) + .then(config => callback(null, config)) + .catch(err => callback(err)); + } - return loadPlugin(config, base); - } + function normalize(plugin) { + if (typeof plugin === "string") + return { packagePath: plugin }; + return plugin; + } - async function loadPlugin(config, base, callback) { - for (let plugin of config) { - if (plugin.hasOwnProperty("setup")) - continue; - - let packagePath = findPackagePath(plugin.packagePath, [].concat(base)); - - if (!packagePath) - throw packageNotFoundError(plugin.packagePath, base); - - let metadata = require(packagePath); - metadata.packagePath = packagePath; - - if (/package[.].json$/.test(packagePath)) { - metadata = metadata.module; - let modulePath = require.resolve(path.dirname(packagePath)); - let module = require(modulePath); - metadata.provides = metadata.provides || module.provides || []; - metadata.consumes = metadata.consumes || module.consumes || []; - metadata.packagePath = modulePath; - } + async function resolveConfig(config, base, callback) { + config = config.map(normalize); - Object.assign(plugin, metadata); + if (callback) + return resolveConfigAsync(config, base, callback); + + return loadPlugin(config, base); } - - return config; - } - - function packageNotFoundError(packagePath, base) { - let err = new Error(`Can't find ${packagePath} relative to ${base}`); - err.code = "ENOENT"; - return err; - } -}()); + async function loadPlugin(config, base) { + for (let plugin of config) { + if (plugin.hasOwnProperty("setup")) + continue; -// Otherwise use amd to load modules. -else (function () { - exports.loadConfig = loadConfig; - exports.resolveConfig = resolveConfig; + let packagePath = findPackagePath(plugin.packagePath, [].concat(base)); - function loadConfig(path, callback) { - require([path], function (config) { - resolveConfig(config, callback); - }); - } + if (!packagePath) + throw packageNotFoundError(plugin.packagePath, base); - function resolveConfig(config, base, callback, errback) { - if (typeof base == "function") - return resolveConfig(config, "", arguments[1], arguments[2]); - - var paths = [], pluginIndexes = {}; - config.forEach(function (plugin, index) { - // Shortcut where string is used for plugin without any options. - if (typeof plugin === "string") { - plugin = config[index] = { packagePath: plugin }; - } - // The plugin is a package over the network. We need to load it. - if (plugin.hasOwnProperty("packagePath") && !plugin.hasOwnProperty("setup")) { - paths.push((base || "") + plugin.packagePath); - pluginIndexes[plugin.packagePath] = index; - } - }); - // Mass-Load path-based plugins using amd's require - require(paths, function () { - var args = arguments; - paths.forEach(function (name, i) { - var module = args[i]; - var plugin = config[pluginIndexes[name]]; - plugin.setup = module; - plugin.provides = module.provides || plugin.provides || []; - plugin.consumes = module.consumes || plugin.consumes || []; - }); - callback(null, config); - }, errback); - } -}()); + let metadata = require(packagePath); + metadata.packagePath = packagePath; -exports.createApp = createApp; -exports.Architect = Architect; + if (/package[.].json$/.test(packagePath)) { + metadata = metadata.module; + let modulePath = require.resolve(path.dirname(packagePath)); + let module = require(modulePath); + metadata.provides = metadata.provides || module.provides || []; + metadata.consumes = metadata.consumes || module.consumes || []; + metadata.packagePath = modulePath; + } -// Check a plugin config list for bad dependencies and throw on error -function checkConfig(config, lookup) { + Object.assign(plugin, metadata); + } - // Check for the required fields in each plugin. - config.forEach(function (plugin) { - if (plugin.checked) { return; } - if (!plugin.hasOwnProperty("setup")) { - throw new Error("Plugin is missing the setup function " + JSON.stringify(plugin)); + return config; } - if (!plugin.hasOwnProperty("provides")) { - throw new Error("Plugin is missing the provides array " + JSON.stringify(plugin)); + + function packageNotFoundError(packagePath, base) { + let err = new Error(`Can't find ${packagePath} relative to ${base}`); + err.code = "ENOENT"; + return err; } - if (!plugin.hasOwnProperty("consumes")) { - throw new Error("Plugin is missing the consumes array " + JSON.stringify(plugin)); + + }()); + + // Otherwise use amd to load modules. + else(function() { + exports.loadConfig = loadConfig; + exports.resolveConfig = resolveConfig; + + function loadConfig(path, callback) { + require([path], function(config) { + resolveConfig(config, callback); + }); } - }); - - return checkCycles(config, lookup); -} - -function checkCycles(config, lookup) { - var plugins = []; - config.forEach(function(pluginConfig, index) { - plugins.push({ - packagePath: pluginConfig.packagePath, - provides: pluginConfig.provides.concat(), - consumes: pluginConfig.consumes.concat(), - i: index - }); - }); - - var resolved = { - hub: true - }; - var changed = true; - var sorted = []; - - while(plugins.length && changed) { - changed = false; - - plugins.concat().forEach(function(plugin) { - var consumes = plugin.consumes.concat(); - - var resolvedAll = true; - for (var i=0; i Date: Mon, 29 Jan 2018 22:16:31 +0000 Subject: [PATCH 11/23] adds sanity tests --- architect_test.js | 150 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) diff --git a/architect_test.js b/architect_test.js index 781ec6c..d45bbe4 100644 --- a/architect_test.js +++ b/architect_test.js @@ -92,3 +92,153 @@ test("resolve config from basepath + node_modules, async", async (assert) => { await unlink(fullPath); assert.end(); }); + +test("it should start an architect app (classic)", async (assert) => { + const fakeConfig = [ + { + packagePath: "foo/plugin", + setup: function(config, imports, register) { + register(null); + }, + provides: [], + consumes: ["bar.plugin"] + }, + { + packagePath: "bar/plugin", + setup: function(config, imports, register) { + register(null, { + "bar.plugin": { + iamBar: true + } + }); + }, + provides: ["bar.plugin"], + consumes: [] + } + ]; + + architect.createApp(fakeConfig, (err) => { + assert.ok(!err, "no err"); + assert.end(); + }); +}); + +test("it detects cyclic dependencies (classic)", async (assert) => { + const fakeConfig = [ + { + packagePath: "foo/plugin", + setup: function(config, imports, register) { + }, + provides: ["foo.plugin"], + consumes: ["bar.plugin"] + }, + { + packagePath: "bar/plugin", + setup: function(config, imports, register) { + }, + provides: ["bar.plugin"], + consumes: ["foo.plugin"] + } + ]; + + architect.createApp(fakeConfig, (err) => { + let expect = "Could not resolve dependencies\nConfig contains cyclic dependencies"; + assert.equal(err.message, expect); + assert.end(); + }); +}); + +test("it checks the provides", async (assert) => { + const fakeConfig = [ + { + packagePath: "foo/plugin", + setup: function(config, imports, register) { + register(null); + }, + provides: [], + consumes: ["bar.plugin"] + }, + { + packagePath: "bar/plugin", + setup: function(config, imports, register) { + register(null, {}); + }, + provides: ["bar.plugin"], + consumes: [] + } + ]; + + architect.createApp(fakeConfig, (err) => { + assert.ok(/Plugin failed to provide bar.plugin service/.test(err.message)); + assert.end(); + }); +}); + +test("it checks all dependencies", async (assert) => { + const fakeConfig = [ + { + packagePath: "foo/plugin", + setup: function(config, imports, register) { + }, + provides: ["foo.plugin"], + consumes: ["bar.plugin"] + }, + { + packagePath: "bar/plugin", + setup: function(config, imports, register) { + }, + provides: [], + consumes: [] + } + ]; + + architect.createApp(fakeConfig, (err) => { + let expect = "Could not resolve dependencies\nMissing services: bar.plugin"; + assert.equal(err.message, expect); + assert.end(); + }); +}); + +test("it validates config (consumes must be present)", async (assert) => { + const fakeConfig = [ + { + packagePath: "foo/plugin", + setup: function(config, imports, register) {}, + provides: [], + } + ]; + + architect.createApp(fakeConfig, (err) => { + assert.ok(/Plugin is missing the consumes array/.test(err.message)); + assert.end(); + }); + +}); + +test("it validates config (provides must be present)", async (assert) => { + const fakeConfig = [ + { + packagePath: "foo/plugin", + setup: function(config, imports, register) {}, + } + ]; + + architect.createApp(fakeConfig, (err) => { + assert.ok(/Plugin is missing the provides array/.test(err.message)); + assert.end(); + }); + +}); + +test("it validates config (setup must be present)", async (assert) => { + const fakeConfig = [ + { + packagePath: "foo/plugin", + } + ]; + + architect.createApp(fakeConfig, (err) => { + assert.ok(/Plugin is missing the setup function/.test(err.message)); + assert.end(); + }); +}); \ No newline at end of file From 0a10c3f455599a1675c19b166be4baa0f16a8469 Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Mon, 29 Jan 2018 22:52:54 +0000 Subject: [PATCH 12/23] adds more tests --- architect_test.js | 348 ++++++++++++++++++++++++++++++---------------- 1 file changed, 230 insertions(+), 118 deletions(-) diff --git a/architect_test.js b/architect_test.js index d45bbe4..329f25b 100644 --- a/architect_test.js +++ b/architect_test.js @@ -10,16 +10,14 @@ const writeFile = promisify(fs.writeFile); const mkdirp = promisify(require("mkdirp")); test("resolve config resolved", assert => { - const config = [ - { - setup: function(){ - // noop - }, - provides: ["foo"], - consumes: ["foo"] - } - ]; - + const config = [{ + setup: function() { + // noop + }, + provides: ["foo"], + consumes: ["foo"] + }]; + architect.resolveConfig(config, "", (err, resolvedConfig) => { assert.ok(!err, "no error"); assert.deepEqual(resolvedConfig, config); @@ -27,7 +25,7 @@ test("resolve config resolved", assert => { }); }); -test("resolve config from basepath + node_modules", async (assert) => { +test("resolve config from basepath + node_modules", async(assert) => { const fakePlugin = ` module.exports = { setup: function(){ @@ -41,15 +39,15 @@ test("resolve config from basepath + node_modules", async (assert) => { let packagePath = "_fake/plugin_" + Date.now(); let packageDir = "/tmp/_architect_test_fixtures/node_modules"; let fullPath = packageDir + "/" + packagePath + ".js"; - + let config = [ packagePath, ]; - + await mkdirp(path.dirname(fullPath)); await writeFile(fullPath, fakePlugin.toString()); - - architect.resolveConfig(config, path.dirname(packageDir), async (err, resolvedConfig) => { + + architect.resolveConfig(config, path.dirname(packageDir), async(err, resolvedConfig) => { assert.ok(!err); assert.equal(resolvedConfig[0].packagePath, fullPath); @@ -61,7 +59,7 @@ test("resolve config from basepath + node_modules", async (assert) => { }); }); -test("resolve config from basepath + node_modules, async", async (assert) => { +test("resolve config from basepath + node_modules, async", async(assert) => { const fakePlugin = ` module.exports = { setup: function(){ @@ -79,10 +77,10 @@ test("resolve config from basepath + node_modules, async", async (assert) => { let config = [ packagePath, ]; - + await mkdirp(path.dirname(fullPath)); await writeFile(fullPath, fakePlugin.toString()); - + let resolvedConfig = await architect.resolveConfig(config, path.dirname(packageDir)); assert.equal(resolvedConfig[0].packagePath, fullPath); @@ -93,152 +91,266 @@ test("resolve config from basepath + node_modules, async", async (assert) => { assert.end(); }); -test("it should start an architect app (classic)", async (assert) => { - const fakeConfig = [ - { - packagePath: "foo/plugin", - setup: function(config, imports, register) { - register(null); - }, - provides: [], - consumes: ["bar.plugin"] - }, - { - packagePath: "bar/plugin", - setup: function(config, imports, register) { - register(null, { - "bar.plugin": { - iamBar: true - } - }); - }, - provides: ["bar.plugin"], - consumes: [] - } +test("it should start an architect app (classic)", async(assert) => { + const fakeConfig = [{ + packagePath: "foo/plugin", + setup: function(config, imports, register) { + register(null); + }, + provides: [], + consumes: ["bar.plugin"] + }, + { + packagePath: "bar/plugin", + setup: function(config, imports, register) { + register(null, { + "bar.plugin": { + iamBar: true + } + }); + }, + provides: ["bar.plugin"], + consumes: [] + } ]; architect.createApp(fakeConfig, (err) => { assert.ok(!err, "no err"); - assert.end(); + assert.end(); }); }); -test("it detects cyclic dependencies (classic)", async (assert) => { - const fakeConfig = [ - { - packagePath: "foo/plugin", - setup: function(config, imports, register) { - }, - provides: ["foo.plugin"], - consumes: ["bar.plugin"] - }, - { - packagePath: "bar/plugin", - setup: function(config, imports, register) { - }, - provides: ["bar.plugin"], - consumes: ["foo.plugin"] - } +test("it should provide imports", async(assert) => { + let iamBar = false; + + const fakeConfig = [{ + packagePath: "foo/plugin", + setup: function(config, imports, register) { + assert.ok(imports["bar.plugin"].iamBar); + iamBar = true; + register(); + }, + provides: [], + consumes: ["bar.plugin"] + }, + { + packagePath: "bar/plugin", + setup: function(config, imports, register) { + register(null, { + "bar.plugin": { + iamBar: true + } + }); + }, + provides: ["bar.plugin"], + consumes: [] + } + ]; + + architect.createApp(fakeConfig, (err) => { + assert.ok(!err, "no err"); + assert.ok(iamBar, "iamBar was imported"); + assert.end(); + }); +}); + +test("it should provide imports", async(assert) => { + let barDestroyed = false; + + const fakeConfig = [{ + packagePath: "foo/plugin", + setup: function(config, imports, register) { + assert.ok(imports["bar.plugin"].iamBar); + register(); + }, + provides: [], + consumes: ["bar.plugin"] + }, + { + packagePath: "bar/plugin", + setup: function(config, imports, register) { + register(null, { + onDestroy: function() { + barDestroyed = true; + }, + "bar.plugin": { + iamBar: true + } + }); + }, + provides: ["bar.plugin"], + consumes: [] + } + ]; + + let app = architect.createApp(fakeConfig, (err) => { + assert.ok(!err, "no err"); + + app.destroy(); + + assert.ok(barDestroyed, "barDestroyed"); + + assert.end(); + }); +}); + +test("it allow loading additionalPlugins", async(assert) => { + const fakeConfig = [{ + packagePath: "foo/plugin", + setup: function(config, imports, register) { + assert.ok(imports["bar.plugin"].iamBar); + register(); + }, + provides: [], + consumes: ["bar.plugin"] + }, + { + packagePath: "bar/plugin", + setup: function(config, imports, register) { + register(null, { + "bar.plugin": { + iamBar: true + } + }); + }, + provides: ["bar.plugin"], + consumes: [] + } + ]; + + let app = architect.createApp(fakeConfig, (err) => { + assert.ok(!err, "no err"); + + let loadedBar = false; + + const fakeAdditional = [ + { + packagePath: "biz/plugin", + setup: function(config, imports, register) { + assert.ok(imports["bar.plugin"].iamBar); + loadedBar = true; + register(); + }, + provides: [], + consumes: ["bar.plugin"] + } + ]; + + app.loadAdditionalPlugins(fakeAdditional, (err) => { + assert.ok(!err, "no err"); + assert.ok(loadedBar, "loadedBar"); + assert.end(); + }); + + + }); +}); + + +test("it detects cyclic dependencies (classic)", async(assert) => { + const fakeConfig = [{ + packagePath: "foo/plugin", + setup: function(config, imports, register) {}, + provides: ["foo.plugin"], + consumes: ["bar.plugin"] + }, + { + packagePath: "bar/plugin", + setup: function(config, imports, register) {}, + provides: ["bar.plugin"], + consumes: ["foo.plugin"] + } ]; architect.createApp(fakeConfig, (err) => { let expect = "Could not resolve dependencies\nConfig contains cyclic dependencies"; assert.equal(err.message, expect); - assert.end(); + assert.end(); }); }); -test("it checks the provides", async (assert) => { - const fakeConfig = [ - { - packagePath: "foo/plugin", - setup: function(config, imports, register) { - register(null); - }, - provides: [], - consumes: ["bar.plugin"] - }, - { - packagePath: "bar/plugin", - setup: function(config, imports, register) { - register(null, {}); - }, - provides: ["bar.plugin"], - consumes: [] - } +test("it checks the provides", async(assert) => { + const fakeConfig = [{ + packagePath: "foo/plugin", + setup: function(config, imports, register) { + register(null); + }, + provides: [], + consumes: ["bar.plugin"] + }, + { + packagePath: "bar/plugin", + setup: function(config, imports, register) { + register(null, {}); + }, + provides: ["bar.plugin"], + consumes: [] + } ]; architect.createApp(fakeConfig, (err) => { assert.ok(/Plugin failed to provide bar.plugin service/.test(err.message)); - assert.end(); + assert.end(); }); }); -test("it checks all dependencies", async (assert) => { - const fakeConfig = [ - { - packagePath: "foo/plugin", - setup: function(config, imports, register) { - }, - provides: ["foo.plugin"], - consumes: ["bar.plugin"] - }, - { - packagePath: "bar/plugin", - setup: function(config, imports, register) { - }, - provides: [], - consumes: [] - } +test("it checks all dependencies", async(assert) => { + const fakeConfig = [{ + packagePath: "foo/plugin", + setup: function(config, imports, register) {}, + provides: ["foo.plugin"], + consumes: ["bar.plugin"] + }, + { + packagePath: "bar/plugin", + setup: function(config, imports, register) {}, + provides: [], + consumes: [] + } ]; architect.createApp(fakeConfig, (err) => { let expect = "Could not resolve dependencies\nMissing services: bar.plugin"; assert.equal(err.message, expect); - assert.end(); + assert.end(); }); }); -test("it validates config (consumes must be present)", async (assert) => { - const fakeConfig = [ - { - packagePath: "foo/plugin", - setup: function(config, imports, register) {}, - provides: [], - } - ]; +test("it validates config (consumes must be present)", async(assert) => { + const fakeConfig = [{ + packagePath: "foo/plugin", + setup: function(config, imports, register) {}, + provides: [], + }]; architect.createApp(fakeConfig, (err) => { assert.ok(/Plugin is missing the consumes array/.test(err.message)); assert.end(); }); - + }); -test("it validates config (provides must be present)", async (assert) => { - const fakeConfig = [ - { - packagePath: "foo/plugin", - setup: function(config, imports, register) {}, - } - ]; +test("it validates config (provides must be present)", async(assert) => { + const fakeConfig = [{ + packagePath: "foo/plugin", + setup: function(config, imports, register) {}, + }]; architect.createApp(fakeConfig, (err) => { assert.ok(/Plugin is missing the provides array/.test(err.message)); assert.end(); }); - + }); -test("it validates config (setup must be present)", async (assert) => { - const fakeConfig = [ - { - packagePath: "foo/plugin", - } - ]; +test("it validates config (setup must be present)", async(assert) => { + const fakeConfig = [{ + packagePath: "foo/plugin", + }]; architect.createApp(fakeConfig, (err) => { assert.ok(/Plugin is missing the setup function/.test(err.message)); assert.end(); }); -}); \ No newline at end of file +}); + From ad9556aa8ad7c1cafb7b0870f800d2aadcabbe28 Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Mon, 29 Jan 2018 23:12:38 +0000 Subject: [PATCH 13/23] factor out destroy --- architect.js | 110 ++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 101 insertions(+), 9 deletions(-) diff --git a/architect.js b/architect.js index d02ef7b..ffb0bb9 100644 --- a/architect.js +++ b/architect.js @@ -50,6 +50,9 @@ async function resolveConfig(config, base, callback) { config = config.map(normalize); + if (typeof base == "function") + return resolveConfig(config, null, base); + if (callback) return resolveConfigAsync(config, base, callback); @@ -237,7 +240,104 @@ return sorted; } + function startPlugins(additional, isAdditionalMode) { + let app = this; + + let services = {}; + let sortedPlugins = this.sortedPlugins; + + let recur = 0, callnext, ready; + + var plugin = sortedPlugins.shift(); + if (!plugin) { + ready = true; + return app.emit(additional ? "ready-additional" : "ready", app); + } + + var imports = {}; + if (plugin.consumes) { + plugin.consumes.forEach(function(name) { + imports[name] = services[name]; + }); + } + + var m = /^plugins\/([^\/]+)|\/plugins\/[^\/]+\/([^\/]+)/.exec(plugin.packagePath); + var packageName = m && (m[1] || m[2]); + if (!app.packages[packageName]) app.packages[packageName] = []; + + try { + recur++; + plugin.setup(plugin, imports, register); + } + catch (e) { + e.plugin = plugin; + app.emit("error", e); + throw e; + } + finally { + while (callnext && recur <= 1) { + callnext = false; + startPlugins.call(app, additional); + } + recur--; + } + + function register(err, provided) { + if (err) { return app.emit("error", err); } + plugin.provides.forEach(function(name) { + if (!provided.hasOwnProperty(name)) { + var err = new Error("Plugin failed to provide " + name + " service. " + JSON.stringify(plugin)); + err.plugin = plugin; + return app.emit("error", err); + } + services[name] = provided[name]; + app.pluginToPackage[name] = { + path: plugin.packagePath, + package: packageName, + version: plugin.version, + isAdditionalMode: isAdditionalMode + }; + app.packages[packageName].push(name); + + app.emit("service", name, services[name], plugin); + }); + + if (provided && provided.hasOwnProperty("onDestroy")) + app.addDestructor(provided.onDestroy); + + app.emit("plugin", plugin); + + if (recur) return (callnext = true); + startPlugins.call(app, additional); + } + } + + + class Architect extends EventEmitter { + get destructors() { + if (!this._destructors) + this._destructors = []; + + return this._destructors; + } + + set destructors(val) { + this._destructors = []; + } + + addDestructor(fn) { + this.destructors.push(fn); + } + + destroy() { + this.destructors.forEach(function(destroy) { + destroy(); + }); + + this._destructors = []; + } + constructor(config) { super(); var app = this; @@ -257,7 +357,6 @@ // Check the config var sortedPlugins = checkConfig(config); - var destructors = []; var recur = 0, callnext, ready; @@ -328,7 +427,7 @@ app.emit("service", name, services[name], plugin); }); if (provided && provided.hasOwnProperty("onDestroy")) - destructors.push(provided.onDestroy); + app.addDestructor(provided.onDestroy); app.emit("plugin", plugin); @@ -368,13 +467,6 @@ }); }; - this.destroy = function() { - destructors.forEach(function(destroy) { - destroy(); - }); - - destructors = []; - }; } } From aa15be4c7114e3d8d2d7f1ca3a1fb0afbbbb6ed2 Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Mon, 29 Jan 2018 23:14:18 +0000 Subject: [PATCH 14/23] remove DEBUG --- architect.js | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/architect.js b/architect.js index ffb0bb9..471143c 100644 --- a/architect.js +++ b/architect.js @@ -378,34 +378,22 @@ var packageName = m && (m[1] || m[2]); if (!app.packages[packageName]) app.packages[packageName] = []; - if (DEBUG) { + try { recur++; plugin.setup(plugin, imports, register); - + } + catch (e) { + e.plugin = plugin; + app.emit("error", e); + throw e; + } + finally { while (callnext && recur <= 1) { callnext = false; startPlugins(additional); } recur--; } - else { - try { - recur++; - plugin.setup(plugin, imports, register); - } - catch (e) { - e.plugin = plugin; - app.emit("error", e); - throw e; - } - finally { - while (callnext && recur <= 1) { - callnext = false; - startPlugins(additional); - } - recur--; - } - } function register(err, provided) { if (err) { return app.emit("error", err); } From e9ee7dedcd0677eb4c8d2a4369fb15825b0c1562 Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Mon, 29 Jan 2018 23:46:46 +0000 Subject: [PATCH 15/23] factor out additional plugins; simplify loader (removing error handling for now) --- architect.js | 189 ++++++++++++--------------------------------------- 1 file changed, 43 insertions(+), 146 deletions(-) diff --git a/architect.js b/architect.js index 471143c..055d2f7 100644 --- a/architect.js +++ b/architect.js @@ -240,79 +240,6 @@ return sorted; } - function startPlugins(additional, isAdditionalMode) { - let app = this; - - let services = {}; - let sortedPlugins = this.sortedPlugins; - - let recur = 0, callnext, ready; - - var plugin = sortedPlugins.shift(); - if (!plugin) { - ready = true; - return app.emit(additional ? "ready-additional" : "ready", app); - } - - var imports = {}; - if (plugin.consumes) { - plugin.consumes.forEach(function(name) { - imports[name] = services[name]; - }); - } - - var m = /^plugins\/([^\/]+)|\/plugins\/[^\/]+\/([^\/]+)/.exec(plugin.packagePath); - var packageName = m && (m[1] || m[2]); - if (!app.packages[packageName]) app.packages[packageName] = []; - - try { - recur++; - plugin.setup(plugin, imports, register); - } - catch (e) { - e.plugin = plugin; - app.emit("error", e); - throw e; - } - finally { - while (callnext && recur <= 1) { - callnext = false; - startPlugins.call(app, additional); - } - recur--; - } - - function register(err, provided) { - if (err) { return app.emit("error", err); } - plugin.provides.forEach(function(name) { - if (!provided.hasOwnProperty(name)) { - var err = new Error("Plugin failed to provide " + name + " service. " + JSON.stringify(plugin)); - err.plugin = plugin; - return app.emit("error", err); - } - services[name] = provided[name]; - app.pluginToPackage[name] = { - path: plugin.packagePath, - package: packageName, - version: plugin.version, - isAdditionalMode: isAdditionalMode - }; - app.packages[packageName].push(name); - - app.emit("service", name, services[name], plugin); - }); - - if (provided && provided.hasOwnProperty("onDestroy")) - app.addDestructor(provided.onDestroy); - - app.emit("plugin", plugin); - - if (recur) return (callnext = true); - startPlugins.call(app, additional); - } - } - - class Architect extends EventEmitter { get destructors() { @@ -338,14 +265,43 @@ this._destructors = []; } + loadAdditionalPlugins(additionalConfig, callback) { + // isAdditionalMode = true; + + let app = this; + let ready = this.ready; + + exports.resolveConfig(additionalConfig, function(err, additionalConfig) { + if (err) return callback(err); + + app.once(ready ? "ready-additional" : "ready", function(app) { + callback(null, app); + }); // What about error state? + + // Check the config - hopefully this works + var _sortedPlugins = checkConfig(additionalConfig, function(name) { + return app.services[name]; + }); + + if (ready) { + var sortedPlugins = _sortedPlugins; + // Start Loading additional plugins + app.startPlugins(sortedPlugins, true); + } + else { + _sortedPlugins.forEach(function(item) { + sortedPlugins.push(item); + }); + } + }); + } + + constructor(config) { super(); var app = this; app.config = config; - app.packages = {}; - app.pluginToPackage = {}; - var isAdditionalMode; var services = app.services = { hub: { on: function(name, callback) { @@ -357,46 +313,25 @@ // Check the config var sortedPlugins = checkConfig(config); - var recur = 0, - callnext, ready; - - function startPlugins(additional) { + function startPlugins(sortedPlugins, additional) { var plugin = sortedPlugins.shift(); + if (!plugin) { - ready = true; + app.ready = true; return app.emit(additional ? "ready-additional" : "ready", app); } var imports = {}; - if (plugin.consumes) { - plugin.consumes.forEach(function(name) { - imports[name] = services[name]; - }); - } + plugin.consumes.forEach(function(name) { + imports[name] = services[name]; + }); - var m = /^plugins\/([^\/]+)|\/plugins\/[^\/]+\/([^\/]+)/.exec(plugin.packagePath); - var packageName = m && (m[1] || m[2]); - if (!app.packages[packageName]) app.packages[packageName] = []; - - try { - recur++; - plugin.setup(plugin, imports, register); - } - catch (e) { - e.plugin = plugin; - app.emit("error", e); - throw e; - } - finally { - while (callnext && recur <= 1) { - callnext = false; - startPlugins(additional); - } - recur--; - } + plugin.setup(plugin, imports, register); + startPlugins(sortedPlugins, additional); function register(err, provided) { if (err) { return app.emit("error", err); } + plugin.provides.forEach(function(name) { if (!provided.hasOwnProperty(name)) { var err = new Error("Plugin failed to provide " + name + " service. " + JSON.stringify(plugin)); @@ -404,57 +339,19 @@ return app.emit("error", err); } services[name] = provided[name]; - app.pluginToPackage[name] = { - path: plugin.packagePath, - package: packageName, - version: plugin.version, - isAdditionalMode: isAdditionalMode - }; - app.packages[packageName].push(name); - app.emit("service", name, services[name], plugin); }); if (provided && provided.hasOwnProperty("onDestroy")) app.addDestructor(provided.onDestroy); app.emit("plugin", plugin); - - if (recur) return (callnext = true); - startPlugins(additional); } } // Give createApp some time to subscribe to our "ready" event - (typeof process === "object" ? process.nextTick : setTimeout)(startPlugins); - - this.loadAdditionalPlugins = function(additionalConfig, callback) { - isAdditionalMode = true; - - exports.resolveConfig(additionalConfig, function(err, additionalConfig) { - if (err) return callback(err); - - app.once(ready ? "ready-additional" : "ready", function(app) { - callback(null, app); - }); // What about error state? - - // Check the config - hopefully this works - var _sortedPlugins = checkConfig(additionalConfig, function(name) { - return services[name]; - }); - - if (ready) { - sortedPlugins = _sortedPlugins; - // Start Loading additional plugins - startPlugins(true); - } - else { - _sortedPlugins.forEach(function(item) { - sortedPlugins.push(item); - }); - } - }); - }; + (typeof process === "object" ? process.nextTick : setTimeout)(startPlugins.bind(null, sortedPlugins)); + this.startPlugins = startPlugins; } } From 861d72c89888783a3b52d37ffeef34b475b66ff6 Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Mon, 29 Jan 2018 23:56:19 +0000 Subject: [PATCH 16/23] factor services out --- architect.js | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/architect.js b/architect.js index 055d2f7..d085bff 100644 --- a/architect.js +++ b/architect.js @@ -296,19 +296,26 @@ }); } + get services() { + if (!this._services) { + this._services = { + hub: { + on: function(name, callback) { + this.on(name, callback); + } + } + }; + } + + return this._services; + } + constructor(config) { super(); var app = this; app.config = config; - var services = app.services = { - hub: { - on: function(name, callback) { - app.on(name, callback); - } - } - }; // Check the config var sortedPlugins = checkConfig(config); @@ -323,7 +330,7 @@ var imports = {}; plugin.consumes.forEach(function(name) { - imports[name] = services[name]; + imports[name] = app.services[name]; }); plugin.setup(plugin, imports, register); @@ -338,8 +345,8 @@ err.plugin = plugin; return app.emit("error", err); } - services[name] = provided[name]; - app.emit("service", name, services[name], plugin); + app.services[name] = provided[name]; + app.emit("service", name, app.services[name], plugin); }); if (provided && provided.hasOwnProperty("onDestroy")) app.addDestructor(provided.onDestroy); From 29e633c9e6d26120f81791cc42ecffc2405e2295 Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Tue, 30 Jan 2018 00:03:30 +0000 Subject: [PATCH 17/23] factor out addService, move calling next into callback (async) --- architect.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/architect.js b/architect.js index d085bff..38085e0 100644 --- a/architect.js +++ b/architect.js @@ -310,6 +310,12 @@ return this._services; } + addService(name, service, plugin) { + this.services[name] = service; + this.emit("service", name, service, plugin); + + } + constructor(config) { super(); @@ -334,7 +340,6 @@ }); plugin.setup(plugin, imports, register); - startPlugins(sortedPlugins, additional); function register(err, provided) { if (err) { return app.emit("error", err); } @@ -345,13 +350,15 @@ err.plugin = plugin; return app.emit("error", err); } - app.services[name] = provided[name]; - app.emit("service", name, app.services[name], plugin); + + app.addService(name, provided[name], plugin); }); + if (provided && provided.hasOwnProperty("onDestroy")) app.addDestructor(provided.onDestroy); app.emit("plugin", plugin); + startPlugins(sortedPlugins, additional); } } From 710d3b460b4f2f0ddba96c7b4ac9e8b75eae0b12 Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Tue, 30 Jan 2018 00:06:25 +0000 Subject: [PATCH 18/23] factor out startPlugin --- architect.js | 59 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/architect.js b/architect.js index 38085e0..a555bab 100644 --- a/architect.js +++ b/architect.js @@ -316,6 +316,38 @@ } + startPlugin(plugin, next) { + var imports = {}; + + var app = this; + + plugin.consumes.forEach(function(name) { + imports[name] = app.services[name]; + }); + + plugin.setup(plugin, imports, register); + + function register(err, provided) { + if (err) { return app.emit("error", err); } + + plugin.provides.forEach(function(name) { + if (!provided.hasOwnProperty(name)) { + var err = new Error("Plugin failed to provide " + name + " service. " + JSON.stringify(plugin)); + err.plugin = plugin; + return app.emit("error", err); + } + + app.addService(name, provided[name], plugin); + }); + + if (provided && provided.hasOwnProperty("onDestroy")) + app.addDestructor(provided.onDestroy); + + app.emit("plugin", plugin); + next(); + } + } + constructor(config) { super(); @@ -334,32 +366,9 @@ return app.emit(additional ? "ready-additional" : "ready", app); } - var imports = {}; - plugin.consumes.forEach(function(name) { - imports[name] = app.services[name]; - }); - - plugin.setup(plugin, imports, register); - - function register(err, provided) { - if (err) { return app.emit("error", err); } - - plugin.provides.forEach(function(name) { - if (!provided.hasOwnProperty(name)) { - var err = new Error("Plugin failed to provide " + name + " service. " + JSON.stringify(plugin)); - err.plugin = plugin; - return app.emit("error", err); - } - - app.addService(name, provided[name], plugin); - }); - - if (provided && provided.hasOwnProperty("onDestroy")) - app.addDestructor(provided.onDestroy); - - app.emit("plugin", plugin); + app.startPlugin(plugin, () => { startPlugins(sortedPlugins, additional); - } + }); } // Give createApp some time to subscribe to our "ready" event From f18433b8470d18c8ff94c5c46d3eb0eb10365ec4 Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Tue, 30 Jan 2018 00:12:15 +0000 Subject: [PATCH 19/23] simplify --- architect.js | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/architect.js b/architect.js index a555bab..0f08521 100644 --- a/architect.js +++ b/architect.js @@ -319,33 +319,29 @@ startPlugin(plugin, next) { var imports = {}; - var app = this; - - plugin.consumes.forEach(function(name) { - imports[name] = app.services[name]; + plugin.consumes.forEach((name) => { + imports[name] = this.services[name]; }); - plugin.setup(plugin, imports, register); - - function register(err, provided) { - if (err) { return app.emit("error", err); } + plugin.setup(plugin, imports, (err, provided) => { + if (err) { return this.emit("error", err); } - plugin.provides.forEach(function(name) { + plugin.provides.forEach((name) => { if (!provided.hasOwnProperty(name)) { var err = new Error("Plugin failed to provide " + name + " service. " + JSON.stringify(plugin)); err.plugin = plugin; - return app.emit("error", err); + return this.emit("error", err); } - app.addService(name, provided[name], plugin); + this.addService(name, provided[name], plugin); }); if (provided && provided.hasOwnProperty("onDestroy")) - app.addDestructor(provided.onDestroy); + this.addDestructor(provided.onDestroy); - app.emit("plugin", plugin); + this.emit("plugin", plugin); next(); - } + }); } From 96d6b713a13d6f9d7ecc72571705fb132a9e8c3c Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Tue, 30 Jan 2018 16:03:45 +0000 Subject: [PATCH 20/23] cleanup --- architect.js | 94 ++++++++++++++++++++--------------------------- architect_test.js | 5 +-- 2 files changed, 41 insertions(+), 58 deletions(-) diff --git a/architect.js b/architect.js index 0f08521..1bbbde0 100644 --- a/architect.js +++ b/architect.js @@ -265,35 +265,31 @@ this._destructors = []; } - loadAdditionalPlugins(additionalConfig, callback) { - // isAdditionalMode = true; + getService(name) { + if (!this.services[name]) + throw new Error("Service '" + name + "' not found in architect app!"); + return this.services[name]; + } + async loadAdditionalPlugins(additionalConfig, callback) { let app = this; - let ready = this.ready; - exports.resolveConfig(additionalConfig, function(err, additionalConfig) { - if (err) return callback(err); + app.on(this.ready ? "ready-additional" : "ready", function(app) { + callback(null, app); + }); - app.once(ready ? "ready-additional" : "ready", function(app) { - callback(null, app); - }); // What about error state? + const sortedPlugins = checkConfig(additionalConfig, function(name) { + return app.services[name]; + }); - // Check the config - hopefully this works - var _sortedPlugins = checkConfig(additionalConfig, function(name) { - return app.services[name]; - }); + await exports.resolveConfig(additionalConfig); - if (ready) { - var sortedPlugins = _sortedPlugins; - // Start Loading additional plugins - app.startPlugins(sortedPlugins, true); - } - else { - _sortedPlugins.forEach(function(item) { - sortedPlugins.push(item); - }); - } - }); + this.sortedPlugins = this.sortedPlugins.concat(sortedPlugins); + + if (this.ready) + return app.startPlugins(); + + callback(); } get services() { @@ -344,52 +340,40 @@ }); } + startPlugins() { + var plugin = this.sortedPlugins.shift(); - constructor(config) { - super(); - var app = this; - app.config = config; + if (!plugin) { + let ready = this.ready; + this.ready = true; + this.emit(ready ? "ready-additional" : "ready", this); + return; + } - // Check the config - var sortedPlugins = checkConfig(config); + this.startPlugin(plugin, () => { + this.startPlugins(); + }); + } - function startPlugins(sortedPlugins, additional) { - var plugin = sortedPlugins.shift(); - if (!plugin) { - app.ready = true; - return app.emit(additional ? "ready-additional" : "ready", app); - } + constructor(config) { + super(); - app.startPlugin(plugin, () => { - startPlugins(sortedPlugins, additional); - }); - } + this.config = config; + this.sortedPlugins = checkConfig(config); - // Give createApp some time to subscribe to our "ready" event - (typeof process === "object" ? process.nextTick : setTimeout)(startPlugins.bind(null, sortedPlugins)); + let start = () => this.startPlugins(); - this.startPlugins = startPlugins; + // Give createApp some time to subscribe to our "ready" event + (typeof process === "object" ? process.nextTick : setTimeout)(start); } + } exports.createApp = createApp; exports.Architect = Architect; - // Architect.prototype = Object.create(EventEmitter.prototype, {constructor:{value:Architect}}); - - // Architect.prototype.getService = function(name) { - // if (!this.services[name]) { - // throw new Error("Service '" + name + "' not found in architect app!"); - // } - // return this.services[name]; - - // } - // } - - // function Architect(config) { - // }; // Returns an event emitter that represents the app. It can emit events. // event: ("service" name, service) emitted when a service is ready to be consumed. diff --git a/architect_test.js b/architect_test.js index 329f25b..2246a54 100644 --- a/architect_test.js +++ b/architect_test.js @@ -220,7 +220,9 @@ test("it allow loading additionalPlugins", async(assert) => { let app = architect.createApp(fakeConfig, (err) => { assert.ok(!err, "no err"); + }); + app.on("ready", () => { let loadedBar = false; const fakeAdditional = [ @@ -241,12 +243,9 @@ test("it allow loading additionalPlugins", async(assert) => { assert.ok(loadedBar, "loadedBar"); assert.end(); }); - - }); }); - test("it detects cyclic dependencies (classic)", async(assert) => { const fakeConfig = [{ packagePath: "foo/plugin", From 15eec867638828fdb4b0b8501180222def821a58 Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Tue, 30 Jan 2018 16:54:51 +0000 Subject: [PATCH 21/23] split constructor and start plugins --- architect.js | 61 +++++++++++++++++++---------------------------- architect_test.js | 3 ++- 2 files changed, 26 insertions(+), 38 deletions(-) diff --git a/architect.js b/architect.js index 1bbbde0..aab351e 100644 --- a/architect.js +++ b/architect.js @@ -272,22 +272,18 @@ } async loadAdditionalPlugins(additionalConfig, callback) { - let app = this; - - app.on(this.ready ? "ready-additional" : "ready", function(app) { - callback(null, app); + this.on(this.ready ? "ready-additional" : "ready", () => { + callback(null, this); }); - const sortedPlugins = checkConfig(additionalConfig, function(name) { - return app.services[name]; - }); + const sortedPlugins = checkConfig(additionalConfig, (name) => this.services[name]); await exports.resolveConfig(additionalConfig); this.sortedPlugins = this.sortedPlugins.concat(sortedPlugins); if (this.ready) - return app.startPlugins(); + return this.startPlugins(); callback(); } @@ -359,14 +355,12 @@ constructor(config) { super(); - this.config = config; - this.sortedPlugins = checkConfig(config); - - let start = () => this.startPlugins(); + } - // Give createApp some time to subscribe to our "ready" event - (typeof process === "object" ? process.nextTick : setTimeout)(start); + start() { + this.sortedPlugins = checkConfig(this.config); + this.startPlugins(); } } @@ -375,6 +369,11 @@ exports.Architect = Architect; + function delay(fn) { + (typeof process === "object" ? process.nextTick : setTimeout)(fn); + } + + // Returns an event emitter that represents the app. It can emit events. // event: ("service" name, service) emitted when a service is ready to be consumed. // event: ("plugin", plugin) emitted when a plugin registers. @@ -383,32 +382,20 @@ // app.services - a hash of all the services in this app // app.config - the plugin config that was passed in. function createApp(config, callback) { - var app; - try { - app = new Architect(config); - } - catch (err) { - if (!callback) throw err; - return callback(err, app); - } - if (callback) { - app.on("error", done); - app.on("ready", onReady); - } - return app; + var app = new Architect(config); - function onReady(app) { - done(); - } + app.once("ready", () => callback(null, app)); + // app.once("error", (err) => callback(err)); - function done(err) { - if (err) { - app.destroy(); + delay(() => { + try { + app.start(); } - app.removeListener("error", done); - app.removeListener("ready", onReady); - callback(err, app); - } + catch(err) { + if (callback) return callback(err); + throw err; + } + }); return app; } diff --git a/architect_test.js b/architect_test.js index 2246a54..ab9fdd0 100644 --- a/architect_test.js +++ b/architect_test.js @@ -183,7 +183,7 @@ test("it should provide imports", async(assert) => { } ]; - let app = architect.createApp(fakeConfig, (err) => { + architect.createApp(fakeConfig, (err, app) => { assert.ok(!err, "no err"); app.destroy(); @@ -194,6 +194,7 @@ test("it should provide imports", async(assert) => { }); }); + test("it allow loading additionalPlugins", async(assert) => { const fakeConfig = [{ packagePath: "foo/plugin", From db233b8844c013d6b182b14782d4104bcf76b550 Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Tue, 30 Jan 2018 23:13:56 +0000 Subject: [PATCH 22/23] cleanup public interface, adds custom error --- architect.js | 166 ++++++++++++++++++++++++++------------------------- 1 file changed, 86 insertions(+), 80 deletions(-) diff --git a/architect.js b/architect.js index aab351e..4e97143 100644 --- a/architect.js +++ b/architect.js @@ -138,6 +138,14 @@ } }()); + class ArchitectError extends Error { + constructor(message, plugin = {}) { + super(); + this.message = `${message} ${JSON.stringify(plugin)}`; + } + } + + // Check a plugin config list for bad dependencies and throw on error function checkConfig(config, lookup) { @@ -146,13 +154,13 @@ config.forEach(function(plugin) { if (plugin.checked) { return; } if (!plugin.hasOwnProperty("setup")) { - throw new Error("Plugin is missing the setup function " + JSON.stringify(plugin)); + throw new ArchitectError("Plugin is missing the setup function", plugin); } if (!plugin.hasOwnProperty("provides")) { - throw new Error("Plugin is missing the provides array " + JSON.stringify(plugin)); + throw new ArchitectError("Plugin is missing the provides array ", plugin); } if (!plugin.hasOwnProperty("consumes")) { - throw new Error("Plugin is missing the consumes array " + JSON.stringify(plugin)); + throw new ArchitectError("Plugin is missing the consumes array ", plugin); } }); @@ -240,8 +248,28 @@ return sorted; } + function asyncPlugin(plugin, imports) { + return new Promise((resolve, reject) => { + plugin.setup(plugin, imports, (err, provided) => { + if (err) return reject(err); + resolve(provided); + }); + }); + } + + function setupPlugin(plugin, imports) { + if (plugin.setup.length > 2) + return asyncPlugin(plugin, imports); + + return plugin.setup(plugin, imports); + } class Architect extends EventEmitter { + constructor(config) { + super(); + this.config = config; + } + get destructors() { if (!this._destructors) this._destructors = []; @@ -249,16 +277,26 @@ return this._destructors; } - set destructors(val) { - this._destructors = []; + get services() { + if (!this._services) { + let hub = { + on: this.on.bind(this) + }; + + this._services = { hub }; + } + + return this._services; } - addDestructor(fn) { - this.destructors.push(fn); + addDestructor(provided) { + if (!provided) return; + if (!provided.hasOwnProperty("onDestroy")) return; + this.destructors.push(provided.onDestroy); } destroy() { - this.destructors.forEach(function(destroy) { + this.destructors.forEach((destroy) => { destroy(); }); @@ -272,95 +310,66 @@ } async loadAdditionalPlugins(additionalConfig, callback) { - this.on(this.ready ? "ready-additional" : "ready", () => { + this.once(this._isReady ? "ready-additional" : "ready", () => { callback(null, this); }); const sortedPlugins = checkConfig(additionalConfig, (name) => this.services[name]); - await exports.resolveConfig(additionalConfig); this.sortedPlugins = this.sortedPlugins.concat(sortedPlugins); - if (this.ready) - return this.startPlugins(); - - callback(); - } - - get services() { - if (!this._services) { - this._services = { - hub: { - on: function(name, callback) { - this.on(name, callback); - } - } - }; + if (this._isReady) { + for (let plugin of this.sortedPlugins) { + await this.startPlugin(plugin); + } } - - return this._services; + else callback(); + this._emitReady(); } - addService(name, service, plugin) { - this.services[name] = service; - this.emit("service", name, service, plugin); - - } - - startPlugin(plugin, next) { + async startPlugin(plugin) { var imports = {}; plugin.consumes.forEach((name) => { imports[name] = this.services[name]; }); - plugin.setup(plugin, imports, (err, provided) => { - if (err) { return this.emit("error", err); } - plugin.provides.forEach((name) => { - if (!provided.hasOwnProperty(name)) { - var err = new Error("Plugin failed to provide " + name + " service. " + JSON.stringify(plugin)); - err.plugin = plugin; - return this.emit("error", err); - } + let provided = await setupPlugin(plugin, imports); - this.addService(name, provided[name], plugin); - }); - - if (provided && provided.hasOwnProperty("onDestroy")) - this.addDestructor(provided.onDestroy); - - this.emit("plugin", plugin); - next(); - }); - } - - startPlugins() { - var plugin = this.sortedPlugins.shift(); + for (let name of plugin.provides) { + if (provided.hasOwnProperty(name)) + continue; - if (!plugin) { - let ready = this.ready; + throw new ArchitectError("Plugin failed to provide " + name + " service. ", plugin); + } - this.ready = true; - this.emit(ready ? "ready-additional" : "ready", this); - return; + for (let name of plugin.provides) { + let service = provided[name]; + this.services[name] = service; + this.emit("service", name, service, plugin); } - this.startPlugin(plugin, () => { - this.startPlugins(); - }); + this.addDestructor(provided); } + async start() { + const sortedPlugins = await checkConfig(this.config); - constructor(config) { - super(); - this.config = config; + for (let plugin of sortedPlugins) { + await this.startPlugin(plugin); + } + + this.sortedPlugins = sortedPlugins; + this._emitReady(); + return this; } - start() { - this.sortedPlugins = checkConfig(this.config); - this.startPlugins(); + _emitReady() { + let ready = this._isReady; + this._isReady = true; + this.emit(ready ? "ready-additional" : "ready", this); } } @@ -368,7 +377,6 @@ exports.createApp = createApp; exports.Architect = Architect; - function delay(fn) { (typeof process === "object" ? process.nextTick : setTimeout)(fn); } @@ -384,17 +392,15 @@ function createApp(config, callback) { var app = new Architect(config); - app.once("ready", () => callback(null, app)); - // app.once("error", (err) => callback(err)); - + // delayed execution allows + // the caller to consume the return value + // and attach eventlisteners delay(() => { - try { - app.start(); - } - catch(err) { - if (callback) return callback(err); - throw err; - } + app.start() + .then((app) => { + callback(null, app); + }) + .catch(callback); }); return app; From ca1c8f78b85d05654edd9ce250ebbde6029ef205 Mon Sep 17 00:00:00 2001 From: Matthijs van Henten Date: Tue, 30 Jan 2018 23:25:17 +0000 Subject: [PATCH 23/23] adds tests for async api --- architect.js | 4 + architect_test.js | 206 +++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 198 insertions(+), 12 deletions(-) diff --git a/architect.js b/architect.js index 4e97143..ca23a77 100644 --- a/architect.js +++ b/architect.js @@ -289,6 +289,10 @@ return this._services; } + get ready() { + return this._isReady; + } + addDestructor(provided) { if (!provided) return; if (!provided.hasOwnProperty("onDestroy")) return; diff --git a/architect_test.js b/architect_test.js index ab9fdd0..5acf492 100644 --- a/architect_test.js +++ b/architect_test.js @@ -226,18 +226,16 @@ test("it allow loading additionalPlugins", async(assert) => { app.on("ready", () => { let loadedBar = false; - const fakeAdditional = [ - { - packagePath: "biz/plugin", - setup: function(config, imports, register) { - assert.ok(imports["bar.plugin"].iamBar); - loadedBar = true; - register(); - }, - provides: [], - consumes: ["bar.plugin"] - } - ]; + const fakeAdditional = [{ + packagePath: "biz/plugin", + setup: function(config, imports, register) { + assert.ok(imports["bar.plugin"].iamBar); + loadedBar = true; + register(); + }, + provides: [], + consumes: ["bar.plugin"] + }]; app.loadAdditionalPlugins(fakeAdditional, (err) => { assert.ok(!err, "no err"); @@ -354,3 +352,187 @@ test("it validates config (setup must be present)", async(assert) => { }); }); +test("it should start an architect app when plugin _returns_ value", async(assert) => { + const fakeConfig = [{ + packagePath: "foo/plugin", + setup: function(config, imports, register) { + register(null); + }, + provides: [], + consumes: ["bar.plugin"] + }, + { + packagePath: "bar/plugin", + setup: function(config, imports) { + return { + "bar.plugin": { + isBar: true + } + }; + }, + provides: ["bar.plugin"], + consumes: [] + } + ]; + + architect.createApp(fakeConfig, (err) => { + assert.ok(!err, "no err"); + assert.end(); + }); +}); + +test("it should start an architect app when plugin awaits", async(assert) => { + const fakeConfig = [{ + packagePath: "foo/plugin", + setup: function(config, imports, register) { + register(null); + }, + provides: [], + consumes: ["bar.plugin"] + }, + { + packagePath: "bar/plugin", + setup: async(config, imports) => { + let delay = new Promise(resolve => { + setTimeout(resolve, 100); + }); + + await delay; + + return { + "bar.plugin": { + isBar: true + } + }; + }, + provides: ["bar.plugin"], + consumes: [] + } + ]; + + architect.createApp(fakeConfig, (err) => { + assert.ok(!err, "no err"); + assert.end(); + }); +}); + +test("it should start an architect app when plugin returns promise", async(assert) => { + const fakeConfig = [{ + packagePath: "foo/plugin", + setup: function(config, imports, register) { + register(null); + }, + provides: [], + consumes: ["bar.plugin"] + }, + { + packagePath: "bar/plugin", + setup: async(config, imports) => { + return new Promise(resolve => { + resolve({ + "bar.plugin": { + isBar: true + } + }); + }); + }, + provides: ["bar.plugin"], + consumes: [] + } + ]; + + architect.createApp(fakeConfig, (err) => { + assert.ok(!err, "no err"); + assert.end(); + }); +}); + +test("it should start an architect app when plugin rejects promise", async(assert) => { + const fakeConfig = [{ + packagePath: "foo/plugin", + setup: function(config, imports, register) { + register(null); + }, + provides: [], + consumes: ["bar.plugin"] + }, + { + packagePath: "bar/plugin", + setup: async(config, imports) => { + return new Promise((resolve, reject) => { + reject("Foo error!"); + }); + }, + provides: ["bar.plugin"], + consumes: [] + } + ]; + + architect.createApp(fakeConfig, (err) => { + assert.equal(err, "Foo error!"); + assert.end(); + }); +}); + +test("it should start an architect app when plugin has an error", async(assert) => { + const fakeConfig = [{ + packagePath: "foo/plugin", + setup: function(config, imports, register) { + register(null); + }, + provides: [], + consumes: ["bar.plugin"] + }, + { + packagePath: "bar/plugin", + setup: async(config, imports) => { + let boink = 1; + boink(); + }, + provides: ["bar.plugin"], + consumes: [] + } + ]; + + architect.createApp(fakeConfig, (err) => { + assert.equal(err.message, "boink is not a function"); + assert.end(); + }); +}); + +test("it should start an architect app with await", async(assert) => { + const fakeConfig = [{ + packagePath: "foo/plugin", + setup: function(config, imports, register) { + register(null); + }, + provides: [], + consumes: ["bar.plugin"] + }, + { + packagePath: "bar/plugin", + setup: async(config, imports) => { + return { + "bar.plugin": { + isBar: true + } + }; + }, + provides: ["bar.plugin"], + consumes: [] + } + ]; + + const app = new architect.Architect(fakeConfig); + + app.on("ready", () => assert.end()); + + await app.start(); + + assert.ok(app.ready); + + let service = app.getService("bar.plugin"); + + assert.deepEqual(service, { isBar: true }); + +});