Pivotal ID # 184364448: Previos version file check taking longer than…

… expected (#678)

https://www.pivotaltracker.com/story/show/184364448
- updated api to avoid re query submission on each call

submission/persistence-common-api/src/main/kotlin/ac/uk/ebi/biostd/persistence/common/service/SubmissionOperations.kt

@@ -49,9 +49,9 @@ interface SubmissionPersistenceQueryService {
      **/
     fun getSubmissionsByUser(owner: String, filter: SubmissionFilter): List<BasicSubmission>
 
-    fun getReferencedFiles(accNo: String, fileListName: String): List<ExtFile>
+    fun getReferencedFiles(sub: ExtSubmission, fileListName: String): List<ExtFile>
 
-    fun findReferencedFile(accNo: String, version: Int, path: String): ExtFile?
+    fun findReferencedFile(sub: ExtSubmission, path: String): ExtFile?
 }
 
 @Suppress("TooManyFunctions")

submission/persistence-mongo/src/main/kotlin/ac/uk/ebi/biostd/persistence/doc/db/data/FileListDocFileDocDataRepository.kt

@@ -3,5 +3,5 @@ package ac.uk.ebi.biostd.persistence.doc.db.data
 import ac.uk.ebi.biostd.persistence.doc.db.repositories.FileListDocFileRepository
 
 class FileListDocFileDocDataRepository(
-    private val fileListDocFileRepository: FileListDocFileRepository
+    private val fileListDocFileRepository: FileListDocFileRepository,
 ) : FileListDocFileRepository by fileListDocFileRepository

submission/persistence-mongo/src/main/kotlin/ac/uk/ebi/biostd/persistence/doc/db/repositories/Repositories.kt

@@ -104,7 +104,7 @@ interface FileListDocFileRepository : MongoRepository<FileListDocFile, ObjectId>
         fileListName: String,
     ): List<FileListDocFile>
 
