Pivotal ID # 184485190: S3 Client For FIRE Download Operations

client/fire-webclient/src/main/kotlin/uk/ac/ebi/fire/client/api/FireWebClient.kt

@@ -48,20 +48,17 @@ internal class FireWebClient(
 
     override fun downloadByPath(
         path: String,
-    ): File? = findByPath(path)?.let {
-        downloadFireFile(path.substringAfterLast("/"), "/objects/blob/path/$path")
-    }
-
-    override fun downloadByFireId(
-        fireOid: String,
-        fileName: String,
-    ): File = downloadFireFile(fileName, "/objects/blob/$fireOid")
+    ): File? {
+        fun toFile(data: ByteArray): File {
+            val tmpFile = File(tmpDirPath, path.substringAfterLast("/"))
+            Files.write(tmpFile.toPath(), data)
+            return tmpFile
+        }
 
-    private fun downloadFireFile(fileName: String, downloadUrl: String): File {
-        val tmpFile = File(tmpDirPath, fileName)
-        val fileContent = template.getForObject<ByteArray?>(downloadUrl)
-        Files.write(tmpFile.toPath(), fileContent ?: byteArrayOf())
-        return tmpFile
+        return when (val fileContent = template.getForObjectOrNull<ByteArray?>("/objects/blob/path/$path")) {
+            null -> null
+            else -> toFile(fileContent)
+        }
     }
 
     override fun findByMd5(md5: String): List<FireApiFile> =

client/fire-webclient/src/main/kotlin/uk/ac/ebi/fire/client/integration/web/FireClient.kt

@@ -13,8 +13,6 @@ interface FireClient {
 
     fun downloadByPath(path: String): File?
 
-    fun downloadByFireId(fireOid: String, fileName: String): File
-
     fun findByMd5(md5: String): List<FireApiFile>
 
     fun findByPath(path: String): FireApiFile?

client/fire-webclient/src/main/kotlin/uk/ac/ebi/fire/client/integration/web/RetryWebClient.kt

@@ -30,11 +30,6 @@ internal class RetryWebClient(
         return template.execute(opt) { fireClient.downloadByPath(path) }
     }
 
-    override fun downloadByFireId(fireOid: String, fileName: String): File {
-        val opt = "Download file fireOid='$fireOid', fileName='$fileName'"
-        return template.execute(opt) { fireClient.downloadByFireId(fireOid, fileName) }
-    }
-
     override fun findByMd5(md5: String): List<FireApiFile> {
         val opt = "Find file md5='$md5'"
         return template.execute(opt) { fireClient.findByMd5(md5) }

client/fire-webclient/src/test/kotlin/uk/ac/ebi/fire/client/api/FireWebClientTest.kt

@@ -88,52 +88,29 @@ class FireWebClientTest(
         @MockK fireFile: FireApiFile,
     ) {
         val file = tmpFolder.createFile("test.txt", "test content")
+        val fireFilePath = "S-BSST1/file1.txt"
 
-        every { template.getForObject<FireApiFile>("/objects/path/S-BSST1/file1.txt") } returns fireFile
         every {
-            template.getForObject("/objects/blob/path/S-BSST1/file1.txt", ByteArray::class.java)
+            template.getForObject("/objects/blob/path/$fireFilePath", ByteArray::class.java)
         } returns file.readBytes()
 
         val downloadedFile = testInstance.downloadByPath("S-BSST1/file1.txt")!!
 
         assertThat(downloadedFile.readText()).isEqualTo("test content")
         assertThat(downloadedFile.absolutePath).isEqualTo("${tmpFolder.root.absolutePath}/file1.txt")
         verify(exactly = 1) {
-            template.getForObject<FireApiFile>("/objects/path/S-BSST1/file1.txt")
-            template.getForObject("/objects/blob/path/S-BSST1/file1.txt", ByteArray::class.java)
+            template.getForObject("/objects/blob/path/$fireFilePath", ByteArray::class.java)
         }
     }
 
     @Test
     fun `download by path not found`() {
+        val fireFilePath = "S-BSST1/file1.txt"
         every {
-            template.getForObject<FireApiFile>("/objects/path/S-BSST1/file1.txt")
+            template.getForObject("/objects/blob/path/$fireFilePath", ByteArray::class.java)
         } throws HttpClientErrorException(NOT_FOUND, "file not found")
 
-        assertThat(testInstance.downloadByPath("S-BSST1/file1.txt")).isNull()
-        verify(exactly = 1) {
-            template.getForObject<FireApiFile>("/objects/path/S-BSST1/file1.txt")
-        }
-        verify(exactly = 0) {
-            template.getForObject("/objects/blob/path/S-BSST1/file1.txt", ByteArray::class.java)
-        }
-    }
-
-    @Test
-    fun `download by fireId`() {
-        val file = tmpFolder.createFile("test.txt", "test content")
-
-        every {
-            template.getForObject("/objects/blob/fireOId", ByteArray::class.java)
-        } returns file.readBytes()
-
-        val downloadedFile = testInstance.downloadByFireId("fireOId", "file1.txt")
-
-        assertThat(downloadedFile.readText()).isEqualTo("test content")
-        assertThat(downloadedFile.absolutePath).isEqualTo("${tmpFolder.root.absolutePath}/file1.txt")
-        verify(exactly = 1) {
-            template.getForObject("/objects/blob/fireOId", ByteArray::class.java)
-        }
+        assertThat(testInstance.downloadByPath(fireFilePath)).isNull()
     }
 
     @Test

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

@@ -68,16 +68,16 @@ class FireFilesService(
     }
 
     private fun getOrCreate(
-        file: FireFile,
+        fireFile: FireFile,
         expectedPath: String,
     ): FireFile {
-        return when (file.firePath) {
-            expectedPath -> file
-            null -> setMetadata(file.fireId, file, expectedPath, file.published)
+        return when (val path = fireFile.firePath) {
+            expectedPath -> fireFile
+            null -> setMetadata(fireFile.fireId, fireFile, expectedPath, fireFile.published)
             else -> {
-                val downloaded = client.downloadByFireId(file.fireId, file.fileName)
-                val saved = client.save(downloaded, file.md5, file.size)
-                setMetadata(saved.fireOid, file, expectedPath, false)
+                val file = requireNotNull(client.downloadByPath(path)) { "Could not download file with path $path" }
+                val saved = client.save(file, fireFile.md5, fireFile.size)
+                setMetadata(saved.fireOid, fireFile, expectedPath, false)
             }
         }
     }

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

@@ -62,7 +62,7 @@ internal class FireFilesServiceTest(
 
             val newFile = tempFolder.createFile("file.txt", "content")
             val newFireFile = fireApiFile(firePath = null)
-            every { fireClient.downloadByFireId(file.fireId, file.fileName) } returns newFile
+            every { fireClient.downloadByPath("/another-path/file.txt") } returns newFile
             every { fireClient.save(newFile, file.md5, file.size) } answers { newFireFile }
             every { fireClient.setPath(newFireFile.fireOid, "/001/Files/folder/file.txt") } answers { nothing }
 
@@ -145,7 +145,7 @@ internal class FireFilesServiceTest(
 
         @Test
         fun `delete submission files`(
-            @MockK submission: ExtSubmission
+            @MockK submission: ExtSubmission,
         ) {
             val file = fireFile(md5 = "md1", firePath = "path_1")
 
@@ -161,7 +161,7 @@ internal class FireFilesServiceTest(
 
         @Test
         fun `delete submission file`(
-            @MockK submission: ExtSubmission
+            @MockK submission: ExtSubmission,
         ) {
             val file = fireFile(firePath = "a file path")
             every { fireClient.delete(file.fireId) } answers { nothing }
@@ -174,7 +174,7 @@ internal class FireFilesServiceTest(
         @Test
         fun `delete nfs file`(
             @MockK nfsFile: NfsFile,
-            @MockK submission: ExtSubmission
+            @MockK submission: ExtSubmission,
         ) {
             val exception = assertFails { testInstance.deleteSubmissionFile(submission, nfsFile) }
             assertThat(exception.message).isEqualTo("FireFilesService should only handle FireFile")

submission/submitter/src/main/kotlin/ac/uk/ebi/biostd/submission/helpers/SubmissionFilesSource.kt

@@ -38,7 +38,7 @@ class SubmissionFilesSource(
     private fun getFile(file: ExtFile): File? {
         return when (file) {
             is NfsFile -> nfsFiles.getFile(file.filePath)
-            is FireFile -> fireClient.downloadByFireId(file.fireId, file.fileName)
+            is FireFile -> fireClient.downloadByPath(file.firePath!!)
         }
     }
 }

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

@@ -52,6 +52,5 @@ class SubmissionRequestCleaner(
     private fun newFilesMap(new: ExtSubmission) =
         filesRequestService
             .getSubmissionRequestFiles(new.accNo, new.version, 0)
-            .groupBy { it.path }
-            .mapValues { it.value.first().file.md5 }
+            .associate { it.path to it.file.md5 }
 }