diff --git a/packages/cli-repl/src/cli-repl.spec.ts b/packages/cli-repl/src/cli-repl.spec.ts index 5a9a3ace4b..434313ba9a 100644 --- a/packages/cli-repl/src/cli-repl.spec.ts +++ b/packages/cli-repl/src/cli-repl.spec.ts @@ -191,7 +191,7 @@ describe('CliRepl', () => { it('returns the list of available config options when asked to', () => { expect(cliRepl.listConfigOptions()).to.deep.equal([ - 'batchSize', + 'displayBatchSize', 'enableTelemetry', 'snippetIndexSourceURLs', 'snippetRegistryURL', diff --git a/packages/i18n/src/locales/en_US.ts b/packages/i18n/src/locales/en_US.ts index 98f23ca766..0740b64b9f 100644 --- a/packages/i18n/src/locales/en_US.ts +++ b/packages/i18n/src/locales/en_US.ts @@ -251,7 +251,7 @@ const translations: Catalog = { description: 'Deprecated. The shell provides auto-formatting so this method is no longer useful' }, batchSize: { - description: 'Specifies the number of documents that mongosh displays at once.', + description: 'Specifies the number of documents to return in each batch of the response from the MongoDB instance.', example: 'db.collection.aggregate(pipeline, options).batchSize(10)' }, projection: { @@ -711,10 +711,6 @@ const translations: Catalog = { }, isExhausted: { description: 'This method is deprecated because because after closing a cursor, the remaining documents in the batch are no longer accessible. If you want to see if the cursor is closed use cursor.isClosed. If you want to see if there are documents left in the batch, use cursor.tryNext. This is a breaking change' - }, - batchSize: { - description: 'Specifies the number of documents that mongosh displays at once.', - example: 'db.collection.aggregate(pipeline, options).batchSize(10)' } } } @@ -744,7 +740,7 @@ const translations: Catalog = { }, batchSize: { link: 'https://docs.mongodb.com/manual/reference/method/cursor.batchSize', - description: 'Specifies the number of documents to return in each batch of the response from the MongoDB instance. In most cases, modifying the batch size will not affect the user or the application, as the mongo shell and most drivers return results as if MongoDB returned a single batch. This also specifies how many entries mongosh displays at once.', + description: 'Specifies the number of documents to return in each batch of the response from the MongoDB instance. In most cases, modifying the batch size will not affect the user or the application, as the mongo shell and most drivers return results as if MongoDB returned a single batch.', example: 'db.collection.find(query, projection).batchSize(10)' }, close: { @@ -1296,7 +1292,7 @@ const translations: Catalog = { }, DBQuery: { help: { - description: 'Deprecated -- use cursor.batchSize(value) or config.set("batchSize", value) instead of DBQuery.batchSize = value', + description: 'Deprecated -- use config.set("displayBatchSize", value) instead of DBQuery.shellBatchSize = value', } }, Explainable: { diff --git a/packages/node-runtime-worker-thread/src/worker-runtime.spec.ts b/packages/node-runtime-worker-thread/src/worker-runtime.spec.ts index e406b2d6cd..937135de96 100644 --- a/packages/node-runtime-worker-thread/src/worker-runtime.spec.ts +++ b/packages/node-runtime-worker-thread/src/worker-runtime.spec.ts @@ -496,7 +496,7 @@ describe('worker', () => { getConfig() {}, setConfig() {}, validateConfig() {}, - listConfigOptions() { return ['batchSize']; }, + listConfigOptions() { return ['displayBatchSize']; }, onRunInterruptible() {} }; @@ -576,9 +576,9 @@ describe('worker', () => { await init('mongodb://nodb/', {}, { nodb: true }); - await evaluate('config.set("batchSize", 200)'); - expect(evalListener.validateConfig).to.have.been.calledWith('batchSize', 200); - expect(evalListener.setConfig).to.have.been.calledWith('batchSize', 200); + await evaluate('config.set("displayBatchSize", 200)'); + expect(evalListener.validateConfig).to.have.been.calledWith('displayBatchSize', 200); + expect(evalListener.setConfig).to.have.been.calledWith('displayBatchSize', 200); }); }); diff --git a/packages/shell-api/src/abstract-cursor.ts b/packages/shell-api/src/abstract-cursor.ts index 0532b8496a..9ec5d783d8 100644 --- a/packages/shell-api/src/abstract-cursor.ts +++ b/packages/shell-api/src/abstract-cursor.ts @@ -18,16 +18,17 @@ import { CursorIterationResult } from './result'; import { iterate, validateExplainableVerbosity, markAsExplainOutput } from './helpers'; @shellApiClassNoHelp -export abstract class AbstractCursor extends ShellApiWithMongoClass { +export abstract class AbstractCursor extends ShellApiWithMongoClass { _mongo: Mongo; - abstract _cursor: ServiceProviderAggregationCursor | ServiceProviderCursor; + _cursor: CursorType; + _currentIterationResult: CursorIterationResult | null = null; - _batchSize: number | null = null; _mapError: Error | null = null; - constructor(mongo: Mongo) { + constructor(mongo: Mongo, cursor: CursorType) { super(); this._mongo = mongo; + this._cursor = cursor; } // Wrap a function with checks before and after that verify whether a .map() @@ -63,14 +64,14 @@ export abstract class AbstractCursor extends ShellApiWithMongoClass { async _it(): Promise { const results = this._currentIterationResult = new CursorIterationResult(); - await iterate(results, this, this._batchSize ?? await this._mongo._batchSize()); + await iterate(results, this, await this._mongo._displayBatchSize()); results.cursorHasMore = !this.isExhausted(); return results; } @returnType('this') batchSize(size: number): this { - this._batchSize = size; + this._cursor.batchSize(size); return this; } diff --git a/packages/shell-api/src/aggregation-cursor.spec.ts b/packages/shell-api/src/aggregation-cursor.spec.ts index f2365e9992..d260f6f054 100644 --- a/packages/shell-api/src/aggregation-cursor.spec.ts +++ b/packages/shell-api/src/aggregation-cursor.spec.ts @@ -45,7 +45,7 @@ describe('AggregationCursor', () => { }; cursor = new AggregationCursor({ _serviceProvider: { platform: ReplPlatform.CLI }, - _batchSize: () => 20 + _displayBatchSize: () => 20 } as any, wrappee); }); @@ -75,7 +75,7 @@ describe('AggregationCursor', () => { describe('Cursor Internals', () => { const mongo = { - _batchSize: () => 20 + _displayBatchSize: () => 20 } as any; describe('#close', () => { let spCursor: StubbedInstance; @@ -212,8 +212,9 @@ describe('AggregationCursor', () => { }); describe('toShellResult', () => { - let shellApiCursor; - let i; + let shellApiCursor: AggregationCursor; + let i: number; + let batchSize: number | undefined; beforeEach(() => { i = 0; @@ -226,6 +227,9 @@ describe('AggregationCursor', () => { if (prop === 'tryNext') { return async() => ({ key: i++ }); } + if (prop === 'batchSize') { + return (size: number) => { batchSize = size; }; + } return (target as any)[prop]; } }); @@ -243,11 +247,12 @@ describe('AggregationCursor', () => { expect(i).to.equal(40); }); - it('lets .batchSize() control the output length', async() => { + it('.batchSize() does not control the output length', async() => { shellApiCursor.batchSize(10); const result = (await toShellResult(shellApiCursor)).printable; - expect(i).to.equal(10); - expect(result).to.have.nested.property('documents.length', 10); + expect(i).to.equal(20); + expect(batchSize).to.equal(10); + expect(result).to.have.nested.property('documents.length', 20); }); }); }); diff --git a/packages/shell-api/src/aggregation-cursor.ts b/packages/shell-api/src/aggregation-cursor.ts index 9dfef5b34c..1f1fd76789 100644 --- a/packages/shell-api/src/aggregation-cursor.ts +++ b/packages/shell-api/src/aggregation-cursor.ts @@ -8,11 +8,8 @@ import type { import { AbstractCursor } from './abstract-cursor'; @shellApiClassDefault -export default class AggregationCursor extends AbstractCursor { - _cursor: ServiceProviderAggregationCursor; - +export default class AggregationCursor extends AbstractCursor { constructor(mongo: Mongo, cursor: ServiceProviderAggregationCursor) { - super(mongo); - this._cursor = cursor; + super(mongo, cursor); } } diff --git a/packages/shell-api/src/change-stream-cursor.spec.ts b/packages/shell-api/src/change-stream-cursor.spec.ts index 6ea977e4d4..24fa4c5db4 100644 --- a/packages/shell-api/src/change-stream-cursor.spec.ts +++ b/packages/shell-api/src/change-stream-cursor.spec.ts @@ -97,16 +97,6 @@ describe('ChangeStreamCursor', () => { expect(spCursor.next.calledWith()).to.equal(true); expect(warnSpy.calledOnce).to.equal(true); }); - it('lets .batchSize() control iteration batch size', async() => { - const cursor2 = new ChangeStreamCursor({ - tryNext: sinon.stub().resolves({ doc: 1 }) - } as any, 'source', { - _internalState: {} - } as Mongo); - cursor2.batchSize(3); - const results = await cursor2._it(); - expect(results.documents).to.deep.equal([{ doc: 1 }, { doc: 1 }, { doc: 1 }]); - }); }); describe('integration', () => { const [ srv0 ] = startTestCluster(['--replicaset'] ); diff --git a/packages/shell-api/src/change-stream-cursor.ts b/packages/shell-api/src/change-stream-cursor.ts index 438831562b..2db324e555 100644 --- a/packages/shell-api/src/change-stream-cursor.ts +++ b/packages/shell-api/src/change-stream-cursor.ts @@ -27,7 +27,6 @@ export default class ChangeStreamCursor extends ShellApiWithMongoClass { _cursor: ChangeStream; _currentIterationResult: CursorIterationResult | null = null; _on: string; - _batchSize: number | null = null; constructor(cursor: ChangeStream, on: string, mongo: Mongo) { super(); @@ -41,7 +40,7 @@ export default class ChangeStreamCursor extends ShellApiWithMongoClass { throw new MongoshRuntimeError('ChangeStreamCursor is closed'); } const result = this._currentIterationResult = new CursorIterationResult(); - return iterate(result, this, this._batchSize ?? await this._mongo._batchSize()); + return iterate(result, this, await this._mongo._displayBatchSize()); } /** @@ -130,10 +129,4 @@ export default class ChangeStreamCursor extends ShellApiWithMongoClass { pretty(): ChangeStreamCursor { return this; } - - @returnType('ChangeStreamCursor') - batchSize(size: number): ChangeStreamCursor { - this._batchSize = size; - return this; - } } diff --git a/packages/shell-api/src/collection.spec.ts b/packages/shell-api/src/collection.spec.ts index 6c89e74f4c..e1f1901c65 100644 --- a/packages/shell-api/src/collection.spec.ts +++ b/packages/shell-api/src/collection.spec.ts @@ -137,7 +137,7 @@ describe('Collection', () => { expect(serviceProvider.aggregate).to.have.been.calledWith( collection._database._name, collection._name, - [{ $piplelineStage: {} }], + [{ $piplelineStage: {} } ], {} ); }); @@ -169,13 +169,13 @@ describe('Collection', () => { it('calls serviceProvider.aggregate with pipleline and options', async() => { await collection.aggregate( [{ $piplelineStage: {} }], - { options: true }); + { options: true, batchSize: 10 }); expect(serviceProvider.aggregate).to.have.been.calledWith( collection._database._name, collection._name, [{ $piplelineStage: {} }], - { options: true } + { options: true, batchSize: 10 } ); }); diff --git a/packages/shell-api/src/collection.ts b/packages/shell-api/src/collection.ts index 4bd1c982b1..0eb9f1a03d 100644 --- a/packages/shell-api/src/collection.ts +++ b/packages/shell-api/src/collection.ts @@ -414,7 +414,12 @@ export default class Collection extends ShellApiWithMongoClass { this._emitCollectionApiCall('find', { query, options }); const cursor = new Cursor( this._mongo, - this._mongo._serviceProvider.find(this._database._name, this._name, query, { ...this._database._baseOptions, ...options }) + this._mongo._serviceProvider.find( + this._database._name, + this._name, + query, + { ...this._database._baseOptions, ...options } + ) ); this._mongo._internalState.currentCursor = cursor; @@ -470,7 +475,12 @@ export default class Collection extends ShellApiWithMongoClass { this._emitCollectionApiCall('findOne', { query, options }); return new Cursor( this._mongo, - this._mongo._serviceProvider.find(this._database._name, this._name, query, { ...this._database._baseOptions, ...options }) + this._mongo._serviceProvider.find( + this._database._name, + this._name, + query, + { ...this._database._baseOptions, ...options } + ) ).limit(1).tryNext(); } diff --git a/packages/shell-api/src/cursor.spec.ts b/packages/shell-api/src/cursor.spec.ts index f5dff595ac..27f8ac67d4 100644 --- a/packages/shell-api/src/cursor.spec.ts +++ b/packages/shell-api/src/cursor.spec.ts @@ -49,7 +49,7 @@ describe('Cursor', () => { }; cursor = new Cursor({ _serviceProvider: { platform: ReplPlatform.CLI }, - _batchSize: () => 20 + _displayBatchSize: () => 20 } as any, wrappee); }); @@ -82,7 +82,7 @@ describe('Cursor', () => { }); describe('Cursor Internals', () => { const mongo = { - _batchSize: () => 20 + _displayBatchSize: () => 20 } as any; describe('#addOption', () => { let spCursor: StubbedInstance; @@ -777,11 +777,11 @@ describe('Cursor', () => { expect(i).to.equal(40); }); - it('lets .batchSize() control the output length', async() => { + it('.batchSize() does not control the output length', async() => { shellApiCursor.batchSize(10); const result = (await toShellResult(shellApiCursor)).printable; - expect(i).to.equal(10); - expect(result).to.have.nested.property('documents.length', 10); + expect(i).to.equal(20); + expect(result).to.have.nested.property('documents.length', 20); }); }); }); diff --git a/packages/shell-api/src/cursor.ts b/packages/shell-api/src/cursor.ts index bc69ba0a4f..81256b81dd 100644 --- a/packages/shell-api/src/cursor.ts +++ b/packages/shell-api/src/cursor.ts @@ -26,13 +26,11 @@ import { printWarning } from './deprecation-warning'; import { AbstractCursor } from './abstract-cursor'; @shellApiClassDefault -export default class Cursor extends AbstractCursor { - _cursor: ServiceProviderCursor; +export default class Cursor extends AbstractCursor { _tailable = false; constructor(mongo: Mongo, cursor: ServiceProviderCursor) { - super(mongo); - this._cursor = cursor; + super(mongo, cursor); } /** @@ -75,13 +73,6 @@ export default class Cursor extends AbstractCursor { return this; } - @returnType('Cursor') - batchSize(size: number): this { - super.batchSize(size); - this._cursor.batchSize(size); - return this; - } - @returnType('Cursor') @serverVersions(['3.4.0', ServerVersions.latest]) collation(spec: CollationOptions): Cursor { diff --git a/packages/shell-api/src/database.spec.ts b/packages/shell-api/src/database.spec.ts index 8954196843..5217427640 100644 --- a/packages/shell-api/src/database.spec.ts +++ b/packages/shell-api/src/database.spec.ts @@ -285,6 +285,17 @@ describe('Database', () => { ); }); + it('calls serviceProvider.aggregateDb with explicit batchSize', async() => { + await database.aggregate( + [{ $piplelineStage: {} }], { options: true, batchSize: 10 }); + + expect(serviceProvider.aggregateDb).to.have.been.calledWith( + database._name, + [{ $piplelineStage: {} }], + { options: true, batchSize: 10 } + ); + }); + it('returns an AggregationCursor that wraps the service provider one', async() => { const toArrayResult = []; serviceProviderCursor.toArray.resolves(toArrayResult); diff --git a/packages/shell-api/src/dbquery.ts b/packages/shell-api/src/dbquery.ts index 5b9ca9ce93..a275a59f01 100644 --- a/packages/shell-api/src/dbquery.ts +++ b/packages/shell-api/src/dbquery.ts @@ -16,13 +16,13 @@ export class DBQuery extends ShellApiClass { this._internalState = internalState; } - get batchSize(): number | undefined { - return this._internalState.batchSizeFromDBQuery; + get shellBatchSize(): number | undefined { + return this._internalState.displayBatchSizeFromDBQuery; } - set batchSize(value: number | undefined) { + set shellBatchSize(value: number | undefined) { printDeprecationWarning( - 'DBQuery.batchSize is deprecated, please use \'batchSize\' on the cursor instead: db.coll.find().batchSize()'); - this._internalState.batchSizeFromDBQuery = value; + 'DBQuery.shellBatchSize is deprecated, please use config.set("displayBatchSize") instead'); + this._internalState.displayBatchSizeFromDBQuery = value; } } diff --git a/packages/shell-api/src/helpers.ts b/packages/shell-api/src/helpers.ts index 7e387c2ba7..c31fba9e71 100644 --- a/packages/shell-api/src/helpers.ts +++ b/packages/shell-api/src/helpers.ts @@ -533,7 +533,7 @@ export function addHiddenDataProperty(target: T, key: string|symbol, va export async function iterate( results: CursorIterationResult, - cursor: AbstractCursor | ChangeStreamCursor, + cursor: AbstractCursor | ChangeStreamCursor, batchSize: number): Promise { if (cursor.isClosed()) { return results; diff --git a/packages/shell-api/src/integration.spec.ts b/packages/shell-api/src/integration.spec.ts index 56e5ee91b8..c3578616d9 100644 --- a/packages/shell-api/src/integration.spec.ts +++ b/packages/shell-api/src/integration.spec.ts @@ -2150,7 +2150,7 @@ describe('Shell API (integration)', function() { }); }); - describe('batchSize precedence', () => { + describe('displayBatchSize precedence', () => { beforeEach(async() => { await collection.insertMany([...Array(100).keys()].map(i => ({ i }))); const cfg = new ShellUserConfig(); @@ -2162,22 +2162,27 @@ describe('Shell API (integration)', function() { expect((await collection.find()._it()).documents).to.have.lengthOf(20); }); - it('config changes affect batchSize', async() => { - await shellApi.config.set('batchSize', 10); + it('config changes affect displayBatchSize', async() => { + await shellApi.config.set('displayBatchSize', 10); expect((await collection.find()._it()).documents).to.have.lengthOf(10); }); - it('DBQuery.batchSize takes precedence over config', async() => { - await shellApi.config.set('batchSize', 10); - shellApi.DBQuery.batchSize = 30; - expect(shellApi.DBQuery.batchSize).to.equal(30); + it('DBQuery.shellBatchSize takes precedence over config', async() => { + await shellApi.config.set('displayBatchSize', 10); + shellApi.DBQuery.shellBatchSize = 30; + expect(shellApi.DBQuery.shellBatchSize).to.equal(30); expect((await collection.find()._it()).documents).to.have.lengthOf(30); }); - it('cursor.batchSize takes precedence over config and DBQuery.batchSize', async() => { - await shellApi.config.set('batchSize', 10); - shellApi.DBQuery.batchSize = 30; - expect((await collection.find().batchSize(50)._it()).documents).to.have.lengthOf(50); + it('cursor.batchSize does not override config displayBatchSize', async() => { + await shellApi.config.set('displayBatchSize', 10); + expect((await collection.find().batchSize(50)._it()).documents).to.have.lengthOf(10); + }); + + it('cursor.batchSize does not override DBQuery.shellBatchSize', async() => { + shellApi.DBQuery.shellBatchSize = 5; + expect(shellApi.DBQuery.shellBatchSize).to.equal(5); + expect((await collection.find().batchSize(50)._it()).documents).to.have.lengthOf(5); }); }); diff --git a/packages/shell-api/src/mongo.ts b/packages/shell-api/src/mongo.ts index 77b6020136..16f169cde1 100644 --- a/packages/shell-api/src/mongo.ts +++ b/packages/shell-api/src/mongo.ts @@ -129,9 +129,9 @@ export default class Mongo extends ShellApiClass { this.__serviceProvider = sp; } - async _batchSize(): Promise { - return this._internalState.batchSizeFromDBQuery ?? - await this._internalState.shellApi.config.get('batchSize'); + async _displayBatchSize(): Promise { + return this._internalState.displayBatchSizeFromDBQuery ?? + await this._internalState.shellApi.config.get('displayBatchSize'); } /** @@ -297,10 +297,10 @@ export default class Mongo extends ShellApiClass { const sysprof = this._internalState.currentDb.getCollection('system.profile'); const profiles = { count: await sysprof.countDocuments({}) } as Document; if (profiles.count !== 0) { - profiles.result = await (sysprof.find({ millis: { $gt: 0 } }) + profiles.result = await sysprof.find({ millis: { $gt: 0 } }) .sort({ $natural: -1 }) .limit(5) - .toArray()); + .toArray(); } return new CommandResult('ShowProfileResult', profiles); case 'users': diff --git a/packages/shell-api/src/shell-api.spec.ts b/packages/shell-api/src/shell-api.spec.ts index e42071e15b..94fcb23f5e 100644 --- a/packages/shell-api/src/shell-api.spec.ts +++ b/packages/shell-api/src/shell-api.spec.ts @@ -686,7 +686,7 @@ describe('ShellApi', () => { }); it('will fall back to defaults', async() => { - expect(await config.get('batchSize')).to.equal(20); + expect(await config.get('displayBatchSize')).to.equal(20); }); it('rejects setting unavailable config keys', async() => { @@ -712,9 +712,9 @@ describe('ShellApi', () => { }); it('will work with defaults', async() => { - expect(await config.get('batchSize')).to.equal(20); + expect(await config.get('displayBatchSize')).to.equal(20); expect((await toShellResult(config)).printable).to.deep.equal( - new Map([['batchSize', 20], ['enableTelemetry', false]] as any)); + new Map([['displayBatchSize', 20], ['enableTelemetry', false]] as any)); }); it('rejects setting all config keys', async() => { diff --git a/packages/shell-api/src/shell-internal-state.ts b/packages/shell-api/src/shell-internal-state.ts index b48ad94d51..e1bb34616d 100644 --- a/packages/shell-api/src/shell-internal-state.ts +++ b/packages/shell-api/src/shell-internal-state.ts @@ -109,7 +109,7 @@ export default class ShellInternalState { public cliOptions: ShellCliOptions; public evaluationListener: EvaluationListener; public mongocryptdSpawnPath: string | null; - public batchSizeFromDBQuery: number | undefined = undefined; + public displayBatchSizeFromDBQuery: number | undefined = undefined; public isInteractive = false; public readonly interrupted = new InterruptFlag(); diff --git a/packages/types/src/index.spec.ts b/packages/types/src/index.spec.ts index 637117d817..f7a80c2248 100644 --- a/packages/types/src/index.spec.ts +++ b/packages/types/src/index.spec.ts @@ -26,11 +26,11 @@ describe('config validation', () => { expect(await validate('redactHistory', 'keep')).to.equal(null); expect(await validate('redactHistory', 'remove')).to.equal(null); expect(await validate('redactHistory', 'remove-redact')).to.equal(null); - expect(await validate('batchSize', 'foo')).to.equal('batchSize must be a positive integer'); - expect(await validate('batchSize', -1)).to.equal('batchSize must be a positive integer'); - expect(await validate('batchSize', 0)).to.equal('batchSize must be a positive integer'); - expect(await validate('batchSize', 1)).to.equal(null); - expect(await validate('batchSize', Infinity)).to.equal(null); + expect(await validate('displayBatchSize', 'foo')).to.equal('displayBatchSize must be a positive integer'); + expect(await validate('displayBatchSize', -1)).to.equal('displayBatchSize must be a positive integer'); + expect(await validate('displayBatchSize', 0)).to.equal('displayBatchSize must be a positive integer'); + expect(await validate('displayBatchSize', 1)).to.equal(null); + expect(await validate('displayBatchSize', Infinity)).to.equal(null); expect(await validate('enableTelemetry', 'foo')).to.equal('enableTelemetry must be a boolean'); expect(await validate('enableTelemetry', -1)).to.equal('enableTelemetry must be a boolean'); expect(await validate('enableTelemetry', false)).to.equal(null); diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index d4dc9f4af4..acc38bfba7 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -298,14 +298,14 @@ export interface MongoshBus { } export class ShellUserConfig { - batchSize = 20; + displayBatchSize = 20; enableTelemetry = false; } export class ShellUserConfigValidator { static async validate(key: K, value: ShellUserConfig[K]): Promise { switch (key) { - case 'batchSize': + case 'displayBatchSize': if (typeof value !== 'number' || value <= 0) { return `${key} must be a positive integer`; }