Changeset 13968 in josm for trunk/src/org/openstreetmap/josm/data

Timestamp:

2018-06-23T01:24:18+02:00 (6 years ago)

Author:

Don-vip

Message:

fix #16310 - check for duplicate addresses (patch by Luzandro, modified)

File:

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

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

@@ -6 +6 @@
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
@@ -13 +14 @@
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Objects;
 import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import org.openstreetmap.josm.data.coor.EastNorth;
+import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
@@ -21 +26 @@
 import org.openstreetmap.josm.data.osm.RelationMember;
 import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.data.preferences.DoubleProperty;
 import org.openstreetmap.josm.data.validation.Severity;
 import org.openstreetmap.josm.data.validation.Test;
 import org.openstreetmap.josm.data.validation.TestError;
-import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.tools.Geometry;
 import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.Pair;
 import org.openstreetmap.josm.tools.SubclassFilteredCollection;
+import org.openstreetmap.josm.tools.Utils;
 
 /**
@@ -41 +47 @@
     protected static final int MULTIPLE_STREET_RELATIONS = 2604;
     protected static final int HOUSE_NUMBER_TOO_FAR = 2605;
+
+    protected static final DoubleProperty MAX_DUPLICATE_DISTANCE = new DoubleProperty("validator.addresses.max_duplicate_distance", 200.0);
+    protected static final DoubleProperty MAX_STREET_DISTANCE = new DoubleProperty("validator.addresses.max_street_distance", 200.0);
 
     // CHECKSTYLE.OFF: SingleSpaceSeparator
@@ -48 +57 @@
     protected static final String ADDR_PLACE         = "addr:place";
     protected static final String ADDR_STREET        = "addr:street";
+    protected static final String ADDR_CITY          = "addr:city";
+    protected static final String ADDR_UNIT          = "addr:unit";
+    protected static final String ADDR_FLATS         = "addr:flats";
+    protected static final String ADDR_HOUSE_NAME    = "addr:housename";
+    protected static final String ADDR_POSTCODE      = "addr:postcode";
     protected static final String ASSOCIATED_STREET  = "associatedStreet";
     // CHECKSTYLE.ON: SingleSpaceSeparator
+
+    private Map<String, Collection<OsmPrimitive>> addresses = null;
+    private Set<String> ignoredAddresses = null;
 
     /**
@@ -103 +120 @@
     }
 
+    static boolean isPOI(OsmPrimitive p) {
+        return p.hasKey("shop", "amenity", "tourism", "leisure", "emergency", "craft", "office", "name");
+    }
+
+    private boolean hasAddress(OsmPrimitive p) {
+        return p.hasKey(ADDR_HOUSE_NUMBER) && p.hasKey(ADDR_STREET, ADDR_PLACE);
+    }
+
+    /**
+     * adds the OsmPrimitive to the address map if it complies to the restrictions
+     * @param p OsmPrimitive that has an address
+     */
+    private void collectAddress(OsmPrimitive p) {
+        if (!isPOI(p)) {
+            String simplifiedAddress = getSimplifiedAddress(p);
+            if (!ignoredAddresses.contains(simplifiedAddress)) {
+                addresses.computeIfAbsent(simplifiedAddress, x -> new ArrayList<>()).add(p);
+            }
+        }
+    }
+
+    protected void initAddressMap(OsmPrimitive primitive) {
+        addresses = new HashMap<>();
+        ignoredAddresses = new HashSet<>();
+        for (OsmPrimitive p : primitive.getDataSet().allNonDeletedPrimitives()) {
+            if (p instanceof Node && p.hasKey(ADDR_UNIT, ADDR_FLATS)) {
+                for (OsmPrimitive r : p.getReferrers()) {
+                    if (hasAddress(r)) {
+                        // ignore addresses of buildings that are connected to addr:unit nodes
+                        // it's quite reasonable that there are more buildings with this address
+                        String simplifiedAddress = getSimplifiedAddress(r);
+                        if (!ignoredAddresses.contains(simplifiedAddress)) {
+                            ignoredAddresses.add(simplifiedAddress);
+                        } else if (addresses.containsKey(simplifiedAddress)) {
+                            addresses.remove(simplifiedAddress);
+                        }
+                    }
+                }
+            }
+            if (hasAddress(p)) {
+                collectAddress(p);
+            }
+        }
+    }
+
+    @Override
+    public void endTest() {
+        addresses = null;
+        ignoredAddresses = null;
+        super.endTest();
+    }
+
+    protected void checkForDuplicate(OsmPrimitive p) {
+        if (addresses == null) {
+            initAddressMap(p);
+        }
+        if (!isPOI(p) && hasAddress(p)) {
+            String simplifiedAddress = getSimplifiedAddress(p);
+            if (ignoredAddresses.contains(simplifiedAddress)) {
+                return;
+            }
+            if (addresses.containsKey(simplifiedAddress)) {
+                double maxDistance = MAX_DUPLICATE_DISTANCE.get();
+                for (OsmPrimitive p2 : addresses.get(simplifiedAddress)) {
+                    if (p == p2) {
+                        continue;
+                    }
+                    Severity severityLevel = Severity.WARNING;
+                    String city1 = p.get(ADDR_CITY);
+                    String city2 = p2.get(ADDR_CITY);
+                    double distance = getDistance(p, p2);
+                    if (city1 != null && city2 != null) {
+                        if (city1.equals(city2)) {
+                            if (!p.hasKey(ADDR_POSTCODE) || !p2.hasKey(ADDR_POSTCODE) || p.get(ADDR_POSTCODE).equals(p2.get(ADDR_POSTCODE))) {
+                                severityLevel = Severity.WARNING;
+                            } else {
+                                // address including city identical but postcode differs
+                                // most likely perfectly fine
+                                severityLevel = Severity.OTHER;
+                            }
+                        } else {
+                            // address differs only by city - notify if very close, otherwise ignore
+                            if (distance < maxDistance) {
+                                severityLevel = Severity.OTHER;
+                            } else {
+                                continue;
+                            }
+                        }
+                    } else {
+                        // at least one address has no city specified
+                        if (p.hasKey(ADDR_POSTCODE) && p2.hasKey(ADDR_POSTCODE) && p.get(ADDR_POSTCODE).equals(p2.get(ADDR_POSTCODE))) {
+                            // address including postcode identical
+                            severityLevel = Severity.WARNING;
+                        } else {
+                            // city/postcode unclear - warn if very close, otherwise only notify
+                            // TODO: get city from surrounding boundaries?
+                            if (distance < maxDistance) {
+                                severityLevel = Severity.WARNING;
+                            } else {
+                                severityLevel = Severity.OTHER;
+                            }
+                        }
+                    }
+                    errors.add(TestError.builder(this, severityLevel, DUPLICATE_HOUSE_NUMBER)
+                            .message(tr("Duplicate house numbers"), marktr("''{0}'' ({1}m)"), simplifiedAddress, (int) distance)
+                            .primitives(Arrays.asList(p, p2)).build());
+                }
+                addresses.get(simplifiedAddress).remove(p); // otherwise we would get every warning two times
+            }
+        }
+    }
+
+    static String getSimplifiedAddress(OsmPrimitive p) {
+        String simplifiedStreetName = p.hasKey(ADDR_STREET) ? p.get(ADDR_STREET) : p.get(ADDR_PLACE);
+        // ignore whitespaces and dashes in street name, so that "Mozart-Gasse", "Mozart Gasse" and "Mozartgasse" are all seen as equal
+        return Utils.strip(Stream.of(
+                simplifiedStreetName.replaceAll("[ -]", ""),
+                p.get(ADDR_HOUSE_NUMBER),
+                p.get(ADDR_HOUSE_NAME),
+                p.get(ADDR_UNIT),
+                p.get(ADDR_FLATS))
+            .filter(Objects::nonNull)
+            .collect(Collectors.joining(" ")))
+                .toUpperCase(Locale.ENGLISH);
+    }
+
     @Override
     public void visit(Node n) {
         checkHouseNumbersWithoutStreet(n);
+        checkForDuplicate(n);
     }
 
