Discussion on: Sharing the context with the model in an Express app

View post

Hi Fernando,
I agree that adding a global filter data like companyId in each controller would be error-prone and insecure. I like your approach - a middleware which "prepares" models.

I would do some very small changes:

1. I wouldn't directly modify the prototype in the middleware, but create a new model instance:

module.exports = (req, res, next) => {
  const companyId = req.context.user.companyId;

  const models = {
    projectModel: new ProjectModel(companyId)
  };

  req.context.models = models;

  next();
};

2. In model there are two classes: Model responsible for fetching/persisting data and a pure Project class to store data:

class Project {
  constructor({ id, companyId, name }) {
    this.id = id;
    this.companyId = companyId;
    this.name = name;
  }
}

class ProjectModel {
  constructor(companyId) {
    this.companyId = companyId;
  }

  find() {
    return PROJECTSDATA.filter(
      project => project.companyId === this.companyId
    ).map(projectData => new Project(projectData));
  }

  // here other methods...
}

module.exports = ProjectModel;

What do you think?

Fernando Calvo • Nov 6 '19

Hi Kryz! thanks for your comment!

I like your idea! I wasn't sure about modifying the prototype in the middleware, I like your approach, but I think it has to be a better approach because now the static methods are in ProjectModel and the instance methods will be in Project.

I think we can export the model like this:

module.exports = (context) => {
  class Project {
    // constructor
    // static and instance methods
  }
  Project.prototype._context = context;
  Project._context = context;
}

This allows us not to modify the prototype from the middleware and do it in the model. It also binds the whole context to the model, this can be important as we could have more things in the context like a logger or any other function that requires the request context.

We could also take advantage of the closures and do something like

module.exports = (context) => {
  class Project {
    // constructor
    find() {
      const companyId = context.user.company;
      // search in the database by companyId
    }
  }
  Project.prototype._context = context;
  Project._context = context;
}

With this approach, the methods are more elegant but we can't split the model into different files.

What do you think?

Thank you for your answer!

Kryz • Nov 8 '19

Hi,
I like your idea!

One more thing - the middleware runs the factory on each request, is it good for the application performance?

Maybe it would be better to delay this calls, for example:

  const models = {
    project: () => ...here your factory call
  };

  req.context.models = models;

and in the controller something like this:

const projectModel = req.context.models.project();

I know, it doesn't look nice, I'm sure there is a better solution..