From 9c6c662d274e5668fa8f6e257b9865ef0770349f Mon Sep 17 00:00:00 2001 From: Ashu Choudhary Date: Fri, 7 Nov 2025 00:05:23 +0530 Subject: [PATCH 1/6] fix: cast id parameter based on schema _id type in DocumentArray.id() Fixes #15724 The id() method now properly casts the id parameter based on the schema's _id type instead of always attempting to cast to ObjectId. This fixes issues when using custom _id types like String, Number, or Object. --- lib/types/documentArray/methods/index.js | 43 ++++++++++++++++++++---- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/lib/types/documentArray/methods/index.js b/lib/types/documentArray/methods/index.js index dae1baf105..4c577f07aa 100644 --- a/lib/types/documentArray/methods/index.js +++ b/lib/types/documentArray/methods/index.js @@ -121,23 +121,48 @@ 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.casterConstructor && schemaType.casterConstructor.schema) { + idSchemaType = schemaType.casterConstructor.schema.path('_id'); + } else if (this.length > 0 && this[0] && this[0].$__schema) { + idSchemaType = this[0].$__schema.path('_id'); + } + + let castedId = id; + if (idSchemaType) { + try { + castedId = idSchemaType.cast(id); + } catch (e) { + castedId = id; + } + } + let casted; try { - casted = castObjectId(id).toString(); + if (isBsonType(castedId, 'ObjectId')) { + casted = castedId.toString(); + } else if (idSchemaType == null) { + casted = castObjectId(id).toString(); + } } catch (e) { casted = null; } + let sid; + let _id; + for (const val of this) { if (!val) { continue; @@ -153,10 +178,14 @@ const methods = { return val; } } else if (!isBsonType(id, 'ObjectId') && !isBsonType(_id, 'ObjectId')) { - if (id == _id || utils.deepEqual(id, _id)) { + if (castedId == _id || utils.deepEqual(castedId, _id)) { + return val; + } + } else if (casted != null && isBsonType(_id, 'ObjectId')) { + if (casted == _id.toString()) { return val; } - } else if (casted == _id) { + } else if (castedId == _id) { return val; } } From 79adc2c27a831accefb19dbc27da3911021efc9a Mon Sep 17 00:00:00 2001 From: Ashu Choudhary Date: Sat, 8 Nov 2025 17:31:22 +0530 Subject: [PATCH 2/6] fix: cast document array ids using schema type --- lib/types/documentArray/methods/index.js | 28 +++++++++--------------- 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/lib/types/documentArray/methods/index.js b/lib/types/documentArray/methods/index.js index 4c577f07aa..725fb9e381 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'); @@ -134,7 +133,9 @@ const methods = { const schemaType = this[arraySchemaSymbol]; let idSchemaType = null; - if (schemaType && schemaType.casterConstructor && schemaType.casterConstructor.schema) { + if (schemaType && schemaType.schema) { + idSchemaType = schemaType.schema.path('_id'); + } else if (schemaType && schemaType.casterConstructor && schemaType.casterConstructor.schema) { idSchemaType = schemaType.casterConstructor.schema.path('_id'); } else if (this.length > 0 && this[0] && this[0].$__schema) { idSchemaType = this[0].$__schema.path('_id'); @@ -145,21 +146,10 @@ const methods = { try { castedId = idSchemaType.cast(id); } catch (e) { - castedId = id; + castedId = null; } } - let casted; - try { - if (isBsonType(castedId, 'ObjectId')) { - casted = castedId.toString(); - } else if (idSchemaType == null) { - casted = castObjectId(id).toString(); - } - } catch (e) { - casted = null; - } - let sid; let _id; @@ -177,15 +167,17 @@ const methods = { if (sid == _id._id) { return val; } - } else if (!isBsonType(id, 'ObjectId') && !isBsonType(_id, 'ObjectId')) { + } else if (castedId != null && !isBsonType(castedId, 'ObjectId') && !isBsonType(_id, 'ObjectId')) { if (castedId == _id || utils.deepEqual(castedId, _id)) { return val; } - } else if (casted != null && isBsonType(_id, 'ObjectId')) { - if (casted == _id.toString()) { + } else if (castedId != null && isBsonType(castedId, 'ObjectId') && isBsonType(_id, 'ObjectId')) { + if (castedId.toString() === _id.toString()) { return val; } - } else if (castedId == _id) { + } else if (castedId != null && castedId == _id) { + return val; + } else if (castedId == null && (id == _id || utils.deepEqual(id, _id))) { return val; } } From d2aaf3761dcf7411668d4ce441c6afd4a1bbb88c Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 10 Nov 2025 10:37:26 -0500 Subject: [PATCH 3/6] Update lib/types/documentArray/methods/index.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- lib/types/documentArray/methods/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/types/documentArray/methods/index.js b/lib/types/documentArray/methods/index.js index 725fb9e381..04bee09916 100644 --- a/lib/types/documentArray/methods/index.js +++ b/lib/types/documentArray/methods/index.js @@ -137,7 +137,7 @@ const methods = { idSchemaType = schemaType.schema.path('_id'); } else if (schemaType && schemaType.casterConstructor && schemaType.casterConstructor.schema) { idSchemaType = schemaType.casterConstructor.schema.path('_id'); - } else if (this.length > 0 && this[0] && this[0].$__schema) { + } else if (this.length > 0 && this[0] != null && this[0].$__schema) { idSchemaType = this[0].$__schema.path('_id'); } From 4f5e752f67f0227f596403a2285384fe9fe3f000 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 10 Nov 2025 11:42:55 -0500 Subject: [PATCH 4/6] fix: cleanup and tests for #15725 --- lib/types/documentArray/methods/index.js | 25 +++++++++--------------- test/types.documentarray.test.js | 17 +++++++++++++++- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/lib/types/documentArray/methods/index.js b/lib/types/documentArray/methods/index.js index 04bee09916..7958b0310b 100644 --- a/lib/types/documentArray/methods/index.js +++ b/lib/types/documentArray/methods/index.js @@ -137,11 +137,9 @@ const methods = { idSchemaType = schemaType.schema.path('_id'); } else if (schemaType && schemaType.casterConstructor && schemaType.casterConstructor.schema) { idSchemaType = schemaType.casterConstructor.schema.path('_id'); - } else if (this.length > 0 && this[0] != null && this[0].$__schema) { - idSchemaType = this[0].$__schema.path('_id'); } - let castedId = id; + let castedId = null; if (idSchemaType) { try { castedId = idSchemaType.cast(id); @@ -150,7 +148,6 @@ const methods = { } } - let sid; let _id; for (const val of this) { @@ -160,24 +157,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 (castedId != null && !isBsonType(castedId, 'ObjectId') && !isBsonType(_id, 'ObjectId')) { - if (castedId == _id || utils.deepEqual(castedId, _id)) { - return val; - } - } else if (castedId != null && isBsonType(castedId, 'ObjectId') && isBsonType(_id, 'ObjectId')) { - if (castedId.toString() === _id.toString()) { + } 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 (castedId != null && castedId == _id) { + } else if (castedId != null && utils.deepEqual(castedId, _id)) { return val; - } else if (castedId == null && (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; } } diff --git a/test/types.documentarray.test.js b/test/types.documentarray.test.js index 2acae98f08..28eb043b1e 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], 'test', 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() { From 82eeb11f0d49173550695155600a69f8f6a30064 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 13 Nov 2025 17:03:00 -0500 Subject: [PATCH 5/6] refactor: remove unnecesary assignment --- lib/types/documentArray/methods/index.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/types/documentArray/methods/index.js b/lib/types/documentArray/methods/index.js index 7958b0310b..8f918f6a32 100644 --- a/lib/types/documentArray/methods/index.js +++ b/lib/types/documentArray/methods/index.js @@ -143,9 +143,7 @@ const methods = { if (idSchemaType) { try { castedId = idSchemaType.cast(id); - } catch (e) { - castedId = null; - } + } catch (_err) {} } let _id; From 64384e0127288468e0c1c77359b83ca588a70ab0 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 13 Nov 2025 17:04:22 -0500 Subject: [PATCH 6/6] Update test/types.documentarray.test.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- test/types.documentarray.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/types.documentarray.test.js b/test/types.documentarray.test.js index 28eb043b1e..8137f33648 100644 --- a/test/types.documentarray.test.js +++ b/test/types.documentarray.test.js @@ -198,7 +198,7 @@ describe('types.documentarray', function() { const parentDoc = { $__: true, $__schema: new Schema({ subdocs: [schema] }) }; - const a = new MongooseDocumentArray([sub1], 'test', parentDoc); + 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'));