Code refactored for atlas start and end route paths (#798)

* Code refactored for atlas start and end route paths * Added comment * Comments corrected --------- Co-authored-by: Raju Gatla <rajugatla@Rajus-MacBook-Pro.local>
osmlab · Nov 10, 2023 · 08a08d8 · 08a08d8
1 parent f525a7a
commit 08a08d8
Show file tree

Hide file tree

Showing 6 changed files with 91 additions and 22 deletions.
diff --git a/gradle.properties b/gradle.properties
@@ -1,5 +1,5 @@
 group=org.openstreetmap.atlas
-version=7.0.9-SNAPSHOT
+version=7.0.10-SNAPSHOT
 
 maven2_url=https://oss.sonatype.org/service/local/staging/deploy/maven2/
 snapshot_url=https://oss.sonatype.org/content/repositories/snapshots/

diff --git a/src/main/java/org/openstreetmap/atlas/geography/atlas/routing/AllPathsRouter.java b/src/main/java/org/openstreetmap/atlas/geography/atlas/routing/AllPathsRouter.java
@@ -193,7 +193,8 @@ private static void allRoutes(final Edge start, final Edge end, final Stack<Edge
         }
         // Add start edge to the path
         path.push(start);
-        onPath.add(start.getIdentifier());
+        // This will avoid adding same edge both in forward and reverse direction
+        onPath.add(start.end().getIdentifier());
 
         if (start.equals(end))
         {
@@ -203,7 +204,7 @@ private static void allRoutes(final Edge start, final Edge end, final Stack<Edge
             if (routes.size() > maximumAllowedPaths)
             {
                 logger.warn("Too many paths found - aborting! Path so far: {}",
-                        path.stream().map(edge -> String.valueOf(edge.getIdentifier()))
+                        path.stream().map(edge -> String.valueOf(edge.getMainEdgeIdentifier()))
                                 .collect(Collectors.toList()));
             }
         }
@@ -214,7 +215,10 @@ private static void allRoutes(final Edge start, final Edge end, final Stack<Edge
             // given filter
             for (final Edge candidate : start.outEdges())
             {
-                if (!candidate.isZeroLength() && !onPath.contains(candidate.getIdentifier())
+                // Proceed if we have not yet visited the edge in any direction (It would be really
+                // weired
+                // to revisit an edge in a BigNode, both in positive and negative directions.
+                if (!candidate.isZeroLength() && !onPath.contains(candidate.end().getIdentifier())
                         && (filter.test(candidate) || candidate.equals(end)))
                 {
                     allRoutes(candidate, end, path, onPath, routes, filter, maximumAllowedPaths);
@@ -224,7 +228,7 @@ private static void allRoutes(final Edge start, final Edge end, final Stack<Edge
 
         // We've explored all paths that go through this edge. Remove it from consideration
         path.pop();
-        onPath.remove(start.getIdentifier());
+        onPath.remove(start.end().getIdentifier());
     }
 
     private AllPathsRouter()

diff --git a/...java/org/openstreetmap/atlas/geography/atlas/items/complex/bignode/BigNodeFinderTest.java b/...java/org/openstreetmap/atlas/geography/atlas/items/complex/bignode/BigNodeFinderTest.java
@@ -313,7 +313,7 @@ public void testPathsThroughComplexJunction()
         logger.info("Atlas: {}", atlas);
         final List<BigNode> bigNodes = Iterables.asList(new BigNodeFinder().find(atlas));
         bigNodes.forEach(complexEntity -> logger.info("{}", complexEntity.toString()));
-        Assert.assertEquals("Expect to find 7 Big Nodes for this atlas", 7, bigNodes.size());
+        Assert.assertEquals("Expect to find 9 Big Nodes for this atlas", 9, bigNodes.size());
 
         Time timeNow = Time.now();
         final Set<Route> shortestRoutes = new HashSet<>();
@@ -322,7 +322,7 @@ public void testPathsThroughComplexJunction()
                 shortestRoutes.size());
 
         logger.info("Big Node Shortest Routes: {} ", shortestRoutes);
-        Assert.assertEquals("Expect to find 18 shortest paths through these Big Nodes", 18,
+        Assert.assertEquals("Expect to find 16 shortest paths through these Big Nodes", 16,
                 shortestRoutes.size());
 
         timeNow = Time.now();
@@ -333,14 +333,14 @@ public void testPathsThroughComplexJunction()
                 allRoutes.size());
 
         logger.info("Big Node All Routes: {} ", allRoutes);
-        Assert.assertEquals("Expect to find 26 total paths through these Big Nodes", 26,
+        Assert.assertEquals("Expect to find 28 total paths through these Big Nodes", 28,
                 allRoutes.size());
 
         Assert.assertTrue("Make sure the shortest routes are a subset of allRoutes",
                 allRoutes.containsAll(shortestRoutes));
 
-        final Route nonShortestValidRoute = Route.forEdges(atlas.edge(-3), atlas.edge(-2),
-                atlas.edge(2), atlas.edge(3));
+        final Route nonShortestValidRoute = Route.forEdges(atlas.edge(4), atlas.edge(11),
+                atlas.edge(-12), atlas.edge(-9), atlas.edge(3));
         Assert.assertTrue("Valid route should be absent from the shortest path set",
                 !shortestRoutes.contains(nonShortestValidRoute));
         Assert.assertTrue("Valid route should be present in the total path set",

diff --git a/.../openstreetmap/atlas/geography/atlas/items/complex/bignode/BigNodeFinderTestCaseRule.java b/.../openstreetmap/atlas/geography/atlas/items/complex/bignode/BigNodeFinderTestCaseRule.java
@@ -575,6 +575,79 @@ public class BigNodeFinderTestCaseRule extends CoreTestRule
                                     "name=West Davison Avenue", "oneway=yes" }) })
     private Atlas complexJunctionAtlas;
 
+    @TestAtlas(
+
+            nodes = { @Node(id = "49", coordinates = @Loc(value = FORTYNINE)),
+                    @Node(id = "50", coordinates = @Loc(value = FIFTY)),
+                    @Node(id = "51", coordinates = @Loc(value = FIFTYONE)),
+                    @Node(id = "52", coordinates = @Loc(value = FIFTYTWO)),
+                    @Node(id = "53", coordinates = @Loc(value = FIFTYTHREE)),
+                    @Node(id = "54", coordinates = @Loc(value = FIFTYFOUR)),
+                    @Node(id = "55", coordinates = @Loc(value = FIFTYFIVE)),
+                    @Node(id = "56", coordinates = @Loc(value = FIFTYSIX)),
+                    @Node(id = "57", coordinates = @Loc(value = FIFTYSEVEN)),
+                    @Node(id = "58", coordinates = @Loc(value = FIFTYEIGHT)),
+                    @Node(id = "59", coordinates = @Loc(value = FIFTYNINE)),
+                    @Node(id = "60", coordinates = @Loc(value = SIXTY)), },
+
+            edges = {
+                    @Edge(id = "1", coordinates = { @Loc(value = FORTYNINE),
+                            @Loc(value = FIFTYTWO) }, tags = { "highway=secondary",
+                                    "name=Soorkie Avenue" }),
+                    @Edge(id = "2", coordinates = { @Loc(value = FIFTYTHREE),
+                            @Loc(value = FIFTY) }, tags = { "highway=secondary",
+                                    "name=Soorkie Avenue" }),
+
+                    @Edge(id = "3", coordinates = { @Loc(value = FIFTYTWO),
+                            @Loc(value = FIFTYONE) }, tags = { "highway=secondary",
+                                    "name=Biryani Avenue" }),
+                    @Edge(id = "4", coordinates = { @Loc(value = FIFTYFIVE),
+                            @Loc(value = FIFTYSIX) }, tags = { "highway=secondary",
+                                    "name=Biryani Avenue" }),
+
+                    @Edge(id = "5", coordinates = { @Loc(value = FIFTYSIX),
+                            @Loc(value = FIFTYNINE) }, tags = { "highway=secondary",
+                                    "name=Soorkie Avenue" }),
+                    @Edge(id = "6", coordinates = { @Loc(value = SIXTY),
+                            @Loc(value = FIFTYSEVEN) }, tags = { "highway=secondary",
+                                    "name=Soorkie Avenue" }),
+
+                    @Edge(id = "7", coordinates = { @Loc(value = FIFTYSEVEN),
+                            @Loc(value = FIFTYEIGHT) }, tags = { "highway=secondary",
+                                    "name=Biryani Avenue" }),
+                    @Edge(id = "8", coordinates = { @Loc(value = FIFTYFOUR),
+                            @Loc(value = FIFTYTHREE) }, tags = { "highway=secondary",
+                                    "name=Biryani Avenue" }),
+
+                    @Edge(id = "9", coordinates = { @Loc(value = FIFTYTWO),
+                            @Loc(value = FIFTYTHREE) }, tags = { "highway=secondary",
+                                    "name=Biryani Avenue" }),
+                    @Edge(id = "-9", coordinates = { @Loc(value = FIFTYTHREE),
+                            @Loc(value = FIFTYTWO) }, tags = { "highway=secondary",
+                                    "name=Biryani Avenue" }),
+
+                    @Edge(id = "10", coordinates = { @Loc(value = FIFTYTWO),
+                            @Loc(value = FIFTYSIX) }, tags = { "highway=secondary",
+                                    "name=Biryani Avenue" }),
+                    @Edge(id = "-10", coordinates = { @Loc(value = FIFTYSIX),
+                            @Loc(value = FIFTYTWO) }, tags = { "highway=secondary",
+                                    "name=Biryani Avenue" }),
+
+                    @Edge(id = "11", coordinates = { @Loc(value = FIFTYSIX),
+                            @Loc(value = FIFTYSEVEN) }, tags = { "highway=secondary",
+                                    "name=Biryani Avenue" }),
+                    @Edge(id = "-11", coordinates = { @Loc(value = FIFTYSEVEN),
+                            @Loc(value = FIFTYSIX) }, tags = { "highway=secondary",
+                                    "name=Biryani Avenue" }),
+
+                    @Edge(id = "12", coordinates = { @Loc(value = FIFTYTHREE),
+                            @Loc(value = FIFTYSEVEN) }, tags = { "highway=secondary",
+                                    "name=Biryani Avenue" }),
+                    @Edge(id = "-12", coordinates = { @Loc(value = FIFTYSEVEN),
+                            @Loc(value = FIFTYTHREE) }, tags = { "highway=secondary",
+                                    "name=Biryani Avenue" }), })
+    private Atlas superComplexJunctionAtlas;
+
     /*
      * Intersections where nearby big nodes overlap
      */
@@ -609,7 +682,7 @@ public Atlas getAtlas()
 
     public Atlas getComplexJunctionAtlas()
     {
-        return this.complexJunctionAtlas;
+        return this.superComplexJunctionAtlas;
     }
 
     public Atlas getDNKAtlasToTestExcludeLinkRoadAsDualCarriageWay()

diff --git a/...streetmap/atlas/geography/atlas/items/complex/restriction/ComplexTurnRestrictionTest.java b/...streetmap/atlas/geography/atlas/items/complex/restriction/ComplexTurnRestrictionTest.java
@@ -230,13 +230,13 @@ public void testTurnRestrictionWithTwoViaNodesInRelation()
     @Test
     public void testTurnRestrictionsFromComplexBigNodes()
     {
-        final int expectedCountOfRestrictedRoutes = 302;
+        final int expectedCountOfRestrictedRoutes = 49;
 
         // There's an only turn restriction (http://www.openstreetmap.org/relation/6643212)
         // specifying that 447301069000000 must go to 447301070000000. This route has a corner case
         // where an otherToOption (-447301069000000) is found before the from edge. Specifically
         // check to make sure this path is restricted.
-        final String expectedRestrictedRoute = "[Route: 447301065000000, -447301070000000, -447301069000000, 447301069000000, 447301074000000, 447301068000000, 338286211000000]";
+        final String expectedRestrictedRoute = "[Route: 447301065000000, -447301070000000, 447301074000000, 447301068000000, 338286211000000]";
 
         final Atlas complexBigNodeAtlas = this.rule.getBigNodeWithOnlyTurnRestrictionsAtlas();
 

diff --git a/src/test/java/org/openstreetmap/atlas/geography/atlas/routing/AllPathsRouterTest.java b/src/test/java/org/openstreetmap/atlas/geography/atlas/routing/AllPathsRouterTest.java
@@ -32,16 +32,8 @@ public void testBidirectionalSimpleRoutes()
         final Set<Route> expectedRoutes = new TreeSet<>(Route.ROUTE_COMPARATOR);
         expectedRoutes.add(Route.forEdges(atlas.edge(315932590), atlas.edge(316932590),
                 atlas.edge(317932590)));
-        expectedRoutes.add(Route.forEdges(atlas.edge(315932590), atlas.edge(-315932590),
-                atlas.edge(-318932590), atlas.edge(-317932590), atlas.edge(-316932590),
-                atlas.edge(316932590), atlas.edge(317932590)));
-        expectedRoutes.add(Route.forEdges(atlas.edge(315932590), atlas.edge(316932590),
-                atlas.edge(-316932590), atlas.edge(-315932590), atlas.edge(-318932590),
-                atlas.edge(-317932590), atlas.edge(317932590)));
-        expectedRoutes.add(Route.forEdges(atlas.edge(315932590), atlas.edge(-315932590),
-                atlas.edge(-318932590), atlas.edge(-317932590), atlas.edge(317932590)));
 
-        Assert.assertEquals("Expect four distinct routes between start and end", 4, routes.size());
+        Assert.assertEquals("Expect one distinct route between start and end", 1, routes.size());
         Assert.assertEquals("Expect deterministic results from the router", expectedRoutes, routes);
     }