🎨 😎 config env usages (#7929)

refs #7488 - remove all ugly env checks - rather use config properties - replace process.env.NODE_ENV by config.get('env')
TryGhost · Feb 3, 2017 · f3d1635 · f3d1635
1 parent a68592a
commit f3d1635
Show file tree

Hide file tree

Showing 18 changed files with 68 additions and 49 deletions.
diff --git a/core/server/api/mail.js b/core/server/api/mail.js
@@ -2,7 +2,6 @@
 // API for sending Mail
 
 var Promise       = require('bluebird'),
-    config        = require('../config'),
     pipeline      = require('../utils/pipeline'),
     errors        = require('../errors'),
     mail          = require('../mail'),
@@ -11,16 +10,14 @@ var Promise       = require('bluebird'),
     notifications = require('./notifications'),
     i18n          = require('../i18n'),
     docName       = 'mail',
-    mode          = config.get('env'),
-    testing       = mode !== 'production' && mode !== 'development',
     mailer,
     apiMail;
 
 /**
  * Send mail helper
  */
 function sendMail(object) {
-    if (!(mailer instanceof mail.GhostMailer) || testing) {
+    if (!(mailer instanceof mail.GhostMailer)) {
         mailer = new mail.GhostMailer();
     }
 

diff --git a/core/server/config/env/config.development.json b/core/server/config/env/config.development.json
@@ -8,5 +8,16 @@
     },
     "paths": {
         "contentPath": "content/"
+    },
+    "privacy": {
+        "useRpcPing": false,
+        "useUpdateCheck": true
+    },
+    "minifyAssets": false,
+    "printErrorStack": true,
+    "caching": {
+        "theme": {
+            "maxAge": 0
+        }
     }
 }
diff --git a/core/server/config/env/config.testing-mysql.json b/core/server/config/env/config.testing-mysql.json
@@ -48,5 +48,10 @@
             "lifetime": 3600,
             "freeRetries":99
         }
-    }
+    },
+    "privacy": {
+        "useTinfoil": true,
+        "useStructuredData": true
+    },
+    "minifyAssets": false
 }
diff --git a/core/server/config/env/config.testing.json b/core/server/config/env/config.testing.json
@@ -45,5 +45,10 @@
             "lifetime": 3600,
             "freeRetries":99
         }
-    }
+    },
+    "privacy": {
+        "useTinfoil": true,
+        "useStructuredData": true
+    },
+    "minifyAssets": false
 }
diff --git a/core/server/config/utils.js b/core/server/config/utils.js
@@ -7,7 +7,13 @@ exports.isPrivacyDisabled = function isPrivacyDisabled(privacyFlag) {
         return false;
     }
 
+    // CASE: disable all privacy features
     if (this.get('privacy').useTinfoil === true) {
+        // CASE: you can still enable single features
+        if (this.get('privacy')[privacyFlag] === true) {
+            return false;
+        }
+
         return true;
     }
 

