From 3f090af6a8583677643a521da7b96c622d0843ed Mon Sep 17 00:00:00 2001 From: SachaMcCormack <175815565+SachaMcCormack@users.noreply.github.com> Date: Thu, 30 Oct 2025 12:50:50 +0000 Subject: [PATCH 1/8] added internal-auth code --- .../hmrc/bindingtarifffilestore/config/AppConfig.scala | 4 ++++ .../connector/ObjectStoreConnector.scala | 9 ++++++--- build.sbt | 2 ++ conf/application.conf | 9 +++++++-- project/AppDependencies.scala | 6 +++--- 5 files changed, 22 insertions(+), 8 deletions(-) diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala b/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala index a7cdfac..ffdfad1 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala @@ -41,6 +41,10 @@ class AppConfig @Inject() ( lazy val filestoreUrl: String = config.get[String]("filestore.url") lazy val filestoreSSL: Boolean = config.get[Boolean]("filestore.ssl") + private lazy val objectStoreHost: String = config.get[String]("microservice.services.object-store.host") + private lazy val objectStorePort: String = config.get[String]("microservice.services.object-store.port") + lazy val objectStoreUrl: String = s"http://$objectStoreHost:$objectStorePort" + lazy val isTestMode: Boolean = config.getOptional[Boolean]("testMode").getOrElse(false) } diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala b/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala index 6a5837a..ec994c2 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala @@ -18,13 +18,14 @@ package uk.gov.hmrc.bindingtarifffilestore.connector import uk.gov.hmrc.objectstore.client.play.PlayObjectStoreClient import com.google.inject.Inject +import org.apache.pekko.actor.ActorSystem import javax.inject.Singleton import uk.gov.hmrc.bindingtarifffilestore.config.AppConfig import uk.gov.hmrc.bindingtarifffilestore.model.FileMetadata import uk.gov.hmrc.bindingtarifffilestore.util.Logging import uk.gov.hmrc.http.HeaderCarrier -import uk.gov.hmrc.objectstore.client.{ObjectSummary, Path} +import uk.gov.hmrc.objectstore.client.{ObjectSummary, Path, Sha256Checksum} import java.net.URI import scala.concurrent.{ExecutionContext, Future} @@ -48,11 +49,12 @@ class ObjectStoreConnector @Inject() (client: PlayObjectStoreClient, config: App client .uploadFromUrl( from = new URI(fileMetaData.url.getOrElse(throw new IllegalArgumentException("Missing URL"))).toURL, - to = directory.file(fileMetaData.fileName.getOrElse("")) + to = directory.file(fileMetaData.fileName.getOrElse("")), + contentSha256 = Some(Sha256Checksum.fromHex(fileMetaData.fileName.get)) ) ) match { case Success(_) => - fileMetaData.copy(url = Some(s"${config.filestoreUrl}/${fileMetaData.id}")) + fileMetaData.copy(url = Some(s"${config.objectStoreUrl}/${fileMetaData.fileName.get}")) case Failure(e: Throwable) => log.error("Failed to upload to the object store.", e) throw e @@ -90,6 +92,7 @@ class ObjectStoreConnector @Inject() (client: PlayObjectStoreClient, config: App exception.printStackTrace() Future.successful(fileMetaData) case scala.util.Success(presignedDownloadUrl) => + println("Successful Upload") val updatedMetaData = fileMetaData.copy(url = Some(presignedDownloadUrl.downloadUrl.toString)) Future.successful(updatedMetaData) } diff --git a/build.sbt b/build.sbt index 5bdeaaf..8a2a960 100644 --- a/build.sbt +++ b/build.sbt @@ -28,3 +28,5 @@ lazy val it = project .settings(DefaultBuildSettings.itSettings()) addCommandAlias("scalafmtAll", "all scalafmtSbt scalafmt Test/scalafmt it/Test/scalafmt") + +resolvers += MavenRepository("HMRC-open-artefacts-maven2", "https://open.artefacts.tax.service.gov.uk/maven2") diff --git a/conf/application.conf b/conf/application.conf index 563b9b6..4b67273 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -107,5 +107,10 @@ upscan { maxFileSize = 104857600 //100 MB } -internal-auth.token = "9253947-99f3-47d7-9af2-b75b4f37fd34" -object-store.default-retention-period = 1-month \ No newline at end of file +auth { + api-token = "9253947-99f3-47d7-9af2-b75b4f37fd34" +} + +internal-auth.token = "" + +object-store.default-retention-period = "1-month" \ No newline at end of file diff --git a/project/AppDependencies.scala b/project/AppDependencies.scala index fd0c633..ec0ea1b 100644 --- a/project/AppDependencies.scala +++ b/project/AppDependencies.scala @@ -2,15 +2,15 @@ import sbt.* object AppDependencies { - private lazy val bootstrapPlayVersion = "10.1.0" - private lazy val hmrcMongoVersion = "2.7.0" + private lazy val bootstrapPlayVersion = "10.3.0" + private lazy val hmrcMongoVersion = "2.10.0" val compile: Seq[ModuleID] = Seq( "uk.gov.hmrc" %% "bootstrap-backend-play-30" % bootstrapPlayVersion, "uk.gov.hmrc.mongo" %% "hmrc-mongo-play-30" % hmrcMongoVersion, "com.fasterxml.jackson.module" %% "jackson-module-scala" % "2.20.0", "org.apache.httpcomponents" % "httpmime" % "4.5.14", - "uk.gov.hmrc.objectstore" %% "object-store-client-play-30" % "2.4.0" + "uk.gov.hmrc.objectstore" %% "object-store-client-play-30" % "2.5.0" ) val test: Seq[ModuleID] = Seq( From 1e43c47e2b2416ccb92c2ae5828519127da33324 Mon Sep 17 00:00:00 2001 From: SachaMcCormack <175815565+SachaMcCormack@users.noreply.github.com> Date: Mon, 6 Oct 2025 14:37:42 +0100 Subject: [PATCH 2/8] rebasing --- .../config/AppConfig.scala | 19 +- .../connector/AmazonS3Connector.scala | 132 ++++++++++ .../controllers/FileStoreController.scala | 26 +- .../model/Pagination.scala | 3 +- .../FileMetadataMongoRepository.scala | 8 +- .../service/FileStoreService.scala | 55 ++--- conf/app.routes | 2 +- .../BaseFeatureSpec.scala | 5 +- .../FileStoreHelpers.scala | 45 ++-- .../FileStoreSpec.scala | 25 +- .../config/AppConfigSpec.scala | 34 ++- .../connector/AmazonS3ConnectorSpec.scala | 227 ++++++++++++++++++ .../connector/ObjectStoreConnectorSpec.scala | 182 -------------- .../connector/UpscanConnectorSpec.scala | 4 +- .../controllers/FileStoreControllerSpec.scala | 179 +++++++------- .../service/FileStoreServiceSpec.scala | 59 ++--- .../util/WithFakeApplication.scala | 7 +- 17 files changed, 594 insertions(+), 418 deletions(-) create mode 100644 app/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3Connector.scala create mode 100644 test/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3ConnectorSpec.scala delete mode 100644 test/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnectorSpec.scala diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala b/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala index ffdfad1..7629097 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala @@ -26,9 +26,13 @@ class AppConfig @Inject() ( servicesConfig: ServicesConfig ) { - lazy val authorization: String = config.get[String]("internal-auth.token") + lazy val authorization: String = config.get[String]("auth.api-token") - lazy val appName: String = config.get[String]("appName") + lazy val s3Configuration: S3Configuration = S3Configuration( + config.get[String]("s3.region"), + config.get[String]("s3.bucket"), + Option(config.get[String]("s3.endpoint")).filter(_.nonEmpty) + ) lazy val upscanInitiateUrl: String = servicesConfig.baseUrl("upscan-initiate") lazy val fileStoreSizeConfiguration: FileStoreSizeConfiguration = FileStoreSizeConfiguration( @@ -36,8 +40,6 @@ class AppConfig @Inject() ( minFileSize = config.get[Int]("upscan.minFileSize") ) - lazy val s3bucket: String = config.get[String]("s3.bucket") - lazy val filestoreUrl: String = config.get[String]("filestore.url") lazy val filestoreSSL: Boolean = config.get[Boolean]("filestore.ssl") @@ -48,6 +50,15 @@ class AppConfig @Inject() ( lazy val isTestMode: Boolean = config.getOptional[Boolean]("testMode").getOrElse(false) } +case class S3Configuration( + region: String, + bucket: String, + endpoint: Option[String] +) { + + def baseUrl: String = endpoint.getOrElse(s"https://s3-$region.amazonaws.com") +} + case class FileStoreSizeConfiguration( minFileSize: Int, maxFileSize: Int diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3Connector.scala b/app/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3Connector.scala new file mode 100644 index 0000000..850a434 --- /dev/null +++ b/app/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3Connector.scala @@ -0,0 +1,132 @@ +/* + * Copyright 2025 HM Revenue & Customs + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package uk.gov.hmrc.bindingtarifffilestore.connector + +import com.amazonaws.auth.{AWSCredentials, BasicAWSCredentials, DefaultAWSCredentialsProviderChain} + +import java.io.BufferedInputStream +import java.net.URL +import java.util +import com.amazonaws.{AmazonClientException, HttpMethod} +import com.amazonaws.client.builder.AwsClientBuilder.EndpointConfiguration +import com.amazonaws.services.s3.model.DeleteObjectsRequest.KeyVersion +import com.amazonaws.services.s3.model._ +import com.amazonaws.services.s3.{AmazonS3, AmazonS3ClientBuilder} +import com.google.inject.Inject + +import javax.inject.Singleton +import uk.gov.hmrc.bindingtarifffilestore.config.AppConfig +import uk.gov.hmrc.bindingtarifffilestore.model.FileMetadata +import uk.gov.hmrc.bindingtarifffilestore.util.Logging + +import scala.jdk.CollectionConverters._ +import scala.util.{Failure, Success, Try} + +@Singleton +class AmazonS3Connector @Inject() (config: AppConfig) extends Logging { + + private lazy val s3Config = config.s3Configuration + + private lazy val s3client: AmazonS3 = { + log.info(s"${s3Config.bucket}:${s3Config.region}") + val builder = AmazonS3ClientBuilder + .standard() + .withPathStyleAccessEnabled(true) + .withCredentials(new LocalDevelopmentS3CredentialsProviderChain()) + + s3Config.endpoint match { + case Some(endpoint) => builder.withEndpointConfiguration(new EndpointConfiguration(endpoint, s3Config.region)) + case _ => builder.withRegion(s3Config.region) + } + + builder.build() + } + + def getAll: Seq[String] = + sequenceOf( + s3client.listObjects(s3Config.bucket).getObjectSummaries + ).map(_.getKey) + + def upload(fileMetaData: FileMetadata): FileMetadata = { + val url: URL = new URL(fileMetaData.url.getOrElse(throw new IllegalArgumentException("Missing URL"))) + + val metadata = new ObjectMetadata + // This .get is scary but our file must have received a positive scan + // result and received metadata from Upscan if it is being published + metadata.setContentType(fileMetaData.mimeType.get) + metadata.setContentLength(contentLengthOf(url)) + + val request = new PutObjectRequest( + s3Config.bucket, + fileMetaData.id, + new BufferedInputStream(url.openStream()), + metadata + ).withCannedAcl(CannedAccessControlList.Private) + + Try(s3client.putObject(request)) match { + case Success(_) => + fileMetaData.copy(url = Some(s"${s3Config.baseUrl}/${s3Config.bucket}/${fileMetaData.id}")) + case Failure(e: Throwable) => + log.error("Failed to upload to the S3 bucket.", e) + throw e + } + } + + def delete(id: String): Unit = + s3client.deleteObject(s3Config.bucket, id) + + def deleteAll(): Unit = { + val keys: Seq[KeyVersion] = getAll.map(new KeyVersion(_)) + if (keys.nonEmpty) { + log.info(s"Removing [${keys.length}] files from S3") + log.info(s"bucket is: ${s3Config.bucket}") + val request = new DeleteObjectsRequest(s3Config.bucket) + .withKeys(keys.toList.asJava) + .withQuiet(false) + s3client.deleteObjects(request) + } else { + log.info(s"No files to remove from S3") + } + } + + def sign(fileMetaData: FileMetadata): FileMetadata = + if (fileMetaData.url.isDefined) { + val authenticatedURLRequest = new GeneratePresignedUrlRequest(config.s3Configuration.bucket, fileMetaData.id) + .withMethod(HttpMethod.GET) + val authenticatedURL: URL = s3client.generatePresignedUrl(authenticatedURLRequest) + fileMetaData.copy(url = Some(authenticatedURL.toString)) + } else { + fileMetaData + } + + private def contentLengthOf(url: URL): Long = + url.openConnection.getContentLengthLong + + private def sequenceOf[T](list: util.List[T]): Seq[T] = + list.iterator.asScala.toSeq + +} + +class LocalDevelopmentS3CredentialsProviderChain() extends DefaultAWSCredentialsProviderChain { + + override def getCredentials(): AWSCredentials = + Try { + super.getCredentials() + }.recover { case _: AmazonClientException => + new BasicAWSCredentials("dummy-access-key", "dummy-secret-key") + }.get +} diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/controllers/FileStoreController.scala b/app/uk/gov/hmrc/bindingtarifffilestore/controllers/FileStoreController.scala index 5a3849e..8b8f6ab 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/controllers/FileStoreController.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/controllers/FileStoreController.scala @@ -18,12 +18,10 @@ package uk.gov.hmrc.bindingtarifffilestore.controllers import play.api.Logging import play.api.libs.Files.TemporaryFile -import play.api.libs.json.Format.GenericFormat -import play.api.libs.json.OFormat.oFormatFromReadsAndOWrites import play.api.libs.json.{JsValue, Json} import play.api.mvc._ import uk.gov.hmrc.bindingtarifffilestore.config.AppConfig -import uk.gov.hmrc.bindingtarifffilestore.model.FileMetadataREST.format +import uk.gov.hmrc.bindingtarifffilestore.model.FileMetadataREST._ import uk.gov.hmrc.bindingtarifffilestore.model._ import uk.gov.hmrc.bindingtarifffilestore.model.upscan.ScanResult import uk.gov.hmrc.bindingtarifffilestore.model.upscan.v2._ @@ -54,13 +52,11 @@ class FileStoreController @Inject() ( def encodeInBase64(text: String): String = Base64.getEncoder.encodeToString(text.getBytes(StandardCharsets.UTF_8)) - private def withFileMetadata(id: String)(f: FileMetadata => Future[Result])(implicit - hc: HeaderCarrier - ): Future[Result] = + private def withFileMetadata(id: String)(f: FileMetadata => Future[Result]): Future[Result] = service.find(id).flatMap { case Some(meta) => logger.info( - s"[FileStoreController][withFileMetadata] Attachment File: $id, Scan succeeded with details fileMetadata: ${encodeInBase64(meta.toString)}" + s"[FileStoreController][withFileMetadata] Attachement File: $id, Scan succeeded with details fileMetadata: ${encodeInBase64(meta.toString)}" ) f(meta) case None => @@ -71,8 +67,6 @@ class FileStoreController @Inject() ( lazy private val testModeFilter = TestMode.actionFilter(appConfig, parse.default) def deleteAll(): Action[AnyContent] = withErrorHandling { - implicit val hc: HeaderCarrier = HeaderCarrier() - testModeFilter.async { service .deleteAll() @@ -80,11 +74,9 @@ class FileStoreController @Inject() ( } } - def delete(id: String, filename: String): Action[AnyContent] = withErrorHandling { _ => - implicit val hc: HeaderCarrier = HeaderCarrier() - + def delete(id: String): Action[AnyContent] = withErrorHandling { _ => service - .delete(id, filename) + .delete(id) .map(_ => NoContent) } @@ -114,10 +106,8 @@ class FileStoreController @Inject() ( } } - def get(id: String): Action[AnyContent] = Action.async { implicit request => - withFileMetadata(id) { meta => - Future.successful(Ok(Json.toJson(meta))) - } + def get(id: String): Action[AnyContent] = Action.async { + withFileMetadata(id)(meta => Future.successful(Ok(Json.toJson(meta)))) } def notification(id: String): Action[JsValue] = withErrorHandling { @@ -144,8 +134,6 @@ class FileStoreController @Inject() ( } def getAll(search: Search, pagination: Option[Pagination]): Action[AnyContent] = withErrorHandling { _ => - implicit val hc: HeaderCarrier = HeaderCarrier() - service.find(search, pagination.getOrElse(Pagination.max)).map { pagedResults => if (pagination.isDefined) { Ok(Json.toJson(pagedResults)) diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/model/Pagination.scala b/app/uk/gov/hmrc/bindingtarifffilestore/model/Pagination.scala index 5e4566b..5615dca 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/model/Pagination.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/model/Pagination.scala @@ -17,6 +17,7 @@ package uk.gov.hmrc.bindingtarifffilestore.model import play.api.mvc.QueryStringBindable +import uk.gov.hmrc.bindingtarifffilestore.model.Pagination.{defaultPageSize, defaultPageStart} case class Pagination( page: Int = defaultPageStart, @@ -27,7 +28,7 @@ object Pagination { val defaultPageStart = 1 val defaultPageSize = 100 - val max: Pagination = Pagination(1, 10000) + val max: Pagination = Pagination(1, Integer.MAX_VALUE) private val pageKey = "page" private val pageSizeKey = "page_size" diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/repository/FileMetadataMongoRepository.scala b/app/uk/gov/hmrc/bindingtarifffilestore/repository/FileMetadataMongoRepository.scala index a278e5a..c443071 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/repository/FileMetadataMongoRepository.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/repository/FileMetadataMongoRepository.scala @@ -17,13 +17,15 @@ package uk.gov.hmrc.bindingtarifffilestore.repository import org.mongodb.scala.bson.conversions.Bson -import org.mongodb.scala.model.* -import org.mongodb.scala.model.Filters.* +import org.mongodb.scala.model.Filters._ +import org.mongodb.scala.model._ import org.mongodb.scala.result.UpdateResult -import uk.gov.hmrc.bindingtarifffilestore.model.* +import uk.gov.hmrc.bindingtarifffilestore.model._ import uk.gov.hmrc.bindingtarifffilestore.util.Logging import uk.gov.hmrc.mongo.MongoComponent import uk.gov.hmrc.mongo.play.json.PlayMongoRepository +import org.mongodb.scala.SingleObservableFuture +import org.mongodb.scala.ObservableFuture import java.time.Instant import javax.inject.{Inject, Singleton} diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/service/FileStoreService.scala b/app/uk/gov/hmrc/bindingtarifffilestore/service/FileStoreService.scala index fee8cab..7d34bb4 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/service/FileStoreService.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/service/FileStoreService.scala @@ -19,7 +19,7 @@ package uk.gov.hmrc.bindingtarifffilestore.service import play.api.Logging import uk.gov.hmrc.bindingtarifffilestore.audit.AuditService import uk.gov.hmrc.bindingtarifffilestore.config.AppConfig -import uk.gov.hmrc.bindingtarifffilestore.connector.{ObjectStoreConnector, UpscanConnector} +import uk.gov.hmrc.bindingtarifffilestore.connector.{AmazonS3Connector, UpscanConnector} import uk.gov.hmrc.bindingtarifffilestore.controllers.routes import uk.gov.hmrc.bindingtarifffilestore.model.ScanStatus.READY import uk.gov.hmrc.bindingtarifffilestore.model.* @@ -27,17 +27,17 @@ import uk.gov.hmrc.bindingtarifffilestore.model.upscan.* import uk.gov.hmrc.bindingtarifffilestore.repository.FileMetadataMongoRepository import uk.gov.hmrc.bindingtarifffilestore.util.HashUtil import uk.gov.hmrc.http.HeaderCarrier - +import java.util.Base64 import java.nio.charset.StandardCharsets -import java.util.{Base64, UUID} + +import java.util.UUID import javax.inject.{Inject, Singleton} import scala.concurrent.{ExecutionContext, Future} -import scala.language.postfixOps @Singleton() class FileStoreService @Inject() ( appConfig: AppConfig, - fileStoreConnector: ObjectStoreConnector, + fileStoreConnector: AmazonS3Connector, repository: FileMetadataMongoRepository, upscanConnector: UpscanConnector, auditService: AuditService @@ -120,19 +120,11 @@ class FileStoreService @Inject() ( } yield update } - def find(id: String)(implicit hc: HeaderCarrier): Future[Option[FileMetadata]] = - repository.get(id) map signingPermanentURL flatMap { - case Some(value) => value.map(data => Some(data)) - case None => Future.successful(None) - } + def find(id: String): Future[Option[FileMetadata]] = + repository.get(id) map signingPermanentURL - def find(search: Search, pagination: Pagination)(implicit hc: HeaderCarrier): Future[Paged[FileMetadata]] = { - val pagedFuture: Future[Paged[Future[FileMetadata]]] = - repository.get(search, pagination: Pagination) map signingPermanentURLs() - pagedFuture.flatMap { paged => - Future.sequence(paged.results).map(resolvedItems => paged.copy(results = resolvedItems)) - } - } + def find(search: Search, pagination: Pagination): Future[Paged[FileMetadata]] = + repository.get(search, pagination: Pagination) map (signingPermanentURLs(_)) // when UpScan comes back to us with the scan result def notify(attachment: FileMetadata, scanResult: ScanResult)(implicit @@ -140,7 +132,7 @@ class FileStoreService @Inject() ( ): Future[Option[FileMetadata]] = { logger.info( - s"[FileStoreService][notify] Attachment File: ${attachment.id}, Scan completed with status [${scanResult.fileStatus}] and Upscan reference [${scanResult.reference}]" + s"[FileStoreService][notify] Attachement File: ${attachment.id}, Scan completed with status [${scanResult.fileStatus}] and Upscan reference [${scanResult.reference}]" ) auditService @@ -151,12 +143,12 @@ class FileStoreService @Inject() ( scanResult match { case FailedScanResult(_, _, details) => logger.info( - s"[FileStoreService][notify] Attachment File: ${attachment.id}, Scan failed because it was [${details.failureReason}] with message [${details.message}]" + s"[FileStoreService][notify] Attachement File: ${attachment.id}, Scan failed because it was [${details.failureReason}] with message [${details.message}]" ) repository.update(updatedAttachment) case SuccessfulScanResult(_, _, _, details) => logger.info( - s"[FileStoreService][notify] Attachment File: ${attachment.id}, Scan succeeded with details [fileName: ${encodeInBase64( + s"[FileStoreService][notify] Attachement File: ${attachment.id}, Scan succeeded with details [fileName: ${encodeInBase64( details.fileName )}, fileMimeType:${details.fileMimeType}, checksum:${encodeInBase64(details.checksum)}, ${details.uploadTimestamp}]" ) @@ -168,7 +160,7 @@ class FileStoreService @Inject() ( publish(metadata) case _ => logger.info( - s"[FileStoreService][notify] Attachment File: ${attachment.id}, Scan completed as READY but it couldn't be published as it no longer exists" + s"[FileStoreService][notify] Attachement File: ${attachment.id}, Scan completed as READY but it couldn't be published as it no longer exists" ) Future.successful(None) } @@ -192,7 +184,6 @@ class FileStoreService @Inject() ( repository .update(metadata.copy(publishable = true, published = true)) .map(signingPermanentURL) - .flatMap(a => a.get.map(Some(_))) // File is safe, unpublished but the download URL has expired. Clean Up. case (Some(READY), false) => @@ -215,12 +206,12 @@ class FileStoreService @Inject() ( } } - def deleteAll()(implicit hc: HeaderCarrier): Future[Unit] = + def deleteAll(): Future[Unit] = repository.deleteAll() map (_ => fileStoreConnector.deleteAll()) - def delete(id: String, filename: String)(implicit hc: HeaderCarrier): Future[Unit] = { + def delete(id: String): Future[Unit] = { logger.info(s"[FileStoreService][delete] Deleting file: $id") - repository.delete(id) map (_ => fileStoreConnector.delete(filename)) + repository.delete(id) map (_ => fileStoreConnector.delete(id)) } private def upscanInitiate(fileMetadata: FileMetadata)(implicit hc: HeaderCarrier): Future[UpscanInitiateResponse] = { @@ -241,18 +232,12 @@ class FileStoreService @Inject() ( } yield initiateResponse } - private def signingPermanentURL(implicit hc: HeaderCarrier): Option[FileMetadata] => Option[Future[FileMetadata]] = { - asd => - asd.map(signingIfPublished) - } + private def signingPermanentURL: Option[FileMetadata] => Option[FileMetadata] = _.map(signingIfPublished) - private def signingPermanentURLs()(implicit hc: HeaderCarrier): Paged[FileMetadata] => Paged[Future[FileMetadata]] = { - asd => - asd.map(signingIfPublished) - } + private def signingPermanentURLs: Paged[FileMetadata] => Paged[FileMetadata] = _.map(signingIfPublished) - private def signingIfPublished(implicit hc: HeaderCarrier): FileMetadata => Future[FileMetadata] = { + private def signingIfPublished: FileMetadata => FileMetadata = { case file if file.published => fileStoreConnector.sign(file) - case other => Future.successful(other) + case other => other } } diff --git a/conf/app.routes b/conf/app.routes index 099b62f..ded0cd4 100644 --- a/conf/app.routes +++ b/conf/app.routes @@ -10,4 +10,4 @@ POST /file/:id/publish @uk.gov.hmrc.bindingtarifffilestore.controll # admin/test endpoints DELETE /file @uk.gov.hmrc.bindingtarifffilestore.controllers.FileStoreController.deleteAll() -DELETE /file/:id @uk.gov.hmrc.bindingtarifffilestore.controllers.FileStoreController.delete(id: String, filename: String) +DELETE /file/:id @uk.gov.hmrc.bindingtarifffilestore.controllers.FileStoreController.delete(id: String) diff --git a/it/test/uk/gov/hmrc/bindingtarifffilestore/BaseFeatureSpec.scala b/it/test/uk/gov/hmrc/bindingtarifffilestore/BaseFeatureSpec.scala index 21900ce..b5b9b29 100644 --- a/it/test/uk/gov/hmrc/bindingtarifffilestore/BaseFeatureSpec.scala +++ b/it/test/uk/gov/hmrc/bindingtarifffilestore/BaseFeatureSpec.scala @@ -17,7 +17,7 @@ package uk.gov.hmrc.bindingtarifffilestore import com.google.common.io.BaseEncoding -import org.scalatest.* +import org.scalatest._ import org.scalatest.featurespec.AnyFeatureSpec import org.scalatest.matchers.should.Matchers import org.scalatestplus.play.guice.GuiceOneServerPerSuite @@ -31,8 +31,9 @@ import org.mongodb.scala.SingleObservableFuture import java.security.MessageDigest import scala.concurrent.{Await, Future} +import scala.concurrent.Await.result import scala.concurrent.ExecutionContext.Implicits.global -import scala.concurrent.duration.* +import scala.concurrent.duration._ abstract class BaseFeatureSpec extends AnyFeatureSpec diff --git a/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreHelpers.scala b/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreHelpers.scala index 2114658..4bd1136 100644 --- a/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreHelpers.scala +++ b/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreHelpers.scala @@ -16,16 +16,16 @@ package uk.gov.hmrc.bindingtarifffilestore -import com.github.tomakehurst.wiremock.client.WireMock.* +import com.github.tomakehurst.wiremock.client.WireMock._ import com.github.tomakehurst.wiremock.stubbing.StubMapping import play.api.http.Status import play.api.libs.Files.SingletonTemporaryFileCreator import play.api.libs.json.Json import uk.gov.hmrc.bindingtarifffilestore.model.FileMetadataREST.format -import uk.gov.hmrc.bindingtarifffilestore.model.* -import uk.gov.hmrc.bindingtarifffilestore.model.upscan.* +import uk.gov.hmrc.bindingtarifffilestore.model._ +import uk.gov.hmrc.bindingtarifffilestore.model.upscan._ import uk.gov.hmrc.bindingtarifffilestore.model.upscan.v2.FileStoreInitiateRequest -import uk.gov.hmrc.http.HttpReads.Implicits.* +import uk.gov.hmrc.http.HttpReads.Implicits._ import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse, StringContextOps} import java.io.File @@ -34,9 +34,9 @@ import java.nio.file.Files import java.time.Instant import scala.concurrent.ExecutionContext.Implicits.global import scala.concurrent.Future -import scala.concurrent.duration.* +import scala.concurrent.duration._ import scala.io.Source -import scala.jdk.CollectionConverters.* +import scala.jdk.CollectionConverters._ import org.mongodb.scala.SingleObservableFuture import play.api.libs.ws.JsonBodyWritables.writeableOf_JsValue @@ -64,8 +64,9 @@ trait FileStoreHelpers extends WiremockFeatureTestServer { httpClientV2.delete(url"$serviceUrl/file").execute[HttpResponse] def deleteFile(id: String)(implicit hc: HeaderCarrier): Future[HttpResponse] = { - stubObjectStoreDeleteOne(id) - httpClientV2.delete(url"$serviceUrl/file?id=$id").execute[HttpResponse] + stubS3DeleteOne(id) + + httpClientV2.delete(url"$serviceUrl/file/$id").execute[HttpResponse] } def getFiles(queryParams: Seq[(String, String)])(implicit hc: HeaderCarrier): Future[HttpResponse] = { @@ -77,7 +78,7 @@ trait FileStoreHelpers extends WiremockFeatureTestServer { } def publishSafeFile(id: String)(implicit hc: HeaderCarrier): Future[HttpResponse] = { - stubObjectStoreSign(id) + stubS3Upload(id) httpClientV2 .post(url"$serviceUrl/file/$id/publish") @@ -85,7 +86,7 @@ trait FileStoreHelpers extends WiremockFeatureTestServer { .execute[HttpResponse] } - // Should NOT call Object Store Upload + // Should NOT call S3 Upload def publishUnsafeFile(id: String)(implicit hc: HeaderCarrier): Future[HttpResponse] = httpClientV2 .post(url"$serviceUrl/file/$id/publish") @@ -125,8 +126,6 @@ trait FileStoreHelpers extends WiremockFeatureTestServer { ): Future[HttpResponse] = { stubUpscanInitiate stubUpscanUpload - stubObjectStoreUpload(id.get) - stubObjectStoreSign(id.get) val tempFile = SingletonTemporaryFileCreator.create(filename) Files.write(tempFile.path, List("foo").asJava) @@ -175,25 +174,16 @@ trait FileStoreHelpers extends WiremockFeatureTestServer { ) ) - def stubObjectStoreUpload(id: String): StubMapping = - stubFor( - post("/object-store/ops/upload-from-url") - .willReturn( - aResponse() - .withStatus(Status.OK) - ) - ) - - def stubObjectStoreSign(id: String): StubMapping = + def stubS3Upload(id: String): StubMapping = stubFor( - post("/object-store/ops/presigned-url") + put(s"/digital-tariffs-local/$id") .willReturn( aResponse() .withStatus(Status.OK) ) ) - def stubObjectStoreListAll(): StubMapping = + def stubS3ListAll(): StubMapping = stubFor( get("/digital-tariffs-local/?encoding-type=url") .willReturn( @@ -203,18 +193,19 @@ trait FileStoreHelpers extends WiremockFeatureTestServer { ) ) - def stubObjectStoreDeleteAll(): StubMapping = + def stubS3DeleteAll(): StubMapping = stubFor( post(s"/digital-tariffs-local/?delete") .willReturn( aResponse() .withStatus(Status.OK) + .withBody(fromFile("aws/delete-objects_response.xml")) ) ) - def stubObjectStoreDeleteOne(id: String): StubMapping = + def stubS3DeleteOne(id: String): StubMapping = stubFor( - delete(s"/digital-tariffs-local/file?id=$id") + delete(s"/digital-tariffs-local/$id") .willReturn( aResponse() .withStatus(Status.OK) diff --git a/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreSpec.scala b/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreSpec.scala index d3e2011..a22f329 100644 --- a/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreSpec.scala +++ b/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreSpec.scala @@ -16,17 +16,17 @@ package uk.gov.hmrc.bindingtarifffilestore -import org.scalatest.concurrent.Eventually import play.api.Application import play.api.http.Status import play.api.inject.bind import play.api.inject.guice.GuiceApplicationBuilder import play.api.libs.json.* -import uk.gov.hmrc.bindingtarifffilestore.model.* import uk.gov.hmrc.bindingtarifffilestore.model.ScanStatus.{FAILED, READY} +import uk.gov.hmrc.bindingtarifffilestore.model.* import uk.gov.hmrc.bindingtarifffilestore.model.upscan.v2.{FileStoreInitiateResponse, UpscanFormTemplate} import uk.gov.hmrc.bindingtarifffilestore.repository.FileMetadataMongoRepository import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse} +import org.scalatest.concurrent.Eventually import java.io.File import java.net.URI @@ -44,7 +44,7 @@ class FileStoreSpec extends FileStoreHelpers with Eventually { override def fakeApplication(): Application = new GuiceApplicationBuilder() .configure( - "microservice.services.object-store.port" -> s"$wirePort", + "s3.endpoint" -> s"http://localhost:$wirePort", "microservice.services.upscan-initiate.port" -> s"$wirePort" ) .overrides(bind[FileMetadataMongoRepository].toInstance(repository)) @@ -73,8 +73,7 @@ class FileStoreSpec extends FileStoreHelpers with Eventually { dbFileStoreSize.shouldBe(1) When("I request the file details") - - val deleteResult = Await.result(deleteFile(file1), 7.seconds) + val deleteResult = Await.result(deleteFile(id1), 7.seconds) Then("The response code should be Ok") @@ -98,8 +97,8 @@ class FileStoreSpec extends FileStoreHelpers with Eventually { await(upload(Some(id2), file2, contentType, publishable = true)) dbFileStoreSize shouldBe 2 - stubObjectStoreListAll() - stubObjectStoreDeleteAll() + stubS3ListAll() + stubS3DeleteAll() When("I delete all documents") val deleteResult = await(deleteFiles()) @@ -243,7 +242,8 @@ class FileStoreSpec extends FileStoreHelpers with Eventually { await(upload(Some(id2), file2, contentType, publishable = false)) dbFileStoreSize shouldBe 2 - stubObjectStoreListAll() + stubS3ListAll() + stubS3DeleteAll() When("I request the file details") @@ -333,12 +333,12 @@ class FileStoreSpec extends FileStoreHelpers with Eventually { Given("A File has been uploaded") - stubObjectStoreSign(id1) + stubS3Upload(id1) await(upload(Some(id1), file1, contentType, publishable = true)) dbFileStoreSize shouldBe 1 - stubObjectStoreListAll() - stubObjectStoreDeleteAll() + stubS3ListAll() + stubS3DeleteAll() When("Notify is Called") @@ -412,6 +412,9 @@ class FileStoreSpec extends FileStoreHelpers with Eventually { And("The response shows the file published") (result.json \\ "url").map(_.as[String]).headOption.getOrElse("") should include(s"$id1") + (result.json \\ "url").map(_.as[String]).headOption.getOrElse("") should include( + "X-Amz-Algorithm=AWS4-HMAC-SHA256" + ) } Scenario("Should mark an un-safe file as publishable, but not persist") { diff --git a/test/uk/gov/hmrc/bindingtarifffilestore/config/AppConfigSpec.scala b/test/uk/gov/hmrc/bindingtarifffilestore/config/AppConfigSpec.scala index 7e6a8b0..e7886c9 100644 --- a/test/uk/gov/hmrc/bindingtarifffilestore/config/AppConfigSpec.scala +++ b/test/uk/gov/hmrc/bindingtarifffilestore/config/AppConfigSpec.scala @@ -42,6 +42,16 @@ class AppConfigSpec extends UnitSpec with WithFakeApplication with BeforeAndAfte new AppConfig(Configuration.from(config), serviceConfig).fileStoreSizeConfiguration } + private def s3ConfigWith(pairs: (String, String)*): S3Configuration = { + var config = Map( + "s3.region" -> "", + "s3.bucket" -> "", + "s3.endpoint" -> "" + ) + pairs.foreach(e => config = config + e) + new AppConfig(Configuration.from(config), serviceConfig).s3Configuration + } + private def upscanConfigWith(host: String, port: String, pairs: (String, String)*): AppConfig = { when(serviceConfig.baseUrl(refEq("upscan-initiate"))).thenReturn(s"http://$host:$port") new AppConfig(Configuration.from(pairs.map(e => e._1 -> e._2).toMap), serviceConfig) @@ -52,8 +62,30 @@ class AppConfigSpec extends UnitSpec with WithFakeApplication with BeforeAndAfte "Config" should { + "return AWS S3 region" in { + s3ConfigWith("s3.region" -> "region").region shouldBe "region" + } + "return AWS S3 bucket" in { - configWith("s3.bucket" -> "bucket").s3bucket shouldBe "bucket" + s3ConfigWith("s3.bucket" -> "bucket").bucket shouldBe "bucket" + } + + "return AWS S3 endpoint" in { + s3ConfigWith("s3.endpoint" -> "endpoint").endpoint shouldBe Some("endpoint") + } + + "return AWS S3 blank endpoint as None" in { + s3ConfigWith("s3.endpoint" -> "").endpoint shouldBe None + } + + "return AWS S3 base URL" in { + s3ConfigWith("s3.endpoint" -> "endpoint").baseUrl shouldBe "endpoint" + } + + "return AWS S3 default base URL" in { + s3ConfigWith( + "s3.region" -> "region" + ).baseUrl shouldBe "https://s3-region.amazonaws.com" } "return application Host" in { diff --git a/test/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3ConnectorSpec.scala b/test/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3ConnectorSpec.scala new file mode 100644 index 0000000..5bc6ab9 --- /dev/null +++ b/test/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3ConnectorSpec.scala @@ -0,0 +1,227 @@ +/* + * Copyright 2025 HM Revenue & Customs + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package uk.gov.hmrc.bindingtarifffilestore.connector + +import com.amazonaws.services.s3.model.AmazonS3Exception +import com.github.tomakehurst.wiremock.client.WireMock +import com.github.tomakehurst.wiremock.client.WireMock.* +import org.mockito.Mockito.{mock, when} +import org.scalatest.BeforeAndAfterEach +import play.api.http.Status +import play.api.libs.Files.SingletonTemporaryFileCreator +import uk.gov.hmrc.bindingtarifffilestore.config.{AppConfig, S3Configuration} +import uk.gov.hmrc.bindingtarifffilestore.model.FileMetadata +import uk.gov.hmrc.bindingtarifffilestore.util.* + +import java.time.LocalDate +import java.time.format.DateTimeFormatter + +class AmazonS3ConnectorSpec extends UnitSpec with WiremockTestServer with BeforeAndAfterEach with ResourceFiles { + + private val s3Config: S3Configuration = + S3Configuration("region", "bucket", Some(s"http://localhost:$wirePort")) + private val config = mock(classOf[AppConfig]) + private val date = LocalDate.now().format(DateTimeFormatter.ofPattern("YYYYMMdd")) + private val connector = new AmazonS3Connector(config) + + override protected def beforeEach(): Unit = { + super.beforeEach() + when(config.s3Configuration).thenReturn(s3Config) + } + + "Get All" should { + + "Delegate to S3" in { + stubFor( + get("/bucket/?encoding-type=url") + .withHeader( + "Authorization", + matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") + ) + .willReturn( + aResponse() + .withStatus(Status.OK) + .withBody(fromFile("aws/list-objects_response.xml")) + ) + ) + + // When + val all: Seq[String] = connector.getAll + + // Then + all should have size 1 + all.head shouldBe "image.jpg" + } + + } + + "Upload" should { + + "Delegate to S3" in { + stubFor( + put("/bucket/id") + .withHeader( + "Authorization", + matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") + ) + .withHeader("Content-Type", equalTo("text/plain")) + .willReturn( + aResponse() + .withStatus(Status.OK) + ) + ) + + val url = SingletonTemporaryFileCreator.create("example.txt").path.toUri.toURL.toString + val fileUploading = FileMetadata("id", Some("file.txt"), Some("text/plain"), Some(url)) + + // Then + val result = connector.upload(fileUploading) + result.id shouldBe "id" + result.fileName shouldBe Some("file.txt") + result.mimeType shouldBe Some("text/plain") + result.url.get shouldBe s"$wireMockUrl/bucket/id" + } + + "Throw Exception on missing URL" in { + val fileUploading = FileMetadata("id", Some("file.txt"), Some("text/plain")) + + val exception = intercept[IllegalArgumentException] { + connector.upload(fileUploading) + } + exception.getMessage shouldBe "Missing URL" + } + + "Throw Exception on upload failure" in { + stubFor( + put("/bucket/id") + .withHeader( + "Authorization", + matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") + ) + .withHeader("Content-Type", equalTo("text/plain")) + .willReturn( + aResponse() + .withStatus(Status.BAD_GATEWAY) + ) + ) + val url = SingletonTemporaryFileCreator.create("example.txt").path.toUri.toURL.toString + val fileUploading = FileMetadata("id", Some("file.txt"), Some("text/plain"), Some(url)) + + // Then + val exception = intercept[AmazonS3Exception] { + connector.upload(fileUploading) + } + exception.getMessage shouldBe + """Bad Gateway (Service: Amazon S3; + | Status Code: 502; + | Error Code: 502 Bad Gateway; + | Request ID: null; + | S3 Extended Request ID: null; + | Proxy: null)""".stripMargin.replaceAll("\n", "") + } + } + + "Sign" should { + "append token to URL" in { + val file = FileMetadata("id", Some("file.txt"), Some("text/plain"), Some("url")) + + connector.sign(file).url.get should startWith(s"$wireMockUrl/bucket/id?") + connector.sign(file).url.get should include("X-Amz-Algorithm=AWS4-HMAC-SHA256") + } + + "not append token to empty URL" in { + val file = FileMetadata("id", Some("file.txt"), Some("text/plain"), None) + + connector.sign(file).url shouldBe None + } + } + + "Delete All" should { + "Delegate to S3" in { + stubFor( + get("/bucket/?encoding-type=url") + .withHeader( + "Authorization", + matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") + ) + .willReturn( + aResponse() + .withStatus(Status.OK) + .withBody(fromFile("aws/list-objects_response.xml")) + ) + ) + stubFor( + post("/bucket/?delete") + .withHeader( + "Authorization", + matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") + ) + .willReturn( + aResponse() + .withStatus(Status.OK) + .withBody(fromFile("aws/delete-objects_response.xml")) + ) + ) + + connector.deleteAll() + + WireMock.verify( + postRequestedFor(urlEqualTo("/bucket/?delete")) + ) + } + + "Do nothing for no files" in { + stubFor( + get("/bucket/?encoding-type=url") + .withHeader( + "Authorization", + matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") + ) + .willReturn( + aResponse() + .withStatus(Status.OK) + .withBody(fromFile("aws/empty-list-objects_response.xml")) + ) + ) + + connector.deleteAll() + + WireMock.verify(0, postRequestedFor(urlEqualTo("/bucket/?delete"))) + } + } + + "Delete One" should { + "Delegate to S3" in { + stubFor( + delete("/bucket/id") + .withHeader( + "Authorization", + matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") + ) + .willReturn( + aResponse() + .withStatus(Status.OK) + ) + ) + + connector.delete("id") + + WireMock.verify(deleteRequestedFor(urlEqualTo("/bucket/id"))) + } + } + +} diff --git a/test/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnectorSpec.scala b/test/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnectorSpec.scala deleted file mode 100644 index c81585d..0000000 --- a/test/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnectorSpec.scala +++ /dev/null @@ -1,182 +0,0 @@ -/* - * Copyright 2025 HM Revenue & Customs - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package uk.gov.hmrc.bindingtarifffilestore.connector - -import org.apache.pekko.stream.Materializer -import org.mockito.Mockito.mock -import org.scalatest.BeforeAndAfterEach -import org.scalatest.time.SpanSugar.convertIntToGrainOfTime -import play.api.inject.guice.GuiceApplicationBuilder -import play.api.{Application, inject} -import uk.gov.hmrc.bindingtarifffilestore.config.AppConfig -import uk.gov.hmrc.bindingtarifffilestore.model.FileMetadata -import uk.gov.hmrc.bindingtarifffilestore.util.* -import uk.gov.hmrc.http.HeaderCarrier -import uk.gov.hmrc.objectstore.client.Path.{Directory, File} -import uk.gov.hmrc.objectstore.client.config.ObjectStoreClientConfig -import uk.gov.hmrc.objectstore.client.play.Implicits.{futureMonad, stringWrite} -import uk.gov.hmrc.objectstore.client.play.PlayObjectStoreClient -import uk.gov.hmrc.objectstore.client.play.test.stub.StubPlayObjectStoreClient -import uk.gov.hmrc.objectstore.client.{ObjectSummary, Path, RetentionPeriod} - -import java.time.{Instant, LocalDate, LocalTime, ZoneId} -import java.util.UUID.randomUUID -import scala.concurrent.Await -import scala.concurrent.ExecutionContext.Implicits.global - -class ObjectStoreConnectorSpec - extends UnitSpec - with WithFakeApplication - with WiremockTestServer - with BeforeAndAfterEach - with ResourceFiles { - - implicit lazy val hc: HeaderCarrier = mock(classOf[HeaderCarrier]) - private implicit val mat: Materializer = mock(classOf[Materializer]) - private val directory: Path.Directory = - Path.Directory("digital-tariffs-local") - - private val file1 = FileMetadata("id1", Some("file1.txt"), Some("text/plain"), Some("http://foo.bar/test-123.txt")) - private val file2 = FileMetadata("id2", Some("file2.txt"), Some("text/plain"), Some("http://foo.bar/test-456.txt")) - - lazy val objectStoreClientStub: StubPlayObjectStoreClient = { - val baseUrl = s"baseUrl-${randomUUID().toString}" - val owner = "binding-tariff-filestore" - val token = s"token-${randomUUID().toString}" - val config = ObjectStoreClientConfig(baseUrl, owner, token, RetentionPeriod.OneWeek) - - new StubPlayObjectStoreClient(config) - } - - override lazy val fakeApplication: Application = new GuiceApplicationBuilder() - .configure( - "microservice.services.object-store.port" -> s"$wirePort" - ) - .bindings(inject.bind(classOf[PlayObjectStoreClient]).to(objectStoreClientStub)) - .build() - - private implicit val config: AppConfig = fakeApplication.injector.instanceOf[AppConfig] - - private val connector = new ObjectStoreConnector(objectStoreClientStub, config) - - val zonedDateTime: Instant = - LocalDate.of(2025, 1, 1).atTime(LocalTime.of(12, 30)).atZone(ZoneId.of("Europe/Paris")).toInstant - - override protected def beforeEach(): Unit = - super.beforeEach() - - "Get All" should { - "list files when no files exist" in { - val all = await(connector.getAll(directory)) - - all shouldBe List.empty[ObjectSummary] - } - - "when files have been added, list files that have been stored" in { - await(objectStoreClientStub.putObject(directory.file(file1.fileName.get), "text", RetentionPeriod.OneDay)) - val all = await(connector.getAll(directory)) - - all.size shouldBe 1 - } - } - - "Upload" should { - - "add file to the object store" in { - - await(connector.upload(file1)) - - val file = await(connector.getAll(directory)) - - file.length shouldBe 1 - file.head.location shouldBe File(Directory("binding-tariff-filestore/digital-tariffs-local"), "file1.txt") - } - - "Throw Exception on missing URL" in { - val fileUploading = FileMetadata("id", Some("file.txt"), Some("text/plain")) - val exception = intercept[IllegalArgumentException] { - connector.upload(fileUploading) - } - - exception.getMessage shouldBe "Missing URL" - } - } - - "Sign" should { - "create a presigned download URL" in { - - await(objectStoreClientStub.putObject(directory.file(file1.fileName.get), "text", RetentionPeriod.OneDay)) - - val result = await(connector.sign(file1)) - - result shouldBe file1 - result.url shouldBe Some("http://foo.bar/test-123.txt") - } - - "not create a presigned download url if there is an empty URL" in { - val file = FileMetadata("id", Some("file.txt"), Some("text/plain"), None) - val result = Await.result(connector.sign(file), 5.seconds) - - result.url shouldBe None - } - } - - "Delete One" should { - "delete file from object store" in { - await( - objectStoreClientStub.putObject(directory.file(file1.fileName.get), file1.mimeType.get, RetentionPeriod.OneDay) - ) - - val result: Unit = await( - connector - .delete(file1.fileName.get) - ) - - val files = await(connector.getAll(directory)) - - result === () - files.size shouldBe 0 - } - } - - "Delete All" should { - "Do nothing for no files" in { - await(connector.deleteAll(), 5.seconds) - - val files = await(connector.getAll(directory)) - - files.size shouldBe 0 - - } - - "delete all files from object store if present" in { - await( - objectStoreClientStub.putObject(directory.file(file1.fileName.get), file1.mimeType.get, RetentionPeriod.OneDay) - ) - await( - objectStoreClientStub.putObject(directory.file(file2.fileName.get), file2.mimeType.get, RetentionPeriod.OneDay) - ) - await(connector.deleteAll()) - - val all = await(connector.getAll(directory)) - - all.size shouldBe 0 - - } - } - -} diff --git a/test/uk/gov/hmrc/bindingtarifffilestore/connector/UpscanConnectorSpec.scala b/test/uk/gov/hmrc/bindingtarifffilestore/connector/UpscanConnectorSpec.scala index 2161b0d..a5b90c5 100644 --- a/test/uk/gov/hmrc/bindingtarifffilestore/connector/UpscanConnectorSpec.scala +++ b/test/uk/gov/hmrc/bindingtarifffilestore/connector/UpscanConnectorSpec.scala @@ -41,7 +41,7 @@ class UpscanConnectorSpec private val config: AppConfig = mock(classOf[AppConfig]) val httpClient: HttpClientV2 = fakeApplication.injector.instanceOf[HttpClientV2] - implicit lazy val headers: HeaderCarrier = HeaderCarrier() + private implicit val headers: HeaderCarrier = HeaderCarrier() private val connector: UpscanConnector = new UpscanConnector(config, httpClient) @@ -134,7 +134,7 @@ class UpscanConnectorSpec (None, Some("text/plain")), (Some("file.txt"), None), (None, None) - ).foreach(args => test.tupled(args)) + ).foreach(args => (test _).tupled(args)) } "Upload with error handling if 502 BAD_GATEWAY is returned" in { diff --git a/test/uk/gov/hmrc/bindingtarifffilestore/controllers/FileStoreControllerSpec.scala b/test/uk/gov/hmrc/bindingtarifffilestore/controllers/FileStoreControllerSpec.scala index cd47303..ed78697 100644 --- a/test/uk/gov/hmrc/bindingtarifffilestore/controllers/FileStoreControllerSpec.scala +++ b/test/uk/gov/hmrc/bindingtarifffilestore/controllers/FileStoreControllerSpec.scala @@ -19,44 +19,42 @@ package uk.gov.hmrc.bindingtarifffilestore.controllers import com.mongodb.{MongoWriteException, ServerAddress, WriteError} import org.apache.pekko.stream.Materializer import org.mockito.ArgumentCaptor -import org.mockito.ArgumentMatchers.{any, eq as eqTo, refEq} -import org.mockito.Mockito.* +import org.mockito.ArgumentMatchers.{any, refEq} +import org.mockito.Mockito._ import org.mongodb.scala.bson.BsonDocument import org.scalatest.BeforeAndAfterEach import org.scalatest.matchers.should.Matchers -import play.api.http.Status.* +import play.api.http.Status._ import play.api.libs.Files.{SingletonTemporaryFileCreator, TemporaryFile} import play.api.libs.json.{JsValue, Json, Writes} -import play.api.mvc.* import play.api.mvc.MultipartFormData.FilePart +import play.api.mvc._ import play.api.test.FakeRequest import uk.gov.hmrc.bindingtarifffilestore.config.AppConfig -import uk.gov.hmrc.bindingtarifffilestore.model.* import uk.gov.hmrc.bindingtarifffilestore.model.FileMetadataREST.format +import uk.gov.hmrc.bindingtarifffilestore.model._ import uk.gov.hmrc.bindingtarifffilestore.model.upscan.v2.{FileStoreInitiateRequest, FileStoreInitiateResponse, UpscanFormTemplate} import uk.gov.hmrc.bindingtarifffilestore.model.upscan.{ScanResult, SuccessfulScanResult, UploadDetails} import uk.gov.hmrc.bindingtarifffilestore.service.FileStoreService -import uk.gov.hmrc.bindingtarifffilestore.util.* +import uk.gov.hmrc.bindingtarifffilestore.util._ import uk.gov.hmrc.http.HeaderCarrier import java.time.Instant import java.util.Collections import scala.concurrent.ExecutionContext.Implicits.global -import scala.concurrent.Future -import scala.concurrent.Future.successful +import scala.concurrent.Future.{failed, successful} class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplication with BeforeAndAfterEach { - implicit lazy val mat: Materializer = mock(classOf[Materializer]) - implicit lazy val hc: HeaderCarrier = mock(classOf[HeaderCarrier]) + private implicit val mat: Materializer = fakeApplication.materializer + private val appConfig: AppConfig = mock(classOf[AppConfig]) private val service: FileStoreService = mock(classOf[FileStoreService]) - private lazy val playBodyParsers: PlayBodyParsers = mock(classOf[PlayBodyParsers]) + private lazy val playBodyParsers: PlayBodyParsers = fakeApplication.injector.instanceOf[PlayBodyParsers] lazy val cc: MessagesControllerComponents = fakeApplication.injector.instanceOf[MessagesControllerComponents] private val controller: FileStoreController = new FileStoreController(appConfig, service, playBodyParsers, cc) - private val fakeRequest: FakeRequest[AnyContentAsEmpty.type] = - FakeRequest() + private val fakeRequest: FakeRequest[AnyContentAsEmpty.type] = FakeRequest() private def jsonRequest[T](body: T)(implicit writes: Writes[T]): Request[AnyContent] = fakeRequest @@ -68,67 +66,6 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic reset(service) } - "Get By ID" should { - "return 200 when found" in { - val id = "id" - val attachment = FileMetadata(id = id, fileName = Some("file"), mimeType = Some("type")) - - when(appConfig.isTestMode).thenReturn(true) - when(service.find(eqTo(id))(any[HeaderCarrier])).thenReturn(Future.successful(Some(attachment))) - - val result = await(controller.get(id)(fakeRequest)) - - status(result) shouldBe OK - bodyOf(result) shouldEqual Json.toJson(attachment).toString() - } - - "return 404 when not found" in { - when(service.find(eqTo("id"))(any[HeaderCarrier])).thenReturn(successful(None)) - - val result = await(controller.get("id")(fakeRequest)) - - status(result) shouldBe NOT_FOUND - } - } - - "Get By Search" should { - "return 200 with empty array" in { - when(service.find(eqTo(Search(ids = Some(Set.empty))), eqTo(Pagination.max))(any[HeaderCarrier])) - .thenReturn(successful(Paged.empty[FileMetadata])) - - val result = await(controller.getAll(Search(ids = Some(Set.empty)), None)(fakeRequest)) - - status(result) shouldBe OK - bodyOf(result) shouldEqual Json.toJson(Seq.empty).toString() - } - - "return 200 with non empty array" in { - val attachment1 = FileMetadata(id = "id1", fileName = Some("file1"), mimeType = Some("type1")) - val attachment2 = FileMetadata(id = "id2", fileName = Some("file2"), mimeType = Some("type2")) - - when(service.find(eqTo(Search(ids = Some(Set("id1", "id2")))), eqTo(Pagination.max))(any[HeaderCarrier])) - .thenReturn(successful(Paged(Seq(attachment1, attachment2)))) - - val result = await(controller.getAll(Search(ids = Some(Set("id1", "id2"))), None)(fakeRequest)) - - status(result) shouldBe OK - bodyOf(result) shouldEqual Json.toJson(Seq(attachment1, attachment2)).toString() - } - - "return 200 with pagination and non empty pager" in { - val attachment1 = FileMetadata(id = "id1", fileName = Some("file1"), mimeType = Some("type1")) - val attachment2 = FileMetadata(id = "id2", fileName = Some("file2"), mimeType = Some("type2")) - - when(service.find(eqTo(Search(ids = Some(Set("id1", "id2")))), eqTo(Pagination()))(any[HeaderCarrier])) - .thenReturn(successful(Paged(Seq(attachment1, attachment2)))) - - val result = await(controller.getAll(Search(ids = Some(Set("id1", "id2"))), Some(Pagination()))(fakeRequest)) - - status(result) shouldBe OK - bodyOf(result) shouldEqual Json.toJson(Paged(Seq(attachment1, attachment2))).toString() - } - } - "Delete All" should { val req = FakeRequest(method = "DELETE", path = "/file") @@ -145,11 +82,8 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic } "return 204 if the test mode is enabled" in { - val attachment = FileMetadata(id = "id", fileName = Some("file"), mimeType = Some("type")) - when(appConfig.isTestMode).thenReturn(true) - when(service.deleteAll()(any[HeaderCarrier])).thenReturn(successful(())) - when(service.find(eqTo("id"))(any[HeaderCarrier])).thenReturn(successful(Some(attachment))) + when(service.deleteAll()).thenReturn(successful(())) val result = await(controller.deleteAll()(req)) @@ -160,7 +94,7 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic val error = new RuntimeException when(appConfig.isTestMode).thenReturn(true) - when(service.deleteAll()(any[HeaderCarrier])).thenReturn(Future.failed(error)) + when(service.deleteAll()).thenReturn(failed(error)) val result = await(controller.deleteAll()(req)) @@ -172,15 +106,14 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic "Delete By ID" should { - val attachment = FileMetadata(id = "id", fileName = Some("file"), mimeType = Some("type")) - val req = FakeRequest(method = "DELETE", path = s"/file/${attachment.id}") + val id = "ABC-123_000" + val req = FakeRequest(method = "DELETE", path = s"/file/$id") "return 204" in { when(appConfig.isTestMode).thenReturn(true) - when(service.delete(eqTo(attachment.id), eqTo(attachment.fileName.get))(any[HeaderCarrier])) - .thenReturn(Future.successful((): Unit)) + when(service.delete(id)).thenReturn(successful((): Unit)) - val result = await(controller.delete(attachment.id, attachment.fileName.get)(req)) + val result = await(controller.delete(id)(req)) status(result) shouldBe NO_CONTENT } @@ -189,10 +122,9 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic val error = new RuntimeException when(appConfig.isTestMode).thenReturn(true) - when(service.delete(eqTo(attachment.id), eqTo(attachment.fileName.get))(any[HeaderCarrier])) - .thenReturn(Future.failed(error)) + when(service.delete(id)).thenReturn(failed(error)) - val result = await(controller.delete(attachment.id, attachment.fileName.get)(req)) + val result = await(controller.delete(id)(req)) status(result) shouldEqual INTERNAL_SERVER_ERROR jsonBodyOf(result).toString() shouldEqual """{"code":"UNKNOWN_ERROR","message":"An unexpected error occurred"}""" @@ -200,6 +132,64 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic } + "Get By ID" should { + "return 200 when found" in { + val attachment = FileMetadata(id = "id", fileName = Some("file"), mimeType = Some("type")) + when(service.find(id = "id")).thenReturn(successful(Some(attachment))) + + val result = await(controller.get("id")(fakeRequest)) + + status(result) shouldBe OK + bodyOf(result) shouldEqual Json.toJson(attachment).toString() + } + + "return 404 when not found" in { + when(service.find(id = "id")).thenReturn(successful(None)) + + val result = await(controller.get("id")(fakeRequest)) + + status(result) shouldBe NOT_FOUND + } + } + + "Get By Search" should { + "return 200 with empty array" in { + when(service.find(Search(ids = Some(Set.empty)), Pagination.max)) + .thenReturn(successful(Paged.empty[FileMetadata])) + + val result = await(controller.getAll(Search(ids = Some(Set.empty)), None)(fakeRequest)) + + status(result) shouldBe OK + bodyOf(result) shouldEqual Json.toJson(Seq.empty).toString() + } + + "return 200 with non empty array" in { + val attachment1 = FileMetadata(id = "id1", fileName = Some("file1"), mimeType = Some("type1")) + val attachment2 = FileMetadata(id = "id2", fileName = Some("file2"), mimeType = Some("type2")) + + when(service.find(Search(ids = Some(Set("id1", "id2"))), Pagination.max)) + .thenReturn(successful(Paged(Seq(attachment1, attachment2)))) + + val result = await(controller.getAll(Search(ids = Some(Set("id1", "id2"))), None)(fakeRequest)) + + status(result) shouldBe OK + bodyOf(result) shouldEqual Json.toJson(Seq(attachment1, attachment2)).toString() + } + + "return 200 with pagination and non empty pager" in { + val attachment1 = FileMetadata(id = "id1", fileName = Some("file1"), mimeType = Some("type1")) + val attachment2 = FileMetadata(id = "id2", fileName = Some("file2"), mimeType = Some("type2")) + + when(service.find(Search(ids = Some(Set("id1", "id2"))), Pagination())) + .thenReturn(successful(Paged(Seq(attachment1, attachment2)))) + + val result = await(controller.getAll(Search(ids = Some(Set("id1", "id2"))), Some(Pagination()))(fakeRequest)) + + status(result) shouldBe OK + bodyOf(result) shouldEqual Json.toJson(Paged(Seq(attachment1, attachment2))).toString() + } + } + "Notify" should { "return 201 when found" in { val scanResult = SuccessfulScanResult( @@ -210,7 +200,7 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic val attachment = FileMetadata(id = "id", fileName = Some("file"), mimeType = Some("type")) val attachmentUpdated = FileMetadata(id = "id", fileName = Some("file"), mimeType = Some("type"), url = Some("url")) - when(service.find(eqTo("id"))(any[HeaderCarrier])).thenReturn(successful(Some(attachment))) + when(service.find(id = "id")).thenReturn(successful(Some(attachment))) when(service.notify(refEq(attachment), refEq(scanResult))(any[HeaderCarrier])) .thenReturn(successful(Some(attachmentUpdated))) @@ -227,7 +217,7 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic downloadUrl = "url", uploadDetails = UploadDetails("file", "type", Instant.now(), "checksum") ) - when(service.find(eqTo("id"))(any[HeaderCarrier])).thenReturn(successful(None)) + when(service.find("id")).thenReturn(successful(None)) val request: FakeRequest[JsValue] = fakeRequest.withBody(Json.toJson[ScanResult](scanResult)) val result: Result = await(controller.notification(id = "id")(request)) @@ -245,7 +235,7 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic ) val attachment: FileMetadata = FileMetadata(id = "id", fileName = Some("file"), mimeType = Some("type")) - when(service.find(eqTo("id"))(any[HeaderCarrier])).thenReturn(successful(Some(attachment))) + when(service.find(id = "id")).thenReturn(successful(Some(attachment))) when(service.notify(refEq(attachment), refEq(scanResult))(any[HeaderCarrier])) .thenThrow( new MongoWriteException( @@ -288,7 +278,7 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic scanStatus = Some(ScanStatus.READY), url = Some("url") ) - when(service.find(eqTo("id"))(any[HeaderCarrier])).thenReturn(successful(Some(attachmentExisting))) + when(service.find(id = "id")).thenReturn(successful(Some(attachmentExisting))) when(service.publish(refEq(attachmentExisting))(any[HeaderCarrier])) .thenReturn(successful(Some(attachmentUpdated))) @@ -299,7 +289,7 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic } "return 404 when not found" in { - when(service.find(eqTo("id"))(any[HeaderCarrier])).thenReturn(successful(None)) + when(service.find(id = "id")).thenReturn(successful(None)) val result: Result = await(controller.publish(id = "id")(fakeRequest)) @@ -309,9 +299,8 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic "return 404 when publish returns not found" in { val attachmentExisting = FileMetadata(id = "id", fileName = Some("file"), mimeType = Some("type"), scanStatus = Some(ScanStatus.READY)) - - when(service.find(eqTo("id"))(any[HeaderCarrier])).thenReturn(successful(Some(attachmentExisting))) - when(service.publish(refEq(attachmentExisting))(any[HeaderCarrier])).thenReturn(Future.successful(None)) + when(service.find(id = "id")).thenReturn(successful(Some(attachmentExisting))) + when(service.publish(refEq(attachmentExisting))(any[HeaderCarrier])).thenReturn(successful(None)) val result: Result = await(controller.publish(id = "id")(fakeRequest)) diff --git a/test/uk/gov/hmrc/bindingtarifffilestore/service/FileStoreServiceSpec.scala b/test/uk/gov/hmrc/bindingtarifffilestore/service/FileStoreServiceSpec.scala index 04f2c3c..eadfc45 100644 --- a/test/uk/gov/hmrc/bindingtarifffilestore/service/FileStoreServiceSpec.scala +++ b/test/uk/gov/hmrc/bindingtarifffilestore/service/FileStoreServiceSpec.scala @@ -18,35 +18,36 @@ package uk.gov.hmrc.bindingtarifffilestore.service import org.mockito.ArgumentCaptor import org.mockito.ArgumentMatchers.any -import org.mockito.Mockito.* +import org.mockito.Mockito._ import org.scalatest.BeforeAndAfterEach import org.scalatest.concurrent.Eventually import play.api.libs.Files.TemporaryFile import uk.gov.hmrc.bindingtarifffilestore.audit.AuditService import uk.gov.hmrc.bindingtarifffilestore.config.{AppConfig, FileStoreSizeConfiguration} -import uk.gov.hmrc.bindingtarifffilestore.connector.{ObjectStoreConnector, UpscanConnector} -import uk.gov.hmrc.bindingtarifffilestore.model.* -import uk.gov.hmrc.bindingtarifffilestore.model.upscan.* +import uk.gov.hmrc.bindingtarifffilestore.connector.{AmazonS3Connector, UpscanConnector} +import uk.gov.hmrc.bindingtarifffilestore.model._ +import uk.gov.hmrc.bindingtarifffilestore.model.upscan._ import uk.gov.hmrc.bindingtarifffilestore.repository.FileMetadataMongoRepository import uk.gov.hmrc.bindingtarifffilestore.util.UnitSpec import uk.gov.hmrc.http.HeaderCarrier import java.time.Instant -import scala.concurrent.{ExecutionContext, Future} +import scala.concurrent.ExecutionContext import scala.concurrent.ExecutionContext.Implicits.global import scala.concurrent.Future.{failed, successful} class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventually { - private val config: AppConfig = mock(classOf[AppConfig]) - private val objectStoreConnector: ObjectStoreConnector = mock(classOf[ObjectStoreConnector]) - private val repository: FileMetadataMongoRepository = mock(classOf[FileMetadataMongoRepository]) - private val upscanConnector: UpscanConnector = mock(classOf[UpscanConnector]) - private val auditService: AuditService = mock(classOf[AuditService]) - private implicit lazy val hc: HeaderCarrier = mock(classOf[HeaderCarrier]) + private val config: AppConfig = mock(classOf[AppConfig]) + private val s3Connector: AmazonS3Connector = mock(classOf[AmazonS3Connector]) + private val repository: FileMetadataMongoRepository = mock(classOf[FileMetadataMongoRepository]) + private val upscanConnector: UpscanConnector = mock(classOf[UpscanConnector]) + private val auditService: AuditService = mock(classOf[AuditService]) + + private implicit val hc: HeaderCarrier = HeaderCarrier() private val service: FileStoreService = - new FileStoreService(config, objectStoreConnector, repository, upscanConnector, auditService) + new FileStoreService(config, s3Connector, repository, upscanConnector, auditService) private final val emulatedFailure: RuntimeException = new RuntimeException("Emulated failure.") @@ -55,7 +56,7 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua override protected def afterEach(): Unit = { super.afterEach() reset(config) - reset(objectStoreConnector) + reset(s3Connector) reset(repository) reset(upscanConnector) reset(auditService) @@ -69,7 +70,7 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua await(service.deleteAll()) shouldBe ((): Unit) verify(repository).deleteAll() - verify(objectStoreConnector).deleteAll() + verify(s3Connector).deleteAll() } "Propagate any error" in { @@ -87,10 +88,10 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua "Clear the Database & File Store" in { when(repository.delete("id")).thenReturn(successful(())) - await(service.delete("id", "fileName")) shouldBe ((): Unit) + await(service.delete("id")) shouldBe ((): Unit) verify(repository).delete("id") - verify(objectStoreConnector).delete("fileName") + verify(s3Connector).delete("id") } "Propagate any error" in { @@ -111,7 +112,7 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua when(attachment.published).thenReturn(true) when(repository.get("id")).thenReturn(successful(Some(attachment))) - when(objectStoreConnector.sign(attachment)).thenReturn(Future.successful(attachmentSigned)) + when(s3Connector.sign(attachment)).thenReturn(attachmentSigned) await(service.find("id")) shouldBe Some(attachmentSigned) } @@ -122,7 +123,7 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua await(service.find("id")) shouldBe Some(attachment) - verify(objectStoreConnector, never).sign(any[FileMetadata])(any[HeaderCarrier]) + verify(s3Connector, never).sign(any[FileMetadata]) } } @@ -141,7 +142,7 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua val attachment2 = mock(classOf[FileMetadata], "attachment2") when(attachment1.published).thenReturn(true) - when(objectStoreConnector.sign(attachment1)).thenReturn(successful(attSigned1)) + when(s3Connector.sign(attachment1)).thenReturn(attSigned1) when(attachment2.published).thenReturn(false) @@ -331,9 +332,9 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua when(attachmentUploadedUpdated.isLive).thenReturn(true) when(repository.update(attachmentUpdating)).thenReturn(successful(Some(attachmentUpdated))) - when(objectStoreConnector.upload(attachmentUpdated)).thenReturn(attachmentUploaded) + when(s3Connector.upload(attachmentUpdated)).thenReturn(attachmentUploaded) when(repository.update(attachmentUploadedUpdating)).thenReturn(successful(Some(attachmentUploadedUpdated))) - when(objectStoreConnector.sign(attachmentUploadedUpdated)).thenReturn(successful(attachmentSigned)) + when(s3Connector.sign(attachmentUploadedUpdated)).thenReturn(attachmentSigned) val test = await(service.notify(attachment, scanResult)) test shouldBe Some(attachmentSigned) @@ -368,8 +369,8 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua await(service.notify(attachment, scanResult)) shouldBe None - verify(objectStoreConnector, never()).upload(any[FileMetadata]())(any[HeaderCarrier]()) - verify(objectStoreConnector, never()).sign(any[FileMetadata]())(any[HeaderCarrier]()) + verify(s3Connector, never).upload(any[FileMetadata]) + verify(s3Connector, never).sign(any[FileMetadata]) verify(auditService, times(1)) .auditFileScanned(fileId = "id", fileName = Some("file"), upScanRef = "ref", upScanStatus = "READY") verify(auditService, never).auditFilePublished(fileId = "id", fileName = "file") @@ -414,9 +415,9 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua when(fileUpdated.published).thenReturn(true) - when(objectStoreConnector.upload(fileUploading)).thenReturn(fileUploaded) + when(s3Connector.upload(fileUploading)).thenReturn(fileUploaded) when(repository.update(any[FileMetadata])).thenReturn(successful(Some(fileUpdated))) - when(objectStoreConnector.sign(fileUpdated)).thenReturn(Future.successful(fileSigned)) + when(s3Connector.sign(fileUpdated)).thenReturn(fileSigned) await(service.publish(fileUploading)) shouldBe Some(fileSigned) @@ -437,7 +438,7 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua await(service.publish(fileUploading)) shouldBe None verify(repository).delete("id") - verifyNoInteractions(auditService, objectStoreConnector) + verifyNoInteractions(auditService, s3Connector) } "Not delegate to the File Store if pre published" in { @@ -448,7 +449,7 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua await(service.publish(fileUploading)) shouldBe Some(fileUploading) - verifyNoInteractions(auditService, objectStoreConnector, repository) + verifyNoInteractions(auditService, s3Connector, repository) } "Not delegate to the File Store if Scanned UnSafe" in { @@ -463,7 +464,7 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua await(service.publish(fileUploading)) shouldBe Some(fileUpdated) - verifyNoInteractions(auditService, objectStoreConnector) + verifyNoInteractions(auditService, s3Connector) } "Not delegate to the File Store if Unscanned" in { @@ -478,7 +479,7 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua await(service.publish(fileUploading)) shouldBe Some(fileUpdated) - verifyNoInteractions(auditService, objectStoreConnector) + verifyNoInteractions(auditService, s3Connector) } } diff --git a/test/uk/gov/hmrc/bindingtarifffilestore/util/WithFakeApplication.scala b/test/uk/gov/hmrc/bindingtarifffilestore/util/WithFakeApplication.scala index 5c75d93..5a591f8 100644 --- a/test/uk/gov/hmrc/bindingtarifffilestore/util/WithFakeApplication.scala +++ b/test/uk/gov/hmrc/bindingtarifffilestore/util/WithFakeApplication.scala @@ -17,7 +17,6 @@ package uk.gov.hmrc.bindingtarifffilestore.util import org.scalatest.{BeforeAndAfterAll, Suite} -import org.apache.pekko.stream.Materializer import play.api.inject.guice.{GuiceApplicationBuilder, GuiceableModule} import play.api.test.Helpers._ import play.api.{Application, Play} @@ -29,11 +28,7 @@ import play.api.{Application, Play} trait WithFakeApplication extends BeforeAndAfterAll { this: Suite => - final implicit val materializer: Materializer = fakeApplication.materializer - - lazy val fakeApplication: Application = new GuiceApplicationBuilder() - .bindings(bindModules*) - .build() + lazy val fakeApplication: Application = new GuiceApplicationBuilder().bindings(bindModules*).build() def bindModules: Seq[GuiceableModule] = Seq() From 72cbf8fa2772b1a2b624d4130fd0fe1e35488f98 Mon Sep 17 00:00:00 2001 From: SachaMcCormack <175815565+SachaMcCormack@users.noreply.github.com> Date: Thu, 30 Oct 2025 12:50:50 +0000 Subject: [PATCH 3/8] added internal-auth code --- .../bindingtarifffilestore/connector/ObjectStoreConnector.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala b/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala index ec994c2..bc0c630 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala @@ -92,7 +92,6 @@ class ObjectStoreConnector @Inject() (client: PlayObjectStoreClient, config: App exception.printStackTrace() Future.successful(fileMetaData) case scala.util.Success(presignedDownloadUrl) => - println("Successful Upload") val updatedMetaData = fileMetaData.copy(url = Some(presignedDownloadUrl.downloadUrl.toString)) Future.successful(updatedMetaData) } From 3476989c74b98feec2b825d34253cff144e03e81 Mon Sep 17 00:00:00 2001 From: SachaMcCormack <175815565+SachaMcCormack@users.noreply.github.com> Date: Thu, 30 Oct 2025 12:57:05 +0000 Subject: [PATCH 4/8] added internal-auth code --- .../hmrc/bindingtarifffilestore/config/AppConfig.scala | 8 ++------ .../connector/ObjectStoreConnector.scala | 1 - 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala b/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala index 7629097..aa2d2b6 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala @@ -28,18 +28,14 @@ class AppConfig @Inject() ( lazy val authorization: String = config.get[String]("auth.api-token") - lazy val s3Configuration: S3Configuration = S3Configuration( - config.get[String]("s3.region"), - config.get[String]("s3.bucket"), - Option(config.get[String]("s3.endpoint")).filter(_.nonEmpty) - ) - lazy val upscanInitiateUrl: String = servicesConfig.baseUrl("upscan-initiate") lazy val fileStoreSizeConfiguration: FileStoreSizeConfiguration = FileStoreSizeConfiguration( maxFileSize = config.get[Int]("upscan.maxFileSize"), minFileSize = config.get[Int]("upscan.minFileSize") ) + lazy val s3bucket: String = config.get[String]("s3.bucket") + lazy val filestoreUrl: String = config.get[String]("filestore.url") lazy val filestoreSSL: Boolean = config.get[Boolean]("filestore.ssl") diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala b/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala index bc0c630..f6fac6b 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala @@ -18,7 +18,6 @@ package uk.gov.hmrc.bindingtarifffilestore.connector import uk.gov.hmrc.objectstore.client.play.PlayObjectStoreClient import com.google.inject.Inject -import org.apache.pekko.actor.ActorSystem import javax.inject.Singleton import uk.gov.hmrc.bindingtarifffilestore.config.AppConfig From 4b794b3be2df7bdc89b999274827de158601cda7 Mon Sep 17 00:00:00 2001 From: SachaMcCormack <175815565+SachaMcCormack@users.noreply.github.com> Date: Thu, 30 Oct 2025 13:17:12 +0000 Subject: [PATCH 5/8] Revert some unneeded added files --- .../connector/AmazonS3Connector.scala | 132 ---------- .../controllers/FileStoreController.scala | 26 +- .../model/Pagination.scala | 3 +- .../FileMetadataMongoRepository.scala | 8 +- .../service/FileStoreService.scala | 55 +++-- conf/app.routes | 2 +- .../BaseFeatureSpec.scala | 5 +- .../FileStoreHelpers.scala | 45 ++-- .../FileStoreSpec.scala | 25 +- .../config/AppConfigSpec.scala | 34 +-- .../connector/AmazonS3ConnectorSpec.scala | 227 ------------------ .../connector/ObjectStoreConnectorSpec.scala | 182 ++++++++++++++ .../connector/UpscanConnectorSpec.scala | 4 +- .../controllers/FileStoreControllerSpec.scala | 179 +++++++------- .../service/FileStoreServiceSpec.scala | 59 +++-- .../util/WithFakeApplication.scala | 7 +- 16 files changed, 414 insertions(+), 579 deletions(-) delete mode 100644 app/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3Connector.scala delete mode 100644 test/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3ConnectorSpec.scala create mode 100644 test/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnectorSpec.scala diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3Connector.scala b/app/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3Connector.scala deleted file mode 100644 index 850a434..0000000 --- a/app/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3Connector.scala +++ /dev/null @@ -1,132 +0,0 @@ -/* - * Copyright 2025 HM Revenue & Customs - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package uk.gov.hmrc.bindingtarifffilestore.connector - -import com.amazonaws.auth.{AWSCredentials, BasicAWSCredentials, DefaultAWSCredentialsProviderChain} - -import java.io.BufferedInputStream -import java.net.URL -import java.util -import com.amazonaws.{AmazonClientException, HttpMethod} -import com.amazonaws.client.builder.AwsClientBuilder.EndpointConfiguration -import com.amazonaws.services.s3.model.DeleteObjectsRequest.KeyVersion -import com.amazonaws.services.s3.model._ -import com.amazonaws.services.s3.{AmazonS3, AmazonS3ClientBuilder} -import com.google.inject.Inject - -import javax.inject.Singleton -import uk.gov.hmrc.bindingtarifffilestore.config.AppConfig -import uk.gov.hmrc.bindingtarifffilestore.model.FileMetadata -import uk.gov.hmrc.bindingtarifffilestore.util.Logging - -import scala.jdk.CollectionConverters._ -import scala.util.{Failure, Success, Try} - -@Singleton -class AmazonS3Connector @Inject() (config: AppConfig) extends Logging { - - private lazy val s3Config = config.s3Configuration - - private lazy val s3client: AmazonS3 = { - log.info(s"${s3Config.bucket}:${s3Config.region}") - val builder = AmazonS3ClientBuilder - .standard() - .withPathStyleAccessEnabled(true) - .withCredentials(new LocalDevelopmentS3CredentialsProviderChain()) - - s3Config.endpoint match { - case Some(endpoint) => builder.withEndpointConfiguration(new EndpointConfiguration(endpoint, s3Config.region)) - case _ => builder.withRegion(s3Config.region) - } - - builder.build() - } - - def getAll: Seq[String] = - sequenceOf( - s3client.listObjects(s3Config.bucket).getObjectSummaries - ).map(_.getKey) - - def upload(fileMetaData: FileMetadata): FileMetadata = { - val url: URL = new URL(fileMetaData.url.getOrElse(throw new IllegalArgumentException("Missing URL"))) - - val metadata = new ObjectMetadata - // This .get is scary but our file must have received a positive scan - // result and received metadata from Upscan if it is being published - metadata.setContentType(fileMetaData.mimeType.get) - metadata.setContentLength(contentLengthOf(url)) - - val request = new PutObjectRequest( - s3Config.bucket, - fileMetaData.id, - new BufferedInputStream(url.openStream()), - metadata - ).withCannedAcl(CannedAccessControlList.Private) - - Try(s3client.putObject(request)) match { - case Success(_) => - fileMetaData.copy(url = Some(s"${s3Config.baseUrl}/${s3Config.bucket}/${fileMetaData.id}")) - case Failure(e: Throwable) => - log.error("Failed to upload to the S3 bucket.", e) - throw e - } - } - - def delete(id: String): Unit = - s3client.deleteObject(s3Config.bucket, id) - - def deleteAll(): Unit = { - val keys: Seq[KeyVersion] = getAll.map(new KeyVersion(_)) - if (keys.nonEmpty) { - log.info(s"Removing [${keys.length}] files from S3") - log.info(s"bucket is: ${s3Config.bucket}") - val request = new DeleteObjectsRequest(s3Config.bucket) - .withKeys(keys.toList.asJava) - .withQuiet(false) - s3client.deleteObjects(request) - } else { - log.info(s"No files to remove from S3") - } - } - - def sign(fileMetaData: FileMetadata): FileMetadata = - if (fileMetaData.url.isDefined) { - val authenticatedURLRequest = new GeneratePresignedUrlRequest(config.s3Configuration.bucket, fileMetaData.id) - .withMethod(HttpMethod.GET) - val authenticatedURL: URL = s3client.generatePresignedUrl(authenticatedURLRequest) - fileMetaData.copy(url = Some(authenticatedURL.toString)) - } else { - fileMetaData - } - - private def contentLengthOf(url: URL): Long = - url.openConnection.getContentLengthLong - - private def sequenceOf[T](list: util.List[T]): Seq[T] = - list.iterator.asScala.toSeq - -} - -class LocalDevelopmentS3CredentialsProviderChain() extends DefaultAWSCredentialsProviderChain { - - override def getCredentials(): AWSCredentials = - Try { - super.getCredentials() - }.recover { case _: AmazonClientException => - new BasicAWSCredentials("dummy-access-key", "dummy-secret-key") - }.get -} diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/controllers/FileStoreController.scala b/app/uk/gov/hmrc/bindingtarifffilestore/controllers/FileStoreController.scala index 8b8f6ab..5a3849e 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/controllers/FileStoreController.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/controllers/FileStoreController.scala @@ -18,10 +18,12 @@ package uk.gov.hmrc.bindingtarifffilestore.controllers import play.api.Logging import play.api.libs.Files.TemporaryFile +import play.api.libs.json.Format.GenericFormat +import play.api.libs.json.OFormat.oFormatFromReadsAndOWrites import play.api.libs.json.{JsValue, Json} import play.api.mvc._ import uk.gov.hmrc.bindingtarifffilestore.config.AppConfig -import uk.gov.hmrc.bindingtarifffilestore.model.FileMetadataREST._ +import uk.gov.hmrc.bindingtarifffilestore.model.FileMetadataREST.format import uk.gov.hmrc.bindingtarifffilestore.model._ import uk.gov.hmrc.bindingtarifffilestore.model.upscan.ScanResult import uk.gov.hmrc.bindingtarifffilestore.model.upscan.v2._ @@ -52,11 +54,13 @@ class FileStoreController @Inject() ( def encodeInBase64(text: String): String = Base64.getEncoder.encodeToString(text.getBytes(StandardCharsets.UTF_8)) - private def withFileMetadata(id: String)(f: FileMetadata => Future[Result]): Future[Result] = + private def withFileMetadata(id: String)(f: FileMetadata => Future[Result])(implicit + hc: HeaderCarrier + ): Future[Result] = service.find(id).flatMap { case Some(meta) => logger.info( - s"[FileStoreController][withFileMetadata] Attachement File: $id, Scan succeeded with details fileMetadata: ${encodeInBase64(meta.toString)}" + s"[FileStoreController][withFileMetadata] Attachment File: $id, Scan succeeded with details fileMetadata: ${encodeInBase64(meta.toString)}" ) f(meta) case None => @@ -67,6 +71,8 @@ class FileStoreController @Inject() ( lazy private val testModeFilter = TestMode.actionFilter(appConfig, parse.default) def deleteAll(): Action[AnyContent] = withErrorHandling { + implicit val hc: HeaderCarrier = HeaderCarrier() + testModeFilter.async { service .deleteAll() @@ -74,9 +80,11 @@ class FileStoreController @Inject() ( } } - def delete(id: String): Action[AnyContent] = withErrorHandling { _ => + def delete(id: String, filename: String): Action[AnyContent] = withErrorHandling { _ => + implicit val hc: HeaderCarrier = HeaderCarrier() + service - .delete(id) + .delete(id, filename) .map(_ => NoContent) } @@ -106,8 +114,10 @@ class FileStoreController @Inject() ( } } - def get(id: String): Action[AnyContent] = Action.async { - withFileMetadata(id)(meta => Future.successful(Ok(Json.toJson(meta)))) + def get(id: String): Action[AnyContent] = Action.async { implicit request => + withFileMetadata(id) { meta => + Future.successful(Ok(Json.toJson(meta))) + } } def notification(id: String): Action[JsValue] = withErrorHandling { @@ -134,6 +144,8 @@ class FileStoreController @Inject() ( } def getAll(search: Search, pagination: Option[Pagination]): Action[AnyContent] = withErrorHandling { _ => + implicit val hc: HeaderCarrier = HeaderCarrier() + service.find(search, pagination.getOrElse(Pagination.max)).map { pagedResults => if (pagination.isDefined) { Ok(Json.toJson(pagedResults)) diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/model/Pagination.scala b/app/uk/gov/hmrc/bindingtarifffilestore/model/Pagination.scala index 5615dca..5e4566b 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/model/Pagination.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/model/Pagination.scala @@ -17,7 +17,6 @@ package uk.gov.hmrc.bindingtarifffilestore.model import play.api.mvc.QueryStringBindable -import uk.gov.hmrc.bindingtarifffilestore.model.Pagination.{defaultPageSize, defaultPageStart} case class Pagination( page: Int = defaultPageStart, @@ -28,7 +27,7 @@ object Pagination { val defaultPageStart = 1 val defaultPageSize = 100 - val max: Pagination = Pagination(1, Integer.MAX_VALUE) + val max: Pagination = Pagination(1, 10000) private val pageKey = "page" private val pageSizeKey = "page_size" diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/repository/FileMetadataMongoRepository.scala b/app/uk/gov/hmrc/bindingtarifffilestore/repository/FileMetadataMongoRepository.scala index c443071..a278e5a 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/repository/FileMetadataMongoRepository.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/repository/FileMetadataMongoRepository.scala @@ -17,15 +17,13 @@ package uk.gov.hmrc.bindingtarifffilestore.repository import org.mongodb.scala.bson.conversions.Bson -import org.mongodb.scala.model.Filters._ -import org.mongodb.scala.model._ +import org.mongodb.scala.model.* +import org.mongodb.scala.model.Filters.* import org.mongodb.scala.result.UpdateResult -import uk.gov.hmrc.bindingtarifffilestore.model._ +import uk.gov.hmrc.bindingtarifffilestore.model.* import uk.gov.hmrc.bindingtarifffilestore.util.Logging import uk.gov.hmrc.mongo.MongoComponent import uk.gov.hmrc.mongo.play.json.PlayMongoRepository -import org.mongodb.scala.SingleObservableFuture -import org.mongodb.scala.ObservableFuture import java.time.Instant import javax.inject.{Inject, Singleton} diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/service/FileStoreService.scala b/app/uk/gov/hmrc/bindingtarifffilestore/service/FileStoreService.scala index 7d34bb4..fee8cab 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/service/FileStoreService.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/service/FileStoreService.scala @@ -19,7 +19,7 @@ package uk.gov.hmrc.bindingtarifffilestore.service import play.api.Logging import uk.gov.hmrc.bindingtarifffilestore.audit.AuditService import uk.gov.hmrc.bindingtarifffilestore.config.AppConfig -import uk.gov.hmrc.bindingtarifffilestore.connector.{AmazonS3Connector, UpscanConnector} +import uk.gov.hmrc.bindingtarifffilestore.connector.{ObjectStoreConnector, UpscanConnector} import uk.gov.hmrc.bindingtarifffilestore.controllers.routes import uk.gov.hmrc.bindingtarifffilestore.model.ScanStatus.READY import uk.gov.hmrc.bindingtarifffilestore.model.* @@ -27,17 +27,17 @@ import uk.gov.hmrc.bindingtarifffilestore.model.upscan.* import uk.gov.hmrc.bindingtarifffilestore.repository.FileMetadataMongoRepository import uk.gov.hmrc.bindingtarifffilestore.util.HashUtil import uk.gov.hmrc.http.HeaderCarrier -import java.util.Base64 -import java.nio.charset.StandardCharsets -import java.util.UUID +import java.nio.charset.StandardCharsets +import java.util.{Base64, UUID} import javax.inject.{Inject, Singleton} import scala.concurrent.{ExecutionContext, Future} +import scala.language.postfixOps @Singleton() class FileStoreService @Inject() ( appConfig: AppConfig, - fileStoreConnector: AmazonS3Connector, + fileStoreConnector: ObjectStoreConnector, repository: FileMetadataMongoRepository, upscanConnector: UpscanConnector, auditService: AuditService @@ -120,11 +120,19 @@ class FileStoreService @Inject() ( } yield update } - def find(id: String): Future[Option[FileMetadata]] = - repository.get(id) map signingPermanentURL + def find(id: String)(implicit hc: HeaderCarrier): Future[Option[FileMetadata]] = + repository.get(id) map signingPermanentURL flatMap { + case Some(value) => value.map(data => Some(data)) + case None => Future.successful(None) + } - def find(search: Search, pagination: Pagination): Future[Paged[FileMetadata]] = - repository.get(search, pagination: Pagination) map (signingPermanentURLs(_)) + def find(search: Search, pagination: Pagination)(implicit hc: HeaderCarrier): Future[Paged[FileMetadata]] = { + val pagedFuture: Future[Paged[Future[FileMetadata]]] = + repository.get(search, pagination: Pagination) map signingPermanentURLs() + pagedFuture.flatMap { paged => + Future.sequence(paged.results).map(resolvedItems => paged.copy(results = resolvedItems)) + } + } // when UpScan comes back to us with the scan result def notify(attachment: FileMetadata, scanResult: ScanResult)(implicit @@ -132,7 +140,7 @@ class FileStoreService @Inject() ( ): Future[Option[FileMetadata]] = { logger.info( - s"[FileStoreService][notify] Attachement File: ${attachment.id}, Scan completed with status [${scanResult.fileStatus}] and Upscan reference [${scanResult.reference}]" + s"[FileStoreService][notify] Attachment File: ${attachment.id}, Scan completed with status [${scanResult.fileStatus}] and Upscan reference [${scanResult.reference}]" ) auditService @@ -143,12 +151,12 @@ class FileStoreService @Inject() ( scanResult match { case FailedScanResult(_, _, details) => logger.info( - s"[FileStoreService][notify] Attachement File: ${attachment.id}, Scan failed because it was [${details.failureReason}] with message [${details.message}]" + s"[FileStoreService][notify] Attachment File: ${attachment.id}, Scan failed because it was [${details.failureReason}] with message [${details.message}]" ) repository.update(updatedAttachment) case SuccessfulScanResult(_, _, _, details) => logger.info( - s"[FileStoreService][notify] Attachement File: ${attachment.id}, Scan succeeded with details [fileName: ${encodeInBase64( + s"[FileStoreService][notify] Attachment File: ${attachment.id}, Scan succeeded with details [fileName: ${encodeInBase64( details.fileName )}, fileMimeType:${details.fileMimeType}, checksum:${encodeInBase64(details.checksum)}, ${details.uploadTimestamp}]" ) @@ -160,7 +168,7 @@ class FileStoreService @Inject() ( publish(metadata) case _ => logger.info( - s"[FileStoreService][notify] Attachement File: ${attachment.id}, Scan completed as READY but it couldn't be published as it no longer exists" + s"[FileStoreService][notify] Attachment File: ${attachment.id}, Scan completed as READY but it couldn't be published as it no longer exists" ) Future.successful(None) } @@ -184,6 +192,7 @@ class FileStoreService @Inject() ( repository .update(metadata.copy(publishable = true, published = true)) .map(signingPermanentURL) + .flatMap(a => a.get.map(Some(_))) // File is safe, unpublished but the download URL has expired. Clean Up. case (Some(READY), false) => @@ -206,12 +215,12 @@ class FileStoreService @Inject() ( } } - def deleteAll(): Future[Unit] = + def deleteAll()(implicit hc: HeaderCarrier): Future[Unit] = repository.deleteAll() map (_ => fileStoreConnector.deleteAll()) - def delete(id: String): Future[Unit] = { + def delete(id: String, filename: String)(implicit hc: HeaderCarrier): Future[Unit] = { logger.info(s"[FileStoreService][delete] Deleting file: $id") - repository.delete(id) map (_ => fileStoreConnector.delete(id)) + repository.delete(id) map (_ => fileStoreConnector.delete(filename)) } private def upscanInitiate(fileMetadata: FileMetadata)(implicit hc: HeaderCarrier): Future[UpscanInitiateResponse] = { @@ -232,12 +241,18 @@ class FileStoreService @Inject() ( } yield initiateResponse } - private def signingPermanentURL: Option[FileMetadata] => Option[FileMetadata] = _.map(signingIfPublished) + private def signingPermanentURL(implicit hc: HeaderCarrier): Option[FileMetadata] => Option[Future[FileMetadata]] = { + asd => + asd.map(signingIfPublished) + } - private def signingPermanentURLs: Paged[FileMetadata] => Paged[FileMetadata] = _.map(signingIfPublished) + private def signingPermanentURLs()(implicit hc: HeaderCarrier): Paged[FileMetadata] => Paged[Future[FileMetadata]] = { + asd => + asd.map(signingIfPublished) + } - private def signingIfPublished: FileMetadata => FileMetadata = { + private def signingIfPublished(implicit hc: HeaderCarrier): FileMetadata => Future[FileMetadata] = { case file if file.published => fileStoreConnector.sign(file) - case other => other + case other => Future.successful(other) } } diff --git a/conf/app.routes b/conf/app.routes index ded0cd4..099b62f 100644 --- a/conf/app.routes +++ b/conf/app.routes @@ -10,4 +10,4 @@ POST /file/:id/publish @uk.gov.hmrc.bindingtarifffilestore.controll # admin/test endpoints DELETE /file @uk.gov.hmrc.bindingtarifffilestore.controllers.FileStoreController.deleteAll() -DELETE /file/:id @uk.gov.hmrc.bindingtarifffilestore.controllers.FileStoreController.delete(id: String) +DELETE /file/:id @uk.gov.hmrc.bindingtarifffilestore.controllers.FileStoreController.delete(id: String, filename: String) diff --git a/it/test/uk/gov/hmrc/bindingtarifffilestore/BaseFeatureSpec.scala b/it/test/uk/gov/hmrc/bindingtarifffilestore/BaseFeatureSpec.scala index b5b9b29..21900ce 100644 --- a/it/test/uk/gov/hmrc/bindingtarifffilestore/BaseFeatureSpec.scala +++ b/it/test/uk/gov/hmrc/bindingtarifffilestore/BaseFeatureSpec.scala @@ -17,7 +17,7 @@ package uk.gov.hmrc.bindingtarifffilestore import com.google.common.io.BaseEncoding -import org.scalatest._ +import org.scalatest.* import org.scalatest.featurespec.AnyFeatureSpec import org.scalatest.matchers.should.Matchers import org.scalatestplus.play.guice.GuiceOneServerPerSuite @@ -31,9 +31,8 @@ import org.mongodb.scala.SingleObservableFuture import java.security.MessageDigest import scala.concurrent.{Await, Future} -import scala.concurrent.Await.result import scala.concurrent.ExecutionContext.Implicits.global -import scala.concurrent.duration._ +import scala.concurrent.duration.* abstract class BaseFeatureSpec extends AnyFeatureSpec diff --git a/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreHelpers.scala b/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreHelpers.scala index 4bd1136..2114658 100644 --- a/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreHelpers.scala +++ b/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreHelpers.scala @@ -16,16 +16,16 @@ package uk.gov.hmrc.bindingtarifffilestore -import com.github.tomakehurst.wiremock.client.WireMock._ +import com.github.tomakehurst.wiremock.client.WireMock.* import com.github.tomakehurst.wiremock.stubbing.StubMapping import play.api.http.Status import play.api.libs.Files.SingletonTemporaryFileCreator import play.api.libs.json.Json import uk.gov.hmrc.bindingtarifffilestore.model.FileMetadataREST.format -import uk.gov.hmrc.bindingtarifffilestore.model._ -import uk.gov.hmrc.bindingtarifffilestore.model.upscan._ +import uk.gov.hmrc.bindingtarifffilestore.model.* +import uk.gov.hmrc.bindingtarifffilestore.model.upscan.* import uk.gov.hmrc.bindingtarifffilestore.model.upscan.v2.FileStoreInitiateRequest -import uk.gov.hmrc.http.HttpReads.Implicits._ +import uk.gov.hmrc.http.HttpReads.Implicits.* import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse, StringContextOps} import java.io.File @@ -34,9 +34,9 @@ import java.nio.file.Files import java.time.Instant import scala.concurrent.ExecutionContext.Implicits.global import scala.concurrent.Future -import scala.concurrent.duration._ +import scala.concurrent.duration.* import scala.io.Source -import scala.jdk.CollectionConverters._ +import scala.jdk.CollectionConverters.* import org.mongodb.scala.SingleObservableFuture import play.api.libs.ws.JsonBodyWritables.writeableOf_JsValue @@ -64,9 +64,8 @@ trait FileStoreHelpers extends WiremockFeatureTestServer { httpClientV2.delete(url"$serviceUrl/file").execute[HttpResponse] def deleteFile(id: String)(implicit hc: HeaderCarrier): Future[HttpResponse] = { - stubS3DeleteOne(id) - - httpClientV2.delete(url"$serviceUrl/file/$id").execute[HttpResponse] + stubObjectStoreDeleteOne(id) + httpClientV2.delete(url"$serviceUrl/file?id=$id").execute[HttpResponse] } def getFiles(queryParams: Seq[(String, String)])(implicit hc: HeaderCarrier): Future[HttpResponse] = { @@ -78,7 +77,7 @@ trait FileStoreHelpers extends WiremockFeatureTestServer { } def publishSafeFile(id: String)(implicit hc: HeaderCarrier): Future[HttpResponse] = { - stubS3Upload(id) + stubObjectStoreSign(id) httpClientV2 .post(url"$serviceUrl/file/$id/publish") @@ -86,7 +85,7 @@ trait FileStoreHelpers extends WiremockFeatureTestServer { .execute[HttpResponse] } - // Should NOT call S3 Upload + // Should NOT call Object Store Upload def publishUnsafeFile(id: String)(implicit hc: HeaderCarrier): Future[HttpResponse] = httpClientV2 .post(url"$serviceUrl/file/$id/publish") @@ -126,6 +125,8 @@ trait FileStoreHelpers extends WiremockFeatureTestServer { ): Future[HttpResponse] = { stubUpscanInitiate stubUpscanUpload + stubObjectStoreUpload(id.get) + stubObjectStoreSign(id.get) val tempFile = SingletonTemporaryFileCreator.create(filename) Files.write(tempFile.path, List("foo").asJava) @@ -174,16 +175,25 @@ trait FileStoreHelpers extends WiremockFeatureTestServer { ) ) - def stubS3Upload(id: String): StubMapping = + def stubObjectStoreUpload(id: String): StubMapping = + stubFor( + post("/object-store/ops/upload-from-url") + .willReturn( + aResponse() + .withStatus(Status.OK) + ) + ) + + def stubObjectStoreSign(id: String): StubMapping = stubFor( - put(s"/digital-tariffs-local/$id") + post("/object-store/ops/presigned-url") .willReturn( aResponse() .withStatus(Status.OK) ) ) - def stubS3ListAll(): StubMapping = + def stubObjectStoreListAll(): StubMapping = stubFor( get("/digital-tariffs-local/?encoding-type=url") .willReturn( @@ -193,19 +203,18 @@ trait FileStoreHelpers extends WiremockFeatureTestServer { ) ) - def stubS3DeleteAll(): StubMapping = + def stubObjectStoreDeleteAll(): StubMapping = stubFor( post(s"/digital-tariffs-local/?delete") .willReturn( aResponse() .withStatus(Status.OK) - .withBody(fromFile("aws/delete-objects_response.xml")) ) ) - def stubS3DeleteOne(id: String): StubMapping = + def stubObjectStoreDeleteOne(id: String): StubMapping = stubFor( - delete(s"/digital-tariffs-local/$id") + delete(s"/digital-tariffs-local/file?id=$id") .willReturn( aResponse() .withStatus(Status.OK) diff --git a/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreSpec.scala b/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreSpec.scala index a22f329..d3e2011 100644 --- a/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreSpec.scala +++ b/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreSpec.scala @@ -16,17 +16,17 @@ package uk.gov.hmrc.bindingtarifffilestore +import org.scalatest.concurrent.Eventually import play.api.Application import play.api.http.Status import play.api.inject.bind import play.api.inject.guice.GuiceApplicationBuilder import play.api.libs.json.* -import uk.gov.hmrc.bindingtarifffilestore.model.ScanStatus.{FAILED, READY} import uk.gov.hmrc.bindingtarifffilestore.model.* +import uk.gov.hmrc.bindingtarifffilestore.model.ScanStatus.{FAILED, READY} import uk.gov.hmrc.bindingtarifffilestore.model.upscan.v2.{FileStoreInitiateResponse, UpscanFormTemplate} import uk.gov.hmrc.bindingtarifffilestore.repository.FileMetadataMongoRepository import uk.gov.hmrc.http.{HeaderCarrier, HttpResponse} -import org.scalatest.concurrent.Eventually import java.io.File import java.net.URI @@ -44,7 +44,7 @@ class FileStoreSpec extends FileStoreHelpers with Eventually { override def fakeApplication(): Application = new GuiceApplicationBuilder() .configure( - "s3.endpoint" -> s"http://localhost:$wirePort", + "microservice.services.object-store.port" -> s"$wirePort", "microservice.services.upscan-initiate.port" -> s"$wirePort" ) .overrides(bind[FileMetadataMongoRepository].toInstance(repository)) @@ -73,7 +73,8 @@ class FileStoreSpec extends FileStoreHelpers with Eventually { dbFileStoreSize.shouldBe(1) When("I request the file details") - val deleteResult = Await.result(deleteFile(id1), 7.seconds) + + val deleteResult = Await.result(deleteFile(file1), 7.seconds) Then("The response code should be Ok") @@ -97,8 +98,8 @@ class FileStoreSpec extends FileStoreHelpers with Eventually { await(upload(Some(id2), file2, contentType, publishable = true)) dbFileStoreSize shouldBe 2 - stubS3ListAll() - stubS3DeleteAll() + stubObjectStoreListAll() + stubObjectStoreDeleteAll() When("I delete all documents") val deleteResult = await(deleteFiles()) @@ -242,8 +243,7 @@ class FileStoreSpec extends FileStoreHelpers with Eventually { await(upload(Some(id2), file2, contentType, publishable = false)) dbFileStoreSize shouldBe 2 - stubS3ListAll() - stubS3DeleteAll() + stubObjectStoreListAll() When("I request the file details") @@ -333,12 +333,12 @@ class FileStoreSpec extends FileStoreHelpers with Eventually { Given("A File has been uploaded") - stubS3Upload(id1) + stubObjectStoreSign(id1) await(upload(Some(id1), file1, contentType, publishable = true)) dbFileStoreSize shouldBe 1 - stubS3ListAll() - stubS3DeleteAll() + stubObjectStoreListAll() + stubObjectStoreDeleteAll() When("Notify is Called") @@ -412,9 +412,6 @@ class FileStoreSpec extends FileStoreHelpers with Eventually { And("The response shows the file published") (result.json \\ "url").map(_.as[String]).headOption.getOrElse("") should include(s"$id1") - (result.json \\ "url").map(_.as[String]).headOption.getOrElse("") should include( - "X-Amz-Algorithm=AWS4-HMAC-SHA256" - ) } Scenario("Should mark an un-safe file as publishable, but not persist") { diff --git a/test/uk/gov/hmrc/bindingtarifffilestore/config/AppConfigSpec.scala b/test/uk/gov/hmrc/bindingtarifffilestore/config/AppConfigSpec.scala index e7886c9..7e6a8b0 100644 --- a/test/uk/gov/hmrc/bindingtarifffilestore/config/AppConfigSpec.scala +++ b/test/uk/gov/hmrc/bindingtarifffilestore/config/AppConfigSpec.scala @@ -42,16 +42,6 @@ class AppConfigSpec extends UnitSpec with WithFakeApplication with BeforeAndAfte new AppConfig(Configuration.from(config), serviceConfig).fileStoreSizeConfiguration } - private def s3ConfigWith(pairs: (String, String)*): S3Configuration = { - var config = Map( - "s3.region" -> "", - "s3.bucket" -> "", - "s3.endpoint" -> "" - ) - pairs.foreach(e => config = config + e) - new AppConfig(Configuration.from(config), serviceConfig).s3Configuration - } - private def upscanConfigWith(host: String, port: String, pairs: (String, String)*): AppConfig = { when(serviceConfig.baseUrl(refEq("upscan-initiate"))).thenReturn(s"http://$host:$port") new AppConfig(Configuration.from(pairs.map(e => e._1 -> e._2).toMap), serviceConfig) @@ -62,30 +52,8 @@ class AppConfigSpec extends UnitSpec with WithFakeApplication with BeforeAndAfte "Config" should { - "return AWS S3 region" in { - s3ConfigWith("s3.region" -> "region").region shouldBe "region" - } - "return AWS S3 bucket" in { - s3ConfigWith("s3.bucket" -> "bucket").bucket shouldBe "bucket" - } - - "return AWS S3 endpoint" in { - s3ConfigWith("s3.endpoint" -> "endpoint").endpoint shouldBe Some("endpoint") - } - - "return AWS S3 blank endpoint as None" in { - s3ConfigWith("s3.endpoint" -> "").endpoint shouldBe None - } - - "return AWS S3 base URL" in { - s3ConfigWith("s3.endpoint" -> "endpoint").baseUrl shouldBe "endpoint" - } - - "return AWS S3 default base URL" in { - s3ConfigWith( - "s3.region" -> "region" - ).baseUrl shouldBe "https://s3-region.amazonaws.com" + configWith("s3.bucket" -> "bucket").s3bucket shouldBe "bucket" } "return application Host" in { diff --git a/test/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3ConnectorSpec.scala b/test/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3ConnectorSpec.scala deleted file mode 100644 index 5bc6ab9..0000000 --- a/test/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3ConnectorSpec.scala +++ /dev/null @@ -1,227 +0,0 @@ -/* - * Copyright 2025 HM Revenue & Customs - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package uk.gov.hmrc.bindingtarifffilestore.connector - -import com.amazonaws.services.s3.model.AmazonS3Exception -import com.github.tomakehurst.wiremock.client.WireMock -import com.github.tomakehurst.wiremock.client.WireMock.* -import org.mockito.Mockito.{mock, when} -import org.scalatest.BeforeAndAfterEach -import play.api.http.Status -import play.api.libs.Files.SingletonTemporaryFileCreator -import uk.gov.hmrc.bindingtarifffilestore.config.{AppConfig, S3Configuration} -import uk.gov.hmrc.bindingtarifffilestore.model.FileMetadata -import uk.gov.hmrc.bindingtarifffilestore.util.* - -import java.time.LocalDate -import java.time.format.DateTimeFormatter - -class AmazonS3ConnectorSpec extends UnitSpec with WiremockTestServer with BeforeAndAfterEach with ResourceFiles { - - private val s3Config: S3Configuration = - S3Configuration("region", "bucket", Some(s"http://localhost:$wirePort")) - private val config = mock(classOf[AppConfig]) - private val date = LocalDate.now().format(DateTimeFormatter.ofPattern("YYYYMMdd")) - private val connector = new AmazonS3Connector(config) - - override protected def beforeEach(): Unit = { - super.beforeEach() - when(config.s3Configuration).thenReturn(s3Config) - } - - "Get All" should { - - "Delegate to S3" in { - stubFor( - get("/bucket/?encoding-type=url") - .withHeader( - "Authorization", - matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") - ) - .willReturn( - aResponse() - .withStatus(Status.OK) - .withBody(fromFile("aws/list-objects_response.xml")) - ) - ) - - // When - val all: Seq[String] = connector.getAll - - // Then - all should have size 1 - all.head shouldBe "image.jpg" - } - - } - - "Upload" should { - - "Delegate to S3" in { - stubFor( - put("/bucket/id") - .withHeader( - "Authorization", - matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") - ) - .withHeader("Content-Type", equalTo("text/plain")) - .willReturn( - aResponse() - .withStatus(Status.OK) - ) - ) - - val url = SingletonTemporaryFileCreator.create("example.txt").path.toUri.toURL.toString - val fileUploading = FileMetadata("id", Some("file.txt"), Some("text/plain"), Some(url)) - - // Then - val result = connector.upload(fileUploading) - result.id shouldBe "id" - result.fileName shouldBe Some("file.txt") - result.mimeType shouldBe Some("text/plain") - result.url.get shouldBe s"$wireMockUrl/bucket/id" - } - - "Throw Exception on missing URL" in { - val fileUploading = FileMetadata("id", Some("file.txt"), Some("text/plain")) - - val exception = intercept[IllegalArgumentException] { - connector.upload(fileUploading) - } - exception.getMessage shouldBe "Missing URL" - } - - "Throw Exception on upload failure" in { - stubFor( - put("/bucket/id") - .withHeader( - "Authorization", - matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") - ) - .withHeader("Content-Type", equalTo("text/plain")) - .willReturn( - aResponse() - .withStatus(Status.BAD_GATEWAY) - ) - ) - val url = SingletonTemporaryFileCreator.create("example.txt").path.toUri.toURL.toString - val fileUploading = FileMetadata("id", Some("file.txt"), Some("text/plain"), Some(url)) - - // Then - val exception = intercept[AmazonS3Exception] { - connector.upload(fileUploading) - } - exception.getMessage shouldBe - """Bad Gateway (Service: Amazon S3; - | Status Code: 502; - | Error Code: 502 Bad Gateway; - | Request ID: null; - | S3 Extended Request ID: null; - | Proxy: null)""".stripMargin.replaceAll("\n", "") - } - } - - "Sign" should { - "append token to URL" in { - val file = FileMetadata("id", Some("file.txt"), Some("text/plain"), Some("url")) - - connector.sign(file).url.get should startWith(s"$wireMockUrl/bucket/id?") - connector.sign(file).url.get should include("X-Amz-Algorithm=AWS4-HMAC-SHA256") - } - - "not append token to empty URL" in { - val file = FileMetadata("id", Some("file.txt"), Some("text/plain"), None) - - connector.sign(file).url shouldBe None - } - } - - "Delete All" should { - "Delegate to S3" in { - stubFor( - get("/bucket/?encoding-type=url") - .withHeader( - "Authorization", - matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") - ) - .willReturn( - aResponse() - .withStatus(Status.OK) - .withBody(fromFile("aws/list-objects_response.xml")) - ) - ) - stubFor( - post("/bucket/?delete") - .withHeader( - "Authorization", - matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") - ) - .willReturn( - aResponse() - .withStatus(Status.OK) - .withBody(fromFile("aws/delete-objects_response.xml")) - ) - ) - - connector.deleteAll() - - WireMock.verify( - postRequestedFor(urlEqualTo("/bucket/?delete")) - ) - } - - "Do nothing for no files" in { - stubFor( - get("/bucket/?encoding-type=url") - .withHeader( - "Authorization", - matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") - ) - .willReturn( - aResponse() - .withStatus(Status.OK) - .withBody(fromFile("aws/empty-list-objects_response.xml")) - ) - ) - - connector.deleteAll() - - WireMock.verify(0, postRequestedFor(urlEqualTo("/bucket/?delete"))) - } - } - - "Delete One" should { - "Delegate to S3" in { - stubFor( - delete("/bucket/id") - .withHeader( - "Authorization", - matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") - ) - .willReturn( - aResponse() - .withStatus(Status.OK) - ) - ) - - connector.delete("id") - - WireMock.verify(deleteRequestedFor(urlEqualTo("/bucket/id"))) - } - } - -} diff --git a/test/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnectorSpec.scala b/test/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnectorSpec.scala new file mode 100644 index 0000000..c81585d --- /dev/null +++ b/test/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnectorSpec.scala @@ -0,0 +1,182 @@ +/* + * Copyright 2025 HM Revenue & Customs + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package uk.gov.hmrc.bindingtarifffilestore.connector + +import org.apache.pekko.stream.Materializer +import org.mockito.Mockito.mock +import org.scalatest.BeforeAndAfterEach +import org.scalatest.time.SpanSugar.convertIntToGrainOfTime +import play.api.inject.guice.GuiceApplicationBuilder +import play.api.{Application, inject} +import uk.gov.hmrc.bindingtarifffilestore.config.AppConfig +import uk.gov.hmrc.bindingtarifffilestore.model.FileMetadata +import uk.gov.hmrc.bindingtarifffilestore.util.* +import uk.gov.hmrc.http.HeaderCarrier +import uk.gov.hmrc.objectstore.client.Path.{Directory, File} +import uk.gov.hmrc.objectstore.client.config.ObjectStoreClientConfig +import uk.gov.hmrc.objectstore.client.play.Implicits.{futureMonad, stringWrite} +import uk.gov.hmrc.objectstore.client.play.PlayObjectStoreClient +import uk.gov.hmrc.objectstore.client.play.test.stub.StubPlayObjectStoreClient +import uk.gov.hmrc.objectstore.client.{ObjectSummary, Path, RetentionPeriod} + +import java.time.{Instant, LocalDate, LocalTime, ZoneId} +import java.util.UUID.randomUUID +import scala.concurrent.Await +import scala.concurrent.ExecutionContext.Implicits.global + +class ObjectStoreConnectorSpec + extends UnitSpec + with WithFakeApplication + with WiremockTestServer + with BeforeAndAfterEach + with ResourceFiles { + + implicit lazy val hc: HeaderCarrier = mock(classOf[HeaderCarrier]) + private implicit val mat: Materializer = mock(classOf[Materializer]) + private val directory: Path.Directory = + Path.Directory("digital-tariffs-local") + + private val file1 = FileMetadata("id1", Some("file1.txt"), Some("text/plain"), Some("http://foo.bar/test-123.txt")) + private val file2 = FileMetadata("id2", Some("file2.txt"), Some("text/plain"), Some("http://foo.bar/test-456.txt")) + + lazy val objectStoreClientStub: StubPlayObjectStoreClient = { + val baseUrl = s"baseUrl-${randomUUID().toString}" + val owner = "binding-tariff-filestore" + val token = s"token-${randomUUID().toString}" + val config = ObjectStoreClientConfig(baseUrl, owner, token, RetentionPeriod.OneWeek) + + new StubPlayObjectStoreClient(config) + } + + override lazy val fakeApplication: Application = new GuiceApplicationBuilder() + .configure( + "microservice.services.object-store.port" -> s"$wirePort" + ) + .bindings(inject.bind(classOf[PlayObjectStoreClient]).to(objectStoreClientStub)) + .build() + + private implicit val config: AppConfig = fakeApplication.injector.instanceOf[AppConfig] + + private val connector = new ObjectStoreConnector(objectStoreClientStub, config) + + val zonedDateTime: Instant = + LocalDate.of(2025, 1, 1).atTime(LocalTime.of(12, 30)).atZone(ZoneId.of("Europe/Paris")).toInstant + + override protected def beforeEach(): Unit = + super.beforeEach() + + "Get All" should { + "list files when no files exist" in { + val all = await(connector.getAll(directory)) + + all shouldBe List.empty[ObjectSummary] + } + + "when files have been added, list files that have been stored" in { + await(objectStoreClientStub.putObject(directory.file(file1.fileName.get), "text", RetentionPeriod.OneDay)) + val all = await(connector.getAll(directory)) + + all.size shouldBe 1 + } + } + + "Upload" should { + + "add file to the object store" in { + + await(connector.upload(file1)) + + val file = await(connector.getAll(directory)) + + file.length shouldBe 1 + file.head.location shouldBe File(Directory("binding-tariff-filestore/digital-tariffs-local"), "file1.txt") + } + + "Throw Exception on missing URL" in { + val fileUploading = FileMetadata("id", Some("file.txt"), Some("text/plain")) + val exception = intercept[IllegalArgumentException] { + connector.upload(fileUploading) + } + + exception.getMessage shouldBe "Missing URL" + } + } + + "Sign" should { + "create a presigned download URL" in { + + await(objectStoreClientStub.putObject(directory.file(file1.fileName.get), "text", RetentionPeriod.OneDay)) + + val result = await(connector.sign(file1)) + + result shouldBe file1 + result.url shouldBe Some("http://foo.bar/test-123.txt") + } + + "not create a presigned download url if there is an empty URL" in { + val file = FileMetadata("id", Some("file.txt"), Some("text/plain"), None) + val result = Await.result(connector.sign(file), 5.seconds) + + result.url shouldBe None + } + } + + "Delete One" should { + "delete file from object store" in { + await( + objectStoreClientStub.putObject(directory.file(file1.fileName.get), file1.mimeType.get, RetentionPeriod.OneDay) + ) + + val result: Unit = await( + connector + .delete(file1.fileName.get) + ) + + val files = await(connector.getAll(directory)) + + result === () + files.size shouldBe 0 + } + } + + "Delete All" should { + "Do nothing for no files" in { + await(connector.deleteAll(), 5.seconds) + + val files = await(connector.getAll(directory)) + + files.size shouldBe 0 + + } + + "delete all files from object store if present" in { + await( + objectStoreClientStub.putObject(directory.file(file1.fileName.get), file1.mimeType.get, RetentionPeriod.OneDay) + ) + await( + objectStoreClientStub.putObject(directory.file(file2.fileName.get), file2.mimeType.get, RetentionPeriod.OneDay) + ) + await(connector.deleteAll()) + + val all = await(connector.getAll(directory)) + + all.size shouldBe 0 + + } + } + +} diff --git a/test/uk/gov/hmrc/bindingtarifffilestore/connector/UpscanConnectorSpec.scala b/test/uk/gov/hmrc/bindingtarifffilestore/connector/UpscanConnectorSpec.scala index a5b90c5..2161b0d 100644 --- a/test/uk/gov/hmrc/bindingtarifffilestore/connector/UpscanConnectorSpec.scala +++ b/test/uk/gov/hmrc/bindingtarifffilestore/connector/UpscanConnectorSpec.scala @@ -41,7 +41,7 @@ class UpscanConnectorSpec private val config: AppConfig = mock(classOf[AppConfig]) val httpClient: HttpClientV2 = fakeApplication.injector.instanceOf[HttpClientV2] - private implicit val headers: HeaderCarrier = HeaderCarrier() + implicit lazy val headers: HeaderCarrier = HeaderCarrier() private val connector: UpscanConnector = new UpscanConnector(config, httpClient) @@ -134,7 +134,7 @@ class UpscanConnectorSpec (None, Some("text/plain")), (Some("file.txt"), None), (None, None) - ).foreach(args => (test _).tupled(args)) + ).foreach(args => test.tupled(args)) } "Upload with error handling if 502 BAD_GATEWAY is returned" in { diff --git a/test/uk/gov/hmrc/bindingtarifffilestore/controllers/FileStoreControllerSpec.scala b/test/uk/gov/hmrc/bindingtarifffilestore/controllers/FileStoreControllerSpec.scala index ed78697..cd47303 100644 --- a/test/uk/gov/hmrc/bindingtarifffilestore/controllers/FileStoreControllerSpec.scala +++ b/test/uk/gov/hmrc/bindingtarifffilestore/controllers/FileStoreControllerSpec.scala @@ -19,42 +19,44 @@ package uk.gov.hmrc.bindingtarifffilestore.controllers import com.mongodb.{MongoWriteException, ServerAddress, WriteError} import org.apache.pekko.stream.Materializer import org.mockito.ArgumentCaptor -import org.mockito.ArgumentMatchers.{any, refEq} -import org.mockito.Mockito._ +import org.mockito.ArgumentMatchers.{any, eq as eqTo, refEq} +import org.mockito.Mockito.* import org.mongodb.scala.bson.BsonDocument import org.scalatest.BeforeAndAfterEach import org.scalatest.matchers.should.Matchers -import play.api.http.Status._ +import play.api.http.Status.* import play.api.libs.Files.{SingletonTemporaryFileCreator, TemporaryFile} import play.api.libs.json.{JsValue, Json, Writes} +import play.api.mvc.* import play.api.mvc.MultipartFormData.FilePart -import play.api.mvc._ import play.api.test.FakeRequest import uk.gov.hmrc.bindingtarifffilestore.config.AppConfig +import uk.gov.hmrc.bindingtarifffilestore.model.* import uk.gov.hmrc.bindingtarifffilestore.model.FileMetadataREST.format -import uk.gov.hmrc.bindingtarifffilestore.model._ import uk.gov.hmrc.bindingtarifffilestore.model.upscan.v2.{FileStoreInitiateRequest, FileStoreInitiateResponse, UpscanFormTemplate} import uk.gov.hmrc.bindingtarifffilestore.model.upscan.{ScanResult, SuccessfulScanResult, UploadDetails} import uk.gov.hmrc.bindingtarifffilestore.service.FileStoreService -import uk.gov.hmrc.bindingtarifffilestore.util._ +import uk.gov.hmrc.bindingtarifffilestore.util.* import uk.gov.hmrc.http.HeaderCarrier import java.time.Instant import java.util.Collections import scala.concurrent.ExecutionContext.Implicits.global -import scala.concurrent.Future.{failed, successful} +import scala.concurrent.Future +import scala.concurrent.Future.successful class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplication with BeforeAndAfterEach { - private implicit val mat: Materializer = fakeApplication.materializer - + implicit lazy val mat: Materializer = mock(classOf[Materializer]) + implicit lazy val hc: HeaderCarrier = mock(classOf[HeaderCarrier]) private val appConfig: AppConfig = mock(classOf[AppConfig]) private val service: FileStoreService = mock(classOf[FileStoreService]) - private lazy val playBodyParsers: PlayBodyParsers = fakeApplication.injector.instanceOf[PlayBodyParsers] + private lazy val playBodyParsers: PlayBodyParsers = mock(classOf[PlayBodyParsers]) lazy val cc: MessagesControllerComponents = fakeApplication.injector.instanceOf[MessagesControllerComponents] private val controller: FileStoreController = new FileStoreController(appConfig, service, playBodyParsers, cc) - private val fakeRequest: FakeRequest[AnyContentAsEmpty.type] = FakeRequest() + private val fakeRequest: FakeRequest[AnyContentAsEmpty.type] = + FakeRequest() private def jsonRequest[T](body: T)(implicit writes: Writes[T]): Request[AnyContent] = fakeRequest @@ -66,6 +68,67 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic reset(service) } + "Get By ID" should { + "return 200 when found" in { + val id = "id" + val attachment = FileMetadata(id = id, fileName = Some("file"), mimeType = Some("type")) + + when(appConfig.isTestMode).thenReturn(true) + when(service.find(eqTo(id))(any[HeaderCarrier])).thenReturn(Future.successful(Some(attachment))) + + val result = await(controller.get(id)(fakeRequest)) + + status(result) shouldBe OK + bodyOf(result) shouldEqual Json.toJson(attachment).toString() + } + + "return 404 when not found" in { + when(service.find(eqTo("id"))(any[HeaderCarrier])).thenReturn(successful(None)) + + val result = await(controller.get("id")(fakeRequest)) + + status(result) shouldBe NOT_FOUND + } + } + + "Get By Search" should { + "return 200 with empty array" in { + when(service.find(eqTo(Search(ids = Some(Set.empty))), eqTo(Pagination.max))(any[HeaderCarrier])) + .thenReturn(successful(Paged.empty[FileMetadata])) + + val result = await(controller.getAll(Search(ids = Some(Set.empty)), None)(fakeRequest)) + + status(result) shouldBe OK + bodyOf(result) shouldEqual Json.toJson(Seq.empty).toString() + } + + "return 200 with non empty array" in { + val attachment1 = FileMetadata(id = "id1", fileName = Some("file1"), mimeType = Some("type1")) + val attachment2 = FileMetadata(id = "id2", fileName = Some("file2"), mimeType = Some("type2")) + + when(service.find(eqTo(Search(ids = Some(Set("id1", "id2")))), eqTo(Pagination.max))(any[HeaderCarrier])) + .thenReturn(successful(Paged(Seq(attachment1, attachment2)))) + + val result = await(controller.getAll(Search(ids = Some(Set("id1", "id2"))), None)(fakeRequest)) + + status(result) shouldBe OK + bodyOf(result) shouldEqual Json.toJson(Seq(attachment1, attachment2)).toString() + } + + "return 200 with pagination and non empty pager" in { + val attachment1 = FileMetadata(id = "id1", fileName = Some("file1"), mimeType = Some("type1")) + val attachment2 = FileMetadata(id = "id2", fileName = Some("file2"), mimeType = Some("type2")) + + when(service.find(eqTo(Search(ids = Some(Set("id1", "id2")))), eqTo(Pagination()))(any[HeaderCarrier])) + .thenReturn(successful(Paged(Seq(attachment1, attachment2)))) + + val result = await(controller.getAll(Search(ids = Some(Set("id1", "id2"))), Some(Pagination()))(fakeRequest)) + + status(result) shouldBe OK + bodyOf(result) shouldEqual Json.toJson(Paged(Seq(attachment1, attachment2))).toString() + } + } + "Delete All" should { val req = FakeRequest(method = "DELETE", path = "/file") @@ -82,8 +145,11 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic } "return 204 if the test mode is enabled" in { + val attachment = FileMetadata(id = "id", fileName = Some("file"), mimeType = Some("type")) + when(appConfig.isTestMode).thenReturn(true) - when(service.deleteAll()).thenReturn(successful(())) + when(service.deleteAll()(any[HeaderCarrier])).thenReturn(successful(())) + when(service.find(eqTo("id"))(any[HeaderCarrier])).thenReturn(successful(Some(attachment))) val result = await(controller.deleteAll()(req)) @@ -94,7 +160,7 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic val error = new RuntimeException when(appConfig.isTestMode).thenReturn(true) - when(service.deleteAll()).thenReturn(failed(error)) + when(service.deleteAll()(any[HeaderCarrier])).thenReturn(Future.failed(error)) val result = await(controller.deleteAll()(req)) @@ -106,14 +172,15 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic "Delete By ID" should { - val id = "ABC-123_000" - val req = FakeRequest(method = "DELETE", path = s"/file/$id") + val attachment = FileMetadata(id = "id", fileName = Some("file"), mimeType = Some("type")) + val req = FakeRequest(method = "DELETE", path = s"/file/${attachment.id}") "return 204" in { when(appConfig.isTestMode).thenReturn(true) - when(service.delete(id)).thenReturn(successful((): Unit)) + when(service.delete(eqTo(attachment.id), eqTo(attachment.fileName.get))(any[HeaderCarrier])) + .thenReturn(Future.successful((): Unit)) - val result = await(controller.delete(id)(req)) + val result = await(controller.delete(attachment.id, attachment.fileName.get)(req)) status(result) shouldBe NO_CONTENT } @@ -122,9 +189,10 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic val error = new RuntimeException when(appConfig.isTestMode).thenReturn(true) - when(service.delete(id)).thenReturn(failed(error)) + when(service.delete(eqTo(attachment.id), eqTo(attachment.fileName.get))(any[HeaderCarrier])) + .thenReturn(Future.failed(error)) - val result = await(controller.delete(id)(req)) + val result = await(controller.delete(attachment.id, attachment.fileName.get)(req)) status(result) shouldEqual INTERNAL_SERVER_ERROR jsonBodyOf(result).toString() shouldEqual """{"code":"UNKNOWN_ERROR","message":"An unexpected error occurred"}""" @@ -132,64 +200,6 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic } - "Get By ID" should { - "return 200 when found" in { - val attachment = FileMetadata(id = "id", fileName = Some("file"), mimeType = Some("type")) - when(service.find(id = "id")).thenReturn(successful(Some(attachment))) - - val result = await(controller.get("id")(fakeRequest)) - - status(result) shouldBe OK - bodyOf(result) shouldEqual Json.toJson(attachment).toString() - } - - "return 404 when not found" in { - when(service.find(id = "id")).thenReturn(successful(None)) - - val result = await(controller.get("id")(fakeRequest)) - - status(result) shouldBe NOT_FOUND - } - } - - "Get By Search" should { - "return 200 with empty array" in { - when(service.find(Search(ids = Some(Set.empty)), Pagination.max)) - .thenReturn(successful(Paged.empty[FileMetadata])) - - val result = await(controller.getAll(Search(ids = Some(Set.empty)), None)(fakeRequest)) - - status(result) shouldBe OK - bodyOf(result) shouldEqual Json.toJson(Seq.empty).toString() - } - - "return 200 with non empty array" in { - val attachment1 = FileMetadata(id = "id1", fileName = Some("file1"), mimeType = Some("type1")) - val attachment2 = FileMetadata(id = "id2", fileName = Some("file2"), mimeType = Some("type2")) - - when(service.find(Search(ids = Some(Set("id1", "id2"))), Pagination.max)) - .thenReturn(successful(Paged(Seq(attachment1, attachment2)))) - - val result = await(controller.getAll(Search(ids = Some(Set("id1", "id2"))), None)(fakeRequest)) - - status(result) shouldBe OK - bodyOf(result) shouldEqual Json.toJson(Seq(attachment1, attachment2)).toString() - } - - "return 200 with pagination and non empty pager" in { - val attachment1 = FileMetadata(id = "id1", fileName = Some("file1"), mimeType = Some("type1")) - val attachment2 = FileMetadata(id = "id2", fileName = Some("file2"), mimeType = Some("type2")) - - when(service.find(Search(ids = Some(Set("id1", "id2"))), Pagination())) - .thenReturn(successful(Paged(Seq(attachment1, attachment2)))) - - val result = await(controller.getAll(Search(ids = Some(Set("id1", "id2"))), Some(Pagination()))(fakeRequest)) - - status(result) shouldBe OK - bodyOf(result) shouldEqual Json.toJson(Paged(Seq(attachment1, attachment2))).toString() - } - } - "Notify" should { "return 201 when found" in { val scanResult = SuccessfulScanResult( @@ -200,7 +210,7 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic val attachment = FileMetadata(id = "id", fileName = Some("file"), mimeType = Some("type")) val attachmentUpdated = FileMetadata(id = "id", fileName = Some("file"), mimeType = Some("type"), url = Some("url")) - when(service.find(id = "id")).thenReturn(successful(Some(attachment))) + when(service.find(eqTo("id"))(any[HeaderCarrier])).thenReturn(successful(Some(attachment))) when(service.notify(refEq(attachment), refEq(scanResult))(any[HeaderCarrier])) .thenReturn(successful(Some(attachmentUpdated))) @@ -217,7 +227,7 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic downloadUrl = "url", uploadDetails = UploadDetails("file", "type", Instant.now(), "checksum") ) - when(service.find("id")).thenReturn(successful(None)) + when(service.find(eqTo("id"))(any[HeaderCarrier])).thenReturn(successful(None)) val request: FakeRequest[JsValue] = fakeRequest.withBody(Json.toJson[ScanResult](scanResult)) val result: Result = await(controller.notification(id = "id")(request)) @@ -235,7 +245,7 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic ) val attachment: FileMetadata = FileMetadata(id = "id", fileName = Some("file"), mimeType = Some("type")) - when(service.find(id = "id")).thenReturn(successful(Some(attachment))) + when(service.find(eqTo("id"))(any[HeaderCarrier])).thenReturn(successful(Some(attachment))) when(service.notify(refEq(attachment), refEq(scanResult))(any[HeaderCarrier])) .thenThrow( new MongoWriteException( @@ -278,7 +288,7 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic scanStatus = Some(ScanStatus.READY), url = Some("url") ) - when(service.find(id = "id")).thenReturn(successful(Some(attachmentExisting))) + when(service.find(eqTo("id"))(any[HeaderCarrier])).thenReturn(successful(Some(attachmentExisting))) when(service.publish(refEq(attachmentExisting))(any[HeaderCarrier])) .thenReturn(successful(Some(attachmentUpdated))) @@ -289,7 +299,7 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic } "return 404 when not found" in { - when(service.find(id = "id")).thenReturn(successful(None)) + when(service.find(eqTo("id"))(any[HeaderCarrier])).thenReturn(successful(None)) val result: Result = await(controller.publish(id = "id")(fakeRequest)) @@ -299,8 +309,9 @@ class FileStoreControllerSpec extends UnitSpec with Matchers with WithFakeApplic "return 404 when publish returns not found" in { val attachmentExisting = FileMetadata(id = "id", fileName = Some("file"), mimeType = Some("type"), scanStatus = Some(ScanStatus.READY)) - when(service.find(id = "id")).thenReturn(successful(Some(attachmentExisting))) - when(service.publish(refEq(attachmentExisting))(any[HeaderCarrier])).thenReturn(successful(None)) + + when(service.find(eqTo("id"))(any[HeaderCarrier])).thenReturn(successful(Some(attachmentExisting))) + when(service.publish(refEq(attachmentExisting))(any[HeaderCarrier])).thenReturn(Future.successful(None)) val result: Result = await(controller.publish(id = "id")(fakeRequest)) diff --git a/test/uk/gov/hmrc/bindingtarifffilestore/service/FileStoreServiceSpec.scala b/test/uk/gov/hmrc/bindingtarifffilestore/service/FileStoreServiceSpec.scala index eadfc45..04f2c3c 100644 --- a/test/uk/gov/hmrc/bindingtarifffilestore/service/FileStoreServiceSpec.scala +++ b/test/uk/gov/hmrc/bindingtarifffilestore/service/FileStoreServiceSpec.scala @@ -18,36 +18,35 @@ package uk.gov.hmrc.bindingtarifffilestore.service import org.mockito.ArgumentCaptor import org.mockito.ArgumentMatchers.any -import org.mockito.Mockito._ +import org.mockito.Mockito.* import org.scalatest.BeforeAndAfterEach import org.scalatest.concurrent.Eventually import play.api.libs.Files.TemporaryFile import uk.gov.hmrc.bindingtarifffilestore.audit.AuditService import uk.gov.hmrc.bindingtarifffilestore.config.{AppConfig, FileStoreSizeConfiguration} -import uk.gov.hmrc.bindingtarifffilestore.connector.{AmazonS3Connector, UpscanConnector} -import uk.gov.hmrc.bindingtarifffilestore.model._ -import uk.gov.hmrc.bindingtarifffilestore.model.upscan._ +import uk.gov.hmrc.bindingtarifffilestore.connector.{ObjectStoreConnector, UpscanConnector} +import uk.gov.hmrc.bindingtarifffilestore.model.* +import uk.gov.hmrc.bindingtarifffilestore.model.upscan.* import uk.gov.hmrc.bindingtarifffilestore.repository.FileMetadataMongoRepository import uk.gov.hmrc.bindingtarifffilestore.util.UnitSpec import uk.gov.hmrc.http.HeaderCarrier import java.time.Instant -import scala.concurrent.ExecutionContext +import scala.concurrent.{ExecutionContext, Future} import scala.concurrent.ExecutionContext.Implicits.global import scala.concurrent.Future.{failed, successful} class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventually { - private val config: AppConfig = mock(classOf[AppConfig]) - private val s3Connector: AmazonS3Connector = mock(classOf[AmazonS3Connector]) - private val repository: FileMetadataMongoRepository = mock(classOf[FileMetadataMongoRepository]) - private val upscanConnector: UpscanConnector = mock(classOf[UpscanConnector]) - private val auditService: AuditService = mock(classOf[AuditService]) - - private implicit val hc: HeaderCarrier = HeaderCarrier() + private val config: AppConfig = mock(classOf[AppConfig]) + private val objectStoreConnector: ObjectStoreConnector = mock(classOf[ObjectStoreConnector]) + private val repository: FileMetadataMongoRepository = mock(classOf[FileMetadataMongoRepository]) + private val upscanConnector: UpscanConnector = mock(classOf[UpscanConnector]) + private val auditService: AuditService = mock(classOf[AuditService]) + private implicit lazy val hc: HeaderCarrier = mock(classOf[HeaderCarrier]) private val service: FileStoreService = - new FileStoreService(config, s3Connector, repository, upscanConnector, auditService) + new FileStoreService(config, objectStoreConnector, repository, upscanConnector, auditService) private final val emulatedFailure: RuntimeException = new RuntimeException("Emulated failure.") @@ -56,7 +55,7 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua override protected def afterEach(): Unit = { super.afterEach() reset(config) - reset(s3Connector) + reset(objectStoreConnector) reset(repository) reset(upscanConnector) reset(auditService) @@ -70,7 +69,7 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua await(service.deleteAll()) shouldBe ((): Unit) verify(repository).deleteAll() - verify(s3Connector).deleteAll() + verify(objectStoreConnector).deleteAll() } "Propagate any error" in { @@ -88,10 +87,10 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua "Clear the Database & File Store" in { when(repository.delete("id")).thenReturn(successful(())) - await(service.delete("id")) shouldBe ((): Unit) + await(service.delete("id", "fileName")) shouldBe ((): Unit) verify(repository).delete("id") - verify(s3Connector).delete("id") + verify(objectStoreConnector).delete("fileName") } "Propagate any error" in { @@ -112,7 +111,7 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua when(attachment.published).thenReturn(true) when(repository.get("id")).thenReturn(successful(Some(attachment))) - when(s3Connector.sign(attachment)).thenReturn(attachmentSigned) + when(objectStoreConnector.sign(attachment)).thenReturn(Future.successful(attachmentSigned)) await(service.find("id")) shouldBe Some(attachmentSigned) } @@ -123,7 +122,7 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua await(service.find("id")) shouldBe Some(attachment) - verify(s3Connector, never).sign(any[FileMetadata]) + verify(objectStoreConnector, never).sign(any[FileMetadata])(any[HeaderCarrier]) } } @@ -142,7 +141,7 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua val attachment2 = mock(classOf[FileMetadata], "attachment2") when(attachment1.published).thenReturn(true) - when(s3Connector.sign(attachment1)).thenReturn(attSigned1) + when(objectStoreConnector.sign(attachment1)).thenReturn(successful(attSigned1)) when(attachment2.published).thenReturn(false) @@ -332,9 +331,9 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua when(attachmentUploadedUpdated.isLive).thenReturn(true) when(repository.update(attachmentUpdating)).thenReturn(successful(Some(attachmentUpdated))) - when(s3Connector.upload(attachmentUpdated)).thenReturn(attachmentUploaded) + when(objectStoreConnector.upload(attachmentUpdated)).thenReturn(attachmentUploaded) when(repository.update(attachmentUploadedUpdating)).thenReturn(successful(Some(attachmentUploadedUpdated))) - when(s3Connector.sign(attachmentUploadedUpdated)).thenReturn(attachmentSigned) + when(objectStoreConnector.sign(attachmentUploadedUpdated)).thenReturn(successful(attachmentSigned)) val test = await(service.notify(attachment, scanResult)) test shouldBe Some(attachmentSigned) @@ -369,8 +368,8 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua await(service.notify(attachment, scanResult)) shouldBe None - verify(s3Connector, never).upload(any[FileMetadata]) - verify(s3Connector, never).sign(any[FileMetadata]) + verify(objectStoreConnector, never()).upload(any[FileMetadata]())(any[HeaderCarrier]()) + verify(objectStoreConnector, never()).sign(any[FileMetadata]())(any[HeaderCarrier]()) verify(auditService, times(1)) .auditFileScanned(fileId = "id", fileName = Some("file"), upScanRef = "ref", upScanStatus = "READY") verify(auditService, never).auditFilePublished(fileId = "id", fileName = "file") @@ -415,9 +414,9 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua when(fileUpdated.published).thenReturn(true) - when(s3Connector.upload(fileUploading)).thenReturn(fileUploaded) + when(objectStoreConnector.upload(fileUploading)).thenReturn(fileUploaded) when(repository.update(any[FileMetadata])).thenReturn(successful(Some(fileUpdated))) - when(s3Connector.sign(fileUpdated)).thenReturn(fileSigned) + when(objectStoreConnector.sign(fileUpdated)).thenReturn(Future.successful(fileSigned)) await(service.publish(fileUploading)) shouldBe Some(fileSigned) @@ -438,7 +437,7 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua await(service.publish(fileUploading)) shouldBe None verify(repository).delete("id") - verifyNoInteractions(auditService, s3Connector) + verifyNoInteractions(auditService, objectStoreConnector) } "Not delegate to the File Store if pre published" in { @@ -449,7 +448,7 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua await(service.publish(fileUploading)) shouldBe Some(fileUploading) - verifyNoInteractions(auditService, s3Connector, repository) + verifyNoInteractions(auditService, objectStoreConnector, repository) } "Not delegate to the File Store if Scanned UnSafe" in { @@ -464,7 +463,7 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua await(service.publish(fileUploading)) shouldBe Some(fileUpdated) - verifyNoInteractions(auditService, s3Connector) + verifyNoInteractions(auditService, objectStoreConnector) } "Not delegate to the File Store if Unscanned" in { @@ -479,7 +478,7 @@ class FileStoreServiceSpec extends UnitSpec with BeforeAndAfterEach with Eventua await(service.publish(fileUploading)) shouldBe Some(fileUpdated) - verifyNoInteractions(auditService, s3Connector) + verifyNoInteractions(auditService, objectStoreConnector) } } diff --git a/test/uk/gov/hmrc/bindingtarifffilestore/util/WithFakeApplication.scala b/test/uk/gov/hmrc/bindingtarifffilestore/util/WithFakeApplication.scala index 5a591f8..5c75d93 100644 --- a/test/uk/gov/hmrc/bindingtarifffilestore/util/WithFakeApplication.scala +++ b/test/uk/gov/hmrc/bindingtarifffilestore/util/WithFakeApplication.scala @@ -17,6 +17,7 @@ package uk.gov.hmrc.bindingtarifffilestore.util import org.scalatest.{BeforeAndAfterAll, Suite} +import org.apache.pekko.stream.Materializer import play.api.inject.guice.{GuiceApplicationBuilder, GuiceableModule} import play.api.test.Helpers._ import play.api.{Application, Play} @@ -28,7 +29,11 @@ import play.api.{Application, Play} trait WithFakeApplication extends BeforeAndAfterAll { this: Suite => - lazy val fakeApplication: Application = new GuiceApplicationBuilder().bindings(bindModules*).build() + final implicit val materializer: Materializer = fakeApplication.materializer + + lazy val fakeApplication: Application = new GuiceApplicationBuilder() + .bindings(bindModules*) + .build() def bindModules: Seq[GuiceableModule] = Seq() From 006c717d1b19ab39dfaa362bad248c7d16e22c55 Mon Sep 17 00:00:00 2001 From: SachaMcCormack <175815565+SachaMcCormack@users.noreply.github.com> Date: Thu, 30 Oct 2025 13:56:43 +0000 Subject: [PATCH 6/8] changes to objectstore --- .../config/AppConfig.scala | 13 +- .../connector/AmazonS3Connector.scala | 132 ---------- .../connector/ObjectStoreConnector.scala | 5 +- conf/app.routes | 2 +- .../connector/AmazonS3ConnectorSpec.scala | 227 ------------------ 5 files changed, 6 insertions(+), 373 deletions(-) delete mode 100644 app/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3Connector.scala delete mode 100644 test/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3ConnectorSpec.scala diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala b/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala index aa2d2b6..ffdfad1 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala @@ -26,7 +26,9 @@ class AppConfig @Inject() ( servicesConfig: ServicesConfig ) { - lazy val authorization: String = config.get[String]("auth.api-token") + lazy val authorization: String = config.get[String]("internal-auth.token") + + lazy val appName: String = config.get[String]("appName") lazy val upscanInitiateUrl: String = servicesConfig.baseUrl("upscan-initiate") lazy val fileStoreSizeConfiguration: FileStoreSizeConfiguration = FileStoreSizeConfiguration( @@ -46,15 +48,6 @@ class AppConfig @Inject() ( lazy val isTestMode: Boolean = config.getOptional[Boolean]("testMode").getOrElse(false) } -case class S3Configuration( - region: String, - bucket: String, - endpoint: Option[String] -) { - - def baseUrl: String = endpoint.getOrElse(s"https://s3-$region.amazonaws.com") -} - case class FileStoreSizeConfiguration( minFileSize: Int, maxFileSize: Int diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3Connector.scala b/app/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3Connector.scala deleted file mode 100644 index 850a434..0000000 --- a/app/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3Connector.scala +++ /dev/null @@ -1,132 +0,0 @@ -/* - * Copyright 2025 HM Revenue & Customs - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package uk.gov.hmrc.bindingtarifffilestore.connector - -import com.amazonaws.auth.{AWSCredentials, BasicAWSCredentials, DefaultAWSCredentialsProviderChain} - -import java.io.BufferedInputStream -import java.net.URL -import java.util -import com.amazonaws.{AmazonClientException, HttpMethod} -import com.amazonaws.client.builder.AwsClientBuilder.EndpointConfiguration -import com.amazonaws.services.s3.model.DeleteObjectsRequest.KeyVersion -import com.amazonaws.services.s3.model._ -import com.amazonaws.services.s3.{AmazonS3, AmazonS3ClientBuilder} -import com.google.inject.Inject - -import javax.inject.Singleton -import uk.gov.hmrc.bindingtarifffilestore.config.AppConfig -import uk.gov.hmrc.bindingtarifffilestore.model.FileMetadata -import uk.gov.hmrc.bindingtarifffilestore.util.Logging - -import scala.jdk.CollectionConverters._ -import scala.util.{Failure, Success, Try} - -@Singleton -class AmazonS3Connector @Inject() (config: AppConfig) extends Logging { - - private lazy val s3Config = config.s3Configuration - - private lazy val s3client: AmazonS3 = { - log.info(s"${s3Config.bucket}:${s3Config.region}") - val builder = AmazonS3ClientBuilder - .standard() - .withPathStyleAccessEnabled(true) - .withCredentials(new LocalDevelopmentS3CredentialsProviderChain()) - - s3Config.endpoint match { - case Some(endpoint) => builder.withEndpointConfiguration(new EndpointConfiguration(endpoint, s3Config.region)) - case _ => builder.withRegion(s3Config.region) - } - - builder.build() - } - - def getAll: Seq[String] = - sequenceOf( - s3client.listObjects(s3Config.bucket).getObjectSummaries - ).map(_.getKey) - - def upload(fileMetaData: FileMetadata): FileMetadata = { - val url: URL = new URL(fileMetaData.url.getOrElse(throw new IllegalArgumentException("Missing URL"))) - - val metadata = new ObjectMetadata - // This .get is scary but our file must have received a positive scan - // result and received metadata from Upscan if it is being published - metadata.setContentType(fileMetaData.mimeType.get) - metadata.setContentLength(contentLengthOf(url)) - - val request = new PutObjectRequest( - s3Config.bucket, - fileMetaData.id, - new BufferedInputStream(url.openStream()), - metadata - ).withCannedAcl(CannedAccessControlList.Private) - - Try(s3client.putObject(request)) match { - case Success(_) => - fileMetaData.copy(url = Some(s"${s3Config.baseUrl}/${s3Config.bucket}/${fileMetaData.id}")) - case Failure(e: Throwable) => - log.error("Failed to upload to the S3 bucket.", e) - throw e - } - } - - def delete(id: String): Unit = - s3client.deleteObject(s3Config.bucket, id) - - def deleteAll(): Unit = { - val keys: Seq[KeyVersion] = getAll.map(new KeyVersion(_)) - if (keys.nonEmpty) { - log.info(s"Removing [${keys.length}] files from S3") - log.info(s"bucket is: ${s3Config.bucket}") - val request = new DeleteObjectsRequest(s3Config.bucket) - .withKeys(keys.toList.asJava) - .withQuiet(false) - s3client.deleteObjects(request) - } else { - log.info(s"No files to remove from S3") - } - } - - def sign(fileMetaData: FileMetadata): FileMetadata = - if (fileMetaData.url.isDefined) { - val authenticatedURLRequest = new GeneratePresignedUrlRequest(config.s3Configuration.bucket, fileMetaData.id) - .withMethod(HttpMethod.GET) - val authenticatedURL: URL = s3client.generatePresignedUrl(authenticatedURLRequest) - fileMetaData.copy(url = Some(authenticatedURL.toString)) - } else { - fileMetaData - } - - private def contentLengthOf(url: URL): Long = - url.openConnection.getContentLengthLong - - private def sequenceOf[T](list: util.List[T]): Seq[T] = - list.iterator.asScala.toSeq - -} - -class LocalDevelopmentS3CredentialsProviderChain() extends DefaultAWSCredentialsProviderChain { - - override def getCredentials(): AWSCredentials = - Try { - super.getCredentials() - }.recover { case _: AmazonClientException => - new BasicAWSCredentials("dummy-access-key", "dummy-secret-key") - }.get -} diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala b/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala index f6fac6b..3db49c2 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala @@ -48,12 +48,11 @@ class ObjectStoreConnector @Inject() (client: PlayObjectStoreClient, config: App client .uploadFromUrl( from = new URI(fileMetaData.url.getOrElse(throw new IllegalArgumentException("Missing URL"))).toURL, - to = directory.file(fileMetaData.fileName.getOrElse("")), - contentSha256 = Some(Sha256Checksum.fromHex(fileMetaData.fileName.get)) + to = directory.file(fileMetaData.fileName.getOrElse("")) ) ) match { case Success(_) => - fileMetaData.copy(url = Some(s"${config.objectStoreUrl}/${fileMetaData.fileName.get}")) + fileMetaData.copy(url = Some(s"${config.filestoreUrl}/${fileMetaData.id}")) case Failure(e: Throwable) => log.error("Failed to upload to the object store.", e) throw e diff --git a/conf/app.routes b/conf/app.routes index 099b62f..ce0cf68 100644 --- a/conf/app.routes +++ b/conf/app.routes @@ -10,4 +10,4 @@ POST /file/:id/publish @uk.gov.hmrc.bindingtarifffilestore.controll # admin/test endpoints DELETE /file @uk.gov.hmrc.bindingtarifffilestore.controllers.FileStoreController.deleteAll() -DELETE /file/:id @uk.gov.hmrc.bindingtarifffilestore.controllers.FileStoreController.delete(id: String, filename: String) +DELETE /file/:id @uk.gov.hmrc.bindingtarifffilestore.controllers.FileStoreController.delete(id: String, name: String) diff --git a/test/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3ConnectorSpec.scala b/test/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3ConnectorSpec.scala deleted file mode 100644 index 5bc6ab9..0000000 --- a/test/uk/gov/hmrc/bindingtarifffilestore/connector/AmazonS3ConnectorSpec.scala +++ /dev/null @@ -1,227 +0,0 @@ -/* - * Copyright 2025 HM Revenue & Customs - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package uk.gov.hmrc.bindingtarifffilestore.connector - -import com.amazonaws.services.s3.model.AmazonS3Exception -import com.github.tomakehurst.wiremock.client.WireMock -import com.github.tomakehurst.wiremock.client.WireMock.* -import org.mockito.Mockito.{mock, when} -import org.scalatest.BeforeAndAfterEach -import play.api.http.Status -import play.api.libs.Files.SingletonTemporaryFileCreator -import uk.gov.hmrc.bindingtarifffilestore.config.{AppConfig, S3Configuration} -import uk.gov.hmrc.bindingtarifffilestore.model.FileMetadata -import uk.gov.hmrc.bindingtarifffilestore.util.* - -import java.time.LocalDate -import java.time.format.DateTimeFormatter - -class AmazonS3ConnectorSpec extends UnitSpec with WiremockTestServer with BeforeAndAfterEach with ResourceFiles { - - private val s3Config: S3Configuration = - S3Configuration("region", "bucket", Some(s"http://localhost:$wirePort")) - private val config = mock(classOf[AppConfig]) - private val date = LocalDate.now().format(DateTimeFormatter.ofPattern("YYYYMMdd")) - private val connector = new AmazonS3Connector(config) - - override protected def beforeEach(): Unit = { - super.beforeEach() - when(config.s3Configuration).thenReturn(s3Config) - } - - "Get All" should { - - "Delegate to S3" in { - stubFor( - get("/bucket/?encoding-type=url") - .withHeader( - "Authorization", - matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") - ) - .willReturn( - aResponse() - .withStatus(Status.OK) - .withBody(fromFile("aws/list-objects_response.xml")) - ) - ) - - // When - val all: Seq[String] = connector.getAll - - // Then - all should have size 1 - all.head shouldBe "image.jpg" - } - - } - - "Upload" should { - - "Delegate to S3" in { - stubFor( - put("/bucket/id") - .withHeader( - "Authorization", - matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") - ) - .withHeader("Content-Type", equalTo("text/plain")) - .willReturn( - aResponse() - .withStatus(Status.OK) - ) - ) - - val url = SingletonTemporaryFileCreator.create("example.txt").path.toUri.toURL.toString - val fileUploading = FileMetadata("id", Some("file.txt"), Some("text/plain"), Some(url)) - - // Then - val result = connector.upload(fileUploading) - result.id shouldBe "id" - result.fileName shouldBe Some("file.txt") - result.mimeType shouldBe Some("text/plain") - result.url.get shouldBe s"$wireMockUrl/bucket/id" - } - - "Throw Exception on missing URL" in { - val fileUploading = FileMetadata("id", Some("file.txt"), Some("text/plain")) - - val exception = intercept[IllegalArgumentException] { - connector.upload(fileUploading) - } - exception.getMessage shouldBe "Missing URL" - } - - "Throw Exception on upload failure" in { - stubFor( - put("/bucket/id") - .withHeader( - "Authorization", - matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") - ) - .withHeader("Content-Type", equalTo("text/plain")) - .willReturn( - aResponse() - .withStatus(Status.BAD_GATEWAY) - ) - ) - val url = SingletonTemporaryFileCreator.create("example.txt").path.toUri.toURL.toString - val fileUploading = FileMetadata("id", Some("file.txt"), Some("text/plain"), Some(url)) - - // Then - val exception = intercept[AmazonS3Exception] { - connector.upload(fileUploading) - } - exception.getMessage shouldBe - """Bad Gateway (Service: Amazon S3; - | Status Code: 502; - | Error Code: 502 Bad Gateway; - | Request ID: null; - | S3 Extended Request ID: null; - | Proxy: null)""".stripMargin.replaceAll("\n", "") - } - } - - "Sign" should { - "append token to URL" in { - val file = FileMetadata("id", Some("file.txt"), Some("text/plain"), Some("url")) - - connector.sign(file).url.get should startWith(s"$wireMockUrl/bucket/id?") - connector.sign(file).url.get should include("X-Amz-Algorithm=AWS4-HMAC-SHA256") - } - - "not append token to empty URL" in { - val file = FileMetadata("id", Some("file.txt"), Some("text/plain"), None) - - connector.sign(file).url shouldBe None - } - } - - "Delete All" should { - "Delegate to S3" in { - stubFor( - get("/bucket/?encoding-type=url") - .withHeader( - "Authorization", - matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") - ) - .willReturn( - aResponse() - .withStatus(Status.OK) - .withBody(fromFile("aws/list-objects_response.xml")) - ) - ) - stubFor( - post("/bucket/?delete") - .withHeader( - "Authorization", - matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") - ) - .willReturn( - aResponse() - .withStatus(Status.OK) - .withBody(fromFile("aws/delete-objects_response.xml")) - ) - ) - - connector.deleteAll() - - WireMock.verify( - postRequestedFor(urlEqualTo("/bucket/?delete")) - ) - } - - "Do nothing for no files" in { - stubFor( - get("/bucket/?encoding-type=url") - .withHeader( - "Authorization", - matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") - ) - .willReturn( - aResponse() - .withStatus(Status.OK) - .withBody(fromFile("aws/empty-list-objects_response.xml")) - ) - ) - - connector.deleteAll() - - WireMock.verify(0, postRequestedFor(urlEqualTo("/bucket/?delete"))) - } - } - - "Delete One" should { - "Delegate to S3" in { - stubFor( - delete("/bucket/id") - .withHeader( - "Authorization", - matching(s"AWS4-HMAC-SHA256 Credential=(.*)/$date/${s3Config.region}/s3/aws4_request, .*") - ) - .willReturn( - aResponse() - .withStatus(Status.OK) - ) - ) - - connector.delete("id") - - WireMock.verify(deleteRequestedFor(urlEqualTo("/bucket/id"))) - } - } - -} From b50cf799b943b983d7f9252a2d44d5a6289dd7c5 Mon Sep 17 00:00:00 2001 From: SachaMcCormack <175815565+SachaMcCormack@users.noreply.github.com> Date: Wed, 5 Nov 2025 14:16:16 +0000 Subject: [PATCH 7/8] removed 403s --- .../bindingtarifffilestore/config/AppConfig.scala | 8 ++++---- .../connector/ObjectStoreConnector.scala | 12 +++++++----- conf/application.conf | 4 ++-- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala b/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala index ffdfad1..48cd33c 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala @@ -26,7 +26,7 @@ class AppConfig @Inject() ( servicesConfig: ServicesConfig ) { - lazy val authorization: String = config.get[String]("internal-auth.token") + lazy val authorization: String = config.get[String]("auth.api-token") lazy val appName: String = config.get[String]("appName") @@ -41,9 +41,9 @@ class AppConfig @Inject() ( lazy val filestoreUrl: String = config.get[String]("filestore.url") lazy val filestoreSSL: Boolean = config.get[Boolean]("filestore.ssl") - private lazy val objectStoreHost: String = config.get[String]("microservice.services.object-store.host") - private lazy val objectStorePort: String = config.get[String]("microservice.services.object-store.port") - lazy val objectStoreUrl: String = s"http://$objectStoreHost:$objectStorePort" +// private lazy val objectStoreHost: String = config.get[String]("microservice.services.object-store.host") +// private lazy val objectStorePort: String = config.get[String]("microservice.services.object-store.port") +// lazy val objectStoreUrl: String = s"http://$objectStoreHost:$objectStorePort" lazy val isTestMode: Boolean = config.getOptional[Boolean]("testMode").getOrElse(false) } diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala b/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala index 3db49c2..1c03179 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala @@ -16,22 +16,24 @@ package uk.gov.hmrc.bindingtarifffilestore.connector -import uk.gov.hmrc.objectstore.client.play.PlayObjectStoreClient import com.google.inject.Inject - -import javax.inject.Singleton import uk.gov.hmrc.bindingtarifffilestore.config.AppConfig import uk.gov.hmrc.bindingtarifffilestore.model.FileMetadata import uk.gov.hmrc.bindingtarifffilestore.util.Logging import uk.gov.hmrc.http.HeaderCarrier -import uk.gov.hmrc.objectstore.client.{ObjectSummary, Path, Sha256Checksum} +import uk.gov.hmrc.objectstore.client.play.PlayObjectStoreClient +import uk.gov.hmrc.objectstore.client.{ObjectSummary, Path} import java.net.URI +import javax.inject.Singleton import scala.concurrent.{ExecutionContext, Future} import scala.util.{Failure, Success, Try} @Singleton -class ObjectStoreConnector @Inject() (client: PlayObjectStoreClient, config: AppConfig)(implicit +class ObjectStoreConnector @Inject() ( + client: PlayObjectStoreClient, + config: AppConfig +)(implicit val ec: ExecutionContext ) extends Logging { diff --git a/conf/application.conf b/conf/application.conf index 4b67273..3ac3c78 100644 --- a/conf/application.conf +++ b/conf/application.conf @@ -111,6 +111,6 @@ auth { api-token = "9253947-99f3-47d7-9af2-b75b4f37fd34" } -internal-auth.token = "" +internal-auth.token = "1234" -object-store.default-retention-period = "1-month" \ No newline at end of file +object-store.default-retention-period = "1-month" From b660d281648b30c8c41ba148cd41a47a786cb27c Mon Sep 17 00:00:00 2001 From: SachaMcCormack <175815565+SachaMcCormack@users.noreply.github.com> Date: Thu, 6 Nov 2025 14:26:38 +0000 Subject: [PATCH 8/8] fixed url issues --- app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala | 4 ---- .../connector/ObjectStoreConnector.scala | 3 ++- .../uk/gov/hmrc/bindingtarifffilestore/FileStoreSpec.scala | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala b/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala index 48cd33c..9af72d9 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/config/AppConfig.scala @@ -41,10 +41,6 @@ class AppConfig @Inject() ( lazy val filestoreUrl: String = config.get[String]("filestore.url") lazy val filestoreSSL: Boolean = config.get[Boolean]("filestore.ssl") -// private lazy val objectStoreHost: String = config.get[String]("microservice.services.object-store.host") -// private lazy val objectStorePort: String = config.get[String]("microservice.services.object-store.port") -// lazy val objectStoreUrl: String = s"http://$objectStoreHost:$objectStorePort" - lazy val isTestMode: Boolean = config.getOptional[Boolean]("testMode").getOrElse(false) } diff --git a/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala b/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala index 1c03179..b330b4e 100644 --- a/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala +++ b/app/uk/gov/hmrc/bindingtarifffilestore/connector/ObjectStoreConnector.scala @@ -54,7 +54,8 @@ class ObjectStoreConnector @Inject() ( ) ) match { case Success(_) => - fileMetaData.copy(url = Some(s"${config.filestoreUrl}/${fileMetaData.id}")) + log.info("File uploaded to Object Store") + fileMetaData case Failure(e: Throwable) => log.error("Failed to upload to the object store.", e) throw e diff --git a/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreSpec.scala b/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreSpec.scala index d3e2011..adeb525 100644 --- a/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreSpec.scala +++ b/it/test/uk/gov/hmrc/bindingtarifffilestore/FileStoreSpec.scala @@ -411,7 +411,7 @@ class FileStoreSpec extends FileStoreHelpers with Eventually { And("The response shows the file published") - (result.json \\ "url").map(_.as[String]).headOption.getOrElse("") should include(s"$id1") + (result.json \\ "url").map(_.as[String]).headOption.getOrElse("") should include(s"$file1") } Scenario("Should mark an un-safe file as publishable, but not persist") {