Skip to content

Commit 9625b2d

Browse files
durrandurran
authored
fix(NODE-6638): throw if all atomic updates are undefined (#4519)
1 parent a8c4023 commit 9625b2d

File tree

9 files changed

+292
-27
lines changed

9 files changed

+292
-27
lines changed

‎src/bulk/common.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,7 @@ export class FindOperators {
705705

706706
/** Add a single update operation to the bulk operation */
707707
updateOne(updateDocument: Document | Document[]): BulkOperationBase {
708-
if (!hasAtomicOperators(updateDocument)) {
708+
if (!hasAtomicOperators(updateDocument, this.bulkOperation.bsonOptions)) {
709709
throw new MongoInvalidArgumentError('Update document requires atomic operators');
710710
}
711711

@@ -1115,7 +1115,7 @@ export abstract class BulkOperationBase {
11151115
...op.updateOne,
11161116
multi: false
11171117
});
1118-
if (!hasAtomicOperators(updateStatement.u)) {
1118+
if (!hasAtomicOperators(updateStatement.u, this.bsonOptions)) {
11191119
throw new MongoInvalidArgumentError('Update document requires atomic operators');
11201120
}
11211121
return this.addToOperationsList(BatchType.UPDATE, updateStatement);
@@ -1129,7 +1129,7 @@ export abstract class BulkOperationBase {
11291129
...op.updateMany,
11301130
multi: true
11311131
});
1132-
if (!hasAtomicOperators(updateStatement.u)) {
1132+
if (!hasAtomicOperators(updateStatement.u, this.bsonOptions)) {
11331133
throw new MongoInvalidArgumentError('Update document requires atomic operators');
11341134
}
11351135
return this.addToOperationsList(BatchType.UPDATE, updateStatement);

‎src/operations/client_bulk_write/command_builder.ts‎

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { BSON, type Document } from '../../bson';
1+
import { BSON, type BSONSerializeOptions, type Document } from '../../bson';
22
import { DocumentSequence } from '../../cmap/commands';
33
import { MongoAPIError, MongoInvalidArgumentError } from '../../error';
44
import { type PkFactory } from '../../mongo_client';
@@ -128,7 +128,7 @@ export class ClientBulkWriteCommandBuilder {
128128

129129
if (nsIndex != null) {
130130
// Build the operation and serialize it to get the bytes buffer.
131-
const operation = buildOperation(model, nsIndex, this.pkFactory);
131+
const operation = buildOperation(model, nsIndex, this.pkFactory, this.options);
132132
let operationBuffer;
133133
try {
134134
operationBuffer = BSON.serialize(operation);
@@ -159,7 +159,12 @@ export class ClientBulkWriteCommandBuilder {
159159
// construct our nsInfo and ops documents and buffers.
160160
namespaces.set(ns, currentNamespaceIndex);
161161
const nsInfo = { ns: ns };
162-
const operation = buildOperation(model, currentNamespaceIndex, this.pkFactory);
162+
const operation = buildOperation(
163+
model,
164+
currentNamespaceIndex,
165+
this.pkFactory,
166+
this.options
167+
);
163168
let nsInfoBuffer;
164169
let operationBuffer;
165170
try {
@@ -339,9 +344,10 @@ export interface ClientUpdateOperation {
339344
*/
340345
export const buildUpdateOneOperation = (
341346
model: ClientUpdateOneModel<Document>,
342-
index: number
347+
index: number,
348+
options: BSONSerializeOptions
343349
): ClientUpdateOperation => {
344-
return createUpdateOperation(model, index, false);
350+
return createUpdateOperation(model, index, false, options);
345351
};
346352

347353
/**
@@ -352,17 +358,18 @@ export const buildUpdateOneOperation = (
352358
*/
353359
export const buildUpdateManyOperation = (
354360
model: ClientUpdateManyModel<Document>,
355-
index: number
361+
index: number,
362+
options: BSONSerializeOptions
356363
): ClientUpdateOperation => {
357-
return createUpdateOperation(model, index, true);
364+
return createUpdateOperation(model, index, true, options);
358365
};
359366

360367
/**
361368
* Validate the update document.
362369
* @param update - The update document.
363370
*/
364-
function validateUpdate(update: Document) {
365-
if (!hasAtomicOperators(update)) {
371+
function validateUpdate(update: Document, options: BSONSerializeOptions) {
372+
if (!hasAtomicOperators(update, options)) {
366373
throw new MongoAPIError(
367374
'Client bulk write update models must only contain atomic modifiers (start with $) and must not be empty.'
368375
);
@@ -375,13 +382,14 @@ function validateUpdate(update: Document) {
375382
function createUpdateOperation(
376383
model: ClientUpdateOneModel<Document> | ClientUpdateManyModel<Document>,
377384
index: number,
378-
multi: boolean
385+
multi: boolean,
386+
options: BSONSerializeOptions
379387
): ClientUpdateOperation {
380388
// Update documents provided in UpdateOne and UpdateMany write models are
381389
// required only to contain atomic modifiers (i.e. keys that start with "$").
382390
// Drivers MUST throw an error if an update document is empty or if the
383391
// document's first key does not start with "$".
384-
validateUpdate(model.update);
392+
validateUpdate(model.update, options);
385393
const document: ClientUpdateOperation = {
386394
update: index,
387395
multi: multi,
@@ -459,7 +467,8 @@ export const buildReplaceOneOperation = (
459467
export function buildOperation(
460468
model: AnyClientBulkWriteModel<Document>,
461469
index: number,
462-
pkFactory: PkFactory
470+
pkFactory: PkFactory,
471+
options: BSONSerializeOptions
463472
): Document {
464473
switch (model.name) {
465474
case 'insertOne':
@@ -469,9 +478,9 @@ export function buildOperation(
469478
case 'deleteMany':
470479
return buildDeleteManyOperation(model, index);
471480
case 'updateOne':
472-
return buildUpdateOneOperation(model, index);
481+
return buildUpdateOneOperation(model, index, options);
473482
case 'updateMany':
474-
return buildUpdateManyOperation(model, index);
483+
return buildUpdateManyOperation(model, index, options);
475484
case 'replaceOne':
476485
return buildReplaceOneOperation(model, index);
477486
}

‎src/operations/find_and_modify.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ export class FindOneAndUpdateOperation extends FindAndModifyOperation {
273273
throw new MongoInvalidArgumentError('Argument "update" must be an object');
274274
}
275275

276-
if (!hasAtomicOperators(update)) {
276+
if (!hasAtomicOperators(update, options)) {
277277
throw new MongoInvalidArgumentError('Update document requires atomic operators');
278278
}
279279

‎src/operations/update.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ export class UpdateOneOperation extends UpdateOperation {
144144
options
145145
);
146146

147-
if (!hasAtomicOperators(update)) {
147+
if (!hasAtomicOperators(update, options)) {
148148
throw new MongoInvalidArgumentError('Update document requires atomic operators');
149149
}
150150
}
@@ -179,7 +179,7 @@ export class UpdateManyOperation extends UpdateOperation {
179179
options
180180
);
181181

182-
if (!hasAtomicOperators(update)) {
182+
if (!hasAtomicOperators(update, options)) {
183183
throw new MongoInvalidArgumentError('Update document requires atomic operators');
184184
}
185185
}

‎src/utils.ts‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,10 @@ export function calculateDurationInMs(started: number | undefined): number {
476476
}
477477

478478
/** @internal */
479-
export function hasAtomicOperators(doc: Document | Document[]): boolean {
479+
export function hasAtomicOperators(
480+
doc: Document | Document[],
481+
options?: CommandOperationOptions
482+
): boolean {
480483
if (Array.isArray(doc)) {
481484
for (const document of doc) {
482485
if (hasAtomicOperators(document)) {
@@ -487,6 +490,23 @@ export function hasAtomicOperators(doc: Document | Document[]): boolean {
487490
}
488491

489492
const keys = Object.keys(doc);
493+
// In this case we need to throw if all the atomic operators are undefined.
494+
if (options?.ignoreUndefined) {
495+
let allUndefined = true;
496+
for (const key of keys) {
497+
// eslint-disable-next-line no-restricted-syntax
498+
if (doc[key] !== undefined) {
499+
allUndefined = false;
500+
break;
501+
}
502+
}
503+
if (allUndefined) {
504+
throw new MongoInvalidArgumentError(
505+
'Update operations require that all atomic operators have defined values, but none were provided.'
506+
);
507+
}
508+
}
509+
490510
return keys.length > 0 && keys[0][0] === '$';
491511
}
492512

‎test/integration/crud/bulk.test.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,56 @@ describe('Bulk', function () {
4646
client = null;
4747
});
4848

49+
describe('#bulkWrite', function () {
50+
context('when including an update with all undefined atomic operators', function () {
51+
context('when ignoreUndefined is true', function () {
52+
context('when performing an update many', function () {
53+
it('throws an error', async function () {
54+
const collection = client.db('test').collection('test');
55+
const error = await collection
56+
.bulkWrite(
57+
[
58+
{
59+
updateMany: {
60+
filter: { age: { $lte: 5 } },
61+
update: { $set: undefined, $unset: undefined }
62+
}
63+
}
64+
],
65+
{ ignoreUndefined: true }
66+
)
67+
.catch(error => error);
68+
expect(error.message).to.include(
69+
'Update operations require that all atomic operators have defined values, but none were provided'
70+
);
71+
});
72+
});
73+
74+
context('when performing an update one', function () {
75+
it('throws an error', async function () {
76+
const collection = client.db('test').collection('test');
77+
const error = await collection
78+
.bulkWrite(
79+
[
80+
{
81+
updateOne: {
82+
filter: { age: { $lte: 5 } },
83+
update: { $set: undefined, $unset: undefined }
84+
}
85+
}
86+
],
87+
{ ignoreUndefined: true }
88+
)
89+
.catch(error => error);
90+
expect(error.message).to.include(
91+
'Update operations require that all atomic operators have defined values, but none were provided'
92+
);
93+
});
94+
});
95+
});
96+
});
97+
});
98+
4999
describe('BulkOperationBase', () => {
50100
describe('#raw()', function () {
51101
context('when called with an undefined operation', function () {

‎test/integration/crud/client_bulk_write.test.ts‎

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,63 @@ describe('Client Bulk Write', function () {
3131

3232
afterEach(async function () {
3333
await client?.close();
34-
await clearFailPoint(this.configuration);
34+
await clearFailPoint(this.configuration).catch(() => null);
35+
});
36+
37+
describe('#bulkWrite', function () {
38+
context('when ignoreUndefined is true', function () {
39+
context('when including an update with all undefined atomic operators', function () {
40+
context('when performing an update many', function () {
41+
beforeEach(async function () {
42+
client = this.configuration.newClient();
43+
});
44+
45+
it('throws an error', async function () {
46+
const error = await client
47+
.bulkWrite(
48+
[
49+
{
50+
name: 'updateMany',
51+
namespace: 'foo.bar',
52+
filter: { age: { $lte: 5 } },
53+
update: { $set: undefined, $unset: undefined }
54+
}
55+
],
56+
{ ignoreUndefined: true }
57+
)
58+
.catch(error => error);
59+
expect(error.message).to.include(
60+
'Update operations require that all atomic operators have defined values, but none were provided'
61+
);
62+
});
63+
});
64+
65+
context('when performing an update one', function () {
66+
beforeEach(async function () {
67+
client = this.configuration.newClient();
68+
});
69+
70+
it('throws an error', async function () {
71+
const error = await client
72+
.bulkWrite(
73+
[
74+
{
75+
name: 'updateOne',
76+
namespace: 'foo.bar',
77+
filter: { age: { $lte: 5 } },
78+
update: { $set: undefined, $unset: undefined }
79+
}
80+
],
81+
{ ignoreUndefined: true }
82+
)
83+
.catch(error => error);
84+
expect(error.message).to.include(
85+
'Update operations require that all atomic operators have defined values, but none were provided'
86+
);
87+
});
88+
});
89+
});
90+
});
3591
});
3692

3793
describe('CSOT enabled', function () {

0 commit comments

Comments
 (0)