fix(updateOne/updateMany): ensure that update documents contain atomic operators

NODE-965

Author: kmahar

lib/collection.js

@@ -938,7 +938,15 @@ define.classMethod('insert', { callback: true, promise: true });
  */
 Collection.prototype.updateOne = function(filter, update, options, callback) {
   var self = this;
+
   if (typeof options === 'function') (callback = options), (options = {});
+
+  var err = checkForAtomicOperators(update);
+  if (err) {
+    if (typeof callback === 'function') return callback(err);
+    return this.s.promiseLibrary.reject(err);
+  }
+
   options = shallowClone(options);
 
   // Add ignoreUndfined
@@ -959,6 +967,19 @@ Collection.prototype.updateOne = function(filter, update, options, callback) {
   });
 };
 
+var checkForAtomicOperators = function(update) {
+  var keys = Object.keys(update);
+
+  // same errors as the server would give for update doc lacking atomic operators
+  if (keys.length === 0) {
+    return toError('The update operation document must contain at least one atomic operator.');
+  }
+
+  if (keys[0][0] !== '$') {
+    return toError('the update operation document must contain atomic operators.');
+  }
+};
+
 var updateOne = function(self, filter, update, options, callback) {
   // Set single document update
   options.multi = false;
@@ -1061,6 +1082,13 @@ define.classMethod('replaceOne', { callback: true, promise: true });
 Collection.prototype.updateMany = function(filter, update, options, callback) {
   var self = this;
   if (typeof options === 'function') (callback = options), (options = {});
+
+  var err = checkForAtomicOperators(update);
+  if (err) {
+    if (typeof callback === 'function') return callback(err);
+    return this.s.promiseLibrary.reject(err);
+  }
+
   options = shallowClone(options);
 
   // Add ignoreUndfined

test/functional/crud_api_tests.js

@@ -1,6 +1,7 @@
 'use strict';
-var test = require('./shared').assert;
-var setupDatabase = require('./shared').setupDatabase;
+var test = require('./shared').assert,
+  setupDatabase = require('./shared').setupDatabase,
+  expect = require('chai').expect;
 
 // instanceof cannot be use reliably to detect the new models in js due to scoping and new
 // contexts killing class info find/distinct/count thus cannot be overloaded without breaking
@@ -1011,4 +1012,81 @@ describe('CRUD API', function() {
       });
     }
   });
+
+  it('should correctly throw error if update doc for updateOne lacks atomic operator', {
+    // Add a tag that our runner can trigger on
+    // in this case we are setting that node needs to be higher than 0.10.X to run
+    metadata: {
+      requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] }
+    },
+
+    // The actual test we wish to run
+    test: function(done) {
+      var configuration = this.configuration;
+      var client = configuration.newClient(configuration.writeConcernMax(), { poolSize: 1 });
+      client.connect(function(err, client) {
+        expect(err).to.not.exist;
+        var db = client.db(configuration.db);
+        var col = db.collection('t21_1');
+        col.insertOne({ a: 1, b: 2, c: 3 }, function(err, r) {
+          expect(err).to.not.exist;
+          expect(r.insertedCount).to.equal(1);
+
+          // empty update document
+          col.updateOne({ a: 1 }, {}, function(err, r) {
+            expect(err).to.exist;
+            expect(r).to.not.exist;
+
+            // update document non empty but still lacks atomic operator
+            col.updateOne({ a: 1 }, { b: 5 }, function(err, r) {
+              expect(err).to.exist;
+              expect(r).to.not.exist;
+
+              client.close();
+              done();
+            });
+          });
+        });
+      });
+    }
+  });
+
+  it('should correctly throw error if update doc for updateMany lacks atomic operator', {
+    // Add a tag that our runner can trigger on
+    // in this case we are setting that node needs to be higher than 0.10.X to run
+    metadata: {
+      requires: { topology: ['single', 'replicaset', 'sharded', 'ssl', 'heap', 'wiredtiger'] }
+    },
+
+    // The actual test we wish to run
+    test: function(done) {
+      var configuration = this.configuration;
+      var client = configuration.newClient(configuration.writeConcernMax(), { poolSize: 1 });
+      client.connect(function(err, client) {
+        expect(err).to.not.exist;
+        var db = client.db(configuration.db);
+        var col = db.collection('t22_1');
+        col.insertMany([{ a: 1, b: 2 }, { a: 1, b: 3 }, { a: 1, b: 4 }], function(err, r) {
+          console.dir(err);
+          expect(err).to.not.exist;
+          expect(r.insertedCount).to.equal(3);
+
+          // empty update document
+          col.updateMany({ a: 1 }, {}, function(err, r) {
+            expect(err).to.exist;
+            expect(r).to.not.exist;
+
+            // update document non empty but still lacks atomic operator
+            col.updateMany({ a: 1 }, { b: 5 }, function(err, r) {
+              expect(err).to.exist;
+              expect(r).to.not.exist;
+
+              client.close();
+              done();
+            });
+          });
+        });
+      });
+    }
+  });
 });

