From 0b95f7eba214f053ac6dc43a7203910426f5fdd6 Mon Sep 17 00:00:00 2001 From: Luca Pizzini Date: Thu, 13 Oct 2022 11:29:04 +0200 Subject: [PATCH 1/6] feat(document): add closes #11849 --- lib/document.js | 21 +++++++++++++++++++++ test/document.test.js | 14 ++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/lib/document.js b/lib/document.js index 5b9d3fcc5c7..dcfdb0c8a19 100644 --- a/lib/document.js +++ b/lib/document.js @@ -4582,6 +4582,27 @@ Document.prototype.getChanges = function() { return changes; }; +/** + * Returns a copy of this document with a deep clone of `_doc` and `$__`. + * + * @return {Document} a copy of this document + * @api private + * @method $clone + * @memberOf Document + * @instance + */ + +Document.prototype.$clone = function() { + const clonedDoc = Object.assign(Object.create(Object.getPrototypeOf(this)), this); + if (this._doc) { + clonedDoc._doc = clone(this._doc); + } + if (this.$__) { + clonedDoc.$__ = Object.assign(Object.create(Object.getPrototypeOf(this.$__)), this.$__); + } + return clonedDoc; +}; + /*! * Module exports. */ diff --git a/test/document.test.js b/test/document.test.js index 2d38adc8629..7d3343bc48e 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -11928,6 +11928,20 @@ describe('document', function() { assert.ok(rawDoc); assert.deepStrictEqual(rawDoc.tags, ['mongodb']); }); + + it('$clone() (gh-11849)', async function() { + const schema = new mongoose.Schema({ name: String }); + const Test = db.model('Test', schema); + + const item = await Test.create({ name: 'Mongoose' }); + + const doc = await Test.findById(item._id); + const clonedDoc = doc.$clone(); + + assert.deepEqual(doc, clonedDoc); + assert.deepEqual(doc._doc, clonedDoc._doc); + assert.deepEqual(doc.$__, clonedDoc.$__); + }); }); describe('Check if instance function that is supplied in schema option is availabe', function() { From c7f051e800b43a02c3563cfcd7c286047b2b16ad Mon Sep 17 00:00:00 2001 From: Luca Pizzini Date: Wed, 19 Oct 2022 10:25:54 +0200 Subject: [PATCH 2/6] applied requested changes --- lib/document.js | 13 ++++++++++-- test/document.test.js | 48 ++++++++++++++++++++++++++++++++++++++----- 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/lib/document.js b/lib/document.js index dcfdb0c8a19..01abac79063 100644 --- a/lib/document.js +++ b/lib/document.js @@ -4593,12 +4593,21 @@ Document.prototype.getChanges = function() { */ Document.prototype.$clone = function() { - const clonedDoc = Object.assign(Object.create(Object.getPrototypeOf(this)), this); + const Model = this.constructor; + const clonedDoc = new Model(); + for (const p of Object.getOwnPropertyNames(this)) { + clonedDoc[p] = clone(this[p]); + } if (this._doc) { clonedDoc._doc = clone(this._doc); } if (this.$__) { - clonedDoc.$__ = Object.assign(Object.create(Object.getPrototypeOf(this.$__)), this.$__); + const Cache = this.$__.constructor; + const clonedCache = new Cache(); + for (const p of Object.getOwnPropertyNames(this.$__)) { + clonedCache[p] = clone(this.$__[p]); + } + clonedDoc.$__ = clonedCache; } return clonedDoc; }; diff --git a/test/document.test.js b/test/document.test.js index 7d3343bc48e..3601f90030e 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -11930,17 +11930,55 @@ describe('document', function() { }); it('$clone() (gh-11849)', async function() { - const schema = new mongoose.Schema({ name: String }); + const schema = new mongoose.Schema({ + name: { + type: String, + validate: { + validator: (v) => v !== 'Invalid' + } + } + }); const Test = db.model('Test', schema); - const item = await Test.create({ name: 'Mongoose' }); + const item = await Test.create({ name: 'Test' }); const doc = await Test.findById(item._id); const clonedDoc = doc.$clone(); - assert.deepEqual(doc, clonedDoc); - assert.deepEqual(doc._doc, clonedDoc._doc); - assert.deepEqual(doc.$__, clonedDoc.$__); + assert.deepEqual(clonedDoc, doc); + assert.deepEqual(clonedDoc._doc, doc._doc); + assert.deepEqual(clonedDoc.$__, doc.$__); + + // Editing a field in the cloned doc does not effect + // the original doc + clonedDoc.name = 'Test 2'; + assert.equal(doc.name, 'Test'); + assert.equal(clonedDoc.name, 'Test 2'); + + // Saving the cloned doc does not effect `modifiedPaths` + // in the original doc + const modifiedPaths = doc.modifiedPaths; + await clonedDoc.save(); + assert.deepEqual(doc.modifiedPaths, modifiedPaths); + + // Cloning a doc with invalid field preserve the + // invalid field value + doc.name = 'Invalid'; + await assert.rejects(async() => { + await doc.validate(); + }); + const invalidClonedDoc = doc.$clone(); + doc.name = 'Test'; + await assert.rejects(async() => { + await invalidClonedDoc.validate(); + }); + + // Setting a session on the cloned doc does not + // affect the session in the original doc + const session = await Test.startSession(); + clonedDoc.$session(session); + assert.strictEqual(doc.$session(), null); + assert.strictEqual(clonedDoc.$session(), session); }); }); From faa3e904201fc586d414685a31177fa033c52aa3 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 20 Oct 2022 14:38:11 -0400 Subject: [PATCH 3/6] refactor: try removing additional property clone --- lib/document.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/document.js b/lib/document.js index 01abac79063..b81909698f6 100644 --- a/lib/document.js +++ b/lib/document.js @@ -4595,9 +4595,6 @@ Document.prototype.getChanges = function() { Document.prototype.$clone = function() { const Model = this.constructor; const clonedDoc = new Model(); - for (const p of Object.getOwnPropertyNames(this)) { - clonedDoc[p] = clone(this[p]); - } if (this._doc) { clonedDoc._doc = clone(this._doc); } From 2a6302cb401130d6888630c0ce195d1c8cb3be37 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 21 Oct 2022 11:35:26 -0400 Subject: [PATCH 4/6] fix: make document $clone() correctly clone active paths re: #12574 --- lib/document.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/document.js b/lib/document.js index b81909698f6..bd28b440222 100644 --- a/lib/document.js +++ b/lib/document.js @@ -4595,15 +4595,20 @@ Document.prototype.getChanges = function() { Document.prototype.$clone = function() { const Model = this.constructor; const clonedDoc = new Model(); + clonedDoc.$isNew = this.$isNew; if (this._doc) { clonedDoc._doc = clone(this._doc); } if (this.$__) { const Cache = this.$__.constructor; const clonedCache = new Cache(); - for (const p of Object.getOwnPropertyNames(this.$__)) { - clonedCache[p] = clone(this.$__[p]); + for (const key of Object.getOwnPropertyNames(this.$__)) { + if (key === 'activePaths') { + continue; + } + clonedCache[key] = clone(this.$__[key]); } + Object.assign(clonedCache.activePaths, clone({ ...this.$__.activePaths })); clonedDoc.$__ = clonedCache; } return clonedDoc; From b0d294bd920bd790c6b4ffc0282e3423b33c9a0f Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Fri, 21 Oct 2022 11:36:12 -0400 Subject: [PATCH 5/6] test: more robust tests for document $clone() --- test/document.test.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/test/document.test.js b/test/document.test.js index 33987537d38..590d0f51268 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -11954,22 +11954,28 @@ describe('document', function() { clonedDoc.name = 'Test 2'; assert.equal(doc.name, 'Test'); assert.equal(clonedDoc.name, 'Test 2'); + assert.ok(!doc.$isModified('name')); + assert.ok(clonedDoc.$isModified('name')); // Saving the cloned doc does not effect `modifiedPaths` // in the original doc - const modifiedPaths = doc.modifiedPaths; + const modifiedPaths = [...doc.modifiedPaths()]; await clonedDoc.save(); - assert.deepEqual(doc.modifiedPaths, modifiedPaths); + assert.deepEqual(doc.modifiedPaths(), modifiedPaths); // Cloning a doc with invalid field preserve the // invalid field value doc.name = 'Invalid'; - await assert.rejects(async() => { + await assert.rejects(async () => { await doc.validate(); }); + + await clonedDoc.validate(); + const invalidClonedDoc = doc.$clone(); doc.name = 'Test'; - await assert.rejects(async() => { + await doc.validate(); + await assert.rejects(async () => { await invalidClonedDoc.validate(); }); From be2715af593a5b55f31722d5ba71ad7039261722 Mon Sep 17 00:00:00 2001 From: Luca Pizzini Date: Sat, 22 Oct 2022 10:15:00 +0200 Subject: [PATCH 6/6] fix lint --- test/document.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/document.test.js b/test/document.test.js index 590d0f51268..b0f4ea132c3 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -11966,7 +11966,7 @@ describe('document', function() { // Cloning a doc with invalid field preserve the // invalid field value doc.name = 'Invalid'; - await assert.rejects(async () => { + await assert.rejects(async() => { await doc.validate(); }); @@ -11975,7 +11975,7 @@ describe('document', function() { const invalidClonedDoc = doc.$clone(); doc.name = 'Test'; await doc.validate(); - await assert.rejects(async () => { + await assert.rejects(async() => { await invalidClonedDoc.validate(); }); @@ -11986,7 +11986,7 @@ describe('document', function() { assert.strictEqual(doc.$session(), null); assert.strictEqual(clonedDoc.$session(), session); }); - + it('can create document with document array and top-level key named `schema` (gh-12480)', async function() { const AuthorSchema = new Schema({ fullName: { type: 'String', required: true }