1
\$\begingroup\$

I am using KeystoneJS to manage data and my APIs (which includes Node.js, MongoDB, and Express.js). It uses Mongoose to connect with MongoDB; Keystone queries are essentially the same as Mongoose queries, just prefixed with keystone.list('listname').model instead of mongoose.model('name').

My use case is loading information about a competition when a relevant page is being visited by a user. I'm using Express.js to identify the competition we're looking for (stored on the key path of my Competition model, and identified in Express as req.params.cid (Competition ID). Other Express.js routes will use the information from the res.locals.competition path (name, date, location, etc.)

What I am currently doing is running one query to get information about the current competition every time the page is visited. This is stored as a middleware function, loadCompetition, which is called for every Express route that matches /competition/cid*.

app.all('/competition/:cid*', middleware.loadCompetition);

My code looks like this:

exports.loadCompetition = function (req, res, next) {
    keystone.list('Competition').model.findOne({key: req.params.cid}).exec(function (err, competition) {
        if (competition && !err) {
            res.locals.competition = competition;
            return next();
        }
        else {
            if (!competition) {
                req.flash("error", "No competition could be found at this page.");
                return res.redirect("/");
            }
            else  {
                req.flash("error", "An unexpected error occurred. Please try again! If this error persists, email [email protected]");
                return res.redirect("/");
            }
        }
    });
};

Is this the most effective way of loading a competition like this? Is it feasible to load information about the competition once, and only need to reload it if something changes? Or is what I'm doing the only realistic way of loading information on a competition? I think I would need to query the competition anyway to see if datum has changed (Keystone can track the date a document was modified at, on the updatedAt path), and that would make this entirely redundant. But I don't know if I am missing something. If anything else looks awry, please let me know as well.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Is this slowing down your app by a significant amount? If not, I wouldn't worry about it.

Other than that I would consider removing the custom error handling, and rather pass it on to some other more general error handler at the end of your middleware stack (just call next(err)), and maybe make the Not Found error a 404 status code.

\$\endgroup\$
1
  • \$\begingroup\$ It isn't slowing it down significantly. I just didn't know if there was a better, or possibly more ideal, way of doing this. I'll keep the code for now and redo the error handling. Thank you very much! \$\endgroup\$ Apr 15, 2017 at 20:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.