diff --git a/core/server/data/xml/xmlrpc.js b/core/server/data/xml/xmlrpc.js
@@ -23,8 +23,7 @@ function ping(post) {
         title = post.title,
         url = utils.url.urlFor('post', {post: post}, true);
 
-    // Only ping when in production and not a page
-    if (config.get('env') !== 'production' || post.page || config.isPrivacyDisabled('useRpcPing')) {
+    if (post.page || config.isPrivacyDisabled('useRpcPing')) {
         return;
     }
 

diff --git a/core/server/ghost-server.js b/core/server/ghost-server.js
@@ -42,7 +42,7 @@ GhostServer.prototype.start = function (externalApp) {
     var self = this,
         rootApp = externalApp ? externalApp : self.rootApp,
         socketConfig, socketValues = {
-            path: path.join(config.get('paths').contentPath, process.env.NODE_ENV + '.socket'),
+            path: path.join(config.get('paths').contentPath, config.get('env') + '.socket'),
             permissions: '660'
         };
 
@@ -188,7 +188,7 @@ GhostServer.prototype.closeConnections = function () {
  */
 GhostServer.prototype.logStartMessages = function () {
     // Startup & Shutdown messages
-    if (process.env.NODE_ENV === 'production') {
+    if (config.get('env') === 'production') {
         console.log(
             chalk.red('Currently running Ghost 1.0.0 Alpha, this is NOT suitable for production! \n'),
             chalk.white('Please switch to the stable branch. \n'),
@@ -200,7 +200,7 @@ GhostServer.prototype.logStartMessages = function () {
             chalk.blue('Welcome to the Ghost 1.0.0 Alpha - this version of Ghost is for development only.')
         );
         console.log(
-            chalk.green(i18n.t('notices.httpServer.ghostIsRunningIn', {env: process.env.NODE_ENV})),
+            chalk.green(i18n.t('notices.httpServer.ghostIsRunningIn', {env: config.get('env')})),
             i18n.t('notices.httpServer.listeningOn'),
             config.get('server').socket || config.get('server').host + ':' + config.get('server').port,
             i18n.t('notices.httpServer.urlConfiguredAs', {url: utils.url.urlFor('home', true)}),
@@ -210,7 +210,8 @@ GhostServer.prototype.logStartMessages = function () {
 
     function shutdown() {
         console.log(chalk.red(i18n.t('notices.httpServer.ghostHasShutdown')));
-        if (process.env.NODE_ENV === 'production') {
+
+        if (config.get('env') === 'production') {
             console.log(
                 i18n.t('notices.httpServer.yourBlogIsNowOffline')
             );
@@ -220,6 +221,7 @@ GhostServer.prototype.logStartMessages = function () {
                 moment.duration(process.uptime(), 'seconds').humanize()
             );
         }
+
         process.exit(0);
     }
     // ensure that Ghost exits correctly on Ctrl+C and SIGTERM

diff --git a/core/server/helpers/asset.js b/core/server/helpers/asset.js
@@ -15,9 +15,11 @@ function asset(path, options) {
         isAdmin = options.hash.ghost;
         minify = options.hash.minifyInProduction;
     }
-    if (config.get('env') !== 'production') {
+
+    if (config.get('minifyAssets') === false) {
         minify = false;
     }
+
     return new hbs.handlebars.SafeString(
         getAssetUrl(path, isAdmin, minify)
     );

diff --git a/core/server/middleware/error-handler.js b/core/server/middleware/error-handler.js
@@ -109,8 +109,7 @@ _private.HTMLErrorRenderer = function HTMLErrorRender(err, req, res, /*jshint un
             code: err.statusCode
         };
 
-    // @TODO revisit use of config.get('env') as part of #7488
-    if (err.statusCode === 500 && config.get('env') !== 'production') {
+    if (err.statusCode === 500 && config.get('printErrorStack')) {
         templateData.stack = err.stack;
     }
 

diff --git a/core/server/middleware/static-theme.js b/core/server/middleware/static-theme.js
@@ -20,9 +20,8 @@ function forwardToExpressStatic(req, res, next) {
     if (!req.app.get('activeTheme')) {
         next();
     } else {
-        express.static(
-            path.join(config.getContentPath('themes'), req.app.get('activeTheme')),
-            {maxAge: config.get('env') === 'development' ? 0 : utils.ONE_YEAR_MS}
+        express.static(path.join(config.getContentPath('themes'), req.app.get('activeTheme')),
+            {maxAge: config.get('caching:theme:maxAge') || utils.ONE_YEAR_MS}
         )(req, res, next);
     }
 }

diff --git a/core/server/models/client.js b/core/server/models/client.js
@@ -10,6 +10,7 @@ Client = ghostBookshelf.Model.extend({
     tableName: 'clients',
 
     defaults: function defaults() {
+        // @TODO: we cannot delete this ugly check here, because ALL routing tests rely on a static client secret
         var env = config.get('env'),
             secret = env.indexOf('testing') !== 0 ? crypto.randomBytes(6).toString('hex') : 'not_available';
 

diff --git a/core/server/update-check.js b/core/server/update-check.js
@@ -36,7 +36,6 @@ var crypto   = require('crypto'),
     i18n     = require('./i18n'),
     currentVersion = require('./utils/ghost-version').full,
     internal = {context: {internal: true}},
-    allowedCheckEnvironments = ['development', 'production'],
     checkEndpoint = 'updates.ghost.org';
 
 function updateCheckError(err) {
@@ -89,7 +88,7 @@ function updateCheckData() {
 
     data.ghost_version   = currentVersion;
     data.node_version    = process.versions.node;
-    data.env             = process.env.NODE_ENV;
+    data.env             = config.get('env');
     data.database_type   = config.get('database').client;
     data.email_transport = mailConfig &&
     (mailConfig.options && mailConfig.options.service ?
@@ -208,13 +207,7 @@ function updateCheckResponse(response) {
 }
 
 function updateCheck() {
-    // The check will not happen if:
-    // 1. updateCheck is defined as false in config.js
-    // 2. we've already done a check this session
-    // 3. we're not in production or development mode
-    // TODO: need to remove config.updateCheck in favor of config.privacy.updateCheck in future version (it is now deprecated)
-    if (config.get('updateCheck') === false || config.isPrivacyDisabled('useUpdateCheck') || _.indexOf(allowedCheckEnvironments, process.env.NODE_ENV) === -1) {
-        // No update check
+    if (config.isPrivacyDisabled('useUpdateCheck')) {
         return Promise.resolve();
     } else {
         return api.settings.read(_.extend({key: 'nextUpdateCheck'}, internal)).then(function then(result) {

diff --git a/core/server/utils/gravatar.js b/core/server/utils/gravatar.js
@@ -9,12 +9,7 @@ module.exports.lookup = function lookup(userData, timeout) {
         '?s=250', image;
 
     return new Promise(function gravatarRequest(resolve) {
-        /**
-         * @TODO:
-         *  - mock gravatar in test env globally, do not check for test env!
-         *  - in test/utils/index.js -> mocks.gravatar.enable();
-         */
-        if (config.isPrivacyDisabled('useGravatar') || config.get('env').indexOf('testing') > -1) {
+        if (config.isPrivacyDisabled('useGravatar')) {
             return resolve();
         }
 

diff --git a/core/test/integration/api/api_configuration_spec.js b/core/test/integration/api/api_configuration_spec.js
@@ -40,7 +40,7 @@ describe('Configuration API', function () {
             });
 
             props.fileStorage.should.eql(true);
-            props.useGravatar.should.eql(true);
+            props.useGravatar.should.eql(false);
             props.publicAPI.should.eql(false);
             props.clientId.should.eql('ghost-admin');
             props.clientSecret.should.eql('not_available');

diff --git a/core/test/integration/api/api_mail_spec.js b/core/test/integration/api/api_mail_spec.js
@@ -1,9 +1,8 @@
-var testUtils       = require('../../utils'),
-    configUtils     = require('../../utils/configUtils'),
-    should          = require('should'),
-    i18n            = require('../../../../core/server/i18n'),
-
-    // test data
+var should = require('should'),
+    _ = require('lodash'),
+    testUtils = require('../../utils'),
+    configUtils = require('../../utils/configUtils'),
+    i18n = require('../../../../core/server/i18n'),
     mailData = {
         mail: [{
             message: {
@@ -22,6 +21,16 @@ describe('Mail API', function () {
     afterEach(testUtils.teardown);
     beforeEach(testUtils.setup('perms:mail', 'perms:init'));
 
+    beforeEach(function () {
+        _.each(require.cache, function (value, key) {
+            if (key.match(/server\/api\/mail/)) {
+                delete require.cache[key];
+            }
+        });
+
+        require('../../../server/api/mail');
+    });
+
     afterEach(function () {
         configUtils.restore();
     });

diff --git a/core/test/integration/update_check_spec.js b/core/test/integration/update_check_spec.js
@@ -3,23 +3,19 @@ var _           = require('lodash'),
     should      = require('should'),
     rewire      = require('rewire'),
     uuid        = require('uuid'),
-
-    // Stuff we are testing
+    configUtils      = require('../utils/configUtils'),
     packageInfo      = require('../../../package'),
     updateCheck      = rewire('../../server/update-check'),
     NotificationsAPI = require('../../server/api/notifications');
 
 describe('Update Check', function () {
     describe('Reporting to UpdateCheck', function () {
-        var environmentsOrig;
-
         before(function () {
-            environmentsOrig = updateCheck.__get__('allowedCheckEnvironments');
-            updateCheck.__set__('allowedCheckEnvironments', ['development', 'production', 'testing']);
+            configUtils.set('privacy:useUpdateCheck', true);
         });
 
         after(function () {
-            updateCheck.__set__('allowedCheckEnvironments', environmentsOrig);
+            configUtils.restore();
         });
 
         beforeEach(testUtils.setup('owner', 'posts', 'perms:setting', 'perms:user', 'perms:init'));

diff --git a/core/test/unit/server_utils_spec.js b/core/test/unit/server_utils_spec.js
@@ -369,7 +369,7 @@ describe('Server Utilities', function () {
 
     describe('gravatar-lookup', function () {
         beforeEach(function () {
-            configUtils.set('env', 'production');
+            configUtils.set('privacy:useGravatar', true);
         });
 
         afterEach(function () {

diff --git a/core/test/unit/xmlrpc_spec.js b/core/test/unit/xmlrpc_spec.js
@@ -20,7 +20,7 @@ describe('XMLRPC', function () {
         sandbox = sinon.sandbox.create();
         eventStub = sandbox.stub(events, 'on');
 
-        configUtils.set('env', 'production');
+        configUtils.set('privacy:useRpcPing', true);
     });
 
     afterEach(function () {