Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NU-1935] Change record indexer behaviour #7471

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading