1
\$\begingroup\$

In an api I'm building, I have multiple routes that require an agency to exist before any action is performed on the requested agency.

To avoid repetition I've written some middleware to check that the agency does indeed exist before continuing (and throws if it does not).

The code I have works but I have to make two requests to the db, I'm wondering if I can accomplish the same goal with only a single query.

Here is my code:

// routes/v1/index.js

const express = require("express")
const router = require("express-promise-router")()

router.use(["/agencies/:id", "/agencies/:id/*"], agencies.checkExistsByReqId)

router
  .route("/agencies/:id")
  .get(agencies.getById)
  .patch(agencies.patch)
  .delete(agencies.delete)

export default router
// controllers/v1/agencies/checkExistsByReqId.js

import db from "../../../db"
import { AgencyDoesNotExistError } from "../../../errors"

export default async (req, res, next) => {
  // note: To make the query as small as possible this just returns an id if exists
  const exists = await db.agency.checkExistsById(req.params.id)

  if (!exists) throw new AgencyDoesNotExistError()

  next()
}

// /controllers/v1/agencies/getById.js

import db from "../../../db"
import { AgencyDoesNotExistError } from "../../../errors"

export default async (req, res) => {
  const agency = await db.agency.getById(req.params.id)

  const isMember = agency.members.find(member => member.authId === req.user.sub)
  if (!isMember) throw new AgencyDoesNotExistError()

  res.send(agency)
}

// db/index.js

export const agency = {
  create: async data => new Agency(data).save(),
  findOne: filter => Agency.findOne(filter),
  findOneAndUpdate: (filter, update, options) =>
    Agency.findOneAndUpdate(filter, update, options),
  getById: async agencyId => Agency.findById(agencyId).populate("members"),
  checkExistsById: agencyId => Agency.findById(agencyId, "_id")
}
\$\endgroup\$
1
\$\begingroup\$

I managed to accomplish this by adding the agency to res.locals. Here's the amended code:

// /routes/v1/index.js

const router = require("express-promise-router")()

router.use(["/agencies/:id", "/agencies/:id/*"], agencies.addAgencyToResLocals, agencies.ensureUserIsMember)

router
  .route("/agencies/:id")
  .get(agencies.getById)
  .patch(agencies.ensureUserIsAdmin, agencies.patch)
  .delete(agencies.ensureUserIsAdmin, agencies.delete)
// /controllers/v1/agencies/addAgencyToResLocals.js

import db from "../../../db"
import { AgencyDoesNotExistError } from "../../../errors"

export default async (req, res, next) => {
  const agency = await db.agency.getById(req.params.id)

  if (!agency) throw new AgencyDoesNotExistError()

  res.locals.agency = agency

  next()
}
// /controllers/v1/agencies/ensureUserIsMember.js

import { UnauthorizedError } from "../../../errors"

export default (req, res, next) => {
  const { agency } = res.locals

  const isMember = agency.members.find(member => member.authId === req.user.sub)

  if (!isMember) throw new UnauthorizedError()

  next()
}

// /controllers/v1/agencies/getById.js

export default async (req, res) => res.send(res.locals.agency)

I'm not sure if this is the best solution but it simplifies my code a lot. The one issue I see is that another person working with the code base for the first time would have to have knowledge about the middleware being applied. That being said I feel it's acceptable to say that someone working with express should have knowledge of middleware and how it works IMO.

\$\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.