diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/MapWithAIPlugin.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/MapWithAIPlugin.java index 36b52d1..c86b63d 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/MapWithAIPlugin.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/MapWithAIPlugin.java @@ -38,6 +38,7 @@ import org.openstreetmap.josm.plugins.mapwithai.backend.MapWithAIUploadHook; import org.openstreetmap.josm.plugins.mapwithai.backend.MergeDuplicateWaysAction; import org.openstreetmap.josm.plugins.mapwithai.data.validation.tests.ConnectingNodeInformationTest; import org.openstreetmap.josm.plugins.mapwithai.data.validation.tests.RoutingIslandsTest; +import org.openstreetmap.josm.plugins.mapwithai.data.validation.tests.StreetAddressOrder; import org.openstreetmap.josm.plugins.mapwithai.data.validation.tests.StreetAddressTest; import org.openstreetmap.josm.plugins.mapwithai.data.validation.tests.StubEndsTest; import org.openstreetmap.josm.plugins.mapwithai.frontend.MapWithAIDownloadReader; @@ -66,7 +67,7 @@ public final class MapWithAIPlugin extends Plugin implements Destroyable { } private final static List> VALIDATORS = Arrays.asList(RoutingIslandsTest.class, - ConnectingNodeInformationTest.class, StubEndsTest.class, StreetAddressTest.class); + ConnectingNodeInformationTest.class, StubEndsTest.class, StreetAddressTest.class, StreetAddressOrder.class); public MapWithAIPlugin(PluginInformation info) { super(info); diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/StreetAddressOrder.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/StreetAddressOrder.java new file mode 100644 index 0000000..eb6f408 --- /dev/null +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/StreetAddressOrder.java @@ -0,0 +1,190 @@ +package org.openstreetmap.josm.plugins.mapwithai.data.validation.tests; + +import static org.openstreetmap.josm.tools.I18n.marktr; +import static org.openstreetmap.josm.tools.I18n.tr; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; +import java.util.Objects; +import java.util.stream.Collectors; + +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.RelationMember; +import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.data.osm.WaySegment; +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.tests.SharpAngles; +import org.openstreetmap.josm.gui.progress.NullProgressMonitor; +import org.openstreetmap.josm.plugins.mapwithai.MapWithAIPlugin; +import org.openstreetmap.josm.tools.Geometry; +import org.openstreetmap.josm.tools.Logging; + +public class StreetAddressOrder extends Test { + private static final SharpAngles ANGLES_TEST = new SharpAngles(); + + public StreetAddressOrder() { + super(tr("Address order ({0})", MapWithAIPlugin.NAME), tr("Check that street address order makes sense")); + } + + @Override + public void visit(Way way) { + if (way.isUsable() && way.hasTag("highway", StreetAddressTest.CLASSIFIED_HIGHWAYS) && way.hasTag("name")) { + String name = way.get("name"); + List addresses = StreetAddressTest.getNearbyAddresses(way).stream().filter(Objects::nonNull) + .filter(w -> w.hasTag("addr:housenumber")).filter(w -> name.equals(w.get("addr:street"))) + .sorted(Comparator.comparing(p -> convertAddrHouseNumberToDouble(p.get("addr:housenumber")))) + .collect(Collectors.toList()); + List leftAddresses = getAddressesInDirection(true, addresses, way); + List rightAddresses = getAddressesInDirection(false, addresses, way); + List potentialBadAddresses = new ArrayList<>(checkOrdering(leftAddresses)); + potentialBadAddresses.addAll(checkOrdering(rightAddresses)); + potentialBadAddresses.forEach(this::createError); + } + } + + /** + * Convert a housenumber (addr:housenumber) to a double + * + * @param housenumber The housenumber to convert + * @return The double representation, or {@link Double#NaN} if not convertible. + */ + public static double convertAddrHouseNumberToDouble(String housenumber) { + String[] parts = housenumber.split(" "); + double number = 0; + for (String part : parts) { + try { + if (part.contains("/")) { + String[] fractional = part.split("/"); + double tmp = Double.parseDouble(fractional[0]); + for (int i = 1; i < fractional.length; i++) { + tmp = tmp / Double.parseDouble(fractional[i]); + } + number += tmp; + } else { + number += Double.parseDouble(part); + } + } catch (NumberFormatException e) { + Logging.debug("{0} found a malformed number {1}", MapWithAIPlugin.NAME, part); + Logging.debug(e); + number = Double.NaN; + } + } + return number; + } + + public void createError(IPrimitive potentialBadAddress) { + if (potentialBadAddress instanceof OsmPrimitive) { + errors.add(TestError.builder(this, Severity.OTHER, 58542100).primitives((OsmPrimitive) potentialBadAddress) + .message(MapWithAIPlugin.NAME, marktr("Potential bad address")).build()); + } + } + + /** + * Check the ordering of primitives by creating nodes at their centroids and + * checking to see if a sharp angle is created. + * + * @param primitives The primitives to check the order of + * @return Primitives that are out of order + * @see SharpAngles + */ + public static List checkOrdering(List primitives) { + if (primitives.isEmpty()) { + return Collections.emptyList(); + } + List centroids = primitives.stream().map(StreetAddressOrder::getCentroid).filter(Objects::nonNull) + .collect(Collectors.toList()); + + Way way = new Way(); + way.setNodes(centroids); + /* + * if (primitives.get(0) instanceof OsmPrimitive) { DataSet ds = ((OsmPrimitive) + * primitives.get(0)).getDataSet(); // TODO remove after finishing debug + * way.getNodes().stream().filter(p -> p.getDataSet() == null && + * !p.isDeleted()).forEach(ds::addPrimitive); ds.addPrimitive(way); } + */ + double maxDistance = 100; + Node previousCentroid = centroids.get(0); + for (Node centroid : centroids) { + if (previousCentroid.equals(centroid)) { + continue; + } + double tDistance = Geometry.getDistance(centroid, previousCentroid); + previousCentroid = centroid; + if (tDistance > maxDistance) { + maxDistance = tDistance; + } + } + way.put("highway", "residential"); // Required for the SharpAngles test + ANGLES_TEST.startTest(NullProgressMonitor.INSTANCE); + ANGLES_TEST.setMaxLength(maxDistance); + ANGLES_TEST.visit(way); + ANGLES_TEST.endTest(); + List issueCentroids = ANGLES_TEST.getErrors().stream().flatMap(e -> e.getHighlighted().stream()) + .filter(Node.class::isInstance).map(Node.class::cast).collect(Collectors.toList()); + ANGLES_TEST.clear(); + way.setNodes(Collections.emptyList()); + return issueCentroids.stream().map(centroids::indexOf).map(primitives::get).collect(Collectors.toList()); + } + + /** + * Get addresses on different sides of the road + * + * @param left If {@code true}, get addresses on the "left" side of the + * road + * @param addresses Addresses to filter for the side on the road + * @param way The road way + * @return Addresses on the appropriate side of the road + */ + public static List getAddressesInDirection(boolean left, Collection addresses, + IWay way) { + List addressesToReturn = new ArrayList<>(); + for (T address : addresses) { + if (address instanceof OsmPrimitive && way instanceof Way) { + Node centroid = getCentroid(address); + WaySegment seg = Geometry.getClosestWaySegment((Way) way, (OsmPrimitive) address); + boolean right = Geometry.angleIsClockwise(seg.getFirstNode(), seg.getSecondNode(), centroid); + if (left != right) { + addressesToReturn.add(address); + } + } + } + return addressesToReturn; + } + + /** + * Get the centroid of a primitive + * + * @param primitive The primitive to get a centroid for + * @return The node that represents the centroid, or {@code null} if no centroid + * can be determined + */ + public static Node getCentroid(IPrimitive primitive) { + if (primitive instanceof Node) { + return (Node) primitive; + } else if (primitive instanceof Way) { + return new Node(Geometry.getCentroid(((Way) primitive).getNodes())); + } else if (primitive instanceof Relation && "multipolygon".equals(((Relation) primitive).get("type"))) { + // This is not perfect by any stretch of the imagination + List nodes = new ArrayList<>(); + for (RelationMember member : ((Relation) primitive).getMembers()) { + if (member.hasRole("outer")) { + nodes.add(getCentroid(member.getMember())); + } + } + if (!nodes.isEmpty()) { + return new Node(Geometry.getCentroid(nodes)); + } + } + return null; + } + +} diff --git a/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/StreetAddressOrderTest.java b/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/StreetAddressOrderTest.java new file mode 100644 index 0000000..7e613d7 --- /dev/null +++ b/test/unit/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/StreetAddressOrderTest.java @@ -0,0 +1,160 @@ +package org.openstreetmap.josm.plugins.mapwithai.data.validation.tests; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +import org.junit.Rule; +import org.junit.Test; +import org.openstreetmap.josm.TestUtils; +import org.openstreetmap.josm.data.coor.EastNorth; +import org.openstreetmap.josm.data.coor.LatLon; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.IPrimitive; +import org.openstreetmap.josm.data.osm.Node; +import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.PrimitiveData; +import org.openstreetmap.josm.data.osm.Relation; +import org.openstreetmap.josm.data.osm.RelationMember; +import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.testutils.JOSMTestRules; +import org.openstreetmap.josm.tools.Geometry; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + +public class StreetAddressOrderTest { + @Rule + @SuppressFBWarnings("URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") + public JOSMTestRules test = new JOSMTestRules().projection(); + + @Test + public void testVisitWay() { + Way way = TestUtils.newWay("", new Node(new LatLon(0, 0)), new Node(new LatLon(1, 1))); + DataSet ds = new DataSet(); + way.getNodes().forEach(ds::addPrimitive); + ds.addPrimitive(way); + Node tNode = new Node(new LatLon(0, 0.00001)); + ds.addPrimitive(tNode); + tNode = new Node(tNode, true); + tNode.put("addr:street", "Test Road"); + ds.addPrimitive(tNode); + + tNode = new Node(tNode, true); + tNode.setCoor(new LatLon(0.00001, 0.00001)); + tNode.put("addr:housenumber", "1"); + ds.addPrimitive(tNode); + + StreetAddressOrder test = new StreetAddressOrder(); + test.visit(way); + assertTrue(test.getErrors().isEmpty()); + way.put("highway", "residential"); + + test.visit(way); + assertTrue(test.getErrors().isEmpty()); + + way.put("name", "Test Road"); + test.visit(way); + assertTrue(test.getErrors().isEmpty()); + + tNode = new Node(tNode, true); + tNode.setCoor(new LatLon(0.00002, 0.00002)); + tNode.put("addr:housenumber", "2"); + ds.addPrimitive(tNode); + test.visit(way); + assertTrue(test.getErrors().isEmpty()); + + tNode = new Node(tNode, true); + tNode.setCoor(new LatLon(0.000015, 0.000015)); + tNode.put("addr:housenumber", "20"); + ds.addPrimitive(tNode); + test.visit(way); + assertEquals(1, test.getErrors().size()); + + test.clear(); + way.setDeleted(true); + test.visit(way); + assertTrue(test.getErrors().isEmpty()); + } + + @Test + public void testCreateError() { + StreetAddressOrder test = new StreetAddressOrder(); + test.createError(new Node(new LatLon(0, 0)).save()); + assertTrue(test.getErrors().isEmpty()); + + test.createError(new Node(new LatLon(0, 0))); + assertEquals(1, test.getErrors().size()); + } + + @Test + public void testConvertAddrHouseNumberToDouble() { + assertEquals(24.5, StreetAddressOrder.convertAddrHouseNumberToDouble("24 1/2")); + assertEquals(24.5, StreetAddressOrder.convertAddrHouseNumberToDouble("24.5")); + assertEquals(24, StreetAddressOrder.convertAddrHouseNumberToDouble("24")); + + assertEquals(25.5, StreetAddressOrder.convertAddrHouseNumberToDouble("24 3/2")); + assertTrue(Double.isNaN(StreetAddressOrder.convertAddrHouseNumberToDouble("Not a number"))); + } + + @Test + public void testCheckOrdering() { + List primitives = new ArrayList<>(); + primitives.add(new Node(new LatLon(0, 0))); + primitives.add(new Node(new LatLon(1, 1))); + primitives.add(new Node(new LatLon(2, 2))); + assertTrue(StreetAddressOrder.checkOrdering(primitives).isEmpty()); + primitives.add(primitives.remove(1)); + assertFalse(StreetAddressOrder.checkOrdering(primitives).isEmpty()); + + assertTrue(StreetAddressOrder.checkOrdering(Collections.emptyList()).isEmpty()); + } + + @Test + public void testGetAddressesInDirection() { + Way way1 = TestUtils.newWay("", new Node(new LatLon(-1, -1)), new Node(new LatLon(1, 1))); + + List addresses = new ArrayList<>(); + assertTrue(StreetAddressOrder.getAddressesInDirection(true, addresses, way1).isEmpty()); + assertTrue(StreetAddressOrder.getAddressesInDirection(false, addresses, way1).isEmpty()); + addresses.add(new Node(new LatLon(1, 0))); + assertSame(addresses.get(0), StreetAddressOrder.getAddressesInDirection(true, addresses, way1).get(0)); + assertTrue(StreetAddressOrder.getAddressesInDirection(false, addresses, way1).isEmpty()); + ((Node) addresses.get(0)).setCoor(new LatLon(0, 1)); + assertSame(addresses.get(0), StreetAddressOrder.getAddressesInDirection(false, addresses, way1).get(0)); + assertTrue(StreetAddressOrder.getAddressesInDirection(true, addresses, way1).isEmpty()); + + assertTrue(StreetAddressOrder.getAddressesInDirection(true, addresses, way1.save()).isEmpty()); + assertTrue(StreetAddressOrder.getAddressesInDirection(false, addresses, way1.save()).isEmpty()); + + List primitiveData = addresses.stream().map(OsmPrimitive::save).collect(Collectors.toList()); + assertTrue(StreetAddressOrder.getAddressesInDirection(true, primitiveData, way1).isEmpty()); + assertTrue(StreetAddressOrder.getAddressesInDirection(false, primitiveData, way1).isEmpty()); + } + + @Test + public void testGetCentroid() { + Node node1 = new Node(new LatLon(0, 0)); + assertSame(node1, StreetAddressOrder.getCentroid(node1)); + assertNull(StreetAddressOrder.getCentroid(node1.save())); + + Way way1 = TestUtils.newWay("", node1, new Node(new LatLon(1, 1)), new Node(new LatLon(0, 1)), node1); + EastNorth way1Centroid = Geometry.getCentroid(way1.getNodes()); + assertEquals(way1Centroid, StreetAddressOrder.getCentroid(way1).getEastNorth()); + + Relation relation1 = TestUtils.newRelation("", new RelationMember("", way1)); + assertNull(StreetAddressOrder.getCentroid(relation1)); + relation1.put("type", "multipolygon"); + assertNull(StreetAddressOrder.getCentroid(relation1)); + relation1.removeMember(0); + relation1.addMember(new RelationMember("outer", way1)); + assertEquals(way1Centroid, StreetAddressOrder.getCentroid(relation1).getEastNorth()); + } + +}