Skip to content

Commit

Permalink
fix: pass optional promise lib to maybePromise
Browse files Browse the repository at this point in the history
passes `promiseLibrary` option from `MongoClient` to `maybePromise` function and removes `devDependency` `bluebird`.

NODE-2490
  • Loading branch information
Thomas Reggi committed Mar 11, 2020
1 parent 0c36a32 commit cde11ec
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 60 deletions.
10 changes: 4 additions & 6 deletions lib/cursor.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ class Cursor extends CoreCursor {
throw MongoError.create({ message: 'Cursor is closed', driver: true });
}

return maybePromise(callback, cb => {
return maybePromise(this, callback, cb => {
const cursor = this;
if (cursor.isNotified()) {
return cb(null, false);
Expand Down Expand Up @@ -230,7 +230,7 @@ class Cursor extends CoreCursor {
* @return {Promise} returns Promise if no callback passed
*/
next(callback) {
return maybePromise(callback, cb => {
return maybePromise(this, callback, cb => {
const cursor = this;
if (cursor.s.state === CursorState.CLOSED || (cursor.isDead && cursor.isDead())) {
cb(MongoError.create({ message: 'Cursor is closed', driver: true }));
Expand Down Expand Up @@ -819,8 +819,7 @@ class Cursor extends CoreCursor {
driver: true
});
}

return maybePromise(callback, cb => {
return maybePromise(this, callback, cb => {
const cursor = this;
const items = [];

Expand Down Expand Up @@ -1047,8 +1046,7 @@ class Cursor extends CoreCursor {
if (this.cmd.readConcern) {
delete this.cmd['readConcern'];
}

return maybePromise(callback, cb => {
return maybePromise(this, callback, cb => {
CoreCursor.prototype._next.apply(this, [cb]);
});
}
Expand Down
12 changes: 3 additions & 9 deletions lib/mongo_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,18 +157,12 @@ function MongoClient(url, options) {
this.s = {
url: url,
options: options || {},
promiseLibrary: null,
promiseLibrary: (options && options.promiseLibrary) || Promise,
dbCache: new Map(),
sessions: new Set(),
writeConcern: WriteConcern.fromOptions(options),
namespace: new MongoDBNamespace('admin')
};

// Get the promiseLibrary
const promiseLibrary = this.s.options.promiseLibrary || Promise;

// Add the promise to the internal state
this.s.promiseLibrary = promiseLibrary;
}

/**
Expand Down Expand Up @@ -214,7 +208,7 @@ MongoClient.prototype.connect = function(callback) {
}

const client = this;
return maybePromise(callback, cb => {
return maybePromise(this, callback, cb => {
const err = validOptions(client.s.options);
if (err) return cb(err);

Expand Down Expand Up @@ -244,7 +238,7 @@ MongoClient.prototype.close = function(force, callback) {
}

const client = this;
return maybePromise(callback, cb => {
return maybePromise(this, callback, cb => {
const completeClose = err => {
client.emit('close', client);

Expand Down
11 changes: 8 additions & 3 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -696,14 +696,19 @@ function* makeCounter(seed) {
/**
* Helper function for either accepting a callback, or returning a promise
*
* @param {Function} [callback] an optional callback.
* @param {Object} parent an instance of parent with promiseLibrary.
* @param {object} parent.s an object containing promiseLibrary.
* @param {function} parent.s.promiseLibrary an object containing promiseLibrary.
* @param {[Function]} callback an optional callback.
* @param {Function} fn A function that takes a callback
* @returns {Promise|void} Returns nothing if a callback is supplied, else returns a Promise.
*/
function maybePromise(callback, fn) {
function maybePromise(parent, callback, fn) {
const PromiseLibrary = (parent && parent.s && parent.s.promiseLibrary) || Promise;

let result;
if (typeof callback !== 'function') {
result = new Promise((resolve, reject) => {
result = new PromiseLibrary((resolve, reject) => {
callback = (err, res) => {
if (err) return reject(err);
resolve(res);
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
"safe-buffer": "^5.1.2"
},
"devDependencies": {
"bluebird": "3.5.0",
"chai": "^4.1.1",
"chai-subset": "^1.6.0",
"chalk": "^2.4.2",
Expand Down
108 changes: 76 additions & 32 deletions test/functional/byo_promises.test.js
Original file line number Diff line number Diff line change
@@ -1,37 +1,81 @@
'use strict';

const maybePromise = require('./../../lib/utils').maybePromise;
var expect = require('chai').expect;

describe('BYO Promises', function() {
it('should Correctly Use Blurbird promises library', {
metadata: {
requires: {
topology: ['single', 'ssl', 'wiredtiger']
}
},

// The actual test we wish to run
test: function(done) {
var self = this;
const configuration = this.configuration;
var Promise = require('bluebird');

const client = configuration.newClient(
{},
{
promiseLibrary: Promise,
sslValidate: false
}
);

client.connect().then(function(client) {
var db = client.db(self.configuration.db);
var promise = db.collection('test').insert({ a: 1 });
expect(promise).to.be.an.instanceOf(Promise);

promise.then(function() {
client.close(done);
});
});
}
class CustomPromise extends Promise {}
CustomPromise.prototype.isCustomMongo = true;

const parent = { s: { promiseLibrary: CustomPromise } };

describe('Optional PromiseLibrary / maybePromise', function() {
it('should correctly implement custom dependency-less promise', function(done) {
const getCustomPromise = v => new CustomPromise(resolve => resolve(v));
const getNativePromise = v => new Promise(resolve => resolve(v));
expect(getNativePromise()).to.not.have.property('isCustomMongo');
expect(getCustomPromise()).to.have.property('isCustomMongo');
expect(getNativePromise()).to.have.property('then');
expect(getCustomPromise()).to.have.property('then');
done();
});

it('should return a promise with extra property CustomMongo', function() {
const prom = maybePromise(parent, undefined, () => 'example');
expect(prom).to.have.property('isCustomMongo');
expect(prom).to.have.property('then');
});

it('should return a native promise with no parent', function(done) {
const prom = maybePromise(undefined, undefined, () => 'example');
expect(prom).to.not.have.property('isCustomMongo');
expect(prom).to.have.property('then');
done();
});

it('should return a native promise with empty parent', function(done) {
const prom = maybePromise({}, undefined, () => 'example');
expect(prom).to.not.have.property('isCustomMongo');
expect(prom).to.have.property('then');
done();
});

it('should return a native promise with emtpy "s"', function(done) {
const prom = maybePromise({ s: {} }, undefined, () => 'example');
expect(prom).to.not.have.property('isCustomMongo');
expect(prom).to.have.property('then');
done();
});

it('should have cursor return native promise', function(done) {
const configuration = this.configuration;
const client = this.configuration.newClient({ w: 1 }, { poolSize: 1 });
client.connect((err, client) => {
expect(err).to.not.exist;
const db = client.db(configuration.db);
const collection = db.collection('test');
const cursor = collection.find({});
const isPromise = cursor.toArray();
expect(isPromise).to.not.have.property('isCustomMongo');
expect(isPromise).to.have.property('then');
isPromise.then(() => client.close(done));
});
});

it('should have cursor return custom promise from new client options', function(done) {
const configuration = this.configuration;
const client = this.configuration.newClient(
{ w: 1 },
{ poolSize: 1, promiseLibrary: CustomPromise }
);
client.connect((err, client) => {
const db = client.db(configuration.db);
expect(err).to.be.null;
const collection = db.collection('test');
const cursor = collection.find({});
const isPromise = cursor.toArray();
expect(isPromise).to.have.property('isCustomMongo');
expect(isPromise).to.have.property('then');
isPromise.then(() => client.close(done));
});
});
});
8 changes: 5 additions & 3 deletions test/functional/promises_db.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ var test = require('./shared').assert;
var setupDatabase = require('./shared').setupDatabase;
var f = require('util').format;

class CustomPromise extends Promise {}
CustomPromise.prototype.isCustomMongo = true;

describe('Promises (Db)', function() {
before(function() {
return setupDatabase(this.configuration);
Expand Down Expand Up @@ -373,7 +376,7 @@ describe('Promises (Db)', function() {
}
});

it('Should correctly execute createCollection using passed down bluebird Promise', {
it('Should correctly execute createCollection using passed down CustomPromise Promise', {
metadata: {
requires: {
topology: ['single']
Expand All @@ -384,9 +387,8 @@ describe('Promises (Db)', function() {
test: function(done) {
var configuration = this.configuration;
var db = null;
var BlueBird = require('bluebird');

const client = configuration.newClient({}, { promiseLibrary: BlueBird });
const client = configuration.newClient({}, { promiseLibrary: CustomPromise });
client
.connect()
.then(function() {
Expand Down
15 changes: 13 additions & 2 deletions test/functional/spec-runner/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
const Promise = require('bluebird');

const path = require('path');
const fs = require('fs');
const chai = require('chai');
Expand All @@ -14,6 +14,17 @@ chai.config.includeStack = true;
chai.config.showDiff = true;
chai.config.truncateThreshold = 0;

// Promise.try alternative https://stackoverflow.com/questions/60624081/promise-try-without-bluebird/60624164?noredirect=1#comment107255389_60624164
function promiseTry(callback) {
return new Promise((resolve, reject) => {
try {
resolve(callback());
} catch (e) {
reject(e);
}
});
}

function escape(string) {
return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
}
Expand Down Expand Up @@ -571,7 +582,7 @@ function testOperation(operation, obj, context, options) {
opPromise = cursor.toArray();
} else {
// wrap this in a `Promise.try` because some operations might throw
opPromise = Promise.try(() => obj[operationName].apply(obj, args));
opPromise = promiseTry(() => obj[operationName].apply(obj, args));
}

if (operation.error) {
Expand Down
9 changes: 5 additions & 4 deletions test/unit/cmap/connection_pool.test.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
'use strict';

const Promise = require('bluebird');
const loadSpecTests = require('../../spec').loadSpecTests;
const ConnectionPool = require('../../../lib/cmap/connection_pool').ConnectionPool;
const EventEmitter = require('events').EventEmitter;
const mock = require('mongodb-mock-server');
const BSON = require('bson');
const util = require('util');
const cmapEvents = require('../../../lib/cmap/events');

const chai = require('chai');
Expand All @@ -26,8 +26,8 @@ const ALL_POOL_EVENTS = new Set([
]);

const PROMISIFIED_POOL_FUNCTIONS = {
checkOut: Promise.promisify(ConnectionPool.prototype.checkOut),
close: Promise.promisify(ConnectionPool.prototype.close)
checkOut: util.promisify(ConnectionPool.prototype.checkOut),
close: util.promisify(ConnectionPool.prototype.close)
};

function closePool(pool) {
Expand Down Expand Up @@ -379,14 +379,15 @@ describe('Connection Pool', function() {
return p
.then(() => {
const connectionsToDestroy = Array.from(orphans).concat(Array.from(connections.values()));
return Promise.each(connectionsToDestroy, conn => {
const promises = connectionsToDestroy.map(conn => {
return new Promise((resolve, reject) =>
conn.destroy({ force: true }, err => {
if (err) return reject(err);
resolve();
})
);
});
return Promise.all(promises);
})
.then(() => {
pool = undefined;
Expand Down

0 comments on commit cde11ec

Please sign in to comment.