Merge pull request #320 from strongloop/feature/tidyup-app.model

Tidy up app.model() to remove duplicate & recusrive call

lib/application.js

@@ -105,37 +105,37 @@ app.disuse = function (route) {
  */
 
 app.model = function (Model, config) {
-  var modelName = Model;
-  if(arguments.length === 1) {
+  var modelName = null;
+  var isPublic = true;
+  if (arguments.length > 1) {
+    config = config || {};
+    modelName = Model;
+    assert(typeof modelName === 'string', 'app.model(name, config) => "name" name must be a string');
+    Model = modelFromConfig(modelName, config, this);
+    isPublic = config.public !== false;
+  } else {
     assert(typeof Model === 'function', 'app.model(Model) => Model must be a function / constructor');
     modelName = Model.modelName;
     assert(modelName, 'Model must have a "modelName" property');
+  }
+
+  this.models[modelName] =
+    this.models[classify(modelName)] =
+      this.models[camelize(modelName)] = Model;
+
+  this.models().push(Model);
+
+  if (isPublic) {
     var remotingClassName = compat.getClassNameForRemoting(Model);
     this.remotes().exports[remotingClassName] = Model;
-    this.models().push(Model);
-    this.models[modelName] =
-    this.models[classify(modelName)] =
-    this.models[camelize(modelName)] = Model;
     clearHandlerCache(this);
-    Model.shared = true;
-    Model.app = this;
-    Model.emit('attached', this);
-    return;
-  }
-  config = config || {};
-  assert(typeof modelName === 'string', 'app.model(name, config) => "name" name must be a string');
-
-  Model = modelFromConfig(modelName, config, this);
-  this.models[modelName] =
-  this.models[classify(modelName)] =
-  this.models[camelize(modelName)] = Model;
-
-  if(config.public !== false) {
-    this.model(Model);
   }
 
+  Model.shared = isPublic; // The base Model has shared = true
+  Model.app = this;
+  Model.emit('attached', this);
   return Model;
-}
+};
 
 /**
  * Get the models exported by the app. Returns only models defined using `app.model()`

test/app.test.js

@@ -32,8 +32,10 @@ describe('app', function() {
     it('registers existing models to app.models', function() {
       var Color = db.createModel('color', {name: String});
       app.model(Color);
-      expect(app.models.color).to.eql(Color);
-      expect(app.models.Color).to.eql(Color);
+      expect(Color.app).to.be.equal(app);
+      expect(Color.shared).to.equal(true);
+      expect(app.models.color).to.equal(Color);
+      expect(app.models.Color).to.equal(Color);
     });
 
     it('updates REST API when a new model is added', function(done) {
@@ -114,6 +116,24 @@ describe('app', function() {
 
       expect(app.models.foo.definition.settings.base).to.equal('Application');
     });
+
+    it('honors config.public options', function() {
+      app.model('foo', {
+        dataSource: 'db',
+        public: false
+      });
+      expect(app.models.foo.app).to.equal(app);
+      expect(app.models.foo.shared).to.equal(false);
+    });
+
+    it('defaults config.public to be true', function() {
+      app.model('foo', {
+        dataSource: 'db'
+      });
+      expect(app.models.foo.app).to.equal(app);
+      expect(app.models.foo.shared).to.equal(true);
+    });
+
   });
 
   describe('app.models', function() {