From 5da9f44837124534903a0b68e53c4a497936219a Mon Sep 17 00:00:00 2001 From: Hafez Date: Sat, 16 May 2020 14:18:11 +0200 Subject: [PATCH 1/6] 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 2/6] 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 3/6] 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 4/6] 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 89451927edbdd91c15dc325b7b1979d484b576c7 Mon Sep 17 00:00:00 2001 From: Hafez Date: Tue, 19 May 2020 19:52:37 +0200 Subject: [PATCH 5/6] test: repro #9032 --- test/index.test.js | 52 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/test/index.test.js b/test/index.test.js index a8025689c86..bb9e070c74a 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -778,5 +778,57 @@ describe('mongoose module:', function() { done(); }); }); + + it('can set `setDefaultsOnInsert` as a global option (gh-9032)', function() { + return co(function* () { + const m = new mongoose.Mongoose(); + m.set('setDefaultsOnInsert', true); + const db = yield m.connect('mongodb://localhost:27017/mongoose_test_9032'); + + const schema = new m.Schema({ + title: String, + genre: { type: String, default: 'Action' } + }, { collection: 'movies_1' }); + + const Movie = db.model('Movie', schema); + + + yield Movie.updateOne( + {}, + { title: 'Cloud Atlas' }, + { upsert: true } + ); + + // lean is necessary to avoid defaults by casting + const movie = yield Movie.findOne({ title: 'Cloud Atlas' }).lean(); + assert.equal(movie.genre, 'Action'); + }); + }); + + it('setting `setDefaultOnInsert` on operation has priority over base option (gh-9032)', function() { + return co(function* () { + const m = new mongoose.Mongoose(); + m.set('setDefaultsOnInsert', true); + const db = yield m.connect('mongodb://localhost:27017/mongoose_test_9032'); + + const schema = new m.Schema({ + title: String, + genre: { type: String, default: 'Action' } + }, { collection: 'movies_2' }); + + const Movie = db.model('Movie', schema); + + + yield Movie.updateOne( + {}, + { title: 'The Man From Earth' }, + { upsert: true, setDefaultsOnInsert: false } + ); + + // lean is necessary to avoid defaults by casting + const movie = yield Movie.findOne({ title: 'The Man From Earth' }).lean(); + assert.ok(!movie.genre); + }); + }); }); }); From 4833de1b3649d89cd38adcbb39143dd5102c9ecc Mon Sep 17 00:00:00 2001 From: Hafez Date: Tue, 19 May 2020 19:54:07 +0200 Subject: [PATCH 6/6] feat(mongoose): add support for setting `setDefaultsOnInsert` as a global option --- lib/helpers/setDefaultsOnInsert.js | 17 +++++++++++------ lib/validoptions.js | 1 + 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/helpers/setDefaultsOnInsert.js b/lib/helpers/setDefaultsOnInsert.js index 0545e80d3d4..747cb61ab8d 100644 --- a/lib/helpers/setDefaultsOnInsert.js +++ b/lib/helpers/setDefaultsOnInsert.js @@ -14,6 +14,17 @@ const modifiedPaths = require('./common').modifiedPaths; */ module.exports = function(filter, schema, castedDoc, options) { + options = options || {}; + + const shouldSetDefaultsOnInsert = + options.hasOwnProperty('setDefaultsOnInsert') ? + options.setDefaultsOnInsert : + schema.base.options.setDefaultsOnInsert; + + if (!options.upsert || !shouldSetDefaultsOnInsert) { + return castedDoc; + } + const keys = Object.keys(castedDoc || {}); const updatedKeys = {}; const updatedValues = {}; @@ -22,12 +33,6 @@ module.exports = function(filter, schema, castedDoc, options) { let hasDollarUpdate = false; - options = options || {}; - - if (!options.upsert || !options.setDefaultsOnInsert) { - return castedDoc; - } - for (let i = 0; i < numKeys; ++i) { if (keys[i].startsWith('$')) { modifiedPaths(castedDoc[keys[i]], '', modified); diff --git a/lib/validoptions.js b/lib/validoptions.js index 9464e4d8611..3ff4ada636c 100644 --- a/lib/validoptions.js +++ b/lib/validoptions.js @@ -16,6 +16,7 @@ const VALID_OPTIONS = Object.freeze([ 'objectIdGetter', 'runValidators', 'selectPopulatedPaths', + 'setDefaultsOnInsert', 'strict', 'toJSON', 'toObject',