refactor(NODE-1812)!: replace returnOriginal with returnDocument opti…

…on (#2803)
mongodb · May 13, 2021 · 1cdc8a8 · 1cdc8a8
1 parent 6c0dfef
commit 1cdc8a8
Show file tree

Hide file tree

Showing 14 changed files with 90 additions and 56 deletions.
diff --git a/src/index.ts b/src/index.ts
@@ -67,6 +67,7 @@ export { BatchType } from './bulk/common';
 export { AuthMechanism } from './cmap/auth/defaultAuthProviders';
 export { CURSOR_FLAGS } from './cursor/abstract_cursor';
 export { Compressor } from './cmap/wire_protocol/compression';
+export { ReturnDocument } from './operations/find_and_modify';
 export { ExplainVerbosity } from './explain';
 export { ReadConcernLevel } from './read_concern';
 export { ReadPreferenceMode } from './read_preference';

diff --git a/src/operations/find_and_modify.ts b/src/operations/find_and_modify.ts
@@ -10,6 +10,15 @@ import { Sort, SortForCmd, formatSort } from '../sort';
 import type { ClientSession } from '../sessions';
 import type { WriteConcern, WriteConcernSettings } from '../write_concern';
 
+/** @public */
+export const ReturnDocument = Object.freeze({
+  BEFORE: 'before',
+  AFTER: 'after'
+} as const);
+
+/** @public */
+export type ReturnDocument = typeof ReturnDocument[keyof typeof ReturnDocument];
+
 /** @public */
 export interface FindOneAndDeleteOptions extends CommandOperationOptions {
   /** An optional hint for query optimization. See the {@link https://docs.mongodb.com/manual/reference/command/update/#update-command-hint|update command} reference for more information.*/
@@ -28,8 +37,8 @@ export interface FindOneAndReplaceOptions extends CommandOperationOptions {
   hint?: Document;
   /** Limits the fields to return for all matching documents. */
   projection?: Document;
-  /** When false, returns the updated document rather than the original. The default is true. */
-  returnOriginal?: boolean;
+  /** When set to 'after', returns the updated document rather than the original. The default is 'before'.  */
+  returnDocument?: ReturnDocument;
   /** Determines which document the operation modifies if the query selects multiple documents. */
   sort?: Sort;
   /** Upsert the document if it does not exist. */
@@ -46,16 +55,14 @@ export interface FindOneAndUpdateOptions extends CommandOperationOptions {
   hint?: Document;
   /** Limits the fields to return for all matching documents. */
   projection?: Document;
-  /** When false, returns the updated document rather than the original. The default is true. */
-  returnOriginal?: boolean;
+  /** When set to 'after', returns the updated document rather than the original. The default is 'before'.  */
+  returnDocument?: ReturnDocument;
   /** Determines which document the operation modifies if the query selects multiple documents. */
   sort?: Sort;
   /** Upsert the document if it does not exist. */
   upsert?: boolean;
 }
 
-// TODO: NODE-1812 to deprecate returnOriginal for returnDocument
-
 /** @internal */
 interface FindAndModifyCmdBase {
   remove: boolean;
@@ -74,7 +81,7 @@ function configureFindAndModifyCmdBaseUpdateOpts(
   cmdBase: FindAndModifyCmdBase,
   options: FindOneAndReplaceOptions | FindOneAndUpdateOptions
 ): FindAndModifyCmdBase {
-  cmdBase.new = options.returnOriginal === false;
+  cmdBase.new = options.returnDocument === ReturnDocument.AFTER;
   cmdBase.upsert = options.upsert === true;
 
   if (options.bypassDocumentValidation === true) {

diff --git a/test/functional/crud_api.test.js b/test/functional/crud_api.test.js
@@ -1,6 +1,7 @@
 'use strict';
 const test = require('./shared').assert;
 const { expect } = require('chai');
+const { ReturnDocument } = require('../../src');
 const setupDatabase = require('./shared').setupDatabase;
 
 // instanceof cannot be use reliably to detect the new models in js due to scoping and new
@@ -784,7 +785,7 @@ describe('CRUD API', function () {
               {
                 projection: { b: 1, c: 1 },
                 sort: { a: 1 },
-                returnOriginal: false,
+                returnDocument: ReturnDocument.AFTER,
                 upsert: true
               },
               function (err, r) {
@@ -816,7 +817,7 @@ describe('CRUD API', function () {
               {
                 projection: { b: 1, d: 1 },
                 sort: { a: 1 },
-                returnOriginal: false,
+                returnDocument: ReturnDocument.AFTER,
                 upsert: true
               },
               function (err, r) {

diff --git a/test/functional/crud_spec.test.js b/test/functional/crud_spec.test.js
@@ -321,13 +321,12 @@ describe('CRUD spec', function () {
     const second = args.update || args.replacement;
     const options = Object.assign({}, args);
     if (options.returnDocument) {
-      options.returnOriginal = options.returnDocument === 'After' ? false : true;
+      options.returnDocument = options.returnDocument.toLowerCase();
     }
 
     delete options.filter;
     delete options.update;
     delete options.replacement;
-    delete options.returnDocument;
 
     const opName = scenarioTest.operation.name;
     const findPromise =

diff --git a/test/functional/find.test.js b/test/functional/find.test.js
@@ -3,7 +3,7 @@ const test = require('./shared').assert;
 const { setupDatabase, withMonitoredClient } = require('./shared');
 const { expect } = require('chai');
 const sinon = require('sinon');
-const { Code, ObjectId, Long, Binary } = require('../../src');
+const { Code, ObjectId, Long, Binary, ReturnDocument } = require('../../src');
 
 describe('Find', function () {
   before(function () {
@@ -810,7 +810,7 @@ describe('Find', function () {
             collection.findOneAndUpdate(
               { a: 1 },
               { $set: { b: 3 } },
-              { returnOriginal: false },
+              { returnDocument: ReturnDocument.AFTER },
               function (err, updated_doc) {
                 test.equal(1, updated_doc.value.a);
                 test.equal(3, updated_doc.value.b);
@@ -846,7 +846,7 @@ describe('Find', function () {
                             collection.findOneAndUpdate(
                               { a: 4 },
                               { $set: { b: 3 } },
-                              { returnOriginal: false, upsert: true },
+                              { returnDocument: ReturnDocument.AFTER, upsert: true },
                               function (err, updated_doc) {
                                 test.equal(4, updated_doc.value.a);
                                 test.equal(3, updated_doc.value.b);
@@ -861,7 +861,10 @@ describe('Find', function () {
                                     collection.findOneAndUpdate(
                                       { a: 100 },
                                       { $set: { b: 5 } },
-                                      { returnOriginal: false, projection: { b: 1 } },
+                                      {
+                                        returnDocument: ReturnDocument.AFTER,
+                                        projection: { b: 1 }
+                                      },
                                       function (err, updated_doc) {
                                         test.equal(2, Object.keys(updated_doc.value).length);
                                         test.equal(
@@ -913,7 +916,7 @@ describe('Find', function () {
             collection.findOneAndUpdate(
               { a: 1 },
               { $set: { b: 3 } },
-              { returnOriginal: false, projection: { a: 1 } },
+              { returnDocument: ReturnDocument.AFTER, projection: { a: 1 } },
               function (err, updated_doc) {
                 test.equal(2, Object.keys(updated_doc.value).length);
                 test.equal(1, updated_doc.value.a);
@@ -1160,7 +1163,7 @@ describe('Find', function () {
             collection.findOneAndUpdate(
               { a: 2 },
               { $set: { b: 3 } },
-              { returnOriginal: false },
+              { returnDocument: ReturnDocument.AFTER },
               function (err, result) {
                 test.equal(2, result.value.a);
                 test.equal(3, result.value.b);
@@ -1252,7 +1255,7 @@ describe('Find', function () {
             collection.findOneAndUpdate(
               { _id: id },
               { $set: { 'c.c': 100 } },
-              { returnOriginal: false },
+              { returnDocument: ReturnDocument.AFTER },
               function (err, item) {
                 test.equal(doc._id.toString(), item.value._id.toString());
                 test.equal(doc.a, item.value.a);
@@ -1289,7 +1292,11 @@ describe('Find', function () {
           collection.findOneAndUpdate(
             { _id: self._id, 'plays.uuid': _uuid },
             { $set: { 'plays.$.active': true } },
-            { returnOriginal: false, projection: { plays: 0, results: 0 }, safe: true },
+            {
+              returnDocument: ReturnDocument.AFTER,
+              projection: { plays: 0, results: 0 },
+              safe: true
+            },
             function (err) {
               expect(err).to.not.exist;
               client.close(done);
@@ -1437,7 +1444,7 @@ describe('Find', function () {
             collection.findOneAndUpdate(
               { a: 1 },
               { $set: { b: 3 } },
-              { returnOriginal: false },
+              { returnDocument: ReturnDocument.AFTER },
               function (err, updated_doc) {
                 expect(err).to.not.exist;
                 expect(updated_doc.value).to.not.exist;
@@ -1500,7 +1507,7 @@ describe('Find', function () {
                     'transactions.id': { $ne: transaction.transactionId }
                   },
                   { $push: { transactions: transaction } },
-                  { returnOriginal: false, safe: true },
+                  { returnDocument: ReturnDocument.AFTER, safe: true },
                   function (err) {
                     expect(err).to.not.exist;
                     client.close(done);
@@ -1587,7 +1594,7 @@ describe('Find', function () {
           function (err, collection) {
             var q = { x: 1 };
             var set = { y: 2, _id: new ObjectId() };
-            var opts = { returnOriginal: false, upsert: true };
+            var opts = { returnDocument: ReturnDocument.AFTER, upsert: true };
             // Original doc
             var doc = { _id: new ObjectId(), x: 1 };
 
@@ -2316,7 +2323,7 @@ describe('Find', function () {
             collection.findOneAndUpdate(
               { a: 1 },
               { $set: { b: 3 } },
-              { returnOriginal: false },
+              { returnDocument: ReturnDocument.AFTER },
               function (err, updated_doc) {
                 test.equal(1, updated_doc.value.a);
                 test.equal(3, updated_doc.value.b);

diff --git a/test/functional/insert.test.js b/test/functional/insert.test.js
@@ -16,7 +16,8 @@ const {
   MinKey,
   MaxKey,
   Code,
-  MongoBulkWriteError
+  MongoBulkWriteError,
+  ReturnDocument
 } = require('../../src');
 
 /**
@@ -967,7 +968,11 @@ describe('Insert', function () {
                 collection.findOneAndUpdate(
                   { str: 'String' },
                   { $set: { f: function () {} } },
-                  { returnOriginal: false, safe: true, serializeFunctions: true },
+                  {
+                    returnDocument: ReturnDocument.AFTER,
+                    safe: true,
+                    serializeFunctions: true
+                  },
                   function (err, result) {
                     test.ok(result.value.f._bsontype === 'Code');
                     client.close(done);

diff --git a/test/functional/multiple_db.test.js b/test/functional/multiple_db.test.js
@@ -1,6 +1,7 @@
 'use strict';
 var test = require('./shared').assert;
 var setupDatabase = require('./shared').setupDatabase;
+const { ReturnDocument } = require('../../src');
 const { expect } = require('chai');
 
 describe('Multiple Databases', function () {
@@ -78,12 +79,15 @@ describe('Multiple Databases', function () {
 
         db_instance.collection('counters', function (err, collection) {
           expect(err).to.not.exist;
-          collection.findOneAndUpdate({}, { $inc: { db: 1 } }, { returnOriginal: false }, function (
-            err
-          ) {
-            expect(err).to.not.exist;
-            client.close(done);
-          });
+          collection.findOneAndUpdate(
+            {},
+            { $inc: { db: 1 } },
+            { returnDocument: ReturnDocument.AFTER },
+            function (err) {
+              expect(err).to.not.exist;
+              client.close(done);
+            }
+          );
         });
       });
     }

diff --git a/test/functional/operation_example.test.js b/test/functional/operation_example.test.js
@@ -3,7 +3,7 @@ const { assert: test } = require('./shared');
 const { setupDatabase } = require('./shared');
 const { format: f } = require('util');
 const { Topology } = require('../../src/sdam/topology');
-const { Code, ObjectId } = require('../../src');
+const { Code, ObjectId, ReturnDocument } = require('../../src');
 
 const chai = require('chai');
 const expect = chai.expect;
@@ -1532,7 +1532,7 @@ describe('Operation Examples', function () {
             collection.findOneAndUpdate(
               { a: 1 },
               { $set: { b1: 1 } },
-              { returnOriginal: false },
+              { returnDocument: ReturnDocument.AFTER },
               function (err, doc) {
                 expect(err).to.not.exist;
                 test.equal(1, doc.value.a);
@@ -1558,7 +1558,11 @@ describe('Operation Examples', function () {
                       collection.findOneAndUpdate(
                         { d: 1 },
                         { $set: { d: 1, f: 1 } },
-                        { returnOriginal: false, upsert: true, writeConcern: { w: 1 } },
+                        {
+                          returnDocument: ReturnDocument.AFTER,
+                          upsert: true,
+                          writeConcern: { w: 1 }
+                        },
                         function (err, doc) {
                           expect(err).to.not.exist;
                           test.equal(1, doc.value.d);
@@ -6406,7 +6410,7 @@ describe('Operation Examples', function () {
             {
               projection: { b: 1, c: 1 },
               sort: { a: 1 },
-              returnOriginal: false,
+              returnDocument: ReturnDocument.AFTER,
               upsert: true
             },
             function (err, r) {
@@ -6462,7 +6466,7 @@ describe('Operation Examples', function () {
             {
               projection: { b: 1, d: 1 },
               sort: { a: 1 },
-              returnOriginal: false,
+              returnDocument: ReturnDocument.AFTER,
               upsert: true
             },
             function (err, r) {

diff --git a/test/functional/operation_generators_example.test.js b/test/functional/operation_generators_example.test.js
@@ -1,7 +1,7 @@
 'use strict';
 var test = require('./shared').assert;
 var setupDatabase = require('./shared').setupDatabase;
-const { Code } = require('../../src');
+const { Code, ReturnDocument } = require('../../src');
 const { expect } = require('chai');
 
 /**************************************************************************
@@ -906,7 +906,7 @@ describe('Operation (Generators)', function () {
         var doc = yield collection.findOneAndUpdate(
           { a: 1 },
           { $set: { b1: 1 } },
-          { returnOriginal: false }
+          { returnDocument: ReturnDocument.AFTER }
         );
         test.equal(1, doc.value.a);
         test.equal(1, doc.value.b1);
@@ -924,7 +924,7 @@ describe('Operation (Generators)', function () {
         doc = yield collection.findOneAndUpdate(
           { d: 1 },
           { $set: { d: 1, f: 1 } },
-          { returnOriginal: false, upsert: true, writeConcern: { w: 1 } }
+          { returnDocument: ReturnDocument.AFTER, upsert: true, writeConcern: { w: 1 } }
         );
         test.equal(1, doc.value.d);
         test.equal(1, doc.value.f);
@@ -4375,7 +4375,7 @@ describe('Operation (Generators)', function () {
           {
             projection: { b: 1, c: 1 },
             sort: { a: 1 },
-            returnOriginal: false,
+            returnDocument: ReturnDocument.AFTER,
             upsert: true
           }
         );
@@ -4430,7 +4430,7 @@ describe('Operation (Generators)', function () {
           {
             projection: { b: 1, d: 1 },
             sort: { a: 1 },
-            returnOriginal: false,
+            returnDocument: ReturnDocument.AFTER,
             upsert: true
           }
         );