remove isClosed() method, depend on closed accessor

Author: mbroadst

package-lock.json

@@ -31,6 +31,7 @@ import { executeOperation, ExecutionResult } from './operations/execute_operatio
 
 const kResumeQueue = Symbol('resumeQueue');
 const kCursorStream = Symbol('cursorStream');
+const kClosed = Symbol('closed');
 
 const CHANGE_STREAM_OPTIONS = ['resumeAfter', 'startAfter', 'startAtOperationTime', 'fullDocument'];
 const CURSOR_OPTIONS = ['batchSize', 'maxAwaitTimeMS', 'collation', 'readPreference'].concat(
@@ -180,10 +181,10 @@ export class ChangeStream extends EventEmitter {
   namespace: MongoDBNamespace;
   type: symbol;
   cursor?: ChangeStreamCursor;
-  closed: boolean;
   streamOptions?: CursorStreamOptions;
   [kResumeQueue]: Denque;
   [kCursorStream]?: Readable;
+  [kClosed]: boolean;
 
   /** @event */
   static readonly CLOSE = 'close' as const;
@@ -241,7 +242,7 @@ export class ChangeStream extends EventEmitter {
     // Create contained Change Stream cursor
     this.cursor = createChangeStreamCursor(this, options);
 
-    this.closed = false;
+    this[kClosed] = false;
 
     // Listen for any `change` listeners being added to ChangeStream
     this.on('newListener', eventName => {
@@ -296,23 +297,20 @@ export class ChangeStream extends EventEmitter {
   }
 
   /** Is the cursor closed */
-  isClosed(): boolean {
-    return this.closed || (this.cursor?.isClosed() ?? false);
+  get closed(): boolean {
+    return this[kClosed] || (this.cursor?.closed ?? false);
   }
 
   /** Close the Change Stream */
   close(callback?: Callback): Promise<void> | void {
-    return maybePromise(callback, cb => {
-      if (this.closed) return cb();
-
-      // flag the change stream as explicitly closed
-      this.closed = true;
+    this[kClosed] = true;
 
-      if (!this.cursor) return cb();
+    return maybePromise(callback, cb => {
+      if (!this.cursor) {
+        return cb();
+      }
 
-      // Tidy up the existing cursor
       const cursor = this.cursor;
-
       return cursor.close(err => {
         endStream(this);
         this.cursor = undefined;
@@ -584,7 +582,7 @@ function processNewChange(
   change: ChangeStreamDocument,
   callback?: Callback
 ) {
-  if (changeStream.closed) {
+  if (changeStream[kClosed]) {
     if (callback) callback(CHANGESTREAM_CLOSED_ERROR);
     return;
   }
@@ -615,8 +613,8 @@ function processError(changeStream: ChangeStream, error: AnyError, callback?: Ca
   const cursor = changeStream.cursor;
 
   // If the change stream has been closed explicitly, do not process error.
-  if (changeStream.closed) {
-    if (callback) callback(new MongoError('ChangeStream is closed'));
+  if (changeStream[kClosed]) {
+    if (callback) callback(CHANGESTREAM_CLOSED_ERROR);
     return;
   }
 
@@ -674,8 +672,8 @@ function processError(changeStream: ChangeStream, error: AnyError, callback?: Ca
  * @param changeStream - the parent ChangeStream
  */
 function getCursor(changeStream: ChangeStream, callback: Callback<ChangeStreamCursor>) {
-  if (changeStream.isClosed()) {
-    callback(new MongoError('ChangeStream is closed.'));
+  if (changeStream[kClosed]) {
+    callback(CHANGESTREAM_CLOSED_ERROR);
     return;
   }
 
@@ -698,8 +696,8 @@ function getCursor(changeStream: ChangeStream, callback: Callback<ChangeStreamCu
 function processResumeQueue(changeStream: ChangeStream, err?: Error) {
   while (changeStream[kResumeQueue].length) {
     const request = changeStream[kResumeQueue].pop();
-    if (changeStream.isClosed() && !err) {
-      request(new MongoError('Change Stream is not open.'));
+    if (changeStream[kClosed] && !err) {
+      request(CHANGESTREAM_CLOSED_ERROR);
       return;
     }
 

src/change_stream.ts

@@ -161,11 +161,6 @@ export abstract class AbstractCursor extends EventEmitter {
     return this[kKilled];
   }
 
-  // NOTE: should we remove these? They are currently needed by a number of tests
-  isClosed(): boolean {
-    return this.closed;
-  }
-
   /** Returns current buffered documents length */
   bufferedCount(): number {
     return this[kDocuments].length;

src/cursor/abstract_cursor.ts

@@ -1,5 +1,9 @@
 import { Instrumentation } from './apm';
+import { AbstractCursor } from './cursor/abstract_cursor';
 import { AggregationCursor } from './cursor/aggregation_cursor';
+import { FindCursor } from './cursor/find_cursor';
+import { ListIndexesCursor } from './operations/indexes';
+import { ListCollectionsCursor } from './operations/list_collections';
 import { PromiseProvider } from './promise_provider';
 import { Admin } from './admin';
 import { MongoClient } from './mongo_client';
@@ -73,7 +77,11 @@ export {
   Collection,
   ReadPreference,
   Logger,
+  AbstractCursor,
   AggregationCursor,
+  FindCursor,
+  ListIndexesCursor,
+  ListCollectionsCursor,
   GridFSBucket
 };
 

src/index.ts

@@ -102,6 +102,7 @@ export class ListCollectionsOperation extends CommandOperation<ListCollectionsOp
   }
 }
 
+/** @public */
 export class ListCollectionsCursor extends AbstractCursor {
   parent: Db;
   filter: Document;

src/operations/list_collections.ts

@@ -64,6 +64,7 @@ export class Monitor extends EventEmitter {
   [kServer]: Server;
   [kConnection]?: Connection;
   [kCancellationToken]: EventEmitter;
+  /** @internal */
   [kMonitorId]?: InterruptableAsyncInterval;
   [kRTTPinger]?: RTTPinger;
 

src/sdam/monitor.ts

@@ -475,14 +475,14 @@ describe('Change Streams', function () {
         const changeStream = database.collection('changeStreamCloseTest').watch(pipeline);
         this.defer(() => changeStream.close());
 
-        assert.equal(changeStream.isClosed(), false);
-        assert.equal(changeStream.cursor.isClosed(), false);
+        assert.equal(changeStream.closed, false);
+        assert.equal(changeStream.cursor.closed, false);
 
         changeStream.close(err => {
           expect(err).to.not.exist;
 
           // Check the cursor is closed
-          assert.equal(changeStream.isClosed(), true);
+          assert.equal(changeStream.closed, true);
           assert.ok(!changeStream.cursor);
           done();
         });
@@ -774,7 +774,7 @@ describe('Change Streams', function () {
               changeStream.hasNext(function (err, hasNext) {
                 expect(err).to.not.exist;
                 assert.equal(hasNext, false);
-                assert.equal(changeStream.isClosed(), true);
+                assert.equal(changeStream.closed, true);
                 done();
               });
             }
@@ -841,7 +841,7 @@ describe('Change Streams', function () {
           .then(() => changeStream.hasNext())
           .then(function (hasNext) {
             assert.equal(hasNext, false);
-            assert.equal(changeStream.isClosed(), true);
+            assert.equal(changeStream.closed, true);
             done();
           });
       });
@@ -1866,7 +1866,7 @@ describe('Change Streams', function () {
     afterEach(function () {
       return Promise.resolve()
         .then(() => {
-          if (changeStream && !changeStream.isClosed()) {
+          if (changeStream && !changeStream.closed) {
             return changeStream.close();
           }
         })

test/functional/change_stream.test.js

@@ -927,7 +927,7 @@ describe('Cursor', function () {
                     cursor.next((err, items) => {
                       expect(err).to.not.exist;
                       test.ok(items == null);
-                      test.ok(cursor.isClosed());
+                      test.ok(cursor.closed);
                       done();
                     });
                   });
@@ -993,7 +993,7 @@ describe('Cursor', function () {
                     cursor.next((err, items) => {
                       expect(err).to.not.exist;
                       test.ok(items == null);
-                      test.ok(cursor.isClosed());
+                      test.ok(cursor.closed);
                       done();
                     });
                   });
@@ -1050,7 +1050,7 @@ describe('Cursor', function () {
                 cursor.next((err, items) => {
                   expect(err).to.not.exist;
                   test.ok(items == null);
-                  test.ok(cursor.isClosed());
+                  test.ok(cursor.closed);
                   done();
                 });
               });
@@ -1208,7 +1208,7 @@ describe('Cursor', function () {
           const cursor = collection.find();
           cursor.close(err => {
             expect(err).to.not.exist;
-            test.equal(true, cursor.isClosed());
+            test.equal(true, cursor.closed);
             done();
           });
         });
@@ -1409,7 +1409,7 @@ describe('Cursor', function () {
 
               cursor.close(err => {
                 expect(err).to.not.exist;
-                test.equal(true, cursor.isClosed());
+                test.equal(true, cursor.closed);
                 done();
               });
             });
@@ -1623,7 +1623,7 @@ describe('Cursor', function () {
               test.equal(1, closed);
               test.equal(1, paused);
               test.equal(1, resumed);
-              test.strictEqual(cursor.isClosed(), true);
+              test.strictEqual(cursor.closed, true);
               done();
             }
           });
@@ -1679,7 +1679,7 @@ describe('Cursor', function () {
                   if (doneCalled === 1) {
                     expect(err).to.not.exist;
                     test.strictEqual(0, i);
-                    test.strictEqual(true, cursor.isClosed());
+                    test.strictEqual(true, cursor.closed);
                     done();
                   }
                 };
@@ -1722,7 +1722,7 @@ describe('Cursor', function () {
             const cursor = collection.find();
             const stream = cursor.stream();
 
-            test.strictEqual(false, cursor.isClosed());
+            test.strictEqual(false, cursor.closed);
 
             stream.on('data', function () {
               if (++i === 5) {
@@ -1740,7 +1740,7 @@ describe('Cursor', function () {
                 test.strictEqual(undefined, err);
                 test.strictEqual(5, i);
                 test.strictEqual(2, finished);
-                test.strictEqual(true, cursor.isClosed());
+                test.strictEqual(true, cursor.closed);
                 done();
               }
             }
@@ -1797,7 +1797,7 @@ describe('Cursor', function () {
                 if (finished === 2) {
                   setTimeout(function () {
                     test.equal(5, i);
-                    test.equal(true, cursor.isClosed());
+                    test.equal(true, cursor.closed);
                     client.close();
 
                     configuration.manager.start().then(function () {

test/functional/cursor.test.js

@@ -5454,9 +5454,6 @@ describe('Operation Examples', function () {
 
   /**
    * A simple example showing the use of the cursor close function.
-   *
-   * @example-class Cursor
-   * @example-method isClosed
    */
   it('shouldStreamDocumentsUsingTheIsCloseFunction', {
     // Add a tag that our runner can trigger on
@@ -5505,7 +5502,7 @@ describe('Operation Examples', function () {
             // Close the cursor, this is the same as reseting the query
             cursor.close(function (err) {
               expect(err).to.not.exist;
-              test.equal(true, cursor.isClosed());
+              test.equal(true, cursor.closed);
 
               client.close(done);
             });