Skip to content

Commit 4f5e752

Browse files
committed
fix: cleanup and tests for #15725
1 parent ce7e016 commit 4f5e752

File tree

2 files changed

+25
-17
lines changed

2 files changed

+25
-17
lines changed

lib/types/documentArray/methods/index.js

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,9 @@ const methods = {
137137
idSchemaType = schemaType.schema.path('_id');
138138
} else if (schemaType && schemaType.casterConstructor && schemaType.casterConstructor.schema) {
139139
idSchemaType = schemaType.casterConstructor.schema.path('_id');
140-
} else if (this.length > 0 && this[0] != null && this[0].$__schema) {
141-
idSchemaType = this[0].$__schema.path('_id');
142140
}
143141

144-
let castedId = id;
142+
let castedId = null;
145143
if (idSchemaType) {
146144
try {
147145
castedId = idSchemaType.cast(id);
@@ -150,7 +148,6 @@ const methods = {
150148
}
151149
}
152150

153-
let sid;
154151
let _id;
155152

156153
for (const val of this) {
@@ -160,24 +157,20 @@ const methods = {
160157

161158
_id = val.get('_id');
162159

163-
if (_id === null || typeof _id === 'undefined') {
160+
if (_id == null) {
164161
continue;
165162
} else if (_id instanceof Document) {
166-
sid || (sid = String(id));
167-
if (sid == _id._id) {
163+
_id = _id.get('_id');
164+
if (castedId != null && utils.deepEqual(castedId, _id)) {
168165
return val;
169-
}
170-
} else if (castedId != null && !isBsonType(castedId, 'ObjectId') && !isBsonType(_id, 'ObjectId')) {
171-
if (castedId == _id || utils.deepEqual(castedId, _id)) {
172-
return val;
173-
}
174-
} else if (castedId != null && isBsonType(castedId, 'ObjectId') && isBsonType(_id, 'ObjectId')) {
175-
if (castedId.toString() === _id.toString()) {
166+
} else if (castedId == null && (id == _id || utils.deepEqual(id, _id))) {
167+
// Backwards compat: compare user-specified param to _id using loose equality
176168
return val;
177169
}
178-
} else if (castedId != null && castedId == _id) {
170+
} else if (castedId != null && utils.deepEqual(castedId, _id)) {
179171
return val;
180-
} else if (castedId == null && (id == _id || utils.deepEqual(id, _id))) {
172+
} else if (castedId == null && (_id == id || utils.deepEqual(id, _id))) {
173+
// Backwards compat: compare user-specified param to _id using loose equality
181174
return val;
182175
}
183176
}

test/types.documentarray.test.js

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ describe('types.documentarray', function() {
8383
sub1.title = 'Hello again to all my friends';
8484
let id = sub1.id;
8585

86-
let a = new MongooseDocumentArray([sub1]);
86+
let a = new MongooseDocumentArray([sub1], 'test', null);
8787
assert.equal(a.id(id).title, 'Hello again to all my friends');
8888
assert.equal(a.id(sub1._id).title, 'Hello again to all my friends');
8989

@@ -186,8 +186,23 @@ describe('types.documentarray', function() {
186186

187187
a = new MongooseDocumentArray([sub]);
188188
assert.equal(a.id(id).title, 'Hello again to all my friends');
189+
});
190+
191+
it('#id with custom schematype (gh-15725)', function() {
192+
const schema = new Schema({ _id: Number, name: String });
193+
const Subdocument = TestDoc(schema);
194+
195+
const sub1 = new Subdocument();
196+
sub1._id = 42;
197+
sub1.title = 'Hello again to all my friends';
189198

199+
const parentDoc = { $__: true, $__schema: new Schema({ subdocs: [schema] }) };
190200

201+
const a = new MongooseDocumentArray([sub1], 'test', parentDoc);
202+
assert.equal(a.id('42').title, 'Hello again to all my friends');
203+
assert.equal(a.id(sub1.id).title, 'Hello again to all my friends');
204+
assert.ok(!a.id('43'));
205+
assert.ok(!a.id('not a number'));
191206
});
192207

193208
describe('inspect', function() {

0 commit comments

Comments
 (0)