Index: src/org/openstreetmap/josm/data/validation/tests/Addresses.java =================================================================== --- src/org/openstreetmap/josm/data/validation/tests/Addresses.java (revision 15609) +++ src/org/openstreetmap/josm/data/validation/tests/Addresses.java (working copy) @@ -140,10 +140,11 @@ */ private void collectAddress(OsmPrimitive p) { if (!isPOI(p)) { - String simplifiedAddress = getSimplifiedAddress(p); + getSimplifiedAddress(p).forEach(simplifiedAddress -> { if (!ignoredAddresses.contains(simplifiedAddress)) { knownAddresses.computeIfAbsent(simplifiedAddress, x -> new ArrayList<>()).add(p); - } + } + }); } } @@ -156,12 +157,13 @@ 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); + getSimplifiedAddress(r).forEach(simplifiedAddress -> { if (!ignoredAddresses.contains(simplifiedAddress)) { ignoredAddresses.add(simplifiedAddress); } else if (knownAddresses.containsKey(simplifiedAddress)) { knownAddresses.remove(simplifiedAddress); - } + } + }); } } } @@ -184,55 +186,62 @@ } if (!isPOI(p) && hasAddress(p)) { List result = new ArrayList<>(); - String simplifiedAddress = getSimplifiedAddress(p); - if (!ignoredAddresses.contains(simplifiedAddress) && knownAddresses.containsKey(simplifiedAddress)) { - double maxDistance = MAX_DUPLICATE_DISTANCE.get(); - for (OsmPrimitive p2 : knownAddresses.get(simplifiedAddress)) { - if (p == p2) { - continue; - } - Severity severityLevel; - 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))) - && (!p.hasKey(ADDR_SUBURB) || !p2.hasKey(ADDR_SUBURB) || p.get(ADDR_SUBURB).equals(p2.get(ADDR_SUBURB)))) { - severityLevel = Severity.WARNING; + List simplifiedAddresses = getSimplifiedAddress(p); + for (String simplifiedAddress : simplifiedAddresses) { + if (!ignoredAddresses.contains(simplifiedAddress) && knownAddresses.containsKey(simplifiedAddress)) { + double maxDistance = MAX_DUPLICATE_DISTANCE.get(); + for (OsmPrimitive p2 : knownAddresses.get(simplifiedAddress)) { + if (p == p2) { + continue; + } + Severity severityLevel; + 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))) + && (!p.hasKey(ADDR_SUBURB) || !p2.hasKey(ADDR_SUBURB) + || p.get(ADDR_SUBURB).equals(p2.get(ADDR_SUBURB)))) { + severityLevel = Severity.WARNING; + } else { + // address including city identical but postcode or suburb differs + // most likely perfectly fine + severityLevel = Severity.OTHER; + } } else { - // address including city identical but postcode or suburb differs - // most likely perfectly fine - severityLevel = Severity.OTHER; + // address differs only by city - notify if very close, otherwise ignore + if (distance < maxDistance) { + severityLevel = Severity.OTHER; + } else { + continue; + } } } 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) { + // 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 { - severityLevel = Severity.OTHER; + // 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; + } } } + result.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()); } - result.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()); + knownAddresses.get(simplifiedAddress).remove(p); // otherwise we would get every warning two times } - knownAddresses.get(simplifiedAddress).remove(p); // otherwise we would get every warning two times } errors.addAll(result); return result; @@ -240,20 +249,30 @@ return Collections.emptyList(); } - static String getSimplifiedAddress(OsmPrimitive p) { + static List 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( + return expandHouseNumber(p.get(ADDR_HOUSE_NUMBER)).stream().map(addrHouseNumber -> Utils.strip(Stream.of( simplifiedStreetName.replaceAll("[ -]", ""), - p.get(ADDR_HOUSE_NUMBER), + addrHouseNumber, p.get(ADDR_HOUSE_NAME), p.get(ADDR_UNIT), p.get(ADDR_FLATS)) .filter(Objects::nonNull) .collect(Collectors.joining(" "))) - .toUpperCase(Locale.ENGLISH); + .toUpperCase(Locale.ENGLISH)).collect(Collectors.toList()); } + /** + * Split addr:housenumber on , and ; (common separators) + * + * @param houseNumber The housenumber to be split + * @return A list of addr:housenumber equivalents + */ + static List expandHouseNumber(String houseNumber) { + return Arrays.asList(houseNumber.split(",|;")); + } + @Override public void visit(Node n) { checkHouseNumbersWithoutStreet(n); Index: test/unit/org/openstreetmap/josm/data/validation/tests/AddressesTest.java =================================================================== --- test/unit/org/openstreetmap/josm/data/validation/tests/AddressesTest.java (revision 15609) +++ test/unit/org/openstreetmap/josm/data/validation/tests/AddressesTest.java (working copy) @@ -57,31 +57,27 @@ */ @Test public void testHouseNumberWithoutStreet() { - assertNull(doTestHouseNumberWithoutStreet( - "", null, null)); - assertNotNull(doTestHouseNumberWithoutStreet( - "addr:housenumber=1", null, null)); - assertNull(doTestHouseNumberWithoutStreet( - "addr:housenumber=1 addr:street=Foo", null, null)); - assertNull(doTestHouseNumberWithoutStreet( - "addr:housenumber=1 addr:place=Foo", null, null)); - assertNull(doTestHouseNumberWithoutStreet( - "addr:housenumber=1 addr:neighbourhood=Foo", null, null)); - assertNotNull(doTestHouseNumberWithoutStreet( - "addr:housenumber=1", null, "type=enforcement")); - assertNull(doTestHouseNumberWithoutStreet( - "addr:housenumber=1", null, "type=associatedStreet")); - assertNotNull(doTestHouseNumberWithoutStreet( - "addr:housenumber=1", "building=yes", null)); - assertNull(doTestHouseNumberWithoutStreet( - "addr:housenumber=1", "addr:interpolation=odd addr:street=Foo", null)); + assertNull(doTestHouseNumberWithoutStreet("", null, null)); + assertNotNull(doTestHouseNumberWithoutStreet("addr:housenumber=1", null, null)); + assertNull(doTestHouseNumberWithoutStreet("addr:housenumber=1 addr:street=Foo", null, null)); + assertNull(doTestHouseNumberWithoutStreet("addr:housenumber=1 addr:place=Foo", null, null)); + assertNull(doTestHouseNumberWithoutStreet("addr:housenumber=1 addr:neighbourhood=Foo", null, null)); + assertNotNull(doTestHouseNumberWithoutStreet("addr:housenumber=1", null, "type=enforcement")); + assertNull(doTestHouseNumberWithoutStreet("addr:housenumber=1", null, "type=associatedStreet")); + assertNotNull(doTestHouseNumberWithoutStreet("addr:housenumber=1", "building=yes", null)); + assertNull( + doTestHouseNumberWithoutStreet("addr:housenumber=1", "addr:interpolation=odd addr:street=Foo", null)); } - private static void doTestDuplicateHouseNumber( - String tags1, LatLon ll1, String tags2, LatLon ll2, Severity expected) { + private static void doTestDuplicateHouseNumber(String tags1, LatLon ll1, String tags2, LatLon ll2, + Severity expected) { DataSet ds = new DataSet(); - Node n1 = TestUtils.newNode(tags1); n1.setCoor(ll1); ds.addPrimitive(n1); - Node n2 = TestUtils.newNode(tags2); n2.setCoor(ll2); ds.addPrimitive(n2); + Node n1 = TestUtils.newNode(tags1); + n1.setCoor(ll1); + ds.addPrimitive(n1); + Node n2 = TestUtils.newNode(tags2); + n2.setCoor(ll2); + ds.addPrimitive(n2); List errors = new Addresses().checkForDuplicate(n2); assertEquals(expected != null ? 1 : 0, errors.size()); if (expected != null) { @@ -107,10 +103,32 @@ // Nothing for different addresses doTestDuplicateHouseNumber(num1, ZERO, num2, ZERO, null); // Info for same address in different cities, warning if same city - doTestDuplicateHouseNumber(num1+city1, ZERO, num1+city2, ZERO, Severity.OTHER); - doTestDuplicateHouseNumber(num1+city1, ZERO, num1+city1, ZERO, Severity.WARNING); - // Info for same address in same city but different suburbs, warning if same suburb - doTestDuplicateHouseNumber(num1+city1+suburb1, ZERO, num1+city1+suburb2, ZERO, Severity.OTHER); - doTestDuplicateHouseNumber(num1+city1+suburb1, ZERO, num1+city1+suburb1, ZERO, Severity.WARNING); + doTestDuplicateHouseNumber(num1 + city1, ZERO, num1 + city2, ZERO, Severity.OTHER); + doTestDuplicateHouseNumber(num1 + city1, ZERO, num1 + city1, ZERO, Severity.WARNING); + // Info for same address in same city but different suburbs, warning if same + // suburb + doTestDuplicateHouseNumber(num1 + city1 + suburb1, ZERO, num1 + city1 + suburb2, ZERO, Severity.OTHER); + doTestDuplicateHouseNumber(num1 + city1 + suburb1, ZERO, num1 + city1 + suburb1, ZERO, Severity.WARNING); } + + /** + * Unit test of {@link Addresses#expandHouseNumber} + */ + @Test + public void testMultiAddressDuplicates() { + String num1 = "addr:housenumber=1,3 addr:street=Foo"; + String num2 = "addr:housenumber=1 addr:street=Foo"; + String num3 = "addr:housenumber=3 addr:street=Foo"; + String num4 = "addr:housenumber=4 addr:street=Foo"; + + doTestDuplicateHouseNumber(num1, ZERO, num2, ZERO, Severity.WARNING); + doTestDuplicateHouseNumber(num1, ZERO, num3, ZERO, Severity.WARNING); + doTestDuplicateHouseNumber(num1, ZERO, num4, ZERO, null); + + num1 = num1.replace(",", ";"); + + doTestDuplicateHouseNumber(num1, ZERO, num2, ZERO, Severity.WARNING); + doTestDuplicateHouseNumber(num1, ZERO, num3, ZERO, Severity.WARNING); + doTestDuplicateHouseNumber(num1, ZERO, num4, ZERO, null); + } }