balmasi / migrate-mongoose

A node based migration framework for mongoose supporting ES6 migrations
MIT License
264 stars 84 forks source link

Give up() and down() functions access to connection #45

Open eweitnauer opened 5 years ago

eweitnauer commented 5 years ago

I'm missing a way to access the DB connection information inside the up and down functions in migrations.

When using the CLI tool there are several ways to pass connection information (db uri): environment variables, the config file, and command line arguments. This is great, and migrate-mongoose uses this information to connect to the database, update the migration collection, and to give the up and down functions of migrations access to mongoose models.

In some of the migrations, I'd like to get more direct access to the mongo db to rename collections. My problem is that I can't figure out how to get the connection information that I provided to migrate-mongoose inside the up and down functions. The this context only lets me create mongoose models.

Ideally, I'd like to be able to do something like the following:

async function up() {
  const uri = this.config.dbConnectionUri // attach config object to mongoose connection.model
  const mongoClient = new MongoClient(uri, { useNewUrlParser: true });
  ... 
}

OR

async function up() {
  const mongoClient = this.client // pass mongoose connection instead of connection.model
  ... 
}

Does this make sense or am I missing something that is already in place?

robodude666 commented 5 years ago

@eweitnauer Can't you access mongoose directly by importing/requiring it? My understanding is mongoose is a singleton.

See: https://thecodebarbarian.com/whats-new-in-mongoose-5-improved-connections https://stackoverflow.com/a/43392287/2583994

eweitnauer commented 5 years ago

@robodude666 Based on what you sent, I tried to do the following in the migration code

async function up() {  
  const mongoose = require('mongoose');
  console.log(mongoose.connection);
}

but only get a disconnected connection object:

NativeConnection {
  base:
   Mongoose {
     connections: [ [Circular] ],
     models: {},
     modelSchemas: {},
     options: { pluralization: true },
     _pluralize: [Function: pluralize],
     Schema:
      { [Function: Schema] reserved: [Object], Types: [Object], ObjectId: [Function] },
     model: [Function],
     plugins: [ [Array], [Array], [Array], [Array] ] },
  collections: {},
  models: {},
  config: { autoIndex: true },
  replica: false,
  options: null,
  otherDbs: [],
  relatedDbs: {},
  states:
   { '0': 'disconnected',
     '1': 'connected',
     '2': 'connecting',
     '3': 'disconnecting',
     '99': 'uninitialized',
     disconnected: 0,
     connected: 1,
     connecting: 2,
     disconnecting: 3,
     uninitialized: 99 },
  _readyState: 0,
  _closeCalled: false,
  _hasOpened: false,
  plugins: [],
  _listening: false }

I am not quite sure why, since mongoose should be connected at the time the script is called (see https://github.com/balmasi/migrate-mongoose/blob/master/src/lib.js#L48 and https://github.com/balmasi/migrate-mongoose/blob/master/src/lib.js#L172).

marceliwac commented 5 years ago

@eweitnauer I've had similar issue in which migrations would not run due to connection being dead. For me the solution was simple, as I've already had a wrapper for mongoose module, which handled connections, so all I've had to do was connect to the database at the top of both up() and down().

Effectively, you should be able to get connected connection object by importing mongoose (as you did) and calling mongoose.connect(...) with your DB credentials.

eweitnauer commented 5 years ago

@marceliwac That is what I ended up doing, too!

My dissatisfaction is with the fact that now I need to tell both sequelize and my own connection handler the connection information. It would be much nicer if there was a way to either get the connection or connection information from sequelize inside the up() and down() methods. This would avoid duplication and potential inconsistencies.

The required change in the sequelize migration code would be minimal. The best way to preserve backwards compatibility that I can think of is to pass the connection object as a second parameter to the up() and down() method. (The first parameter is an error callback.) Does that sound reasonable? If so, I can open a PR.

marceliwac commented 5 years ago

@eweitnauer That does sound like a solution. Either way, currently it seems like the behaviour concerning the mongoose connection seems rather poorly documented so it would be great to continue this dialog (perhaps with someone from the group of more regular maintainers - @balmasi ?).