diff --git a/packages/mongo/collection.js b/packages/mongo/collection.js index eca28116c5..44c1fdfcad 100644 --- a/packages/mongo/collection.js +++ b/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, }; } }, diff --git a/packages/mongo/mongo_driver.js b/packages/mongo/mongo_driver.js index 5a8c754406..c9f496f1e6 100644 --- a/packages/mongo/mongo_driver.js +++ b/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) { diff --git a/packages/mongo/mongo_livedata_tests.js b/packages/mongo/mongo_livedata_tests.js index 4ad47bb2fa..2d490e90de 100644 --- a/packages/mongo/mongo_livedata_tests.js +++ b/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'}); diff --git a/packages/mongo/mongo_utils.js b/packages/mongo/mongo_utils.js new file mode 100644 index 0000000000..e97e722fd3 --- /dev/null +++ b/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 } : {}), + }; +}; diff --git a/packages/mongo/observe_changes_tests.js b/packages/mongo/observe_changes_tests.js index 0d920deac7..56ab79db7c 100644 --- a/packages/mongo/observe_changes_tests.js +++ b/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' } }); } - ); + ); } diff --git a/packages/tinytest/tinytest.js b/packages/tinytest/tinytest.js index fadb83d3eb..045548c6de 100644 --- a/packages/tinytest/tinytest.js +++ b/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 : ""), }); }