Second pass code review

src/main/scala/uk/gov/nationalarchives/tdr/api/service/FileMetadataService.scala

@@ -145,10 +145,6 @@ object FileMetadataService {
   val Description = "description"
   val DescriptionAlternate = "DescriptionAlternate"
 
-  /** Default values for these properties stored in FilePropertyValues table. (TDR-2207) Save default values for these properties because TDR currently only supports records which
-    * are Open, in English, etc. Users agree to these conditions at a consignment level, so it's OK to save these as defaults for every file. They need to be saved so they can be
-    * included in the export package. The defaults may be removed in future once we let users upload a wider variety of records.
-    */
   val RightsCopyright = "RightsCopyright"
   val LegalStatus = "LegalStatus"
   val HeldBy = "HeldBy"
@@ -179,8 +175,6 @@ object FileMetadataService {
     )
   }
 
-  case class StaticMetadata(name: String, value: String)
-
   case class FileMetadataValue(name: String, value: String)
 
   case class AddFileMetadataInput(fileId: UUID, value: String, userId: UUID, filePropertyName: String)

src/test/scala/uk/gov/nationalarchives/tdr/api/routes/ConsignmentRouteSpec.scala

@@ -228,7 +228,7 @@ class ConsignmentRouteSpec extends TestContainerUtils with Matchers with TestReq
 
     utils.addFileMetadata("06209e0d-95d0-4f13-8933-e5b9d00eb435", fileOneId, SHA256ServerSideChecksum)
     utils.addFileMetadata("c4759aae-dc68-45ec-aee1-5a562c7b42cc", fileTwoId, SHA256ServerSideChecksum)
