Changeset 12037 in josm for trunk/src/org/openstreetmap/josm/data/validation/tests/LongSegment.java

Timestamp:

2017-05-02T01:29:35+02:00 (7 years ago)

Author:

michael2402

Message:

See #14708: For long segment test, notify the user about the exact segment. Do not check segments that have not been touched.

File:

• trunk/src/org/openstreetmap/josm/data/validation/tests/LongSegment.java (modified) (4 diffs)

trunk/src/org/openstreetmap/josm/data/validation/tests/LongSegment.java

@@ -5 +5 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
 
 import org.openstreetmap.josm.Main;
+import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.data.osm.WaySegment;
 import org.openstreetmap.josm.data.validation.Severity;
 import org.openstreetmap.josm.data.validation.Test;
@@ -28 +31 @@
     /** Maximum segment length for this test */
     protected int maxlength;
-    /** set of visited ways */
-    private final Set<Way> visitedWays = new HashSet<>();
+    /** set of visited ways. Tracking this increases performance when checking single nodes. */
+    private Set<Way> visitedWays;
+
+    /** set of way segments that have been reported */
+    protected Set<WaySegment> reported;
 
     /**
@@ -41 +47 @@
     @Override
     public void visit(Node n) {
-        for (Way w : n.getParentWays()) {
-            if (isPrimitiveUsable(w)) {
-                testWay(w);
+        // Test all way segments around this node.
+        // If there is an error in the unchanged part of the way, we do not need to warn the user about it.
+        for (Way way : n.getParentWays()) {
+                if (ignoreWay(way)) {
+                    continue;
+                }
+                // Do not simply use index of - a node may be in a way multiple times
+                for (int i = 0; i < way.getNodesCount(); i++) {
+                    if (n == way.getNode(i)) {
+                        if (i > 0) {
+                            visitWaySegment(way, i - 1);
+                        }
+                        if (i < way.getNodesCount() - 1) {
+                            visitWaySegment(way, i);
+                        }
+                    }
+                }
+            }
+    }
+
+    @Override
+    public void visit(Way w) {
+        if (ignoreWay(w)) {
+            return;
+        }
+        visitedWays.add(w);
+
+        testWay(w);
+    }
+
+    private void testWay(Way w) {
+        for (int i = 0; i < w.getNodesCount() - 1; i++) {
+            visitWaySegment(w, i);
+        }
+    }
+
+    private boolean ignoreWay(Way w) {
+        return visitedWays.contains(w) || w.hasTag("route", "ferry");
+    }
+
+    private void visitWaySegment(Way w, int i) {
+        LatLon coor1 = w.getNode(i).getCoor();
+        LatLon coor2 = w.getNode(i + 1).getCoor();
+
+        if (coor1 != null && coor2 != null) {
+            Double length = coor1.greatCircleDistance(coor2);
+            if (length > maxlength) {
+                addErrorForSegment(new WaySegment(w, i), length / 1000.0);
             }
         }
     }
 
-    @Override
-    public void visit(Way w) {
-        testWay(w);
-    }
-
-    private void testWay(Way w) {
-        if (visitedWays.contains(w) || w.hasTag("route", "ferry")) {
-            return;
-        }
-        visitedWays.add(w);
-        Double length = w.getLongestSegmentLength();
-        if (length > maxlength) {
-            length /= 1000.0;
+    private void addErrorForSegment(WaySegment waySegment, Double length) {
+        if (reported.add(waySegment)) {
             errors.add(TestError.builder(this, Severity.WARNING, LONG_SEGMENT)
                     .message(tr("Long segments"), marktr("Very long segment of {0} kilometers"), length.intValue())
-                    .primitives(w)
+                    .primitives(waySegment.way)
+                    .highlightWaySegments(Collections.singleton(waySegment))
                     .build());
         }
@@ -72 +113 @@
         super.startTest(monitor);
         maxlength = Main.pref.getInteger("validator.maximum.segment.length", 15_000);
-        visitedWays.clear();
+        reported = new HashSet<>();
+        visitedWays = new HashSet<>();
     }
 
     @Override
     public void endTest() {
-        visitedWays.clear();
         super.endTest();
+        // free memory
+        visitedWays = null;
+        reported = null;
     }
 
     @Override
     public boolean isPrimitiveUsable(OsmPrimitive p) {
-        // test only nodes and Ways with at least 2 nodes
-        return p.isUsable() && ((p instanceof Node && p.isDrawable()) || (p instanceof Way && ((Way) p).getNodesCount() > 1));
+        if (!p.isUsable()) {
+            return false;
+        } else if (p instanceof Way && ((Way) p).getNodesCount() > 1) {
+            // test only Ways with at least 2 nodes
+            return true;
+        } else if (p instanceof Node && p.isDrawable()) {
+            // test all nodes - ways referred by them may not be checked automatically.
+            return true;
+        } else {
+            return false;
+        }
     }
 }