improve the backend tree validator

app/models/annotation/nml/NmlParser.scala

@@ -49,6 +49,7 @@ object NmlParser extends LazyLogging with ProtoGeometryImplicits {
         trees <- extractTrees(data \ "thing", branchPoints, comments)
         treeGroups = extractTreeGroups(data \ "groups")
         volumes = extractVolumes(data \ "volume")
+        _ <- TreeValidator.validateAdditionalInformation(trees, branchPoints, comments, treeGroups)
       } yield {
         val dataSetName = parseDataSetName(parameters \ "experiment")
         val description = parseDescription(parameters \ "experiment")

test/backend/NMLUnitTestSuite.scala

@@ -73,16 +73,14 @@ class NMLUnitTestSuite extends FlatSpec with LazyLogging {
     val wrongTree = dummyTracing.trees(1).copy(comments = Seq(Comment(99, "test")))
     val newTracing = dummyTracing.copy(trees = Seq(dummyTracing.trees(0), wrongTree))
 
-    assert(true)//!isParseSuccessful(getParsedTracing(newTracing)))
-    //TODO: The parser currently doesn't check this
+    assert(!isParseSuccessful(getParsedTracing(newTracing)))
   }
 
   it should "throw an error for invalid branchPoint state" in {
     val wrongTree = dummyTracing.trees(1).copy(branchPoints = Seq(BranchPoint(99, 0)))
     val newTracing = dummyTracing.copy(trees = Seq(dummyTracing.trees(0), wrongTree))
 
-    assert(true)//!isParseSuccessful(getParsedTracing(newTracing)))
-    //TODO: The parser currently doesn't check this
+    assert(!isParseSuccessful(getParsedTracing(newTracing)))
   }
 
   it should "throw an error for invalid edge state" in {
@@ -131,14 +129,12 @@ class NMLUnitTestSuite extends FlatSpec with LazyLogging {
     val wrongTree = dummyTracing.trees(1).copy(groupId = Some(9999))
     val newTracing = dummyTracing.copy(trees = Seq(dummyTracing.trees(0), wrongTree))
 
-    assert(true)//!isParseSuccessful(getParsedTracing(newTracing)))
-    //TODO: The parser currently doesn't check this
+    assert(!isParseSuccessful(getParsedTracing(newTracing)))
   }
 
   it should "throw an error for duplicate groupId state" in {
     val newTracing = dummyTracing.copy(treeGroups = TreeGroup("Group", 3) +: dummyTracing.treeGroups)
 
-    assert(true)//!isParseSuccessful(getParsedTracing(newTracing)))
-    //TODO: The parser currently doesn't check this
+    assert(!isParseSuccessful(getParsedTracing(newTracing)))
   }
 }

webknossos-datastore/app/com/scalableminds/webknossos/datastore/tracings/skeleton/TreeValidator.scala

@@ -1,6 +1,6 @@
 package com.scalableminds.webknossos.datastore.tracings.skeleton
 
-import com.scalableminds.webknossos.datastore.SkeletonTracing.{Edge, Tree}
+import com.scalableminds.webknossos.datastore.SkeletonTracing._
 import com.scalableminds.util.datastructures.UnionFind
 import net.liftweb.common.{Box, Failure, Full}
 import net.liftweb.util.A
@@ -9,6 +9,14 @@ import scala.collection.mutable
 
 object TreeValidator {
 
+  def validateAdditionalInformation(trees: Seq[Tree], branchPoints: Seq[BranchPoint], comments: Seq[Comment], treeGroups: Seq[TreeGroup]): Box[Unit] =
+    for{
+      _ <- checkNoDuplicateTreeGroupIds(treeGroups)
+      _ <- checkAllTreeGroupIdsUsedExist(trees, treeGroups)
+      _ <- checkAllNodesUsedInBranchPointsExist(trees, branchPoints)
+      _ <- checkAllNodesUsedInCommentsExist(trees, comments)
+    } yield Full(())
+
   def validateTrees(trees: Seq[Tree]): Box[Unit] = {
     for {
       _ <- checkNoDuplicateTreeIds(trees)
@@ -102,7 +110,51 @@ object TreeValidator {
       }
     }
 
+  private def checkNoDuplicateTreeGroupIds(treeGroups: Seq[TreeGroup]) = {
+    val treeGroupIds = getAllTreeGroupIds(treeGroups, Seq[Int]())
+    val distinctTreeGroupIds = treeGroupIds.distinct
+    if (treeGroupIds.size == distinctTreeGroupIds.size) {
+      Full(())
+    } else {
+      Failure(s"Duplicate treeGroupIds: ${treeGroupIds.diff(distinctTreeGroupIds).mkString(", ")}")
+    }
+  }
+
+  private def checkAllNodesUsedInCommentsExist(trees: Seq[Tree], comments: Seq[Comment]): Box[Unit] = {
+    val nodesInAllTrees = trees.flatMap(_.nodes.map(_.id))
+    val nodesInComments = comments.map(_.nodeId).distinct
+
+    val nodesOnlyInComments = nodesInComments.diff(nodesInAllTrees)
+    if (nodesOnlyInComments.isEmpty) {
+      Full(())
+    } else {
+      Failure(s"Some comments refer to non-existent nodes. comments: ${nodesOnlyInComments.mkString(", ")}")
+    }
+  }
+
+  private def checkAllNodesUsedInBranchPointsExist(trees: Seq[Tree], branchPoints: Seq[BranchPoint]): Box[Unit] = {
+      val nodesInAllTrees = trees.flatMap(_.nodes).map(_.id)
+      val nodesInBranchPoints = branchPoints.map(_.nodeId).distinct
+
+      val nodesOnlyInBranchPoints = nodesInBranchPoints.diff(nodesInAllTrees)
+      if (nodesOnlyInBranchPoints.isEmpty) {
+        Full(())
+      } else {
+        Failure(s"Some branchPoints refer to non-existent nodes. branchPoints: ${nodesOnlyInBranchPoints.mkString(", ")}")
+      }
+    }
+
+  private def checkAllTreeGroupIdsUsedExist(trees: Seq[Tree], treeGroups: Seq[TreeGroup]) = {
+    val existingTreeGroups = getAllTreeGroupIds(treeGroups, Seq[Int]())
+    val treeGroupsInTrees = trees.flatMap(_.groupId).distinct
 
+    val treeGroupsOnlyInTrees = treeGroupsInTrees.diff(existingTreeGroups)
+    if (treeGroupsOnlyInTrees.isEmpty) {
+      Full(())
+    } else {
+      Failure(s"Some treeGroups used in trees don't exist. treeGroups: ${treeGroupsOnlyInTrees.mkString(", ")}")
+    }
+  }
 
   private def foldOverTrees(trees: Seq[Tree])(block: Tree => Box[Unit]) = {
     trees.foldLeft[Box[Unit]](Full(())){
@@ -113,4 +165,11 @@ object TreeValidator {
     }
   }
 
+  private def getAllTreeGroupIds(treeGroups: Seq[TreeGroup], ids: Seq[Int]): Seq[Int] = {
+    treeGroups match {
+      case (head :: tail) => getAllTreeGroupIds(tail ++ head.children, head.groupId +: ids)
+      case _ => ids
+    }
+  }
+
 }