From f1fa976f50ad6867a60e1a6d54fb81c2c3a9e005 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= <mbajtoss@gmail.com>
Date: Thu, 25 Jul 2019 11:33:59 +0200
Subject: [PATCH] Fix coercion of PK value in `replaceById` method
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Before this change, when both the PK value (`id`) and the `data` object
were provided as plain-data values (e.g. as received in a JSON request),
and the connector was using a complex PK type (e.g. `ObjectID`
in MongoDB), then `replaceById` operation was printing confusing
warnings:

    WARNING: id property cannot be changed from 5d39775a59f5f541513c5e05
        to 5d39775a59f5f541513c5e05 for model:Post
        in 'before save' operation hook

    WARNING: id property cannot be changed from 5d39775a59f5f541513c5e05
        to 5d39775a59f5f541513c5e05 for model:Post
        in 'loaded' operation hook

This commit fixes the problem by applying the same type coercion on the
PK value (`id`) as has been applied by the model constructor on the PK
property (`data.id`).

Signed-off-by: Miroslav Bajtoš <mbajtoss@gmail.com>
---
 lib/dao.js                |  5 +++++
 test/manipulation.test.js | 27 +++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/lib/dao.js b/lib/dao.js
index 887711df..4db12e8d 100644
--- a/lib/dao.js
+++ b/lib/dao.js
@@ -2609,6 +2609,11 @@ DataAccessObject.replaceById = function(id, data, options, cb) {
     err.statusCode = 400;
     process.nextTick(function() { cb(err); });
     return cb.promise;
+  } else {
+    // Ensure any type conversion applied by the instance constructor
+    // on `data.id` is applied on the `id` value too.
+    // Typically, MongoDB converts PK value from a string to an ObjectID.
+    id = inst[pkName];
   }
 
   let context = {
diff --git a/test/manipulation.test.js b/test/manipulation.test.js
index b49e186b..c6015b19 100644
--- a/test/manipulation.test.js
+++ b/test/manipulation.test.js
@@ -1576,6 +1576,33 @@ describe('manipulation', function() {
         done();
       });
     });
+
+    it('correctly coerces the PK value', async () => {
+      const created = await Post.create({
+        title: 'a title',
+        content: 'a content',
+      });
+
+      // Emulate what happens when model instance is received by REST API clients
+      const data = JSON.parse(JSON.stringify(created));
+
+      // Modify some of the data
+      data.title = 'Draft';
+
+      // Call replaceById to modify the database record
+      await Post.replaceById(data.id, data);
+
+      // Verify what has been stored
+      const found = await Post.findById(data.id);
+      found.toObject().should.eql({
+        id: created.id,
+        title: 'Draft',
+        content: 'a content',
+      });
+
+      // Verify that no warnings were triggered
+      Object.keys(Post._warned).should.be.empty();
+    });
   });
 
   describe('findOrCreate', function() {