Simplifying the connector-mongodb PR after database-juggler fix

README.md

@@ -27,20 +27,28 @@ To use it you need `loopback-datasource-juggler@1.0.x`.
         ...
     ```
 
-## About MongoDB ID field
 
-MongoDB uses a specific ID field with BSON `ObjectID` type, named `_id`
+### About MongoDB _id field
 
-This connector does not expose this `_id` by default, to keep consistency with other connectors.
+MongoDB uses a specific ID field with BSON `ObjectID` type, named `_id`
 
-Default ID field keeps MongoDB `ObjectID` type, but field name is `id`, or any other field name specified in the loopback model's definition.
+This connector does not expose MongoDB `_id` by default, to keep consistency with other connectors. Instead, it is transparently mapped to the `id` field - which is declared by default in the model if you do not define any `id`. 
 
-If you want to use the original MongoDB `_id` field in your loopback model, please explicitly specify the type **DataSource.ObjectID** / **MongoDB.ObjectID** type in your model's definition.
+If you wish to still be able to access `_id` property, you must define it explicitely as your model ID, along with its type.
 
 *Example :*
 
     var ds = app.dataSources.db;
-    MyModel = ds.createModel('mymodel', {_id: ds.ObjectID});
+    MyModel = ds.createModel('mymodel', {
+        _id: { type: ds.ObjectID, id: true }
+    });
+
+*Example with a Number _id :
+
+    MyModel = ds.createModel('mymodel', {
+        _id: { type: Number, id: true }
+    });
+
 
 ## Customizing MongoDB configuration for tests/examples
 

lib/mongodb.js

@@ -120,50 +120,6 @@ MongoDB.prototype.connect = function (callback) {
   }
 };
 
-/**
- * Hook the setter if model use MongoDB _id property as ID
- * @param {Object} modelDefinition The model definition
- */
-MongoDB.prototype.define = function (modelDefinition) {
-  var _idIsModelID = modelDefinition && 
-                     modelDefinition.properties && 
-                     modelDefinition.properties._id && 
-                     modelDefinition.properties._id.id;
-  if (_idIsModelID) {
-    if (modelDefinition.properties._id.type !== this.dataSource.ObjectID) {
-      throw new Error('To use MongoDB _id as loopback model ID ' +
-                   'please use type [MongoDB|DataSource].ObjectID ' +
-                   'instead of ' + modelDefinition.properties._id.type);
-    }
-    modelDefinition.model.setter._id = function(id) {
-      if ((id instanceof this.getDataSource().ObjectID)) return id;
-      return this.getDataSource().ObjectID(id);
-    }
-  }
-  return Connector.prototype.define.call(this, modelDefinition);
-};
-
-/**
- * Hook the setter if model use MongoDB _id property as ID
- * @param {String} model The model name
- * @param {String} propertyName The property name
- * @param {Object} propertyDefinition The object for property metadata
- */
-MongoDB.prototype.defineProperty = function (model, propertyName, propertyDefinition) {
-  if (propertyName = '_id' && propertyDefinition.id) {
-    if (propertyDefinition.type !== this.dataSource.ObjectID) {
-      throw new Error('To use MongoDB _id as loopback model ID ' +
-                   'please use type [MongoDB|DataSource].ObjectID ' +
-                   'instead of ' + propertyDefinition.type);
-    }
-    this._models[model].model.setter._id = function(id) {
-      if ((id instanceof this.getDataSource().ObjectID)) return id;
-      return this.getDataSource().ObjectID(id);
-    }
-  }
-  return Connector.prototype.defineProperty.call(this, model, propertyName, propertyDefinition);
-};
-
 MongoDB.prototype.getTypes = function () {
   return ['db', 'nosql', 'mongodb'];
 };

package.json

@@ -1,6 +1,6 @@
 {
   "name": "loopback-connector-mongodb",
-  "version": "1.1.7",
+  "version": "1.1.6",
   "description": "LoopBack MongoDB Connector",
   "keywords": [ "StrongLoop", "LoopBack", "MongoDB", "DataSource", "Connector" ],
   "main": "index.js",

test/mongodb.test.js

@@ -31,6 +31,12 @@ describe('mongodb', function () {
       content: { type: String }
     });
 
+    PostWithNumberId = db.define('PostWithNumberId', {
+      _id: {type: Number, id: true},
+      title: { type: String, length: 255, index: true },
+      content: { type: String }
+    });
+
     User.hasMany(Post);
     Post.belongsTo(User);
   });
@@ -39,44 +45,35 @@ describe('mongodb', function () {
     User.destroyAll(function () {
       Post.destroyAll(function () {
         PostWithObjectId.destroyAll(function () {
-          done();
+          PostWithNumberId.destroyAll(function () {
+            done();
+          });
         });
       });
     });
   });
 
-
-  it('choosing _id as a model ID should throw an error if type is not MongoDBObjectID', function (done) {
-    (function() {
-      db.define('GoodModel', {_id: {type: db.ObjectID, id: true}})
-    }).should.not.throw();
-
-    (function() {
-      db.define('BadModel', {_id: {type: String, id: true}})
-    }).should.throw();
-    (function() {
-      db.define('BadModel', {_id: {type: Number, id: true}})
-    }).should.throw();
+  it('should have created models with correct _id types', function (done) {
+    PostWithObjectId.definition.properties._id.type.should.be.equal(db.ObjectID);
+    should.not.exist(PostWithObjectId.definition.properties.id);
+    PostWithNumberId.definition.properties._id.type.should.be.equal(Number);
+    should.not.exist(PostWithNumberId.definition.properties.id);
 
     done();
   });
 
-  it('should correctly define _is by model or property definition', function (done) {
-    Person = db.define('Person', {_id: {type: db.ObjectID, id: true}});
-    Animal = db.define('Animal');
-    Animal.defineProperty('_id', {type: db.ObjectID, id: true});
-    Person.definition.properties._id.type.should.be.equal(db.ObjectID);
-    Animal.definition.properties._id.type.should.be.equal(db.ObjectID);
-
-    done();
-  });
-
-  it('create with `_id` as defined id should return _id', function (done) {
-    PostWithObjectId.create(function (err, post) {
+  it('should handle correctly type Number for id field _id', function (done) {
+    PostWithNumberId.create({_id: 3, content: "test"}, function (err, person) {
       should.not.exist(err);
-      post._id.should.be.an.instanceOf(db.ObjectID);
-
-      done();
+      should.not.exist(person.id);
+      person._id.should.be.equal(3);
+
+      PostWithNumberId.findById(person._id, function (err, p) {
+        should.not.exist(err);
+        p.content.should.be.equal("test");
+
+        done();
+      });
     });
   });
 
@@ -309,7 +306,9 @@ describe('mongodb', function () {
   after(function (done) {
     User.destroyAll(function () {
       Post.destroyAll(function () {
-        PostWithObjectId.destroyAll(done);
+        PostWithObjectId.destroyAll(function () {
+          PostWithNumberId.destroyAll(done);
+        });
       });
     });
   });