Merge pull request #308 from strongloop/fix/instance-status-after-save

Set isNewInstance to undefined for after save hook on replaceOrCreate and updateOrCreate

lib/mysql.js

@@ -280,13 +280,13 @@ MySQL.prototype._modifyOrCreate = function(model, data, options, fields, cb) {
       data.id = info.insertId;
     }
     var meta = {};
-    if (info) {
-      // When using the INSERT ... ON DUPLICATE KEY UPDATE statement,
-      // the returned value is as follows:
-      // 1 for each successful INSERT.
-      // 2 for each successful UPDATE.
-      meta.isNewInstance = (info.affectedRows === 1);
-    }
+    // When using the INSERT ... ON DUPLICATE KEY UPDATE statement,
+    // the returned value is as follows:
+    // 1 for each successful INSERT.
+    // 2 for each successful UPDATE.
+    // 1 also for UPDATE with same values, so we cannot accurately
+    // report if we have a new instance.
+    meta.isNewInstance = undefined;
     cb(err, data, meta);
   });
 };

test/mysql.test.js

@@ -4,10 +4,11 @@
 // License text available at https://opensource.org/licenses/MIT
 
 'use strict';
+var async = require('async');
 var should = require('./init.js');
 var sinon = require('sinon');
 
-var Post, PostWithStringId, PostWithUniqueTitle, PostWithNumId, db;
+var Post, PostWithStringId, PostWithUniqueTitle, PostWithNumId, Student, db;
 
 // Mock up mongodb ObjectID
 function ObjectID(id) {
@@ -57,18 +58,24 @@ describe('mysql', function() {
       content: {type: String},
     });
 
-    db.automigrate(['PostWithDefaultId', 'PostWithStringId', 'PostWithUniqueTitle', 'PostWithNumId'], function(err) {
-      should.not.exist(err);
-      done(err);
+    Student = db.define('Student', {
+      name: {type: String, length: 255},
+      age: {type: Number},
+    }, {
+      forceId: false,
     });
+
+    db.automigrate(
+      ['PostWithDefaultId', 'PostWithStringId',
+        'PostWithUniqueTitle', 'PostWithNumId', 'Student'],
+      function(err) {
+        should.not.exist(err);
+        done(err);
+      });
   });
 
-  beforeEach(function(done) {
-    Post.destroyAll(function() {
-      PostWithStringId.destroyAll(function() {
-        PostWithUniqueTitle.destroyAll(done);
-      });
-    });
+  beforeEach(function() {
+    return deleteAllModelInstances();
   });
 
   it('should allow array or object', function(done) {
@@ -236,6 +243,33 @@ describe('mysql', function() {
         });
       });
     });
+
+    it('isNewInstance should be undefined for after save hook', function(done) {
+      var student = {name: 'Joe', age: 20};
+      var newStudent = {};
+      var isNewInstanceBefore = false;
+      var isNewInstanceAfter = false;
+      Student.create(student, function(err, createdStudent) {
+        if (err) return done(err);
+        newStudent.id = createdStudent.id;
+        newStudent.name = 'Hannah';
+        newStudent.age = 25;
+        Student.observe('before save', function(ctx, next) {
+          isNewInstanceBefore = ctx.isNewInstance;
+          next();
+        });
+        Student.observe('after save', function(ctx, next) {
+          isNewInstanceAfter = ctx.isNewInstance;
+          next();
+        });
+        Student.replaceOrCreate(newStudent, function(err, s) {
+          if (err) return done(err);
+          should.not.exist(isNewInstanceBefore);
+          should.not.exist(isNewInstanceAfter);
+          done();
+        });
+      });
+    });
   });
 
   it('save should update the instance with the same id', function(done) {
@@ -876,11 +910,14 @@ describe('mysql', function() {
     });
   });
 
-  after(function(done) {
-    Post.destroyAll(function() {
-      PostWithStringId.destroyAll(function() {
-        PostWithUniqueTitle.destroyAll(done);
-      });
-    });
+  function deleteAllModelInstances() {
+    const models = [
+      Post, PostWithStringId, PostWithUniqueTitle, PostWithNumId, Student,
+    ];
+    return Promise.all(models.map(m => m.destroyAll()));
+  }
+
+  after(function() {
+    return deleteAllModelInstances();
   });
 });

test/persistence-hooks.test.js

@@ -8,5 +8,5 @@ var should = require('./init');
 var suite = require('loopback-datasource-juggler/test/persistence-hooks.suite.js');
 
 suite(global.getDataSource(), should, {
-  replaceOrCreateReportsNewInstance: true,
+  replaceOrCreateReportsNewInstance: false,
 });