Skip to content

db.load vs orm.express #291

Closed
Closed
@colegleason

Description

@colegleason

I have a single models.js module that I'd like to load in both Express request handlers and non-Express code. It would be nice if the function definitions for these two cases could be similar so that the models.exports function could be written once well.

Express expects a function with db and models as an argument:

define: function (db, models) {
    models.person = db.define("person", { ... });
}

On the other hand, db.load wants to pass just a callback:

db.load("./models", function (err) {
    // loaded!
    var Person = db.models.person;
    var Pet    = db.models.pet;
});

Activity

dxg

dxg commented on Aug 11, 2013

@dxg
Collaborator

+1 this needs to be solved as it's causing confusion.

The whole idea of integrating model definitions into express has bugged me from the begining. People get really confused when they need to eg have a "sync" task runnable from the command line.

Personally, I use something like this:

// database.js

var connection = null;

function setup(db) {
  var User  = db.define('user', ...);
  var Email = db.define('email' ...);
  Email.hasOne('user', User, ...);
}

module.exports = function (cb) {
  if (connection) return connection;

  var opts = {
    host     : 'localhost',
    database : 'blah',
    protocol : 'mysql',
    port     : '3306',
    query    : { pool: true }
  };

  orm.connect(opts, function (err, db) {
    if (err) return cb(err);

    connections = db;
    setup(db);

    cb(null, db);
  });  
};

// middleware

var database = require('./database');

app.use(function (req, res, next) {
  database(function (err, db) {
    if (err) return res.send(500, "cannot connect ot database");

    req.db = db;
    req.models = db.models;

    next();
  });
});

I omit the built in methods of loading altogether as they don't work how I'd like.
We could probably integrate db.load into the above, and provide a simple middleware to add req.db and req.models.
I'm also planning to add a simple example app with express, a sync task, and some basic functionality so that people have an easy template to follow, as issues like this one keep coming up.

colegleason

colegleason commented on Aug 13, 2013

@colegleason
Author

I'm running into more issues here.

db.load's callbacks work fine, which means I can essentially 'block' until db has synced. The express middleware, on the other hand, does not expose access to the next() call in order to chain callbacks.

The issue here is that the below does not work:

exports.express = function(db, models) {                                                                                                                                                      
    load(db, function() {  // will be called once db.sync() completes                                                                                                                                                                  
        _.extend(models, db.models);                                                                                                                                                          
    });                                                                                                                                                                                       
};   

In unit testing with my test database, the request is executed quick enough that request.models is not set in the callback above. More accurately, it isn't set until after the test fails, but you know.

I'm not sure this extra middleware is worth it in the current state.

grahamreeds

grahamreeds commented on Aug 14, 2013

@grahamreeds

I use

    app.use(orm.express("sqlite://sqlite3.s3db",{
        define: function(db, models){
            db.load('./models', function(err){
                models.person = db.models.Person;
                models.phone = db.models.Phone;
                // etc.
            });
        }
    }));

where my models/index.js looks like

module.exports = function(db,fn){
    db.load('person',   function(err){ if (err) { return fn(err); } });
    db.load('phone',    function(err){ if (err) { return fn(err); } });
    return fn();
};

Could be neater (especially the exports) but works for me.

dxg

dxg commented on Aug 14, 2013

@dxg
Collaborator

models/index.js has a bug; Since db.load is asynchronous, it means return fn() could be called first, and then once the async load finishes (with an error) it would call fn a second time.

That said, I'm not convinced that load needs to be async in the first place.

I've started work on the example app, but nothing to show just yet. Once it's done will have a discussion with everyone and hopefully update the api+docs.

dresende

dresende commented on Aug 19, 2013

@dresende
Owner

db.load was created as async because you could have your model definition remotely or given by a service or something. The first part does a require of the given path but then the function called can take some time. The Express part should be changed to be async too.

dresende

dresende commented on Aug 20, 2013

@dresende
Owner

@colegleason please try this latest commit. If you define your function with a 3rd argument it will assume it's a next callback that you call when everything is loaded.

SamuelBolduc

SamuelBolduc commented on Aug 22, 2013

@SamuelBolduc

Just a heads up, this should be updated in the readme as I had a pretty hard time figuring out how to declare it the correct way using the middleware (and not all in my app.js file)

ptnplanet

ptnplanet commented on Aug 24, 2013

@ptnplanet

I just want to share my solution:

File stack.js

require defineModels = require('./model').define;
app.use(orm.express('sqlite://sqlite3.s3dbt', { define: defineModels }));

File model.js

var async = require('async'),
    definedModels;
module.exports.define = function (db, models, next) {
    definedModels = models;
    var curriedLoad = function (file) { return function (cb) { db.load(file, cb); }; };

    async.waterfall([
            curriedLoad('.model/user'),
            curriedLoad('.model/posting'),
            // ...
    ], function (err) {
        db.models.user.hasMany(db.models.posting);
        db.sync(next);
    });
};

// I use passport.js and need models without being able to access them through the request object.
module.exports.model = function (name) {
    return definedModels[name];
}
dresende

dresende commented on Aug 26, 2013

@dresende
Owner

This will pass to the readme or the wiki as soon as we release a new version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      db.load vs orm.express · Issue #291 · dresende/node-orm2