Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions lib/mongodb.js
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,25 @@ MongoDB.prototype.updateOrCreate = function updateOrCreate(model, data, options,
});
};

/**
* Replace model instance if it exists or create a new one if it doesn't
*
* @param {String} model The name of the model
* @param {Object} data The model instance data
* @param {Object} options The options object
* @param {Function} [cb] The callback function
*/
MongoDB.prototype.replaceOrCreate = function(model, data, options, cb) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

model -> modelName is more descriptive

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Simon, I applied all of your feedback except changing model to modelName in a few places you mentioned; this change makes the code inconsistent with the rest of the code; in other words, in other methods others are using model as well and refactoring the other code written by others seems huge changes...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. The rename can be done in a separate commit/PR.

if (this.debug) debug('replaceOrCreate', model, data);

var id = this.getIdValue(model, data);
var oid = this.coerceId(model, id);
var idName = this.idName(model);
data._id = data[idName];
delete data[idName];
this.replaceWithOptions(model, oid, data, {upsert: true}, cb);
};

/**
* Delete a model instance by id
* @param {String} model The model name
Expand Down Expand Up @@ -812,6 +831,59 @@ MongoDB.prototype.count = function count(model, where, options, callback) {
});
};

/**
* Replace properties for the model instance data
* @param {String} model The name of the model
* @param {*} id The instance id
* @param {Object} data The model data
* @param {Object} options The options object
* @param {Function} [cb] The callback function
*/
MongoDB.prototype.replaceById = function replace(model, id, data, options, cb) {
if (this.debug) debug('replace', model, id, data);
var oid = this.coerceId(model, id);
this.replaceWithOptions(model, oid, data, {upsert: false}, function(err, data) {
cb(err, data);
});
};

/**
* Update a model instance with id
* @param {String} model The name of the model
* @param {Object} id The id of the model instance
* @param {Object} data The property/value pairs to be updated or inserted if {upsert: true} is passed as options
* @param {Object} options The options you want to pass for update, e.g, {upsert: true}
* @callback {Function} [cb] Callback function
*/
MongoDB.prototype.replaceWithOptions = function(model, id, data, options, cb) {
var self = this;
this.execute(model, 'update', {_id: id}, data, options, function(err, info) {
if (this.debug) debug('updateWithOptions.callback', model, {_id: id}, data, err, info);
if (err) return cb && cb(err);
var result;
var cbInfo = {};
if (info.result && info.result.n == 1) {
result = data;
delete result._id;
var idName = self.idName(model);
result[idName] = id;
// create result formats:
// 2.4.x :{ ok: 1, n: 1, upserted: [ Object ] }
// { ok: 1, nModified: 0, n: 1, upserted: [ Object ] }
//
// replace result formats:
// 2.4.x: { ok: 1, n: 1 }
// { ok: 1, nModified: 1, n: 1 }
if (info.result.nModified !== undefined) {
cbInfo.isNewInstance = info.result.nModified === 0;
}
} else {
result = undefined;
}
cb && cb(err, result, cbInfo);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@raymondfeng: I had forgot to add this cbInfo which is used for replaceOrCreate to see if it is a isNewInstance or not in juggler for after save operation hook...
CC: @bajtos

});
};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation of replaceAttributes and replaceOrCreate look mostly identical to me. Is there a way how to extract shared code to a single shared method?


