Changeset 6390 in josm for trunk/src

Timestamp:

2013-11-18T02:20:41+01:00 (10 years ago)

Author:

Don-vip

Message:

fix #9318 - validator: avoids redundant checks between UnclosedWays and MultipolygonTest

Location:

trunk/src/org/openstreetmap/josm

Files:

• data/validation/tests/MultipolygonTest.java (modified) (6 diffs)
• data/validation/tests/UnclosedWays.java (modified) (3 diffs)
• gui/mappaint/ElemStyles.java (modified) (1 diff)

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

@@ -9 +9 @@
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Set;
 
 import org.openstreetmap.josm.Main;
@@ -22 +24 @@
 import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon.PolyData.Intersection;
 import org.openstreetmap.josm.data.osm.visitor.paint.relations.MultipolygonCache;
+import org.openstreetmap.josm.data.validation.OsmValidator;
 import org.openstreetmap.josm.data.validation.Severity;
 import org.openstreetmap.josm.data.validation.Test;
@@ -28 +31 @@
 import org.openstreetmap.josm.gui.mappaint.ElemStyles;
 import org.openstreetmap.josm.gui.mappaint.MapPaintStyles;
+import org.openstreetmap.josm.gui.progress.ProgressMonitor;
 
 /**
@@ -50 +54 @@
 
     private final List<List<Node>> nonClosedWays = new ArrayList<List<Node>>();
+    private final Set<String> keysCheckedByAnotherTest = new HashSet<String>();
 
     /**
@@ -62 +67 @@
     public void initialize() {
         styles = MapPaintStyles.getStyles();
+    }
+    
+    @Override
+    public void startTest(ProgressMonitor progressMonitor) {
+        super.startTest(progressMonitor);
+        keysCheckedByAnotherTest.clear();
+        for (Test t : OsmValidator.getEnabledTests(false)) {
+            if (t instanceof UnclosedWays) {
+                keysCheckedByAnotherTest.addAll(((UnclosedWays)t).getCheckedKeys());
+                break;
+            }
+        }
     }
 
@@ -124 +141 @@
             List<Node> nodes = w.getNodes();
             if (nodes.size()<1) return; // fix zero nodes bug
-            // Fix #9291 - Do not raise warning for sport=climbing + natural=cliff
-            if (!(w.hasTag("sport", "climbing") && w.hasTag("natural", "cliff"))) {
-                errors.add(new TestError(this, Severity.WARNING, tr("Area style way is not closed"), NOT_CLOSED,
-                        Collections.singletonList(w), Arrays.asList(nodes.get(0), nodes.get(nodes.size() - 1))));
-            }
+            for (String key : keysCheckedByAnotherTest) {
+                if (w.hasKey(key)) {
+                    return;
+                }
+            }
+            errors.add(new TestError(this, Severity.WARNING, tr("Area style way is not closed"), NOT_CLOSED,
+                    Collections.singletonList(w), Arrays.asList(nodes.get(0), nodes.get(nodes.size() - 1))));
         }
     }

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

@@ -7 +7 @@
 import java.text.MessageFormat;
 import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
 
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
@@ -20 +24 @@
  *
  * @author stoecker
+ * @since 3669
  */
 public class UnclosedWays extends Test {
 
     /**
-     * Constructor
+     * Constructs a new {@code UnclosedWays} test.
      */
     public UnclosedWays() {
@@ -30 +35 @@
     }
 
-    private String type;
-    private String etype;
-    private int mode;
-
-    public void set(int m, String text, String desc) {
-        etype = MessageFormat.format(text, desc);
-        type = tr(text, tr(desc));
-        mode = m;
+    /**
+     * A check performed by UnclosedWays test.
+     * @since 6390
+     */
+    private class UnclosedWaysCheck {
+        /** The unique numeric code for this check */
+        public final int code;
+        /** The OSM key checked */
+        public final String key;
+        /** The English message */
+        private final String engMessage;
+        /** The special values, to be ignored if ignore is set to true; to be considered only if ignore is set to false */
+        private final List<String> specialValues;
+        /** The boolean indicating if special values must be ignored or considered only */
+        private final boolean ignore;
+        
+        /**
+         * Constructs a new {@code UnclosedWaysCheck}. 
+         * @param code The unique numeric code for this check
+         * @param key The OSM key checked
+         * @param engMessage The English message
+         */
+        @SuppressWarnings("unchecked")
+        public UnclosedWaysCheck(int code, String key, String engMessage) {
+            this(code, key, engMessage, (List<String>) Collections.EMPTY_LIST);
+        }
+        
+        /**
+         * Constructs a new {@code UnclosedWaysCheck}. 
+         * @param code The unique numeric code for this check
+         * @param key The OSM key checked
+         * @param engMessage The English message
+         * @param ignoredValues The ignored values.
+         */
+        public UnclosedWaysCheck(int code, String key, String engMessage, List<String> ignoredValues) {
+            this(code, key, engMessage, ignoredValues, true);
+        }
+        
+        /**
+         * Constructs a new {@code UnclosedWaysCheck}. 
+         * @param code The unique numeric code for this check
+         * @param key The OSM key checked
+         * @param engMessage The English message
+         * @param specialValues The special values, to be ignored if ignore is set to true; to be considered only if ignore is set to false
+         * @param ignore indicates if special values must be ignored or considered only 
+         */
+        public UnclosedWaysCheck(int code, String key, String engMessage, List<String> specialValues, boolean ignore) {
+            this.code = code;
+            this.key = key;
+            this.engMessage = engMessage;
+            this.specialValues = specialValues;
+            this.ignore = ignore;
+        }
+        
+        /**
+         * Returns the test error of the given way, if any.
+         * @param w The way to check
+         * @return The test error if the way is erroneous, {@code null} otherwise 
+         */
+        public final TestError getTestError(Way w) {
+            String value = w.get(key);
+            if (isValueErroneous(value)) {
+                String  type = engMessage.contains("{0}") ? tr(engMessage, tr(value)) : tr(engMessage);
+                String etype = engMessage.contains("{0}") ? MessageFormat.format(engMessage, value) : engMessage;
+                return new TestError(UnclosedWays.this, Severity.WARNING, tr("Unclosed way"),
+                        type, etype, code, Arrays.asList(w),
+                        // The important parts of an unclosed way are the first and
+                        // the last node which should be connected, therefore we highlight them
+                        Arrays.asList(w.firstNode(), w.lastNode()));
+            }
+            return null;
+        }
+        
+        protected boolean isValueErroneous(String value) {
+            return value != null && ignore != specialValues.contains(value);
+        }
     }
 
-    public void set(int m, String text) {
-        etype = text;
-        type = tr(text);
-        mode = m;
+    /**
+     * A check performed by UnclosedWays test where the key is treated as boolean.
+     * @since 6390
+     */
+    private final class UnclosedWaysBooleanCheck extends UnclosedWaysCheck {
+
+        /**
+         * Constructs a new {@code UnclosedWaysBooleanCheck}. 
+         * @param code The unique numeric code for this check
+         * @param key The OSM key checked
+         * @param engMessage The English message
+         */
+        public UnclosedWaysBooleanCheck(int code, String key, String engMessage) {
+            super(code, key, engMessage);
+        }
+
+        @Override
+        protected boolean isValueErroneous(String value) {
+            Boolean btest = OsmUtils.getOsmBoolean(value);
+            // Not a strict boolean comparison to handle building=house like a building=yes
+            return (btest != null && btest) || (btest == null && value != null);
+        }
+    }
+
+    private final UnclosedWaysCheck[] checks = {
+        new UnclosedWaysCheck(1101, "natural",   marktr("natural type {0}"), Arrays.asList("coastline", "cliff", "tree_row")),
+        new UnclosedWaysCheck(1102, "landuse",   marktr("landuse type {0}")),
+        new UnclosedWaysCheck(1103, "amenities", marktr("amenities type {0}")),
+        new UnclosedWaysCheck(1104, "sport",     marktr("sport type {0}"), Arrays.asList("water_slide", "climbing")),
+        new UnclosedWaysCheck(1105, "tourism",   marktr("tourism type {0}")),
+        new UnclosedWaysCheck(1106, "shop",      marktr("shop type {0}")),
+        new UnclosedWaysCheck(1107, "leisure",   marktr("leisure type {0}"), Arrays.asList("track")),
+        new UnclosedWaysCheck(1108, "waterway",  marktr("waterway type {0}"), Arrays.asList("riverbank"), false),
+        new UnclosedWaysBooleanCheck(1120, "building", marktr("building")),
+        new UnclosedWaysBooleanCheck(1130, "area",     marktr("area")),
+    };
+    
+    /**
+     * Returns the set of checked OSM keys.
+     * @return The set of checked OSM keys.
+     * @since 6390
+     */
+    public Set<String> getCheckedKeys() {
+        Set<String> keys = new HashSet<String>();
+        for (UnclosedWaysCheck c : checks) {
+            keys.add(c.key);
+        }
+        return keys;
     }
 
     @Override
     public void visit(Way w) {
-        String test;
-        type = etype = null;
-        mode = 0;
 
-        if (!w.isUsable())
+        if (!w.isUsable() || w.isArea())
             return;
 
-        test = w.get("natural");
-        if (test != null && !"coastline".equals(test) && !"cliff".equals(test) && !"tree_row".equals(test)) {
-            set(1101, marktr("natural type {0}"), test);
-        }
-        test = w.get("landuse");
-        if (test != null) {
-            set(1102, marktr("landuse type {0}"), test);
-        }
-        test = w.get("amenities");
-        if (test != null) {
-            set(1103, marktr("amenities type {0}"), test);
-        }
-        test = w.get("sport");
-        if (test != null && !test.equals("water_slide") && !test.equals("climbing")) {
-            set(1104, marktr("sport type {0}"), test);
-        }
-        test = w.get("tourism");
-        if (test != null) {
-            set(1105, marktr("tourism type {0}"), test);
-        }
-        test = w.get("shop");
-        if (test != null) {
-            set(1106, marktr("shop type {0}"), test);
-        }
-        test = w.get("leisure");
-        if (test != null && !"track".contains(test)) {
-            set(1107, marktr("leisure type {0}"), test);
-        }
-        test = w.get("waterway");
-        if ("riverbank".equals(test)) {
-            set(1108, marktr("waterway type {0}"), test);
-        }
-        Boolean btest = OsmUtils.getOsmBoolean(w.get("building"));
-        if (btest != null && btest) {
-            set(1120, marktr("building"));
-        }
-        btest = OsmUtils.getOsmBoolean(w.get("area"));
-        if (btest != null && btest) {
-            set(1130, marktr("area"));
+        for (OsmPrimitive parent: w.getReferrers()) {
+            if (parent instanceof Relation && ((Relation)parent).isMultipolygon())
+                return;
         }
 
-        if (type != null && !w.isArea()) {
-            for (OsmPrimitive parent: w.getReferrers()) {
-                if (parent instanceof Relation && ((Relation)parent).isMultipolygon())
-                    return;
+        for (UnclosedWaysCheck c : checks) {
+            TestError error = c.getTestError(w);
+            if (error != null) {
+                errors.add(error);
+                return;
             }
-
-            errors.add(new TestError(this, Severity.WARNING, tr("Unclosed way"),
-                    type, etype, mode,
-                    Arrays.asList(w),
-                    // The important parts of an unclosed way are the first and
-                    // the last node which should be connected, therefore we highlight them
-                    Arrays.asList(w.firstNode(), w.lastNode())));
         }
     }

trunk/src/org/openstreetmap/josm/gui/mappaint/ElemStyles.java

@@ -450 +450 @@
      * we pretend it is. This is useful for generating area styles from the (segmented)
      * outer ways of a multipolygon.
-     * @return {@code true} iff primitive has an AreaElemStyle
+     * @return {@code true} if primitive has an AreaElemStyle
      */
     public static boolean hasAreaElemStyle(OsmPrimitive p, boolean pretendWayIsClosed) {