Merge pull request #219 from fabien/fix/relation-tests

Fix  BelongsTo.prototype.create + remaining issues with new relation types

lib/relation-definition.js

@@ -101,8 +101,8 @@ function RelationDefinition(definition) {
   this.keyFrom = definition.keyFrom;
   this.modelTo = definition.modelTo;
   this.keyTo = definition.keyTo;
-  this.discriminator = definition.discriminator;
+  this.polymorphic = definition.polymorphic;
-  if (!this.discriminator) {
+  if (typeof this.polymorphic !== 'object') {
     assert(this.modelTo, 'Target model is required');
   }
   this.modelThrough = definition.modelThrough;
@@ -163,8 +163,13 @@ RelationDefinition.prototype.applyScope = function(modelInstance, filter) {
   filter = filter || {};
   filter.where = filter.where || {};
   if ((this.type !== 'belongsTo' || this.type === 'hasOne')
-    && typeof this.discriminator === 'string') { // polymorphic
+    && typeof this.polymorphic === 'object') { // polymorphic
-    filter.where[this.discriminator] = this.modelFrom.modelName;
+    var discriminator = this.polymorphic.discriminator;
+    if (this.polymorphic.invert) {
+      filter.where[discriminator] = this.modelTo.modelName;
+    } else {
+      filter.where[discriminator] = this.modelFrom.modelName;
+    }
   }
   if (typeof this.scope === 'function') {
     var scope = this.scope.call(this, modelInstance, filter);
@@ -181,21 +186,30 @@ RelationDefinition.prototype.applyScope = function(modelInstance, filter) {
  * @param {Object} modelInstance
  * @param {Object} target
  */
-RelationDefinition.prototype.applyProperties = function(modelInstance, target) {
+RelationDefinition.prototype.applyProperties = function(modelInstance, obj) {
+  var source = modelInstance, target = obj;
+  if (this.options.invertProperties) {
+    source = obj, target = modelInstance;
+  }
   if (typeof this.properties === 'function') {
-    var data = this.properties.call(this, modelInstance);
+    var data = this.properties.call(this, source);
     for(var k in data) {
       target[k] = data[k];
     }
   } else if (typeof this.properties === 'object') {
     for(var k in this.properties) {
       var key = this.properties[k];
-      target[key] = modelInstance[k];
+      target[key] = source[k];
     }
   }
   if ((this.type !== 'belongsTo' || this.type === 'hasOne')
-    && typeof this.discriminator === 'string') { // polymorphic
+    && typeof this.polymorphic === 'object') { // polymorphic
-    target[this.discriminator] = this.modelFrom.modelName;
+    var discriminator = this.polymorphic.discriminator;
+    if (this.polymorphic.invert) {
+      target[discriminator] = this.modelTo.modelName;
+    } else {
+      target[discriminator] = this.modelFrom.modelName;
+    }
   }
 };
 
@@ -492,10 +506,11 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) {
   var fk = params.foreignKey || i8n.camelize(thisClassName + '_id', true);
 
   var idName = modelFrom.dataSource.idName(modelFrom.modelName) || 'id';
-  var discriminator;
+  var discriminator, polymorphic;
   
   if (params.polymorphic) {
-    var polymorphic = polymorphicParams(params.polymorphic);
+    polymorphic = polymorphicParams(params.polymorphic);
+    polymorphic.invert = !!params.invert;
     discriminator = polymorphic.discriminator;
     if (!params.invert) {
       fk = polymorphic.foreignKey;
@@ -511,12 +526,12 @@ RelationDefinition.hasMany = function hasMany(modelFrom, modelTo, params) {
     modelFrom: modelFrom,
     keyFrom: idName,
     keyTo: fk,
-    discriminator: discriminator,
     modelTo: modelTo,
     multiple: true,
     properties: params.properties,
     scope: params.scope,
-    options: params.options
+    options: params.options,
+    polymorphic: polymorphic
   });
   
   definition.modelThrough = params.through;
@@ -732,9 +747,13 @@ var throughKeys = function(definition) {
   var modelThrough = definition.modelThrough;
   var pk2 = definition.modelTo.definition.idName();
   
-  if (definition.discriminator) { // polymorphic
+  if (typeof definition.polymorphic === 'object') { // polymorphic
     var fk1 = definition.keyTo;
-    var fk2 = definition.keyThrough;
+    if (definition.polymorphic.invert) {
+      var fk2 = definition.polymorphic.foreignKey;
+    } else {
+      var fk2 = definition.keyThrough;
+    }
   } else {
     var fk1 = findBelongsTo(modelThrough, definition.modelFrom,
       definition.keyFrom);
@@ -1036,11 +1055,11 @@ RelationDefinition.belongsTo = function (modelFrom, modelTo, params) {
     modelFrom: modelFrom,
     keyFrom: fk,
     keyTo: idName,
-    discriminator: discriminator,
     modelTo: modelTo,
     properties: params.properties,
     scope: params.scope,
-    options: params.options
+    options: params.options,
+    polymorphic: polymorphic
   });
   
   // Define a property for the scope so that we have 'this' for the scoped methods
@@ -1082,14 +1101,17 @@ BelongsTo.prototype.create = function(targetModelData, cb) {
     cb = targetModelData;
     targetModelData = {};
   }
-
+  
   this.definition.applyProperties(modelInstance, targetModelData || {});
   
   modelTo.create(targetModelData, function(err, targetModel) {
     if(!err) {
       modelInstance[fk] = targetModel[pk];
-      self.resetCache(targetModel);
+      modelInstance.save(function(err, inst) {
-      cb && cb(err, targetModel);
+        if (cb && err) return cb && cb(err);
+        self.resetCache(targetModel);
+        cb && cb(err, targetModel);
+      });
     } else {
       cb && cb(err);
     }
@@ -1117,10 +1139,10 @@ BelongsTo.prototype.related = function (refresh, params) {
   var self = this;
   var modelFrom = this.definition.modelFrom;
   var modelTo = this.definition.modelTo;
-  var discriminator = this.definition.discriminator;
   var pk = this.definition.keyTo;
   var fk = this.definition.keyFrom;
   var modelInstance = this.modelInstance;
+  var discriminator;
   
   if (arguments.length === 1) {
     params = refresh;
@@ -1129,6 +1151,10 @@ BelongsTo.prototype.related = function (refresh, params) {
     throw new Error('Method can\'t be called with more than two arguments');
   }
   
+  if (typeof this.definition.polymorphic === 'object') {
+    discriminator = this.definition.polymorphic.discriminator;
+  }
+  
   var cachedValue;
   if (!refresh) {
     cachedValue = self.getCache();
@@ -1136,11 +1162,12 @@ BelongsTo.prototype.related = function (refresh, params) {
   if (params instanceof ModelBaseClass) { // acts as setter
     modelTo = params.constructor;
     modelInstance[fk] = params[pk];
-    if (discriminator) modelInstance[discriminator] = params.constructor.modelName;
     
-    var data = {};
+    if (discriminator) {
-    this.definition.applyProperties(params, data);
+      modelInstance[discriminator] = params.constructor.modelName;
-    modelInstance.setAttributes(data);
+    }
+    
+    this.definition.applyProperties(modelInstance, params);
     
     self.resetCache(params);
   } else if (typeof params === 'function') { // acts as async getter
@@ -1277,10 +1304,10 @@ RelationDefinition.hasOne = function (modelFrom, modelTo, params) {
   var relationName = params.as || i8n.camelize(modelTo.modelName, true);
 
   var fk = params.foreignKey || i8n.camelize(modelFrom.modelName + '_id', true);
-  var discriminator;
+  var discriminator, polymorphic;
   
   if (params.polymorphic) {
-    var polymorphic = polymorphicParams(params.polymorphic);
+    polymorphic = polymorphicParams(params.polymorphic);
     fk = polymorphic.foreignKey;
     discriminator = polymorphic.discriminator;
     if (!params.through) {
@@ -1294,10 +1321,10 @@ RelationDefinition.hasOne = function (modelFrom, modelTo, params) {
     modelFrom: modelFrom,
     keyFrom: pk,
     keyTo: fk,
-    discriminator: discriminator,
     modelTo: modelTo,
     properties: params.properties,
-    options: params.options
+    options: params.options,
+    polymorphic: polymorphic
   });
 
   modelFrom.dataSource.defineForeignKey(modelTo.modelName, fk, modelFrom.modelName);
@@ -1625,11 +1652,97 @@ RelationDefinition.embedsMany = function embedsMany(modelFrom, modelTo, params)
   return definition;
 };
 
+EmbedsMany.prototype.prepareEmbeddedInstance = function(inst) {
+  if (inst && inst.triggerParent !== 'function') {
+    var self = this;
+    var propertyName = this.definition.keyFrom;
+    var modelInstance = this.modelInstance;
+    inst.triggerParent = function(actionName, callback) {
+      if (actionName === 'save' || actionName === 'destroy') {
+        var embeddedList = self.embeddedList();
+        if (actionName === 'destroy') {
+          var index = embeddedList.indexOf(inst);
+          if (index > -1) embeddedList.splice(index, 1);
+        }
+        modelInstance.updateAttribute(propertyName,
+          embeddedList, function(err, modelInst) {
+          callback(err, err ? null : modelInst);
+        });
+      } else {
+        process.nextTick(callback);
+      }
+    };
+    var originalTrigger = inst.trigger;
+    inst.trigger = function(actionName, work, data, callback) {
+      if (typeof work === 'function') {
+        var originalWork = work;
+        work = function(next) {
+          originalWork.call(this, function(done) {
+            inst.triggerParent(actionName, function(err, inst) {
+              next(done); // TODO [fabien] - error handling?
+            });
+          });
+        };
+      }
+      originalTrigger.call(this, actionName, work, data, callback);
+    };
+  }
+};
+
+EmbedsMany.prototype.embeddedList = function(modelInstance) {
+  modelInstance = modelInstance || this.modelInstance;
+  var embeddedList = modelInstance[this.definition.keyFrom] || [];
+  embeddedList.forEach(this.prepareEmbeddedInstance.bind(this));
+  return embeddedList;
+};
+
+EmbedsMany.prototype.prepareEmbeddedInstance = function(inst) {
+  if (inst && inst.triggerParent !== 'function') {
+    var self = this;
+    var relationName = this.definition.name;
+    var modelInstance = this.modelInstance;
+    inst.triggerParent = function(actionName, callback) {
+      if (actionName === 'save' || actionName === 'destroy') {
+        var embeddedList = self.embeddedList();
+        if (actionName === 'destroy') {
+          var index = embeddedList.indexOf(inst);
+          if (index > -1) embeddedList.splice(index, 1);
+        }
+        modelInstance.updateAttribute(relationName,
+          embeddedList, function(err, modelInst) {
+          callback(err, err ? null : modelInst);
+        });
+      } else {
+        process.nextTick(callback);
+      }
+    };
+    var originalTrigger = inst.trigger;
+    inst.trigger = function(actionName, work, data, callback) {
+      if (typeof work === 'function') {
+        var originalWork = work;
+        work = function(next) {
+          originalWork.call(this, function(done) {
+            inst.triggerParent(actionName, function(err, inst) {
+              next(done); // TODO [fabien] - error handling?
+            });
+          });
+        };
+      }
+      originalTrigger.call(this, actionName, work, data, callback);
+    };
+  }
+};
+
+EmbedsMany.prototype.embeddedList = function(modelInstance) {
+  modelInstance = modelInstance || this.modelInstance;
+  var embeddedList = modelInstance[this.definition.name] || [];
+  embeddedList.forEach(this.prepareEmbeddedInstance.bind(this));
+  return embeddedList;
+};
+
 EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb) {
   var modelTo = this.definition.modelTo;
-  var propertyName = this.definition.keyFrom;
   var modelInstance = this.modelInstance;
-  var self = receiver;
   
   var actualCond = {};
   var actualRefresh = false;
@@ -1646,13 +1759,13 @@ EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb
     throw new Error('Method can be only called with one or two arguments');
   }
   
-  var embeddedList = self[propertyName] || [];
+  var embeddedList = this.embeddedList(receiver);
   
-  this.definition.applyScope(modelInstance, actualCond);
+  this.definition.applyScope(receiver, actualCond);
   
   var params = mergeQuery(actualCond, scopeParams);
   
-  if (params.where) {
+  if (params.where) { // TODO [fabien] Support order/sorting
     embeddedList = embeddedList ? embeddedList.filter(applyFilter(params)) : embeddedList;
   }
   
@@ -1670,10 +1783,9 @@ EmbedsMany.prototype.related = function(receiver, scopeParams, condOrRefresh, cb
 EmbedsMany.prototype.findById = function (fkId, cb) {
   var pk = this.definition.keyTo;
   var modelTo = this.definition.modelTo;
-  var propertyName = this.definition.keyFrom;
   var modelInstance = this.modelInstance;
   
-  var embeddedList = modelInstance[propertyName] || [];
+  var embeddedList = this.embeddedList();
   
   var find = function(id) {
     for (var i = 0; i < embeddedList.length; i++) {
@@ -1713,7 +1825,7 @@ EmbedsMany.prototype.updateById = function (fkId, data, cb) {
   var propertyName = this.definition.keyFrom;
   var modelInstance = this.modelInstance;
   
-  var embeddedList = modelInstance[propertyName] || [];
+  var embeddedList = this.embeddedList();
   
   var inst = this.findById(fkId);
   
@@ -1747,7 +1859,7 @@ EmbedsMany.prototype.destroyById = function (fkId, cb) {
   var propertyName = this.definition.keyFrom;
   var modelInstance = this.modelInstance;
   
-  var embeddedList = modelInstance[propertyName] || [];
+  var embeddedList = this.embeddedList();
   
   var inst = (fkId instanceof modelTo) ? fkId : this.findById(fkId);
   
@@ -1772,10 +1884,9 @@ EmbedsMany.prototype.unset = EmbedsMany.prototype.destroyById;
 
 EmbedsMany.prototype.at = function (index, cb) {
   var modelTo = this.definition.modelTo;
-  var propertyName = this.definition.keyFrom;
   var modelInstance = this.modelInstance;
   
-  var embeddedList = modelInstance[propertyName] || [];
+  var embeddedList = this.embeddedList();
   
   var item = embeddedList[parseInt(index)];
   item = (item instanceof modelTo) ? item : null;
@@ -1802,7 +1913,7 @@ EmbedsMany.prototype.create = function (targetModelData, cb) {
   }
   targetModelData = targetModelData || {};
   
-  var embeddedList = modelInstance[propertyName] || [];
+  var embeddedList = this.embeddedList();
   
   var inst = this.build(targetModelData);
   
@@ -1823,11 +1934,10 @@ EmbedsMany.prototype.create = function (targetModelData, cb) {
 EmbedsMany.prototype.build = function(targetModelData) {
   var pk = this.definition.keyTo;
   var modelTo = this.definition.modelTo;
-  var propertyName = this.definition.keyFrom;
   var modelInstance = this.modelInstance;
   var autoId = this.definition.options.autoId !== false;
   
-  var embeddedList = modelInstance[propertyName] || [];
+  var embeddedList = this.embeddedList();
   
   targetModelData = targetModelData || {};
   
@@ -1852,6 +1962,8 @@ EmbedsMany.prototype.build = function(targetModelData) {
     embeddedList.push(inst);
   }
   
+  this.prepareEmbeddedInstance(inst);
+  
   return inst;
 };
 

test/relations.test.js

@@ -1040,6 +1040,40 @@ describe('relations', function () {
         });
       });
     });
+    
+    it('should create polymorphic item through relation scope', function (done) {
+      Picture.findById(anotherPicture.id, function(err, p) {
+        p.authors.create({ name: 'Author 3' }, function(err, a) {
+          should.not.exist(err);
+          author = a;
+          author.name.should.equal('Author 3');
+          done();
+        });
+      });
+    });
+    
+    it('should create polymorphic through model - new author', function (done) {
+      PictureLink.findOne({ where: { 
+          pictureId: anotherPicture.id, imageableId: author.id, imageableType: 'Author'
+        } }, function(err, link) {
+        should.not.exist(err);
+        link.pictureId.should.eql(anotherPicture.id);
+        link.imageableId.should.eql(author.id);
+        link.imageableType.should.equal('Author');
+        done();
+      });
+    });
+    
+    it('should find polymorphic items - new author', function (done) {
+      Author.findById(author.id, function(err, author) {
+        author.pictures(function(err, pics) {
+          pics.should.have.length(1);
+          pics[0].id.should.eql(anotherPicture.id);
+          pics[0].name.should.equal('Example');
+          done();
+        });
+      });
+    });
   
   });
 
@@ -1119,11 +1153,9 @@ describe('relations', function () {
       p.person.create({name: 'Fred', age: 36 }, function(err, person) {
         personCreated = person;
         p.personId.should.equal(person.id);
-        p.save(function (err, p) {
+        person.name.should.equal('Fred');
-          person.name.should.equal('Fred');
+        person.passportNotes.should.equal('Some notes...');
-          person.passportNotes.should.equal('Some notes...');
+        done();
-          done();
-        });
       });
     });
     
@@ -1201,7 +1233,6 @@ describe('relations', function () {
       Article.create(function (e, article) {
         article.tags.create({name: 'popular'}, function (e, t) {
           t.should.be.an.instanceOf(Tag);
-          // console.log(t);
           ArticleTag.findOne(function (e, at) {
             should.exist(at);
             at.tagId.toString().should.equal(t.id.toString());
@@ -1576,19 +1607,15 @@ describe('relations', function () {
   
   describe('embedsMany - relations, scope and properties', function () {
     
-    var product1, product2, product3;
+    var category, product1, product2, product3;
     
-    before(function (done) {
+    before(function () {
       db = getSchema();
       Category = db.define('Category', {name: String});
       Product = db.define('Product', {name: String});
       Link = db.define('Link', {name: String});
-
-      db.automigrate(function () {
-        Person.destroyAll(done);
-      });
     });
-
+    
     it('can be declared', function (done) {
       Category.embedsMany(Link, { 
         as: 'items', // rename
@@ -1598,6 +1625,7 @@ describe('relations', function () {
       Link.belongsTo(Product, { 
         foreignKey: 'id', // re-use the actual product id
         properties: { id: 'id', name: 'name' }, // denormalize, transfer id
+        options: { invertProperties: true }
       });
       db.automigrate(function() {
         Product.create({ name: 'Product 0' }, done); // offset ids for tests
@@ -1617,7 +1645,7 @@ describe('relations', function () {
       });
     });
     
-    it('should create items on scope', function(done) {
+    it('should associate items on scope', function(done) {
       Category.create({ name: 'Category A' }, function(err, cat) {
         var link = cat.items.build();
         link.product(product1);
@@ -1728,13 +1756,82 @@ describe('relations', function () {
       });
     });
     
-    it('should remove embedded items by reference id', function(done) {
+    it('should have removed embedded items by reference id', function(done) {
       Category.findOne(function(err, cat) {
         cat.links.should.have.length(1);
         done();
       });
     });
     
+    it('should create items on scope', function(done) {
+      Category.create({ name: 'Category B' }, function(err, cat) {
+        category = cat;
+        var link = cat.items.build({ notes: 'Some notes...' });
+        link.product.create({ name: 'Product 1' }, function(err, p) {
+          cat.links[0].id.should.eql(p.id);
+          cat.links[0].name.should.equal('Product 1'); // denormalized
+          cat.links[0].notes.should.equal('Some notes...');
+          cat.items.at(0).should.equal(cat.links[0]);
+          done();
+        });
+      });
+    });
+    
+    it('should find items on scope', function(done) {
+      Category.findById(category.id, function(err, cat) {
+        cat.name.should.equal('Category B');
+        cat.links.toObject().should.eql([
+          {id: 5, name: 'Product 1', notes: 'Some notes...'}
+        ]);
+        cat.items.at(0).should.equal(cat.links[0]);
+        cat.items(function(err, items) { // alternative access
+          items.should.be.an.array;
+          items.should.have.length(1);
+          items[0].product(function(err, p) {
+            p.name.should.equal('Product 1'); // actual value
+            done();
+          });
+        });
+      });
+    });
+    
+    it('should update items on scope - and save parent', function(done) {
+      Category.findById(category.id, function(err, cat) {
+        var link = cat.items.at(0);
+        link.updateAttributes({notes: 'Updated notes...'}, function(err, link) {
+          link.notes.should.equal('Updated notes...');
+          done();
+        });
+      });
+    });
+    
+    it('should find items on scope - verify update', function(done) {
+      Category.findById(category.id, function(err, cat) {
+        cat.name.should.equal('Category B');
+        cat.links.toObject().should.eql([
+          {id: 5, name: 'Product 1', notes: 'Updated notes...'}
+        ]);
+        done();
+      });
+    });
+    
+    it('should remove items from scope - and save parent', function(done) {
+      Category.findById(category.id, function(err, cat) {
+        cat.items.at(0).destroy(function(err, link) {
+          cat.links.should.eql([]);
+          done();
+        });
+      });
+    });
+    
+    it('should find items on scope - verify destroy', function(done) {
+      Category.findById(category.id, function(err, cat) {
+        cat.name.should.equal('Category B');
+        cat.links.should.eql([]);
+        done();
+      });
+    });
+    
   });
   
   describe('embedsMany - polymorphic relations', function () {
@@ -1767,7 +1864,8 @@ describe('relations', function () {
       });      
       Link.belongsTo('linked', {
         polymorphic: true, // needs unique auto-id
-        properties: { name: 'name' } // denormalized
+        properties: { name: 'name' }, // denormalized
+        options: { invertProperties: true }
       });
       db.automigrate(done);
     });