Problem
My goal is to create a simple restful api that will be accessed by an AngularJS front end. Even though it’s fairly simple, I’d like understand how to make it more reliable, secure, and best-practices compliant. The whole api is contained in a single file, I’m not overly concerned about organization at the moment (unless it can help with the items I’ve mentioned above). I’m using mongoose, hapi, and joi for validation.
server.js
'use strict';
var Hapi = require('hapi');
var Good = require('good');
var Mongoose = require('mongoose');
var Joi = require('joi');
Mongoose.connect('mongodb://xxxx:xxxx@proximus.modulusmongo.net:xxxx/xxxx');
var todoSchema = new Mongoose.Schema({
title: { type: String, required: true },
isCompleted: { type: Boolean, required: true },
createdAt: { type: Date, required: true },
updatedAt: { type: Date, required: true }
});
var Todo = Mongoose.model('Todo', todoSchema, 'Todos');
var server = new Hapi.Server(3000);
server.route({
method: 'GET',
path: '/api/v1/todos',
handler: function(request, reply) {
Todo.find(function(err, todos) {
if (err) {
reply(err);
return;
}
reply(todos);
});
}
});
server.route({
method: 'GET',
path: '/api/v1/todos/{id}',
handler: function(request, reply) {
Todo.find({ _id: request.params.id }, function(err, todo) {
if (err) {
reply(err);
return;
}
reply(todo);
});
},
config: {
validate: {
params: {
id: Joi.string().required()
}
}
}
});
server.route({
method: 'PUT',
path: '/api/v1/todos/{id}',
handler: function(request, reply) {
Todo.findOne({ _id: request.params.id }, function(err, todo) {
if (err) {
reply(err);
return;
}
todo.title = request.payload.title;
todo.isCompleted = request.payload.isCompleted;
todo.updatedAt = new Date();
todo.save();
reply(todo);
});
},
config: {
validate: {
params: {
id: Joi.string().required()
},
payload: {
_id: Joi.string(),
__v: Joi.number(),
title: Joi.string(),
isCompleted: Joi.boolean(),
createdAt: Joi.date(),
updatedAt: Joi.date()
}
}
}
});
server.route({
method: 'POST',
path: '/api/v1/todos',
handler: function(request, reply) {
var newTodo = new Todo({
title: request.payload.title,
isCompleted: request.payload.isCompleted,
createdAt: new Date(),
updatedAt: new Date()
});
newTodo.save(function(err, todo) {
if (err) {
reply(err);
return;
}
reply(todo);
});
},
config: {
validate: {
payload: {
title: Joi.string(),
isCompleted: Joi.boolean(),
}
}
}
});
server.route({
method: 'DELETE',
path: '/api/v1/todos/{id}',
handler: function(request, reply) {
Todo.findByIdAndRemove(request.params.id, function(err, todo) {
if (err) {
reply(err);
return;
}
reply(todo);
});
},
config: {
validate: {
params: {
id: Joi.string().required()
}
}
}
});
server.pack.register(Good, function(err) {
if (err) {
throw err;
}
server.start(function() {
server.log('info', 'Server running at: ' + server.info.uri);
});
});
Solution
I really like your code.
- Use of
'use strict';
- No comments, but the code is written so that they are not needed anywhere
- Good naming, good size of functions, things are in their logical place
The only thing is that I would not put uid/pwd in the source code, but use environment variables:
Mongoose.connect('mongodb://xxxx:xxxx@proximus.modulusmongo.net:xxxx/xxxx');
few remarks that apply even if the code is for Hapi Pre 8 version:
- I would suggest to have just one call to
route
with an array - Structure with a plugin, so that you specify the prefix when loading
/api/v1/links
- Eventualy extra schemas and handler into objects, that you point to in the route config.
Otherwise good, very good as @konjin said 🙂