From 1b190b63f6067e950a385b900e94d7ad0ec2b3d6 Mon Sep 17 00:00:00 2001 From: Jeremy Philippe Date: Sat, 16 May 2020 12:32:30 +0200 Subject: [PATCH 01/10] fix(documentarray): make sure you can call `unshift()` after `map()` Fix #9012 --- lib/types/core_array.js | 12 +++++++++--- test/types.documentarray.test.js | 21 +++++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/lib/types/core_array.js b/lib/types/core_array.js index b6632359daa..c4c6b33b441 100644 --- a/lib/types/core_array.js +++ b/lib/types/core_array.js @@ -879,7 +879,7 @@ class CoreMongooseArray extends Array { * * ####Note: * - * _marks the entire array as modified, which if saved, will store it as a `$set` operation, potentially overwritting any changes that happen between when you retrieved the object and when you save it._ + * _marks the entire array as modified, which if saved, will store it as a `$set` operation, potentially overwriting any changes that happen between when you retrieved the object and when you save it._ * * @api public * @method unshift @@ -889,8 +889,14 @@ class CoreMongooseArray extends Array { unshift() { _checkManualPopulation(this, arguments); - let values = [].map.call(arguments, this._cast, this); - values = this[arraySchemaSymbol].applySetters(values, this[arrayParentSymbol]); + let values; + if (this[arraySchemaSymbol] == null) { + values = arguments; + } else { + values = [].map.call(arguments, this._cast, this); + values = this[arraySchemaSymbol].applySetters(values, this[arrayParentSymbol]); + } + [].unshift.apply(this, values); this._registerAtomic('$set', this); this._markModified(); diff --git a/test/types.documentarray.test.js b/test/types.documentarray.test.js index 22d9db368a9..93bafeccaa8 100644 --- a/test/types.documentarray.test.js +++ b/test/types.documentarray.test.js @@ -611,6 +611,27 @@ describe('types.documentarray', function() { 'd' ]); }); + + it('unshift() after map() works (gh-9012)', function() { + const MyModel = db.model('Test', Schema({ + myArray: [{ name: String }] + })); + + const doc = new MyModel({ + myArray: [{ name: 'b' }, { name: 'c' }] + }); + let myArray = doc.myArray; + + myArray = myArray.map(val => ({ name: `${val.name} mapped` })); + + myArray.unshift({ name: 'a inserted' }); + + assert.deepEqual(myArray.map(v => v.name), [ + 'a inserted', + 'b mapped', + 'c mapped' + ]); + }); }); it('cleans modified subpaths on splice() (gh-7249)', function() { From 2fd29b0339710a1ec7f91b5fa5f83b9e28d105e1 Mon Sep 17 00:00:00 2001 From: Hafez Date: Sat, 16 May 2020 13:38:40 +0200 Subject: [PATCH 02/10] docs: add anchor tag to strictQuery --- docs/guide.pug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guide.pug b/docs/guide.pug index 442496d09cc..42a21890aed 100644 --- a/docs/guide.pug +++ b/docs/guide.pug @@ -739,7 +739,7 @@ block content thing.save(); // iAmNotInTheSchema is never saved to the db ``` -

option: strictQuery

+

option: strictQuery

