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

7.5 #13796

Merged
merged 31 commits into from Aug 29, 2023
Merged

7.5 #13796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
80d0490
add deprecation warning
IslandRhythms Jul 24, 2023
eb73b2a
change message
IslandRhythms Jul 24, 2023
e76a902
moved it
IslandRhythms Jul 24, 2023
bd15a91
Update query.js
vkarpov15 Jul 25, 2023
370c32d
Merge pull request #13651 from Automattic/IslandRhythms/deprecate-ove…
vkarpov15 Jul 25, 2023
7a2adb9
enhancement: can index using the alias
IslandRhythms Jul 26, 2023
7b39ac6
Merge pull request #13659 from Automattic/IslandRhythms/gh-13276
vkarpov15 Jul 26, 2023
114acac
feature: `pathsToSkip` option for Model.validate
IslandRhythms Jul 27, 2023
e55385a
made changes
IslandRhythms Jul 27, 2023
a93b9b5
fix:lint
IslandRhythms Jul 27, 2023
6bf5490
fix(query): remove unnecessary check for atomic operators in findOneA…
vkarpov15 Jul 31, 2023
95ee76c
Merge branch 'master' into 7.5
vkarpov15 Aug 1, 2023
7ddb381
Merge pull request #13678 from Automattic/vkarpov15/remove-unnecessar…
vkarpov15 Aug 1, 2023
dd01db2
Merge pull request #13663 from Automattic/IslandRhythms/gh-10353
vkarpov15 Aug 1, 2023
743c556
test(document): repro #13748
vkarpov15 Aug 23, 2023
bebdc09
fix(document): make array getters avoid unintentionally modifying arr…
vkarpov15 Aug 23, 2023
0d4891b
Merge branch 'master' into 7.5
vkarpov15 Aug 25, 2023
2aa453c
feat: use mongodb driver 5.8.1
vkarpov15 Aug 25, 2023
6eeb875
Merge branch 'master' into vkarpov15/gh-13748
vkarpov15 Aug 25, 2023
069651e
Merge branch 'master' into 7.5
vkarpov15 Aug 25, 2023
65245a4
style: fix lint
vkarpov15 Aug 25, 2023
fafa5d5
Merge branch 'master' into 7.5
vkarpov15 Aug 26, 2023
40146c8
feat: allow top level dollar keys with findOneAndUpdate(), update()
vkarpov15 Aug 26, 2023
fe7f80b
test: add tests
vkarpov15 Aug 26, 2023
0d956ce
test: fix tests
vkarpov15 Aug 28, 2023
c1d5b76
Merge pull request #13774 from Automattic/vkarpov15/gh-13748
vkarpov15 Aug 28, 2023
41e63bd
fix(document): avoid double-calling array getters when using `.get('a…
vkarpov15 Aug 28, 2023
29de9c4
Merge pull request #13786 from Automattic/vkarpov15/handle-top-level-…
vkarpov15 Aug 29, 2023
d9a4bc4
Merge pull request #13793 from Automattic/vkarpov15/gh-13774
vkarpov15 Aug 29, 2023
a92e8e8
Merge branch 'master' into 7.5
vkarpov15 Aug 29, 2023
419c64d
docs: add comment about top-level dollar keys re: #13796 code review …
vkarpov15 Aug 29, 2023
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
8 changes: 8 additions & 0 deletions lib/document.js
Expand Up @@ -4091,6 +4091,14 @@ function applyGetters(self, json, options) {
if (ii === last) {
const val = self.$get(path);
branch[part] = clone(val, options);
if (Array.isArray(branch[part]) && schema.paths[path].$embeddedSchemaType) {
for (let i = 0; i < branch[part].length; ++i) {
branch[part][i] = schema.paths[path].$embeddedSchemaType.applyGetters(
branch[part][i],
self
);
}
}
} else if (v == null) {
if (part in cur) {
branch[part] = v;
Expand Down
24 changes: 21 additions & 3 deletions lib/helpers/query/castUpdate.js
Expand Up @@ -14,6 +14,24 @@ const schemaMixedSymbol = require('../../schema/symbols').schemaMixedSymbol;
const setDottedPath = require('../path/setDottedPath');
const utils = require('../../utils');

const mongodbUpdateOperators = new Set([
'$currentDate',
'$inc',
'$min',
'$max',
'$mul',
'$rename',
'$set',
'$setOnInsert',
'$unset',
'$addToSet',
'$pop',
'$pull',
'$push',
'$pullAll',
'$bit'
]);

/**
* Casts an update op based on the given schema
*
Expand Down Expand Up @@ -58,7 +76,7 @@ module.exports = function castUpdate(schema, obj, options, context, filter) {
while (i--) {
const op = ops[i];
// if overwrite is set, don't do any of the special $set stuff
if (op[0] !== '$' && !overwrite) {
if (!mongodbUpdateOperators.has(op) && !overwrite) {
// fix up $set sugar
if (!ret.$set) {
if (obj.$set) {
Expand Down Expand Up @@ -88,7 +106,7 @@ module.exports = function castUpdate(schema, obj, options, context, filter) {
if (val &&
typeof val === 'object' &&
!Buffer.isBuffer(val) &&
(!overwrite || hasDollarKey)) {
(!overwrite || mongodbUpdateOperators.has(op))) {
walkUpdatePath(schema, val, op, options, context, filter);
} else if (overwrite && ret && typeof ret === 'object') {
walkUpdatePath(schema, ret, '$set', options, context, filter);
Expand Down Expand Up @@ -540,7 +558,7 @@ function castUpdateVal(schema, val, op, $conditional, context, path) {
return Boolean(val);
}

if (/^\$/.test($conditional)) {
if (mongodbUpdateOperators.has($conditional)) {
return schema.castForQuery(
$conditional,
val,
Expand Down
19 changes: 15 additions & 4 deletions lib/model.js
Expand Up @@ -4083,13 +4083,13 @@ Model.aggregate = function aggregate(pipeline, options) {
* }
*
* @param {Object} obj
* @param {Array|String} pathsToValidate
* @param {Object|Array|String} pathsOrOptions
* @param {Object} [context]
* @return {Promise|undefined}
* @api public
*/

Model.validate = async function validate(obj, pathsToValidate, context) {
Model.validate = async function validate(obj, pathsOrOptions, context) {
if ((arguments.length < 3) || (arguments.length === 3 && typeof arguments[2] === 'function')) {
// For convenience, if we're validating a document or an object, make `context` default to
// the model so users don't have to always pass `context`, re: gh-10132, gh-10346
Expand All @@ -4106,9 +4106,20 @@ Model.validate = async function validate(obj, pathsToValidate, context) {
}
let paths = Object.keys(schema.paths);

if (pathsToValidate != null) {
const _pathsToValidate = typeof pathsToValidate === 'string' ? new Set(pathsToValidate.split(' ')) : new Set(pathsToValidate);
if (pathsOrOptions != null) {
const _pathsToValidate = typeof pathsOrOptions === 'string' ? new Set(pathsOrOptions.split(' ')) : Array.isArray(pathsOrOptions) ? new Set(pathsOrOptions) : new Set(paths);
paths = paths.filter(p => {
if (pathsOrOptions.pathsToSkip) {
if (Array.isArray(pathsOrOptions.pathsToSkip)) {
if (pathsOrOptions.pathsToSkip.find(x => x == p)) {
return false;
}
} else if (typeof pathsOrOptions.pathsToSkip == 'string') {
if (pathsOrOptions.pathsToSkip.includes(p)) {
return false;
}
}
}
const pieces = p.split('.');
let cur = pieces[0];

Expand Down
16 changes: 13 additions & 3 deletions lib/query.js
Expand Up @@ -1683,6 +1683,15 @@ const printRawResultDeprecationWarning = util.deprecate(
'The `rawResult` option for Mongoose queries is deprecated. Use `includeResultMetadata: false` as a replacement for `rawResult: true`.'
);

/*!
* ignore
*/

const printOverwriteDeprecationWarning = util.deprecate(
function printOverwriteDeprecationWarning() {},
'The `overwrite` option for `findOneAndUpdate()` is deprecated. use `findOneAndReplace()` instead.'
);

/**
* Sets the [`explain` option](https://www.mongodb.com/docs/manual/reference/method/cursor.explain/),
* which makes this query return detailed execution stats instead of the actual
Expand Down Expand Up @@ -3283,6 +3292,10 @@ Query.prototype.findOneAndUpdate = function(filter, doc, options) {

Query.prototype._findOneAndUpdate = async function _findOneAndUpdate() {
// For backwards compability with Mongoose 6 re: #13550

if (this._mongooseOptions.overwrite != null) {
printOverwriteDeprecationWarning();
}
if (this._mongooseOptions.overwrite) {
this.op = 'findOneAndReplace';
return this._findOneAndReplace();
Expand Down Expand Up @@ -3587,9 +3600,6 @@ Query.prototype.findOneAndReplace = function(filter, replacement, options) {
}

if (replacement != null) {
if (hasDollarKeys(replacement)) {
throw new Error('The replacement document must not contain atomic operators.');
}
this._mergeUpdate(replacement);
}

Expand Down
6 changes: 6 additions & 0 deletions lib/schema.js
Expand Up @@ -2045,6 +2045,12 @@ Schema.prototype.index = function(fields, options) {
if (options.expires) {
utils.expires(options);
}
for (const key in fields) {
if (this.aliases[key]) {
fields[this.aliases[key]] = fields[key];
delete fields[key];
}
}

for (const field of Object.keys(fields)) {
if (fields[field] === 'ascending' || fields[field] === 'asc') {
Expand Down
7 changes: 0 additions & 7 deletions lib/schema/array.js
Expand Up @@ -289,13 +289,6 @@ SchemaArray.prototype.applyGetters = function(value, scope) {
}

const ret = SchemaType.prototype.applyGetters.call(this, value, scope);
if (Array.isArray(ret)) {
const rawValue = utils.isMongooseArray(ret) ? ret.__array : ret;
const len = rawValue.length;
for (let i = 0; i < len; ++i) {
rawValue[i] = this.caster.applyGetters(rawValue[i], scope);
}
}
return ret;
};

Expand Down
3 changes: 3 additions & 0 deletions lib/types/array/index.js
Expand Up @@ -90,6 +90,9 @@ function MongooseArray(values, path, doc, schematype) {
if (mongooseArrayMethods.hasOwnProperty(prop)) {
return mongooseArrayMethods[prop];
}
if (typeof prop === 'string' && numberRE.test(prop) && schematype?.$embeddedSchemaType != null) {
return schematype.$embeddedSchemaType.applyGetters(__array[prop], doc);
}

return __array[prop];
},
Expand Down
5 changes: 4 additions & 1 deletion lib/utils.js
Expand Up @@ -640,7 +640,10 @@ exports.getValue = function(path, obj, map) {
const mapGetterOptions = Object.freeze({ getters: false });

function getValueLookup(obj, part) {
const _from = obj?._doc || obj;
let _from = obj?._doc || obj;
if (_from != null && _from.isMongooseArrayProxy) {
_from = _from.__array;
}
return _from instanceof Map ?
_from.get(part, mapGetterOptions) :
_from[part];
Expand Down
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -21,7 +21,7 @@
"dependencies": {
"bson": "^5.4.0",
"kareem": "2.5.1",
"mongodb": "5.7.0",
"mongodb": "5.8.1",
"mpath": "0.9.0",
"mquery": "5.0.0",
"ms": "2.1.3",
Expand Down
51 changes: 50 additions & 1 deletion test/document.test.js
Expand Up @@ -5564,6 +5564,7 @@ describe('document', function() {
const Test = db.model('Test', testSchema);

const doc = new Test({ arr: [new mongoose.Types.ObjectId()] });
assert.equal(called, 0);
assert.deepEqual(doc.toObject({ getters: true }).arr, [42]);
assert.equal(called, 1);
});
Expand All @@ -5582,7 +5583,6 @@ describe('document', function() {

const Test = db.model('Test', testSchema);


let doc = await Test.create({ arr: [new mongoose.Types.ObjectId()] });
assert.equal(called, 1);

Expand Down Expand Up @@ -12343,6 +12343,55 @@ describe('document', function() {
assert.strictEqual(Object.polluted, undefined);
});

it('does not modify array when calling getters (gh-13748)', async function() {
// create simple setter to add a sufix
const addSufix = (name) => {
return name + '-sufix';
};

// create simple gettrer to remove last 6 letters (should be "-sufix")
const removeSufix = (name) => {
return ('' + name).slice(0, -6);
};

const userSchema = new mongoose.Schema(
{
name: String,
age: Number,
profession: {
type: String,
get: removeSufix,
set: addSufix
},
hobbies: [{ type: String, get: removeSufix, set: addSufix }]
},
{
toObject: { getters: true },
toJSON: { getters: true }
}
);
const User = db.model('User', userSchema);

const usr = await User.create({
name: 'John',
age: 18,
profession: 'teacher',
hobbies: ['swimming', 'football']
});

const oneUser = await User.findById(usr._id).orFail();
assert.equal(oneUser.profession, 'teacher');
assert.equal(oneUser.profession, 'teacher');
assert.equal(oneUser.profession, 'teacher');
assert.equal(oneUser.hobbies[0], 'swimming');
assert.equal(oneUser.hobbies[0], 'swimming');
assert.equal(oneUser.hobbies[0], 'swimming');

assert.equal(oneUser.get('hobbies.0'), 'swimming');
assert.equal(oneUser.get('hobbies.0'), 'swimming');
assert.equal(oneUser.get('hobbies.0'), 'swimming');
});

it('sets defaults on subdocs with subdoc projection (gh-13720)', async function() {
const subSchema = new mongoose.Schema({
propertyA: { type: String, default: 'A' },
Expand Down
18 changes: 18 additions & 0 deletions test/model.updateOne.test.js
Expand Up @@ -3051,6 +3051,24 @@ describe('model: updateOne: ', function() {
await Person.updateOne({ name: 'Anakin' }, { name: 'The Chosen One' }).orFail();
}, { message: 'No document found for query "{ name: \'Anakin\' }" on model "gh-11620"' });
});
it('updateOne with top level key that starts with $ (gh-13786)', async function() {
const [major] = await start.mongodVersion();
// Top-level dollar key support was added in MongoDB 5
if (major < 5) {
return this.skip();
vkarpov15 marked this conversation as resolved.
Show resolved Hide resolved
}

const schema = new mongoose.Schema({
$myKey: String
});

const Test = db.model('Test', schema);

const _id = new mongoose.Types.ObjectId();
await Test.updateOne({ _id }, { $myKey: 'gh13786' }, { upsert: true });
const doc = await Test.findById(_id);
assert.equal(doc.$myKey, 'gh13786');
});
});

async function delay(ms) {
Expand Down
39 changes: 39 additions & 0 deletions test/model.validate.test.js
Expand Up @@ -144,4 +144,43 @@ describe('model: validate: ', function() {
const err2 = await User.validate({ name: 'Sam' }).then(() => null, err => err);
assert.ok(err2 === null);
});
it('Model.validate(...) supports passing in an object, array or string (gh-10353)', async function() {
const testSchema = new Schema({
docs: {
type: String,
validate: () => false
},
name: {
type: String,
validate: () => false
},
obj: {
type: String,
validate: () => false
},
arr: {
type: String,
validate: () => false
}
});

const Test = mongoose.model('Test', testSchema);

let test = { obj: 1, docs: 'a doc' };
let pathsOrOptions = { pathsToSkip: ['obj'] };
await assert.rejects(async() => {
await Test.validate(test, pathsOrOptions);
}, { message: 'Validation failed: docs: Validator failed for path `docs` with value `a doc`' });
test = { arr: 1, docs: 'a doc' };
pathsOrOptions = ['arr'];
await assert.rejects(async() => {
await Test.validate(test, pathsOrOptions);
}, { message: 'Validation failed: arr: Validator failed for path `arr` with value `1`' });
test = { name: 1, docs: 'a doc' };
pathsOrOptions = 'name';
await assert.rejects(async() => {
await Test.validate(test, pathsOrOptions);
}, { message: 'Validation failed: name: Validator failed for path `name` with value `1`' });

});
});
12 changes: 12 additions & 0 deletions test/schema.alias.test.js
Expand Up @@ -183,6 +183,18 @@ describe('schema alias option', function() {
assert.ok(schema.virtuals['name1']);
assert.ok(schema.virtuals['name2']);
});
it('supports passing the alias name for an index (gh-13276)', function() {
const testSchema = new Schema({
fullName: {
type: String,
alias: 'short'
}
});
testSchema.index({ short: 1 });
const indexes = testSchema.indexes();
assert.equal(indexes[0][0].fullName, 1);
});

it('should disable the id virtual entirely if there\'s a field with alias `id` gh-13650', async function() {

const testSchema = new Schema({
Expand Down