Fixing tests in mongo livedata and observe changes

packages/mongo/collection.js

@@ -1,6 +1,8 @@
 // options.connection, if given, is a LivedataClient or LivedataServer
 // XXX presently there is no way to destroy/clean up a Collection
 
+import { normalizeProjection } from "./mongo_utils";
+
 /**
  * @summary Namespace for MongoDB-related items
  * @namespace
@@ -320,15 +322,18 @@ Object.assign(Mongo.Collection.prototype, {
   },
 
   _getFindOptions(args) {
+    const [, options] = args || [];
+    const newOptions = normalizeProjection(options);
+
     var self = this;
     if (args.length < 2) {
       return { transform: self._transform };
     } else {
       check(
-        args[1],
+        newOptions,
         Match.Optional(
           Match.ObjectIncluding({
-            fields: Match.Optional(Match.OneOf(Object, undefined)),
+            projection: Match.Optional(Match.OneOf(Object, undefined)),
             sort: Match.Optional(
               Match.OneOf(Object, Array, Function, undefined)
             ),
@@ -338,9 +343,17 @@ Object.assign(Mongo.Collection.prototype, {
         )
       );
 
+      const { projection } = newOptions;
+
+      // this error: "Cannot do exclusion on field _id in inclusion projection"
+      // happens on MongoDB CLI but doesn't happen in the Node.js Driver for MongoDB 5.0+
+      if (projection && projection._id != null && !projection._id) {
+        throw new Error(`Cannot do exclusion on field _id in inclusion projection, collectionName=${self._name}`);
+      }
+
       return {
         transform: self._transform,
-        ...args[1],
+        ...newOptions,
       };
     }
   },

packages/mongo/mongo_driver.js

@@ -1,3 +1,5 @@
+import { normalizeProjection } from "./mongo_utils";
+
 /**
  * Provide a synchronous Collection API using fibers, backed by
  * MongoDB.  This is only for use on the server, and mostly identical
@@ -422,15 +424,15 @@ MongoConnection.prototype._remove = function (collection_name, selector,
 
   try {
     var collection = self.rawCollection(collection_name);
-    const result = {};
+    let result = {};
     try {
       const {deletedCount} = collection.deleteMany(replaceTypes(selector, replaceMeteorAtomWithMongo),
           {safe: true}).await();
-      result.numberAffected = deletedCount;
+      result.modifiedCount = deletedCount;
     }catch(err){
       callback(err)
     }
-    callback(null, result)
+    callback(null, transformResult({result}).numberAffected)
   } catch (err) {
     write.committed();
     throw err;
@@ -642,6 +644,8 @@ MongoConnection.prototype._update = function (collection_name, selector, mod,
 };
 
 var transformResult = function (driverResult) {
+  console.log(`driverResult`, driverResult);
+
   var meteorResult = { numberAffected: 0 };
   if (driverResult) {
     var mongoResult = driverResult.result;
@@ -655,7 +659,9 @@ var transformResult = function (driverResult) {
         meteorResult.insertedId = mongoResult.upsertedId;
       }
     } else {
-      meteorResult.numberAffected = mongoResult.n;
+      // n was used before Mongo 5.0, in Mongo 5.0 we are not receiving this n
+      // field and so we are using modifiedCount instead
+      meteorResult.numberAffected = mongoResult.n || mongoResult.modifiedCount;
     }
   }
 
@@ -875,12 +881,6 @@ CursorDescription = function (collectionName, selector, options) {
   self.collectionName = collectionName;
   self.selector = Mongo.Collection._rewriteSelector(selector);
   self.options = options || {};
-  // transform fields key in projection
-  const { fields, projection, ...otherOptions } = self.options;
-  // TODO: enable this comment when deprecating the fields option
-  // Log.debug(`fields option has been deprecated, please use the new 'projection' instead`)
-
-  self.options = { ...otherOptions, ...(projection || fields ? {projection: fields || projection} : {}) };
 };
 
 Cursor = function (mongo, cursorDescription) {

packages/mongo/mongo_livedata_tests.js

@@ -357,6 +357,8 @@ Tinytest.addAsync("mongo-livedata - basics, " + idGeneration, function (test, on
 
   expectObserve('', function () {
     var count = coll.update({run: run, x: -1}, {$inc: {x: 2}}, {multi: true});
+    console.log(`count`, count);
+
     test.equal(count, 0);
   });
 
@@ -563,7 +565,7 @@ if (Meteor.isServer) {
     function error() {
       throw new Meteor.Error('unsafe object mutation');
     }
-    
+
     const denyModifications = {
       get(target, key) {
         const type = Object.prototype.toString.call(target[key]);
@@ -577,7 +579,7 @@ if (Meteor.isServer) {
       deleteProperty: error,
       defineProperty: error,
     };
-    
+
     // Object.freeze only throws in silent mode
     // So we make our own version that always throws.
     function freeze(obj) {
@@ -589,7 +591,7 @@ if (Meteor.isServer) {
       // Make sure that if anything touches the original object, this will throw
       return origApplyCallback.call(this, callback, freeze(args));
     }
-    
+
     const run = test.runId();
     const coll = new Mongo.Collection(`livedata_test_scribble_collection_${run}`, collectionOptions);
     const expectMutatable = (o) => {
@@ -623,7 +625,7 @@ if (Meteor.isServer) {
       coll.insert({run, a: [ {c: 1} ]});
       coll.update({run}, { $set: { 'a.0.c': 2 } });
     });
- 
+
     handle.stop();
     handle2.stop();
 
@@ -720,6 +722,8 @@ if (Meteor.isServer) {
       var output = [];
       var callbacks = {
         changed: function (newDoc) {
+          console.log(`newDoc`, newDoc);
+
           output.push({changed: newDoc._id});
         }
       };
@@ -764,11 +768,16 @@ if (Meteor.isServer) {
     test.isTrue(observeMultiplexer);
     test.isTrue(observeMultiplexer === o2.handle._multiplexer);
 
+    console.log('runInFence filipe')
+    console.log(`o1.output`, o1.output);
+
     // Update. Both observes fire.
     runInFence(function () {
       coll.update(docId1, {$set: {x: 'y'}});
     });
-    test.length(o1.output, 1);
+    console.log('runInFence filipe 2')
+    console.log(`o1.output `, o1.output);
+    test.length(o1.output, 1, 'filipe');
     test.length(o2.output, 1);
     test.equal(o1.output.shift(), {changed: docId1});
     test.equal(o2.output.shift(), {changed: docId1});
@@ -2231,7 +2240,7 @@ _.each(Meteor.isServer ? [true, false] : [true], function (minimongo) {
 });  // end idGeneration parametrization
 
 Tinytest.add('mongo-livedata - rewrite selector', function (test) {
- 
+
   test.equal(Mongo.Collection._rewriteSelector('foo'),
              {_id: 'foo'});
 

packages/mongo/mongo_utils.js

@@ -0,0 +1,11 @@
+export const normalizeProjection = options => {
+  // transform fields key in projection
+  const { fields, projection, ...otherOptions } = options || {};
+  // TODO: enable this comment when deprecating the fields option
+  // Log.debug(`fields option has been deprecated, please use the new 'projection' instead`)
+
+  return {
+    ...otherOptions,
+    ...(projection || fields ? { projection: fields || projection } : {}),
+  };
+};

packages/mongo/observe_changes_tests.js

@@ -48,17 +48,16 @@ _.each ([{added: 'added', forceOrdered: true},
 
     handle.stop();
 
-    var badCursor = c.find({}, {fields: {noodles: 1, _id: false}});
     test.throws(function () {
-      badCursor.observeChanges(logger);
-    });
+      c.find({}, {fields: {noodles: 1, _id: false}})
+    }, undefined, 'bad cursor excluding _id from projection');
 
     onComplete();
     });
   });
 });
 
-Tinytest.addAsync("observeChanges - callback isolation", function (test, onComplete) {
+Tinytest.onlyAsync("observeChanges - callback isolation", function (test, onComplete) {
   var c = makeCollection();
   withCallbackLogger(test, ["added", "changed", "removed"], Meteor.isServer, function (logger) {
     var handles = [];
@@ -420,5 +419,5 @@ if (Meteor.isServer) {
       });
       c.insert({ type: { name: 'foobar' } });
     }
-  );  
+  );
 }

packages/tinytest/tinytest.js

@@ -203,7 +203,7 @@ export class TestCaseResults {
   // The upshot is, if you want to test whether an error is of a
   // particular class, use a predicate function.
   //
-  throws(f, expected) {
+  throws(f, expected, message) {
     var actual, predicate;
 
     if (expected === undefined) {
@@ -236,9 +236,9 @@ export class TestCaseResults {
     else
       this.fail({
         type: "throws",
-        message: actual ?
+        message: (actual ?
           "wrong error thrown: " + actual.message :
-          "did not throw an error as expected"
+          "did not throw an error as expected") + (message ? ": " + message : ""),
       });
   }