diff --git a/lapis-docs/astro.config.mjs b/lapis-docs/astro.config.mjs index 1c4a78dad..87dd83079 100644 --- a/lapis-docs/astro.config.mjs +++ b/lapis-docs/astro.config.mjs @@ -102,6 +102,10 @@ export default defineConfig({ label: 'Request ID', link: '/concepts/request-id', }, + { + label: 'Computed fields', + link: '/concepts/computed-fields', + }, { label: 'Filters', items: [ diff --git a/lapis-docs/src/content/docs/concepts/computed-fields.mdx b/lapis-docs/src/content/docs/concepts/computed-fields.mdx new file mode 100644 index 000000000..2423bdd20 --- /dev/null +++ b/lapis-docs/src/content/docs/concepts/computed-fields.mdx @@ -0,0 +1,35 @@ +--- +title: Computed fields +description: Computed fields +--- + +In addition to plain metadata field names, the `fields` parameter of `/sample/aggregated` accepts computed fields +using the syntax `.`, e.g. `date.isoWeek`. +A computed field applies a scalar function to a metadata field and groups by the result, +instead of grouping by the raw field value. + +**Example:** to count sequences per ISO week instead of per exact date: + +``` +[URL to LAPIS instance]/sample/aggregated?fields=date.isoWeek +``` + +```json +{ + "fields": ["date.isoWeek"] +} +``` + +The response uses the full `.` string as the column/property key, e.g. `date.isoWeek`. + +## Available functions + +| Function | Applicable field types | Description | +| --------- | ---------------------- | ------------------------------------ | +| `isoWeek` | `date` | The ISO 8601 week of the date field. | + +:::note +Computed fields are only supported in the `fields` parameter of `/sample/aggregated` +(and the analogous endpoints for other sequence types). +They are not supported by endpoints that return the underlying data rows, such as `/sample/details`. +::: diff --git a/lapis-docs/src/content/docs/references/introduction.mdx b/lapis-docs/src/content/docs/references/introduction.mdx index 224e62e0f..1a2661524 100644 --- a/lapis-docs/src/content/docs/references/introduction.mdx +++ b/lapis-docs/src/content/docs/references/introduction.mdx @@ -29,3 +29,4 @@ Fields allow you to dictate the grouping of the returned data. For example, to find out the number of sequences from each country,you would use `GET /sample/aggregated?fields=country`. The available fields are documented in [Fields](../references/fields), and you can test them at [Open API / Swagger](../references/open-api-definition). +Some fields also support [computed fields](../concepts/computed-fields), e.g. `fields=date.isoWeek`. diff --git a/lapis-docs/tests/docs.spec.ts b/lapis-docs/tests/docs.spec.ts index e17d8bfac..2ee821341 100644 --- a/lapis-docs/tests/docs.spec.ts +++ b/lapis-docs/tests/docs.spec.ts @@ -47,6 +47,7 @@ const conceptsPages = prependToRelativeUrl( { title: 'Customizable FASTA headers', relativeUrl: '/customizable-fasta-headers' }, { title: 'Data versions', relativeUrl: '/data-versions' }, { title: 'Request ID', relativeUrl: '/request-id' }, + { title: 'Computed fields', relativeUrl: '/computed-fields' }, { title: 'Mutation filters', relativeUrl: '/mutation-filters' }, { title: 'Insertion filters', relativeUrl: '/insertion-filters' }, { title: 'Ambiguous symbols', relativeUrl: '/ambiguous-symbols' }, diff --git a/lapis-e2e/test/aggregated.spec.ts b/lapis-e2e/test/aggregated.spec.ts index 2e54adff1..d34089cf6 100644 --- a/lapis-e2e/test/aggregated.spec.ts +++ b/lapis-e2e/test/aggregated.spec.ts @@ -172,6 +172,56 @@ describe('The /aggregated endpoint', () => { expect(resultJson.error.detail).to.include("Unknown field: 'notAField', known values are [primaryKey,"); }); + it('should stratify by a computed field using dot notation', async () => { + const result = await lapisClient.postAggregated({ + aggregatedPostRequest: { + date: '2021-06-05', + fields: ['date.isoWeek'], + }, + }); + + expect(result.data).to.have.length(1); + expect(result.data[0]).to.have.property('count', 1); + expect(result.data[0]).to.have.property('date.isoWeek', 22); + }); + + it('should order by a computed field using dot notation', async () => { + const result = await lapisClient.postAggregated({ + aggregatedPostRequest: { + date: '2021-06-05', + fields: ['date.isoWeek'], + orderBy: [{ field: 'date.isoWeek', type: 'ascending' }], + }, + }); + + expect(result.data).to.have.length(1); + expect(result.data[0]).to.have.property('date.isoWeek', 22); + }); + + it('should return bad request for an unknown scalar function', async () => { + const urlParams = new URLSearchParams({ + fields: 'date.notAFunction', + }); + + const result = await getAggregated(urlParams); + + expect(result.status).equals(400); + const resultJson = await result.json(); + expect(resultJson.error.detail).to.include("Unknown scalar function 'notAFunction'"); + }); + + it('should return bad request for a scalar function applied to a field of the wrong type', async () => { + const urlParams = new URLSearchParams({ + fields: 'country.isoWeek', + }); + + const result = await getAggregated(urlParams); + + expect(result.status).equals(400); + const resultJson = await result.json(); + expect(resultJson.error.detail).to.include("is not valid for field 'country'"); + }); + it('should return bad request for invalid variant query', async () => { const urlParams = new URLSearchParams({ variantQuery: 'not a valid variant query', diff --git a/lapis-e2e/test/details.spec.ts b/lapis-e2e/test/details.spec.ts index c0aa9a86f..3250c6759 100644 --- a/lapis-e2e/test/details.spec.ts +++ b/lapis-e2e/test/details.spec.ts @@ -32,6 +32,18 @@ describe('The /details endpoint', () => { }); }); + it('should return bad request for a computed field', async () => { + const result = await fetch( + basePath + '/sample/details?' + new URLSearchParams({ fields: 'date.isoWeek' }) + ); + + expect(result.status).equals(400); + const resultJson = await result.json(); + expect(resultJson.error.detail).to.include( + 'Scalar functions are not supported in fields for this endpoint: date.isoWeek' + ); + }); + it('should fetch the correct key for "isNull" filter', async () => { const result = await lapisClient.postDetails({ detailsPostRequest: { diff --git a/lapis/src/main/kotlin/org/genspectrum/lapis/controller/ControllerDescriptions.kt b/lapis/src/main/kotlin/org/genspectrum/lapis/controller/ControllerDescriptions.kt index 48b31a67a..3d116317a 100644 --- a/lapis/src/main/kotlin/org/genspectrum/lapis/controller/ControllerDescriptions.kt +++ b/lapis/src/main/kotlin/org/genspectrum/lapis/controller/ControllerDescriptions.kt @@ -69,11 +69,14 @@ const val AGGREGATED_GROUP_BY_FIELDS_DESCRIPTION = """The fields to stratify by. If empty, only the overall count is returned. If requesting CSV or TSV data, the columns are ordered in the same order as the fields are specified here. -You can provide metadata fields.""" +You can provide metadata fields. +A field can also be a computed field of the form "." (e.g. "date.isoWeek"), which applies a +scalar function to the field and stratifies by the result. The response uses the full "." string +as the field name. Currently, the only supported function is "isoWeek", which is only valid for date fields.""" const val AGGREGATED_ORDER_BY_FIELDS_DESCRIPTION = - """The fields of the response to order by. - Fields specified here must either be \"count\" or also be present in \"fields\". - You can also use \"random\" or \"random(SEED)\" where SEED is an integer.""" + """The fields of the response to order by. + Fields specified here must either be "count" or also be present in "fields". + You can also use "random" or "random(SEED)" where SEED is an integer.""" const val DETAILS_FIELDS_DESCRIPTION = """The fields that the response items should contain. If empty, all fields are returned. diff --git a/lapis/src/main/kotlin/org/genspectrum/lapis/model/SiloQueryModel.kt b/lapis/src/main/kotlin/org/genspectrum/lapis/model/SiloQueryModel.kt index 7e787da36..78519a861 100644 --- a/lapis/src/main/kotlin/org/genspectrum/lapis/model/SiloQueryModel.kt +++ b/lapis/src/main/kotlin/org/genspectrum/lapis/model/SiloQueryModel.kt @@ -4,6 +4,7 @@ import org.genspectrum.lapis.config.DatabaseConfig import org.genspectrum.lapis.config.ReferenceGenomeSchema import org.genspectrum.lapis.request.AggregatedFiltersRequest import org.genspectrum.lapis.request.CommonSequenceFilters +import org.genspectrum.lapis.request.ComputedField import org.genspectrum.lapis.request.DetailsFiltersRequest import org.genspectrum.lapis.request.MRCASequenceFiltersRequest import org.genspectrum.lapis.request.MutationProportionsRequest @@ -46,6 +47,7 @@ class SiloQueryModel( limit = sequenceFilters.limit, offset = sequenceFilters.offset, sequencePositionFields = sequenceFilters.fields.filterIsInstance(), + computedFields = sequenceFilters.fields.filterIsInstance(), ), siloFilterExpressionMapper.map(sequenceFilters), ), diff --git a/lapis/src/main/kotlin/org/genspectrum/lapis/model/mutationsOverTime/QueriesOverTimeModel.kt b/lapis/src/main/kotlin/org/genspectrum/lapis/model/mutationsOverTime/QueriesOverTimeModel.kt index de32a08a9..6e14d8bc9 100644 --- a/lapis/src/main/kotlin/org/genspectrum/lapis/model/mutationsOverTime/QueriesOverTimeModel.kt +++ b/lapis/src/main/kotlin/org/genspectrum/lapis/model/mutationsOverTime/QueriesOverTimeModel.kt @@ -339,10 +339,10 @@ class QueriesOverTimeModel( siloClient.sendQueryAndGetDataVersion( SiloQuery( SiloAction.aggregated( - listOf(dateField), - OrderBySpec.EMPTY, - null, - null, + groupByFields = listOf(dateField), + orderByFields = OrderBySpec.EMPTY, + limit = null, + offset = null, ), And( children = listOfNotNull( diff --git a/lapis/src/main/kotlin/org/genspectrum/lapis/request/Field.kt b/lapis/src/main/kotlin/org/genspectrum/lapis/request/Field.kt index 517cc6775..8a68a161a 100644 --- a/lapis/src/main/kotlin/org/genspectrum/lapis/request/Field.kt +++ b/lapis/src/main/kotlin/org/genspectrum/lapis/request/Field.kt @@ -18,3 +18,10 @@ data class SequencePositionField( /** Used both as the SaneQL alias and as the response column key, e.g. `S[501]` or `[501]` for shorthand. */ override val outputColumnName: String get() = if (isSingleSegment) "[$position]" else "$sequenceName[$position]" } + +data class ComputedField( + val sourceField: String, + val function: ScalarFunction, +) : Field { + override val outputColumnName: String get() = "$sourceField.${function.saneQlMethodName}" +} diff --git a/lapis/src/main/kotlin/org/genspectrum/lapis/request/PhyloTreeSequenceFiltersRequest.kt b/lapis/src/main/kotlin/org/genspectrum/lapis/request/PhyloTreeSequenceFiltersRequest.kt index 3d5d2fa46..7c496ec69 100644 --- a/lapis/src/main/kotlin/org/genspectrum/lapis/request/PhyloTreeSequenceFiltersRequest.kt +++ b/lapis/src/main/kotlin/org/genspectrum/lapis/request/PhyloTreeSequenceFiltersRequest.kt @@ -60,7 +60,7 @@ class PhyloTreeSequenceFiltersRequestDeserializer( parsedCommonFields.orderByFields, parsedCommonFields.limit, parsedCommonFields.offset, - phyloTreeField.fieldName, + phyloTreeField.outputColumnName, ) } } @@ -89,7 +89,7 @@ class MRCASequenceFiltersRequestDeserializer( parsedCommonFields.orderByFields, parsedCommonFields.limit, parsedCommonFields.offset, - phyloTreeField.fieldName, + phyloTreeField.outputColumnName, printNodesNotInTree = printNodesNotInTree, ) } diff --git a/lapis/src/main/kotlin/org/genspectrum/lapis/request/ScalarFunction.kt b/lapis/src/main/kotlin/org/genspectrum/lapis/request/ScalarFunction.kt new file mode 100644 index 000000000..0940e45f8 --- /dev/null +++ b/lapis/src/main/kotlin/org/genspectrum/lapis/request/ScalarFunction.kt @@ -0,0 +1,14 @@ +package org.genspectrum.lapis.request + +import org.genspectrum.lapis.config.MetadataType + +/** + * An enum of scalar functions supported by SILO. + * New functions need to be whitelisted here explicitly. + */ +enum class ScalarFunction( + val saneQlMethodName: String, + val validForTypes: Set, +) { + ISO_WEEK("isoWeek", setOf(MetadataType.DATE)), +} diff --git a/lapis/src/main/kotlin/org/genspectrum/lapis/request/converter/AggregatedFieldConverter.kt b/lapis/src/main/kotlin/org/genspectrum/lapis/request/converter/AggregatedFieldConverter.kt index e58ccbf3e..f0701fe6b 100644 --- a/lapis/src/main/kotlin/org/genspectrum/lapis/request/converter/AggregatedFieldConverter.kt +++ b/lapis/src/main/kotlin/org/genspectrum/lapis/request/converter/AggregatedFieldConverter.kt @@ -3,12 +3,15 @@ package org.genspectrum.lapis.request.converter import org.genspectrum.lapis.request.Field import org.springframework.stereotype.Component -/** Resolves a field that may be a plain metadata field or a sequence position field. Used by `/aggregated`. */ +/** Resolves a field that may be a plain metadata field, sequence position field, or scalar function field. Used by `/aggregated`. */ @Component class AggregatedFieldConverter( private val sequencePositionFieldConverter: SequencePositionFieldConverter, + private val scalarFunctionFieldConverter: ScalarFunctionFieldConverter, private val metadataFieldConverter: MetadataFieldConverter, ) : FieldConverter { override fun convert(source: String): Field = - sequencePositionFieldConverter.tryConvert(source) ?: metadataFieldConverter.convert(source) + sequencePositionFieldConverter.tryConvert(source) + ?: scalarFunctionFieldConverter.tryConvert(source) + ?: metadataFieldConverter.convert(source) } diff --git a/lapis/src/main/kotlin/org/genspectrum/lapis/request/converter/OrderByFieldsCleaner.kt b/lapis/src/main/kotlin/org/genspectrum/lapis/request/converter/OrderByFieldsCleaner.kt index f5a6ca496..caa66bd6c 100644 --- a/lapis/src/main/kotlin/org/genspectrum/lapis/request/converter/OrderByFieldsCleaner.kt +++ b/lapis/src/main/kotlin/org/genspectrum/lapis/request/converter/OrderByFieldsCleaner.kt @@ -1,10 +1,19 @@ package org.genspectrum.lapis.request.converter +import org.genspectrum.lapis.controller.BadRequestException import org.springframework.stereotype.Component @Component class OrderByFieldsCleaner( private val caseInsensitiveFieldsCleaner: CaseInsensitiveFieldsCleaner, + private val scalarFunctionFieldConverter: ScalarFunctionFieldConverter, ) { - fun clean(fieldName: String): String = caseInsensitiveFieldsCleaner.clean(fieldName) ?: fieldName + fun clean(fieldName: String): String = + try { + scalarFunctionFieldConverter.tryConvert(fieldName)?.outputColumnName + ?: caseInsensitiveFieldsCleaner.clean(fieldName) + ?: fieldName + } catch (e: BadRequestException) { + fieldName + } } diff --git a/lapis/src/main/kotlin/org/genspectrum/lapis/request/converter/PlainFieldConverter.kt b/lapis/src/main/kotlin/org/genspectrum/lapis/request/converter/PlainFieldConverter.kt index 26fa91446..1572da7d2 100644 --- a/lapis/src/main/kotlin/org/genspectrum/lapis/request/converter/PlainFieldConverter.kt +++ b/lapis/src/main/kotlin/org/genspectrum/lapis/request/converter/PlainFieldConverter.kt @@ -17,6 +17,11 @@ class PlainFieldConverter( "Sequence position fields are not supported here: ${it.outputColumnName}", ) } + if ('.' in source) { + throw BadRequestException( + "Scalar functions are not supported in fields for this endpoint: $source", + ) + } return metadataFieldConverter.convert(source) } } diff --git a/lapis/src/main/kotlin/org/genspectrum/lapis/request/converter/ScalarFunctionFieldConverter.kt b/lapis/src/main/kotlin/org/genspectrum/lapis/request/converter/ScalarFunctionFieldConverter.kt new file mode 100644 index 000000000..f7ae46a63 --- /dev/null +++ b/lapis/src/main/kotlin/org/genspectrum/lapis/request/converter/ScalarFunctionFieldConverter.kt @@ -0,0 +1,46 @@ +package org.genspectrum.lapis.request.converter + +import org.genspectrum.lapis.config.DatabaseConfig +import org.genspectrum.lapis.config.MetadataType +import org.genspectrum.lapis.controller.BadRequestException +import org.genspectrum.lapis.request.ComputedField +import org.genspectrum.lapis.request.ScalarFunction +import org.springframework.stereotype.Component + +@Component +class ScalarFunctionFieldConverter( + private val caseInsensitiveFieldsCleaner: CaseInsensitiveFieldsCleaner, + private val databaseConfig: DatabaseConfig, +) { + private val fieldTypesByLowercaseName: Map = + databaseConfig.schema.metadata.associateBy({ it.name.lowercase() }, { it.type }) + + fun tryConvert(source: String): ComputedField? { + if ('.' !in source) return null + val dotIndex = source.lastIndexOf('.') + val rawField = source.substring(0, dotIndex) + val rawFunction = source.substring(dotIndex + 1) + + val cleanedField = caseInsensitiveFieldsCleaner.clean(rawField) + ?: throw BadRequestException( + "Unknown field '$rawField' in '$source'. " + + "Known fields: ${caseInsensitiveFieldsCleaner.getKnownFields()}", + ) + + val function = ScalarFunction.entries.find { it.saneQlMethodName.equals(rawFunction, ignoreCase = true) } + ?: throw BadRequestException( + "Unknown scalar function '$rawFunction' in '$source'. " + + "Available functions: ${ScalarFunction.entries.joinToString { it.saneQlMethodName }}", + ) + + val fieldType = fieldTypesByLowercaseName[cleanedField.lowercase()]!! + if (fieldType !in function.validForTypes) { + throw BadRequestException( + "Scalar function '${function.saneQlMethodName}' is not valid for field '$cleanedField' of type " + + "$fieldType. Valid types: ${function.validForTypes.joinToString()}", + ) + } + + return ComputedField(cleanedField, function) + } +} diff --git a/lapis/src/main/kotlin/org/genspectrum/lapis/silo/SaneQlAst.kt b/lapis/src/main/kotlin/org/genspectrum/lapis/silo/SaneQlAst.kt index 24df00f84..8e868347f 100644 --- a/lapis/src/main/kotlin/org/genspectrum/lapis/silo/SaneQlAst.kt +++ b/lapis/src/main/kotlin/org/genspectrum/lapis/silo/SaneQlAst.kt @@ -62,7 +62,7 @@ data class SaneQlIdentifier( /** * A `{item1, item2, ...}` list literal. Acts as a record when its [items] are [SaneQlAssignment]s, - * e.g. `{count:=count()}`. + * e.g. `{"count":=count()}`. */ data class SaneQlList( val items: List, diff --git a/lapis/src/main/kotlin/org/genspectrum/lapis/silo/SiloQuery.kt b/lapis/src/main/kotlin/org/genspectrum/lapis/silo/SiloQuery.kt index 08af9d757..54e0fd448 100644 --- a/lapis/src/main/kotlin/org/genspectrum/lapis/silo/SiloQuery.kt +++ b/lapis/src/main/kotlin/org/genspectrum/lapis/silo/SiloQuery.kt @@ -3,6 +3,7 @@ package org.genspectrum.lapis.silo import com.fasterxml.jackson.annotation.JsonIgnore import com.fasterxml.jackson.annotation.JsonInclude import com.fasterxml.jackson.annotation.JsonProperty +import org.genspectrum.lapis.request.ComputedField import org.genspectrum.lapis.request.Order import org.genspectrum.lapis.request.OrderByField import org.genspectrum.lapis.request.OrderBySpec @@ -85,6 +86,7 @@ sealed class SiloAction( companion object { fun aggregated( groupByFields: List = emptyList(), + computedFields: List = emptyList(), orderByFields: OrderBySpec = OrderBySpec.EMPTY, limit: Int? = null, offset: Int? = null, @@ -93,6 +95,7 @@ sealed class SiloAction( AggregatedAction( groupByFields = groupByFields, sequencePositionFields = sequencePositionFields, + computedFields = computedFields, orderByFields = getOrderByFieldsList(orderByFields), randomize = getRandomize(orderByFields), limit = limit, @@ -228,6 +231,7 @@ sealed class SiloAction( data class AggregatedAction( val groupByFields: List, val sequencePositionFields: List = emptyList(), + val computedFields: List = emptyList(), override val orderByFields: List = emptyList(), override val randomize: RandomizeConfig? = null, override val limit: Int? = null, @@ -240,29 +244,34 @@ sealed class SiloAction( override fun ownSaneQlSteps() = buildList { - if (sequencePositionFields.isNotEmpty()) { + val allComputedMappings = + sequencePositionFields.map { field -> + SaneQlAssignment( + field.outputColumnName, + SaneQlMethodCall( + SaneQlIdentifier(field.sequenceName), + "at", + listOf(SaneQlInt(field.position)), + ), + ) + } + computedFields.map { field -> + SaneQlAssignment( + field.outputColumnName, + SaneQlMethodCall(id(field.sourceField), field.function.saneQlMethodName), + ) + } + if (allComputedMappings.isNotEmpty()) { add( SaneQlStep( "map", - positionalArgs = listOf( - SaneQlList( - sequencePositionFields.map { field -> - SaneQlAssignment( - field.outputColumnName, - SaneQlMethodCall( - SaneQlIdentifier(field.sequenceName), - "at", - listOf(SaneQlInt(field.position)), - ), - ) - }, - ), - ), + positionalArgs = listOf(SaneQlList(allComputedMappings)), ), ) } val allGroupByColumns = - groupByFields.map { id(it) } + sequencePositionFields.map { id(it.outputColumnName) } + groupByFields.map { id(it) } + + sequencePositionFields.map { id(it.outputColumnName) } + + computedFields.map { id(it.outputColumnName) } add( SaneQlStep( "groupBy", diff --git a/lapis/src/main/resources/templates/llms.txt b/lapis/src/main/resources/templates/llms.txt index a10f0a965..4c144b383 100644 --- a/lapis/src/main/resources/templates/llms.txt +++ b/lapis/src/main/resources/templates/llms.txt @@ -58,6 +58,18 @@ Syntax: `SequenceName[position]`, e.g. `S[501]` for position 501 of segment/gene Example: `{"fields": ["[(${firstGene})][123]"]}` groups sequences by the residue at position 123 of gene [(${firstGene})]. Response column names always use the canonical sequence name from the reference genome, regardless of the case used in the request, e.g. `s[501]` -> `S[501]`. +### Computed Fields + +The `fields` parameter of [sample/aggregated](sample/aggregated) (not [sample/details](sample/details)) also accepts +computed fields using the syntax `.`, e.g. `date.isoWeek`. +A computed field applies a scalar function to a metadata field and groups by the result instead of the raw value. +The response uses the full `.` string as the column name. + +Available functions: +- `isoWeek`: Returns the ISO 8601 week number of a date field. Only valid for `date` fields. + +Example: `{"fields": ["date.isoWeek"]}` groups sequences by ISO week of the date field. + ### How to Filter by Mutations You can filter sequences by nucleotide and amino acid mutations. diff --git a/lapis/src/test/kotlin/org/genspectrum/lapis/model/SiloQueryModelTest.kt b/lapis/src/test/kotlin/org/genspectrum/lapis/model/SiloQueryModelTest.kt index 984741866..bd443080d 100644 --- a/lapis/src/test/kotlin/org/genspectrum/lapis/model/SiloQueryModelTest.kt +++ b/lapis/src/test/kotlin/org/genspectrum/lapis/model/SiloQueryModelTest.kt @@ -14,12 +14,14 @@ import org.genspectrum.lapis.controller.sequenceFiltersRequest import org.genspectrum.lapis.databaseConfig import org.genspectrum.lapis.request.AggregatedFiltersRequest import org.genspectrum.lapis.request.CommonSequenceFilters +import org.genspectrum.lapis.request.ComputedField import org.genspectrum.lapis.request.DetailsFiltersRequest import org.genspectrum.lapis.request.MutationsField import org.genspectrum.lapis.request.Order import org.genspectrum.lapis.request.OrderByField import org.genspectrum.lapis.request.OrderBySpec import org.genspectrum.lapis.request.PlainField +import org.genspectrum.lapis.request.ScalarFunction import org.genspectrum.lapis.request.SequenceFiltersRequest import org.genspectrum.lapis.request.converter.CaseInsensitiveFieldsCleaner import org.genspectrum.lapis.request.toOrderBySpec @@ -529,4 +531,73 @@ class SiloQueryModelTest { ) } } + + @Test + fun `getAggregated splits plain and computed fields into groupByFields and computedFields`() { + every { siloClientMock.sendQuery(any>()) } returns Stream.empty() + every { siloFilterExpressionMapperMock.map(any()) } returns True + + underTest.getAggregated( + AggregatedFiltersRequest( + emptyMap(), + emptyList(), + emptyList(), + emptyList(), + emptyList(), + listOf( + PlainField("date"), + ComputedField("date", ScalarFunction.ISO_WEEK), + ), + OrderBySpec.EMPTY, + ), + ) + + verify { + siloClientMock.sendQuery( + SiloQuery( + SiloAction.aggregated( + groupByFields = listOf("date"), + computedFields = listOf(ComputedField("date", ScalarFunction.ISO_WEEK)), + ), + True, + ), + ) + } + } + + @Test + fun `getAggregated passes orderBy fields through unchanged for computed fields`() { + val isoWeekField = ComputedField("date", ScalarFunction.ISO_WEEK) + every { siloFilterExpressionMapperMock.map(any()) } returns True + every { siloClientMock.sendQuery(any>()) } returns Stream.empty() + + val orderByFields = OrderBySpec.ByFields( + listOf(OrderByField(field = isoWeekField.outputColumnName, order = Order.ASCENDING)), + ) + + underTest.getAggregated( + AggregatedFiltersRequest( + emptyMap(), + emptyList(), + emptyList(), + emptyList(), + emptyList(), + listOf(isoWeekField), + orderByFields, + ), + ) + + verify { + siloClientMock.sendQuery( + SiloQuery( + SiloAction.aggregated( + groupByFields = emptyList(), + computedFields = listOf(isoWeekField), + orderByFields = orderByFields, + ), + True, + ), + ) + } + } } diff --git a/lapis/src/test/kotlin/org/genspectrum/lapis/model/mutationsOverTime/Helpers.kt b/lapis/src/test/kotlin/org/genspectrum/lapis/model/mutationsOverTime/Helpers.kt index 094cbc83f..fbb45ee13 100644 --- a/lapis/src/test/kotlin/org/genspectrum/lapis/model/mutationsOverTime/Helpers.kt +++ b/lapis/src/test/kotlin/org/genspectrum/lapis/model/mutationsOverTime/Helpers.kt @@ -45,10 +45,10 @@ val DUMMY_DATE_BETWEEN_ALL = const val DUMMY_DATE_FIELD = "date" val AGGREGATED_SILO_ACTION = SiloAction.aggregated( - listOf(DUMMY_DATE_FIELD), - OrderBySpec.EMPTY, - null, - null, + groupByFields = listOf(DUMMY_DATE_FIELD), + orderByFields = OrderBySpec.EMPTY, + limit = null, + offset = null, ) fun mockSiloCallInfo( diff --git a/lapis/src/test/kotlin/org/genspectrum/lapis/request/ScalarFunctionFieldTest.kt b/lapis/src/test/kotlin/org/genspectrum/lapis/request/ScalarFunctionFieldTest.kt new file mode 100644 index 000000000..fd396d4a0 --- /dev/null +++ b/lapis/src/test/kotlin/org/genspectrum/lapis/request/ScalarFunctionFieldTest.kt @@ -0,0 +1,69 @@ +package org.genspectrum.lapis.request + +import org.genspectrum.lapis.config.DatabaseMetadata +import org.genspectrum.lapis.config.MetadataType +import org.genspectrum.lapis.controller.BadRequestException +import org.genspectrum.lapis.databaseConfig +import org.genspectrum.lapis.request.converter.CaseInsensitiveFieldsCleaner +import org.genspectrum.lapis.request.converter.ScalarFunctionFieldConverter +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows + +private val testDatabaseConfig = databaseConfig( + primaryKey = "accession", + metadata = listOf( + DatabaseMetadata("accession", MetadataType.STRING), + DatabaseMetadata("date", MetadataType.DATE), + DatabaseMetadata("country", MetadataType.STRING), + ), +) + +private val underTest = ScalarFunctionFieldConverter( + caseInsensitiveFieldsCleaner = CaseInsensitiveFieldsCleaner(testDatabaseConfig), + databaseConfig = testDatabaseConfig, +) + +class ScalarFunctionFieldTest { + @Test + fun `plain field name (no dot) returns null`() { + assertNull(underTest.tryConvert("date")) + } + + @Test + fun `date_isoWeek returns ComputedField`() { + val result = underTest.tryConvert("date.isoWeek") + assertEquals(ComputedField("date", ScalarFunction.ISO_WEEK), result) + assertEquals("date.isoWeek", result?.outputColumnName) + } + + @Test + fun `function name matching is case insensitive`() { + val result = underTest.tryConvert("date.ISOWEEK") + assertEquals(ComputedField("date", ScalarFunction.ISO_WEEK), result) + } + + @Test + fun `base field name in computed field is case insensitive`() { + val result = underTest.tryConvert("DATE.isoWeek") + assertEquals(ComputedField("date", ScalarFunction.ISO_WEEK), result) + } + + @Test + fun `unknown base field in computed syntax throws BadRequestException`() { + assertThrows { underTest.tryConvert("unknown.isoWeek") } + } + + @Test + fun `unknown function name throws BadRequestException`() { + assertThrows { underTest.tryConvert("date.unknownFunction") } + } + + @Test + fun `isoWeek on non-date field throws BadRequestException`() { + val ex = assertThrows { underTest.tryConvert("country.isoWeek") } + assertTrue(ex.message.orEmpty().contains("STRING"), "Expected error to mention type, got: ${ex.message}") + } +} diff --git a/lapis/src/test/kotlin/org/genspectrum/lapis/request/converter/AggregatedFieldConverterTest.kt b/lapis/src/test/kotlin/org/genspectrum/lapis/request/converter/AggregatedFieldConverterTest.kt index 336b35d1c..0260af2d1 100644 --- a/lapis/src/test/kotlin/org/genspectrum/lapis/request/converter/AggregatedFieldConverterTest.kt +++ b/lapis/src/test/kotlin/org/genspectrum/lapis/request/converter/AggregatedFieldConverterTest.kt @@ -13,6 +13,10 @@ import org.junit.jupiter.api.Test class AggregatedFieldConverterTest { @Test fun `convert resolves shorthand position syntax on a single-segmented genome`() { + val dbConfig = databaseConfig( + primaryKey = "primaryKey", + metadata = listOf(DatabaseMetadata(name = "primaryKey", type = MetadataType.STRING)), + ) val underTest = AggregatedFieldConverter( sequencePositionFieldConverter = SequencePositionFieldConverter( referenceGenomeSchema = ReferenceGenomeSchema( @@ -20,13 +24,12 @@ class AggregatedFieldConverterTest { genes = emptyList(), ), ), + scalarFunctionFieldConverter = ScalarFunctionFieldConverter( + caseInsensitiveFieldsCleaner = CaseInsensitiveFieldsCleaner(dbConfig), + databaseConfig = dbConfig, + ), metadataFieldConverter = MetadataFieldConverter( - caseInsensitiveFieldsCleaner = CaseInsensitiveFieldsCleaner( - databaseConfig( - primaryKey = "primaryKey", - metadata = listOf(DatabaseMetadata(name = "primaryKey", type = MetadataType.STRING)), - ), - ), + caseInsensitiveFieldsCleaner = CaseInsensitiveFieldsCleaner(dbConfig), ), ) diff --git a/lapis/src/test/kotlin/org/genspectrum/lapis/request/converter/OrderByFieldConverterTest.kt b/lapis/src/test/kotlin/org/genspectrum/lapis/request/converter/OrderByFieldConverterTest.kt index 1586e4952..de8eb54cd 100644 --- a/lapis/src/test/kotlin/org/genspectrum/lapis/request/converter/OrderByFieldConverterTest.kt +++ b/lapis/src/test/kotlin/org/genspectrum/lapis/request/converter/OrderByFieldConverterTest.kt @@ -51,4 +51,28 @@ class OrderByFieldConverterTest { assertThat(result.field, equalTo("country")) assertThat(result.order, equalTo(Order.ASCENDING)) } + + @Test + fun `GIVEN a computed field THEN converts to OrderByField with the same computed field`() { + val result = orderByFieldConverter.convert("date.isoWeek") + + assertThat(result.field, equalTo("date.isoWeek")) + assertThat(result.order, equalTo(Order.ASCENDING)) + } + + @Test + fun `GIVEN a differently-cased computed field THEN converts to the same canonical field as 'fields' would`() { + val result = orderByFieldConverter.convert("DATE.ISOWEEK") + + assertThat(result.field, equalTo("date.isoWeek")) + assertThat(result.order, equalTo(Order.ASCENDING)) + } + + @Test + fun `GIVEN 'count' THEN converts to OrderByField with field 'count' unchanged`() { + val result = orderByFieldConverter.convert("count") + + assertThat(result.field, equalTo("count")) + assertThat(result.order, equalTo(Order.ASCENDING)) + } } diff --git a/lapis/src/test/kotlin/org/genspectrum/lapis/silo/SiloQueryTest.kt b/lapis/src/test/kotlin/org/genspectrum/lapis/silo/SiloQueryTest.kt index 9cafebd0c..bdbbebf5c 100644 --- a/lapis/src/test/kotlin/org/genspectrum/lapis/silo/SiloQueryTest.kt +++ b/lapis/src/test/kotlin/org/genspectrum/lapis/silo/SiloQueryTest.kt @@ -79,13 +79,13 @@ class SiloQueryTest { ), Arguments.of( SiloAction.aggregated( - listOf("field1", "field2"), - listOf( + groupByFields = listOf("field1", "field2"), + orderByFields = listOf( OrderByField("field3", Order.ASCENDING), OrderByField("field4", Order.DESCENDING), ).toOrderBySpec(), - 100, - 50, + limit = 100, + offset = 50, ), """ { diff --git a/lapis/src/test/kotlin/org/genspectrum/lapis/silo/SiloQueryToSaneQlTest.kt b/lapis/src/test/kotlin/org/genspectrum/lapis/silo/SiloQueryToSaneQlTest.kt index b9f671015..12e90885f 100644 --- a/lapis/src/test/kotlin/org/genspectrum/lapis/silo/SiloQueryToSaneQlTest.kt +++ b/lapis/src/test/kotlin/org/genspectrum/lapis/silo/SiloQueryToSaneQlTest.kt @@ -1,8 +1,10 @@ package org.genspectrum.lapis.silo +import org.genspectrum.lapis.request.ComputedField import org.genspectrum.lapis.request.Order import org.genspectrum.lapis.request.OrderByField import org.genspectrum.lapis.request.OrderBySpec +import org.genspectrum.lapis.request.ScalarFunction import org.genspectrum.lapis.request.SequencePositionField import org.genspectrum.lapis.request.toOrderBySpec import org.hamcrest.MatcherAssert.assertThat @@ -80,8 +82,8 @@ class SiloQueryToSaneQlTest { fun `GIVEN orderBy field with injection attempt THEN payload is quoted as identifier`() { val query = SiloQuery( SiloAction.aggregated( - listOf("country"), - listOf( + groupByFields = listOf("country"), + orderByFields = listOf( OrderByField("count}).filter(true).groupBy({evil:=count()", Order.ASCENDING), ).toOrderBySpec(), ), @@ -114,13 +116,20 @@ class SiloQueryToSaneQlTest { ), Arguments.of( SiloAction.aggregated( - listOf("field1", "field2"), - listOf( + groupByFields = listOf("country"), + computedFields = listOf(ComputedField("date", ScalarFunction.ISO_WEEK)), + ), + """.map({"date.isoWeek":="date".isoWeek()}).groupBy({"count":=count()}, {"country", "date.isoWeek"})""", + ), + Arguments.of( + SiloAction.aggregated( + groupByFields = listOf("field1", "field2"), + orderByFields = listOf( OrderByField("field3", Order.ASCENDING), OrderByField("field4", Order.DESCENDING), ).toOrderBySpec(), - 100, - 50, + limit = 100, + offset = 50, ), """.groupBy({"count":=count()}, {"field1", "field2"}).orderBy({"field3", "field4".desc()}).offset(50).limit(100)""", ),