What makes a valid ObjectId #11227
Comments
vkarpov15
added
priority
|
See #11209 (comment). We made |
|
Would you prefer to continue in discussion #11221 or right here, in a closed issue? |
|
@Maximusya here is fine. |
|
Well, new bugs and issues are being added as we speak.
Well, you state that Here https://github.com/mongodb/js-bson/blob/master/src/objectid.ts#L295 in the first line Meanwhile in our business code we resorted to our own very narrow guard: export const isValidObjectId = value => (
value != null
&& (
value instanceof mongoose.Types.ObjectId
|| (typeof value === 'string' && value.length === 24 && /^[0-9A-Fa-f]{24}$/.test(value))
)
);And as a precausion we added this set of tests to guard against future unexpected behavior (which just happened haha): import mongoose from 'mongoose';
import { isValidObjectId } from '../app/validation';
describe('isValidObjectId', () => {
const isValid = value => isValidObjectId(value);
const valid = [
new mongoose.Types.ObjectId(),
'580e0797bb495f0a200e91ad',
'580E0797BB495F0A200E91AD',
];
const invald = [
undefined,
null,
7,
{ },
'7 chars',
'12 charsssss',
'15 charssssssss',
'24 non-hex charsssssssss',
{ _id: new mongoose.Types.ObjectId() },
{ id: new mongoose.Types.ObjectId() },
{ _id: '580e0797bb495f0a200e91ad' },
{ id: '580e0797bb495f0a200e91ad' },
{ _id: '12 charsssss' },
{ id: '12 charsssss' },
{ _id: null },
{ id: null },
{ _id: undefined },
{ id: undefined },
Buffer.alloc(3),
Buffer.alloc(12),
Buffer.alloc(15),
{ _id: Buffer.alloc(3) },
{ _id: Buffer.alloc(12) },
{ _id: Buffer.alloc(15) },
{ id: Buffer.alloc(3) },
{ id: Buffer.alloc(12) },
{ id: Buffer.alloc(15) },
];
it('returns true for valid inputs', () => {
valid.forEach(value => expect(value).toSatisfy(isValid));
});
it('returns false for invalid inputs', () => {
invald.forEach(value => expect(value).not.toSatisfy(isValid));
});
});
describe('mongoose.isValidObjectId', () => {
const isValid = value => mongoose.isValidObjectId(value);
const valid = [
undefined,
null,
new mongoose.Types.ObjectId(),
'580e0797bb495f0a200e91ad',
'580E0797BB495F0A200E91AD',
'12 charsssss',
{ _id: new mongoose.Types.ObjectId() },
{ _id: '580e0797bb495f0a200e91ad' },
{ _id: '12 charsssss' },
];
const invald = [
{ },
7,
'7 chars',
'15 charssssssss',
{ id: new mongoose.Types.ObjectId() },
{ id: '580e0797bb495f0a200e91ad' },
{ id: '12 charsssss' },
{ _id: null },
{ id: null },
{ _id: undefined },
{ id: undefined },
Buffer.alloc(3),
Buffer.alloc(12),
Buffer.alloc(15),
{ _id: Buffer.alloc(3) },
{ _id: Buffer.alloc(12) },
{ _id: Buffer.alloc(15) },
{ id: Buffer.alloc(3) },
{ id: Buffer.alloc(12) },
{ id: Buffer.alloc(15) },
];
it('returns true for valid inputs', () => {
valid.forEach(value => expect(value).toSatisfy(isValid));
});
it('returns false for invalid inputs', () => {
invald.forEach(value => expect(value).not.toSatisfy(isValid));
});
});
describe('mongoose.Types.ObjectId.isValid', () => {
const isValid = value => mongoose.Types.ObjectId.isValid(value);
const valid = [
7,
new mongoose.Types.ObjectId(),
'580e0797bb495f0a200e91ad',
'580E0797BB495F0A200E91AD',
'12 charsssss',
{ id: '580e0797bb495f0a200e91ad' },
{ id: '12 charsssss' },
Buffer.alloc(12),
{ id: Buffer.alloc(12) },
];
const invald = [
undefined,
null,
{ },
'7 chars',
'15 charssssssss',
{ _id: new mongoose.Types.ObjectId() },
{ id: new mongoose.Types.ObjectId() },
{ _id: '580e0797bb495f0a200e91ad' },
{ _id: '12 charsssss' },
{ _id: null },
{ id: null },
{ _id: undefined },
{ id: undefined },
Buffer.alloc(3),
Buffer.alloc(15),
{ _id: Buffer.alloc(3) },
{ _id: Buffer.alloc(12) },
{ _id: Buffer.alloc(15) },
{ id: Buffer.alloc(3) },
{ id: Buffer.alloc(15) },
];
it('returns true for valid inputs', () => {
valid.forEach(value => expect(value).toSatisfy(isValid));
});
it('returns false for invalid inputs', () => {
invald.forEach(value => expect(value).not.toSatisfy(isValid));
});
}); |
Discussed in #11221
Originally posted by Maximusya January 13, 2022
Right now I am migrating from Mongoose 5.x to 6.x and got about half of integration tests fail just because of undocumented changes in ObjectId validation.
These two mongoose validation methods differ in behavior both between the two in one version of mongoose and between mongoose versions (+dependencies):
mongoose.Types.ObjectId.isValidmongoose.isValidObjectIdI get the expectation that once a variable has passed any of these validations, it can be safely specified both as
new mongoose.Type.ObjectId(...)ctor arg, and any ofMyModel.findById(..)et al.Another kind of expectation (but probably far fetched): if a variable has passed any of these validations IT IS an id, but not a full document loaded from DB. Imagine a helper method that loads missing data if the argument passed in has properties that are not yet populated/loaded.
Example:
[email protected] (same for @6.1.5) + [email protected]:
[email protected] + [email protected]
Note: [email protected] is demonstrated because since [email protected]
{ id: '580e0797bb495f0a200e91ad' }is considered a valid ObjectId. See #11209.You can install this version today using version override in package.json:
[email protected] + [email protected]
And some old versions (precisely from the codebase I am migrating now)
You can see how much has changed from those "old times".
Notable changes between versions
I can't help quoting a famous anecdote (translated):
MongoDB and mongoose seem to be mature products. And having a long history of changes/bugs with such base entity as ObjectId (regarding what is a valid ObjectId) - is appalling to say the least.
The text was updated successfully, but these errors were encountered: