diff --git a/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala b/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala index 8e4cec7ee42..0c016839665 100644 --- a/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala +++ b/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/AssignabilityDeterminer.scala @@ -32,6 +32,9 @@ object AssignabilityDeterminer { def isAssignableStrict(from: TypingResult, to: TypingResult): ValidatedNel[String, Unit] = isAssignable(from, to, StrictConversionChecker) + def isAssignableWithoutConversion(from: TypingResult, to: TypingResult): ValidatedNel[String, Unit] = + isAssignable(from, to, WithoutConversionChecker) + private def isAssignable(from: TypingResult, to: TypingResult, conversionChecker: ConversionChecker) = { (from, to) match { case (_, Unknown) => ().validNel @@ -223,6 +226,19 @@ object AssignabilityDeterminer { } + private object WithoutConversionChecker extends ConversionChecker { + + override def isConvertable( + from: SingleTypingResult, + to: TypedClass + ): ValidatedNel[String, Unit] = { + val errMsgPrefix = + s"${from.runtimeObjType.display} is not the same as ${to.display}" + condNel(from.withoutValue == to.withoutValue, (), errMsgPrefix) + } + + } + private object StrictConversionChecker extends ConversionChecker { override def isConvertable( diff --git a/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/typing.scala b/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/typing.scala index 1aff3f9a890..343af1e201d 100644 --- a/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/typing.scala +++ b/components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/typing.scala @@ -40,6 +40,12 @@ object typing { final def canBeStrictlyConvertedTo(typingResult: TypingResult): Boolean = AssignabilityDeterminer.isAssignableStrict(this, typingResult).isValid + /** + * Checks if the conversion to a given typingResult can be made without any conversion. + */ + final def canBeConvertedWithoutConversionTo(typingResult: TypingResult): Boolean = + AssignabilityDeterminer.isAssignableWithoutConversion(this, typingResult).isValid + def valueOpt: Option[Any] def withoutValue: TypingResult 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..23344aab195 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) @@ -218,7 +224,17 @@ private[spel] class Typer( // TODO: validate indexer key - the only valid key is an integer - but its more complicated with references withTypedChildren(_ => valid(param)) case TypedClass(clazz, keyParam :: valueParam :: Nil) if clazz.isAssignableFrom(classOf[java.util.Map[_, _]]) => - withTypedChildren(_ => valid(valueParam)) + withTypedChildren { + // Spel implementation of map indexer (in class org.springframework.expression.spel.ast.Indexer, line 154) tries to convert + // indexer to key type of map, but this conversion can be accomplished only if key type of map is known to spel. + // Currently .asMap extension is implemented in such a way, that spel does not know key type of the resulting map + // (that is when spel evaluates this expression it only knows that it got map, but does not know its type parameters). + // It would be hard to change implementation of .asMap extension so we partially turn off this feature of indexer conversion + // by allowing in typing only situations when map key type and indexer type are the same (though we have to allow + // indexing with unknown type) + case indexKey :: Nil if indexKey.canBeConvertedWithoutConversionTo(keyParam) => valid(valueParam) + case _ => invalid(IllegalIndexingOperation) + } case d: TypedDict => dictTyper.typeDictValue(d, e).map(toNodeResult) case union: TypedUnion => typeUnion(e, union) case TypedTaggedValue(underlying, _) => typeIndexer(e, underlying) 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..c59145cb0ad 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) @@ -897,6 +906,28 @@ class SpelExpressionSpec extends AnyFunSuite with Matchers with ValidatedValuesD parse[java.math.BigDecimal]("-1.1", ctx) shouldBe Symbol("valid") } + test("should not validate map indexing if index type and map key type are different") { + parse[Any]("""{{key: "a", value: 5}}.toMap[0]""") shouldBe Symbol("invalid") + parse[Any]("""{{key: 1, value: 5}}.toMap["0"]""") shouldBe Symbol("invalid") + parse[Any]("""{{key: 1.toLong, value: 5}}.toMap[0]""") shouldBe Symbol("invalid") + parse[Any]("""{{key: 1, value: 5}}.toMap[0.toLong]""") shouldBe Symbol("invalid") + } + + test("should validate map indexing if index type and map key type are the same") { + parse[Any]("""{{key: 1, value: 5}}.toMap[0]""") shouldBe Symbol("valid") + } + + test("should handle map indexing with unknown key type") { + val context = Context("sth").withVariables( + Map( + "unknownString" -> ContainerOfUnknown("a"), + ) + ) + + evaluate[Int]("""{{key: "a", value: 5}}.toMap[#unknownString.value]""", context) shouldBe 5 + evaluate[Integer]("""{{key: "b", value: 5}}.toMap[#unknownString.value]""", context) shouldBe null + } + test("validate ternary operator") { parse[Long]("'d'? 3 : 4", ctx) should not be Symbol("valid") parse[String]("1 > 2 ? 12 : 23", ctx) should not be Symbol("valid") 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,