From bb09e65a0b23efb150838bc3953bba9539ec928d Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Tue, 2 Feb 2021 17:16:48 -0500 Subject: [PATCH 1/4] Update document.test.js --- test/document.test.js | 47 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/document.test.js b/test/document.test.js index a96e19111e9..3d1bd52e391 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -9931,4 +9931,51 @@ describe('document', function() { assert.ok(doc); }); }); + it('gh-9885', function() { + const SubSchema = new Schema({ + myValue: { + type: String + } + }, {}); + SubSchema.pre('remove', function(){ + console.log('Subdoc got removed!'); + }); + + const thisSchema = new Schema({ + foo: { + type: String, + required: true + }, + mySubdoc: { + type: [SubSchema], + required: true + } + }, {minimize: false, collection: 'test'}); + + const Model = db.model('TestModel',thisSchema); + const test = co(function*(){ + yield Model.deleteMany({}); // remove all existing documents + const newModel = { + foo: 'bar', + mySubdoc: [{myValue: 'some value'}] + }; + const document = yield Model.create(newModel); + console.log('Created Document'); + console.log('document', document); + console.log('Removing subDocument'); + document.mySubdoc[0].remove(); + console.log('Saving document'); + yield document.save().catch((error) => { + console.error(error); + process.exit(1); + }); + console.log('document: ',document); + console.log(`Notice that SubSchema.pre('remove') never ran`); + }); + const main = co(function*(){ + yield test; + process.exit(0); + }); + main; + }); }); From a031543e4cbe2bdade12607ab3d4840ca56fd55a Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Tue, 2 Feb 2021 18:02:28 -0500 Subject: [PATCH 2/4] for work tomorrow --- test/document.test.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/test/document.test.js b/test/document.test.js index 3d1bd52e391..2b03e4d8744 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -9937,9 +9937,9 @@ describe('document', function() { type: String } }, {}); - SubSchema.pre('remove', function(){ + SubSchema.pre('remove', co.wrap(function*(){ console.log('Subdoc got removed!'); - }); + })); const thisSchema = new Schema({ foo: { @@ -9965,17 +9965,19 @@ describe('document', function() { console.log('Removing subDocument'); document.mySubdoc[0].remove(); console.log('Saving document'); - yield document.save().catch((error) => { + // had to remove yield for the rest to execute but now saying error. + document.save().catch((error) => { console.error(error); process.exit(1); }); - console.log('document: ',document); + console.log('document: ', document); console.log(`Notice that SubSchema.pre('remove') never ran`); }); - const main = co(function*(){ - yield test; - process.exit(0); + const main = co.wrap(function*(){ + return yield test; + }); + main(true).then(function(){ + process.exit(1); }); - main; }); }); From f1f2efc1c20947f33792c01daa3eb9a1961eaf1c Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Wed, 3 Feb 2021 14:02:30 -0500 Subject: [PATCH 3/4] fix: pre-remove hooks will now be called for subdocuments --- lib/document.js | 5 ----- lib/types/embedded.js | 9 +++++---- lib/types/subdocument.js | 1 - test/document.test.js | 32 +++++++++++--------------------- 4 files changed, 16 insertions(+), 31 deletions(-) diff --git a/lib/document.js b/lib/document.js index 7bf725fdeef..6c14f869db2 100644 --- a/lib/document.js +++ b/lib/document.js @@ -2256,7 +2256,6 @@ function _getPathsToValidate(doc) { const skipSchemaValidators = {}; _evaluateRequiredFunctions(doc); - // only validate required fields when necessary let paths = new Set(Object.keys(doc.$__.activePaths.states.require).filter(function(path) { if (!doc.$__isSelected(path) && !doc.isModified(path)) { @@ -2340,7 +2339,6 @@ function _getPathsToValidate(doc) { } } - for (const path of paths) { // Single nested paths (paths embedded under single nested subdocs) will // be validated on their own when we call `validate()` on the subdoc itself. @@ -2434,11 +2432,9 @@ Document.prototype.$__validate = function(pathsToValidate, options, callback) { pathDetails[0].filter((path) => this.isModified(path)) : pathDetails[0]; const skipSchemaValidators = pathDetails[1]; - if (Array.isArray(pathsToValidate)) { paths = _handlePathsToValidate(paths, pathsToValidate); } - if (paths.length === 0) { return process.nextTick(function() { const error = _complete(); @@ -2606,7 +2602,6 @@ Document.prototype.validateSync = function(pathsToValidate, options) { if (Array.isArray(pathsToValidate)) { paths = _handlePathsToValidate(paths, pathsToValidate); } - const validating = {}; paths.forEach(function(path) { diff --git a/lib/types/embedded.js b/lib/types/embedded.js index 531128d404b..5bcdc412f06 100644 --- a/lib/types/embedded.js +++ b/lib/types/embedded.js @@ -201,6 +201,9 @@ function registerRemoveListener(sub) { */ EmbeddedDocument.prototype.$__remove = function(cb) { + if (cb == null) { + return; + } return cb(null, this); }; @@ -218,7 +221,7 @@ EmbeddedDocument.prototype.remove = function(options, fn) { options = undefined; } if (!this.__parentArray || (options && options.noop)) { - fn && fn(null); + this.$__remove(fn); return this; } @@ -234,9 +237,7 @@ EmbeddedDocument.prototype.remove = function(options, fn) { registerRemoveListener(this); } - if (fn) { - fn(null); - } + this.$__remove(fn); return this; }; diff --git a/lib/types/subdocument.js b/lib/types/subdocument.js index bc37342467c..182fa229f13 100644 --- a/lib/types/subdocument.js +++ b/lib/types/subdocument.js @@ -255,7 +255,6 @@ Subdocument.prototype.remove = function(options, callback) { callback = options; options = null; } - registerRemoveListener(this); // If removing entire doc, no need to remove subdoc diff --git a/test/document.test.js b/test/document.test.js index 2b03e4d8744..f2e2cd5227a 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -9931,16 +9931,18 @@ describe('document', function() { assert.ok(doc); }); }); - it('gh-9885', function() { + it('Makes sure pre remove hook is executed gh-9885', function() { + const tracker = new assert.CallTracker(); const SubSchema = new Schema({ myValue: { type: String } }, {}); - SubSchema.pre('remove', co.wrap(function*(){ - console.log('Subdoc got removed!'); - })); - + let count = 0; + SubSchema.pre('remove', function(next) { + count++; + next(); + }); const thisSchema = new Schema({ foo: { type: String, @@ -9953,31 +9955,19 @@ describe('document', function() { }, {minimize: false, collection: 'test'}); const Model = db.model('TestModel',thisSchema); - const test = co(function*(){ + + return co(function*() { yield Model.deleteMany({}); // remove all existing documents const newModel = { foo: 'bar', mySubdoc: [{myValue: 'some value'}] }; const document = yield Model.create(newModel); - console.log('Created Document'); - console.log('document', document); - console.log('Removing subDocument'); document.mySubdoc[0].remove(); - console.log('Saving document'); - // had to remove yield for the rest to execute but now saying error. - document.save().catch((error) => { + yield document.save().catch((error) => { console.error(error); - process.exit(1); }); - console.log('document: ', document); - console.log(`Notice that SubSchema.pre('remove') never ran`); - }); - const main = co.wrap(function*(){ - return yield test; - }); - main(true).then(function(){ - process.exit(1); + assert.equal(count,1); }); }); }); From f79e9a71ee5f092aedf1a03d05d3c99cf423e6ef Mon Sep 17 00:00:00 2001 From: Daniel Diaz <39510674+IslandRhythms@users.noreply.github.com> Date: Wed, 3 Feb 2021 14:05:58 -0500 Subject: [PATCH 4/4] fix linter complaints --- test/document.test.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/document.test.js b/test/document.test.js index f2e2cd5227a..6a9c50c13c1 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -9932,7 +9932,6 @@ describe('document', function() { }); }); it('Makes sure pre remove hook is executed gh-9885', function() { - const tracker = new assert.CallTracker(); const SubSchema = new Schema({ myValue: { type: String @@ -9952,22 +9951,22 @@ describe('document', function() { type: [SubSchema], required: true } - }, {minimize: false, collection: 'test'}); + }, { minimize: false, collection: 'test' }); - const Model = db.model('TestModel',thisSchema); + const Model = db.model('TestModel', thisSchema); return co(function*() { yield Model.deleteMany({}); // remove all existing documents const newModel = { foo: 'bar', - mySubdoc: [{myValue: 'some value'}] + mySubdoc: [{ myValue: 'some value' }] }; const document = yield Model.create(newModel); document.mySubdoc[0].remove(); yield document.save().catch((error) => { console.error(error); }); - assert.equal(count,1); + assert.equal(count, 1); }); }); });