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

[Backport 1.15] fix: filter expression with numeric function #516

Merged
merged 4 commits into from
Sep 20, 2022
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
Original file line number Diff line number Diff line change
Expand Up @@ -935,15 +935,37 @@ class FeelInterpreter {
private def filterList(list: List[Val], filter: Val => Val): Val = {
val conditionNotFulfilled = ValString("_")

mapEither[Val, Val](
val withBooleanFilter = (list: List[Val]) => mapEither[Val, Val](
list,
item =>
withBoolean(filter(item), {
case true => item
case true => item
case false => conditionNotFulfilled
}).toEither,
items => ValList(items.filterNot(_ == conditionNotFulfilled))
)

// The filter function could return a boolean or a number. If it returns a number then we use
// the number as the index for the list. Otherwise, the boolean function determine if the
// condition is fulfilled for the given item.
// Note that the code could look more elegant but we want to avoid unintended invocations of
// the function because the invocations could be observed by the function provider (see #359).
list.headOption.map(head =>
withVal(filter(head), {
case ValNumber(index) => filterList(list, index)
case ValBoolean(isFulFilled) => withBooleanFilter(list.tail) match {
case ValList(fulFilledItems) if isFulFilled => ValList(head :: fulFilledItems)
case fulFilledItems: ValList => fulFilledItems
case error => error
}
case other => ValError(s"Expected boolean filter or number but found '$other'")
})
).getOrElse(
// Return always an empty list if the given list is empty. Note that we would return `null`
// instead, if the filter is a number. But if it is a function, we would need to evaluate the
// function first to see that it returns a number.
ValList(List.empty)
)
}

private def filterList(list: List[Val], index: Number): Val = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,13 @@ class InterpreterListExpressionTest
ValList(List(ValNumber(2), ValNumber(4))))
}

it should "be filtered via custom function" in {
it should "be filtered via numeric function" in {
eval("[1,2,3,4][abs(1)]") should be(ValNumber(1))

eval("[1,2,3,4][modulo(2,4)]") should be(ValNumber(2))
}

it should "be filtered via custom boolean function" in {
val functionInvocations: ListBuffer[Val] = ListBuffer.empty

val result = eval(
Expand All @@ -156,6 +162,28 @@ class InterpreterListExpressionTest
)
}

it should "be filtered via custom numeric function" in {
val functionInvocations: ListBuffer[Val] = ListBuffer.empty

val result = eval(
expression = "[1,2,3,4][f(item)]",
variables = Map(),
functions = Map("f" -> ValFunction(
params = List("x"),
invoke = {
case List(x) =>
functionInvocations += x
ValNumber(3)
}
)))

result should be(ValNumber(3))

functionInvocations should be(List(
ValNumber(1))
)
}

it should "be filtered multiple times (from literal)" in {
eval("[[1]][1][1]") should be(ValNumber(1))
eval("[[[1]]][1][1][1]") should be(ValNumber(1))
Expand Down Expand Up @@ -206,6 +234,18 @@ class InterpreterListExpressionTest
ValNumber(1))
}

it should "fail if the filter doesn't return a boolean or a number" in {
eval(""" [1,2,3,4]["not a valid filter"] """) should be (
ValError("Expected boolean filter or number but found 'ValString(not a valid filter)'")
)
}

it should "fail if the filter doesn't return always a boolean" in {
eval("[1,2,3,4][if item < 3 then true else null]") should be (
ValError("expected Boolean but found 'ValNull'")
)
}

it should "fail if one element fails" in {

eval("[1, {}.x]") should be(
Expand Down