DDCE-4709 Address comments

hmrc · Apr 26, 2024 · b060207 · b060207
1 parent d8b61f4
commit b060207
Show file tree

Hide file tree

Showing 6 changed files with 40 additions and 43 deletions.
diff --git a/app/uk/gov/hmrc/rasapi/connectors/UpscanConnector.scala b/app/uk/gov/hmrc/rasapi/connectors/UpscanConnector.scala
@@ -32,13 +32,13 @@ class UpscanConnector @Inject()(val wsHttp: WSHttp,
 
   def getUpscanFile(downloadUrl: String, reference: String, userId: String): Future[Option[InputStream]] = {
     implicit val system: ActorSystem = ActorSystem()
-    logger.info(s"[FileUploadConnector][getFile] Trying to retrieve file from Upscan: $reference")
+    logger.info(s"[UpscanConnector][getUpscanFile] Trying to retrieve file from Upscan: $reference")
 
     wsHttp.buildRequestWithStream(uri = downloadUrl).map { res =>
       Some(res.bodyAsSource.runWith(StreamConverters.asInputStream()))
     } recover {
       case ex: Throwable =>
-        logger.error(s"[FileUploadConnector][getFile] Exception thrown while retrieving file / converting to InputStream for userId ($userId). ${ex.getMessage}}.", ex)
+        logger.error(s"[UpscanConnector][getUpscanFile] Exception thrown while retrieving file / converting to InputStream for userId ($userId). ${ex.getMessage}}.", ex)
         throw new RuntimeException("Error Streaming file from Upscan service")
     }
   }

diff --git a/app/uk/gov/hmrc/rasapi/controllers/FileProcessingController.scala b/app/uk/gov/hmrc/rasapi/controllers/FileProcessingController.scala
@@ -54,7 +54,7 @@ class FileProcessingController @Inject()(
                 sessionCacheService.updateFileSession(userId, callbackData, None, None)
               }
             case STATUS_FAILED => logger.error(s"[FileProcessingController][statusCallback] There is a problem with the " +
-              s"file for userId ($userId) FAILED (${callbackData.reference}), the failure reason is: ${callbackData.failureReason} and the message is: ${callbackData.message}")
+              s"file for userId ($userId) and fileId: (${callbackData.reference}), the failure reason is: ${callbackData.failureReason} and the message is: ${callbackData.message}")
               sessionCacheService.updateFileSession(userId, callbackData, None, None)
             case _ => logger.warn(s"[FileProcessingController][statusCallback] There is a problem with the file (${callbackData.reference}) for userId ($userId), the status is:" +
               s" ${callbackData.fileStatus}")

diff --git a/app/uk/gov/hmrc/rasapi/models/CallbackData.scala b/app/uk/gov/hmrc/rasapi/models/CallbackData.scala
@@ -25,18 +25,9 @@ import java.time.Instant
 case class FileMetadata(id: String, name: Option[String], created: Option[String])
 
 object FileMetadata {
-  def fromCallbackData(cd: UpscanCallbackData): FileMetadata =
-    FileMetadata(
-      cd.reference,
-      Some(cd.uploadDetails.getOrElse(UploadDetails.empty).fileName),
-      Some(cd.uploadDetails.getOrElse(UploadDetails.empty).uploadTimestamp.toString)
-    )
-
   implicit val format: OFormat[FileMetadata] = Json.format[FileMetadata]
 }
 
-//case class CallbackData(downloadUrl: String, fileId: String, status: String, reason: Option[String], uploadDetails: Option[UploadDetails] = None)
-
 case class UploadDetails(uploadTimestamp: Instant, checksum: String, fileMimeType: String, fileName: String, size: Int)
 
 case class FailureDetails(failureReason: String, message: String)
@@ -45,19 +36,26 @@ case class FailureDetails(failureReason: String, message: String)
   }
 
 object UploadDetails {
-  val empty: UploadDetails = UploadDetails(Instant.now(), "", "", "", 0)
   implicit val formats: OFormat[UploadDetails] = Json.format[UploadDetails]
 }
+
 case class UpscanCallbackData(reference: String, downloadUrl: Option[String], fileStatus: String, uploadDetails: Option[UploadDetails], failureDetails: Option[FailureDetails]) {
   val failureReason: String = this.failureDetails.getOrElse(FailureDetails("unknown", "")).failureReason
   val message: String = this.failureDetails.getOrElse(FailureDetails("unknown", "")).message
+
+  def toFileMetadata: FileMetadata =
+    FileMetadata(
+      this.reference,
+      Some(this.uploadDetails.getOrElse(UploadDetails(Instant.now(), "", "", "", 0)).fileName),
+      Some(this.uploadDetails.getOrElse(UploadDetails(Instant.now(), "", "", "", 0)).uploadTimestamp.toString)
+    )
 }
 
 object UpscanCallbackData {
   implicit val formats: OFormat[UpscanCallbackData] = Json.format[UpscanCallbackData]
 }
 
-case class ResultsFileMetaData (id: String, filename: String,uploadDate: Long, chunkSize: Int, length: Long)
+case class ResultsFileMetaData(id: String, filename: String, uploadDate: Long, chunkSize: Int, length: Long)
 
 object ResultsFileMetaData {
   implicit val formats: OFormat[ResultsFileMetaData] = Json.format[ResultsFileMetaData]
@@ -67,13 +65,13 @@ case class Chunks(_id: ObjectId, files_id: ObjectId)
 
 object Chunks {
   implicit val objectIdformats: Format[ObjectId] = MongoFormats.objectIdFormat
-  implicit  val format: OFormat[Chunks] = Json.format[Chunks]
+  implicit val format: OFormat[Chunks] = Json.format[Chunks]
 }
 
 case class FileSession(userFile: Option[UpscanCallbackData],
                        resultsFile: Option[ResultsFileMetaData],
                        userId: String,
-                       uploadTimeStamp : Option[Long],
+                       uploadTimeStamp: Option[Long],
                        fileMetadata: Option[FileMetadata]
                       )
 

diff --git a/app/uk/gov/hmrc/rasapi/services/FileProcessingService.scala b/app/uk/gov/hmrc/rasapi/services/FileProcessingService.scala
@@ -99,13 +99,13 @@ class FileProcessingService @Inject()(
       }
     }
 
