Skip to content

refactor(NODE-1812)!: replace returnOriginal with returnDocument option #2803

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 13, 2021
Merged

refactor(NODE-1812)!: replace returnOriginal with returnDocument option #2803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
252aa1f
refactor(NODE-1812)!: Replace returnOriginal with returnDocument in f…
dariakp May 10, 2021
5e86b7f
refactor: Use lowercase for ReturnDocument enum values
dariakp May 11, 2021
235dc7c
test: lowercase returnDocument for spec tests
dariakp May 12, 2021
82a9f4b
test: Add translateOptions helper to unified runner
dariakp May 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/index.ts
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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