diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index f3edc9b084..5947fa0cbd 100644 --- a/CHANGELOG.unreleased.md +++ b/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) diff --git a/app/models/annotation/nml/NmlParser.scala b/app/models/annotation/nml/NmlParser.scala index 2d3bd77581..d525f2af8d 100755 --- a/app/models/annotation/nml/NmlParser.scala +++ b/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")) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 323727fc34..b047cfeeb3 100644 --- a/project/Dependencies.scala +++ b/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 } diff --git a/test/backend/NMLUnitTestSuite.scala b/test/backend/NMLUnitTestSuite.scala index b0cb7412dd..77f3dd4f29 100644 --- a/test/backend/NMLUnitTestSuite.scala +++ b/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))) diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/MultiComponentTreeSplitter.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/MultiComponentTreeSplitter.scala new file mode 100644 index 0000000000..0112d74d0a --- /dev/null +++ b/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)) + } + } + } + +} diff --git a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/TreeValidator.scala b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/TreeValidator.scala index 7ade7138a7..99e4e28fa5 100644 --- a/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/TreeValidator.scala +++ b/webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/TreeValidator.scala @@ -3,12 +3,15 @@ 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) @@ -16,6 +19,10 @@ object TreeValidator { _ <- 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] = {