For backwards compatibility, the `strict` option does **not** apply to the `filter` parameter for queries. From 5da9f44837124534903a0b68e53c4a497936219a Mon Sep 17 00:00:00 2001 From: Hafez Date: Sat, 16 May 2020 14:18:11 +0200 Subject: [PATCH 03/10] test: repro #6658 strictQuery --- test/schema.test.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/schema.test.js b/test/schema.test.js index 0a1e00f6b80..fa45ee07f90 100644 --- a/test/schema.test.js +++ b/test/schema.test.js @@ -2430,4 +2430,26 @@ describe('schema', function() { assert.throws(buildInvalidSchema, /`db` may not be used as a schema pathname/); }); }); + + it('setting `strictQuery` on base sets strictQuery to schema', function() { + // Arrange + mongoose.set('strictQuery', 'some value'); + + // Act + const schema = new Schema(); + + // Assert + assert.equal(schema.get('strictQuery'), 'some value'); + }); + + it('`strictQuery` set on base gets overwritten by option set on schema', function() { + // Arrange + mongoose.set('strictQuery', 'base option'); + + // Act + const schema = new Schema({}, { strictQuery: 'schema option' }); + + // Assert + assert.equal(schema.get('strictQuery'), 'schema option'); + }); }); From f7f9b4fbe2980f444a5d7c3a5e03c27924f9afb8 Mon Sep 17 00:00:00 2001 From: Hafez Date: Sat, 16 May 2020 14:21:42 +0200 Subject: [PATCH 04/10] fix(base): allow setting strictQuery globally --- lib/index.js | 1 + lib/schema.js | 1 + lib/validoptions.js | 1 + 3 files changed, 3 insertions(+) diff --git a/lib/index.js b/lib/index.js index 8c7c9f0aa90..30e51fd26f1 100644 --- a/lib/index.js +++ b/lib/index.js @@ -158,6 +158,7 @@ Mongoose.prototype.driver = require('./driver'); * - 'toObject': `{ transform: true, flattenDecimals: true }` by default. Overwrites default objects to [`toObject()`](/docs/api.html#document_Document-toObject) * - 'toJSON': `{ transform: true, flattenDecimals: true }` by default. Overwrites default objects to [`toJSON()`](/docs/api.html#document_Document-toJSON), for determining how Mongoose documents get serialized by `JSON.stringify()` * - 'strict': true by default, may be `false`, `true`, or `'throw'`. Sets the default strict mode for schemas. + * - 'strictQuery': false by default, may be `false`, `true`, or `'throw'`. Sets the default [strictQuery](/docs/guide.html#strictQuery) mode for schemas. * - 'selectPopulatedPaths': true by default. Set to false to opt out of Mongoose adding all fields that you `populate()` to your `select()`. The schema-level option `selectPopulatedPaths` overwrites this one. * - 'typePojoToMixed': true by default, may be `false` or `true`. Sets the default typePojoToMixed for schemas. * - 'maxTimeMS': If set, attaches [maxTimeMS](https://docs.mongodb.com/manual/reference/operator/meta/maxTimeMS/) to every query diff --git a/lib/schema.js b/lib/schema.js index 777cfce4434..996fd04f023 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -400,6 +400,7 @@ Schema.prototype.defaultOptions = function(options) { const baseOptions = get(this, 'base.options', {}); options = utils.options({ strict: 'strict' in baseOptions ? baseOptions.strict : true, + strictQuery: 'strictQuery' in baseOptions ? baseOptions.strictQuery : false, bufferCommands: true, capped: false, // { size, max, autoIndexId } versionKey: '__v', diff --git a/lib/validoptions.js b/lib/validoptions.js index 9464e4d8611..68a7b424dfd 100644 --- a/lib/validoptions.js +++ b/lib/validoptions.js @@ -17,6 +17,7 @@ const VALID_OPTIONS = Object.freeze([ 'runValidators', 'selectPopulatedPaths', 'strict', + 'strictQuery', 'toJSON', 'toObject', 'useCreateIndex', From 80d40e557645f9cca1ec54fd09283efde52acb6d Mon Sep 17 00:00:00 2001 From: Hafez Date: Sat, 16 May 2020 14:42:16 +0200 Subject: [PATCH 05/10] fix test causing other tests to fail --- test/schema.test.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/schema.test.js b/test/schema.test.js index fa45ee07f90..43b3e14d431 100644 --- a/test/schema.test.js +++ b/test/schema.test.js @@ -2431,8 +2431,9 @@ describe('schema', function() { }); }); - it('setting `strictQuery` on base sets strictQuery to schema', function() { + it('setting `strictQuery` on base sets strictQuery to schema (gh-6658)', function() { // Arrange + const originalValue = mongoose.get('strictQuery'); mongoose.set('strictQuery', 'some value'); // Act @@ -2440,10 +2441,14 @@ describe('schema', function() { // Assert assert.equal(schema.get('strictQuery'), 'some value'); + + // Cleanup + mongoose.set('strictQuery', originalValue); }); - it('`strictQuery` set on base gets overwritten by option set on schema', function() { + it('`strictQuery` set on base gets overwritten by option set on schema (gh-6658)', function() { // Arrange + const originalValue = mongoose.get('strictQuery'); mongoose.set('strictQuery', 'base option'); // Act @@ -2451,5 +2456,8 @@ describe('schema', function() { // Assert assert.equal(schema.get('strictQuery'), 'schema option'); + + // Cleanup + mongoose.set('strictQuery', originalValue); }); }); From 93b3ed6dbc2c8ec2a113b9db9bc19dd9d105422b Mon Sep 17 00:00:00 2001 From: Hafez Date: Sat, 16 May 2020 14:50:16 +0200 Subject: [PATCH 06/10] test: refactor tests re #6658, use beforeEach to guard against failed tests not cleaning up after failing which will in turn cause more tests to fail, making it difficult to know the root cause of all the failing tests --- test/schema.test.js | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/test/schema.test.js b/test/schema.test.js index 43b3e14d431..88dca91175d 100644 --- a/test/schema.test.js +++ b/test/schema.test.js @@ -2431,33 +2431,32 @@ describe('schema', function() { }); }); - it('setting `strictQuery` on base sets strictQuery to schema (gh-6658)', function() { - // Arrange - const originalValue = mongoose.get('strictQuery'); - mongoose.set('strictQuery', 'some value'); + describe('mongoose.set(`strictQuery`, value); (gh-6658)', function() { + let strictQueryOriginalValue; - // Act - const schema = new Schema(); + this.beforeEach(() => strictQueryOriginalValue = mongoose.get('strictQuery')); + this.afterEach(() => mongoose.set('strictQuery', strictQueryOriginalValue)); - // Assert - assert.equal(schema.get('strictQuery'), 'some value'); + it('setting `strictQuery` on base sets strictQuery to schema (gh-6658)', function() { + // Arrange + mongoose.set('strictQuery', 'some value'); - // Cleanup - mongoose.set('strictQuery', originalValue); - }); + // Act + const schema = new Schema(); - it('`strictQuery` set on base gets overwritten by option set on schema (gh-6658)', function() { - // Arrange - const originalValue = mongoose.get('strictQuery'); - mongoose.set('strictQuery', 'base option'); + // Assert + assert.equal(schema.get('strictQuery'), 'some value'); + }); - // Act - const schema = new Schema({}, { strictQuery: 'schema option' }); + it('`strictQuery` set on base gets overwritten by option set on schema (gh-6658)', function() { + // Arrange + mongoose.set('strictQuery', 'base option'); - // Assert - assert.equal(schema.get('strictQuery'), 'schema option'); + // Act + const schema = new Schema({}, { strictQuery: 'schema option' }); - // Cleanup - mongoose.set('strictQuery', originalValue); + // Assert + assert.equal(schema.get('strictQuery'), 'schema option'); + }); }); }); From 3c46473c6d8a0fdc5265bba6750d03353479ccf1 Mon Sep 17 00:00:00 2001 From: Hafez Date: Sat, 16 May 2020 15:17:16 +0200 Subject: [PATCH 07/10] docs(guide): add anchor tag to strict option --- docs/guide.pug | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/guide.pug b/docs/guide.pug index 42a21890aed..e71cfefe32d 100644 --- a/docs/guide.pug +++ b/docs/guide.pug @@ -688,7 +688,7 @@ block content _Note that Mongoose does not send the `shardcollection` command for you. You must configure your shards yourself._ -

