4
\$\begingroup\$

As a learning project I recently started writing a node module to generate / handle the creation of CRUD endpoints in a DRY way.

The problem I was initially wanting to solve was that in my generator-angular-fullstack project I had several basic CRUD endpoints that all had identical controllers (e.g. this).

My idea was a module that would generate those CRUD methods for different database types, something like:

var dryCrud = new DryCrud({
    type: 'mongoose',
    collection: MyMongooseCollection
});

app.get('/', dryCrud.read);
app.get('/:', dryCrud.read({
    query: {'_id':'^id'}   // <--  the ^ = req.params.
}));

Then I had the idea of generating a full set of basic CRUD endpoints:

dryCrud.init('routePrefix', app);

The .init creates a create, read, read/:id, update/:id and delete endpoint using the supplied express app (or router) with each route prefixed by the 'routePrefix', connecting with the database specified in the dryCrud creation.

I have created a pretty basic module, and two database adaptors (Mongo native and Mongoose) here.

I feel like I have been staring at this in isolation for a while, and I would love to know if anyone else thinks a module like this would be useful. Is it worth developing it further? I would also welcome any feedback on my code / structure.

The Crudbrella module

index.js

var _ = require('lodash'),
    utils = require("./lib/utils-prototype"),
    Crudbrella;

Crudbrella = function(config){

    //Check for required config
    if(!config.collection || !config.type){

        //Raise an error here...
        console.error("dryCrud error: you must provide a database object and type");
        process.exit(e.code);

        return false;

    }

    //Constructor of crudbrella object being returned to the user
    var newCrud = function(){
        this.collection = config.collection;
        this.type = config.type;
    };

    //Include core functionality and utility methods
    _.extend(newCrud.prototype, utils);

    //If the type provided is a string
    if (typeof config.type == 'string' || config.type instanceof String){

        //attempt to load and use the module
        try {

            var adaptorModule = require(config.type);

        } catch(e) {

            console.error("Adaptor " + config.typre + " is not found");
            process.exit(e.code);

        }

    //If the type provided is a module
    }else{

        adaptorModule = config.type;

    }

    //use the module
    _.extend(newCrud.prototype, adaptorModule);

    return new newCrud();

};

module.exports = Crudbrella;

./libs/utils-prototype.js

var _  = require('lodash');

module.exports = {

    //Default success callback (can be replaced at runtime)
    successCallback: function(res, result){

        if(!result) { return res.send(404, ""); }

        res.send(result);

    },

    //Default error callback (can be replaced at runtime)
    errorCallback: function(res, error){

        res.send(500, error);

    },

    //Standard db callback
    dbCallback: function(data){

        //If there is an error
        if(data.err && data.err !== null){

            //Use a user defined or default error callback
            return data.onError(data.res, data.err || "empty");

        }

        if(data.req.method == 'DELETE'){
            data.result = true;
        }

        //Use a user defined or default success callback
        return data.onSuccess(data.res, data.result);

    },
    init: function(root, app){

        root = root.replace(/\/$/, "");

        app.get(root + '/', this.read());
        app.get(root + '/:id', this.read({query:{_id:'^id'}}));
        app.post(root + '/', this.create());
        app.put(root + '/:id', this.update());
        app.delete(root + '/:id', this.delete());

    },
    utils: {
        //Depopulate populated mongoose documents (move this to the mongoose adaptor)
        depopulate: function (model, doc){

            //loop through each item in the model
            _.each(model.schema.tree, function(item, key){

                var schemaDetails = JSON.stringify(item) || "";

                //If the item has a 'ref' property
                if (schemaDetails.indexOf('ref') !== -1){

                    //For that item in the current document
                    doc[key] = _.map(doc[key], function(value){

                        return value._id;

                    });

                }

            });

        }

    }
};

The mongoose adaptor

var _  = require('lodash');