-    (clientSideProperties ++ staticMetadataProperties).foreach(propertyName => {
+    (clientSideProperties ++ defaultMetadataProperties).foreach(propertyName => {
       utils.addFileProperty(propertyName)
       val value = propertyName match {
         case ClientSideFileLastModifiedDate => "2021-03-11 12:30:30.592853"
@@ -365,7 +365,7 @@ class ConsignmentRouteSpec extends TestContainerUtils with Matchers with TestReq
     val consignmentId = UUID.fromString("e72d94d5-ae79-4a05-bee9-86d9dea2bcc9")
     utils.createConsignment(consignmentId, userId)
     utils.addFileProperty(SHA256ServerSideChecksum)
-    staticMetadataProperties.foreach(utils.addFileProperty)
+    defaultMetadataProperties.foreach(utils.addFileProperty)
     clientSideProperties.foreach(utils.addFileProperty)
     val topDirectory = UUID.fromString("ce0a51a5-a224-474f-b3a4-df75effd5b34")
     val subDirectory = UUID.fromString("2753ceca-4df3-436b-8891-78ad38e2e8c5")
@@ -374,12 +374,10 @@ class ConsignmentRouteSpec extends TestContainerUtils with Matchers with TestReq
     utils.createFile(topDirectory, consignmentId, NodeType.directoryTypeIdentifier, "directory", fileRef = Some("REF1"))
     utils.createFile(subDirectory, consignmentId, NodeType.directoryTypeIdentifier, "subDirectory", parentId = subDirectory.some, fileRef = Some("REF2"), parentRef = Some("REF1"))
 
-    // staticMetadataProperties.foreach(p => utils.addFileMetadata(UUID.randomUUID().toString, topDirectory.toString, p.name, p.value))
-    addStaticMetaData(utils, topDirectory, false)
+    addDefaultMetaData(utils, topDirectory, false)
     utils.addFileMetadata(UUID.randomUUID.toString, topDirectory.toString, ClientSideOriginalFilepath, "directory")
 
-    // staticMetadataProperties.foreach(p => utils.addFileMetadata(UUID.randomUUID().toString, subDirectory.toString, p.name, p.value))
-    addStaticMetaData(utils, subDirectory, false)
+    addDefaultMetaData(utils, subDirectory, false)
     utils.addFileMetadata(UUID.randomUUID.toString, subDirectory.toString, ClientSideOriginalFilepath, "directory")
 
     setUpFileAndStandardMetadata(consignmentId, fileId, utils, subDirectory.some, fileRef = Some("REF3"))
@@ -521,7 +519,7 @@ class ConsignmentRouteSpec extends TestContainerUtils with Matchers with TestReq
     val consignmentId = UUID.fromString("e72d94d5-ae79-4a05-bee9-86d9dea2bcc9")
     utils.createConsignment(consignmentId, userId)
     utils.addFileProperty(SHA256ServerSideChecksum)
-    staticMetadataProperties.foreach(utils.addFileProperty)
+    defaultMetadataProperties.foreach(utils.addFileProperty)
     clientSideProperties.foreach(utils.addFileProperty)
     utils.addFileProperty(OriginalFilepath)
     val originalFilePath = "/an/original/file/path"
@@ -837,7 +835,7 @@ class ConsignmentRouteSpec extends TestContainerUtils with Matchers with TestReq
   }
 
   private def setUpConsignments(consignmentParams: List[ConsignmentParams], utils: TestUtils, fileRef: String): Unit = {
-    (staticMetadataProperties ++ clientSideProperties).foreach(prop => utils.addFileProperty(prop))
+    (defaultMetadataProperties ++ clientSideProperties).foreach(prop => utils.addFileProperty(prop))
 
     setUpConsignmentsFor(consignmentParams, utils, userId, fileRef)
   }
@@ -875,7 +873,7 @@ class ConsignmentRouteSpec extends TestContainerUtils with Matchers with TestReq
   }
 
   private def generateMetadataPropertiesForFile(fileId: UUID, utils: TestUtils, addProperty: Boolean): Unit = {
-    addStaticMetaData(utils, fileId, addProperty)
+    addDefaultMetaData(utils, fileId, addProperty)
     addClientSideProperties(utils, fileId, addProperty)
   }
 
@@ -889,22 +887,21 @@ class ConsignmentRouteSpec extends TestContainerUtils with Matchers with TestReq
       parentRef: Option[Reference] = None
   ): UUID = {
     utils.createFile(fileId, consignmentId, NodeType.directoryTypeIdentifier, fileName, parentId = parentId, fileRef = fileRef, parentRef = parentRef)
-    addStaticMetaData(utils, fileId, false)
-    // staticMetadataProperties.foreach(p => utils.addFileMetadata(UUID.randomUUID().toString, fileId.toString, p.name, p.value))
+    addDefaultMetaData(utils, fileId, false)
     utils.addFileMetadata(UUID.randomUUID.toString, fileId.toString, ClientSideOriginalFilepath, fileName)
     fileId
   }
 
-  private def addStaticMetaData(utils: TestUtils, parentId: UUID, addProperty: Boolean): Unit = {
-    staticMetadataProperties.foreach { staticMetadataProperty =>
+  private def addDefaultMetaData(utils: TestUtils, parentId: UUID, addProperty: Boolean): Unit = {
+    defaultMetadataProperties.foreach { defaultMetadataProperty =>
       if (addProperty) {
-        utils.addFileProperty(staticMetadataProperty)
+        utils.addFileProperty(defaultMetadataProperty)
       }
       utils.addFileMetadata(
         UUID.randomUUID().toString,
         parentId.toString,
-        staticMetadataProperty,
-        staticMetadataProperty match {
+        defaultMetadataProperty,
+        defaultMetadataProperty match {
           case RightsCopyright  => defaultCopyright
           case LegalStatus      => defaultLegalStatus
           case HeldBy           => defaultHeldBy

src/test/scala/uk/gov/nationalarchives/tdr/api/routes/FileRouteSpec.scala

@@ -4,9 +4,7 @@ import akka.http.scaladsl.model.headers.OAuth2BearerToken
 import com.dimafeng.testcontainers.PostgreSQLContainer
 import io.circe.generic.extras.Configuration
 import io.circe.generic.extras.auto._
-import org.scalatest.Assertion
 import org.scalatest.matchers.should.Matchers
-import uk.gov.nationalarchives.tdr.api.service.FileMetadataService.{FileReference, FileType, Filename, ParentReference, clientSideProperties}
 import uk.gov.nationalarchives.tdr.api.utils.TestAuthUtils._
 import uk.gov.nationalarchives.tdr.api.utils.TestContainerUtils._
 import uk.gov.nationalarchives.tdr.api.utils.TestUtils._
@@ -51,13 +49,13 @@ class FileRouteSpec extends TestContainerUtils with Matchers with TestRequest {
   "The api" should "add files and metadata entries for files and directories" in withContainers { case container: PostgreSQLContainer =>
     val consignmentId = UUID.fromString("c44f1b9b-1275-4bc3-831c-808c50a0222d")
     val utils = TestUtils(container.database)
-    (clientSideProperties ++ serverSideProperties ++ staticMetadataProperties).foreach(utils.addFileProperty)
+    (clientSideProperties ++ serverSideProperties ++ defaultMetadataProperties).foreach(utils.addFileProperty)
     utils.createConsignment(consignmentId, userId)
 
-    staticMetadataProperties.foreach { smp =>
+    defaultMetadataProperties.foreach { defaultMetadataProperty =>
       utils.createFilePropertyValues(
-        smp,
-        smp match {
+        defaultMetadataProperty,
+        defaultMetadataProperty match {
           case RightsCopyright  => defaultCopyright
           case LegalStatus      => defaultLegalStatus
           case HeldBy           => defaultHeldBy
@@ -72,8 +70,8 @@ class FileRouteSpec extends TestContainerUtils with Matchers with TestRequest {
     val res = runTestMutationFileMetadata("mutation_alldata_2", validUserToken())
     val distinctDirectoryCount = 3
     val fileCount = 5
-    val expectedCount = ((Filename :: FileType :: FileReference :: ParentReference :: staticMetadataProperties).size * distinctDirectoryCount) +
-      (staticMetadataProperties.size * fileCount) +
+    val expectedCount = ((Filename :: FileType :: FileReference :: ParentReference :: defaultMetadataProperties).size * distinctDirectoryCount) +
+      (defaultMetadataProperties.size * fileCount) +
       (clientSideProperties.size * fileCount) +
       (serverSideProperties.size * fileCount) +
       distinctDirectoryCount
@@ -92,7 +90,7 @@ class FileRouteSpec extends TestContainerUtils with Matchers with TestRequest {
   "The api" should "return file ids matched with sequence ids for addFilesAndMetadata" in withContainers { case container: PostgreSQLContainer =>
     val consignmentId = UUID.fromString("1cd5e07a-34c8-4751-8e81-98edd17d1729")
     val utils = TestUtils(container.database)
-    (clientSideProperties ++ serverSideProperties ++ staticMetadataProperties).foreach(utils.addFileProperty)
+    (clientSideProperties ++ serverSideProperties ++ defaultMetadataProperties).foreach(utils.addFileProperty)
     utils.createConsignment(consignmentId, userId)
 
     val expectedResponse = expectedFilesAndMetadataMutationResponse("data_all")

src/test/scala/uk/gov/nationalarchives/tdr/api/service/FileServiceSpec.scala

@@ -736,7 +736,7 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S
     })
     val expectedSize = 56
     metadataRows.size should equal(expectedSize)
-    staticMetadataProperties.foreach(prop => {
+    defaultMetadataProperties.foreach(prop => {
       metadataRows.count(_.propertyname == prop) should equal(5)
     })
 
@@ -822,7 +822,7 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S
     file.get.parentreference should equal(Some("ref2"))
     val expectedSize = 56
     metadataRows.size should equal(expectedSize)
-    staticMetadataProperties.foreach(prop => {
+    defaultMetadataProperties.foreach(prop => {
       metadataRows.count(_.propertyname == prop) should equal(5)
     })
 
@@ -907,7 +907,7 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S
     })
     val expectedSize = 36
     metadataRows.size should equal(expectedSize)
-    staticMetadataProperties.foreach(prop => {
+    defaultMetadataProperties.foreach(prop => {
       metadataRows.count(_.propertyname == prop) should equal(3)
     })
 
@@ -1341,10 +1341,10 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S
   }
 
   private def mockCustomMetadataValuesResponse(customMetadataMock: CustomMetadataPropertiesRepository): ScalaOngoingStubbing[Future[Seq[FilepropertyvaluesRow]]] = {
-    val staticMetadataRows = staticMetadataProperties.map(staticMetadata => {
+    val defaultMetadataRows = defaultMetadataProperties.map(defaultMetadata => {
       FilepropertyvaluesRow(
-        staticMetadata,
-        staticMetadata match {
+        defaultMetadata,
+        defaultMetadata match {
           case RightsCopyright  => defaultCopyright
           case LegalStatus      => defaultLegalStatus
           case HeldBy           => defaultHeldBy
@@ -1355,6 +1355,6 @@ class FileServiceSpec extends AnyFlatSpec with MockitoSugar with Matchers with S
       )
     })
 
-    when(customMetadataMock.getCustomMetadataValuesWithDefault).thenReturn(Future(staticMetadataRows))
+    when(customMetadataMock.getCustomMetadataValuesWithDefault).thenReturn(Future(defaultMetadataRows))
   }
 }

src/test/scala/uk/gov/nationalarchives/tdr/api/utils/TestUtils.scala

@@ -579,7 +579,7 @@ object TestUtils {
 
   val defaultFileId: UUID = UUID.fromString("07a3a4bd-0281-4a6d-a4c1-8fa3239e1313")
   val serverSideProperties: List[String] = List(FileReference, ParentReference)
-  val staticMetadataProperties: List[String] = List(RightsCopyright, LegalStatus, HeldBy, Language, FoiExemptionCode)
+  val defaultMetadataProperties: List[String] = List(RightsCopyright, LegalStatus, HeldBy, Language, FoiExemptionCode)
   val defaultCopyright: String = "Crown Copyright"
   val defaultLegalStatus: String = "Public Record"
   val defaultHeldBy: String = "TNA"