From 367ec0fa71e4eff6f14f7ee741c47d8300b7cbae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 1 Jul 2014 10:11:26 +0200 Subject: [PATCH 1/8] Omit extra properties from json files Fix ComponentDefinition.saveToFs to remove extra properties before writing json files (config, models, datasources). --- models/component-definition.js | 5 +++++ test/component-definition.js | 21 +++++++++++++++++---- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/models/component-definition.js b/models/component-definition.js index 998682a6..8450d851 100644 --- a/models/component-definition.js +++ b/models/component-definition.js @@ -150,6 +150,7 @@ ComponentDefinition.saveToFs = function(cache, componentDef, cb) { var configFile = ComponentDefinition.getConfigFile(componentName, componentDef); // remove extra data that shouldn't be persisted to the fs delete componentDef.configFile; + delete componentDef.name; configFile.data = componentDef; filesToSave.push(configFile); @@ -173,7 +174,9 @@ ComponentDefinition.saveToFs = function(cache, componentDef, cb) { if(dataSourceDef.componentName === componentName) { dataSourcePath = DataSourceDefinition.getPath(componentName, dataSourceDef); dataSoureConfig[dataSourceDef.name] = dataSourceDef; + // remove extra data that shouldn't be persisted to the fs delete dataSourceDef.name; + delete dataSourceDef.componentName; } }); @@ -189,7 +192,9 @@ ComponentDefinition.saveToFs = function(cache, componentDef, cb) { cachedComponentModels.forEach(function(componentModel) { componentModelsConfig[componentModel.name] = componentModel; + // remove extra data that shouldn't be persisted to the fs delete componentModel.name; + delete componentModel.componentName; }); filesToSave.push(componentModelFile); diff --git a/test/component-definition.js b/test/component-definition.js index 6b0b6819..7b6ad63f 100644 --- a/test/component-definition.js +++ b/test/component-definition.js @@ -1,8 +1,21 @@ -var async = require('async'); -var app = require('../app'); -var ComponentDefinition = app.models.ComponentDefinition; -var ConfigFile = app.models.ConfigFile; +var fs = require('fs-extra'); describe('ComponentDefinition', function() { + describe('componentDefinition.saveToFs', function() { + beforeEach(givenBasicWorkspace); + it('omits `name` in config.json', function() { + var content = fs.readJsonFileSync(SANDBOX + '/rest/config.json'); + expect(content).to.not.have.property('name'); + }); + it('omits `componentName` in models.json', function() { + var content = fs.readJsonFileSync(SANDBOX + '/rest/models.json'); + expect(content.user).to.not.have.property('componentName'); + }); + + it('omits `componentName` in datasources.json', function() { + var content = fs.readJsonFileSync(SANDBOX + '/rest/datasources.json'); + expect(content.db).to.not.have.property('componentName'); + }); + }); }); From 09a6ffa31b16b1b484baad93e3031f06bbc79ba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 1 Jul 2014 10:35:02 +0200 Subject: [PATCH 2/8] Fix `name` in the root `package.json` Fix `Workspace.createFromTemplate` to set the root package name to the value supplied by the user, instead of hard-coded `.`. --- models/workspace.js | 3 ++- test/workspace.js | 14 +++++--------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/models/workspace.js b/models/workspace.js index ddaf4b4b..d0b6327e 100644 --- a/models/workspace.js +++ b/models/workspace.js @@ -39,6 +39,7 @@ Workspace.addComponent = function(options, cb) { var template; var templateName = options.template || DEFAULT_TEMPLATE; var name = options.name || templateName; + var packageName = options.packageName || name; if(options.root) name = '.'; var fileTemplatesDir = path.join(TEMPLATE_DIR, templateName, 'template'); @@ -84,7 +85,7 @@ Workspace.addComponent = function(options, cb) { } if(template.package) { - template.package.name = name; + template.package.name = packageName; setComponentName(template.package); steps.push(function(cb) { PackageDefinition.create(template.package, cb); diff --git a/test/workspace.js b/test/workspace.js index cb10d69d..eb7c645e 100644 --- a/test/workspace.js +++ b/test/workspace.js @@ -1,4 +1,5 @@ var async = require('async'); +var fs = require('fs-extra'); var app = require('../app'); var TestDataBuilder = require('loopback-testing').TestDataBuilder; var Workspace = app.models.Workspace; @@ -61,15 +62,10 @@ describe('Workspace', function() { expect(dataSourceNames).to.contain('db'); }); - it('should create a runnable set of components', function (done) { - ComponentDefinition.findOne({ - where: { - name: '.' - } - }, function(err, component) { - if(err) return done(err); - done(); - }); + it('should set correct name in package.json', function() { + var pkg = fs.readJsonFileSync(SANDBOX + '/package.json'); + // project name is hard-coded in support.js as 'sandbox' + expect(pkg.name).to.equal('sandbox'); }); }); From edc4abbd8e76213c333a34ee13f0143c0e51e0ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 1 Jul 2014 10:44:02 +0200 Subject: [PATCH 3/8] Fix serialization of component models The `models.json` should contain only models of the current component. --- models/component-definition.js | 1 + test/component-definition.js | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/models/component-definition.js b/models/component-definition.js index 8450d851..eb090b85 100644 --- a/models/component-definition.js +++ b/models/component-definition.js @@ -191,6 +191,7 @@ ComponentDefinition.saveToFs = function(cache, componentDef, cb) { var componentModelsConfig = componentModelFile.data = {}; cachedComponentModels.forEach(function(componentModel) { + if (componentModel.componentName !== componentName) return; componentModelsConfig[componentModel.name] = componentModel; // remove extra data that shouldn't be persisted to the fs delete componentModel.name; diff --git a/test/component-definition.js b/test/component-definition.js index 7b6ad63f..7182dee1 100644 --- a/test/component-definition.js +++ b/test/component-definition.js @@ -17,5 +17,12 @@ describe('ComponentDefinition', function() { var content = fs.readJsonFileSync(SANDBOX + '/rest/datasources.json'); expect(content.db).to.not.have.property('componentName'); }); + + it('saves component models to correct file', function() { + var restModels = fs.readJsonFileSync(SANDBOX + '/rest/models.json'); + var serverModels = fs.readJsonFileSync(SANDBOX + '/server/models.json'); + expect(Object.keys(restModels), 'rest models').to.not.be.empty; + expect(Object.keys(serverModels), 'server models').to.be.empty; + }); }); }); From 8b93ffadeb2f19218a0c420e827543435a5267ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 1 Jul 2014 10:48:05 +0200 Subject: [PATCH 4/8] Move restApiRoot from api-server to server The root project does not have an app to load config.json. The configuration of REST API endpoint belongs to `server` component, because that's the component registering all different sub-apps. --- templates/api-server/component.js | 1 - templates/server/component.js | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/api-server/component.js b/templates/api-server/component.js index 656c02cc..7bcdd35c 100644 --- a/templates/api-server/component.js +++ b/templates/api-server/component.js @@ -4,7 +4,6 @@ var template = module.exports; var component = template.component = { - restApiRoot: '/api' }; template.config = { diff --git a/templates/server/component.js b/templates/server/component.js index c8c2d9aa..c5b17b36 100644 --- a/templates/server/component.js +++ b/templates/server/component.js @@ -4,6 +4,7 @@ var template = module.exports; var component = template.component = { + restApiRoot: '/api', host: 'localhost' }; From 9d81b2653ab19d8fa7406c54e352cdb76f31ab4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 1 Jul 2014 13:50:40 +0200 Subject: [PATCH 5/8] Omit json config files in the root component The `api-server` root component should not include the following files, since there is no `app` to read them during boot: - config.json - datasources.json - models.json --- models/component-definition.js | 94 +++++++++++++++++++++------------- test/component-definition.js | 9 ++++ test/data-source-definition.js | 7 +-- test/support.js | 4 +- 4 files changed, 74 insertions(+), 40 deletions(-) diff --git a/models/component-definition.js b/models/component-definition.js index eb090b85..6b67b87f 100644 --- a/models/component-definition.js +++ b/models/component-definition.js @@ -61,7 +61,15 @@ ComponentDefinition.loadIntoCache = function(cache, componentName, components, c cb(); }); } else { - debug('component configFile does not exist'); + steps.push(function(cb) { + var componentData = { + name: componentName, + configFile: path.join(componentName, 'config.json') + }; + debug('adding to cache component entry [%s]', componentData.configFile); + ComponentDefinition.addToCache(cache, componentName, componentData); + cb(); + }); } if(componentModels) { @@ -147,13 +155,17 @@ ComponentDefinition.saveToFs = function(cache, componentDef, cb) { var debug = require('debug')('workspace:component:save:' + componentName); - var configFile = ComponentDefinition.getConfigFile(componentName, componentDef); - // remove extra data that shouldn't be persisted to the fs - delete componentDef.configFile; - delete componentDef.name; - configFile.data = componentDef; + var hasApp = ComponentDefinition.hasApp(componentDef); - filesToSave.push(configFile); + if (hasApp) { + var configFile = ComponentDefinition.getConfigFile(componentName, componentDef); + // remove extra data that shouldn't be persisted to the fs + delete componentDef.configFile; + delete componentDef.name; + configFile.data = componentDef; + + filesToSave.push(configFile); + } PackageDefinition.allFromCache(cache).forEach(function(package) { if(package.componentName === componentName) { @@ -166,39 +178,41 @@ ComponentDefinition.saveToFs = function(cache, componentDef, cb) { } }); - var dataSoureConfig = {}; - var dataSourcePath = path.join(componentName, 'datasources.json'); - var cachedDataSources = DataSourceDefinition.allFromCache(cache); - - cachedDataSources.forEach(function(dataSourceDef) { - if(dataSourceDef.componentName === componentName) { - dataSourcePath = DataSourceDefinition.getPath(componentName, dataSourceDef); - dataSoureConfig[dataSourceDef.name] = dataSourceDef; - // remove extra data that shouldn't be persisted to the fs - delete dataSourceDef.name; - delete dataSourceDef.componentName; - } - }); + if (hasApp) { + var dataSoureConfig = {}; + var dataSourcePath = path.join(componentName, 'datasources.json'); + var cachedDataSources = DataSourceDefinition.allFromCache(cache); + + cachedDataSources.forEach(function(dataSourceDef) { + if (dataSourceDef.componentName === componentName) { + dataSourcePath = DataSourceDefinition.getPath(componentName, dataSourceDef); + dataSoureConfig[dataSourceDef.name] = dataSourceDef; + // remove extra data that shouldn't be persisted to the fs + delete dataSourceDef.name; + delete dataSourceDef.componentName; + } + }); - filesToSave.push(new ConfigFile({ - path: dataSourcePath, - data: dataSoureConfig - })); + filesToSave.push(new ConfigFile({ + path: dataSourcePath, + data: dataSoureConfig + })); - var cachedComponentModels = ComponentModel.allFromCache(cache); - var componentModelsPath = path.join(componentName, ComponentModel.settings.defaultConfigFile); - var componentModelFile = new ConfigFile({path: componentModelsPath}); // models.json - var componentModelsConfig = componentModelFile.data = {}; + var cachedComponentModels = ComponentModel.allFromCache(cache); + var componentModelsPath = path.join(componentName, ComponentModel.settings.defaultConfigFile); + var componentModelFile = new ConfigFile({path: componentModelsPath}); // models.json + var componentModelsConfig = componentModelFile.data = {}; - cachedComponentModels.forEach(function(componentModel) { - if (componentModel.componentName !== componentName) return; - componentModelsConfig[componentModel.name] = componentModel; - // remove extra data that shouldn't be persisted to the fs - delete componentModel.name; - delete componentModel.componentName; - }); + cachedComponentModels.forEach(function(componentModel) { + if (componentModel.componentName !== componentName) return; + componentModelsConfig[componentModel.name] = componentModel; + // remove extra data that shouldn't be persisted to the fs + delete componentModel.name; + delete componentModel.componentName; + }); - filesToSave.push(componentModelFile); + filesToSave.push(componentModelFile); + } var cachedModels = ModelDefinition.allFromCache(cache); @@ -222,3 +236,11 @@ ComponentDefinition.saveToFs = function(cache, componentDef, cb) { cb(); }); } + +ComponentDefinition.hasApp = function(componentDef) { + // At the moment, the root component does not have `app.js`, + // all other components (rest, server) have their app.js + // In the future, we should read this from component, + // e.g. package.json > loopback-workspace > app: true|false + return componentDef.name !== '.'; +}; diff --git a/test/component-definition.js b/test/component-definition.js index 7182dee1..1ad73f80 100644 --- a/test/component-definition.js +++ b/test/component-definition.js @@ -24,5 +24,14 @@ describe('ComponentDefinition', function() { expect(Object.keys(restModels), 'rest models').to.not.be.empty; expect(Object.keys(serverModels), 'server models').to.be.empty; }); + + it('omits json config files in the root of api-server component', function() { + var files = fs.readdirSync(SANDBOX); + expect(files).to.not.include.members([ + 'config.json', + 'datasources.json', + 'models.json'] + ); + }); }); }); diff --git a/test/data-source-definition.js b/test/data-source-definition.js index 22603678..3317421b 100644 --- a/test/data-source-definition.js +++ b/test/data-source-definition.js @@ -13,18 +13,19 @@ describe('DataSourceDefinition', function() { describe('DataSourceDefinition.create(def, cb)', function () { beforeEach(givenEmptyWorkspace); beforeEach(function(done) { + var emptyComponent = this.emptyComponent; this.configFile = new ConfigFile({ - path: 'datasources.json' + path: emptyComponent + '/datasources.json' }); async.parallel([function(cb) { DataSourceDefinition.create({ - componentName: '.', + componentName: emptyComponent, name: 'foo', connector: 'memory' }, cb); }, function(cb) { DataSourceDefinition.create({ - componentName: '.', + componentName: emptyComponent, name: 'bar', connector: 'memory' }, cb); diff --git a/test/support.js b/test/support.js index 967c1169..602aef49 100644 --- a/test/support.js +++ b/test/support.js @@ -56,12 +56,14 @@ givenFile = function(name, pathToFile) { } givenEmptyWorkspace = function(cb) { + var test = this; + test.emptyComponent = 'empty'; resetWorkspace(function(err) { if(err) return cb(err); givenEmptySandbox(function(err) { if(err) return cb(err); models.ComponentDefinition.create({ - name: '.' + name: test.emptyComponent }, cb); }); }); From 750d29d8f7123fdce17bb25df15aa32df795d37f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 1 Jul 2014 14:39:52 +0200 Subject: [PATCH 6/8] templates: clean up + upgrade to loopback-boot 2.0 - Include `loopback-boot 2.x` in the root `package.json` - Include express 4.x middleware packages in the root `package.json` - Drop `email` datasource. It is no longer needed, since loopback-boot does not call `loopback.autoAttach`. - Rename model names in `rest/models.json` to reference the built-in loopback models. Make internal models like `AccessToken` not public. - Refactor the `server` template to use the boot-based registration of middleware (see loopback-example-full-stack). --- templates/api-server/component.js | 3 ++ templates/rest/component.js | 40 ++++++------------- .../rest/template/boot/authentication.js | 8 ++-- templates/server/template/boot/explorer.js | 23 +++++++++++ templates/server/template/boot/rest-api.js | 5 +++ templates/server/template/server.js | 24 +++++------ test/component-definition.js | 2 +- test/workspace.js | 1 - 8 files changed, 59 insertions(+), 47 deletions(-) create mode 100644 templates/server/template/boot/explorer.js create mode 100644 templates/server/template/boot/rest-api.js diff --git a/templates/api-server/component.js b/templates/api-server/component.js index 7bcdd35c..cfa2b7ad 100644 --- a/templates/api-server/component.js +++ b/templates/api-server/component.js @@ -14,7 +14,10 @@ template.package = { "version": "0.0.0", "main": "server/server.js", "dependencies": { + "compression": "^1.0.3", + "errorhandler": "^1.1.1", "loopback": "~2.0.0-beta3", + "loopback-boot": "~2.0.0-beta1", "loopback-datasource-juggler": "~2.0.0-beta2" }, "optionalDependencies": { diff --git a/templates/rest/component.js b/templates/rest/component.js index c427beab..441a2c4d 100644 --- a/templates/rest/component.js +++ b/templates/rest/component.js @@ -12,51 +12,37 @@ template.package = { template.componentModels = [ { - name: 'user', + name: 'User', dataSource: 'db' }, { - name: 'user', - dataSource: 'db' + name: 'AccessToken', + dataSource: 'db', + public: false }, { - name: 'acl', - dataSource: 'db' + name: 'ACL', + dataSource: 'db', + public: false }, { - name: 'roleMapping', - dataSource: 'db' + name: 'RoleMapping', + dataSource: 'db', + public: false }, { - name: 'role', - dataSource: 'db' + name: 'Role', + dataSource: 'db', + public: false } ]; template.relations = [ - { - fromModel: 'user', - model: 'access-token', - type: 'hasMany', - foreignKey: 'userId' - }, - { - fromModel: 'role', - type: 'hasMany', - model: 'roleMapping', - foreignKey: 'roleId' - } ]; template.datasources = [ { name: 'db', - defaultForType: 'db', connector: 'memory' - }, - { - name: 'mail', - defaultForType: 'mail', - connector: 'mail' } ]; diff --git a/templates/rest/template/boot/authentication.js b/templates/rest/template/boot/authentication.js index 9219ffed..d9e17615 100644 --- a/templates/rest/template/boot/authentication.js +++ b/templates/rest/template/boot/authentication.js @@ -1,4 +1,4 @@ -var rest = require('../rest'); - -// enable authentication -rest.enableAuth(); +module.exports = function enableAuthentication(rest) { + // enable authentication + rest.enableAuth(); +}; diff --git a/templates/server/template/boot/explorer.js b/templates/server/template/boot/explorer.js new file mode 100644 index 00000000..67e3ca73 --- /dev/null +++ b/templates/server/template/boot/explorer.js @@ -0,0 +1,23 @@ +module.exports = function mountLoopBackExplorer(server) { + var explorer; + try { + explorer = require('loopback-explorer'); + } catch(err) { + console.log( + 'Run `npm install loopback-explorer` to enable the LoopBack explorer' + ); + return; + } + + var restApp = require('../../rest'); + var restApiRoot = server.get('restApiRoot'); + + var explorerApp = explorer(restApp, { basePath: restApiRoot }); + server.use('/explorer', explorerApp); + server.once('started', function(baseUrl) { + // express 4.x (loopback 2.x) uses `mountpath` + // express 3.x (loopback 1.x) uses `route` + var explorerPath = explorerApp.mountpath || explorerApp.route; + console.log('Browse your REST API at %s%s', baseUrl, explorerPath); + }); +}; diff --git a/templates/server/template/boot/rest-api.js b/templates/server/template/boot/rest-api.js new file mode 100644 index 00000000..4ad3ab0f --- /dev/null +++ b/templates/server/template/boot/rest-api.js @@ -0,0 +1,5 @@ +module.exports = function mountRestApi(server) { + var restApp = require('../../rest'); + var restApiRoot = server.get('restApiRoot'); + server.use(restApiRoot, restApp); +}; diff --git a/templates/server/template/server.js b/templates/server/template/server.js index acfc0acd..36147165 100644 --- a/templates/server/template/server.js +++ b/templates/server/template/server.js @@ -1,25 +1,21 @@ -var path = require('path'); var loopback = require('loopback'); var boot = require('loopback-boot'); -var explorer = require('loopback-explorer'); var app = module.exports = loopback(); -boot(app, __dirname); - -// middleware +// request pre-processing middleware app.use(loopback.compress()); -// mount the REST API and the API explorer -var restApp = require('../rest'); -var restApiRoot = app.get('restApiRoot'); -app.use(restApiRoot, restApp); +// -- Add your pre-processing middleware here -- + +// boot scripts mount components like REST API +boot(app, __dirname); -var explorerApp = explorer(restApp, { basePath: restApiRoot }); -app.use('/explorer', explorerApp); -app.once('started', function(baseUrl) { - console.log('Browse your REST API at %s%s', baseUrl, explorerApp.mountpath); -}); +// -- Mount static files here-- +// All static middleware should be registered at the end, as all requests +// passing the static middleware are hitting the file system +// Example: +// app.use(loopback.static(path.resolve(__dirname', '../client-app'))); // Requests that get this far won't be handled // by any middleware. Convert them into a 404 error diff --git a/test/component-definition.js b/test/component-definition.js index 1ad73f80..a17513e9 100644 --- a/test/component-definition.js +++ b/test/component-definition.js @@ -10,7 +10,7 @@ describe('ComponentDefinition', function() { it('omits `componentName` in models.json', function() { var content = fs.readJsonFileSync(SANDBOX + '/rest/models.json'); - expect(content.user).to.not.have.property('componentName'); + expect(content.User).to.not.have.property('componentName'); }); it('omits `componentName` in datasources.json', function() { diff --git a/test/workspace.js b/test/workspace.js index eb7c645e..a61b90a3 100644 --- a/test/workspace.js +++ b/test/workspace.js @@ -58,7 +58,6 @@ describe('Workspace', function() { it('it should create a set of data source definitions', function() { var dataSourceNames = toNames(this.dataSources); - expect(dataSourceNames).to.contain('mail'); expect(dataSourceNames).to.contain('db'); }); From 8722483e9a96c67c057468460353bdd78cdd8f61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 1 Jul 2014 15:09:40 +0200 Subject: [PATCH 7/8] Mark all `componentName` properties as required. --- models.json | 3 ++- models/component-model.js | 4 +++- models/data-source-definition.js | 4 +++- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/models.json b/models.json index b645a871..965ba551 100644 --- a/models.json +++ b/models.json @@ -16,6 +16,7 @@ }, "PackageDefinition": { "properties": { + "componentName": {"type": "string", "required": true} }, "public": false, "dataSource": "db", @@ -94,7 +95,7 @@ "scopes": "array", "public": "boolean", "indexes": "array", - "componentName": "string", + "componentName": {"type": "string", "required": true}, "dataSource": "string" }, "public": true, diff --git a/models/component-model.js b/models/component-model.js index 1b198ac1..c5626dac 100644 --- a/models/component-model.js +++ b/models/component-model.js @@ -12,9 +12,11 @@ var ComponentModel = app.models.ComponentModel; /** * - `name` is required and must be unique per `ComponentDefinition` + * - `componentName` is required and must refer to an existing component * * @header Property Validation */ ComponentModel.validatesUniquenessOf('name', { scopedTo: ['component'] }); -ComponentModel.validatesPresenceOf('name'); \ No newline at end of file +ComponentModel.validatesPresenceOf('name'); +ComponentModel.validatesPresenceOf('componentName'); diff --git a/models/data-source-definition.js b/models/data-source-definition.js index 6eddec4c..36b38c6b 100644 --- a/models/data-source-definition.js +++ b/models/data-source-definition.js @@ -19,12 +19,14 @@ var DataSourceDefinition = app.models.DataSourceDefinition; /** * - `name` must be unique per `ComponentDefinition` * - `name` and `connector` are required - * + * - `componentName` is required and must refer to an existing component + * * @header Property Validation */ DataSourceDefinition.validatesUniquenessOf('name', { scopedTo: ['componentName'] }); DataSourceDefinition.validatesPresenceOf('name', 'connector'); +DataSourceDefinition.validatesPresenceOf('componentName'); /** * Test the datasource definition connection. From feb3f5a794fe6c7fc3b7016aeb7ef94f233db3e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 1 Jul 2014 15:59:48 +0200 Subject: [PATCH 8/8] Implement Workspace.isValidDir A basic implementation that checks whether all components can be loaded and whether there is at least one component present. --- connector.js | 18 ++++++++++-------- models/config-file.js | 3 +++ models/workspace.js | 20 ++++++++++++++++++++ test/workspace.js | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 8 deletions(-) diff --git a/connector.js b/connector.js index 244eae4b..a5fd824f 100644 --- a/connector.js +++ b/connector.js @@ -17,23 +17,25 @@ connector.saveToFile = function() { connector.loadFromFile = function() { var cb = arguments[arguments.length - 1]; - var loader = connector.loader; + var recursiveCall = !!connector.loader; - if(loader) { - loader.once('complete', cb); - loader.once('error', cb); - return; + if (!recursiveCall) { + connector.loader = new EventEmitter(); + connector.loader.setMaxListeners(100); } - loader = connector.loader = new EventEmitter(); + var loader = connector.loader; + loader.once('complete', cb); + loader.once('error', cb); + + if (recursiveCall) return; + var done = function(err) { if (err) loader.emit('error', err); else loader.emit('complete'); - connector.loader = null; - cb(err); }; // reset the cache diff --git a/models/config-file.js b/models/config-file.js index ef1f30e0..fc8b67c7 100644 --- a/models/config-file.js +++ b/models/config-file.js @@ -70,6 +70,9 @@ ConfigFile.prototype.load = function(cb) { function load(exists, cb) { if(exists) { fs.readJson(absolutePath, function(err, data) { + if (err && err.name === 'SyntaxError') { + err.message = 'Cannot parse ' + configFile.path + ': ' + err.message; + } cb(err, err ? undefined : data); }); } else { diff --git a/models/workspace.js b/models/workspace.js index d0b6327e..1e9d699a 100644 --- a/models/workspace.js +++ b/models/workspace.js @@ -180,3 +180,23 @@ var staticConnectorList = require('../available-connectors'); Workspace.listAvailableConnectors = function(cb) { cb(null, staticConnectorList); }; + +/** + * Check if the project is a valid directory. + * The callback is called with no arguments when the project is valid. + * @param {function(Error=)} cb + */ +Workspace.isValidDir = function(cb) { + // Every call of `Model.find()` triggers reload from the filesystem + // This allows us to catch basic errors in config files + ComponentDefinition.find(function(err, list) { + if (err) { + cb(err); + } else if (!list.length) { + cb(new Error('Invalid workspace: no components found.')); + } else { + // TODO(bajtos) Add more sophisticated validation based on component types + cb(); + } + }); +}; diff --git a/test/workspace.js b/test/workspace.js index a61b90a3..a54d8f0a 100644 --- a/test/workspace.js +++ b/test/workspace.js @@ -85,4 +85,36 @@ describe('Workspace', function() { expect(names).to.contain('memory'); }); }); + + describe('Workspace.isValidDir(cb)', function() { + beforeEach(resetWorkspace); + beforeEach(givenEmptySandbox); + + it('returns no errors for a valid workspace dir', function(done) { + givenBasicWorkspace(function(err) { + if (err) return done(err); + Workspace.isValidDir(function(err) { + // the test passes when no error is reported + done(err); + }); + }); + }); + + it('should fail when the directory is empty', function(done) { + Workspace.isValidDir(function(err) { + expect(err && err.message) + .to.match(/Invalid workspace: no components found/); + done(); + }); + }); + + it('should fail when a json file is malformed', function(done) { + fs.writeFileSync(SANDBOX + '/package.json', '{malformed}', 'utf-8'); + Workspace.isValidDir(function(err) { + expect(err && err.message) + .to.match(/Cannot parse package.json/); + done(); + }); + }); + }); });