Skip to content

Commit

Permalink
fix(model): allow bulkWrite upsert with empty update
Browse files Browse the repository at this point in the history
Re: #8698
  • Loading branch information
vkarpov15 committed Mar 27, 2020
1 parent 6c66b06 commit 7c2d74e
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 24 deletions.
1 change: 1 addition & 0 deletions lib/helpers/model/castBulkWrite.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ module.exports = function castBulkWrite(model, op, options) {
strict: model.schema.options.strict,
overwrite: false
});

if (op['updateOne'].setDefaultsOnInsert) {
setDefaultsOnInsert(op['updateOne']['filter'], model.schema, op['updateOne']['update'], {
setDefaultsOnInsert: true,
Expand Down
17 changes: 2 additions & 15 deletions lib/helpers/query/castUpdate.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ module.exports = function castUpdate(schema, obj, options, context, filter) {
const ops = Object.keys(obj);
let i = ops.length;
const ret = {};
let hasKeys;
let val;
let hasDollarKey = false;
const overwrite = options.overwrite;
Expand Down Expand Up @@ -81,12 +80,6 @@ module.exports = function castUpdate(schema, obj, options, context, filter) {
// cast each value
i = ops.length;

// if we get passed {} for the update, we still need to respect that when it
// is an overwrite scenario
if (overwrite) {
hasKeys = true;
}

while (i--) {
const op = ops[i];
val = ret[op];
Expand All @@ -96,14 +89,8 @@ module.exports = function castUpdate(schema, obj, options, context, filter) {
typeof val === 'object' &&
!Buffer.isBuffer(val) &&
(!overwrite || hasDollarKey)) {
hasKeys |= walkUpdatePath(schema, val, op, options, context, filter);
walkUpdatePath(schema, val, op, options, context, filter);
} else if (overwrite && ret && typeof ret === 'object') {
// if we are just using overwrite, cast the query and then we will
// *always* return the value, even if it is an empty object. We need to
// set hasKeys above because we need to account for the case where the
// user passes {} and wants to clobber the whole document
// Also, _walkUpdatePath expects an operation, so give it $set since that
// is basically what we're doing
walkUpdatePath(schema, ret, '$set', options, context, filter);
} else {
const msg = 'Invalid atomic update value for ' + op + '. '
Expand All @@ -116,7 +103,7 @@ module.exports = function castUpdate(schema, obj, options, context, filter) {
}
}

return hasKeys && ret;
return ret;
};

/*!
Expand Down
2 changes: 1 addition & 1 deletion lib/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -3472,7 +3472,7 @@ Query.prototype._findAndModify = function(type, callback) {
if (!isOverwriting) {
this._update = castDoc(this, opts.overwrite);
this._update = setDefaultsOnInsert(this._conditions, schema, this._update, opts);
if (!this._update) {
if (!this._update || Object.keys(this._update).length === 0) {
if (opts.upsert) {
// still need to do the upsert to empty doc
const doc = utils.clone(castedQuery);
Expand Down
22 changes: 22 additions & 0 deletions test/model.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6474,4 +6474,26 @@ describe('Model', function() {
assert.strictEqual(res.breed, 'Chorkie');
});
});

it('bulkWrite upsert works when update casts to empty (gh-8698)', function() {
const userSchema = new Schema({
name: String
});
const User = db.model('User', userSchema);
mongoose.set('debug', true)

return co(function*() {
yield User.bulkWrite([{
updateOne: {
filter: { name: 'test' },
update: { notInSchema: true },
upsert: true
}
}]);

const doc = yield User.findOne();
assert.ok(doc);
assert.strictEqual(doc.notInSchema, null);
});
});
});
8 changes: 0 additions & 8 deletions test/query.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1645,14 +1645,6 @@ describe('Query', function() {
assert.strictEqual(opts.maxTimeMS, 1000);
});

describe('update', function() {
it('when empty, nothing is run', function(done) {
const q = new Query;
assert.equal(false, !!q._castUpdate({}));
done();
});
});

describe('bug fixes', function() {
describe('collations', function() {
before(function(done) {
Expand Down

0 comments on commit 7c2d74e

Please sign in to comment.