5
\$\begingroup\$

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);
    });
});
\$\endgroup\$
1
\$\begingroup\$

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');
\$\endgroup\$
  • 1
    \$\begingroup\$ how about letting monoose validate the model, not letting the controller to get involved in validation? I think the model should be able to validate itself @konijn \$\endgroup\$ – Developerium Oct 19 '15 at 8:10
1
\$\begingroup\$

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 :)

\$\endgroup\$

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.