feat(Update): add the ability to specify a pipeline to an update command #2017
Conversation
lib/collection.js
Outdated
| @@ -686,7 +686,10 @@ Collection.prototype.updateOne = function(filter, update, options, callback) { | |||
| if (typeof options === 'function') (callback = options), (options = {}); | |||
| options = options || {}; | |||
|
|
|||
| const err = checkForAtomicOperators(update); | |||
| const err = Array.isArray(update) | |||
| ? update.forEach(checkForAtomicOperators) | |||
There was a problem hiding this comment.
from what I can tell, this is not going to return an err, as Array.prototype.forEach returns undefined.
Is the goal here to get the first checkForAtomicOperators error on the pipeline?
There was a problem hiding this comment.
Yep, I believe that we want the first checkForAtomicOperators error in the pipeline.
There was a problem hiding this comment.
In that case you could build this up imperatively:
let err;
if (Array.isArray(update)) {
for (let i = 0; !err && i < update.length; i++) {
err = checkForAtomicOperators(update);
}
} else {
err = checkForAtomicOperators(update);
}
test/functional/collection_tests.js
Outdated
| it('should correctly update with pipeline', { | ||
| metadata: { | ||
| requires: { | ||
| topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'], |
There was a problem hiding this comment.
AFAIK we don't use the heap and wiredtiger environments anymore.
There was a problem hiding this comment.
We can handle this by remove this entire line, which will cause this test to run for every topology that we run them against.
test/functional/collection_tests.js
Outdated
| it('should correctly update with pipeline', { | ||
| metadata: { | ||
| requires: { | ||
| topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'], |
There was a problem hiding this comment.
We can handle this by remove this entire line, which will cause this test to run for every topology that we run them against.
test/functional/collection_tests.js
Outdated
| [{ $set: { a: 1 } }, { $set: { b: 1 } }, { $set: { d: 1 } }], | ||
| configuration.writeConcernMax(), | ||
| (err, r) => { | ||
| expect(err).to.equal(null); |
There was a problem hiding this comment.
We can use expect(err).to.not.exist.
| configuration.writeConcernMax(), | ||
| (err, r) => { | ||
| expect(err).to.not.exist; | ||
| expect(r.result.n).to.equal(0); |
There was a problem hiding this comment.
Why do we expect this to be 0?
…and (mongodb#2017) * feat(Update): add the ability to specify a pipeline to an update command NODE-1920
…and (mongodb#2017) * feat(Update): add the ability to specify a pipeline to an update command NODE-1920
…and (mongodb#2017) * feat(Update): add the ability to specify a pipeline to an update command NODE-1920
Fixes NODE-1920
Description
This provides extended functionality for the
updatecommand by allowing the specification of a pipeline.updatecan now take in an object or an array.What changed?
To allow the user to pass in an array to
updatein addition to preserving the functionality of providing an object, the corresponding argument in theupdateOnefunction was modified to check for atomic operators on each element in the array, if passed in as such.