Merge pull request #11 from strongloop/model-boot-improvements

[2.0] Model boot improvements

docs/configuration.md

@@ -165,7 +165,7 @@ All code samples are referring to the sample project described above.
 
   *models.json*
 
-  ```js
+  ```json
     {
       "car": {
         "dataSource": "db"
@@ -176,7 +176,6 @@ All code samples are referring to the sample project described above.
  2. Change per-model javascript files to export a function that adds
  custom methods to the model class.
 
-
   *models/car.js*
 
   ```js
@@ -188,18 +187,20 @@ All code samples are referring to the sample project described above.
     };
   ```
 
- 4. Modify the boot configuration to list the directory containing
+ 3. If your model definitions are not in `./models`, then add an entry
-  model definitions.
+  to `models.json` to specify the paths where to look for model definitions.
 
- ```js
+  *models.json*
-    var loopback = require('loopback');
-    var boot = require('loopback-boot');
 
-    var app = loopback();
+  ```json
-    boot(app, {
+  {
-     appRootDir: __dirname,
+      "_meta": {
-     modelSources: ['./models']
+        "sources": ["./custom/path/to/models"]
-   });
+      },
+      "Car": {
+        "dataSource": "db"
+      }
+  }
   ```
 
 #### Attaching built-in models

lib/compiler.js

@@ -1,6 +1,7 @@
 var assert = require('assert');
 var fs = require('fs');
 var path = require('path');
+var toposort = require('toposort');
 var ConfigLoader = require('./config-loader');
 var debug = require('debug')('loopback:boot:compiler');
 
@@ -42,9 +43,12 @@ module.exports = function compile(options) {
   // require directories
   var bootScripts = findScripts(path.join(appRootDir, 'boot'));
 
-  var modelSources = options.modelSources || ['./models'];
+  var modelsMeta = modelsConfig._meta || {};
+  delete modelsConfig._meta;
+
+  var modelSources = modelsMeta.sources || ['./models'];
   var modelInstructions = buildAllModelInstructions(
-    appRootDir, modelsConfig, modelSources);
+    modelsRootDir, modelsConfig, modelSources);
 
   return {
     app: appConfig,
@@ -173,15 +177,15 @@ function addAllBaseModels(registry, modelNames) {
 
   while (modelNames.length) {
     var name = modelNames.shift();
+
+    if (visited[name]) continue;
+    visited[name] = true;
     result.push(name);
 
     var definition = registry[name] && registry[name].definition;
     if (!definition) continue;
 
-    var base = definition.base || definition.options && definition.options.base;
+    var base = getBaseModelName(definition);
-    if (!base || base in visited) continue;
-
-    visited[base] = true;
 
     // ignore built-in models like User
     if (!registry[base]) continue;
@@ -192,9 +196,37 @@ function addAllBaseModels(registry, modelNames) {
   return result;
 }
 
+function getBaseModelName(modelDefinition) {
+  if (!modelDefinition)
+    return undefined;
+
+  return modelDefinition.base ||
+    modelDefinition.options && modelDefinition.options.base;
+}
+
 function sortByInheritance(instructions) {
-  // TODO implement topological sort
+  // create edges Base name -> Model name
-  return instructions.reverse();
+  var edges = instructions
+    .map(function(inst) {
+      return [getBaseModelName(inst.definition), inst.name];
+    });
+
+  var sortedNames = toposort(edges);
+
+  var instructionsByModelName = {};
+  instructions.forEach(function(inst) {
+    instructionsByModelName[inst.name] = inst;
+  });
+
+  return sortedNames
+    // convert to instructions
+    .map(function(name) {
+      return instructionsByModelName[name];
+    })
+    // remove built-in models
+    .filter(function(inst) {
+      return !!inst;
+    });
 }
 
 function findModelDefinitions(rootDir, sources) {

lib/executor.js

@@ -95,6 +95,17 @@ function setupDataSources(app, instructions) {
 }
 
 function setupModels(app, instructions) {
+  defineModels(instructions);
+
+  instructions.models.forEach(function(data) {
+    // Skip base models that are not exported to the app
+    if (!data.config) return;
+
+    app.model(data._model, data.config);
+  });
+}
+
+function defineModels(instructions) {
   instructions.models.forEach(function(data) {
     var name = data.name;
     var model;
@@ -122,10 +133,7 @@ function setupModels(app, instructions) {
       }
     }
 
-    // Skip base models that are not exported to the app
+    data._model = model;
-    if (!data.config) return;
-
-    app.model(model, data.config);
   });
 }
 

package.json

@@ -23,9 +23,10 @@
     "url": "https://github.com/strongloop/loopback-boot/blob/master/LICENSE"
   },
   "dependencies": {
-    "underscore": "^1.6.0",
+    "commondir": "0.0.1",
     "debug": "^0.8.1",
-    "commondir": "0.0.1"
+    "toposort": "^0.2.10",
+    "underscore": "^1.6.0"
   },
   "devDependencies": {
     "loopback": "^1.5.0",

test/compiler.test.js

@@ -267,17 +267,17 @@ describe('compiler', function() {
       });
     });
 
-    it('supports `modelSources` option', function() {
+    it('supports `sources` option in `models.json`', function() {
       appdir.createConfigFilesSync({}, {}, {
+        _meta: {
+          sources: ['./custom-models']
+        },
         Car: { dataSource: 'db' }
       });
       appdir.writeConfigFileSync('custom-models/car.json', { name: 'Car' });
       appdir.writeFileSync('custom-models/car.js', '');
 
-      var instructions = boot.compile({
+      var instructions = boot.compile(appdir.PATH);
-        appRootDir: appdir.PATH,
-        modelSources: ['./custom-models']
-      });
 
       expect(instructions.models).to.have.length(1);
       expect(instructions.models[0]).to.eql({
@@ -359,6 +359,48 @@ describe('compiler', function() {
       var modelNames = instructions.models.map(getNameProperty);
       expect(modelNames).to.eql(['Car']);
     });
+
+    it('sorts models, base models first', function() {
+      appdir.createConfigFilesSync({}, {}, {
+        Vehicle: { dataSource: 'db' },
+        FlyingCar: { dataSource: 'db' },
+        Car: { dataSource: 'db' }
+      });
+      appdir.writeConfigFileSync('models/car.json', {
+        name: 'Car',
+        base: 'Vehicle'
+      });
+      appdir.writeConfigFileSync('models/vehicle.json', {
+        name: 'Vehicle'
+      });
+      appdir.writeConfigFileSync('models/flying-car.json', {
+        name: 'FlyingCar',
+        base: 'Car'
+      });
+
+      var instructions = boot.compile(appdir.PATH);
+
+      var modelNames = instructions.models.map(getNameProperty);
+      expect(modelNames).to.eql(['Vehicle', 'Car', 'FlyingCar']);
+    });
+
+    it('detects circular Model dependencies', function() {
+      appdir.createConfigFilesSync({}, {}, {
+        Vehicle: { dataSource: 'db' },
+        Car: { dataSource: 'db' }
+      });
+      appdir.writeConfigFileSync('models/car.json', {
+        name: 'Car',
+        base: 'Vehicle'
+      });
+      appdir.writeConfigFileSync('models/vehicle.json', {
+        name: 'Vehicle',
+        base: 'Car'
+      });
+
+      expect(function() { boot.compile(appdir.PATH); })
+        .to.throw(/cyclic dependency/i);
+    });
   });
 });
 

test/executor.test.js

@@ -62,7 +62,6 @@ describe('executor', function() {
       }.toString());
 
     boot.execute(app, someInstructions({
-      dataSources: { db: { connector: 'memory' } },
       models: [
         {
           name: 'Customer',
@@ -83,7 +82,6 @@ describe('executor', function() {
 
   it('defines model without attaching it', function() {
     boot.execute(app, someInstructions({
-      dataSources: { db: { connector: 'memory' } },
       models: [
         {
           name: 'Vehicle',
@@ -113,6 +111,35 @@ describe('executor', function() {
     assert.equal(app.models.User.dataSource, app.dataSources.theDb);
   });
 
+  it('defines all models first before running the config phase', function() {
+    appdir.writeFileSync('models/Customer.js', 'module.exports = ' +
+      function(Customer/*, Base*/) {
+        Customer.on('attached', function() {
+          Customer._modelsWhenAttached =
+            Object.keys(Customer.modelBuilder.models);
+        });
+      }.toString());
+
+    boot.execute(app, someInstructions({
+      models: [
+        {
+          name: 'Customer',
+          config: { dataSource: 'db' },
+          definition: { name: 'Customer' },
+          sourceFile: path.resolve(appdir.PATH, 'models', 'Customer.js')
+        },
+        {
+          name: 'UniqueName',
+          config: { dataSource: 'db' },
+          definition: { name: 'UniqueName' },
+          sourceFile: undefined
+        }
+      ]
+    }));
+
+    expect(app.models.Customer._modelsWhenAttached).to.include('UniqueName');
+  });
+
   it('instantiates data sources', function() {
     boot.execute(app, dummyInstructions);
     assert(app.dataSources);
@@ -257,7 +284,7 @@ function someInstructions(values) {
   var result = {
     app: values.app || {},
     models: values.models || [],
-    dataSources: values.dataSources || {},
+    dataSources: values.dataSources || { db: { connector: 'memory' } },
     files: {
       boot: []
     }