Split Multi-Component Trees in Backend NML Parser

CHANGELOG.unreleased.md

@@ -18,7 +18,8 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
 
 ### Changed
 - Separated the permissions of Team Managers (now actually team-scoped) and Dataset Managers (who can see all datasets). The database evolution makes all Team Managers also Dataset Managers, so no workflows should be interrupted. New users may have to be made Dataset Managers, though. For more information, refer to the updated documentation. [#4663](https://github.com/scalableminds/webknossos/pull/4663)
-- Refined all webKnossos emails for user signups etc. Switched emails to use HTML templates for more bling bling [#4676](https://github.com/scalableminds/webknossos/pull/4676)
+- Refined all webKnossos emails for user signups etc. Switched emails to use HTML templates for more bling bling. [#4676](https://github.com/scalableminds/webknossos/pull/4676)
+- Backend NML parser no longer rejects NMLs with trees that have multiple connected components. Instead, it splits those into one separate tree per component. [#4688](https://github.com/scalableminds/webknossos/pull/4688)
 
 ### Fixed
 - Fixed that merger mode didn't work with undo and redo. Also fixed that the mapping was not disabled when disabling merger mode. [#4669](https://github.com/scalableminds/webknossos/pull/4669)

app/models/annotation/nml/NmlParser.scala

@@ -2,19 +2,20 @@ package models.annotation.nml
 
 import java.io.InputStream
 
-import com.scalableminds.util.geometry.{BoundingBox, Point3D, Scale, Vector3D}
-import com.scalableminds.util.tools.ExtendedTypes.{ExtendedDouble, ExtendedString}
 import com.scalableminds.webknossos.datastore.models.datasource.ElementClass
 import com.scalableminds.webknossos.tracingstore.SkeletonTracing._
 import com.scalableminds.webknossos.tracingstore.VolumeTracing.VolumeTracing
 import com.scalableminds.webknossos.tracingstore.geometry.{Color, NamedBoundingBox}
 import com.scalableminds.webknossos.tracingstore.tracings.{ColorGenerator, ProtoGeometryImplicits}
 import com.scalableminds.webknossos.tracingstore.tracings.skeleton.{
+  MultiComponentTreeSplitter,
   NodeDefaults,
   SkeletonTracingDefaults,
   TreeValidator
 }
 import com.scalableminds.webknossos.tracingstore.tracings.volume.Volume
+import com.scalableminds.util.geometry.{BoundingBox, Point3D, Scale, Vector3D}
+import com.scalableminds.util.tools.ExtendedTypes.{ExtendedString, ExtendedDouble}
 import com.typesafe.scalalogging.LazyLogging
 import net.liftweb.common.Box._
 import net.liftweb.common.{Box, Empty, Failure}
@@ -52,13 +53,13 @@ object NmlParser extends LazyLogging with ProtoGeometryImplicits with ColorGener
         time = parseTime(parameters \ "time")
         comments <- parseComments(data \ "comments")
         branchPoints <- parseBranchPoints(data \ "branchpoints", time)
-        trees <- extractTrees(data \ "thing", branchPoints, comments)
+        trees <- parseTrees(data \ "thing", branchPoints, comments)
         treeGroups <- extractTreeGroups(data \ "groups")
         volumes = extractVolumes(data \ "volume")
-        _ <- TreeValidator.checkNoDuplicateTreeGroupIds(treeGroups)
-        _ <- TreeValidator.checkAllTreeGroupIdsUsedExist(trees, treeGroups)
-        _ <- TreeValidator.checkAllNodesUsedInBranchPointsExist(trees, branchPoints)
-        _ <- TreeValidator.checkAllNodesUsedInCommentsExist(trees, comments)
+        treesAndGroupsAfterSplitting = MultiComponentTreeSplitter.splitMulticomponentTrees(trees, treeGroups)
+        treesSplit = treesAndGroupsAfterSplitting._1
+        treeGroupsAfterSplit = treesAndGroupsAfterSplitting._2
+        _ <- TreeValidator.validateTrees(treesSplit, treeGroupsAfterSplit, branchPoints, comments)
       } yield {
         val dataSetName = overwritingDataSetName.getOrElse(parseDataSetName(parameters \ "experiment"))
         val description = parseDescription(parameters \ "experiment")
@@ -77,7 +78,7 @@ object NmlParser extends LazyLogging with ProtoGeometryImplicits with ColorGener
           case Right(value) => userBoundingBoxes = userBoundingBoxes :+ value
         }
 
-        logger.debug(s"Parsed NML file. Trees: ${trees.size}, Volumes: ${volumes.size}")
+        logger.debug(s"Parsed NML file. Trees: ${treesSplit.size}, Volumes: ${volumes.size}")
 
         val volumeTracingWithDataLocation =
           if (volumes.isEmpty) None
@@ -102,11 +103,11 @@ object NmlParser extends LazyLogging with ProtoGeometryImplicits with ColorGener
             )
 
         val skeletonTracing =
-          if (trees.isEmpty) None
+          if (treesSplit.isEmpty) None
           else
             Some(
               SkeletonTracing(dataSetName,
-                              trees,
+                              treesSplit,
                               time,
                               taskBoundingBox,
                               activeNodeId,
@@ -115,7 +116,7 @@ object NmlParser extends LazyLogging with ProtoGeometryImplicits with ColorGener
                               zoomLevel,
                               version = 0,
                               None,
-                              treeGroups,
+                              treeGroupsAfterSplit,
                               userBoundingBoxes)
             )
 
@@ -135,13 +136,6 @@ object NmlParser extends LazyLogging with ProtoGeometryImplicits with ColorGener
         Failure(s"Failed to parse NML '$name': " + e.toString)
     }
 
-  def extractTrees(treeNodes: NodeSeq, branchPoints: Seq[BranchPoint], comments: Seq[Comment])(
-      implicit m: MessagesProvider): Box[Seq[Tree]] =
-    for {
-      trees <- parseTrees(treeNodes, branchPoints, comments)
-      _ <- TreeValidator.validateTrees(trees)
-    } yield trees
-
   def extractTreeGroups(treeGroupContainerNodes: NodeSeq)(implicit m: MessagesProvider): Box[List[TreeGroup]] = {
     val treeGroupNodes = treeGroupContainerNodes.flatMap(_ \ "group")
     treeGroupNodes.map(parseTreeGroup).toList.toSingleBox(Messages("nml.element.invalid", "tree groups"))

project/Dependencies.scala

@@ -3,7 +3,7 @@ import sbt._
 
 object Dependencies {
   val akkaVersion = "2.5.22"
-  val log4jVersion = "2.0-beta9"
+  val log4jVersion = "2.13.3"
   val webknossosWrapVersion = "1.1.7"
 
   val akkaAgent = "com.typesafe.akka" %% "akka-agent" % akkaVersion
@@ -20,8 +20,8 @@ object Dependencies {
   val scalapbRuntimeGrpc = "com.thesamet.scalapb" %% "scalapb-runtime-grpc" % scalapb.compiler.Version.scalapbVersion
   val liftCommon = "net.liftweb" %% "lift-common" % "3.0.2"
   val liftUtil = "net.liftweb" %% "lift-util" % "3.0.2"
-  val log4jApi = "org.apache.logging.log4j" % "log4j-core" % log4jVersion
-  val log4jCore = "org.apache.logging.log4j" % "log4j-api" % log4jVersion
+  val log4jApi = "org.apache.logging.log4j" % "log4j-core" % log4jVersion % Provided
+  val log4jCore = "org.apache.logging.log4j" % "log4j-api" % log4jVersion % Provided
   val playFramework = "com.typesafe.play" %% "play" % "2.7.1"
   val playJson = "com.typesafe.play" %% "play-json" % "2.7.2"
   val playIteratees = "com.typesafe.play" %% "play-iteratees" % "2.6.1"
@@ -41,6 +41,7 @@ object Dependencies {
   val woodstoxXml = "org.codehaus.woodstox" % "wstx-asl" % "3.2.3"
   val redis = "net.debasishg" %% "redisclient" % "3.9"
   val spire = "org.typelevel" %% "spire" % "0.14.1"
+  val jgrapht = "org.jgrapht" % "jgrapht-core" % "1.4.0"
 
   val sql = Seq(
     "com.typesafe.slick" %% "slick" % "3.2.3",
@@ -82,7 +83,8 @@ object Dependencies {
     grpc,
     grpcServices,
     redis,
-    scalapbRuntimeGrpc
+    scalapbRuntimeGrpc,
+    jgrapht
   )
 
   val webknossosDependencies = Seq(
@@ -98,7 +100,7 @@ object Dependencies {
     triremeNode,
     urlHelper,
     xmlWriter,
-    woodstoxXml
+    woodstoxXml,
   ) ++ sql
 
 }

test/backend/NMLUnitTestSuite.scala

@@ -97,13 +97,6 @@ class NMLUnitTestSuite extends FlatSpec {
     assert(!isParseSuccessful(writeAndParseTracing(newTracing)))
   }
 
-  it should "throw an error for disconnected tree state" in {
-    val wrongTree = dummyTracing.trees(1).copy(edges = Seq(Edge(4, 5)))
-    val newTracing = dummyTracing.copy(trees = Seq(dummyTracing.trees(0), wrongTree))
-
-    assert(!isParseSuccessful(writeAndParseTracing(newTracing)))
-  }
-
   it should "throw an error for duplicate tree state" in {
     val newTracing = dummyTracing.copy(trees = Seq(dummyTracing.trees(0), dummyTracing.trees(0)))
 

webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/MultiComponentTreeSplitter.scala

@@ -0,0 +1,77 @@
+package com.scalableminds.webknossos.tracingstore.tracings.skeleton
+
+import com.scalableminds.webknossos.tracingstore.SkeletonTracing.{Tree, TreeGroup}
+import org.jgrapht.alg.connectivity.ConnectivityInspector
+import org.jgrapht.graph.{Multigraph, _}
+
+import scala.collection.JavaConverters._
+
+object MultiComponentTreeSplitter {
+
+  def splitMulticomponentTrees(trees: Seq[Tree], treeGroups: Seq[TreeGroup]): (Seq[Tree], Seq[TreeGroup]) = {
+    var largestTreeId = if (trees.isEmpty) 0 else trees.map(_.treeId).max
+    var largestGroupId = if (treeGroups.isEmpty) 0 else treeGroups.map(_.groupId).max
+    var treeGroupsMutable: Seq[TreeGroup] = treeGroups
+    val treeLists = trees.map { tree =>
+      val g = new Multigraph[Int, DefaultEdge](classOf[DefaultEdge])
+      tree.nodes.foreach { node =>
+        g.addVertex(node.id)
+      }
+      tree.edges.foreach { edge =>
+        g.addEdge(edge.source, edge.target)
+      }
+      val inspector = new ConnectivityInspector[Int, DefaultEdge](g)
+      val connectedSets: java.util.List[java.util.Set[Int]] = inspector.connectedSets()
+      if (connectedSets.size() <= 1) {
+        List(tree)
+      } else {
+        largestGroupId += 1
+        val newTreeGroup = TreeGroup(tree.name, largestGroupId, List())
+        val parentTreeGroupIdOpt: Option[Int] = tree.groupId
+        parentTreeGroupIdOpt.foreach { parentTreeGroupId =>
+          treeGroupsMutable = addTreeGroupAsChild(treeGroups, parentTreeGroupId, newTreeGroup)
+        }
+        if (parentTreeGroupIdOpt.isEmpty) {
+          treeGroupsMutable = newTreeGroup +: treeGroupsMutable
+        }
+
+        connectedSets.asScala.zipWithIndex.map {
+          case (connectedNodeSet, index) =>
+            val nodes = tree.nodes.filter(node => connectedNodeSet.contains(node.id))
+            val edges = tree.edges.filter(edge =>
+              connectedNodeSet.contains(edge.source) && connectedNodeSet.contains(edge.target))
+            val branchPoints = tree.branchPoints.filter(bp => connectedNodeSet.contains(bp.nodeId))
+            val comments = tree.comments.filter(comment => connectedNodeSet.contains(comment.nodeId))
+            val treeId = largestTreeId
+            val name = tree.name + "_" + index
+            largestTreeId += 1
+            Tree(treeId,
+                 nodes,
+                 edges,
+                 tree.color,
+                 branchPoints,
+                 comments,
+                 name,
+                 tree.createdTimestamp,
+                 Some(largestGroupId),
+                 tree.isVisible)
+        }
+      }
+    }
+    (treeLists.flatten, treeGroupsMutable)
+  }
+
+  def addTreeGroupAsChild(treeGroups: Seq[TreeGroup], parentTreeGroupId: Int, newTreeGroup: TreeGroup): Seq[TreeGroup] =
+    treeGroups.map { treeGroup =>
+      if (treeGroup.groupId == parentTreeGroupId) {
+        treeGroup.copy(children = newTreeGroup +: treeGroup.children)
+      } else {
+        if (treeGroup.children.isEmpty) {
+          treeGroup
+        } else {
+          treeGroup.copy(children = addTreeGroupAsChild(treeGroup.children, parentTreeGroupId, newTreeGroup))
+        }
+      }
+    }
+
+}

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

@@ -3,19 +3,26 @@ package com.scalableminds.webknossos.tracingstore.tracings.skeleton
 import com.scalableminds.webknossos.tracingstore.SkeletonTracing._
 import com.scalableminds.util.datastructures.UnionFind
 import net.liftweb.common.{Box, Failure, Full}
-import net.liftweb.util.A
 
 import scala.collection.mutable
 
 object TreeValidator {
-  def validateTrees(trees: Seq[Tree]): Box[Unit] =
+
+  def validateTrees(trees: Seq[Tree],
+                    treeGroups: Seq[TreeGroup],
+                    branchPoints: Seq[BranchPoint],
+                    comments: Seq[Comment]): Box[Unit] =
     for {
       _ <- checkNoDuplicateTreeIds(trees)
       _ <- checkNoDuplicateNodeIds(trees)
       _ <- checkNoDuplicateEdges(trees)
       _ <- checkAllNodesUsedInEdgesExist(trees)
       _ <- checkNoEdgesWithSameSourceAndTarget(trees)
       _ <- checkTreesAreConnected(trees)
+      _ <- checkNoDuplicateTreeGroupIds(treeGroups)
+      _ <- checkAllTreeGroupIdsUsedExist(trees, treeGroups)
+      _ <- checkAllNodesUsedInBranchPointsExist(trees, branchPoints)
+      _ <- checkAllNodesUsedInCommentsExist(trees, comments)
     } yield Full(())
 
   private def checkNoDuplicateTreeIds(trees: Seq[Tree]): Box[Unit] = {