diff --git a/src/bulk/common.ts b/src/bulk/common.ts index da972d52f1..782b4041e3 100644 --- a/src/bulk/common.ts +++ b/src/bulk/common.ts @@ -705,7 +705,7 @@ export class FindOperators { /** Add a single update operation to the bulk operation */ updateOne(updateDocument: Document | Document[]): BulkOperationBase { - if (!hasAtomicOperators(updateDocument)) { + if (!hasAtomicOperators(updateDocument, this.bulkOperation.bsonOptions)) { throw new MongoInvalidArgumentError('Update document requires atomic operators'); } @@ -1115,7 +1115,7 @@ export abstract class BulkOperationBase { ...op.updateOne, multi: false }); - if (!hasAtomicOperators(updateStatement.u)) { + if (!hasAtomicOperators(updateStatement.u, this.bsonOptions)) { throw new MongoInvalidArgumentError('Update document requires atomic operators'); } return this.addToOperationsList(BatchType.UPDATE, updateStatement); @@ -1129,7 +1129,7 @@ export abstract class BulkOperationBase { ...op.updateMany, multi: true }); - if (!hasAtomicOperators(updateStatement.u)) { + if (!hasAtomicOperators(updateStatement.u, this.bsonOptions)) { throw new MongoInvalidArgumentError('Update document requires atomic operators'); } return this.addToOperationsList(BatchType.UPDATE, updateStatement); diff --git a/src/operations/client_bulk_write/command_builder.ts b/src/operations/client_bulk_write/command_builder.ts index 9ec3e6b029..6e937f058c 100644 --- a/src/operations/client_bulk_write/command_builder.ts +++ b/src/operations/client_bulk_write/command_builder.ts @@ -1,4 +1,4 @@ -import { BSON, type Document } from '../../bson'; +import { BSON, type BSONSerializeOptions, type Document } from '../../bson'; import { DocumentSequence } from '../../cmap/commands'; import { MongoAPIError, MongoInvalidArgumentError } from '../../error'; import { type PkFactory } from '../../mongo_client'; @@ -128,7 +128,7 @@ export class ClientBulkWriteCommandBuilder { if (nsIndex != null) { // Build the operation and serialize it to get the bytes buffer. - const operation = buildOperation(model, nsIndex, this.pkFactory); + const operation = buildOperation(model, nsIndex, this.pkFactory, this.options); let operationBuffer; try { operationBuffer = BSON.serialize(operation); @@ -159,7 +159,12 @@ export class ClientBulkWriteCommandBuilder { // construct our nsInfo and ops documents and buffers. namespaces.set(ns, currentNamespaceIndex); const nsInfo = { ns: ns }; - const operation = buildOperation(model, currentNamespaceIndex, this.pkFactory); + const operation = buildOperation( + model, + currentNamespaceIndex, + this.pkFactory, + this.options + ); let nsInfoBuffer; let operationBuffer; try { @@ -339,9 +344,10 @@ export interface ClientUpdateOperation { */ export const buildUpdateOneOperation = ( model: ClientUpdateOneModel, - index: number + index: number, + options: BSONSerializeOptions ): ClientUpdateOperation => { - return createUpdateOperation(model, index, false); + return createUpdateOperation(model, index, false, options); }; /** @@ -352,17 +358,18 @@ export const buildUpdateOneOperation = ( */ export const buildUpdateManyOperation = ( model: ClientUpdateManyModel, - index: number + index: number, + options: BSONSerializeOptions ): ClientUpdateOperation => { - return createUpdateOperation(model, index, true); + return createUpdateOperation(model, index, true, options); }; /** * Validate the update document. * @param update - The update document. */ -function validateUpdate(update: Document) { - if (!hasAtomicOperators(update)) { +function validateUpdate(update: Document, options: BSONSerializeOptions) { + if (!hasAtomicOperators(update, options)) { throw new MongoAPIError( 'Client bulk write update models must only contain atomic modifiers (start with $) and must not be empty.' ); @@ -375,13 +382,14 @@ function validateUpdate(update: Document) { function createUpdateOperation( model: ClientUpdateOneModel | ClientUpdateManyModel, index: number, - multi: boolean + multi: boolean, + options: BSONSerializeOptions ): ClientUpdateOperation { // Update documents provided in UpdateOne and UpdateMany write models are // required only to contain atomic modifiers (i.e. keys that start with "$"). // Drivers MUST throw an error if an update document is empty or if the // document's first key does not start with "$". - validateUpdate(model.update); + validateUpdate(model.update, options); const document: ClientUpdateOperation = { update: index, multi: multi, @@ -459,7 +467,8 @@ export const buildReplaceOneOperation = ( export function buildOperation( model: AnyClientBulkWriteModel, index: number, - pkFactory: PkFactory + pkFactory: PkFactory, + options: BSONSerializeOptions ): Document { switch (model.name) { case 'insertOne': @@ -469,9 +478,9 @@ export function buildOperation( case 'deleteMany': return buildDeleteManyOperation(model, index); case 'updateOne': - return buildUpdateOneOperation(model, index); + return buildUpdateOneOperation(model, index, options); case 'updateMany': - return buildUpdateManyOperation(model, index); + return buildUpdateManyOperation(model, index, options); case 'replaceOne': return buildReplaceOneOperation(model, index); } diff --git a/src/operations/find_and_modify.ts b/src/operations/find_and_modify.ts index 651bcccb62..759cb02e72 100644 --- a/src/operations/find_and_modify.ts +++ b/src/operations/find_and_modify.ts @@ -273,7 +273,7 @@ export class FindOneAndUpdateOperation extends FindAndModifyOperation { throw new MongoInvalidArgumentError('Argument "update" must be an object'); } - if (!hasAtomicOperators(update)) { + if (!hasAtomicOperators(update, options)) { throw new MongoInvalidArgumentError('Update document requires atomic operators'); } diff --git a/src/operations/update.ts b/src/operations/update.ts index d020e2cb85..f731e77945 100644 --- a/src/operations/update.ts +++ b/src/operations/update.ts @@ -144,7 +144,7 @@ export class UpdateOneOperation extends UpdateOperation { options ); - if (!hasAtomicOperators(update)) { + if (!hasAtomicOperators(update, options)) { throw new MongoInvalidArgumentError('Update document requires atomic operators'); } } @@ -179,7 +179,7 @@ export class UpdateManyOperation extends UpdateOperation { options ); - if (!hasAtomicOperators(update)) { + if (!hasAtomicOperators(update, options)) { throw new MongoInvalidArgumentError('Update document requires atomic operators'); } } diff --git a/src/utils.ts b/src/utils.ts index 4a436c2a35..09e86b7349 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -476,7 +476,10 @@ export function calculateDurationInMs(started: number | undefined): number { } /** @internal */ -export function hasAtomicOperators(doc: Document | Document[]): boolean { +export function hasAtomicOperators( + doc: Document | Document[], + options?: CommandOperationOptions +): boolean { if (Array.isArray(doc)) { for (const document of doc) { if (hasAtomicOperators(document)) { @@ -487,6 +490,23 @@ export function hasAtomicOperators(doc: Document | Document[]): boolean { } const keys = Object.keys(doc); + // In this case we need to throw if all the atomic operators are undefined. + if (options?.ignoreUndefined) { + let allUndefined = true; + for (const key of keys) { + // eslint-disable-next-line no-restricted-syntax + if (doc[key] !== undefined) { + allUndefined = false; + break; + } + } + if (allUndefined) { + throw new MongoInvalidArgumentError( + 'Update operations require that all atomic operators have defined values, but none were provided.' + ); + } + } + return keys.length > 0 && keys[0][0] === '$'; } diff --git a/test/integration/crud/bulk.test.ts b/test/integration/crud/bulk.test.ts index f32b340a3e..dfeb45ce62 100644 --- a/test/integration/crud/bulk.test.ts +++ b/test/integration/crud/bulk.test.ts @@ -46,6 +46,56 @@ describe('Bulk', function () { client = null; }); + describe('#bulkWrite', function () { + context('when including an update with all undefined atomic operators', function () { + context('when ignoreUndefined is true', function () { + context('when performing an update many', function () { + it('throws an error', async function () { + const collection = client.db('test').collection('test'); + const error = await collection + .bulkWrite( + [ + { + updateMany: { + filter: { age: { $lte: 5 } }, + update: { $set: undefined, $unset: undefined } + } + } + ], + { ignoreUndefined: true } + ) + .catch(error => error); + expect(error.message).to.include( + 'Update operations require that all atomic operators have defined values, but none were provided' + ); + }); + }); + + context('when performing an update one', function () { + it('throws an error', async function () { + const collection = client.db('test').collection('test'); + const error = await collection + .bulkWrite( + [ + { + updateOne: { + filter: { age: { $lte: 5 } }, + update: { $set: undefined, $unset: undefined } + } + } + ], + { ignoreUndefined: true } + ) + .catch(error => error); + expect(error.message).to.include( + 'Update operations require that all atomic operators have defined values, but none were provided' + ); + }); + }); + }); + }); + }); + describe('BulkOperationBase', () => { describe('#raw()', function () { context('when called with an undefined operation', function () { diff --git a/test/integration/crud/client_bulk_write.test.ts b/test/integration/crud/client_bulk_write.test.ts index 5acf6029fe..c2a626fe9d 100644 --- a/test/integration/crud/client_bulk_write.test.ts +++ b/test/integration/crud/client_bulk_write.test.ts @@ -31,7 +31,63 @@ describe('Client Bulk Write', function () { afterEach(async function () { await client?.close(); - await clearFailPoint(this.configuration); + await clearFailPoint(this.configuration).catch(() => null); + }); + + describe('#bulkWrite', function () { + context('when ignoreUndefined is true', function () { + context('when including an update with all undefined atomic operators', function () { + context('when performing an update many', function () { + beforeEach(async function () { + client = this.configuration.newClient(); + }); + + it('throws an error', async function () { + const error = await client + .bulkWrite( + [ + { + name: 'updateMany', + namespace: 'foo.bar', + filter: { age: { $lte: 5 } }, + update: { $set: undefined, $unset: undefined } + } + ], + { ignoreUndefined: true } + ) + .catch(error => error); + expect(error.message).to.include( + 'Update operations require that all atomic operators have defined values, but none were provided' + ); + }); + }); + + context('when performing an update one', function () { + beforeEach(async function () { + client = this.configuration.newClient(); + }); + + it('throws an error', async function () { + const error = await client + .bulkWrite( + [ + { + name: 'updateOne', + namespace: 'foo.bar', + filter: { age: { $lte: 5 } }, + update: { $set: undefined, $unset: undefined } + } + ], + { ignoreUndefined: true } + ) + .catch(error => error); + expect(error.message).to.include( + 'Update operations require that all atomic operators have defined values, but none were provided' + ); + }); + }); + }); + }); }); describe('CSOT enabled', function () { diff --git a/test/integration/crud/crud_api.test.ts b/test/integration/crud/crud_api.test.ts index 2f82c6164c..5ad2aa4108 100644 --- a/test/integration/crud/crud_api.test.ts +++ b/test/integration/crud/crud_api.test.ts @@ -16,7 +16,6 @@ import { } from '../../mongodb'; import { type FailPoint } from '../../tools/utils'; import { assert as test } from '../shared'; - // instanceof cannot be use reliably to detect the new models in js due to scoping and new // contexts killing class info find/distinct/count thus cannot be overloaded without breaking // backwards compatibility in a fundamental way @@ -896,17 +895,86 @@ describe('CRUD API', function () { }); }); + describe('#updateOne', function () { + let collection; + + beforeEach(async function () { + collection = client.db().collection('updateOneTest'); + }); + + context( + 'when including an update with all undefined atomic operators ignoring undefined', + function () { + beforeEach(async function () { + client = this.configuration.newClient(); + }); + + it('throws an error', async function () { + const error = await collection + .updateOne({ a: 1 }, { $set: undefined, $unset: undefined }, { ignoreUndefined: true }) + .catch(error => error); + expect(error.message).to.include( + 'Update operations require that all atomic operators have defined values, but none were provided' + ); + }); + } + ); + }); + + describe('#updateMany', function () { + let collection; + + beforeEach(async function () { + collection = client.db().collection('updateManyTest'); + }); + + context( + 'when including an update with all undefined atomic operators ignoring undefined', + function () { + beforeEach(async function () { + client = this.configuration.newClient(); + }); + + it('throws an error', async function () { + const error = await collection + .updateMany({ a: 1 }, { $set: undefined, $unset: undefined }, { ignoreUndefined: true }) + .catch(error => error); + expect(error.message).to.include( + 'Update operations require that all atomic operators have defined values, but none were provided' + ); + }); + } + ); + }); + describe('#findOneAndUpdate', function () { let collection; beforeEach(async function () { - await client.connect(); collection = client.db().collection('findAndModifyTest'); }); - afterEach(async function () { - await collection.drop(); - }); + context( + 'when including an update with all undefined atomic operators ignoring undefined', + function () { + beforeEach(async function () { + client = this.configuration.newClient(); + }); + + it('throws an error', async function () { + const error = await collection + .findOneAndUpdate( + { a: 1 }, + { $set: undefined, $unset: undefined }, + { ignoreUndefined: true } + ) + .catch(error => error); + expect(error.message).to.include( + 'Update operations require that all atomic operators have defined values, but none were provided' + ); + }); + } + ); context('when includeResultMetadata is true', function () { beforeEach(async function () { diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts index 26c6211075..6fa7e9784a 100644 --- a/test/unit/utils.test.ts +++ b/test/unit/utils.test.ts @@ -11,6 +11,7 @@ import { compareObjectId, decorateWithExplain, Explain, + hasAtomicOperators, HostAddress, hostMatchesWildcards, isHello, @@ -19,6 +20,7 @@ import { List, MongoDBCollectionNamespace, MongoDBNamespace, + MongoInvalidArgumentError, MongoRuntimeError, ObjectId, shuffle @@ -26,6 +28,66 @@ import { import { sleep } from '../tools/utils'; describe('driver utils', function () { + describe('.hasAtomicOperators', function () { + context('when ignoreUndefined is true', function () { + const options = { ignoreUndefined: true }; + + context('when no operator is undefined', function () { + const document = { $set: { n: 1 }, $unset: '' }; + + it('returns true', function () { + expect(hasAtomicOperators(document, options)).to.be.true; + }); + }); + + context('when some operators are undefined', function () { + const document = { $set: { n: 1 }, $unset: undefined }; + + it('returns true', function () { + expect(hasAtomicOperators(document, options)).to.be.true; + }); + }); + + context('when all operators are undefined', function () { + const document = { $set: undefined, $unset: undefined }; + + it('throws an error', function () { + expect(() => { + hasAtomicOperators(document, options); + }).to.throw(MongoInvalidArgumentError); + }); + }); + }); + + context('when ignoreUndefined is false', function () { + const options = { ignoreUndefined: false }; + + context('when no operator is undefined', function () { + const document = { $set: { n: 1 }, $unset: '' }; + + it('returns true', function () { + expect(hasAtomicOperators(document, options)).to.be.true; + }); + }); + + context('when some operators are undefined', function () { + const document = { $set: { n: 1 }, $unset: undefined }; + + it('returns true', function () { + expect(hasAtomicOperators(document, options)).to.be.true; + }); + }); + + context('when all operators are undefined', function () { + const document = { $set: undefined, $unset: undefined }; + + it('returns true', function () { + expect(hasAtomicOperators(document, options)).to.be.true; + }); + }); + }); + }); + describe('.hostMatchesWildcards', function () { context('when using domains', function () { context('when using exact match', function () {