@@ -111 +255 @@
     public void visit(Way w) {
         checkHouseNumbersWithoutStreet(w);
+        checkForDuplicate(w);
     }
 
@@ -116 +261 @@
     public void visit(Relation r) {
         checkHouseNumbersWithoutStreet(r);
+        checkForDuplicate(r);
         if (r.hasTag("type", ASSOCIATED_STREET)) {
             // Used to count occurences of each house number in order to find duplicates
@@ -186 +332 @@
     }
 
+    /**
+     * returns rough distance between two OsmPrimitives
+     * @param a primitive a
+     * @param b primitive b
+     * @return distance of center of bounding boxes in meters
+     */
+    static double getDistance(OsmPrimitive a, OsmPrimitive b) {
+        LatLon centerA = a.getBBox().getCenter();
+        LatLon centerB = b.getBBox().getCenter();
+        return (centerA.greatCircleDistance(centerB));
+    }
+
     protected void checkDistance(OsmPrimitive house, Collection<Way> street) {
         EastNorth centroid;
@@ -205 +363 @@
         }
         if (centroid == null) return; // fix #8305
-        double maxDistance = Config.getPref().getDouble("validator.addresses.max_street_distance", 200.0);
+        double maxDistance = MAX_STREET_DISTANCE.get();
         boolean hasIncompleteWays = false;
         for (Way streetPart : street) {