Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/Addresses.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/Addresses.java (revision 15042) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/Addresses.java (revision 15043) @@ -8,4 +8,5 @@ import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -62,4 +63,5 @@ protected static final String ADDR_PLACE = "addr:place"; protected static final String ADDR_STREET = "addr:street"; + protected static final String ADDR_SUBURB = "addr:suburb"; protected static final String ADDR_CITY = "addr:city"; protected static final String ADDR_UNIT = "addr:unit"; @@ -72,5 +74,4 @@ private Map> knownAddresses; private Set ignoredAddresses; - /** @@ -104,23 +105,24 @@ } - protected void checkHouseNumbersWithoutStreet(OsmPrimitive p) { - List associatedStreets = getAndCheckAssociatedStreets(p); + /** + * Checks for house numbers for which the street is unknown. + * @param p primitive to test + * @return error found, or null + */ + protected TestError checkHouseNumbersWithoutStreet(OsmPrimitive p) { // Find house number without proper location // (neither addr:street, associatedStreet, addr:place, addr:neighbourhood or addr:interpolation) - if (p.hasKey(ADDR_HOUSE_NUMBER) && !p.hasKey(ADDR_STREET, ADDR_PLACE, ADDR_NEIGHBOURHOOD)) { - for (Relation r : associatedStreets) { - if (r.hasTag("type", ASSOCIATED_STREET)) { - return; - } - } - if (p.referrers(Way.class).anyMatch(w -> w.hasKey(ADDR_INTERPOLATION) && w.hasKey(ADDR_STREET))) { - return; - } - // No street found - errors.add(TestError.builder(this, Severity.WARNING, HOUSE_NUMBER_WITHOUT_STREET) - .message(tr("House number without street")) - .primitives(p) - .build()); - } + if (p.hasKey(ADDR_HOUSE_NUMBER) && !p.hasKey(ADDR_STREET, ADDR_PLACE, ADDR_NEIGHBOURHOOD) + && getAndCheckAssociatedStreets(p).isEmpty() + && p.referrers(Way.class).noneMatch(w -> w.hasKey(ADDR_INTERPOLATION) && w.hasKey(ADDR_STREET))) { + // no street found + TestError e = TestError.builder(this, Severity.WARNING, HOUSE_NUMBER_WITHOUT_STREET) + .message(tr("House number without street")) + .primitives(p) + .build(); + errors.add(e); + return e; + } + return null; } @@ -177,14 +179,12 @@ } - protected void checkForDuplicate(OsmPrimitive p) { + protected List checkForDuplicate(OsmPrimitive p) { if (knownAddresses == null) { initAddressMap(p); } if (!isPOI(p) && hasAddress(p)) { + List result = new ArrayList<>(); String simplifiedAddress = getSimplifiedAddress(p); - if (ignoredAddresses.contains(simplifiedAddress)) { - return; - } - if (knownAddresses.containsKey(simplifiedAddress)) { + if (!ignoredAddresses.contains(simplifiedAddress) && knownAddresses.containsKey(simplifiedAddress)) { double maxDistance = MAX_DUPLICATE_DISTANCE.get(); for (OsmPrimitive p2 : knownAddresses.get(simplifiedAddress)) { @@ -198,8 +198,9 @@ 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))) { + 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 differs + // address including city identical but postcode or suburb differs // most likely perfectly fine severityLevel = Severity.OTHER; @@ -228,5 +229,5 @@ } } - errors.add(TestError.builder(this, severityLevel, DUPLICATE_HOUSE_NUMBER) + 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()); @@ -234,5 +235,8 @@ knownAddresses.get(simplifiedAddress).remove(p); // otherwise we would get every warning two times } - } + errors.addAll(result); + return result; + } + return Collections.emptyList(); } Index: /trunk/test/unit/org/openstreetmap/josm/data/validation/tests/AddressesTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/data/validation/tests/AddressesTest.java (revision 15043) +++ /trunk/test/unit/org/openstreetmap/josm/data/validation/tests/AddressesTest.java (revision 15043) @@ -0,0 +1,116 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.data.validation.tests; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.openstreetmap.josm.data.coor.LatLon.NORTH_POLE; +import static org.openstreetmap.josm.data.coor.LatLon.SOUTH_POLE; +import static org.openstreetmap.josm.data.coor.LatLon.ZERO; + +import java.util.List; + +import org.junit.Rule; +import org.junit.Test; +import org.openstreetmap.josm.TestUtils; +import org.openstreetmap.josm.data.coor.LatLon; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.Node; +import org.openstreetmap.josm.data.osm.RelationMember; +import org.openstreetmap.josm.data.validation.Severity; +import org.openstreetmap.josm.data.validation.TestError; +import org.openstreetmap.josm.testutils.JOSMTestRules; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + +/** + * JUnit Test of {@link Addresses} validation test. + */ +public class AddressesTest { + + /** + * Setup test. + */ + @Rule + @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") + public JOSMTestRules test = new JOSMTestRules(); + + private static Node createAddressNode(String nodeTags, String wayTags, String relationTags) { + DataSet ds = new DataSet(); + Node n = TestUtils.newNode(nodeTags); + ds.addPrimitive(n); + if (wayTags != null) { + ds.addPrimitive(TestUtils.newWay(wayTags, n)); + } + if (relationTags != null) { + ds.addPrimitive(TestUtils.newRelation(relationTags, new RelationMember(null, n))); + } + return n; + } + + private static TestError doTestHouseNumberWithoutStreet(String nodeTags, String wayTags, String relationTags) { + return new Addresses().checkHouseNumbersWithoutStreet(createAddressNode(nodeTags, wayTags, relationTags)); + } + + /** + * Unit test of {@link Addresses#HOUSE_NUMBER_WITHOUT_STREET} + */ + @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)); + } + + 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); + List errors = new Addresses().checkForDuplicate(n2); + assertEquals(expected != null ? 1 : 0, errors.size()); + if (expected != null) { + assertEquals(expected, errors.get(0).getSeverity()); + } + } + + /** + * Unit test of {@link Addresses#DUPLICATE_HOUSE_NUMBER} + */ + @Test + public void testDuplicateHouseNumber() { + String num1 = "addr:housenumber=1 addr:street=Foo "; + String num2 = "addr:housenumber=2 addr:street=Foo "; + String city1 = "addr:city=Gotham "; + String city2 = "addr:city=Metropolis "; + String suburb1 = "addr:suburb=Queens "; + String suburb2 = "addr:suburb=Bronx "; + // Warning for same addresses at close distance + doTestDuplicateHouseNumber(num1, ZERO, num1, ZERO, Severity.WARNING); + // Info for same addresses at long distance + doTestDuplicateHouseNumber(num1, SOUTH_POLE, num1, NORTH_POLE, Severity.OTHER); + // 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); + } +} Index: /trunk/test/unit/org/openstreetmap/josm/data/validation/tests/CoastlinesTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/data/validation/tests/CoastlinesTest.java (revision 15042) +++ /trunk/test/unit/org/openstreetmap/josm/data/validation/tests/CoastlinesTest.java (revision 15043) @@ -11,5 +11,5 @@ /** - * JUnit Test of coastline validation test. + * JUnit Test of {@link Coastlines} validation test. */ public class CoastlinesTest {