diff --git a/backend/src/main/kotlin/org/loculus/backend/api/SubmissionTypes.kt b/backend/src/main/kotlin/org/loculus/backend/api/SubmissionTypes.kt index 147307f8ae..508f210603 100644 --- a/backend/src/main/kotlin/org/loculus/backend/api/SubmissionTypes.kt +++ b/backend/src/main/kotlin/org/loculus/backend/api/SubmissionTypes.kt @@ -331,7 +331,6 @@ data class AccessionVersionOriginalMetadata( data class GetOriginalDataRequest( @Schema( description = "The group ID to download data for.", - required = true, ) val groupId: Int, @Schema( @@ -343,6 +342,7 @@ data class GetOriginalDataRequest( data class OriginalDataResponse( override val accession: Accession, override val version: Version, + val submissionId: String, val originalData: OriginalData, ) : AccessionVersionInterface diff --git a/backend/src/main/kotlin/org/loculus/backend/controller/SubmissionController.kt b/backend/src/main/kotlin/org/loculus/backend/controller/SubmissionController.kt index 08602c4263..4ccc90093f 100644 --- a/backend/src/main/kotlin/org/loculus/backend/controller/SubmissionController.kt +++ b/backend/src/main/kotlin/org/loculus/backend/controller/SubmissionController.kt @@ -443,7 +443,6 @@ open class SubmissionController( organism, groupIdsFilter?.takeIf { it.isNotEmpty() }, statusesFilter?.takeIf { it.isNotEmpty() }, - null, ) headers.add(X_TOTAL_RECORDS, totalRecords.toString()) // TODO(https://github.com/loculus-project/loculus/issues/2778) @@ -459,7 +458,6 @@ open class SubmissionController( groupIdsFilter?.takeIf { it.isNotEmpty() }, statusesFilter?.takeIf { it.isNotEmpty() }, fields?.takeIf { it.isNotEmpty() }, - null, ) } @@ -535,17 +533,16 @@ open class SubmissionController( } } } + val duration = System.currentTimeMillis() - startTime + log.info { "[get-original-data] Completed in ${duration}ms" } } catch (e: Exception) { val duration = System.currentTimeMillis() - startTime log.error(e) { "[get-original-data] Error after ${duration}ms: $e" } throw e + } finally { + MDC.remove(REQUEST_ID_MDC_KEY) + MDC.remove(ORGANISM_MDC_KEY) } - - val duration = System.currentTimeMillis() - startTime - log.info { "[get-original-data] Completed in ${duration}ms" } - - MDC.remove(REQUEST_ID_MDC_KEY) - MDC.remove(ORGANISM_MDC_KEY) } return ResponseEntity(streamBody, headers, HttpStatus.OK) @@ -556,7 +553,7 @@ open class SubmissionController( outputStream: java.io.OutputStream, isMultiSegmented: Boolean, ) { - val metadataKeys = data.flatMap { it.originalData.metadata.keys }.toSet().sorted() + val metadataKeys = data.flatMapTo(mutableSetOf()) { it.originalData.metadata.keys }.sorted() val headers = if (isMultiSegmented) { listOf("id", "accession", "fastaIds") + metadataKeys } else { @@ -565,11 +562,10 @@ open class SubmissionController( TsvWriter(outputStream, headers).use { writer -> for (entry in data) { - val id = "${entry.accession}.${entry.version}" + val id = entry.submissionId val metadataValues = metadataKeys.map { entry.originalData.metadata[it] ?: "" } val row = if (isMultiSegmented) { val fastaIds = entry.originalData.unalignedNucleotideSequences.keys - .map { originalFastaId -> "$id|$originalFastaId" } .joinToString(" ") listOf(id, entry.accession, fastaIds) + metadataValues } else { @@ -587,10 +583,9 @@ open class SubmissionController( ) { FastaWriter(outputStream).use { writer -> for (entry in data) { - val id = "${entry.accession}.${entry.version}" for ((originalFastaId, sequence) in entry.originalData.unalignedNucleotideSequences) { if (sequence != null) { - val header = if (isMultiSegmented) "$id|$originalFastaId" else id + val header = if (isMultiSegmented) originalFastaId else entry.submissionId writer.write(FastaEntry(header, sequence)) } } diff --git a/backend/src/main/kotlin/org/loculus/backend/service/submission/SubmissionDatabaseService.kt b/backend/src/main/kotlin/org/loculus/backend/service/submission/SubmissionDatabaseService.kt index 12a68ac776..c715b1ce95 100644 --- a/backend/src/main/kotlin/org/loculus/backend/service/submission/SubmissionDatabaseService.kt +++ b/backend/src/main/kotlin/org/loculus/backend/service/submission/SubmissionDatabaseService.kt @@ -75,7 +75,6 @@ import org.loculus.backend.api.getFileId import org.loculus.backend.auth.AuthenticatedUser import org.loculus.backend.config.BackendSpringProperty import org.loculus.backend.controller.BadRequestException -import org.loculus.backend.controller.ForbiddenException import org.loculus.backend.controller.ProcessingValidationException import org.loculus.backend.controller.UnprocessableEntityException import org.loculus.backend.log.AuditLogger @@ -1186,7 +1185,6 @@ class SubmissionDatabaseService( organism: Organism, groupIdsFilter: List?, statusesFilter: List?, - accessionVersionsFilter: List?, ): Op { val organismCondition = SequenceEntriesView.organismIs(organism) val groupCondition = getGroupCondition(groupIdsFilter, authenticatedUser) @@ -1195,14 +1193,7 @@ class SubmissionDatabaseService( } else { Op.TRUE } - val accessionVersionCondition = if (accessionVersionsFilter != null) { - SequenceEntriesView.accessionVersionIsIn(accessionVersionsFilter) - } else { - Op.TRUE - } - val conditions = organismCondition and groupCondition and statusCondition and accessionVersionCondition - - return conditions + return organismCondition and groupCondition and statusCondition } fun countOriginalMetadata( @@ -1210,7 +1201,6 @@ class SubmissionDatabaseService( organism: Organism, groupIdsFilter: List?, statusesFilter: List?, - accessionVersionsFilter: List?, ): Long = SequenceEntriesView .selectAll() .where( @@ -1219,7 +1209,6 @@ class SubmissionDatabaseService( organism, groupIdsFilter, statusesFilter, - accessionVersionsFilter, ), ) .count() @@ -1230,7 +1219,6 @@ class SubmissionDatabaseService( groupIdsFilter: List?, statusesFilter: List?, fields: List?, - accessionVersionsFilter: List?, ): Sequence { val originalMetadata = SequenceEntriesView.originalDataColumn // It's actually ?> but exposed does not support nullable types here @@ -1251,7 +1239,6 @@ class SubmissionDatabaseService( organism, groupIdsFilter, statusesFilter, - accessionVersionsFilter, ), ) .fetchSize(streamBatchSize) @@ -1315,6 +1302,7 @@ class SubmissionDatabaseService( .select( SequenceEntriesView.accessionColumn, SequenceEntriesView.versionColumn, + SequenceEntriesView.submissionIdColumn, SequenceEntriesView.originalDataColumn, ) .where(originalDataConditions(organism, groupId, accessionsFilter)) @@ -1328,6 +1316,7 @@ class SubmissionDatabaseService( OriginalDataResponse( it[SequenceEntriesView.accessionColumn], it[SequenceEntriesView.versionColumn], + it[SequenceEntriesView.submissionIdColumn], decompressedOriginalData, ) } diff --git a/backend/src/test/kotlin/org/loculus/backend/controller/submission/GetOriginalDataEndpointTest.kt b/backend/src/test/kotlin/org/loculus/backend/controller/submission/GetOriginalDataEndpointTest.kt index c65de6505f..3f096590c0 100644 --- a/backend/src/test/kotlin/org/loculus/backend/controller/submission/GetOriginalDataEndpointTest.kt +++ b/backend/src/test/kotlin/org/loculus/backend/controller/submission/GetOriginalDataEndpointTest.kt @@ -13,7 +13,6 @@ import org.loculus.backend.controller.expectUnauthorizedResponse import org.loculus.backend.controller.generateJwtFor import org.loculus.backend.controller.groupmanagement.GroupManagementControllerClient import org.loculus.backend.controller.groupmanagement.andGetGroupId -import org.loculus.backend.controller.jwtForDefaultUser import org.loculus.backend.controller.submission.SubmitFiles.DefaultFiles import org.springframework.beans.factory.annotation.Autowired import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content @@ -95,7 +94,7 @@ class GetOriginalDataEndpointTest( @Test fun `GIVEN data exists THEN metadata TSV contains id and accession columns`() { val submissionResult = convenienceClient.submitDefaultFiles() - val accessionVersions = convenienceClient.prepareDefaultSequenceEntriesToApprovedForRelease( + convenienceClient.prepareDefaultSequenceEntriesToApprovedForRelease( groupId = submissionResult.groupId, ) @@ -116,14 +115,14 @@ class GetOriginalDataEndpointTest( assertThat(header[1], `is`("accession")) val firstDataRow = lines[1].split("\t") - assertThat(firstDataRow[0], `is`(accessionVersions[0].displayAccessionVersion())) - assertThat(firstDataRow[1], `is`(accessionVersions[0].accession)) + assertThat(firstDataRow[0], `is`(submissionResult.submissionIdMappings[0].submissionId)) + assertThat(firstDataRow[1], `is`(submissionResult.submissionIdMappings[0].accession)) } @Test fun `GIVEN data exists THEN FASTA headers match metadata ids`() { val submissionResult = convenienceClient.submitDefaultFiles() - val accessionVersions = convenienceClient.prepareDefaultSequenceEntriesToApprovedForRelease( + convenienceClient.prepareDefaultSequenceEntriesToApprovedForRelease( groupId = submissionResult.groupId, ) @@ -150,7 +149,7 @@ class GetOriginalDataEndpointTest( assertThat(fastaIds, `is`(metadataIds)) - val expectedIds = accessionVersions.map { it.displayAccessionVersion() }.toSet() + val expectedIds = submissionResult.submissionIdMappings.map { it.submissionId }.toSet() assertThat(metadataIds, `is`(expectedIds)) } @@ -278,9 +277,9 @@ class GetOriginalDataEndpointTest( } @Test - fun `GIVEN multi-segmented organism THEN fastaIds contain accession version and original fasta id`() { + fun `GIVEN multi-segmented organism THEN fastaIds contain original fasta ids`() { val groupId = groupManagementClient.createNewGroup().andGetGroupId() - val accessionVersions = convenienceClient.prepareDefaultSequenceEntriesToApprovedForRelease( + convenienceClient.prepareDefaultSequenceEntriesToApprovedForRelease( organism = OTHER_ORGANISM, groupId = groupId, ) @@ -303,16 +302,16 @@ class GetOriginalDataEndpointTest( val fastaIdsIndex = header.indexOf("fastaIds") val firstDataRow = lines[1].split("\t") + val submissionId = firstDataRow[0] val fastaIds = firstDataRow[fastaIdsIndex] - val accessionVersion = accessionVersions[0].displayAccessionVersion() - assertThat(fastaIds, containsString("$accessionVersion|")) + assertThat(fastaIds, containsString("${submissionId}_")) } @Test - fun `GIVEN multi-segmented organism THEN FASTA headers contain accession version and original fasta id`() { + fun `GIVEN multi-segmented organism THEN FASTA headers contain original fasta ids`() { val groupId = groupManagementClient.createNewGroup().andGetGroupId() - val accessionVersions = convenienceClient.prepareDefaultSequenceEntriesToApprovedForRelease( + convenienceClient.prepareDefaultSequenceEntriesToApprovedForRelease( organism = OTHER_ORGANISM, groupId = groupId, ) @@ -331,14 +330,8 @@ class GetOriginalDataEndpointTest( val (_, sequencesFasta) = extractZipContents(zipContent) val fastaHeaders = sequencesFasta.lines().filter { it.startsWith(">") } - val accessionVersion = accessionVersions[0].displayAccessionVersion() - val matchingHeaders = fastaHeaders.filter { it.contains("$accessionVersion|") } - assertThat(matchingHeaders.size, org.hamcrest.Matchers.greaterThan(0)) - - for (header in matchingHeaders) { - assertThat(header, containsString("|")) - } + assertThat(fastaHeaders.contains(">custom0_notOnlySegment"), `is`(true)) } @Test diff --git a/backend/src/test/kotlin/org/loculus/backend/controller/submission/OriginalDataRevisionWorkflowTest.kt b/backend/src/test/kotlin/org/loculus/backend/controller/submission/OriginalDataRevisionWorkflowTest.kt index 5964d2167f..b470201db9 100644 --- a/backend/src/test/kotlin/org/loculus/backend/controller/submission/OriginalDataRevisionWorkflowTest.kt +++ b/backend/src/test/kotlin/org/loculus/backend/controller/submission/OriginalDataRevisionWorkflowTest.kt @@ -33,7 +33,6 @@ class OriginalDataRevisionWorkflowTest( assertThat(accessionVersions, hasSize(greaterThan(0))) val firstAccession = accessionVersions[0].accession - val firstAccessionVersion = accessionVersions[0].displayAccessionVersion() val response = submissionControllerClient.getOriginalData(groupId = groupId) .andExpect(status().isOk) @@ -58,9 +57,9 @@ class OriginalDataRevisionWorkflowTest( val dateIndex = headers.indexOf("date") val firstDataRow = metadataLines[1].split("\t") - assertThat(firstDataRow[idIndex], `is`(firstAccessionVersion)) + assertThat(firstDataRow[idIndex], `is`("custom0")) assertThat(firstDataRow[accessionIndex], `is`(firstAccession)) - assertThat(sequencesFasta.contains(">$firstAccessionVersion"), `is`(true)) + assertThat(sequencesFasta.contains(">custom0"), `is`(true)) val originalDate = firstDataRow[dateIndex] val newDate = "2099-01-01" diff --git a/integration-tests/tests/specs/features/download-original-data.spec.ts b/integration-tests/tests/specs/features/download-original-data.spec.ts index 43194f63bb..a84780135c 100644 --- a/integration-tests/tests/specs/features/download-original-data.spec.ts +++ b/integration-tests/tests/specs/features/download-original-data.spec.ts @@ -21,10 +21,14 @@ test.describe('Download Original Data', () => { const submissionPage = new SingleSequenceSubmissionPage(page); const timestamp = Date.now(); + const submissionIds = Array.from( + { length: 2 }, + (_, i) => `download-test-${timestamp}-${i}`, + ); - for (let i = 0; i < 2; i++) { + for (const submissionId of submissionIds) { await submissionPage.completeSubmission( - createTestMetadata({ submissionId: `download-test-${timestamp}-${i}` }), + createTestMetadata({ submissionId }), createTestSequenceData(), ); } @@ -84,8 +88,11 @@ test.describe('Download Original Data', () => { .slice(1) .map((line) => line.split('\t')[accessionIndex]); + for (const submissionId of submissionIds) { + expect(downloadedIds).toContain(submissionId); + } + for (const av of accessionVersions) { - expect(downloadedIds).toContain(av.accessionVersion); expect(downloadedAccessions).toContain(av.accession); } @@ -94,8 +101,8 @@ test.describe('Download Original Data', () => { expect(fastaHeaders.length).toBe(2); - for (const av of accessionVersions) { - expect(fastaHeaders.some((h) => h.includes(av.accessionVersion))).toBe(true); + for (const submissionId of submissionIds) { + expect(fastaHeaders.some((h) => h.includes(submissionId))).toBe(true); } } finally { fs.rmSync(tmpDir, { recursive: true, force: true }); diff --git a/website/src/components/SearchPage/DownloadDialog/DownloadOriginalDataButton.tsx b/website/src/components/SearchPage/DownloadDialog/DownloadOriginalDataButton.tsx index c9384a8a4f..7ecb85e241 100644 --- a/website/src/components/SearchPage/DownloadDialog/DownloadOriginalDataButton.tsx +++ b/website/src/components/SearchPage/DownloadDialog/DownloadOriginalDataButton.tsx @@ -65,11 +65,6 @@ export const DownloadOriginalDataButton: FC = ( accessionVersions = selectedVersions; } else { accessionVersions = await fetchAccessions(); - if (accessionVersions.length > MAX_DOWNLOAD_ENTRIES) { - throw new Error( - `Too many sequences (${accessionVersions.length}). Please filter to ${MAX_DOWNLOAD_ENTRIES} or fewer.`, - ); - } } const accessions = extractAccessions(accessionVersions); @@ -125,7 +120,11 @@ export const DownloadOriginalDataButton: FC = ( {error !== null && (
{error} -
diff --git a/website/src/services/backendClientSideApi.ts b/website/src/services/backendClientSideApi.ts index 85246880c4..a59c8562a7 100644 --- a/website/src/services/backendClientSideApi.ts +++ b/website/src/services/backendClientSideApi.ts @@ -38,12 +38,23 @@ export async function getOriginalData( if (!response.ok) { const errorText = await response.text(); + let detail = errorText; + try { + const errorJson = JSON.parse(errorText) as { detail?: unknown; message?: unknown }; + detail = + (typeof errorJson.detail === 'string' && errorJson.detail) || + (typeof errorJson.message === 'string' && errorJson.message) || + errorText; + } catch { + // Keep text response when the backend did not return JSON. + } + return { ok: false, error: { status: response.status, statusText: response.statusText, - detail: errorText, + detail, }, }; }