Permalink
Browse files

Make our internal `rc` into a function that accepts a namespace arg (…

…defaults to `sails`)

This allows `node app.js` to share in the glory of humanized env vars, and protects against the returned object getting mutated
  • Loading branch information...
1 parent b865f30 commit bd455afd87c25ba438b492903b1110730631b379 @sgress454 sgress454 committed Dec 8, 2016
View
@@ -2,7 +2,7 @@
* Module dependencies
*/
-var rc = require('rc');
+var rc = require('../lib/app/configuration/rc');
/**
@@ -9,7 +9,7 @@ var _ = require('@sailshq/lodash');
var chalk = require('chalk');
var CaptainsLog = require('captains-log');
-var rconf = require('../lib/app/configuration/rc');
+var rconf = require('../lib/app/configuration/rc')();
var Sails = require('../lib/app');
var SharedErrorHelpers = require('../errors');
var readReplHistoryAndStartTranscribing = require('./private/read-repl-history-and-start-transcribing');
View
@@ -5,7 +5,7 @@
var _ = require('@sailshq/lodash');
var util = require('util');
var path = require('path');
-var rconf = require('../lib/app/configuration/rc');
+var rconf = require('../lib/app/configuration/rc')();
/**
* `sails deploy`
@@ -10,7 +10,7 @@ var async = require('async');
var CaptainsLog = require('captains-log');
var sailsGen = require('sails-generate');
var package = require('../package.json');
-var rconf = require('../lib/app/configuration/rc');
+var rconf = require('../lib/app/configuration/rc')();
/**
* `sails generate`
View
@@ -7,7 +7,7 @@ var _ = require('@sailshq/lodash');
var chalk = require('chalk');
var captains = require('captains-log');
-var rconf = require('../lib/app/configuration/rc');
+var rconf = require('../lib/app/configuration/rc')();
var Sails = require('../lib/app');
var SharedErrorHelpers = require('../errors');
View
@@ -6,7 +6,7 @@ var nodepath = require('path');
var _ = require('@sailshq/lodash');
var sailsgen = require('sails-generate');
var package = require('../package.json');
-var rconf = require('../lib/app/configuration/rc');
+var rconf = require('../lib/app/configuration/rc')();
var CaptainsLog = require('captains-log');
View
@@ -6,7 +6,7 @@ var nodepath = require('path');
var _ = require('@sailshq/lodash');
var CaptainsLog = require('captains-log');
var Process = require('machinepack-process');
-var rconf = require('../lib/app/configuration/rc');
+var rconf = require('../lib/app/configuration/rc')();
View
@@ -9,7 +9,7 @@ var CaptainsLog = require('captains-log');
// Once per process:
// Build logger using best-available information
// when this module is initially required.
-var rconf = require('../lib/app/configuration/rc');
+var rconf = require('../lib/app/configuration/rc')();
var log = CaptainsLog(rconf.log);
View
@@ -8,7 +8,7 @@ var CaptainsLog = require('captains-log');
// Once per process:
// Build logger using best-available information
// when this module is initially required.
-var rconf = require('../lib/app/configuration/rc');
+var rconf = require('../lib/app/configuration/rc')();
var log = CaptainsLog(rconf.log);
@@ -7,61 +7,65 @@ var rttc = require('rttc');
var minimist = require('minimist');
/**
- * Locate and load .sailsrc files if they exist.
- *
- * NOTE: this occurs almost immediately when sails is required,
- * and since `rc` is synchronous, the examination of env variables,
- * cmdline opts, and .sailsrc files is immediate, and happens only once.
- *
- * @type {Object}
+ * Load configuration from .rc files and env vars
+ * @param {string} namespace Namespace to look for env vars under (defaults to `sails`)
+ * @return {dictionary} A dictionary of config values gathered from .rc files, with env vars and command line options merged on top
*/
-var conf = rc('sails');
-
-// Load in overrides from the environment, using `rttc.parseHuman` to
-// guesstimate the types.
-// NOTE -- the code below is lifted from the `rc` module, and modified to:
-// 1. Pass JSHint
-// 2. Run parseHuman on values
-// If at some point `rc` exposes metadata about which configs came from
-// the environment, we can simplify our code by just running `parseHuman`
-// on those values instead of doing the work to pluck them from the env.
-
-// We're only interested in env vars prefixed with `sails_`.
-var prefix = 'sails_';
-// Cache the prefix length so we don't have to keep looking it up.
-var l = prefix.length;
-
-// Loop through the env vars, looking for ones with the right prefix.
-_.each(process.env, function(val, key) {
-
- // If this var's name has the right prefix...
- if((key.indexOf(prefix)) === 0) {
-
- // Replace double-underscores with dots, to work with Lodash _.set().
- var keypath = key.substring(l).replace(/__/g,'.');
-
- // Attempt to parse the value as JSON.
- try {
- val = rttc.parseHuman(val, 'json');
- }
- // If that doesn't work, humanize the value without providing a schema.
- catch(e) {
- val = rttc.parseHuman(val);
- }
+module.exports = function(namespace) {
+
+ // Default namespace to `sails`.
+ namespace = namespace || 'sails';
+
+ // Locate and load .rc files if they exist.
+ var conf = rc(namespace);
+
+ // Load in overrides from the environment, using `rttc.parseHuman` to
+ // guesstimate the types.
+ // NOTE -- the code below is lifted from the `rc` module, and modified to:
+ // 1. Pass JSHint
+ // 2. Run parseHuman on values
+ // If at some point `rc` exposes metadata about which configs came from
+ // the environment, we can simplify our code by just running `parseHuman`
+ // on those values instead of doing the work to pluck them from the env.
+
+ // Construct the expected env var prefix from the namespace.
+ var prefix = namespace + '_';
+
+ // Cache the prefix length so we don't have to keep looking it up.
+ var l = prefix.length;
- // Override the current value at this keypath in `conf` (which currently contains
- // the string value of the env var) with the now (possibly) humanized value.
- _.set(conf, keypath, val);
+ // Loop through the env vars, looking for ones with the right prefix.
+ _.each(process.env, function(val, key) {
+
+ // If this var's name has the right prefix...
+ if((key.indexOf(prefix)) === 0) {
+
+ // Replace double-underscores with dots, to work with Lodash _.set().
+ var keypath = key.substring(l).replace(/__/g,'.');
+
+ // Attempt to parse the value as JSON.
+ try {
+ val = rttc.parseHuman(val, 'json');
+ }
+ // If that doesn't work, humanize the value without providing a schema.
+ catch(e) {
+ val = rttc.parseHuman(val);
+ }
+
+ // Override the current value at this keypath in `conf` (which currently contains
+ // the string value of the env var) with the now (possibly) humanized value.
+ _.set(conf, keypath, val);
+
+ }
- }
+ });
-});
+ // Load command line arguments, since they need to take precedence over env.
+ var argv = minimist(process.argv.slice(2));
-// Load command line arguments, since they need to take precedence over env.
-var argv = minimist(process.argv.slice(2));
+ // Merge the command line arguments back on top.
+ conf = _.merge(conf, argv);
-// Merge the command line arguments back on top.
-conf = _.merge(conf, argv);
+ return conf;
-// Expose the final conf object to the world.
-module.exports = conf;
+};
@@ -39,8 +39,8 @@ var MProcess = require('machinepack-process');
*/
module.exports = function testSpawningSailsLiftChildProcessInCwd (opts){
- if (!_.isArray(opts.liftCliArgs)){
- throw new Error('Consistency violation: Missing or invalid option (`liftCliArgs` should be an array) in `testSpawningSailsLiftChildProcessInCwd()`. I may just be a test helper, but I\'m serious about assertions!!!');
+ if (!_.isArray(opts.liftCliArgs) && !_.isArray(opts.cliArgs)){
@mikermcneil
mikermcneil Dec 8, 2016 Member

@sgress454 re: cliArgs - this test util was intended to be for sails lift -- so i'd rather not have it test node app or other stuff. Looks like it's only being used this place at one point in the code though, so easy to change.

Also, if we're adding envVars (which I think is a good idea), then we've just got to add them to the comments above as well.

+ throw new Error('Consistency violation: Missing or invalid option (either `cliArgs` or `liftCliArgs` should be an array) in `testSpawningSailsLiftChildProcessInCwd()`. I may just be a test helper, but I\'m serious about assertions!!!');
}
if (!_.isString(opts.pathToSailsCLI)){
throw new Error('Consistency violation: Missing or invalid option (`pathToSailsCLI` should be a string) in `testSpawningSailsLiftChildProcessInCwd()`. I may just be a test helper, but I\'m serious about assertions!!!');
@@ -76,11 +76,14 @@ module.exports = function testSpawningSailsLiftChildProcessInCwd (opts){
// This variable will hold the reference to the child process.
var sailsLiftProc;
+ var cliArgs = opts.cliArgs ? opts.cliArgs : [opts.pathToSailsCLI, 'lift'].concat(opts.liftCliArgs);
+
// Spawn the child process
before(function(done) {
sailsLiftProc = MProcess.spawnChildProcess({
command: 'node',
- cliArgs: [opts.pathToSailsCLI, 'lift'].concat(opts.liftCliArgs)
+ cliArgs: cliArgs,
+ environmentVars: opts.envVars
}).execSync();
// For debugging, as needed:
@@ -6,7 +6,10 @@ var path = require('path');
var util = require('util');
var tmp = require('tmp');
var request = require('request');
+var assert = require('assert');
+var _ = require('@sailshq/lodash');
var MProcess = require('machinepack-process');
+var Filesystem = require('machinepack-fs');
var testSpawningSailsLiftChildProcessInCwd = require('../helpers/test-spawning-sails-lift-child-process-in-cwd');
var appHelper = require('./helpers/appHelper');
@@ -46,20 +49,65 @@ describe('Starting sails server with `sails lift`', function() {
// And CD in.
before(function (){
process.chdir(pathToTestApp);
+ Filesystem.writeSync({
+ force: true,
+ destination: 'api/controllers/getconf.js',
+ string: 'module.exports = function (req, res) { return res.json(sails.config); }'
+ }).execSync();
+
});
// Test `sails lift` in the CWD.
describe('running `sails lift`', function (){
testSpawningSailsLiftChildProcessInCwd({
pathToSailsCLI: pathToSailsCLI,
liftCliArgs: ['--hooks.pubsub=false'],
+ envVars: _.extend({ 'sails_foo__bar': '{"abc": 123}'}, process.env),
httpRequestInstructions: {
method: 'GET',
uri: 'http://localhost:1337',
+ },
+ fnWithAdditionalTests: function (){
+ it('should humanize the config passed in via env vars', function (done){
+ request({
+ method: 'GET',
+ uri: 'http://localhost:1337/getconf',
+ }, function(err, response, body) {
+ if (err) { return done(err); }
+ body = JSON.parse(body);
@mikermcneil
mikermcneil Dec 8, 2016 Member

needs status code assertion first, then try/catch around parse and assert

+ assert.equal(body.foo && body.foo.bar && body.foo.bar.abc, 123);
+ return done();
+ });
+ });
}
});
});
+ // Test `sails lift` in the CWD with env vars for config.
+ describe('running `node app.js`', function (){
+
+ testSpawningSailsLiftChildProcessInCwd({
+ pathToSailsCLI: pathToSailsCLI,
+ cliArgs: ['app.js', '--hooks.pubsub=false'],
+ envVars: _.extend({ 'sails_foo__bar': '{"abc": 123}'}, process.env),
+ fnWithAdditionalTests: function (){
+ it('should humanize the config passed in via env vars', function (done){
+ request({
+ method: 'GET',
+ uri: 'http://localhost:1337/getconf',
+ }, function(err, response, body) {
+ if (err) { return done(err); }
+ body = JSON.parse(body);
@mikermcneil
mikermcneil Dec 8, 2016 Member

same as above

+ assert.equal(body.foo && body.foo.bar && body.foo.bar.abc, 123);
+ return done();
+ });
+ });
+ }
+ });
+
+ });
+
+
// Test `sails lift --port=1492` in the CWD.
describe('running `sails lift --port=1492`', function (){
testSpawningSailsLiftChildProcessInCwd({

1 comment on commit bd455af

@mikermcneil
Member

For future reference, the test is failing because:

stdout: info: No local Sails install detected; using globally-installed Sails.
...
...
stderr: debug: -------------------------------------------------------

          ✓ should still be lifted
          ✓ should respond with a 200 status code when a `GET` request is sent to `http://localhost:1337`
          ✓ should humanize the config passed in via env vars
      running `node app.js`
        and then waiting for a bit
stderr: Encountered an error when attempting to require('sails'):

stderr: Error: Cannot find module 'sails'
    at Function.Module._resolveFilename (module.js:326:15)
    at Function.Module._load (module.js:277:25)
    at Module.require (module.js:354:17)
    at require (internal/module.js:12:17)
    at Object.<anonymous> (/private/var/folders/_s/347n05_x2rgb_0w6s6y0ytr00000gn/T/tmp-291179ZUWIae91wSm/testApp/app.js:35:11)
    at Module._compile (module.js:410:26)
    at Object.Module._extensions..js (module.js:417:10)
    at Module.load (module.js:344:32)
    at Function.Module._load (module.js:301:12)
    at Function.Module.runMain (module.js:442:10)

stderr: --

stderr: To run an app using `node app.js`, you usually need to have a version of `sails` installed in the same directory as your app.
To do that, run `npm install sails`

stderr: 
Alternatively, if you have sails installed globally (i.e. you did `npm install -g sails`), you can use `sails lift`.
When you run `sails lift`, your app will still use a local `./node_modules/sails` dependency if it exists,

stderr: but if it doesn't, the app will run with the global sails instead!

          ✓ should still be lifted
          1) should humanize the config passed in via env vars
          2) "after all" hook

Please sign in to comment.