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?