Skip to content

Commit

Permalink
fix(shell-api): allow pipeline-style bulk update operations MONGOSH-1281
Browse files Browse the repository at this point in the history 
 (#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.
  • Loading branch information
@addaleax
addaleax authored Sep 19, 2022
1 parent 62c57b8 commit fbbf54a
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 35 deletions.
17 changes: 6 additions & 11 deletions packages/shell-api/src/bulk.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,12 +371,10 @@ describe('Bulk API', () => {

it('calls serviceProviderBulkOp.update and returns parent when hint/arrayFilter set', () => {
bulkFindOp.hint({ hint: 1 });
// bulkFindOp.arrayFilters(['filter']);
bulkFindOp.arrayFilters([{ x: 1 }]);
bulkFindOp.update({ updateDoc: 1 });
expect(innerStub.update).to.have.been.calledWith({
updateDoc: 1,
hint: { hint: 1 },
// arrayFilters: [ 'filter' ]
updateDoc: 1
});
expect(bulk._batchCounts.nUpdateOps).to.equal(1);
});
Expand All @@ -400,12 +398,10 @@ describe('Bulk API', () => {

it('calls serviceProviderBulkOp.updateOne and returns parent when hint/arrayFilter set', () => {
bulkFindOp.hint({ hint: 1 });
// bulkFindOp.arrayFilters(['filter']);
bulkFindOp.arrayFilters([{ x: 1 }]);
bulkFindOp.updateOne({ updateOneDoc: 1 });
expect(innerStub.updateOne).to.have.been.calledWith({
updateOneDoc: 1,
hint: { hint: 1 },
// arrayFilters: [ 'filter' ]
updateOneDoc: 1
});
expect(bulk._batchCounts.nUpdateOps).to.equal(1);
});
Expand All @@ -432,8 +428,7 @@ describe('Bulk API', () => {
bulkFindOp.hint({ hint: 1 });
bulkFindOp.replaceOne({ replaceOneDoc: 1 });
expect(innerStub.replaceOne).to.have.been.calledWith({
replaceOneDoc: 1,
hint: { hint: 1 }
replaceOneDoc: 1
});
expect(bulk._batchCounts.nUpdateOps).to.equal(1);
});
Expand All @@ -452,7 +447,7 @@ describe('Bulk API', () => {
it('sets the attribute and returns self', () => {
const attr = { hint: 1 };
expect(bulkFindOp.hint(attr)).to.equal(bulkFindOp);
expect(bulkFindOp._hint).to.deep.equal(attr);
expect(innerStub.hint).to.have.been.calledWith(attr);
});
});
describe('arrayFilters', () => {
Expand Down
31 changes: 7 additions & 24 deletions packages/shell-api/src/bulk.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,14 @@ import {
CollationOptions
} from '@mongosh/service-provider-core';
import { asPrintable } from './enums';
import { assertArgsDefinedType } from './helpers';
import { assertArgsDefinedType, shallowClone } from './helpers';
import { BulkWriteResult } from './result';
import type Collection from './collection';

@shellApiClassDefault
export class BulkFindOp extends ShellApiWithMongoClass {
_serviceProviderBulkFindOp: FindOperators;
_parentBulk: Bulk;
_hint: Document | undefined;
_arrayFilters: Document[] | undefined;
constructor(innerFind: FindOperators, parentBulk: Bulk) {
super();
this._serviceProviderBulkFindOp = innerFind;
Expand Down Expand Up @@ -53,7 +51,7 @@ export class BulkFindOp extends ShellApiWithMongoClass {
@apiVersions([1])
hint(hintDoc: Document): BulkFindOp {
assertArgsDefinedType([hintDoc], [true], 'BulkFindOp.hint');
this._hint = hintDoc;
this._serviceProviderBulkFindOp.hint(hintDoc);
return this;
}

Expand Down Expand Up @@ -92,41 +90,26 @@ export class BulkFindOp extends ShellApiWithMongoClass {
replaceOne(replacement: Document): Bulk {
this._parentBulk._batchCounts.nUpdateOps++;
assertArgsDefinedType([replacement], [true], 'BulkFindOp.replacement');
const op = { ...replacement };
if (this._hint) {
op.hint = this._hint;
}
const op = shallowClone(replacement);
this._serviceProviderBulkFindOp.replaceOne(op);
return this._parentBulk;
}

@returnType('Bulk')
@apiVersions([1])
updateOne(update: Document): Bulk {
updateOne(update: Document | Document[]): Bulk {
this._parentBulk._batchCounts.nUpdateOps++;
assertArgsDefinedType([update], [true], 'BulkFindOp.update');
const op = { ...update };
if (this._hint) {
op.hint = this._hint;
}
if (this._arrayFilters) {
op.arrayFilters = this._arrayFilters;
}
const op = shallowClone(update);
this._serviceProviderBulkFindOp.updateOne(op);
return this._parentBulk;
}

@returnType('Bulk')
update(update: Document): Bulk {
update(update: Document | Document[]): Bulk {
this._parentBulk._batchCounts.nUpdateOps++;
assertArgsDefinedType([update], [true], 'BulkFindOp.update');
const op = { ...update };
if (this._hint) {
op.hint = this._hint;
}
if (this._arrayFilters) {
op.arrayFilters = this._arrayFilters;
}
const op = shallowClone(update);
this._serviceProviderBulkFindOp.update(op);
return this._parentBulk;
}
Expand Down
5 changes: 5 additions & 0 deletions packages/shell-api/src/helpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -804,3 +804,8 @@ improvements and to suggest MongoDB products and deployment options to you.
To enable free monitoring, run the following command: db.enableFreeMonitoring()
To permanently disable this reminder, run the following command: db.disableFreeMonitoring()
`;

export function shallowClone<T>(input: T): T {
if (!input || typeof input !== 'object') return input;
return Array.isArray(input) ? ([...input] as unknown as T) : { ...input };
}
60 changes: 60 additions & 0 deletions packages/shell-api/src/integration.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1855,6 +1855,66 @@ describe('Shell API (integration)', function() {
expect(op.operations.length).to.equal(1);
});
});
context('with >= 4.2 server', () => {
skipIfServerVersion(testServer, '< 4.2');
describe('update() with pipeline update', () => {
beforeEach(async() => {
bulk = await collection[m]();
for (let i = 0; i < size; i++) {
await collection.insertOne({ x: i });
}
expect(await collection.countDocuments()).to.equal(size);
expect(await collection.countDocuments({ y: { $exists: true } })).to.equal(0);
bulk.find({ y: 0 }).upsert().update([{ $set: { y: 1 } }]);
await bulk.execute();
});
afterEach(async() => {
await collection.drop();
});
it('toJSON returns correctly', () => {
expect(bulk.toJSON()).to.deep.equal({ nInsertOps: 0, nUpdateOps: 1, nRemoveOps: 0, nBatches: 1 });
});
it('executes', async() => {
expect(await collection.countDocuments()).to.equal(size + 1);
expect(await collection.countDocuments({ y: { $exists: true } })).to.equal(1);
});
it('getOperations returns correctly', () => {
const ops = bulk.getOperations();
expect(ops.length).to.equal(1);
const op = ops[0];
expect(op.originalZeroIndex).to.equal(0);
expect(op.batchType).to.equal(2);
expect(op.operations.length).to.equal(1);
});
});
describe('updateOne() with pipeline update', () => {
beforeEach(async() => {
bulk = await collection[m]();
for (let i = 0; i < size; i++) {
await collection.insertOne({ x: i });
}
expect(await collection.countDocuments()).to.equal(size);
expect(await collection.countDocuments({ y: { $exists: true } })).to.equal(0);
bulk.find({ y: 0 }).upsert().updateOne([{ $set: { y: 1 } }]);
await bulk.execute();
});
it('toJSON returns correctly', () => {
expect(bulk.toJSON()).to.deep.equal({ nInsertOps: 0, nUpdateOps: 1, nRemoveOps: 0, nBatches: 1 });
});
it('executes', async() => {
expect(await collection.countDocuments()).to.equal(size + 1);
expect(await collection.countDocuments({ y: { $exists: true } })).to.equal(1);
});
it('getOperations returns correctly', () => {
const ops = bulk.getOperations();
expect(ops.length).to.equal(1);
const op = ops[0];
expect(op.originalZeroIndex).to.equal(0);
expect(op.batchType).to.equal(2);
expect(op.operations.length).to.equal(1);
});
});
});
describe('error states', () => {
it('cannot be executed twice', async() => {
bulk = await collection[m]();
Expand Down

0 comments on commit fbbf54a

Please sign in to comment.