Cleaned up theme service (#10884)

refs #10790

- Following TODO in theme index file was waiting for 2 years, and today is the day to cross it out:
- "Reduced the amount of things we expose to the outside world"
- "Made this a nice clean sensible API we can all understand!" - by @ErisDS
- Cleaned exposed methods from themes module
- Removed unused storage getter
- Removed list method
- Removed validate method
- Renamed Storage to ThemeStorage
  - Named the file the same way the class defined inside of it is named
  - Naming was conflicting with coming rename of  `settings` -> `storage`
- Renamed theme settings to storage

core/frontend/services/themes/index.js

@@ -5,14 +5,10 @@ const themeLoader = require('./loader');
 const active = require('./active');
 const activate = require('./activate');
 const validate = require('./validate');
-const Storage = require('./Storage');
+const list = require('./list');
 const settingsCache = require('../../../server/services/settings/cache');
 const engineDefaults = require('./engines/defaults');
 
-let themeStorage;
-
-// @TODO: reduce the amount of things we expose to the outside world
-// Make this a nice clean sensible API we can all understand!
 module.exports = {
     // Init themes module
     // TODO: move this once we're clear what needs to happen here
@@ -78,17 +74,7 @@ module.exports = {
                 common.logging.error(err);
             });
     },
-    // Load themes, soon to be removed and exposed via specific function.
+    getJSON: require('./to-json'),
-    loadAll: themeLoader.loadAllThemes,
-    loadOne: themeLoader.loadOneTheme,
-    get storage() {
-        themeStorage = themeStorage || new Storage();
-
-        return themeStorage;
-    },
-    list: require('./list'),
-    validate: validate,
-    toJSON: require('./to-json'),
     getActive: active.get,
     getApiVersion: function getApiVersion() {
         if (this.getActive()) {
@@ -98,7 +84,7 @@ module.exports = {
         }
     },
     activate: function (themeName) {
-        const loadedTheme = this.list.get(themeName);
+        const loadedTheme = list.get(themeName);
 
         if (!loadedTheme) {
             return Promise.reject(new common.errors.ValidationError({
@@ -115,6 +101,6 @@ module.exports = {
                 return checkedTheme;
             });
     },
-    settings: require('./settings'),
+    storage: require('./storage'),
     middleware: require('./middleware')
 };

core/frontend/services/themes/storage.js

@@ -3,7 +3,7 @@ const fs = require('fs-extra');
 const activate = require('./activate');
 const validate = require('./validate');
 const list = require('./list');
-const Storage = require('./Storage');
+const ThemeStorage = require('./ThemeStorage');
 const themeLoader = require('./loader');
 const toJSON = require('./to-json');
 
@@ -14,13 +14,12 @@ const debug = require('ghost-ignition').debug('api:themes');
 let themeStorage;
 
 const getStorage = () => {
-    themeStorage = themeStorage || new Storage();
+    themeStorage = themeStorage || new ThemeStorage();
 
     return themeStorage;
 };
 
 module.exports = {
-    get: require('./to-json'),
     getZip: (themeName) => {
         const theme = list.get(themeName);
 

core/server/api/v0.1/themes.js

@@ -26,7 +26,7 @@ themes = {
             // Main action
             .then(() => {
                 // Return JSON result
-                return themeService.settings.get();
+                return themeService.getJSON();
             });
     },
 
@@ -52,7 +52,7 @@ themes = {
             })
             .then((checkedTheme) => {
                 // Return JSON result
-                return themeService.settings.get(themeName, checkedTheme);
+                return themeService.getJSON(themeName, checkedTheme);
             });
     },
 
@@ -72,7 +72,7 @@ themes = {
             .handlePermissions('themes', 'add')(options)
             // Validation
             .then(() => {
-                return themeService.settings.setFromZip(zip);
+                return themeService.storage.setFromZip(zip);
             })
             .then((theme) => {
                 common.events.emit('theme.uploaded');
@@ -87,7 +87,7 @@ themes = {
         // Permissions
             .handlePermissions('themes', 'read')(options)
             .then(() => {
-                return themeService.settings.getZip(themeName);
+                return themeService.storage.getZip(themeName);
             });
     },
 
@@ -103,7 +103,7 @@ themes = {
             .handlePermissions('themes', 'destroy')(options)
             // Validation
             .then(() => {
-                return themeService.settings.destroy(themeName);
+                return themeService.storage.destroy(themeName);
             });
     }
 };

core/server/api/v2/themes.js

@@ -8,7 +8,7 @@ module.exports = {
     browse: {
         permissions: true,
         query() {
-            return themeService.settings.get();
+            return themeService.getJSON();
         }
     },
 
@@ -41,7 +41,7 @@ module.exports = {
                         .then(() => checkedTheme);
                 })
                 .then((checkedTheme) => {
-                    return themeService.settings.get(themeName, checkedTheme);
+                    return themeService.getJSON(themeName, checkedTheme);
                 });
         }
     },
@@ -60,7 +60,7 @@ module.exports = {
                 name: frame.file.originalname
             };
 
-            return themeService.settings.setFromZip(zip)
+            return themeService.storage.setFromZip(zip)
                 .then((theme) => {
                     common.events.emit('theme.uploaded');
                     return theme;
@@ -85,7 +85,7 @@ module.exports = {
         query(frame) {
             let themeName = frame.options.name;
 
-            return themeService.settings.getZip(themeName);
+            return themeService.storage.getZip(themeName);
         }
     },
 
@@ -108,7 +108,7 @@ module.exports = {
         query(frame) {
             let themeName = frame.options.name;
 
-            return themeService.settings.destroy(themeName);
+            return themeService.storage.destroy(themeName);
         }
     }
 };

core/test/unit/helpers/body_class_spec.js

@@ -1,5 +1,5 @@
 var should = require('should'),
-    themeList = require('../../../frontend/services/themes').list,
+    themeList = require('../../../frontend/services/themes/list'),
 
 // Stuff we are testing
     helpers = require('../../../frontend/helpers');

core/test/unit/services/themes/list_spec.js

@@ -1,8 +1,7 @@
 var should = require('should'),
     sinon = require('sinon'),
     _ = require('lodash'),
-    themes = require('../../../../frontend/services/themes'),
+    themeList = require('../../../../frontend/services/themes/list');
-    themeList = themes.list;
 
 describe('Themes', function () {
     afterEach(function () {

core/test/unit/services/themes/loader_spec.js

@@ -4,8 +4,8 @@ var should = require('should'),
     tmp = require('tmp'),
     join = require('path').join,
     config = require('../../../../server/config'),
-    themes = require('../../../../frontend/services/themes'),
+    loader = require('../../../../frontend/services/themes/loader'),
-    themeList = themes.list;
+    themeList = require('../../../../frontend/services/themes/list');
 
 describe('Themes', function () {
     afterEach(function () {
@@ -36,7 +36,7 @@ describe('Themes', function () {
                 fs.writeFileSync(join(themePath.name, 'casper', 'index.hbs'));
                 fs.writeFileSync(join(themePath.name, 'casper', 'partials', 'navigation.hbs'));
 
-                themes.loadAll()
+                loader.loadAllThemes()
                     .then(function (result) {
                         var themeResult = themeList.getAll();
 
@@ -69,7 +69,7 @@ describe('Themes', function () {
                     JSON.stringify({name: 'casper', version: '0.1.2'})
                 );
 
-                themes.loadAll()
+                loader.loadAllThemes()
                     .then(function (result) {
                         var themeResult = themeList.getAll();
 
@@ -111,7 +111,7 @@ describe('Themes', function () {
                 fs.mkdirSync(join(themePath.name, 'not-casper'));
                 fs.writeFileSync(join(themePath.name, 'not-casper', 'index.hbs'));
 
-                themes.loadOne('casper')
+                loader.loadOneTheme('casper')
                     .then(function (themeResult) {
                         themeResult.should.eql({
                             name: 'casper',
@@ -129,7 +129,7 @@ describe('Themes', function () {
                 fs.writeFileSync(join(themePath.name, 'casper.zip'));
                 fs.writeFileSync(join(themePath.name, '.DS_Store'));
 
-                themes.loadOne('casper')
+                loader.loadOneTheme('casper')
                     .then(function () {
                         done('Should have thrown an error');
                     })

core/test/unit/services/themes/validate_spec.js

@@ -2,8 +2,7 @@ const should = require('should');
 const sinon = require('sinon');
 const _ = require('lodash');
 
-const themes = require('../../../../frontend/services/themes');
+const validate = require('../../../../frontend/services/themes/validate');
-const validate = themes.validate;
 
 const gscan = require('gscan');