Node module for DRY CRUD endpoints

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.prot

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.

StackExchange Code Review Q#71260, answer score: 4