Skip to content

Commit

Permalink
Merge branch 'main' into enforce-schema
Browse files Browse the repository at this point in the history
  • Loading branch information
corneliusroemer authored Nov 20, 2024
2 parents db0b033 + a2907ce commit dc40b08
Show file tree
Hide file tree
Showing 11 changed files with 124 additions and 82 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,7 @@ class SubmissionDatabaseService(
else -> stringParam(versionComment)
},
SequenceEntriesTable.submissionIdColumn,
SequenceEntriesTable.submitterColumn,
stringParam(authenticatedUser.username),
SequenceEntriesTable.groupIdColumn,
dateTimeParam(dateProvider.getCurrentDateTime()),
booleanParam(true), SequenceEntriesTable.organismColumn,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import kotlinx.datetime.plus
import kotlinx.datetime.toLocalDateTime
import org.hamcrest.MatcherAssert.assertThat
import org.hamcrest.Matchers
import org.hamcrest.Matchers.allOf
import org.hamcrest.Matchers.containsInAnyOrder
import org.hamcrest.Matchers.hasEntry
import org.hamcrest.Matchers.`is`
import org.hamcrest.Matchers.not
import org.loculus.backend.api.AccessionVersion
Expand All @@ -19,8 +22,10 @@ import org.loculus.backend.api.Status
import org.loculus.backend.utils.DateProvider
import org.springframework.test.web.servlet.MvcResult
import org.springframework.test.web.servlet.ResultActions
import org.springframework.test.web.servlet.ResultMatcher
import org.springframework.test.web.servlet.result.MockMvcResultMatchers
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
import org.testcontainers.shaded.org.awaitility.Awaitility.await

