Feedback

loopbackio · May 31, 2019 · 9114a18 · 9114a18
1 parent e6d504b
commit 9114a18
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 18 deletions.
diff --git 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,15 @@ MongoDB.prototype.ping = function(cb) {
   }
 };
 
-// Determine if a property must have an ObjectID value
-function mustBeObjectID(propDef) {
+// 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;
   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 +1890,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;
@@ -1919,21 +1919,19 @@ function isObjectIDProperty(modelCtor, propDef, value, options) {
     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 (isStoredAsObjectID(propDef)) {
+      // dataType looks like an ObjectID
+      if (value.match(ObjectIdValueRegex)) return true;
       // if dataType is specified as ObjectID, value is expected to an ObjectID-like string
       return false;
-    } else if (!value.match(ObjectIdRegex)) {
+    } else if (!value.match(ObjectIdValueRegex)) {
       // 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);
+    return !isStrictObjectIDCoercionEnabled(modelCtor, options);
   } else {
     return false;
   }

diff --git a/test/id.test.js b/test/id.test.js
@@ -345,18 +345,31 @@ 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() {
       const user = await User.create({id: objectIdLikeString, name: 'John'});
       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) {