/**
* Update properties for the model instance data
* @param {String} model The model name
Expand Down
107 changes: 107 additions & 0 deletions test/mongodb.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1072,6 +1072,113 @@ describe('mongodb connector', function () {
});
});

describe('replaceOrCreate', function() {
it('should create a model instance even if it already exists', function(done) {
Product.replaceOrCreate({name: 'newFoo'}, function(err, updatedProduct) {
if (err) return done(err);
should.not.exist(updatedProduct._id);
should.exist(updatedProduct.id);
verifyData(updatedProduct.id);
});
function verifyData(id) {
Product.findById(id, function(err, data) {
data.name.should.be.equal('newFoo');
done(err);
});
};
});

it('should replace a model instance if the passing key already exists', function(done) {
Product.create({name: 'bread', price: 100}, function(err, product) {
if (err) return done(err);
replaceOrCreate({id: product.id, name: 'milk'});
});
function replaceOrCreate(data) {
Product.replaceOrCreate(data, function(err, updatedProduct) {
if (err) return done(err);
should.not.exist(updatedProduct._id);
updatedProduct.name.should.be.equal('milk');
should.exist(updatedProduct.id);
verify(data.id);
});
}
function verify(id) {
Product.findById(id, function(err, data) {
data.name.should.be.equal('milk');
should.not.exist(data.price);
done(err);
});
}
});

it('should remove extraneous properties that are not defined in the model', function(done) {
Product.create({name: 'bread', price: 100, bar: 'baz'}, function(err, product) {
if (err) return done(err);
replaceOrCreate({id: product.id, name: 'milk'});
});
function replaceOrCreate(data) {
Product.replaceOrCreate(data, function(err, updatedProduct) {
if (err) return done(err);
should.not.exist(updatedProduct.bar);
verify(data.id);
});
}
function verify(id) {
Product.findById(id, function(err, data) {
should.not.exist(data.bar);
done(err);
});
}
});
});

describe('replace', function() {
it('should replace the model instance if the provided key already exists', function(done) {
Product.create({name: 'bread', price: 100}, function(err, product) {
if (err) return done(err);
replace(product, {name: 'milk'}, product.id);
});
function replace(product, data, id) {
product.replaceAttributes(data, function(err, updatedProduct) {
if (err) return done(err);
should.not.exist(updatedProduct._id);
updatedProduct.name.should.be.equal('milk');
should.exist(updatedProduct.id);
verify(id);
});
}
function verify(id) {
Product.findById(id, function(err, data) {
data.name.should.be.equal('milk');
should.not.exist(data.price);
done(err);
});
}
});

it('should remove extraneous properties that are not defined in the model', function(done) {
Product.create({name: 'bread', price: 100, bar: 'baz'}, function(err, product) {
if (err) return done(err);
replace(product, {name: 'milk'}, product.id);

});
function replace(product, data, id) {
product.replaceAttributes(data, function(err, updatedProduct) {
if (err) return done(err);
should.not.exist(updatedProduct.bar);
verify(id);
});
}
function verify(id) {
Product.findById(id, function(err, data) {
data.name.should.be.equal('milk');
should.not.exist(data.bar);
done(err);
});
}
});
});

it('updateOrCreate: should handle combination of operators and top level properties without errors', function (done) {
Product.dataSource.settings.allowExtendedOperators = true;
Product.create({name: 'bread', price: 100, ingredients:['flour'],pricehistory:[{'2014-11-11':90},{ '2014-10-10':80 }]}, function (err, product) {
Expand Down
13 changes: 12 additions & 1 deletion test/persistence-hooks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,16 @@ if (process.env.MONGODB_VERSION &&
for(var i in config) {
customConfig[i] = config[i];
}
var DB_VERSION = process.env.MONGODB_VERSION;

suite(global.getDataSource(customConfig), should);
if (!DB_VERSION) {
console.log('The ENV variable MONGODB_VERSION is not set.'
+ ' Assuming MongoDB version 2.6 or newer.');
}

var DB_HAS_2_6_FEATURES = (!DB_VERSION ||

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bajtos: Please note: if you do npm test locally we assume you have mongodb 2.6+, which supports isNewInstance

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a console.log to let the users know about this fact?

For example:

if (!DB_VERSION) {
  console.log('The ENV variable MONGODB_VERSION is not set. Assuming MongoDB version 2.6 or newer.');
}

semver.satisfies(DB_VERSION, '>=2.6.0'));

suite(global.getDataSource(customConfig), should, {
replaceOrCreateReportsNewInstance: DB_HAS_2_6_FEATURES
});