module.exports =  {

    //Create a new record
    create: function (options){

        return function(req, res){

            //If no options are provided default to an empty object
            options = options || {};

            this.collection.create(req.body, function(err, result) {

                //Pass results and req/res to the dbCallback
                this.dbCallback({
                    req: req, 
                    res: res, 
                    err: err, 
                    result: result,
                    onSuccess:options.onSuccess || this.successCallback,
                    onError:options.onError || this.errorCallback
                });

            }.bind(this));

        }.bind(this);

    },

    //Read an existing record / records
    read: function(options){

        return function (req, res){

            //If no options are provided default to an empty object
            options = options || {};
            options.query = options.query || {};

            //Create a local copy of the query definition to be parsed
            var query = JSON.parse(JSON.stringify(options.query)) || {};

            //Loop through every object in query
            _.each(query, function(item, key){

                //If the value starts with a ^ use the item value as a key for req.params
                if(item[0] === "^"){
                    query[key] = req.params[item.replace("^","")];
                }

            });

            //Find using optional query or find all
            this.collection.find(query || {})

                //Process options populate string / object or default to empty string
                .populate(options.populate || '')

                //Execute the query
                .exec(function(err, items){

                    if(items.length === 1){
                        items = items[0];
                    }

                    //Pass results and req/res to the dbCallback
                    this.dbCallback({
                        req: req, 
                        res: res, 
                        err: err, 
                        result: items,
                        onSuccess:options.onSuccess || this.successCallback,
                        onError:options.onError || this.errorCallback
                    });

                }.bind(this)

            );

        }.bind(this);
    },

    //Update a record
    update: function(options){

        return function(req, res){

            //If no options are provided default to an empty object
            options = options || {};

            //If the id has been included in the body, remove it
            var x = req.body._id; 
            delete req.body._id;

            //Check if the body contains any populated fields and depopulate them
            this.utils.depopulate(this.collection, req.body);

            //Use crudbrella read to find the document to be updated
            this.read({

                query: {_id: x},

                //custom success handler to complete update, use default for errors
                onSuccess: function(innerRes, result){

                    var updated = _.extend(result, req.body);

                    updated.save(function (err, actualResult) {

                        //Pass results and req/res to the dbCallback
                        this.dbCallback({
                            req: req, 
                            res: res, 
                            err: err, 
                            result: actualResult,
                            onSuccess:options.onSuccess || this.successCallback,
                            onError:options.onError || this.errorCallback
                        });

                    }.bind(this));

                }.bind(this)

            })(req, res);

        }.bind(this);

    },

    //Delete a record
    delete: function(options){

        return function(req, res){

            //If no options are provided default to an empty object
            options = options || {};

            //If the id has been included in the body, remove it
            if(req.body._id) { delete req.body._id; }

            //Check if the body contains any populated fields and depopulate them
            this.utils.depopulate(this.collection, req.body);

            //Use dryCrud.read to find the document to be deleted
            this.collection.findOneAndRemove({_id: req.params.id}, function(err, result){

                //Pass results and req/res to the dbCallback
                this.dbCallback({
                    req: req, 
                    res: res, 
                    err: err, 
                    result: "",
                    onSuccess:options.onSuccess || this.successCallback,
                    onError:options.onError || this.errorCallback
                });

            }.bind(this));

        }.bind(this);

    }

};

The native mongo adaptor

var _  = require('lodash'),
        ObjectId = require('mongodb').ObjectID;

module.exports =  {

    //Create a new record
    create: function (options){

        return function(req, res){

            //If no options are provided default to an empty object
            options = options || {};

            //Save the object into the collection
            this.collection.insert(req.body, function(err, result) {

                //Pass results and req/res to the dbCallback
                this.dbCallback({
                    req: req, 
                    res: res, 
                    err: err, 
                    result: result,
                    onSuccess:options.onSuccess || this.successCallback,
                    onError:options.onError || this.errorCallback
                });

            }.bind(this));

        }.bind(this);

    },
    //Check if a given string is a valid mongo ObjectId
    validId: function(id){

        checkForHexRegExp = /^(?=[a-f\d]{24}$)(\d+[a-f]|[a-f]+\d)/i;

        if (id.match(checkForHexRegExp)) {
            return true;
        }else{
            return false;
        }

    },
    //Read an existing record / records
    read: function(options){

        return function (req, res){

            //If no options are provided default to an empty object
            options = options || {};

            //If no query has been provided default to an empty object
            options.query = options.query || {};

            //Create a local copy of the query definition to be parsed
            var query = JSON.parse(JSON.stringify(options.query)) || {};

            //Loop through every object in query
            _.each(query, function(item, key){

                //If the value starts with a ^ use the item value as a key for req.params
                if(item[0] === "^"){

                    if(this.validId(req.params[item.replace("^","")])){ 

                        query[key] = ObjectId(req.params[item.replace("^","")]);

                    }else{

                        query[key] = req.params[item.replace("^","")];  

                    }

                }

            }.bind(this));

            //Find using optional query or find all
            this.collection.find(query || {}).toArray(function(err, items){

                if(items.length === 1){
                    items = items[0];
                }

                //Pass results and req/res to the dbCallback
                this.dbCallback({
                    req: req, 
                    res: res, 
                    err: err, 
                    result: items,
                    onSuccess:options.onSuccess || this.successCallback,
                    onError:options.onError || this.errorCallback
                });

            }.bind(this));

        }.bind(this);

    },

    //Update a record
    update: function(options){

        return function(req, res){

            var requestId;

            //If no options are provided default to an empty object
            options = options || {};

            //If the id has been included in the body, remove it
            requestId = ObjectId(req.params.id); 
            delete req.body._id;

            //Check if the body contains any populated fields and depopulate them
            this.collection.update({'_id': requestId},{$set: req.body}, function(err, result){

                if(!err){
                    result = req.body;
                }

                this.dbCallback({
                    req: req, 
                    res: res, 
                    err: err, 
                    result: result,
                    onSuccess:options.onSuccess || this.successCallback,
                    onError:options.onError || this.errorCallback
                });

            }.bind(this));

        }.bind(this);

    },

    //Delete a record
    delete: function(options){

        return function(req, res){

            //If no options are provided default to an empty object
            options = options || {};

            //If the id has been included in the body, remove it
            if(req.body._id) { delete req.body._id; }

            //Use dryCrud.read to find the document to be deleted
            this.collection.remove({_id: ObjectId(req.params.id)}, function(err, result){

                //Pass results and req/res to the dbCallback
                this.dbCallback({
                    req: req, 
                    res: res, 
                    err: err, 
                    result: "",
                    onSuccess:options.onSuccess || this.successCallback,
                    onError:options.onError || this.errorCallback
                });

            }.bind(this));

        }.bind(this);

    }

};
\$\endgroup\$
4
+50
\$\begingroup\$

