Skip to content

Commit 03e6c9f

Browse files
saig0saig0
authored
Merge pull request #918 from camunda/771-backport_1.16
[Backport 1.16] fix: Detect duplicated context values in `distinct values()` + `union()`
2 parents 7cab1f5 + 8f79978 commit 03e6c9f

File tree

7 files changed

+330
-190
lines changed

7 files changed

+330
-190
lines changed

‎src/main/scala/org/camunda/feel/FeelEngine.scala‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ class FeelEngine(
107107
val clock: FeelEngineClock = FeelEngine.defaultClock
108108
) {
109109

110-
val interpreter = new FeelInterpreter()
110+
val interpreter = new FeelInterpreter(valueMapper)
111111

112112
val validator = new ExpressionValidator(
113113
externalFunctionsEnabled = configuration.externalFunctionsEnabled

‎src/main/scala/org/camunda/feel/impl/builtin/ListBuiltinFunctions.scala‎

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package org.camunda.feel.impl.builtin
1818

1919
import org.camunda.feel.impl.builtin.BuiltinFunction.builtinFunction
2020
import org.camunda.feel.{Number, logger}
21+
import org.camunda.feel.impl.interpreter.ValComparator
2122
import org.camunda.feel.syntaxtree.{
2223
Val,
2324
ValBoolean,
@@ -28,10 +29,13 @@ import org.camunda.feel.syntaxtree.{
2829
ValNumber,
2930
ValString
3031
}
32+
import org.camunda.feel.valuemapper.ValueMapper
3133

3234
import scala.annotation.tailrec
3335

34-
object ListBuiltinFunctions {
36+
class ListBuiltinFunctions(private val valueMapper: ValueMapper) {
37+
38+
private val valueComparator = new ValComparator(valueMapper)
3539

3640
def functions = Map(
3741
"list contains" -> List(listContainsFunction),
@@ -373,26 +377,31 @@ object ListBuiltinFunctions {
373377
private def unionFunction = builtinFunction(
374378
params = List("lists"),
375379
invoke = { case List(ValList(lists)) =>
376-
ValList(
377-
lists
378-
.flatMap(_ match {
379-
case ValList(list) => list
380-
case v => List(v)
381-
})
382-
.toList
383-
.distinct
384-
)
380+
val listOfLists = lists.flatMap {
381+
case ValList(list) => list
382+
case v => List(v)
383+
}
384+
ValList(distinct(listOfLists))
385385
},
386386
hasVarArgs = true
387387
)
388388

389389
private def distinctValuesFunction =
390390
builtinFunction(
391391
params = List("list"),
392-
invoke = { case List(ValList(list)) =>
393-
ValList(list.distinct)
392+
invoke = { case List(ValList(list)) => ValList(distinct(list)) }
393+
)
394+
395+
private def distinct(list: List[Val]): List[Val] = {
396+
list.foldLeft(List[Val]())((result, item) =>
397+
if (result.exists(y => valueComparator.equals(item, y))) {
398+
// duplicate value
399+
result
400+
} else {
401+
result :+ item
394402
}
395403
)
404+
}
396405

397406
private def flattenFunction =
398407
builtinFunction(

‎src/main/scala/org/camunda/feel/impl/interpreter/BuiltinFunctions.scala‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ class BuiltinFunctions(clock: FeelEngineClock, valueMapper: ValueMapper) extends
4242
ConversionBuiltinFunctions.functions ++
4343
BooleanBuiltinFunctions.functions ++
4444
StringBuiltinFunctions.functions ++
45-
ListBuiltinFunctions.functions ++
45+
new ListBuiltinFunctions(valueMapper).functions ++
4646
NumericBuiltinFunctions.functions ++
4747
new ContextBuiltinFunctions(valueMapper).functions ++
4848
RangeBuiltinFunction.functions ++

‎src/main/scala/org/camunda/feel/impl/interpreter/FeelInterpreter.scala‎

Lines changed: 12 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@ import scala.reflect.ClassTag
2929
/** @author
3030
* Philipp Ossler
3131
*/
32-
class FeelInterpreter {
32+
class FeelInterpreter(private val valueMapper: ValueMapper) {
33+
34+
private val valueComparator = new ValComparator(valueMapper)
3335

3436
def eval(expression: Exp)(implicit context: EvalContext): Val =
3537
expression match {
@@ -65,7 +67,7 @@ class FeelInterpreter {
6567

6668
// simple unary tests
6769
case InputEqualTo(x) =>
68-
withVal(getImplicitInputValue, i => checkEquality(i, eval(x), _ == _, ValBoolean))
70+
withVal(getImplicitInputValue, i => checkEquality(i, eval(x)))
6971
case InputLessThan(x) =>
7072
withVal(getImplicitInputValue, i => dualOp(i, eval(x), _ < _, ValBoolean))
7173
case InputLessOrEqual(x) =>
@@ -103,7 +105,7 @@ class FeelInterpreter {
103105
withValOrNull(withNumber(eval(x), x => ValNumber(-x)))
104106

105107
// dual comparators
106-
case Equal(x, y) => checkEquality(eval(x), eval(y), _ == _, ValBoolean)
108+
case Equal(x, y) => checkEquality(eval(x), eval(y))
107109
case LessThan(x, y) => dualOp(eval(x), eval(y), _ < _, ValBoolean)
108110
case LessOrEqual(x, y) => dualOp(eval(x), eval(y), _ <= _, ValBoolean)
109111
case GreaterThan(x, y) => dualOp(eval(x), eval(y), _ > _, ValBoolean)
@@ -464,69 +466,20 @@ class FeelInterpreter {
464466
}
465467
}
466468

467-
private def checkEquality(x: Val, y: Val, c: (Any, Any) => Boolean, f: Boolean => Val)(implicit
468-
context: EvalContext
469-
): Val = {
469+
private def checkEquality(x: Val, y: Val)(implicit context: EvalContext): Val = {
470470
(x, y) match {
471471
case (fatalError: ValFatalError, _) => fatalError
472472
case (_, fatalError: ValFatalError) => fatalError
473-
case (ValNull, _) => f(c(ValNull, y.toOption.getOrElse(ValNull)))
474-
case (_, ValNull) => f(c(x.toOption.getOrElse(ValNull), ValNull))
473+
case (ValNull, _) => ValBoolean(ValNull == y.toOption.getOrElse(ValNull))
474+
case (_, ValNull) => ValBoolean(x.toOption.getOrElse(ValNull) == ValNull)
475475
case _ =>
476476
withValues(
477477
x,
478478
y,
479-
{
480-
case (ValNull, _) => f(c(ValNull, y.toOption.getOrElse(ValNull)))
481-
case (_, ValNull) => f(c(x.toOption.getOrElse(ValNull), ValNull))
482-
case (ValNumber(x), ValNumber(y)) => f(c(x, y))
483-
case (ValBoolean(x), ValBoolean(y)) => f(c(x, y))
484-
case (ValString(x), ValString(y)) => f(c(x, y))
485-
case (ValDate(x), ValDate(y)) => f(c(x, y))
486-
case (ValLocalTime(x), ValLocalTime(y)) => f(c(x, y))
487-
case (ValTime(x), ValTime(y)) => f(c(x, y))
488-
case (ValLocalDateTime(x), ValLocalDateTime(y)) => f(c(x, y))
489-
case (ValDateTime(x), ValDateTime(y)) => f(c(x, y))
490-
case (ValYearMonthDuration(x), ValYearMonthDuration(y)) => f(c(x, y))
491-
case (ValDayTimeDuration(x), ValDayTimeDuration(y)) => f(c(x, y))
492-
case (ValList(x), ValList(y)) =>
493-
if (x.size != y.size) {
494-
f(false)
495-
496-
} else {
497-
val isEqual = x.zip(y).foldRight(true) { case ((x, y), listIsEqual) =>
498-
listIsEqual && {
499-
checkEquality(x, y, c, f) match {
500-
case ValBoolean(itemIsEqual) => itemIsEqual
501-
case _ => false
502-
}
503-
}
504-
}
505-
f(isEqual)
506-
}
507-
case (ValContext(x), ValContext(y)) =>
508-
val xVars = x.variableProvider.getVariables
509-
val yVars = y.variableProvider.getVariables
510-
511-
if (xVars.keys != yVars.keys) {
512-
f(false)
513-
514-
} else {
515-
val isEqual = xVars.keys.foldRight(true) { case (key, contextIsEqual) =>
516-
contextIsEqual && {
517-
val xVal = context.valueMapper.toVal(xVars(key))
518-
val yVal = context.valueMapper.toVal(yVars(key))
519-
520-
checkEquality(xVal, yVal, c, f) match {
521-
case ValBoolean(entryIsEqual) => entryIsEqual
522-
case _ => false
523-
}
524-
}
525-
}
526-
f(isEqual)
527-
}
528-
case _ =>
479+
{ (x, y) =>
480+
valueComparator.compare(x, y).toOption.getOrElse {
529481
error(x, s"Can't compare '$x' with '$y'")
482+
}
530483
}
531484
)
532485
}
@@ -683,7 +636,7 @@ class FeelInterpreter {
683636
// the expression contains the input value
684637
ValBoolean(true)
685638
case x =>
686-
checkEquality(inputValue, x, _ == _, ValBoolean) match {
639+
checkEquality(inputValue, x) match {
687640
case ValBoolean(true) =>
688641
// the expression is the input value
689642
ValBoolean(true)

‎src/main/scala/org/camunda/feel/impl/interpreter/ValComparator.scala‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/*
2+
* Copyright Camunda Services GmbH and/or licensed to Camunda Services GmbH
3+
* under one or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information regarding copyright
5+
* ownership. Camunda licenses this file to you under the Apache License,
6+
* Version 2.0; you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.camunda.feel.impl.interpreter
18+
19+
import org.camunda.feel.context.Context
20+
import org.camunda.feel.syntaxtree._
21+
import org.camunda.feel.valuemapper.ValueMapper
22+
23+
class ValComparator(private val valueMapper: ValueMapper) {
24+
25+
def equals(x: Val, y: Val): Boolean = compare(x, y) match {
26+
case ValBoolean(isEqual) => isEqual
27+
case _ => false
28+
}
29+
30+
def compare(x: Val, y: Val): Val = (x, y) match {
31+
// both values are null
32+
case (ValNull, _) => ValBoolean(ValNull == y.toOption.getOrElse(ValNull))
33+
case (_, ValNull) => ValBoolean(x.toOption.getOrElse(ValNull) == ValNull)
34+
// compare values of the same type
35+
case (ValNumber(x), ValNumber(y)) => ValBoolean(x == y)
36+
case (ValBoolean(x), ValBoolean(y)) => ValBoolean(x == y)
37+
case (ValString(x), ValString(y)) => ValBoolean(x == y)
38+
case (ValDate(x), ValDate(y)) => ValBoolean(x == y)
39+
case (ValLocalTime(x), ValLocalTime(y)) => ValBoolean(x == y)
40+
case (ValTime(x), ValTime(y)) => ValBoolean(x == y)
41+
case (ValLocalDateTime(x), ValLocalDateTime(y)) => ValBoolean(x == y)
42+
case (ValDateTime(x), ValDateTime(y)) => ValBoolean(x == y)
43+
case (ValYearMonthDuration(x), ValYearMonthDuration(y)) => ValBoolean(x == y)
44+
case (ValDayTimeDuration(x), ValDayTimeDuration(y)) => ValBoolean(x == y)
45+
case (ValList(x), ValList(y)) => compare(x, y)
46+
case (ValContext(x), ValContext(y)) => compare(x, y)
47+
// values have a different type
48+
case _ => ValError(s"Can't compare '$x' with '$y'")
49+
}
50+
51+
private def compare(x: List[Val], y: List[Val]): ValBoolean = {
52+
ValBoolean(
53+
x.size == y.size && x.zip(y).forall { case (itemX, itemY) => equals(itemX, itemY) }
54+
)
55+
}
56+
57+
private def compare(x: Context, y: Context): ValBoolean = {
58+
val xVars = x.variableProvider.getVariables
59+
val yVars = y.variableProvider.getVariables
60+
61+
ValBoolean(xVars.keys == yVars.keys && xVars.keys.forall { key =>
62+
val xVal = valueMapper.toVal(xVars(key))
63+
val yVal = valueMapper.toVal(yVars(key))
64+
65+
equals(xVal, yVal)
66+
})
67+
}
68+
69+
}

‎src/test/scala/org/camunda/feel/impl/FeelIntegrationTest.scala‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ import org.camunda.feel.{
3737
trait FeelIntegrationTest {
3838

3939
val interpreter: FeelInterpreter =
40-
new FeelInterpreter
40+
new FeelInterpreter(ValueMapper.defaultValueMapper)
4141

4242
private val clock: TimeTravelClock = new TimeTravelClock
4343

0 commit comments

Comments
 (0)