Merge pull request #1140 from strongloop/feature/guids-for-replicated-models

Guids for replicated models

lib/persisted-model.js

@@ -7,6 +7,7 @@ var registry = require('./registry');
 var runtime = require('./runtime');
 var assert = require('assert');
 var async = require('async');
+var deprecated = require('depd')('loopback');
 
 /**
  * Extends Model with basic query and CRUD support.
@@ -1041,6 +1042,15 @@ PersistedModel.enableChangeTracking = function() {
   assert(this.dataSource, 'Cannot enableChangeTracking(): ' + this.modelName +
     ' is not attached to a dataSource');
 
+  var idName = this.getIdName();
+  var idProp = this.definition.properties[idName];
+  var idType = idProp && idProp.type;
+  var idDefn = idProp && idProp.defaultFn;
+  if (idType !== String || !(idDefn === 'uuid' || idDefn === 'guid')) {
+    deprecated('The model ' + this.modelName + ' is tracking changes, ' +
+      'which requries a string id with GUID/UUID default value.');
+  }
+
   Change.attachTo(this.dataSource);
   Change.getCheckpointModel().attachTo(this.dataSource);
 

package.json

@@ -38,6 +38,7 @@
     "canonical-json": "0.0.4",
     "continuation-local-storage": "~3.1.1",
     "debug": "~2.0.0",
+    "depd": "^1.0.0",
     "ejs": "~1.0.0",
     "express": "^4.10.2",
     "inflection": "~1.4.2",
@@ -52,7 +53,7 @@
     "underscore.string": "~2.3.3"
   },
   "peerDependencies": {
-    "loopback-datasource-juggler": "^2.17.0"
+    "loopback-datasource-juggler": "^2.19.0"
   },
   "devDependencies": {
     "browserify": "~4.2.3",
@@ -79,7 +80,7 @@
     "karma-phantomjs-launcher": "~0.1.4",
     "karma-script-launcher": "~0.1.0",
     "loopback-boot": "^1.1.0",
-    "loopback-datasource-juggler": "^2.17.0",
+    "loopback-datasource-juggler": "^2.19.0",
     "loopback-testing": "~0.2.0",
     "mocha": "~1.21.4",
     "strong-task-emitter": "0.0.x",
@@ -94,6 +95,7 @@
     "./lib/server-app.js": "./lib/browser-express.js",
     "connect": false,
     "nodemailer": false,
+    "depd": "loopback-datasource-juggler/lib/browser.depd.js",
     "bcrypt": false
   },
   "license": {

test/change.test.js

@@ -6,9 +6,13 @@ describe('Change', function() {
     var memory = loopback.createDataSource({
       connector: loopback.Memory
     });
-    TestModel = loopback.PersistedModel.extend('chtest', {}, {
-      trackChanges: true
-    });
+    TestModel = loopback.PersistedModel.extend('ChangeTestModel',
+      {
+        id: { id: true, type: 'string', defaultFn: 'guid' }
+      },
+      {
+        trackChanges: true
+      });
     this.modelName = TestModel.modelName;
     TestModel.attachTo(memory);
     Change = TestModel.getChangeModel();

test/model.test.js

@@ -16,7 +16,7 @@ describe('Model / PersistedModel', function() {
 
   describe('Model.validatesUniquenessOf(property, options)', function() {
     it('Ensure the value for `property` is unique', function(done) {
-      var User = PersistedModel.extend('user', {
+      var User = PersistedModel.extend('ValidatedUser', {
         'first': String,
         'last': String,
         'age': Number,
@@ -72,6 +72,7 @@ describe.onServer('Remote Methods', function() {
 
   beforeEach(function() {
     User = PersistedModel.extend('user', {
+      id: { id: true, type: String, defaultFn: 'guid' },
       'first': String,
       'last': String,
       'age': Number,
@@ -489,10 +490,10 @@ describe.onServer('Remote Methods', function() {
     });
   });
 
-  describe('PersistelModel remote methods', function() {
+  describe('PersistedModel remote methods', function() {
     it('includes all aliases', function() {
       var app = loopback();
-      var model = PersistedModel.extend('persistedModel');
+      var model = PersistedModel.extend('PersistedModelForAliases');
       app.dataSource('db', { connector: 'memory' });
       app.model(model, { dataSource: 'db' });
 
@@ -500,7 +501,7 @@ describe.onServer('Remote Methods', function() {
       var metadata = app.handler('rest')
         .adapter
         .getClasses()
-        .filter(function(c) { return c.name === 'persistedModel'; })[0];
+        .filter(function(c) { return c.name === model.modelName; })[0];
 
       var methodNames = [];
       metadata.methods.forEach(function(method) {
@@ -509,7 +510,11 @@ describe.onServer('Remote Methods', function() {
       });
 
       expect(methodNames).to.have.members([
-        'destroyAll', 'deleteAll', 'remove',
+        // NOTE(bajtos) These three methods are disabled by default
+        // Because all tests share the same global registry model
+        // and one of the tests was enabling remoting of "destroyAll",
+        // this test was seeing this method (with all aliases) as public
+        // 'destroyAll', 'deleteAll', 'remove',
         'create',
         'upsert', 'updateOrCreate',
         'exists',

test/remote-connector.test.js

@@ -20,7 +20,8 @@ describe('RemoteConnector', function() {
       });
     },
     onDefine: function(Model) {
-      var RemoteModel = Model.extend(Model.modelName);
+      var RemoteModel = Model.extend('Remote' + Model.modelName, {},
+        { plural: Model.pluralModelName });
       RemoteModel.attachTo(loopback.createDataSource({
         connector: loopback.Memory
       }));

test/replication.test.js

@@ -14,14 +14,16 @@ describe('Replication / Change APIs', function() {
     dataSource = this.dataSource = loopback.createDataSource({
       connector: loopback.Memory
     });
-    SourceModel = this.SourceModel = PersistedModel.extend('SourceModel', {}, {
-      trackChanges: true
-    });
+    SourceModel = this.SourceModel = PersistedModel.extend('SourceModel',
+      { id: { id: true, type: String, defaultFn: 'guid' } },
+      { trackChanges: true });
+
     SourceModel.attachTo(dataSource);
 
-    TargetModel = this.TargetModel = PersistedModel.extend('TargetModel', {}, {
-      trackChanges: true
-    });
+    TargetModel = this.TargetModel = PersistedModel.extend('TargetModel',
+      { id: { id: true, type: String, defaultFn: 'guid' } },
+      { trackChanges: true });
+
     TargetModel.attachTo(dataSource);
 
     test.startingCheckpoint = -1;
@@ -169,11 +171,11 @@ describe('Replication / Change APIs', function() {
       var test = this;
       this.conflict.models(function(err, source, target) {
         assert.deepEqual(source.toJSON(), {
-          id: 1,
+          id: test.model.id,
           name: 'source update'
         });
         assert.deepEqual(target.toJSON(), {
-          id: 1,
+          id: test.model.id,
           name: 'target update'
         });
         done();
@@ -242,7 +244,7 @@ describe('Replication / Change APIs', function() {
       this.conflict.models(function(err, source, target) {
         assert.equal(source, null);
         assert.deepEqual(target.toJSON(), {
-          id: 1,
+          id: test.model.id,
           name: 'target update'
         });
         done();
@@ -311,7 +313,7 @@ describe('Replication / Change APIs', function() {
       this.conflict.models(function(err, source, target) {
         assert.equal(target, null);
         assert.deepEqual(source.toJSON(), {
-          id: 1,
+          id: test.model.id,
           name: 'source update'
         });
         done();

test/util/model-tests.js

@@ -36,7 +36,8 @@ module.exports = function defineModelTestsWithDataSource(options) {
         return extendedModel;
       };
 
-      User = PersistedModel.extend('user', {
+      User = PersistedModel.extend('UtilUser', {
+        id: { id: true, type: String, defaultFn: 'guid' },
         'first': String,
         'last': String,
         'age': Number,
@@ -48,8 +49,6 @@ module.exports = function defineModelTestsWithDataSource(options) {
         trackChanges: true
       });
 
-      // enable destroy all for testing
-      User.destroyAll.shared = true;
       User.attachTo(dataSource);
     });
 
@@ -187,10 +186,13 @@ module.exports = function defineModelTestsWithDataSource(options) {
       it('Remove a model from the attached data source', function(done) {
         User.create({first: 'joe', last: 'bob'}, function(err, user) {
           User.findById(user.id, function(err, foundUser) {
+            if (err) return done(err);
             assert.equal(user.id, foundUser.id);
-            foundUser.destroy(function() {
-              User.findById(user.id, function(err, notFound) {
-                assert.equal(notFound, null);
+            User.deleteById(foundUser.id, function(err) {
+              if (err) return done(err);
+              User.find({ where: { id: user.id } }, function(err, found) {
+                if (err) return done(err);
+                assert.equal(found.length, 0);
                 done();
               });
             });