From 26155c61636218a7f807e00bd36d2e6a9af783d4 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Mon, 23 Jan 2023 15:09:17 -0500 Subject: [PATCH 1/2] fix(NODE-4988): cloned cursors are not legacy api --- src/legacy_wrappers/cursors.js | 44 ++++++++++++++++++-- test/tools/api.js | 6 +++ test/unit/legacy_wrappers/collection.test.js | 12 ++++++ test/unit/legacy_wrappers/db.test.js | 4 ++ 4 files changed, 62 insertions(+), 4 deletions(-) diff --git a/src/legacy_wrappers/cursors.js b/src/legacy_wrappers/cursors.js index 417b9fd..ba861b0 100644 --- a/src/legacy_wrappers/cursors.js +++ b/src/legacy_wrappers/cursors.js @@ -7,6 +7,11 @@ module.exports = Object.create(null); Object.defineProperty(module.exports, '__esModule', { value: true }); module.exports.makeLegacyFindCursor = function (baseClass) { + function toLegacyHelper(cursor) { + Object.setPrototypeOf(cursor, LegacyFindCursor.prototype); + return cursor; + } + class LegacyFindCursor extends baseClass { /** @deprecated Use `collection.estimatedDocumentCount` or `collection.countDocuments` instead */ count(options, callback) { @@ -56,12 +61,16 @@ module.exports.makeLegacyFindCursor = function (baseClass) { tryNext(callback) { return maybeCallback(super.tryNext(), callback); } + clone() { + const cursor = super.clone(); + return toLegacyHelper(cursor); + } } Object.defineProperty(baseClass.prototype, toLegacy, { enumerable: false, value: function () { - return Object.setPrototypeOf(this, LegacyFindCursor.prototype); + return toLegacyHelper(this); } }); @@ -69,6 +78,11 @@ module.exports.makeLegacyFindCursor = function (baseClass) { }; module.exports.makeLegacyListCollectionsCursor = function (baseClass) { + function toLegacyHelper(cursor) { + Object.setPrototypeOf(cursor, LegacyListCollectionsCursor.prototype); + return cursor; + } + class LegacyListCollectionsCursor extends baseClass { close(options, callback) { callback = @@ -95,12 +109,16 @@ module.exports.makeLegacyListCollectionsCursor = function (baseClass) { tryNext(callback) { return maybeCallback(super.tryNext(), callback); } + clone() { + const cursor = super.clone(); + return toLegacyHelper(cursor); + } } Object.defineProperty(baseClass.prototype, toLegacy, { enumerable: false, value: function () { - return Object.setPrototypeOf(this, LegacyListCollectionsCursor.prototype); + return toLegacyHelper(this); } }); @@ -108,6 +126,11 @@ module.exports.makeLegacyListCollectionsCursor = function (baseClass) { }; module.exports.makeLegacyListIndexesCursor = function (baseClass) { + function toLegacyHelper(cursor) { + Object.setPrototypeOf(cursor, LegacyListIndexesCursor.prototype); + return cursor; + } + class LegacyListIndexesCursor extends baseClass { close(options, callback) { callback = @@ -134,12 +157,16 @@ module.exports.makeLegacyListIndexesCursor = function (baseClass) { tryNext(callback) { return maybeCallback(super.tryNext(), callback); } + clone() { + const cursor = super.clone(); + return toLegacyHelper(cursor); + } } Object.defineProperty(baseClass.prototype, toLegacy, { enumerable: false, value: function () { - return Object.setPrototypeOf(this, LegacyListIndexesCursor.prototype); + return toLegacyHelper(this); } }); @@ -147,6 +174,11 @@ module.exports.makeLegacyListIndexesCursor = function (baseClass) { }; module.exports.makeLegacyAggregationCursor = function (baseClass) { + function toLegacyHelper(cursor) { + Object.setPrototypeOf(cursor, LegacyAggregationCursor.prototype); + return cursor; + } + class LegacyAggregationCursor extends baseClass { explain(verbosity, callback) { callback = @@ -184,12 +216,16 @@ module.exports.makeLegacyAggregationCursor = function (baseClass) { tryNext(callback) { return maybeCallback(super.tryNext(), callback); } + clone() { + const cursor = super.clone(); + return toLegacyHelper(cursor); + } } Object.defineProperty(baseClass.prototype, toLegacy, { enumerable: false, value: function () { - return Object.setPrototypeOf(this, LegacyAggregationCursor.prototype); + return toLegacyHelper(this); } }); diff --git a/test/tools/api.js b/test/tools/api.js index c72c37c..14dd011 100644 --- a/test/tools/api.js +++ b/test/tools/api.js @@ -40,6 +40,12 @@ const api = [ { className: 'Admin', method: 'validateCollection', returnType: 'Promise' }, { className: 'AggregationCursor', method: 'explain', returnType: 'Promise' }, + { className: 'AggregationCursor', method: 'clone', returnType: 'AggregationCursor', notAsync: true }, + + { className: 'FindCursor', method: 'clone', returnType: 'FindCursor', notAsync: true }, + { className: 'ListIndexesCursor', method: 'clone', returnType: 'ListIndexesCursor', notAsync: true }, + { className: 'ListCollectionsCursor', method: 'clone', returnType: 'ListCollectionsCursor', notAsync: true }, + // Super class of Unordered/Ordered Bulk operations // This is listed here as a reference for completeness, but it is tested by the subclass overrides of execute diff --git a/test/unit/legacy_wrappers/collection.test.js b/test/unit/legacy_wrappers/collection.test.js index a09a44b..9aee2e6 100644 --- a/test/unit/legacy_wrappers/collection.test.js +++ b/test/unit/legacy_wrappers/collection.test.js @@ -43,6 +43,18 @@ describe('legacy_wrappers/collection.js', () => { expect(collection.find()).to.be.instanceOf(LegacyFindCursor); }); + it('collection.listIndexes().clone() should return legacy listIndexes cursor', () => { + expect(collection.listIndexes().clone()).to.be.instanceOf(LegacyListIndexesCursor); + }); + + it('collection.aggregate().clone() should return legacy ChangeStream', () => { + expect(collection.aggregate().clone()).to.be.instanceOf(LegacyAggregationCursor); + }); + + it('collection.find().clone() should return legacy FindCursor', () => { + expect(collection.find().clone()).to.be.instanceOf(LegacyFindCursor); + }); + describe('rename()', () => { let client; let collection; diff --git a/test/unit/legacy_wrappers/db.test.js b/test/unit/legacy_wrappers/db.test.js index 6340cda..48ecb8d 100644 --- a/test/unit/legacy_wrappers/db.test.js +++ b/test/unit/legacy_wrappers/db.test.js @@ -28,6 +28,10 @@ describe('legacy_wrappers/db.js', () => { expect(db.listCollections()).to.be.instanceOf(LegacyListCollectionsCursor); }); + it('collection.listCollections().clone() should return legacy listCollections cursor', () => { + expect(db.listCollections().clone()).to.be.instanceOf(LegacyListCollectionsCursor); + }); + it('should return legacy ChangeStream', () => { expect(db.watch()).to.be.instanceOf(LegacyChangeStream); }); From 4af26afc19edb1a623d9d26e468dde920b48d036 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 25 Jan 2023 11:11:51 -0500 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Warren James <28974128+W-A-James@users.noreply.github.com> --- test/unit/legacy_wrappers/collection.test.js | 2 +- test/unit/legacy_wrappers/db.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/legacy_wrappers/collection.test.js b/test/unit/legacy_wrappers/collection.test.js index 9aee2e6..4189e8d 100644 --- a/test/unit/legacy_wrappers/collection.test.js +++ b/test/unit/legacy_wrappers/collection.test.js @@ -47,7 +47,7 @@ describe('legacy_wrappers/collection.js', () => { expect(collection.listIndexes().clone()).to.be.instanceOf(LegacyListIndexesCursor); }); - it('collection.aggregate().clone() should return legacy ChangeStream', () => { + it('collection.aggregate().clone() should return legacy AggregationCursor', () => { expect(collection.aggregate().clone()).to.be.instanceOf(LegacyAggregationCursor); }); diff --git a/test/unit/legacy_wrappers/db.test.js b/test/unit/legacy_wrappers/db.test.js index 48ecb8d..b8c9a97 100644 --- a/test/unit/legacy_wrappers/db.test.js +++ b/test/unit/legacy_wrappers/db.test.js @@ -28,7 +28,7 @@ describe('legacy_wrappers/db.js', () => { expect(db.listCollections()).to.be.instanceOf(LegacyListCollectionsCursor); }); - it('collection.listCollections().clone() should return legacy listCollections cursor', () => { + it('db.listCollections().clone() should return legacy listCollections cursor', () => { expect(db.listCollections().clone()).to.be.instanceOf(LegacyListCollectionsCursor); });