Skip to content

Commit fbbf54a

Browse files
addaleaxaddaleax
authored
fix(shell-api): allow pipeline-style bulk update operations MONGOSH-1281 (#1347)
Fix cloning of the update documents to allow arrays as well as objects, so that users can use aggregation pipelines for bulk updates. Also, remove the buggy `hint` and `arrayFilters` implementation that was storing extra properties in the wrong place on the outgoing documents, and instead fully rely on driver helpers to implement these.
1 parent 62c57b8 commit fbbf54a

File tree

4 files changed

+78
-35
lines changed

4 files changed

+78
-35
lines changed

‎packages/shell-api/src/bulk.spec.ts‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -371,12 +371,10 @@ describe('Bulk API', () => {
371371

372372
it('calls serviceProviderBulkOp.update and returns parent when hint/arrayFilter set', () => {
373373
bulkFindOp.hint({ hint: 1 });
374-
// bulkFindOp.arrayFilters(['filter']);
374+
bulkFindOp.arrayFilters([{ x: 1 }]);
375375
bulkFindOp.update({ updateDoc: 1 });
376376
expect(innerStub.update).to.have.been.calledWith({
377-
updateDoc: 1,
378-
hint: { hint: 1 },
379-
// arrayFilters: [ 'filter' ]
377+
updateDoc: 1
380378
});
381379
expect(bulk._batchCounts.nUpdateOps).to.equal(1);
382380
});
@@ -400,12 +398,10 @@ describe('Bulk API', () => {
400398

401399
it('calls serviceProviderBulkOp.updateOne and returns parent when hint/arrayFilter set', () => {
402400
bulkFindOp.hint({ hint: 1 });
403-
// bulkFindOp.arrayFilters(['filter']);
401+
bulkFindOp.arrayFilters([{ x: 1 }]);
404402
bulkFindOp.updateOne({ updateOneDoc: 1 });
405403
expect(innerStub.updateOne).to.have.been.calledWith({
406-
updateOneDoc: 1,
407-
hint: { hint: 1 },
408-
// arrayFilters: [ 'filter' ]
404+
updateOneDoc: 1
409405
});
410406
expect(bulk._batchCounts.nUpdateOps).to.equal(1);
411407
});
@@ -432,8 +428,7 @@ describe('Bulk API', () => {
432428
bulkFindOp.hint({ hint: 1 });
433429
bulkFindOp.replaceOne({ replaceOneDoc: 1 });
434430
expect(innerStub.replaceOne).to.have.been.calledWith({
435-
replaceOneDoc: 1,
436-
hint: { hint: 1 }
431+
replaceOneDoc: 1
437432
});
438433
expect(bulk._batchCounts.nUpdateOps).to.equal(1);
439434
});
@@ -452,7 +447,7 @@ describe('Bulk API', () => {
452447
it('sets the attribute and returns self', () => {
453448
const attr = { hint: 1 };
454449
expect(bulkFindOp.hint(attr)).to.equal(bulkFindOp);
455-
expect(bulkFindOp._hint).to.deep.equal(attr);
450+
expect(innerStub.hint).to.have.been.calledWith(attr);
456451
});
457452
});
458453
describe('arrayFilters', () => {

‎packages/shell-api/src/bulk.ts‎

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,14 @@ import {
1111
CollationOptions
1212
} from '@mongosh/service-provider-core';
1313
import { asPrintable } from './enums';
14-
import { assertArgsDefinedType } from './helpers';
14+
import { assertArgsDefinedType, shallowClone } from './helpers';
1515
import { BulkWriteResult } from './result';
1616
import type Collection from './collection';
1717

1818
@shellApiClassDefault
1919
export class BulkFindOp extends ShellApiWithMongoClass {
2020
_serviceProviderBulkFindOp: FindOperators;
2121
_parentBulk: Bulk;
22-
_hint: Document | undefined;
23-
_arrayFilters: Document[] | undefined;
2422
constructor(innerFind: FindOperators, parentBulk: Bulk) {
2523
super();
2624
this._serviceProviderBulkFindOp = innerFind;
@@ -53,7 +51,7 @@ export class BulkFindOp extends ShellApiWithMongoClass {
5351
@apiVersions([1])
5452
hint(hintDoc: Document): BulkFindOp {
5553
assertArgsDefinedType([hintDoc], [true], 'BulkFindOp.hint');
56-
this._hint = hintDoc;
54+
this._serviceProviderBulkFindOp.hint(hintDoc);
5755
return this;
5856
}
5957

@@ -92,41 +90,26 @@ export class BulkFindOp extends ShellApiWithMongoClass {
9290
replaceOne(replacement: Document): Bulk {
9391
this._parentBulk._batchCounts.nUpdateOps++;
9492
assertArgsDefinedType([replacement], [true], 'BulkFindOp.replacement');
95-
const op = { ...replacement };
96-
if (this._hint) {
97-
op.hint = this._hint;
98-
}
93+
const op = shallowClone(replacement);
9994
this._serviceProviderBulkFindOp.replaceOne(op);
10095
return this._parentBulk;
10196
}
10297

10398
@returnType('Bulk')
10499
@apiVersions([1])
105-
updateOne(update: Document): Bulk {
100+
updateOne(update: Document | Document[]): Bulk {
106101
this._parentBulk._batchCounts.nUpdateOps++;
107102
assertArgsDefinedType([update], [true], 'BulkFindOp.update');
108-
const op = { ...update };
109-
if (this._hint) {
110-
op.hint = this._hint;
111-
}
112-
if (this._arrayFilters) {
113-
op.arrayFilters = this._arrayFilters;
114-
}
103+
const op = shallowClone(update);
115104
this._serviceProviderBulkFindOp.updateOne(op);
116105
return this._parentBulk;
117106
}
118107

119108
@returnType('Bulk')
120-
update(update: Document): Bulk {
109+
update(update: Document | Document[]): Bulk {
121110
this._parentBulk._batchCounts.nUpdateOps++;
122111
assertArgsDefinedType([update], [true], 'BulkFindOp.update');
123-
const op = { ...update };
124-
if (this._hint) {
125-
op.hint = this._hint;
126-
}
127-
if (this._arrayFilters) {
128-
op.arrayFilters = this._arrayFilters;
129-
}
112+
const op = shallowClone(update);
130113
this._serviceProviderBulkFindOp.update(op);
131114
return this._parentBulk;
132115
}

‎packages/shell-api/src/helpers.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -804,3 +804,8 @@ improvements and to suggest MongoDB products and deployment options to you.
804804
To enable free monitoring, run the following command: db.enableFreeMonitoring()
805805
To permanently disable this reminder, run the following command: db.disableFreeMonitoring()
806806
`;
807+
808+
export function shallowClone<T>(input: T): T {
809+
if (!input || typeof input !== 'object') return input;
810+
return Array.isArray(input) ? ([...input] as unknown as T) : { ...input };
811+
}

‎packages/shell-api/src/integration.spec.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1855,6 +1855,66 @@ describe('Shell API (integration)', function() {
18551855
expect(op.operations.length).to.equal(1);
18561856
});
18571857
});
1858+
context('with >= 4.2 server', () => {
1859+
skipIfServerVersion(testServer, '< 4.2');
1860+
describe('update() with pipeline update', () => {
1861+
beforeEach(async() => {
1862+
bulk = await collection[m]();
1863+
for (let i = 0; i < size; i++) {
1864+
await collection.insertOne({ x: i });
1865+
}
1866+
expect(await collection.countDocuments()).to.equal(size);
1867+
expect(await collection.countDocuments({ y: { $exists: true } })).to.equal(0);
1868+
bulk.find({ y: 0 }).upsert().update([{ $set: { y: 1 } }]);
1869+
await bulk.execute();
1870+
});
1871+
afterEach(async() => {
1872+
await collection.drop();
1873+
});
1874+
it('toJSON returns correctly', () => {
1875+
expect(bulk.toJSON()).to.deep.equal({ nInsertOps: 0, nUpdateOps: 1, nRemoveOps: 0, nBatches: 1 });
1876+
});
1877+
it('executes', async() => {
1878+
expect(await collection.countDocuments()).to.equal(size + 1);
1879+
expect(await collection.countDocuments({ y: { $exists: true } })).to.equal(1);
1880+
});
1881+
it('getOperations returns correctly', () => {
1882+
const ops = bulk.getOperations();
1883+
expect(ops.length).to.equal(1);
1884+
const op = ops[0];
1885+
expect(op.originalZeroIndex).to.equal(0);
1886+
expect(op.batchType).to.equal(2);
1887+
expect(op.operations.length).to.equal(1);
1888+
});
1889+
});
1890+
describe('updateOne() with pipeline update', () => {
1891+
beforeEach(async() => {
1892+
bulk = await collection[m]();
1893+
for (let i = 0; i < size; i++) {
1894+
await collection.insertOne({ x: i });
1895+
}
1896+
expect(await collection.countDocuments()).to.equal(size);
1897+
expect(await collection.countDocuments({ y: { $exists: true } })).to.equal(0);
1898+
bulk.find({ y: 0 }).upsert().updateOne([{ $set: { y: 1 } }]);
1899+
await bulk.execute();
1900+
});
1901+
it('toJSON returns correctly', () => {
1902+
expect(bulk.toJSON()).to.deep.equal({ nInsertOps: 0, nUpdateOps: 1, nRemoveOps: 0, nBatches: 1 });
1903+
});
1904+
it('executes', async() => {
1905+
expect(await collection.countDocuments()).to.equal(size + 1);
1906+
expect(await collection.countDocuments({ y: { $exists: true } })).to.equal(1);
1907+
});
1908+
it('getOperations returns correctly', () => {
1909+
const ops = bulk.getOperations();
1910+
expect(ops.length).to.equal(1);
1911+
const op = ops[0];
1912+
expect(op.originalZeroIndex).to.equal(0);
1913+
expect(op.batchType).to.equal(2);
1914+
expect(op.operations.length).to.equal(1);
1915+
});
1916+
});
1917+
});
18581918
describe('error states', () => {
18591919
it('cannot be executed twice', async() => {
18601920
bulk = await collection[m]();

0 commit comments

Comments
 (0)