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`.
  • Loading branch information
Thomas Reggi authored Apr 29, 2020
1 parent fa052a5 commit 52be01f
Show file tree
Hide file tree
Showing 9 changed files with 115 additions and 68 deletions.
10 changes: 4 additions & 6 deletions lib/cursor/cursor.js
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,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 @@ -221,7 +221,7 @@ class Cursor extends CoreCursor {
* @returns {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 @@ -840,8 +840,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 @@ -1070,8 +1069,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 @@ -154,18 +154,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;
}

inherits(MongoClient, EventEmitter);
Expand Down Expand Up @@ -209,7 +203,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 @@ -240,7 +234,7 @@ MongoClient.prototype.close = function(force, callback) {
}

const client = this;
return maybePromise(callback, cb => {
return maybePromise(this, callback, cb => {
if (client.topology == null) {
cb();
return;
Expand Down
15 changes: 10 additions & 5 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -689,22 +689,27 @@ function* makeCounter(seed) {
/**
* Helper function for either accepting a callback, or returning a promise
*
* @param {Function} [callback] an optional callback.
* @param {Function} fn A function that takes a callback
* @param {object} parent an instance of parent with promiseLibrary.
* @param {object} parent.s parent internal state
* @param {any} parent.s.promiseLibrary an object containing promiseLibrary.
* @param {?Function} callback The last function argument in exposed method, controls if a Promise is returned
* @param {Function} wrapper A function that wraps the callback
* @returns {Promise|void} Returns nothing if a callback is supplied, else returns a Promise.
*/
function maybePromise(callback, fn) {
function maybePromise(parent, callback, wrapper) {
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);
};
});
}

fn(function(err, res) {
wrapper(function(err, res) {
if (err != null) {
try {
callback(err);
Expand Down
6 changes: 0 additions & 6 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
},
"devDependencies": {
"@types/node": "^13.9.8",
"bluebird": "3.5.0",
"chai": "^4.1.1",
"chai-subset": "^1.6.0",
"chalk": "^2.4.2",
Expand Down
107 changes: 76 additions & 31 deletions test/functional/byo_promises.test.js
Original file line number Diff line number Diff line change
@@ -1,36 +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']
}
},

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 @@ -361,7 +364,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 @@ -371,9 +374,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: 12 additions & 3 deletions test/functional/spec-runner/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
'use strict';
const Promise = require('bluebird');
const path = require('path');
const fs = require('fs');
const chai = require('chai');
Expand All @@ -8,6 +7,16 @@ const { EJSON } = require('bson');
const TestRunnerContext = require('./context').TestRunnerContext;
const resolveConnectionString = require('./utils').resolveConnectionString;

function promiseTry(callback) {
return new Promise((resolve, reject) => {
try {
resolve(callback());
} catch (e) {
reject(e);
}
});
}

chai.use(require('chai-subset'));
chai.use(require('./matcher').default);
chai.config.includeStack = true;
Expand Down Expand Up @@ -638,8 +647,8 @@ function testOperation(operation, obj, context, options) {
const cursor = obj[operationName].apply(obj, args);
opPromise = cursor.toArray();
} else {
// wrap this in a `Promise.try` because some operations might throw
opPromise = Promise.try(() => obj[operationName].apply(obj, args));
// wrap this in a `promiseTry` because some operations might throw
opPromise = promiseTry(() => obj[operationName].apply(obj, args));
}
}

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,6 +1,6 @@
'use strict';

const Promise = require('bluebird');
const util = require('util');
const loadSpecTests = require('../../spec').loadSpecTests;
const ConnectionPool = require('../../../lib/cmap/connection_pool').ConnectionPool;
const EventEmitter = require('events').EventEmitter;
Expand All @@ -25,8 +25,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 @@ -370,14 +370,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 52be01f

Please sign in to comment.