Merge pull request #599 from PradnyaBaviskar/issue-441

Add new hook 'loaded'

lib/dao.js

@@ -285,6 +285,22 @@ DataAccessObject.create = function (data, options, cb) {
           }
           obj.__persisted = true;
 
+          var context = {
+            Model: Model,
+            data: val,
+            isNewInstance: true,
+            hookState: hookState,
+            options: options
+          };
+          Model.notifyObserversOf('loaded', context, function(err) {
+
+            // By default, the instance passed to create callback is NOT updated 
+            // with the changes made through persist/loaded hooks. To preserve 
+            // backwards compatibility, we introduced a new setting updateOnLoad, 
+            // which if set, will apply these changes to the model instance too.
+            if(Model.settings.updateOnLoad) {
+              obj.setAttributes(context.data);
+            }
             saveDone.call(obj, function () {
               createDone.call(obj, function () {
                 if (err) {
@@ -297,12 +313,14 @@ DataAccessObject.create = function (data, options, cb) {
                   hookState: hookState,
                   options: options
                 };
+
                 Model.notifyObserversOf('after save', context, function(err) {
                   cb(err, obj);
                   if(!err) Model.emit('changed', obj);
                 });
               });
             });
+          });
         }
 
         context = {
@@ -475,6 +493,13 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data
           });
         }
         function done(err, data, info) {
+          var context = {
+            Model: Model,
+            data: data,
+            hookState: ctx.hookState,
+            options: options
+          };
+          Model.notifyObserversOf('loaded', context, function(err) {
             var obj;
             if (data && !(data instanceof Model)) {
               inst._initProperties(data, { persisted: true });
@@ -495,6 +520,7 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data
                 hookState: hookState,
                 options: options
               };
+
               Model.notifyObserversOf('after save', context, function(err) {
                 cb(err, obj);
                 if(!err) {
@@ -502,6 +528,7 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data
                 }
               });
             }
+          });
         }
       });
     } else {
@@ -586,6 +613,14 @@ DataAccessObject.findOrCreate = function findOrCreate(query, data, options, cb)
   function _findOrCreate(query, data, currentInstance) {
     var modelName = self.modelName;
     function findOrCreateCallback(err, data, created) {
+      var context = {
+        Model: Model,
+        data: data,
+        isNewInstance: created,
+        hookState: hookState,
+        options: options
+      };
+      Model.notifyObserversOf('loaded', context, function(err) {
         var obj, Model = self.lookupModel(data);
 
         if (data) {
@@ -609,6 +644,7 @@ DataAccessObject.findOrCreate = function findOrCreate(query, data, options, cb)
             }
             if (!err) Model.emit('changed', obj);
           });
+
         } else {
           if (cb.promise) {
             cb(err, [obj, created]);
@@ -616,6 +652,7 @@ DataAccessObject.findOrCreate = function findOrCreate(query, data, options, cb)
             cb(err, obj, created);
           }
         }
+      });
     }
 
     data = removeUndefined(data);
@@ -1329,7 +1366,16 @@ DataAccessObject.find = function find(query, options, cb) {
         var Model = self.lookupModel(d);
         var obj = new Model(d, {fields: query.fields, applySetters: false, persisted: true});
 
+        context = {
+          Model: Model,
+          instance: obj,
+          isNewInstance: false,
+          hookState: hookState,
+          options: options
+        };
+
         if (query && query.include) {
+          Model.notifyObserversOf('loaded', context, function(err) {
             if (query.collect) {
               // The collect property indicates that the query is to return the
               // standalone items for a related model, not as child of the parent object
@@ -1358,9 +1404,12 @@ DataAccessObject.find = function find(query, options, cb) {
               });
               delete obj.__data.__cachedRelations;
             }
+          });
         }
         if (obj !== undefined) {
+          Model.notifyObserversOf('loaded', context, function(err) {
             results.push(obj);
+          });
         }
       }
 
@@ -1803,7 +1852,17 @@ DataAccessObject.prototype.save = function (options, cb) {
             if (err) {
               return cb(err, inst);
             }
+
+            var context = {
+              Model: Model,
+              data: data,
+              isNewInstance: result && result.isNewInstance,
+              hookState: hookState,
+              options: options
+            };
+            Model.notifyObserversOf('loaded', context, function(err) {
               inst._initProperties(data, { persisted: true });
+
               var context = {
                 Model: Model,
                 instance: inst,
@@ -1822,6 +1881,7 @@ DataAccessObject.prototype.save = function (options, cb) {
                   });
                 });
               });
