Basic REST API for manipularing a MongoDB collection, using Node, Express, and Mongoose

Question

I'm practicing back-end programming and NodeJS. As an exercise, I'm attempting to build a REST API for a MongoDB collection. I'm also learning to use the Express and Mongoose middle-wares, so that's what we'll use for the server and database respectively. Also practicing async / await to handle promises.

The requirements for this basic REST API and exercise are:

• Support get and delete on individual resources.
• Support get and post on the resource collection.
• Apply generalization and separation of concerns.
• Protect against Mongo injection.
• Use async / await to handle promises.

This is the current working implementation:

---

app.js

const express = require('express')
const mongoose = require('mongoose')
const morgan = require('morgan')

const songRouter = require('./routes/song-router.js')

const mongurl = 'mongodb://localhost:27017/library'
const port = 3000

const app = express()
app.use(morgan('combined'))
app.use('/songs', songRouter)

mongoose.connect(mongurl, () => {
  console.log(`\n    >> Mongoose connected to ${mongurl}`)
})

app.listen(port, () => {
  console.log(`\n    >> Node listening to port ${port}`)
})

---

models/song-model.js

const mongoose = require('mongoose')

const song = {
  name: {
    type: String,
    required: true
  },

  author: {
    type: String,
    required: true
  },

  key: String
}

const options = {
  timestamps: true
}

const schema = new mongoose.Schema(song, options)

module.exports = mongoose.model('song', schema)

---

routes/song-router.js

const express = require('express')

const control = require('../controllers/song-control.js')

const router = express.Router()
router.use(express.json())

router
  .route('/')
  .get(control.getAll)
  .post(control.postOne)

router
  .route('/:songId')
  .get(control.getOne)
  .delete(control.deleteOne)

module.exports = router

---

controllers/song-control.js (version 1, without generalization)

const songModel = require('../models/song-model.js')

exports.getAll = async (req, res, nxt) => {
  try {
    const allSongs = await songModel.find({})
    res.status(200).json(allSongs)
  } catch (err) {
    nxt(err)
  }
}

exports.getOne = async (req, res, nxt) => {
  try {
    const oneSong = await songModel.findById(req.params.songId)
    res.status(200).json(oneSong)
  } catch (err) {
    nxt(err)
  }
}

exports.postOne = async (req, res, nxt) => {
  try {
    const postedSong = await songModel.create(req.body)
    res.status(200).json(postedSong)
  } catch (err) {
    nxt(err)
  }
}

exports.deleteOne = async (req, res, nxt) => {
  try {
    const deletedSong = await songModel.findByIdAndDelete(req.params.songId)
    res.status(200).json(deletedSong)
  } catch (err) {
    nxt(err)
  }
}

---

controllers/song-control.js (version 2, first attempt at generalization)

const songModel = require('../models/song-model.js')

exports.getAll = buildMongoFunction('find')

exports.getOne = buildMongoFunction('findById', true)

exports.postOne = buildMongoFunction('create', false)

exports.deleteOne = buildMongoFunction('findByIdAndDelete', true)

function buildMongoFunction (funName, argIsParam) {
  return async (req, res, nxt) => {
    const arg = argIsParam ? req.params.songId : req.body

    try {
      const reply = await songModel[funName](arg)
      res.status(200).json(reply)
    } catch (err) {
      nxt(err)
    }
  }
}

---

---

I'm looking forward to all kinds and types of feedback: style, bugs, anti-patterns, ways to do this more concise / maintainable / redeable, conventions, best practices; whatever you think can be improved, please share.

I have some specific questions, but please feel free to ignore these and comment on something else!

• The generalization of controllers/song-control.js feels hacky. Is there a better way to implement the generalization of that pattern? How'd you do it?

• How well are these concepts being applied: generalization, separation of concerns? Would you separate responsibilities even further? Or are they too separated? Can something be further generalized?

• How well is async / await being used?

• Should I sanitize inputs? Or is enforcing Mongoose models and schemas protection enough against Mongo injections?

• Seems that Mongoose queries do not return promises. Is the async / await code here doing any actual asynchronous job?

• What would you recommend doing in a different way?

Answer 1

This is not a full response to your question but what I could quickly jot down while looking at your post. Sorry, no insights to share. I would discourage you from choosing this answer in case someone with a more fleshed out response decides to come along and provide more value.

I looked over the song controller. I considered style and organization choices and decided on the following. This is more about javascript styling in general, not necessarily about node, or mongoose, or server-side technology.

Please take a look and see if it helps you think differently about anything you are looking forward to understanding.

const buildMongoFunction = name => async (req, res, nxt) => {
  const { params: { songId }, body } = req;
  
  try {
    const needsSongID = ['findById', 'findByIdAndDelete'].includes(name);
    res.status(200).json(await songModel[name](needsSongID ? songID : body));
  } catch (error) { nxt(error); }
};

const toExports = ['find', 'findById', 'create', 'findByIdAndDelete']
  .reduce((toExport, name) => ({ ...toExport, [name]: buildMongoFunction(name) }), {});

Object.assign(exports, toExports);