[NU-2152] Missing parameters in node are only reported as warnings in logs - no validation error is returned

arkadius · arkadius · commit 81ddd4c33333 · 2025-04-30T18:58:26.000+02:00
diff --git a/common-api/src/main/scala/pl/touk/nussknacker/engine/api/parameter/ParameterName.scala b/common-api/src/main/scala/pl/touk/nussknacker/engine/api/parameter/ParameterName.scala
@@ -5,6 +5,8 @@ import io.circe.generic.extras.semiauto.{deriveUnwrappedDecoder, deriveUnwrapped
 
 final case class ParameterName(value: String) {
   def withBranchId(branchId: String): ParameterName = ParameterName(s"$value for branch $branchId")
+
+  override def toString: String = value
 }
 
 object ParameterName {
diff --git a/designer/client/src/components/graph/node-modal/NodeTypeDetailsContent.tsx b/designer/client/src/components/graph/node-modal/NodeTypeDetailsContent.tsx
@@ -51,8 +51,8 @@ export function useNodeAdjust() {
     const adjustNode = useCallback(
         (node: NodeType) => {
             const parameterDefinitions = getParameterDefinitions(node);
-            const { adjustedNode } = adjustParameters(node, parameterDefinitions);
-            return generateUUIDs(adjustedNode, ["fields", "parameters"]);
+            // const { adjustedNode } = adjustParameters(node, parameterDefinitions);
+            return generateUUIDs(node, ["fields", "parameters"]);
         },
         [getParameterDefinitions],
     );
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
@@ -160,14 +160,14 @@ class ExpressionCompiler(
       jobData: JobData
   ): IorNel[PartSubGraphCompilationError, List[(TypedParameter, Parameter)]] = {
 
-    val missingValidation = Validations.validateRedundantAndMissingParameters(
+    val adjustedParameters = NodeParametersAdjuster.adjustNonBranchParameters(
       parameterDefinitions,
-      nodeParameters ++ nodeBranchParameters.flatMap(_.parameters)
+      nodeParameters
     )
     val paramValidatorsMap = parameterValidatorsMap(parameterDefinitions, ctx.globalVariables)
     val paramDefMap        = parameterDefinitions.map(p => p.name -> p).toMap
 
-    val compiledParams = nodeParameters
+    val compiledParams = adjustedParameters
       .flatMap { nodeParam =>
         paramDefMap
           .get(nodeParam.name)
@@ -186,13 +186,8 @@ class ExpressionCompiler(
 
     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 <- missingValidation.map(_ => List()).toIor.combine(paramsAfterValidation)
-    } yield combinedParams
+      _              <- Validations.validateWithCustomValidators(compiledParams, paramValidatorsMap).toIor
+    } yield compiledParams
   }
 
   private def parameterValidatorsMap(parameterDefinitions: List[Parameter], globalVariables: Map[String, TypingResult])(
diff --git a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/compile/NodeParametersAdjuster.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/compile/NodeParametersAdjuster.scala
@@ -0,0 +1,50 @@
+package pl.touk.nussknacker.engine.compile
+
+import cats.data.Validated.valid
+import cats.implicits.catsSyntaxValidatedId
+import com.typesafe.scalalogging.LazyLogging
+import pl.touk.nussknacker.engine.api.{JobData, NodeId}
+import pl.touk.nussknacker.engine.api.context.ProcessCompilationError.MissingParameters
+import pl.touk.nussknacker.engine.api.definition.Parameter
+import pl.touk.nussknacker.engine.api.parameter.ParameterName
+import pl.touk.nussknacker.engine.graph.evaluatedparam.{Parameter => NodeParameter}
+
+object NodeParametersAdjuster extends LazyLogging {
+
+  def adjustNonBranchParameters(parameterDefinitions: List[Parameter], parameters: List[NodeParameter])(
+      implicit nodeId: NodeId,
+      jobData: JobData
+  ): List[NodeParameter] = {
+    val definedParamNamesSet = parameterDefinitions.filter(!_.branchParam).map(_.name).toSet
+    val usedParamNamesSet    = parameters.map(_.name).toSet
+
+    checkRedundancyAndWarnIfNeeded(definedParamNamesSet, usedParamNamesSet)
+    validateMissingness(definedParamNamesSet, usedParamNamesSet)
+    // FIXME abr
+    parameters
+  }
+
+  private def checkRedundancyAndWarnIfNeeded(
+      definedParamNamesSet: Set[ParameterName],
+      usedParamNamesSet: Set[ParameterName]
+  )(
+      implicit nodeId: NodeId,
+      jobData: JobData
+  ): Unit = {
+    val redundantParams = usedParamNamesSet.diff(definedParamNamesSet)
+    if (redundantParams.nonEmpty) {
+      logger.warn(
+        s"Scenario [${jobData.metaData.name}] node [$nodeId] compilation warning. " +
+          s"Found redundant parameters: ${redundantParams.toList.map(_.value).sorted.mkString(", ")}. They will be skipped."
+      )
+    }
+  }
+
+  private def validateMissingness(definedParamNamesSet: Set[ParameterName], usedParamNamesSet: Set[ParameterName])(
+      implicit nodeId: NodeId
+  ) = {
+    val notUsedParams = definedParamNamesSet.diff(usedParamNamesSet)
+    if (notUsedParams.nonEmpty) MissingParameters(notUsedParams).invalidNel[Unit] else valid(())
+  }
+
+}
diff --git a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/compile/Validations.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/compile/Validations.scala
@@ -1,81 +1,35 @@
 package pl.touk.nussknacker.engine.compile
 
-import cats.data.Validated.valid
-import com.typesafe.scalalogging.LazyLogging
-import pl.touk.nussknacker.engine.api.{JobData, NodeId}
+import cats.data.{NonEmptyList, Validated}
+import pl.touk.nussknacker.engine.api.NodeId
 import pl.touk.nussknacker.engine.api.context._
-import pl.touk.nussknacker.engine.api.context.ProcessCompilationError.MissingParameters
 import pl.touk.nussknacker.engine.api.definition.{Parameter, Validator}
 import pl.touk.nussknacker.engine.api.parameter.ParameterName
 import pl.touk.nussknacker.engine.compiledgraph.TypedParameter
 import pl.touk.nussknacker.engine.expression.parse.{TypedExpression, TypedExpressionMap}
-import pl.touk.nussknacker.engine.graph.evaluatedparam.{Parameter => NodeParameter}
 import pl.touk.nussknacker.engine.graph.expression.Expression
 
-object Validations extends LazyLogging {
+object Validations {
 
   import cats.data.ValidatedNel
   import cats.implicits._
 
-  def validateRedundantAndMissingParameters(
-      parameterDefinitions: List[Parameter],
-      parameters: List[NodeParameter]
-  )(
-      implicit nodeId: NodeId,
-      jobData: JobData
-  ): ValidatedNel[PartSubGraphCompilationError, Unit] = {
-    val definedParamNamesSet = parameterDefinitions.map(_.name).toSet
-    val usedParamNamesSet    = parameters.map(_.name).toSet
-
-    checkRedundancyAndWarnIfNeeded(definedParamNamesSet, usedParamNamesSet)
-    validateMissingness(definedParamNamesSet, usedParamNamesSet)
-  }
-
   def validateWithCustomValidators(
       parameters: List[(TypedParameter, Parameter)],
       paramValidatorsMap: Map[ParameterName, ValidatedNel[PartSubGraphCompilationError, List[Validator]]]
   )(
       implicit nodeId: NodeId
-  ): ValidatedNel[PartSubGraphCompilationError, List[(TypedParameter, Parameter)]] =
+  ): ValidatedNel[PartSubGraphCompilationError, Unit] =
     parameters
       .map { case (typedParam, _) =>
         paramValidatorsMap(typedParam.name).andThen(validator => validate(validator, typedParam))
       }
       .sequence
-      .map(_ => parameters)
-
-  private def checkRedundancyAndWarnIfNeeded(
-      definedParamNamesSet: Set[ParameterName],
-      usedParamNamesSet: Set[ParameterName]
-  )(
-      implicit nodeId: NodeId,
-      jobData: JobData
-  ): Unit = {
-    val redundantParams = usedParamNamesSet.diff(definedParamNamesSet)
-    if (redundantParams.nonEmpty) {
-      logger.warn(
-        s"Scenario [${jobData.metaData.name}] node [$nodeId] compilation warning. " +
-          s"Found redundant parameters: ${redundantParams.toList.map(_.value).sorted.mkString(", ")}. They will be skipped."
-      )
-    }
-  }
-
-  private def validateMissingness(definedParamNamesSet: Set[ParameterName], usedParamNamesSet: Set[ParameterName])(
-      implicit nodeId: NodeId
-  ) = {
-    val notUsedParams = definedParamNamesSet.diff(usedParamNamesSet)
-    if (notUsedParams.nonEmpty) MissingParameters(notUsedParams).invalidNel[Unit] else valid(())
-  }
-
-  def validate[T](validators: List[Validator], parameter: (TypedParameter, T))(
-      implicit nodeId: NodeId
-  ): ValidatedNel[PartSubGraphCompilationError, (TypedParameter, T)] = {
-    validate(validators, parameter._1).map((_, parameter._2))
-  }
+      .map(_ => ())
 
   def validate(validators: List[Validator], parameter: TypedParameter)(
       implicit nodeId: NodeId
-  ): ValidatedNel[PartSubGraphCompilationError, TypedParameter] = {
+  ): Validated[NonEmptyList[PartSubGraphCompilationError], Unit] = {
     val paramWithValueAndExpressionList = parameter.typedValue match {
       case te: TypedExpression => List((parameter.name, te.typingInfo.typingResult.valueOpt, te.expression))
       case tem: TypedExpressionMap =>
@@ -95,7 +49,7 @@ object Validations extends LazyLogging {
         }
       }
       .sequence
-      .map(_ => parameter)
+      .map(_ => ())
   }
 
 }
diff --git a/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/compile/nodecompilation/DynamicNodeValidator.scala b/scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/compile/nodecompilation/DynamicNodeValidator.scala
@@ -195,8 +195,7 @@ class DynamicNodeValidator(
             validatorsCompilationResult.andThen { validators =>
               expressionCompiler
                 .compileBranchParam(branchParams, branchContexts, parameter)
-                .map((_, None))
-                .andThen(Validations.validate(validators, _))
+                .andThen(typedParam => Validations.validate(validators, typedParam).map(_ => (typedParam, None)))
             }
           }
       } else {
@@ -220,8 +219,9 @@ class DynamicNodeValidator(
         validatorsCompilationResult.andThen { validators =>
           expressionCompiler
             .compileParam(singleParam, ctxToUse, parameter)
-            .map((_, extraNodeParamOpt))
-            .andThen(Validations.validate(validators, _))
+            .andThen(typedParam =>
+              Validations.validate(validators, typedParam).map(_ => (typedParam, extraNodeParamOpt))
+            )
         }
       }
     }

Original file line number	Diff line number	Diff line change
`@@ -5,6 +5,8 @@ import io.circe.generic.extras.semiauto.{deriveUnwrappedDecoder, deriveUnwrapped`
`5`	`5`
`6`	`6`	`final case class ParameterName(value: String) {`
`7`	`7`	`def withBranchId(branchId: String): ParameterName = ParameterName(s"$value for branch $branchId")`
	`8`	`+`
	`9`	`+ override def toString: String = value`
`8`	`10`	`}`
`9`	`11`
`10`	`12`	`object ParameterName {`
Original file line number	Diff line number	Diff line change
`@@ -195,8 +195,7 @@ class DynamicNodeValidator(`
`195`	`195`	`validatorsCompilationResult.andThen { validators =>`
`196`	`196`	`expressionCompiler`
`197`	`197`	`.compileBranchParam(branchParams, branchContexts, parameter)`
`198`		`- .map((_, None))`
`199`		`- .andThen(Validations.validate(validators, _))`
	`198`	`+ .andThen(typedParam => Validations.validate(validators, typedParam).map(_ => (typedParam, None)))`
`200`	`199`	`}`
`201`	`200`	`}`
`202`	`201`	`} else {`
`@@ -220,8 +219,9 @@ class DynamicNodeValidator(`
`220`	`219`	`validatorsCompilationResult.andThen { validators =>`
`221`	`220`	`expressionCompiler`
`222`	`221`	`.compileParam(singleParam, ctxToUse, parameter)`
`223`		`- .map((_, extraNodeParamOpt))`
`224`		`- .andThen(Validations.validate(validators, _))`
	`222`	`+ .andThen(typedParam =>`
	`223`	`+ Validations.validate(validators, typedParam).map(_ => (typedParam, extraNodeParamOpt))`
	`224`	`+ )`
`225`	`225`	`}`
`226`	`226`	`}`
`227`	`227`	`}`