Ticket #20057: 20057-beta.patch

src/org/openstreetmap/josm/data/validation/tests/UnconnectedWays.java

@@ -241 +241 @@
         minmiddledist = Config.getPref().getDouble(PREFIX + ".way_way_distance", 0.0);
         ds = OsmDataManager.getInstance().getActiveDataSet();
         dsArea = ds == null ? null : ds.getDataSourceArea();
+        maxLen = DETOUR_FACTOR * mindist;
     }
 
     protected Map<Node, MyWaySegment> getHighwayEndNodesNearOtherHighway() {
@@ -350 +351 @@
             Node node = error.getKey();
             MyWaySegment ws = error.getValue();
             errors.add(TestError.builder(this, severity, code)
-                    .message(message)
+                    .message(this.name, message)
                     .primitives(node, ws.w)
                     .highlight(node)
                     .build());
@@ -371 +372 @@
         }
         fillSearchNodes(endnodes);
         if (!searchNodes.isEmpty()) {
-            maxLen = DETOUR_FACTOR * mindist;
             if (isHighwayTest) {
-                addErrors(Severity.WARNING, getHighwayEndNodesNearOtherHighway(), tr("Way end node near other highway"));
+                addErrors(Severity.WARNING, getHighwayEndNodesNearOtherHighway(), tr("Way end node near other way"));
             } else {
                 addErrors(Severity.WARNING, getWayEndNodesNearOtherWay(), tr("Way end node near other way"));
             }
@@ -382 +382 @@
         /* the following two should use a shorter distance */
         boolean includeOther = isBeforeUpload ? ValidatorPrefHelper.PREF_OTHER_UPLOAD.get() : ValidatorPrefHelper.PREF_OTHER.get();
         if (minmiddledist > 0.0 && includeOther) {
-            maxLen = DETOUR_FACTOR * minmiddledist;
             fillSearchNodes(middlenodes);
             addErrors(Severity.OTHER, getWayNodesNearOtherWay(), tr("Way node near other way"));
             fillSearchNodes(othernodes);
@@ -415 +414 @@
         private final Node n2;
         private final boolean concernsArea;
 
-        MyWaySegment(Way w, Node n1, Node n2, boolean concersArea) {
+        MyWaySegment(Way w, Node n1, Node n2, boolean concernsArea) {
             this.w = w;
             this.n1 = n1;
             this.n2 = n2;
-            this.concernsArea = concersArea;
+            this.concernsArea = concernsArea;
         }
 
         /**
@@ -444 +443 @@
                 return false;
             }
             if (n1 == node || n2 == node) {
-                Node uncon = visited.iterator().next();
-                LatLon cl = ProjectionRegistry.getProjection().eastNorth2latlon(calcClosest(uncon));
+                Node startNode = visited.iterator().next();
+                LatLon cl = ProjectionRegistry.getProjection().eastNorth2latlon(calcClosest(startNode));
                 // calculate real detour length, closest point might be somewhere between n1 and n2
                 double detourLen = len + node.getCoor().greatCircleDistance(cl);
                 if (detourLen > maxLen)
                     return false;
-                // see #17914: flag also nodes which are very close
-                double directDist = getDist(uncon);
-                if (directDist <= 0.1)
+                if (!endnodes.contains(startNode))
+                    return true;
+                // see #17914: flag also unconnected nodes which are very close
+                double directDist = getDist(startNode);
+                if (directDist <= 0.1) {
                     return false;
+                }
                 return directDist > 0.5 || (visited.size() == 2 && directDist * 1.5 > detourLen);
             }
-            if (visited != null) {
-                visited.add(node);
-                List<Way> wantedParents = node.getParentWays().stream().filter(pw -> isWantedWay(pw))
-                        .collect(Collectors.toList());
-                if (wantedParents.size() > 1 && wantedParents.indexOf(parent) != wantedParents.size() - 1) {
+            List<Way> wantedParents = node.getParentWays().stream().filter(UnconnectedWays.this::isWantedWay)
+                    .collect(Collectors.toList());
+            if (wantedParents.size() > 1) {
+                int posWanted = wantedParents.indexOf(parent);
+                if (posWanted >= 0 && posWanted != wantedParents.size() - 1) {
                     // we want to find a different way. so move known way to the end of the list
                     wantedParents.remove(parent);
                     wantedParents.add(parent);
                 }
-
-                for (final Way way : wantedParents) {
-                    List<Node> nextNodes = new ArrayList<>();
-                    int pos = way.getNodes().indexOf(node);
-                    if (pos > 0) {
-                        nextNodes.add(way.getNode(pos - 1));
+            }
+            for (final Way way : wantedParents) {
+                if (len == 0)
+                    visited.clear(); // first node might not be an end node
+                visited.add(node);
+                List<Node> nextNodes = new ArrayList<>();
+                int pos = way.getNodes().indexOf(node);
+                if (pos > 0) {
+                    nextNodes.add(way.getNode(pos - 1));
+                }
+                if (pos + 1 < way.getNodesCount()) {
+                    nextNodes.add(way.getNode(pos + 1));
+                }
+                for (Node next : nextNodes) {
+                    final boolean containsN = visited.contains(next);
+                    visited.add(next);
+                    if (!containsN && isConnectedTo(next, visited,
+                            len + node.getCoor().greatCircleDistance(next.getCoor()), way)) {
+                        return true;
                     }
-                    if (pos + 1 < way.getNodesCount()) {
-                        nextNodes.add(way.getNode(pos + 1));
-                    }
-                    for (Node next : nextNodes) {
-                        final boolean containsN = visited.contains(next);
-                        visited.add(next);
-                        if (!containsN && isConnectedTo(next, visited,
-                                len + node.getCoor().greatCircleDistance(next.getCoor()), way)) {
-                            return true;
-                        }
-                    }
                 }
             }
             return false;
@@ -585 +589 @@
             return ret;
 
         int size = w.getNodesCount();
+        // TODO: concernsArea probably only useful with waterway=riverbank
         boolean concersArea = w.concernsArea();
         for (int i = 1; i < size; ++i) {
             if (i < size-1) {

test/unit/org/openstreetmap/josm/data/validation/tests/UnconnectedWaysTest.java

@@ -21 +21 @@
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.io.IllegalDataException;
 import org.openstreetmap.josm.io.OsmReader;
+import org.openstreetmap.josm.spi.preferences.Config;
 
 /**
  * Unit tests of {@code UnconnectedWays} class.
@@ -80 +81 @@
     }
 
     /**
-     * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/18136">Bug #18106</a>.
+     * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/18106">Bug #18106</a>.
      * @throws IOException if any I/O error occurs
      * @throws IllegalDataException if the OSM data cannot be parsed
      * @throws FileNotFoundException if the data file cannot be found
@@ -138 +139 @@
             assertThat(bib.getErrors(), isEmpty());
         }
     }
+
+    /**
+     * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/20057">Bug #20057</a>.
+     * @throws IOException if any I/O error occurs
+     * @throws IllegalDataException if the OSM data cannot be parsed
+     * @throws FileNotFoundException if the data file cannot be found
+     */
+    @Test
+    void testTicket20057() throws IOException, IllegalDataException, FileNotFoundException {
+        try (InputStream fis = TestUtils.getRegressionDataStream(20057, "data.osm")) {
+            final DataSet ds = OsmReader.parseDataSet(fis, NullProgressMonitor.INSTANCE);
+            MainApplication.getLayerManager().addLayer(new OsmDataLayer(ds, null, null));
+            Config.getPref().putDouble(UnconnectedWays.PREFIX + ".way_way_distance", 1.0);
+            Config.getPref().putBoolean("validator.other", true);
+            bib.startTest(null);
+            bib.setBeforeUpload(false);
+            bib.visit(ds.allPrimitives());
+            bib.endTest();
+            assertThat(bib.getErrors(), isEmpty());
+        }
+    }
 }