Mikey Mikey - 1 month ago 17
Node.js Question

Deleting uploaded files in Node.js with MongoDB

I have a one-to-many relationship between two entities: courses and files.

So, the way I set it up my database is to use two separate collections: courses and files.

var CourseSchema = new mongoose.Schema({
name: { type: String, required: true },
code: { type: String, required: true, unique: 1, uppercase: 1 },
files: [{ type: mongoose.Schema.Types.ObjectId, ref: 'File' }]
// more fields
});

var FileSchema = new mongoose.Schema({
name: { type: String, required: true }
// more fields
});


I have a working page that allows users to upload and add files within a course. As well, they can delete selected files on this same page.

My concern is when I delete selected files. After selecting the files that need to be deleted and clicking on the submit button, I send a DELETE request passing a list of file IDs. Below is my handler for deleting files:

var fs = require('fs');

var async = require('async'),
mongoose = require('mongoose');

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

exports.deleteFiles = function (req, res) {
// loop through selected file-ids
async.eachSeries(req.body.files, function (id, done) {
// remove file from File collection
models.File.findByIdAndRemove(id, function (err, file) {
if (err) {
return done(err);
} else if (!file) {
return done();
}
// remove file reference from Course document
req.course.files.pull(mongoose.Types.ObjectId(id));
req.course.save(function (err) {
if (err) {
return done(err);
}
var path = __dirname + '/../public/upl/' + req.course.id + '/' + file.name;
// remove file from filesystem
fs.stat(path, function (err, stats) {
if (err && err.code === 'ENOENT') {
return done();
} else if (err) {
return done(err);
}
if (stats.isFile()) {
fs.unlink(path, done);
}
});
});
});
}, function (err) {
if (err) {
console.log(err);
req.flash('failure', 'Unable to delete files at this time.');
} else {
req.flash('success', 'The files have been deleted successfully.');
}
res.redirect('/admin/courses/' + req.course.id + '/files');
});
};


It's pretty messy as I have to do several steps for each file ID: delete the ID from the course's file array, delete the file from the actual collection, and delete the file from the filesystem. As well, I have some error handling code at the beginning of each step.

Can this be improved using less steps and/or better error-handling?

Answer

I would write it this way, much cleaner imho, using each and waterfall, to iterate over collection and pass file from findByIdAndRemoveFn function

var async = require('async');
var File = require('../models/File'); //assume it's File.js

exports.deleteFiles = function (req, res) {

    var files = req.body.files;
    var course = req.course;
    var prePath = __dirname + '/../public/upl/' + course.id + '/';

    async.each(files, function(fileId, cb) {
        async.waterfall([
            function findByIdAndRemoveFn(parallelCb) {
                File.findByIdAndRemove(fileId, function(err, file) {
                    if(err) return parallelCb(err);
                    parallelCb(null, file);
                });
            },
            function pullFn(file, parallelCb) {
                course.update({$pull: {files: fileId}}, function(err) {
                    if(err) return parallelCb(err);
                    parallelCb(null, file);
                });
            },
            function unlinkFn(file, parallelCb) {
                var path = prePath + file.name;
                fs.stat(path, function(err, stats) {
                    if(err) return parallelCb(err);
                    else if(stats.isFile()) fs.unlink(path, parallelCb);
                    else parallelCb();
                });
            }
        ], cb);
    }, function(err) {
        if(err) req.flash('failure', 'Unable to delete files at this time.');
        else req.flash('success', 'The files have been deleted successfully.');
        res.redirect('/admin/courses/' + course.id + '/files');
    });
}