diff --git a/lib/types/documentArray/methods/index.js b/lib/types/documentArray/methods/index.js index dae1baf105..8f918f6a32 100644 --- a/lib/types/documentArray/methods/index.js +++ b/lib/types/documentArray/methods/index.js @@ -2,7 +2,6 @@ const ArrayMethods = require('../../array/methods'); const Document = require('../../../document'); -const castObjectId = require('../../../cast/objectid'); const getDiscriminatorByValue = require('../../../helpers/discriminator/getDiscriminatorByValue'); const internalToObjectOptions = require('../../../options').internalToObjectOptions; const utils = require('../../../utils'); @@ -121,23 +120,34 @@ const methods = { * * @return {EmbeddedDocument|null} the subdocument or null if not found. * @param {ObjectId|String|Number|Buffer} id - * @TODO cast to the _id based on schema for proper comparison * @method id * @api public * @memberOf MongooseDocumentArray */ id(id) { - let casted; - let sid; - let _id; + if (id == null) { + return null; + } + + const schemaType = this[arraySchemaSymbol]; + let idSchemaType = null; + + if (schemaType && schemaType.schema) { + idSchemaType = schemaType.schema.path('_id'); + } else if (schemaType && schemaType.casterConstructor && schemaType.casterConstructor.schema) { + idSchemaType = schemaType.casterConstructor.schema.path('_id'); + } - try { - casted = castObjectId(id).toString(); - } catch (e) { - casted = null; + let castedId = null; + if (idSchemaType) { + try { + castedId = idSchemaType.cast(id); + } catch (_err) {} } + let _id; + for (const val of this) { if (!val) { continue; @@ -145,18 +155,20 @@ const methods = { _id = val.get('_id'); - if (_id === null || typeof _id === 'undefined') { + if (_id == null) { continue; } else if (_id instanceof Document) { - sid || (sid = String(id)); - if (sid == _id._id) { + _id = _id.get('_id'); + if (castedId != null && utils.deepEqual(castedId, _id)) { return val; - } - } else if (!isBsonType(id, 'ObjectId') && !isBsonType(_id, 'ObjectId')) { - if (id == _id || utils.deepEqual(id, _id)) { + } else if (castedId == null && (id == _id || utils.deepEqual(id, _id))) { + // Backwards compat: compare user-specified param to _id using loose equality return val; } - } else if (casted == _id) { + } else if (castedId != null && utils.deepEqual(castedId, _id)) { + return val; + } else if (castedId == null && (_id == id || utils.deepEqual(id, _id))) { + // Backwards compat: compare user-specified param to _id using loose equality return val; } } diff --git a/test/types.documentarray.test.js b/test/types.documentarray.test.js index 2acae98f08..8137f33648 100644 --- a/test/types.documentarray.test.js +++ b/test/types.documentarray.test.js @@ -83,7 +83,7 @@ describe('types.documentarray', function() { sub1.title = 'Hello again to all my friends'; let id = sub1.id; - let a = new MongooseDocumentArray([sub1]); + let a = new MongooseDocumentArray([sub1], 'test', null); assert.equal(a.id(id).title, 'Hello again to all my friends'); assert.equal(a.id(sub1._id).title, 'Hello again to all my friends'); @@ -186,8 +186,23 @@ describe('types.documentarray', function() { a = new MongooseDocumentArray([sub]); assert.equal(a.id(id).title, 'Hello again to all my friends'); + }); + + it('#id with custom schematype (gh-15725)', function() { + const schema = new Schema({ _id: Number, name: String }); + const Subdocument = TestDoc(schema); + + const sub1 = new Subdocument(); + sub1._id = 42; + sub1.title = 'Hello again to all my friends'; + const parentDoc = { $__: true, $__schema: new Schema({ subdocs: [schema] }) }; + const a = new MongooseDocumentArray([sub1], 'subdocs', parentDoc); + assert.equal(a.id('42').title, 'Hello again to all my friends'); + assert.equal(a.id(sub1.id).title, 'Hello again to all my friends'); + assert.ok(!a.id('43')); + assert.ok(!a.id('not a number')); }); describe('inspect', function() {