diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/StreetAddressTest.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/StreetAddressTest.java index bfd56dab..8ec2633f 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/StreetAddressTest.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/StreetAddressTest.java @@ -4,31 +4,36 @@ import static org.openstreetmap.josm.tools.I18n.marktr; import static org.openstreetmap.josm.tools.I18n.tr; +import java.awt.geom.Point2D; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; -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.ILatLon; import org.openstreetmap.josm.data.osm.BBox; -import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.INode; import org.openstreetmap.josm.data.osm.IPrimitive; import org.openstreetmap.josm.data.osm.IWay; import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.Relation; import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.data.projection.ProjectionRegistry; +import org.openstreetmap.josm.data.validation.OsmValidator; 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.data.validation.util.ValUtil; import org.openstreetmap.josm.plugins.mapwithai.MapWithAIPlugin; -import org.openstreetmap.josm.spi.preferences.Config; import org.openstreetmap.josm.tools.CheckParameterUtil; import org.openstreetmap.josm.tools.Pair; @@ -40,6 +45,8 @@ public class StreetAddressTest extends Test { public static final double BBOX_EXPANSION = 0.002; private static final String ADDR_STREET = "addr:street"; private final Set namePrimitiveMap = new HashSet<>(); + private final Map> nameMap = new HashMap<>(); + private final Map> primitiveCellMap = new HashMap<>(); /** * Classified highways. This uses a {@link Set} instead of a {@link List} since * the MapWithAI code doesn't care about order. @@ -76,10 +83,29 @@ public void visit(Node node) { @Override public void endTest() { + for (Map.Entry> entry : this.primitiveCellMap.entrySet()) { + Collection names = getSurroundingHighwayNames(entry.getKey()); + for (OsmPrimitive primitive : entry.getValue()) { + if (!primitive.isOutsideDownloadArea()) { + if (!names.contains(primitive.get(ADDR_STREET))) { + namePrimitiveMap.add(primitive); + } + if (this.isCanceled()) { + break; + } + } + } + if (this.isCanceled()) { + break; + } + } + Map> values = namePrimitiveMap.stream() .collect(Collectors.groupingBy(p -> p.get(ADDR_STREET))); values.forEach(this::createError); namePrimitiveMap.clear(); + this.nameMap.clear(); + this.primitiveCellMap.clear(); } /** @@ -96,34 +122,66 @@ public void createError(String addrStreet, List primitives) { } private void realVisit(OsmPrimitive primitive) { - if (primitive.isUsable() && hasStreetAddressTags(primitive) && !primitive.isOutsideDownloadArea()) { - Collection surroundingWays = getSurroundingHighways(primitive); - Collection names = getWayNames(surroundingWays); - if (!names.contains(primitive.get(ADDR_STREET))) { - namePrimitiveMap.add(primitive); + if (primitive.isUsable()) { + final double gridDetail = OsmValidator.getGridDetail() / 100; + if (isHighway(primitive)) { + Collection names = getWayNames((Way) primitive); + if (names.isEmpty()) { + return; + } + List nodes = ((Way) primitive).getNodes(); + for (int i = 0; i < nodes.size() - 1; i++) { + // Populate the name map + INode n1 = nodes.get(i); + INode n2 = nodes.get(i + 1); + for (Point2D cell : ValUtil.getSegmentCells(n1, n2, gridDetail)) { + this.nameMap.computeIfAbsent(cell, k -> new HashSet<>()).addAll(names); + } + } + } else if (hasStreetAddressTags(primitive) && !primitive.isOutsideDownloadArea()) { + final EastNorth en; + if (primitive instanceof Node) { + en = ((Node) primitive).getEastNorth(); + } else { + en = primitive.getBBox().getCenter().getEastNorth(ProjectionRegistry.getProjection()); + } + long x = (long) Math.floor(en.getX() * gridDetail); + long y = (long) Math.floor(en.getY() * gridDetail); + Point2D point = new Point2D.Double(x, y); + primitiveCellMap.computeIfAbsent(point, p -> new ArrayList<>()).add(primitive); } } } - private static Collection getWayNames(Collection ways) { - return ways.stream().flatMap(w -> w.getInterestingTags().entrySet().stream()) + private static Collection getWayNames(Way way) { + return way.getInterestingTags().entrySet().stream() .filter(e -> (e.getKey().contains("name") || e.getKey().contains("ref")) && !e.getKey().contains("tiger")) .map(Map.Entry::getValue).flatMap(s -> Stream.of(s.split(";", -1))).map(String::trim) .filter(s -> !s.isEmpty()).collect(Collectors.toSet()); } - private static Collection getSurroundingHighways(OsmPrimitive address) { - Objects.requireNonNull(address.getDataSet(), "Node must be part of a dataset"); - DataSet ds = address.getDataSet(); - BBox addrBox = expandBBox(new BBox(address.getBBox()), BBOX_EXPANSION); - int expansions = 0; - int maxExpansions = Config.getPref().getInt("mapwithai.validator.streetaddresstest.maxexpansions", 20); - while (ds.searchWays(addrBox).stream().noneMatch(StreetAddressTest::isHighway) && expansions < maxExpansions) { - expandBBox(addrBox, BBOX_EXPANSION); - expansions++; + private Collection getSurroundingHighwayNames(Point2D point2D) { + if (this.nameMap.isEmpty()) { + return Collections.emptySet(); + } + Set surroundingWays = new HashSet<>(); + int surrounding = 1; + while (surroundingWays.isEmpty()) { + for (int x = -surrounding; x <= surrounding; x++) { + for (int y = -surrounding; y <= surrounding; y++) { + Point2D key = new Point2D.Double((long) Math.floor(point2D.getX() + x), + (long) Math.floor(point2D.getY() + y)); + if (this.nameMap.containsKey(key)) { + surroundingWays.addAll(this.nameMap.get(key)); + } + } + } + if (surroundingWays.isEmpty()) { + surrounding++; + } } - return ds.searchWays(addrBox).stream().filter(StreetAddressTest::isHighway).collect(Collectors.toSet()); + return surroundingWays; } /** @@ -154,7 +212,7 @@ static Pair distanceToWay(Way way, OsmPrimitive prim) { final Node b = wayNodes.get(i + 1); for (Node node : nodes) { double tDist = getSegmentNodeDistSq(a, b, node); - if (Double.isNaN(tDist) || (!Double.isNaN(tDist) && tDist < dist)) { + if (Double.isNaN(dist) || (!Double.isNaN(tDist) && tDist < dist)) { dist = tDist; } } diff --git a/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/StreetAddressTestTest.java b/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/StreetAddressTestTest.java index 85cb6100..a38b8c7a 100644 --- a/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/StreetAddressTestTest.java +++ b/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/StreetAddressTestTest.java @@ -8,6 +8,7 @@ import java.lang.reflect.Method; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.openstreetmap.josm.TestUtils; @@ -17,52 +18,59 @@ import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.data.validation.OsmValidator; import org.openstreetmap.josm.testutils.JOSMTestRules; -import org.openstreetmap.josm.tools.Geometry; +import org.openstreetmap.josm.testutils.annotations.BasicPreferences; import org.openstreetmap.josm.tools.Pair; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +@BasicPreferences // Needed due to OsmValidator static initialization initializing tests which in + // turn need preferences class StreetAddressTestTest { private final static String ADDR_STREET = "addr:street"; @RegisterExtension @SuppressFBWarnings("URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") - JOSMTestRules test = new JOSMTestRules().projection(); + static JOSMTestRules test = new JOSMTestRules().projection(); + + @BeforeAll + static void setup() { + OsmValidator.initializeGridDetail(); + } @Test void testVisitWay() throws ReflectiveOperationException { StreetAddressTest test = new StreetAddressTest(); Way way1 = TestUtils.newWay("", new Node(new LatLon(0, 0)), new Node(new LatLon(1, 1))); DataSet ds = new DataSet(); - way1.getNodes().forEach(ds::addPrimitive); - ds.addPrimitive(way1); + ds.addPrimitiveRecursive(way1); Node node1 = new Node(new LatLon(1, 1.00001)); node1.put(ADDR_STREET, "Test"); ds.addPrimitive(node1); - test.visit(way1); + test.visit(ds.allPrimitives()); test.endTest(); - assertTrue(test.getErrors().isEmpty()); + assertFalse(test.getErrors().isEmpty(), "There should be an error, as the way has no name or highway tag"); way1.put("highway", "residential"); - test.visit(node1); + test.visit(ds.allPrimitives()); test.endTest(); assertFalse(test.getErrors().isEmpty()); way1.put("name", "Test1"); test.clear(); - test.visit(node1); + test.visit(ds.allPrimitives()); test.endTest(); assertFalse(test.getErrors().isEmpty()); way1.put("name", "Test"); test.clear(); - test.visit(node1); + test.visit(ds.allPrimitives()); test.endTest(); assertTrue(test.getErrors().isEmpty()); node1.remove(ADDR_STREET); test.clear(); - test.visit(node1); + test.visit(ds.allPrimitives()); test.endTest(); assertTrue(test.getErrors().isEmpty()); @@ -73,7 +81,7 @@ void testVisitWay() throws ReflectiveOperationException { setIncomplete.setAccessible(true); setIncomplete.invoke(firstNode, true); assertTrue(way1.firstNode().isIncomplete()); - test.visit(node1); + test.visit(ds.allPrimitives()); test.endTest(); assertTrue(test.getErrors().isEmpty()); } @@ -89,7 +97,8 @@ void testDistanceToWay() { distance = StreetAddressTest.distanceToWay(way1, node1); assertSame(way1, distance.a); - assertEquals(Geometry.getDistance(way1, node1), distance.b, 0.0); + assertEquals(0, distance.b, 0.0, + "node1 should be on the line (lat-lon). It is not quite on the line (East-North)"); } @Test