Preserve typing information during expression compilation (#6925)

TouK · Sep 26, 2024 · ab4f210 · ab4f210
1 parent 0e7a583
commit ab4f210
Show file tree

Hide file tree

Showing 5 changed files with 106 additions and 30 deletions.
diff --git a/designer/server/src/test/scala/pl/touk/nussknacker/ui/integration/DictsFlowTest.scala b/designer/server/src/test/scala/pl/touk/nussknacker/ui/integration/DictsFlowTest.scala
@@ -2,6 +2,7 @@ package pl.touk.nussknacker.ui.integration
 
 import com.typesafe.config.Config
 import io.circe.Json
+import io.circe.parser._
 import io.circe.syntax.EncoderOps
 import org.scalatest.OptionValues
 import org.scalatest.funsuite.AnyFunSuiteLike
@@ -84,7 +85,7 @@ class DictsFlowTest
 
     saveProcessAndTestIt(
       process,
-      expressionUsingDictWithLabel = None,
+      expectedExpressionUsingDictWithLabel = None,
       expectedResult = """RGBDict value to lowercase: h000000
          |LongDict value + 1: None
          |BooleanDict value negation: Some(false)
@@ -117,7 +118,7 @@ class DictsFlowTest
   test("save process with expression using dicts and get it back") {
     val expressionUsingDictWithLabel = s"#DICT['$Label']"
     val process = sampleProcessWithExpression(UUID.randomUUID().toString, expressionUsingDictWithLabel)
-    saveProcessAndExtractValidationResult(process, Some(expressionUsingDictWithLabel)) shouldBe Json.obj()
+    saveProcessAndExtractProcessValidationResult(process, Some(expressionUsingDictWithLabel), None) shouldBe Json.obj()
   }
 
   test("save process with invalid expression using dicts and get it back with validation results") {
@@ -203,13 +204,68 @@ class DictsFlowTest
     returnedEndResultExpression shouldEqual expressionUsingDictWithKey
   }
 
+  test("should get dict with correct label when node does not pass the validation") {
+    val process = ScenarioBuilder
+      .streaming("processWithDictParameterEditor")
+      .source("source", "csv-source-lite")
+      .enricher(
+        "customNode",
+        "data",
+        "serviceWithDictParameterEditor",
+        "RGBDict"     -> Expression(Language.DictKeyWithLabel, ""), // This field is mandatory and we leave it empty
+        "BooleanDict" -> Expression.dictKeyWithLabel("true", Some("OLD LABEL")),
+        "LongDict"    -> Expression(Language.DictKeyWithLabel, ""),
+        "RGBDictRAW"  -> Expression.spel("'someOtherColour'"),
+      )
+      .emptySink(EndNodeId, "dead-end-lite")
+    val invalidNodeValidationResult = parse(
+      """
+        |{
+        |  "customNode" : [
+        |    {
+        |      "typ" : "EmptyMandatoryParameter",
+        |      "message" : "This field is mandatory and can not be empty",
+        |      "description" : "Please fill field for this parameter",
+        |      "fieldName" : "RGBDict",
+        |      "errorType" : "SaveAllowed",
+        |      "details" : null
+        |    }
+        |  ]
+        |}
+        |""".stripMargin
+    ).rightValue
+
+    saveProcessAndExtractProcessValidationResult(process, None, Some(invalidNodeValidationResult))
+
+    val response2 = httpClient.send(
+      quickRequest
+        .get(uri"$nuDesignerHttpAddress/api/processes/${process.name}")
+        .auth
+        .basic("admin", "admin")
+    )
+    response2.code shouldEqual StatusCode.Ok
+
+    response2.bodyAsJson.hcursor
+      .downField("scenarioGraph")
+      .downField("nodes")
+      .downAt(_.hcursor.get[String]("id").rightValue == "customNode")
+      .downField("service")
+      .downField("parameters")
+      .downAt(_.hcursor.get[String]("name").rightValue == "BooleanDict")
+      .downField("expression")
+      .downField("expression")
+      .as[String]
+      .rightValue shouldBe """{"key":"true","label":"ON"}"""
+  }
+
   private def saveProcessAndTestIt(
       process: CanonicalProcess,
-      expressionUsingDictWithLabel: Option[String],
+      expectedExpressionUsingDictWithLabel: Option[String],
       expectedResult: String,
       variableToCheck: String = VariableName
   ) = {
-    saveProcessAndExtractValidationResult(process, expressionUsingDictWithLabel) shouldBe Json.obj()
+    saveProcessAndExtractProcessValidationResult(process, expectedExpressionUsingDictWithLabel, None) shouldBe Json
+      .obj()
 
     val response = httpClient.send(
       quickRequest
@@ -238,9 +294,10 @@ class DictsFlowTest
       .buildSimpleVariable(VariableNodeId, VariableName, variableExpression.spel)
       .emptySink(EndNodeId, "dead-end-lite")
 
-  private def saveProcessAndExtractValidationResult(
+  private def saveProcessAndExtractProcessValidationResult(
       process: CanonicalProcess,
-      endResultExpressionToPost: Option[String]
+      expectedEndResultExpressionToPost: Option[String],
+      expectedValidationResult: Option[Json]
   ): Json = {
     createEmptyScenario(process.name)
 
@@ -255,9 +312,11 @@ class DictsFlowTest
         .basic("admin", "admin")
     )
     response1.code shouldEqual StatusCode.Ok
-    response1.extractFieldJsonValue("errors", "invalidNodes").asObject.value shouldBe empty
+    response1.extractFieldJsonValue("errors", "invalidNodes").asJson shouldBe expectedValidationResult.getOrElse(
+      Json.obj()
+    )
 
-    extractValidationResult(process).asObject.value shouldBe empty
+    extractValidationResult(process).asJson shouldBe expectedValidationResult.getOrElse(Json.obj())
 
     val response2 = httpClient.send(
       quickRequest
@@ -266,9 +325,9 @@ class DictsFlowTest
         .basic("admin", "admin")
     )
     response2.code shouldEqual StatusCode.Ok
-    endResultExpressionToPost.foreach { endResultExpressionToPost =>
+    expectedEndResultExpressionToPost.foreach { expectedResult =>
       val returnedEndResultExpression = extractVariableExpressionFromGetProcessResponse(response2.bodyAsJson)
-      returnedEndResultExpression shouldEqual endResultExpressionToPost
+      returnedEndResultExpression shouldEqual expectedResult
     }
     response2.extractFieldJsonValue("validationResult", "errors", "invalidNodes")
   }

diff --git a/docs/Changelog.md b/docs/Changelog.md
@@ -60,6 +60,7 @@
   * [#6823](https://github.com/TouK/nussknacker/pull/6823) 
     * old process actions and comments migrated to new table in the db
     * Scenario Activity API implementation
+* [#6925](https://github.com/TouK/nussknacker/pull/6925) Fix situation when preset labels were presented as `null` when node didn't pass the validation.
 
 ## 1.17
 

diff --git a/...compiler/src/main/scala/pl/touk/nussknacker/engine/compile/ComponentExecutorFactory.scala b/...compiler/src/main/scala/pl/touk/nussknacker/engine/compile/ComponentExecutorFactory.scala
@@ -1,6 +1,6 @@
 package pl.touk.nussknacker.engine.compile
 
-import cats.data.ValidatedNel
+import cats.data.IorNel
 import com.typesafe.scalalogging.LazyLogging
 import pl.touk.nussknacker.engine.api.context.ProcessCompilationError
 import pl.touk.nussknacker.engine.api.definition.Parameter
@@ -24,7 +24,7 @@ class ComponentExecutorFactory(parameterEvaluator: ParameterEvaluator) extends L
   )(
       implicit nodeId: NodeId,
       metaData: MetaData
-  ): ValidatedNel[ProcessCompilationError, ComponentExecutor] = {
+  ): IorNel[ProcessCompilationError, ComponentExecutor] = {
     NodeValidationExceptionHandler.handleExceptions {
       doCreateComponentExecutor[ComponentExecutor](
         component,
@@ -34,7 +34,7 @@ class ComponentExecutorFactory(parameterEvaluator: ParameterEvaluator) extends L
         componentUseCase,
         nonServicesLazyParamStrategy
       )
-    }
+    }.toIor
   }
 
   private def doCreateComponentExecutor[ComponentExecutor](

diff --git a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/compile/ExpressionCompiler.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/compile/ExpressionCompiler.scala
@@ -1,7 +1,7 @@
 package pl.touk.nussknacker.engine.compile
 
-import cats.data.Validated.{Valid, invalid, invalidNel, valid}
-import cats.data.{NonEmptyList, Validated, ValidatedNel}
+import cats.data.Validated.{Invalid, Valid, invalid, invalidNel, valid}
+import cats.data.{Ior, IorNel, NonEmptyList, Validated, ValidatedNel}
 import cats.instances.list._
 import pl.touk.nussknacker.engine.ModelData
 import pl.touk.nussknacker.engine.api.{MetaData, NodeId}
@@ -125,7 +125,7 @@ class ExpressionCompiler(
   )(
       implicit nodeId: NodeId,
       metaData: MetaData
-  ): ValidatedNel[PartSubGraphCompilationError, List[CompiledParameter]] = {
+  ): IorNel[PartSubGraphCompilationError, List[CompiledParameter]] = {
     compileNodeParameters(
       parameterDefinitions,
       nodeParameters,
@@ -152,7 +152,7 @@ class ExpressionCompiler(
   )(
       implicit nodeId: NodeId,
       metaData: MetaData
-  ): ValidatedNel[PartSubGraphCompilationError, List[(TypedParameter, Parameter)]] = {
+  ): IorNel[PartSubGraphCompilationError, List[(TypedParameter, Parameter)]] = {
 
     val redundantMissingValidation = Validations.validateRedundantAndMissingParameters(
       parameterDefinitions,
@@ -178,9 +178,15 @@ class ExpressionCompiler(
     }
     val allCompiledParams = (compiledParams ++ compiledBranchParams).sequence
 
-    allCompiledParams
-      .andThen(allParams => Validations.validateWithCustomValidators(allParams, paramValidatorsMap))
-      .combine(redundantMissingValidation.map(_ => List()))
+    for {
+      compiledParams <- allCompiledParams.toIor
+      paramsAfterValidation = Validations.validateWithCustomValidators(compiledParams, paramValidatorsMap) match {
+        case Valid(a) => Ior.right(a)
+        // We want to preserve typing information from allCompiledParams even if custom validators give us some errors
+        case Invalid(e) => Ior.both(e, compiledParams)
+      }
+      combinedParams <- redundantMissingValidation.map(_ => List()).toIor.combine(paramsAfterValidation)
+    } yield combinedParams
   }
 
   private def parameterValidatorsMap(parameterDefinitions: List[Parameter], globalVariables: Map[String, TypingResult])(

diff --git a/...iler/src/main/scala/pl/touk/nussknacker/engine/compile/nodecompilation/NodeCompiler.scala b/...iler/src/main/scala/pl/touk/nussknacker/engine/compile/nodecompilation/NodeCompiler.scala
@@ -281,16 +281,18 @@ class NodeCompiler(
       }
     val validParams =
       expressionCompiler.compileExecutorComponentNodeParameters(validParamDefs.value, ref.parameters, ctx)
-    val validParamsCombinedErrors = validParams.combine(
-      NonEmptyList
-        .fromList(validParamDefs.written)
-        .map(invalid)
-        .getOrElse(valid(List.empty[CompiledParameter]))
-    )
+    val validParamsCombinedErrors = validParams
+      .fold(Invalid(_), Valid(_), (a, _) => Invalid(a))
+      .combine(
+        NonEmptyList
+          .fromList(validParamDefs.written)
+          .map(invalid)
+          .getOrElse(valid(List.empty[CompiledParameter]))
+      )
     val expressionTypingInfo =
       validParams
         .map(_.map(p => p.name.value -> p.typingInfo).toMap)
-        .valueOr(_ => Map.empty[String, ExpressionTypingInfo])
+        .getOrElse(Map.empty[String, ExpressionTypingInfo])
     NodeCompilationResult(expressionTypingInfo, None, newCtx, validParamsCombinedErrors)
   }
 
@@ -558,7 +560,7 @@ class NodeCompiler(
         ctx,
         branchContexts
       )
-      .andThen { compiledParameters =>
+      .flatMap { compiledParameters =>
         factory
           .createComponentExecutor[ComponentExecutor](
             componentDefinition,
@@ -581,7 +583,10 @@ class NodeCompiler(
             (typingInfo, componentExecutor)
           }
       }
-    (compiledObjectWithTypingInfo.map(_._1).valueOr(_ => Map.empty), compiledObjectWithTypingInfo.map(_._2))
+    (
+      compiledObjectWithTypingInfo.map(_._1).getOrElse(Map.empty),
+      compiledObjectWithTypingInfo.map(_._2).fold(Invalid(_), Valid(_), (a, _) => Invalid(a))
+    )
   }
 
   private def contextAfterMethodBasedCreatedComponentExecutor[ComponentExecutor](
@@ -683,7 +688,12 @@ class NodeCompiler(
         )
       }
       val nodeTypingInfo = computedParameters.map(_.map(p => p.name.value -> p.typingInfo).toMap).getOrElse(Map.empty)
-      NodeCompilationResult(nodeTypingInfo, None, outputCtx, serviceRef)
+      NodeCompilationResult(
+        nodeTypingInfo,
+        None,
+        outputCtx,
+        serviceRef.fold(Invalid(_), Valid(_), (a, _) => Invalid(a))
+      )
     }
 
   }