Changes to mproch's comments

TouK · Aug 9, 2022 · 70ddd13 · 70ddd13
1 parent 990dce4
commit 70ddd13
Show file tree

Hide file tree

Showing 9 changed files with 23 additions and 17 deletions.
diff --git a/docs/MigrationGuide.md b/docs/MigrationGuide.md
@@ -4,7 +4,8 @@
 To see the biggest differences please consult the [changelog](Changelog.md).
 
 ## In version 1.6.0 (Not released yet)
-* [#TODO](https://github.com/TouK/nussknacker/pull/TODO) *
+* [#3370](https://github.com/TouK/nussknacker/pull/3370) Feature: scenario node category verification on validation
+  From now import scenario with nodes from other categories than scenario category will be not allowed.
 
 ## In version 1.5.0 (Not released yet)
 

diff --git a/...test/scala/pl/touk/nussknacker/engine/flink/util/transformer/ForEachTransformerSpec.scala b/...test/scala/pl/touk/nussknacker/engine/flink/util/transformer/ForEachTransformerSpec.scala
@@ -43,7 +43,7 @@ class ForEachTransformerSpec extends FunSuite with FlinkSpec with Matchers with
     val model = modelData(List(TestRecord()), collectingListener)
 
     val testProcess = aProcessWithForEachNode(elements = "{'one', 'other'}", resultExpression = s"#$forEachOutputVariableName + '_1'")
-    val processValidator = model.prepareValidatorForCategory("Category1")
+    val processValidator = model.prepareValidatorForCategory(None)
 
     val forEachResultValidationContext = processValidator.validate(testProcess).typing(forEachNodeResultId)
     forEachResultValidationContext.inputValidationContext.get(forEachOutputVariableName) shouldBe Some(Typed[String])

diff --git a/...cala/pl/touk/nussknacker/engine/flink/util/transformer/UnionWithMemoTransformerSpec.scala b/...cala/pl/touk/nussknacker/engine/flink/util/transformer/UnionWithMemoTransformerSpec.scala
@@ -124,7 +124,7 @@ class UnionWithMemoTransformerSpec extends FunSuite with FlinkSpec with Matchers
     val collectingListener = ResultsCollectingListenerHolder.registerRun(identity)
 
     val model = LocalModelData(ConfigFactory.empty(), new UnionWithMemoTransformerSpec.Creator(sourceFoo, sourceBar, collectingListener))
-    val processValidator = model.prepareValidatorForCategory("Category1")
+    val processValidator = model.prepareValidatorForCategory(None)
     val validationResult = processValidator.validate(process).result
 
     val expectedMessage = s"""Input node can not be named "${UnionWithMemoTransformer.KeyField}"""
@@ -165,7 +165,7 @@ class UnionWithMemoTransformerSpec extends FunSuite with FlinkSpec with Matchers
     val collectingListener = ResultsCollectingListenerHolder.registerRun(identity)
 
     val model = LocalModelData(ConfigFactory.empty(), new UnionWithMemoTransformerSpec.Creator(sourceFoo, sourceBar, collectingListener))
-    val processValidator = model.prepareValidatorForCategory("Category1")
+    val processValidator = model.prepareValidatorForCategory(None)
     val validationResult = processValidator.validate(process).result
 
     val expectedMessage = s"""Nodes "$BranchFooId", "$BranchBarId" have too similar names"""

diff --git a/...uk/nussknacker/engine/flink/util/transformer/aggregate/FullOuterJoinTransformerSpec.scala b/...uk/nussknacker/engine/flink/util/transformer/aggregate/FullOuterJoinTransformerSpec.scala
@@ -367,7 +367,7 @@ class FullOuterJoinTransformerSpec extends FunSuite with FlinkSpec with Matchers
     val collectingListener = ResultsCollectingListenerHolder.registerRun(identity)
 
     val model = LocalModelData(ConfigFactory.empty(), new FullOuterJoinTransformerSpec.Creator(sourceFoo, sourceBar, collectingListener))
-    val processValidator = model.prepareValidatorForCategory("Category1")
+    val processValidator = model.prepareValidatorForCategory(None)
     val validationResult = processValidator.validate(process).result
     assert(validationResult.isInvalid)
   }
@@ -408,7 +408,7 @@ class FullOuterJoinTransformerSpec extends FunSuite with FlinkSpec with Matchers
     val collectingListener = ResultsCollectingListenerHolder.registerRun(identity)
 
     val model = LocalModelData(ConfigFactory.empty(), new FullOuterJoinTransformerSpec.Creator(sourceFoo, sourceBar, collectingListener))
-    val processValidator = model.prepareValidatorForCategory("Category1")
+    val processValidator = model.prepareValidatorForCategory(None)
     val validationResult = processValidator.validate(process).result
     assert(validationResult.isInvalid)
   }

diff --git a/.../scala/pl/touk/nussknacker/engine/flink/util/transformer/aggregate/TransformersTest.scala b/.../scala/pl/touk/nussknacker/engine/flink/util/transformer/aggregate/TransformersTest.scala
@@ -41,7 +41,7 @@ class TransformersTest extends FunSuite with FlinkSpec with Matchers with Inside
   def modelData(list: List[TestRecord] = List()): LocalModelData = LocalModelData(ConfigFactory
     .empty().withValue("useTypingResultTypeInformation", fromAnyRef(true)), new Creator(list))
 
