diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/model/SiloFilterExpressionMapper.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/model/SiloFilterExpressionMapper.kt index 930f3dcd..de14008f 100644 --- a/lapis2/src/main/kotlin/org/genspectrum/lapis/model/SiloFilterExpressionMapper.kt +++ b/lapis2/src/main/kotlin/org/genspectrum/lapis/model/SiloFilterExpressionMapper.kt @@ -34,7 +34,7 @@ import java.time.LocalDate import java.time.format.DateTimeParseException import java.util.Locale -data class SequenceFilterValue(val type: SequenceFilterFieldType, val values: List, val originalKey: String) +data class SequenceFilterValue(val type: SequenceFilterFieldType, val values: List, val originalKey: String) typealias SequenceFilterFieldName = String @@ -177,6 +177,9 @@ class SiloFilterExpressionMapper( values: List, ) = Or( values[0].values.map { + if (it.isNullOrBlank()) { + return@map BooleanEquals(siloColumnName, null) + } val value = try { it.lowercase().toBooleanStrict() } catch (e: IllegalArgumentException) { @@ -186,8 +189,8 @@ class SiloFilterExpressionMapper( }, ) - private fun mapToVariantQueryFilter(variantQuery: String): SiloFilterExpression { - if (variantQuery.isBlank()) { + private fun mapToVariantQueryFilter(variantQuery: String?): SiloFilterExpression { + if (variantQuery.isNullOrBlank()) { throw BadRequestException("variantQuery must not be empty") } @@ -249,6 +252,7 @@ class SiloFilterExpressionMapper( ) = Or( values[0].values.map { when { + it.isNullOrBlank() -> throw BadRequestException("Invalid pango lineage: $it must not be empty") it.endsWith(".*") -> PangoLineageEquals(column, it.substringBeforeLast(".*"), includeSublineages = true) it.endsWith('*') -> PangoLineageEquals(column, it.substringBeforeLast('*'), includeSublineages = true) it.endsWith('.') -> throw BadRequestException( @@ -265,6 +269,9 @@ class SiloFilterExpressionMapper( values: List, ): SiloFilterExpression { val value = extractSingleFilterValue(values[0]) + if (value.isNullOrBlank()) { + return IntEquals(siloColumnName, null) + } try { return IntEquals(siloColumnName, value.toInt()) } catch (exception: NumberFormatException) { @@ -280,6 +287,9 @@ class SiloFilterExpressionMapper( values: List, ): SiloFilterExpression { val value = extractSingleFilterValue(values[0]) + if (value.isNullOrBlank()) { + return FloatEquals(siloColumnName, null) + } try { return FloatEquals(siloColumnName, value.toDouble()) } catch (exception: NumberFormatException) { @@ -308,7 +318,7 @@ class SiloFilterExpressionMapper( val value = extractSingleFilterValue(values, originalKey) try { - return value.toInt() + return value?.toInt() } catch (exception: NumberFormatException) { throw BadRequestException( "$originalKey '$value' is not a valid integer: ${exception.message}", @@ -335,7 +345,7 @@ class SiloFilterExpressionMapper( val value = extractSingleFilterValue(values, originalKey) try { - return value.toDouble() + return value?.toDouble() } catch (exception: NumberFormatException) { throw BadRequestException( "$originalKey '$value' is not a valid float: ${exception.message}", @@ -408,9 +418,14 @@ class SiloFilterExpressionMapper( extractSingleFilterValue(value.values, value.originalKey) private fun extractSingleFilterValue( - values: List, + values: List, originalKey: String, - ) = values.singleOrNull() ?: throw BadRequestException( - "Expected exactly one value for '$originalKey' but got ${values.size} values.", - ) + ): String? { + if (values.size > 1) { + throw BadRequestException( + "Expected exactly one value for '$originalKey' but got ${values.size} values.", + ) + } + return values[0] + } } diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/openApi/OpenApiDocs.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/openApi/OpenApiDocs.kt index 2ac76ff7..8bd2689c 100644 --- a/lapis2/src/main/kotlin/org/genspectrum/lapis/openApi/OpenApiDocs.kt +++ b/lapis2/src/main/kotlin/org/genspectrum/lapis/openApi/OpenApiDocs.kt @@ -340,17 +340,33 @@ private fun primitiveSequenceFilterFieldSchemas(sequenceFilterFields: SequenceFi private fun filterFieldSchema(fieldType: SequenceFilterFieldType) = when (fieldType) { - SequenceFilterFieldType.String, SequenceFilterFieldType.PangoLineage -> + SequenceFilterFieldType.PangoLineage -> Schema().anyOf( listOf( - Schema().type(fieldType.openApiType), - arraySchema(Schema().type(fieldType.openApiType)), + nonNullableStringSchema(fieldType.openApiType), + nonNullableStringArraySchema(fieldType.openApiType), ), ) - else -> Schema().type(fieldType.openApiType) + SequenceFilterFieldType.String -> + Schema().anyOf( + listOf( + nullableStringSchema(fieldType.openApiType), + nullableStringArraySchema(fieldType.openApiType), + ), + ) + + else -> nullableStringSchema(fieldType.openApiType) } +private fun nullableStringSchema(type: String) = Schema().type(type).nullable(true) + +private fun nullableStringArraySchema(type: String) = arraySchema(nullableStringSchema(type)) + +private fun nonNullableStringSchema(type: String) = Schema().type(type) + +private fun nonNullableStringArraySchema(type: String) = arraySchema(nonNullableStringSchema(type)) + private fun requestSchemaForCommonSequenceFilters( requestProperties: Map>, ): Schema<*> = diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/request/CommonSequenceFilters.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/request/CommonSequenceFilters.kt index dceff0e9..6d8c91c9 100644 --- a/lapis2/src/main/kotlin/org/genspectrum/lapis/request/CommonSequenceFilters.kt +++ b/lapis2/src/main/kotlin/org/genspectrum/lapis/request/CommonSequenceFilters.kt @@ -15,7 +15,7 @@ import org.genspectrum.lapis.controller.ORDER_BY_PROPERTY import org.genspectrum.lapis.controller.SPECIAL_REQUEST_PROPERTIES import org.springframework.util.MultiValueMap -typealias SequenceFilters = Map> +typealias SequenceFilters = Map> typealias GetRequestSequenceFilters = MultiValueMap interface CommonSequenceFilters { @@ -124,10 +124,11 @@ private fun getValuesList( value: JsonNode, key: String, ) = when { - value.isValueNode -> listOf(value.asText()) + value.isValueNode -> listOf(getValueNode(value)) + value.nodeType == JsonNodeType.ARRAY -> value.map { when { - it.isValueNode -> it.asText() + it.isValueNode -> getValueNode(it) else -> throw BadRequestException( "Found unexpected array value $it of type ${it.nodeType} for $key, expected a primitive", ) @@ -138,3 +139,10 @@ private fun getValuesList( "Found unexpected value $value of type ${value.nodeType} for $key, expected primitive or array", ) } + +private fun getValueNode(value: JsonNode): String? { + if (value.isNull) { + return null + } + return value.asText() +} diff --git a/lapis2/src/main/kotlin/org/genspectrum/lapis/silo/SiloQuery.kt b/lapis2/src/main/kotlin/org/genspectrum/lapis/silo/SiloQuery.kt index 2b8c9e92..147974a6 100644 --- a/lapis2/src/main/kotlin/org/genspectrum/lapis/silo/SiloQuery.kt +++ b/lapis2/src/main/kotlin/org/genspectrum/lapis/silo/SiloQuery.kt @@ -214,7 +214,7 @@ sealed class SiloAction( sealed class SiloFilterExpression(val type: String) -data class StringEquals(val column: String, val value: String) : SiloFilterExpression("StringEquals") +data class StringEquals(val column: String, val value: String?) : SiloFilterExpression("StringEquals") data class BooleanEquals(val column: String, val value: Boolean?) : SiloFilterExpression("BooleanEquals") @@ -262,11 +262,11 @@ data class Maybe(val child: SiloFilterExpression) : SiloFilterExpression("Maybe" data class NOf(val numberOfMatchers: Int, val matchExactly: Boolean, val children: List) : SiloFilterExpression("N-Of") -data class IntEquals(val column: String, val value: Int) : SiloFilterExpression("IntEquals") +data class IntEquals(val column: String, val value: Int?) : SiloFilterExpression("IntEquals") data class IntBetween(val column: String, val from: Int?, val to: Int?) : SiloFilterExpression("IntBetween") -data class FloatEquals(val column: String, val value: Double) : SiloFilterExpression("FloatEquals") +data class FloatEquals(val column: String, val value: Double?) : SiloFilterExpression("FloatEquals") data class FloatBetween(val column: String, val from: Double?, val to: Double?) : SiloFilterExpression("FloatBetween") diff --git a/lapis2/src/test/kotlin/org/genspectrum/lapis/model/SiloFilterExpressionMapperTest.kt b/lapis2/src/test/kotlin/org/genspectrum/lapis/model/SiloFilterExpressionMapperTest.kt index d2f8a808..f6e9b795 100644 --- a/lapis2/src/test/kotlin/org/genspectrum/lapis/model/SiloFilterExpressionMapperTest.kt +++ b/lapis2/src/test/kotlin/org/genspectrum/lapis/model/SiloFilterExpressionMapperTest.kt @@ -517,6 +517,10 @@ class SiloFilterExpressionMapperTest { mapOf("floatFieldFrom" to listOf("0.1", "0.2")), "Expected exactly one value for 'floatFieldFrom' but got 2 values.", ), + Arguments.of( + mapOf("pangoLineage" to listOf(null)), + "Invalid pango lineage: null must not be empty", + ), ) @JvmStatic @@ -532,6 +536,14 @@ class SiloFilterExpressionMapperTest { Or(StringEquals("other_metadata", "def")), ), ), + Arguments.of( + mapOf( + "some_metadata" to listOf(null), + ), + And( + Or(StringEquals("some_metadata", null)), + ), + ), Arguments.of( mapOf("pangoLineage" to listOf("A.1.2.3")), And(Or(PangoLineageEquals("pangoLineage", "A.1.2.3", includeSublineages = false))), @@ -620,6 +632,12 @@ class SiloFilterExpressionMapperTest { ), And(IntEquals("intField", 42)), ), + Arguments.of( + mapOf( + "intField" to listOf(null), + ), + And(IntEquals("intField", null)), + ), Arguments.of( mapOf( "intFieldFrom" to listOf("42"), @@ -638,6 +656,12 @@ class SiloFilterExpressionMapperTest { ), And(FloatEquals("floatField", 42.45)), ), + Arguments.of( + mapOf( + "floatField" to listOf(null), + ), + And(FloatEquals("floatField", null)), + ), Arguments.of( mapOf( "floatFieldFrom" to listOf("42.45"), @@ -674,12 +698,13 @@ class SiloFilterExpressionMapperTest { ), Arguments.of( mapOf( - "test_boolean_column" to listOf("true", "false"), + "test_boolean_column" to listOf("true", "false", null), ), And( Or( BooleanEquals("test_boolean_column", true), BooleanEquals("test_boolean_column", false), + BooleanEquals("test_boolean_column", null), ), ), ), diff --git a/lapis2/src/test/kotlin/org/genspectrum/lapis/request/SequenceFiltersRequestWithFieldsTest.kt b/lapis2/src/test/kotlin/org/genspectrum/lapis/request/SequenceFiltersRequestWithFieldsTest.kt index dfc1de68..40725011 100644 --- a/lapis2/src/test/kotlin/org/genspectrum/lapis/request/SequenceFiltersRequestWithFieldsTest.kt +++ b/lapis2/src/test/kotlin/org/genspectrum/lapis/request/SequenceFiltersRequestWithFieldsTest.kt @@ -220,6 +220,21 @@ class SequenceFiltersRequestWithFieldsTest { emptyList(), ), ), + Arguments.of( + """ + { + "country": null + } + """, + SequenceFiltersRequestWithFields( + mapOf("country" to listOf(null)), + emptyList(), + emptyList(), + emptyList(), + emptyList(), + emptyList(), + ), + ), ) @JvmStatic diff --git a/lapis2/src/test/kotlin/org/genspectrum/lapis/silo/SiloQueryTest.kt b/lapis2/src/test/kotlin/org/genspectrum/lapis/silo/SiloQueryTest.kt index 1e644b0e..36f5d3c7 100644 --- a/lapis2/src/test/kotlin/org/genspectrum/lapis/silo/SiloQueryTest.kt +++ b/lapis2/src/test/kotlin/org/genspectrum/lapis/silo/SiloQueryTest.kt @@ -361,6 +361,16 @@ class SiloQueryTest { } """, ), + Arguments.of( + StringEquals("theColumn", null), + """ + { + "type": "StringEquals", + "column": "theColumn", + "value": null + } + """, + ), Arguments.of( PangoLineageEquals("fieldName", "ABC", includeSublineages = false), """ @@ -598,6 +608,58 @@ class SiloQueryTest { } """, ), + Arguments.of( + FloatEquals( + column = "theColumn", + value = 1.0, + ), + """ + { + "type": "FloatEquals", + "column": "theColumn", + "value": 1.0 + } + """, + ), + Arguments.of( + FloatEquals( + column = "theColumn", + value = null, + ), + """ + { + "type": "FloatEquals", + "column": "theColumn", + "value": null + } + """, + ), + Arguments.of( + IntEquals( + column = "theColumn", + value = 1, + ), + """ + { + "type": "IntEquals", + "column": "theColumn", + "value": 1 + } + """, + ), + Arguments.of( + IntEquals( + column = "theColumn", + value = null, + ), + """ + { + "type": "IntEquals", + "column": "theColumn", + "value": null + } + """, + ), ) } } diff --git a/siloLapisTests/test/aggregated.spec.ts b/siloLapisTests/test/aggregated.spec.ts index fde4155c..ee287149 100644 --- a/siloLapisTests/test/aggregated.spec.ts +++ b/siloLapisTests/test/aggregated.spec.ts @@ -46,7 +46,7 @@ describe('The /aggregated endpoint', () => { }) ); - it('should correctly handle mutliple mutation requests in GET requests', async () => { + it('should correctly handle multiple mutation requests in GET requests', async () => { const urlParams = new URLSearchParams({ nucleotideMutations: 'T1-,A23062T', aminoAcidMutations: 'S:501Y,ORF1b:12', @@ -200,4 +200,64 @@ age country count `.trim() + '\n' ); }); + + it('should handle null values for boolean filters in GET requests', async () => { + const urlParams = new URLSearchParams({ + test_boolean_column: '', + }); + + const result = await getAggregated(urlParams); + + expect(result.status).equals(200); + const resultJson = await result.json(); + expect(resultJson.data[0]).to.have.property('count', 33); + }); + + it('should handle null values for int filters in GET requests', async () => { + const urlParams = new URLSearchParams({ + age: '', + }); + + const result = await getAggregated(urlParams); + + expect(result.status).equals(200); + const resultJson = await result.json(); + expect(resultJson.data[0]).to.have.property('count', 2); + }); + + it('should handle null values for float filters in GET requests', async () => { + const urlParams = new URLSearchParams({ + qc_value: '', + }); + + const result = await getAggregated(urlParams); + + expect(result.status).equals(200); + const resultJson = await result.json(); + expect(resultJson.data[0]).to.have.property('count', 2); + }); + + it('should handle null values for string filters in GET requests', async () => { + const urlParams = new URLSearchParams({ + region: '', + }); + + const result = await getAggregated(urlParams); + + expect(result.status).equals(200); + const resultJson = await result.json(); + expect(resultJson.data[0]).to.have.property('count', 1); + }); + + it('should throw for null values for pango lineage filters in GET requests', async () => { + const urlParams = new URLSearchParams({ + pangoLineage: '', + }); + + const result = await getAggregated(urlParams); + + expect(result.status).equals(200); + const resultJson = await result.json(); + expect(resultJson.data[0]).to.have.property('count', 1); + }); }); diff --git a/siloLapisTests/test/aggregatedQueries/filterByNullValue.json b/siloLapisTests/test/aggregatedQueries/filterByNullValue.json new file mode 100644 index 00000000..adc7a1ba --- /dev/null +++ b/siloLapisTests/test/aggregatedQueries/filterByNullValue.json @@ -0,0 +1,11 @@ +{ + "testCaseName": "filter by null", + "lapisRequest": { + "testBooleanColumn": null + }, + "expected": [ + { + "count": 33 + } + ] +}