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

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

Hide file tree

Showing 5 changed files with 65 additions and 7 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
@@ -35,7 +35,7 @@ class CsvWriter {
                 it.printRecord(*datum.asArray())
             }
         }
-        return stringWriter.toString().trim()
+        return stringWriter.toString().trimEnd('\n')
     }
 }
 

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,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(

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,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 }

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