Skip to content

Commit

Permalink
Revert "Revert "[NU-1935] Change record indexer behaviour (#7443)""
Browse files Browse the repository at this point in the history
This reverts commit 470be6e.
  • Loading branch information
Pawel Czajka committed Jan 17, 2025
1 parent 470be6e commit 033a652
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 8 deletions.
1 change: 1 addition & 0 deletions docs/Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
* [#7364](https://github.com/TouK/nussknacker/pull/7364) PeriodicDeploymentManger is no longer a separate DM, but instead is an optional functionality and decorator for all DMs
* in order to use it, DM must implement interface `schedulingSupported`, that handles deployments on a specific engine
* implementation provided for Flink DM
* [#7443](https://github.com/TouK/nussknacker/pull/7443) Indexing on record is more similar to indexing on map. The change lets us access record values dynamically. For example now spel expression "{a: 5, b: 10}[#input.field]" compiles and has type "Integer" inferred from types of values of the record. This lets us access record value based on user input, for instance if user passes "{"field": "b"}" to scenario we will get value "10", whereas input {"field": "c"} would result in "null". Expression "{a: 5}["b"]" still does not compile because it is known at compile time that record does not have property "b".

## 1.18

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ object SpelExpressionParseError {
override def message: String = s"There is no property '$property' in type: ${typ.display}"
}

case class NoPropertyTypeError(typ: TypingResult, propertyType: TypingResult) extends MissingObjectError {
override def message: String = s"There is no property of type '${propertyType.display}' in type: ${typ.display}"
}

case class UnknownMethodError(methodName: String, displayableType: String) extends MissingObjectError {
override def message: String = s"Unknown method '$methodName' in $displayableType"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import pl.touk.nussknacker.engine.spel.SpelExpressionParseError.IllegalOperation
import pl.touk.nussknacker.engine.spel.SpelExpressionParseError.MissingObjectError.{
ConstructionOfUnknown,
NoPropertyError,
NoPropertyTypeError,
NonReferenceError,
UnresolvedReferenceError
}
Expand Down Expand Up @@ -199,11 +200,16 @@ private[spel] class Typer(
case _ => typeFieldNameReferenceOnRecord(indexString, record)
}
case indexKey :: Nil if indexKey.canBeConvertedTo(Typed[String]) =>
if (dynamicPropertyAccessAllowed) valid(Unknown) else invalid(DynamicPropertyAccessError)
case _ :: Nil =>
if (dynamicPropertyAccessAllowed) valid(Unknown)
else
record.runtimeObjType.params match {
case _ :: value :: Nil if record.runtimeObjType.klass == classOf[java.util.Map[_, _]] => valid(value)
case _ => valid(Unknown)
}
case e :: Nil =>
indexer.children match {
case (ref: PropertyOrFieldReference) :: Nil => typeFieldNameReferenceOnRecord(ref.getName, record)
case _ => if (dynamicPropertyAccessAllowed) valid(Unknown) else invalid(DynamicPropertyAccessError)
case _ => if (dynamicPropertyAccessAllowed) valid(Unknown) else invalid(NoPropertyTypeError(record, e))
}
case _ =>
invalid(IllegalIndexingOperation)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,15 @@ class SpelExpressionSpec extends AnyFunSuite with Matchers with ValidatedValuesD
private def evaluate[T: TypeTag](expr: String, context: Context = ctx): T =
parse[T](expr = expr, context = context).validExpression.evaluateSync[T](context)

test("should be able to dynamically index record") {
evaluate[Int]("{a: 5, b: 10}[#input.toString()]", Context("abc").withVariable("input", "a")) shouldBe 5
evaluate[Integer]("{a: 5, b: 10}[#input.toString()]", Context("abc").withVariable("input", "asdf")) shouldBe null
}

test("should figure out result type when dynamically indexing record") {
evaluate[Int]("{a: {g: 5, h: 10}, b: {g: 50, h: 100}}[#input.toString()].h", Context("abc").withVariable("input", "b")) shouldBe 100
}

test("parsing first selection on array") {
parse[Any]("{1,2,3,4,5,6,7,8,9,10}.^[(#this%2==0)]").validExpression
.evaluateSync[java.util.ArrayList[Int]](ctx) should equal(2)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ import pl.touk.nussknacker.engine.api.typed.typing._
import pl.touk.nussknacker.engine.definition.clazz.ClassDefinitionTestUtils
import pl.touk.nussknacker.engine.dict.{KeysDictTyper, SimpleDictRegistry}
import pl.touk.nussknacker.engine.expression.PositionRange
import pl.touk.nussknacker.engine.spel.SpelExpressionParseError.IllegalOperationError.DynamicPropertyAccessError
import pl.touk.nussknacker.engine.spel.SpelExpressionParseError.MissingObjectError.NoPropertyError
import pl.touk.nussknacker.engine.spel.SpelExpressionParseError.MissingObjectError.{
NoPropertyError,
NoPropertyTypeError
}
import pl.touk.nussknacker.engine.spel.SpelExpressionParseError.UnsupportedOperationError.MapWithExpressionKeysError
import pl.touk.nussknacker.engine.spel.Typer.TypingResultWithContext
import pl.touk.nussknacker.engine.spel.TyperSpecTestData.TestRecord._
Expand Down Expand Up @@ -162,10 +164,8 @@ class TyperSpec extends AnyFunSuite with Matchers with ValidatedValuesDetailedMe
typeExpression(s"$testRecordExpr[#var]", "var" -> s"$nonPresentKey").invalidValue.toList should matchPattern {
case NoPropertyError(typingResult, key) :: Nil if typingResult == testRecordTyped && key == nonPresentKey =>
}
// TODO: this behavior is to be fixed - ideally this should behave the same as above
typeExpression(s"$testRecordExpr[$nonPresentKey]").invalidValue.toList should matchPattern {
case NoPropertyError(typingResult, key) :: DynamicPropertyAccessError :: Nil
if typingResult == testRecordTyped && key == nonPresentKey =>
case NoPropertyError(typingResult, key) :: Nil if typingResult == testRecordTyped && key == nonPresentKey =>
}
}

Expand All @@ -183,6 +183,17 @@ class TyperSpec extends AnyFunSuite with Matchers with ValidatedValuesDetailedMe
}
}

test("indexing on records with key which is not known at compile time treats record as map") {
typeExpression("{a: 5, b: 10}[#var.toString()]", "var" -> "a").validValue.finalResult.typingResult shouldBe Typed
.typedClass[Int]
}

test("indexing on records with non string key produces error") {
typeExpression("{a: 5, b: 10}[4]").invalidValue.toList should matchPattern {
case NoPropertyTypeError(_, _) :: Nil =>
}
}

private def buildTyper(dynamicPropertyAccessAllowed: Boolean = false) = new Typer(
dictTyper = new KeysDictTyper(new SimpleDictRegistry(Map.empty)),
strictMethodsChecking = false,
Expand Down

0 comments on commit 033a652

Please sign in to comment.