Skip to content

Commit

Permalink
feat(lapis2): implement boolean columns
Browse files Browse the repository at this point in the history
resolves #740
  • Loading branch information
fengelniederhammer committed Apr 24, 2024
1 parent b424e0c commit 57e0691
Show file tree
Hide file tree
Showing 16 changed files with 244 additions and 116 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ enum class MetadataType {
@JsonProperty("float")
FLOAT,

@JsonProperty("boolean")
BOOLEAN,

@JsonProperty("insertion")
NUCLEOTIDE_INSERTION,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,13 @@ private fun mapToSequenceFilterField(databaseMetadata: DatabaseMetadata) =
),
)

MetadataType.BOOLEAN -> listOf(
SequenceFilterField(
name = databaseMetadata.name,
type = SequenceFilterFieldType.Boolean,
),
)

MetadataType.NUCLEOTIDE_INSERTION -> emptyList()
MetadataType.AMINO_ACID_INSERTION -> emptyList()
}
Expand All @@ -113,6 +120,8 @@ sealed class SequenceFilterFieldType(val openApiType: kotlin.String) {

data object Date : SequenceFilterFieldType("string")

data object Boolean : SequenceFilterFieldType("boolean")

data object VariantQuery : SequenceFilterFieldType("string")

data class DateFrom(val associatedField: SequenceFilterFieldName) : SequenceFilterFieldType("string")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import org.genspectrum.lapis.request.NucleotideMutation
import org.genspectrum.lapis.silo.AminoAcidInsertionContains
import org.genspectrum.lapis.silo.AminoAcidSymbolEquals
import org.genspectrum.lapis.silo.And
import org.genspectrum.lapis.silo.BooleanEquals
import org.genspectrum.lapis.silo.DateBetween
import org.genspectrum.lapis.silo.FloatBetween
import org.genspectrum.lapis.silo.FloatEquals
Expand Down Expand Up @@ -73,6 +74,7 @@ class SiloFilterExpressionMapper(
Filter.IntBetween -> mapToIntBetweenFilter(siloColumnName, values)
Filter.FloatEquals -> mapToFloatEqualsFilter(siloColumnName, values)
Filter.FloatBetween -> mapToFloatBetweenFilter(siloColumnName, values)
Filter.BooleanEquals -> mapToBooleanEqualsFilters(siloColumnName, values)
}
}

Expand Down Expand Up @@ -109,6 +111,7 @@ class SiloFilterExpressionMapper(
SequenceFilterFieldType.Float -> Pair(field.name, Filter.FloatEquals)
is SequenceFilterFieldType.FloatFrom -> Pair(type.associatedField, Filter.FloatBetween)
is SequenceFilterFieldType.FloatTo -> Pair(type.associatedField, Filter.FloatBetween)
SequenceFilterFieldType.Boolean -> Pair(field.name, Filter.BooleanEquals)

null -> throw BadRequestException(
"'$key' is not a valid sequence filter key. Valid keys are: " +
Expand Down Expand Up @@ -169,6 +172,20 @@ class SiloFilterExpressionMapper(
values: List<SequenceFilterValue>,
) = Or(values[0].values.map { StringEquals(siloColumnName, it) })

private fun mapToBooleanEqualsFilters(
siloColumnName: SequenceFilterFieldName,
values: List<SequenceFilterValue>,
) = Or(
values[0].values.map {
val value = try {
it.lowercase().toBooleanStrict()
} catch (e: IllegalArgumentException) {
throw BadRequestException("'$it' is not a valid boolean.")
}
BooleanEquals(siloColumnName, value)
},
)

private fun mapToVariantQueryFilter(variantQuery: String): SiloFilterExpression {
if (variantQuery.isBlank()) {
throw BadRequestException("variantQuery must not be empty")
Expand Down Expand Up @@ -382,6 +399,7 @@ class SiloFilterExpressionMapper(
IntBetween,
FloatEquals,
FloatBetween,
BooleanEquals,
}

private val variantQueryTypes = listOf(Filter.PangoLineage)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ private fun mapToOpenApiType(type: MetadataType): String =
MetadataType.FLOAT -> "number"
MetadataType.NUCLEOTIDE_INSERTION -> "string"
MetadataType.AMINO_ACID_INSERTION -> "string"
MetadataType.BOOLEAN -> "boolean"
}

private fun primitiveSequenceFilterFieldSchemas(sequenceFilterFields: SequenceFilterFields) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,8 @@ sealed class SiloFilterExpression(val type: String)

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) :
SiloFilterExpression("PangoLineage")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ val dummySequenceFilterFields = SequenceFilterFields(
"floatField" to SequenceFilterFieldType.Float,
"floatFieldTo" to SequenceFilterFieldType.FloatTo("floatField"),
"floatFieldFrom" to SequenceFilterFieldType.FloatFrom("floatField"),
"test_boolean_column" to SequenceFilterFieldType.Boolean,
)
.map { (name, type) -> name.lowercase() to SequenceFilterField(name, type) }
.toMap(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class DatabaseConfigTest {
DatabaseMetadata(name = "region", type = MetadataType.STRING),
DatabaseMetadata(name = "country", type = MetadataType.STRING),
DatabaseMetadata(name = "pangoLineage", type = MetadataType.PANGO_LINEAGE),
DatabaseMetadata(name = "test_boolean_column", type = MetadataType.BOOLEAN),
DatabaseMetadata(name = "nucInsertion", type = MetadataType.NUCLEOTIDE_INSERTION),
DatabaseMetadata(name = "aaInsertion", type = MetadataType.AMINO_ACID_INSERTION),
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import org.genspectrum.lapis.request.SequenceFilters
import org.genspectrum.lapis.silo.AminoAcidInsertionContains
import org.genspectrum.lapis.silo.AminoAcidSymbolEquals
import org.genspectrum.lapis.silo.And
import org.genspectrum.lapis.silo.BooleanEquals
import org.genspectrum.lapis.silo.DateBetween
import org.genspectrum.lapis.silo.FloatBetween
import org.genspectrum.lapis.silo.FloatEquals
Expand Down Expand Up @@ -448,6 +449,21 @@ class SiloFilterExpressionMapperTest {
)
}

@Test
fun `GIVEN a boolean equals with non-boolean value THEN should throw an error`() {
val filterParameter = getSequenceFilters(
mapOf(
"test_boolean_column" to "not a boolean",
),
)

val exception = assertThrows<BadRequestException> { underTest.map(filterParameter) }
assertThat(
exception.message,
containsString("'not a boolean' is not a valid boolean."),
)
}

@ParameterizedTest
@MethodSource("getFilterParametersWithMultipleValues")
fun `GIVEN multiple value for date field THEN throws an error`(
Expand Down Expand Up @@ -656,6 +672,17 @@ class SiloFilterExpressionMapperTest {
),
),
),
Arguments.of(
mapOf(
"test_boolean_column" to listOf("true", "false"),
),
And(
Or(
BooleanEquals("test_boolean_column", true),
BooleanEquals("test_boolean_column", false),
),
),
),
)
}