-    @Query("{ 'submissionAccNo': ?0, 'submissionVersion': ?1, 'file.filePath':  ?2}")
+    @Query("{ 'submissionAccNo': ?0, 'submissionVersion': ?1, 'file.filePath': ?2}")
     fun findBySubmissionAccNoAndSubmissionVersionAndFilePath(
         accNo: String,
         version: Int,

submission/persistence-mongo/src/main/kotlin/ac/uk/ebi/biostd/persistence/doc/integration/MongoDbReposConfig.kt

@@ -23,20 +23,20 @@ class MongoDbReposConfig {
     @Bean
     internal fun submissionDocDataRepository(
         mongoTemplate: MongoTemplate,
-        submissionMongoRepository: SubmissionMongoRepository
+        submissionMongoRepository: SubmissionMongoRepository,
     ): SubmissionDocDataRepository = SubmissionDocDataRepository(submissionMongoRepository, mongoTemplate)
 
     @Bean
     internal fun submissionStatsDataRepository(
         mongoTemplate: MongoTemplate,
-        submissionStatsRepository: SubmissionStatsRepository
+        submissionStatsRepository: SubmissionStatsRepository,
     ): SubmissionStatsDataRepository = SubmissionStatsDataRepository(mongoTemplate, submissionStatsRepository)
 
     @Bean
     internal fun submissionRequestDocDataRepository(
         mongoTemplate: MongoTemplate,
         extSerializationService: ExtSerializationService,
-        submissionRequestRepository: SubmissionRequestRepository
+        submissionRequestRepository: SubmissionRequestRepository,
     ): SubmissionRequestDocDataRepository = SubmissionRequestDocDataRepository(
         mongoTemplate,
         extSerializationService,
@@ -46,11 +46,11 @@ class MongoDbReposConfig {
     @Bean
     internal fun submissionDraftDocDataRepository(
         mongoTemplate: MongoTemplate,
-        submissionDraftRepository: SubmissionDraftRepository
+        submissionDraftRepository: SubmissionDraftRepository,
     ): SubmissionDraftDocDataRepository = SubmissionDraftDocDataRepository(submissionDraftRepository, mongoTemplate)
 
     @Bean
     internal fun fileListDocFileDocDataRepository(
-        fileListDocFileRepository: FileListDocFileRepository
+        fileListDocFileRepository: FileListDocFileRepository,
     ): FileListDocFileDocDataRepository = FileListDocFileDocDataRepository(fileListDocFileRepository)
 }

submission/persistence-mongo/src/main/kotlin/ac/uk/ebi/biostd/persistence/doc/integration/MongoDbServicesConfig.kt

@@ -6,6 +6,7 @@ import ac.uk.ebi.biostd.persistence.common.service.SubmissionDraftPersistenceSer
 import ac.uk.ebi.biostd.persistence.common.service.SubmissionPersistenceQueryService
 import ac.uk.ebi.biostd.persistence.common.service.SubmissionRequestFilesPersistenceService
 import ac.uk.ebi.biostd.persistence.common.service.SubmissionRequestPersistenceService
+import ac.uk.ebi.biostd.persistence.doc.db.data.FileListDocFileDocDataRepository
 import ac.uk.ebi.biostd.persistence.doc.db.data.SubmissionDocDataRepository
 import ac.uk.ebi.biostd.persistence.doc.db.data.SubmissionDraftDocDataRepository
 import ac.uk.ebi.biostd.persistence.doc.db.data.SubmissionRequestDocDataRepository
@@ -38,7 +39,7 @@ import uk.ac.ebi.serialization.common.FilesResolver
 class MongoDbServicesConfig {
     @Bean
     internal fun submissionQueryService(
-        fileListDocFileRepository: FileListDocFileRepository,
+        fileListDocFileRepository: FileListDocFileDocDataRepository,
         submissionDocDataRepository: SubmissionDocDataRepository,
         submissionRequestDocDataRepository: SubmissionRequestDocDataRepository,
         serializationService: ExtSerializationService,

submission/persistence-mongo/src/main/kotlin/ac/uk/ebi/biostd/persistence/doc/migrations/DatabaseChangeLog.kt

@@ -1,6 +1,7 @@
 package ac.uk.ebi.biostd.persistence.doc.migrations
 
 import ac.uk.ebi.biostd.persistence.doc.commons.ensureExists
+import ac.uk.ebi.biostd.persistence.doc.db.converters.shared.DocFileFields
 import ac.uk.ebi.biostd.persistence.doc.db.converters.shared.DocSectionFields.SEC_TYPE
 import ac.uk.ebi.biostd.persistence.doc.db.converters.shared.DocSubmissionFields.SUB
 import ac.uk.ebi.biostd.persistence.doc.db.converters.shared.DocSubmissionFields.SUB_ACC_NO
@@ -18,6 +19,7 @@ import ac.uk.ebi.biostd.persistence.doc.db.converters.shared.DocSubmissionReques
 import ac.uk.ebi.biostd.persistence.doc.db.converters.shared.DocSubmissionRequestFileFields.RQT_FILE_PATH
 import ac.uk.ebi.biostd.persistence.doc.db.converters.shared.DocSubmissionRequestFileFields.RQT_FILE_SUB_ACC_NO
 import ac.uk.ebi.biostd.persistence.doc.db.converters.shared.DocSubmissionRequestFileFields.RQT_FILE_SUB_VERSION
+import ac.uk.ebi.biostd.persistence.doc.db.converters.shared.FileListDocFileFields.FILE_LIST_DOC_FILE_FILE
 import ac.uk.ebi.biostd.persistence.doc.db.converters.shared.FileListDocFileFields.FILE_LIST_DOC_FILE_FILE_LIST_NAME
 import ac.uk.ebi.biostd.persistence.doc.db.converters.shared.FileListDocFileFields.FILE_LIST_DOC_FILE_INDEX
 import ac.uk.ebi.biostd.persistence.doc.db.converters.shared.FileListDocFileFields.FILE_LIST_DOC_FILE_SUBMISSION_ACC_NO
@@ -187,3 +189,18 @@ class ChangeLog007 {
         }
     }
 }
+
+@ChangeLog
+class ChangeLog008 {
+    @ChangeSet(order = "008", id = "File List files index", author = "System")
+    fun changeSet007(template: MongockTemplate) {
+        template.indexOps(FileListDocFile::class.java).apply {
+            ensureIndex(
+                Index()
+                    .on(FILE_LIST_DOC_FILE_SUBMISSION_ACC_NO, ASC)
+                    .on(FILE_LIST_DOC_FILE_SUBMISSION_VERSION, ASC)
+                    .on("$FILE_LIST_DOC_FILE_FILE.${DocFileFields.FILE_DOC_FILEPATH}", ASC)
+            )
+        }
+    }
+}

submission/persistence-mongo/src/main/kotlin/ac/uk/ebi/biostd/persistence/doc/service/SubmissionMongoPersistenceQueryService.kt

@@ -3,9 +3,9 @@ package ac.uk.ebi.biostd.persistence.doc.service
 import ac.uk.ebi.biostd.persistence.common.model.BasicSubmission
 import ac.uk.ebi.biostd.persistence.common.request.SubmissionFilter
 import ac.uk.ebi.biostd.persistence.common.service.SubmissionPersistenceQueryService
+import ac.uk.ebi.biostd.persistence.doc.db.data.FileListDocFileDocDataRepository
 import ac.uk.ebi.biostd.persistence.doc.db.data.SubmissionDocDataRepository
 import ac.uk.ebi.biostd.persistence.doc.db.data.SubmissionRequestDocDataRepository
-import ac.uk.ebi.biostd.persistence.doc.db.repositories.FileListDocFileRepository
 import ac.uk.ebi.biostd.persistence.doc.db.repositories.getByAccNo
 import ac.uk.ebi.biostd.persistence.doc.mapping.to.ToExtSubmissionMapper
 import ac.uk.ebi.biostd.persistence.doc.mapping.to.toExtFile
@@ -23,7 +23,7 @@ internal class SubmissionMongoPersistenceQueryService(
     private val submissionRepo: SubmissionDocDataRepository,
     private val toExtSubmissionMapper: ToExtSubmissionMapper,
     private val serializationService: ExtSerializationService,
-    private val fileListDocFileRepository: FileListDocFileRepository,
+    private val fileListDocFileRepository: FileListDocFileDocDataRepository,
     private val requestRepository: SubmissionRequestDocDataRepository,
 ) : SubmissionPersistenceQueryService {
     override fun existByAccNo(accNo: String): Boolean {
@@ -72,20 +72,24 @@ internal class SubmissionMongoPersistenceQueryService(
             .plus(findSubmissions(owner, submissionFilter))
     }
 
-    override fun getReferencedFiles(accNo: String, fileListName: String): List<ExtFile> {
-        val subData = submissionRepo.getSubData(accNo)
+    override fun getReferencedFiles(
+        sub: ExtSubmission,
+        fileListName: String,
+    ): List<ExtFile> {
         return fileListDocFileRepository
-            .findAllBySubmissionAccNoAndSubmissionVersionGreaterThanAndFileListName(accNo, 0, fileListName)
-            .map { it.file.toExtFile(subData.released, subData.relPath) }
+            .findAllBySubmissionAccNoAndSubmissionVersionGreaterThanAndFileListName(sub.accNo, 0, fileListName)
+            .map { it.file.toExtFile(sub.released, sub.relPath) }
     }
 
-    override fun findReferencedFile(accNo: String, version: Int, path: String): ExtFile? {
-        val subData = submissionRepo.getSubData(accNo)
+    override fun findReferencedFile(
+        sub: ExtSubmission,
+        path: String,
+    ): ExtFile? {
         return fileListDocFileRepository
-            .findBySubmissionAccNoAndSubmissionVersionAndFilePath(accNo, version, path)
+            .findBySubmissionAccNoAndSubmissionVersionAndFilePath(sub.accNo, sub.version, path)
             .firstOrNull()
             ?.file
-            ?.toExtFile(subData.released, submissionRepo.getSubData(accNo).relPath)
+            ?.toExtFile(sub.released, sub.relPath)
     }
 
     private fun findSubmissions(owner: String, filter: SubmissionFilter): List<BasicSubmission> =

submission/persistence-mongo/src/test/kotlin/ac/uk/ebi/biostd/persistence/doc/db/repositories/FileListDocFileRepositoryTest.kt

@@ -0,0 +1,64 @@
+package ac.uk.ebi.biostd.persistence.doc.db.repositories
+
+import ac.uk.ebi.biostd.persistence.doc.db.data.FileListDocFileDocDataRepository
+import ac.uk.ebi.biostd.persistence.doc.integration.MongoDbReposConfig
+import ac.uk.ebi.biostd.persistence.doc.model.FileListDocFile
+import ac.uk.ebi.biostd.persistence.doc.model.FireDocFile
+import ebi.ac.uk.db.MINIMUM_RUNNING_TIME
+import ebi.ac.uk.db.MONGO_VERSION
+import ebi.ac.uk.extended.model.ExtFileType
+import org.assertj.core.api.Assertions.assertThat
+import org.bson.types.ObjectId
+import org.junit.jupiter.api.Test
+import org.junit.jupiter.api.extension.ExtendWith
+import org.springframework.beans.factory.annotation.Autowired
+import org.springframework.boot.test.context.SpringBootTest
+import org.springframework.test.context.DynamicPropertyRegistry
+import org.springframework.test.context.DynamicPropertySource
+import org.springframework.test.context.junit.jupiter.SpringExtension
+import org.testcontainers.containers.MongoDBContainer
+import org.testcontainers.containers.startupcheck.MinimumDurationRunningStartupCheckStrategy
+import org.testcontainers.junit.jupiter.Container
+import org.testcontainers.junit.jupiter.Testcontainers
+import org.testcontainers.utility.DockerImageName
+import java.time.Duration
+
+@ExtendWith(SpringExtension::class)
+@Testcontainers
+@SpringBootTest(classes = [MongoDbReposConfig::class])
+class FileListDocFileRepositoryTest(
+    @Autowired private val repository: FileListDocFileDocDataRepository,
+) {
+
+    @Test
+    fun findBySubmissionAccNoAndSubmissionVersionAndFilePath() {
+        val file = FireDocFile("filename", "filePath", "relPath", "fireId", listOf(), "md5", 1L, ExtFileType.FILE.value)
+        val fileListFile = FileListDocFile(
+            id = ObjectId(),
+            submissionId = ObjectId(),
+            file = file,
+            fileListName = "file-list",
+            index = 0,
+            submissionVersion = 1,
+            submissionAccNo = "S-TEST123"
+        )
+        repository.save(fileListFile)
+
+        val result = repository.findBySubmissionAccNoAndSubmissionVersionAndFilePath("S-TEST123", 1, "filePath")
+
+        assertThat(result).containsExactly(fileListFile)
+    }
+
+    companion object {
+        @Container
+        val mongoContainer: MongoDBContainer = MongoDBContainer(DockerImageName.parse(MONGO_VERSION))
+            .withStartupCheckStrategy(MinimumDurationRunningStartupCheckStrategy(Duration.ofSeconds(MINIMUM_RUNNING_TIME)))
+
+        @JvmStatic
+        @DynamicPropertySource
+        fun propertySource(register: DynamicPropertyRegistry) {
+            register.add("spring.data.mongodb.uri") { mongoContainer.getReplicaSetUrl("biostudies-test") }
+            register.add("spring.data.mongodb.database") { "biostudies-test" }
+        }
+    }
+}

submission/persistence-mongo/src/test/kotlin/ac/uk/ebi/biostd/persistence/doc/service/SubmissionMongoQueryServiceTest.kt

@@ -7,10 +7,11 @@ import ac.uk.ebi.biostd.persistence.common.model.RequestStatus.FILES_COPIED
 import ac.uk.ebi.biostd.persistence.common.model.RequestStatus.LOADED
 import ac.uk.ebi.biostd.persistence.common.model.RequestStatus.REQUESTED
 import ac.uk.ebi.biostd.persistence.common.request.SubmissionFilter
+import ac.uk.ebi.biostd.persistence.doc.db.data.FileListDocFileDocDataRepository
 import ac.uk.ebi.biostd.persistence.doc.db.data.SubmissionDocDataRepository
 import ac.uk.ebi.biostd.persistence.doc.db.data.SubmissionRequestDocDataRepository
-import ac.uk.ebi.biostd.persistence.doc.db.repositories.FileListDocFileRepository
 import ac.uk.ebi.biostd.persistence.doc.integration.MongoDbReposConfig
+import ac.uk.ebi.biostd.persistence.doc.integration.MongoDbServicesConfig
 import ac.uk.ebi.biostd.persistence.doc.mapping.from.toDocFile
 import ac.uk.ebi.biostd.persistence.doc.mapping.to.ToExtSubmissionMapper
 import ac.uk.ebi.biostd.persistence.doc.model.DocAttribute
@@ -39,7 +40,6 @@ import ebi.ac.uk.model.constants.ProcessingStatus.PROCESSING
 import ebi.ac.uk.util.collections.second
 import io.github.glytching.junit.extension.folder.TemporaryFolder
 import io.github.glytching.junit.extension.folder.TemporaryFolderExtension
-import io.mockk.impl.annotations.MockK
 import io.mockk.junit5.MockKExtension
 import org.assertj.core.api.Assertions.assertThat
 import org.bson.types.ObjectId
@@ -51,6 +51,8 @@ import org.junit.jupiter.api.assertThrows
 import org.junit.jupiter.api.extension.ExtendWith
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.boot.test.context.SpringBootTest
+import org.springframework.context.annotation.Bean
+import org.springframework.context.annotation.Configuration
 import org.springframework.test.context.DynamicPropertyRegistry
 import org.springframework.test.context.DynamicPropertySource
 import org.springframework.test.context.junit.jupiter.SpringExtension
@@ -61,6 +63,8 @@ import org.testcontainers.junit.jupiter.Testcontainers
 import org.testcontainers.utility.DockerImageName
 import uk.ac.ebi.extended.serialization.integration.ExtSerializationConfig.extSerializationService
 import uk.ac.ebi.extended.serialization.service.ExtSerializationService
+import uk.ac.ebi.serialization.common.FilesResolver
+import java.nio.file.Files
 import java.time.Duration.ofSeconds
 import java.time.Instant
 import java.time.OffsetDateTime
@@ -73,12 +77,12 @@ import ac.uk.ebi.biostd.persistence.doc.test.doc.testDocSubmission as docSubmiss
 
 @ExtendWith(MockKExtension::class, SpringExtension::class, TemporaryFolderExtension::class)
 @Testcontainers
-@SpringBootTest(classes = [MongoDbReposConfig::class])
+@SpringBootTest(classes = [MongoDbReposConfig::class, MongoDbServicesConfig::class, TestConfiguration::class])
 internal class SubmissionMongoQueryServiceTest(
     private val tempFolder: TemporaryFolder,
-    @MockK private val toExtSubmissionMapper: ToExtSubmissionMapper,
+    @Autowired private val toExtSubmissionMapper: ToExtSubmissionMapper,
     @Autowired private val submissionRepo: SubmissionDocDataRepository,
-    @Autowired private val fileListDocFileRepository: FileListDocFileRepository,
+    @Autowired private val fileListDocFileRepository: FileListDocFileDocDataRepository,
     @Autowired private val requestRepository: SubmissionRequestDocDataRepository,
 ) {
     private val serializationService: ExtSerializationService = extSerializationService()
@@ -376,25 +380,25 @@ internal class SubmissionMongoQueryServiceTest(
         private val nfsReferencedFile = tempFolder.createFile("nfsReferenced.txt")
         private val nfsFile = createNfsFile("nfsReferenced.txt", "Files/nfsReferenced.txt", nfsReferencedFile)
         private val nfsFileListFile = FileListDocFile(
-            ObjectId(),
-            testDocSubmission.id,
-            nfsFile.toDocFile(),
+            id = ObjectId(),
+            submissionId = testDocSubmission.id,
+            file = nfsFile.toDocFile(),
             fileListName = "test-file-list",
             index = 1,
             submissionVersion = testDocSubmission.version,
             submissionAccNo = testDocSubmission.accNo
         )
         private val fireReferencedFile = tempFolder.createFile("fireReferenced.txt")
         private val fireFile = FireFile(
-            "fire-oid",
-            null,
-            false,
-            "fireReferenced.txt",
-            "Files/fireReferenced.txt",
-            fireReferencedFile.md5(),
-            fireReferencedFile.size(),
-            FILE,
-            emptyList()
+            fireId = "fire-oid",
+            firePath = null,
+            published = false,
+            filePath = "fireReferenced.txt",
+            relPath = "Files/fireReferenced.txt",
+            md5 = fireReferencedFile.md5(),
+            size = fireReferencedFile.size(),
+            type = FILE,
+            attributes = emptyList()
         )
         private val fireFileListFile = FileListDocFile(
             ObjectId(),
@@ -418,7 +422,8 @@ internal class SubmissionMongoQueryServiceTest(
 
         @Test
         fun `get referenced files`() {
-            val files = testInstance.getReferencedFiles(SUB_ACC_NO, "test-file-list")
+            val submission = testInstance.getExtByAccNo(SUB_ACC_NO)
+            val files = testInstance.getReferencedFiles(submission, "test-file-list")
             assertThat(files).hasSize(2)
 
             val nfsFile = files.first() as NfsFile
@@ -441,15 +446,17 @@ internal class SubmissionMongoQueryServiceTest(
             submissionRepo.save(innerSectionSubmission)
             fileListDocFileRepository.save(nfsFileListFile.copy(submissionAccNo = innerSectionSubmission.accNo))
 
-            val files = testInstance.getReferencedFiles("S-REF1", "test-file-list")
+            val submission = testInstance.getExtByAccNo("S-REF1")
+            val files = testInstance.getReferencedFiles(submission, "test-file-list")
             assertThat(files).hasSize(1)
             assertThat((files.first() as NfsFile).file).isEqualTo(nfsReferencedFile)
             assertThat(nfsFile.fullPath).isEqualTo(nfsReferencedFile.absolutePath)
         }
 
         @Test
         fun `non existing referenced files`() {
-            assertThat(testInstance.getReferencedFiles(SUB_ACC_NO, "non-existing-fileListName")).hasSize(0)
+            val submission = testInstance.getExtByAccNo(SUB_ACC_NO)
+            assertThat(testInstance.getReferencedFiles(submission, "non-existing-fileListName")).hasSize(0)
         }
     }
 
@@ -472,3 +479,11 @@ internal class SubmissionMongoQueryServiceTest(
         }
     }
 }
+
+@Configuration
+class TestConfiguration {
+    @Bean
+    fun fileResolver(): FilesResolver {
+        return FilesResolver(Files.createTempDirectory("sub-requests").toFile())
+    }
+}