diff --git a/client/fire-webclient/src/main/kotlin/uk/ac/ebi/fire/client/api/FireWebClient.kt b/client/fire-webclient/src/main/kotlin/uk/ac/ebi/fire/client/api/FireWebClient.kt index 52521e726..8c9ce5e0c 100644 --- a/client/fire-webclient/src/main/kotlin/uk/ac/ebi/fire/client/api/FireWebClient.kt +++ b/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(downloadUrl) - Files.write(tmpFile.toPath(), fileContent ?: byteArrayOf()) - return tmpFile + return when (val fileContent = template.getForObjectOrNull("/objects/blob/path/$path")) { + null -> null + else -> toFile(fileContent) + } } override fun findByMd5(md5: String): List = diff --git a/client/fire-webclient/src/main/kotlin/uk/ac/ebi/fire/client/integration/web/FireClient.kt b/client/fire-webclient/src/main/kotlin/uk/ac/ebi/fire/client/integration/web/FireClient.kt index 08d7338c7..e1d0aa5dc 100644 --- a/client/fire-webclient/src/main/kotlin/uk/ac/ebi/fire/client/integration/web/FireClient.kt +++ b/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 fun findByPath(path: String): FireApiFile? diff --git a/client/fire-webclient/src/main/kotlin/uk/ac/ebi/fire/client/integration/web/RetryWebClient.kt b/client/fire-webclient/src/main/kotlin/uk/ac/ebi/fire/client/integration/web/RetryWebClient.kt index 30572c639..9417c1fb8 100644 --- a/client/fire-webclient/src/main/kotlin/uk/ac/ebi/fire/client/integration/web/RetryWebClient.kt +++ b/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 { val opt = "Find file md5='$md5'" return template.execute(opt) { fireClient.findByMd5(md5) } diff --git a/client/fire-webclient/src/test/kotlin/uk/ac/ebi/fire/client/api/FireWebClientTest.kt b/client/fire-webclient/src/test/kotlin/uk/ac/ebi/fire/client/api/FireWebClientTest.kt index 791ba6bcb..745219512 100644 --- a/client/fire-webclient/src/test/kotlin/uk/ac/ebi/fire/client/api/FireWebClientTest.kt +++ b/client/fire-webclient/src/test/kotlin/uk/ac/ebi/fire/client/api/FireWebClientTest.kt @@ -88,10 +88,10 @@ class FireWebClientTest( @MockK fireFile: FireApiFile, ) { val file = tmpFolder.createFile("test.txt", "test content") + val fireFilePath = "S-BSST1/file1.txt" - every { template.getForObject("/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")!! @@ -99,41 +99,18 @@ class FireWebClientTest( assertThat(downloadedFile.readText()).isEqualTo("test content") assertThat(downloadedFile.absolutePath).isEqualTo("${tmpFolder.root.absolutePath}/file1.txt") verify(exactly = 1) { - template.getForObject("/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("/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("/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 diff --git a/submission/persistence-filesystem/src/main/kotlin/ac/uk/ebi/biostd/persistence/filesystem/fire/FireFilesService.kt b/submission/persistence-filesystem/src/main/kotlin/ac/uk/ebi/biostd/persistence/filesystem/fire/FireFilesService.kt index 40facec64..7a87effa5 100644 --- a/submission/persistence-filesystem/src/main/kotlin/ac/uk/ebi/biostd/persistence/filesystem/fire/FireFilesService.kt +++ b/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) } } } diff --git a/submission/persistence-filesystem/src/test/kotlin/ac/uk/ebi/biostd/persistence/filesystem/fire/FireFilesServiceTest.kt b/submission/persistence-filesystem/src/test/kotlin/ac/uk/ebi/biostd/persistence/filesystem/fire/FireFilesServiceTest.kt index f0dc70cc3..9dfb62e5a 100644 --- a/submission/persistence-filesystem/src/test/kotlin/ac/uk/ebi/biostd/persistence/filesystem/fire/FireFilesServiceTest.kt +++ b/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") diff --git a/submission/submitter/src/main/kotlin/ac/uk/ebi/biostd/submission/helpers/SubmissionFilesSource.kt b/submission/submitter/src/main/kotlin/ac/uk/ebi/biostd/submission/helpers/SubmissionFilesSource.kt index 18832f935..9630ccc43 100644 --- a/submission/submitter/src/main/kotlin/ac/uk/ebi/biostd/submission/helpers/SubmissionFilesSource.kt +++ b/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!!) } } } diff --git a/submission/submitter/src/main/kotlin/ac/uk/ebi/biostd/submission/submitter/request/SubmissionRequestCleaner.kt b/submission/submitter/src/main/kotlin/ac/uk/ebi/biostd/submission/submitter/request/SubmissionRequestCleaner.kt index f16a08067..c62396f36 100644 --- a/submission/submitter/src/main/kotlin/ac/uk/ebi/biostd/submission/submitter/request/SubmissionRequestCleaner.kt +++ b/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 } }