diff --git a/src/change_stream.ts b/src/change_stream.ts index ee6876acc1..d4b1c7eec8 100644 --- a/src/change_stream.ts +++ b/src/change_stream.ts @@ -82,7 +82,7 @@ export type OperationTime = Timestamp; * Options that can be passed to a ChangeStream. Note that startAfter, resumeAfter, and startAtOperationTime are all mutually exclusive, and the server will error if more than one is specified. * @public */ -export interface ChangeStreamOptions extends AggregateOptions { +export interface ChangeStreamOptions extends Omit { /** * Allowed values: 'updateLookup', 'whenAvailable', 'required'. * @@ -533,7 +533,14 @@ export class ChangeStream< TChange extends Document = ChangeStreamDocument > extends TypedEventEmitter> { pipeline: Document[]; - options: ChangeStreamOptions; + /** + * @remarks WriteConcern can still be present on the options because + * we inherit options from the client/db/collection. The + * key must be present on the options in order to delete it. + * This allows typescript to delete the key but will + * not allow a writeConcern to be assigned as a property on options. + */ + options: ChangeStreamOptions & { writeConcern?: never }; parent: MongoClient | Db | Collection; namespace: MongoDBNamespace; type: symbol; @@ -586,7 +593,8 @@ export class ChangeStream< super(); this.pipeline = pipeline; - this.options = options; + this.options = { ...options }; + delete this.options.writeConcern; if (parent instanceof Collection) { this.type = CHANGE_DOMAIN_TYPES.COLLECTION; diff --git a/src/operations/aggregate.ts b/src/operations/aggregate.ts index a48a89ca5b..ea0b7fa438 100644 --- a/src/operations/aggregate.ts +++ b/src/operations/aggregate.ts @@ -44,7 +44,7 @@ export class AggregateOperation extends CommandOperation { constructor(ns: MongoDBNamespace, pipeline: Document[], options?: AggregateOptions) { super(undefined, { ...options, dbName: ns.db }); - this.options = options ?? {}; + this.options = { ...options }; // Covers when ns.collection is null, undefined or the empty string, use DB_AGGREGATE_COLLECTION this.target = ns.collection || DB_AGGREGATE_COLLECTION; @@ -65,6 +65,8 @@ export class AggregateOperation extends CommandOperation { if (this.hasWriteStage) { this.trySecondaryWrite = true; + } else { + delete this.options.writeConcern; } if (this.explain && this.writeConcern) { diff --git a/src/operations/find.ts b/src/operations/find.ts index 33aedd6b5f..f1e98eb4d3 100644 --- a/src/operations/find.ts +++ b/src/operations/find.ts @@ -14,7 +14,8 @@ import { Aspect, defineAspects, Hint } from './operation'; * @typeParam TSchema - Unused schema definition, deprecated usage, only specify `FindOptions` with no generic */ // eslint-disable-next-line @typescript-eslint/no-unused-vars -export interface FindOptions extends CommandOperationOptions { +export interface FindOptions + extends Omit { /** Sets the limit of documents returned in the query. */ limit?: number; /** Set to sort the documents coming back from the query. Array of indexes, `[['a', 1]]` etc. */ @@ -66,7 +67,14 @@ export interface FindOptions extends Comman /** @internal */ export class FindOperation extends CommandOperation { - override options: FindOptions; + /** + * @remarks WriteConcern can still be present on the options because + * we inherit options from the client/db/collection. The + * key must be present on the options in order to delete it. + * This allows typescript to delete the key but will + * not allow a writeConcern to be assigned as a property on options. + */ + override options: FindOptions & { writeConcern?: never }; filter: Document; constructor( @@ -77,7 +85,8 @@ export class FindOperation extends CommandOperation { ) { super(collection, options); - this.options = options; + this.options = { ...options }; + delete this.options.writeConcern; this.ns = ns; if (typeof filter !== 'object' || Array.isArray(filter)) { diff --git a/src/operations/indexes.ts b/src/operations/indexes.ts index 8d095e1650..f8690836df 100644 --- a/src/operations/indexes.ts +++ b/src/operations/indexes.ts @@ -102,7 +102,7 @@ export interface IndexDescription } /** @public */ -export interface CreateIndexesOptions extends CommandOperationOptions { +export interface CreateIndexesOptions extends Omit { /** Creates the index in the background, yielding whenever possible. */ background?: boolean; /** Creates an unique index. */ @@ -382,20 +382,28 @@ export class DropIndexesOperation extends DropIndexOperation { } /** @public */ -export interface ListIndexesOptions extends CommandOperationOptions { +export interface ListIndexesOptions extends Omit { /** The batchSize for the returned command cursor or if pre 2.8 the systems batch collection */ batchSize?: number; } /** @internal */ export class ListIndexesOperation extends CommandOperation { - override options: ListIndexesOptions; + /** + * @remarks WriteConcern can still be present on the options because + * we inherit options from the client/db/collection. The + * key must be present on the options in order to delete it. + * This allows typescript to delete the key but will + * not allow a writeConcern to be assigned as a property on options. + */ + override options: ListIndexesOptions & { writeConcern?: never }; collectionNamespace: MongoDBNamespace; constructor(collection: Collection, options?: ListIndexesOptions) { super(collection, options); - this.options = options ?? {}; + this.options = { ...options }; + delete this.options.writeConcern; this.collectionNamespace = collection.s.namespace; } diff --git a/src/operations/list_collections.ts b/src/operations/list_collections.ts index ae1c15ec20..f9cb7d57ac 100644 --- a/src/operations/list_collections.ts +++ b/src/operations/list_collections.ts @@ -7,7 +7,7 @@ import { CommandOperation, CommandOperationOptions } from './command'; import { Aspect, defineAspects } from './operation'; /** @public */ -export interface ListCollectionsOptions extends CommandOperationOptions { +export interface ListCollectionsOptions extends Omit { /** Since 4.0: If true, will only return the collection name in the response, and will omit additional info */ nameOnly?: boolean; /** Since 4.0: If true and nameOnly is true, allows a user without the required privilege (i.e. listCollections action on the database) to run the command when access control is enforced. */ @@ -18,7 +18,14 @@ export interface ListCollectionsOptions extends CommandOperationOptions { /** @internal */ export class ListCollectionsOperation extends CommandOperation { - override options: ListCollectionsOptions; + /** + * @remarks WriteConcern can still be present on the options because + * we inherit options from the client/db/collection. The + * key must be present on the options in order to delete it. + * This allows typescript to delete the key but will + * not allow a writeConcern to be assigned as a property on options. + */ + override options: ListCollectionsOptions & { writeConcern?: never }; db: Db; filter: Document; nameOnly: boolean; @@ -28,7 +35,8 @@ export class ListCollectionsOperation extends CommandOperation { constructor(db: Db, filter: Document, options?: ListCollectionsOptions) { super(db, options); - this.options = options ?? {}; + this.options = { ...options }; + delete this.options.writeConcern; this.db = db; this.filter = filter; this.nameOnly = !!this.options.nameOnly; diff --git a/test/integration/read-write-concern/write_concern.test.js b/test/integration/read-write-concern/write_concern.test.js deleted file mode 100644 index f0e17919bb..0000000000 --- a/test/integration/read-write-concern/write_concern.test.js +++ /dev/null @@ -1,75 +0,0 @@ -'use strict'; - -const { expect } = require('chai'); -const { LEGACY_HELLO_COMMAND } = require('../../mongodb'); - -const mock = require('../../tools/mongodb-mock/index'); -const { MongoClient } = require('../../mongodb'); - -describe('Write Concern', function () { - it('should respect writeConcern from uri', function (done) { - const client = this.configuration.newClient( - `${this.configuration.url()}&w=0&monitorCommands=true` - ); - const events = []; - client.on('commandStarted', event => { - if (event.commandName === 'insert') { - events.push(event); - } - }); - - expect(client.writeConcern).to.eql({ w: 0 }); - client - .db('test') - .collection('test') - .insertOne({ a: 1 }, (err, result) => { - expect(err).to.not.exist; - expect(result).to.exist; - expect(events).to.be.an('array').with.lengthOf(1); - expect(events[0]).to.containSubset({ - commandName: 'insert', - command: { - writeConcern: { w: 0 } - } - }); - client.close(done); - }); - }); - - describe('mock server write concern test', () => { - let server; - before(() => { - return mock.createServer().then(s => { - server = s; - }); - }); - - after(() => mock.cleanup()); - - // TODO: NODE-3816 - it.skip('should pipe writeConcern from client down to API call', function () { - server.setMessageHandler(request => { - if (request.document && request.document[LEGACY_HELLO_COMMAND]) { - return request.reply(mock.HELLO); - } - expect(request.document.writeConcern).to.exist; - expect(request.document.writeConcern.w).to.equal('majority'); - return request.reply({ ok: 1 }); - }); - - const uri = `mongodb://${server.uri()}`; - const client = new MongoClient(uri, { writeConcern: 'majority' }); - return client - .connect() - .then(() => { - const db = client.db('wc_test'); - const collection = db.collection('wc'); - - return collection.insertMany([{ a: 2 }]); - }) - .then(() => { - return client.close(); - }); - }); - }); -}); diff --git a/test/integration/read-write-concern/write_concern.test.ts b/test/integration/read-write-concern/write_concern.test.ts new file mode 100644 index 0000000000..9e1eb3bc6b --- /dev/null +++ b/test/integration/read-write-concern/write_concern.test.ts @@ -0,0 +1,170 @@ +import { expect } from 'chai'; +import { on, once } from 'events'; + +import { + Collection, + CommandStartedEvent, + Db, + LEGACY_HELLO_COMMAND, + MongoClient +} from '../../mongodb'; +import * as mock from '../../tools/mongodb-mock/index'; +import { filterForCommands } from '../shared'; + +describe('Write Concern', function () { + context('when the WriteConcern is set in the uri', function () { + let client; + const events: CommandStartedEvent[] = []; + beforeEach(function () { + client = this.configuration.newClient(`${this.configuration.url()}&w=0&monitorCommands=true`); + client.on('commandStarted', filterForCommands(['insert'], events)); + }); + afterEach(() => client.close()); + + it('respects the writeConcern from uri', async function () { + expect(client.writeConcern).to.deep.equal({ w: 0 }); + const result = await client.db('test').collection('test').insertOne({ a: 1 }); + expect(result).to.exist; + expect(events).to.be.an('array').with.lengthOf(1); + expect(events[0]).to.containSubset({ + commandName: 'insert', + command: { + writeConcern: { w: 0 } + } + }); + }); + }); + + describe('mock server write concern test', () => { + let server; + before(() => { + return mock.createServer().then(s => { + server = s; + }); + }); + + after(() => mock.cleanup()); + + it('should pipe writeConcern from client down to API call', function () { + server.setMessageHandler(request => { + if (request.document && request.document[LEGACY_HELLO_COMMAND]) { + return request.reply(mock.HELLO); + } + if (request.document && request.document.endSessions) { + return request.reply({ ok: 1 }); + } + expect(request.document.writeConcern).to.exist; + expect(request.document.writeConcern.w).to.equal('majority'); + return request.reply({ ok: 1 }); + }); + + const uri = `mongodb://${server.uri()}`; + const client = new MongoClient(uri, { writeConcern: { w: 'majority' } }); + return client + .connect() + .then(() => { + const db = client.db('wc_test'); + const collection = db.collection('wc'); + + return collection.insertMany([{ a: 2 }]); + }) + .then(() => { + return client.close(); + }); + }); + }); + + context('when performing read operations', function () { + context('when writeConcern = 0', function () { + describe('cursor creating operations with a getMore', function () { + let client: MongoClient; + let db: Db; + let col: Collection; + + beforeEach(async function () { + client = this.configuration.newClient({ writeConcern: { w: 0 } }); + await client.connect(); + db = client.db('writeConcernTest'); + col = db.collection('writeConcernTest'); + + const docs = Array.from({ length: 100 }, (_, i) => ({ a: i, b: i + 1 })); + await col.insertMany(docs); + }); + + afterEach(async function () { + await db.dropDatabase(); + await client.close(); + }); + + it('succeeds on find', async function () { + const findResult = col.find({}, { batchSize: 2 }); + const err = await findResult.toArray().catch(e => e); + + expect(err).to.not.be.instanceOf(Error); + }); + + it('succeeds on listCollections', async function () { + const collections = Array.from({ length: 10 }, (_, i) => `writeConcernTestCol${i + 1}`); + + await Promise.allSettled( + collections.map(colName => db.createCollection(colName).catch(() => null)) + ); + + const cols = db.listCollections({}, { batchSize: 2 }); + + const err = await cols.toArray().catch(e => e); + + expect(err).to.not.be.instanceOf(Error); + }); + + it('succeeds on aggregate', async function () { + const aggResult = col.aggregate([{ $match: { a: { $gte: 0 } } }], { batchSize: 2 }); + const err = await aggResult.toArray().catch(e => e); + + expect(err).to.not.be.instanceOf(Error); + }); + + it('succeeds on listIndexes', async function () { + await col.createIndex({ a: 1 }); + await col.createIndex({ b: -1 }); + await col.createIndex({ a: 1, b: -1 }); + + const listIndexesResult = col.listIndexes({ batchSize: 2 }); + const err = await listIndexesResult.toArray().catch(e => e); + + expect(err).to.not.be.instanceOf(Error); + }); + + it('succeeds on changeStream', { + metadata: { requires: { topology: 'replicaset' } }, + async test() { + const changeStream = col.watch(undefined, { batchSize: 2 }); + const changes = on(changeStream, 'change'); + await once(changeStream.cursor, 'init'); + + await col.insertMany( + [ + { a: 10 }, + { a: 10 }, + { a: 10 }, + { a: 10 }, + { a: 10 }, + { a: 10 }, + { a: 10 }, + { a: 10 }, + { a: 10 }, + { a: 10 }, + { a: 10 }, + { a: 10 } + ], + { writeConcern: { w: 'majority' } } + ); + + const err = await changes.next().catch(e => e); + expect(err).to.not.be.instanceOf(Error); + } + }); + }); + }); + }); +}); diff --git a/test/types/write_concern.test-d.ts b/test/types/write_concern.test-d.ts new file mode 100644 index 0000000000..b4249de86c --- /dev/null +++ b/test/types/write_concern.test-d.ts @@ -0,0 +1,13 @@ +import { expectNotAssignable } from 'tsd'; + +import type { + ChangeStreamOptions, + FindOptions, + ListCollectionsOptions, + ListIndexesOptions +} from '../mongodb'; + +expectNotAssignable({ writeConcern: { w: 0 } }); +expectNotAssignable({ writeConcern: { w: 0 } }); +expectNotAssignable({ writeConcern: { w: 0 } }); +expectNotAssignable({ writeConcern: { w: 0 } }); diff --git a/test/unit/operations/find.test.ts b/test/unit/operations/find.test.ts index 579937eba8..e2445228b7 100644 --- a/test/unit/operations/find.test.ts +++ b/test/unit/operations/find.test.ts @@ -21,15 +21,15 @@ describe('FindOperation', function () { const operation = new FindOperation(undefined, namespace, filter, options); it('sets the namespace', function () { - expect(operation.ns).to.equal(namespace); + expect(operation.ns).to.deep.equal(namespace); }); it('sets options', function () { - expect(operation.options).to.equal(options); + expect(operation.options).to.deep.equal(options); }); it('sets filter', function () { - expect(operation.filter).to.equal(filter); + expect(operation.filter).to.deep.equal(filter); }); });