refactor admin config to include explicit value types

closes TryGhost#6266 - add "type" to valid keys in configuration api - refactor ember config service to parse values based on provided type
barthc · Jan 19, 2016 · 7d304a0 · 7d304a0
1 parent 1b5b6cf
commit 7d304a0
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 31 deletions.
diff --git a/core/client/app/index.html b/core/client/app/index.html
@@ -32,7 +32,7 @@
     <meta name="msapplication-square310x310logo" content="{{asset "img/large.png" ghost="true"}}" />
 
     {{#each configuration}}
-        <meta name="env-{{this.key}}" content="{{this.value}}" />
+        <meta name="env-{{this.key}}" content="{{this.value}}" data-type="{{this.type}}" />
     {{/each}}
 
     {{#unless skip_google_fonts}}

diff --git a/core/client/app/services/config.js b/core/client/app/services/config.js
@@ -6,23 +6,20 @@ function isNumeric(num) {
     return Ember.$.isNumeric(num);
 }
 
-function _mapType(val) {
+function _mapType(val, type) {
     if (val === '') {
         return null;
-    } else if (val === 'true') {
-        return true;
-    } else if (val === 'false') {
-        return false;
-    } else if (isNumeric(val)) {
+    } else if (type === 'bool') {
+        return (val === 'true') ? true : false;
+    } else if (type === 'int' && isNumeric(val)) {
         return +val;
-    } else if (val.indexOf('{') === 0) {
+    } else if (type === 'json') {
         try {
             return JSON.parse(val);
         } catch (e) {
-            /*jshint unused:false */
             return val;
         }
-    } else {
+    } else { // assume string if type is null or matches nothing else
         return val;
     }
 }
@@ -35,9 +32,11 @@ export default Service.extend(_ProxyMixin, {
         metaConfigTags.each((i, el) => {
             let key = el.name;
             let value = el.content;
+            let type = el.getAttribute('data-type');
+
             let propertyName = key.substring(4);
 
-            config[propertyName] = _mapType(value);
+            config[propertyName] = _mapType(value, type);
         });
 
         return config;

diff --git a/core/server/api/configuration.js b/core/server/api/configuration.js
@@ -8,23 +8,38 @@ var _                  = require('lodash'),
 
     configuration;
 
+function labsFlag(key) {
+    return {
+        value: (config[key] === true),
+        type: 'bool'
+    };
+}
+
 function getValidKeys() {
     var validKeys = {
-            fileStorage: config.fileStorage === false ? false : true,
-            publicAPI: config.publicAPI === true ? true : false,
-            apps: config.apps === true ? true : false,
-            version: config.ghostVersion,
+            fileStorage: {value: (config.fileStorage !== false), type: 'bool'},
+            publicAPI: labsFlag('publicAPI'),
+            apps: {value: (config.apps === true), type: 'bool'},
+            version: {value: config.ghostVersion, type: 'string'},
             environment: process.env.NODE_ENV,
             database: config.database.client,
             mail: _.isObject(config.mail) ? config.mail.transport : '',
-            blogUrl: config.url.replace(/\/$/, ''),
-            blogTitle: config.theme.title,
-            routeKeywords: JSON.stringify(config.routeKeywords)
+            blogUrl: {value: config.url.replace(/\/$/, ''), type: 'string'},
+            blogTitle: {value: config.theme.title, type: 'string'},
+            routeKeywords: {value: JSON.stringify(config.routeKeywords), type: 'json'}
         };
 
     return validKeys;
 }
 
+function formatConfigurationObject(val, key) {
+    return {
+        key: key,
+        value: (_.isObject(val) && _.has(val, 'value')) ? val.value : val,
+        type: _.isObject(val) ? (val.type || null) : null
+    };
+}
+
 /**
  * ## Configuration API Methods
  *
@@ -38,12 +53,7 @@ configuration = {
      * @returns {Promise(Configurations)}
      */
     browse: function browse() {
-        return Promise.resolve({configuration: _.map(getValidKeys(), function (value, key) {
-            return {
-                key: key,
-                value: value
-            };
-        })});
+        return Promise.resolve({configuration: _.map(getValidKeys(), formatConfigurationObject)});
     },
 
     /**
@@ -54,10 +64,7 @@ configuration = {
         var data = getValidKeys();
 
         if (_.has(data, options.key)) {
-            return Promise.resolve({configuration: [{
-                key: options.key,
-                value: data[options.key]
-            }]});
+            return Promise.resolve({configuration: [formatConfigurationObject(data[options.key], options.key)]});
         } else {
             return Promise.reject(new errors.NotFoundError(i18n.t('errors.api.configuration.invalidKey')));
         }

diff --git a/core/server/controllers/admin.js b/core/server/controllers/admin.js
@@ -20,8 +20,8 @@ adminControllers = {
             }).then(function getAPIClient() {
                 return api.clients.read({slug: 'ghost-admin'});
             }).then(function renderIndex(adminClient) {
-                configuration.push({key: 'clientId', value: adminClient.clients[0].slug});
-                configuration.push({key: 'clientSecret', value: adminClient.clients[0].secret});
+                configuration.push({key: 'clientId', value: adminClient.clients[0].slug, type: 'string'});
+                configuration.push({key: 'clientSecret', value: adminClient.clients[0].secret, type: 'string'});
 
                 var apiConfig = _.omit(configuration, function omit(value) {
                     return _.contains(['environment', 'database', 'mail', 'version'], value.key);

diff --git a/core/test/integration/api/api_configuration_spec.js b/core/test/integration/api/api_configuration_spec.js
@@ -59,6 +59,7 @@ describe('Configuration API', function () {
             testUtils.API.checkResponse(response.configuration[0], 'configuration');
             response.configuration[0].key.should.equal('database');
             response.configuration[0].value.should.equal('mysql');
+            response.configuration[0].type.should.be.null();
             /*jshint unused:false */
             done();
         }).catch(function (error) {

diff --git a/core/test/utils/api.js b/core/test/utils/api.js
@@ -7,7 +7,7 @@ var _               = require('lodash'),
     port            = config.server.port,
     schema          = 'http://',
     expectedProperties = {
-        configuration: ['key', 'value'],
+        configuration: ['key', 'value', 'type'],
         posts: ['posts', 'meta'],
         tags: ['tags', 'meta'],
         users: ['users', 'meta'],