Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(base): allow global option mongoose.set('strictQuery', true); #9016

Merged
merged 13 commits into from May 18, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/index.js
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/schema.js
Expand Up @@ -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',
Expand Down
1 change: 1 addition & 0 deletions lib/validoptions.js
Expand Up @@ -17,6 +17,7 @@ const VALID_OPTIONS = Object.freeze([
'runValidators',
'selectPopulatedPaths',
'strict',
'strictQuery',
'toJSON',
'toObject',
'useCreateIndex',
Expand Down
29 changes: 29 additions & 0 deletions test/schema.test.js
Expand Up @@ -2430,4 +2430,33 @@ describe('schema', function() {
assert.throws(buildInvalidSchema, /`db` may not be used as a schema pathname/);
});
});

describe('mongoose.set(`strictQuery`, value); (gh-6658)', function() {
let strictQueryOriginalValue;

this.beforeEach(() => strictQueryOriginalValue = mongoose.get('strictQuery'));
this.afterEach(() => mongoose.set('strictQuery', strictQueryOriginalValue));
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vkarpov15 I'd like to know what you think of this approach.

I initially made the cleanup in each test, but if one of those tests had failed, the cleanup wouldn't have happened, causing more tests to fail, making it difficult to find the root cause of all the failing tests.

I think this mitigates this concern, but I still don't like it. How would you do that?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach is ok, but the better approach would be to create a new Mongoose instance using const _mongoose = new mongoose.Mongoose(); and use _mongoose.set(). Check out the test for #6578 for an example.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this attempt to create a new connection with the same URI for the database? Making tests run significantly slower?


it('setting `strictQuery` on base sets strictQuery to schema (gh-6658)', 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 (gh-6658)', function() {
// Arrange
mongoose.set('strictQuery', 'base option');

// Act
const schema = new Schema({}, { strictQuery: 'schema option' });

// Assert
assert.equal(schema.get('strictQuery'), 'schema option');
});
});
});