Fix way-section identifier bug for closed loops (#187)

* Fixing way section identifier for line with a loop * Adding explicit identifier validity checks for cases with loops
osmlab · Aug 6, 2018 · 3fa8864 · 3fa8864
1 parent 7f1e7a6
commit 3fa8864
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 2 deletions.
diff --git a/...main/java/org/openstreetmap/atlas/geography/atlas/raw/sectioning/WaySectionProcessor.java b/...main/java/org/openstreetmap/atlas/geography/atlas/raw/sectioning/WaySectionProcessor.java
@@ -1001,10 +1001,11 @@ private List<TemporaryEdge> splitNonRingLineIntoEdges(final WaySectionChangeSet
                     // identifier will start at 000.
                     final long edgeIdentifier;
                     if (!line.isClosed() && nodesToSectionAt.size() == 2
-                            && polyline.size() - 1 == index)
+                            && polyline.size() - 1 == index && newEdgesForLine.isEmpty())
                     {
                         // The only time we want to do this is if there are two nodes, the line
-                        // isn't a ring and the last node is the end of the polyline
+                        // isn't a ring, the last node is the end of the polyline and if we haven't
+                        // split this line previously
                         edgeIdentifier = line.getIdentifier();
                     }
                     else

diff --git a/.../java/org/openstreetmap/atlas/geography/atlas/raw/sectioning/WaySectionProcessorTest.java b/.../java/org/openstreetmap/atlas/geography/atlas/raw/sectioning/WaySectionProcessorTest.java
@@ -43,6 +43,7 @@ public void testBidirectionalRing()
                 finalAtlas.numberOfEdges());
         Assert.assertEquals("Two nodes", 2, finalAtlas.numberOfNodes());
 
+        // Explicit check for expected identifiers
         Assert.assertTrue(finalAtlas.edge(317579533000001L) != null);
         Assert.assertTrue(finalAtlas.edge(-317579533000001L) != null);
         Assert.assertTrue(finalAtlas.edge(317579533000002L) != null);
@@ -62,6 +63,12 @@ public void testLineWithLoopAtEnd()
         Assert.assertEquals("Two nodes", 2, finalAtlas.numberOfNodes());
         Assert.assertEquals(4, finalAtlas.node(4566499618000000L).connectedEdges().size());
         Assert.assertEquals(2, finalAtlas.node(4566499619000000L).connectedEdges().size());
+
+        // Explicit check for expected identifiers
+        Assert.assertTrue(finalAtlas.edge(461101743000001L) != null);
+        Assert.assertTrue(finalAtlas.edge(-461101743000001L) != null);
+        Assert.assertTrue(finalAtlas.edge(461101743000002L) != null);
+        Assert.assertTrue(finalAtlas.edge(-461101743000002L) != null);
     }
 
     @Test
@@ -78,6 +85,14 @@ public void testLineWithLoopAtStart()
         Assert.assertEquals(4, finalAtlas.node(4560902689000000L).connectedEdges().size());
         Assert.assertEquals(2, finalAtlas.node(4560902695000000L).connectedEdges().size());
         Assert.assertEquals(4, finalAtlas.node(4560902693000000L).connectedEdges().size());
+
+        // Explicit check for expected identifiers
+        Assert.assertTrue(finalAtlas.edge(460419987000001L) != null);
+        Assert.assertTrue(finalAtlas.edge(-460419987000001L) != null);
+        Assert.assertTrue(finalAtlas.edge(460419987000002L) != null);
+        Assert.assertTrue(finalAtlas.edge(-460419987000002L) != null);
+        Assert.assertTrue(finalAtlas.edge(460419987000003L) != null);
+        Assert.assertTrue(finalAtlas.edge(-460419987000003L) != null);
     }
 
     @Test
@@ -96,6 +111,16 @@ public void testLineWithLoopInMiddle()
         Assert.assertEquals(4, finalAtlas.node(4560902693000000L).connectedEdges().size());
         Assert.assertEquals(2, finalAtlas.node(4560902612000000L).connectedEdges().size());
         Assert.assertEquals(6, finalAtlas.node(4560902689000000L).connectedEdges().size());
+
+        // Explicit check for expected identifiers
+        Assert.assertTrue(finalAtlas.edge(460419987000001L) != null);
+        Assert.assertTrue(finalAtlas.edge(-460419987000001L) != null);
+        Assert.assertTrue(finalAtlas.edge(460419987000002L) != null);
+        Assert.assertTrue(finalAtlas.edge(-460419987000003L) != null);
+        Assert.assertTrue(finalAtlas.edge(460419987000003L) != null);
+        Assert.assertTrue(finalAtlas.edge(-460419987000003L) != null);
+        Assert.assertTrue(finalAtlas.edge(460419987000004L) != null);
+        Assert.assertTrue(finalAtlas.edge(-460419987000004L) != null);
     }
 
     @Test