From c36fb80f9ee919bc0af822cd45ab5ffbf07439bd Mon Sep 17 00:00:00 2001 From: Olusola David Date: Tue, 14 Aug 2018 00:37:30 +0100 Subject: [PATCH] bug(API): fix double callback errors in async controllers - refactor controllers to use async/await for cleaner logic - fix double callbacks [Delivers #159752185] --- package.json | 1 + server/controllers/entryController.js | 183 +++++++++++--------------- server/controllers/userController.js | 118 ++++++++--------- server/index.js | 7 + server/utils/validate.js | 7 + 5 files changed, 145 insertions(+), 171 deletions(-) diff --git a/package.json b/package.json index a5347dd..dc40378 100644 --- a/package.json +++ b/package.json @@ -40,6 +40,7 @@ }, "dependencies": { "babel-cli": "^6.26.0", + "babel-polyfill": "^6.26.0", "babel-preset-env": "^1.7.0", "bcrypt": "^3.0.0", "body-parser": "^1.18.3", diff --git a/server/controllers/entryController.js b/server/controllers/entryController.js index f344cb5..29625a9 100644 --- a/server/controllers/entryController.js +++ b/server/controllers/entryController.js @@ -1,3 +1,4 @@ +import 'babel-polyfill'; import { query } from '../db/index'; import validate from '../utils/validate'; import queries from '../db/queries'; @@ -7,28 +8,26 @@ class entryController { * Get all of a user's diary entries * Requires auth token to be passed in authorization header * @static - * @param {*} req - Client request object - * @param {*} res - Server response object - * @returns {object} token + * @param {*} req - request object + * @param {*} res - response object + * @returns {object} json * @memberof userController */ - static getAllEntries(req, res) { - let { limit, page } = req.query; - // validate queries - limit = validate.isNumber(limit) ? limit : 20; - page = validate.isNumber(page) ? page : 0; - // get entries - query( - queries.getAllEntries, - [req.authorizedUser.email, limit, page * limit], - (err, result) => { - if (err) { - res.status(500).json({ error: { message: 'An error occurred on the server' } }); - return; - } - res.status(200).json({ entries: result.rows, meta: { limit, page } }); - }, - ); + static async getAllEntries(req, res, next) { + try { + let { limit, page } = req.query; + + // validate queries + limit = validate.isNumber(limit) ? limit : 20; + page = validate.isNumber(page) ? page : 0; + + // get entries + const userEntries = await query(queries.getAllEntries, + [req.authorizedUser.email, limit, page * limit]); + res.status(200).json({ entries: userEntries.rows, meta: { limit, page } }); + } catch (error) { + next(error); + } } /** @@ -37,24 +36,20 @@ class entryController { * @static * @param {*} req - Request object with title, content, is_favorite properties * @param {*} res - Response object - * @returns {object} response + * @returns {object} json * @memberof entryController */ - static addEntry(req, res) { - const { title, content, is_favorite: isFavorite } = req.body; + static async addEntry(req, res, next) { + try { + const { title, content, is_favorite: isFavorite } = req.body; - // add entry to database - query( - queries.insertOneEntry, - [req.authorizedUser.email, title, content, isFavorite], - (err, result) => { - if (err) { - res.status(500).json({ error: { message: 'An error occurred on the server' } }); - return; - } - res.status(201).json(result.rows[0]); - }, - ); + // add entry to database + const newEntry = await query(queries.insertOneEntry, + [req.authorizedUser.email, title, content, isFavorite]); + res.status(201).json(newEntry.rows[0]); + } catch (error) { + next(error); + } } /** @@ -65,21 +60,16 @@ class entryController { * @param {*} res - Response object * @memberof entryController */ - static getEntry(req, res) { - query( - queries.getOneEntry, [req.authorizedUser.email, req.params.id], - (err, result) => { - if (err) { - res.status(500).json({ error: { message: 'An error occurred on the server' } }); - return; - } - if (!result.rowCount) { - res.status(404).json({ error: { message: 'Entry not found' } }); - return; - } - res.status(200).json(result.rows[0]); - }, - ); + static async getEntry(req, res, next) { + try { + const entry = await query(queries.getOneEntry, [req.authorizedUser.email, req.params.id]); + if (!entry.rowCount) { + res.status(404).json({ error: { message: 'Entry not found' } }); + } + res.status(200).json(entry.rows[0]); + } catch (error) { + next(error); + } } /** @@ -88,49 +78,36 @@ class entryController { * @static * @param {*} req - Request object * @param {*} res - Response object - * @returns {obj} response + * @returns {obj} json * @memberof entryController */ - static modifyEntry(req, res) { - // deconstruct request body - let { title, content } = req.body; - title = title || null; - content = content || null; - const isFavorite = validate.booleanOrNull(req.body.is_favorite); + static async modifyEntry(req, res, next) { + try { + // deconstruct request body + let { title, content } = req.body; + + // convert them to null if they are undefined + title = title || null; + content = content || null; + const isFavorite = validate.booleanOrNull(req.body.is_favorite); + + // check if entry exists and is already older than a day + const entryDate = await query(queries.getEntryCreationDate, + [req.authorizedUser.email, req.params.id]); + if (!entryDate.rowCount) res.status(404).json({ error: { message: 'Entry not found' } }); - // check if entry is already older than a day - query( - queries.getEntryCreationDate, [req.authorizedUser.email, req.params.id], - (err, result) => { - if (err) { - res.status(500).json({ error: { message: 'An error occurred on the server' } }); - return; - } - if (!result.rowCount) { - res.status(404).json({ error: { message: 'Entry not found' } }); - return; - } - const createdOn = new Date(result.rows[0].created_on).getTime(); - const hoursSinceCreated = (Date.now() - createdOn) / (1000 * 60 * 60); - if (hoursSinceCreated > 24) { - res.status(403).json({ - error: { message: 'Cannot update entry after 24 hours' }, - }); - } - }, - ); - // add entry to database - query( - queries.updateOneEntry, - [title, content, isFavorite, req.authorizedUser.email, req.params.id], - (err, result) => { - if (err) { - res.status(500).json({ error: { message: 'An error occurred on the server' } }); - return; - } - res.status(200).json(result.rows[0]); - }, - ); + // older than a day? + if (!validate.isWithinLast24hours(entryDate.rows[0].created_on)) { + res.status(403).json({ error: { message: 'Cannot update entry after 24 hours' } }); + } else { + // add entry to database + const entry = await query(queries.updateOneEntry, + [title, content, isFavorite, req.authorizedUser.email, req.params.id]); + res.status(200).json(entry.rows[0]); + } + } catch (error) { + next(error); + } } /** @@ -141,21 +118,17 @@ class entryController { * @param {*} res - Response object * @memberof entryController */ - static deleteEntry(req, res) { - query( - queries.deleteOneEntry, [req.authorizedUser.email, req.params.id], - (err, result) => { - if (err) { - res.status(500).json({ error: { message: 'An error occurred on the server' } }); - return; - } - if (!result.rowCount) { - res.status(404).json({ error: { message: 'Entry not found' } }); - return; - } - res.sendStatus(204); - }, - ); + static async deleteEntry(req, res, next) { + try { + const deleted = await query(queries.deleteOneEntry, + [req.authorizedUser.email, req.params.id]); + if (!deleted.rowCount) { + res.status(404).json({ error: { message: 'Entry not found' } }); + } + res.sendStatus(204); + } catch (error) { + next(error); + } } } diff --git a/server/controllers/userController.js b/server/controllers/userController.js index ac98e82..29f374b 100644 --- a/server/controllers/userController.js +++ b/server/controllers/userController.js @@ -1,3 +1,4 @@ +import 'babel-polyfill'; import bcrypt from 'bcrypt'; import { query } from '../db/index'; import queries from '../db/queries'; @@ -13,45 +14,33 @@ class userController { * @returns {object} token * @memberof userController */ - static createUser(req, res) { - // get email and password in request body - const { email, password } = req.body; + static async createUser(req, res, next) { + try { + // get email and password in request body + const { email, password } = req.body; - // check if email already exists - 409 - query(queries.getOneUser, [email], (qErr, user) => { - if (qErr) { - res.status(500).json({ error: { message: 'An error occurred on the server.' } }); - return; - } + // check if user already exists - 409 + const user = await query(queries.getOneUser, [email]); if (user.rows.length) { res.status(409).json({ error: { message: 'User already exists. Please login.' } }); - return; } // hash the password - bcrypt.hash(password, 5, (bErr, hash) => { - if (bErr) { - res.status(500).json({ error: { message: 'An error occurred on the server.' } }); - return; - } + const passwordHash = await bcrypt.hash(password, 5); - // insert new user into table, returning data - query(queries.insertOneUser, [email, hash], (qErr2, newUser) => { - if (qErr2) { - res.status(500).json({ error: { message: 'An error occurred on the server.' } }); - return; - } + // insert new user into table, returning data + const newUser = await query(queries.insertOneUser, [email, passwordHash]); - // create token using new data and sign with password hash+lastLogin+lastLogout - const userInfo = newUser.rows[0]; - const data = { email: userInfo.email, createdOn: userInfo.created_on }; - const token = signAuthToken(data); + // create token using new data + const userInfo = newUser.rows[0]; + const data = { email: userInfo.email, createdOn: userInfo.created_on }; + const token = signAuthToken(data); - // signed token - 201 - res.status(201).json({ token }); - }); - }); - }); + // signed token - 201 + res.status(201).json({ token }); + } catch (error) { + next(error); + } } /** @@ -63,36 +52,35 @@ class userController { * @returns {token} * @memberof userController */ - static loginUser(req, res) { - // get email and password in request body - const { email, password } = req.body; + static async loginUser(req, res, next) { + try { + // get email and password in request body + const { email, password } = req.body; - // fetch user - query(queries.getOneUser, [email], (qErr, userData) => { - if (qErr) { - res.status(404).json({ error: { message: 'An error occurred on the server.' } }); - return; + // fetch user + const user = await query(queries.getOneUser, [email]); + + // if error in fetch, user does not exist - 422 + if (!user.rows.length) { + res.status(422).json({ error: { message: 'Email or Password is incorrect' } }); } - // if error in fetch, user does not exist - 404 - if (!userData.rows.length) { + + // check password + const passwordIsValid = await bcrypt.compare(password, user.rows[0].password); + if (!passwordIsValid) { res.status(422).json({ error: { message: 'Email or Password is incorrect' } }); return; } - // check password - bcrypt.compare(password, userData.rows[0].password, (bErr, valid) => { - if (!valid) { - res.status(422).json({ error: { message: 'Email or Password is incorrect' } }); - return; - } - // create token - const data = { - email: userData.rows[0].email, createdOn: userData.rows[0].created_on, - }; - const token = signAuthToken(data); - // return signed token - 200 - res.status(200).json({ token }); - }); - }); + + // create token + const data = { + email: user.rows[0].email, createdOn: user.rows[0].created_on, + }; + const token = signAuthToken(data); + res.status(200).json({ token }); + } catch (error) { + next(error); + } } /** @@ -103,18 +91,16 @@ class userController { * @param {*} res - Response object * @memberof userController */ - static getProfile(req, res) { - query(queries.getEntriesCount, [req.authorizedUser.email], - (err, result) => { - if (err) { - res.status(500).json({ error: { message: 'An error occurred on the server' } }); - return; - } - res.status(200).json({ - email: req.authorizedUser.email, - entriesCount: result.rows[0].count, - }); + static async getProfile(req, res, next) { + try { + const entriesCounter = await query(queries.getEntriesCount, [req.authorizedUser.email]); + res.status(200).json({ + email: req.authorizedUser.email, + entriesCount: entriesCounter.rows[0].count, }); + } catch (error) { + next(error); + } } } diff --git a/server/index.js b/server/index.js index 04ad88c..4e4ff9d 100644 --- a/server/index.js +++ b/server/index.js @@ -25,6 +25,13 @@ app.use(morgan('dev')); app.use(cors()); app.use('/api/v1', router); app.use('/api/docs', swaggerUi.serve, swaggerUi.setup(docs)); +app.use((err, req, res, next) => { + if (res.headersSent) { + next(err); + } else { + res.status(500).json({ error: { message: 'An error occurred on the server' } }); + } +}); app.listen(PORT, () => { console.log(`Server listening on port ${PORT}!`); diff --git a/server/utils/validate.js b/server/utils/validate.js index 24dae35..14ef06a 100644 --- a/server/utils/validate.js +++ b/server/utils/validate.js @@ -8,6 +8,13 @@ class validate { static booleanOrNull(bool) { return (typeof bool === 'undefined' ? null : bool); } + + static isWithinLast24hours(dateString) { + const dateMilli = new Date(dateString).getTime(); + const hoursSinceCreated = (Date.now() - dateMilli) / (1000 * 60 * 60); + if (hoursSinceCreated > 24) return false; + return true; + } } validate.signupInputs = [