Optimizing async joins for mongodb (mongoose) using async.js

Question

I'm building a media viewer web app for a kiosk in node that uses data in mongo. To render the viewer, it gathers together all the Asset objects (representing video files), each of which belong to a Category, each of which belong to a Medium.

This is my first time using mongo and node together, so I decided to use async.js to organize the code, but I don't know if I'm doing things efficiently or not. Is there a better way to organize the data or perform the queries? Does this look remotely sane?

Additionally, I commented out some code I thought looked correct but would mysteriously overwrite the object simply by adding another key to it. What am I doing wrong?

models.js : mongoose models

var mongoose = require('mongoose');
/* Mediums... Film, Video, TV */
var mediumSchema = mongoose.Schema({
 name: String,
 id: String,
 description: String
});
var Medium = mongoose.model('mediums', mediumSchema);
/* Categories ... */
var categorySchema = mongoose.Schema({
 medium: String, // refers to medium.id, not medium._id
 name: String,
 shortName: String,
 years: String,
 description: String
});
var Category = mongoose.model('categories', categorySchema);
/* Assets */
var assetSchema = mongoose.Schema({
 title: String
, year: Number
, duration: Number
, medium_info: String
, copyright: String
, extended_info: String
, filename: String
, category_id: mongoose.Schema.Types.ObjectId
});
var Asset = mongoose.model('assets', assetSchema);
exports.Medium = Medium;
exports.Category = Category;
exports.Asset = Asset;

viewer.js : the handler that retrieves all the data and renders the html

exports.index = function(req, res){
 res.render('index');
};
exports.viewer = function(req, res) {
 var models = require('./models');
 var async = require("async");
 var mongoose = require('mongoose');
 mongoose.connect('mongodb://localhost/test');
 var db = mongoose.connection;
 db.on('error', console.error.bind(console, 'connection error:'));
 db.once('open', function callback () {});
 // provide category object, returns assets in that category
 function getAssetsForCategory(category, callback) {
 console.log("Looking for assets for category:" + category.name);
 models.Asset.find({category_id: category._id}, function(err, assets) {
 callback(err, assets);
 });
 }
 // provide medium label ("FILM","VIDEO","TV") returns list of categories
 function getCategoriesForMedium(medium, callback) {
 console.log("finding categories for " + medium);
 models.Category.find({medium: medium}, function(err, categories) {
 callback(err, categories);
 });
 }
 data = {};
 async.each(
 ["FILM","VIDEO","TV"],
 function(mediumName, mediumLookupComplete) {
 var mediumResults = {};
 var categoryResults = {};
 var assetResults = {};
 async.waterfall(
 [
 function(findMediumComplete) {
 // get medium object for its additional metadata
 models.Medium.findOne({id: mediumName}, findMediumComplete)
 },
 function(medium, findCategoriesComplete) {
 mediumResults.name = medium.name;
 mediumResults.description = medium.description;
 getCategoriesForMedium(medium.id, findCategoriesComplete);
 },
 function(categories, assetFetchComplete) {
 async.each(
 categories,
 function(category, categoryAssetLookupDone) {
 categoryResults[category._id] = category;
 /* I originally wanted to just insert the list of assets
 into their appropriate category object here, but doing so
 would overwrite the entire object--explanations? */
 // categoryResults[category._id].assets = [];
 getAssetsForCategory(category, function(err, assets) {
 /* instead I have to use a separate temp object to store the
 asset results, rather than just saying
 categoryResults[category._id].assets = assets; */
 assetResults[category._id] = assets;
 categoryAssetLookupDone();
 });
 },
 assetFetchComplete
 );
 }
 ],
 function(err) {
 console.log("asset fetch complete");
 data[mediumName] = {};
 data[mediumName]["categories"] = categoryResults;
 data[mediumName]["description"] = mediumResults.description;
 data[mediumName]["name"] = mediumResults.name;
 data[mediumName].categories = [];
 for (var catId in assetResults) {
 data[mediumName].categories.push({info: categoryResults[catId], assets: assetResults[catId]});
 //data[mediumName]["categories"][catId] = {info: data[mediumName].categories[catId], assets: assetResults[catId]};
 }
 //data[mediumName]["assets"] = assetResults;
 //{medium: mediumResults, ass: assetResults};
 mediumLookupComplete(err);
 }
 );
 },
 function(err) {
 mongoose.connection.close();
 //res.send({data:data});
 res.render('viewer', {data:data});
 }
 );
};

• Marc-Andre: Welcome to Code Review! Your question looks good to me if your code work as is, without the commented code. We don't fix broken code, since your question is asking for a review it looks on-topic to me.
• Luc: Yes, it works as provided, just seems convoluted. Thanks!
• Marc-Andre: Thanks you for the confirmation! I hope you'll get good review!
• jackdbernier: Instead of using the _id of the other objects you could use a reference. See mongoosejs.com/docs/populate.html Then it would allow you to populate the fields

Answer 1

A short review;

• Consider using JsHint
• Declare data with var, otherwise you are polluting the global namespace
• Consider err more often, the assumption that there will be no error will bite you
• Do not simply log to console.log, it is one of the most common bottlenecks
• Remove your commented out code, it keeps things clean

• My assumption is that

  • The meta data of ["FILM","VIDEO","TV"] does not change often
  • The categories do no change often

  I would generate a javascript file with the meta data and categories in 1 big Object Notation assignment, and re-generate it whenever mediums/categories change, this would greatly simplify your code, and make it way faster.

• With Object Notation, this

   data[mediumName] = {};
   data[mediumName]["categories"] = categoryResults;
   data[mediumName]["description"] = mediumResults.description;
   data[mediumName]["name"] = mediumResults.name;
  

  would be

   data[mediumName] = {
   categories : [];
   description : mediumResults.description;
   name : mediumResults.name;
   };
  

  note that you override the results of data[mediumName]["categories"] = categoryResults; with data[mediumName].categories = [];