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);
}
};
1 Answer 1
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
) overprocess.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 asdata.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.
-
1\$\begingroup\$ Welcome to Code Review! You passed through the First Post Review with flying colors! \$\endgroup\$RubberDuck– RubberDuck2014年12月31日 13:56:08 +00:00Commented Dec 31, 2014 at 13:56