Skip to content

Commit

Permalink
fix(backend): address PR
Browse files Browse the repository at this point in the history
  • Loading branch information
TobiasKampmann committed Feb 6, 2024
1 parent cabcadb commit e22d69a
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ data class SequenceEntryStatus(
override val accession: Accession,
override val version: Version,
val status: Status,
val group: Group,
val group: String,
val isRevocation: Boolean = false,
) : AccessionVersionInterface

Expand Down Expand Up @@ -202,10 +202,6 @@ enum class Status {
return stringToEnumMap[statusString]
?: throw IllegalArgumentException("Unknown status: $statusString")
}

fun getListOfStatuses(): List<Status> {
return stringToEnumMap.values.toList()
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ The accession version must be in status 'HAS_ERRORS' or 'AWAITING_APPROVAL'.
"""

const val GET_SEQUENCES_DESCRIPTION = """
Get a list of submitted accession versions and their status.
There are two optional parameter that filter for groups and statuses.
When no constraints are given the endpoint returns all sequences of all groups the user is member of.
If a group is filtered for that the user is not a member of the endpoint will return an error.
Retrieve a list of submitted accession versions along with their statuses.
There are two optional parameters that filter by groups and statuses.
When no constraints are applied, the endpoint returns all sequences from all groups the user is a member of.
If a filter is applied for a group the user is not a member of, the endpoint will return an error.
"""

const val APPROVE_PROCESSED_DATA_DESCRIPTION = """
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,19 +361,18 @@ class SubmissionDatabaseService(
groupsFilter: List<String>?,
statusesFilter: List<Status>?,
): List<SequenceEntryStatus> {
log.info { "getting sequence for user $username (groupFilter: $groupsFilter in statuses $statusesFilter" }

val validatedGroupNames = if (groupsFilter === null) {
groupManagementDatabaseService.getGroupsOfUser(username)
} else {
groupManagementPreconditionValidator.validateUserInExistingGroups(groupsFilter, username)
groupsFilter.map { Group(it) }
}

val listOfStatuses = statusesFilter ?: Status.getListOfStatuses()

log.info { "getting sequence for user $username from groups $validatedGroupNames in statuses $listOfStatuses" }
val listOfStatuses = statusesFilter ?: Status.entries

sequenceEntriesTableProvider.get(organism).let { table ->

return table
.slice(
table.accessionColumn,
Expand All @@ -394,7 +393,7 @@ class SubmissionDatabaseService(
row[table.accessionColumn],
row[table.versionColumn],
Status.fromString(row[table.statusColumn]),
Group(row[table.groupNameColumn]),
row[table.groupNameColumn],
row[table.isRevocationColumn],
)
}
Expand Down Expand Up @@ -463,7 +462,7 @@ class SubmissionDatabaseService(
it[table.accessionColumn],
it[table.versionColumn],
AWAITING_APPROVAL_FOR_REVOCATION,
Group(it[table.groupNameColumn]),
it[table.groupNameColumn],
it[table.isRevocationColumn],
)
}.sortedBy { it.accession }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,16 @@ import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource
import org.loculus.backend.api.AccessionVersion
import org.loculus.backend.api.Status
import org.loculus.backend.api.Status.AWAITING_APPROVAL
import org.loculus.backend.api.Status.IN_PROCESSING
import org.loculus.backend.controller.ALTERNATIVE_DEFAULT_GROUP_NAME
import org.loculus.backend.controller.ALTERNATIVE_DEFAULT_USER_NAME
import org.loculus.backend.controller.DEFAULT_GROUP_NAME
import org.loculus.backend.controller.DEFAULT_ORGANISM
import org.loculus.backend.controller.EndpointTest
import org.loculus.backend.controller.OTHER_ORGANISM
import org.loculus.backend.controller.expectUnauthorizedResponse
import org.loculus.backend.controller.generateJwtFor
import org.loculus.backend.controller.getAccessionVersions
import org.loculus.backend.controller.submission.SubmitFiles.DefaultFiles.NUMBER_OF_SEQUENCES
import org.loculus.backend.controller.submission.SubmitFiles.DefaultFiles.firstAccession
Expand All @@ -39,29 +42,29 @@ class GetSequencesEndpointTest(
@Test
fun `GIVEN data submitted from a group member THEN another group member sees the data`() {
convenienceClient.submitDefaultFiles(DEFAULT_USER_NAME)

val sequencesOfUser = convenienceClient.getSequenceEntries(
username = DEFAULT_USER_NAME,
groupsFilter = listOf(DEFAULT_GROUP_NAME),
statusesFilter = listOf(Status.RECEIVED),
organism = DEFAULT_ORGANISM,
)

assertThat(sequencesOfUser, hasSize(NUMBER_OF_SEQUENCES))

val sequencesOfAlternativeUser = convenienceClient.getSequenceEntries(
username = ALTERNATIVE_DEFAULT_USER_NAME,
groupsFilter = listOf(DEFAULT_GROUP_NAME),
statusesFilter = listOf(Status.RECEIVED),
organism = DEFAULT_ORGANISM,
)

assertThat(sequencesOfAlternativeUser, hasSize(NUMBER_OF_SEQUENCES))
}

@Test
fun `GIVEN data submitted to a group WHEN querying another group THEN only shows entries of the given group`() {
convenienceClient.submitDefaultFiles(DEFAULT_USER_NAME, DEFAULT_GROUP_NAME)

val sequencesOfUser = convenienceClient.getSequenceEntries(
username = DEFAULT_USER_NAME,
groupsFilter = listOf(ALTERNATIVE_DEFAULT_GROUP_NAME),
)

assertThat(sequencesOfUser, hasSize(0))
}

Expand All @@ -87,6 +90,7 @@ class GetSequencesEndpointTest(
username = DEFAULT_USER_NAME,
organism = OTHER_ORGANISM,
)

assertThat(
sequencesOfUser.getAccessionVersions(),
containsInAnyOrder(*otherOrganismData.getAccessionVersions().toTypedArray()),
Expand All @@ -97,6 +101,41 @@ class GetSequencesEndpointTest(
)
}

@Test
fun `WHEN querying sequences of an existing group that you're not a member of THEN this is forbidden`() {
client.getSequenceEntries(
groupsFilter = listOf(ALTERNATIVE_DEFAULT_GROUP_NAME),
jwt = generateJwtFor(ALTERNATIVE_DEFAULT_USER_NAME),
).andExpect(status().isForbidden).andExpect {
jsonPath(
"$.detail",
containsString(
"User $ALTERNATIVE_DEFAULT_USER_NAME is not " +
"a member of groups $ALTERNATIVE_DEFAULT_GROUP_NAME.",
),
)
}
}

@Test
fun `GIVEN data in many statuses WHEN querying sequences for a certain one THEN return only those sequences`() {
convenienceClient.prepareDataTo(AWAITING_APPROVAL)

val sequencesInAwaitingApproval = convenienceClient.getSequenceEntries(
username = ALTERNATIVE_DEFAULT_USER_NAME,
statusesFilter = listOf(AWAITING_APPROVAL),
)

assertThat(sequencesInAwaitingApproval, hasSize(10))

val sequencesInProcessing = convenienceClient.getSequenceEntries(
username = ALTERNATIVE_DEFAULT_USER_NAME,
statusesFilter = listOf(IN_PROCESSING),
)

assertThat(sequencesInProcessing, hasSize(0))
}

@ParameterizedTest(name = "{arguments}")
@MethodSource("provideStatusScenarios")
fun `GIVEN database in prepared state THEN returns sequence entries in expected status`(scenario: Scenario) {
Expand All @@ -122,7 +161,7 @@ class GetSequencesEndpointTest(
Scenario(
setupDescription = "I started processing sequence entries",
prepareDatabase = { it.prepareDefaultSequenceEntriesToInProcessing() },
expectedStatus = Status.IN_PROCESSING,
expectedStatus = IN_PROCESSING,
expectedIsRevocation = false,
),
Scenario(
Expand All @@ -138,7 +177,7 @@ class GetSequencesEndpointTest(
PreparedProcessedData.successfullyProcessed(),
)
},
expectedStatus = Status.AWAITING_APPROVAL,
expectedStatus = AWAITING_APPROVAL,
expectedIsRevocation = false,
),
Scenario(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package org.loculus.backend.controller.submission

import com.fasterxml.jackson.databind.ObjectMapper
import mu.KotlinLogging
import org.loculus.backend.api.AccessionVersion
import org.loculus.backend.api.DataUseTerms
import org.loculus.backend.api.Status
Expand All @@ -25,7 +24,6 @@ import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.multi
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post

const val DEFAULT_USER_NAME = "testuser"
private val logger = KotlinLogging.logger {}
class SubmissionControllerClient(private val mockMvc: MockMvc, private val objectMapper: ObjectMapper) {
fun submit(
metadataFile: MockMultipartFile,
Expand Down Expand Up @@ -83,18 +81,15 @@ class SubmissionControllerClient(private val mockMvc: MockMvc, private val objec

fun getSequenceEntries(
organism: String = DEFAULT_ORGANISM,
groupsFilter: List<String> = listOf(DEFAULT_GROUP_NAME),
statusesFilter: List<Status> = Status.getListOfStatuses(),
groupsFilter: List<String>? = null,
statusesFilter: List<Status>? = null,
jwt: String? = jwtForDefaultUser,
): ResultActions {
logger.info {
"Getting sequences for organism $organism, groupNames $groupsFilter, statuses $statusesFilter"
}
return mockMvc.perform(
get(addOrganismToPath("/get-sequences", organism = organism))
.withAuth(jwt)
.param("groupsFilter", groupsFilter.joinToString { it })
.param("statusesFilter", statusesFilter.joinToString { it.name }),
.param("groupsFilter", groupsFilter?.joinToString { it })
.param("statusesFilter", statusesFilter?.joinToString { it.name }),
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ class SubmissionConvenienceClient(

fun getSequenceEntries(
username: String = DEFAULT_USER_NAME,
groupsFilter: List<String> = listOf(DEFAULT_GROUP_NAME),
statusesFilter: List<Status> = Status.getListOfStatuses(),
groupsFilter: List<String>? = null,
statusesFilter: List<Status>? = null,
organism: String = DEFAULT_ORGANISM,
): List<SequenceEntryStatus> = deserializeJsonResponse(
client.getSequenceEntries(
Expand Down

0 comments on commit e22d69a

Please sign in to comment.