Skip to content

Commit

Permalink
fix: write empty string to CSV/TSV for null values #708
Browse files Browse the repository at this point in the history
  • Loading branch information
fengelniederhammer committed Apr 2, 2024
1 parent 0cd36a9 commit 1d0beb4
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class CsvWriter {
it.printRecord(*datum.asArray())
}
}
return stringWriter.toString().trim()
return stringWriter.toString().trimEnd('\n')
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import com.fasterxml.jackson.databind.JsonDeserializer
import com.fasterxml.jackson.databind.JsonNode
import com.fasterxml.jackson.databind.JsonSerializer
import com.fasterxml.jackson.databind.SerializerProvider
import com.fasterxml.jackson.databind.node.NullNode
import io.swagger.v3.oas.annotations.media.Schema
import org.genspectrum.lapis.config.DatabaseConfig
import org.genspectrum.lapis.controller.CsvRecord
Expand All @@ -19,17 +20,25 @@ data class AggregationData(
val count: Int,
@Schema(hidden = true) val fields: Map<String, JsonNode>,
) : CsvRecord {
override fun asArray() = fields.values.map { it.asText() }.plus(count.toString()).toTypedArray()
override fun asArray() = fields.values
.map { it.toCsvValue() }
.plus(count.toString())
.toTypedArray()

override fun getHeader() = fields.keys.plus(COUNT_PROPERTY).toTypedArray()
}

data class DetailsData(val map: Map<String, JsonNode>) : Map<String, JsonNode> by map, CsvRecord {
override fun asArray() = values.map { it.asText() }.toTypedArray()
override fun asArray() = values.map { it.toCsvValue() }.toTypedArray()

override fun getHeader() = keys.toTypedArray()
}

private fun JsonNode.toCsvValue() = when (this) {
is NullNode -> ""
else -> asText()
}

@JsonComponent
class DetailsDataDeserializer : JsonDeserializer<DetailsData>() {
override fun deserialize(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
package org.genspectrum.lapis.controller

import com.fasterxml.jackson.databind.node.NullNode
import com.fasterxml.jackson.databind.node.TextNode
import com.ninjasquad.springmockk.MockkBean
import io.mockk.every
import org.genspectrum.lapis.controller.LapisMediaType.TEXT_CSV
import org.genspectrum.lapis.controller.LapisMediaType.TEXT_CSV_WITHOUT_HEADERS
import org.genspectrum.lapis.controller.LapisMediaType.TEXT_TSV
import org.genspectrum.lapis.model.SiloQueryModel
import org.genspectrum.lapis.request.LapisInfo
import org.genspectrum.lapis.response.AggregationData
import org.genspectrum.lapis.response.DetailsData
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 Down Expand Up @@ -133,6 +138,50 @@ class LapisControllerCsvTest(
.drop(1)
.joinToString("\n")

@Test
fun `GIVEN aggregated endpoint returns result with null values THEN CSV contains empty strings instead`() {
every { siloQueryModelMock.getAggregated(any()) } returns listOf(
AggregationData(
1,
mapOf("firstKey" to TextNode("someValue"), "keyWithNullValue" to NullNode.instance),
),
)

val expectedCsv = """
firstKey,keyWithNullValue,count
someValue,,1
""".trimIndent()

mockMvc.perform(getSample("/aggregated?country=Switzerland").header(ACCEPT, "text/csv"))
.andExpect(status().isOk)
.andExpect(header().string("Content-Type", "text/csv;charset=UTF-8"))
.andExpect(content().string(expectedCsv))
}

@Test
fun `GIVEN details endpoint returns result with null values THEN CSV contains empty strings instead`() {
every { siloQueryModelMock.getDetails(any()) } returns listOf(
DetailsData(
mapOf(
"firstKey" to TextNode("some first value"),
"keyWithNullValue" to NullNode.instance,
"someOtherKey" to TextNode("someValue"),
),
),
)

val expectedCsv = """
firstKey,keyWithNullValue,someOtherKey
some first value,,someValue
""".trimIndent()

mockMvc.perform(getSample("/details?country=Switzerland").header(ACCEPT, "text/csv"))
.andExpect(status().isOk)
.andExpect(header().string("Content-Type", "text/csv;charset=UTF-8"))
.andExpect(content().string(expectedCsv))
}


private companion object {
@JvmStatic
val endpoints = SampleRoute.entries.filter { !it.servesFasta }.map { it.pathSegment }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,12 +191,12 @@ object MockDataForEndpoints {
expectedCsv = """
country,age,floatValue
Switzerland,42,3.14
Switzerland,43,null
Switzerland,43,
""".trimIndent(),
expectedTsv = """
country age floatValue
Switzerland 42 3.14
Switzerland 43 null
Switzerland 43
""".trimIndent(),
)

Expand Down

0 comments on commit 1d0beb4

Please sign in to comment.