Changeset 14906 in josm for trunk/src/org

Timestamp:

2019-03-19T06:27:58+01:00 (6 years ago)

Author:

GerdP

Message:

fix #17475: Validator should mark obsolete type=associatedStreet relations

File:

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

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

@@ -19 +19 @@
 import java.util.stream.Stream;
 
+import org.openstreetmap.josm.command.Command;
+import org.openstreetmap.josm.command.DeleteCommand;
 import org.openstreetmap.josm.data.coor.EastNorth;
 import org.openstreetmap.josm.data.coor.LatLon;
@@ -25 +27 @@
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.RelationMember;
+import org.openstreetmap.josm.data.osm.TagMap;
 import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.data.preferences.DoubleProperty;
@@ -34 +37 @@
 import org.openstreetmap.josm.tools.Pair;
 import org.openstreetmap.josm.tools.SubclassFilteredCollection;
+import org.openstreetmap.josm.tools.Territories;
 import org.openstreetmap.josm.tools.Utils;
 
@@ -47 +51 @@
     protected static final int MULTIPLE_STREET_RELATIONS = 2604;
     protected static final int HOUSE_NUMBER_TOO_FAR = 2605;
+    protected static final int OBSOLETE_RELATION = 2606;
 
     protected static final DoubleProperty MAX_DUPLICATE_DISTANCE = new DoubleProperty("validator.addresses.max_duplicate_distance", 200.0);
@@ -67 +72 @@
     private Map<String, Collection<OsmPrimitive>> knownAddresses;
     private Set<String> ignoredAddresses;
+
 
     /**
@@ -262 +268 @@
         checkForDuplicate(r);
         if (r.hasTag("type", ASSOCIATED_STREET)) {
+            checkIfObsolete(r);
             // Used to count occurrences of each house number in order to find duplicates
             Map<String, List<OsmPrimitive>> map = new HashMap<>();
@@ -390 +397 @@
                 .build());
     }
+
+    /**
+     * Check if an associatedStreet Relation is obsolete. This test marks only those relations which
+     * are complete and don't contain any information which isn't also tagged on the members.
+     * The strategy is to avoid any false positive.
+     * @param r the relation
+     */
+    private void checkIfObsolete(Relation r) {
+        if (r.isIncomplete())
+            return;
+        /** array of country codes for which the test should be performed. For now, only Germany */
+        String[] countryCodes = {"DE"};
+        TagMap neededtagsForHouse = new TagMap();
+        for (Entry<String, String> tag : r.getKeys().entrySet()) {
+            String key = tag.getKey();
+            if (key.startsWith("name:")) {
+                return; // maybe check if all members have corresponding tags?
+            } else if (key.startsWith("addr:")) {
+                neededtagsForHouse.put(key, tag.getValue());
+            } else {
+                switch (key) {
+                case "name":
+                case "type":
+                case "source":
+                    break;
+                default:
+                    // unexpected tag in relation
+                    return;
+                }
+            }
+        }
+
+        for (RelationMember m : r.getMembers()) {
+            if (m.getMember().isIncomplete() || m.isRelation() || !isInWarnCountry(m, countryCodes))
+                return;
+
+            String role = m.getRole();
+            if ("".equals(role)) {
+                if (m.isWay() && m.getMember().hasKey("highway")) {
+                    role = "street";
+                } else if (m.getMember().hasTag("building"))
+                    role = "house";
+            }
+            switch (role) {
+            case "house":
+            case "addr:houselink":
+            case "address":
+                if (!m.getMember().hasTag(ADDR_STREET) || !m.getMember().hasTag(ADDR_HOUSE_NUMBER))
+                    return;
+                for (Entry<String, String> tag : neededtagsForHouse.entrySet()) {
+                    if (!m.getMember().hasTag(tag.getKey(), tag.getValue()))
+                        return;
+                }
+                break;
+            case "street":
+                if (!m.getMember().hasTag("name") && r.hasTag("name"))
+                    return;
+                break;
+            default:
+                // unknown role: don't create auto-fix
+                return;
+            }
+        }
+        errors.add(TestError.builder(this, Severity.WARNING, OBSOLETE_RELATION)
+                .message(tr("Relation is obsolete"))
+                .primitives(r)
+                .build());
+    }
+
+    private static boolean isInWarnCountry(RelationMember m, String[] countryCodes) {
+        if (countryCodes.length == 0)
+            return true;
+        LatLon center = null;
+
+        if (m.isNode()) {
+            center = m.getNode().getCoor();
+        } else if (m.isWay()) {
+            center = m.getWay().getBBox().getCenter();
+        }
+        if (center == null)
+            return true;
+        for (String country : countryCodes) {
+            if (Territories.isIso3166Code(country, center))
+                return true;
+        }
+        return false;
+    }
+
+    /**
+     * remove obsolete relation.
+     */
+    @Override
+    public Command fixError(TestError testError) {
+        return new DeleteCommand(testError.getPrimitives());
+    }
+
+    @Override
+    public boolean isFixable(TestError testError) {
+        if (!(testError.getTester() instanceof Addresses))
+            return false;
+        return testError.getCode() == OBSOLETE_RELATION;
+    }
+
 }