-  private val processValidator: ProcessValidator = modelData().prepareValidatorForCategory("Category1")
+  private val processValidator: ProcessValidator = modelData().prepareValidatorForCategory(None)
 
   test("aggregates are properly validated") {
     validateOk("#AGG.approxCardinality","#input.str",  Typed[Long])

diff --git a/interpreter/src/main/scala/pl/touk/nussknacker/engine/ModelData.scala b/interpreter/src/main/scala/pl/touk/nussknacker/engine/ModelData.scala
@@ -94,9 +94,9 @@ trait ModelData extends BaseModelData with AutoCloseable {
   lazy val dictServices: UiDictServices =
     DictServicesFactoryLoader.justOne(modelClassLoader.classLoader).createUiDictServices(processDefinition.expressionConfig.dictionaries, processConfig)
 
-  def prepareValidatorForCategory(category: String): ProcessValidator =
+  def prepareValidatorForCategory(category: Option[String]): ProcessValidator =
     ProcessValidator.default(
-      processWithObjectsDefinition.forCategory(category),
+      category.map(processWithObjectsDefinition.forCategory).getOrElse(processWithObjectsDefinition),
       dictServices.dictRegistry,
       modelClassLoader.classLoader
     )

diff --git a/interpreter/src/main/scala/pl/touk/nussknacker/engine/definition/DefinitionExtractor.scala b/interpreter/src/main/scala/pl/touk/nussknacker/engine/definition/DefinitionExtractor.scala
@@ -87,7 +87,7 @@ object DefinitionExtractor {
     // TODO: Use ContextTransformation API to check if custom node is adding some output variable
     def hasNoReturn: Boolean = Set[TypingResult](Typed[Void], Typed[Unit], Typed[BoxedUnit]).contains(returnType)
 
-    def availableForCategory(category: String): Boolean = categories.isEmpty || categories.exists(_.isEmpty) || categories.exists(_.contains(category))
+    def availableForCategory(category: String): Boolean = categories.isEmpty || categories.exists(_.contains(category))
   }
 
   case class ObjectWithType(obj: Any, typ: TypingResult)

diff --git a/ui/server/src/main/scala/pl/touk/nussknacker/ui/validation/ProcessValidation.scala b/ui/server/src/main/scala/pl/touk/nussknacker/ui/validation/ProcessValidation.scala
@@ -38,7 +38,11 @@ class ProcessValidation(modelData: ProcessingTypeDataProvider[ModelData],
 
   val uiValidationError = "UiValidation"
 
-  private val processValidatorCache = new DefaultCache[String, ProcessValidator](CacheConfig())
+  /**
+   * We cache there model with category as a key, because model can be reloaded.
+   * In consequence of that we have to make sure that we use actual state of model
+   */
+  private val processValidatorCache = new DefaultCache[ValidatorKey, ProcessValidator](CacheConfig())
 
   import pl.touk.nussknacker.engine.util.Implicits._
 
@@ -89,8 +93,8 @@ class ProcessValidation(modelData: ProcessingTypeDataProvider[ModelData],
   }
 
   private def validateUsingTypeValidator(canonical: CanonicalProcess, modelData: ModelData, category: Category): ValidationResult = {
-    val processValidator = processValidatorCache.getOrCreate(category) {
-      val modelCategoryValidator = modelData.prepareValidatorForCategory(category)
+    val processValidator = processValidatorCache.getOrCreate(ValidatorKey(modelData, category)) {
+      val modelCategoryValidator = modelData.prepareValidatorForCategory(Some(category))
 
       expressionParsers
         .map(modelCategoryValidator.withExpressionParsers)
@@ -210,4 +214,6 @@ class ProcessValidation(modelData: ProcessingTypeDataProvider[ModelData],
       .map(_.validate(process))
       .getOrElse(ValidationResult.success)
   }
+
+  private case class ValidatorKey(modelData: ModelData, category: Category)
 }
diff --git a/ui/server/src/test/scala/pl/touk/nussknacker/ui/integration/DictsFlowTest.scala b/ui/server/src/test/scala/pl/touk/nussknacker/ui/integration/DictsFlowTest.scala
@@ -55,10 +55,9 @@ class DictsFlowTest extends FunSuite with ScalatestRouteTest with FailFastCirceS
         "label" -> Json.fromString(Label)))
     }
 
-//FIXME: Request was rejected??
-//    Get(s"/api/88/${TestProcessingTypes.Streaming}/dict/notExisting/entry?label=fo") ~> addCredentials(credentials) ~> mainRoute ~> checkWithClue {
-//      status shouldEqual StatusCodes.NotFound
-//    }
+    Get(s"/api/processDefinitionData/${TestProcessingTypes.Streaming}/dict/notExisting/entry?label=fo") ~> addCredentials(credentials) ~> mainRoute ~> checkWithClue {
+      status shouldEqual StatusCodes.NotFound
+    }
 
     Get(s"/api/processDefinitionData/${TestProcessingTypes.Streaming}/dict/$DictId/entry?label=notexisting") ~> addCredentials(credentials) ~> mainRoute ~> checkWithClue {
       status shouldEqual StatusCodes.OK