Provide a way to include existing mongo connection

[Risto-Stevcev]

The library currently opens its own connection to a mongo uri, but it would be nice to be able to just pass in an existing connection and reuse its connection pool. Something like:

//  connect-mongodb-session/index.js
...
  var MongoDBStore = function(options, callback) {
     ...
     var initCollection = function(db) {
       db.
         collection(options.collection).
         ensureIndex({ expires: 1 }, { expireAfterSeconds: 0 }, function(error) {
           if (error) {
             var e = new Error('Error creating index: ' + error.message);
             return _this._errorHandler(e, callback);
           }
           _this.db = db;
           _this._emitter.emit('connected');

           return callback && callback();
         });
      }
      ...
      if (options.db) {
        initCollection(options.db)
      }
      else {
        mongodb.MongoClient.connect(options.uri, connOptions, function(error, db) {
          if (error) {
            var e = new Error('Error connecting to db: ' + error.message);
            return _this._errorHandler(e, callback);
          }
          initCollection(db)
       });
    }
  ...

// example.js
var express = require('express');
var mongoose = require('mongoose');
var session = require('express-session');
var MongoDBStore = require('connect-mongodb-session')(session);

var db = mongoose.createConnection('mongodb://user:pass@localhost:port/database');
var store = new MongoDBStore({ 
    db: db
    collection: 'mySessions'
})

[vkarpov15]

Sorry, but this feature is left out intentionally, I don't think exposing the ability to reuse existing connections is a good idea. See my reasoning in #15 (comment). I'm always open to a good debate on the subject though :)

[Risto-Stevcev]

My main rebuttal is that the mongo driver for this library could differ from the driver you use for mongoose (or in general), which could cause unexpected and hard-to-debug problems. I don't really see your reasoning for #15. For point 1, I'm not familiar with the mongoose issues relating to old versions of connect-mongo, but it looks like that hasn't been an issue for quite a while, and the library gives the option to open it's own connection rather than re-using and existing one. Point 2 doesn't really seem valid either, because if you can't reason about performance and adjust and scale properly, you'll have problems with your app in general -- introducing another connection just makes that process more complicated. Point 3 doesn't make sense to me either, because you can advertise the uri as the main method of using the library, but still include the option for db in the API docs, like connect-mongo does.

Another reason is that all of the features and customizations available from the latest driver might not be (and currently isn't) available for your library. For example, what if I wanted to include SSL (which is perfectly reasonable)? at best, if I could include the options I wanted, I would have to copy the same configuration for the session connection, which would repeat code and potentially introduce bugs. And if the driver versions between the two connections differ in the worst ways possible, it could become a real pain to both debug and fix.

[vkarpov15]

The reason for using a separate driver is precisely to reduce the potential for unexpected and hard-to-debug problems. A connection is a complex object, and connect-mongo has a long history of weird monkey-patching of mongoose connections that gives me headaches. Instantiating a new connection using the driver gives this module a nice rigid setup where its hard to do anything horribly wrong.

Re: point 2 and point 3, most users don't want to read the code or a long blob of configuration options, they just want something that works out of the box with minimum tweaking (myself included). I think this module is the best way to obtain that behavior for a mongodb connect session store, and adding extra knobs just adds docs overhead and additional moving parts that can break.

Also, re: all features and customizations, we do have the latest mongodb driver . And SSL is pretty easy, if you have the correct uri and options for the mongodb driver, you already have the correct uri and options for this module as well.

require('mongodb').MongoClient.connect(mongoUri, mongoOptions, function(err, db) {
});

var store = new MongoDBStore({ 
  uri: mongoUri,
  connectionOptions: mongoOptions,
  collection: 'mySessions'
});

[Risto-Stevcev]

I looked at their source code, and I didn't see any monkey-patching of mongoose connections. But breaking some inversion of control seems like a bad idea.. what if I wanted to use something like mockgoose? It just gives the library less flexibility in general, and I wouldn't call adding a single db attribute to the docs as the same thing as introducing a long blob of configuation options.

I see what you're saying, but I personally would just have the default way of using the library apparent in the README, and that would prevent the vast majority of people from doing wonky things and producing weird errors, since most users don't want to read a bunch of specs or config options as you said and will just do what the README says. If I really wanted to drive the point, I would put a disclaimer next to the db option saying something like "use at your own risk".

[vkarpov15]

If you wanted to use mockgoose, I'd say you're making a terrible mistake. First, supporting mockgoose just adds another flavor of mongodb that this library needs to make sure to support in addition to versions 2.2, 2.4, 2.6, 3.0, and 3.2. If you're going to do an integration test, then test against the actual thing you're integrating against. If you're going to stub something out, write an actual stub. Don't write something that attempts to mimic the real functionality, because then you're not testing your integration with mongodb, you're testing your integration with mockgoose, which is not accurate.

While I see your point about wanting to test against a stub, its not the your responsibility as a user of this module to unit test its internals. If you want to test your integration with this module, plenty of examples as to how to do that in the tests: https://github.com/mongodb-js/connect-mongodb-session/blob/master/test/examples.test.js .

While I agree that we can just hide the proposed option from the end user, its still another moving part that will need support, even if its marked with 'use at your own risk'. The core principle of this module is that smaller API surface area = fewer bugs. If you're comfortable managing connection state and reasoning about the performance implications of your session store, you probably already understand enough to write your own and can always fork :)

[Risto-Stevcev]

I haven't used mockgoose, I use a test database for integration tests for similar reasons you mentioned. I just mentioned it as an example of something that might make you wish you had more granular control. But I see your point though. Your library is pretty small and lightweight, and therefore easy fork and customize if necessary.

[vkarpov15]

yeah that's really the point atm. If you really want to customize to fit your use case, its easy to fork this module because it focuses on minimizing bloat.