feat(lapis2): allow filtering for null

- except pango lineage wip: swagger ui
GenSpectrum · May 7, 2024 · f680305 · f680305
1 parent c7a2912
commit f680305
Show file tree

Hide file tree

Showing 9 changed files with 576 additions and 283 deletions.
diff --git 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<String>, val originalKey: String)
+data class SequenceFilterValue(val type: SequenceFilterFieldType, val values: List<String?>, val originalKey: String)
 
 typealias SequenceFilterFieldName = String
 
@@ -177,6 +177,9 @@ class SiloFilterExpressionMapper(
         values: List<SequenceFilterValue>,
     ) = 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")
         }
 
@@ -236,6 +239,10 @@ class SiloFilterExpressionMapper(
         val (_, values, originalKey) = sequenceFilterValue ?: return null
         val value = extractSingleFilterValue(values, originalKey)
 
+        if (value.isNullOrBlank()) {
+            return null
+        }
+
         try {
             return LocalDate.parse(value)
         } catch (exception: DateTimeParseException) {
@@ -249,6 +256,7 @@ class SiloFilterExpressionMapper(
     ) = Or(
         values[0].values.map {
             when {
+                it.isNullOrBlank() -> PangoLineageEquals(column, null, includeSublineages = false)
                 it.endsWith(".*") -> PangoLineageEquals(column, it.substringBeforeLast(".*"), includeSublineages = true)
                 it.endsWith('*') -> PangoLineageEquals(column, it.substringBeforeLast('*'), includeSublineages = true)
                 it.endsWith('.') -> throw BadRequestException(
@@ -265,6 +273,9 @@ class SiloFilterExpressionMapper(
         values: List<SequenceFilterValue>,
     ): SiloFilterExpression {
         val value = extractSingleFilterValue(values[0])
+        if (value.isNullOrBlank()) {
+            return IntEquals(siloColumnName, null)
+        }
         try {
             return IntEquals(siloColumnName, value.toInt())
         } catch (exception: NumberFormatException) {
@@ -280,6 +291,9 @@ class SiloFilterExpressionMapper(
         values: List<SequenceFilterValue>,
     ): SiloFilterExpression {
         val value = extractSingleFilterValue(values[0])
+        if (value.isNullOrBlank()) {
+            return FloatEquals(siloColumnName, null)
+        }
         try {
             return FloatEquals(siloColumnName, value.toDouble())
         } catch (exception: NumberFormatException) {
@@ -307,6 +321,10 @@ class SiloFilterExpressionMapper(
         val (_, values, originalKey) = dateRangeFilters.find { (type, _, _) -> type is T } ?: return null
         val value = extractSingleFilterValue(values, originalKey)
 
+        if (value.isNullOrBlank()) {
+            return null
+        }
+
         try {
             return value.toInt()
         } catch (exception: NumberFormatException) {
@@ -334,6 +352,10 @@ class SiloFilterExpressionMapper(
         val (_, values, originalKey) = dateRangeFilters.find { (type, _, _) -> type is T } ?: return null
         val value = extractSingleFilterValue(values, originalKey)
 
+        if (value.isNullOrBlank()) {
+            return null
+        }
+
         try {
             return value.toDouble()
         } catch (exception: NumberFormatException) {
@@ -408,9 +430,14 @@ class SiloFilterExpressionMapper(
         extractSingleFilterValue(value.values, value.originalKey)
 
     private fun extractSingleFilterValue(
-        values: List<String>,
+        values: List<String?>,
         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
@@ -343,14 +343,18 @@ private fun filterFieldSchema(fieldType: SequenceFilterFieldType) =
         SequenceFilterFieldType.String, SequenceFilterFieldType.PangoLineage ->
             Schema<String>().anyOf(
                 listOf(
-                    Schema<String>().type(fieldType.openApiType),
-                    arraySchema(Schema<String>().type(fieldType.openApiType)),
+                    nullableStringSchema(fieldType.openApiType),
+                    nullableStringArraySchema(fieldType.openApiType),
                 ),
             )
 
-        else -> Schema<String>().type(fieldType.openApiType)
+        else -> nullableStringSchema(fieldType.openApiType)
     }
 
+private fun nullableStringSchema(type: String) = Schema<String>().type(type).nullable(true)
+
+private fun nullableStringArraySchema(type: String) = arraySchema(nullableStringSchema(type))
+
 private fun requestSchemaForCommonSequenceFilters(
     requestProperties: Map<SequenceFilterFieldName, Schema<out Any>>,
 ): 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
@@ -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<String, List<String>>
+typealias SequenceFilters = Map<String, List<String?>>
 typealias GetRequestSequenceFilters = MultiValueMap<String, String>
 
 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
@@ -214,11 +214,11 @@ sealed class SiloAction<ResponseType>(
 
 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")
 
-data class PangoLineageEquals(val column: String, val value: String, val includeSublineages: Boolean) :
+data class PangoLineageEquals(val column: String, val value: String?, val includeSublineages: Boolean) :
     SiloFilterExpression("PangoLineage")
 
 @JsonInclude(JsonInclude.Include.NON_NULL)
@@ -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>) :
     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
@@ -532,10 +532,26 @@ 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))),
                 ),
+                Arguments.of(
+                    mapOf("pangoLineage" to listOf("")),
+                    And(Or(PangoLineageEquals("pangoLineage", null, includeSublineages = false))),
+                ),
+                Arguments.of(
+                    mapOf("pangoLineage" to listOf(null)),
+                    And(Or(PangoLineageEquals("pangoLineage", null, includeSublineages = false))),
+                ),
                 Arguments.of(
                     mapOf("pangoLineage" to listOf("A.1.2.3*")),
                     And(Or(PangoLineageEquals("pangoLineage", "A.1.2.3", includeSublineages = true))),
@@ -570,12 +586,24 @@ class SiloFilterExpressionMapperTest {
                     ),
                     And(DateBetween("date", from = null, to = LocalDate.of(2021, 6, 3))),
                 ),
+                Arguments.of(
+                    mapOf(
+                        "dateTo" to listOf(null),
+                    ),
+                    And(DateBetween("date", from = null, to = null)),
+                ),
                 Arguments.of(
                     mapOf(
                         "dateFrom" to listOf("2021-03-28"),
                     ),
                     And(DateBetween("date", from = LocalDate.of(2021, 3, 28), to = null)),
                 ),
+                Arguments.of(
+                    mapOf(
+                        "dateFrom" to listOf(null),
+                    ),
+                    And(DateBetween("date", from = null, to = null)),
+                ),
                 Arguments.of(
                     mapOf(
                         "dateFrom" to listOf("2021-03-28"),
@@ -620,36 +648,102 @@ class SiloFilterExpressionMapperTest {
                     ),
                     And(IntEquals("intField", 42)),
                 ),
+                Arguments.of(
+                    mapOf(
+                        "intField" to listOf(null),
+                    ),
+                    And(IntEquals("intField", null)),
+                ),
+                Arguments.of(
+                    mapOf(
+                        "intField" to listOf(""),
+                    ),
+                    And(IntEquals("intField", null)),
+                ),
                 Arguments.of(
                     mapOf(
                         "intFieldFrom" to listOf("42"),
                     ),
                     And(IntBetween("intField", 42, null)),
                 ),
+                Arguments.of(
+                    mapOf(
+                        "intFieldFrom" to listOf(""),
+                    ),
+                    And(IntBetween("intField", null, null)),
+                ),
+                Arguments.of(
+                    mapOf(
+                        "intFieldFrom" to listOf(null),
+                    ),
+                    And(IntBetween("intField", null, null)),
+                ),
                 Arguments.of(
                     mapOf(
                         "intFieldTo" to listOf("42"),
                     ),
                     And(IntBetween("intField", null, 42)),
                 ),
+                Arguments.of(
+                    mapOf(
+                        "intFieldTo" to listOf(""),
+                    ),
+                    And(IntBetween("intField", null, null)),
+                ),
+                Arguments.of(
+                    mapOf(
+                        "intFieldTo" to listOf(null),
+                    ),
+                    And(IntBetween("intField", null, null)),
+                ),
                 Arguments.of(
                     mapOf(
                         "floatField" to listOf("42.45"),
                     ),
                     And(FloatEquals("floatField", 42.45)),
                 ),
+                Arguments.of(
+                    mapOf(
+                        "floatField" to listOf(null),
+                    ),
+                    And(FloatEquals("floatField", null)),
+                ),
                 Arguments.of(
                     mapOf(
                         "floatFieldFrom" to listOf("42.45"),
                     ),
                     And(FloatBetween("floatField", 42.45, null)),
                 ),
+                Arguments.of(
+                    mapOf(
+                        "floatFieldFrom" to listOf(""),
+                    ),
+                    And(FloatBetween("floatField", null, null)),
+                ),
+                Arguments.of(
+                    mapOf(
+                        "floatFieldFrom" to listOf(null),
+                    ),
+                    And(FloatBetween("floatField", null, null)),
+                ),
                 Arguments.of(
                     mapOf(
                         "floatFieldTo" to listOf("42.45"),
                     ),
                     And(FloatBetween("floatField", null, 42.45)),
                 ),
+                Arguments.of(
+                    mapOf(
+                        "floatFieldTo" to listOf(""),
+                    ),
+                    And(FloatBetween("floatField", null, null)),
+                ),
+                Arguments.of(
+                    mapOf(
+                        "floatFieldTo" to listOf(null),
+                    ),
+                    And(FloatBetween("floatField", null, null)),
+                ),
                 Arguments.of(
                     mapOf(
                         "some_metadata" to listOf("value1", "value2"),
@@ -674,12 +768,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
@@ -220,6 +220,21 @@ class SequenceFiltersRequestWithFieldsTest {
                         emptyList(),
                     ),
                 ),
+                Arguments.of(
+                    """
+                    {
+                        "country": null
+                    }
+                    """,
+                    SequenceFiltersRequestWithFields(
+                        mapOf("country" to listOf(null)),
+                        emptyList(),
+                        emptyList(),
+                        emptyList(),
+                        emptyList(),
+                        emptyList(),
+                    ),
+                ),
             )
 
         @JvmStatic