From 0eb815a037b34b946b6e6136cadbc28e6c3209fc Mon Sep 17 00:00:00 2001 From: Joe Catera Date: Tue, 19 Mar 2013 14:51:51 -0700 Subject: [PATCH 01/38] Adding the option --reuseSession to the run.js command. --- docs/dev_guide/topics/mojito_testing.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/dev_guide/topics/mojito_testing.rst b/docs/dev_guide/topics/mojito_testing.rst index 6e7483a3e..bb16ec837 100644 --- a/docs/dev_guide/topics/mojito_testing.rst +++ b/docs/dev_guide/topics/mojito_testing.rst @@ -665,7 +665,7 @@ or unit tests with one command. ``$ java -jar path/to/selenium-server.jar &`` #. Run the unit tests for the framework and client: - ``$ ./run.js test -u --path unit --group fw,client,server`` + ``$ ./run.js test -u --path unit --group fw,client,server --reuseSession`` #. You can also run all the functional tests with the below command. ``$ ./run.js test -f --path func --port 4000`` @@ -676,7 +676,7 @@ or unit tests with one command. #. To run individual unit and functional tests, you pass the test descriptor to ``run.js``. - ``$ ./run.js test -f --path func --descriptor examples/newsboxes/newsboxes_descriptor.json --port 4000`` + ``$ ./run.js test -f --path func --descriptor examples/newsboxes/newsboxes_descriptor.json --port 4000 --reuseSession`` The command above runs the functional test for the ``newsboxes`` application. The ``--path`` option indicates that the From 113621a09b03f06fc4759a22d962c03f78813338 Mon Sep 17 00:00:00 2001 From: Isao Yagi Date: Wed, 20 Mar 2013 15:26:38 -0700 Subject: [PATCH 02/38] fix bz6160815: port argument must be an integer --- lib/app/commands/start.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/app/commands/start.js b/lib/app/commands/start.js index 29d187462..826d7e07c 100644 --- a/lib/app/commands/start.js +++ b/lib/app/commands/start.js @@ -76,7 +76,9 @@ exports.run = function(params, opts, callback) { pack = store.config.readConfigJSON(path.join(root, 'package.json')); - options.port = params[0] || appConfig.appPort || process.env.PORT || 8666; + options.port = parseInt(params[0], 10) || appConfig.appPort; + options.port = options.port || process.env.PORT || 8666; + if (inputOptions.context) { options.context = inputOptions.context; } From c17ecf88c270533bb93f1d385a74b8800003a6b2 Mon Sep 17 00:00:00 2001 From: Isao Yagi Date: Wed, 20 Mar 2013 15:48:28 -0700 Subject: [PATCH 03/38] add tests for mojito start port args --- tests/unit/lib/app/commands/test-start.js | 28 +++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/unit/lib/app/commands/test-start.js b/tests/unit/lib/app/commands/test-start.js index 454d03556..810da46db 100644 --- a/tests/unit/lib/app/commands/test-start.js +++ b/tests/unit/lib/app/commands/test-start.js @@ -84,6 +84,34 @@ YUI().use('mojito', 'mojito-test-extra', 'test', function(Y) { A.areSame(8667, port); }, + 'test run start string port': function() { + A.areSame(0, listenCalls); + start.run(['8888'], null, function() {}); + A.areSame(1, listenCalls); + A.areSame(8888, port); + }, + + 'test run start funny string port': function() { + A.areSame(0, listenCalls); + start.run(['07777a.bc'], null, function() {}); + A.areSame(1, listenCalls); + A.areSame(7777, port); + }, + + 'test run start port is "foo"': function() { + A.areSame(0, listenCalls); + start.run(['foo'], null, function() {}); + A.areSame(1, listenCalls); + A.areSame(8666, port); + }, + + 'test run start with args ["app", "."]': function() { + A.areSame(0, listenCalls); + start.run(["app", "."], null, function() {}); + A.areSame(1, listenCalls); + A.areSame(8666, port); + }, + 'test run start context': function() { var options = { context: "environment:production" From 98f634b1ae373d74f947b26860f856d4fb932cd8 Mon Sep 17 00:00:00 2001 From: Alexander Shusta Date: Fri, 22 Mar 2013 08:04:50 -0700 Subject: [PATCH 04/38] Updated docs to explain there are no log files Used Caridy's explanation from https://github.com/yahoo/mojito/issues/1025#issuecomment-15270635 to call out that there are no log files. --- docs/dev_guide/topics/mojito_logging.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/dev_guide/topics/mojito_logging.rst b/docs/dev_guide/topics/mojito_logging.rst index bc8805375..1c14bb9aa 100644 --- a/docs/dev_guide/topics/mojito_logging.rst +++ b/docs/dev_guide/topics/mojito_logging.rst @@ -7,6 +7,11 @@ log messages are handled by a YUI instance that Mojito creates based on YUI conf defined in ``application.json`` or ``application.yaml``. You can set logging levels to control the degree of detail in your log reports. +Mojito does not write logs into a file instead, it writes into the node.js console. Whatever +node.js does with the logs, writing them into a file, transmiting them into an aggregated +hub for multiple cores, or whatever other crazy idea people decide to implement has very +little to do with Mojito. + .. _mojito_logging-levels: Log Levels From e61c90039acce9499081e3d78d79fda7b2dad22b Mon Sep 17 00:00:00 2001 From: Joe Catera Date: Mon, 25 Mar 2013 16:19:42 -0700 Subject: [PATCH 05/38] Fixing typo. --- docs/dev_guide/intro/mojito_mvc.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/dev_guide/intro/mojito_mvc.rst b/docs/dev_guide/intro/mojito_mvc.rst index 005365462..17fcc11b4 100644 --- a/docs/dev_guide/intro/mojito_mvc.rst +++ b/docs/dev_guide/intro/mojito_mvc.rst @@ -870,7 +870,7 @@ Yahoo! pages to ``ac.done``: ... In the template, we can now use the block helper ``each`` to -create links with the objects and their properties``name`` and ``url``: +create links with the objects and their properties ``name`` and ``url``: .. code-block: html From 9ed57ebc5512743d932301a5d89d4f5b25676320 Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Thu, 21 Mar 2013 15:36:18 -0700 Subject: [PATCH 06/38] refactor tunnel middleware into two phases By splitting the tunnel logic into a detection phase and a handling phase, applications are free to override the handling logic. In order to maintain backwards-compatibility, instead of refactoring the existing tunnel middleware, a tunnel-demux middleware to detect the type of tunnel request, and tunnel-(rpc|specs|type) middleware to handle each type of request have been added. Mojito itself will now use these new middleware in place of the single tunnel middleware, which will remain for those applications that are specifying it in their configuration. --- .../middleware/mojito-handler-tunnel-demux.js | 120 ++++++++++++++++++ .../middleware/mojito-handler-tunnel-rpc.js | 66 ++++++++++ .../middleware/mojito-handler-tunnel-specs.js | 66 ++++++++++ .../middleware/mojito-handler-tunnel-type.js | 56 ++++++++ lib/mojito.js | 5 +- 5 files changed, 312 insertions(+), 1 deletion(-) create mode 100644 lib/app/middleware/mojito-handler-tunnel-demux.js create mode 100644 lib/app/middleware/mojito-handler-tunnel-rpc.js create mode 100644 lib/app/middleware/mojito-handler-tunnel-specs.js create mode 100644 lib/app/middleware/mojito-handler-tunnel-type.js diff --git a/lib/app/middleware/mojito-handler-tunnel-demux.js b/lib/app/middleware/mojito-handler-tunnel-demux.js new file mode 100644 index 000000000..2ce9c46c1 --- /dev/null +++ b/lib/app/middleware/mojito-handler-tunnel-demux.js @@ -0,0 +1,120 @@ +/* + * Copyright (c) 2011-2013, Yahoo! Inc. All rights reserved. + * Copyrights licensed under the New BSD License. + * See the accompanying LICENSE file for terms. + */ + +/*global require, module*/ +/*jslint sloppy:true, nomen:true*/ + + +var liburl = require('url'), + RE_REPEATING_SLASH = /\/{2,}/g; + +function trimSlash(str) { + if (str.charAt(0) === '/') { + str = str.substring(1, str.length); + } + if (str.charAt(str.length - 1) === '/') { + str = str.substring(0, str.length - 1); + } + return str; +} + + +/** + * Export a function which can create the handler. + * @param {Object} config Data to configure the handler. + * @return {Object} The newly constructed handler. + */ +module.exports = function (config) { + var that = this, + appConfig = config.store.getAppConfig({}) || {}, + staticPrefix, + tunnelPrefix; + + staticPrefix = appConfig.staticHandling && appConfig.staticHandling.prefix; + tunnelPrefix = appConfig.tunnelPrefix; + + if (staticPrefix) { + staticPrefix = '/' + trimSlash(staticPrefix); + } + if (tunnelPrefix) { + tunnelPrefix = '/' + trimSlash(tunnelPrefix); + } + + this.staticPrefix = staticPrefix || '/static'; + this.tunnelPrefix = tunnelPrefix || '/tunnel'; + + return function (req, res, next) { + var hasTunnelPrefix = req.url.indexOf(that.tunnelPrefix) === 0, + hasTunnelHeader = req.headers['x-mojito-header'] === 'tunnel', + name, + type, + url, + parts; + + // If we are not tunneling get out of here fast! + if (!hasTunnelPrefix && !hasTunnelHeader) { + return next(); + } + + req._tunnel = {}; + + /** + Tunnel examples + + RPC tunnel: + /tunnel (or it could just have the tunnel header) + + Type tunnel: + /static/{type}/definition.json + /{tunnelPrefix}/{type}/definition.json // custom prefix + /tunnel/static/{type}/definition.json // according to a UT + + Spec tunnel: + /static/{type}/specs/default.json + /{staticPrefix}/{type}/specs/default.json // custom prefix + /tunnel/static/{type}/specs/default.json // according to a UT + **/ + + url = req.url.split('?')[0]; + + // Normalization step to handle `/{tunnelPrefix}`, `/{staticPrefix}`, + // and `/{tunnelPrefix}/{staticPrefix}` URLs. + url = url.replace(that.staticPrefix, '') + .replace(that.tunnelPrefix, '') + .replace(RE_REPEATING_SLASH, '/'); + + parts = url.split('/'); + + if (parts.length) { + name = parts[parts.length - 1]; + type = parts[1]; + + // Spec tunnel + if (parts[parts.length - 2] === 'specs') { + req._tunnel.specsReq = { + type: type, + name: name + }; + return next(); + } + // Type tunnel + if (name === 'definition.json') { + req._tunnel.typeReq = { + type: type + }; + return next(); + } + } + + // RPC tunnel + if (req.url === that.tunnelPrefix && req.method === 'POST') { + req._tunnel.rpcReq = {}; + return next(); + } + + return next(); + }; +}; diff --git a/lib/app/middleware/mojito-handler-tunnel-rpc.js b/lib/app/middleware/mojito-handler-tunnel-rpc.js new file mode 100644 index 000000000..bcf39309e --- /dev/null +++ b/lib/app/middleware/mojito-handler-tunnel-rpc.js @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2011-2013, Yahoo! Inc. All rights reserved. + * Copyrights licensed under the New BSD License. + * See the accompanying LICENSE file for terms. + */ + +/*global exports, module*/ +/*jslint sloppy:true, nomen:true*/ + + +function sendData(res, data, code) { + res.writeHead((code || 200), { + 'content-type': 'application/json; charset="utf-8"' + }); + res.end(JSON.stringify(data, null, 4)); +} + +function sendError(res, msg, code) { + sendData(res, {error: msg}, (code || 500)); +} + + +/** + * Exports a middleware factory that can handle RPC tunnel requests. + * + * @param {Object} config The configuration. + * @return {Function} The handler. + */ +module.exports = function (config) { + return function (req, res, next) { + var command = req.body, + instance = command.instance; + + command.context = command.context || {}; + + if (!req._tunnel || !req._tunnel.rpcReq) { + return next(); + } + + // When switching from the client context to the server context, we + // have to override the runtime. + command.context.runtime = 'server'; + + // All we need to do is expand the instance given within the RPC call + // and attach it within a "tunnelCommand", which will be handled by + // Mojito instead of looking up a route for it. + config.store.expandInstance(instance, command.context, function (err, instance) { + // Replace with the expanded instance. + command.instance = instance; + req.command = { + action: command.action, + instance: { + // Magic here to delegate to tunnelProxy. + base: 'tunnelProxy' + }, + params: { + body: { + proxyCommand: command + } + }, + context: command.context + }; + return next(); + }); + }; +}; diff --git a/lib/app/middleware/mojito-handler-tunnel-specs.js b/lib/app/middleware/mojito-handler-tunnel-specs.js new file mode 100644 index 000000000..0244cd48b --- /dev/null +++ b/lib/app/middleware/mojito-handler-tunnel-specs.js @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2011-2013, Yahoo! Inc. All rights reserved. + * Copyrights licensed under the New BSD License. + * See the accompanying LICENSE file for terms. + */ + +/*global module*/ +/*jslint sloppy:true, nomen:true*/ + + +function sendData(res, data, code) { + res.writeHead((code || 200), { + 'content-type': 'application/json; charset="utf-8"' + }); + res.end(JSON.stringify(data, null, 4)); +} + +function sendError(res, msg, code) { + sendData(res, {error: msg}, (code || 500)); +} + + +/** + * Exports a middleware factory that can handle spec tunnel requests. + * + * @param {Object} config The configuration. + * @return {Function} The handler. + */ +module.exports = function (config) { + return function (req, res, next) { + var specsReq = req._tunnel && req._tunnel.specsReq, + instance = {}, + type, + name; + + if (!specsReq) { + return next(); + } + + type = specsReq.type; + name = specsReq.name; + name = name && name.split('.').slice(0, -1).join('.'); + + if (!type || !name) { + return sendError(res, 'Not found: ' + req.url, 500); + } + + instance.base = type; + + if (name !== 'default') { + instance.base += ':' + name; + } + + config.store.expandInstanceForEnv('client', instance, req.context, + function (err, data) { + if (err) { + return sendError( + res, + 'Error opening: ' + req.url + '\n' + err, + 500 + ); + } + return sendData(res, data); + }); + }; +}; diff --git a/lib/app/middleware/mojito-handler-tunnel-type.js b/lib/app/middleware/mojito-handler-tunnel-type.js new file mode 100644 index 000000000..e0874332d --- /dev/null +++ b/lib/app/middleware/mojito-handler-tunnel-type.js @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2011-2013, Yahoo! Inc. All rights reserved. + * Copyrights licensed under the New BSD License. + * See the accompanying LICENSE file for terms. + */ + +/*global module*/ +/*jslint sloppy:true, nomen:true*/ + + +function sendData(res, data, code) { + res.writeHead((code || 200), { + 'content-type': 'application/json; charset="utf-8"' + }); + res.end(JSON.stringify(data, null, 4)); +} + +function sendError(res, msg, code) { + sendData(res, {error: msg}, (code || 500)); +} + + +/** + * Exports a middleware factory that can handle type tunnel requests. + * + * @param {Object} config The configuration. + * @return {Function} The handler. + */ +module.exports = function (config) { + return function (req, res, next) { + var typeReq = req._tunnel && req._tunnel.typeReq, + instance = {}; + + if (!typeReq) { + return next(); + } + + if (!typeReq.type) { + return sendError(res, 'Not found: ' + req.url, 500); + } + + instance.type = typeReq.type; + + config.store.expandInstanceForEnv('client', instance, req.context, + function (err, data) { + if (err) { + return sendError( + res, + 'Error opening: ' + req.url + '\n' + err, + 500 + ); + } + return sendData(res, data); + }); + }; +}; diff --git a/lib/mojito.js b/lib/mojito.js index f73652262..ec3c7ec1a 100644 --- a/lib/mojito.js +++ b/lib/mojito.js @@ -90,7 +90,10 @@ MojitoServer.MOJITO_MIDDLEWARE = [ 'mojito-parser-body', 'mojito-parser-cookies', 'mojito-contextualizer', - 'mojito-handler-tunnel', + 'mojito-handler-tunnel-demux', + 'mojito-handler-tunnel-rpc', + 'mojito-handler-tunnel-specs', + 'mojito-handler-tunnel-type', 'mojito-router', 'mojito-handler-dispatcher' ]; From 14dc572e19e0928c18c42292eea541fbd42a4a78 Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Thu, 21 Mar 2013 18:35:25 -0700 Subject: [PATCH 07/38] implement feedback from pull #1044 - delay object creation for performance reasons - use core library methods for parsing url/path - fix http response code (500 => 404) - eliminate `that = this` usage by closing over prefix variables directly --- .../middleware/mojito-handler-tunnel-demux.js | 36 +++++++------ .../middleware/mojito-handler-tunnel-rpc.js | 53 +++++++++++-------- .../middleware/mojito-handler-tunnel-specs.js | 33 +++++------- .../middleware/mojito-handler-tunnel-type.js | 30 +++++------ 4 files changed, 77 insertions(+), 75 deletions(-) diff --git a/lib/app/middleware/mojito-handler-tunnel-demux.js b/lib/app/middleware/mojito-handler-tunnel-demux.js index 2ce9c46c1..5774c521d 100644 --- a/lib/app/middleware/mojito-handler-tunnel-demux.js +++ b/lib/app/middleware/mojito-handler-tunnel-demux.js @@ -8,8 +8,8 @@ /*jslint sloppy:true, nomen:true*/ -var liburl = require('url'), - RE_REPEATING_SLASH = /\/{2,}/g; +var liburl = require('url'), + libpath = require('path'); function trimSlash(str) { if (str.charAt(0) === '/') { @@ -28,8 +28,7 @@ function trimSlash(str) { * @return {Object} The newly constructed handler. */ module.exports = function (config) { - var that = this, - appConfig = config.store.getAppConfig({}) || {}, + var appConfig = config.store.getAppConfig({}) || {}, staticPrefix, tunnelPrefix; @@ -43,15 +42,15 @@ module.exports = function (config) { tunnelPrefix = '/' + trimSlash(tunnelPrefix); } - this.staticPrefix = staticPrefix || '/static'; - this.tunnelPrefix = tunnelPrefix || '/tunnel'; + staticPrefix = staticPrefix || '/static'; + tunnelPrefix = tunnelPrefix || '/tunnel'; return function (req, res, next) { - var hasTunnelPrefix = req.url.indexOf(that.tunnelPrefix) === 0, + var hasTunnelPrefix = req.url.indexOf(tunnelPrefix) === 0, hasTunnelHeader = req.headers['x-mojito-header'] === 'tunnel', name, type, - url, + path, parts; // If we are not tunneling get out of here fast! @@ -78,18 +77,23 @@ module.exports = function (config) { /tunnel/static/{type}/specs/default.json // according to a UT **/ - url = req.url.split('?')[0]; + path = liburl.parse(req.url).pathname; + + // Replace multiple slashes with a single one. + path = libpath.resolve(path); // Normalization step to handle `/{tunnelPrefix}`, `/{staticPrefix}`, // and `/{tunnelPrefix}/{staticPrefix}` URLs. - url = url.replace(that.staticPrefix, '') - .replace(that.tunnelPrefix, '') - .replace(RE_REPEATING_SLASH, '/'); + path = path.replace(staticPrefix, '') + .replace(tunnelPrefix, ''); - parts = url.split('/'); + parts = path.split('/'); if (parts.length) { - name = parts[parts.length - 1]; + // Get the basename without the .json extension. + name = libpath.basename(path, '.json'); + + // Get the mojit type. type = parts[1]; // Spec tunnel @@ -101,7 +105,7 @@ module.exports = function (config) { return next(); } // Type tunnel - if (name === 'definition.json') { + if (name === 'definition') { req._tunnel.typeReq = { type: type }; @@ -110,7 +114,7 @@ module.exports = function (config) { } // RPC tunnel - if (req.url === that.tunnelPrefix && req.method === 'POST') { + if (req.url === tunnelPrefix && req.method === 'POST') { req._tunnel.rpcReq = {}; return next(); } diff --git a/lib/app/middleware/mojito-handler-tunnel-rpc.js b/lib/app/middleware/mojito-handler-tunnel-rpc.js index bcf39309e..249202a96 100644 --- a/lib/app/middleware/mojito-handler-tunnel-rpc.js +++ b/lib/app/middleware/mojito-handler-tunnel-rpc.js @@ -28,15 +28,18 @@ function sendError(res, msg, code) { */ module.exports = function (config) { return function (req, res, next) { - var command = req.body, - instance = command.instance; + var rpcReq = req._tunnel && req._tunnel.rpcReq, + command, + instance; - command.context = command.context || {}; - - if (!req._tunnel || !req._tunnel.rpcReq) { + if (!rpcReq) { return next(); } + command = req.body; + instance = command.instance; + command.context = command.context || {}; + // When switching from the client context to the server context, we // have to override the runtime. command.context.runtime = 'server'; @@ -44,23 +47,27 @@ module.exports = function (config) { // All we need to do is expand the instance given within the RPC call // and attach it within a "tunnelCommand", which will be handled by // Mojito instead of looking up a route for it. - config.store.expandInstance(instance, command.context, function (err, instance) { - // Replace with the expanded instance. - command.instance = instance; - req.command = { - action: command.action, - instance: { - // Magic here to delegate to tunnelProxy. - base: 'tunnelProxy' - }, - params: { - body: { - proxyCommand: command - } - }, - context: command.context - }; - return next(); - }); + config.store.expandInstance( + instance, + command.context, + function (err, instance) { + // Replace with the expanded instance. + command.instance = instance; + req.command = { + action: command.action, + instance: { + // Magic here to delegate to tunnelProxy. + base: 'tunnelProxy' + }, + params: { + body: { + proxyCommand: command + } + }, + context: command.context + }; + return next(); + } + ); }; }; diff --git a/lib/app/middleware/mojito-handler-tunnel-specs.js b/lib/app/middleware/mojito-handler-tunnel-specs.js index 0244cd48b..fd03d9d04 100644 --- a/lib/app/middleware/mojito-handler-tunnel-specs.js +++ b/lib/app/middleware/mojito-handler-tunnel-specs.js @@ -29,7 +29,6 @@ function sendError(res, msg, code) { module.exports = function (config) { return function (req, res, next) { var specsReq = req._tunnel && req._tunnel.specsReq, - instance = {}, type, name; @@ -39,28 +38,22 @@ module.exports = function (config) { type = specsReq.type; name = specsReq.name; - name = name && name.split('.').slice(0, -1).join('.'); if (!type || !name) { - return sendError(res, 'Not found: ' + req.url, 500); + return sendError(res, 'Not found: ' + req.url, 404); } - instance.base = type; - - if (name !== 'default') { - instance.base += ':' + name; - } - - config.store.expandInstanceForEnv('client', instance, req.context, - function (err, data) { - if (err) { - return sendError( - res, - 'Error opening: ' + req.url + '\n' + err, - 500 - ); - } - return sendData(res, data); - }); + config.store.expandInstanceForEnv('client', { + base: (name === 'default') ? type : type + ':' + name + }, req.context, function (err, data) { + if (err) { + return sendError( + res, + 'Error opening: ' + req.url + '\n' + err, + 500 + ); + } + return sendData(res, data); + }); }; }; diff --git a/lib/app/middleware/mojito-handler-tunnel-type.js b/lib/app/middleware/mojito-handler-tunnel-type.js index e0874332d..5bcacdb17 100644 --- a/lib/app/middleware/mojito-handler-tunnel-type.js +++ b/lib/app/middleware/mojito-handler-tunnel-type.js @@ -28,29 +28,27 @@ function sendError(res, msg, code) { */ module.exports = function (config) { return function (req, res, next) { - var typeReq = req._tunnel && req._tunnel.typeReq, - instance = {}; + var typeReq = req._tunnel && req._tunnel.typeReq; if (!typeReq) { return next(); } if (!typeReq.type) { - return sendError(res, 'Not found: ' + req.url, 500); + return sendError(res, 'Not found: ' + req.url, 404); } - instance.type = typeReq.type; - - config.store.expandInstanceForEnv('client', instance, req.context, - function (err, data) { - if (err) { - return sendError( - res, - 'Error opening: ' + req.url + '\n' + err, - 500 - ); - } - return sendData(res, data); - }); + config.store.expandInstanceForEnv('client', { + type: typeReq.type + }, req.context, function (err, data) { + if (err) { + return sendError( + res, + 'Error opening: ' + req.url + '\n' + err, + 500 + ); + } + return sendData(res, data); + }); }; }; From 91122081b3da18db3bbb453a5096e8f4f6384c14 Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Fri, 22 Mar 2013 11:16:50 -0700 Subject: [PATCH 08/38] clean up the logic around path inspection - Replaced the brittle `trimSlash` stuff with `require('path').normalize`. --- .../middleware/mojito-handler-tunnel-demux.js | 28 ++++++------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/lib/app/middleware/mojito-handler-tunnel-demux.js b/lib/app/middleware/mojito-handler-tunnel-demux.js index 5774c521d..4c69b647a 100644 --- a/lib/app/middleware/mojito-handler-tunnel-demux.js +++ b/lib/app/middleware/mojito-handler-tunnel-demux.js @@ -11,15 +11,6 @@ var liburl = require('url'), libpath = require('path'); -function trimSlash(str) { - if (str.charAt(0) === '/') { - str = str.substring(1, str.length); - } - if (str.charAt(str.length - 1) === '/') { - str = str.substring(0, str.length - 1); - } - return str; -} /** @@ -36,14 +27,15 @@ module.exports = function (config) { tunnelPrefix = appConfig.tunnelPrefix; if (staticPrefix) { - staticPrefix = '/' + trimSlash(staticPrefix); + staticPrefix = '/' + staticPrefix; } if (tunnelPrefix) { - tunnelPrefix = '/' + trimSlash(tunnelPrefix); + tunnelPrefix = '/' + tunnelPrefix; } - staticPrefix = staticPrefix || '/static'; - tunnelPrefix = tunnelPrefix || '/tunnel'; + // normalize() will squash multiple slashes into one slash. + staticPrefix = libpath.normalize(staticPrefix) || '/static'; + tunnelPrefix = libpath.normalize(tunnelPrefix) || '/tunnel'; return function (req, res, next) { var hasTunnelPrefix = req.url.indexOf(tunnelPrefix) === 0, @@ -79,20 +71,18 @@ module.exports = function (config) { path = liburl.parse(req.url).pathname; - // Replace multiple slashes with a single one. - path = libpath.resolve(path); - // Normalization step to handle `/{tunnelPrefix}`, `/{staticPrefix}`, // and `/{tunnelPrefix}/{staticPrefix}` URLs. path = path.replace(staticPrefix, '') .replace(tunnelPrefix, ''); - parts = path.split('/'); - - if (parts.length) { + if (path) { // Get the basename without the .json extension. name = libpath.basename(path, '.json'); + // Time to get dirty. + parts = path.split('/'); + // Get the mojit type. type = parts[1]; From 31295f4cda6cae19a3187a00f6a9f7c452fe6c2c Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Fri, 22 Mar 2013 21:27:15 -0700 Subject: [PATCH 09/38] replace tunnel middleware with refactored version - Remove trailing slashes from tunnel/static prefixes. - Add flag and flag check to determine when to stop processing the request. --- ...mux.js => mojito-handler-tunnel-parser.js} | 36 ++- .../middleware/mojito-handler-tunnel-rpc.js | 3 +- .../middleware/mojito-handler-tunnel-specs.js | 49 ++-- .../middleware/mojito-handler-tunnel-type.js | 45 ++-- lib/app/middleware/mojito-handler-tunnel.js | 235 ++++-------------- lib/mojito.js | 5 +- 6 files changed, 107 insertions(+), 266 deletions(-) rename lib/app/middleware/{mojito-handler-tunnel-demux.js => mojito-handler-tunnel-parser.js} (78%) diff --git a/lib/app/middleware/mojito-handler-tunnel-demux.js b/lib/app/middleware/mojito-handler-tunnel-parser.js similarity index 78% rename from lib/app/middleware/mojito-handler-tunnel-demux.js rename to lib/app/middleware/mojito-handler-tunnel-parser.js index 4c69b647a..7e73a86af 100644 --- a/lib/app/middleware/mojito-handler-tunnel-demux.js +++ b/lib/app/middleware/mojito-handler-tunnel-parser.js @@ -5,21 +5,19 @@ */ /*global require, module*/ -/*jslint sloppy:true, nomen:true*/ - - -var liburl = require('url'), - libpath = require('path'); - +/*jslint sloppy:true, nomen:true, white:true*/ +var RE_TRAILING_SLASHES = /\/+$/; /** - * Export a function which can create the handler. - * @param {Object} config Data to configure the handler. - * @return {Object} The newly constructed handler. + * Export a function which can parse tunnel requests. + * @param {Object} config The configuration. + * @return {Object} The parser. */ module.exports = function (config) { - var appConfig = config.store.getAppConfig({}) || {}, + var liburl = require('url'), + libpath = require('path'), + appConfig = config.store.getAppConfig({}) || {}, staticPrefix, tunnelPrefix; @@ -27,9 +25,11 @@ module.exports = function (config) { tunnelPrefix = appConfig.tunnelPrefix; if (staticPrefix) { + staticPrefix = staticPrefix.replace(RE_TRAILING_SLASHES, ''); staticPrefix = '/' + staticPrefix; } if (tunnelPrefix) { + tunnelPrefix = tunnelPrefix.replace(RE_TRAILING_SLASHES, ''); tunnelPrefix = '/' + tunnelPrefix; } @@ -50,8 +50,6 @@ module.exports = function (config) { return next(); } - req._tunnel = {}; - /** Tunnel examples @@ -86,27 +84,23 @@ module.exports = function (config) { // Get the mojit type. type = parts[1]; - // Spec tunnel + // "Spec" tunnel request if (parts[parts.length - 2] === 'specs') { req._tunnel.specsReq = { type: type, name: name }; - return next(); } - // Type tunnel - if (name === 'definition') { + // "Type" tunnel request + else if (name === 'definition') { req._tunnel.typeReq = { type: type }; - return next(); } } - - // RPC tunnel - if (req.url === tunnelPrefix && req.method === 'POST') { + // "RPC" tunnel request + else if (req.url === tunnelPrefix && req.method === 'POST') { req._tunnel.rpcReq = {}; - return next(); } return next(); diff --git a/lib/app/middleware/mojito-handler-tunnel-rpc.js b/lib/app/middleware/mojito-handler-tunnel-rpc.js index 249202a96..164868bcd 100644 --- a/lib/app/middleware/mojito-handler-tunnel-rpc.js +++ b/lib/app/middleware/mojito-handler-tunnel-rpc.js @@ -37,7 +37,6 @@ module.exports = function (config) { } command = req.body; - instance = command.instance; command.context = command.context || {}; // When switching from the client context to the server context, we @@ -48,7 +47,7 @@ module.exports = function (config) { // and attach it within a "tunnelCommand", which will be handled by // Mojito instead of looking up a route for it. config.store.expandInstance( - instance, + command.instance, command.context, function (err, instance) { // Replace with the expanded instance. diff --git a/lib/app/middleware/mojito-handler-tunnel-specs.js b/lib/app/middleware/mojito-handler-tunnel-specs.js index fd03d9d04..4da822c52 100644 --- a/lib/app/middleware/mojito-handler-tunnel-specs.js +++ b/lib/app/middleware/mojito-handler-tunnel-specs.js @@ -7,19 +7,6 @@ /*global module*/ /*jslint sloppy:true, nomen:true*/ - -function sendData(res, data, code) { - res.writeHead((code || 200), { - 'content-type': 'application/json; charset="utf-8"' - }); - res.end(JSON.stringify(data, null, 4)); -} - -function sendError(res, msg, code) { - sendData(res, {error: msg}, (code || 500)); -} - - /** * Exports a middleware factory that can handle spec tunnel requests. * @@ -29,6 +16,7 @@ function sendError(res, msg, code) { module.exports = function (config) { return function (req, res, next) { var specsReq = req._tunnel && req._tunnel.specsReq, + instance, type, name; @@ -40,20 +28,31 @@ module.exports = function (config) { name = specsReq.name; if (!type || !name) { - return sendError(res, 'Not found: ' + req.url, 404); + return req._tunnel.sendError(res, 'Not found: ' + req.url, 404); + } + + instance = { + base: type + }; + + if (name !== 'default') { + instance.base += ':' + name; } - config.store.expandInstanceForEnv('client', { - base: (name === 'default') ? type : type + ':' + name - }, req.context, function (err, data) { - if (err) { - return sendError( - res, - 'Error opening: ' + req.url + '\n' + err, - 500 - ); + config.store.expandInstanceForEnv( + 'client', + instance, + req.context, + function (err, data) { + if (err) { + return req._tunnel.sendError( + res, + 'Error opening: ' + req.url + '\n' + err, + 500 + ); + } + return req._tunnel.sendData(res, data); } - return sendData(res, data); - }); + ); }; }; diff --git a/lib/app/middleware/mojito-handler-tunnel-type.js b/lib/app/middleware/mojito-handler-tunnel-type.js index 5bcacdb17..c31733d3e 100644 --- a/lib/app/middleware/mojito-handler-tunnel-type.js +++ b/lib/app/middleware/mojito-handler-tunnel-type.js @@ -7,19 +7,6 @@ /*global module*/ /*jslint sloppy:true, nomen:true*/ - -function sendData(res, data, code) { - res.writeHead((code || 200), { - 'content-type': 'application/json; charset="utf-8"' - }); - res.end(JSON.stringify(data, null, 4)); -} - -function sendError(res, msg, code) { - sendData(res, {error: msg}, (code || 500)); -} - - /** * Exports a middleware factory that can handle type tunnel requests. * @@ -28,27 +15,35 @@ function sendError(res, msg, code) { */ module.exports = function (config) { return function (req, res, next) { - var typeReq = req._tunnel && req._tunnel.typeReq; + var typeReq = req._tunnel && req._tunnel.typeReq, + instance; if (!typeReq) { return next(); } if (!typeReq.type) { - return sendError(res, 'Not found: ' + req.url, 404); + return req._tunnel.sendError(res, 'Not found: ' + req.url, 404); } - config.store.expandInstanceForEnv('client', { + instance = { type: typeReq.type - }, req.context, function (err, data) { - if (err) { - return sendError( - res, - 'Error opening: ' + req.url + '\n' + err, - 500 - ); + }; + + config.store.expandInstanceForEnv( + 'client', + instance, + req.context, + function (err, data) { + if (err) { + return req._tunnel.sendError( + res, + 'Error opening: ' + req.url + '\n' + err, + 500 + ); + } + return req._tunnel.sendData(res, data); } - return sendData(res, data); - }); + ); }; }; diff --git a/lib/app/middleware/mojito-handler-tunnel.js b/lib/app/middleware/mojito-handler-tunnel.js index 2a979e6dd..8d86812ca 100644 --- a/lib/app/middleware/mojito-handler-tunnel.js +++ b/lib/app/middleware/mojito-handler-tunnel.js @@ -4,202 +4,59 @@ * See the accompanying LICENSE file for terms. */ +/*global require, module*/ +/*jslint sloppy:true, nomen:true*/ -/*jslint anon:true, sloppy:true, nomen:true*/ - - -var liburl = require('url'), - logger, - RX_MULTI_SLASH_ALL = /\/+/g; - - -function trimSlash(str) { - if ('/' === str.charAt(str.length - 1)) { - return str.substring(0, str.length - 1); - } - return str; -} - - -function TunnelServer() {} - -/* -* store.client.js expandInstance() makes an RPC call to the TunnelServer. -* The header 'x-mojito-header' (read here and set in -* store.client.js) tells the server not to try to route the URL, it gets handled -* by this critter. The targeted URL _might actually exist_ but we need to make -* sure that it _does not_ if the mojito header is set to 'tunnel'. -*/ -TunnelServer.prototype = { - - handle: function(store, globalLogger) { - var self = this, - config; - logger = globalLogger; - //console.log('creating handle'); - this._store = store; - config = store.getAppConfig({}); - this.tunnelPrefix = (config && config.tunnelPrefix) ? - config.tunnelPrefix : - '/tunnel'; - this.staticPrefix = '/static'; - if (config && config.staticHandling && - config.staticHandling.hasOwnProperty('prefix')) { - this.staticPrefix = (config.staticHandling.prefix ? - '/' + config.staticHandling.prefix : - ''); - } - this.tunnelPrefix = trimSlash(this.tunnelPrefix); - this.staticPrefix = trimSlash(this.staticPrefix); - if (!this.tunnelPrefix) { - // this makes the logic below a bit simpler - this.tunnelPrefix = '/'; - } - - return function(req, res, next) { - var url, parts; - - // If we are not in a tunnel get out of here fast - if (req.url.indexOf(self.tunnelPrefix) !== 0 && - req.headers['x-mojito-header'] !== 'tunnel') { - return next(); +/** + * Export a middleware aggregate. + * @param {Object} The configuration. + * @return {Object} The handler. + */ +module.exports = function (config) { + var tunnelSubstack = [ + require('./mojito-handler-tunnel-parser')(config), + require('./mojito-handler-tunnel-rpc')(config), + require('./mojito-handler-tunnel-specs')(config), + require('./mojito-handler-tunnel-type')(config) + ]; + + return function (req, res, next) { + var len, + i; + + // Connects the tunnel middleware substack. + function connect(err) { + // Exit the substack on error. + if (err) { + next(err); } + } - url = req.url.replace(self.tunnelPrefix, '').replace( - self.staticPrefix, - '' - ); - url = url.replace(RX_MULTI_SLASH_ALL, '/'); - url = url.split('?')[0]; - parts = url.split('/'); - - if (parts.length === 4 && parts[2] === 'specs') { - return self._handleSpec(req, res, next, parts[1], parts[3]); + req._tunnel = { + sendData: function (res, data, code) { + res.writeHead((code || 200), { + 'content-type': 'application/json; charset="utf-8"' + }); + res.end(JSON.stringify(data, null, 4)); + + // Flag the end of this request. + req._tunnel.done = true; + }, + sendError: function (res, msg, code) { + this.sendData(res, {error: msg}, (code || 500)); } - if (parts.length === 3 && parts[2] === 'definition.json') { - return self._handleType(req, res, next, parts[1]); - } - if (req.url === self.tunnelPrefix && 'POST' === req.method) { - return self._handleRpc(req, res, next); - } - next(); }; - }, - - - _handleSpec: function(req, res, next, type, basename) { - var name, - instance = {}, - my = this; - - name = basename.split('.').slice(0, -1).join('.') || null; - if (!type || !name) { - my._sendError(res, 'Not found: ' + req.url, 500); - return; - } - - instance.base = type; - - if (name !== 'default') { - instance.base += ':' + name; - } - - this._store.expandInstanceForEnv('client', instance, req.context, - function(err, data) { - if (err) { - my._sendError(res, 'Error opening: ' + req.url + '\n' + - err, - 500 - ); - return; - } - my._sendData(res, data); - }); - }, - - - _handleType: function(req, res, next, type) { - var instance = {}, - my = this; - - if (!type) { - my._sendError(res, 'Not found: ' + req.url, 500); - return; - } - - instance.type = type; - - this._store.expandInstanceForEnv('client', instance, req.context, - function(err, data) { - if (err) { - my._sendError(res, 'Error opening: ' + req.url + '\n' + - err, - 'debug', - 'Tunnel:specs' - ); - return; - } - my._sendData(res, data); - }); - }, + // Iterate over the tunnel middleware substack. + for (i = 0, len = tunnelSubstack.length; i < len; i += 1) { + tunnelSubstack[i](req, res, connect); - - _handleRpc: function(req, res, next) { - var data = req.body, - command = data; - - - // when taking in the client context on the server side, we have to - // override the runtime, because the runtime switches from client to server - if (!command.context) { - command.context = {}; + // End this request if we've provided an end-point in the stack. + if (req._tunnel.done) { + return; + } } - command.context.runtime = 'server'; - - // all we need to do is expand the instance given within the RPC call - // and attach it within a "tunnelCommand", which will be handled by - // Mojito instead of looking up a route for it. - this._store.expandInstance(command.instance, command.context, - function(err, inst) { - // replace with the expanded instance - command.instance = inst; - req.command = { - action: command.action, - instance: { - // Magic here to delegate to tunnelProxy. - base: 'tunnelProxy' - }, - params: { - body: { - proxyCommand: command - } - }, - context: data.context - }; - next(); - }); - }, - - _sendError: function(res, msg, code) { - this._sendData(res, {error: msg}, (code || 500)); - }, - _sendData: function(res, data, code) { - res.writeHead((code || 200), { - 'content-type': 'application/json; charset="utf-8"' - }); - res.end(JSON.stringify(data, null, 4)); - } -}; - - -/** - * Export a function which can create the handler. - * @param {Object} config Data to configure the handler. - * @return {Object} The newly constructed handler. - */ -module.exports = function(config) { - var tunnel = new TunnelServer(); - return tunnel.handle(config.store, config.logger); + next(); + }; }; diff --git a/lib/mojito.js b/lib/mojito.js index ec3c7ec1a..f73652262 100644 --- a/lib/mojito.js +++ b/lib/mojito.js @@ -90,10 +90,7 @@ MojitoServer.MOJITO_MIDDLEWARE = [ 'mojito-parser-body', 'mojito-parser-cookies', 'mojito-contextualizer', - 'mojito-handler-tunnel-demux', - 'mojito-handler-tunnel-rpc', - 'mojito-handler-tunnel-specs', - 'mojito-handler-tunnel-type', + 'mojito-handler-tunnel', 'mojito-router', 'mojito-handler-dispatcher' ]; From ea75f3aa8027e5000e7e75ebe0dde7e704ef047a Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Mon, 25 Mar 2013 09:50:34 -0700 Subject: [PATCH 10/38] fix bug where configured prefixes can be undefined --- lib/app/middleware/mojito-handler-tunnel-parser.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/app/middleware/mojito-handler-tunnel-parser.js b/lib/app/middleware/mojito-handler-tunnel-parser.js index 7e73a86af..a5bd9b9f8 100644 --- a/lib/app/middleware/mojito-handler-tunnel-parser.js +++ b/lib/app/middleware/mojito-handler-tunnel-parser.js @@ -24,18 +24,18 @@ module.exports = function (config) { staticPrefix = appConfig.staticHandling && appConfig.staticHandling.prefix; tunnelPrefix = appConfig.tunnelPrefix; + // normalize() will squash multiple slashes into one slash. if (staticPrefix) { staticPrefix = staticPrefix.replace(RE_TRAILING_SLASHES, ''); - staticPrefix = '/' + staticPrefix; + staticPrefix = libpath.normalize('/' + staticPrefix); } if (tunnelPrefix) { tunnelPrefix = tunnelPrefix.replace(RE_TRAILING_SLASHES, ''); - tunnelPrefix = '/' + tunnelPrefix; + tunnelPrefix = libpath.normalize('/' + tunnelPrefix); } - // normalize() will squash multiple slashes into one slash. - staticPrefix = libpath.normalize(staticPrefix) || '/static'; - tunnelPrefix = libpath.normalize(tunnelPrefix) || '/tunnel'; + staticPrefix = staticPrefix || '/static'; + tunnelPrefix = tunnelPrefix || '/tunnel'; return function (req, res, next) { var hasTunnelPrefix = req.url.indexOf(tunnelPrefix) === 0, From 68ac17c12b97b8a92bd6748fe8ad10bad2c94f8e Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Mon, 25 Mar 2013 09:52:52 -0700 Subject: [PATCH 11/38] fix test mock and remove unused variables --- .../middleware/mojito-handler-tunnel-rpc.js | 16 +---------- .../lib/app/middleware/test-handler-tunnel.js | 28 ++++++++++--------- 2 files changed, 16 insertions(+), 28 deletions(-) diff --git a/lib/app/middleware/mojito-handler-tunnel-rpc.js b/lib/app/middleware/mojito-handler-tunnel-rpc.js index 164868bcd..ac2b0e1f9 100644 --- a/lib/app/middleware/mojito-handler-tunnel-rpc.js +++ b/lib/app/middleware/mojito-handler-tunnel-rpc.js @@ -7,19 +7,6 @@ /*global exports, module*/ /*jslint sloppy:true, nomen:true*/ - -function sendData(res, data, code) { - res.writeHead((code || 200), { - 'content-type': 'application/json; charset="utf-8"' - }); - res.end(JSON.stringify(data, null, 4)); -} - -function sendError(res, msg, code) { - sendData(res, {error: msg}, (code || 500)); -} - - /** * Exports a middleware factory that can handle RPC tunnel requests. * @@ -29,8 +16,7 @@ function sendError(res, msg, code) { module.exports = function (config) { return function (req, res, next) { var rpcReq = req._tunnel && req._tunnel.rpcReq, - command, - instance; + command; if (!rpcReq) { return next(); diff --git a/tests/unit/lib/app/middleware/test-handler-tunnel.js b/tests/unit/lib/app/middleware/test-handler-tunnel.js index 6225705fd..97c156dec 100644 --- a/tests/unit/lib/app/middleware/test-handler-tunnel.js +++ b/tests/unit/lib/app/middleware/test-handler-tunnel.js @@ -123,14 +123,15 @@ YUI().use('mojito-test-extra', 'test', function(Y) { 'handler should override execution context to server (with /tunnel prefix)': function() { var nextCalls = 0, writeCalls = 0, endCalls = 0, req = { - 'url': '/tunnel', - 'method': 'POST', - 'body': { - 'context': { - 'runtime': 'client', - 'myKey': 'myValue' + url: '/tunnel', + method: 'POST', + body: { + context: { + runtime: 'client', + myKey: 'myValue' } - } + }, + headers: {} }, res = { }; @@ -151,13 +152,14 @@ YUI().use('mojito-test-extra', 'test', function(Y) { 'handler should set execution context to server (with /tunnel prefix)': function() { var nextCalls = 0, writeCalls = 0, endCalls = 0, req = { - 'url': '/tunnel', - 'method': 'POST', - 'body': { - 'reqs': [{ - 'data': {} + url: '/tunnel', + method: 'POST', + body: { + reqs: [{ + data: {} }] - } + }, + headers: {} }, res = { }; From 8de521c8c8f792e78c85e626c7c3a55c53c61355 Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Mon, 25 Mar 2013 09:56:08 -0700 Subject: [PATCH 12/38] execute tunnel middleware as an async queue --- lib/app/middleware/mojito-handler-tunnel.js | 52 ++++++++++----------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/lib/app/middleware/mojito-handler-tunnel.js b/lib/app/middleware/mojito-handler-tunnel.js index 8d86812ca..cf5dc8e2d 100644 --- a/lib/app/middleware/mojito-handler-tunnel.js +++ b/lib/app/middleware/mojito-handler-tunnel.js @@ -13,50 +13,48 @@ * @return {Object} The handler. */ module.exports = function (config) { - var tunnelSubstack = [ - require('./mojito-handler-tunnel-parser')(config), - require('./mojito-handler-tunnel-rpc')(config), - require('./mojito-handler-tunnel-specs')(config), - require('./mojito-handler-tunnel-type')(config) - ]; + var parser = require('./mojito-handler-tunnel-parser')(config), + rpc = require('./mojito-handler-tunnel-rpc')(config), + specs = require('./mojito-handler-tunnel-specs')(config), + type = require('./mojito-handler-tunnel-type')(config); return function (req, res, next) { - var len, - i; - - // Connects the tunnel middleware substack. - function connect(err) { - // Exit the substack on error. - if (err) { - next(err); - } - } - + var middleware = [ + parser, + rpc, + specs, + type + ]; + + // Helper methods. req._tunnel = { sendData: function (res, data, code) { res.writeHead((code || 200), { 'content-type': 'application/json; charset="utf-8"' }); res.end(JSON.stringify(data, null, 4)); - - // Flag the end of this request. - req._tunnel.done = true; }, sendError: function (res, msg, code) { this.sendData(res, {error: msg}, (code || 500)); } }; - // Iterate over the tunnel middleware substack. - for (i = 0, len = tunnelSubstack.length; i < len; i += 1) { - tunnelSubstack[i](req, res, connect); + function run() { + var m = middleware.shift(); - // End this request if we've provided an end-point in the stack. - if (req._tunnel.done) { - return; + if (!m) { + req._tunnel = null; + return next(); } + + m(req, res, function (err) { + if (err) { + return next(err); + } + run(); + }); } - next(); + run(); }; }; From d4d3c6da876505ccd311046f4be2456d51336634 Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Mon, 25 Mar 2013 13:24:48 -0700 Subject: [PATCH 13/38] streamline middleware error handling Uses the convention where a middleware with an arity of 4 is assumed to be the error handler. --- lib/app/middleware/mojito-handler-error.js | 23 +++++++++++++++++++ .../mojito-handler-tunnel-parser.js | 2 ++ .../middleware/mojito-handler-tunnel-rpc.js | 6 +++++ .../middleware/mojito-handler-tunnel-specs.js | 17 +++++++++----- .../middleware/mojito-handler-tunnel-type.js | 17 +++++++++----- lib/app/middleware/mojito-handler-tunnel.js | 13 ----------- lib/mojito.js | 6 ++--- 7 files changed, 55 insertions(+), 29 deletions(-) create mode 100644 lib/app/middleware/mojito-handler-error.js diff --git a/lib/app/middleware/mojito-handler-error.js b/lib/app/middleware/mojito-handler-error.js new file mode 100644 index 000000000..109ac7d3b --- /dev/null +++ b/lib/app/middleware/mojito-handler-error.js @@ -0,0 +1,23 @@ +/* + * Copyright (c) 2011-2013, Yahoo! Inc. All rights reserved. + * Copyrights licensed under the New BSD License. + * See the accompanying LICENSE file for terms. + */ + +/*global require, module*/ +/*jslint sloppy:true, nomen:true*/ + +/** + * Export a middleware error handler. + * @param {Object} The configuration. + * @return {Object} The handler. + */ +module.exports = function (config) { + return function (err, req, res, next) { + var statusCode = res.statusCode || 500; + res.send(statusCode, { + code: statusCode, + error: err.message + }); + }; +}; diff --git a/lib/app/middleware/mojito-handler-tunnel-parser.js b/lib/app/middleware/mojito-handler-tunnel-parser.js index a5bd9b9f8..86ae9db95 100644 --- a/lib/app/middleware/mojito-handler-tunnel-parser.js +++ b/lib/app/middleware/mojito-handler-tunnel-parser.js @@ -74,6 +74,8 @@ module.exports = function (config) { path = path.replace(staticPrefix, '') .replace(tunnelPrefix, ''); + req._tunnel = {}; + if (path) { // Get the basename without the .json extension. name = libpath.basename(path, '.json'); diff --git a/lib/app/middleware/mojito-handler-tunnel-rpc.js b/lib/app/middleware/mojito-handler-tunnel-rpc.js index ac2b0e1f9..8d166cf46 100644 --- a/lib/app/middleware/mojito-handler-tunnel-rpc.js +++ b/lib/app/middleware/mojito-handler-tunnel-rpc.js @@ -36,8 +36,13 @@ module.exports = function (config) { command.instance, command.context, function (err, instance) { + if (err) { + next(err); + } + // Replace with the expanded instance. command.instance = instance; + req.command = { action: command.action, instance: { @@ -51,6 +56,7 @@ module.exports = function (config) { }, context: command.context }; + return next(); } ); diff --git a/lib/app/middleware/mojito-handler-tunnel-specs.js b/lib/app/middleware/mojito-handler-tunnel-specs.js index 4da822c52..7f8a96d62 100644 --- a/lib/app/middleware/mojito-handler-tunnel-specs.js +++ b/lib/app/middleware/mojito-handler-tunnel-specs.js @@ -28,7 +28,8 @@ module.exports = function (config) { name = specsReq.name; if (!type || !name) { - return req._tunnel.sendError(res, 'Not found: ' + req.url, 404); + res.statusCode = 404; + return next(new Error('Not found: ' + req.url)); } instance = { @@ -45,13 +46,17 @@ module.exports = function (config) { req.context, function (err, data) { if (err) { - return req._tunnel.sendError( - res, - 'Error opening: ' + req.url + '\n' + err, - 500 + res.statusCode = 500; + return next( + new Error('Error opening: ' + req.url + '\n' + err) ); } - return req._tunnel.sendData(res, data); + // TODO: Use the express sugar method res.json([status], + // [body]) after we rewrite the existing tests. + res.writeHead(res.statusCode || 200, { + 'content-type': 'application/json; charset="utf-8"' + }); + res.end(JSON.stringify(data, null, 4)); } ); }; diff --git a/lib/app/middleware/mojito-handler-tunnel-type.js b/lib/app/middleware/mojito-handler-tunnel-type.js index c31733d3e..21f49d58d 100644 --- a/lib/app/middleware/mojito-handler-tunnel-type.js +++ b/lib/app/middleware/mojito-handler-tunnel-type.js @@ -23,7 +23,8 @@ module.exports = function (config) { } if (!typeReq.type) { - return req._tunnel.sendError(res, 'Not found: ' + req.url, 404); + res.statusCode = 404; + return next(new Error('Not found: ' + req.url)); } instance = { @@ -36,13 +37,17 @@ module.exports = function (config) { req.context, function (err, data) { if (err) { - return req._tunnel.sendError( - res, - 'Error opening: ' + req.url + '\n' + err, - 500 + res.statusCode = 500; + return next( + new Error('Error opening: ' + req.url + '\n' + err) ); } - return req._tunnel.sendData(res, data); + // TODO: Use the express sugar method res.json([status], + // [body]) after we rewrite the existing tests. + res.writeHead(res.statusCode || 200, { + 'content-type': 'application/json; charset="utf-8"' + }); + res.end(JSON.stringify(data, null, 4)); } ); }; diff --git a/lib/app/middleware/mojito-handler-tunnel.js b/lib/app/middleware/mojito-handler-tunnel.js index cf5dc8e2d..d8f13adc0 100644 --- a/lib/app/middleware/mojito-handler-tunnel.js +++ b/lib/app/middleware/mojito-handler-tunnel.js @@ -26,19 +26,6 @@ module.exports = function (config) { type ]; - // Helper methods. - req._tunnel = { - sendData: function (res, data, code) { - res.writeHead((code || 200), { - 'content-type': 'application/json; charset="utf-8"' - }); - res.end(JSON.stringify(data, null, 4)); - }, - sendError: function (res, msg, code) { - this.sendData(res, {error: msg}, (code || 500)); - } - }; - function run() { var m = middleware.shift(); diff --git a/lib/mojito.js b/lib/mojito.js index f73652262..1aa716f93 100644 --- a/lib/mojito.js +++ b/lib/mojito.js @@ -92,7 +92,8 @@ MojitoServer.MOJITO_MIDDLEWARE = [ 'mojito-contextualizer', 'mojito-handler-tunnel', 'mojito-router', - 'mojito-handler-dispatcher' + 'mojito-handler-dispatcher', + 'mojito-handler-error' ]; @@ -358,9 +359,6 @@ MojitoServer.prototype._configureAppInstance = function(app, options) { // attach middleware pieces this._useMiddleware(app, dispatcher, options.dir, midConfig, middleware); - // TODO: [Issue 82] The last middleware in the stack should be an - // error handler - Y.log('Mojito HTTP Server initialized in ' + ((new Date().getTime()) - Mojito.MOJITO_INIT) + 'ms.'); }; From ff14787e0701cd12d8e5db72ed728e548da16d9b Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Mon, 25 Mar 2013 15:50:08 -0700 Subject: [PATCH 14/38] initial unit test commit for refactored tunnel middleware --- .../middleware_test_descriptor.json | 16 + .../middleware/test-handler-tunnel-parser.js | 199 +++++++++++ .../app/middleware/test-handler-tunnel-rpc.js | 99 ++++++ .../lib/app/middleware/test-handler-tunnel.js | 326 ++---------------- 4 files changed, 351 insertions(+), 289 deletions(-) create mode 100644 tests/unit/lib/app/middleware/test-handler-tunnel-parser.js create mode 100644 tests/unit/lib/app/middleware/test-handler-tunnel-rpc.js diff --git a/tests/unit/lib/app/middleware/middleware_test_descriptor.json b/tests/unit/lib/app/middleware/middleware_test_descriptor.json index 99599d281..b802c0b50 100644 --- a/tests/unit/lib/app/middleware/middleware_test_descriptor.json +++ b/tests/unit/lib/app/middleware/middleware_test_descriptor.json @@ -32,6 +32,22 @@ }, "group": "fw,unit,server" }, + "handler-tunnel-parser": { + "params": { + "lib": "$$config.lib$$/app/middleware/mojito-handler-tunnel-parser.js", + "test": "./test-handler-tunnel-parser.js", + "driver": "nodejs" + }, + "group": "fw,unit,server" + }, + "handler-tunnel-rpc": { + "params": { + "lib": "$$config.lib$$/app/middleware/mojito-handler-tunnel-rpc.js", + "test": "./test-handler-tunnel-rpc.js", + "driver": "nodejs" + }, + "group": "fw,unit,server" + }, "router": { "params": { "lib": "$$config.lib$$/app/middleware/mojito-handler-tunnel.js,../../../../../lib/app/autoload/route-maker.common.js,../../../../../lib/app/autoload/util.common.js", diff --git a/tests/unit/lib/app/middleware/test-handler-tunnel-parser.js b/tests/unit/lib/app/middleware/test-handler-tunnel-parser.js new file mode 100644 index 000000000..6db1ff41a --- /dev/null +++ b/tests/unit/lib/app/middleware/test-handler-tunnel-parser.js @@ -0,0 +1,199 @@ +/* + * Copyright (c) 2011-2013, Yahoo! Inc. All rights reserved. + * Copyrights licensed under the New BSD License. + * See the accompanying LICENSE file for terms. + */ + +/*global YUI, require*/ +/*jslint nomen:true*/ + +YUI().use('mojito-test-extra', 'test', function (Y) { + 'use strict'; + + var A = Y.Assert, + AA = Y.ArrayAssert, + OA = Y.ObjectAssert, + + factory = require(Y.MOJITO_DIR + 'lib/app/middleware/mojito-handler-tunnel-parser'), + req, + nextCallCount, + middleware, + store, + config; + + + Y.Test.Runner.add(new Y.Test.Case({ + name: 'tunnel handler parser tests', + + setUp: function () { + nextCallCount = 0; + + store = { + getAppConfig: function () { + return {}; + } + }; + + config = { + store: store + }; + + req = { + url: '/tunnel', + headers: { + 'x-mojito-header': 'tunnel' + }, + method: 'POST' + }; + }, + + tearDown: function () { + store = null; + config = null; + nextCallCount = null; + req = null; + }, + + 'test trailing slashes are removed from tunnel URIs': function () { + config.store.getAppConfig = function () { + return { + tunnelPrefix: '/spinach/' + }; + }; + req.url = '/spinach'; + req.headers['x-mojito-header'] = 'nottunnel'; + + middleware = factory(config); + middleware(req, null, function () { + nextCallCount += 1; + }); + + A.isObject(req._tunnel.rpcReq, 'request should have been identified as a tunnel request'); + A.areSame(1, nextCallCount, 'next() should have been called'); + }, + + 'test multiple slashes are squashed into one': function () { + config.store.getAppConfig = function () { + return { + staticHandling: { + prefix: 'brusselsprouts//' + } + }; + }; + req.url = '/brusselsprouts/MojitX/definition.json'; + + middleware = factory(config); + middleware(req, null, function () { + nextCallCount += 1; + }); + + A.isObject(req._tunnel.typeReq, 'request should have been identified as a tunnel request'); + A.areSame('MojitX', req._tunnel.typeReq.type, 'should have gotten the correct mojit type'); + A.areSame(1, nextCallCount, 'next() should have been called'); + }, + + 'test prefix defaults are used if custom prefixes are not declared': function () { + middleware = factory(config); + middleware(req, null, function () { + nextCallCount += 1; + }); + + A.isObject(req._tunnel.rpcReq, 'request should have been identified as an rpc request'); + A.areSame(1, nextCallCount, 'next() should have been called for the rpc request'); + + req.url = '/static/MojitX/definition.json'; + middleware(req, null, function () { + nextCallCount += 1; + }); + + A.isObject(req._tunnel.typeReq, 'request should have been identified as a type request'); + A.areSame(2, nextCallCount, 'next() should have been called for the type request'); + }, + + 'test exit early if not tunnel request': function () { + req.url = '/spinach'; + req.headers['x-mojito-header'] = 'brusselsprouts'; + + middleware = factory(config); + middleware(req, null, function () { + nextCallCount += 1; + }); + + A.areSame(1, nextCallCount, 'next() should have been called'); + A.isUndefined(req._tunnel, 'next() should have been called immediately'); + }, + + 'test tunnel request paths are normalized correctly': function () { + req.url = '/static/MojitX/specs/broccoli.json'; + + middleware = factory(config); + middleware(req, null, function () { + nextCallCount += 1; + }); + + A.areSame(1, nextCallCount, 'next() should have been called'); + A.isObject(req._tunnel.specsReq, 'should have been identified as a specs request'); + + req.url = '/tunnel/static/MojitX/specs/broccoli.json'; + middleware(req, null, function () { + nextCallCount += 1; + }); + + A.areSame(2, nextCallCount, 'next() should have been called'); + A.isObject(req._tunnel.specsReq, 'should have been identified as a specs request'); + }, + + 'test tunnel spec request is correctly parsed': function () { + req.url = '/static/MojitX/specs/broccoli.json'; + + middleware = factory(config); + middleware(req, null, function () { + nextCallCount += 1; + }); + + A.areSame(1, nextCallCount, 'next() should have been called'); + A.isObject(req._tunnel.specsReq, 'should have been identified as a specs request'); + A.areSame('MojitX', req._tunnel.specsReq.type, 'should have parsed out the mojit type correctly'); + A.areSame('broccoli', req._tunnel.specsReq.name, 'should have parsed out the file name correctly'); + }, + + 'test tunnel type request is correctly parsed': function () { + req.url = '/static/MojitY/definition.json'; + + middleware = factory(config); + middleware(req, null, function () { + nextCallCount += 1; + }); + + A.areSame(1, nextCallCount, 'next() should have been called'); + A.isObject(req._tunnel.typeReq, 'should have been identified as a type request'); + A.areSame('MojitY', req._tunnel.typeReq.type, 'should have parsed out the mojit type correctly'); + }, + + 'test tunnel rpc request is correctly parsed': function () { + middleware = factory(config); + middleware(req, null, function () { + nextCallCount += 1; + }); + + A.areSame(1, nextCallCount, 'next() should have been called'); + A.isObject(req._tunnel.rpcReq, 'should have been identified as an rpc request'); + + // Although 'wipes' is not a tunnel header, we should identify this + // request as a tunnel request because the tunnelPrefix matches. + req.headers['x-mojito-header'] = 'wipes'; + req.url = '/diapers'; + config.store.getAppConfig = function () { + return { + tunnelPrefix: '/diapers' + }; + }; + middleware(req, null, function () { + nextCallCount += 1; + }); + + A.areSame(2, nextCallCount, 'next() should have been called'); + A.isObject(req._tunnel.rpcReq, 'should have been identified as an rpc request'); + } + })); +}); diff --git a/tests/unit/lib/app/middleware/test-handler-tunnel-rpc.js b/tests/unit/lib/app/middleware/test-handler-tunnel-rpc.js new file mode 100644 index 000000000..94edbb176 --- /dev/null +++ b/tests/unit/lib/app/middleware/test-handler-tunnel-rpc.js @@ -0,0 +1,99 @@ +/* + * Copyright (c) 2011-2013, Yahoo! Inc. All rights reserved. + * Copyrights licensed under the New BSD License. + * See the accompanying LICENSE file for terms. + */ + +/*global YUI, require*/ +/*jslint nomen:true*/ + +YUI().use('mojito-test-extra', 'test', function (Y) { + 'use strict'; + + var A = Y.Assert, + AA = Y.ArrayAssert, + OA = Y.ObjectAssert, + + factory = require(Y.MOJITO_DIR + 'lib/app/middleware/mojito-handler-tunnel-rpc'), + expandedContext = null, + req, + nextCallCount, + middleware, + store, + config; + + + Y.Test.Runner.add(new Y.Test.Case({ + name: 'tunnel handler rpc tests', + + setUp: function () { + nextCallCount = 0; + + store = { + expandInstance: function (instance, context, callback) { + expandedContext = context; + callback(null, instance); + } + }; + + config = { + store: store + }; + + req = { + _tunnel: { + rpcReq: {} + }, + action: 'eatallyourspinach', + body: { + instance: {}, + context: { + runtime: 'client' + } + } + }; + }, + + tearDown: function () { + store = null; + config = null; + nextCallCount = null; + req = null; + expandedContext = null; + }, + + 'handler should exit early if not an rpc request': function () { + req._tunnel.rpcReq = null; + + middleware = factory(config); + middleware(req, null, function () { + nextCallCount += 1; + }); + + A.areSame(1, nextCallCount, 'next() handler should have been called'); + A.isNull(expandedContext, 'instance should not have been expanded'); + }, + + 'handler should override execution context to "server"': function () { + middleware = factory(config); + middleware(req, null, function () { + nextCallCount += 1; + }); + + A.areSame(1, nextCallCount, 'next() handler should have been called'); + A.areSame(expandedContext.runtime, 'server', 'instance should have server context'); + }, + + 'handler should set execution context to "server"': function () { + req.body.context.runtime = null; + + middleware = factory(config); + middleware(req, null, function () { + nextCallCount += 1; + }); + + A.areSame(1, nextCallCount, 'next() handler should have been called'); + A.areSame(expandedContext.runtime, 'server', 'instance should have server context'); + } + })); +}); diff --git a/tests/unit/lib/app/middleware/test-handler-tunnel.js b/tests/unit/lib/app/middleware/test-handler-tunnel.js index 97c156dec..d9a9fa052 100644 --- a/tests/unit/lib/app/middleware/test-handler-tunnel.js +++ b/tests/unit/lib/app/middleware/test-handler-tunnel.js @@ -3,307 +3,55 @@ * Copyrights licensed under the New BSD License. * See the accompanying LICENSE file for terms. */ -YUI().use('mojito-test-extra', 'test', function(Y) { - var A = Y.Assert, - AA = Y.ArrayAssert, - OA = Y.ObjectAssert, - cases = {}, - - factory = require(Y.MOJITO_DIR + 'lib/app/middleware/mojito-handler-tunnel'), - expandedContext; - - cases = { - name: 'tunnel handler tests', - - _handler: null, - - setUp: function() { - var store = { - getAppConfig: function() { return { obj: 'appConfig' }; }, - getSpec: function(env, id, ctx, cb) { - cb(null, { - env: env, - id: id, - ctx: ctx - }); - }, - getType: function(env, type, ctx, cb) { - cb(null, { - env: env, - type: type, - ctx: ctx - }); - }, - expandInstance: function(instance, context, callback) { - expandedContext = context; - callback(null, instance); - }, - expandInstanceForEnv: function(env, instance, context, callback) { - expandedContext = context; - callback(null, instance); - } - }, - globalLogger = null; - this._handler = factory({ - context: {}, - store: store, - logger: globalLogger - }); - - expandedContext = null; - }, - - 'handler calls next() when tunnel url or HTTP header not present': function() { - var callCount = 0, - req = { - url: '/notunnel', - headers: {} - }; +/*global YUI, require*/ +/*jslint nomen:true*/ - this._handler(req, null, function() { - callCount++; - }); - - A.areEqual(1, callCount, 'next() handler should have been called'); - }, +YUI().use('mojito-test-extra', 'test', function (Y) { + 'use strict'; - 'test trailing slashes get removed from tunnels uris': function() { - var store = { - getAppConfig: function() { - return { - obj: 'appConfig', - tunnelPrefix: '/mytunnelprefix/' - }; - }, - getSpec: function(env, id, ctx, cb) { - cb(null, { - env: env, - id: id, - ctx: ctx - }); - }, - getType: function(env, type, ctx, cb) { - cb(null, { - env: env, - type: type, - ctx: ctx - }); - }, - expandInstance: function(instance, context, callback) { - expandedContext = context; - callback(null, instance); - }, - expandInstanceForEnv: function(env, instance, context, callback) { - expandedContext = context; - callback(null, instance); - } - }, - globalLogger = null, - callCount = 0, - req = { - url: '/notunnel', - headers: {} - }; - - this._handler = factory({ - context: {}, - store: store, - logger: globalLogger - }); - - this._handler(req, null, function() { - callCount++; - }); - - A.areEqual(1, callCount, 'next() handler should have been called'); - }, - - - 'handler should override execution context to server (with /tunnel prefix)': function() { - var nextCalls = 0, writeCalls = 0, endCalls = 0, - req = { - url: '/tunnel', - method: 'POST', - body: { - context: { - runtime: 'client', - myKey: 'myValue' - } - }, - headers: {} - }, - res = { - }; - - this._handler(req, res, function() { - nextCalls++; - }); - - A.isObject(expandedContext, 'Expanded context should be an object'); - A.areEqual('myValue', expandedContext.myKey, 'custom context property should have been preserved'); - A.areEqual('server', expandedContext.runtime, 'context.runtime should have been set to "server"'); - - A.areEqual(1, nextCalls, 'next() handler should have been called'); - A.areEqual(0, writeCalls, 'res.writeHead() should have been called'); - A.areEqual(0, endCalls, 'res.end() should have been called'); - }, - - 'handler should set execution context to server (with /tunnel prefix)': function() { - var nextCalls = 0, writeCalls = 0, endCalls = 0, - req = { - url: '/tunnel', - method: 'POST', - body: { - reqs: [{ - data: {} - }] - }, - headers: {} - }, - res = { - }; - - this._handler(req, res, function() { - nextCalls++; - }); - - A.isObject(expandedContext, 'Expanded context should be an object'); - A.areEqual('server', expandedContext.runtime, 'context.runtime should have been set to "server"'); - - A.areEqual(1, nextCalls, 'next() handler should have been called'); - A.areEqual(0, writeCalls, 'res.writeHead() should have been called'); - A.areEqual(0, endCalls, 'res.end() should have been called'); - }, - - 'handles specs (with /tunnel prefix)': function() { - var nextCalls = 0, writeCalls = 0, endCalls = 0; - req = { - url: '/tunnel/static/MojitA/specs/orange.json', - headers: { 'x-mojito-header': 'tunnel' } - }, - res = { - writeHead: function(code, headers) { - writeCalls++; - A.areEqual('200', code, 'should have gotten 200'); - A.areEqual('application/json; charset="utf-8"', headers['content-type'], 'should have gotten application/json, utf-8'); - }, - end: function(data) { - var expected = { - "base": "MojitA:orange" - }; - endCalls++; - A.areEqual(Y.JSON.stringify(expected,null,4), data, 'should have gotten spec'); - } - - }; - - this._handler(req, res, function() { - nextCalls++; - }); - - A.areEqual(0, nextCalls, 'next() handler should not have been called'); - A.areEqual(1, writeCalls, 'res.writeHead() should have been called'); - A.areEqual(1, endCalls, 'res.end() should have been called'); - }, + var A = Y.Assert, + AA = Y.ArrayAssert, + OA = Y.ObjectAssert, - 'handles specs (no /tunnel prefix)': function() { - var nextCalls = 0, writeCalls = 0, endCalls = 0; - req = { - url: '/static/MojitA/specs/orange.json', - headers: { 'x-mojito-header': 'tunnel' } - }, - res = { - writeHead: function(code, headers) { - writeCalls++; - A.areEqual('200', code, 'should have gotten 200'); - A.areEqual('application/json; charset="utf-8"', headers['content-type'], 'should have gotten application/json, utf-8'); - }, - end: function(data) { - var expected = { - "base": "MojitA:orange" - }; - endCalls++; - A.areEqual(Y.JSON.stringify(expected,null,4), data, 'should have gotten spec'); - } + factory = require(Y.MOJITO_DIR + 'lib/app/middleware/mojito-handler-tunnel'), + req, + nextCallCount, + middleware, + store, + config; - }; - this._handler(req, res, function() { - nextCalls++; - }); + Y.Test.Runner.add(new Y.Test.Case({ + name: 'tunnel handler tunnel tests', - A.areEqual(0, nextCalls, 'next() handler should not have been called'); - A.areEqual(1, writeCalls, 'res.writeHead() should have been called'); - A.areEqual(1, endCalls, 'res.end() should have been called'); + setUp: function () { + nextCallCount = 0; + config = { + store: { + getAppConfig: function () {} + } + }; + req = { + url: '/nodessertunlessyoueatallyourveggies', + headers: {} + }; }, - 'handles type (with /tunnel prefix)': function() { - var nextCalls = 0, writeCalls = 0, endCalls = 0; - req = { - url: '/tunnel/static/MojitA/definition.json?x=y', - headers: { 'x-mojito-header': 'tunnel' } - }, - res = { - writeHead: function(code, headers) { - writeCalls++; - A.areEqual('200', code, 'should have gotten 200'); - A.areEqual('application/json; charset="utf-8"', headers['content-type'], 'should have gotten application/json, utf-8'); - }, - end: function(data) { - var expected = { - "type": "MojitA" - }; - endCalls++; - A.areEqual(Y.JSON.stringify(expected,null,4), data, 'should have gotten spec'); - } - - }; - - this._handler(req, res, function() { - nextCalls++; - }); - - A.areEqual(0, nextCalls, 'next() handler should not have been called'); - A.areEqual(1, writeCalls, 'res.writeHead() should have been called'); - A.areEqual(1, endCalls, 'res.end() should have been called'); + tearDown: function () { + nextCallCount = null; + config = null; + req = null; }, - 'handles type (no /tunnel prefix)': function() { - var nextCalls = 0, writeCalls = 0, endCalls = 0; - req = { - url: '/static/MojitA/definition.json', - headers: { 'x-mojito-header': 'tunnel' } - }, - res = { - writeHead: function(code, headers) { - writeCalls++; - A.areEqual('200', code, 'should have gotten 200'); - A.areEqual('application/json; charset="utf-8"', headers['content-type'], 'should have gotten application/json, utf-8'); - }, - end: function(data) { - var expected = { - "type": "MojitA" - }; - endCalls++; - A.areEqual(Y.JSON.stringify(expected,null,4), data, 'should have gotten spec'); - } - - }; - - this._handler(req, res, function() { - nextCalls++; + 'handler should run all tunnel middleware if not a tunnel request': function () { + middleware = factory(config); + middleware(req, null, function () { + nextCallCount += 1; }); - A.areEqual(0, nextCalls, 'next() handler should not have been called'); - A.areEqual(1, writeCalls, 'res.writeHead() should have been called'); - A.areEqual(1, endCalls, 'res.end() should have been called'); - }, - - 'ignore:': function () { - + A.areSame(1, nextCallCount, 'next() handler should have been called'); + A.isNull(req._tunnel, 'should have cleaned up private variable'); } - }; - - Y.Test.Runner.add(new Y.Test.Case(cases)); + })); }); From 77f731ed3f12dae802767a7b074f58350c669cac Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Mon, 25 Mar 2013 19:38:14 -0700 Subject: [PATCH 15/38] simplify response logic with express response sugar With unit tests! --- .../middleware/mojito-handler-tunnel-specs.js | 7 +- .../middleware/mojito-handler-tunnel-type.js | 7 +- .../middleware_test_descriptor.json | 16 ++ .../middleware/test-handler-tunnel-specs.js | 148 ++++++++++++++++++ .../middleware/test-handler-tunnel-type.js | 134 ++++++++++++++++ 5 files changed, 300 insertions(+), 12 deletions(-) create mode 100644 tests/unit/lib/app/middleware/test-handler-tunnel-specs.js create mode 100644 tests/unit/lib/app/middleware/test-handler-tunnel-type.js diff --git a/lib/app/middleware/mojito-handler-tunnel-specs.js b/lib/app/middleware/mojito-handler-tunnel-specs.js index 7f8a96d62..9efe5fbca 100644 --- a/lib/app/middleware/mojito-handler-tunnel-specs.js +++ b/lib/app/middleware/mojito-handler-tunnel-specs.js @@ -51,12 +51,7 @@ module.exports = function (config) { new Error('Error opening: ' + req.url + '\n' + err) ); } - // TODO: Use the express sugar method res.json([status], - // [body]) after we rewrite the existing tests. - res.writeHead(res.statusCode || 200, { - 'content-type': 'application/json; charset="utf-8"' - }); - res.end(JSON.stringify(data, null, 4)); + res.send(200, data); } ); }; diff --git a/lib/app/middleware/mojito-handler-tunnel-type.js b/lib/app/middleware/mojito-handler-tunnel-type.js index 21f49d58d..bc1a3bd67 100644 --- a/lib/app/middleware/mojito-handler-tunnel-type.js +++ b/lib/app/middleware/mojito-handler-tunnel-type.js @@ -42,12 +42,7 @@ module.exports = function (config) { new Error('Error opening: ' + req.url + '\n' + err) ); } - // TODO: Use the express sugar method res.json([status], - // [body]) after we rewrite the existing tests. - res.writeHead(res.statusCode || 200, { - 'content-type': 'application/json; charset="utf-8"' - }); - res.end(JSON.stringify(data, null, 4)); + res.send(200, data); } ); }; diff --git a/tests/unit/lib/app/middleware/middleware_test_descriptor.json b/tests/unit/lib/app/middleware/middleware_test_descriptor.json index b802c0b50..1c2e327a9 100644 --- a/tests/unit/lib/app/middleware/middleware_test_descriptor.json +++ b/tests/unit/lib/app/middleware/middleware_test_descriptor.json @@ -48,6 +48,22 @@ }, "group": "fw,unit,server" }, + "handler-tunnel-specs": { + "params": { + "lib": "$$config.lib$$/app/middleware/mojito-handler-tunnel-specs.js", + "test": "./test-handler-tunnel-specs.js", + "driver": "nodejs" + }, + "group": "fw,unit,server" + }, + "handler-tunnel-type": { + "params": { + "lib": "$$config.lib$$/app/middleware/mojito-handler-tunnel-type.js", + "test": "./test-handler-tunnel-type.js", + "driver": "nodejs" + }, + "group": "fw,unit,server" + }, "router": { "params": { "lib": "$$config.lib$$/app/middleware/mojito-handler-tunnel.js,../../../../../lib/app/autoload/route-maker.common.js,../../../../../lib/app/autoload/util.common.js", diff --git a/tests/unit/lib/app/middleware/test-handler-tunnel-specs.js b/tests/unit/lib/app/middleware/test-handler-tunnel-specs.js new file mode 100644 index 000000000..7fa33cebd --- /dev/null +++ b/tests/unit/lib/app/middleware/test-handler-tunnel-specs.js @@ -0,0 +1,148 @@ +/* + * Copyright (c) 2011-2013, Yahoo! Inc. All rights reserved. + * Copyrights licensed under the New BSD License. + * See the accompanying LICENSE file for terms. + */ + +/*global YUI, require*/ +/*jslint nomen:true*/ + +YUI().use('mojito-test-extra', 'test', function (Y) { + 'use strict'; + + var A = Y.Assert, + AA = Y.ArrayAssert, + OA = Y.ObjectAssert, + + factory = require(Y.MOJITO_DIR + 'lib/app/middleware/mojito-handler-tunnel-specs'), + expandInstanceInvoked = false, + error, + req, + res, + sendData, + statusCode, + nextCallCount, + middleware, + store, + config; + + + Y.Test.Runner.add(new Y.Test.Case({ + name: 'tunnel handler specs tests', + + setUp: function () { + nextCallCount = 0; + + store = { + expandInstanceForEnv: function (env, instance, context, callback) { + expandInstanceInvoked = true; + } + }; + + config = { + store: store + }; + + req = { + url: '/tunnel', + _tunnel: { + specsReq: { + type: 'MojitX', + name: 'default' + } + } + }; + + res = { + send: function (code, data) { + sendData = data; + statusCode = code; + } + }; + }, + + tearDown: function () { + expandInstanceInvoked = false; + + nextCallCount = 0; + sendData = undefined; + statusCode = undefined; + store = null; + config = null; + req = null; + res = null; + middleware = null; + error = undefined; + }, + + 'handler should exit early if not specs request': function () { + req._tunnel.specsReq = null; + middleware = factory(config); + middleware(req, res, function () { + nextCallCount += 1; + }); + + A.areSame(1, nextCallCount, 'next() handler should have been called'); + A.isFalse(expandInstanceInvoked, 'should not have attempted to expand the instance'); + }, + + 'handler should error if "type" is missing': function () { + req._tunnel.specsReq.type = null; + middleware = factory(config); + middleware(req, res, function (err) { + error = err; + nextCallCount += 1; + }); + + A.areSame(1, nextCallCount, 'next() handler should have been called'); + A.isNotUndefined(error, 'next() handler should have received an error'); + A.areSame(404, res.statusCode, 'status code should have been set to 404'); + }, + + 'handler should error if "name" is missing': function () { + req._tunnel.specsReq.name = null; + middleware = factory(config); + middleware(req, res, function (err) { + error = err; + nextCallCount += 1; + }); + + A.areSame(1, nextCallCount, 'next() handler should have been called'); + A.isNotUndefined(error, 'next() handler should have received an error'); + A.areSame(404, res.statusCode, 'status code should have been set to 404'); + }, + + 'handler should error if expandInstanceForEnv errors': function () { + config.store.expandInstanceForEnv = function (env, instance, context, callback) { + callback(new Error('you have 10 seconds to eat that tomato')); + }; + middleware = factory(config); + middleware(req, res, function (err) { + error = err; + nextCallCount += 1; + }); + + A.areSame(1, nextCallCount, 'next() handler should have been called'); + A.areSame(500, res.statusCode, 'status code should have been set to 500'); + A.isNotUndefined(error, 'next() handler should have received an error'); + A.isUndefined(sendData, 'data should not have been sent'); + }, + + 'test handler response for success': function () { + var data = 'good job, here is your dessert!'; + config.store.expandInstanceForEnv = function (env, instance, context, callback) { + callback(null, data); + }; + middleware = factory(config); + middleware(req, res, function (err) { + error = err; + nextCallCount += 1; + }); + + A.areSame(0, nextCallCount, 'next() handler should not have been called'); + A.areSame(200, statusCode, 'status code should have been set to 200'); + A.isUndefined(error, 'next() handler should have received an error'); + A.areSame(data, sendData, 'data should have been sent'); + } + })); +}); diff --git a/tests/unit/lib/app/middleware/test-handler-tunnel-type.js b/tests/unit/lib/app/middleware/test-handler-tunnel-type.js new file mode 100644 index 000000000..af28991d8 --- /dev/null +++ b/tests/unit/lib/app/middleware/test-handler-tunnel-type.js @@ -0,0 +1,134 @@ +/* + * Copyright (c) 2011-2013, Yahoo! Inc. All rights reserved. + * Copyrights licensed under the New BSD License. + * See the accompanying LICENSE file for terms. + */ + +/*global YUI, require*/ +/*jslint nomen:true*/ + +YUI().use('mojito-test-extra', 'test', function (Y) { + 'use strict'; + + var A = Y.Assert, + AA = Y.ArrayAssert, + OA = Y.ObjectAssert, + + factory = require(Y.MOJITO_DIR + 'lib/app/middleware/mojito-handler-tunnel-type'), + expandInstanceInvoked = false, + error, + req, + res, + sendData, + statusCode, + nextCallCount, + middleware, + store, + config; + + + Y.Test.Runner.add(new Y.Test.Case({ + name: 'tunnel handler type tests', + + setUp: function () { + nextCallCount = 0; + + store = { + expandInstanceForEnv: function (env, instance, context, callback) { + expandInstanceInvoked = true; + } + }; + + config = { + store: store + }; + + req = { + url: '/tunnel', + _tunnel: { + typeReq: { + type: 'MojitX' + } + } + }; + + res = { + send: function (code, data) { + sendData = data; + statusCode = code; + } + }; + }, + + tearDown: function () { + expandInstanceInvoked = false; + + nextCallCount = 0; + sendData = undefined; + statusCode = undefined; + store = null; + config = null; + req = null; + res = null; + middleware = null; + error = undefined; + }, + + 'handler should exit early if not type request': function () { + req._tunnel.typeReq = null; + middleware = factory(config); + middleware(req, res, function () { + nextCallCount += 1; + }); + + A.areSame(1, nextCallCount, 'next() handler should have been called'); + A.isFalse(expandInstanceInvoked, 'should not have attempted to expand the instance'); + }, + + 'handler should error if "type" is missing': function () { + req._tunnel.typeReq.type = null; + middleware = factory(config); + middleware(req, res, function (err) { + error = err; + nextCallCount += 1; + }); + + A.areSame(1, nextCallCount, 'next() handler should have been called'); + A.isNotUndefined(error, 'next() handler should have received an error'); + A.areSame(404, res.statusCode, 'status code should have been set to 404'); + }, + + 'handler should error if expandInstanceForEnv errors': function () { + config.store.expandInstanceForEnv = function (env, instance, context, callback) { + callback(new Error('you have 10 seconds to eat that tomato')); + }; + middleware = factory(config); + middleware(req, res, function (err) { + error = err; + nextCallCount += 1; + }); + + A.areSame(1, nextCallCount, 'next() handler should have been called'); + A.areSame(500, res.statusCode, 'status code should have been set to 500'); + A.isNotUndefined(error, 'next() handler should have received an error'); + A.isUndefined(sendData, 'data should not have been sent'); + }, + + 'test handler response for success': function () { + var data = 'good job, here is your dessert!'; + config.store.expandInstanceForEnv = function (env, instance, context, callback) { + callback(null, data); + }; + middleware = factory(config); + middleware(req, res, function (err) { + error = err; + nextCallCount += 1; + }); + + A.areSame(0, nextCallCount, 'next() handler should not have been called'); + A.areSame(200, statusCode, 'status code should have been set to 200'); + A.isUndefined(error, 'next() handler should have received an error'); + A.areSame(data, sendData, 'data should have been sent'); + } + })); +}); From a5d5e64baa2c54e09b7aa819aa6e4417d3806df2 Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Mon, 25 Mar 2013 23:20:44 -0700 Subject: [PATCH 16/38] enable per-request tunnel url configuration The configurable `tunnelUrl` value will still be validated against `tunnelPrefix`, which can be configured in application.json and defaults to `/tunnel`. --- lib/app/autoload/mojit-proxy.client.js | 4 +++ lib/app/autoload/tunnel-client.common.js | 5 ++++ .../mojito-handler-tunnel-parser.js | 10 +++---- .../app/autoload/test-mojit-proxy.client.js | 28 +++++++++++++++++++ .../lib/app/autoload/test-tunnel.common.js | 16 +++++++++++ .../middleware/test-handler-tunnel-parser.js | 14 ++++++++++ 6 files changed, 72 insertions(+), 5 deletions(-) diff --git a/lib/app/autoload/mojit-proxy.client.js b/lib/app/autoload/mojit-proxy.client.js index 7497445a8..8e25b1e80 100644 --- a/lib/app/autoload/mojit-proxy.client.js +++ b/lib/app/autoload/mojit-proxy.client.js @@ -196,6 +196,10 @@ YUI.add('mojito-mojit-proxy', function(Y, NAME) { rpc: options.rpc || false }; + if (options.tunnelUrl) { + command._tunnelUrl = options.tunnelUrl; + } + this._client.executeAction(command, this.getId(), callback); }, diff --git a/lib/app/autoload/tunnel-client.common.js b/lib/app/autoload/tunnel-client.common.js index 954615220..2fc25ce61 100644 --- a/lib/app/autoload/tunnel-client.common.js +++ b/lib/app/autoload/tunnel-client.common.js @@ -25,6 +25,11 @@ YUI.add('mojito-tunnel-client', function(Y, NAME) { url = this._appConfig.tunnelPrefix; + if (command._tunnelUrl) { + url = command._tunnelUrl; + command._tunnelUrl = undefined; + } + cfg = { method: 'POST', data: Y.JSON.stringify(command), diff --git a/lib/app/middleware/mojito-handler-tunnel-parser.js b/lib/app/middleware/mojito-handler-tunnel-parser.js index 86ae9db95..7fde47c9e 100644 --- a/lib/app/middleware/mojito-handler-tunnel-parser.js +++ b/lib/app/middleware/mojito-handler-tunnel-parser.js @@ -74,15 +74,15 @@ module.exports = function (config) { path = path.replace(staticPrefix, '') .replace(tunnelPrefix, ''); + parts = path.split('/'); + req._tunnel = {}; - if (path) { + // If there was a '/' in the path. + if (parts.length > 1) { // Get the basename without the .json extension. name = libpath.basename(path, '.json'); - // Time to get dirty. - parts = path.split('/'); - // Get the mojit type. type = parts[1]; @@ -101,7 +101,7 @@ module.exports = function (config) { } } // "RPC" tunnel request - else if (req.url === tunnelPrefix && req.method === 'POST') { + else if (hasTunnelPrefix && req.method === 'POST') { req._tunnel.rpcReq = {}; } diff --git a/tests/unit/lib/app/autoload/test-mojit-proxy.client.js b/tests/unit/lib/app/autoload/test-mojit-proxy.client.js index 53887451a..930b0bc22 100644 --- a/tests/unit/lib/app/autoload/test-mojit-proxy.client.js +++ b/tests/unit/lib/app/autoload/test-mojit-proxy.client.js @@ -164,6 +164,34 @@ YUI({useBrowserConsole: true}).use( Y.Mock.verify(mojitProxy._client); }, + "test invoke with tunnelUrl option": function () { + var mojitProxy = this.mojitProxy, + mojitProxyConfig = this.mojitProxyConfig, + tunnelUrl = '/tunnel;_ylt=A0oGdV8GMC1RcBgAQNhXNyoA'; + + mojitProxy._client = Y.Mock(); + mojitProxy.query = {}; // Avoid window calls + + Y.Mock.expect(mojitProxy._client, { + method: 'executeAction', + args: [Y.Mock.Value.Object, Y.Mock.Value.String, Y.Mock.Value.Function], + run: function (command, id, cb) { + Y.Assert.areSame(tunnelUrl, command._tunnelUrl); + } + }); + + mojitProxy.invoke('index', { + params: { + body: { + testKey: 'testVal' + } + }, + tunnelUrl: tunnelUrl, + rpc: true + }); + Y.Mock.verify(mojitProxy._client); + }, + "test refreshView": function () { var mojitProxy = this.mojitProxy; mojitProxy._client = Y.Mock(); diff --git a/tests/unit/lib/app/autoload/test-tunnel.common.js b/tests/unit/lib/app/autoload/test-tunnel.common.js index 5fdfc06b5..f76a90cd3 100644 --- a/tests/unit/lib/app/autoload/test-tunnel.common.js +++ b/tests/unit/lib/app/autoload/test-tunnel.common.js @@ -35,6 +35,22 @@ YUI({useBrowserConsole: true}).use( Y.Assert.areEqual(this.appConfig, tunnelClient._appConfig); }, + "test tunnelUrl override": function () { + var appConfig = this.appConfig, + tunnelClient = this.tunnelClient, + tunnelUrl = '/tunnel;_ylt=A0oGdV8GMC1RcBgAQNhXNyoA', + command = { + _tunnelUrl: tunnelUrl + }; + + tunnelClient._makeRequest = function (url) { + Y.Assert.isString(url); + Y.Assert.areEqual(tunnelUrl, url); + }; + + tunnelClient.rpc(command); + }, + "test rpc success": function () { var appConfig = this.appConfig, tunnelClient = this.tunnelClient, diff --git a/tests/unit/lib/app/middleware/test-handler-tunnel-parser.js b/tests/unit/lib/app/middleware/test-handler-tunnel-parser.js index 6db1ff41a..a137cbb81 100644 --- a/tests/unit/lib/app/middleware/test-handler-tunnel-parser.js +++ b/tests/unit/lib/app/middleware/test-handler-tunnel-parser.js @@ -170,6 +170,20 @@ YUI().use('mojito-test-extra', 'test', function (Y) { A.areSame('MojitY', req._tunnel.typeReq.type, 'should have parsed out the mojit type correctly'); }, + 'test compatibility with tunnelUrl option': function () { + req.url = '/tunnel;_ylt=A0oGdV8GMC1RcBgAQNhXNyoA;_ylu=X3oDMTE5aWhtbjdhBHNlYwNvdi10b3AEY29sbwNzazEEdnRpZANTTUUwNDFfMTU0BHBvcwMx'; + req.headers['x-mojito-header'] = 'huggies'; + + middleware = factory(config); + middleware(req, null, function () { + nextCallCount += 1; + }); + + A.areSame(1, nextCallCount, 'next() handler should have been called'); + A.isObject(req._tunnel.rpcReq, 'should have been identified as an rpc request'); + }, + + 'test tunnel rpc request is correctly parsed': function () { middleware = factory(config); middleware(req, null, function () { From 7455255425f74524adf02ea4e63daf546e7f6fd9 Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Mon, 25 Mar 2013 23:52:31 -0700 Subject: [PATCH 17/38] add history line items related to middleware changes for 0.5.7 --- HISTORY.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/HISTORY.md b/HISTORY.md index 0668d08b3..ae7e253d8 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -3,6 +3,10 @@ version @VERSION@ Notes ------------ +* A middleware called `mojito-handler-error` has been added to handle + middleware errors. If you have redefined the middleware stack and do not have + your own error handler, then it is your responsibility to add it so that + errors can be handled appropriately. Deprecations ------------ @@ -10,6 +14,14 @@ Deprecations Features ------------ * Upgraded to YUI 3.9.0 +* [issue #979](/yahoo/mojito/issues/979): + * The `mojito-handler-tunnel` middleware was refactored into a middleware + substack that more loosens the coupling between the parsing and handling + phases of a tunnel request. This means that applications will have an + easier time overriding and customizing tunnel behavior. + * The URL is now customizable per request using the `tunnelUrl` option for + `mojitProxy.invoke()`, but is still subject to the `tunnelPrefix` + restriction. Bug Fixes ------------ From 42385544f14c685310766a78db1f7eb0f11a98d9 Mon Sep 17 00:00:00 2001 From: Caridy Patino Date: Wed, 27 Mar 2013 16:40:44 -0400 Subject: [PATCH 18/38] upgrading to 3.9.1 to solve the issue with handlebars --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e9743f09b..002478b11 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "semver": "1.0.14", "wrench": "~1.3.9", "ycb": "~1.0.0", - "yui": "~3.9.0", + "yui": "~3.9.1", "yuidocjs": "~0.3.14", "yuitest": "~0.7.4", "yuitest-coverage": "~0.0.6" From ebf6a0c80385c195feca0e988393b0dd7a69aa8f Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Wed, 27 Mar 2013 18:33:30 -0700 Subject: [PATCH 19/38] fix functional tests --- lib/app/middleware/mojito-handler-tunnel-specs.js | 5 ++++- lib/app/middleware/mojito-handler-tunnel-type.js | 5 ++++- .../lib/app/middleware/test-handler-tunnel-specs.js | 13 +++++-------- .../lib/app/middleware/test-handler-tunnel-type.js | 11 ++++------- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/app/middleware/mojito-handler-tunnel-specs.js b/lib/app/middleware/mojito-handler-tunnel-specs.js index 9efe5fbca..bec15ffb4 100644 --- a/lib/app/middleware/mojito-handler-tunnel-specs.js +++ b/lib/app/middleware/mojito-handler-tunnel-specs.js @@ -51,7 +51,10 @@ module.exports = function (config) { new Error('Error opening: ' + req.url + '\n' + err) ); } - res.send(200, data); + res.writeHead(200, { + 'content-type': 'application/json' + }); + res.end(JSON.stringify(data)); } ); }; diff --git a/lib/app/middleware/mojito-handler-tunnel-type.js b/lib/app/middleware/mojito-handler-tunnel-type.js index bc1a3bd67..9c46690a7 100644 --- a/lib/app/middleware/mojito-handler-tunnel-type.js +++ b/lib/app/middleware/mojito-handler-tunnel-type.js @@ -42,7 +42,10 @@ module.exports = function (config) { new Error('Error opening: ' + req.url + '\n' + err) ); } - res.send(200, data); + res.writeHead(200, { + 'content-type': 'application/json' + }); + res.end(JSON.stringify(data)); } ); }; diff --git a/tests/unit/lib/app/middleware/test-handler-tunnel-specs.js b/tests/unit/lib/app/middleware/test-handler-tunnel-specs.js index 7fa33cebd..730963c7e 100644 --- a/tests/unit/lib/app/middleware/test-handler-tunnel-specs.js +++ b/tests/unit/lib/app/middleware/test-handler-tunnel-specs.js @@ -20,7 +20,6 @@ YUI().use('mojito-test-extra', 'test', function (Y) { req, res, sendData, - statusCode, nextCallCount, middleware, store, @@ -54,9 +53,9 @@ YUI().use('mojito-test-extra', 'test', function (Y) { }; res = { - send: function (code, data) { - sendData = data; - statusCode = code; + writeHead: function () {}, + end: function (data) { + sendData = data; } }; }, @@ -66,7 +65,6 @@ YUI().use('mojito-test-extra', 'test', function (Y) { nextCallCount = 0; sendData = undefined; - statusCode = undefined; store = null; config = null; req = null; @@ -140,9 +138,8 @@ YUI().use('mojito-test-extra', 'test', function (Y) { }); A.areSame(0, nextCallCount, 'next() handler should not have been called'); - A.areSame(200, statusCode, 'status code should have been set to 200'); - A.isUndefined(error, 'next() handler should have received an error'); - A.areSame(data, sendData, 'data should have been sent'); + A.isUndefined(error, 'next() handler should not have received an error'); + A.areEqual(JSON.stringify(data), sendData, 'data should have been sent'); } })); }); diff --git a/tests/unit/lib/app/middleware/test-handler-tunnel-type.js b/tests/unit/lib/app/middleware/test-handler-tunnel-type.js index af28991d8..e06a393ca 100644 --- a/tests/unit/lib/app/middleware/test-handler-tunnel-type.js +++ b/tests/unit/lib/app/middleware/test-handler-tunnel-type.js @@ -20,7 +20,6 @@ YUI().use('mojito-test-extra', 'test', function (Y) { req, res, sendData, - statusCode, nextCallCount, middleware, store, @@ -53,9 +52,9 @@ YUI().use('mojito-test-extra', 'test', function (Y) { }; res = { - send: function (code, data) { - sendData = data; - statusCode = code; + writeHead: function () {}, + end: function (data) { + sendData = data; } }; }, @@ -65,7 +64,6 @@ YUI().use('mojito-test-extra', 'test', function (Y) { nextCallCount = 0; sendData = undefined; - statusCode = undefined; store = null; config = null; req = null; @@ -126,9 +124,8 @@ YUI().use('mojito-test-extra', 'test', function (Y) { }); A.areSame(0, nextCallCount, 'next() handler should not have been called'); - A.areSame(200, statusCode, 'status code should have been set to 200'); A.isUndefined(error, 'next() handler should have received an error'); - A.areSame(data, sendData, 'data should have been sent'); + A.areEqual(JSON.stringify(data), sendData, 'data should have been sent'); } })); }); From a5d7dcf9bf05b0c3836a4c015cccafffc3bfda7c Mon Sep 17 00:00:00 2001 From: Isao Yagi Date: Thu, 28 Mar 2013 14:14:07 -0700 Subject: [PATCH 20/38] History edits for 0.5.7 --- HISTORY.md | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index ae7e253d8..50163de79 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,5 +1,5 @@ -version @VERSION@ -================= +version 0.5.7 +============= Notes ------------ @@ -8,12 +8,15 @@ Notes your own error handler, then it is your responsibility to add it so that errors can be handled appropriately. -Deprecations ------------- +* An early preview of [`mojito-cli`](https://github.com/yahoo/mojito-cli) has been published. Users can choose to try it with `npm install --global mojito-cli`. There should be no significant changes in functionality. It is intended to replace the functionality provided by installing the mojito npm package globally (which has been deprecated). Notes: + * users install mojito-cli package globally (if they choose to in this preview release period). + * users should install the mojito package _locally_, as an npm dependency of their application. + * all existing mojito command line commands should continue to operate in much the same way. + * `mojito create app Foo`, when mojito-cli has been installed, will use `npm` to install `mojito` locally automatically after generating the app files and directories. Features ------------ -* Upgraded to YUI 3.9.0 +* Upgraded to YUI 3.9.1 * [issue #979](/yahoo/mojito/issues/979): * The `mojito-handler-tunnel` middleware was refactored into a middleware substack that more loosens the coupling between the parsing and handling @@ -23,8 +26,10 @@ Features `mojitProxy.invoke()`, but is still subject to the `tunnelPrefix` restriction. + Bug Fixes ------------ +* issue bz6160815: port argument must be an integer version 0.5.6 ================= From 04d33e1ff5c46b7cbf2ef63711821925cbfa898a Mon Sep 17 00:00:00 2001 From: Isao Yagi Date: Thu, 28 Mar 2013 14:17:43 -0700 Subject: [PATCH 21/38] prepend blank @VERSION@ boilerplate --- HISTORY.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/HISTORY.md b/HISTORY.md index 50163de79..337c3a329 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,3 +1,19 @@ +version @VERSION@ +================= + +Notes +----- + +Deprecations +------------ + +Features +-------- + +Bug Fixes +--------- + + version 0.5.7 ============= From ad2d8a81198e0caf355e8131f8c687c74a200017 Mon Sep 17 00:00:00 2001 From: Isao Yagi Date: Thu, 28 Mar 2013 14:27:52 -0700 Subject: [PATCH 22/38] release 0.5.7 (based on 0.5.6-39-ge3156ad) --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 002478b11..addb57e23 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "mojito", - "version": "0.5.6", + "version": "0.5.7", "description": "Mojito provides an architecture, components and tools for developers to build complex web applications faster.", "author": "Drew Folta ", "contributors": [ From c43503c50d5633e1ab375706c0b47a4033b42ed0 Mon Sep 17 00:00:00 2001 From: Joe Catera Date: Mon, 1 Apr 2013 16:10:01 -0700 Subject: [PATCH 23/38] Rewriting a paragraph given in a PR for noting that Node.js handles what happens to log messages after receiving them from Mojito. --- docs/dev_guide/topics/mojito_logging.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/dev_guide/topics/mojito_logging.rst b/docs/dev_guide/topics/mojito_logging.rst index 1c14bb9aa..d82074330 100644 --- a/docs/dev_guide/topics/mojito_logging.rst +++ b/docs/dev_guide/topics/mojito_logging.rst @@ -7,10 +7,10 @@ log messages are handled by a YUI instance that Mojito creates based on YUI conf defined in ``application.json`` or ``application.yaml``. You can set logging levels to control the degree of detail in your log reports. -Mojito does not write logs into a file instead, it writes into the node.js console. Whatever -node.js does with the logs, writing them into a file, transmiting them into an aggregated -hub for multiple cores, or whatever other crazy idea people decide to implement has very -little to do with Mojito. +Mojito does not write logs to a file. Instead, Mojito writes logs to the Node.js console. +Thus, once logs are passed to Node.js, Mojito has no control over whether Node.js writes +logs to a file, transmits them into an aggregated hub for multiple cores, or uses another +implementation for logging. .. _mojito_logging-levels: From 94a47a68b689cb48a53fe5a5784d14420ad4be0c Mon Sep 17 00:00:00 2001 From: Isao Yagi Date: Mon, 1 Apr 2013 16:47:31 -0700 Subject: [PATCH 24/38] rm redundant preload() in failing test --- tests/unit/lib/app/autoload/test-store.server.js | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unit/lib/app/autoload/test-store.server.js b/tests/unit/lib/app/autoload/test-store.server.js index 12e537315..c08c6fa6e 100644 --- a/tests/unit/lib/app/autoload/test-store.server.js +++ b/tests/unit/lib/app/autoload/test-store.server.js @@ -498,7 +498,6 @@ YUI().use( 'load node_modules': function() { var fixtures = libpath.join(__dirname, '../../../../fixtures/packages'), store = new Y.mojito.ResourceStore({ root: fixtures }); - store.preload(); if (!store._mojitRVs.a && !store._mojitRVs.aa && !store._mojitRVs.ba) { // This happens when mojito is installed via npm, since npm From 47218d0aeb4d8b519a75a62a9b42ae9d19bb790a Mon Sep 17 00:00:00 2001 From: Drew Folta Date: Tue, 2 Apr 2013 10:14:58 -0700 Subject: [PATCH 25/38] use page object to store/use staticAppConfig --- lib/app/autoload/action-context.common.js | 2 +- lib/app/autoload/mojito-client.client.js | 3 +++ lib/mojito.js | 2 ++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/app/autoload/action-context.common.js b/lib/app/autoload/action-context.common.js index a1b790cfb..12da10d79 100644 --- a/lib/app/autoload/action-context.common.js +++ b/lib/app/autoload/action-context.common.js @@ -308,7 +308,7 @@ YUI.add('mojito-action-context', function(Y, NAME) { this._adapter = opts.adapter; // pathToRoot, viewEngine, amoung others will be available through this. - this.staticAppConfig = store.getStaticAppConfig(); + this.staticAppConfig = (this._adapter.page && this._adapter.page.staticAppConfig) || store.getStaticAppConfig(); // Create a function which will properly delegate to the dispatcher to // perform the actual processing. diff --git a/lib/app/autoload/mojito-client.client.js b/lib/app/autoload/mojito-client.client.js index 6098e26c8..b00db484c 100644 --- a/lib/app/autoload/mojito-client.client.js +++ b/lib/app/autoload/mojito-client.client.js @@ -338,6 +338,9 @@ YUI.add('mojito-client', function(Y, NAME) { // pass globalHookhandler to addons that may want to use hooks globalHookHandler: globalHookHandler }; + + this.page.staticAppConfig = config.appConfig; + fireLifecycle('pre-init', forwardConfig); // if we didn't originaly have hooks enabled, copy back from config object. // This is the case where an add-on module wants to turn on hooks and diff --git a/lib/mojito.js b/lib/mojito.js index 1aa716f93..adc025aed 100644 --- a/lib/mojito.js +++ b/lib/mojito.js @@ -340,6 +340,8 @@ MojitoServer.prototype._configureAppInstance = function(app, options) { log: Y.log }); + outputHandler.page.staticAppConfig = store.getStaticAppConfig(); + // HookSystem::StartBlock // enabling perf group if (appConfig.perf) { From 8b973bcca871bf3f4268dd9768e2cfb53cf230d5 Mon Sep 17 00:00:00 2001 From: Drew Folta Date: Tue, 2 Apr 2013 12:12:13 -0700 Subject: [PATCH 26/38] encorporating feedback from PR --- lib/app/autoload/mojito-client.client.js | 2 +- tests/base/mojito-test.js | 14 ++++- .../autoload/test-action-context.common.js | 9 +++- tests/unit/lib/test-mojito.js | 53 ++++++++++++++----- 4 files changed, 60 insertions(+), 18 deletions(-) diff --git a/lib/app/autoload/mojito-client.client.js b/lib/app/autoload/mojito-client.client.js index b00db484c..dfea4579c 100644 --- a/lib/app/autoload/mojito-client.client.js +++ b/lib/app/autoload/mojito-client.client.js @@ -339,7 +339,7 @@ YUI.add('mojito-client', function(Y, NAME) { globalHookHandler: globalHookHandler }; - this.page.staticAppConfig = config.appConfig; + this.page.staticAppConfig = appConfig; fireLifecycle('pre-init', forwardConfig); // if we didn't originaly have hooks enabled, copy back from config object. diff --git a/tests/base/mojito-test.js b/tests/base/mojito-test.js index 8698d0db7..3e909f8be 100644 --- a/tests/base/mojito-test.js +++ b/tests/base/mojito-test.js @@ -65,7 +65,19 @@ YUI.add('mojito-hb', function(Y, NAME) {}); /* AUTOLOAD */ YUI.add('mojito-action-context', function(Y, NAME) {}); -YUI.add('mojito-dispatcher', function(Y, NAME) {}); +YUI.add('mojito-dispatcher', function(Y, NAME) { + // We need to grab the output handler while testing lib/mojito.js. + var mock = { + init: function(store) { + return { + dispatch: function(command, outputHandler) { + mock.outputHandler = outputHandler; + } + }; + } + }; + Y.namespace('mojito').Dispatcher = mock; +}); YUI.add('mojito-mojit-proxy', function(Y, NAME) {}); YUI.add('mojito-output-handler', function(Y, NAME) {}); YUI.add('mojito-perf', function(Y, NAME) {}); diff --git a/tests/unit/lib/app/autoload/test-action-context.common.js b/tests/unit/lib/app/autoload/test-action-context.common.js index 30685cd5d..ae7cc16eb 100644 --- a/tests/unit/lib/app/autoload/test-action-context.common.js +++ b/tests/unit/lib/app/autoload/test-action-context.common.js @@ -3,7 +3,7 @@ * Copyrights licensed under the New BSD License. * See the accompanying LICENSE file for terms. */ -YUI().use('mojito-action-context', 'test', function (Y) { +YUI().use('mojito-action-context', 'mojito-tests', 'test', function (Y) { var suite = new Y.Test.Suite('mojito-action-context tests'), acStash = {}, @@ -197,7 +197,11 @@ YUI().use('mojito-action-context', 'test', function (Y) { views: 'views' } }, - adapter: Y.Mock(), + adapter: { + page: { + staticAppConfig: {foo: 'bar'} + } + }, models: {}, controller: {index: function() {}}, store: store @@ -206,6 +210,7 @@ YUI().use('mojito-action-context', 'test', function (Y) { A.areSame('Type', ac.type, 'bad type'); A.areSame('index', ac.action, 'bad action'); A.areSame('context', ac.context, 'bad context'); + Y.TEST_CMP({foo: 'bar'}, ac.staticAppConfig, 'bad staticAppConfig'); A.areSame('the dispatcher', ac.dispatcher, "dispatcher wasn't stashed."); diff --git a/tests/unit/lib/test-mojito.js b/tests/unit/lib/test-mojito.js index 8fb137783..8d5f8f587 100644 --- a/tests/unit/lib/test-mojito.js +++ b/tests/unit/lib/test-mojito.js @@ -336,7 +336,7 @@ YUI().use('mojito', 'mojito-test-extra', 'test', function (Y) { _options: {verbose: false} }; - cb = function(/*err, app*/) {}; + cb = function(err, app) {}; Y.Mock.expect(app, { method: 'listen', @@ -615,14 +615,24 @@ YUI().use('mojito', 'mojito-test-extra', 'test', function (Y) { name: '_configureAppInstance suite', 'test configureAppInstance': function () { - var appwtf = { + var dispatcher, + madeY, // the Y instance made in mojito.js + appwtf = { store: { + getAllURLDetails: function () { + return {}; + }, getAppConfig: function () { A.isTrue(true); return { debugMemory: true, - middleware: ['mojito-router'], - perf: {} + middleware: ['mojito-handler-dispatcher'] + }; + }, + getStaticAppConfig: function () { + return { + debugMemory: true, + middleware: ['mojito-handler-dispatcher'] }; }, getStaticContext: function () { @@ -632,21 +642,36 @@ YUI().use('mojito', 'mojito-test-extra', 'test', function (Y) { getConfigShared: function () { return {}; }, + getModulesConfig: function () { + return { + modules: { + 'mojito-hooks': { + fullpath: __dirname + '/../../base/mojito-test.js' + }, + 'mojito-dispatcher': { + fullpath: __dirname + '/../../base/mojito-test.js' + } + } + }; + }, + getYUIURLDetails: function () { + return {}; + } } }, - use: function () {} + use: function (x) { + dispatcher = x; + } }; - Y.namespace('mojito.Dispatcher').init = function(store) { - Y.isObject(store); - return { - dispatch: function (cmd, outputHandler) {} - }; + Mojito.Server.prototype._configureLogger = function(y) { + madeY = y; }; - - try { - Mojito.Server.prototype._configureAppInstance(appwtf); - } catch (err) {} + Mojito.Server.prototype._configureAppInstance(appwtf); + dispatcher({command: {}}, {}, function() {}); + A.isObject(madeY.mojito.Dispatcher.outputHandler); + A.isObject(madeY.mojito.Dispatcher.outputHandler.page); + A.isObject(madeY.mojito.Dispatcher.outputHandler.page.staticAppConfig); } })); From ebab3898e6213c4a8a2c52a1e6da1cea62d84291 Mon Sep 17 00:00:00 2001 From: Joe Catera Date: Tue, 2 Apr 2013 18:23:25 -0700 Subject: [PATCH 27/38] Added a deprecation message and links to the Shaker GH repo and docs. --- docs/dev_guide/reference/mojito_cmdline.rst | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/docs/dev_guide/reference/mojito_cmdline.rst b/docs/dev_guide/reference/mojito_cmdline.rst index c791fde1a..feadb1247 100644 --- a/docs/dev_guide/reference/mojito_cmdline.rst +++ b/docs/dev_guide/reference/mojito_cmdline.rst @@ -284,11 +284,16 @@ applications. .. _mj_cmdlne-compile_sys: -Compile System -============== +Compile System (Deprecated) +=========================== -Mojito comes with a compile command for generating files to optimize an application for -production. +The ``compile`` command for generating files to optimize an application for +production has been **deprecated** and may not be available in the future. + +We recommend that you use the npm module `Shaker `_, +which lets you compile (create rollups) one or more input files. See the +`Shaker documentation `_ +to learn how to use Shaker in Mojito applications. .. _compile_sys-syntax From 428be6ff7d1e25058cd54b3ef53df0979a67d0be Mon Sep 17 00:00:00 2001 From: Joe Catera Date: Tue, 2 Apr 2013 18:25:49 -0700 Subject: [PATCH 28/38] Made a copyedit. --- docs/dev_guide/reference/mojito_cmdline.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/dev_guide/reference/mojito_cmdline.rst b/docs/dev_guide/reference/mojito_cmdline.rst index feadb1247..660126d66 100644 --- a/docs/dev_guide/reference/mojito_cmdline.rst +++ b/docs/dev_guide/reference/mojito_cmdline.rst @@ -291,7 +291,7 @@ The ``compile`` command for generating files to optimize an application for production has been **deprecated** and may not be available in the future. We recommend that you use the npm module `Shaker `_, -which lets you compile (create rollups) one or more input files. See the +which lets you compile (i.e., create rollups of) one or more input files. See the `Shaker documentation `_ to learn how to use Shaker in Mojito applications. From 6a4f0003b7b3218dbdf4e3e3e547ff556c6f7925 Mon Sep 17 00:00:00 2001 From: Isao Yagi Date: Mon, 1 Apr 2013 16:49:47 -0700 Subject: [PATCH 29/38] partial fix issue #1053; bundles without locations err on path.join() --- lib/app/autoload/store.server.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/app/autoload/store.server.js b/lib/app/autoload/store.server.js index e8c83a57c..e631df64b 100644 --- a/lib/app/autoload/store.server.js +++ b/lib/app/autoload/store.server.js @@ -1692,11 +1692,11 @@ YUI.add('mojito-resource-store', function(Y, NAME) { switch (info.pkg.yahoo.mojito.type) { case 'bundle': - dir = this._libs.path.join(info.dir, info.pkg.yahoo.mojito.location); + dir = this._libs.path.join(info.dir, info.pkg.yahoo.mojito.location || ''); this._preloadDirBundle(dir, pkg); break; case 'mojit': - dir = this._libs.path.join(info.dir, info.pkg.yahoo.mojito.location); + dir = this._libs.path.join(info.dir, info.pkg.yahoo.mojito.location || ''); this._preloadDirMojit(dir, 'pkg', pkg); break; default: From a80e0279ccaca209da0562a8371e4bcb0dbb10a9 Mon Sep 17 00:00:00 2001 From: Isao Yagi Date: Mon, 1 Apr 2013 17:28:09 -0700 Subject: [PATCH 30/38] partial fix issue #1053; test unset path part data also changed mock conf to always start with a clean copy --- .../lib/app/commands/build/test-shared.js | 175 +++++++----------- 1 file changed, 68 insertions(+), 107 deletions(-) diff --git a/tests/unit/lib/app/commands/build/test-shared.js b/tests/unit/lib/app/commands/build/test-shared.js index 4249ee2c7..1a48a683e 100644 --- a/tests/unit/lib/app/commands/build/test-shared.js +++ b/tests/unit/lib/app/commands/build/test-shared.js @@ -11,42 +11,42 @@ YUI().use('mojito-test-extra', 'test', function(Y) { cases, shared = require(Y.MOJITO_DIR + 'lib/app/commands/build/shared.js'), - count, - conf; - - conf = { - mojitodir: '/Users/isao/Repos/mojito/myfork/', - app: { - name: 'staticpf', - version: '0.0.1', - specs: { - frame: {}, - tunnelProxy: {} + count; + + function getConf() { + return { + mojitodir: '/Users/isao/Repos/mojito/myfork/', + app: { + name: 'staticpf', + version: '0.0.1', + specs: { + frame: {}, + tunnelProxy: {} + }, + dir: '/path/to/app' }, - dir: '/path/to/app' - }, - snapshot: { - name: '', - tag: '', - packages: {} - }, - build: { - attachManifest: false, - forceRelativePaths: false, - insertCharset: 'UTF-8', - port: 1111, - dir: '/path/to/build/dir', - type: 'html5app', - uris: [] - }, - context: { - device: 'iphone' - }, - contextqs: '?device=iphone', - tunnelpf: '/tunnel', - staticpf: 'staticpf' - }; - + snapshot: { + name: '', + tag: '', + packages: {} + }, + build: { + attachManifest: false, + forceRelativePaths: false, + insertCharset: 'UTF-8', + port: 1111, + dir: '/path/to/build/dir', + type: 'html5app', + uris: [] + }, + context: { + device: 'iphone' + }, + contextqs: '?device=iphone', + tunnelpf: '/tunnel', + staticpf: 'staticpf' + }; + } cases = { name: 'build/shared cases', @@ -81,7 +81,7 @@ YUI().use('mojito-test-extra', 'test', function(Y) { '/staticpf/top_frame/assets/index.css?device=iphone': '/staticpf/top_frame/assets/index.css' }; - shared.mapStoreUris(buildmap, conf, storemap); + shared.mapStoreUris(buildmap, getConf(), storemap); OA.areEqual(expected, buildmap); }, @@ -103,7 +103,7 @@ YUI().use('mojito-test-extra', 'test', function(Y) { '/staticpf/top_frame/package.json?device=iphone': '/staticpf/top_frame/package.json' }; - shared.mapStoreUris(buildmap, conf, storemap); + shared.mapStoreUris(buildmap, getConf(), storemap); OA.areEqual(expected, buildmap); }, @@ -123,7 +123,7 @@ YUI().use('mojito-test-extra', 'test', function(Y) { }; A.areSame(0, count); - shared.mapStoreUris(buildmap, conf, storemap); + shared.mapStoreUris(buildmap, getConf(), storemap); OA.areEqual(expected, buildmap); A.areSame(2, count); }, @@ -158,7 +158,7 @@ YUI().use('mojito-test-extra', 'test', function(Y) { expected = {'/tunnel/yahoo.application.test50/top_frame/definition.json?device=iphone': '/yahoo.application.test50/top_frame/definition.json'}; A.areSame(0, count); - shared.mapDefxUris(buildmap, conf, store); + shared.mapDefxUris(buildmap, getConf(), store); OA.areEqual(expected, buildmap); A.areSame(0, count); }, @@ -193,7 +193,7 @@ YUI().use('mojito-test-extra', 'test', function(Y) { expected = {}; A.areSame(0, count); - shared.mapDefxUris(buildmap, conf, store); + shared.mapDefxUris(buildmap, getConf(), store); OA.areEqual(expected, buildmap); A.areSame(0, count); }, @@ -206,7 +206,7 @@ YUI().use('mojito-test-extra', 'test', function(Y) { }; A.areSame(0, count); - shared.mapFunkySpecUris(buildmap, conf); + shared.mapFunkySpecUris(buildmap, getConf()); OA.areEqual(expected, buildmap); A.areSame(0, count); }, @@ -239,7 +239,7 @@ YUI().use('mojito-test-extra', 'test', function(Y) { newstr; A.areSame(0, count); - newstr = shared.mungePage(conf, uri, oldstr); + newstr = shared.mungePage(getConf(), uri, oldstr); A.areSame(newstr, oldstr); A.areSame(1, count); }, @@ -249,22 +249,18 @@ YUI().use('mojito-test-extra', 'test', function(Y) { oldstr = 'blah blah blah blah', newstr, expected = 'blah blah blah blah', - oldbuildconf = conf.build; + conf = getConf(); - conf.build = { - attachManifest: true, - forceRelativePaths: true, - insertCharset: 'UTF-8', - }; + conf.build.attachManifest = true; + conf.build.forceRelativePaths = true; + conf.build.insertCharset = 'UTF-8'; A.areSame(0, count); - newstr = shared.mungePage(conf, uri, oldstr); + newstr = shared.mungePage(getConf(), uri, oldstr); A.areSame(newstr, oldstr); A.areSame(1, count); - - conf.build = oldbuildconf; }, 'test mungePage for attachManifest:true': function () { @@ -272,13 +268,10 @@ YUI().use('mojito-test-extra', 'test', function(Y) { oldstr = 'blah blah blah blah', newstr, expected = 'blah blah blah blah', - oldbuildconf = conf.build; + conf = getConf(); - conf.build = { - attachManifest: true, - forceRelativePaths: true, - insertCharset: 'UTF-8', - }; + conf.build.attachManifest = true; + conf.build.forceRelativePaths = true; A.areSame(0, count); @@ -287,8 +280,6 @@ YUI().use('mojito-test-extra', 'test', function(Y) { A.areNotSame(newstr, oldstr); A.areSame(expected, newstr); A.areSame(1, count); - - conf.build = oldbuildconf; }, 'test mungePage for attachManifest:true can apply relative path': function () { @@ -296,13 +287,10 @@ YUI().use('mojito-test-extra', 'test', function(Y) { oldstr = 'blah blah blah blah', newstr, expected = 'blah blah blah blah', - oldbuildconf = conf.build; + conf = getConf(); - conf.build = { - attachManifest: true, - forceRelativePaths: true, - insertCharset: 'UTF-8', - }; + conf.build.attachManifest = true; + conf.build.forceRelativePaths = true; A.areSame(0, count); @@ -311,8 +299,6 @@ YUI().use('mojito-test-extra', 'test', function(Y) { A.areNotSame(newstr, oldstr); A.areSame(expected, newstr); A.areSame(1, count); - - conf.build = oldbuildconf; }, 'test mungePage for forceRelativePaths:true': function () { @@ -320,13 +306,10 @@ YUI().use('mojito-test-extra', 'test', function(Y) { oldstr = 'blah blah blah blah', newstr, expected = 'blah blah blah blah', - oldbuildconf = conf.build; + conf = getConf(); - conf.build = { - attachManifest: true, - forceRelativePaths: true, - insertCharset: 'UTF-8', - }; + conf.build.attachManifest = true; + conf.build.forceRelativePaths = true; A.areSame(0, count); @@ -344,8 +327,6 @@ YUI().use('mojito-test-extra', 'test', function(Y) { A.areNotSame(newstr, oldstr); A.areSame(expected, newstr); A.areSame(2, count); - - conf.build = oldbuildconf; }, 'test mungePage for forceRelativePaths:true with no common root': function () { @@ -353,13 +334,10 @@ YUI().use('mojito-test-extra', 'test', function(Y) { oldstr = 'blah blah blah blah', newstr, expected = 'blah blah blah blah', - oldbuildconf = conf.build; + conf = getConf(); - conf.build = { - attachManifest: true, - forceRelativePaths: true, - insertCharset: 'UTF-8', - }; + conf.build.attachManifest = true; + conf.build.forceRelativePaths = true; A.areSame(0, count); @@ -377,8 +355,6 @@ YUI().use('mojito-test-extra', 'test', function(Y) { A.areNotSame(newstr, oldstr); A.areSame(expected, newstr); A.areSame(2, count); - - conf.build = oldbuildconf; }, 'test mungePage for insertCharset:true': function () { @@ -386,13 +362,10 @@ YUI().use('mojito-test-extra', 'test', function(Y) { oldstr = 'blah blah blah blah', newstr, expected = 'blah blah \n\n blah blah', - oldbuildconf = conf.build; + conf = getConf(); - conf.build = { - attachManifest: true, - forceRelativePaths: true, - insertCharset: 'UTF-8', - }; + conf.build.attachManifest = true; + conf.build.forceRelativePaths = true; A.areSame(0, count); @@ -401,8 +374,6 @@ YUI().use('mojito-test-extra', 'test', function(Y) { A.areNotSame(newstr, oldstr); A.areSame(expected, newstr); A.areSame(1, count); - - conf.build = oldbuildconf; }, 'test mungePage for insertCharset:true simple tag': function () { @@ -410,13 +381,10 @@ YUI().use('mojito-test-extra', 'test', function(Y) { oldstr = 'blah blah blah blah', newstr, expected = 'blah blah \n\n blah blah', - oldbuildconf = conf.build; + conf = getConf(); - conf.build = { - attachManifest: true, - forceRelativePaths: true, - insertCharset: 'UTF-8', - }; + conf.build.attachManifest = true; + conf.build.forceRelativePaths = true; A.areSame(0, count); @@ -425,21 +393,16 @@ YUI().use('mojito-test-extra', 'test', function(Y) { A.areNotSame(newstr, oldstr); A.areSame(expected, newstr); A.areSame(1, count); - - conf.build = oldbuildconf; }, 'test mungePage for insertCharset:true does nothing if there already is a charset metatag': function () { var uri = '/', oldstr = 'blah blah blah blah', newstr, - oldbuildconf = conf.build; + conf = getConf(); - conf.build = { - attachManifest: true, - forceRelativePaths: true, - insertCharset: 'UTF-8', - }; + conf.build.attachManifest = true; + conf.build.forceRelativePaths = true; A.areSame(0, count); @@ -447,8 +410,6 @@ YUI().use('mojito-test-extra', 'test', function(Y) { A.areSame(newstr, oldstr); A.areSame(1, count); - - conf.build = oldbuildconf; }, }; From a80d88400a8c095663f2d9494fc4a7c3ecc1f223 Mon Sep 17 00:00:00 2001 From: Isao Yagi Date: Tue, 2 Apr 2013 09:15:36 -0700 Subject: [PATCH 31/38] partial fix issue #1053; pass "" to test instead of nothing --- tests/unit/lib/app/commands/test-version.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/lib/app/commands/test-version.js b/tests/unit/lib/app/commands/test-version.js index 1d7de079f..cbe8bc3ac 100644 --- a/tests/unit/lib/app/commands/test-version.js +++ b/tests/unit/lib/app/commands/test-version.js @@ -61,7 +61,7 @@ YUI().use('test', function(Y) { } }); libutils.test.setConsole(mockConsole); - version.run(["mojit"], null, function() {}); + version.run(["mojit", ""], null, function() {}); }, 'test run app': function() { From c5d5fa507e48d57def7b718b5286884c106e967e Mon Sep 17 00:00:00 2001 From: Isao Yagi Date: Wed, 3 Apr 2013 10:47:41 -0700 Subject: [PATCH 32/38] partial fix issue #1053; use real addon/rs.url.js instead of mock. back out previous `res.url || ''` guard. root cause of undefined res.url in the unit tests was poor mock. --- tests/unit/lib/app/addons/rs/rs_test_descriptor.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/lib/app/addons/rs/rs_test_descriptor.json b/tests/unit/lib/app/addons/rs/rs_test_descriptor.json index 571bf0fd1..1e2664426 100644 --- a/tests/unit/lib/app/addons/rs/rs_test_descriptor.json +++ b/tests/unit/lib/app/addons/rs/rs_test_descriptor.json @@ -18,7 +18,7 @@ }, "dispatch-helper.server": { "params": { - "lib": "$$config.lib$$/app/addons/rs/dispatch-helper.js,../../../../../../lib/app/addons/rs/config.js,../../../../../../lib/app/addons/rs/selector.js,../../../../../../lib/app/addons/rs/yui.js,../../../../../../lib/app/autoload/store.server.js", + "lib": "$$config.lib$$/app/addons/rs/dispatch-helper.js,../../../../../../lib/app/addons/rs/config.js,../../../../../../lib/app/addons/rs/selector.js,../../../../../../lib/app/addons/rs/yui.js,../../../../../../lib/app/autoload/store.server.js,../../../../../../lib/app/addons/rs/url.js", "test": "./test-dispatch-helper.js", "driver": "nodejs" }, From 55e5bc99c03e294a5b9e717553e366e19df04ad8 Mon Sep 17 00:00:00 2001 From: Joe Catera Date: Wed, 3 Apr 2013 11:10:52 -0700 Subject: [PATCH 33/38] Added note that ac.flash is asynchronous. --- docs/dev_guide/topics/mojito_run_dyn_defined_mojits.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/dev_guide/topics/mojito_run_dyn_defined_mojits.rst b/docs/dev_guide/topics/mojito_run_dyn_defined_mojits.rst index 02501eae9..5dc6df453 100644 --- a/docs/dev_guide/topics/mojito_run_dyn_defined_mojits.rst +++ b/docs/dev_guide/topics/mojito_run_dyn_defined_mojits.rst @@ -52,6 +52,8 @@ Mojito also provides the ``dispatch`` method that can be called from the ``ActionContext`` object to run a dynamically defined child mojit. The ``dispatch`` method also allows you to define your own ``flush``, ``done``, and ``error`` functions for the child mojit instance. +The ``done`` and ``error`` methods are executed synchronously, +but the ``flush`` method is executed asynchronously. .. _dyn_defined_mojits-use_cases: From d57585ec379e6f40f03f197029556936d4d99e22 Mon Sep 17 00:00:00 2001 From: Joe Catera Date: Wed, 3 Apr 2013 13:53:13 -0700 Subject: [PATCH 34/38] Updated the Selenium and made Firefox v20 a prerequisite. Also added a note that if you use different versions of either Firefox/Selenium, users should consult the platforms supported by Selenium. --- docs/dev_guide/topics/mojito_testing.rst | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/dev_guide/topics/mojito_testing.rst b/docs/dev_guide/topics/mojito_testing.rst index bb16ec837..ae3566370 100644 --- a/docs/dev_guide/topics/mojito_testing.rst +++ b/docs/dev_guide/topics/mojito_testing.rst @@ -581,6 +581,7 @@ Required Software - `Java `_ - `Node.js 0.6 or higher (packaged with npm) `_ - `Git `_ +- `Firefox v20 `_ .. _func_unit_reqs-macs: @@ -632,15 +633,20 @@ Installing Selenium (recommended) The following instructions work for both Macs and Linux. -#. `Download the Selenium JAR executable `_. +#. `Download the Selenium v2.13.0 JAR executable `_. #. Start the Selenium server: - ``$ java -jar path/to/selenium-server.jar`` + ``$ java -jar path/to/selenium-server-standalone-2.31.0.jar`` #. Confirm Selenium is running by going to the following URL: `http://localhost:4444/wd/hub/static/resource/hub.html `_ #. Shut down the Selenium server with ``Ctrl-C`` command. +.. important:: If you use other versions of Firefox v20 and the Selenium Standalone Server v2.31.0, you + my run into backward compatibility issues. Please see the + `Platforms Supported by Selenium `_ + to learn what Selenium versions and browser versions are compatible. + .. _func_unit-run: Running Tests From 6937346ab70bf33a013035b83c1b9aaaf037ba6d Mon Sep 17 00:00:00 2001 From: Joe Catera Date: Wed, 3 Apr 2013 13:58:55 -0700 Subject: [PATCH 35/38] Copy edited a note. --- docs/dev_guide/topics/mojito_testing.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/dev_guide/topics/mojito_testing.rst b/docs/dev_guide/topics/mojito_testing.rst index ae3566370..066e7270a 100644 --- a/docs/dev_guide/topics/mojito_testing.rst +++ b/docs/dev_guide/topics/mojito_testing.rst @@ -642,7 +642,7 @@ The following instructions work for both Macs and Linux. `http://localhost:4444/wd/hub/static/resource/hub.html `_ #. Shut down the Selenium server with ``Ctrl-C`` command. -.. important:: If you use other versions of Firefox v20 and the Selenium Standalone Server v2.31.0, you +.. warning:: If you not using Firefox v20 and the Selenium Standalone Server v2.31.0, you my run into backward compatibility issues. Please see the `Platforms Supported by Selenium `_ to learn what Selenium versions and browser versions are compatible. From c65909ba3756b54641e4d275b2ba3d8f89c3efcb Mon Sep 17 00:00:00 2001 From: Joe Catera Date: Wed, 3 Apr 2013 14:24:26 -0700 Subject: [PATCH 36/38] More copy edits. --- docs/dev_guide/topics/mojito_testing.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/dev_guide/topics/mojito_testing.rst b/docs/dev_guide/topics/mojito_testing.rst index 066e7270a..58b676501 100644 --- a/docs/dev_guide/topics/mojito_testing.rst +++ b/docs/dev_guide/topics/mojito_testing.rst @@ -642,10 +642,10 @@ The following instructions work for both Macs and Linux. `http://localhost:4444/wd/hub/static/resource/hub.html `_ #. Shut down the Selenium server with ``Ctrl-C`` command. -.. warning:: If you not using Firefox v20 and the Selenium Standalone Server v2.31.0, you - my run into backward compatibility issues. Please see the +.. warning:: If you are not using Firefox v20 and the Selenium Standalone Server v2.31.0, you + may run into backward compatibility issues. Please see the `Platforms Supported by Selenium `_ - to learn what Selenium versions and browser versions are compatible. + to learn what Selenium and browser versions are compatible. .. _func_unit-run: From 47df2c6355ec2093ef4cd1c8400d484584a676d5 Mon Sep 17 00:00:00 2001 From: Drew Folta Date: Wed, 3 Apr 2013 16:54:46 -0700 Subject: [PATCH 37/38] trimmed a bunch of copy()s --- lib/app/addons/rs/config.js | 6 ++++-- lib/app/addons/rs/selector.js | 3 ++- lib/app/autoload/package-walker.server.js | 4 +--- lib/app/autoload/route-maker.common.js | 6 ++++-- lib/app/autoload/store.server.js | 7 ++----- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/app/addons/rs/config.js b/lib/app/addons/rs/config.js index cf6cf3306..53f80bd2a 100644 --- a/lib/app/addons/rs/config.js +++ b/lib/app/addons/rs/config.js @@ -63,7 +63,8 @@ YUI.add('addon-rs-config', function(Y, NAME) { * @return {object} the YCB dimensions structure for the app */ getDimensions: function() { - return Y.mojito.util.copy(this._ycbDims); + // NOTE: We used to copy() here. Research suggested that it was safe to drop. + return this._ycbDims; }, @@ -101,7 +102,8 @@ YUI.add('addon-rs-config', function(Y, NAME) { } this._jsonCache[fullPath] = json; } - return Y.mojito.util.copy(json); + // NOTE: We used to copy() here. Research suggested that it was safe to drop. + return json; }, diff --git a/lib/app/addons/rs/selector.js b/lib/app/addons/rs/selector.js index 6e46e9f17..f75e15b11 100644 --- a/lib/app/addons/rs/selector.js +++ b/lib/app/addons/rs/selector.js @@ -91,7 +91,8 @@ YUI.add('addon-rs-selector', function(Y, NAME) { } this._poslCache[cacheKey] = posl; } - return Y.mojito.util.copy(posl); + // NOTE: We used to copy() here. Research suggested that it was safe to drop. + return posl; }, diff --git a/lib/app/autoload/package-walker.server.js b/lib/app/autoload/package-walker.server.js index c81183935..746df88e4 100644 --- a/lib/app/autoload/package-walker.server.js +++ b/lib/app/autoload/package-walker.server.js @@ -45,7 +45,6 @@ function BreadthFirstPackageWalker(root) { * pkg {object} contents of the package's package.json * depth {interger} how deeply nested the package is * parents {array} list of ancestor package names - * inherit {anything} contents of the info.inherit from an ancestor's callback * * @param cb {function(error, info)} callback called for each package * @return {nothing} results returned via callback @@ -136,8 +135,7 @@ BreadthFirstPackageWalker.prototype._walkModules = function(work) { this.todo.push({ depth: work.depth + 1, parents: parents, - dir: libpath.join(modulesDir, subdir), - inherit: copy(work.inherit) + dir: libpath.join(modulesDir, subdir) }); } }; diff --git a/lib/app/autoload/route-maker.common.js b/lib/app/autoload/route-maker.common.js index 6a8e0149e..bf2684ff0 100644 --- a/lib/app/autoload/route-maker.common.js +++ b/lib/app/autoload/route-maker.common.js @@ -292,7 +292,8 @@ YUI.add('mojito-route-maker', function(Y, NAME) { // Y.log('[UriRouter] found route: ' + JSON.stringify(route)); - match = Y.mojito.util.copy(route); + // NOTE: We used to copy() here. Research suggested that it was safe to drop. + match = route; // Add the extracted URI params to the query obj ret = new RegExp(route.ext_match).exec(uri); @@ -323,7 +324,8 @@ YUI.add('mojito-route-maker', function(Y, NAME) { * @return {object} computed routes. */ getComputedRoutes: function() { - return Y.mojito.util.copy(CACHE.routes); + // NOTE: We used to copy() here. Research suggested that it was safe to drop. + return CACHE.routes; }, diff --git a/lib/app/autoload/store.server.js b/lib/app/autoload/store.server.js index e8c83a57c..764d5ed7a 100644 --- a/lib/app/autoload/store.server.js +++ b/lib/app/autoload/store.server.js @@ -582,7 +582,7 @@ YUI.add('mojito-resource-store', function(Y, NAME) { * @param {string} args.mojitType name of mojit * @param {object} mojit the mojit type details */ - getMojitTypeDetails: function(env, ctx, mojitType, dest, DANGERDANGERreturnRawCacheValue) { + getMojitTypeDetails: function(env, ctx, mojitType, dest) { //Y.log('getMojitTypeDetails('+env+', '+JSON.stringify(ctx)+', '+mojitType+')', 'debug', NAME); var posl = this.selector.getPOSLFromContext(ctx), poslKey = JSON.stringify(posl), @@ -617,10 +617,7 @@ YUI.add('mojito-resource-store', function(Y, NAME) { Y.log('The "dest" parameter to store.getMojitTypeDetails() is deprecated.', 'warn', NAME); Y.mojito.util.mergeRecursive(dest, cacheValue); } - if (DANGERDANGERreturnRawCacheValue) { - return cacheValue; - } - return Y.mojito.util.copy(cacheValue); + return cacheValue; }, From 592c89dcd64e320b2777f228f497758a8f3f0359 Mon Sep 17 00:00:00 2001 From: Drew Folta Date: Thu, 4 Apr 2013 15:02:15 -0700 Subject: [PATCH 38/38] Revert risky parts of "trimmed a bunch of copy()s" This reverts risky parts of commit 47df2c6355ec2093ef4cd1c8400d484584a676d5. --- lib/app/addons/rs/config.js | 6 ++---- lib/app/autoload/store.server.js | 8 ++------ 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/lib/app/addons/rs/config.js b/lib/app/addons/rs/config.js index 53f80bd2a..cf6cf3306 100644 --- a/lib/app/addons/rs/config.js +++ b/lib/app/addons/rs/config.js @@ -63,8 +63,7 @@ YUI.add('addon-rs-config', function(Y, NAME) { * @return {object} the YCB dimensions structure for the app */ getDimensions: function() { - // NOTE: We used to copy() here. Research suggested that it was safe to drop. - return this._ycbDims; + return Y.mojito.util.copy(this._ycbDims); }, @@ -102,8 +101,7 @@ YUI.add('addon-rs-config', function(Y, NAME) { } this._jsonCache[fullPath] = json; } - // NOTE: We used to copy() here. Research suggested that it was safe to drop. - return json; + return Y.mojito.util.copy(json); }, diff --git a/lib/app/autoload/store.server.js b/lib/app/autoload/store.server.js index b08c67273..3fbe7a038 100644 --- a/lib/app/autoload/store.server.js +++ b/lib/app/autoload/store.server.js @@ -538,7 +538,7 @@ YUI.add('mojito-resource-store', function(Y, NAME) { // type details try { - typeDetails = this.getMojitTypeDetails(env, ctx, spec.type, null, true); + typeDetails = this.getMojitTypeDetails(env, ctx, spec.type); } catch (err2) { return cb(err2); } @@ -567,8 +567,6 @@ YUI.add('mojito-resource-store', function(Y, NAME) { * @param {object} ctx the context * @param {string} mojitType mojit type * @param {object} dest DEPRECATED: object in which to place the results - * @param {boolean} DANGERDANGERreturnRawCacheValue optional indicates - * that the cache value should be returned directly, instead of a copy. defaults to false. * @return {object} details about the mojit type */ /** @@ -582,7 +580,7 @@ YUI.add('mojito-resource-store', function(Y, NAME) { * @param {string} args.mojitType name of mojit * @param {object} mojit the mojit type details */ - getMojitTypeDetails: function(env, ctx, mojitType, dest) { + getMojitTypeDetails: function(env, ctx, mojitType, dest, DANGERDANGERreturnRawCacheValue) { //Y.log('getMojitTypeDetails('+env+', '+JSON.stringify(ctx)+', '+mojitType+')', 'debug', NAME); var posl = this.selector.getPOSLFromContext(ctx), poslKey = JSON.stringify(posl), @@ -1658,12 +1656,10 @@ YUI.add('mojito-resource-store', function(Y, NAME) { var dir, pkg, visitKey; - // FUTURE: use info.inherit to scope mojit dependencies /* console.log('--PACKAGE-- ' + info.depth + ' ' + info.pkg.name + '@' + info.pkg.version + ' \t' + (info.pkg.yahoo && info.pkg.yahoo.mojito && info.pkg.yahoo.mojito.type) + ' \t[' + info.parents.join(',') + ']' - // + ' \t-- ' + JSON.stringify(info.inherit) ); */ pkg = {