From e38260e6e74cb3e1677ddb4f920a657b889cceef Mon Sep 17 00:00:00 2001 From: ssedoudbgouv Date: Thu, 25 Jan 2024 13:57:04 +0100 Subject: [PATCH] TRELLO-2176: filter non existing files from zip download --- app/controllers/ReportController.scala | 2 +- .../ReportFileOrchestrator.scala | 6 +-- .../ReportZipExportService.scala | 43 ++++++++++++------- app/services/S3Service.scala | 3 ++ app/services/S3ServiceInterface.scala | 1 + test/utils/S3ServiceMock.scala | 2 + 6 files changed, 37 insertions(+), 20 deletions(-) diff --git a/app/controllers/ReportController.scala b/app/controllers/ReportController.scala index 7735315e..04b779c8 100644 --- a/app/controllers/ReportController.scala +++ b/app/controllers/ReportController.scala @@ -199,7 +199,7 @@ class ReportController( reportWithDataOrchestrator .getReportFull(reportId, request.identity) .flatMap(_.liftTo[Future](AppError.ReportNotFound(reportId))) - .map(reportData => massImportService.reportSummaryWithAttachmentsZip(reportData)) + .flatMap(reportData => massImportService.reportSummaryWithAttachmentsZip(reportData)) .map(pdfSource => Ok.chunked( content = pdfSource, diff --git a/app/orchestrators/ReportFileOrchestrator.scala b/app/orchestrators/ReportFileOrchestrator.scala index 943acaee..9ac45e40 100644 --- a/app/orchestrators/ReportFileOrchestrator.scala +++ b/app/orchestrators/ReportFileOrchestrator.scala @@ -115,11 +115,11 @@ class ReportFileOrchestrator( for { reportFiles <- reportFileRepository .retrieveReportFiles(report.id) - filteredFilesByOrigin = reportFiles.filter { f => origin.contains(f.origin) || origin.isEmpty } - _ <- Future.successful(filteredFilesByOrigin).ensure(AppError.NoReportFiles)(_.nonEmpty) - } yield reportZipExportService.reportAttachmentsZip(report.creationDate, filteredFilesByOrigin) + _ <- Future.successful(filteredFilesByOrigin).ensure(AppError.NoReportFiles)(_.nonEmpty) + res <- reportZipExportService.reportAttachmentsZip(report.creationDate, filteredFilesByOrigin) + } yield res } diff --git a/app/orchestrators/ReportZipExportService.scala b/app/orchestrators/ReportZipExportService.scala index 87a838e1..580efa8f 100644 --- a/app/orchestrators/ReportZipExportService.scala +++ b/app/orchestrators/ReportZipExportService.scala @@ -5,6 +5,7 @@ import akka.stream.IOResult import akka.stream.Materializer import akka.stream.scaladsl.Source import akka.util.ByteString +import cats.implicits.toTraverseOps import controllers.HtmlFromTemplateGenerator import models.report.ReportFile import play.api.Logger @@ -39,29 +40,39 @@ class ReportZipExportService( def reportSummaryWithAttachmentsZip( reportWithData: ReportWithData - ): Source[ByteString, Future[IOResult]] = { + ): Future[Source[ByteString, Future[IOResult]]] = for { + reportAttachmentSources <- buildReportAttachmentsSources(reportWithData.report.creationDate, reportWithData.files) + reportPdfSummarySource = buildReportPdfSummarySource(reportWithData) + fileSourcesFutures = reportAttachmentSources :+ reportPdfSummarySource + } yield ZipBuilder.buildZip(fileSourcesFutures) - val reportAttachmentSources = reportWithData.files.zipWithIndex.map { case (file, i) => - buildReportAttachmentSource(reportWithData.report.creationDate, file, i) + private def buildReportAttachmentsSources( + creationDate: OffsetDateTime, + reportFiles: Seq[ReportFile] + ) = for { + existingFiles <- reportFiles.traverse(f => + s3Service.exists(f.storageFilename).map(exists => (f, exists)) + ) map (_.collect { case (file, true) => + file + }) + reportAttachmentSources = existingFiles.zipWithIndex.map { case (file, i) => + buildReportAttachmentSource(creationDate, file, i + 1) } - val reportPdfSummarySource = buildReportPdfSummarySource(reportWithData) - - val fileSourcesFutures = reportAttachmentSources :+ reportPdfSummarySource - - ZipBuilder.buildZip(fileSourcesFutures) - } + } yield reportAttachmentSources def reportAttachmentsZip( creationDate: OffsetDateTime, - reports: Seq[ReportFile] - ): Source[ByteString, Future[IOResult]] = { - - val reportAttachmentSources = reports.zipWithIndex.map { case (file, i) => + reportFiles: Seq[ReportFile] + ): Future[Source[ByteString, Future[IOResult]]] = for { + existingFiles <- reportFiles.traverse(f => + s3Service.exists(f.storageFilename).map(exists => (f, exists)) + ) map (_.collect { case (file, true) => + file + }) + reportAttachmentSources = existingFiles.zipWithIndex.map { case (file, i) => buildReportAttachmentSource(creationDate, file, i + 1) } - - ZipBuilder.buildZip(reportAttachmentSources) - } + } yield ZipBuilder.buildZip(reportAttachmentSources) private def buildReportPdfSummarySource( reportWithData: ReportWithData diff --git a/app/services/S3Service.scala b/app/services/S3Service.scala index b54835ee..9fa6513d 100644 --- a/app/services/S3Service.scala +++ b/app/services/S3Service.scala @@ -62,6 +62,9 @@ class S3Service(implicit alpakkaS3Client .getObject(bucketName, bucketKey) + def exists(bucketKey: String): Future[Boolean] = + S3.getObjectMetadata(bucketName, bucketKey).runWith(Sink.headOption).map(_.isDefined) + override def delete(bucketKey: String): Future[Done] = alpakkaS3Client.deleteObject(bucketName, bucketKey).runWith(Sink.head) diff --git a/app/services/S3ServiceInterface.scala b/app/services/S3ServiceInterface.scala index 5bb1f537..f59d2b71 100644 --- a/app/services/S3ServiceInterface.scala +++ b/app/services/S3ServiceInterface.scala @@ -22,4 +22,5 @@ trait S3ServiceInterface { def getSignedUrl(bucketKey: String, method: HttpMethod = HttpMethod.GET): String def downloadFromBucket(bucketKey: String): Source[ByteString, Future[ObjectMetadata]] + def exists(bucketKey: String): Future[Boolean] } diff --git a/test/utils/S3ServiceMock.scala b/test/utils/S3ServiceMock.scala index 1e6a1203..e52274b6 100644 --- a/test/utils/S3ServiceMock.scala +++ b/test/utils/S3ServiceMock.scala @@ -25,4 +25,6 @@ class S3ServiceMock extends S3ServiceInterface { override def getSignedUrl(bucketKey: String, method: HttpMethod): String = ??? override def downloadFromBucket(bucketKey: String): Source[ByteString, Future[ObjectMetadata]] = ??? + + override def exists(bucketKey: String): Future[Boolean] = ??? }