From 1cdc8a8738d8c2425e0ff76751331636894256b8 Mon Sep 17 00:00:00 2001 From: Daria Pardue <81593090+dariakp@users.noreply.github.com> Date: Thu, 13 May 2021 18:56:49 -0400 Subject: [PATCH] refactor(NODE-1812)!: replace returnOriginal with returnDocument option (#2803) --- src/index.ts | 1 + src/operations/find_and_modify.ts | 21 ++++++++----- test/functional/crud_api.test.js | 5 +-- test/functional/crud_spec.test.js | 3 +- test/functional/find.test.js | 31 ++++++++++++------- test/functional/insert.test.js | 9 ++++-- test/functional/multiple_db.test.js | 16 ++++++---- test/functional/operation_example.test.js | 14 ++++++--- .../operation_generators_example.test.js | 10 +++--- .../operation_promises_example.test.js | 10 +++--- test/functional/retryable_writes.test.js | 3 +- test/functional/spec-runner/index.js | 2 +- .../unified-spec-runner/operations.ts | 12 +++---- .../unified-spec-runner/unified-utils.ts | 9 ++++++ 14 files changed, 90 insertions(+), 56 deletions(-) diff --git a/src/index.ts b/src/index.ts index 5a33fd8976..ce943e8a97 100644 --- 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 index 0e54be717a..096b252d1b 100644 --- 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 index ddbffb0063..6f08d39fab 100644 --- 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 index cfe6e9db97..14c4da4244 100644 --- 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 index f54f048087..f82fa7bde7 100644 --- 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 index b46d4d2427..b913479a3e 100644 --- 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 index e6982ac449..bebf4ee066 100644 --- 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 index 36a9cbb827..45ae5ba58e 100644 --- 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 index 73802e6d8d..4e70da6683 100644 --- 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 } ); diff --git a/test/functional/operation_promises_example.test.js b/test/functional/operation_promises_example.test.js index 7897cd1cd9..88b58171c3 100644 --- a/test/functional/operation_promises_example.test.js +++ b/test/functional/operation_promises_example.test.js @@ -2,7 +2,7 @@ var f = require('util').format; var test = require('./shared').assert; var setupDatabase = require('./shared').setupDatabase; -const { Code } = require('../../src'); +const { Code, ReturnDocument } = require('../../src'); const { expect } = require('chai'); var delay = function (ms) { @@ -918,7 +918,7 @@ describe('Operation (Promises)', function () { return collection.findOneAndUpdate( { a: 1 }, { $set: { b1: 1 } }, - { returnOriginal: false } + { returnDocument: ReturnDocument.AFTER } ); }) .then(function (doc) { @@ -943,7 +943,7 @@ describe('Operation (Promises)', function () { return collection.findOneAndUpdate( { d: 1 }, { $set: { d: 1, f: 1 } }, - { returnOriginal: false, upsert: true, writeConcern: { w: 1 } } + { returnDocument: ReturnDocument.AFTER, upsert: true, writeConcern: { w: 1 } } ); }) .then(function (doc) { @@ -4766,7 +4766,7 @@ describe('Operation (Promises)', function () { { projection: { b: 1, c: 1 }, sort: { a: 1 }, - returnOriginal: false, + returnDocument: ReturnDocument.AFTER, upsert: true } ) @@ -4821,7 +4821,7 @@ describe('Operation (Promises)', function () { { projection: { b: 1, d: 1 }, sort: { a: 1 }, - returnOriginal: false, + returnDocument: ReturnDocument.AFTER, upsert: true } ); diff --git a/test/functional/retryable_writes.test.js b/test/functional/retryable_writes.test.js index 47dbe163ac..1c79a06fcd 100644 --- a/test/functional/retryable_writes.test.js +++ b/test/functional/retryable_writes.test.js @@ -139,8 +139,7 @@ function generateArguments(test) { } else if (arg === 'upsert') { options.upsert = test.operation.arguments[arg]; } else if (arg === 'returnDocument') { - const returnDocument = test.operation.arguments[arg]; - options.returnOriginal = returnDocument === 'Before'; + options.returnDocument = test.operation.arguments[arg].toLowerCase(); } else { args.push(test.operation.arguments[arg]); } diff --git a/test/functional/spec-runner/index.js b/test/functional/spec-runner/index.js index bed31325a5..bc34dbaf71 100644 --- a/test/functional/spec-runner/index.js +++ b/test/functional/spec-runner/index.js @@ -681,7 +681,7 @@ function testOperation(operation, obj, context, options) { } if (key === 'returnDocument') { - opOptions.returnOriginal = operation.arguments[key] === 'Before' ? true : false; + opOptions.returnDocument = operation.arguments[key].toLowerCase(); return; } diff --git a/test/functional/unified-spec-runner/operations.ts b/test/functional/unified-spec-runner/operations.ts index 178a78c39f..b3310ff3c1 100644 --- a/test/functional/unified-spec-runner/operations.ts +++ b/test/functional/unified-spec-runner/operations.ts @@ -10,6 +10,7 @@ import { EntitiesMap } from './entities'; import { expectErrorCheck, resultCheck } from './match'; import type { OperationDescription } from './schema'; import { CommandStartedEvent } from '../../../src/cmap/command_monitoring_events'; +import { translateOptions } from './unified-utils'; interface OperationFunctionParams { client: MongoClient; @@ -234,17 +235,14 @@ operations.set('find', async ({ entities, operation }) => { operations.set('findOneAndReplace', async ({ entities, operation }) => { const collection = entities.getEntity('collection', operation.object); - return collection.findOneAndReplace(operation.arguments.filter, operation.arguments.replacement); + const { filter, replacement, ...opts } = operation.arguments; + return collection.findOneAndReplace(filter, replacement, translateOptions(opts)); }); operations.set('findOneAndUpdate', async ({ entities, operation }) => { const collection = entities.getEntity('collection', operation.object); - const returnOriginal = operation.arguments.returnDocument === 'Before'; - return ( - await collection.findOneAndUpdate(operation.arguments.filter, operation.arguments.update, { - returnOriginal - }) - ).value; + const { filter, update, ...opts } = operation.arguments; + return (await collection.findOneAndUpdate(filter, update, translateOptions(opts))).value; }); operations.set('failPoint', async ({ entities, operation }) => { diff --git a/test/functional/unified-spec-runner/unified-utils.ts b/test/functional/unified-spec-runner/unified-utils.ts index 420df85cbb..57baf94afa 100644 --- a/test/functional/unified-spec-runner/unified-utils.ts +++ b/test/functional/unified-spec-runner/unified-utils.ts @@ -1,4 +1,5 @@ import { expect } from 'chai'; +import type { Document } from '../../../src'; import type { CollectionOrDatabaseOptions, RunOnRequirement } from './schema'; import { gte as semverGte, lte as semverLte } from 'semver'; import { CollectionOptions, DbOptions, MongoClient } from '../../../src'; @@ -82,3 +83,11 @@ export function patchCollectionOptions(options: CollectionOrDatabaseOptions): Co // TODO return { ...options } as CollectionOptions; } + +export function translateOptions(options: Document): Document { + const translatedOptions = { ...options }; + if (options.returnDocument) { + translatedOptions.returnDocument = options.returnDocument.toLowerCase(); + } + return translatedOptions as Document; +}