feat(collection): updating find API

Find api is now updated and clarified. You can no longer pass in
fields, and you cannot pass in individual options as parameters.

BREAKING CHANGE:
`find` and `findOne` no longer support the `fields` parameter.
You can achieve the same results as the `fields` parameter by
either using `Cursor.prototype.project`, or by passing the `projection`
property in on the `options` object. Additionally, `find` does not
support individual options like `skip` and `limit` as positional
parameters. You must pass in these parameters in the `options` object

Author: daprahamian

lib/collection.js

@@ -192,57 +192,48 @@ Object.defineProperty(Collection.prototype, 'hint', {
 /**
  * Creates a cursor for a query that can be used to iterate over results from MongoDB
  * @method
- * @param {object} query The cursor query object.
- * @param {Object} [options] Optional settings
+ * @param {object} [query={}] The cursor query object.
+ * @param {object} [options=null] Optional settings.
+ * @param {number} [options.limit=0] Sets the limit of documents returned in the query.
+ * @param {(array|object)} [options.sort=null] Set to sort the documents coming back from the query. Array of indexes, [['a', 1]] etc.
+ * @param {object} [options.projection=null] The fields to return in the query. Object of fields to include or exclude (not both), {'a':1}
+ * @param {object} [options.fields=null] **Deprecated** Use `options.projection` instead
+ * @param {number} [options.skip=0] Set to skip N documents ahead in your query (useful for pagination).
+ * @param {Object} [options.hint=null] Tell the query to use specific indexes in the query. Object of indexes to use, {'_id':1}
+ * @param {boolean} [options.explain=false] Explain the query instead of returning the data.
+ * @param {boolean} [options.snapshot=false] Snapshot query.
+ * @param {boolean} [options.timeout=false] Specify if the cursor can timeout.
+ * @param {boolean} [options.tailable=false] Specify if the cursor is tailable.
+ * @param {number} [options.batchSize=0] Set the batchSize for the getMoreCommand when iterating over the query results.
+ * @param {boolean} [options.returnKey=false] Only return the index key.
+ * @param {number} [options.maxScan=null] Limit the number of items to scan.
+ * @param {number} [options.min=null] Set index bounds.
+ * @param {number} [options.max=null] Set index bounds.
+ * @param {boolean} [options.showDiskLoc=false] Show disk location of results.
+ * @param {string} [options.comment=null] You can put a $comment field on a query to make looking in the profiler logs simpler.
+ * @param {boolean} [options.raw=false] Return document results as raw BSON buffers.
+ * @param {boolean} [options.promoteLongs=true] Promotes Long values to number if they fit inside the 53 bits resolution.
+ * @param {boolean} [options.promoteValues=true] Promotes BSON values to native types where possible, set to false to only receive wrapper types.
+ * @param {boolean} [options.promoteBuffers=false] Promotes Binary BSON values to native Node Buffers.
+ * @param {(ReadPreference|string)} [options.readPreference=null] The preferred read preference (ReadPreference.PRIMARY, ReadPreference.PRIMARY_PREFERRED, ReadPreference.SECONDARY, ReadPreference.SECONDARY_PREFERRED, ReadPreference.NEAREST).
+ * @param {boolean} [options.partial=false] Specify if the cursor should return partial results when querying against a sharded system
+ * @param {number} [options.maxTimeMS=null] Number of miliseconds to wait before aborting the query.
+ * @param {object} [options.collation=null] Specify collation (MongoDB 3.4 or higher) settings for update operation (see 3.4 documentation for available fields).
  * @param {ClientSession} [options.session] optional session to use for this operation
  * @throws {MongoError}
  * @return {Cursor}
  */
-Collection.prototype.find = function() {
-  var options,
-    args = Array.prototype.slice.call(arguments, 0),
-    has_callback = typeof args[args.length - 1] === 'function',
-    has_weird_callback = typeof args[0] === 'function',
-    callback = has_callback ? args.pop() : has_weird_callback ? args.shift() : null,
-    len = args.length,
-    selector = len >= 1 ? args[0] : {},
-    fields = len >= 2 ? args[1] : undefined;
-
-  if (len === 1 && has_weird_callback) {
-    // backwards compat for callback?, options case
-    selector = {};
-    options = args[0];
-  }
-
-  if (len === 2 && fields !== undefined && !Array.isArray(fields)) {
-    var fieldKeys = Object.keys(fields);
-    var is_option = false;
-
-    for (var i = 0; i < fieldKeys.length; i++) {
-      if (testForFields[fieldKeys[i]] != null) {
-        is_option = true;
-        break;
-      }
-    }
-
-    if (is_option) {
-      options = fields;
-      fields = undefined;
-    } else {
-      options = {};
-    }
-  } else if (len === 2 && Array.isArray(fields) && !Array.isArray(fields[0])) {
-    var newFields = {};
-    // Rewrite the array
-    for (i = 0; i < fields.length; i++) {
-      newFields[fields[i]] = 1;
+Collection.prototype.find = function(query, options, callback) {
+  let selector = query;
+  // figuring out arguments
+  if (typeof callback !== 'function') {
+    if (typeof options === 'function') {
+      callback = options;
+      options = undefined;
+    } else if (options == null) {
+      callback = typeof selector === 'function' ? selector : undefined;
+      selector = typeof selector === 'object' ? selector : undefined;
     }
-    // Set the fields
-    fields = newFields;
-  }
-
-  if (3 === len) {
-    options = args[2];
   }
 
   // Ensure selector is not null
@@ -264,50 +255,24 @@ Collection.prototype.find = function() {
     }
   }
 
-  // Validate correctness of the field selector
-  object = fields;
-  if (Buffer.isBuffer(object)) {
-    object_size = object[0] | (object[1] << 8) | (object[2] << 16) | (object[3] << 24);
-    if (object_size !== object.length) {
-      error = new Error(
-        'query fields raw message size does not match message header size [' +
-          object.length +
-          '] != [' +
-          object_size +
-          ']'
-      );
-      error.name = 'MongoError';
-      throw error;
-    }
-  }
-
   // Check special case where we are using an objectId
   if (selector != null && selector._bsontype === 'ObjectID') {
     selector = { _id: selector };
   }
 
-  // If it's a serialized fields field we need to just let it through
-  // user be warned it better be good
-  if (options && options.fields && !Buffer.isBuffer(options.fields)) {
-    fields = {};
+  if (!options) options = {};
 
-    if (Array.isArray(options.fields)) {
-      if (!options.fields.length) {
-        fields['_id'] = 1;
-      } else {
-        var l = options.fields.length;
+  let projection = options.projection || options.fields;
 
-        for (i = 0; i < l; i++) {
-          fields[options.fields[i]] = 1;
-        }
-      }
-    } else {
-      fields = options.fields;
-    }
+  if (projection && !Buffer.isBuffer(projection) && Array.isArray(projection)) {
+    projection = projection.length
+      ? projection.reduce((result, field) => {
+          result[field] = 1;
+          return result;
+        }, {})
+      : { _id: 1 };
   }
 
-  if (!options) options = {};
-
   var newOptions = {};
 
   // Make a shallow copy of the collection options
@@ -323,13 +288,11 @@ Collection.prototype.find = function() {
   }
 
   // Unpack options
-  newOptions.skip = len > 3 ? args[2] : options.skip ? options.skip : 0;
-  newOptions.limit = len > 3 ? args[3] : options.limit ? options.limit : 0;
-  newOptions.raw =
-    options.raw != null && typeof options.raw === 'boolean' ? options.raw : this.s.raw;
+  newOptions.skip = options.skip ? options.skip : 0;
+  newOptions.limit = options.limit ? options.limit : 0;
+  newOptions.raw = typeof options.raw === 'boolean' ? options.raw : this.s.raw;
   newOptions.hint = options.hint != null ? normalizeHintField(options.hint) : this.s.collectionHint;
-  newOptions.timeout =
-    len === 5 ? args[4] : typeof options.timeout === 'undefined' ? undefined : options.timeout;
+  newOptions.timeout = typeof options.timeout === 'undefined' ? undefined : options.timeout;
   // // If we have overridden slaveOk otherwise use the default db setting
   newOptions.slaveOk = options.slaveOk != null ? options.slaveOk : this.s.db.slaveOk;
 
@@ -372,26 +335,7 @@ Collection.prototype.find = function() {
     }
   }
 
-  // Format the fields
-  var formatFields = function(fields) {
-    var object = {};
-    if (Array.isArray(fields)) {
-      for (var i = 0; i < fields.length; i++) {
-        if (Array.isArray(fields[i])) {
-          object[fields[i][0]] = fields[i][1];
-        } else {
-          object[fields[i][0]] = 1;
-        }
-      }
-    } else {
-      object = fields;
-    }
-
-    return object;
-  };
-
-  // Special treatment for the fields selector
-  if (fields) findCommand.fields = formatFields(fields);
+  if (projection) findCommand.fields = projection;
 
   // Add db object to the new options
   newOptions.db = this.s.db;
@@ -1361,7 +1305,8 @@ define.classMethod('save', { callback: true, promise: true });
  * @param {object} [options=null] Optional settings.
  * @param {number} [options.limit=0] Sets the limit of documents returned in the query.
  * @param {(array|object)} [options.sort=null] Set to sort the documents coming back from the query. Array of indexes, [['a', 1]] etc.
- * @param {object} [options.fields=null] The fields to return in the query. Object of fields to include or exclude (not both), {'a':1}
+ * @param {object} [options.projection=null] The fields to return in the query. Object of fields to include or exclude (not both), {'a':1}
+ * @param {object} [options.fields=null] **Deprecated** Use `options.projection` instead
  * @param {number} [options.skip=0] Set to skip N documents ahead in your query (useful for pagination).
  * @param {Object} [options.hint=null] Tell the query to use specific indexes in the query. Object of indexes to use, {'_id':1}
  * @param {boolean} [options.explain=false] Explain the query instead of returning the data.
@@ -2253,7 +2198,8 @@ define.classMethod('findOneAndUpdate', { callback: true, promise: true });
  * @param {boolean} [options.remove=false] Set to true to remove the object before returning.
  * @param {boolean} [options.upsert=false] Perform an upsert operation.
  * @param {boolean} [options.new=false] Set to true if you want to return the modified object rather than the original. Ignored for remove.
- * @param {object} [options.fields=null] Object containing the field projection for the result returned from the operation.
+ * @param {object} [options.projection=null] Object containing the field projection for the result returned from the operation.
+ * @param {object} [options.fields=null] **Deprecated** Use `options.projection` instead
  * @param {ClientSession} [options.session] optional session to use for this operation
  * @param {Collection~findAndModifyCallback} [callback] The command result callback
  * @return {Promise} returns Promise if no callback passed
@@ -2297,8 +2243,10 @@ var findAndModify = function(self, query, sort, doc, options, callback) {
   queryObject.remove = options.remove ? true : false;
   queryObject.upsert = options.upsert ? true : false;
 
-  if (options.fields) {
-    queryObject.fields = options.fields;
+  const projection = options.projection || options.fields;
+
+  if (projection) {
+    queryObject.fields = projection;
   }
 
   if (options.arrayFilters) {
@@ -3272,39 +3220,4 @@ var getReadPreference = function(self, options, db) {
   return options;
 };
 
-var testForFields = {
-  limit: 1,
-  sort: 1,
-  fields: 1,
-  skip: 1,
-  hint: 1,
-  explain: 1,
-  snapshot: 1,
-  timeout: 1,
-  tailable: 1,
-  tailableRetryInterval: 1,
-  numberOfRetries: 1,
-  awaitdata: 1,
-  awaitData: 1,
-  exhaust: 1,
-  batchSize: 1,
-  returnKey: 1,
-  maxScan: 1,
-  min: 1,
-  max: 1,
-  showDiskLoc: 1,
-  comment: 1,
-  raw: 1,
-  readPreference: 1,
-  partial: 1,
-  read: 1,
-  dbName: 1,
-  oplogReplay: 1,
-  connection: 1,
-  maxTimeMS: 1,
-  transforms: 1,
-  collation: 1,
-  noCursorTimeout: 1
-};
-
 module.exports = Collection;

test/functional/cursor_tests.js

@@ -1719,20 +1719,16 @@ describe('Cursor', function() {
           collection.save({ x: 1, a: 2 }, configuration.writeConcernMax(), function(err) {
             test.equal(null, err);
 
-            collection.find({}, { fields: ['a'] }).toArray(function(err, items) {
-              test.equal(null, err);
-              test.equal(1, items.length);
-              test.equal(2, items[0].a);
-              test.equal(undefined, items[0].x);
-            });
-
-            collection.findOne({}, { fields: ['a'] }, function(err, item) {
-              test.equal(null, err);
-              test.equal(2, item.a);
-              test.equal(undefined, item.x);
-              client.close();
-              done();
-            });
+            collection
+              .find({})
+              .project({ a: 1 })
+              .toArray(function(err, items) {
+                test.equal(null, err);
+                test.equal(1, items.length);
+                test.equal(2, items[0].a);
+                test.equal(undefined, items[0].x);
+                done();
+              });
           });
         });
       });
@@ -4139,28 +4135,28 @@ describe('Cursor', function() {
         test.equal(null, err);
 
         db.collection('cursor_count_test1', { readConcern: { level: 'local' } }).count({
-          project: '123'
-        },
-        {
-          readConcern: { level: 'local' },
-          limit: 5,
-          skip: 5,
-          hint: { project: 1 }
-        },
-        function(err) {
-          test.equal(null, err);
-          test.equal(1, started.length);
-          if (started[0].command.readConcern)
-            test.deepEqual({ level: 'local' }, started[0].command.readConcern);
-          test.deepEqual({ project: 1 }, started[0].command.hint);
-          test.equal(5, started[0].command.skip);
-          test.equal(5, started[0].command.limit);
+            project: '123'
+          },
+          {
+            readConcern: { level: 'local' },
+            limit: 5,
+            skip: 5,
+            hint: { project: 1 }
+          },
+          function(err) {
+            test.equal(null, err);
+            test.equal(1, started.length);
+            if (started[0].command.readConcern)
+              test.deepEqual({ level: 'local' }, started[0].command.readConcern);
+            test.deepEqual({ project: 1 }, started[0].command.hint);
+            test.equal(5, started[0].command.skip);
+            test.equal(5, started[0].command.limit);
 
-          listener.uninstrument();
+            listener.uninstrument();
 
-          client.close();
-          done();
-        });
+            client.close();
+            done();
+          });
       });
     }
   });

test/functional/db_tests.js

@@ -96,7 +96,9 @@ describe('Db', function() {
 
         try {
           coll.findOne({}, null, function() {
-            //e - Cannot convert undefined or null to object
+            //e - errors b/c findOne needs a query selector
+            test.equal(1, count);
+            done();
           });
         } catch (e) {
           process.nextTick(function() {