Skip to content

Commit

Permalink
Merge branch 'master' into chore-extract-and-refactor-document-types
Browse files Browse the repository at this point in the history
  • Loading branch information
Uzlopak committed Mar 10, 2022
2 parents 6eb20e6 + f148e62 commit 6597230
Show file tree
Hide file tree
Showing 20 changed files with 526 additions and 285 deletions.
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,21 @@
6.2.5 / 2022-03-09
==================
* fix(mongoose): add isObjectIdOrHexString() to better capture the most common use case for `isValidObjectId()` #11419
* fix(query): prevent modifying discriminator key in updates using operators other than `$set` #11456
* fix(populate+types): call foreignField functions with doc as 1st param, better typings for `localField` and `foreignField` functions #11321
* fix(populate): return an array when using populate count on an array localField #11307
* fix(query): avoid error when using $not with arrays #11467
* perf: only deep clone validators if necessary #11412 [Uzlopak](https://github.com/Uzlopak)
* fix(types): rename definition files to lowercase to avoid typescript bug #11469
* fix(types): aggregate.sort() accepts a string but also `{ field: 'asc'|'ascending'|'desc'|'descending' }` #11479 [simonbrunel](https://github.com/simonbrunel)
* fix(types): extract and refactor aggregationcursor and querycursor #11488 [Uzlopak](https://github.com/Uzlopak)
* fix(types): extract and refactor schemaoptions #11484 [Uzlopak](https://github.com/Uzlopak)
* fix(types): make first param to `Query.prototype.populate()` a string #11475 [minhthinhls](https://github.com/minhthinhls)
* fix(types): improve type checking for doc arrays in schema definitions #11241
* docs: fix length comparaison in lean.test.js #11493 [zazapeta](https://github.com/zazapeta)
* docs(timestamps): fix typo #11481 [saibbyweb](https://github.com/saibbyweb)
* docs: fix broken link to rawResult #11459 [chhiring90](https://github.com/chhiring90)

