Ticket #18590: 18590.2.patch

File 18590.2.patch, 14.8 KB (added by taylor.smock, 5 years ago)

getSimplifiedAddress -> getSimplifiedAddresses

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

     
    140140     */
    141141    private void collectAddress(OsmPrimitive p) {
    142142        if (!isPOI(p)) {
    143             String simplifiedAddress = getSimplifiedAddresses(p);
     143            getSimplifiedAddresses(p).forEach(simplifiedAddress -> {
    144144            if (!ignoredAddresses.contains(simplifiedAddress)) {
    145145                knownAddresses.computeIfAbsent(simplifiedAddress, x -> new ArrayList<>()).add(p);
    146             }
     146                }
     147            });
    147148        }
    148149    }
    149150
     
    156157                    if (hasAddress(r)) {
    157158                        // ignore addresses of buildings that are connected to addr:unit nodes
    158159                        // it's quite reasonable that there are more buildings with this address
    159                         String simplifiedAddress = getSimplifiedAddresses(r);
     160                        getSimplifiedAddresses(r).forEach(simplifiedAddress -> {
    160161                        if (!ignoredAddresses.contains(simplifiedAddress)) {
    161162                            ignoredAddresses.add(simplifiedAddress);
    162163                        } else if (knownAddresses.containsKey(simplifiedAddress)) {
    163164                            knownAddresses.remove(simplifiedAddress);
    164                         }
     165                            }
     166                        });
    165167                    }
    166168                }
    167169            }
     
    184186        }
    185187        if (!isPOI(p) && hasAddress(p)) {
    186188            List<TestError> result = new ArrayList<>();
    187             String simplifiedAddress = getSimplifiedAddresses(p);
    188             if (!ignoredAddresses.contains(simplifiedAddress) && knownAddresses.containsKey(simplifiedAddress)) {
    189                 double maxDistance = MAX_DUPLICATE_DISTANCE.get();
    190                 for (OsmPrimitive p2 : knownAddresses.get(simplifiedAddress)) {
    191                     if (p == p2) {
    192                         continue;
    193                     }
    194                     Severity severityLevel;
    195                     String city1 = p.get(ADDR_CITY);
    196                     String city2 = p2.get(ADDR_CITY);
    197                     double distance = getDistance(p, p2);
    198                     if (city1 != null && city2 != null) {
    199                         if (city1.equals(city2)) {
    200                             if ((!p.hasKey(ADDR_POSTCODE) || !p2.hasKey(ADDR_POSTCODE) || p.get(ADDR_POSTCODE).equals(p2.get(ADDR_POSTCODE)))
    201                              && (!p.hasKey(ADDR_SUBURB) || !p2.hasKey(ADDR_SUBURB) || p.get(ADDR_SUBURB).equals(p2.get(ADDR_SUBURB)))) {
    202                                 severityLevel = Severity.WARNING;
     189            List<String> simplifiedAddresses = getSimplifiedAddresses(p);
     190            for (String simplifiedAddress : simplifiedAddresses) {
     191                if (!ignoredAddresses.contains(simplifiedAddress) && knownAddresses.containsKey(simplifiedAddress)) {
     192                    double maxDistance = MAX_DUPLICATE_DISTANCE.get();
     193                    for (OsmPrimitive p2 : knownAddresses.get(simplifiedAddress)) {
     194                        if (p == p2) {
     195                            continue;
     196                        }
     197                        Severity severityLevel;
     198                        String city1 = p.get(ADDR_CITY);
     199                        String city2 = p2.get(ADDR_CITY);
     200                        double distance = getDistance(p, p2);
     201                        if (city1 != null && city2 != null) {
     202                            if (city1.equals(city2)) {
     203                                if ((!p.hasKey(ADDR_POSTCODE) || !p2.hasKey(ADDR_POSTCODE)
     204                                        || p.get(ADDR_POSTCODE).equals(p2.get(ADDR_POSTCODE)))
     205                                        && (!p.hasKey(ADDR_SUBURB) || !p2.hasKey(ADDR_SUBURB)
     206                                                || p.get(ADDR_SUBURB).equals(p2.get(ADDR_SUBURB)))) {
     207                                    severityLevel = Severity.WARNING;
     208                                } else {
     209                                    // address including city identical but postcode or suburb differs
     210                                    // most likely perfectly fine
     211                                    severityLevel = Severity.OTHER;
     212                                }
    203213                            } else {
    204                                 // address including city identical but postcode or suburb differs
    205                                 // most likely perfectly fine
    206                                 severityLevel = Severity.OTHER;
     214                                // address differs only by city - notify if very close, otherwise ignore
     215                                if (distance < maxDistance) {
     216                                    severityLevel = Severity.OTHER;
     217                                } else {
     218                                    continue;
     219                                }
    207220                            }
    208221                        } else {
    209                             // address differs only by city - notify if very close, otherwise ignore
    210                             if (distance < maxDistance) {
    211                                 severityLevel = Severity.OTHER;
    212                             } else {
    213                                 continue;
    214                             }
    215                         }
    216                     } else {
    217                         // at least one address has no city specified
    218                         if (p.hasKey(ADDR_POSTCODE) && p2.hasKey(ADDR_POSTCODE) && p.get(ADDR_POSTCODE).equals(p2.get(ADDR_POSTCODE))) {
    219                             // address including postcode identical
    220                             severityLevel = Severity.WARNING;
    221                         } else {
    222                             // city/postcode unclear - warn if very close, otherwise only notify
    223                             // TODO: get city from surrounding boundaries?
    224                             if (distance < maxDistance) {
     222                            // at least one address has no city specified
     223                            if (p.hasKey(ADDR_POSTCODE) && p2.hasKey(ADDR_POSTCODE)
     224                                    && p.get(ADDR_POSTCODE).equals(p2.get(ADDR_POSTCODE))) {
     225                                // address including postcode identical
    225226                                severityLevel = Severity.WARNING;
    226227                            } else {
    227                                 severityLevel = Severity.OTHER;
     228                                // city/postcode unclear - warn if very close, otherwise only notify
     229                                // TODO: get city from surrounding boundaries?
     230                                if (distance < maxDistance) {
     231                                    severityLevel = Severity.WARNING;
     232                                } else {
     233                                    severityLevel = Severity.OTHER;
     234                                }
    228235                            }
    229236                        }
     237                        result.add(
     238                                TestError.builder(this, severityLevel, DUPLICATE_HOUSE_NUMBER)
     239                                        .message(tr("Duplicate house numbers"), marktr("''{0}'' ({1}m)"),
     240                                                simplifiedAddress, (int) distance)
     241                                        .primitives(Arrays.asList(p, p2)).build());
    230242                    }
    231                     result.add(TestError.builder(this, severityLevel, DUPLICATE_HOUSE_NUMBER)
    232                             .message(tr("Duplicate house numbers"), marktr("''{0}'' ({1}m)"), simplifiedAddress, (int) distance)
    233                             .primitives(Arrays.asList(p, p2)).build());
     243                    knownAddresses.get(simplifiedAddress).remove(p); // otherwise we would get every warning two times
    234244                }
    235                 knownAddresses.get(simplifiedAddress).remove(p); // otherwise we would get every warning two times
    236245            }
    237246            errors.addAll(result);
    238247            return result;
     
    240249        return Collections.emptyList();
    241250    }
    242251
    243     static String getSimplifiedAddresses(OsmPrimitive p) {
     252    static List<String> getSimplifiedAddresses(OsmPrimitive p) {
    244253        String simplifiedStreetName = p.hasKey(ADDR_STREET) ? p.get(ADDR_STREET) : p.get(ADDR_PLACE);
    245254        // ignore whitespaces and dashes in street name, so that "Mozart-Gasse", "Mozart Gasse" and "Mozartgasse" are all seen as equal
    246         return Utils.strip(Stream.of(
     255        return expandHouseNumber(p.get(ADDR_HOUSE_NUMBER)).stream().map(addrHouseNumber -> Utils.strip(Stream.of(
    247256                simplifiedStreetName.replaceAll("[ -]", ""),
    248                 p.get(ADDR_HOUSE_NUMBER),
     257                addrHouseNumber,
    249258                p.get(ADDR_HOUSE_NAME),
    250259                p.get(ADDR_UNIT),
    251260                p.get(ADDR_FLATS))
    252261            .filter(Objects::nonNull)
    253262            .collect(Collectors.joining(" ")))
    254                 .toUpperCase(Locale.ENGLISH);
     263                .toUpperCase(Locale.ENGLISH)).collect(Collectors.toList());
    255264    }
    256265
     266    /**
     267     * Split addr:housenumber on , and ; (common separators)
     268     *
     269     * @param houseNumber The housenumber to be split
     270     * @return A list of addr:housenumber equivalents
     271     */
     272    static List<String> expandHouseNumber(String houseNumber) {
     273        return Arrays.asList(houseNumber.split(",|;"));
     274    }
     275
    257276    @Override
    258277    public void visit(Node n) {
    259278        checkHouseNumbersWithoutStreet(n);
  • test/unit/org/openstreetmap/josm/data/validation/tests/AddressesTest.java

     
    5757     */
    5858    @Test
    5959    public void testHouseNumberWithoutStreet() {
    60         assertNull(doTestHouseNumberWithoutStreet(
    61                 "", null, null));
    62         assertNotNull(doTestHouseNumberWithoutStreet(
    63                 "addr:housenumber=1", null, null));
    64         assertNull(doTestHouseNumberWithoutStreet(
    65                 "addr:housenumber=1 addr:street=Foo", null, null));
    66         assertNull(doTestHouseNumberWithoutStreet(
    67                 "addr:housenumber=1 addr:place=Foo", null, null));
    68         assertNull(doTestHouseNumberWithoutStreet(
    69                 "addr:housenumber=1 addr:neighbourhood=Foo", null, null));
    70         assertNotNull(doTestHouseNumberWithoutStreet(
    71                 "addr:housenumber=1", null, "type=enforcement"));
    72         assertNull(doTestHouseNumberWithoutStreet(
    73                 "addr:housenumber=1", null, "type=associatedStreet"));
    74         assertNotNull(doTestHouseNumberWithoutStreet(
    75                 "addr:housenumber=1", "building=yes", null));
    76         assertNull(doTestHouseNumberWithoutStreet(
    77                 "addr:housenumber=1", "addr:interpolation=odd addr:street=Foo", null));
     60        assertNull(doTestHouseNumberWithoutStreet("", null, null));
     61        assertNotNull(doTestHouseNumberWithoutStreet("addr:housenumber=1", null, null));
     62        assertNull(doTestHouseNumberWithoutStreet("addr:housenumber=1 addr:street=Foo", null, null));
     63        assertNull(doTestHouseNumberWithoutStreet("addr:housenumber=1 addr:place=Foo", null, null));
     64        assertNull(doTestHouseNumberWithoutStreet("addr:housenumber=1 addr:neighbourhood=Foo", null, null));
     65        assertNotNull(doTestHouseNumberWithoutStreet("addr:housenumber=1", null, "type=enforcement"));
     66        assertNull(doTestHouseNumberWithoutStreet("addr:housenumber=1", null, "type=associatedStreet"));
     67        assertNotNull(doTestHouseNumberWithoutStreet("addr:housenumber=1", "building=yes", null));
     68        assertNull(
     69                doTestHouseNumberWithoutStreet("addr:housenumber=1", "addr:interpolation=odd addr:street=Foo", null));
    7870    }
    7971
    80     private static void doTestDuplicateHouseNumber(
    81             String tags1, LatLon ll1, String tags2, LatLon ll2, Severity expected) {
     72    private static void doTestDuplicateHouseNumber(String tags1, LatLon ll1, String tags2, LatLon ll2,
     73            Severity expected) {
    8274        DataSet ds = new DataSet();
    83         Node n1 = TestUtils.newNode(tags1); n1.setCoor(ll1); ds.addPrimitive(n1);
    84         Node n2 = TestUtils.newNode(tags2); n2.setCoor(ll2); ds.addPrimitive(n2);
     75        Node n1 = TestUtils.newNode(tags1);
     76        n1.setCoor(ll1);
     77        ds.addPrimitive(n1);
     78        Node n2 = TestUtils.newNode(tags2);
     79        n2.setCoor(ll2);
     80        ds.addPrimitive(n2);
    8581        List<TestError> errors = new Addresses().checkForDuplicate(n2);
    8682        assertEquals(expected != null ? 1 : 0, errors.size());
    8783        if (expected != null) {
     
    107103        // Nothing for different addresses
    108104        doTestDuplicateHouseNumber(num1, ZERO, num2, ZERO, null);
    109105        // Info for same address in different cities, warning if same city
    110         doTestDuplicateHouseNumber(num1+city1, ZERO, num1+city2, ZERO, Severity.OTHER);
    111         doTestDuplicateHouseNumber(num1+city1, ZERO, num1+city1, ZERO, Severity.WARNING);
    112         // Info for same address in same city but different suburbs, warning if same suburb
    113         doTestDuplicateHouseNumber(num1+city1+suburb1, ZERO, num1+city1+suburb2, ZERO, Severity.OTHER);
    114         doTestDuplicateHouseNumber(num1+city1+suburb1, ZERO, num1+city1+suburb1, ZERO, Severity.WARNING);
     106        doTestDuplicateHouseNumber(num1 + city1, ZERO, num1 + city2, ZERO, Severity.OTHER);
     107        doTestDuplicateHouseNumber(num1 + city1, ZERO, num1 + city1, ZERO, Severity.WARNING);
     108        // Info for same address in same city but different suburbs, warning if same
     109        // suburb
     110        doTestDuplicateHouseNumber(num1 + city1 + suburb1, ZERO, num1 + city1 + suburb2, ZERO, Severity.OTHER);
     111        doTestDuplicateHouseNumber(num1 + city1 + suburb1, ZERO, num1 + city1 + suburb1, ZERO, Severity.WARNING);
    115112    }
     113
     114    /**
     115     * Unit test of {@link Addresses#expandHouseNumber}
     116     */
     117    @Test
     118    public void testMultiAddressDuplicates() {
     119        String num1 = "addr:housenumber=1,3 addr:street=Foo";
     120        String num2 = "addr:housenumber=1 addr:street=Foo";
     121        String num3 = "addr:housenumber=3 addr:street=Foo";
     122        String num4 = "addr:housenumber=4 addr:street=Foo";
     123
     124        doTestDuplicateHouseNumber(num1, ZERO, num2, ZERO, Severity.WARNING);
     125        doTestDuplicateHouseNumber(num1, ZERO, num3, ZERO, Severity.WARNING);
     126        doTestDuplicateHouseNumber(num1, ZERO, num4, ZERO, null);
     127
     128        num1 = num1.replace(",", ";");
     129
     130        doTestDuplicateHouseNumber(num1, ZERO, num2, ZERO, Severity.WARNING);
     131        doTestDuplicateHouseNumber(num1, ZERO, num3, ZERO, Severity.WARNING);
     132        doTestDuplicateHouseNumber(num1, ZERO, num4, ZERO, null);
     133    }
    116134}