Skip to content

Commit

Permalink
refactor(NODE-1812)!: replace returnOriginal with returnDocument opti…
Browse files Browse the repository at this point in the history
…on (#2803)
  • Loading branch information
dariakp authored and ljhaywar committed Nov 9, 2021
1 parent 71a612a commit 2efa081
Show file tree
Hide file tree
Showing 14 changed files with 90 additions and 56 deletions.
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
21 changes: 14 additions & 7 deletions src/operations/find_and_modify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.*/
Expand All @@ -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. */
Expand All @@ -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;
Expand All @@ -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) {
Expand Down
5 changes: 3 additions & 2 deletions test/functional/crud_api.test.js
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 1 addition & 2 deletions test/functional/crud_spec.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
31 changes: 19 additions & 12 deletions test/functional/find.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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(
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 };

Expand Down Expand Up @@ -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);
Expand Down
9 changes: 7 additions & 2 deletions test/functional/insert.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ const {
MinKey,
MaxKey,
Code,
MongoBulkWriteError
MongoBulkWriteError,
ReturnDocument
} = require('../../src');

/**
Expand Down Expand Up @@ -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);
Expand Down
16 changes: 10 additions & 6 deletions test/functional/multiple_db.test.js
Original file line number Diff line number Diff line change
@@ -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 () {
Expand Down Expand Up @@ -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);
}
);
});
});
}
Expand Down
14 changes: 9 additions & 5 deletions test/functional/operation_example.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
10 changes: 5 additions & 5 deletions test/functional/operation_generators_example.test.js
Original file line number Diff line number Diff line change
@@ -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');

/**************************************************************************
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -4375,7 +4375,7 @@ describe('Operation (Generators)', function () {
{
projection: { b: 1, c: 1 },
sort: { a: 1 },
returnOriginal: false,
returnDocument: ReturnDocument.AFTER,
upsert: true
}
);
Expand Down Expand Up @@ -4430,7 +4430,7 @@ describe('Operation (Generators)', function () {
{
projection: { b: 1, d: 1 },
sort: { a: 1 },
returnOriginal: false,
returnDocument: ReturnDocument.AFTER,
upsert: true
}
);
Expand Down
Loading

0 comments on commit 2efa081

Please sign in to comment.