Index: trunk/test/data/regress/18367/nocombine.osm
===================================================================
--- trunk/test/data/regress/18367/nocombine.osm (revision 15574)
+++ trunk/test/data/regress/18367/nocombine.osm (revision 15574)
@@ -0,0 +1,22 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
Index: trunk/test/data/regress/18367/silent-revert.osm
===================================================================
--- trunk/test/data/regress/18367/silent-revert.osm (revision 15574)
+++ trunk/test/data/regress/18367/silent-revert.osm (revision 15574)
@@ -0,0 +1,19 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
Index: trunk/test/data/regress/18367/split-and-reverse.osm
===================================================================
--- trunk/test/data/regress/18367/split-and-reverse.osm (revision 15574)
+++ trunk/test/data/regress/18367/split-and-reverse.osm (revision 15574)
@@ -0,0 +1,19 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
Index: trunk/test/unit/org/openstreetmap/josm/actions/CombineWayActionTest.java
===================================================================
--- trunk/test/unit/org/openstreetmap/josm/actions/CombineWayActionTest.java (revision 15563)
+++ trunk/test/unit/org/openstreetmap/josm/actions/CombineWayActionTest.java (revision 15574)
@@ -3,12 +3,13 @@
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
+import java.util.LinkedList;
import java.util.List;
import java.util.Set;
@@ -19,5 +20,4 @@
import org.openstreetmap.josm.data.osm.DataSet;
import org.openstreetmap.josm.data.osm.Node;
-import org.openstreetmap.josm.data.osm.NodeGraph;
import org.openstreetmap.josm.data.osm.NodePair;
import org.openstreetmap.josm.data.osm.Way;
@@ -50,6 +50,5 @@
try (InputStream is = TestUtils.getRegressionDataStream(11957, "data.osm")) {
DataSet ds = OsmReader.parseDataSet(is, null);
- NodeGraph graph = NodeGraph.createNearlyUndirectedGraphFromNodeWays(ds.getWays());
- List path = graph.buildSpanningPathNoRemove();
+ List path = CombineWayAction.tryJoin(ds.getWays());
assertEquals(10, path.size());
Set firstAndLastObtained = new HashSet<>();
@@ -72,7 +71,6 @@
try (InputStream is = TestUtils.getRegressionDataStream(18385, "data.osm")) {
DataSet ds = OsmReader.parseDataSet(is, null);
- NodeGraph graph = NodeGraph.createNearlyUndirectedGraphFromNodeWays(ds.getWays());
- List path = graph.buildSpanningPathNoRemove();
- assertNull(path);
+ List path = CombineWayAction.tryJoin(ds.getWays());
+ assertFalse(path.isEmpty());
}
}
@@ -91,9 +89,86 @@
if (!selection.get(0).isNew())
Collections.reverse(selection);
- NodeGraph graph = NodeGraph.createNearlyUndirectedGraphFromNodeWays(selection);
- List path = graph.buildSpanningPathNoRemove();
- assertTrue(path != null);
+ double expectedLen = getOriginalLength(selection);
+ List path = CombineWayAction.tryJoin(selection);
+ assertFalse(path.isEmpty());
+ Way combined = new Way(0);
+ combined.setNodes(path);
+ assertEquals(expectedLen, combined.getLength(), 0.01);
}
}
+
+ /**
+ * Non-regression test for bug #18367 (Lines cannot be combined when they share an overlapping segment)
+ * @throws IOException if any I/O error occurs
+ * @throws IllegalDataException if OSM parsing fails
+ */
+ @Test
+ public void testTicket18367() throws IOException, IllegalDataException {
+ try (InputStream is = TestUtils.getRegressionDataStream(18367, "nocombine.osm")) {
+ DataSet ds = OsmReader.parseDataSet(is, null);
+ ArrayList selection = new ArrayList<>(ds.getWays());
+ assertEquals(2, selection.size());
+ double expectedLen = getOriginalLength(selection);
+ List path = CombineWayAction.tryJoin(selection);
+ assertFalse(path.isEmpty());
+ Way combined = new Way(0);
+ combined.setNodes(path);
+ assertEquals(expectedLen, combined.getLength(), 1e-7);
+ }
+ }
+
+
+ /**
+ * Non-regression test for bug #18367
+ * @throws IOException if any I/O error occurs
+ * @throws IllegalDataException if OSM parsing fails
+ */
+ @Test
+ public void testTicket18367NeedsSplit() throws IOException, IllegalDataException {
+ try (InputStream is = TestUtils.getRegressionDataStream(18367, "split-and-reverse.osm")) {
+ DataSet ds = OsmReader.parseDataSet(is, null);
+ ArrayList selection = new ArrayList<>(ds.getWays());
+ assertEquals(2, selection.size());
+ double expectedLen = getOriginalLength(selection);
+ List path = CombineWayAction.tryJoin(selection);
+ assertFalse(path.isEmpty());
+ Way combined = new Way(0);
+ combined.setNodes(path);
+ assertEquals(expectedLen, combined.getLength(), 1e-7);
+ List reversedWays = new LinkedList<>();
+ List unreversedWays = new LinkedList<>();
+ CombineWayAction.detectReversedWays(selection, path, reversedWays, unreversedWays);
+ assertFalse(reversedWays.isEmpty());
+ }
+ }
+
+
+ /**
+ * Test for bad reverse way detection. See #18367
+ * @throws IOException if any I/O error occurs
+ * @throws IllegalDataException if OSM parsing fails
+ */
+ @Test
+ public void testDetectReversedWays() throws IOException, IllegalDataException {
+ try (InputStream is = TestUtils.getRegressionDataStream(18367, "silent-revert.osm")) {
+ DataSet ds = OsmReader.parseDataSet(is, null);
+ ArrayList selection = new ArrayList<>(ds.getWays());
+ assertEquals(2, selection.size());
+ // make sure that short way is first
+ if (selection.get(0).getNodesCount() != 2)
+ Collections.reverse(selection);
+ double expectedLen = getOriginalLength(selection);
+ List path = CombineWayAction.tryJoin(selection);
+ assertFalse(path.isEmpty());
+ Way combined = new Way(0);
+ combined.setNodes(path);
+ assertEquals(expectedLen, combined.getLength(), 1e-7);
+ List reversedWays = new LinkedList<>();
+ List unreversedWays = new LinkedList<>();
+ CombineWayAction.detectReversedWays(selection, path, reversedWays, unreversedWays);
+ assertFalse(reversedWays.contains(selection.get(0)));
+ }
+ }
+
/**
@@ -107,3 +182,12 @@
.verify();
}
+
+ private static double getOriginalLength(Collection ways) {
+ double len = 0;
+ for (Way w : ways) {
+ len += w.getLength();
+ }
+ return len;
+ }
+
}