Sequelize different pattern to import Model

[roccomuso]

I'm using this kind of pattern (provided on the sequelize documentation) to import Models from files:

var fs        = require("fs");
var path      = require("path");
var Sequelize = require("sequelize");
var env       = process.env.NODE_ENV || "development";
var config    = require(path.join(__dirname, '..', 'config', 'config.json'))[env];
var sequelize = new Sequelize(config.database, config.username, config.password, config);
var db        = {};

fs
  .readdirSync(__dirname)
  .filter(function(file) {
    return (file.indexOf(".") !== 0) && (file !== "index.js");
  })
  .forEach(function(file) {
    var model = sequelize.import(path.join(__dirname, file));
    db[model.name] = model;
  });

Object.keys(db).forEach(function(modelName) {
  if ("associate" in db[modelName]) {
    db[modelName].associate(db);
  }
});

db.sequelize = sequelize;
db.Sequelize = Sequelize;

module.exports = db;

Now I'm trying to apply the cache layer adding a final loop in this way, to expose the "wrapped" Models:

Object.keys(db).forEach(function(modelName){
  db[modelName] = cacher(modelName).ttl(5);
});

And I'm having this error:

` Possibly unhandled Error: Include unexpected. Element has to be either a Model, an Association or an object.

/node_modules/sequelize/lib/model.js:301:11 `

Up to the stackTrace until Line: index.js#L68

Have you ever tested sequelize-redis-cache with this kind of pattern? what's the right wat to apply it?

[rfink]

Hello,

Can you provide the code for bootstrapping the models? I believe this error comes from trying to add an association on a non-model. I haven't used this pattern before to wire up the cacher, but I think I've seen this error.

[roccomuso]

This is the User model:

module.exports = function(sequelize, DataTypes) {
  var User = sequelize.define("User", {
    username: DataTypes.STRING
  }, {
    classMethods: {
      associate: function(models) {
        User.hasMany(models.Task);
      }
    }
  });

  return User;
};

[rfink]

So I think I know what's happening here. Here, you are calling associate:

Object.keys(db).forEach(function(modelName) {
  if ("associate" in db[modelName]) {
    db[modelName].associate(db);
  }
});

Which stores the association internally to the core model (User, for example). However, when you go to prime the models with the cacher instance, you are updating the reference on the "db" object to the cacher instance, instead of the model (and because of how javascript handles object references, the association also becomes an instance of the cacher). When you go to run the query, sequelize checks the type of the association and sees that it is not a Sequelize Instance, and throws the error. I think something like this might work:

db.cached = {};
Object.keys(db).forEach(function(modelName){
  db.cached[modelName] = cacher(modelName).ttl(5);
});

That way you can access the models with and without the cacher, be either db.User.find or db.cached.User.find. Does that make sense?

[roccomuso]

Tried and got the same error:

Possibly unhandled Error: Include unexpected. Element has to be either a Model, an Association or an object.

[rfink]

Interesting. What version of sequelize are you using?

[roccomuso]

@rfink version 3.3.2. Have you tested that code? are you having the same issue?

[rfink]

I haven't, but now that I know your version, I'm going to give it a shot. Can you also shoot me what the Task model looks like?

[roccomuso]

This is the Task model:

module.exports = function(sequelize, DataTypes) {
  var Task = sequelize.define("Task", {
    title: DataTypes.STRING
  }, {
    classMethods: {
      associate: function(models) {
        Task.belongsTo(models.User, {
          onDelete: "CASCADE",
          foreignKey: {
            allowNull: false
          }
        });
      }
    }
  });

  return Task;
};

[roccomuso]

Guess i found something to investigate on...

We're exporting the Cached istances of the Model... And then in the web server routes we have something like:

var models  = require('../models');
var express = require('express');
var router  = express.Router();

router.get('/', function(req, res) {
  models.User.findAll({
    include: [ models.Task ]
  }).then(function(users) {
    res.render('index', {
      partials: {body: 'body'},
      title: 'Express',
      users: users
    });
  });
});

module.exports = router;

Now on line 7 we're doing include: [models.Task] but this is not the Sequelize Model instance, instead a Cacher instance:

{ include: 
   [ Cacher {
       engine: [Object],
       method: 'find',
       modelName: 'Task',
       model: Task,
       options: {},
       seconds: 120,
       cacheHit: false,
       cachePrefix: 'cacher' } ] }

So editing the routes line 7 in this way should work: include: [models.Task.model] Does that make sense?

[rfink]

Right, this is back to the issue of the object referencing on the "db" object in the models/index.js file. This is what I've tried below:

var redis     = require('redis');
var initCache = require('sequelize-redis-cache');
var fs        = require("fs");
var path      = require("path");
var Sequelize = require("sequelize");
var env       = process.env.NODE_ENV || "development";
var config    = require(path.join(__dirname, '..', 'config', 'config.json'))[env];
var sequelize = new Sequelize(config.database, config.username, config.password, config);
var db        = {};
var rc        = redis.createClient();

fs
  .readdirSync(__dirname)
  .filter(function(file) {
    return (file.indexOf(".") !== 0) && (file !== "index.js");
  })
  .forEach(function(file) {
    var model = sequelize.import(path.join(__dirname, file));
    db[model.name] = model;
  });

Object.keys(db).forEach(function(modelName) {
  if ("associate" in db[modelName]) {
    db[modelName].associate(db);
  }
});

var cacher = initCache(sequelize, rc);

Object.keys(db).forEach(function(modelName){
  db[modelName] = cacher(modelName).ttl(5);
});

db.sequelize = sequelize;
db.Sequelize = Sequelize;

module.exports = db;

db.User.findlAll({ where: { id: 1 } });

And that actually worked. This is my package.json:

{
  "name": "project",
  "version": "1.0.0",
  "main": "index.js",
  "dependencies": {
    "mysql": "^2.11.1",
    "redis": "^2.6.2",
    "sequelize": "^3.3.2",
    "sequelize-redis-cache": "^1.2.0"
  }
}

Are your versions comparable?

[roccomuso]

Yeah, the problem were in the express route:

var models  = require('../models');
var express = require('express');
var router  = express.Router();

router.get('/', function(req, res) {
  models.User.findAll({
    include: [ models.Task ]
  }).then(function(users) {
    res.render('index', {
      partials: {body: 'body'},
      title: 'Express',
      users: users
    });
  });
});

module.exports = router;

That models.Task refer to the Cacher instance instead of the Sequelize one. To fix simply use models.Task.model. That's it I guess :)

[rfink]

Well I'm very happy that it was found. I still would recommend, for the sake of debugging, to set up the cacher as a separate object on the db, e.g.

db.cacher = {};
Object.keys(db).forEach(function(modelName){
  db.cacher[modelName] = cacher(modelName).ttl(5);
});

That way you can still use the exported db object just like the docs, or you can flip using the cacher at any time, e.g.

models.cacher.User.findAll({
  include: [ models.Task ]
})

Best of luck!

[roccomuso]

;) @rfink thank you.

PS. I'm going to open a new issue to ask for the best cache policies to adopt and if we could discuss about the introduction of the cache invalidation methods.

[rfink]

Sounds great! I'll go ahead and close this one.