Revert "Fix TypingResult.valueOpt for the purpose of SpEL validator (…

…#6269)" (#6408)

components-api/src/main/scala/pl/touk/nussknacker/engine/api/typed/typing.scala

@@ -71,8 +71,8 @@ object typing {
       objType: TypedClass,
       additionalInfo: Map[String, AdditionalDataValue] = Map.empty
   ) extends SingleTypingResult {
-    override def valueOpt: Option[java.util.Map[String, Any]] =
-      fields.map { case (k, v) => v.valueOpt.map((k, _)) }.toList.sequence.map(Map(_: _*).asJava)
+    override def valueOpt: Option[Map[String, Any]] =
+      fields.map { case (k, v) => v.valueOpt.map((k, _)) }.toList.sequence.map(Map(_: _*))
 
     override def withoutValue: TypedObjectTypingResult =
       TypedObjectTypingResult(fields.mapValuesNow(_.withoutValue), objType, additionalInfo)
@@ -257,12 +257,6 @@ object typing {
       cl
     }
 
-    def typedList[T](elementType: TypingResult, elements: List[T]): TypedObjectWithValue =
-      TypedObjectWithValue(
-        Typed.genericTypeClass(classOf[java.util.List[_]], List(elementType)),
-        elements.asJava
-      )
-
     private def toRuntime[T: ClassTag]: Class[_] = implicitly[ClassTag[T]].runtimeClass
 
     // parameters - None if you are not in generic aware context, Some - otherwise

docs/Changelog.md

@@ -5,13 +5,11 @@
 * [#6398](https://github.com/TouK/nussknacker/pull/6398) Added possibility to define hint texts for scenario properties in config.
 * [#6282](https://github.com/TouK/nussknacker/pull/6184) From now on, the existence of Kafka topics used in Sources and/or 
   Sinks will always be validated. (`topicsExistenceValidationConfig.enabled` default was changed from `false` to `true`)
-* [#6384](https://github.com/TouK/nussknacker/pull/6384) Value of [Realm](https://datatracker.ietf.org/doc/html/rfc2617#section-1.2) 
+* [6384](https://github.com/TouK/nussknacker/pull/6384) Value of [Realm](https://datatracker.ietf.org/doc/html/rfc2617#section-1.2) 
   can be customized using env `AUTHENTICATION_REALM` (its default value "nussknacker" remains un changed)
-* [#6363](https://github.com/TouK/nussknacker/pull/6363) Improvement on SpEL suggestions mechanism, now we are able to 
+* [6363](https://github.com/TouK/nussknacker/pull/6363) Improvement on SpEL suggestions mechanism, now we are able to 
   provide suggestions even if the whole expression does not evaluate to proper SpEL expression. 
 * [#6388](https://github.com/TouK/nussknacker/pull/6388) Fix issue with suggestion expression mode and any value with suggestion in fragmentInput component, now supporting SpEL expressions.
-* [#6269](https://github.com/TouK/nussknacker/pull/6269) SpEL expression validator now works with values that are lists or maps.
-  * NOTE: selection (`.?`), projection (`.!`) or operations from the `#COLLECTIONS` helper aren't properly supported in validation expressions, and will cause the result to always be invalid
 * [#6418](https://github.com/TouK/nussknacker/pull/6418) Improvement: Pass implicit nodeId to `EagerServiceWithStaticParameters.returnType`
 
 1.16.1 (16 July 2024)

docs/MigrationGuide.md

@@ -12,11 +12,6 @@ To see the biggest differences please consult the [changelog](Changelog.md).
 * [6282](https://github.com/TouK/nussknacker/pull/6184) If you relied on the default value of the `topicsExistenceValidationConfig.enabled`
   setting, you must now be aware that topics will be validated by default (Kafka's `auto.create.topics.enable` setting
   is only considered in case of Sinks). Create proper topics manually if needed.
-* [#6269](https://github.com/TouK/nussknacker/pull/6269) Changes to `TypingResult` of SpEL expressions that are maps or lists:
-    * `TypedObjectTypingResult.valueOpt` now returns a `java.util.map` instead of `scala.collection.immutable.Map`
-        * NOTE: selection (`.?`) or operations from the `#COLLECTIONS` helper cause the map to lose track of its keys/values, reverting its `fields` to an empty Map
-    * SpEL list expression are now typed as `TypedObjectWithValue`, with the `underlying` `TypedClass` equal to the `TypedClass` before this change, and with `value` equal to a `java.util.List` of the elements' values.
-        * NOTE: selection (`.?`), projection (`.!`) or operations from the `#COLLECTIONS` helper cause the list to lose track of its values, reverting it to a value-less `TypedClass` like before the change
 * [#6418](https://github.com/TouK/nussknacker/pull/6418) Improvement: Pass implicit nodeId to `EagerServiceWithStaticParameters.returnType`
   * Now method `returnType` from `EagerServiceWithStaticParameters` requires implicit nodeId param
 

engine/lite/components/kafka-tests/src/test/scala/pl/touk/nussknacker/engine/lite/components/LiteKafkaUniversalAvroSchemaFunctionalTest.scala

@@ -453,7 +453,7 @@ class LiteKafkaUniversalAvroSchemaFunctionalTest
         ),
         (
           rConfig(sampleInteger, recordIntegerSchema, recordArrayOfNumbersSchema, List(sampleString)),
-          invalidTypes(s"path 'field[]' actual: '${typedStr.withoutValue.display}' expected: 'Integer | Double'")
+          invalidTypes(s"path 'field[]' actual: '${typedStr.display}' expected: 'Integer | Double'")
         ),
         // FIXME: List[Unknown] (rConfig(sampleInteger, recordIntegerSchema, recordWithArrayOfNumbers, s"""{$sampleBoolean, "$sampleString"}"""), invalidTypes(s"path 'field[]' actual: '${typeBool.display} | ${typeStr.display}' expected: 'Integer | Double'")),
         (
@@ -474,7 +474,7 @@ class LiteKafkaUniversalAvroSchemaFunctionalTest
         ),
         (
           rConfig(sampleInteger, recordIntegerSchema, recordMaybeArrayOfNumbersSchema, List(sampleString)),
-          invalidTypes(s"path 'field[]' actual: '${typedStr.withoutValue.display}' expected: 'Integer | Double'")
+          invalidTypes(s"path 'field[]' actual: '${typedStr.display}' expected: 'Integer | Double'")
         ),
         // FIXME: List[Unknown] (rConfig(sampleInteger, recordIntegerSchema, recordWithMaybeArrayOfNumbers, s"""{$sampleBoolean, "$sampleString"}"""), invalidTypes("path 'field[]' actual: '${typeBool.display} | ${typeStr.display}' expected: 'Integer | Double'")),
         (
@@ -496,7 +496,7 @@ class LiteKafkaUniversalAvroSchemaFunctionalTest
         // FIXME: List[Unknown]  (rConfig(sampleInteger, recordIntegerSchema, recordWithMaybeArrayOfNumbers, s"""{$sampleBoolean, "$sampleString"}"""), invalidTypes("path 'field[]' actual: '${typeBool.display} | ${typeStr.display}' expected: 'Integer | Double'")),
         (
           rConfig(sampleInteger, recordIntegerSchema, recordMaybeArrayOfNumbersSchema, List(sampleString)),
-          invalidTypes(s"path 'field[]' actual: '${typedStr.withoutValue.display}' expected: 'Integer | Double'")
+          invalidTypes(s"path 'field[]' actual: '${typedStr.display}' expected: 'Integer | Double'")
         ),
         (
           rConfig(sampleInteger, recordIntegerSchema, recordOptionalArrayOfNumbersSchema, sampleInteger),
@@ -535,13 +535,11 @@ class LiteKafkaUniversalAvroSchemaFunctionalTest
             recordOptionalArrayOfArraysNumbersSchema,
             List(List(sampleString))
           ),
-          invalidTypes(s"path 'field[][]' actual: '${typedStr.withoutValue.display}' expected: 'Integer | Double'")
+          invalidTypes(s"path 'field[][]' actual: '${typedStr.display}' expected: 'Integer | Double'")
         ),
         (
           rConfig(sampleInteger, recordIntegerSchema, recordOptionalArrayOfArraysNumbersSchema, List(sampleInteger)),
-          invalidTypes(
-            s"path 'field[]' actual: '${typedInt.withoutValue.display}' expected: 'Null | List[Integer | Double]'"
-          )
+          invalidTypes(s"path 'field[]' actual: '${typedInt.display}' expected: 'Null | List[Integer | Double]'")
         ),
         (
           rConfig(sampleInteger, recordIntegerSchema, recordOptionalArrayOfArraysNumbersSchema, sampleInteger),
@@ -575,7 +573,7 @@ class LiteKafkaUniversalAvroSchemaFunctionalTest
             recordOptionalArrayOfRecordsSchema,
             List(Map("price" -> sampleString))
           ),
-          invalidTypes(s"path 'field[].price' actual: '${typedStr.withoutValue.display}' expected: 'Null | Double'")
+          invalidTypes(s"path 'field[].price' actual: '${typedStr.display}' expected: 'Null | Double'")
         ),
         (
           rConfig(sampleInteger, recordIntegerSchema, recordOptionalArrayOfRecordsSchema, sampleInteger),
@@ -586,7 +584,7 @@ class LiteKafkaUniversalAvroSchemaFunctionalTest
         (
           rConfig(sampleInteger, recordIntegerSchema, recordOptionalArrayOfRecordsSchema, List(sampleInteger)),
           invalidTypes(
-            s"""path 'field[]' actual: '${typedInt.withoutValue.display}' expected: 'Null | Record{price: Null | Double}'"""
+            s"""path 'field[]' actual: '${typedInt.display}' expected: 'Null | Record{price: Null | Double}'"""
           )
         ),
       )
@@ -615,7 +613,7 @@ class LiteKafkaUniversalAvroSchemaFunctionalTest
         ),
         (
           rConfig(sampleInteger, recordIntegerSchema, recordMapOfIntsSchema, Nil),
-          invalidTypes("path 'field' actual: 'List[Unknown]([])' expected: 'Map[String,Null | Integer]'")
+          invalidTypes("path 'field' actual: 'List[Unknown]' expected: 'Map[String,Null | Integer]'")
         ),
         (
           rConfig(sampleInteger, recordIntegerSchema, recordMapOfIntsSchema, null),
@@ -635,7 +633,7 @@ class LiteKafkaUniversalAvroSchemaFunctionalTest
         ),
         (
           rConfig(sampleInteger, recordIntegerSchema, recordMaybeMapOfIntsSchema, Nil),
-          invalidTypes("path 'field' actual: 'List[Unknown]([])' expected: 'Null | Map[String,Null | Integer]'")
+          invalidTypes("path 'field' actual: 'List[Unknown]' expected: 'Null | Map[String,Null | Integer]'")
         ),
         (
           rConfig(
@@ -670,7 +668,7 @@ class LiteKafkaUniversalAvroSchemaFunctionalTest
         (
           rConfig(sampleInteger, recordIntegerSchema, recordOptionalMapOfMapsIntsSchema, Nil),
           invalidTypes(
-            "path 'field' actual: 'List[Unknown]([])' expected: 'Null | Map[String,Null | Map[String,Null | Integer]]'"
+            "path 'field' actual: 'List[Unknown]' expected: 'Null | Map[String,Null | Map[String,Null | Integer]]'"
           )
         ),
         (
@@ -718,7 +716,7 @@ class LiteKafkaUniversalAvroSchemaFunctionalTest
         (
           rConfig(sampleInteger, recordIntegerSchema, recordOptionalMapOfRecordsSchema, Nil),
           invalidTypes(
-            "path 'field' actual: 'List[Unknown]([])' expected: 'Null | Map[String,Null | Record{price: Null | Double}]'"
+            "path 'field' actual: 'List[Unknown]' expected: 'Null | Map[String,Null | Record{price: Null | Double}]'"
           )
         ),
         (
@@ -784,7 +782,7 @@ class LiteKafkaUniversalAvroSchemaFunctionalTest
         (
           rConfig(sampleInteger, recordIntegerSchema, nestedRecordSchema, Nil),
           invalidTypes(
-            "path 'field' actual: 'List[Unknown]([])' expected: 'Null | Record{sub: Null | Record{price: Null | Double}}'"
+            "path 'field' actual: 'List[Unknown]' expected: 'Null | Record{sub: Null | Record{price: Null | Double}}'"
           )
         ),
         (

scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/spel/Typer.scala

@@ -16,7 +16,6 @@ import pl.touk.nussknacker.engine.api.context.ValidationContext
 import pl.touk.nussknacker.engine.api.expression._
 import pl.touk.nussknacker.engine.api.generics.ExpressionParseError
 import pl.touk.nussknacker.engine.api.typed.supertype.{CommonSupertypeFinder, NumberTypesPromotionStrategy}
-import pl.touk.nussknacker.engine.api.typed.typing.Typed.typedList
 import pl.touk.nussknacker.engine.api.typed.typing._
 import pl.touk.nussknacker.engine.definition.clazz.ClassDefinitionSet
 import pl.touk.nussknacker.engine.definition.globalvariables.ExpressionConfigDefinition
@@ -52,7 +51,6 @@ import pl.touk.nussknacker.engine.spel.internal.EvaluationContextPreparer
 import pl.touk.nussknacker.engine.spel.typer.{MapLikePropertyTyper, MethodReferenceTyper, TypeReferenceTyper}
 import pl.touk.nussknacker.engine.util.MathUtils
 
-import scala.jdk.CollectionConverters._
 import scala.annotation.tailrec
 import scala.reflect.runtime._
 import scala.util.{Failure, Success, Try}
@@ -226,7 +224,6 @@ private[spel] class Typer(
         // TODO: how to handle other cases?
         case TypedNull =>
           invalidNodeResult(IllegalIndexingOperation)
-        case TypedObjectWithValue(underlying, _) => typeIndexer(e, underlying)
         case _ =>
           val w = validNodeResult(Unknown)
           if (dynamicPropertyAccessAllowed) w else w.tell(List(DynamicPropertyAccessError))
@@ -295,10 +292,8 @@ private[spel] class Typer(
           def getSupertype(a: TypingResult, b: TypingResult): TypingResult =
             CommonSupertypeFinder.Default.commonSupertype(a, b)
 
-          val elementType           = if (children.isEmpty) Unknown else children.reduce(getSupertype).withoutValue
-          val childrenCombinedValue = children.flatMap(_.valueOpt)
-
-          valid(typedList(elementType, childrenCombinedValue))
+          val elementType = if (children.isEmpty) Unknown else children.reduce(getSupertype)
+          valid(Typed.genericTypeClass[java.util.List[_]](List(elementType)))
         }
 
       case e: InlineMap =>
@@ -413,8 +408,6 @@ private[spel] class Typer(
           elementType <- extractIterativeType(iterateType)
           result <- typeChildren(validationContext, node, current.pushOnStack(elementType)) {
             case result :: Nil =>
-              // Limitation: projection on an iterative type makes it loses it's known value,
-              // as properly determining it would require evaluating the projection expression for each element (likely working on the AST)
               valid(Typed.genericTypeClass[java.util.List[_]](List(result)))
             case other =>
               invalid(IllegalSelectionTypeError(other))
@@ -499,23 +492,7 @@ private[spel] class Typer(
       childElementType: TypingResult
   ) = {
     val isSingleElementSelection = List("$", "^").map(node.toStringAST.startsWith(_)).foldLeft(false)(_ || _)
-
-    if (isSingleElementSelection)
-      childElementType
-    else {
-      // Limitation: selection from an iterative type makes it loses it's known value,
-      // as properly determining it would require evaluating the selection expression for each element (likely working on the AST)
-      parentType match {
-        case tc: SingleTypingResult if tc.objType.canBeSubclassOf(Typed[java.util.Collection[_]]) =>
-          tc.withoutValue
-        case tc: SingleTypingResult if tc.objType.klass.isArray =>
-          tc.withoutValue
-        case tc: SingleTypingResult if tc.objType.canBeSubclassOf(Typed[java.util.Map[_, _]]) =>
-          Typed.record(Map.empty)
-        case _ =>
-          parentType
-      }
-    }
+    if (isSingleElementSelection) childElementType else parentType
   }
 
   private def checkEqualityLikeOperation(

scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/compile/GenericTransformationValidationSpec.scala

@@ -27,7 +27,6 @@ import pl.touk.nussknacker.engine.dict.SimpleDictRegistry
 import pl.touk.nussknacker.engine.modelconfig.ComponentsUiConfig
 import pl.touk.nussknacker.engine.testing.ModelDefinitionBuilder
 import pl.touk.nussknacker.engine.CustomProcessValidatorLoader
-import pl.touk.nussknacker.engine.api.typed.typing.Typed.typedList
 
 class GenericTransformationValidationSpec extends AnyFunSuite with Matchers with OptionValues with Inside {
 
@@ -99,7 +98,7 @@ class GenericTransformationValidationSpec extends AnyFunSuite with Matchers with
       Map(
         "val1" -> Typed.fromInstance("aa"),
         "val2" -> Typed.fromInstance(11),
-        "val3" -> typedList(Typed[java.lang.Boolean], List(false))
+        "val3" -> Typed.genericTypeClass(classOf[java.util.List[_]], List(Typed.fromInstance(false)))
       )
     )
 
@@ -128,7 +127,7 @@ class GenericTransformationValidationSpec extends AnyFunSuite with Matchers with
       Map(
         "val1" -> Typed.fromInstance("aa"),
         "val2" -> Typed.fromInstance(11),
-        "val3" -> typedList(Typed[java.lang.Boolean], List(false))
+        "val3" -> Typed.genericTypeClass(classOf[java.util.List[_]], List(Typed.fromInstance(false)))
       )
     )
 

scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/SpelExpressionSpec.scala

@@ -22,7 +22,6 @@ import pl.touk.nussknacker.engine.api.generics.{
 }
 import pl.touk.nussknacker.engine.api.process.ExpressionConfig._
 import pl.touk.nussknacker.engine.api.typed.TypedMap
-import pl.touk.nussknacker.engine.api.typed.typing.Typed.typedList
 import pl.touk.nussknacker.engine.api.typed.typing.{Typed, _}
 import pl.touk.nussknacker.engine.api.{Context, NodeId, SpelExpressionExcludeList}
 import pl.touk.nussknacker.engine.definition.clazz.{ClassDefinitionSet, JavaClassWithVarargs}
@@ -351,9 +350,10 @@ class SpelExpressionSpec extends AnyFunSuite with Matchers with ValidatedValuesD
 
     parse[Any]("null").toOption.get.returnType shouldBe TypedNull
     parse[java.util.List[String]]("{'t', null, 'a'}").toOption.get.returnType shouldBe
-      typedList(Typed[String], List("t", null, "a"))
+      Typed.genericTypeClass(classOf[java.util.List[_]], List(Typed[String]))
     parse[java.util.List[Any]]("{5, 't', null}").toOption.get.returnType shouldBe
-      typedList(Typed[Any], List(5, "t", null))
+      Typed.genericTypeClass(classOf[java.util.List[_]], List(Typed[Any]))
+
     parse[Int]("true ? 8 : null").toOption.get.returnType shouldBe Typed[Int]
   }
 
@@ -815,7 +815,7 @@ class SpelExpressionSpec extends AnyFunSuite with Matchers with ValidatedValuesD
     )
     shouldHaveBadType(
       parse[String]("{1, 2, 3}", ctx),
-      s"Bad expression type, expected: String, found: ${typedList(Typed[Int], List(1, 2, 3)).display}"
+      s"Bad expression type, expected: String, found: ${Typed.genericTypeClass(classOf[java.util.List[_]], List(Typed.typedClass[Int])).display}"
     )
     shouldHaveBadType(
       parse[java.util.Map[_, _]]("'alaMa'", ctx),

scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/spel/TyperSpec.scala

@@ -19,8 +19,6 @@ import pl.touk.nussknacker.engine.spel.TyperSpecTestData.TestRecord._
 import pl.touk.nussknacker.engine.util.Implicits.RichScalaMap
 import pl.touk.nussknacker.test.ValidatedValuesDetailedMessage
 
-import scala.jdk.CollectionConverters._
-
 class TyperSpec extends AnyFunSuite with Matchers with ValidatedValuesDetailedMessage {
 
   private implicit val defaultTyper: Typer = buildTyper()
@@ -47,23 +45,9 @@ class TyperSpec extends AnyFunSuite with Matchers with ValidatedValuesDetailedMe
     )
   }
 
-  test("detect proper List type with value") {
-    typeExpression("{1,2}").validValue.finalResult.typingResult shouldBe
-      TypedObjectWithValue(
-        Typed.genericTypeClass(classOf[java.util.List[_]], List(Typed.typedClass[Int])),
-        List(1, 2).asJava
-      )
-  }
-
-  test("detect proper selection types - List") {
+  test("detect proper selection types") {
     typeExpression("{1,2}.?[(#this==1)]").validValue.finalResult.typingResult shouldBe
       Typed.genericTypeClass(classOf[java.util.List[_]], List(Typed.typedClass[Int]))
-    // see comment in Typer.resolveSelectionTypingResult
-  }
-
-  test("detect proper selection types - Map") {
-    typeExpression("{'field1': 1, 'field2': 2}.?[(#this.value==1)]").validValue.finalResult.typingResult shouldBe
-      Typed.record(Map.empty) // see comment in Typer.resolveSelectionTypingResult
   }
 
   test("detect proper first selection types") {