Changeset 16200 in josm

Timestamp:

2020-03-24T07:37:08+01:00 (5 years ago)

Author:

GerdP

Message:

see #18863 split ways: false positive warnings about missing members
Solution for the problem case given in the unit test looks OK.
Ticket mentiones two problems, so I am not sure if both are fixed now.

Location:

trunk

Files:

• src/org/openstreetmap/josm/command/SplitWayCommand.java (modified) (1 diff)
• test/data/regress/18863 (added)
• test/data/regress/18863/data.osm.bz2 (added)
• test/unit/org/openstreetmap/josm/command/SplitWayCommandTest.java (modified) (2 diffs)

trunk/src/org/openstreetmap/josm/command/SplitWayCommand.java

@@ -501 +501 @@
                                 direction = Direction.IRRELEVANT;
                             } else {
+                                boolean previousWayMemberMissing = true;
+                                boolean nextWayMemberMissing = true;
+
                                 // For ordered relations, looking beyond the nearest neighbour members is not required,
                                 // and can even cause the wrong direction to be guessed (with closed loops).
                                 if (ir - 1 >= 0 && relationMembers.get(ir - 1).isWay()) {
                                     Way w = relationMembers.get(ir - 1).getWay();
-                                    if (w.lastNode() == way.firstNode() || w.firstNode() == way.firstNode()) {
-                                        direction = Direction.FORWARDS;
-                                    } else if (w.firstNode() == way.lastNode() || w.lastNode() == way.lastNode()) {
-                                        direction = Direction.BACKWARDS;
+                                    if (!w.isIncomplete()) {
+                                        previousWayMemberMissing = false;
+                                        if (w.lastNode() == way.firstNode() || w.firstNode() == way.firstNode()) {
+                                            direction = Direction.FORWARDS;
+                                        } else if (w.firstNode() == way.lastNode() || w.lastNode() == way.lastNode()) {
+                                            direction = Direction.BACKWARDS;
+                                        }
                                     }
+                                } else {
+                                    previousWayMemberMissing = false;
                                 }
                                 if (ir + 1 < relationMembers.size() && relationMembers.get(ir + 1).isWay()) {
                                     Way w = relationMembers.get(ir + 1).getWay();
-                                    if (w.lastNode() == way.firstNode() || w.firstNode() == way.firstNode()) {
-                                        direction = Direction.BACKWARDS;
-                                    } else if (w.firstNode() == way.lastNode() || w.lastNode() == way.lastNode()) {
-                                        direction = Direction.FORWARDS;
+                                    if (!w.isIncomplete()) {
+                                        nextWayMemberMissing = false;
+                                        if (w.lastNode() == way.firstNode() || w.firstNode() == way.firstNode()) {
+                                            direction = Direction.BACKWARDS;
+                                        } else if (w.firstNode() == way.lastNode() || w.lastNode() == way.lastNode()) {
+                                            direction = Direction.FORWARDS;
+                                        }
                                     }
+                                } else {
+                                    nextWayMemberMissing = false;
+                                }
+
+                                if (direction == Direction.UNKNOWN
+                                        && !previousWayMemberMissing
+                                        && !nextWayMemberMissing) {
+                                    // If both the next and previous way member in the relation are already known at
+                                    // this point, and they are not connected to this one, then we can safely
+                                    // assume that the direction doesn't matter. Downloading any more members
+                                    // won't help in any case.
+                                    direction = Direction.IRRELEVANT;
                                 }
                             }

trunk/test/unit/org/openstreetmap/josm/command/SplitWayCommandTest.java

@@ -290 +290 @@
 
     /**
-     * Non-regression test for issue #17400 ( Warn when splitting way in not fully downloaded region)
+     * Non-regression test for issue #17400 (Warn when splitting way in not fully downloaded region)
      *
      * Bus route 190 gets broken when the split occurs, because the two new way parts are inserted in the relation in
@@ -333 +333 @@
         }
     }
+
+    /**
+     * Non-regression test for issue #18863 (Asking for download of missing members when not needed)
+     *
+     * A split on node 4518025255 caused the 'download missing members?' dialog to pop up for relation 68745 (CB 2),
+     * even though the way members next to the split way were already downloaded. This happened because this relation
+     * does not have its members connected at all.
+     *
+     * This split should not trigger any download action at all.
+     *
+     * @throws IOException if any I/O error occurs
+     * @throws IllegalDataException if OSM parsing fails
+     */
+    @Test
+    public void testTicket18863() throws IOException, IllegalDataException {
+        try (InputStream is = TestUtils.getRegressionDataStream(18863, "data.osm.bz2")) {
+            DataSet ds = OsmReader.parseDataSet(is, null);
+
+            Way splitWay = (Way) ds.getPrimitiveById(290581177L, OsmPrimitiveType.WAY);
+            Node splitNode = (Node) ds.getPrimitiveById(4518025255L, OsmPrimitiveType.NODE);
+
+            final Optional<SplitWayCommand> result = SplitWayCommand.splitWay(
+                    splitWay,
+                    SplitWayCommand.buildSplitChunks(splitWay, Collections.singletonList(splitNode)),
+                    new ArrayList<>(),
+                    Strategy.keepLongestChunk(),
+                    // This split requires no additional downloads. If any are needed, this command will fail.
+                    SplitWayCommand.WhenRelationOrderUncertain.ABORT
+            );
+
+            // Should not result in aborting the split.
+            assertTrue(result.isPresent());
+        }
+    }
 }