From cbb87eccfb08d8c0d318c02d5efcdd1899b016df Mon Sep 17 00:00:00 2001 From: thanhz Date: Fri, 22 Sep 2023 15:32:04 +0100 Subject: [PATCH 01/10] initial commit --- build.sbt | 2 ++ src/main/resources/application.base.conf | 5 +++ src/main/resources/application.conf | 5 +++ .../tdr/api/http/ApiServer.scala | 2 ++ .../service/ReferenceGeneratorService.scala | 33 +++++++++++++++++++ 5 files changed, 47 insertions(+) create mode 100644 src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala diff --git a/build.sbt b/build.sbt index e81f5e5a4..8772acdf6 100644 --- a/build.sbt +++ b/build.sbt @@ -42,6 +42,7 @@ lazy val akkaHttpVersion = "10.2.10" lazy val circeVersion = "0.14.6" lazy val testContainersVersion = "0.41.0" val http4sVersion = "0.23.23" +val sttpVersion = "3.9.0" libraryDependencies ++= Seq( "org.sangria-graphql" %% "sangria" % "4.0.2", @@ -63,6 +64,7 @@ libraryDependencies ++= Seq( "io.circe" %% "circe-optics" % "0.14.1", "io.circe" %% "circe-generic" % circeVersion, "io.circe" %% "circe-generic-extras" % "0.14.3", + "com.softwaremill.sttp.client3" %% "core" % sttpVersion, "uk.gov.nationalarchives" %% "consignment-api-db" % "0.1.36", "org.postgresql" % "postgresql" % "42.6.0", "com.typesafe.slick" %% "slick" % "3.4.0", diff --git a/src/main/resources/application.base.conf b/src/main/resources/application.base.conf index 604d01a02..19befb2e5 100644 --- a/src/main/resources/application.base.conf +++ b/src/main/resources/application.base.conf @@ -33,8 +33,13 @@ fileUpload { featureAccessBlock { http4s = ${BLOCK_HTTP4S} + assignFileReferences = ${BLOCK_ASSIGN_FILE_REFERENCES} } +referenceGeneratorUrl = ${REFERENCE_GENERATOR_URL} + +environment = ${ENVIRONMENT} + akka.http { server { idle-timeout = 180 s diff --git a/src/main/resources/application.conf b/src/main/resources/application.conf index a221d217a..11659fcf8 100644 --- a/src/main/resources/application.conf +++ b/src/main/resources/application.conf @@ -12,8 +12,13 @@ frontend = { featureAccessBlock { http4s = true + assignFileReferences = true } +referenceGeneratorUrl = "https://q59yzinibd.execute-api.eu-west-2.amazonaws.com" + +environment = "intg" + consignmentapi = { useIamAuth = false db { diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/http/ApiServer.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/http/ApiServer.scala index 15dd71698..19123ea38 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/http/ApiServer.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/http/ApiServer.scala @@ -7,6 +7,7 @@ import org.postgresql.Driver import slick.basic.DatabaseConfig import slick.jdbc.{JdbcBackend, JdbcProfile} import slick.jdbc.hikaricp.HikariCPJdbcDataSource +import uk.gov.nationalarchives.tdr.api.service.ReferenceGeneratorService import scala.language.postfixOps @@ -18,6 +19,7 @@ object ApiServer extends IOApp { override def run(args: List[String]): IO[ExitCode] = { if (blockHttp4s) { + new ReferenceGeneratorService() val akkaHttpServer = new AkkaHttpServer() val serverBindingFuture = akkaHttpServer.start val finalIO = IO diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala new file mode 100644 index 000000000..d02e35835 --- /dev/null +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala @@ -0,0 +1,33 @@ +package uk.gov.nationalarchives.tdr.api.service + +import com.typesafe.config.{Config, ConfigFactory} +import io.circe.parser._ +import sttp.client3.{Response, SimpleHttpClient, UriContext, basicRequest} +import uk.gov.nationalarchives.tdr.api.service.ReferenceGeneratorService.reference + +class ReferenceGeneratorService() { + private val client: SimpleHttpClient = SimpleHttpClient() + val config: Config = ConfigFactory.load() + val environment: String = config.getString("environment") + val refGeneratorUrl: String = config.getString("referenceGeneratorUrl") + + try { + val response: Response[Either[String, reference]] = client.send(basicRequest.get(uri"$refGeneratorUrl/$environment/counter?numberofrefs=2")) + + response.body match { + case Left(body) => println(s"Non-2xx response to GET with code ${response.code}:\n$body") + case Right(body) => + val references = parse(body).getOrElse(null) + val listOfReferences = references.as[List[reference]].getOrElse(null) + println(s"2xx response to GET:\n$listOfReferences") + } + + println("---\n") + + } finally client.close() + +} + +object ReferenceGeneratorService { + type reference = String +} From 97cf7703d8c604a33c8f05a677e6bf5b7b473e68 Mon Sep 17 00:00:00 2001 From: Thanh Date: Tue, 3 Oct 2023 11:17:48 +0100 Subject: [PATCH 02/10] Assign references to files & folders --- .../tdr/api/http/ApiServer.scala | 1 - .../tdr/api/service/FileService.scala | 17 ++++++++-- .../service/ReferenceGeneratorService.scala | 31 ++++++++++--------- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/http/ApiServer.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/http/ApiServer.scala index 19123ea38..79795ccd4 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/http/ApiServer.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/http/ApiServer.scala @@ -19,7 +19,6 @@ object ApiServer extends IOApp { override def run(args: List[String]): IO[ExitCode] = { if (blockHttp4s) { - new ReferenceGeneratorService() val akkaHttpServer = new AkkaHttpServer() val serverBindingFuture = akkaHttpServer.start val finalIO = IO diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/FileService.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/FileService.scala index 09141a3ea..0f11d7542 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/FileService.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/FileService.scala @@ -16,6 +16,7 @@ import uk.gov.nationalarchives.tdr.api.model.file.NodeType.{FileTypeHelper, dire import uk.gov.nationalarchives.tdr.api.service.FileMetadataService._ import uk.gov.nationalarchives.tdr.api.service.FileService._ import uk.gov.nationalarchives.tdr.api.service.FileStatusService.{FFID, allFileStatusTypes, defaultStatuses} +import uk.gov.nationalarchives.tdr.api.service.ReferenceGeneratorService.reference import uk.gov.nationalarchives.tdr.api.utils.NaturalSorting.{ArrayOrdering, natural} import uk.gov.nationalarchives.tdr.api.utils.TimeUtils.LongUtils import uk.gov.nationalarchives.tdr.api.utils.TreeNodesUtils @@ -53,10 +54,22 @@ class FileService( val row: (UUID, String, String) => FilemetadataRow = FilemetadataRow(uuidSource.uuid, _, _, now, userId, _) val rows: Future[List[Rows]] = customMetadataPropertiesRepository.getCustomMetadataValuesWithDefault.map(filePropertyValue => { - ((allEmptyDirectoryNodes ++ allFileNodes) map { case (path, treeNode) => + val filesAndFolderCombined = allEmptyDirectoryNodes ++ allFileNodes + val generatedReferences = new ReferenceGeneratorService().getReferences(filesAndFolderCombined.size) + val fileAndFolderAssignedRef: Map[String, (TreeNode, reference)] = filesAndFolderCombined.keys.zip(filesAndFolderCombined.values.zip(generatedReferences)).toMap + (fileAndFolderAssignedRef map { case (path, (treeNode, reference)) => val parentId = treeNode.parentPath.map(path => allFileNodes.getOrElse(path, allEmptyDirectoryNodes(path)).id) val fileId = treeNode.id - val fileRow = FileRow(fileId, consignmentId, userId, now, filetype = Some(treeNode.treeNodeType), filename = Some(treeNode.name), parentid = parentId) + val fileRow = FileRow( + fileId, + consignmentId, + userId, + now, + filetype = Some(treeNode.treeNodeType), + filename = Some(treeNode.name), + parentid = parentId, + filereference = Some(reference) + ) val commonMetadataRows = List( row(fileId, path, ClientSideOriginalFilepath), diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala index d02e35835..f830e11d3 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala @@ -1,31 +1,32 @@ package uk.gov.nationalarchives.tdr.api.service import com.typesafe.config.{Config, ConfigFactory} +import com.typesafe.scalalogging.Logger import io.circe.parser._ import sttp.client3.{Response, SimpleHttpClient, UriContext, basicRequest} import uk.gov.nationalarchives.tdr.api.service.ReferenceGeneratorService.reference class ReferenceGeneratorService() { private val client: SimpleHttpClient = SimpleHttpClient() + val logger: Logger = Logger("ReferenceGeneratorService") val config: Config = ConfigFactory.load() val environment: String = config.getString("environment") val refGeneratorUrl: String = config.getString("referenceGeneratorUrl") - try { - val response: Response[Either[String, reference]] = client.send(basicRequest.get(uri"$refGeneratorUrl/$environment/counter?numberofrefs=2")) - - response.body match { - case Left(body) => println(s"Non-2xx response to GET with code ${response.code}:\n$body") - case Right(body) => - val references = parse(body).getOrElse(null) - val listOfReferences = references.as[List[reference]].getOrElse(null) - println(s"2xx response to GET:\n$listOfReferences") - } - - println("---\n") - - } finally client.close() - + def getReferences(numberOfRefs: Int): List[reference] = { + try { + val response: Response[Either[String, reference]] = client.send(basicRequest.get(uri"$refGeneratorUrl/$environment/counter?numberofrefs=$numberOfRefs")) + response.body match { + case Left(body) => logger.error(s"Non-2xx response to GET with code ${response.code}:\n$body") + Nil + case Right(body) => + val references = parse(body).getOrElse(null) + val listOfReferences = references.as[List[reference]].getOrElse(null) + logger.info(s"2xx response to GET:\n$listOfReferences") + listOfReferences + } + } finally client.close() + } } object ReferenceGeneratorService { From a538558afc5458e636b0bbe5fb2c729850eac801 Mon Sep 17 00:00:00 2001 From: Thanh Date: Fri, 6 Oct 2023 11:49:33 +0100 Subject: [PATCH 03/10] Recursively fetch references + unit test --- src/main/resources/application.base.conf | 6 +- src/main/resources/application.conf | 5 +- .../tdr/api/http/ApiServer.scala | 1 - .../tdr/api/http/GraphQLServerBase.scala | 3 + .../tdr/api/service/FileService.scala | 3 +- .../service/ReferenceGeneratorService.scala | 56 ++++++++++++++----- src/test/resources/application.conf | 5 ++ .../tdr/api/service/FileServiceSpec.scala | 13 +++++ .../ReferenceGeneratorServiceSpec.scala | 50 +++++++++++++++++ 9 files changed, 123 insertions(+), 19 deletions(-) create mode 100644 src/test/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorServiceSpec.scala diff --git a/src/main/resources/application.base.conf b/src/main/resources/application.base.conf index 19befb2e5..e97a754ec 100644 --- a/src/main/resources/application.base.conf +++ b/src/main/resources/application.base.conf @@ -36,7 +36,11 @@ featureAccessBlock { assignFileReferences = ${BLOCK_ASSIGN_FILE_REFERENCES} } -referenceGeneratorUrl = ${REFERENCE_GENERATOR_URL} +referenceGenerator { + referenceGeneratorUrl = ${REFERENCE_GENERATOR_URL} + referenceLimit = ${REFERENCE_GENERATOR_LIMIT} +} + environment = ${ENVIRONMENT} diff --git a/src/main/resources/application.conf b/src/main/resources/application.conf index 11659fcf8..b85d1e574 100644 --- a/src/main/resources/application.conf +++ b/src/main/resources/application.conf @@ -15,7 +15,10 @@ featureAccessBlock { assignFileReferences = true } -referenceGeneratorUrl = "https://q59yzinibd.execute-api.eu-west-2.amazonaws.com" +referenceGenerator { + referenceGeneratorUrl = "https://q59yzinibd.execute-api.eu-west-2.amazonaws.com" + referenceLimit = 500 +} environment = "intg" diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/http/ApiServer.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/http/ApiServer.scala index 79795ccd4..15dd71698 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/http/ApiServer.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/http/ApiServer.scala @@ -7,7 +7,6 @@ import org.postgresql.Driver import slick.basic.DatabaseConfig import slick.jdbc.{JdbcBackend, JdbcProfile} import slick.jdbc.hikaricp.HikariCPJdbcDataSource -import uk.gov.nationalarchives.tdr.api.service.ReferenceGeneratorService import scala.language.postfixOps diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/http/GraphQLServerBase.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/http/GraphQLServerBase.scala index d9d88a1e1..ee3693048 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/http/GraphQLServerBase.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/http/GraphQLServerBase.scala @@ -5,6 +5,7 @@ import com.typesafe.scalalogging.Logger import sangria.execution._ import sangria.marshalling.ResultMarshaller import slick.jdbc.JdbcBackend +import sttp.client3.SimpleHttpClient import uk.gov.nationalarchives.tdr.api.auth.AuthorisationException import uk.gov.nationalarchives.tdr.api.consignmentstatevalidation.ConsignmentStateException import uk.gov.nationalarchives.tdr.api.db.repository._ @@ -81,6 +82,7 @@ trait GraphQLServerBase { new FFIDMetadataService(ffidMetadataRepository, ffidMetadataMatchesRepository, timeSource, uuidSource) val fileStatusService = new FileStatusService(fileStatusRepository) val displayPropertiesService = new DisplayPropertiesService(displayPropertiesRepository) + val referenceGeneratorService = new ReferenceGeneratorService(config, SimpleHttpClient()) val fileService = new FileService( fileRepository, consignmentRepository, @@ -89,6 +91,7 @@ trait GraphQLServerBase { antivirusMetadataService, fileStatusService, fileMetadataService, + referenceGeneratorService, new CurrentTimeSource, uuidSource, config diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/FileService.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/FileService.scala index 0f11d7542..918966d84 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/FileService.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/FileService.scala @@ -35,6 +35,7 @@ class FileService( avMetadataService: AntivirusMetadataService, fileStatusService: FileStatusService, fileMetadataService: FileMetadataService, + referenceGeneratorService: ReferenceGeneratorService, timeSource: TimeSource, uuidSource: UUIDSource, config: Config @@ -55,7 +56,7 @@ class FileService( val row: (UUID, String, String) => FilemetadataRow = FilemetadataRow(uuidSource.uuid, _, _, now, userId, _) val rows: Future[List[Rows]] = customMetadataPropertiesRepository.getCustomMetadataValuesWithDefault.map(filePropertyValue => { val filesAndFolderCombined = allEmptyDirectoryNodes ++ allFileNodes - val generatedReferences = new ReferenceGeneratorService().getReferences(filesAndFolderCombined.size) + val generatedReferences = referenceGeneratorService.getReferences(filesAndFolderCombined.size) val fileAndFolderAssignedRef: Map[String, (TreeNode, reference)] = filesAndFolderCombined.keys.zip(filesAndFolderCombined.values.zip(generatedReferences)).toMap (fileAndFolderAssignedRef map { case (path, (treeNode, reference)) => val parentId = treeNode.parentPath.map(path => allFileNodes.getOrElse(path, allEmptyDirectoryNodes(path)).id) diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala index f830e11d3..569034790 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala @@ -3,29 +3,55 @@ package uk.gov.nationalarchives.tdr.api.service import com.typesafe.config.{Config, ConfigFactory} import com.typesafe.scalalogging.Logger import io.circe.parser._ +import org.apache.http.client.HttpResponseException import sttp.client3.{Response, SimpleHttpClient, UriContext, basicRequest} import uk.gov.nationalarchives.tdr.api.service.ReferenceGeneratorService.reference -class ReferenceGeneratorService() { - private val client: SimpleHttpClient = SimpleHttpClient() +import scala.annotation.tailrec + +class ReferenceGeneratorService(config: Config, client: SimpleHttpClient) { val logger: Logger = Logger("ReferenceGeneratorService") - val config: Config = ConfigFactory.load() val environment: String = config.getString("environment") - val refGeneratorUrl: String = config.getString("referenceGeneratorUrl") + val refGeneratorUrl: String = config.getString("referenceGenerator.referenceGeneratorUrl") + val refGeneratorLimit: Int = config.getInt("referenceGenerator.referenceLimit") def getReferences(numberOfRefs: Int): List[reference] = { - try { - val response: Response[Either[String, reference]] = client.send(basicRequest.get(uri"$refGeneratorUrl/$environment/counter?numberofrefs=$numberOfRefs")) - response.body match { - case Left(body) => logger.error(s"Non-2xx response to GET with code ${response.code}:\n$body") - Nil - case Right(body) => - val references = parse(body).getOrElse(null) - val listOfReferences = references.as[List[reference]].getOrElse(null) - logger.info(s"2xx response to GET:\n$listOfReferences") - listOfReferences + + def processResponse(response: Response[Either[String, reference]]): List[reference] = { + try { + response.body match { + case Left(body) => + logger.error(s"Non-2xx response to GET with code ${response.code}:\n$body") + throw new HttpResponseException(response.code.code, "Failed to get references from reference generator api") + case Right(body) => + val references = parse(body).getOrElse(null) + val listOfReferences = references.as[List[reference]].getOrElse(null) + logger.info(s"2xx response to GET:\n$listOfReferences") + listOfReferences + } + } finally { + client.close() } - } finally client.close() + } + + @tailrec + def recursivelyFetchReferences(numberOfRefs: Int, acc: List[reference]): List[reference] = { + if (numberOfRefs <= 0) acc + else { + val batchSize = Math.min(numberOfRefs, refGeneratorLimit) + val response: Response[Either[String, reference]] = client.send(basicRequest.get(uri"$refGeneratorUrl/$environment/counter?numberofrefs=$batchSize")) + recursivelyFetchReferences(numberOfRefs - batchSize, acc ++ processResponse(response)) + } + } + + if (numberOfRefs > refGeneratorLimit) { + recursivelyFetchReferences(numberOfRefs, Nil) + } else { + try { + val response: Response[Either[String, reference]] = client.send(basicRequest.get(uri"$refGeneratorUrl/$environment/counter?numberofrefs=$numberOfRefs")) + processResponse(response) + } finally client.close() + } } } diff --git a/src/test/resources/application.conf b/src/test/resources/application.conf index c38928974..d195637ff 100644 --- a/src/test/resources/application.conf +++ b/src/test/resources/application.conf @@ -37,3 +37,8 @@ fileUpload { featureAccessBlock { http4s = true } + +referenceGenerator { + referenceGeneratorUrl = "https://q59yzinibd.execute-api.eu-west-2.amazonaws.com" + referenceLimit = 2 +} diff --git a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/FileServiceSpec.scala b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/FileServiceSpec.scala index 2324ecece..fb4d82f0b 100644 --- a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/FileServiceSpec.scala +++ b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/FileServiceSpec.scala @@ -59,6 +59,7 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S val ffidMetadataRepositoryMock: FFIDMetadataRepository = mock[FFIDMetadataRepository] val antivirusMetadataRepositoryMock: AntivirusMetadataRepository = mock[AntivirusMetadataRepository] val validateFileMetadataServiceMock: ValidateFileMetadataService = mock[ValidateFileMetadataService] + val referenceGeneratorServiceMock: ReferenceGeneratorService = mock[ReferenceGeneratorService] val consignmentStatusService = new ConsignmentStatusService(consignmentStatusRepositoryMock, fileStatusRepositoryMock, uuidSource, FixedTimeSource) val fileMetadataService = new FileMetadataService( @@ -130,6 +131,7 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S antivirusMetadataService, fileStatusService, fileMetadataService, + referenceGeneratorServiceMock, FixedTimeSource, fixedUuidSource, ConfigFactory.load() @@ -175,6 +177,7 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S mock[AntivirusMetadataService], mock[FileStatusService], mock[FileMetadataService], + referenceGeneratorServiceMock, FixedTimeSource, fixedUuidSource, ConfigFactory.load() @@ -253,6 +256,7 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S antivirusMetadataService, fileStatusService, fileMetadataService, + referenceGeneratorServiceMock, FixedTimeSource, fixedUuidSource, ConfigFactory.load() @@ -392,6 +396,7 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S antivirusMetadataService, fileStatusService, fileMetadataService, + referenceGeneratorServiceMock, FixedTimeSource, fixedUuidSource, ConfigFactory.load() @@ -490,6 +495,7 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S antivirusMetadataService, fileStatusService, fileMetadataService, + referenceGeneratorServiceMock, FixedTimeSource, fixedUuidSource, ConfigFactory.load() @@ -601,6 +607,7 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S antivirusMetadataService, fileStatusService, fileMetadataService, + referenceGeneratorServiceMock, FixedTimeSource, fixedUuidSource, ConfigFactory.load() @@ -688,6 +695,7 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S antivirusMetadataService, fileStatusService, fileMetadataService, + referenceGeneratorServiceMock, FixedTimeSource, fixedUuidSource, ConfigFactory.load() @@ -733,6 +741,7 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S when(fileRepositoryMock.addFiles(fileRowCaptor.capture(), metadataRowCaptor.capture())).thenReturn(Future(())) when(fileStatusServiceMock.addFileStatuses(any[AddMultipleFileStatusesInput])).thenReturn(Future(Nil)) + when(referenceGeneratorServiceMock.getReferences(any[Int])).thenReturn(List("ref1", "ref2", "ref3", "ref4", "ref5")) mockCustomMetadataValuesResponse(customMetadataPropertiesRepositoryMock) val service = new FileService( @@ -743,6 +752,7 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S antivirusMetadataService, fileStatusServiceMock, fileMetadataService, + referenceGeneratorServiceMock, FixedTimeSource, fixedUuidSource, ConfigFactory.load() @@ -835,6 +845,7 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S val metadataInputTwo = ClientSideMetadataInput("", "", 1L, 1L, 2) when(fileRepositoryMock.addFiles(fileRowCaptor.capture(), metadataRowCaptor.capture())).thenReturn(Future(())) + when(referenceGeneratorServiceMock.getReferences(any[Int])).thenReturn(List("ref1", "ref2", "ref3")) mockCustomMetadataValuesResponse(customMetadataPropertiesRepositoryMock) val service = new FileService( @@ -845,6 +856,7 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S antivirusMetadataService, fileStatusService, fileMetadataService, + referenceGeneratorServiceMock, FixedTimeSource, fixedUuidSource, ConfigFactory.load() @@ -1285,6 +1297,7 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S antivirusMetadataService, fileStatusService, fileMetadataService, + referenceGeneratorServiceMock, FixedTimeSource, fixedUuidSource, ConfigFactory.load() diff --git a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorServiceSpec.scala b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorServiceSpec.scala new file mode 100644 index 000000000..8b414df15 --- /dev/null +++ b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorServiceSpec.scala @@ -0,0 +1,50 @@ +package uk.gov.nationalarchives.tdr.api.service + +import com.typesafe.config.ConfigFactory +import org.mockito.MockitoSugar +import org.scalatest.concurrent.ScalaFutures +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers +import org.scalatest.prop.TableDrivenPropertyChecks +import sttp.client3.SimpleHttpClient +import sttp.client3.testing.SttpBackendStub +import sttp.model.Method + +import scala.concurrent.ExecutionContext + +class ReferenceGeneratorServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with ScalaFutures with TableDrivenPropertyChecks { + implicit val executionContext: ExecutionContext = ExecutionContext.Implicits.global + + case class User(id: String) + + "getReferences" should "return a list of references" in { + val sttpBackendStub = SttpBackendStub.synchronous + .whenRequestMatches(_.uri.path.startsWith(List("intg", "counter"))) + .thenRespond("""["REF1","REF2"]""") + val referenceGeneratorService = new ReferenceGeneratorService(ConfigFactory.load, SimpleHttpClient(sttpBackendStub)) + val getReferences = referenceGeneratorService.getReferences(2) + + getReferences shouldBe List("REF1", "REF2") + } + + "getReferences" should "return a list of references if referenceLimit is exceeded" in { + val sttpBackendStub = SttpBackendStub.synchronous + .whenRequestMatches(_.uri.path.startsWith(List("intg", "counter"))) + .thenRespondCyclic("""["REF1","REF2"]""", """["REF3"]""") + val referenceGeneratorService = new ReferenceGeneratorService(ConfigFactory.load, SimpleHttpClient(sttpBackendStub)) + val getReferences = referenceGeneratorService.getReferences(3) + + getReferences shouldBe List("REF1", "REF2", "REF3") + } + + "getReferences" should "throw an exception if the call to the reference service fails" in { + val sttpBackendStub = SttpBackendStub.synchronous + .whenRequestMatches(_.method == Method.GET) + .thenRespondServerError() + val referenceGeneratorService = new ReferenceGeneratorService(ConfigFactory.load, SimpleHttpClient(sttpBackendStub)) + val thrown = intercept[Exception] { + referenceGeneratorService.getReferences(3) + } + assert(thrown.getMessage === "status code: 500, reason phrase: Failed to get references from reference generator api") + } +} From 1b8959548ced310941b28e5fc35e23ae52345f8e Mon Sep 17 00:00:00 2001 From: Thanh Date: Sun, 8 Oct 2023 21:43:01 +0100 Subject: [PATCH 04/10] Use reference generator feature block --- .../tdr/api/service/FileService.scala | 12 ++++++++---- src/test/resources/application.conf | 3 +++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/FileService.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/FileService.scala index 918966d84..e9c7c76be 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/FileService.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/FileService.scala @@ -16,7 +16,6 @@ import uk.gov.nationalarchives.tdr.api.model.file.NodeType.{FileTypeHelper, dire import uk.gov.nationalarchives.tdr.api.service.FileMetadataService._ import uk.gov.nationalarchives.tdr.api.service.FileService._ import uk.gov.nationalarchives.tdr.api.service.FileStatusService.{FFID, allFileStatusTypes, defaultStatuses} -import uk.gov.nationalarchives.tdr.api.service.ReferenceGeneratorService.reference import uk.gov.nationalarchives.tdr.api.utils.NaturalSorting.{ArrayOrdering, natural} import uk.gov.nationalarchives.tdr.api.utils.TimeUtils.LongUtils import uk.gov.nationalarchives.tdr.api.utils.TreeNodesUtils @@ -45,6 +44,7 @@ class FileService( private val fileUploadBatchSize: Int = config.getInt("fileUpload.batchSize") private val filePageMaxLimit: Int = config.getInt("pagination.filesMaxLimit") + private val referenceGeneratorFeatureBlock: Boolean = config.getBoolean("featureAccessBlock.assignFileReferences") def addFile(addFileAndMetadataInput: AddFileAndMetadataInput, userId: UUID): Future[List[FileMatches]] = { val now = Timestamp.from(timeSource.now) @@ -56,8 +56,12 @@ class FileService( val row: (UUID, String, String) => FilemetadataRow = FilemetadataRow(uuidSource.uuid, _, _, now, userId, _) val rows: Future[List[Rows]] = customMetadataPropertiesRepository.getCustomMetadataValuesWithDefault.map(filePropertyValue => { val filesAndFolderCombined = allEmptyDirectoryNodes ++ allFileNodes - val generatedReferences = referenceGeneratorService.getReferences(filesAndFolderCombined.size) - val fileAndFolderAssignedRef: Map[String, (TreeNode, reference)] = filesAndFolderCombined.keys.zip(filesAndFolderCombined.values.zip(generatedReferences)).toMap + val fileAndFolderAssignedRef = if (referenceGeneratorFeatureBlock) { + filesAndFolderCombined.keys.zip(filesAndFolderCombined.values.zip(List.fill(filesAndFolderCombined.size)(None))).toMap + } else { + val generatedReferences = referenceGeneratorService.getReferences(filesAndFolderCombined.size) + filesAndFolderCombined.keys.zip(filesAndFolderCombined.values.zip(generatedReferences.map(Some(_)))).toMap + } (fileAndFolderAssignedRef map { case (path, (treeNode, reference)) => val parentId = treeNode.parentPath.map(path => allFileNodes.getOrElse(path, allEmptyDirectoryNodes(path)).id) val fileId = treeNode.id @@ -69,7 +73,7 @@ class FileService( filetype = Some(treeNode.treeNodeType), filename = Some(treeNode.name), parentid = parentId, - filereference = Some(reference) + filereference = reference ) val commonMetadataRows = List( diff --git a/src/test/resources/application.conf b/src/test/resources/application.conf index d195637ff..6cfd28c67 100644 --- a/src/test/resources/application.conf +++ b/src/test/resources/application.conf @@ -36,9 +36,12 @@ fileUpload { featureAccessBlock { http4s = true + assignFileReferences = true } referenceGenerator { referenceGeneratorUrl = "https://q59yzinibd.execute-api.eu-west-2.amazonaws.com" referenceLimit = 2 } + +environment = "intg" From 65c21e82b7c08d6b53ad73e728862b917cbbd2bb Mon Sep 17 00:00:00 2001 From: Thanh Date: Sun, 8 Oct 2023 22:18:46 +0100 Subject: [PATCH 05/10] Test --- .../tdr/api/service/FileServiceSpec.scala | 95 ++++++++++++++++++- 1 file changed, 93 insertions(+), 2 deletions(-) diff --git a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/FileServiceSpec.scala b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/FileServiceSpec.scala index fb4d82f0b..9fdd8ce1a 100644 --- a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/FileServiceSpec.scala +++ b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/FileServiceSpec.scala @@ -1,7 +1,7 @@ package uk.gov.nationalarchives.tdr.api.service import cats.implicits.catsSyntaxOptionId -import com.typesafe.config.ConfigFactory +import com.typesafe.config.{ConfigFactory, ConfigValueFactory} import org.mockito.ArgumentMatchers._ import org.mockito.stubbing.ScalaOngoingStubbing import org.mockito.{ArgumentCaptor, ArgumentMatchers, MockitoSugar} @@ -741,7 +741,6 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S when(fileRepositoryMock.addFiles(fileRowCaptor.capture(), metadataRowCaptor.capture())).thenReturn(Future(())) when(fileStatusServiceMock.addFileStatuses(any[AddMultipleFileStatusesInput])).thenReturn(Future(Nil)) - when(referenceGeneratorServiceMock.getReferences(any[Int])).thenReturn(List("ref1", "ref2", "ref3", "ref4", "ref5")) mockCustomMetadataValuesResponse(customMetadataPropertiesRepositoryMock) val service = new FileService( @@ -803,6 +802,98 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S verify(consignmentStatusRepositoryMock, times(0)).updateConsignmentStatus(any[UUID], any[String], any[String], any[Timestamp]) } + "addFile" should "assign file references to all files, directories and add all files, directories, client and static metadata when total number of files and folders are greater than batch size" in { + val ffidMetadataService = mock[FFIDMetadataService] + val antivirusMetadataService = mock[AntivirusMetadataService] + val fileRepositoryMock = mock[FileRepository] + val customMetadataPropertiesRepositoryMock = mock[CustomMetadataPropertiesRepository] + val fileMetadataService = + new FileMetadataService( + fileMetadataRepositoryMock, + consignmentStatusService, + customMetadataServiceMock, + validateFileMetadataServiceMock, + FixedTimeSource, + new FixedUUIDSource() + ) + val fixedUuidSource = new FixedUUIDSource() + val fileStatusServiceMock = mock[FileStatusService] + + val consignmentId = UUID.randomUUID() + val userId = UUID.randomUUID() + val file1Id = UUID.fromString("47e365a4-fc1e-4375-b2f6-dccb6d361f5f") + val file2Id = UUID.fromString("6e3b76c4-1745-4467-8ac5-b4dd736e1b3e") + + val fileRowCaptor: ArgumentCaptor[List[FileRow]] = ArgumentCaptor.forClass(classOf[List[FileRow]]) + val metadataRowCaptor: ArgumentCaptor[List[FilemetadataRow]] = ArgumentCaptor.forClass(classOf[List[FilemetadataRow]]) + + val metadataInputOne = ClientSideMetadataInput("/a/nested/path/OriginalPath1", "Checksum1", 1L, 1L, 1) + val metadataInputTwo = ClientSideMetadataInput("OriginalPath2", "Checksum2", 1L, 1L, 2) + + when(fileRepositoryMock.addFiles(fileRowCaptor.capture(), metadataRowCaptor.capture())).thenReturn(Future(())) + when(fileStatusServiceMock.addFileStatuses(any[AddMultipleFileStatusesInput])).thenReturn(Future(Nil)) + when(referenceGeneratorServiceMock.getReferences(any[Int])).thenReturn(List("ref1", "ref2", "ref3", "ref4", "ref5")) + mockCustomMetadataValuesResponse(customMetadataPropertiesRepositoryMock) + + val service = new FileService( + fileRepositoryMock, + consignmentRepositoryMock, + customMetadataPropertiesRepositoryMock, + ffidMetadataService, + antivirusMetadataService, + fileStatusServiceMock, + fileMetadataService, + referenceGeneratorServiceMock, + FixedTimeSource, + fixedUuidSource, + ConfigFactory.load().withValue("featureAccessBlock.assignFileReferences", ConfigValueFactory.fromAnyRef("false")) + ) + + val expectedStatusInput = AddMultipleFileStatusesInput( + List( + AddFileStatusInput(file1Id, "ClosureMetadata", "NotEntered"), + AddFileStatusInput(file1Id, "DescriptiveMetadata", "NotEntered"), + AddFileStatusInput(file2Id, "ClosureMetadata", "NotEntered"), + AddFileStatusInput(file2Id, "DescriptiveMetadata", "NotEntered") + ) + ) + + val response = service.addFile(AddFileAndMetadataInput(consignmentId, List(metadataInputOne, metadataInputTwo)), userId).futureValue + + verify(fileRepositoryMock, times(2)).addFiles(any[List[FileRow]](), any[List[FilemetadataRow]]()) + verify(fileStatusServiceMock, times(1)).addFileStatuses(expectedStatusInput) + + val fileRows: List[FileRow] = fileRowCaptor.getAllValues.asScala.flatten.toList + val metadataRows: List[FilemetadataRow] = metadataRowCaptor.getAllValues.asScala.flatten.toList + + response.head.fileId should equal(file1Id) + response.head.matchId should equal(2) + + response.last.fileId should equal(file2Id) + response.last.matchId should equal(1) + + val expectedFileRows = 5 + fileRows.size should equal(expectedFileRows) + fileRows.foreach(row => { + row.consignmentid should equal(consignmentId) + row.userid should equal(userId) + }) + val expectedSize = 46 + metadataRows.size should equal(expectedSize) + staticMetadataProperties.foreach(prop => { + metadataRows.count(r => r.propertyname == prop.name && r.value == prop.value) should equal(5) + }) + + clientSideProperties.foreach(prop => { + val count = metadataRows.count(r => r.propertyname == prop) + prop match { + case ClientSideOriginalFilepath | Filename | FileType => count should equal(5) // Directories have this set + case _ => count should equal(2) + } + }) + verify(consignmentStatusRepositoryMock, times(0)).updateConsignmentStatus(any[UUID], any[String], any[String], any[Timestamp]) + } + def checkFileStatusRows(rows: List[FilestatusRow], response: List[FileMatches], statusType: String, firstStatus: String, secondStatus: String): Unit = { rows.head.filestatusid != null shouldBe true From 8199437469f2df6614b0c34a9a56fff8476bd566 Mon Sep 17 00:00:00 2001 From: Thanh Date: Mon, 9 Oct 2023 10:23:20 +0100 Subject: [PATCH 06/10] build.sbt --- build.sbt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/build.sbt b/build.sbt index 8772acdf6..996863537 100644 --- a/build.sbt +++ b/build.sbt @@ -42,7 +42,6 @@ lazy val akkaHttpVersion = "10.2.10" lazy val circeVersion = "0.14.6" lazy val testContainersVersion = "0.41.0" val http4sVersion = "0.23.23" -val sttpVersion = "3.9.0" libraryDependencies ++= Seq( "org.sangria-graphql" %% "sangria" % "4.0.2", @@ -64,7 +63,7 @@ libraryDependencies ++= Seq( "io.circe" %% "circe-optics" % "0.14.1", "io.circe" %% "circe-generic" % circeVersion, "io.circe" %% "circe-generic-extras" % "0.14.3", - "com.softwaremill.sttp.client3" %% "core" % sttpVersion, + "com.softwaremill.sttp.client3" %% "core" % "3.9.0", "uk.gov.nationalarchives" %% "consignment-api-db" % "0.1.36", "org.postgresql" % "postgresql" % "42.6.0", "com.typesafe.slick" %% "slick" % "3.4.0", From d4b3fcaf9164774a5fa62d0e7be2f9a7d8b5ef3a Mon Sep 17 00:00:00 2001 From: Thanh Date: Tue, 10 Oct 2023 01:22:21 +0100 Subject: [PATCH 07/10] Minor refactor --- .../tdr/api/service/ReferenceGeneratorService.scala | 13 ++++++------- .../api/service/ReferenceGeneratorServiceSpec.scala | 5 +---- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala index 569034790..0e286b4d5 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala @@ -1,6 +1,6 @@ package uk.gov.nationalarchives.tdr.api.service -import com.typesafe.config.{Config, ConfigFactory} +import com.typesafe.config.Config import com.typesafe.scalalogging.Logger import io.circe.parser._ import org.apache.http.client.HttpResponseException @@ -21,8 +21,9 @@ class ReferenceGeneratorService(config: Config, client: SimpleHttpClient) { try { response.body match { case Left(body) => - logger.error(s"Non-2xx response to GET with code ${response.code}:\n$body") - throw new HttpResponseException(response.code.code, "Failed to get references from reference generator api") + val message = "Failed to get references from reference generator api" + logger.error(s"${response.code} $message:\n$body") + throw new HttpResponseException(response.code.code, message) case Right(body) => val references = parse(body).getOrElse(null) val listOfReferences = references.as[List[reference]].getOrElse(null) @@ -47,10 +48,8 @@ class ReferenceGeneratorService(config: Config, client: SimpleHttpClient) { if (numberOfRefs > refGeneratorLimit) { recursivelyFetchReferences(numberOfRefs, Nil) } else { - try { - val response: Response[Either[String, reference]] = client.send(basicRequest.get(uri"$refGeneratorUrl/$environment/counter?numberofrefs=$numberOfRefs")) - processResponse(response) - } finally client.close() + val response: Response[Either[String, reference]] = client.send(basicRequest.get(uri"$refGeneratorUrl/$environment/counter?numberofrefs=$numberOfRefs")) + processResponse(response) } } } diff --git a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorServiceSpec.scala b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorServiceSpec.scala index 8b414df15..a81657f41 100644 --- a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorServiceSpec.scala +++ b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorServiceSpec.scala @@ -1,18 +1,15 @@ package uk.gov.nationalarchives.tdr.api.service import com.typesafe.config.ConfigFactory -import org.mockito.MockitoSugar -import org.scalatest.concurrent.ScalaFutures import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers -import org.scalatest.prop.TableDrivenPropertyChecks import sttp.client3.SimpleHttpClient import sttp.client3.testing.SttpBackendStub import sttp.model.Method import scala.concurrent.ExecutionContext -class ReferenceGeneratorServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with ScalaFutures with TableDrivenPropertyChecks { +class ReferenceGeneratorServiceSpec extends AnyFlatSpec with Matchers { implicit val executionContext: ExecutionContext = ExecutionContext.Implicits.global case class User(id: String) From 28b184c40cf690598fb05344dc0d5a09e9975431 Mon Sep 17 00:00:00 2001 From: Thanh Date: Tue, 10 Oct 2023 01:26:04 +0100 Subject: [PATCH 08/10] Scala fmt --- .../tdr/api/service/ReferenceGeneratorServiceSpec.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorServiceSpec.scala b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorServiceSpec.scala index a81657f41..c6bdba764 100644 --- a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorServiceSpec.scala +++ b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorServiceSpec.scala @@ -9,7 +9,7 @@ import sttp.model.Method import scala.concurrent.ExecutionContext -class ReferenceGeneratorServiceSpec extends AnyFlatSpec with Matchers { +class ReferenceGeneratorServiceSpec extends AnyFlatSpec with Matchers { implicit val executionContext: ExecutionContext = ExecutionContext.Implicits.global case class User(id: String) From ff15cf923dcf7578819fd93074eeca8402fea7ca Mon Sep 17 00:00:00 2001 From: Thanh Date: Tue, 10 Oct 2023 15:26:38 +0100 Subject: [PATCH 09/10] pr comments --- src/main/resources/application.base.conf | 1 - .../tdr/api/http/GraphQLServerBase.scala | 1 - .../tdr/api/service/FileService.scala | 13 ++++++------- .../tdr/api/service/ReferenceGeneratorService.scala | 6 +++--- src/test/resources/application.conf | 2 +- .../tdr/api/service/FileServiceSpec.scala | 12 ------------ .../api/service/ReferenceGeneratorServiceSpec.scala | 2 +- 7 files changed, 11 insertions(+), 26 deletions(-) diff --git a/src/main/resources/application.base.conf b/src/main/resources/application.base.conf index e97a754ec..10d5fd89e 100644 --- a/src/main/resources/application.base.conf +++ b/src/main/resources/application.base.conf @@ -41,7 +41,6 @@ referenceGenerator { referenceLimit = ${REFERENCE_GENERATOR_LIMIT} } - environment = ${ENVIRONMENT} akka.http { diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/http/GraphQLServerBase.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/http/GraphQLServerBase.scala index 1bd58113d..ead88f042 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/http/GraphQLServerBase.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/http/GraphQLServerBase.scala @@ -87,7 +87,6 @@ trait GraphQLServerBase { val referenceGeneratorService = new ReferenceGeneratorService(config, SimpleHttpClient()) val fileService = new FileService( fileRepository, - consignmentRepository, customMetadataPropertiesRepository, ffidMetadataService, antivirusMetadataService, diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/FileService.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/FileService.scala index e9c7c76be..025e7d507 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/FileService.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/FileService.scala @@ -28,7 +28,6 @@ import scala.math.min class FileService( fileRepository: FileRepository, - consignmentRepository: ConsignmentRepository, customMetadataPropertiesRepository: CustomMetadataPropertiesRepository, ffidMetadataService: FFIDMetadataService, avMetadataService: AntivirusMetadataService, @@ -55,14 +54,14 @@ class FileService( val row: (UUID, String, String) => FilemetadataRow = FilemetadataRow(uuidSource.uuid, _, _, now, userId, _) val rows: Future[List[Rows]] = customMetadataPropertiesRepository.getCustomMetadataValuesWithDefault.map(filePropertyValue => { - val filesAndFolderCombined = allEmptyDirectoryNodes ++ allFileNodes - val fileAndFolderAssignedRef = if (referenceGeneratorFeatureBlock) { - filesAndFolderCombined.keys.zip(filesAndFolderCombined.values.zip(List.fill(filesAndFolderCombined.size)(None))).toMap + val allNodes = allEmptyDirectoryNodes ++ allFileNodes + val allNodesWithReference = if (referenceGeneratorFeatureBlock) { + allNodes.keys.zip(allNodes.values.zip(List.fill(allNodes.size)(None))).toMap } else { - val generatedReferences = referenceGeneratorService.getReferences(filesAndFolderCombined.size) - filesAndFolderCombined.keys.zip(filesAndFolderCombined.values.zip(generatedReferences.map(Some(_)))).toMap + val generatedReferences = referenceGeneratorService.getReferences(allNodes.size) + allNodes.keys.zip(allNodes.values.zip(generatedReferences.map(Some(_)))).toMap } - (fileAndFolderAssignedRef map { case (path, (treeNode, reference)) => + (allNodesWithReference map { case (path, (treeNode, reference)) => val parentId = treeNode.parentPath.map(path => allFileNodes.getOrElse(path, allEmptyDirectoryNodes(path)).id) val fileId = treeNode.id val fileRow = FileRow( diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala index 0e286b4d5..79eb3de09 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala @@ -36,17 +36,17 @@ class ReferenceGeneratorService(config: Config, client: SimpleHttpClient) { } @tailrec - def recursivelyFetchReferences(numberOfRefs: Int, acc: List[reference]): List[reference] = { + def fetchReferences(numberOfRefs: Int, acc: List[reference]): List[reference] = { if (numberOfRefs <= 0) acc else { val batchSize = Math.min(numberOfRefs, refGeneratorLimit) val response: Response[Either[String, reference]] = client.send(basicRequest.get(uri"$refGeneratorUrl/$environment/counter?numberofrefs=$batchSize")) - recursivelyFetchReferences(numberOfRefs - batchSize, acc ++ processResponse(response)) + fetchReferences(numberOfRefs - batchSize, acc ++ processResponse(response)) } } if (numberOfRefs > refGeneratorLimit) { - recursivelyFetchReferences(numberOfRefs, Nil) + fetchReferences(numberOfRefs, Nil) } else { val response: Response[Either[String, reference]] = client.send(basicRequest.get(uri"$refGeneratorUrl/$environment/counter?numberofrefs=$numberOfRefs")) processResponse(response) diff --git a/src/test/resources/application.conf b/src/test/resources/application.conf index 6cfd28c67..11916b0b8 100644 --- a/src/test/resources/application.conf +++ b/src/test/resources/application.conf @@ -40,7 +40,7 @@ featureAccessBlock { } referenceGenerator { - referenceGeneratorUrl = "https://q59yzinibd.execute-api.eu-west-2.amazonaws.com" + referenceGeneratorUrl = "https://dummy-reference-url.com" referenceLimit = 2 } diff --git a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/FileServiceSpec.scala b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/FileServiceSpec.scala index 9fdd8ce1a..dc7dced64 100644 --- a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/FileServiceSpec.scala +++ b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/FileServiceSpec.scala @@ -49,7 +49,6 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S private val bodyId = UUID.randomUUID() val uuidSource: FixedUUIDSource = new FixedUUIDSource() - val consignmentRepositoryMock: ConsignmentRepository = mock[ConsignmentRepository] val consignmentStatusRepositoryMock: ConsignmentStatusRepository = mock[ConsignmentStatusRepository] val customMetadataPropertiesRepositoryMock: CustomMetadataPropertiesRepository = mock[CustomMetadataPropertiesRepository] val customMetadataServiceMock: CustomMetadataPropertiesService = mock[CustomMetadataPropertiesService] @@ -125,7 +124,6 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S val fileService = new FileService( fileRepositoryMock, - consignmentRepositoryMock, customMetadataPropertiesRepositoryMock, ffidMetadataService, antivirusMetadataService, @@ -171,7 +169,6 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S val service = new FileService( fileRepositoryMock, - mock[ConsignmentRepository], mock[CustomMetadataPropertiesRepository], mock[FFIDMetadataService], mock[AntivirusMetadataService], @@ -250,7 +247,6 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S val service = new FileService( fileRepositoryMock, - consignmentRepositoryMock, customMetadataPropertiesRepositoryMock, ffidMetadataService, antivirusMetadataService, @@ -390,7 +386,6 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S val service = new FileService( fileRepositoryMock, - consignmentRepositoryMock, customMetadataPropertiesRepositoryMock, ffidMetadataService, antivirusMetadataService, @@ -489,7 +484,6 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S val service = new FileService( fileRepositoryMock, - consignmentRepositoryMock, customMetadataPropertiesRepositoryMock, ffidMetadataService, antivirusMetadataService, @@ -601,7 +595,6 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S val service = new FileService( fileRepositoryMock, - consignmentRepositoryMock, customMetadataPropertiesRepositoryMock, ffidMetadataService, antivirusMetadataService, @@ -689,7 +682,6 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S val service = new FileService( fileRepositoryMock, - consignmentRepositoryMock, customMetadataPropertiesRepositoryMock, ffidMetadataService, antivirusMetadataService, @@ -745,7 +737,6 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S val service = new FileService( fileRepositoryMock, - consignmentRepositoryMock, customMetadataPropertiesRepositoryMock, ffidMetadataService, antivirusMetadataService, @@ -837,7 +828,6 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S val service = new FileService( fileRepositoryMock, - consignmentRepositoryMock, customMetadataPropertiesRepositoryMock, ffidMetadataService, antivirusMetadataService, @@ -941,7 +931,6 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S val service = new FileService( fileRepositoryMock, - consignmentRepositoryMock, customMetadataPropertiesRepositoryMock, ffidMetadataService, antivirusMetadataService, @@ -1382,7 +1371,6 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S new FileService( fileRepositoryMock, - consignmentRepositoryMock, customMetadataPropertiesRepositoryMock, ffidMetadataService, antivirusMetadataService, diff --git a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorServiceSpec.scala b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorServiceSpec.scala index c6bdba764..121e3c87f 100644 --- a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorServiceSpec.scala +++ b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorServiceSpec.scala @@ -24,7 +24,7 @@ class ReferenceGeneratorServiceSpec extends AnyFlatSpec with Matchers { getReferences shouldBe List("REF1", "REF2") } - "getReferences" should "return a list of references if referenceLimit is exceeded" in { + "getReferences" should "return the correct number of reference if 'referenceLimit' is exceeded in the request" in { val sttpBackendStub = SttpBackendStub.synchronous .whenRequestMatches(_.uri.path.startsWith(List("intg", "counter"))) .thenRespondCyclic("""["REF1","REF2"]""", """["REF3"]""") From a49758a68cf94cd1f53db09e3b97564613a4836b Mon Sep 17 00:00:00 2001 From: Thanh Date: Wed, 11 Oct 2023 12:54:59 +0100 Subject: [PATCH 10/10] pr comments --- .../service/ReferenceGeneratorService.scala | 61 ++++++------- .../tdr/api/service/FileServiceSpec.scala | 88 +++++++++++++++++++ 2 files changed, 116 insertions(+), 33 deletions(-) diff --git a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala index 79eb3de09..1acb20733 100644 --- a/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala +++ b/src/main/scala/uk/gov/nationalarchives/tdr/api/service/ReferenceGeneratorService.scala @@ -5,55 +5,50 @@ import com.typesafe.scalalogging.Logger import io.circe.parser._ import org.apache.http.client.HttpResponseException import sttp.client3.{Response, SimpleHttpClient, UriContext, basicRequest} -import uk.gov.nationalarchives.tdr.api.service.ReferenceGeneratorService.reference +import uk.gov.nationalarchives.tdr.api.service.ReferenceGeneratorService.Reference import scala.annotation.tailrec class ReferenceGeneratorService(config: Config, client: SimpleHttpClient) { val logger: Logger = Logger("ReferenceGeneratorService") - val environment: String = config.getString("environment") - val refGeneratorUrl: String = config.getString("referenceGenerator.referenceGeneratorUrl") - val refGeneratorLimit: Int = config.getInt("referenceGenerator.referenceLimit") - - def getReferences(numberOfRefs: Int): List[reference] = { - - def processResponse(response: Response[Either[String, reference]]): List[reference] = { - try { - response.body match { - case Left(body) => - val message = "Failed to get references from reference generator api" - logger.error(s"${response.code} $message:\n$body") - throw new HttpResponseException(response.code.code, message) - case Right(body) => - val references = parse(body).getOrElse(null) - val listOfReferences = references.as[List[reference]].getOrElse(null) - logger.info(s"2xx response to GET:\n$listOfReferences") - listOfReferences - } - } finally { - client.close() - } - } + private val environment: String = config.getString("environment") + private val refGeneratorUrl: String = config.getString("referenceGenerator.referenceGeneratorUrl") + private val refGeneratorLimit: Int = config.getInt("referenceGenerator.referenceLimit") + def getReferences(numberOfRefs: Int): List[Reference] = { @tailrec - def fetchReferences(numberOfRefs: Int, acc: List[reference]): List[reference] = { + def fetchReferences(numberOfRefs: Int, acc: List[Reference]): List[Reference] = { if (numberOfRefs <= 0) acc else { val batchSize = Math.min(numberOfRefs, refGeneratorLimit) - val response: Response[Either[String, reference]] = client.send(basicRequest.get(uri"$refGeneratorUrl/$environment/counter?numberofrefs=$batchSize")) - fetchReferences(numberOfRefs - batchSize, acc ++ processResponse(response)) + fetchReferences(numberOfRefs - batchSize, acc ++ sendRequest(batchSize)) } } - if (numberOfRefs > refGeneratorLimit) { - fetchReferences(numberOfRefs, Nil) - } else { - val response: Response[Either[String, reference]] = client.send(basicRequest.get(uri"$refGeneratorUrl/$environment/counter?numberofrefs=$numberOfRefs")) - processResponse(response) + fetchReferences(numberOfRefs, Nil) + } + + private def sendRequest(numberOfRefs: Int): List[Reference] = { + val response: Response[Either[String, Reference]] = client.send(basicRequest.get(uri"$refGeneratorUrl/$environment/counter?numberofrefs=$numberOfRefs")) + + try { + response.body match { + case Left(body) => + val message = "Failed to get references from reference generator api" + logger.error(s"${response.code} $message:\n$body") + throw new HttpResponseException(response.code.code, message) + case Right(body) => + val references = parse(body).getOrElse(null) + val listOfReferences = references.as[List[Reference]].getOrElse(null) + logger.info(s"2xx response to GET:\n$listOfReferences") + listOfReferences + } + } finally { + client.close() } } } object ReferenceGeneratorService { - type reference = String + type Reference = String } diff --git a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/FileServiceSpec.scala b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/FileServiceSpec.scala index dc7dced64..0bb200a68 100644 --- a/src/test/scala/uk/gov/nationalarchives/tdr/api/service/FileServiceSpec.scala +++ b/src/test/scala/uk/gov/nationalarchives/tdr/api/service/FileServiceSpec.scala @@ -977,6 +977,94 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S verify(consignmentStatusRepositoryMock, times(0)).updateConsignmentStatus(any[UUID], any[String], any[String], any[Timestamp]) } + "addFile" should "not add files, directories, client and static metadata when reference generator throws an exception" in { + val ffidMetadataService = mock[FFIDMetadataService] + val antivirusMetadataService = mock[AntivirusMetadataService] + val fileRepositoryMock = mock[FileRepository] + val customMetadataPropertiesRepositoryMock = mock[CustomMetadataPropertiesRepository] + val fileMetadataService = + new FileMetadataService( + fileMetadataRepositoryMock, + consignmentStatusService, + customMetadataServiceMock, + validateFileMetadataServiceMock, + FixedTimeSource, + new FixedUUIDSource() + ) + val fixedUuidSource = new FixedUUIDSource() + val fileStatusServiceMock = mock[FileStatusService] + + val consignmentId = UUID.randomUUID() + val userId = UUID.randomUUID() + val file1Id = UUID.fromString("47e365a4-fc1e-4375-b2f6-dccb6d361f5f") + val file2Id = UUID.fromString("6e3b76c4-1745-4467-8ac5-b4dd736e1b3e") + + val fileRowCaptor: ArgumentCaptor[List[FileRow]] = ArgumentCaptor.forClass(classOf[List[FileRow]]) + val metadataRowCaptor: ArgumentCaptor[List[FilemetadataRow]] = ArgumentCaptor.forClass(classOf[List[FilemetadataRow]]) + + val metadataInputOne = ClientSideMetadataInput("/a/nested/path/OriginalPath1", "Checksum1", 1L, 1L, 1) + val metadataInputTwo = ClientSideMetadataInput("OriginalPath2", "Checksum2", 1L, 1L, 2) + + when(fileRepositoryMock.addFiles(fileRowCaptor.capture(), metadataRowCaptor.capture())).thenReturn(Future(())) + when(fileStatusServiceMock.addFileStatuses(any[AddMultipleFileStatusesInput])).thenReturn(Future(Nil)) + when(referenceGeneratorServiceMock.getReferences(any[Int])).thenThrow(new Exception("some exception")) + mockCustomMetadataValuesResponse(customMetadataPropertiesRepositoryMock) + + val service = new FileService( + fileRepositoryMock, + customMetadataPropertiesRepositoryMock, + ffidMetadataService, + antivirusMetadataService, + fileStatusServiceMock, + fileMetadataService, + referenceGeneratorServiceMock, + FixedTimeSource, + fixedUuidSource, + ConfigFactory.load().withValue("featureAccessBlock.assignFileReferences", ConfigValueFactory.fromAnyRef("false")) + ) + + val expectedStatusInput = AddMultipleFileStatusesInput( + List( + AddFileStatusInput(file1Id, "ClosureMetadata", "NotEntered"), + AddFileStatusInput(file1Id, "DescriptiveMetadata", "NotEntered"), + AddFileStatusInput(file2Id, "ClosureMetadata", "NotEntered"), + AddFileStatusInput(file2Id, "DescriptiveMetadata", "NotEntered") + ) + ) + + val thrown = intercept[Exception] { + service.addFile(AddFileAndMetadataInput(consignmentId, List(metadataInputOne, metadataInputTwo)), userId).futureValue + } + assert(thrown.getMessage === "The future returned an exception of type: java.lang.Exception, with message: some exception.") + + verify(fileRepositoryMock, times(0)).addFiles(any[List[FileRow]](), any[List[FilemetadataRow]]()) + verify(fileStatusServiceMock, times(0)).addFileStatuses(expectedStatusInput) + + val fileRows: List[FileRow] = fileRowCaptor.getAllValues.asScala.flatten.toList + val metadataRows: List[FilemetadataRow] = metadataRowCaptor.getAllValues.asScala.flatten.toList + + val expectedFileRows = 0 + fileRows.size should equal(expectedFileRows) + fileRows.foreach(row => { + row.consignmentid should equal(consignmentId) + row.userid should equal(userId) + }) + val expectedSize = 0 + metadataRows.size should equal(expectedSize) + staticMetadataProperties.foreach(prop => { + metadataRows.count(r => r.propertyname == prop.name && r.value == prop.value) should equal(0) + }) + + clientSideProperties.foreach(prop => { + val count = metadataRows.count(r => r.propertyname == prop) + prop match { + case ClientSideOriginalFilepath | Filename | FileType => count should equal(0) // Directories have this set + case _ => count should equal(0) + } + }) + verify(consignmentStatusRepositoryMock, times(0)).updateConsignmentStatus(any[UUID], any[String], any[String], any[Timestamp]) + } + "getPaginatedFiles" should "return all the file edges after the cursor to the limit" in { val fileRepositoryMock = mock[FileRepository] val consignmentId = UUID.randomUUID()