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 bd37ddd
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 18 deletions.
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
package org.genspectrum.lapis.controller

import com.fasterxml.jackson.annotation.JsonIgnore
import org.apache.commons.csv.CSVFormat
import org.apache.commons.csv.CSVPrinter
import org.springframework.stereotype.Component
import java.io.StringWriter

interface CsvRecord {
fun asArray(): Array<String>
@JsonIgnore
fun getValuesList(): List<String?>

@JsonIgnore
fun getHeader(): Array<String>
}

Expand All @@ -24,6 +27,7 @@ class CsvWriter {
CSVFormat.DEFAULT.builder()
.setRecordSeparator("\n")
.setDelimiter(delimiter.value)
.setNullString("")
.also {
when {
headers != null -> it.setHeader(*headers)
Expand All @@ -32,10 +36,10 @@ class CsvWriter {
.build(),
).use {
for (datum in data) {
it.printRecord(*datum.asArray())
it.printRecord(datum.getValuesList())
}
}
return stringWriter.toString().trim()
return stringWriter.toString().trimEnd('\n')
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.genspectrum.lapis.response

import com.fasterxml.jackson.annotation.JsonIgnore
import io.swagger.v3.oas.annotations.media.Schema
import org.genspectrum.lapis.controller.CsvRecord

Expand All @@ -23,9 +22,8 @@ data class NucleotideMutationResponse(
"given filter criteria with non-ambiguous reads at that position",
) val proportion: Double,
) : CsvRecord {
override fun asArray() = arrayOf(mutation, count.toString(), proportion.toString())
override fun getValuesList() = listOf(mutation, count.toString(), proportion.toString())

@JsonIgnore
override fun getHeader() = arrayOf("mutation", "count", "proportion")
}

Expand All @@ -45,9 +43,8 @@ data class AminoAcidMutationResponse(
"given filter criteria with non-ambiguous reads at that position",
) val proportion: Double,
) : CsvRecord {
override fun asArray() = arrayOf(mutation, count.toString(), proportion.toString())
override fun getValuesList() = listOf(mutation, count.toString(), proportion.toString())

@JsonIgnore
override fun getHeader() = arrayOf("mutation", "count", "proportion")
}

Expand All @@ -65,9 +62,8 @@ data class NucleotideInsertionResponse(
)
val count: Int,
) : CsvRecord {
override fun asArray() = arrayOf(insertion, count.toString())
override fun getValuesList() = listOf(insertion, count.toString())

@JsonIgnore
override fun getHeader() = arrayOf("insertion", "count")
}

Expand All @@ -84,8 +80,7 @@ data class AminoAcidInsertionResponse(
)
val count: Int,
) : CsvRecord {
override fun asArray() = arrayOf(insertion, count.toString())
override fun getValuesList() = listOf(insertion, count.toString())

@JsonIgnore
override fun getHeader() = arrayOf("insertion", "count")
}
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,26 @@ 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 getValuesList() =
fields.values
.map { it.toCsvValue() }
.plus(count.toString())

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 getValuesList() = values.map { it.toCsvValue() }

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

private fun JsonNode.toCsvValue() =
when (this) {
is NullNode -> null
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,49 @@ 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
4 changes: 2 additions & 2 deletions siloLapisTests/test/aggregated.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ describe('The /aggregated endpoint', () => {
expect(await result.text()).to.be.equal(
String.raw`
age,country,count
null,Switzerland,2
,Switzerland,2
4,Switzerland,2
5,Switzerland,1
6,Switzerland,1
Expand Down Expand Up @@ -183,7 +183,7 @@ null,Switzerland,2
expect(await result.text()).to.be.equal(
String.raw`
age country count
null Switzerland 2
Switzerland 2
4 Switzerland 2
5 Switzerland 1
6 Switzerland 1
Expand Down

0 comments on commit bd37ddd

Please sign in to comment.