-    try{
+    try {
       val dataIterator = inputFileData.get.toList
       logger.info(s"[FileProcessingService][manipulateFile] File data size ${dataIterator.size} for user $userId")
       writeResultToFile(writer._2, s"National Insurance number,First name,Last name,Date of birth,$getTaxYearHeadings", userId)
       dataIterator.foreach(row =>
         if (row.nonEmpty) {
-          writeResultToFile(writer._2,fetchResult(removeDoubleQuotes(row),userId,callbackData.reference, apiVersion = apiVersion), userId)
+          writeResultToFile(writer._2, fetchResult(removeDoubleQuotes(row), userId, callbackData.reference, apiVersion = apiVersion), userId)
         }
       )
       closeWriter(writer._2)
@@ -115,41 +115,40 @@ class FileProcessingService @Inject()(
 
       saveFile(writer._1, userId, callbackData)
 
-    } catch
-      {
-        case ex:Throwable =>
-          logger.error(s"[FileProcessingService][manipulateFile] Error for userId ($userId) in File processing -> ${ex.getMessage}", ex)
-          sessionCacheService.updateFileSession(userId, callbackData.copy(fileStatus = STATUS_FAILED), None, None)
-          fileResultsMetrics.stop
-      }
+    } catch {
+      case ex: Throwable =>
+        logger.error(s"[FileProcessingService][manipulateFile] Error for userId ($userId) in File processing -> ${ex.getMessage}", ex)
+        sessionCacheService.updateFileSession(userId, callbackData.copy(fileStatus = STATUS_FAILED), None, None)
+        fileResultsMetrics.stop
+    }
     finally {
       closeWriter(writer._2)
-      clearFile( writer._1, userId)
+      clearFile(writer._1, userId)
     }
   }
 
   def saveFile(filePath: Path, userId: String, callbackData: UpscanCallbackData): Unit = {
 
     logger.info("[FileProcessingService][saveFile] Starting to save file...")
     val fileSaveMetrics = metrics.register(fileSave).time
-      fileRepo.saveFile(userId, callbackData.reference, filePath).onComplete {
-        result =>
-          result match {
-            case Success(file) =>
-              logger.info(s"[FileProcessingService][saveFile] Starting to save the file (${file.getId}) for user ID: $userId")
+    fileRepo.saveFile(userId, callbackData.reference, filePath).onComplete {
+      result =>
+        result match {
+          case Success(file) =>
+            logger.info(s"[FileProcessingService][saveFile] Starting to save the file (${file.getId}) for user ID: $userId")
 
-              val resultsFileMetaData = Some(ResultsFileMetaData(file.getId.toString, file.getFilename, file.getUploadDate.getTime, file.getChunkSize, file.getLength))
+            val resultsFileMetaData = Some(ResultsFileMetaData(file.getId.toString, file.getFilename, file.getUploadDate.getTime, file.getChunkSize, file.getLength))
 
-                  sessionCacheService.updateFileSession(userId, callbackData, resultsFileMetaData, Some(FileMetadata.fromCallbackData(callbackData)))
+            sessionCacheService.updateFileSession(userId, callbackData, resultsFileMetaData, Some(callbackData.toFileMetadata))
 
-              logger.info(s"[FileProcessingService][saveFile] Completed saving the file (${file.getId}) for user ID: $userId")
-            case Failure(ex) =>
-              logger.error(s"[FileProcessingService][saveFile] results file for userId ($userId) generation/saving failed with Exception ${ex.getMessage}", ex)
-              sessionCacheService.updateFileSession(userId, callbackData.copy(fileStatus = STATUS_FAILED), None, None)
-          }
-          fileSaveMetrics.stop
+            logger.info(s"[FileProcessingService][saveFile] Completed saving the file (${file.getId}) for user ID: $userId")
+          case Failure(ex) =>
+            logger.error(s"[FileProcessingService][saveFile] results file for userId ($userId) generation/saving failed with Exception ${ex.getMessage}", ex)
+            sessionCacheService.updateFileSession(userId, callbackData.copy(fileStatus = STATUS_FAILED), None, None)
+        }
+        fileSaveMetrics.stop
 
-      }
+    }
   }
 
   def getTaxYearHeadings: String = {

diff --git a/it/uk/gov/hmrc/rasapi/repositories/RasFileRepositoryISpec.scala b/it/uk/gov/hmrc/rasapi/repositories/RasFileRepositoryISpec.scala
@@ -50,11 +50,11 @@ class RasFileRepositoryISpec extends PlaySpec with ScalaFutures with GuiceOneApp
     }
 
     def fileCount: Long = {
-      await(rasFileRepository.gridFSG.find().collect().head().map(res => res.length))
+      await(rasFileRepository.gridFSG.find().collect().head().map(_.length))
     }
 
     def chunksCount: Int =
-      await(rasChunksRepository.collection.find().collect().head().map(res => res.length))
+      await(rasChunksRepository.collection.find().collect().head().map(_.length))
 
     def saveFile(): Unit = {
       await(rasFileRepository.saveFile(

diff --git a/project/build.properties b/project/build.properties
@@ -1 +1 @@
-sbt.version=1.9.7
+sbt.version=1.9.9