+            });
           }
 
           context = {
@@ -1956,6 +2016,7 @@ DataAccessObject.updateAll = function (where, data, options, cb) {
 
     function updateCallback(err, info) {
       if (err) return cb (err);
+
       var context = {
         Model: Model,
         where: where,
@@ -2291,10 +2352,26 @@ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, op
           }
 
           function updateAttributesCallback(err) {
+            var context = {
+              Model: Model,
+              data: data,
+              hookState: hookState,
+              options: options
+            };
+            Model.notifyObserversOf('loaded', context, function(err) {
               if (!err) inst.__persisted = true;
+
+              // By default, the instance passed to updateAttributes callback is NOT updated 
+              // with the changes made through persist/loaded hooks. To preserve 
+              // backwards compatibility, we introduced a new setting updateOnLoad, 
+              // which if set, will apply these changes to the model instance too.
+              if(Model.settings.updateOnLoad) {
+                inst.setAttributes(context.data);
+              }
               done.call(inst, function () {
                 saveDone.call(inst, function () {
                   if (err) return cb(err, inst);
+
                   var context = {
                     Model: Model,
                     instance: inst,
@@ -2308,6 +2385,7 @@ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, op
                   });
                 });
               });
+            });
           }
 
           context = {

test/persistence-hooks.suite.js

@@ -6,6 +6,7 @@ module.exports = function(dataSource, should) {
     var observedContexts, expectedError, observersCalled;
     var TestModel, existingInstance;
     var migrated = false, lastId;
+    var triggered;
 
     var undefinedValue = undefined;
 
@@ -112,6 +113,24 @@ module.exports = function(dataSource, should) {
     });
 
     describe('PersistedModel.create', function() {
+      it('triggers hooks in the correct order', function(done) {
+        monitorHookExecution();
+
+        TestModel.create(
+          { name: 'created' },
+          function(err, record, created) {
+            if (err) return done(err);
+
+            triggered.should.eql([
+              'before save',
+              'persist',
+              'loaded',
+              'after save'
+            ]);
+            done();
+          });
+      });
+
       it('triggers `before save` hook', function(done) {
         TestModel.observe('before save', pushContextAndNext());
 
@@ -210,16 +229,19 @@ module.exports = function(dataSource, should) {
           ctx.data.extra = 'hook data';
         }));
 
+        // By default, the instance passed to create callback is NOT updated 
+        // with the changes made through persist/loaded hooks. To preserve 
+        // backwards compatibility, we introduced a new setting updateOnLoad, 
+        // which if set, will apply these changes to the model instance too.
+        TestModel.settings.updateOnLoad = true;
         TestModel.create(
           { id: 'new-id', name: 'a name' },
           function(err, instance) {
             if (err) return done(err);
 
-            // the, instance returned by `create` context does not have the
+            instance.should.have.property('extra', 'hook data');
-            // values updated from `persist` hook
-            instance.should.not.have.property('extra', 'hook data');
 
-            // So, we must query the database here because on `create`
+            // Also query the database here to verify that, on `create` 
             // updates from `persist` hook are reflected into database
             TestModel.findById('new-id', function(err, dbInstance) {
               if (err) return done(err);
@@ -234,6 +256,48 @@ module.exports = function(dataSource, should) {
           });
       });
 
+      it('triggers `loaded` hook', function(done) {
+        TestModel.observe('loaded', pushContextAndNext());
+
+        // By default, the instance passed to create callback is NOT updated 
+        // with the changes made through persist/loaded hooks. To preserve 
+        // backwards compatibility, we introduced a new setting updateOnLoad, 
+        // which if set, will apply these changes to the model instance too.
+        TestModel.settings.updateOnLoad = true;
+        TestModel.create(
+          { id: 'new-id', name: 'a name' },
+          function(err, instance) {
+            if (err) return done(err);
+
+            observedContexts.should.eql(aTestModelCtx({
+              data: { id: 'new-id', name: 'a name' },
+              isNewInstance: true
+            }));
+
+            done();
+          });
+      });
+
+      it('applies updates from `loaded` hook', function(done) {
+        TestModel.observe('loaded', pushContextAndNext(function(ctx){
+          ctx.data.extra = 'hook data';
+        }));
+
+        // By default, the instance passed to create callback is NOT updated 
+        // with the changes made through persist/loaded hooks. To preserve 
+        // backwards compatibility, we introduced a new setting updateOnLoad, 
+        // which if set, will apply these changes to the model instance too.
+        TestModel.settings.updateOnLoad = true;
+        TestModel.create(
+          { id: 'new-id', name: 'a name' },
+          function(err, instance) {
+            if (err) return done(err);
+
+            instance.should.have.property('extra', 'hook data');
+            done();
+          });
+      });
+
       it('triggers `after save` hook', function(done) {
         TestModel.observe('after save', pushContextAndNext());
 
@@ -405,12 +469,7 @@ module.exports = function(dataSource, should) {
       });
 
       it('triggers hooks in the correct order when not found', function(done) {
-        var triggered = [];
+        monitorHookExecution();
-        TestModel._notify = TestModel.notifyObserversOf;
-        TestModel.notifyObserversOf = function(operation, context, callback) {
-          triggered.push(operation);
-          this._notify.apply(this, arguments);
-        };
 
         TestModel.findOrCreate(
           { where: { name: 'new-record' } },
@@ -421,12 +480,39 @@ module.exports = function(dataSource, should) {
               'access',
               'before save',
               'persist',
+              'loaded',
               'after save'
             ]);
             done();
           });
       });
 
+      it('triggers hooks in the correct order when found', function(done) {
+        monitorHookExecution();
+
+        TestModel.findOrCreate(
+          { where: { name: existingInstance.name } },
+          { name: existingInstance.name },
+          function(err, record, created) {
+            if (err) return done(err);
+
+            if (dataSource.connector.findOrCreate) {
+              triggered.should.eql([
+                'access',
+                'before save',
+                'persist',
+                'loaded'
+              ]);
+            } else {
+              triggered.should.eql([
+                'access',
+                'loaded'
+              ]);
+            }
+            done();
+          });
+      });
+
       it('aborts when `access` hook fails', function(done) {
         TestModel.observe('access', nextWithError(expectedError));
 
@@ -596,6 +682,98 @@ module.exports = function(dataSource, should) {
           });
       });
 
+      if (dataSource.connector.findOrCreate) {
+        it('triggers `loaded` hook when found', function(done) {
+          TestModel.observe('loaded', pushContextAndNext());
+
+          TestModel.findOrCreate(
+            { where: { name: existingInstance.name } },
+            { name: existingInstance.name },
+            function(err, record, created) {
+              if (err) return done(err);
+
+              record.id.should.eql(existingInstance.id);
+
+               // After the call to `connector.findOrCreate`, since the record 
+               // already exists, `data.id` matches `existingInstance.id` 
+               // as against the behaviour noted for `persist` hook 
+              observedContexts.should.eql(aTestModelCtx({
+                data: {
+                  id: existingInstance.id,
+                  name: existingInstance.name
+                },
+                isNewInstance: false
+              }));
+
+              done();
+            });
+        });
+      }
+
+      it('triggers `loaded` hook when not found', function(done) {
+        TestModel.observe('loaded', pushContextAndNext());
+
+        TestModel.findOrCreate(
+          { where: { name: 'new-record' } },
+          { name: 'new-record' },
+          function(err, record, created) {
+            if (err) return done(err);
+
+            observedContexts.should.eql(aTestModelCtx({
+              data: {
+                id: record.id,
+                name: 'new-record'
+              },
+              isNewInstance: true
+            }));
+
+            done();
+          });
+      });
+
+      if (dataSource.connector.findOrCreate) {
+        it('applies updates from `loaded` hook when found', function(done) {
+          TestModel.observe('loaded', pushContextAndNext(function(ctx){
+            ctx.data.extra = 'hook data';
+          }));
+
+          TestModel.findOrCreate(
+            { where: { name: existingInstance.name } },
+            { name: existingInstance.name },
+            function(err, instance) {
+              if (err) return done(err);
+
+              instance.should.have.property('extra', 'hook data');
+
+              done();
+            });
+        });
+      }
+
+      it('applies updates from `loaded` hook when not found', function(done) {
+        TestModel.observe('loaded', pushContextAndNext(function(ctx){
+          ctx.data.extra = 'hook data';
+        }));
+
+        // Unoptimized connector gives a call to `create. But,
+        // by default, the instance passed to create callback is NOT updated 
+        // with the changes made through persist/loaded hooks. To preserve 
+        // backwards compatibility, we introduced a new setting updateOnLoad, 
+        // which if set, will apply these changes to the model instance too.
+        // Note - in case of findOrCreate, this setting is needed ONLY for
+        // unoptimized connector.
+        TestModel.settings.updateOnLoad = true;
+        TestModel.findOrCreate(
+          { where: { name: 'new-record' } },
+          { name: 'new-record' },
+          function(err, instance) {
+            if (err) return done(err);
+
+            instance.should.have.property('extra', 'hook data');
+            done();
+          });
+      });
+
       it('triggers `after save` hook when not found', function(done) {
         TestModel.observe('after save', pushContextAndNext());
 
@@ -658,6 +836,22 @@ module.exports = function(dataSource, should) {
     });
 
     describe('PersistedModel.prototype.save', function() {
+      it('triggers hooks in the correct order', function(done) {
+        monitorHookExecution();
+
+        existingInstance.save(
+          function(err, record, created) {
+            if (err) return done(err);
+            triggered.should.eql([
+              'before save',
+              'persist',
+              'loaded',
+              'after save'
+            ]);
+            done();
+          });
+      });
+
       it('triggers `before save` hook', function(done) {
         TestModel.observe('before save', pushContextAndNext());
 
@@ -745,6 +939,38 @@ module.exports = function(dataSource, should) {
         });
       });
 
+      it('triggers `loaded` hook', function(done) {
+        TestModel.observe('loaded', pushContextAndNext());
+
+        existingInstance.name = 'changed';
+        existingInstance.save(function(err, instance) {
+          if (err) return done(err);
+
+          observedContexts.should.eql(aTestModelCtx({
+            data: {
+              id: existingInstance.id,
+              name: 'changed'
+            },
+            isNewInstance: false,
+            options: { throws: false, validate: true }
+          }));
+
+          done();
+        });
+      });
+
+      it('applies updates from `loaded` hook', function(done) {
+        TestModel.observe('loaded', pushContextAndNext(function(ctx){
+          ctx.data.extra = 'hook data';
+        }));
+
+        existingInstance.save(function(err, instance) {
+          if (err) return done(err);
+          instance.should.have.property('extra', 'hook data');
+          done();
+        });
+      });
+
       it('triggers `after save` hook on update', function(done) {
         TestModel.observe('after save', pushContextAndNext());
 
@@ -811,6 +1037,23 @@ module.exports = function(dataSource, should) {
     });
 
     describe('PersistedModel.prototype.updateAttributes', function() {
+      it('triggers hooks in the correct order', function(done) {
+        monitorHookExecution();
+
+        existingInstance.updateAttributes(
+          { name: 'changed' },
+          function(err, record, created) {
+            if (err) return done(err);
+            triggered.should.eql([
+              'before save',
+              'persist',
+              'loaded',
+              'after save'
+            ]);
+            done();
+          });
+      });
+
       it('triggers `before save` hook', function(done) {
         TestModel.observe('before save', pushContextAndNext());
 
@@ -894,7 +1137,42 @@ module.exports = function(dataSource, should) {
           ctx.data.extra = 'hook data';
         }));
 
-        existingInstance.save(function(err, instance) {
+        // By default, the instance passed to updateAttributes callback is NOT updated 
+        // with the changes made through persist/loaded hooks. To preserve 
+        // backwards compatibility, we introduced a new setting updateOnLoad, 
+        // which if set, will apply these changes to the model instance too.
+        TestModel.settings.updateOnLoad = true;
+        existingInstance.updateAttributes({ name: 'changed' }, function(err, instance) {
+          if (err) return done(err);
+          instance.should.have.property('extra', 'hook data');
+          done();
+        });
+      });
+
+      it('triggers `loaded` hook', function(done) {
+        TestModel.observe('loaded', pushContextAndNext());
+        existingInstance.updateAttributes({ name: 'changed' }, function(err) {
+          if (err) return done(err);
+
+          observedContexts.should.eql(aTestModelCtx({
+            data: { name: 'changed' }
+          }));
+
+          done();
+        });
+      });
+
+      it('applies updates from `loaded` hook updateAttributes', function(done) {
+        TestModel.observe('loaded', pushContextAndNext(function(ctx){
+          ctx.data.extra = 'hook data';
+        }));
+
+        // By default, the instance passed to updateAttributes callback is NOT updated 
+        // with the changes made through persist/loaded hooks. To preserve 
+        // backwards compatibility, we introduced a new setting updateOnLoad, 
+        // which if set, will apply these changes to the model instance too.
+        TestModel.settings.updateOnLoad = true;
+        existingInstance.updateAttributes({ name: 'changed' }, function(err, instance) {
           if (err) return done(err);
           instance.should.have.property('extra', 'hook data');
           done();
@@ -944,6 +1222,53 @@ module.exports = function(dataSource, should) {
     });
 
     describe('PersistedModel.updateOrCreate', function() {
+      it('triggers hooks in the correct order on create', function(done) {
+        monitorHookExecution();
+
+        TestModel.updateOrCreate(
+          { id: 'not-found', name: 'not found' },
+          function(err, record, created) {
+            if (err) return done(err);
+            triggered.should.eql([
+              'access',
+              'before save',
+              'persist',
+              'loaded',
+              'after save'
+            ]);
+            done();
+          });
+      });
+
+      it('triggers hooks in the correct order on update', function(done) {
+        monitorHookExecution();
+
+        TestModel.updateOrCreate(
+          { id: existingInstance.id, name: 'new name' },
+          function(err, record, created) {
+            if (err) return done(err);
+            if (dataSource.connector.updateOrCreate) {
+              triggered.should.eql([
+                'access',
+                'before save',
+                'persist',
+                'loaded',
+                'after save'
+              ]);
+            } else {
+              triggered.should.eql([
+                'access',
+                'loaded',
+                'before save',
+                'persist',
+                'loaded',
+                'after save'
+              ]);
+            }
+            done();
+          });
+      });
+
       it('triggers `access` hook on create', function(done) {
         TestModel.observe('access', pushContextAndNext());
 
@@ -1225,6 +1550,71 @@ module.exports = function(dataSource, should) {
           });
       });
 
+      it('triggers `loaded` hook on create', function(done) {
+        TestModel.observe('loaded', pushContextAndNext());
+
+        TestModel.updateOrCreate(
+          { id: 'new-id', name: 'a name' },
+          function(err, instance) {
+            if (err) return done(err);
+
+            if (dataSource.connector.updateOrCreate) {
+              observedContexts.should.eql(aTestModelCtx({
+                data: { id: 'new-id', name: 'a name' }
+              }));
+            } else {
+              observedContexts.should.eql(aTestModelCtx({
+                data: {
+                  id: 'new-id',
+                  name: 'a name'
+                },
+                isNewInstance: true
+              }));
+            }
+            done();
+          });
+      });
+
+      it('triggers `loaded` hook on update', function(done) {
+        TestModel.observe('loaded', pushContextAndNext());
+
+        TestModel.updateOrCreate(
+          { id: existingInstance.id, name: 'updated name' },
+          function(err, instance) {
+            if (err) return done(err);
+
+            if (dataSource.connector.updateOrCreate) {
+              observedContexts.should.eql(aTestModelCtx({
+                data: {
+                  id: existingInstance.id,
+                  name: 'updated name'
+                }
+              }));
+            } else {
+              // For Unoptimized connector, the callback function `pushContextAndNext`  
+              // is called twice. As a result, observedContexts
+              // returns an array and NOT a single instance.
+              observedContexts.should.eql([
+                aTestModelCtx({
+                  instance: {
+                    id: existingInstance.id,
+                    name: 'first',
+                    extra: null
+                  },
+                  isNewInstance: false,
+                  options: { notify: false }
+                }),
+                aTestModelCtx({
+                  data: {
+                    id: existingInstance.id,
+                    name: 'updated name'
+                  }
+                })
+              ]);
+            }
+            done();
+          });
+      });
 
       it('triggers `after save` hook on update', function(done) {
         TestModel.observe('after save', pushContextAndNext());
@@ -1646,6 +2036,19 @@ module.exports = function(dataSource, should) {
           });
       });
 
+      it('does not trigger `loaded`', function(done) {
+        TestModel.observe('loaded', pushContextAndNext());
+
+        TestModel.updateAll(
+          { where: { id: existingInstance.id } },
+          { name: 'changed' },
+          function(err, instance) {
+            if (err) return done(err);
+            observedContexts.should.eql("hook not called");
+            done();
+          });
+      });
+
       it('triggers `after save` hook', function(done) {
         TestModel.observe('after save', pushContextAndNext());
 
@@ -1759,6 +2162,15 @@ module.exports = function(dataSource, should) {
     function getLastGeneratedUid() {
       return '' + lastId;
     }
+
+    function monitorHookExecution() {
+      triggered = [];
+      TestModel._notify = TestModel.notifyObserversOf;
+      TestModel.notifyObserversOf = function(operation, context, callback) {
+        triggered.push(operation);
+        this._notify.apply(this, arguments);
+      };
+    }
   });
 
   function deepCloneToObject(obj) {