refactor: Refactor attachment upload into a service (#1547)

* refactor: Refactor attachment upload into a service

* Make changes based on mantariksh@'s comments

* Add formId to logging metadata for attachment uploading

* remove extraneous space

src/app/modules/core/core.errors.ts

@@ -73,3 +73,12 @@ export class MissingFeatureError extends ApplicationError {
     )
   }
 }
+
+/**
+ * Error thrown when attachment upload fails
+ */
+export class AttachmentUploadError extends ApplicationError {
+  constructor(message = 'Error while uploading encrypted attachments to S3') {
+    super(message)
+  }
+}

src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts

@@ -1,20 +1,17 @@
-import crypto from 'crypto'
 import { RequestHandler } from 'express'
 import { Query } from 'express-serve-static-core'
 import { StatusCodes } from 'http-status-codes'
 import JSONStream from 'JSONStream'
 import mongoose from 'mongoose'
 import { SetOptional } from 'type-fest'
 
-import { aws as AwsConfig } from '../../../../config/config'
 import { createLoggerWithLabel } from '../../../../config/logger'
 import {
   AuthType,
   EncryptedSubmissionDto,
   ResWithHashedFields,
   ResWithUinFin,
   SubmissionMetadataList,
-  WithParsedResponses,
 } from '../../../../types'
 import { ErrorDto } from '../../../../types/api'
 import { getEncryptSubmissionModel } from '../../../models/submission.server.model'
@@ -45,6 +42,7 @@ import {
   getSubmissionMetadataList,
   transformAttachmentMetasToSignedUrls,
   transformAttachmentMetaStream,
+  uploadAttachments,
 } from './encrypt-submission.service'
 import { EncryptSubmissionBody } from './encrypt-submission.types'
 import {
@@ -167,12 +165,6 @@ export const handleEncryptedSubmission: RequestHandler = async (req, res) => {
     })
   }
   const processedResponses = processedResponsesResult.value
-  // eslint-disable-next-line @typescript-eslint/no-extra-semi
-  ;(req.body as WithParsedResponses<
-    typeof req.body
-  >).parsedResponses = processedResponses
-  // Prevent downstream functions from using responses by deleting it.
-  // TODO(#1104): We want to remove the mutability of state that comes with delete.
   delete (req.body as SetOptional<EncryptSubmissionBody, 'responses'>).responses
 
   // Checks if user is SPCP-authenticated before allowing submission
