Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow filtering for null #760

Merged
merged 1 commit into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
JonasKellerer marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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) {
Expand All @@ -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")
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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(
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<*> =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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",
)
Expand All @@ -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()
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))),
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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"),
Expand All @@ -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),
),
),
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,21 @@ class SequenceFiltersRequestWithFieldsTest {
emptyList(),
),
),
Arguments.of(
"""
{
"country": null
}
""",
SequenceFiltersRequestWithFields(
mapOf("country" to listOf(null)),
emptyList(),
emptyList(),
emptyList(),
emptyList(),
emptyList(),
),
),
)

@JvmStatic
Expand Down
Loading
Loading