Expand All @@ -35,6 +40,16 @@ fun AccessionVersionInterface.toAccessionVersion() = AccessionVersion(this.acces

fun List<AccessionVersionInterface>.getAccessionVersions() = map { it.toAccessionVersion() }

fun List<AccessionVersionInterface>.toAccessionVersionMatcher() = map { entry ->
allOf(
hasEntry("accession", entry.accession),
hasEntry("version", entry.version.toInt()),
)
}

fun jsonContainsAccessionVersionsInAnyOrder(expectedVersions: List<AccessionVersionInterface>): ResultMatcher =
jsonPath("\$[*]", containsInAnyOrder(expectedVersions.toAccessionVersionMatcher()))

fun addOrganismToPath(path: String, organism: String = DEFAULT_ORGANISM) = "/$organism/${path.trimStart('/')}"

val jacksonObjectMapper: ObjectMapper = jacksonObjectMapper().findAndRegisterModules()
Expand Down Expand Up @@ -77,6 +92,25 @@ fun SequenceEntryStatus.assertHasError(error: Boolean): SequenceEntryStatus {
return this
}

fun SequenceEntryStatus.assertSubmitterIs(submitter: String): SequenceEntryStatus {
assertThat(this.submitter, `is`(submitter))
return this
}

fun SequenceEntryStatus.assertGroupIdIs(groupId: Int): SequenceEntryStatus {
assertThat(this.groupId, `is`(groupId))
return this
}

fun SequenceEntryStatus.assertIsRevocationIs(revoked: Boolean): SequenceEntryStatus {
if (revoked) {
assertThat(this.isRevocation, `is`(true))
} else {
assertThat(this.isRevocation, `is`(false))
}
return this
}

fun expectUnauthorizedResponse(isModifyingRequest: Boolean = false, apiCall: (jwt: String?) -> ResultActions) {
val response = apiCall(null)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package org.loculus.backend.controller.submission

import org.hamcrest.MatcherAssert.assertThat
import org.hamcrest.Matchers.containsString
import org.hamcrest.Matchers.hasItem
import org.hamcrest.Matchers.hasSize
import org.hamcrest.Matchers.`is`
import org.junit.jupiter.api.Test
Expand All @@ -22,6 +21,7 @@ import org.loculus.backend.controller.assertStatusIs
import org.loculus.backend.controller.expectUnauthorizedResponse
import org.loculus.backend.controller.generateJwtFor
import org.loculus.backend.controller.getAccessionVersions
import org.loculus.backend.controller.jsonContainsAccessionVersionsInAnyOrder
import org.loculus.backend.controller.jwtForSuperUser
import org.loculus.backend.controller.submission.SubmitFiles.DefaultFiles.NUMBER_OF_SEQUENCES
import org.springframework.beans.factory.annotation.Autowired
Expand Down Expand Up @@ -54,7 +54,7 @@ class ApproveProcessedDataEndpointTest(
client.approveProcessedSequenceEntries(scope = ALL, accessionVersions)
.andExpect(status().isOk)
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(jsonPath("\$[*].accession").value(accessionVersions.map { it.accession }))
.andExpect(jsonContainsAccessionVersionsInAnyOrder(accessionVersions))

assertThat(
convenienceClient.getSequenceEntries().statusCounts[APPROVED_FOR_RELEASE],
Expand All @@ -69,7 +69,7 @@ class ApproveProcessedDataEndpointTest(
client.approveProcessedSequenceEntries(scope = ALL, accessionVersions)
.andExpect(status().isOk)
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(jsonPath("\$[*].accession").value(accessionVersions.map { it.accession }))
.andExpect(jsonContainsAccessionVersionsInAnyOrder(accessionVersions))

assertThat(
convenienceClient.getSequenceEntries().statusCounts[APPROVED_FOR_RELEASE],
Expand All @@ -79,12 +79,12 @@ class ApproveProcessedDataEndpointTest(

@Test
fun `WHEN I approve without accession filter or with full scope THEN all data is approved`() {
val approvableSequences = convenienceClient.prepareDataTo(PROCESSED).map { it.accession }
val accessionVersions = convenienceClient.prepareDataTo(PROCESSED)

client.approveProcessedSequenceEntries(scope = ALL)
.andExpect(status().isOk)
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(jsonPath("\$[*].accession").value(approvableSequences))
.andExpect(jsonContainsAccessionVersionsInAnyOrder(accessionVersions))

assertThat(
convenienceClient.getSequenceEntriesOfUserInState(status = APPROVED_FOR_RELEASE),
Expand Down Expand Up @@ -117,31 +117,32 @@ class ApproveProcessedDataEndpointTest(
fun `WHEN I approve a sequence entry that does not exist THEN no accession should be approved`() {
val nonExistentAccession = "999"

val accessionVersions = convenienceClient.prepareDataTo(PROCESSED).getAccessionVersions()
val processedAccessionVersion = convenienceClient.prepareDataTo(PROCESSED).getAccessionVersions().first()

client.approveProcessedSequenceEntries(
scope = ALL,
listOf(
accessionVersions.first(),
processedAccessionVersion,
AccessionVersion(nonExistentAccession, 1),
),
)
.andExpect(status().isUnprocessableEntity)
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(jsonPath("$.detail", containsString("Accession versions 999.1 do not exist")))
.andExpect(jsonPath("$.detail", containsString("Accession versions $nonExistentAccession.1 do not exist")))

convenienceClient.getSequenceEntry(accessionVersions.first()).assertStatusIs(PROCESSED)
convenienceClient.getSequenceEntry(processedAccessionVersion).assertStatusIs(PROCESSED)
}

@Test
fun `WHEN I approve a sequence entry that does not exist THEN no sequence should be approved`() {
val accessionVersions = convenienceClient.prepareDataTo(PROCESSED).getAccessionVersions()
val nonExistingVersion = accessionVersions[1].copy(version = 999L)
val processedAccessionVersion = accessionVersions.first()

client.approveProcessedSequenceEntries(
scope = ALL,
listOf(
accessionVersions.first(),
processedAccessionVersion,
nonExistingVersion,
),
)
Expand All @@ -157,7 +158,7 @@ class ApproveProcessedDataEndpointTest(
),
)

convenienceClient.getSequenceEntry(accessionVersions.first()).assertStatusIs(PROCESSED).assertHasError(false)
convenienceClient.getSequenceEntry(processedAccessionVersion).assertStatusIs(PROCESSED).assertHasError(false)
}

@Test
Expand Down Expand Up @@ -240,8 +241,7 @@ class ApproveProcessedDataEndpointTest(
)
.andExpect(status().isOk)
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
.andExpect(jsonPath("$.[*]", hasSize<List<*>>(otherOrganismData.size)))
.andExpect(jsonPath("$.[*].accession", hasItem(otherOrganismData.first().accession)))
.andExpect(jsonContainsAccessionVersionsInAnyOrder(otherOrganismData))

convenienceClient.getSequenceEntry(
accession = defaultOrganismData.first().accession,
Expand Down Expand Up @@ -357,8 +357,7 @@ class ApproveProcessedDataEndpointTest(

client.approveProcessedSequenceEntries(scope = ALL, jwt = jwtForSuperUser)
.andExpect(status().isOk)
.andExpect(jsonPath("\$.length()").value(accessionVersions.size))
.andExpect(jsonPath("\$[*].accession").value(accessionVersions.map { it.accession }))
.andExpect(jsonContainsAccessionVersionsInAnyOrder(accessionVersions))

convenienceClient.getSequenceEntry(accessionVersions.first()).assertStatusIs(APPROVED_FOR_RELEASE)
}
Expand All @@ -379,8 +378,7 @@ class ApproveProcessedDataEndpointTest(
submitterNamesFilter = listOf(DEFAULT_USER_NAME),
)
.andExpect(status().isOk)
.andExpect(jsonPath("\$.length()").value(accessionVersions.size))
.andExpect(jsonPath("\$[*].accession").value(accessionVersions.map { it.accession }))
.andExpect(jsonContainsAccessionVersionsInAnyOrder(accessionVersions))

convenienceClient.getSequenceEntry(accessionVersions.first()).assertStatusIs(APPROVED_FOR_RELEASE)
convenienceClient.getSequenceEntry(
Expand All @@ -402,8 +400,7 @@ class ApproveProcessedDataEndpointTest(
jwt = jwtForSuperUser,
)
.andExpect(status().isOk)
.andExpect(jsonPath("\$.length()").value(accessionVersions.size))
.andExpect(jsonPath("\$[*].accession").value(accessionVersions.map { it.accession }))
.andExpect(jsonContainsAccessionVersionsInAnyOrder(accessionVersions))

convenienceClient.getSequenceEntry(accessionVersions.first()).assertStatusIs(APPROVED_FOR_RELEASE)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
package org.loculus.backend.controller.submission

import org.hamcrest.MatcherAssert.assertThat
import org.hamcrest.Matchers.allOf
import org.hamcrest.Matchers.containsInAnyOrder
import org.hamcrest.Matchers.containsString
import org.hamcrest.Matchers.equalTo
import org.hamcrest.Matchers.hasEntry
import org.hamcrest.Matchers.hasItem
import org.hamcrest.Matchers.hasProperty
import org.hamcrest.Matchers.hasSize
Expand All @@ -28,6 +25,7 @@ import org.loculus.backend.controller.OTHER_ORGANISM
import org.loculus.backend.controller.assertStatusIs
import org.loculus.backend.controller.expectUnauthorizedResponse
import org.loculus.backend.controller.generateJwtFor
import org.loculus.backend.controller.jsonContainsAccessionVersionsInAnyOrder
import org.loculus.backend.controller.jwtForSuperUser
import org.loculus.backend.controller.submission.SubmitFiles.DefaultFiles.NUMBER_OF_SEQUENCES
import org.loculus.backend.controller.toAccessionVersion
Expand Down Expand Up @@ -79,11 +77,7 @@ class DeleteSequencesEndpointTest(
deletionResult
.andExpect(status().isOk)
.andExpect(content().contentType(MediaType.APPLICATION_JSON_VALUE))
.andExpect(jsonPath("\$.length()").value(accessionVersionsToDelete.size))

accessionVersionsToDelete.forEach {
deletionResult.andExpect(jsonPath("\$[*].accession", hasItem(it.accession)))
}
.andExpect(jsonContainsAccessionVersionsInAnyOrder(accessionVersionsToDelete))

assertThat(
convenienceClient.getSequenceEntriesOfUserInState(
Expand Down Expand Up @@ -157,14 +151,16 @@ class DeleteSequencesEndpointTest(
val erroneousSequences = convenienceClient.prepareDataTo(Status.PROCESSED, errors = true)
val approvableSequences = convenienceClient.prepareDataTo(Status.PROCESSED)

val allSequences = erroneousSequences + approvableSequences

assertThat(
convenienceClient.getSequenceEntries().sequenceEntries,
hasSize(erroneousSequences.size + approvableSequences.size),
hasSize(allSequences.size),
)

client.deleteSequenceEntries(scope = ALL)
.andExpect(status().isOk)
.andExpect(jsonPath("\$.length()").value(2 * NUMBER_OF_SEQUENCES))
.andExpect(jsonContainsAccessionVersionsInAnyOrder(allSequences))

assertThat(
convenienceClient.getSequenceEntries().sequenceEntries,
Expand Down Expand Up @@ -193,7 +189,7 @@ class DeleteSequencesEndpointTest(

client.deleteSequenceEntries(scope = DeleteSequenceScope.PROCESSED_WITH_ERRORS)
.andExpect(status().isOk)
.andExpect(jsonPath("\$.length()").value(NUMBER_OF_SEQUENCES))
.andExpect(jsonContainsAccessionVersionsInAnyOrder(erroneousSequences))

assertThat(
convenienceClient.getProcessingResultCount(ProcessingResult.HAS_ERRORS),
Expand Down Expand Up @@ -242,8 +238,7 @@ class DeleteSequencesEndpointTest(
)
.andExpect(status().isOk)
.andExpect(content().contentType(MediaType.APPLICATION_JSON_VALUE))
.andExpect(jsonPath("$.[*]", hasSize<List<*>>(otherOrganismData.size)))
.andExpect(jsonPath("$.[*].accession", hasItem(otherOrganismData.first().accession)))
.andExpect(jsonContainsAccessionVersionsInAnyOrder(otherOrganismData))

convenienceClient.getSequenceEntry(
accession = defaultOrganismData.first().accession,
Expand Down Expand Up @@ -299,9 +294,7 @@ class DeleteSequencesEndpointTest(
client.deleteSequenceEntries(scope = ALL, jwt = jwtForSuperUser)
.andExpect(status().isOk)
.andExpect(content().contentType(MediaType.APPLICATION_JSON_VALUE))
.andExpect(jsonPath("\$.length()").value(accessionVersions.size))
.andExpect(jsonPath("\$[0].accession").value(accessionVersions.first().accession))
.andExpect(jsonPath("\$[0].version").value(accessionVersions.first().version))
.andExpect(jsonContainsAccessionVersionsInAnyOrder(accessionVersions))
}

@Test
Expand All @@ -312,22 +305,14 @@ class DeleteSequencesEndpointTest(
)
.submissionIdMappings

val expectedPairs = accessionVersions.map { entry ->
allOf(
hasEntry("accession", entry.accession),
hasEntry("version", entry.version.toInt()),
)
}

client.deleteSequenceEntries(
scope = ALL,
accessionVersionsFilter = accessionVersions,
jwt = jwtForSuperUser,
)
.andExpect(status().isOk)
.andExpect(content().contentType(MediaType.APPLICATION_JSON_VALUE))
.andExpect(jsonPath("\$.length()").value(accessionVersions.size))
.andExpect(jsonPath("\$[*]", containsInAnyOrder(expectedPairs)))
.andExpect(jsonContainsAccessionVersionsInAnyOrder(accessionVersions))
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ import org.loculus.backend.controller.DEFAULT_ORGANISM
import org.loculus.backend.controller.DEFAULT_USER_NAME
import org.loculus.backend.controller.EndpointTest
import org.loculus.backend.controller.OTHER_ORGANISM
import org.loculus.backend.controller.SUPER_USER_NAME
import org.loculus.backend.controller.assertGroupIdIs
import org.loculus.backend.controller.assertHasError
import org.loculus.backend.controller.assertIsRevocationIs
import org.loculus.backend.controller.assertStatusIs
import org.loculus.backend.controller.assertSubmitterIs
import org.loculus.backend.controller.expectUnauthorizedResponse
import org.loculus.backend.controller.generateJwtFor
import org.loculus.backend.controller.jwtForSuperUser
Expand Down Expand Up @@ -36,7 +41,7 @@ class RevokeEndpointTest(
}

@Test
fun `GIVEN entries with 'APPROVED_FOR_RELEASE' THEN returns revocation version in status AWAITING_APPROVAL`() {
fun `GIVEN entries with 'APPROVED_FOR_RELEASE' THEN returns revocation version in status PROCESSED`() {
val accessions = convenienceClient.prepareDefaultSequenceEntriesToApprovedForRelease().map { it.accession }

client.revokeSequenceEntries(accessions)
Expand All @@ -46,8 +51,14 @@ class RevokeEndpointTest(
.andExpect(jsonPath("\$[0].accession").value(accessions.first()))
.andExpect(jsonPath("\$[0].version").value(2))

val originalEntry = convenienceClient.getSequenceEntry(accession = accessions.first(), version = 1)

convenienceClient.getSequenceEntry(accession = accessions.first(), version = 2)
.assertStatusIs(PROCESSED)
.assertHasError(false)
.assertGroupIdIs(originalEntry.groupId)
.assertIsRevocationIs(true)
.assertSubmitterIs(DEFAULT_USER_NAME)
}

@Test
Expand Down Expand Up @@ -94,7 +105,7 @@ class RevokeEndpointTest(
}

@Test
fun `WHEN superuser revokes entries of other group THEN revocation version is created`() {
fun `WHEN superuser revokes entries of other group THEN superuser is submitter of revocation entry`() {
val accessions = convenienceClient
.prepareDefaultSequenceEntriesToApprovedForRelease(username = DEFAULT_USER_NAME)
.map { it.accession }
Expand All @@ -106,8 +117,14 @@ class RevokeEndpointTest(
.andExpect(jsonPath("\$[0].accession").value(accessions.first()))
.andExpect(jsonPath("\$[0].version").value(2))

val originalEntry = convenienceClient.getSequenceEntry(accession = accessions.first(), version = 1)

convenienceClient.getSequenceEntry(accession = accessions.first(), version = 2)
.assertStatusIs(PROCESSED)
.assertHasError(false)
.assertGroupIdIs(originalEntry.groupId)
.assertIsRevocationIs(true)
.assertSubmitterIs(SUPER_USER_NAME)
}

@Test
Expand Down
6 changes: 6 additions & 0 deletions docs/src/content/docs/reference/helm-chart-config.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,12 @@ The configuration for the Helm chart is provided as a YAML file. It has the foll
<td>true</td>
<td>If true, runs a development Keycloak database within the cluster.</td>
</tr>
<tr>
<td>`developmentDatabasePersistence`</td>
<td>Boolean</td>
<td>true</td>
<td>If true, makes the database on the argocd preview persistent.</td>
</tr>
</tbody>
</table>

Expand Down
Loading

0 comments on commit dc40b08

Please sign in to comment.