test/functional/examples_tests.js

@@ -1035,8 +1035,16 @@ describe('Examples', function() {
               .updateOne(
                 { item: 'paper' },
                 {
-                  item: 'paper',
-                  instock: [{ warehouse: 'A', qty: 60 }, { warehouse: 'B', qty: 40 }]
+                  $set: {
+                    item: 'paper',
+                    instock: [{ warehouse: 'A', qty: 60 }, { warehouse: 'B', qty: 40 }]
+                  },
+                  $unset: {
+                    qty: '',
+                    size: '',
+                    status: '',
+                    lastModified: ''
+                  }
                 }
               )
               .then(function(result) {

test/functional/operation_example_tests.js

@@ -3531,7 +3531,7 @@ describe('Operation Examples', function() {
         // Get a collection
         var collection = db.collection('update_a_simple_document_upsert');
         // Update the document using an upsert operation, ensuring creation if it does not exist
-        collection.updateOne({ a: 1 }, { b: 2, a: 1 }, { upsert: true, w: 1 }, function(
+        collection.updateOne({ a: 1 }, { $set: { b: 2, a: 1 } }, { upsert: true, w: 1 }, function(
           err,
           result
         ) {
@@ -6777,7 +6777,7 @@ describe('Operation Examples', function() {
 
           db
             .collection('mongoclient_test')
-            .updateOne({ a: 1 }, { b: 1 }, { upsert: true }, function(err, result) {
+            .updateOne({ a: 1 }, { $set: { b: 1 } }, { upsert: true }, function(err, result) {
               test.equal(null, err);
               test.equal(1, result.result.n);
 

test/functional/operation_generators_example_tests.js

@@ -2697,7 +2697,11 @@ describe('Operation (Generators)', function() {
         // Get a collection
         var collection = db.collection('update_a_simple_document_upsert_with_generators');
         // Update the document using an upsert operation, ensuring creation if it does not exist
-        var result = yield collection.updateOne({ a: 1 }, { b: 2, a: 1 }, { upsert: true, w: 1 });
+        var result = yield collection.updateOne(
+          { a: 1 },
+          { $set: { b: 2, a: 1 } },
+          { upsert: true, w: 1 }
+        );
         test.equal(1, result.result.n);
 
         // Fetch the document that we modified and check if it got inserted correctly

test/functional/operation_promises_example_tests.js

@@ -2789,7 +2789,7 @@ describe('Operation (Promises)', function() {
 
         // Update the document using an upsert operation, ensuring creation if it does not exist
         return collection
-          .updateOne({ a: 1 }, { b: 2, a: 1 }, { upsert: true, w: 1 })
+          .updateOne({ a: 1 }, { $set: { b: 2, a: 1 } }, { upsert: true, w: 1 })
           .then(function(result) {
             test.equal(1, result.result.n);
 
@@ -5077,7 +5077,7 @@ describe('Operation (Promises)', function() {
         // BEGIN
         return db
           .collection('mongoclient_test_with_promise')
-          .updateOne({ a: 1 }, { b: 1 }, { upsert: true })
+          .updateOne({ a: 1 }, { $set: { b: 1 } }, { upsert: true })
           .then(function(result) {
             test.equal(1, result.result.n);
             client.close();