fix: regression on Operation Hooks

Fix the regression introduced by 9af79cf51a where updateAttributes
and replaceAttributes was incorrectly handling the response returned
by the database.

This commit restores the old behaviour where `context.data` is updated
only by a connector-provided function and reverts an incorrect change
of a test file made as part of the faulty fix.

Besides the fixes, this patch also renames `data` to `dbResponse` and
add comments explaining the structure of the response object for more
clarity.

lib/dao.js

@@ -3087,11 +3087,15 @@ DataAccessObject.replaceById = function(id, data, options, cb) {
         var typedData = convertSubsetOfPropertiesByType(inst, data);
         context.data = typedData;
 
-        function replaceCallback(err, data) {
+        // Depending on the connector, the database response can
+        // contain information about the updated record(s). This object
+        // has usually database-specific structure and does not match
+        // model properties. For example, MySQL returns OkPacket:
+        //   {fieldCount, affectedRows, insertId, /*...*/, changedRows}
+        function replaceCallback(err, dbResponse) {
           if (err) return cb(err);
-          context.data = data;
           if (typeof connector.generateContextData === 'function') {
-            context = connector.generateContextData(context, data);
+            context = connector.generateContextData(context, dbResponse);
           }
           var ctx = {
             Model: Model,
@@ -3138,6 +3142,8 @@ DataAccessObject.replaceById = function(id, data, options, cb) {
         };
         Model.notifyObserversOf('persist', ctx, function(err) {
           if (err) return cb(err);
+          // apply updates made by "persist" observers
+          context.data = ctx.data;
           invokeConnectorMethod(connector, 'replaceById', Model, [id, Model._forDB(ctx.data)],
             options, replaceCallback);
         });
@@ -3281,11 +3287,13 @@ function(data, options, cb) {
             var typedData = convertSubsetOfPropertiesByType(inst, data);
             context.data = typedData;
 
-            function updateAttributesCallback(err, data) {
+            // Depending on the connector, the database response can
+            // contain information about the updated record(s), but also
+            // just a number of updated rows (e.g. {count: 1} for MySQL).
+            function updateAttributesCallback(err, dbResponse) {
               if (err) return cb(err);
-              context.data = data;
               if (typeof connector.generateContextData === 'function') {
-                context = connector.generateContextData(context, data);
+                context = connector.generateContextData(context, dbResponse);
               }
               var ctx = {
                 Model: Model,
@@ -3336,6 +3344,8 @@ function(data, options, cb) {
             };
             Model.notifyObserversOf('persist', ctx, function(err) {
               if (err) return cb(err);
+              // apply updates made by "persist" observers
+              context.data = ctx.data;
               invokeConnectorMethod(connector, 'updateAttributes', Model,
                 [getIdValue(Model, inst), Model._forDB(ctx.data)],
                 options, updateAttributesCallback);

test/persistence-hooks.suite.js

@@ -1529,7 +1529,7 @@ module.exports = function(dataSource, should, connectorCapabilities) {
           if (err) return done(err);
 
           ctxRecorder.records.should.eql(aCtxForModel(TestModel, {
-            data: {id: existingInstance.id, name: 'changed'},
+            data: {name: 'changed'},
             isNewInstance: false,
           }));
 
@@ -2765,6 +2765,14 @@ module.exports = function(dataSource, should, connectorCapabilities) {
         });
 
         it('triggers `persist` hook', function(done) {
+          // "extra" property is undefined by default. As a result,
+          // NoSQL connectors omit this property from the data. Because
+          // SQL connectors store it as null, we have different results
+          // depending on the database used.
+          // By enabling "persistUndefinedAsNull", we force NoSQL connectors
+          // to store unset properties using "null" value and thus match SQL.
+          TestModel.settings.persistUndefinedAsNull = true;
+
           TestModel.observe('persist', ctxRecorder.recordAndNext());
 
           existingInstance.name = 'replaced name';
@@ -2779,11 +2787,12 @@ module.exports = function(dataSource, should, connectorCapabilities) {
                 data: {
                   id: existingInstance.id,
                   name: 'replaced name',
+                  extra: null,
                 },
                 currentInstance: {
                   id: existingInstance.id,
                   name: 'replaced name',
-                  extra: undefined,
+                  extra: null,
                 },
               };