option: strict

+

option: strict

The strict option, (enabled by default), ensures that values passed to our model constructor that were not specified in our schema do not get saved to From e736a738a2047f7a0ce85dfd098aa33642648eaa Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 17 May 2020 17:19:41 -0400 Subject: [PATCH 08/10] test(model): repro #8938 --- test/model.test.js | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/model.test.js b/test/model.test.js index 52ad90d9252..56081f17db3 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -4528,6 +4528,37 @@ describe('Model', function() { } }); + it('insertMany() `writeErrors` if only one error (gh-8938)', function() { + const QuestionType = new mongoose.Schema({ + code: { type: String, required: true, unique: true }, + text: String + }); + const Question = db.model('Test', QuestionType); + + return co(function*() { + yield Question.init(); + + yield Question.create({ code: 'MEDIUM', text: '123' }); + const data = [ + { code: 'MEDIUM', text: '1111' }, + { code: 'test', text: '222' }, + { code: 'HARD', text: '2222' } + ]; + const opts = { ordered: false, rawResult: true }; + let err = yield Question.insertMany(data, opts).catch(err => err); + assert.ok(Array.isArray(err.writeErrors)); + assert.equal(err.writeErrors.length, 1); + + yield Question.deleteMany({}); + yield Question.create({ code: 'MEDIUM', text: '123' }); + yield Question.create({ code: 'HARD', text: '123' }); + + err = yield Question.insertMany(data, opts).catch(err => err); + assert.ok(Array.isArray(err.writeErrors)); + assert.equal(err.writeErrors.length, 2); + }); + }); + it('insertMany() ordered option for single validation error', function(done) { start.mongodVersion(function(err, version) { if (err) { From faaff4494a67874ec7be8e1f3d9b7d172318647d Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 17 May 2020 17:19:57 -0400 Subject: [PATCH 09/10] fix(model): ensure consistent `writeErrors` property on insertMany error with `ordered: false`, even if only one op failed Re: #8938 --- lib/model.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/model.js b/lib/model.js index d7a6feda7ab..901228e2120 100644 --- a/lib/model.js +++ b/lib/model.js @@ -3381,6 +3381,10 @@ Model.$__insertMany = function(arr, options, callback) { _this.collection.insertMany(docObjects, options, function(error, res) { if (error) { + if (error.writeErrors == null && + get(error, 'result.result.writeErrors') != null) { + error.writeErrors = error.result.result.writeErrors; + } callback(error, null); return; } From b5c52117841edfe956656a4ed62d1d7b39d771ef Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 17 May 2020 17:33:14 -0400 Subject: [PATCH 10/10] fix(model): report `insertedDocs` on `insertMany()` errors Fix #8938 --- lib/model.js | 9 +++++++++ test/model.test.js | 5 +++++ 2 files changed, 14 insertions(+) diff --git a/lib/model.js b/lib/model.js index 901228e2120..73d38045073 100644 --- a/lib/model.js +++ b/lib/model.js @@ -3381,10 +3381,19 @@ Model.$__insertMany = function(arr, options, callback) { _this.collection.insertMany(docObjects, options, function(error, res) { if (error) { + // `writeErrors` is a property reported by the MongoDB driver, + // just not if there's only 1 error. if (error.writeErrors == null && get(error, 'result.result.writeErrors') != null) { error.writeErrors = error.result.result.writeErrors; } + + // `insertedDocs` is a Mongoose-specific property + const erroredIndexes = new Set(error.writeErrors.map(err => err.index)); + error.insertedDocs = docAttributes.filter((doc, i) => { + return !erroredIndexes.has(i); + }); + callback(error, null); return; } diff --git a/test/model.test.js b/test/model.test.js index 56081f17db3..3c59e5afb73 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -4548,6 +4548,9 @@ describe('Model', function() { let err = yield Question.insertMany(data, opts).catch(err => err); assert.ok(Array.isArray(err.writeErrors)); assert.equal(err.writeErrors.length, 1); + assert.equal(err.insertedDocs.length, 2); + assert.equal(err.insertedDocs[0].code, 'test'); + assert.equal(err.insertedDocs[1].code, 'HARD'); yield Question.deleteMany({}); yield Question.create({ code: 'MEDIUM', text: '123' }); @@ -4556,6 +4559,8 @@ describe('Model', function() { err = yield Question.insertMany(data, opts).catch(err => err); assert.ok(Array.isArray(err.writeErrors)); assert.equal(err.writeErrors.length, 2); + assert.equal(err.insertedDocs.length, 1); + assert.equal(err.insertedDocs[0].code, 'test'); }); });