Expand Down
56 changes: 41 additions & 15 deletions lapis2/src/test/kotlin/org/genspectrum/lapis/silo/SiloQueryTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -553,24 +553,50 @@ class SiloQueryTest {
),
),
"""
{
"type": "N-Of",
"numberOfMatchers": 2,
"matchExactly": true,
"children": [
{
"type": "StringEquals",
"column": "theColumn",
"value": "theValue"
},
"type": "N-Of",
"numberOfMatchers": 2,
"matchExactly": true,
"children": [
{
"type": "StringEquals",
"column": "theColumn",
"value": "theValue"
},
{
"type": "StringEquals",
"column": "theOtherColumn",
"value": "theOtherValue"
}
]
}
""",
),
Arguments.of(
BooleanEquals(
column = "theColumn",
value = true,
),
"""
{
"type": "StringEquals",
"column": "theOtherColumn",
"value": "theOtherValue"
"type": "BooleanEquals",
"column": "theColumn",
"value": true
}
]
}
""",
""",
),
Arguments.of(
BooleanEquals(
column = "theColumn",
value = null,
),
"""
{
"type": "BooleanEquals",
"column": "theColumn",
"value": null
}
""",
),
)
}
Expand Down
2 changes: 2 additions & 0 deletions lapis2/src/test/resources/config/testDatabaseConfig.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ schema:
type: string
- name: pangoLineage
type: pango_lineage
- name: test_boolean_column
type: boolean
- name: nucInsertion
type: insertion
- name: aaInsertion
Expand Down
11 changes: 11 additions & 0 deletions siloLapisTests/test/aggregatedQueries/filterByBoolean.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"testCaseName": "boolean equals",
"lapisRequest": {
"testBooleanColumn": true
},
"expected": [
{
"count": 34
}
]
}
19 changes: 19 additions & 0 deletions siloLapisTests/test/aggregatedQueries/groupByBoolean.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"testCaseName": "boolean equals",
"lapisRequest": {
"fields": ["test_boolean_column"]
},
"expected": [
{
"count": 34,
"testBooleanColumn": true
},
{
"count": 33
},
{
"count": 33,
"testBooleanColumn": false
}
]
}
4 changes: 4 additions & 0 deletions siloLapisTests/test/details.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ describe('The /details endpoint', () => {
pangoLineage: 'B.1.617.2',
qcValue: undefined,
region: undefined,
testBooleanColumn: undefined,
});
});

Expand All @@ -40,6 +41,7 @@ describe('The /details endpoint', () => {
pangoLineage: 'B.1.617.2',
qcValue: 0.96,
region: 'Europe',
testBooleanColumn: false,
});
});

Expand Down Expand Up @@ -140,6 +142,7 @@ Solothurn B.1 key_1002052
pangoLineage: 'P.1',
qcValue: 0.93,
region: 'Europe',
testBooleanColumn: undefined,
};

const result = await lapisClient.postDetails1({
Expand All @@ -162,6 +165,7 @@ Solothurn B.1 key_1002052
pangoLineage: 'AY.43',
qcValue: 0.98,
region: 'Europe',
testBooleanColumn: true,
};

const result = await lapisClient.postDetails1({
Expand Down
2 changes: 2 additions & 0 deletions siloLapisTests/testData/protectedTestDatabaseConfig.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ schema:
type: int
- name: qc_value
type: float
- name: test_boolean_column
type: boolean
- name: insertions
type: insertion
- name: aaInsertions
Expand Down
Loading

0 comments on commit 57e0691

Please sign in to comment.