Ticket #17914: 17914.patch

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

@@ -13 +13 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Objects;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import org.openstreetmap.josm.data.coor.EastNorth;
 import org.openstreetmap.josm.data.coor.LatLon;
@@ -53 +55 @@
     private final int code;
     private final boolean isHighwayTest;
 
+    static final double DETOUR_FACTOR = 4;
+
     protected abstract boolean isCandidate(OsmPrimitive p);
 
     protected boolean isWantedWay(Way w) {
@@ -374 +378 @@
         }
         fillSearchNodes(endnodes);
         if (!searchNodes.isEmpty()) {
-            maxLen = 4 * mindist;
+            maxLen = DETOUR_FACTOR * mindist;
             if (isHighwayTest) {
                 addErrors(Severity.WARNING, getHighwayEndNodesNearOtherHighway(), tr("Way end node near other highway"));
             } else {
@@ -385 +389 @@
         /* 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 = 4 * minmiddledist;
+            maxLen = DETOUR_FACTOR * minmiddledist;
             fillSearchNodes(middlenodes);
             addErrors(Severity.OTHER, getWayNodesNearOtherWay(), tr("Way node near other way"));
             fillSearchNodes(othernodes);
@@ -431 +435 @@
          * @return true if a reasonable connection was found
          */
         boolean isConnectedTo(Node startNode) {
-            return isConnectedTo(startNode, new HashSet<>(), 0);
+            return isConnectedTo(startNode, new LinkedHashSet<>(), 0, w);
         }
 
         /**
@@ -439 +443 @@
          * @param node the given node
          * @param visited set of visited nodes
          * @param len length of the travelled route
+         * @param parent the previous parent way
          * @return true if a reasonable connection was found
          */
-        private boolean isConnectedTo(Node node, Set<Node> visited, double len) {
+        private boolean isConnectedTo(Node node, Set<Node> visited, double len, Way parent) {
+            if (len > maxLen) {
+                return false;
+            }
             if (n1 == node || n2 == node) {
                 return true;
             }
-            if (len > maxLen) {
-                return false;
-            }
             if (visited != null) {
                 visited.add(node);
-                for (final Way way : node.getParentWays()) {
-                    if (isWantedWay(way)) {
-                        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()))) {
-                                return true;
+                List<Way> wantedParents = node.getParentWays().stream().filter(p -> isWantedWay(p))
+                        .collect(Collectors.toList());
+                if (wantedParents.size() > 1 && wantedParents.indexOf(parent) != 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));
+                    }
+                    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) {
+                            double lastLen = node.getCoor().greatCircleDistance(next.getCoor());
+                            if (isConnectedTo(next, visited, len + lastLen, way)) {
+                                Node uncon = visited.iterator().next();
+                                EastNorth cl = calcClosest(uncon);
+                                double detourLen = len + lastLen + next.getCoor()
+                                        .greatCircleDistance(ProjectionRegistry.getProjection().eastNorth2latlon(cl));
+                                if (detourLen > maxLen)
+                                    return false;
+                                double directDist = getDist(uncon);
+                                return directDist > 0.1 && (visited.size() > 2 || DETOUR_FACTOR * directDist > detourLen);
                             }
                         }
                     }
@@ -474 +494 @@
             return false;
         }
 
-        double getDist(Node n) {
+        private EastNorth calcClosest(Node n) {
             EastNorth coord = n.getEastNorth();
             if (coord == null)
-                return Double.NaN;
-            EastNorth closest = Geometry.closestPointToSegment(n1.getEastNorth(), n2.getEastNorth(), coord);
-            return n.getCoor().greatCircleDistance(ProjectionRegistry.getProjection().eastNorth2latlon(closest));
+                return null;
+            return Geometry.closestPointToSegment(n1.getEastNorth(), n2.getEastNorth(), coord);
         }
 
+        double getDist(Node n) {
+            EastNorth closest = calcClosest(n);
+            return closest != null
+                    ? n.getCoor().greatCircleDistance(ProjectionRegistry.getProjection().eastNorth2latlon(closest))
+                    : Double.NaN;
+        }
+
         private boolean nearby(Node n, double dist) {
             if (w.containsNode(n))
                 return false;