fix(shell-api): configurable batch size option applies only to display MONGOSH-877 (#992)

Author: rose-m

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',

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: {

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);
       });
     });
 

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<CursorType extends ServiceProviderAggregationCursor | ServiceProviderCursor> 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<CursorIterationResult> {
     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;
   }
 

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<SPAggregationCursor>;
@@ -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);
       });
     });
   });

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<ServiceProviderAggregationCursor> {
   constructor(mongo: Mongo, cursor: ServiceProviderAggregationCursor) {
-    super(mongo);
-    this._cursor = cursor;
+    super(mongo, cursor);
   }
 }

packages/shell-api/src/change-stream-cursor.spec.ts

@@ -98,16 +98,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'] );

packages/shell-api/src/change-stream-cursor.ts

@@ -27,7 +27,6 @@ export default class ChangeStreamCursor extends ShellApiWithMongoClass {
   _cursor: ChangeStream<Document>;
   _currentIterationResult: CursorIterationResult | null = null;
   _on: string;
-  _batchSize: number | null = null;
 
   constructor(cursor: ChangeStream<Document>, 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;
-  }
 }

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 }
         );
       });
 

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();
   }