Skip to content

Commit

Permalink
[Nu-7573] Relax validation of absent variables in fragment input vali…
Browse files Browse the repository at this point in the history
…dation (#7574)

* [Nu-7573] relax missing variables validation

Co-authored-by: DeamonDev <[email protected]>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: DeamonDev <[email protected]>
  • Loading branch information
3 people authored Feb 24, 2025
1 parent 5f4ba7e commit 16eaf0d
Show file tree
Hide file tree
Showing 11 changed files with 109 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class DefaultConfigCreator extends EmptyProcessConfigCreator {

override def expressionConfig(modelDependencies: ProcessObjectDependencies): ExpressionConfig = {
ExpressionConfig(
Map(
globalProcessVariables = Map(
"GEO" -> anyCategory(geo),
"NUMERIC" -> anyCategory(numeric),
"CONV" -> anyCategory(conversion),
Expand All @@ -21,7 +21,7 @@ class DefaultConfigCreator extends EmptyProcessConfigCreator {
"RANDOM" -> anyCategory(random),
"BASE64" -> anyCategory(base64)
),
List()
globalImports = List()
)
}

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import pl.touk.nussknacker.engine.testing.{LocalModelData, ModelDefinitionBuilde
import pl.touk.nussknacker.engine.util.Implicits.RichScalaMap
import pl.touk.nussknacker.engine.util.service.EagerServiceWithStaticParametersAndReturnType
import pl.touk.nussknacker.engine.CustomProcessValidator
import pl.touk.nussknacker.engine.graph.expression.Expression.Language.Spel
import pl.touk.nussknacker.engine.util.functions.collection
import pl.touk.nussknacker.restmodel.validation.ValidationResults.NodeValidationErrorType.{
RenderNotAllowed,
Expand Down Expand Up @@ -797,6 +798,49 @@ class UIProcessValidatorSpec extends AnyFunSuite with Matchers with TableDrivenP
}
}

test("validates fragment input definition while validating fragment - accepting absent variables") {
val fragmentWithValidParam =
CanonicalProcess(
MetaData("fragment1", FragmentSpecificData()),
List(
FlatNode(
FragmentInputDefinition(
"in",
List(
FragmentParameter(
ParameterName("param1"),
FragmentClazzRef[String],
initialValue = Some(FixedExpressionValue("#input", "inputValue")),
hintText = None,
valueEditor = None,
valueCompileTimeValidation = None
),
FragmentParameter(
ParameterName("param2"),
FragmentClazzRef[java.util.List[Boolean]],
initialValue = Some(FixedExpressionValue("{1, 2}.![#this > 1]", "mappedValue")),
hintText = None,
valueEditor = None,
valueCompileTimeValidation = None
),
)
)
),
FlatNode(
FragmentOutputDefinition("out", "out1", List.empty)
)
),
List.empty
)

val fragmentGraph =
CanonicalProcessConverter.toScenarioGraph(fragmentWithValidParam)

val validationResult = validateWithConfiguredProperties(fragmentGraph)

validationResult.errors.invalidNodes shouldBe empty
}

test("validates fragment input definition while validating fragment - ValueInputWithDictEditor") {
val fragmentWithInvalidParam =
CanonicalProcess(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ object node {
initialValue: Option[FixedExpressionValue],
hintText: Option[String],
valueEditor: Option[ParameterValueInput],
valueCompileTimeValidation: Option[ParameterValueCompileTimeValidation],
valueCompileTimeValidation: Option[ParameterValueCompileTimeValidation]
)

object FragmentParameter {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -416,4 +416,10 @@ class ExpressionCompiler(
}
}

def withExpressionParsers(
modify: Map[Language, ExpressionParser] => Map[Language, ExpressionParser]
): ExpressionCompiler = {
new ExpressionCompiler(modify(expressionParsers), dictRegistry, expressionEvaluator)
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import pl.touk.nussknacker.engine.graph.expression._
import pl.touk.nussknacker.engine.graph.node._
import pl.touk.nussknacker.engine.graph.service.ServiceRef
import pl.touk.nussknacker.engine.resultcollector.ResultCollector
import pl.touk.nussknacker.engine.spel.SpelExpressionParser
import pl.touk.nussknacker.engine.variables.GlobalVariablesPreparer
import pl.touk.nussknacker.engine.{api, compiledgraph}
import shapeless.Typeable
Expand Down Expand Up @@ -199,7 +200,13 @@ class NodeCompiler(
fragmentParameterValidator.validateFixedExpressionValues(
param,
validationContext,
expressionCompiler
expressionCompiler.withExpressionParsers(expressionParsers =>
expressionParsers.map {
case (language, parser: SpelExpressionParser) =>
language -> parser.withValidator(v => v.withTyper(t => t.withAbsentVariableReferenceAllowed(true)))
case other => other
}
)
)
}
.sequence
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,17 @@ class SpelExpressionParser(
prepareEvaluationContext
)

def withValidator(modify: SpelExpressionValidator => SpelExpressionValidator): SpelExpressionParser = {
new SpelExpressionParser(
parser,
modify(validator),
dictRegistry,
enableSpelForceCompile,
flavour,
prepareEvaluationContext
)
}

}

object SpelExpressionParser extends LazyLogging {
Expand Down Expand Up @@ -286,7 +297,7 @@ object SpelExpressionParser extends LazyLogging {
dictRegistry: DictRegistry,
enableSpelForceCompile: Boolean,
flavour: Flavour,
classDefinitionSet: ClassDefinitionSet
classDefinitionSet: ClassDefinitionSet,
): SpelExpressionParser = {

val parser = new org.springframework.expression.spel.standard.SpelExpressionParser(
Expand All @@ -295,7 +306,7 @@ object SpelExpressionParser extends LazyLogging {
)
val evaluationContextPreparer = EvaluationContextPreparer.default(classLoader, expressionConfig, classDefinitionSet)
val validator = new SpelExpressionValidator(
Typer.default(classLoader, expressionConfig, new KeysDictTyper(dictRegistry), classDefinitionSet)
Typer.default(classLoader, expressionConfig, new KeysDictTyper(dictRegistry), classDefinitionSet, false)
)
new SpelExpressionParser(
parser,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,16 @@ class SpelExpressionSuggester(
classLoader: ClassLoader
) {
private val successfulNil = Future.successful[List[ExpressionSuggestion]](Nil)

private val typer =
Typer.default(classLoader, expressionConfig, new LabelsDictTyper(uiDictServices.dictRegistry), clssDefinitions)
Typer.default(
classLoader,
expressionConfig,
new LabelsDictTyper(uiDictServices.dictRegistry),
clssDefinitions,
false
)

private val nuSpelNodeParser = new NuSpelNodeParser(typer)
private val dictQueryService = uiDictServices.dictQueryService

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ private[spel] class Typer(
classDefinitionSet: ClassDefinitionSet,
evaluationContextPreparer: EvaluationContextPreparer,
anyMethodExecutionForUnknownAllowed: Boolean,
dynamicPropertyAccessAllowed: Boolean
dynamicPropertyAccessAllowed: Boolean,
absentVariableReferenceAllowed: Boolean
) extends LazyLogging {

import ast.SpelAst._
Expand Down Expand Up @@ -499,7 +500,7 @@ private[spel] class Typer(
.get(name)
.orElse(current.stackHead.filter(_ => name == "this"))
.map(valid)
.getOrElse(invalid(UnresolvedReferenceError(name)))
.getOrElse(if (absentVariableReferenceAllowed) valid(Unknown) else invalid(UnresolvedReferenceError(name)))
.map(toNodeResult)
})
}
Expand Down Expand Up @@ -786,7 +787,20 @@ private[spel] class Typer(
classDefinitionSet,
evaluationContextPreparer,
anyMethodExecutionForUnknownAllowed,
dynamicPropertyAccessAllowed
dynamicPropertyAccessAllowed,
absentVariableReferenceAllowed
)

def withAbsentVariableReferenceAllowed(value: Boolean): Typer =
new Typer(
dictTyper,
strictMethodsChecking,
staticMethodInvocationsChecking,
classDefinitionSet,
evaluationContextPreparer,
anyMethodExecutionForUnknownAllowed,
dynamicPropertyAccessAllowed,
absentVariableReferenceAllowed = value
)

}
Expand All @@ -797,7 +811,8 @@ object Typer {
classLoader: ClassLoader,
expressionConfig: ExpressionConfigDefinition,
spelDictTyper: SpelDictTyper,
classDefinitionSet: ClassDefinitionSet
classDefinitionSet: ClassDefinitionSet,
absentVariableReferenceAllowed: Boolean
): Typer = {
val evaluationContextPreparer = EvaluationContextPreparer.default(classLoader, expressionConfig, classDefinitionSet)

Expand All @@ -808,7 +823,8 @@ object Typer {
classDefinitionSet,
evaluationContextPreparer,
expressionConfig.methodExecutionForUnknownAllowed,
expressionConfig.dynamicPropertyAccessAllowed
expressionConfig.dynamicPropertyAccessAllowed,
absentVariableReferenceAllowed
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1070,7 +1070,7 @@ class NodeDataValidatorSpec extends AnyFunSuite with Matchers with Inside with T
}
}

test("shouldn't allow expressions that reference unknown variables in FragmentInputDefinition") {
test("should allow expressions that reference unknown variables in FragmentInputDefinition") {
val nodeId: String = "in"
val invalidReferencingExpression = "#unknownVar"

Expand All @@ -1094,8 +1094,8 @@ class NodeDataValidatorSpec extends AnyFunSuite with Matchers with Inside with T
Map.empty,
outgoingEdges = List(OutgoingEdge("any", Some(FragmentOutput("out1"))))
)
) { case ValidationPerformed((error: ExpressionParserCompilationErrorInFragmentDefinition) :: Nil, None, None) =>
error.message should include("Unresolved reference 'unknownVar'")
) { case ValidationPerformed(errors, None, None) =>
errors shouldBe empty
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ class TyperSpec extends AnyFunSuite with Matchers with ValidatedValuesDetailedMe
classDefinitionSet = ClassDefinitionTestUtils.createDefinitionWithDefaultsAndExtensions,
evaluationContextPreparer = null,
anyMethodExecutionForUnknownAllowed = false,
dynamicPropertyAccessAllowed = dynamicPropertyAccessAllowed
dynamicPropertyAccessAllowed = dynamicPropertyAccessAllowed,
absentVariableReferenceAllowed = false
)

private def typeExpression(
Expand Down

0 comments on commit 16eaf0d

Please sign in to comment.