Merge pull request #716 from tandartsnl/bugfix/eagerloading_and_save

Bugfix for the problems related to eager loading associations and subsequent saving of DAO instances

lib/dao-factory.js

@@ -441,7 +441,7 @@ module.exports = (function() {
       daos.forEach(function(dao) {
       daos.forEach(function(dao) {
         var values = {};
         var values = {};
         fields.forEach(function(field) {
         fields.forEach(function(field) {
-          values[field] = dao.values[field]
+          values[field] = dao.dataValues[field]
         })
         })
         if (self.options.timestamps) {
         if (self.options.timestamps) {
           values[updatedAtAttr] = Utils.now()
           values[updatedAtAttr] = Utils.now()
@@ -451,7 +451,7 @@ module.exports = (function() {
 
 
     } else {
     } else {
       daos.forEach(function(dao) {
       daos.forEach(function(dao) {
-        records.push(dao.values)
+        records.push(dao.dataValues)
       })
       })
     }
     }
 
 

lib/dao-validator.js

@@ -35,9 +35,9 @@ var validateAttributes = function() {
   var errors = {}
   var errors = {}
 
 
   // for each field and value
   // for each field and value
-  Utils._.each(this.model.values, function(value, field) {
+  Utils._.each(this.model.dataValues, function(value, field) {
     var rawAttribute   = this.model.rawAttributes[field]
     var rawAttribute   = this.model.rawAttributes[field]
-      , hasAllowedNull = ((rawAttribute.allowNull === true) && ((value === null) || (value === undefined)))
+      , hasAllowedNull = ((rawAttribute === undefined || rawAttribute.allowNull === true) && ((value === null) || (value === undefined)))
 
 
     if (this.model.validators.hasOwnProperty(field) && !hasAllowedNull) {
     if (this.model.validators.hasOwnProperty(field) && !hasAllowedNull) {
       errors = Utils._.merge(errors, validateAttribute.call(this, value, field))
       errors = Utils._.merge(errors, validateAttribute.call(this, value, field))

lib/dao.js

@@ -93,7 +93,7 @@ module.exports = (function() {
   // only those fields will be updated
   // only those fields will be updated
   DAO.prototype.save = function(fields) {
   DAO.prototype.save = function(fields) {
     var self          = this
     var self          = this
-      , values        = fields ? {} : this.values
+      , values        = fields ? {} : this.dataValues
       , updatedAtAttr = this.__options.underscored ? 'updated_at' : 'updatedAt'
       , updatedAtAttr = this.__options.underscored ? 'updated_at' : 'updatedAt'
       , createdAtAttr = this.__options.underscored ? 'created_at' : 'createdAt'
       , createdAtAttr = this.__options.underscored ? 'created_at' : 'createdAt'
 
 
@@ -108,7 +108,7 @@ module.exports = (function() {
         }
         }
       }
       }
 
 
-      var tmpVals = self.values
+      var tmpVals = self.dataValues
 
 
       fields.forEach(function(field) {
       fields.forEach(function(field) {
         if (tmpVals[field] !== undefined) {
         if (tmpVals[field] !== undefined) {
@@ -279,7 +279,7 @@ module.exports = (function() {
   DAO.prototype.equals = function(other) {
   DAO.prototype.equals = function(other) {
     var result = true
     var result = true
 
 
-    Utils._.each(this.values, function(value, key) {
+    Utils._.each(this.dataValues, function(value, key) {
       if(Utils._.isDate(value) && Utils._.isDate(other[key])) {
       if(Utils._.isDate(value) && Utils._.isDate(other[key])) {
         result = result && (value.getTime() == other[key].getTime())
         result = result && (value.getTime() == other[key].getTime())
       } else {
       } else {

spec/associations/has-many.spec.js

@@ -372,7 +372,7 @@ describe(Helpers.getTestDialectTeaser("HasMany"), function() {
         this.Task = this.sequelize.define('Task', { title: Sequelize.STRING }, {timestamps: false})
         this.Task = this.sequelize.define('Task', { title: Sequelize.STRING }, {timestamps: false})
 
 
         this.User.hasMany(this.Task)
         this.User.hasMany(this.Task)
-        this.Task.hasMany(this.User)  
+        this.Task.hasMany(this.User)
 
 
         this.sequelize.sync({force: true}).success(done)
         this.sequelize.sync({force: true}).success(done)
       })
       })
@@ -385,7 +385,7 @@ describe(Helpers.getTestDialectTeaser("HasMany"), function() {
             this.Task.create({ title: 'task2' }).success(function(task2) {
             this.Task.create({ title: 'task2' }).success(function(task2) {
               user.setTasks([task1, task2]).on('sql', spy).on('sql', _.after(2, function (sql) {
               user.setTasks([task1, task2]).on('sql', spy).on('sql', _.after(2, function (sql) {
                 expect(sql).toMatch("INSERT INTO")
                 expect(sql).toMatch("INSERT INTO")
-                expect(sql).toMatch("VALUES (1,1),(2,1)")
+                expect(sql).toMatch("VALUES (1,1),(1,2)")
               })).success(function () {
               })).success(function () {
                 expect(spy).toHaveBeenCalledTwice() // Once for SELECT, once for INSERT into
                 expect(spy).toHaveBeenCalledTwice() // Once for SELECT, once for INSERT into
                 done()
                 done()
@@ -415,7 +415,7 @@ describe(Helpers.getTestDialectTeaser("HasMany"), function() {
         }.bind(this))
         }.bind(this))
       })
       })
     }) // end optimization using bulk create, destroy and update
     }) // end optimization using bulk create, destroy and update
-  
+
     describe('join table creation', function () {
     describe('join table creation', function () {
       before(function (done) {
       before(function (done) {
         this.User = this.sequelize.define('User',
         this.User = this.sequelize.define('User',
@@ -430,7 +430,7 @@ describe(Helpers.getTestDialectTeaser("HasMany"), function() {
         this.User.hasMany(this.Task,
         this.User.hasMany(this.Task,
           { joinTableName: 'user_has_tasks' }
           { joinTableName: 'user_has_tasks' }
         )
         )
-        this.Task.hasMany(this.User)  
+        this.Task.hasMany(this.User)
 
 
         this.sequelize.sync({force: true}).success(done)
         this.sequelize.sync({force: true}).success(done)
       })
       })

spec/dao-factory.spec.js

@@ -1004,8 +1004,8 @@ describe(Helpers.getTestDialectTeaser("DAOFactory"), function() {
         ]
         ]
         , done = _.after(2 * permutations.length, _done);
         , done = _.after(2 * permutations.length, _done);
 
 
-      this.User.create({name: 'jack'}).success(function (jack) {
-        this.User.create({name: 'jill'}).success(function (jill) {
+      this.User.create({username: 'jack'}).success(function (jack) {
+        this.User.create({username: 'jill'}).success(function (jill) {
           permutations.forEach(function(perm) {
           permutations.forEach(function(perm) {
             this.User.find(perm).done(function(err, user) {
             this.User.find(perm).done(function(err, user) {
               expect(err).toBeNull();
               expect(err).toBeNull();

spec/dao.spec.js

@@ -444,6 +444,140 @@ describe(Helpers.getTestDialectTeaser("DAO"), function() {
         })
         })
       })
       })
     })
     })
+
+    before(function(done) {
+      this.UserEager    = this.sequelize.define('UserEagerLoadingSaves', {
+        username    : Helpers.Sequelize.STRING,
+        age         : Helpers.Sequelize.INTEGER
+      }, { timestamps: false })
+
+      this.ProjectEager = this.sequelize.define('ProjectEagerLoadingSaves', {
+        title       : Helpers.Sequelize.STRING,
+        overdue_days: Helpers.Sequelize.INTEGER
+      }, { timestamps: false })
+
+      this.UserEager.hasMany(this.ProjectEager,   { as: 'Projects'   })
+      this.ProjectEager.belongsTo(this.UserEager, { as: 'Poobah'     })
+
+      this.sequelize.sync({ force: true }).success(done)
+    })
+
+    it('saves one object that has a collection of eagerly loaded objects', function(done) {
+      this.UserEager.create({ username: 'joe', age: 1 }).success(function(user) {
+        this.ProjectEager.create({ title: 'project-joe1', overdue_days: 0 }).success(function(project1) {
+          this.ProjectEager.create({ title: 'project-joe2', overdue_days: 0 }).success(function(project2)  {
+            user.setProjects([project1, project2]).success(function() {
+
+              this.UserEager.find({where: {age: 1}, include: [{model: this.ProjectEager, as: 'Projects'}]}).success(function(user) {
+
+                expect(user.username).toEqual('joe')
+                expect(user.age).toEqual(1)
+                expect(user.projects).toBeDefined()
+                expect(user.projects.length).toEqual(2)
+
+                user.age = user.age + 1; // happy birthday joe
+
+                user.save().success(function(saveduser) {
+                  expect(user.username).toEqual('joe')
+                  expect(user.age).toEqual(2)
+                  expect(user.projects).toBeDefined()
+                  expect(user.projects.length).toEqual(2)
+                  done()
+                })
+              }.bind(this))
+            }.bind(this))
+          }.bind(this))
+        }.bind(this))
+      }.bind(this))
+    })
+
+    it('saves many objects that each a have collection of eagerly loaded objects', function(done) {
+      this.UserEager.create({ username: 'bart', age: 20 }).success(function(bart) {
+        this.UserEager.create({ username: 'lisa', age: 20 }).success(function(lisa) {
+
+          this.ProjectEager.create({ title: 'detention1', overdue_days: 0 }).success(function(detention1) {
+            this.ProjectEager.create({ title: 'detention2', overdue_days: 0 }).success(function(detention2)  {
+              this.ProjectEager.create({ title: 'exam1', overdue_days: 0 }).success(function(exam1) {
+                this.ProjectEager.create({ title: 'exam2', overdue_days: 0 }).success(function(exam2)  {
+                  bart.setProjects([detention1, detention2]).success(function() {
+                    lisa.setProjects([exam1, exam2]).success(function() {
+                      this.UserEager.findAll({where: {age: 20}, order: 'username ASC', include: [{model: this.ProjectEager, as: 'Projects'}]}).success(function(simpsons) {
+                        var _bart, _lisa;
+
+                        expect(simpsons.length).toEqual(2)
+
+                        _bart = simpsons[0];
+                        _lisa = simpsons[1];
+
+                        expect(_bart.projects).toBeDefined()
+                        expect(_lisa.projects).toBeDefined()
+                        expect(_bart.projects.length).toEqual(2)
+                        expect(_lisa.projects.length).toEqual(2)
+
+                        _bart.age = _bart.age + 1; // happy birthday bart - off to Moe's
+
+                        _bart.save().success(function(savedbart) {
+                          expect(savedbart.username).toEqual('bart')
+                          expect(savedbart.age).toEqual(21)
+
+                          _lisa.username = 'lsimpson';
+
+                          _lisa.save().success(function(savedlisa) {
+                            expect(savedlisa.username).toEqual('lsimpson')
+                            expect(savedlisa.age).toEqual(20)
+
+                            done()
+                          })
+                        })
+                      })
+                    }.bind(this))
+                  }.bind(this))
+                }.bind(this))
+              }.bind(this))
+            }.bind(this))
+          }.bind(this))
+        }.bind(this))
+      }.bind(this))
+    })
+
+    it('saves many objects that each has one eagerly loaded object (to which they belong)', function(done) {
+      this.UserEager.create({ username: 'poobah', age: 18 }).success(function(user) {
+        this.ProjectEager.create({ title: 'homework', overdue_days: 10 }).success(function(homework) {
+          this.ProjectEager.create({ title: 'party', overdue_days: 2 }).success(function(party)  {
+            user.setProjects([homework, party]).success(function() {
+              this.ProjectEager.findAll({include: [{model: this.UserEager, as: 'Poobah'}]}).success(function(projects) {
+
+                expect(projects.length).toEqual(2)
+                expect(projects[0].poobah).toBeDefined()
+                expect(projects[1].poobah).toBeDefined()
+                expect(projects[0].poobah.username).toEqual('poobah')
+                expect(projects[1].poobah.username).toEqual('poobah')
+
+                projects[0].title        = 'partymore';
+                projects[1].title        = 'partymore';
+                projects[0].overdue_days = 0;
+                projects[1].overdue_days = 0;
+
+                projects[0].save().success(function() {
+                  projects[1].save().success(function() {
+                    this.ProjectEager.findAll({where: {title: 'partymore', overdue_days: 0}, include: [{model: this.UserEager, as: 'Poobah'}]}).success(function(savedprojects) {
+
+                      expect(savedprojects.length).toEqual(2)
+                      expect(savedprojects[0].poobah).toBeDefined()
+                      expect(savedprojects[1].poobah).toBeDefined()
+                      expect(savedprojects[0].poobah.username).toEqual('poobah')
+                      expect(savedprojects[1].poobah.username).toEqual('poobah')
+
+                      done()
+                    })
+                  }.bind(this))
+                }.bind(this))
+              }.bind(this))
+            }.bind(this))
+          }.bind(this))
+        }.bind(this))
+      }.bind(this))
+    })
   })
   })
 
 
   describe('toJSON', function toJSON() {
   describe('toJSON', function toJSON() {