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

fix: Detect duplicated context values in distinct values() + union() + duplicate values() #908

Merged
merged 11 commits into from
Sep 3, 2024
2 changes: 1 addition & 1 deletion src/main/scala/org/camunda/feel/FeelEngine.scala
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ class FeelEngine(
val clock: FeelEngineClock = FeelEngine.defaultClock
) {

private val interpreter = new FeelInterpreter()
private val interpreter = new FeelInterpreter(valueMapper)

private val validator = new ExpressionValidator(
externalFunctionsEnabled = configuration.externalFunctionsEnabled
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package org.camunda.feel.impl.builtin

import org.camunda.feel.impl.builtin.BuiltinFunction.builtinFunction
import org.camunda.feel.Number
import org.camunda.feel.impl.interpreter.ValComparator
import org.camunda.feel.syntaxtree.{
Val,
ValBoolean,
Expand All @@ -28,10 +29,13 @@ import org.camunda.feel.syntaxtree.{
ValNumber,
ValString
}
import org.camunda.feel.valuemapper.ValueMapper

import scala.annotation.tailrec

object ListBuiltinFunctions {
class ListBuiltinFunctions(private val valueMapper: ValueMapper) {

private val valueComparator = new ValComparator(valueMapper)

def functions = Map(
"list contains" -> List(listContainsFunction),
Expand Down Expand Up @@ -375,32 +379,40 @@ object ListBuiltinFunctions {
private def unionFunction = builtinFunction(
params = List("lists"),
invoke = { case List(ValList(lists)) =>
ValList(
lists
.flatMap(_ match {
case ValList(list) => list
case v => List(v)
})
.toList
.distinct
)
val listOfLists = lists.flatMap {
case ValList(list) => list
case v => List(v)
}
ValList(distinct(listOfLists))
},
hasVarArgs = true
)

private def distinctValuesFunction =
builtinFunction(
params = List("list"),
invoke = { case List(ValList(list)) =>
ValList(list.distinct)
invoke = { case List(ValList(list)) => ValList(distinct(list)) }
)

private def distinct(list: List[Val]): List[Val] = {
list.foldLeft(List[Val]())((result, item) =>
if (result.exists(y => valueComparator.equals(item, y))) {
// duplicate value
result
} else {
result :+ item
}
)
}

private def duplicateValuesFunction =
builtinFunction(
params = List("list"),
invoke = { case List(ValList(list)) =>
ValList(list.distinct.filter(x => list.count(_ == x) > 1))
val duplicatedValues =
distinct(list).filter(x => list.count(valueComparator.equals(_, x)) > 1)

ValList(duplicatedValues)
}
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class BuiltinFunctions(clock: FeelEngineClock, valueMapper: ValueMapper) extends
new ConversionBuiltinFunctions(valueMapper).functions ++
BooleanBuiltinFunctions.functions ++
StringBuiltinFunctions.functions ++
ListBuiltinFunctions.functions ++
new ListBuiltinFunctions(valueMapper).functions ++
NumericBuiltinFunctions.functions ++
new ContextBuiltinFunctions(valueMapper).functions ++
RangeBuiltinFunction.functions ++
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ import scala.reflect.ClassTag
/** @author
* Philipp Ossler
*/
class FeelInterpreter {
class FeelInterpreter(private val valueMapper: ValueMapper) {

private val valueComparator = new ValComparator(valueMapper)

def eval(expression: Exp)(implicit context: EvalContext): Val = {
// Check if the current thread was interrupted, otherwise long-running evaluations can not be interrupted and fully block the thread
Expand Down Expand Up @@ -80,7 +82,7 @@ class FeelInterpreter {

// simple unary tests
case InputEqualTo(x) =>
withVal(getImplicitInputValue, i => checkEquality(i, eval(x), _ == _, ValBoolean))
withVal(getImplicitInputValue, i => checkEquality(i, eval(x)))
case InputLessThan(x) =>
withVal(getImplicitInputValue, i => dualOp(i, eval(x), _ < _, ValBoolean))
case InputLessOrEqual(x) =>
Expand Down Expand Up @@ -118,7 +120,7 @@ class FeelInterpreter {
withValOrNull(withNumber(eval(x), x => ValNumber(-x)))

// dual comparators
case Equal(x, y) => checkEquality(eval(x), eval(y), _ == _, ValBoolean)
case Equal(x, y) => checkEquality(eval(x), eval(y))
case LessThan(x, y) => dualOp(eval(x), eval(y), _ < _, ValBoolean)
case LessOrEqual(x, y) => dualOp(eval(x), eval(y), _ <= _, ValBoolean)
case GreaterThan(x, y) => dualOp(eval(x), eval(y), _ > _, ValBoolean)
Expand Down Expand Up @@ -513,65 +515,15 @@ class FeelInterpreter {
}
}

private def checkEquality(x: Val, y: Val, c: (Any, Any) => Boolean, f: Boolean => Val)(implicit
context: EvalContext
): Val =
private def checkEquality(x: Val, y: Val)(implicit context: EvalContext): Val =
withValues(
x,
y,
{
case (ValNull, _) => f(c(ValNull, y.toOption.getOrElse(ValNull)))
case (_, ValNull) => f(c(x.toOption.getOrElse(ValNull), ValNull))
case (ValNumber(x), ValNumber(y)) => f(c(x, y))
case (ValBoolean(x), ValBoolean(y)) => f(c(x, y))
case (ValString(x), ValString(y)) => f(c(x, y))
case (ValDate(x), ValDate(y)) => f(c(x, y))
case (ValLocalTime(x), ValLocalTime(y)) => f(c(x, y))
case (ValTime(x), ValTime(y)) => f(c(x, y))
case (ValLocalDateTime(x), ValLocalDateTime(y)) => f(c(x, y))
case (ValDateTime(x), ValDateTime(y)) => f(c(x, y))
case (ValYearMonthDuration(x), ValYearMonthDuration(y)) => f(c(x, y))
case (ValDayTimeDuration(x), ValDayTimeDuration(y)) => f(c(x, y))
case (ValList(x), ValList(y)) =>
if (x.size != y.size) {
f(false)

} else {
val isEqual = x.zip(y).foldRight(true) { case ((x, y), listIsEqual) =>
listIsEqual && {
checkEquality(x, y, c, f) match {
case ValBoolean(itemIsEqual) => itemIsEqual
case _ => false
}
}
}
f(isEqual)
}
case (ValContext(x), ValContext(y)) =>
val xVars = x.variableProvider.getVariables
val yVars = y.variableProvider.getVariables

if (xVars.keys != yVars.keys) {
f(false)

} else {
val isEqual = xVars.keys.foldRight(true) { case (key, contextIsEqual) =>
contextIsEqual && {
val xVal = context.valueMapper.toVal(xVars(key))
val yVal = context.valueMapper.toVal(yVars(key))

checkEquality(xVal, yVal, c, f) match {
case ValBoolean(entryIsEqual) => entryIsEqual
case _ => false
}
}
}
f(isEqual)
}
case _ =>
(x, y) =>
valueComparator.compare(x, y).toOption.getOrElse {
error(EvaluationFailureType.NOT_COMPARABLE, s"Can't compare '$x' with '$y'")
ValNull
}
}
)

private def dualOp(x: Val, y: Val, c: (Val, Val) => Boolean, f: Boolean => Val)(implicit
Expand Down Expand Up @@ -715,7 +667,7 @@ class FeelInterpreter {
// the expression contains the input value
ValBoolean(true)
case x =>
checkEquality(inputValue, x, _ == _, ValBoolean) match {
checkEquality(inputValue, x) match {
case ValBoolean(true) =>
// the expression is the input value
ValBoolean(true)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
* Copyright Camunda Services GmbH and/or licensed to Camunda Services GmbH
* under one or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information regarding copyright
* ownership. Camunda licenses this file to you under the Apache License,
* Version 2.0; you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.camunda.feel.impl.interpreter

import org.camunda.feel.context.Context
import org.camunda.feel.syntaxtree._
import org.camunda.feel.valuemapper.ValueMapper

class ValComparator(private val valueMapper: ValueMapper) {

def equals(x: Val, y: Val): Boolean = compare(x, y) match {
case ValBoolean(isEqual) => isEqual
case _ => false
}

def compare(x: Val, y: Val): Val = (x, y) match {
// both values are null
case (ValNull, _) => ValBoolean(ValNull == y.toOption.getOrElse(ValNull))
case (_, ValNull) => ValBoolean(x.toOption.getOrElse(ValNull) == ValNull)
saig0 marked this conversation as resolved.
Show resolved Hide resolved
// compare values of the same type
case (ValNumber(x), ValNumber(y)) => ValBoolean(x == y)
case (ValBoolean(x), ValBoolean(y)) => ValBoolean(x == y)
case (ValString(x), ValString(y)) => ValBoolean(x == y)
case (ValDate(x), ValDate(y)) => ValBoolean(x == y)
case (ValLocalTime(x), ValLocalTime(y)) => ValBoolean(x == y)
case (ValTime(x), ValTime(y)) => ValBoolean(x == y)
case (ValLocalDateTime(x), ValLocalDateTime(y)) => ValBoolean(x == y)
case (ValDateTime(x), ValDateTime(y)) => ValBoolean(x == y)
case (ValYearMonthDuration(x), ValYearMonthDuration(y)) => ValBoolean(x == y)
case (ValDayTimeDuration(x), ValDayTimeDuration(y)) => ValBoolean(x == y)
case (ValList(x), ValList(y)) => compare(x, y)
case (ValContext(x), ValContext(y)) => compare(x, y)
// values have a different type
case _ => ValError(s"Can't compare '$x' with '$y'")
}

private def compare(x: List[Val], y: List[Val]): ValBoolean = {
if (x.size != y.size) {
ValBoolean(false)

} else {
val itemsAreEqual = x.zip(y).foldRight(true) { case ((x, y), listIsEqual) =>
saig0 marked this conversation as resolved.
Show resolved Hide resolved
listIsEqual && {
compare(x, y) match {
case ValBoolean(itemIsEqual) => itemIsEqual
case _ => false
}
}
}
ValBoolean(itemsAreEqual)
}
}

private def compare(x: Context, y: Context): ValBoolean = {
saig0 marked this conversation as resolved.
Show resolved Hide resolved
val xVars = x.variableProvider.getVariables
val yVars = y.variableProvider.getVariables

if (xVars.keys != yVars.keys) {
ValBoolean(false)

} else {
val itemsAreEqual = xVars.keys.foldRight(true) { case (key, contextIsEqual) =>
contextIsEqual && {
val xVal = valueMapper.toVal(xVars(key))
val yVal = valueMapper.toVal(yVars(key))

compare(xVal, yVal) match {
case ValBoolean(entryIsEqual) => entryIsEqual
case _ => false
}
}
}
ValBoolean(itemsAreEqual)
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import org.camunda.feel.{
trait FeelIntegrationTest {

val interpreter: FeelInterpreter =
new FeelInterpreter
new FeelInterpreter(ValueMapper.defaultValueMapper)

private val clock: TimeTravelClock = new TimeTravelClock

Expand Down
Loading