Pivotal ID # 184822139: Delete Submission Folder After Resubmission

submission/persistence-filesystem/src/main/kotlin/ac/uk/ebi/biostd/persistence/filesystem/api/FileStorageService.kt

@@ -9,9 +9,7 @@ interface FileStorageService {
 
     fun persistSubmissionFile(sub: ExtSubmission, file: ExtFile): ExtFile
 
-    fun deleteSubmissionFiles(sub: ExtSubmission)
-
     fun deleteSubmissionFile(sub: ExtSubmission, file: ExtFile)
 
-    fun deleteFtpFile(sub: ExtSubmission, file: ExtFile)
+    fun deleteSubmissionFiles(sub: ExtSubmission)
 }

submission/persistence-filesystem/src/main/kotlin/ac/uk/ebi/biostd/persistence/filesystem/api/FilesService.kt

@@ -6,8 +6,6 @@ import ebi.ac.uk.extended.model.ExtSubmission
 internal interface FilesService {
     fun persistSubmissionFile(sub: ExtSubmission, file: ExtFile): ExtFile
 
-    fun deleteSubmissionFiles(sub: ExtSubmission)
-
     fun deleteSubmissionFile(sub: ExtSubmission, file: ExtFile)
 
     fun deleteFtpFile(sub: ExtSubmission, file: ExtFile)

submission/persistence-filesystem/src/main/kotlin/ac/uk/ebi/biostd/persistence/filesystem/fire/FireFilesService.kt

@@ -7,16 +7,10 @@ import ebi.ac.uk.extended.model.FireFile
 import ebi.ac.uk.extended.model.NfsFile
 import ebi.ac.uk.extended.model.asFireFile
 import ebi.ac.uk.extended.model.expectedPath
-import mu.KotlinLogging
-import uk.ac.ebi.extended.serialization.service.ExtSerializationService
-import uk.ac.ebi.extended.serialization.service.fileSequence
 import uk.ac.ebi.fire.client.integration.web.FireClient
 
-private val logger = KotlinLogging.logger {}
-
 class FireFilesService(
     private val client: FireClient,
-    private val serializationService: ExtSerializationService,
 ) : FilesService {
     /**
      * Get or persist the given ext file from FIRE. Note that this method assumes that all the fire files belonging to
@@ -71,18 +65,4 @@ class FireFilesService(
     override fun deleteFtpFile(sub: ExtSubmission, file: ExtFile) {
         // No need to delete FTP links on FIRE as file deleting complete this
     }
-
-    override fun deleteSubmissionFiles(sub: ExtSubmission) {
-        fun deleteFile(index: Int, file: FireFile) {
-            logger.info { "${sub.accNo} ${sub.owner} Deleting file $index, path='${file.filePath}'" }
-            deleteSubmissionFile(sub, file)
-        }
-
-        logger.info { "${sub.accNo} ${sub.owner} Started cleaning submission files for ${sub.accNo}" }
-        serializationService
-            .fileSequence(sub)
-            .filterIsInstance(FireFile::class.java)
-            .forEachIndexed { index, file -> deleteFile(index, file) }
-        logger.info { "${sub.accNo} ${sub.owner} Finished cleaning submission files for ${sub.accNo}" }
-    }
 }

submission/persistence-filesystem/src/main/kotlin/ac/uk/ebi/biostd/persistence/filesystem/nfs/NfsFilesService.kt

@@ -62,14 +62,12 @@ class NfsFilesService(
         return getOrCreateFolder(submissionPath, permissions).toFile()
     }
 
-    override fun deleteSubmissionFiles(sub: ExtSubmission) {
-        deleteFtpFolder(sub)
-        deleteSubFolder(sub)
-    }
-
     override fun deleteSubmissionFile(sub: ExtSubmission, file: ExtFile) {
         require(file is NfsFile) { "NfsFilesService should only handle NfsFile" }
-        FileUtils.deleteFile(folderResolver.getSubFolder(sub.relPath).resolve(file.relPath).toFile())
+
+        val subDirectory = folderResolver.getSubFolder(sub.relPath)
+        val subFile = subDirectory.resolve(file.relPath).toFile()
+        FileUtils.deleteFile(subFile)
     }
 
     override fun deleteFtpFile(sub: ExtSubmission, file: ExtFile) {
@@ -78,16 +76,4 @@ class NfsFilesService(
         FileUtils.deleteFile(subFolder.resolve(file.relPath).toFile())
         logger.info { "${sub.accNo} ${sub.owner} Finished un-publishing files of submission ${sub.accNo} on NFS" }
     }
-
-    private fun deleteSubFolder(sub: ExtSubmission) {
-        logger.info { "${sub.accNo} ${sub.owner} Started deleting files of submission ${sub.accNo} on NFS" }
-        FileUtils.deleteFile(folderResolver.getSubFolder(sub.relPath).toFile())
-        logger.info { "${sub.accNo} ${sub.owner} Finished deleting files of submission ${sub.accNo} on NFS" }
-    }
-
-    private fun deleteFtpFolder(sub: ExtSubmission) {
-        logger.info { "${sub.accNo} ${sub.owner} Started un-publishing files of submission ${sub.accNo} on NFS" }
-        FileUtils.deleteFile(folderResolver.getSubmissionFtpFolder(sub.relPath).toFile())
-        logger.info { "${sub.accNo} ${sub.owner} Finished un-publishing files of submission ${sub.accNo} on NFS" }
-    }
 }

submission/persistence-filesystem/src/main/kotlin/ac/uk/ebi/biostd/persistence/filesystem/service/StorageService.kt

@@ -10,13 +10,19 @@ import ebi.ac.uk.extended.model.ExtSubmission
 import ebi.ac.uk.extended.model.StorageMode
 import ebi.ac.uk.extended.model.StorageMode.FIRE
 import ebi.ac.uk.extended.model.StorageMode.NFS
+import mu.KotlinLogging
+import uk.ac.ebi.extended.serialization.service.ExtSerializationService
+import uk.ac.ebi.extended.serialization.service.fileSequence
+
+private val logger = KotlinLogging.logger {}
 
 @Suppress("LongParameterList")
 class StorageService(
     private val fireFtpService: FireFtpService,
     private val fireFilesService: FireFilesService,
     private val nfsFtpService: NfsFtpService,
     private val nfsFilesService: NfsFilesService,
+    private val serializationService: ExtSerializationService,
 ) : FileStorageService {
     override fun persistSubmissionFile(sub: ExtSubmission, file: ExtFile): ExtFile =
         when (sub.storageMode) {
@@ -31,21 +37,19 @@ class StorageService(
         }
     }
 
-    override fun deleteFtpFile(sub: ExtSubmission, file: ExtFile) =
-        when (sub.storageMode) {
-            FIRE -> fireFilesService.deleteFtpFile(sub, file)
-            NFS -> nfsFilesService.deleteFtpFile(sub, file)
+    override fun deleteSubmissionFile(sub: ExtSubmission, file: ExtFile) = when (sub.storageMode) {
+        FIRE -> {
+            fireFilesService.deleteSubmissionFile(sub, file)
+            fireFilesService.deleteFtpFile(sub, file)
         }
 
-    override fun deleteSubmissionFile(sub: ExtSubmission, file: ExtFile) =
-        when (sub.storageMode) {
-            FIRE -> fireFilesService.deleteSubmissionFile(sub, file)
-            NFS -> nfsFilesService.deleteSubmissionFile(sub, file)
+        NFS -> {
+            nfsFilesService.deleteSubmissionFile(sub, file)
+            nfsFilesService.deleteFtpFile(sub, file)
         }
+    }
 
-    override fun deleteSubmissionFiles(sub: ExtSubmission) =
-        when (sub.storageMode) {
-            FIRE -> fireFilesService.deleteSubmissionFiles(sub)
-            NFS -> nfsFilesService.deleteSubmissionFiles(sub)
-        }
+    override fun deleteSubmissionFiles(sub: ExtSubmission) {
+        serializationService.fileSequence(sub).forEach { file -> deleteSubmissionFile(sub, file) }
+    }
 }

submission/persistence-filesystem/src/test/kotlin/ac/uk/ebi/biostd/persistence/filesystem/fire/FireFilesServiceTest.kt

@@ -20,7 +20,6 @@ import org.junit.jupiter.api.Nested
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.extension.ExtendWith
 import uk.ac.ebi.extended.serialization.service.ExtSerializationService
-import uk.ac.ebi.extended.serialization.service.fileSequence
 import uk.ac.ebi.fire.client.integration.web.FireClient
 import uk.ac.ebi.fire.client.model.FileSystemEntry
 import uk.ac.ebi.fire.client.model.FireApiFile
@@ -34,7 +33,7 @@ internal class FireFilesServiceTest(
     @MockK private val submission: ExtSubmission,
     @MockK private val serializationService: ExtSerializationService,
 ) {
-    private val testInstance = FireFilesService(fireClient, serializationService)
+    private val testInstance = FireFilesService(fireClient)
 
     @BeforeEach
     fun beforeEach() {
@@ -142,22 +141,6 @@ internal class FireFilesServiceTest(
             mockkStatic("uk.ac.ebi.extended.serialization.service.ExtSerializationServiceExtKt")
         }
 
-        @Test
-        fun `delete submission files`(
-            @MockK submission: ExtSubmission,
-        ) {
-            val file = fireFile(md5 = "md1", firePath = "path_1")
-
-            every { submission.accNo } returns "S-BSST1"
-            every { submission.owner } returns "user@mail.org"
-            every { serializationService.fileSequence(submission) } returns sequenceOf(file)
-            every { fireClient.delete(file.fireId) } answers { nothing }
-
-            testInstance.deleteSubmissionFiles(submission)
-
-            verify(exactly = 1) { fireClient.delete(file.fireId) }
-        }
-
         @Test
         fun `delete submission file`(
             @MockK submission: ExtSubmission,

submission/persistence-filesystem/src/test/kotlin/ac/uk/ebi/biostd/persistence/filesystem/nfs/NfsFilesServiceTest.kt

@@ -59,18 +59,6 @@ class NfsFilesServiceTest(
         assertThat(Files.exists(Paths.get("${subFolder.absolutePath}/${sub.relPath}/${fireFile.relPath}"))).isTrue()
     }
 
-    @Test
-    fun `delete submission files`() {
-        val subFolder = subFolder.createDirectory("S-BSST1")
-        val ftpFolder = ftpFolder.createDirectory("S-BSST1")
-        val sub = basicExtSubmission.copy(relPath = "S-BSST1")
-
-        testInstance.deleteSubmissionFiles(sub)
-
-        assertThat(Files.exists(subFolder.toPath())).isFalse()
-        assertThat(Files.exists(ftpFolder.toPath())).isFalse()
-    }
-
     @Test
     fun `delete ftp links`() {
         val ftpFolder = ftpFolder.createDirectory("S-BSST2")

submission/submission-webapp/src/main/kotlin/ac/uk/ebi/biostd/common/config/FilePersistenceConfig.kt

@@ -36,7 +36,9 @@ class FilePersistenceConfig(
         fireFilesService: FireFilesService,
         nfsFtpService: NfsFtpService,
         nfsFilesService: NfsFilesService,
-    ): FileStorageService = StorageService(fireFtpService, fireFilesService, nfsFtpService, nfsFilesService)
+        extSerializationService: ExtSerializationService,
+    ): FileStorageService =
+        StorageService(fireFtpService, fireFilesService, nfsFtpService, nfsFilesService, extSerializationService)
 
     @Bean
     fun nfsFtpService(): NfsFtpService = NfsFtpService(folderResolver)
@@ -48,9 +50,7 @@ class FilePersistenceConfig(
     fun fireFtpService(): FireFtpService = FireFtpService(fireClient)
 
     @Bean
-    fun fireFileService(
-        serializationService: ExtSerializationService,
-    ): FireFilesService = FireFilesService(fireClient, serializationService)
+    fun fireFileService(): FireFilesService = FireFilesService(fireClient)
 
     @Bean
     fun pageTabService(

submission/submission-webapp/src/main/kotlin/ac/uk/ebi/biostd/common/config/SubmissionConfig.kt

@@ -71,8 +71,8 @@ class SubmissionConfig(
         extSubmissionSubmitter,
         submissionSubmitter,
         eventsPublisherService,
-        fileStorageService,
         submissionPersistenceService,
+        fileStorageService
     )
 
     @Bean

submission/submission-webapp/src/main/kotlin/ac/uk/ebi/biostd/submission/domain/service/SubmissionService.kt

@@ -24,8 +24,8 @@ class SubmissionService(
     private val extSubmissionSubmitter: ExtSubmissionSubmitter,
     private val submissionSubmitter: SubmissionSubmitter,
     private val eventsPublisherService: EventsPublisherService,
-    private val fileStorageService: FileStorageService,
     private val submissionPersistenceService: SubmissionPersistenceService,
+    private val fileStorageService: FileStorageService,
 ) {
     fun submit(rqt: SubmitRequest): ExtSubmission {
         logger.info { "${rqt.accNo} ${rqt.owner} Received sync submit request with draft key '${rqt.draftKey}'" }

submission/submission-webapp/src/main/resources/application.yml

@@ -72,6 +72,11 @@ app:
     username: # FIRE user
     password: # FIRE password
     version: v1.1
+    s3AccessKey: # FIRE s3 access key
+    s3SecretKey: # FIRE s3 access key
+    s3region: anyRegion # Fire s3 region
+    s3endpoint: # Fire s3 region
+    s3bucket: # Fire s3 region
     retry:
       maxAttempts: 20
       initialInterval: 100

submission/submitter/src/main/kotlin/ac/uk/ebi/biostd/submission/submitter/request/SubmissionRequestCleaner.kt

@@ -40,7 +40,6 @@ class SubmissionRequestCleaner(
         fun deleteFile(index: Int, file: ExtFile) {
             logger.info { "${current.accNo} ${current.owner} Deleting file $index, path='${file.filePath}'" }
             storageService.deleteSubmissionFile(current, file)
-            storageService.deleteFtpFile(current, file)
         }
 
         fun shouldDelete(newFiles: Map<String, FileEntry>, existing: ExtFile): Boolean =

submission/submitter/src/main/kotlin/ac/uk/ebi/biostd/submission/submitter/request/SubmissionRequestFinalizer.kt

@@ -30,22 +30,24 @@ class SubmissionRequestFinalizer(
         return sub
     }
 
-    private fun deleteRemainingFiles(sub: ExtSubmission, previous: ExtSubmission) {
+    private fun deleteRemainingFiles(current: ExtSubmission?, previous: ExtSubmission) {
         fun deleteFile(index: Int, file: ExtFile) {
             logger.info { "${previous.accNo} ${previous.owner} Deleting file $index, path='${file.filePath}'" }
             storageService.deleteSubmissionFile(previous, file)
         }
 
-        val subFiles = subFilesSet(sub)
-        logger.info { "${sub.accNo} ${sub.owner} Started deleting remaining submission files" }
+        val subFiles = subFilesSet(current)
+        logger.info { "${previous.accNo} ${previous.owner} Started deleting remaining submission files" }
         serializationService.fileSequence(previous)
-            .filter { subFiles.contains(it.filePath).not() || it.storageMode != sub.storageMode }
+            .filter { subFiles.contains(it.filePath).not() || it.storageMode != current?.storageMode }
             .forEachIndexed { index, file -> deleteFile(index, file) }
-        logger.info { "${sub.accNo} ${sub.owner} Finished deleting remaining submission files" }
+        logger.info { "${previous.accNo} ${previous.owner} Finished deleting remaining submission files" }
     }
 
-    private fun subFilesSet(sub: ExtSubmission) =
-        serializationService.fileSequence(sub)
-            .map { it.filePath }
-            .toSet()
+    private fun subFilesSet(sub: ExtSubmission?): Set<String> {
+        return when (sub) {
+            null -> emptySet()
+            else -> serializationService.fileSequence(sub).mapTo(mutableSetOf()) { it.filePath }
+        }
+    }
 }

submission/submitter/src/test/kotlin/ac/uk/ebi/biostd/submission/submitter/request/SubmissionRequestCleanerTest.kt

@@ -64,7 +64,6 @@ class SubmissionRequestCleanerTest(
 
         verify(exactly = 1) { requestService.saveSubmissionRequest(cleanedRequest) }
         verify(exactly = 0) {
-            storageService.deleteFtpFile(any(), any())
             storageService.deleteSubmissionFile(any(), any())
         }
     }
@@ -89,7 +88,6 @@ class SubmissionRequestCleanerTest(
 
         every { loadedRequest.submission } returns new
 
-        every { storageService.deleteFtpFile(current, currentFile) } answers { nothing }
         every { loadedRequest.withNewStatus(CLEANED) } returns cleanedRequest
         every { queryService.findExtByAccNo("S-BSST1", true) } returns current
         every { requestService.getLoadedRequest("S-BSST1", 2) } returns loadedRequest
@@ -101,7 +99,6 @@ class SubmissionRequestCleanerTest(
         testInstance.cleanCurrentVersion("S-BSST1", 2)
 
         verify(exactly = 1) {
-            storageService.deleteFtpFile(current, currentFile)
             requestService.saveSubmissionRequest(cleanedRequest)
             storageService.deleteSubmissionFile(current, currentFile)
         }