From 50cb6163bd4d7825ac644fdac73c2ef9f6c4ef1c Mon Sep 17 00:00:00 2001 From: Ben Gourley Date: Fri, 23 Feb 2018 16:56:05 +0000 Subject: [PATCH] Better preparation for cloned object serialization When Utils.cloneObject() was called on an instance of a class, such as the mongo driver's `ObjectId` instance, the copy would bring across its enumerable private properties, but none of its methods, resulting in a bad serialization like so: "id": { "id": { "0": 88, "1": 129, "2": 222, "3": 117, "4": 187, "5": 221, "6": 189, "7": 57, "8": 86, "9": 86, "10": 65, "11": 202 }, "bsontype": "ObjectID" } Now just before cloning, objects are checked for a toJSON() method. If they have one, the result of that is used for the clone resulting in better serialization, such as: { "oid": "5a9045f5be123c8c5d811d35" } --- lib/utils.js | 26 +++++++++++++++----------- package.json | 1 + test/utils.js | 13 ++++++++++++- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 47274e5..91ec3a5 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -64,17 +64,18 @@ var Utils = { return clonedResults[existingIdx]; } - var copy = obj; - var type = Utils.typeOf(obj); + var orig = obj + var maybeJsonified = (Utils.typeOf(obj) === "object" && typeof obj.toJSON === 'function') ? obj.toJSON() : obj; + var type = Utils.typeOf(maybeJsonified); if (type === "object") { - copy = {}; - clonedSources.push(obj); + var copy = {}; + clonedSources.push(orig); clonedResults.push(copy); - Object.keys(obj).forEach(function (key) { - var val = obj[key]; - if (!Utils.checkOwnProperty(obj, key)) { + Object.keys(maybeJsonified).forEach(function (key) { + var val = maybeJsonified[key]; + if (!Utils.checkOwnProperty(maybeJsonified, key)) { return; } @@ -90,14 +91,17 @@ var Utils = { copy[key] = Utils.cloneObject(val, options); }); + return copy; } else if (type === "array") { - copy = []; - clonedSources.push(obj); + var copy = []; + clonedSources.push(maybeJsonified); clonedResults.push(copy); - for (var i = 0; i < obj.length; ++i) { - copy.push(Utils.cloneObject(obj[i], options)); + for (var i = 0; i < maybeJsonified.length; ++i) { + copy.push(Utils.cloneObject(maybeJsonified[i], options)); } + } else { + copy = maybeJsonified } return copy; diff --git a/package.json b/package.json index 776afb9..631ec7f 100644 --- a/package.json +++ b/package.json @@ -14,6 +14,7 @@ "stack-trace": "~0.0.9" }, "devDependencies": { + "bson-objectid": "^1.2.2", "chai": "~1.5.0", "coveralls": "^2.13.1", "cuid": "^1.3.8", diff --git a/test/utils.js b/test/utils.js index 6d7c31f..bb934e4 100644 --- a/test/utils.js +++ b/test/utils.js @@ -1,7 +1,8 @@ "use strict"; var should = require("chai").should(), - Utils = require("../lib/utils"); + Utils = require("../lib/utils"), + ObjectID = require("bson-objectid"); describe("utils", function() { describe("typeOf", function() { @@ -135,6 +136,16 @@ describe("utils", function() { clone.should.have.keys("firstKey", "secondKey"); return clone.secondKey.should.be.an("object"); }); + + it("should call an object's toJSON (if it exists) before copying", function () { + var source = { + oid: ObjectID() + }; + var clone = Utils.cloneObject(source); + should.exist(clone); + clone.should.have.keys("oid"); + (typeof clone.oid).should.equal("string"); + }); }); describe("mergeObjects", function() {