From 8900d40eaa0476ea672b5d01089229453cacca58 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Tue, 3 Jan 2023 19:53:38 +0100 Subject: [PATCH] feat(NODE-3695)!: remove lastop and optime from bulk result (#3504) --- etc/notes/CHANGES_5.0.0.md | 5 ++ src/bulk/common.ts | 63 +--------------- test/unit/bulk/common.test.js | 132 ---------------------------------- 3 files changed, 6 insertions(+), 194 deletions(-) delete mode 100644 test/unit/bulk/common.test.js diff --git a/etc/notes/CHANGES_5.0.0.md b/etc/notes/CHANGES_5.0.0.md index 483cbecf53..cd61bf8cf5 100644 --- a/etc/notes/CHANGES_5.0.0.md +++ b/etc/notes/CHANGES_5.0.0.md @@ -16,6 +16,11 @@ The following is a detailed collection of the changes in the major v5 release of ## Changes +### Bulk results no longer contain `lastOp()` and `opTime` + +The `lastOp()` method and `opTime` property on the `BulkResult` have been removed. Merging of bulk results +no longer normalizes the values. There is no new method or property to replace them. + ### `CursorCloseOptions` removed When calling `close()` on a `Cursor`, no more options can be provided. This removes support for the diff --git a/src/bulk/common.ts b/src/bulk/common.ts index e6aff6f39e..e9f2a1fb68 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -1,11 +1,4 @@ -import { - BSONSerializeOptions, - Document, - Long, - ObjectId, - resolveBSONOptions, - Timestamp -} from '../bson'; +import { BSONSerializeOptions, Document, ObjectId, resolveBSONOptions } from '../bson'; import type { Collection } from '../collection'; import { AnyError, @@ -146,7 +139,6 @@ export interface BulkResult { nModified: number; nRemoved: number; upserted: Document[]; - opTime?: Document; } /** @@ -300,15 +292,6 @@ export class BulkWriteResult { return this.result.writeErrors; } - /** - * Retrieve lastOp if available - * - * @deprecated Will be removed in 5.0 - */ - getLastOp(): Document | undefined { - return this.result.opTime; - } - /** Retrieve the write concern error if one exists */ getWriteConcernError(): WriteConcernError | undefined { if (this.result.writeConcernErrors.length === 0) { @@ -449,12 +432,6 @@ export class WriteError { } } -/** Converts the number to a Long or returns it. */ -function longOrConvert(value: number | Long | Timestamp): Long | Timestamp { - // TODO(NODE-2674): Preserve int64 sent from MongoDB - return typeof value === 'number' ? Long.fromNumber(value) : value; -} - /** Merges results into shared data structure */ export function mergeBatchResults( batch: Batch, @@ -491,44 +468,6 @@ export function mergeBatchResults( return; } - // The server write command specification states that lastOp is an optional - // mongod only field that has a type of timestamp. Across various scarce specs - // where opTime is mentioned, it is an "opaque" object that can have a "ts" and - // "t" field with Timestamp and Long as their types respectively. - // The "lastOp" field of the bulk write result is never mentioned in the driver - // specifications or the bulk write spec, so we should probably just keep its - // value consistent since it seems to vary. - // See: https://github.com/mongodb/specifications/blob/master/source/driver-bulk-update.rst#results-object - if (result.opTime || result.lastOp) { - let opTime = result.lastOp || result.opTime; - - // If the opTime is a Timestamp, convert it to a consistent format to be - // able to compare easily. Converting to the object from a timestamp is - // much more straightforward than the other direction. - if (opTime._bsontype === 'Timestamp') { - opTime = { ts: opTime, t: Long.ZERO }; - } - - // If there's no lastOp, just set it. - if (!bulkResult.opTime) { - bulkResult.opTime = opTime; - } else { - // First compare the ts values and set if the opTimeTS value is greater. - const lastOpTS = longOrConvert(bulkResult.opTime.ts); - const opTimeTS = longOrConvert(opTime.ts); - if (opTimeTS.greaterThan(lastOpTS)) { - bulkResult.opTime = opTime; - } else if (opTimeTS.equals(lastOpTS)) { - // If the ts values are equal, then compare using the t values. - const lastOpT = longOrConvert(bulkResult.opTime.t); - const opTimeT = longOrConvert(opTime.t); - if (opTimeT.greaterThan(lastOpT)) { - bulkResult.opTime = opTime; - } - } - } - } - // If we have an insert Batch type if (isInsertBatch(batch) && result.n) { bulkResult.nInserted = bulkResult.nInserted + result.n; diff --git a/test/unit/bulk/common.test.js b/test/unit/bulk/common.test.js deleted file mode 100644 index 2bfe652d77..0000000000 --- a/test/unit/bulk/common.test.js +++ /dev/null @@ -1,132 +0,0 @@ -/* eslint-disable no-loss-of-precision */ -/* eslint-disable @typescript-eslint/no-loss-of-precision */ -// TODO(NODE-3774): Lower the integer literals below JS max precision -'use strict'; - -const { expect } = require('chai'); -const { mergeBatchResults } = require('../../../src/bulk/common'); -const { Timestamp, Long } = require('../../../src/bson'); - -describe('bulk/common', function () { - describe('#mergeBatchResults', function () { - let opTime; - let lastOp; - const bulkResult = { - ok: 1, - writeErrors: [], - writeConcernErrors: [], - insertedIds: [], - nInserted: 0, - nUpserted: 0, - nMatched: 0, - nModified: 0, - nRemoved: 1, - upserted: [] - }; - const result = { - n: 8, - nModified: 8, - electionId: '7fffffff0000000000000028', - ok: 1, - $clusterTime: { - clusterTime: '7020546605669417498', - signature: { - hash: 'AAAAAAAAAAAAAAAAAAAAAAAAAAA=', - keyId: 0 - } - }, - operationTime: '7020546605669417498' - }; - const batch = []; - - context('when lastOp is an object', function () { - context('when the opTime is a Timestamp', function () { - before(function () { - lastOp = { ts: 7020546605669417496, t: 10 }; - opTime = Timestamp.fromNumber(8020546605669417496); - bulkResult.opTime = lastOp; - result.opTime = opTime; - }); - - it('replaces the opTime with the properly formatted object', function () { - mergeBatchResults(batch, bulkResult, null, result); - expect(bulkResult.opTime).to.deep.equal({ ts: opTime, t: Long.ZERO }); - }); - }); - - context('when the opTime is an object', function () { - context('when the ts is greater', function () { - before(function () { - lastOp = { ts: 7020546605669417496, t: 10 }; - opTime = { ts: 7020546605669417497, t: 10 }; - bulkResult.opTime = lastOp; - result.opTime = opTime; - }); - - it('replaces the opTime with the new opTime', function () { - mergeBatchResults(batch, bulkResult, null, result); - expect(bulkResult.opTime).to.deep.equal(opTime); - }); - }); - - context('when the ts is equal', function () { - context('when the t is greater', function () { - before(function () { - lastOp = { ts: 7020546605669417496, t: 10 }; - opTime = { ts: 7020546605669417496, t: 20 }; - bulkResult.opTime = lastOp; - result.opTime = opTime; - }); - - it('replaces the opTime with the new opTime', function () { - mergeBatchResults(batch, bulkResult, null, result); - expect(bulkResult.opTime).to.deep.equal(opTime); - }); - }); - - context('when the t is equal', function () { - before(function () { - lastOp = { ts: 7020546605669417496, t: 10 }; - opTime = { ts: 7020546605669417496, t: 10 }; - bulkResult.opTime = lastOp; - result.opTime = opTime; - }); - - it('does not replace the opTime with the new opTime', function () { - mergeBatchResults(batch, bulkResult, null, result); - expect(bulkResult.opTime).to.deep.equal(lastOp); - }); - }); - - context('when the t is less', function () { - before(function () { - lastOp = { ts: 7020546605669417496, t: 10 }; - opTime = { ts: 7020546605669417496, t: 5 }; - bulkResult.opTime = lastOp; - result.opTime = opTime; - }); - - it('does not replace the opTime with the new opTime', function () { - mergeBatchResults(batch, bulkResult, null, result); - expect(bulkResult.opTime).to.deep.equal(lastOp); - }); - }); - }); - - context('when the ts is less', function () { - before(function () { - lastOp = { ts: 7020546605669417496, t: 10 }; - opTime = { ts: 7020546605669417495, t: 10 }; - bulkResult.opTime = lastOp; - result.opTime = opTime; - }); - - it('does not replace the opTime with the new opTime', function () { - mergeBatchResults(batch, bulkResult, null, result); - expect(bulkResult.opTime).to.deep.equal(lastOp); - }); - }); - }); - }); - }); -});