Skip to content

fix(lapis): reject unknown data formats #1093

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 3, 2025
Merged

fix(lapis): reject unknown data formats #1093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d7c529f
refactor(lapis): rename test
fengelniederhammer Feb 26, 2025
e0e99f0
fix(lapis): reject unknown data formats
fengelniederhammer Feb 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .../src/main/kotlin/org/genspectrum/lapis/controller/middleware/DataFormatParameterFilter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,14 @@ class DataFormatParameterFilter(val objectMapper: ObjectMapper) : OncePerRequest

private fun findAcceptHeaderOverwriteValue(reReadableRequest: CachedBodyHttpServletRequest) =
when (reReadableRequest.getStringField(FORMAT_PROPERTY)?.uppercase()) {
null -> null
DataFormat.CSV -> LapisMediaType.TEXT_CSV_VALUE
DataFormat.CSV_WITHOUT_HEADERS -> LapisMediaType.TEXT_CSV_WITHOUT_HEADERS_VALUE
DataFormat.TSV -> LapisMediaType.TEXT_TSV_VALUE
DataFormat.JSON -> MediaType.APPLICATION_JSON_VALUE
SequencesDataFormat.FASTA -> LapisMediaType.TEXT_X_FASTA_VALUE
SequencesDataFormat.NDJSON -> MediaType.APPLICATION_NDJSON_VALUE

else -> null
else -> "unknown/unknown"
}
}
12 changes: 8 additions & 4 deletions lapis/src/test/kotlin/org/genspectrum/lapis/controller/LapisControllerCompressionTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ fun getRequests(
callDescription = "GET $endpoint as $dataFormat with request parameter",
mockData = MockDataForEndpoints.getMockData(endpoint).expecting(dataFormat),
request = getSample(
"$endpoint?country=Switzerland&dataFormat=$dataFormat&compression=$compressionFormat",
"$endpoint?country=Switzerland&dataFormat=${dataFormat.fileFormat}&compression=$compressionFormat",
),
compressionFormat = compressionFormat,
expectedContentType = getContentTypeForCompressionFormat(compressionFormat),
Expand All @@ -278,7 +278,7 @@ fun getRequests(
RequestScenario(
callDescription = "GET $endpoint as $dataFormat with accept header",
mockData = MockDataForEndpoints.getMockData(endpoint).expecting(dataFormat),
request = getSample("$endpoint?country=Switzerland&dataFormat=$dataFormat")
request = getSample("$endpoint?country=Switzerland&dataFormat=${dataFormat.fileFormat}")
.header(ACCEPT_ENCODING, compressionFormat),
compressionFormat = compressionFormat,
expectedContentType = getContentTypeForDataFormat(dataFormat),
Expand All @@ -289,7 +289,11 @@ fun getRequests(
mockData = MockDataForEndpoints.getMockData(endpoint).expecting(dataFormat),
request = postSample(endpoint)
.content(
"""{"country": "Switzerland", "dataFormat": "$dataFormat", "compression": "$compressionFormat"}""",
"""{
"country": "Switzerland",
"dataFormat": "${dataFormat.fileFormat}",
"compression": "$compressionFormat"}
""".trimMargin(),
)
.contentType(APPLICATION_JSON),
compressionFormat = compressionFormat,
Expand All @@ -300,7 +304,7 @@ fun getRequests(
callDescription = "POST JSON $endpoint as $dataFormat with accept header",
mockData = MockDataForEndpoints.getMockData(endpoint).expecting(dataFormat),
request = postSample(endpoint)
.content("""{"country": "Switzerland", "dataFormat": "$dataFormat"}""")
.content("""{"country": "Switzerland", "dataFormat": "${dataFormat.fileFormat}"}""")
.contentType(APPLICATION_JSON)
.header(ACCEPT_ENCODING, compressionFormat),
compressionFormat = compressionFormat,
Expand Down
31 changes: 30 additions & 1 deletion ...apis/controller/LapisControllerCsvTest.kt → ...ntroller/LapisControllerDataFormatTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ private const val DATA_VERSION = "1234"

@SpringBootTest
@AutoConfigureMockMvc
class LapisControllerCsvTest(
class LapisControllerDataFormatTest(
@Autowired private val mockMvc: MockMvc,
@Autowired private val objectMapper: ObjectMapper,
) {
Expand All @@ -49,6 +49,31 @@ class LapisControllerCsvTest(
every { lapisInfo.dataVersion } returns DATA_VERSION
}

@ParameterizedTest(name = "{0} returns bad request")
@MethodSource("getRequestsWithInvalidDataFormat")
fun `GIVEN unknown data format THEN returns not acceptable`(requestsScenario: RequestScenario) {
mockMvc.perform(requestsScenario.request)
.andExpect(status().isNotAcceptable)
.andExpect(header().string("Content-Type", "application/problem+json"))
.andExpect(jsonPath("\$.detail", startsWith("Acceptable representations:")))
}

@Test
fun `GIVEN aggregated request with dataFormat fasta THEN returns not acceptable`() {
mockMvc.perform(getSample("/${SampleRoute.AGGREGATED.pathSegment}?dataFormat=fasta"))
.andExpect(status().isNotAcceptable)
.andExpect(header().string("Content-Type", "application/problem+json"))
.andExpect(jsonPath("\$.detail", startsWith("Acceptable representations:")))
}

@Test
fun `GIVEN amino acid sequences request with dataFormat csv THEN returns not acceptable`() {
mockMvc.perform(getSample("/${SampleRoute.ALIGNED_AMINO_ACID_SEQUENCES.pathSegment}/S?dataFormat=csv"))
.andExpect(status().isNotAcceptable)
.andExpect(header().string("Content-Type", "application/problem+json"))
.andExpect(jsonPath("\$.detail", startsWith("Acceptable representations:")))
}

@ParameterizedTest(name = "{0} returns empty JSON")
@MethodSource("getJsonRequests")
fun `returns empty json`(requestsScenario: RequestScenario) {
Expand Down Expand Up @@ -260,6 +285,7 @@ class LapisControllerCsvTest(
"csv-without-headers" -> TEXT_CSV_WITHOUT_HEADERS_VALUE
"tsv" -> TEXT_TSV_VALUE
"json" -> MediaType.APPLICATION_JSON_VALUE
"invalid" -> "invalid/invalid"
else -> throw IllegalArgumentException("Unknown data format: $dataFormat")
}

Expand All @@ -275,6 +301,9 @@ class LapisControllerCsvTest(
@JvmStatic
fun getJsonRequests() = getRequests("json")

@JvmStatic
fun getRequestsWithInvalidDataFormat() = getRequests("invalid")

@JvmStatic
fun getColumnOrderRequests() =
listOf(
Expand Down
13 changes: 13 additions & 0 deletions .../src/test/kotlin/org/genspectrum/lapis/controller/MultiSegmentedSequenceControllerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import org.genspectrum.lapis.model.SiloQueryModel
import org.genspectrum.lapis.response.SequenceData
import org.genspectrum.lapis.silo.DataVersion
import org.genspectrum.lapis.silo.SequenceType
import org.hamcrest.Matchers.startsWith
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.Arguments
import org.junit.jupiter.params.provider.MethodSource
Expand All @@ -20,6 +22,7 @@ import org.springframework.test.web.servlet.MockMvc
import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.header
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
import java.util.stream.Stream

Expand Down Expand Up @@ -57,6 +60,16 @@ class MultiSegmentedSequenceControllerTest(
} returns "1234"
}

@Test
fun `GIVEN nucleotide sequences request with dataFormat csv THEN returns not acceptable`() {
mockMvc
.perform(
getSample("/${SampleRoute.ALIGNED_NUCLEOTIDE_SEQUENCES.pathSegment}/$SEGMENT_NAME?dataFormat=csv"),
).andExpect(status().isNotAcceptable)
.andExpect(header().string("Content-Type", "application/problem+json"))
.andExpect(jsonPath("\$.detail", startsWith("Acceptable representations:")))
}

@ParameterizedTest(name = "should {0} alignedNucleotideSequences with empty filter")
@MethodSource("getRequestsWithoutFilter")
fun `should call alignedNucleotideSequences with empty filter`(
Expand Down
12 changes: 12 additions & 0 deletions ...src/test/kotlin/org/genspectrum/lapis/controller/SingleSegmentedSequenceControllerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import org.genspectrum.lapis.model.SiloQueryModel
import org.genspectrum.lapis.response.SequenceData
import org.genspectrum.lapis.silo.DataVersion
import org.genspectrum.lapis.silo.SequenceType
import org.hamcrest.Matchers.startsWith
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource
import org.springframework.beans.factory.annotation.Autowired
Expand All @@ -18,6 +20,7 @@ import org.springframework.test.web.servlet.MockMvc
import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.header
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath
import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status
import java.util.stream.Stream

Expand Down Expand Up @@ -55,6 +58,15 @@ class SingleSegmentedSequenceControllerTest(
} returns "1234"
}

@Test
fun `GIVEN nucleotide sequences request with dataFormat csv THEN returns not acceptable`() {
mockMvc
.perform(getSample("/${SampleRoute.ALIGNED_NUCLEOTIDE_SEQUENCES.pathSegment}?dataFormat=csv"))
.andExpect(status().isNotAcceptable)
.andExpect(header().string("Content-Type", "application/problem+json"))
.andExpect(jsonPath("\$.detail", startsWith("Acceptable representations:")))
}

@ParameterizedTest(name = "should {0} alignedNucleotideSequences with empty filter")
@MethodSource("org.genspectrum.lapis.controller.MultiSegmentedSequenceControllerTest#getRequestsWithoutFilter")
fun `should call alignedNucleotideSequences with empty filter`(
Expand Down