Merge pull request #2401 from strongloop/compat_flag_cleanup

[SEMVER-MAJOR] Discard sugar method for model creation
strongloop · Sep 7, 2016 · da09876 · da09876
2 parents a6f8ec6 + 832e2c3
commit da09876
Show file tree

Hide file tree

Showing 11 changed files with 152 additions and 183 deletions.
diff --git a/3.0-RELEASE-NOTES.md b/3.0-RELEASE-NOTES.md
@@ -194,3 +194,12 @@ module:
 
 See also [loopback#2564](https://github.com/strongloop/loopback/pull/2564)
 and the official [documentation](https://docs.strongloop.com/display/APIC/Using+current+context)
+
+## Remove sugar for creating models
+
+`app.model(modelName, settings)`, a sugar for creating non-existing model, is
+now removed in favor of promoting use of:
+- `app.registry.createModel(modelName, properties, options)` to create new model
+- `app.model(modelCtor, config)` to update existing model and attach it to app
+
+Please see [related code change](https://github.com/strongloop/loopback/pull/2401) here.
diff --git a/example/colors/app.js b/example/colors/app.js
@@ -14,9 +14,9 @@ var schema = {
   name: String,
 };
 
-var Color = app.model('color', schema);
-
-app.dataSource('db', { adapter: 'memory' }).attach(Color);
+app.dataSource('db', { connector: 'memory' });
+var Color = app.registry.createModel('color', schema);
+app.model(Color, { dataSource: 'db' });
 
 Color.create({ name: 'red' });
 Color.create({ name: 'green' });

diff --git a/example/replication/app.js b/example/replication/app.js
@@ -5,9 +5,11 @@
 
 var loopback = require('../../');
 var app = loopback();
-var db = app.dataSource('db', { connector: loopback.Memory });
-var Color = app.model('color', { dataSource: 'db', options: { trackChanges: true }});
-var Color2 = app.model('color2', { dataSource: 'db', options: { trackChanges: true }});
+var db = app.dataSource('db', { connector: 'memory' });
+var Color = app.registry.createModel('color', {}, { trackChanges: true });
+app.model(Color, { dataSource: 'db' });
+var Color2 = app.registry.createModel('color2', {}, { trackChanges: true });
+app.model(Color2, { dataSource: 'db' });
 var target = Color2;
 var source = Color;
 var SPEED = process.env.SPEED || 100;

diff --git a/lib/application.js b/lib/application.js
@@ -101,7 +101,7 @@ app.disuse = function(route) {
  * app.model(User, { dataSource: 'db' });
  *```
  *
- * @param {Object|String} Model The model to attach.
+ * @param {Object} Model The model to attach.
  * @options {Object} config The model's configuration.
  * @property {String|DataSource} dataSource The `DataSource` to which to attach the model.
  * @property {Boolean} [public] Whether the model should be exposed via REST API.
@@ -114,29 +114,15 @@ app.model = function(Model, config) {
   var isPublic = true;
   var registry = this.registry;
 
+  if (typeof Model === 'string') {
+    var msg = 'app.model(modelName, settings) is no longer supported. ' +
+      'Use app.registry.createModel(modelName, definition) and ' +
+      'app.model(ModelCtor, config) instead.';
+    throw new Error(msg);
+  }
+
   if (arguments.length > 1) {
     config = config || {};
-    if (typeof Model === 'string') {
-      // create & attach the model - backwards compatibility
-
-      // create config for loopback.modelFromConfig
-      var modelConfig = extend({}, config);
-      modelConfig.options = extend({}, config.options);
-      modelConfig.name = Model;
-
-      // modeller does not understand `dataSource` option
-      delete modelConfig.dataSource;
-
-      Model = registry.createModel(modelConfig);
-
-      // delete config options already applied
-      ['relations', 'base', 'acls', 'hidden', 'methods'].forEach(function(prop) {
-        delete config[prop];
-        if (config.options) delete config.options[prop];
-      });
-      delete config.properties;
-    }
-
     configureModel(Model, config, this);
     isPublic = config.public !== false;
   } else {
@@ -186,7 +172,7 @@ app.model = function(Model, config) {
  * });
  * ```
  *
- * 2. Use `app.model` to access a model by name.
+ * 2. Use `app.models` to access a model by name.
  * `app.models` has properties for all defined models.
  *
  * The following example illustrates accessing the `Product` and `CustomerReceipt` models
@@ -201,8 +187,10 @@ app.model = function(Model, config) {
  *   }
  * });
  *
- * app.model('product', {dataSource: 'db'});
- * app.model('customer-receipt', {dataSource: 'db'});
+ * var productModel = app.registry.createModel('product');
+ * app.model(productModel, {dataSource: 'db'});
+ * var customerReceiptModel = app.registry.createModel('customer-receipt');
+ * app.model(customerReceiptModel, {dataSource: 'db'});
  *
  * // available based on the given name
  * var Product = app.models.Product;

diff --git a/test/acl.test.js b/test/acl.test.js
@@ -393,20 +393,18 @@ describe('security ACLs', function() {
 });
 
 describe('access check', function() {
-  var app;
-  before(function() {
-    app = loopback();
-    app.set('remoting', { errorHandler: { debug: true, log: false }});
-    app.use(loopback.rest());
-    app.enableAuth();
-    app.dataSource('test', { connector: 'memory' });
-  });
-
   it('should occur before other remote hooks', function(done) {
-    var MyTestModel = app.model('MyTestModel', { base: 'PersistedModel', dataSource: 'test' });
+    var app = loopback();
+    var MyTestModel = app.registry.createModel('MyTestModel');
     var checkAccessCalled = false;
     var beforeHookCalled = false;
 
+    app.use(loopback.rest());
+    app.set('remoting', { errorHandler: { debug: true, log: false }});
+    app.enableAuth();
+    app.dataSource('test', { connector: 'memory' });
+    app.model(MyTestModel, { dataSource: 'test' });
+
     // fake / spy on the checkAccess method
     MyTestModel.checkAccess = function() {
       var cb = arguments[arguments.length - 1];

diff --git a/test/app.test.js b/test/app.test.js
@@ -611,11 +611,12 @@ describe('app', function() {
   });
 
   describe('app.model(Model)', function() {
-    var app, db;
+    var app, db, MyTestModel;
     beforeEach(function() {
       app = loopback();
       app.set('remoting', { errorHandler: { debug: true, log: false }});
       db = loopback.createDataSource({ connector: loopback.Memory });
+      MyTestModel = app.registry.createModel('MyTestModel');
     });
 
     it('Expose a `Model` to remote clients', function() {
@@ -626,7 +627,7 @@ describe('app', function() {
       expect(app.models()).to.eql([Color]);
     });
 
-    it('uses singlar name as app.remoteObjects() key', function() {
+    it('uses singular name as app.remoteObjects() key', function() {
       var Color = PersistedModel.extend('color', { name: String });
       app.model(Color);
       Color.attachTo(db);
@@ -689,76 +690,26 @@ describe('app', function() {
       });
     });
 
-    it('accepts null dataSource', function() {
-      app.model('MyTestModel', { dataSource: null });
-    });
-
-    it('accepts false dataSource', function() {
-      app.model('MyTestModel', { dataSource: false });
+    it('accepts null dataSource', function(done) {
+      app.model(MyTestModel, { dataSource: null });
+      expect(MyTestModel.dataSource).to.eql(null);
+      done();
     });
 
-    it('should not require dataSource', function() {
-      app.model('MyTestModel', {});
-    });
-  });
-
-  describe('app.model(name, config)', function() {
-    var app;
-
-    beforeEach(function() {
-      app = loopback();
-      app.dataSource('db', {
-        connector: 'memory',
-      });
+    it('accepts false dataSource', function(done) {
+      app.model(MyTestModel, { dataSource: false });
+      expect(MyTestModel.getDataSource()).to.eql(null);
+      done();
     });
 
-    it('Sugar for defining a fully built model', function() {
-      app.model('foo', {
-        dataSource: 'db',
-      });
-
-      var Foo = app.models.foo;
-      var f = new Foo();
-
-      assert(f instanceof app.registry.getModel('Model'));
+    it('does not require dataSource', function(done) {
+      app.model(MyTestModel);
+      done();
     });
 
-    it('interprets extra first-level keys as options', function() {
-      app.model('foo', {
-        dataSource: 'db',
-        base: 'User',
-      });
-
-      expect(app.models.foo.definition.settings.base).to.equal('User');
-    });
-
-    it('prefers config.options.key over config.key', function() {
-      app.model('foo', {
-        dataSource: 'db',
-        base: 'User',
-        options: {
-          base: 'Application',
-        },
-      });
-
-      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);
+    it('throws error if model typeof string is passed', function() {
+      var fn = function() { app.model('MyTestModel'); };
+      expect(fn).to.throw(/app(\.model|\.registry)/);
     });
   });
 
@@ -772,15 +723,17 @@ describe('app', function() {
       }
 
       assert(!previousModel || !previousModel.dataSource);
-      app.model('TestModel', { dataSource: 'db' });
+      var TestModel = app.registry.createModel('TestModel');
+      app.model(TestModel, { dataSource: 'db' });
       expect(app.models.TestModel.dataSource).to.equal(app.dataSources.db);
     });
   });
 
   describe('app.models', function() {
     it('is unique per app instance', function() {
       app.dataSource('db', { connector: 'memory' });
-      var Color = app.model('Color', { dataSource: 'db' });
+      var Color = app.registry.createModel('Color');
+      app.model(Color, { dataSource: 'db' });
       expect(app.models.Color).to.equal(Color);
       var anotherApp = loopback();
       expect(anotherApp.models.Color).to.equal(undefined);

diff --git a/test/change-stream.test.js b/test/change-stream.test.js
@@ -9,7 +9,8 @@ describe('PersistedModel.createChangeStream()', function() {
       var test = this;
       var app = loopback({ localRegistry: true });
       var ds = app.dataSource('ds', { connector: 'memory' });
-      this.Score = app.model('Score', {
+      var Score = app.registry.createModel('Score');
+      this.Score = app.model(Score, {
         dataSource: 'ds',
         changeDataSource: false, // use only local observers
       });

diff --git a/test/integration.test.js b/test/integration.test.js
@@ -40,7 +40,8 @@ describe('loopback application', function() {
       loopback.ACL.attachTo(db);
       loopback.User.hasMany(loopback.AccessToken, { as: 'accessTokens' });
 
-      var Streamer = app.model('Streamer', { dataSource: 'db' });
+      var Streamer = app.registry.createModel('Streamer');
+      app.model(Streamer, { dataSource: 'db' });
       Streamer.read = function(req, res, cb) {
         var body = new Buffer(0);
         req.on('data', function(chunk) {

diff --git a/test/registries.test.js b/test/registries.test.js
@@ -26,15 +26,17 @@ describe('Registry', function() {
       var dsFoo = appFoo.dataSource('dsFoo', { connector: 'memory' });
       var dsBar = appFoo.dataSource('dsBar', { connector: 'memory' });
 
-      var FooModel = appFoo.model(modelName, settings);
-      var FooSubModel = appFoo.model(subModelName, settings);
-      var BarModel = appBar.model(modelName, settings);
-      var BarSubModel = appBar.model(subModelName, settings);
+      var FooModel = appFoo.registry.createModel(modelName, {}, settings);
+      appFoo.model(FooModel, { dataSource: dsFoo });
 
-      FooModel.attachTo(dsFoo);
-      FooSubModel.attachTo(dsFoo);
-      BarModel.attachTo(dsBar);
-      BarSubModel.attachTo(dsBar);
+      var FooSubModel = appFoo.registry.createModel(subModelName, {}, settings);
+      appFoo.model(FooSubModel, { dataSource: dsFoo });
+
+      var BarModel = appBar.registry.createModel(modelName, {}, settings);
+      appBar.model(BarModel, { dataSource: dsBar });
+
+      var BarSubModel = appBar.registry.createModel(subModelName, {}, settings);
+      appBar.model(BarSubModel, { dataSource: dsBar });
 
       FooModel.hasMany(FooSubModel);
       BarModel.hasMany(BarSubModel);