@@ -214,7 +206,7 @@ export const handleEncryptedSubmission: RequestHandler = async (req, res) => {
       uinFin,
       formId,
     ).andThen((hashes) =>
-      MyInfoFactory.checkMyInfoHashes(req.body.parsedResponses, hashes),
+      MyInfoFactory.checkMyInfoHashes(processedResponses, hashes),
     )
     if (myinfoResult.isErr()) {
       logger.error({
@@ -279,35 +271,26 @@ export const handleEncryptedSubmission: RequestHandler = async (req, res) => {
   }
 
   // Save Responses to Database
-  // TODO(frankchn): Extract S3 upload functionality to a service
   const formData = req.body.encryptedContent
-  const attachmentData = req.body.attachments || {}
   const { verified } = res.locals
-  const attachmentMetadata = new Map()
-  const attachmentUploadPromises = []
-
-  // Object.keys(attachmentData[fieldId].encryptedFile) [ 'submissionPublicKey', 'nonce', 'binary' ]
-  for (const fieldId in attachmentData) {
-    const individualAttachment = JSON.stringify(attachmentData[fieldId])
-
-    const hashStr = crypto
-      .createHash('sha256')
-      .update(individualAttachment)
-      .digest('hex')
-
-    const uploadKey =
-      form._id + '/' + crypto.randomBytes(20).toString('hex') + '/' + hashStr
-
-    attachmentMetadata.set(fieldId, uploadKey)
-    attachmentUploadPromises.push(
-      AwsConfig.s3
-        .upload({
-          Bucket: AwsConfig.attachmentS3Bucket,
-          Key: uploadKey,
-          Body: Buffer.from(individualAttachment),
-        })
-        .promise(),
+  let attachmentMetadata = new Map<string, string>()
+
+  if (req.body.attachments) {
+    const attachmentUploadResult = await uploadAttachments(
+      form._id,
+      req.body.attachments,
     )
+
+    if (attachmentUploadResult.isErr()) {
+      const { statusCode, errorMessage } = mapRouteError(
+        attachmentUploadResult.error,
+      )
+      return res.status(statusCode).json({
+        message: errorMessage,
+      })
+    } else {
+      attachmentMetadata = attachmentUploadResult.value
+    }
   }
 
   const submission = new EncryptSubmission({
@@ -320,22 +303,6 @@ export const handleEncryptedSubmission: RequestHandler = async (req, res) => {
     version: req.body.version,
   })
 
-  try {
-    await Promise.all(attachmentUploadPromises)
-  } catch (err) {
-    logger.error({
-      message: 'Attachment upload error',
-      meta: logMeta,
-      error: err,
-    })
-    return res.status(StatusCodes.BAD_REQUEST).json({
-      message:
-        'Could not send submission. For assistance, please contact the person who asked you to fill in this form.',
-      submissionId: submission._id,
-      spcpSubmissionFailure: false,
-    })
-  }
-
   let savedSubmission
   try {
     savedSubmission = await submission.save()
@@ -385,7 +352,7 @@ export const handleEncryptedSubmission: RequestHandler = async (req, res) => {
 
   return sendEmailConfirmations({
     form,
-    parsedResponses: req.body.parsedResponses,
+    parsedResponses: processedResponses,
     submission: savedSubmission,
   }).mapErr((error) => {
     logger.error({

src/app/modules/submission/encrypt-submission/encrypt-submission.service.ts

@@ -1,4 +1,6 @@
+import { ManagedUpload } from 'aws-sdk/clients/s3'
 import Bluebird from 'bluebird'
+import crypto from 'crypto'
 import mongoose from 'mongoose'
 import { err, errAsync, ok, okAsync, Result, ResultAsync } from 'neverthrow'
 import { Transform } from 'stream'
@@ -17,17 +19,90 @@ import {
 import { getEncryptSubmissionModel } from '../../../models/submission.server.model'
 import { isMalformedDate } from '../../../utils/date'
 import { getMongoErrorMessage } from '../../../utils/handle-mongo-error'
-import { DatabaseError, MalformedParametersError } from '../../core/core.errors'
+import {
+  AttachmentUploadError,
+  DatabaseError,
+  MalformedParametersError,
+} from '../../core/core.errors'
 import { CreatePresignedUrlError } from '../../form/admin-form/admin-form.errors'
 import { isFormEncryptMode } from '../../form/form.utils'
 import {
   ResponseModeError,
   SubmissionNotFoundError,
 } from '../submission.errors'
 
+import { AttachmentMetadata } from './encrypt-submission.types'
+
 const logger = createLoggerWithLabel(module)
 const EncryptSubmissionModel = getEncryptSubmissionModel(mongoose)
 
+type AttachmentReducerData = {
+  attachmentMetadata: AttachmentMetadata // type alias for Map<string, string>
+  attachmentUploadPromises: Promise<ManagedUpload.SendData>[]
+}
+
+/**
+ * Uploads a set of submissions to S3 and returns a map of attachment IDs to S3 object keys
+ *
+ * @param formId the id of the form to upload attachments for
+ * @param attachmentData Attachment blob data from the client (including the attachment)
+ *
+ * @returns ok(AttachmentMetadata) A map of field id to the s3 key of the uploaded attachment
+ * @returns err(AttachmentUploadError) if the upload has failed
+ */
+export const uploadAttachments = (
+  formId: string,
+  attachmentData: Record<string, unknown>,
+): ResultAsync<AttachmentMetadata, AttachmentUploadError> => {
+  const { attachmentMetadata, attachmentUploadPromises } = Object.keys(
+    attachmentData,
+  ).reduce<AttachmentReducerData>(
+    (accumulator: AttachmentReducerData, fieldId: string) => {
+      const individualAttachment = JSON.stringify(attachmentData[fieldId])
+
+      const hashStr = crypto
+        .createHash('sha256')
+        .update(individualAttachment)
+        .digest('hex')
+
+      const uploadKey =
+        formId + '/' + crypto.randomBytes(20).toString('hex') + '/' + hashStr
+
+      accumulator.attachmentMetadata.set(fieldId, uploadKey)
+      accumulator.attachmentUploadPromises.push(
+        AwsConfig.s3
+          .upload({
+            Bucket: AwsConfig.attachmentS3Bucket,
+            Key: uploadKey,
+            Body: Buffer.from(individualAttachment),
+          })
+          .promise(),
+      )
+
+      return accumulator
+    },
+    {
+      attachmentMetadata: new Map<string, string>(),
+      attachmentUploadPromises: [],
+    },
+  )
+
+  return ResultAsync.fromPromise(
+    Promise.all(attachmentUploadPromises),
+    (error) => {
+      logger.error({
+        message: 'S3 attachment upload error',
+        meta: {
+          action: 'uploadAttachments',
+          formId,
+        },
+        error,
+      })
+      return new AttachmentUploadError()
+    },
+  ).map(() => attachmentMetadata)
+}
+
 /**
  * Returns a cursor to the stream of the submissions of the given form id.
  *

src/app/modules/submission/encrypt-submission/encrypt-submission.types.ts

@@ -34,3 +34,5 @@ export type EncryptSubmissionBodyAfterProcess = {
 export type WithAttachmentsData<T> = T & { attachmentData: Attachments }
 
 export type WithFormData<T> = T & { formData: string }
+
+export type AttachmentMetadata = Map<string, string>

src/app/modules/submission/encrypt-submission/encrypt-submission.utils.ts

@@ -10,6 +10,7 @@ import {
   VerifyCaptchaError,
 } from '../../../services/captcha/captcha.errors'
 import {
+  AttachmentUploadError,
   DatabaseConflictError,
   DatabaseError,
   DatabasePayloadSizeError,
@@ -54,6 +55,12 @@ export const mapRouteError: MapRouteError = (
   coreErrorMessage = 'Sorry, something went wrong. Please try again.',
 ) => {
   switch (error.constructor) {
+    case AttachmentUploadError:
+      return {
+        statusCode: StatusCodes.BAD_REQUEST,
+        errorMessage:
+          'Could not upload attachments for submission. For assistance, please contact the person who asked you to fill in this form.',
+      }
     case MissingFeatureError:
     case CreateRedirectUrlError:
       return {