From 033a652098dd28dc724f3cd6bdce38cc08d718f4 Mon Sep 17 00:00:00 2001 From: Pawel Czajka Date: Fri, 17 Jan 2025 12:55:34 +0100 Subject: [PATCH] Revert "Revert "[NU-1935] Change record indexer behaviour (#7443)"" This reverts commit 470be6e61b9864cb3cba5dd75d6f046959742827. --- docs/Changelog.md | 1 + .../spel/SpelExpressionParseError.scala | 4 ++++ .../touk/nussknacker/engine/spel/Typer.scala | 12 ++++++++--- .../engine/spel/SpelExpressionSpec.scala | 9 ++++++++ .../nussknacker/engine/spel/TyperSpec.scala | 21 ++++++++++++++----- 5 files changed, 39 insertions(+), 8 deletions(-) diff --git a/docs/Changelog.md b/docs/Changelog.md index 370f3e7431d..1d669514b45 100644 --- a/docs/Changelog.md +++ b/docs/Changelog.md @@ -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 diff --git a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/SpelExpressionParseError.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/SpelExpressionParseError.scala index 49b27a7f186..5182bd6bae6 100644 --- a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/SpelExpressionParseError.scala +++ b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/SpelExpressionParseError.scala @@ -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" } diff --git a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala index 94398a81cd6..995a8b6afe6 100644 --- a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala +++ b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala @@ -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 } @@ -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) diff --git a/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala b/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala index 6291e7c8ff4..3455fad4c08 100644 --- a/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala +++ b/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala @@ -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) diff --git a/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/TyperSpec.scala b/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/TyperSpec.scala index f69278af040..9da82310d9f 100644 --- a/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/TyperSpec.scala +++ b/scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/TyperSpec.scala @@ -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._ @@ -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 => } } @@ -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,