6.2.4 / 2022-02-28
==================
* fix(query): correctly return full deleteOne(), deleteMany() result #11211
Expand Down
26 changes: 26 additions & 0 deletions docs/migrating_to_6.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ If you're still on Mongoose 4.x, please read the [Mongoose 4.x to 5.x migration
* [`Model.exists()` Returns a lean document instead of Boolean](#model-exists-returns-a-lean-document-instead-of-boolean)
* [`strictQuery` is now equal to `strict` by default](#strictquery-is-removed-and-replaced-by-strict)
* [MongoError is now MongoServerError](#mongoerror-is-now-mongoservererror)
* [Simplified `isValidObjectId()` and separate `isObjectIdOrHexString()`](#simplified-isvalidobjectid-and-separate-isobjectidorhexstring)
* [Clone Discriminator Schemas By Default](#clone-discriminator-schemas-by-default)
* [Schema Defined Document Key Order](#schema-defined-document-key-order)
* [`sanitizeFilter` and `trusted()`](#sanitizefilter-and-trusted)
Expand Down Expand Up @@ -171,6 +172,31 @@ User.discriminator('author', authorSchema.clone());
User.discriminator('author', authorSchema, { clone: false });
```

<h3 id="simplified-isvalidobjectid-and-separate-isobjectidorhexstring"><a href="#simplified-isvalidobjectid-and-separate-isobjectidorhexstring">Simplified <code>isValidObjectId()</code> and separate <code>isObjectIdOrHexString()</code></a></h3>

In Mongoose 5, `mongoose.isValidObjectId()` returned `false` for values like numbers, which was inconsistent with the MongoDB driver's `ObjectId.isValid()` function.
Technically, any JavaScript number can be converted to a MongoDB ObjectId.

In Mongoose 6, `mongoose.isValidObjectId()` is just a wrapper for `mongoose.Types.ObjectId.isValid()` for consistency.

Mongoose 6.2.5 now includes a `mongoose.isObjectIdOrHexString()` function, which does a better job of capturing the more common use case for `isValidObjectId()`: is the given value an `ObjectId` instance or a 24 character hex string representing an `ObjectId`?

```javascript
// `isValidObjectId()` returns `true` for some surprising values, because these
// values are _technically_ ObjectId representations
mongoose.isValidObjectId(new mongoose.Types.ObjectId()); // true
mongoose.isValidObjectId('0123456789ab'); // true
mongoose.isValidObjectId(6); // true
mongoose.isValidObjectId(new User({ name: 'test' })); // true

// `isObjectIdOrHexString()` instead only returns `true` for ObjectIds and 24
// character hex strings.
mongoose.isObjectIdOrHexString(new mongoose.Types.ObjectId()); // true
mongoose.isObjectIdOrHexString('62261a65d66c6be0a63c051f'); // true
mongoose.isValidObjectId('0123456789ab'); // false
mongoose.isValidObjectId(6); // false
```

<h3 id="schema-defined-document-key-order"><a href="#schema-defined-document-key-order">Schema Defined Document Key Order</a></h3>

Mongoose now saves objects with keys in the order the keys are specified in the schema, not in the user-defined object. So whether `Object.keys(new User({ name: String, email: String }).toObject()` is `['name', 'email']` or `['email', 'name']` depends on the order `name` and `email` are defined in your schema.
Expand Down
22 changes: 22 additions & 0 deletions lib/helpers/isSimpleValidator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
'use strict';

/*!
* Determines if `arg` is a flat object.
*
* @param {Object|Array|String|Function|RegExp|any} arg
* @api private
* @return {Boolean}
*/

module.exports = function isSimpleValidator(obj) {
const keys = Object.keys(obj);
let result = true;
for (let i = 0, len = keys.length; i < len; ++i) {
if (typeof obj[keys[i]] === 'object' && obj[keys[i]] !== null) {
result = false;
break;
}
}

return result;
};
2 changes: 1 addition & 1 deletion lib/helpers/populate/getModelsMapForPopulate.js
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ function _virtualPopulate(model, docs, options, _virtualRes) {
localField = localField.call(doc, doc);
}
if (typeof foreignField === 'function') {
foreignField = foreignField.call(doc);
foreignField = foreignField.call(doc, doc);
}

data.isRefPath = false;
Expand Down
27 changes: 21 additions & 6 deletions lib/helpers/query/castUpdate.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,8 @@ function walkUpdatePath(schema, obj, op, options, context, filter, pref) {

let aggregatedError = null;

const strictMode = strict != null ? strict : schema.options.strict;

while (i--) {
key = keys[i];
val = obj[key];
Expand All @@ -192,6 +194,17 @@ function walkUpdatePath(schema, obj, op, options, context, filter, pref) {
}
}

if (schema.discriminatorMapping != null && key === schema.options.discriminatorKey && !options.overwriteDiscriminatorKey) {
if (strictMode === 'throw') {
const err = new Error('Can\'t modify discriminator key "' + key + '" on discriminator model');
aggregatedError = _appendError(err, context, key, aggregatedError);
continue;
} else if (strictMode) {
delete obj[key];
continue;
}
}

if (getConstructorName(val) === 'Object') {
// watch for embedded doc schemas
schematype = schema._getSchema(prefix + key);
Expand All @@ -217,7 +230,7 @@ function walkUpdatePath(schema, obj, op, options, context, filter, pref) {
$each: castUpdateVal(schematype, val.$each, op, key, context, prefix + key)
};
} catch (error) {
aggregatedError = _handleCastError(error, context, key, aggregatedError);
aggregatedError = _appendError(error, context, key, aggregatedError);
}

if (val.$slice != null) {
Expand All @@ -237,13 +250,13 @@ function walkUpdatePath(schema, obj, op, options, context, filter, pref) {
try {
obj[key] = schematype.castForQuery(val, context, { strict: _strict });
} catch (error) {
aggregatedError = _handleCastError(error, context, key, aggregatedError);
aggregatedError = _appendError(error, context, key, aggregatedError);
}
} else {
try {
obj[key] = castUpdateVal(schematype, val, op, key, context, prefix + key);
} catch (error) {
aggregatedError = _handleCastError(error, context, key, aggregatedError);
aggregatedError = _appendError(error, context, key, aggregatedError);
}
}

Expand All @@ -259,7 +272,7 @@ function walkUpdatePath(schema, obj, op, options, context, filter, pref) {
try {
obj[key] = castUpdateVal(schematype, val, op, key, context, prefix + key);
} catch (error) {
aggregatedError = _handleCastError(error, context, key, aggregatedError);
aggregatedError = _appendError(error, context, key, aggregatedError);
}

if (obj[key] === void 0) {
Expand Down Expand Up @@ -350,7 +363,7 @@ function walkUpdatePath(schema, obj, op, options, context, filter, pref) {
delete obj[key];
}
} catch (error) {
aggregatedError = _handleCastError(error, context, key, aggregatedError);
aggregatedError = _appendError(error, context, key, aggregatedError);
}

if (Array.isArray(obj[key]) && (op === '$addToSet' || op === '$push') && key !== '$each') {
Expand Down Expand Up @@ -383,7 +396,7 @@ function walkUpdatePath(schema, obj, op, options, context, filter, pref) {
* ignore
*/

function _handleCastError(error, query, key, aggregatedError) {
function _appendError(error, query, key, aggregatedError) {
if (typeof query !== 'object' || !query.options.multipleCastError) {
throw error;
}
Expand Down Expand Up @@ -455,6 +468,8 @@ function castUpdateVal(schema, val, op, $conditional, context, path) {
return val;
}

// console.log('CastUpdateVal', path, op, val, schema);

const cond = schema.caster && op in castOps &&
(utils.isObject(val) || Array.isArray(val));
if (cond && !overwriteOps[op]) {
Expand Down
40 changes: 38 additions & 2 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ const defaultMongooseSymbol = Symbol.for('mongoose:default');

require('./helpers/printJestWarning');

const objectIdHexRegexp = /^[0-9A-Fa-f]{24}$/;

/**
* Mongoose constructor.
*
Expand Down Expand Up @@ -945,11 +947,13 @@ Mongoose.prototype.ObjectId = SchemaTypes.ObjectId;
* mongoose.isValidObjectId(new mongoose.Types.ObjectId()); // true
* mongoose.isValidObjectId('0123456789ab'); // true
* mongoose.isValidObjectId(6); // true
* mongoose.isValidObjectId(new User({ name: 'test' })); // true
*
* mongoose.isValidObjectId({ test: 42 }); // false
*
* @method isValidObjectId
* @param {Any} value
* @returns {boolean} true if it is a valid ObjectId
* @param {Any} v
* @returns {boolean} true if `v` is something Mongoose can coerce to an ObjectId
* @api public
*/

Expand All @@ -958,6 +962,38 @@ Mongoose.prototype.isValidObjectId = function(v) {
return _mongoose.Types.ObjectId.isValid(v);
};

/**
* Returns true if the given value is a Mongoose ObjectId (using `instanceof`) or if the
* given value is a 24 character hex string, which is the most commonly used string representation
* of an ObjectId.
*
* This function is similar to `isValidObjectId()`, but considerably more strict, because
* `isValidObjectId()` will return `true` for _any_ value that Mongoose can convert to an
* ObjectId. That includes Mongoose documents, any string of length 12, and any number.
* `isObjectIdOrHexString()` returns true only for `ObjectId` instances or 24 character hex
* strings, and will return false for numbers, documents, and strings of length 12.
*
* ####Example:
*
* mongoose.isObjectIdOrHexString(new mongoose.Types.ObjectId()); // true
* mongoose.isObjectIdOrHexString('62261a65d66c6be0a63c051f'); // true
*
* mongoose.isObjectIdOrHexString('0123456789ab'); // false
* mongoose.isObjectIdOrHexString(6); // false
* mongoose.isObjectIdOrHexString(new User({ name: 'test' })); // false
* mongoose.isObjectIdOrHexString({ test: 42 }); // false
*
* @method isObjectIdOrHexString
* @param {Any} v
* @returns {boolean} true if `v` is an ObjectId instance _or_ a 24 char hex string
* @api public
*/

Mongoose.prototype.isObjectIdOrHexString = function(v) {
const _mongoose = this instanceof Mongoose ? this : mongoose;
return v instanceof _mongoose.Types.ObjectId || (typeof v === 'string' && objectIdHexRegexp.test(v));
};

/**
*
* Syncs all the indexes for the models registered with this connection.
Expand Down
1 change: 1 addition & 0 deletions lib/schema/array.js
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,7 @@ handle.$gte =
handle.$lt =
handle.$lte =
handle.$ne =
handle.$not =
handle.$regex = SchemaArray.prototype.castForQuery;

// `$in` is special because you can also include an empty array in the query
Expand Down
29 changes: 15 additions & 14 deletions lib/schematype.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const $exists = require('./schema/operators/exists');
const $type = require('./schema/operators/type');
const handleImmutable = require('./helpers/schematype/handleImmutable');
const isAsyncFunction = require('./helpers/isAsyncFunction');
const isSimpleValidator = require('./helpers/isSimpleValidator');
const immediate = require('./helpers/immediate');
const schemaTypeSymbol = require('./helpers/symbols').schemaTypeSymbol;
const utils = require('./utils');
Expand Down Expand Up @@ -886,7 +887,7 @@ SchemaType.prototype.validate = function(obj, message, type) {
properties = { validator: obj, message: message };
properties.type = type || 'user defined';
} else if (message instanceof Object && !type) {
properties = utils.clone(message);
properties = isSimpleValidator(message) ? Object.assign({}, message) : utils.clone(message);
if (!properties.message) {
properties.message = properties.msg;
}
Expand Down Expand Up @@ -914,8 +915,8 @@ SchemaType.prototype.validate = function(obj, message, type) {
arg = arguments[i];
if (!utils.isPOJO(arg)) {
const msg = 'Invalid validator. Received (' + typeof arg + ') '
+ arg
+ '. See https://mongoosejs.com/docs/api.html#schematype_SchemaType-validate';
+ arg
+ '. See https://mongoosejs.com/docs/api.html#schematype_SchemaType-validate';

throw new Error(msg);
}
Expand Down Expand Up @@ -1242,7 +1243,7 @@ SchemaType.prototype.doValidate = function(value, fn, scope, options) {

// Avoid non-object `validators`
const validators = this.validators.
filter(v => v != null && typeof v === 'object');
filter(v => typeof v === 'object' && v !== null);

let count = validators.length;

Expand All @@ -1251,15 +1252,15 @@ SchemaType.prototype.doValidate = function(value, fn, scope, options) {
}

for (let i = 0, len = validators.length; i < len; ++i) {
const v = validators[i];
if (err) {
break;
}

const v = validators[i];
const validator = v.validator;
let ok;

const validatorProperties = utils.clone(v);
const validatorProperties = isSimpleValidator(v) ? Object.assign({}, v) : utils.clone(v);
validatorProperties.path = options && options.path ? options.path : path;
validatorProperties.value = value;

Expand Down Expand Up @@ -1373,12 +1374,12 @@ SchemaType.prototype.doValidateSync = function(value, scope, options) {

const v = validators[i];

if (v == null || typeof v !== 'object') {
if (v === null || typeof v !== 'object') {
continue;
}

const validator = v.validator;
const validatorProperties = utils.clone(v);
const validatorProperties = isSimpleValidator(v) ? Object.assign({}, v) : utils.clone(v);
validatorProperties.path = options && options.path ? options.path : path;
validatorProperties.value = value;
let ok = false;
Expand Down Expand Up @@ -1453,8 +1454,8 @@ SchemaType._isRef = function(self, value, doc, init) {
return true;
}
if (!Buffer.isBuffer(value) && // buffers are objects too
value._bsontype !== 'Binary' // raw binary value from the db
&& utils.isObject(value) // might have deselected _id in population query
value._bsontype !== 'Binary' // raw binary value from the db
&& utils.isObject(value) // might have deselected _id in population query
) {
return true;
}
Expand Down Expand Up @@ -1495,10 +1496,10 @@ SchemaType.prototype._castRef = function _castRef(value, doc, init) {
const pop = owner.$populated(path, true);
let ret = value;
if (!doc.$__.populated ||
!doc.$__.populated[path] ||
!doc.$__.populated[path].options ||
!doc.$__.populated[path].options.options ||
!doc.$__.populated[path].options.options.lean) {
!doc.$__.populated[path] ||
!doc.$__.populated[path].options ||
!doc.$__.populated[path].options.options ||
!doc.$__.populated[path].options.options.lean) {
ret = new pop.options[populateModelSymbol](value);
ret.$__.wasPopulated = true;
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "mongoose",
"description": "Mongoose MongoDB ODM",
"version": "6.2.4",
"version": "6.2.5",
"author": "Guillermo Rauch <guillermo@learnboost.com>",
"keywords": [
"mongodb",
Expand Down
4 changes: 2 additions & 2 deletions test/docs/lean.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('Lean Tutorial', function() {
// In case you were wondering, the JSON form of a Mongoose doc is the same
// as the POJO. This additional memory only affects how much memory your
// Node.js process uses, not how much data is sent over the network.
JSON.stringify(normalDoc).length === JSON.stringify(leanDoc.length); // true
JSON.stringify(normalDoc).length === JSON.stringify(leanDoc).length; // true
// acquit:ignore:start
assert.ok(v8Serialize(normalDoc).length >= 300 && v8Serialize(normalDoc).length <= 800, v8Serialize(normalDoc).length);
assert.equal(v8Serialize(leanDoc).length, 32);
Expand Down Expand Up @@ -194,4 +194,4 @@ describe('Lean Tutorial', function() {
assert.equal(group.members[1].name, 'Kira Nerys');
// acquit:ignore:end
});
});
});

0 comments on commit 6597230

Please sign in to comment.