Combine validators with handler functions and export them atomically

[seaerchin]

Problem

Currently, in our routes.ts, we have the validator set up within the route itself and the handler imported from the controller. For some handlers, the validation might be a precondition in that the handler requires that the incoming data be valid.

Hence, the best practice would be to combine the handling function together with the validator in the controller itself and export them together as an array such as export const handleSomething = [validateSomething, doSomething]. This would prevent human errors where there are no validations due to forgetting them.

Notes

The validation should be defined in the same file (preferably close to the handler). It is ok to redefine it because validation differs on a case by case basis.

[tehtea]

Hello! I've been looking through this repo a bit to learn some best practices to adopt and I will like to pay it forward in return, but wanted to understand this issue better, and also double check if anyone is working on this before working on it.
Just to clarify, it seems like some of the handler functions are being implemented in a different pattern, e.g.

Pattern One

// src/app/routes/api/v3/admin/forms/admin-forms.preview.routes.ts
AdminFormsPreviewRouter.post(
  '/:formId([a-fA-F0-9]{24})/preview/submissions/email',
  AdminFormController.handleEmailPreviewSubmission,
)

// src/app/modules/form/admin-form/admin-form.controller.ts
export const handleEmailPreviewSubmission = [
  EmailSubmissionMiddleware.receiveEmailSubmission,
  EmailSubmissionMiddleware.validateResponseParams,
  submitEmailPreview,
] as ControllerHandler[]

Pattern Two

// src/app/modules/user/user.routes.ts
UserRouter.post(
  '/contact/verifyotp',
  celebrate({
    [Segments.BODY]: Joi.object().keys({
      userId: Joi.string().required(),
      otp: Joi.string()
        .required()
        .regex(/^\d{6}$/),
      contact: Joi.string().required(),
    }),
  }),
  UserController.handleContactVerifyOtp,
)

// src/app/modules/user/user.controller.ts
export const handleContactVerifyOtp: ControllerHandler<
  unknown,
  string | IPopulatedUser,
  {
    userId: string
    otp: string
    contact: string
  }
> = async (req, res) => {
  // Joi validation ensures existence.
  const { userId, otp, contact } = req.body
  const sessionUserId = getUserIdFromSession(req.session)

  // Guard against user updating for a different user, or if user is not logged
  // in.
  if (!sessionUserId || sessionUserId !== userId) {
    return res.status(StatusCodes.UNAUTHORIZED).json('User is unauthorized.')
  }

  const logMeta = {
    action: 'handleContactVerifyOtp',
    userId,
    ip: getRequestIp(req),
  }

  // Step 1: Verify contact and otp of user matches with stored hash.
  const verifyResult = await verifyContactOtp(otp, contact, userId)

  if (verifyResult.isErr()) {
    const { error } = verifyResult
    logger.error({
      message: 'Error verifying contact verification OTP',
      meta: logMeta,
      error,
    })

    const { errorMessage, statusCode } = mapRouteError(error)
    return res.status(statusCode).json(errorMessage)
  }

  // Step 2: Contact and OTP hashes match, update user with new contact.
  const updateResult = await updateUserContact(contact, userId)
  if (updateResult.isErr()) {
    const { error } = updateResult
    logger.error({
      message: 'Error updating user emergency contact number',
      meta: logMeta,
      error,
    })

    const { errorMessage, statusCode } = mapRouteError(error)
    return res.status(statusCode).json(errorMessage)
  }

  // No errors, return updated user to client.
  return res.status(StatusCodes.OK).json(updateResult.value)
}

This ticket is about making sure the controllers confirm with pattern one, right?

[seaerchin]

Hi,

i raised the ticket but i'm an ex-contributor so maybe i'm not the best person for this @__@

you're right! it's about conforming to pattern 1; the reason for doing so is because we don't want developer overhead (most route level functions are actually comprised of a check followed by the actual service function containing the logic).

pattern 2 is a run-time check and differs from pattern 1: pattern 1 merely wishes to assert that the input is of a valid shape whereas pattern 2 wishes to assert that the input is actually correct.

i (think) this is an example of parse, don't validate for pattern 1, where the runtime information is parsed and then the information is persisted through the type of the input shape itself!

[tehtea]

Thank you for the context! So adding an additional layer (minimally) to do a shape assertion for each handler which follows pattern 2 seems like the approach to get this done