From ad44168d4ae7693c1576ddf72cb725f485f80e2b Mon Sep 17 00:00:00 2001
From: biniam <biniam@ca.ibm.com>
Date: Wed, 19 Jul 2017 00:06:21 -0400
Subject: [PATCH] isNewInstance undefined for after save hook

Since using REPLACE or INSERT...ON DUPLICATE KEY
statements both give us affected rows of 1 for a
new row or a row which got updated with the same
values, we make isNewInstance undefined on after
save hook for save, replaceOrCreate, and
updateOrCreate methods. Also, disable juggler
tests for that functionality.
---
 lib/mysql.js                   | 14 +++----
 test/mysql.test.js             | 69 ++++++++++++++++++++++++++--------
 test/persistence-hooks.test.js |  2 +-
 3 files changed, 61 insertions(+), 24 deletions(-)

diff --git a/lib/mysql.js b/lib/mysql.js
index f817300..9c9b4ea 100644
--- a/lib/mysql.js
+++ b/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);
   });
 };
diff --git a/test/mysql.test.js b/test/mysql.test.js
index d94b7cb..9d088e7 100644
--- a/test/mysql.test.js
+++ b/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();
   });
 });
diff --git a/test/persistence-hooks.test.js b/test/persistence-hooks.test.js
index ddcb28b..e241e2a 100644
--- a/test/persistence-hooks.test.js
+++ b/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,
 });