Supporting reversed edges to build original OSM geometry. (osmlab#486)

* fix to build original osm geom for reverse ways. modified existent unit tests accordingly.

* fix to build original osm geom for reverse ways. modified existent unit tests accordingly.

* fix merging conflicts

* spotless

src/main/java/org/openstreetmap/atlas/checks/utility/CommonMethods.java

@@ -4,12 +4,16 @@
 import java.util.HashSet;
 import java.util.List;
 
+import org.openstreetmap.atlas.exception.CoreException;
 import org.openstreetmap.atlas.geography.PolyLine;
 import org.openstreetmap.atlas.geography.atlas.items.Edge;
 import org.openstreetmap.atlas.geography.atlas.items.Relation;
 import org.openstreetmap.atlas.geography.atlas.items.RelationMember;
 import org.openstreetmap.atlas.geography.atlas.walker.OsmWayWalker;
+import org.openstreetmap.atlas.tags.oneway.OneWayTag;
 import org.openstreetmap.atlas.utilities.collections.Iterables;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Hold common Methods (should be used in more than one check)
@@ -18,6 +22,7 @@
  */
 public final class CommonMethods
 {
+    private static final Logger logger = LoggerFactory.getLogger(CommonMethods.class);
     private static final int MODULUS = 1000000;
 
     /**
@@ -32,11 +37,26 @@ public static PolyLine buildOriginalOsmWayGeometry(final Edge edge)
     {
         // Identify and sort by IDs all sections of original OSM way
         final List<Edge> sortedEdges = new ArrayList<>(new OsmWayWalker(edge).collectEdges());
-        // Build original OSM polyline
-        PolyLine geometry = new PolyLine(sortedEdges.get(0).getRawGeometry());
-        for (int index = 1; index < sortedEdges.size(); index++)
+        // Build original OSM polyline.
+        PolyLine geometry = null;
+
+        try
+        {
+            geometry = OneWayTag.isOneWayReversed(edge)
+                    ? new PolyLine(sortedEdges.get(0).getRawGeometry()).reversed()
+                    : new PolyLine(sortedEdges.get(0).getRawGeometry());
+
+            for (int index = 1; index < sortedEdges.size(); index++)
+            {
+                geometry = OneWayTag.isOneWayReversed(edge)
+                        ? geometry.append(sortedEdges.get(index).asPolyLine().reversed())
+                        : geometry.append(sortedEdges.get(index).asPolyLine());
+            }
+        }
+        catch (final CoreException exception)
         {
-            geometry = geometry.append(sortedEdges.get(index).asPolyLine());
+            logger.warn("Unable to build geometry for edge {}({}): {}", edge.getIdentifier(),
+                    edge.getOsmIdentifier(), exception.getMessage());
         }
         return geometry;
     }
@@ -55,8 +75,7 @@ public static long getOSMRelationMemberSize(final Relation relation)
             // De-duplicating either Point or Node with same OSM Id
             if (entity.getType().toString().matches("POINT|NODE"))
             {
-                return String.valueOf("PointNode")
-                        .concat(String.valueOf(entity.getOsmIdentifier()));
+                return "PointNode".concat(String.valueOf(entity.getOsmIdentifier()));
             }
             return entity.getType().toString().concat(String.valueOf(entity.getOsmIdentifier()));
         }).distinct().count();

src/test/java/org/openstreetmap/atlas/checks/utility/CommonMethodsTest.java

@@ -27,16 +27,14 @@ public void testClosedWay()
 
     @Test
     public void testFirstSection()
-
     {
-        assertTrue(CommonMethods.isFirstWaySection(this.setup.getClosedWay().edge(12000001)));
+        assertTrue(CommonMethods.isFirstWaySection(this.setup.getFirstSection().edge(12000001)));
     }
 
     @Test
     public void testFirstSectionNegative()
-
     {
-        assertFalse(CommonMethods.isFirstWaySection(this.setup.getClosedWay().edge(12000002)));
+        assertFalse(CommonMethods.isFirstWaySection(this.setup.getFirstSection().edge(12000002)));
     }
 
     @Test
@@ -83,6 +81,17 @@ public void testOriginalWayGeometry()
                 .toString());
     }
 
+    @Test
+    public void testOriginalWayGeometryReversed()
+    {
+        final String origGeom = "LINESTRING (-71.7194204 18.4360044, -71.6970306 18.4360737, -71.7052283 18.4273807, -71.7194204 18.4360044)";
+        assertEquals(origGeom,
+                CommonMethods
+                        .buildOriginalOsmWayGeometry(
+                                this.setup.getOriginalWayGeometryReversed().edge(12000003))
+                        .toString());
+    }
+
     @Test
     public void testUnClosedWay()
     {

src/test/java/org/openstreetmap/atlas/checks/utility/CommonMethodsTestRule.java

@@ -146,6 +146,21 @@ public class CommonMethodsTestRule extends CoreTestRule
                             @Loc(value = ONE) }) })
     private Atlas originalWayGeometry;
 
+    @TestAtlas(
+            // nodes
+            nodes = { @Node(id = "1", coordinates = @Loc(value = ONE)),
+                    @Node(id = "2", coordinates = @Loc(value = TWO)),
+                    @Node(id = "3", coordinates = @Loc(value = THREE)) },
+            // edges
+            edges = {
+                    @Edge(id = "12000001", coordinates = { @Loc(value = TWO),
+                            @Loc(value = ONE) }, tags = { "oneway=-1" }),
+                    @Edge(id = "12000002", coordinates = { @Loc(value = THREE),
+                            @Loc(value = TWO) }, tags = { "oneway=-1" }),
+                    @Edge(id = "12000003", coordinates = { @Loc(value = ONE),
+                            @Loc(value = THREE) }, tags = { "oneway=-1" }) })
+    private Atlas originalWayGeometryReversed;
+
     @TestAtlas(
             // nodes
             nodes = { @Node(id = "1", coordinates = @Loc(value = ONE)),
@@ -198,6 +213,11 @@ public Atlas getOriginalWayGeometry()
         return this.originalWayGeometry;
     }
 
+    public Atlas getOriginalWayGeometryReversed()
+    {
+        return this.originalWayGeometryReversed;
+    }
+
     public Atlas getUnClosedWay()
     {
         return this.unClosedWay;

src/test/java/org/openstreetmap/atlas/checks/validation/linear/edges/MalformedRoundaboutCheckTest.java

@@ -124,7 +124,9 @@ public void counterClockwiseRoundaboutRightDrivingOutsideConnectionTest()
     {
         this.verifier.actual(
                 this.setup.counterClockwiseRoundaboutRightDrivingOutsideConnectionAtlas(),
-                new MalformedRoundaboutCheck(ConfigurationResolver.emptyConfiguration()));
+                new MalformedRoundaboutCheck(ConfigurationResolver.inlineConfiguration(
+                        "{\"MalformedRoundaboutCheck\":{\"angle.threshold.maximum_degree\":" + 179.0
+                                + ",\"min.nodes\":" + 0.0 + "}}")));
         this.verifier.globallyVerify(flags -> Assert.assertEquals(0, flags.size()));
     }
 
@@ -167,7 +169,9 @@ public void testClockwiseRoundaboutLeftDrivingAtlas()
     public void testClockwiseRoundaboutLeftDrivingConcave()
     {
         this.verifier.actual(this.setup.clockwiseRoundaboutLeftDrivingConcaveAtlas(),
-                new MalformedRoundaboutCheck(ConfigurationResolver.emptyConfiguration()));
+                new MalformedRoundaboutCheck(ConfigurationResolver.inlineConfiguration(
+                        "{\"MalformedRoundaboutCheck\":{\"angle.threshold.maximum_degree\":" + 179.0
+                                + ",\"min.nodes\":" + 0.0 + "}}")));
         this.verifier.globallyVerify(flags -> Assert.assertEquals(0, flags.size()));
     }
 
@@ -234,7 +238,9 @@ public void testFewerThanMinOSMNodesRoundaboutAtlas()
     public void testMultiDirectionalRoundaboutAtlas()
     {
         this.verifier.actual(this.setup.multiDirectionalRoundaboutAtlas(),
-                new MalformedRoundaboutCheck(ConfigurationResolver.emptyConfiguration()));
+                new MalformedRoundaboutCheck(ConfigurationResolver.inlineConfiguration(
+                        "{\"MalformedRoundaboutCheck\":{\"angle.threshold.maximum_degree\":" + 179.0
+                                + ",\"min.nodes\":" + 0.0 + "}}")));
         this.verifier.globallyVerify(flags -> Assert.assertEquals(
                 "1. This roundabout is malformed.\n"
                         + "2. This roundabout does not form a single, one-way, complete, car navigable route.",
@@ -245,7 +251,9 @@ public void testMultiDirectionalRoundaboutAtlas()
     public void testRoundaboutWithEnclosedMultiLayerNavigableRoad()
     {
         this.verifier.actual(this.setup.enclosedMultiLayerNavigableRoad(),
-                new MalformedRoundaboutCheck(ConfigurationResolver.emptyConfiguration()));
+                new MalformedRoundaboutCheck(ConfigurationResolver.inlineConfiguration(
+                        "{\"MalformedRoundaboutCheck\":{\"angle.threshold.maximum_degree\":" + 179.0
+                                + ",\"min.nodes\":" + 0.0 + "}}")));
         this.verifier.verifyEmpty();
     }