diff --git a/lib/mongodb.js b/lib/mongodb.js index 79e50d89a..2ccbe43dd 100644 --- a/lib/mongodb.js +++ b/lib/mongodb.js @@ -18,7 +18,7 @@ const Connector = require('loopback-connector').Connector; const debug = require('debug')('loopback:connector:mongodb'); const Decimal128 = mongodb.Decimal128; -const ObjectIdRegex = /^[0-9a-fA-F]{24}$/; +const ObjectIdValueRegex = /^[0-9a-fA-F]{24}$/; const ObjectIdTypeRegex = /objectid/i; exports.ObjectID = ObjectID; @@ -38,7 +38,7 @@ function ObjectID(id) { // MongoDB's ObjectID constructor accepts number, 12-byte string or 24-byte // hex string. For LoopBack, we only allow 24-byte hex string, but 12-byte // string such as 'line-by-line' should be kept as string - if (ObjectIdRegex.test(id)) { + if (ObjectIdValueRegex.test(id)) { return bson.ObjectID(id); } else { return id; @@ -1873,15 +1873,31 @@ MongoDB.prototype.ping = function(cb) { } }; -// Determine if a property must have an ObjectID value -function mustBeObjectID(propDef) { +// Case insensitive check if a string looks like an Object ID +function typeIsObjectId(input) { + if (!input) return false; + return typeof input === 'string' && input.match(/objectid/i); +} + +// Determine if a property must be stored as ObjectID +function isStoredAsObjectID(propDef) { if (!propDef) return false; - if (propDef.mongodb && ObjectIdTypeRegex.test(propDef.mongodb.dataType)) return true; + + if (propDef.mongodb) { + if (ObjectIdTypeRegex.test(propDef.mongodb.dataType)) return true; + } else if (propDef.type) { + if (typeof propDef.type === 'string' && typeIsObjectId(propDef.type)) return true; + else if (Array.isArray(propDef.type)) { + if (propDef.type[0] === ObjectID || typeIsObjectId(propDef.type[0])) { + return true; + } + } + } return false; } // Determine if strictObjectIDCoercion should be enabled -function enableStrictObjectIDCoercion(modelCtor, options) { +function isStrictObjectIDCoercionEnabled(modelCtor, options) { const settings = modelCtor.settings; return (settings && settings.strictObjectIDCoercion) || (modelCtor.model && modelCtor.model.getConnector().settings.strictObjectIDCoercion) || @@ -1890,17 +1906,17 @@ function enableStrictObjectIDCoercion(modelCtor, options) { } function coerceToObjectId(modelCtor, propDef, propValue) { - if (mustBeObjectID(propDef)) { + if (isStoredAsObjectID(propDef)) { if (isObjectIDProperty(modelCtor, propDef, propValue)) { return ObjectID(propValue); } else { throw new Error(`${propValue} is not an ObjectID string`); } - } else if (enableStrictObjectIDCoercion(modelCtor)) { + } else if (isStrictObjectIDCoercionEnabled(modelCtor)) { if (isObjectIDProperty(modelCtor, propDef, propValue)) { return ObjectID(propValue); } - } else if (ObjectIdRegex.test(propValue)) { + } else if (ObjectIdValueRegex.test(propValue)) { return ObjectID(propValue); } return propValue; @@ -1911,29 +1927,13 @@ function coerceToObjectId(modelCtor, propDef, propValue) { * */ function isObjectIDProperty(modelCtor, propDef, value, options) { - if ( - propDef && - (propDef.type === ObjectID || - (Array.isArray(propDef.type) && propDef.type[0] === ObjectID)) - ) { + if (!propDef) return false; + + if (typeof value === 'string' && value.match(ObjectIdValueRegex)) { + if (isStoredAsObjectID(propDef)) return true; + else return !isStrictObjectIDCoercionEnabled(modelCtor, options); + } else if (value instanceof ObjectID) { return true; - } else if ('string' === typeof value) { - // dataType is specified as ObjectID - if (propDef && propDef.mongodb && ObjectIdTypeRegex.test(propDef.mongodb.dataType)) { - // value looks like an ObjectID, and it should be treated as one - // because `prop.mongodb.dataType` is set to 'ObjectID' - // this condition overrides the `strictObjectIDCoercion` setting - if (value.match(ObjectIdRegex)) return true; - // if dataType is specified as ObjectID, value is expected to an ObjectID-like string - return false; - } else if (!value.match(ObjectIdRegex)) { - // obviously not an ObjectID - return false; - } - - // value looks like an ObjectID, determine its nature based on `strictObjectIDCoercion` - // coerce only when strictObjectIDCoercion is disabled - return !enableStrictObjectIDCoercion(modelCtor, options); } else { return false; } diff --git a/test/id.test.js b/test/id.test.js index a9133553e..90d630c1a 100644 --- a/test/id.test.js +++ b/test/id.test.js @@ -345,8 +345,18 @@ describe('strictObjectIDCoercion', function() { {strictObjectIDCoercion: true} ); + const User1 = ds.createModel( + 'user4', + { + id: {type: String, id: true, mongodb: {dataType: 'objectid'}}, + name: String, + }, + {strictObjectIDCoercion: true} + ); + beforeEach(function(done) { - User.deleteAll(done); + User.deleteAll(); + User1.deleteAll(done); }); it('should coerce to ObjectID', async function() { @@ -354,9 +364,12 @@ describe('strictObjectIDCoercion', function() { user.id.should.be.an.instanceOf(ds.ObjectID); }); + it('should coerce to ObjectID (lowercase)', async function() { + const user = await User1.create({id: objectIdLikeString, name: 'John'}); + user.id.should.be.an.instanceOf(ds.ObjectID); + }); + it('should throw if id is not a ObjectID-like string', async function() { - // Why is this not working? - // await User.create({id: 'abc', name: 'John'}).should.be.rejectedWith(/not an ObjectID string/); try { await User.create({id: 'abc', name: 'John'}); } catch (e) {