Skip to content

Commit a97109c

Browse files
DeamonDevDeamonDev
authored
[Nu-7983] Improve FragmentParameterTypingParser (#7985)
* [Nu-7983] Improve FragmentParameterTypingParser * [Nu-7983] Add test for unbalanced brackets * [Nu-7983] Remove redundant implicit dependency * [Nu-7983] Add even more test * [Nu-7983] Optimize imports * [Nu-7983] Simple rename * [Nu-7983] Reorder parsers in code * [Nu-7983] Add one more test
1 parent 65ae690 commit a97109c

File tree

4 files changed

+152
-29
lines changed

4 files changed

+152
-29
lines changed

‎build.sbt

+2
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,7 @@ val jacksonV = "2.17.2"
328328
val catsV = "2.12.0"
329329
val catsEffectV = "3.5.4"
330330
val everitSchemaV = "1.14.4"
331+
val fastParseV = "3.1.1"
331332
val slf4jV = "1.7.36"
332333
val scalaLoggingV = "3.9.5"
333334
val scalaCompatV = "1.0.2"
@@ -848,6 +849,7 @@ lazy val scenarioCompiler = (project in file("scenario-compiler"))
848849
Seq(
849850
"org.typelevel" %% "cats-effect" % catsEffectV,
850851
"org.scala-lang.modules" %% "scala-java8-compat" % scalaCompatV,
852+
"com.lihaoyi" %% "fastparse" % fastParseV,
851853
"org.apache.avro" % "avro" % avroV % Test,
852854
"org.scalacheck" %% "scalacheck" % scalaCheckV % Test,
853855
"com.cronutils" % "cron-utils" % cronParserV % Test,

‎scenario-compiler/src/main/scala/pl/touk/nussknacker/engine/definition/fragment/FragmentParameterTypingParser.scala

+38-29
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,54 @@
11
package pl.touk.nussknacker.engine.definition.fragment
22

3+
import fastparse._
34
import org.apache.commons.lang3.ClassUtils
45
import pl.touk.nussknacker.engine.api.typed.typing.{Typed, TypingResult}
56
import pl.touk.nussknacker.engine.definition.clazz.ClassDefinitionSet
67

78
import scala.util.Try
89

10+
import SingleLineWhitespace._
11+
912
class FragmentParameterTypingParser(classLoader: ClassLoader, classDefinitions: ClassDefinitionSet) {
1013

11-
private val mapPattern = "Map\\[(.+),\\s*(.+)\\]".r
12-
private val listPattern = "List\\[(.+)\\]".r
13-
private val setPattern = "Set\\[(.+)\\]".r
14+
private def identifier[_: P]: P[String] =
15+
P(CharIn("a-zA-Z_") ~ CharsWhileIn("a-zA-Z0-9_.", 0)).!
16+
17+
private def simpleType[_: P]: P[TypingResult] =
18+
identifier.map(resolveInnerClass)
19+
20+
private def mapType[_: P]: P[TypingResult] =
21+
P("Map[" ~/ typParser ~ "," ~ typParser ~ "]")
22+
.map { case (tr1, tr2) => Typed.genericTypeClass[java.util.Map[_, _]](List(tr1, tr2)) }
23+
24+
private def listType[_: P]: P[TypingResult] =
25+
P("List[" ~/ typParser ~ "]")
26+
.map(tr => Typed.genericTypeClass[java.util.List[_]](List(tr)))
27+
28+
private def setType[_: P]: P[TypingResult] =
29+
P("Set[" ~/ typParser ~ "]")
30+
.map(tr => Typed.genericTypeClass[java.util.Set[_]](List(tr)))
31+
32+
private def typParser[_: P]: P[TypingResult] =
33+
P(mapType | listType | setType | simpleType)
34+
1435
private val classDefinitionsByName = classDefinitions.byName
1536

37+
private def resolveInnerClass(simpleClassName: String): TypingResult =
38+
classDefinitionsByName.get(simpleClassName) match {
39+
case Some(resolvedClassDefinition) =>
40+
resolvedClassDefinition.clazzName
41+
case None =>
42+
// This is fallback - it may be removed and `ClassNotFound` exception may be thrown here after cleaning up the mess with `FragmentClazzRef` class
43+
Typed(ClassUtils.getClass(classLoader, simpleClassName))
44+
}
45+
1646
def parseClassNameToTypingResult(className: String): Try[TypingResult] = {
17-
/*
18-
TODO: Write this parser in a way that handles arbitrary depth expressions
19-
One should not use regexes for doing so and rather build AST
20-
*/
21-
def resolveInnerClass(simpleClassName: String): TypingResult =
22-
classDefinitionsByName.get(simpleClassName) match {
23-
case Some(resolvedClassDefinition) =>
24-
resolvedClassDefinition.clazzName
25-
case None =>
26-
// This is fallback - it may be removed and `ClassNotFound` exception may be thrown here after cleaning up the mess with `FragmentClazzRef` class
27-
Typed(ClassUtils.getClass(classLoader, simpleClassName))
28-
}
29-
30-
Try(className match {
31-
case mapPattern(x, y) =>
32-
val resolvedFirstTypeParam = resolveInnerClass(x)
33-
val resolvedSecondTypeParam = resolveInnerClass(y)
34-
Typed.genericTypeClass[java.util.Map[_, _]](List(resolvedFirstTypeParam, resolvedSecondTypeParam))
35-
case listPattern(x) =>
36-
val resolvedTypeParam = resolveInnerClass(x)
37-
Typed.genericTypeClass[java.util.List[_]](List(resolvedTypeParam))
38-
case setPattern(x) =>
39-
val resolvedTypeParam = resolveInnerClass(x)
40-
Typed.genericTypeClass[java.util.Set[_]](List(resolvedTypeParam))
41-
case simpleClassName =>
42-
resolveInnerClass(simpleClassName)
47+
48+
Try(parse(className, implicit ctx => typParser ~ End) match {
49+
case Parsed.Success(typingResult, _) => typingResult
50+
case Parsed.Failure(label, index, extra) =>
51+
throw new IllegalArgumentException(s"Parsing failed at $index: $label\n${extra.trace().longMsg}")
4352
})
4453
}
4554

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

+65
Original file line numberDiff line numberDiff line change
@@ -1160,6 +1160,71 @@ class NodeDataValidatorSpec extends AnyFunSuite with Matchers with Inside with T
11601160
}
11611161
}
11621162

1163+
test("should allow usage of nested generic type in FragmentInputDefinition parameter") {
1164+
val nodeId: String = "in"
1165+
val paramName = "param1"
1166+
1167+
inside(
1168+
validate(
1169+
FragmentInputDefinition(
1170+
nodeId,
1171+
List(
1172+
FragmentParameter(
1173+
ParameterName(paramName),
1174+
FragmentClazzRef("Map[List[Integer], Map[String, Map[Double, List[Integer]]]]"),
1175+
required = false,
1176+
initialValue = None,
1177+
hintText = None,
1178+
valueEditor = None,
1179+
valueCompileTimeValidation = None
1180+
)
1181+
),
1182+
),
1183+
ValidationContext.empty,
1184+
Map.empty,
1185+
outgoingEdges = List(OutgoingEdge("any", Some(FragmentOutput("out1"))))
1186+
)
1187+
) { case ValidationPerformed(errors, None, None) =>
1188+
errors shouldBe empty
1189+
}
1190+
}
1191+
1192+
test("should not allow type definition with unbalanced brackets") {
1193+
val nodeId: String = "in"
1194+
val paramName = "param1"
1195+
val additionalBracket = "]"
1196+
1197+
inside(
1198+
validate(
1199+
FragmentInputDefinition(
1200+
nodeId,
1201+
List(
1202+
FragmentParameter(
1203+
ParameterName(paramName),
1204+
FragmentClazzRef(s"Map[List[Integer], Map[String, Map[Double, List[Integer]]]]$additionalBracket"),
1205+
required = false,
1206+
initialValue = None,
1207+
hintText = None,
1208+
valueEditor = None,
1209+
valueCompileTimeValidation = None
1210+
)
1211+
),
1212+
),
1213+
ValidationContext.empty,
1214+
Map.empty,
1215+
outgoingEdges = List(OutgoingEdge("any", Some(FragmentOutput("out1"))))
1216+
)
1217+
) { case ValidationPerformed(errors, None, None) =>
1218+
errors shouldBe List(
1219+
FragmentParamClassLoadError(
1220+
ParameterName("param1"),
1221+
"Map[List[Integer], Map[String, Map[Double, List[Integer]]]]]",
1222+
"in"
1223+
)
1224+
)
1225+
}
1226+
}
1227+
11631228
test(
11641229
"should not allow usage of generic type in FragmentInputDefinition parameter when occurring type is not on classpath"
11651230
) {

‎scenario-compiler/src/test/scala/pl/touk/nussknacker/engine/definition/fragment/FragmentParameterTypingParserSpec.scala

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package pl.touk.nussknacker.engine.definition.fragment
2+
3+
import org.scalatest.funsuite.AnyFunSuite
4+
import org.scalatest.matchers.should.Matchers
5+
import pl.touk.nussknacker.engine.api.typed.typing.TypedClass
6+
import pl.touk.nussknacker.engine.definition.clazz.{ClassDefinitionSet, ClassDefinitionTestUtils}
7+
8+
import scala.util.{Failure, Success}
9+
10+
class FragmentParameterTypingParserSpec extends AnyFunSuite with Matchers {
11+
private val classLoader = getClass.getClassLoader
12+
13+
private val classDefinitionExtractor = ClassDefinitionTestUtils.DefaultExtractor
14+
15+
private val clazzDefinitions: ClassDefinitionSet = ClassDefinitionSet(
16+
Set(
17+
classDefinitionExtractor.extract(classOf[String]),
18+
classDefinitionExtractor.extract(classOf[Integer]),
19+
)
20+
)
21+
22+
private val fragmentParameterTypingParser = new FragmentParameterTypingParser(classLoader, clazzDefinitions)
23+
24+
test("should parse nested generic Map type into TypedClass") {
25+
val triedTypingResult =
26+
fragmentParameterTypingParser.parseClassNameToTypingResult("Map[List[Integer], Map[String, List[Integer]]]")
27+
28+
triedTypingResult match {
29+
case Failure(ex) => fail(s"Unexpected failure: $ex")
30+
case Success(tc: TypedClass) =>
31+
tc.display shouldBe "Map[List[Integer],Map[String,List[Integer]]]"
32+
case Success(tr) =>
33+
fail(s"Expected a result type which is TypedClass but got instead: ${tr.getClass.getSimpleName}")
34+
}
35+
}
36+
37+
test("should fail to parse a class name when it contains a non-existing type") {
38+
val triedTypingResult =
39+
fragmentParameterTypingParser.parseClassNameToTypingResult("Map[String, NotExistingType]")
40+
41+
triedTypingResult match {
42+
case Failure(ex) => ex shouldBe a[ClassNotFoundException]
43+
case Success(tr) => fail(s"Expected failure due to non-existing type, but got success with: $tr")
44+
}
45+
}
46+
47+
}

0 commit comments

Comments
 (0)