Changeset 15558 in josm

Timestamp:

2019-12-04T11:42:24+01:00 (5 years ago)

Author:

GerdP

Message:

fix #18387 Combine way action refuses to combine new way with oneway (regression from r15554)
I somehow expected that NodeGraph.getOutboundPairs(Node node) would return predessors and successors.
Maybe that routine should be renamed to getSuccessorPairs?

Location:

trunk

Files:

• src/org/openstreetmap/josm/data/osm/NodeGraph.java (modified) (2 diffs)
• test/data/regress/18387 (added)
• test/data/regress/18387/data.osm (added)
• test/unit/org/openstreetmap/josm/actions/CombineWayActionTest.java (modified) (4 diffs)

trunk/src/org/openstreetmap/josm/data/osm/NodeGraph.java

@@ -221 +221 @@
     }
 
+    private List<NodePair> getConnectedPairs (Node node) {
+        List<NodePair> connected = new ArrayList<>();
+        connected.addAll(Optional.ofNullable(successors.get(node)).orElseGet(Collections::emptyList));
+        connected.addAll(Optional.ofNullable(predecessors.get(node)).orElseGet(Collections::emptyList));
+        return connected;
+    }
+
     protected List<NodePair> getOutboundPairs(NodePair pair) {
         return getOutboundPairs(pair.getB());
@@ -340 +347 @@
             Node n = toVisit.pop();
             if (!visited.contains(n)) {
-                List<NodePair> neighbours = getOutboundPairs(n);
-                for (NodePair pair : neighbours) {
-                    toVisit.addLast(pair.getA());
-                    toVisit.addLast(pair.getB());
+                for (NodePair pair : getConnectedPairs(n)) {
+                    if (n != pair.getA())
+                        toVisit.addLast(pair.getA());
+                    if (n != pair.getB())
+                        toVisit.addLast(pair.getB());
                 }
                 visited.add(n);

trunk/test/unit/org/openstreetmap/josm/actions/CombineWayActionTest.java

@@ -4 +4 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
@@ -18 +21 @@
 import org.openstreetmap.josm.data.osm.NodeGraph;
 import org.openstreetmap.josm.data.osm.NodePair;
+import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.io.IllegalDataException;
 import org.openstreetmap.josm.io.OsmReader;
@@ -60 +64 @@
 
     /**
-     * Non-regression test for bug #1835 (combine way with overlapping ways)
+     * Non-regression test for bug #18385 (combine way with overlapping ways)
      * @throws IOException if any I/O error occurs
      * @throws IllegalDataException if OSM parsing fails
@@ -75 +79 @@
 
     /**
+     * Non-regression test for bug #18387 (combine new way with oneway)
+     * @throws IOException if any I/O error occurs
+     * @throws IllegalDataException if OSM parsing fails
+     */
+    @Test
+    public void testTicket18387() throws IOException, IllegalDataException {
+        try (InputStream is = TestUtils.getRegressionDataStream(18387, "data.osm")) {
+            DataSet ds = OsmReader.parseDataSet(is, null);
+            ArrayList<Way> selection = new ArrayList<>(ds.getWays());
+            assertEquals(2, selection.size());
+            if (!selection.get(0).isNew())
+                Collections.reverse(selection);
+            NodeGraph graph = NodeGraph.createNearlyUndirectedGraphFromNodeWays(selection);
+            List<Node> path = graph.buildSpanningPathNoRemove();
+            assertTrue(path != null);
+        }
+    }
+
+    /**
      * Unit test of methods {@link NodePair#equals} and {@link NodePair#hashCode}.
      */