To answer your question: yes, I think it could be interesting as a stand-alone module. There's can be a lot of repetitive boilerplate involved in connecting a REST api to a CRUD backend, reducing that is certainly worth a module, in my opinion.

Now to dive into the code...

index.js

if(!config.collection || !config.type){
   //Raise an error here...
   console.error("dryCrud error: you must provide a database object and type");
   process.exit(e.code);
   return false;
 }

It makes more sense to me to throw an exception here than to exit the process.

catch(e) {
       console.error("Adaptor " + config.typre + " is not found");
       process.exit(e.code);
}
  • typre strikes me as a typo.
  • Again, I would prefer an throwing an exception (e.g. e) over process.exit.

utils-prototype.js

successCallback

if(!result) { return res.send(404, ""); }

Are you positive you want to return 404 on falsy values? Also, as far as I can tell this is Express 3.x syntax, the two-argument version of send is not mentioned in the Express 4.x api documentation. Using res.status(404).send("Not Found") is compatible with either version.

errorCallback

Passing the error message directly to the user may be convenient during development, but it is generally considered insecure. Sending error to the user doesn't strike me as a good default.

dbCallback

   //If there is an error
   if(data.err && data.err !== null){
       //Use a user defined or default error callback
       return data.onError(data.res, data.err || "empty");
   }
  • the check in the if condition is redundant, as null is falsy.
  • the || "empty" is redundant as data.err is always truthy in this path.
if(data.req.method == 'DELETE'){
        data.result = true;
}

I did not expect to find this here at all. It seems to be working around the fact that there is no meaningful response body to send back for a delete request, but the result argument to onSuccess mustn't be falsy, or you'll get a 404. What I would in fact expect in this case is a 204 (No Content) response. To achieve this, you probably need different default success handlers for different request types.

utils.depopulate

This function definitely belongs in the mongoose adapter as the comment already states.

var schemaDetails = JSON.stringify(item) || "";
           //If the item has a 'ref' property
           if (schemaDetails.indexOf('ref') !== -1){

I sincerely doubt that this is what you want to do. What if the item has a prefered or prefix property (those also contain ref)? The fact that you're calling JSON.stringify here suggests to me that item actually has a richer structure and you might actually test if item.ref !== undefined or something like that (I am not familiar enough with mongoose to know the metadata structure you are traversing here).

The adapters

I'll cover both adapters in one go. The interaction with mongo / mongoose is sufficiently similar.

//Create a local copy of the query definition to be parsed
var query = JSON.parse(JSON.stringify(options.query)) || {};

Shouldn't you use something like _.clone or _.cloneDeep for this?

if(items.length === 1){
    items = items[0];
}

It seems like this is again working around the limitations in your default success handler. When I query a single object using the get('/:id') route, I expect a single object in the response. But when I query a route that can return multiple objects, like get('/'), I expect to always get an array back, even when there happens to be just one element in the array. Again, I think this logic should be replaced by having a different success callback for different routes.

The update and delete methods are missing any way to prevent concurrent modifications. It might be a little tricky to prevent this as it depends on information embedded in the data (like a modification timestamp or version field). From a usability perspective, preventing users from overwriting or removing data they haven't even seen is pretty important though, if you ask me.

\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review! You passed through the First Post Review with flying colors! \$\endgroup\$ – RubberDuck Dec 31 '14 at 13:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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