Skip to content

Do not return MongoDB-specific _id to client API, except if specified in model definition #19

Merged
merged 2 commits into from Feb 27, 2014
27 README.md
@@ -27,6 +27,29 @@ 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`
+
+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 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: { 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
By default, examples and tests from this module assume there is a MongoDB server
@@ -64,3 +87,7 @@ authentication enabled.
## Running tests
npm test
+
+## Release notes
+
+ * 1.1.7 - Do not return MongoDB-specific _id to client API, except if specifically specified in the model definition
View
19 lib/mongodb.js
@@ -159,7 +159,7 @@ MongoDB.prototype.create = function (model, data, callback) {
} else {
var oid = ObjectID(idValue); // Is it an Object ID?
data._id = oid; // Set it to _id
- delete data[idName];
+ idName != '_id' && delete data[idName];
}
this.collection(model).insert(data, {safe: true}, function (err, m) {
if (self.debug) {
@@ -175,6 +175,8 @@ MongoDB.prototype.create = function (model, data, callback) {
if (!isNaN(num)) {
idValue = num;
}
+ } else if (idType === ObjectID) {
+ idValue = ObjectID(idValue);
}
callback(err, err ? null : idValue);
});
@@ -196,11 +198,12 @@ MongoDB.prototype.save = function (model, data, callback) {
var oid = ObjectID(idValue);
data._id = oid;
- delete data[idName];
+ idName != '_id' && delete data[idName];
this.collection(model).update({_id: oid}, data, {safe: true, upsert: true}, function (err, result) {
if (!err) {
self.setIdValue(model, data, idValue);
+ idName != '_id' && delete data._id;
}
if (self.debug) {
console.log('save.callback', model, err, result);
@@ -241,11 +244,14 @@ MongoDB.prototype.find = function find(model, id, callback) {
if (self.debug) {
console.log('find', model, id);
}
+ var idName = self.idName(model);
var oid = ObjectID(id);
this.collection(model).findOne({_id: oid}, function (err, data) {
if (self.debug) {
console.log('find.callback', model, id, err, data);
}
+
+ data && idName != '_id' && delete data._id;
callback && callback(err, data);
});
};
@@ -264,6 +270,7 @@ MongoDB.prototype.updateOrCreate = function updateOrCreate(model, data, callback
}
var idValue = self.getIdValue(model, data);
+ var idName = self.idName(model);
if (idValue === null || idValue === undefined) {
return this.create(data, callback);
@@ -281,6 +288,7 @@ MongoDB.prototype.updateOrCreate = function updateOrCreate(model, data, callback
}
if (id) {
self.setIdValue(model, data, id);
+ data && idName != '_id' && delete data._id;
callback(null, data);
} else {
callback(null, null); // wtf?
@@ -435,7 +443,7 @@ MongoDB.prototype.all = function all(model, filter, callback) {
self.setIdValue(model, o, o._id);
}
// Don't pass back _id if the fields is set
- if (fields) {
+ if (fields || idName != '_id') {
delete o._id;
}
return o;
@@ -505,8 +513,8 @@ MongoDB.prototype.updateAttributes = function updateAttrs(model, id, data, cb) {
console.log('updateAttributes', model, id, data);
}
var oid = ObjectID(id);
- delete data[this.idName(model)];
-
+ var idName = this.idName(model);
+ delete data[idName];
this.collection(model).findAndModify({_id: oid}, [
['_id', 'asc']
], {$set: data}, {}, function (err, object) {
@@ -518,6 +526,7 @@ MongoDB.prototype.updateAttributes = function updateAttrs(model, id, data, cb) {
err = 'No ' + model + ' found for id ' + id;
}
self.setIdValue(model, object, id);
+ object && idName != '_id' && delete object._id;
cb && cb(err, object);
});
};
View
188 test/mongodb.test.js
@@ -25,22 +25,151 @@ describe('mongodb', function () {
content: { type: String }
});
+ PostWithObjectId = db.define('PostWithObjectId', {
+ _id: {type: db.ObjectID, id: true},
+ title: { type: String, length: 255, index: true },
+ 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);
});
beforeEach(function (done) {
User.destroyAll(function () {
Post.destroyAll(function () {
+ PostWithObjectId.destroyAll(function () {
+ PostWithNumberId.destroyAll(function () {
+ done();
+ });
+ });
+ });
+ });
+ });
+
+ 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 handle correctly type Number for id field _id', function (done) {
+ PostWithNumberId.create({_id: 3, content: "test"}, function (err, person) {
+ should.not.exist(err);
+ 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();
+ });
+ });
+ });
+
+ it('should allow to find post by id string if `_id` is defined id', function (done) {
+ PostWithObjectId.create(function (err, post) {
+ PostWithObjectId.find(post._id.toString(), function (err, p) {
+ should.not.exist(err);
+ post = p[0];
+ should.exist(post);
+ post._id.should.be.an.instanceOf(db.ObjectID);
+
done();
});
});
});
+ it('find with `_id` as defined id should return an object with _id instanceof ObjectID', function (done) {
+ PostWithObjectId.create(function (err, post) {
+ PostWithObjectId.findById(post._id, function (err, post) {
+ should.not.exist(err);
+ post._id.should.be.an.instanceOf(db.ObjectID);
+
+ done();
+ });
+
+ });
+ });
+
+ it('should update the instance with `_id` as defined id', function (done) {
+ PostWithObjectId.create({title: 'a', content: 'AAA'}, function (err, post) {
+ post.title = 'b';
+ PostWithObjectId.updateOrCreate(post, function (err, p) {
+ should.not.exist(err);
+ p._id.should.be.equal(post._id);
+
+ PostWithObjectId.findById(post._id, function (err, p) {
+ should.not.exist(err);
+ p._id.should.be.eql(post._id);
+ p.content.should.be.equal(post.content);
+ p.title.should.be.equal('b');
+ });
+
+ PostWithObjectId.find({where: {title: 'b'}}, function (err, posts) {
+ should.not.exist(err);
+ p = posts[0];
+ p._id.should.be.eql(post._id);
+ p.content.should.be.equal(post.content);
+ p.title.should.be.equal('b');
+ posts.should.have.lengthOf(1);
+ done();
+ });
+ });
+
+ });
+ });
+
+ it('all should return object (with `_id` as defined id) with an _id instanceof ObjectID', function (done) {
+ var post = new PostWithObjectId({title: 'a', content: 'AAA'})
+ post.save(function (err, post) {
+ PostWithObjectId.all({where: {title: 'a'}}, function (err, posts) {
+ should.not.exist(err);
+ posts.should.have.lengthOf(1);
+ post = posts[0];
+ post.should.have.property('title', 'a');
+ post.should.have.property('content', 'AAA');
+ post._id.should.be.an.instanceOf(db.ObjectID);
+
+ done();
+ });
+
+ });
+ });
+
+ it('all return should honor filter.fields, with `_id` as defined id', function (done) {
+ var post = new PostWithObjectId({title: 'a', content: 'AAA'})
+ post.save(function (err, post) {
+ PostWithObjectId.all({fields: ['title'], where: {title: 'a'}}, function (err, posts) {
+ should.not.exist(err);
+ posts.should.have.lengthOf(1);
+ post = posts[0];
+ post.should.have.property('title', 'a');
+ post.should.not.have.property('content');
+ should.not.exist(post._id);
+
+ done();
+ });
+
+ });
+ });
+
+
+
it('hasMany should support additional conditions', function (done) {
User.create(function (e, u) {
u.posts.create({}, function (e, p) {
- u.posts({where: {_id: p.id}}, function (err, posts) {
+ u.posts({where: {id: p.id}}, function (err, posts) {
should.not.exist(err);
posts.should.have.lengthOf(1);
@@ -50,23 +179,51 @@ describe('mongodb', function () {
});
});
+
+
+ it('create should return id field but not mongodb _id', function (done) {
+ Post.create(function (err, post) {
+ //console.log('create should', err, post);
+ should.not.exist(err);
+ should.exist(post.id);
+ should.not.exist(post._id);
+
+ done();
+ });
+ });
+
it('should allow to find by id string', function (done) {
Post.create(function (err, post) {
- Post.find(post.id.toString(), function (err, post) {
+ Post.find(post.id.toString(), function (err, p) {
should.not.exist(err);
- should.exist(post);
+ should.exist(p);
done();
});
});
});
- it('find should return an object with an id, which is instanceof ObjectId', function (done) {
+
+ it('save should not return mongodb _id', function (done) {
+ Post.create(function (err, post) {
+ post.content = 'AAA';
+ post.save(function(err, p) {
+ should.not.exist(err)
+ should.not.exist(p._id);
+ p.id.should.be.equal(post.id);
+ p.content.should.be.equal('AAA')
+
+ done();
+ });
+ });
+ });
+
+ it('find should return an object with an id, which is instanceof ObjectID, but not mongodb _id', function (done) {
Post.create(function (err, post) {
Post.findById(post.id, function (err, post) {
should.not.exist(err);
post.id.should.be.an.instanceOf(db.ObjectID);
- post._id.should.be.an.instanceOf(db.ObjectID);
+ should.not.exist(post._id);
done();
});
@@ -81,19 +238,22 @@ describe('mongodb', function () {
should.not.exist(err);
p.id.should.be.equal(post.id);
p.content.should.be.equal(post.content);
+ should.not.exist(p._id);
Post.findById(post.id, function (err, p) {
p.id.should.be.equal(post.id);
+ should.not.exist(p._id);
p.content.should.be.equal(post.content);
p.title.should.be.equal('b');
+
done();
});
});
});
});
- it('all should return object with an id, which is instanceof ObjectID', function (done) {
+ it('all should return object with an id, which is instanceof ObjectID, but not mongodb _id', function (done) {
var post = new Post({title: 'a', content: 'AAA'})
post.save(function (err, post) {
Post.all({where: {title: 'a'}}, function (err, posts) {
@@ -103,15 +263,15 @@ describe('mongodb', function () {
post.should.have.property('title', 'a');
post.should.have.property('content', 'AAA');
post.id.should.be.an.instanceOf(db.ObjectID);
- post._id.should.be.an.instanceOf(db.ObjectID);
+ should.not.exist(post._id);
done();
});
});
});
- it('all should return honor filter.fields', function (done) {
+ it('all return should honor filter.fields', function (done) {
var post = new Post({title: 'b', content: 'BBB'})
post.save(function (err, post) {
Post.all({fields: ['title'], where: {title: 'b'}}, function (err, posts) {
@@ -120,6 +280,9 @@ describe('mongodb', function () {
post = posts[0];
post.should.have.property('title', 'b');
post.should.not.have.property('content');
+ should.not.exist(post._id);
+ should.not.exist(post.id);
+
done();
});
@@ -132,16 +295,21 @@ describe('mongodb', function () {
PostWithStringId.create({id: oid, title: 'c', content: 'CCC'}, function (err, post) {
PostWithStringId.findById(oid, function (err, post) {
should.not.exist(err);
+ should.not.exist(post._id);
post.id.should.be.equal(oid);
+
done();
});
});
});
after(function (done) {
User.destroyAll(function () {
- Post.destroyAll(done);
+ Post.destroyAll(function () {
+ PostWithObjectId.destroyAll(function () {
+ PostWithNumberId.destroyAll(done);
+ });
+ });
});
});
});
-
Something went wrong with that request. Please try again.