fix: write empty string to CSV/TSV for null values #708

GenSpectrum · Apr 2, 2024 · c8872ba · c8872ba
1 parent 0cd36a9
commit c8872ba
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 18 deletions.
diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/CsvWriter.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/CsvWriter.kt
@@ -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>
 }
 
@@ -24,6 +27,7 @@ class CsvWriter {
             CSVFormat.DEFAULT.builder()
                 .setRecordSeparator("\n")
                 .setDelimiter(delimiter.value)
+                .setNullString("")
                 .also {
                     when {
                         headers != null -> it.setHeader(*headers)
@@ -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')
     }
 }
 

diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/response/LapisResponse.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/response/LapisResponse.kt
@@ -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
 
@@ -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")
 }
 
@@ -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")
 }
 
@@ -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")
 }
 
@@ -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")
 }
diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/response/SiloResponse.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/response/SiloResponse.kt
@@ -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
@@ -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(

diff --git a/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/LapisControllerCsvTest.kt b/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/LapisControllerCsvTest.kt
@@ -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
@@ -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 }

diff --git a/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/MockData.kt b/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/MockData.kt
@@ -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(),
     )
 

diff --git a/siloLapisTests/test/aggregated.spec.ts b/siloLapisTests/test/aggregated.spec.ts
@@ -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
@@ -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