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 index 45addd01d1b..7e6468e84b0 100644 --- 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 index c44dd105df4..21acc6c1735 100644 --- 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/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/compile/ComponentExecutorFactory.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/compile/ComponentExecutorFactory.scala index 88c15f80672..a8c24350db3 100644 --- a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/compile/ComponentExecutorFactory.scala +++ b/scenario-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 index a67780e9c2d..cd1d62e8c71 100644 --- 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/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/compile/nodecompilation/NodeCompiler.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/compile/nodecompilation/NodeCompiler.scala index dd07146afcd..5d9d786049a 100644 --- a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/compile/nodecompilation/NodeCompiler.scala +++ b/scenario-compiler/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)) + ) } }