Skip to content

Commit

Permalink
Merge pull request #735 from nationalarchives/TDR-3680_cleanup_old_va…
Browse files Browse the repository at this point in the history
…lidation_code

TDR-3680 - Remove old code to validate metadata
  • Loading branch information
vimleshtna authored Feb 2, 2024
2 parents 4f1f1e9 + 26e3431 commit fb89352
Show file tree
Hide file tree
Showing 9 changed files with 182 additions and 480 deletions.
4 changes: 0 additions & 4 deletions src/main/resources/application.base.conf
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ fileUpload {
batchSize = 250
}

featureAccessBlock {
blockValidationLibrary = ${BLOCK_VALIDATION_LIBRARY}
}

referenceGenerator {
referenceGeneratorUrl = ${REFERENCE_GENERATOR_URL}
referenceLimit = ${REFERENCE_GENERATOR_LIMIT}
Expand Down
4 changes: 0 additions & 4 deletions src/main/resources/application.conf
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ frontend = {
]
}

featureAccessBlock {
blockValidationLibrary = false
}

referenceGenerator {
referenceGeneratorUrl = "https://q59yzinibd.execute-api.eu-west-2.amazonaws.com"
referenceLimit = 500
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@ trait GraphQLServerBase {
val customMetadataPropertiesRepository = new CustomMetadataPropertiesRepository(db)
val customMetadataPropertiesService = new CustomMetadataPropertiesService(customMetadataPropertiesRepository)
val displayPropertiesService = new DisplayPropertiesService(displayPropertiesRepository)
val validateFileMetadataService =
new ValidateFileMetadataService(customMetadataPropertiesService, displayPropertiesService, fileMetadataRepository, fileStatusRepository, config)
val validateFileMetadataService = new ValidateFileMetadataService(customMetadataPropertiesService, displayPropertiesService, fileMetadataRepository, fileStatusRepository)
val consignmentStatusService = new ConsignmentStatusService(consignmentStatusRepository, fileStatusRepository, uuidSource, timeSource)
val fileMetadataService = new FileMetadataService(fileMetadataRepository, consignmentStatusService, customMetadataPropertiesService, validateFileMetadataService)
val ffidMetadataService = new FFIDMetadataService(ffidMetadataRepository, ffidMetadataMatchesRepository, timeSource, uuidSource)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class FileMetadataService(
for {
_ <- fileMetadataRepository.deleteFileMetadata(uniqueFileIds, distinctPropertyNames)
addedRows <- fileMetadataRepository.addFileMetadata(generateFileMetadataInput(uniqueFileIds, distinctMetadataProperties, userId))
_ <- validateFileMetadataService.validateAdditionalMetadata(uniqueFileIds, distinctPropertyNames)
_ <- validateFileMetadataService.validateAndAddAdditionalMetadataStatuses(uniqueFileIds, distinctPropertyNames)
_ <- consignmentStatusService.updateMetadataConsignmentStatus(consignmentId, List(DescriptiveMetadata, ClosureMetadata))
metadataPropertiesAdded = addedRows.map(r => { FileMetadata(r.propertyname, r.value) }).toSet
} yield BulkFileMetadata(uniqueFileIds.toSeq, metadataPropertiesAdded.toSeq)
Expand Down Expand Up @@ -92,7 +92,7 @@ class FileMetadataService(
}.toSeq
_ <- fileMetadataRepository.deleteFileMetadata(fileIds, allPropertiesToDelete)
_ <- fileMetadataRepository.addFileMetadata(metadataToReset)
_ <- validateFileMetadataService.validateAdditionalMetadata(fileIds, allPropertiesToDelete)
_ <- validateFileMetadataService.validateAndAddAdditionalMetadataStatuses(fileIds, allPropertiesToDelete)
_ <- consignmentStatusService.updateMetadataConsignmentStatus(consignmentId, List(DescriptiveMetadata, ClosureMetadata))
} yield DeleteFileMetadata(fileIds.toSeq, allPropertiesToDelete.toSeq)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package uk.gov.nationalarchives.tdr.api.service

import com.typesafe.config.Config
import uk.gov.nationalarchives.Tables
import uk.gov.nationalarchives.Tables.{FilemetadataRow, FilestatusRow}
import uk.gov.nationalarchives.Tables.FilestatusRow
import uk.gov.nationalarchives.tdr.api.db.repository.{FileMetadataRepository, FileStatusRepository}
import uk.gov.nationalarchives.tdr.api.graphql.fields.CustomMetadataFields.{CustomMetadataField, CustomMetadataValues}
import uk.gov.nationalarchives.tdr.api.graphql.fields.FileStatusFields.AddFileStatusInput
Expand All @@ -17,12 +15,9 @@ class ValidateFileMetadataService(
customMetadataService: CustomMetadataPropertiesService,
displayPropertiesService: DisplayPropertiesService,
fileMetadataRepository: FileMetadataRepository,
fileStatusRepository: FileStatusRepository,
config: Config
fileStatusRepository: FileStatusRepository
)(implicit val ec: ExecutionContext) {

val blockValidationLibrary: Boolean = config.getBoolean("featureAccessBlock.blockValidationLibrary")

def toPropertyNames(fields: Seq[CustomMetadataField]): Set[String] = fields.map(_.name).toSet

def toAdditionalMetadataFieldGroups(fields: Seq[CustomMetadataField]): Seq[FieldGroup] = {
Expand All @@ -38,21 +33,7 @@ class ValidateFileMetadataService(
})
}

def validateAdditionalMetadata(fileIds: Set[UUID], propertiesToValidate: Set[String]): Future[List[FilestatusRow]] = {
if (blockValidationLibrary) {
for {
customMetadataFields <- customMetadataService.getCustomMetadata
existingMetadataProperties: Seq[FilemetadataRow] <- fileMetadataRepository.getFileMetadata(None, Some(fileIds), Some(toPropertyNames(customMetadataFields)))
rows <- addMetadataFileStatuses(fileIds, propertiesToValidate, customMetadataFields, existingMetadataProperties)
} yield {
rows
}
} else {
validateAndAddAdditionalMetadataStatuses(fileIds, propertiesToValidate)
}
}

private def validateAndAddAdditionalMetadataStatuses(fileIds: Set[UUID], propertiesToValidate: Set[String]): Future[List[FilestatusRow]] = {
def validateAndAddAdditionalMetadataStatuses(fileIds: Set[UUID], propertiesToValidate: Set[String]): Future[List[FilestatusRow]] = {
for {
propertyNames <- displayPropertiesService.getActiveDisplayPropertyNames
result <- {
Expand Down Expand Up @@ -115,85 +96,5 @@ class ValidateFileMetadataService(
}
}

private def addMetadataFileStatuses(
fileIds: Set[UUID],
propertiesToValidate: Set[String],
customMetadataFields: Seq[CustomMetadataField],
existingMetadataProperties: Seq[Tables.FilemetadataRow]
): Future[List[Tables.FilestatusRow]] = {
val additionalMetadataFieldGroups: Seq[FieldGroup] = toAdditionalMetadataFieldGroups(customMetadataFields)
val additionalMetadataPropertyNames: Set[String] = additionalMetadataFieldGroups.flatMap(g => toPropertyNames(g.fields)).toSet

if (!propertiesToValidate.subsetOf(additionalMetadataPropertyNames)) {
Future.successful(List())
} else {
val additionalMetadataStatuses = {

additionalMetadataFieldGroups
.flatMap(group => {
val states = group.fields.flatMap(f => checkPropertyState(fileIds, f, existingMetadataProperties))
val filesWithNoAdditionalMetadataStatuses = fileIds
.filter(id => !states.map(_.fileId).contains(id))
.map(id => {
AddFileStatusInput(id, group.groupName, NotEntered)
})

val statuses = states
.groupBy(_.fileId)
.map(s => {
val (fileId, fileStates) = s
val status: String = s match {
case _ if fileStates.forall(_.existingValueMatchesDefault == true) => NotEntered
case _ if fileStates.forall(_.missingDependencies == false) => Completed
case _ => Incomplete
}
AddFileStatusInput(fileId, group.groupName, status)
})
statuses ++ filesWithNoAdditionalMetadataStatuses
})
.toList
}

for {
_ <- fileStatusRepository.deleteFileStatus(fileIds, Set(ClosureMetadata, DescriptiveMetadata))
rows <- fileStatusRepository.addFileStatuses(additionalMetadataStatuses)
} yield rows.toList
}
}

def checkPropertyState(fileIds: Set[UUID], fieldToCheck: CustomMetadataField, existingProperties: Seq[FilemetadataRow]): Seq[FilePropertyState] = {
val propertyToCheckName: String = fieldToCheck.name
val valueDependenciesGroups: Seq[FieldGroup] = toValueDependenciesGroups(fieldToCheck)
val fieldDefaultValue: Option[String] = fieldToCheck.defaultValue
val dependencyValues: Seq[String] = valueDependenciesGroups.map(_.groupName)

fileIds
.flatMap(id => {
val allExistingFileProperties: Seq[FilemetadataRow] = existingProperties.filter(_.fileid == id)
val existingPropertiesToValidate: Seq[FilemetadataRow] = allExistingFileProperties.filter(_.propertyname == propertyToCheckName)
val existingPropertiesWithDependencies: Seq[FilemetadataRow] = existingPropertiesToValidate.filter(ep => dependencyValues.contains(ep.value))
val expectedDependencies: Seq[CustomMetadataField] =
existingPropertiesWithDependencies.flatMap(epd => valueDependenciesGroups.filter(_.groupName == epd.value)).flatMap(_.fields)
val actualDependencyProperties = allExistingFileProperties.filter(p => expectedDependencies.map(_.name).contains(p.propertyname))

if (existingPropertiesToValidate.isEmpty) {
None
} else {
existingPropertiesToValidate.map(existingProperty => {
val existingPropertyValue: String = existingProperty.value
val missingDependencies: Boolean = actualDependencyProperties.size < expectedDependencies.size
val matchesDefault: Boolean = fieldDefaultValue match {
case Some(value) => value == existingPropertyValue
case _ => false
}
FilePropertyState(id, propertyToCheckName, missingDependencies, matchesDefault)
})
}
})
.toSeq
}

case class FilePropertyState(fileId: UUID, propertyName: String, missingDependencies: Boolean, existingValueMatchesDefault: Boolean)

case class FieldGroup(groupName: String, fields: Seq[CustomMetadataField])
}
4 changes: 0 additions & 4 deletions src/test/resources/application.conf
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@ fileUpload {
batchSize = 3
}

featureAccessBlock {
blockValidationLibrary = true
}

referenceGenerator {
referenceGeneratorUrl = "http://localhost:8008"
referenceLimit = 2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,15 @@ class FileMetadataRouteSpec extends TestContainerUtils with Matchers with TestRe
val fileOneId = UUID.fromString("51c55218-1322-4453-9ef8-2300ef1c0fef")
val fileTwoId = UUID.fromString("7076f399-b596-4161-a95d-e686c6435710")
val fileThreeId = UUID.fromString("d2e64eed-faff-45ac-9825-79548f681323")
utils.addFileProperty("ClosureType", propertyGroup)
utils.addFileProperty("newProperty1", propertyGroup)
utils.addFileProperty("existingPropertyUpdated1", propertyGroup)
utils.addFileProperty("existingPropertyNotUpdated1", propertyGroup)

utils.createDisplayProperty("newProperty1", "Active", "true", "boolean")
utils.createDisplayProperty("existingPropertyUpdated1", "Active", "true", "boolean")
utils.createDisplayProperty("existingPropertyNotUpdated1", "Active", "true", "boolean")

utils.createFile(fileOneId, consignmentId, NodeType.fileTypeIdentifier, "fileName", Some(folderOneId))
utils.createFile(fileTwoId, consignmentId)
utils.createFile(fileThreeId, consignmentId)
Expand Down
Loading

0 comments on commit fb89352

Please sign in to comment.