#368 improve structure analysis for roundabouts

server/src/main/scala/kpn/server/analyzer/engine/analysis/route/RouteDetailMainAnalyzer.scala

@@ -28,6 +28,7 @@ import kpn.server.analyzer.engine.analysis.route.analyzers.RouteMemberAnalyzer
 import kpn.server.analyzer.engine.analysis.route.analyzers.RouteNameAnalyzer
 import kpn.server.analyzer.engine.analysis.route.analyzers.RouteNetworkTypeAnalyzer
 import kpn.server.analyzer.engine.analysis.route.analyzers.RouteNodeAnalyzer
+import kpn.server.analyzer.engine.analysis.route.analyzers.RouteOneWayAnalyzer
 import kpn.server.analyzer.engine.analysis.route.analyzers.RouteStructureAnalyzer
 import kpn.server.analyzer.engine.analysis.route.analyzers.RouteTagAnalyzer
 import kpn.server.analyzer.engine.analysis.route.analyzers.RouteTileAnalyzer
@@ -80,6 +81,7 @@ class RouteDetailMainAnalyzer(
         RouteLinkAnalyzer,
         RouteNodeAnalyzer,
         RouteSegmentAnalyzer,
+        RouteOneWayAnalyzer,
         RouteStructureAnalyzer,
 
         RouteMemberAnalyzer,

server/src/main/scala/kpn/server/analyzer/engine/analysis/route/analyzers/RouteOneWayAnalyzer.scala

@@ -0,0 +1,25 @@
+package kpn.server.analyzer.engine.analysis.route.analyzers
+
+import kpn.server.analyzer.engine.analysis.route.domain.RouteDetailAnalysisContext
+
+object RouteOneWayAnalyzer extends RouteAnalyzer {
+  def analyze(context: RouteDetailAnalysisContext): RouteDetailAnalysisContext = {
+    new RouteOneWayAnalyzer(context).analyze
+  }
+}
+
+class RouteOneWayAnalyzer(context: RouteDetailAnalysisContext) {
+
+  def analyze: RouteDetailAnalysisContext = {
+
+    val oneWayRouteForward = context.relation.hasTag("direction", "forward")
+    val oneWayRouteBackward = context.relation.hasTag("direction", "backward")
+
+    val oneWayRoute = context.relation.hasTag("oneway", "yes") || context.relation.hasTag("signed_direction", "yes")
+
+    context.copy(
+      _oneWayRouteForward = Some(oneWayRouteForward || oneWayRoute),
+      _oneWayRouteBackward = Some(oneWayRouteBackward),
+    )
+  }
+}

server/src/main/scala/kpn/server/analyzer/engine/analysis/route/analyzers/RouteStructureAnalyzer.scala

@@ -33,23 +33,15 @@ class RouteStructureAnalyzer(context: RouteDetailAnalysisContext) {
 
     if (!Seq(RouteAnalysisFailed, RouteWithoutNodes, RouteNodeMissingInWays).exists(context.facts.contains)) {
 
-      val oneWayRouteForward = context.relation.hasTag("direction", "forward")
-      val oneWayRouteBackward = context.relation.hasTag("direction", "backward")
-
-      val oneWayRoute = context.relation.hasTag("oneway", "yes") || context.relation.hasTag("signed_direction", "yes")
-
-      val hasValidForwardPath = structure.forwardPath.isDefined // TODO redesign && !structure.forwardPath.exists(_.broken)
-      val hasValidBackwardPath = structure.backwardPath.isDefined // TODO redesign && !structure.backwardPath.exists(_.broken)
-
-      if (hasValidForwardPath) {
-        if (hasValidBackwardPath) {
-          if (oneWayRoute || oneWayRouteForward || oneWayRouteBackward) {
+      if (structure.forwardPath.isDefined) {
+        if (structure.backwardPath.isDefined) {
+          if (context.oneWayRouteForward || context.oneWayRouteBackward) {
             facts += RouteNotOneWay
           }
         }
         else {
           if (!isSingleWayRoundabout()) {
-            if (oneWayRoute || oneWayRouteForward) {
+            if (context.oneWayRouteForward) {
               facts += RouteOneWay
             }
             else {
@@ -58,16 +50,16 @@ class RouteStructureAnalyzer(context: RouteDetailAnalysisContext) {
           }
         }
       }
-      else if (hasValidBackwardPath) {
-        if (oneWayRoute || oneWayRouteBackward) {
+      else if (structure.backwardPath.isDefined) {
+        if (context.oneWayRouteBackward) {
           facts += RouteOneWay
         }
         else {
           facts += RouteNotForward
         }
       }
       else {
-        if (oneWayRoute || oneWayRouteForward || oneWayRouteBackward) {
+        if (context.oneWayRouteForward || context.oneWayRouteBackward) {
           facts += RouteNotOneWay
         }
         facts += RouteNotForward

server/src/main/scala/kpn/server/analyzer/engine/analysis/route/domain/RouteDetailAnalysisContext.scala

@@ -36,6 +36,8 @@ case class RouteDetailAnalysisContext(
   _nodes: Option[RouteAnalysisNodes] = None,
   expectedName: Option[String] = None,
   suspiciousWayIds: Option[Seq[Long]] = None,
+  _oneWayRouteForward: Option[Boolean] = None,
+  _oneWayRouteBackward: Option[Boolean] = None,
   _structure: Option[Structure] = None,
   _routeMembers: Option[Seq[RouteMember]] = None,
   allWayNodes: Option[Seq[Node]] = None,
@@ -113,6 +115,10 @@ case class RouteDetailAnalysisContext(
 
   def nodes: RouteAnalysisNodes = _nodes.getOrElse(throw new PreconditionMissingException)
 
+  def oneWayRouteForward: Boolean = _oneWayRouteForward.getOrElse(throw new PreconditionMissingException)
+
+  def oneWayRouteBackward: Boolean = _oneWayRouteBackward.getOrElse(throw new PreconditionMissingException)
+
   def structure: Structure = _structure.getOrElse(throw new PreconditionMissingException)
 
   def routeMembers: Seq[RouteMember] = _routeMembers.getOrElse(throw new PreconditionMissingException)

server/src/main/scala/kpn/server/analyzer/engine/analysis/route/structure/StructureAnalyzer.scala

@@ -52,8 +52,18 @@ class StructureAnalyzer(context: RouteDetailAnalysisContext, traceEnabled: Boole
     mainEndNode: RouteAnalysisNode
   ): Option[Structure] = {
 
-    val forwardPath = nodeNetworkForwardPath(mainStartNode, mainEndNode)
-    val backwardPath = nodeNetworkBackwardPath(mainStartNode, mainEndNode)
+    val forwardPath = if (context.oneWayRouteBackward) {
+      None
+    }
+    else {
+      nodeNetworkForwardPath(mainStartNode, mainEndNode)
+    }
+    val backwardPath = if (context.oneWayRouteForward) {
+      None
+    }
+    else {
+      nodeNetworkBackwardPath(mainStartNode, mainEndNode)
+    }
     val startTentaclePaths = nodeNetworkStartTentaclePaths(forwardPath, backwardPath)
     val endTentaclePaths = nodeNetworkEndTentaclePaths(forwardPath, backwardPath, startTentaclePaths)
     val otherPaths = nodeNetworkOtherPaths(forwardPath, backwardPath, startTentaclePaths, endTentaclePaths)
@@ -225,36 +235,46 @@ class StructureAnalyzer(context: RouteDetailAnalysisContext, traceEnabled: Boole
     }
     else {
       val forwardPath = {
-        val elements = findNonNodeNetworkRouteForwardPath(Seq.empty, context.segments.flatMap(_.elements))
-        if (elements.nonEmpty) {
-          Some(
-            StructurePath(
-              pathIds.next(),
-              elements.head.startNodeId,
-              elements.last.endNodeId,
-              elements
-            )
-          )
+        if (context.oneWayRouteBackward) {
+          None
         }
         else {
-          None
+          val elements = findNonNodeNetworkRouteForwardPath(Seq.empty, context.segments.flatMap(_.elements))
+          if (elements.nonEmpty) {
+            Some(
+              StructurePath(
+                pathIds.next(),
+                elements.head.startNodeId,
+                elements.last.endNodeId,
+                elements
+              )
+            )
+          }
+          else {
+            None
+          }
         }
       }
 
       val backwardPath = {
-        val elements = findNonNodeNetworkRouteBackwardPath(Seq.empty, context.segments.flatMap(_.elements).reverse)
-        if (elements.nonEmpty) {
-          Some(
-            domain.StructurePath(
-              pathIds.next(),
-              elements.head.startNodeId,
-              elements.last.endNodeId,
-              elements
-            )
-          )
+        if (context.oneWayRouteForward) {
+          None
         }
         else {
-          None
+          val elements = findNonNodeNetworkRouteBackwardPath(Seq.empty, context.segments.flatMap(_.elements).reverse)
+          if (elements.nonEmpty) {
+            Some(
+              domain.StructurePath(
+                pathIds.next(),
+                elements.head.startNodeId,
+                elements.last.endNodeId,
+                elements
+              )
+            )
+          }
+          else {
+            None
+          }
         }
       }
 

server/src/test/scala/kpn/server/analyzer/engine/analysis/route/structure/test/StructureTestSetupBuilder.scala

@@ -46,6 +46,17 @@ class StructureTestSetupBuilder extends SharedTestObjects {
     )
   }
 
+  def nodeWithTags(id: Long, tags: Seq[Tag]): RawNode = {
+    rawNode(
+      newRawNode(
+        id,
+        "0",
+        "0",
+        tags = tags
+      )
+    )
+  }
+
   private def rawNode(rawNode: RawNode): RawNode = {
     nodeBuffer += rawNode
     rawNode

server/src/test/scala/kpn/server/analyzer/engine/analysis/route/structure/test/Structure_N20_Test.scala

@@ -1,5 +1,6 @@
 package kpn.server.analyzer.engine.analysis.route.structure
 
+import kpn.api.custom.Fact.RouteOneWay
 import kpn.api.custom.ScopedNetworkType
 import kpn.api.custom.Tags
 import kpn.core.util.UnitTest
@@ -9,24 +10,17 @@ import kpn.server.analyzer.engine.analysis.route.structure.test.StructureTestSet
 class Structure_N20_Test extends UnitTest {
 
   private def setup = new StructureTestSetupBuilder() {
-    node(1, "01")
-    node(3, "02")
+    nodeWithTags(1, Tags.from("network:type" -> "node_network", "rcn_ref" -> "01"))
+    nodeWithTags(3, Tags.from("network:type" -> "node_network", "rcn_ref" -> "02"))
     memberWayWithTags(10, "", Tags.from("highway" -> "road", "oneway" -> "yes"), 1, 2, 3)
   }.build("01", "02", ScopedNetworkType.rcn, Tags.from("direction" -> "forward"))
 
   test("analyze") {
 
     val context = setup.analyze()
 
-    context.facts.foreach(a => println(s""""$a","""))
-    context.links.foreach(a => println(s""""$a","""))
-    context.nodes.foreach(a => println(s""""$a","""))
-    context.segments.foreach(a => println(s""""$a","""))
-    context.paths.foreach(a => println(s""""$a","""))
+    context.facts.shouldMatchTo(Set(RouteOneWay))
 
-    pending
-
-    // TODO context.facts.shouldMatchTo(Set(RouteOneWay))
     context.links.shouldMatchTo(
       Seq(
         "1    p     n     loop     fp     bp     head     tail     d unconnected",
@@ -48,11 +42,9 @@ class Structure_N20_Test extends UnitTest {
       )
     )
 
-    pending
     context.paths.shouldMatchTo(
       Seq(
         "forward=1>3 nodes=1, 2, 3",
-        "backward=1>3 nodes=3, 2, 1", // TODO there should be no backward path
       )
     )
   }