diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/CsvWriter.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/controller/CsvWriter.kt index 0817e5c8..95eb4105 100644 --- 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 + @JsonIgnore + fun getValuesList(): List + @JsonIgnore fun getHeader(): Array } @@ -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 index 5f615976..9cf516b0 100644 --- 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 index 9e7b9a9f..3eaa449e 100644 --- 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, ) : 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) : Map 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() { 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 index 2217ec0b..a99426e6 100644 --- a/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/LapisControllerCsvTest.kt +++ b/lapis2/src/test/kotlin/org/genspectrum/lapis/controller/LapisControllerCsvTest.kt @@ -1,5 +1,7 @@ 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 @@ -7,7 +9,10 @@ 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 index 7f6b917d..f729845a 100644 --- 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 index 5630a775..af2876b2 100644 --- 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