camunda / feel-scala

FEEL parser and interpreter written in Scala
https://camunda.github.io/feel-scala/
Apache License 2.0
119 stars 46 forks source link

The `string()` function doesn't work for a context with custom value types #804

Closed saig0 closed 2 months ago

saig0 commented 4 months ago

Describe the bug

The string() function doesn't return the expected string representation from a context value if it contains custom value types.

The problem doesn't occur with standard Java/Scala types but with custom types. See the issue here on how to reproduce the issue in Camunda 8.

string(myContextVar)     
// --> "{key1:UnsafeBuffer{addressOffset=16, capacity=12, byteArray=byte[12], byteBuffer=null}}"

To Reproduce Steps to reproduce the behavior:

  1. Run the following unit test
  2. Verify that the test fails on the second case
case class CustomValue(value: Int)

  class MyCustomValueMapper extends CustomValueMapper {
    def toVal(x: Any, innerValueMapper: Any => Val): Option[Val] = x match {
      case CustomValue(value) => Some(ValNumber(value))
      case _              => None
    }

    override def unpackVal(value: Val, innerValueMapper: Val => Any): Option[Any] = None
  }

  it should "convert a custom context" in {

    val engine = FeelEngineBuilder()
      .withCustomValueMapper(new MyCustomValueMapper())
      .build()

    engine.evaluateExpression(
      expression =  " string(context) ",
      variables = Map("context" -> Map("a" -> CustomValue(1)))
    ) should returnResult("{a:1}") // --> works

    engine.evaluateExpression(
      expression =  " string(context) ",
      variables = Map("context" -> ValContext(new MyCustomContext(Map("a" -> CustomValue(1)))))
    ) should returnResult("{a:1}") // --> fails with "the evaluation didn't returned '{a:1}' but '{a:CustomValue(1)}'"
  }

Expected behavior The string() function returns the string representation for custom value types. It uses the configured value mapper of the engine to transform the custom types.

Environment

mustafadagher commented 3 months ago

NVM, that was a problem with my intellij runner, it ran another test after adding the lines.

@saig0 I was trying the test case attached in the description for curiosity, the problem in the above test case lies in the test setup.

in the MyCustomValueMapper provided the method unpackVal resolves to None

while if you change it a tiny bit to match the implementation of DefaultValueMapper as follows, The test will succeed.

override def unpackVal(value: Val, innerValueMapper: Val => Any): Option[Any] = value match {
  case ValContext(c: Context) =>
    Some(
      c.variableProvider.getVariables.map { case (key, value) =>
        value match {
          case packed: Val => key -> innerValueMapper(packed)
          case unpacked => key -> unpacked
        }
      }
    )
  case _ => None
}