Skip to content

Commit

Permalink
feat(lapis2): allow filtering for null
Browse files Browse the repository at this point in the history
- except pango lineage
  • Loading branch information
JonasKellerer committed Apr 29, 2024
1 parent 5131283 commit 1a58e67
Show file tree
Hide file tree
Showing 8 changed files with 211 additions and 17 deletions.
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 @@ -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(
Expand All @@ -265,6 +269,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 +287,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 @@ -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}",
Expand All @@ -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}",
Expand Down Expand Up @@ -408,9 +418,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 @@ -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 @@ -122,10 +122,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 @@ -136,3 +137,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,7 +214,7 @@ 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")

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

@JvmStatic
Expand Down
62 changes: 62 additions & 0 deletions lapis2/src/test/kotlin/org/genspectrum/lapis/silo/SiloQueryTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,16 @@ class SiloQueryTest {
}
""",
),
Arguments.of(
StringEquals("theColumn", null),
"""
{
"type": "StringEquals",
"column": "theColumn",
"value": null
}
""",
),
Arguments.of(
PangoLineageEquals("fieldName", "ABC", includeSublineages = false),
"""
Expand Down Expand Up @@ -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
}
""",
),
)
}
}
60 changes: 59 additions & 1 deletion siloLapisTests/test/aggregated.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -200,4 +200,62 @@ 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', 1);
});

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(400);
});
});
Loading

0 comments on commit 1a58e67

Please sign in to comment.