From 7e89da6b3fbb1d9fc3b49ca7a22e8677700a8257 Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Mon, 3 Feb 2020 15:12:27 -0700 Subject: [PATCH] Improve error output (point at the addresses that cause the issue) Signed-off-by: Taylor Smock --- .../validation/tests/StreetAddressOrder.java | 42 ++++++++++++++++--- .../tests/StreetAddressOrderTest.java | 37 +++++++++++++++- 2 files changed, 71 insertions(+), 8 deletions(-) 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 index eb6f408..26dc4f7 100644 --- 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 @@ -7,7 +7,9 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Comparator; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.stream.Collectors; @@ -27,6 +29,7 @@ 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; +import org.openstreetmap.josm.tools.Pair; public class StreetAddressOrder extends Test { private static final SharpAngles ANGLES_TEST = new SharpAngles(); @@ -45,8 +48,8 @@ public class StreetAddressOrder extends Test { .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)); + Map> potentialBadAddresses = new HashMap<>(checkOrdering(leftAddresses)); + potentialBadAddresses.putAll(checkOrdering(rightAddresses)); potentialBadAddresses.forEach(this::createError); } } @@ -81,9 +84,11 @@ public class StreetAddressOrder extends Test { return number; } - public void createError(IPrimitive potentialBadAddress) { + public void createError(IPrimitive potentialBadAddress, Collection surroundingAddresses) { if (potentialBadAddress instanceof OsmPrimitive) { errors.add(TestError.builder(this, Severity.OTHER, 58542100).primitives((OsmPrimitive) potentialBadAddress) + .highlight(surroundingAddresses.stream().filter(OsmPrimitive.class::isInstance) + .map(OsmPrimitive.class::cast).collect(Collectors.toSet())) .message(MapWithAIPlugin.NAME, marktr("Potential bad address")).build()); } } @@ -96,9 +101,9 @@ public class StreetAddressOrder extends Test { * @return Primitives that are out of order * @see SharpAngles */ - public static List checkOrdering(List primitives) { + public static Map> checkOrdering(List primitives) { if (primitives.isEmpty()) { - return Collections.emptyList(); + return Collections.emptyMap(); } List centroids = primitives.stream().map(StreetAddressOrder::getCentroid).filter(Objects::nonNull) .collect(Collectors.toList()); @@ -132,7 +137,32 @@ public class StreetAddressOrder extends Test { .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()); + List badPrimitives = issueCentroids.stream().map(centroids::indexOf).map(primitives::get) + .collect(Collectors.toList()); + Map> badPrimitivesNeighborMap = badPrimitives.stream() + .map(p -> new Pair>(p, getNeighbors(p, primitives))) + .collect(Collectors.toMap(p -> p.a, p -> p.b)); + return badPrimitivesNeighborMap; + } + + /** + * Get the neighbors of a primitive from a list + * + * @param The type of the primitive + * @param p The primitive to get neighbors for + * @param orderedNeighbors The ordered list of primitives of which p is part of + * @return The primitive before/after p + */ + public static List getNeighbors(T p, List orderedNeighbors) { + int index = orderedNeighbors.indexOf(p); + List neighbors = new ArrayList<>(); + if (index > 0) { + neighbors.add(orderedNeighbors.get(index - 1)); + } + if (index < orderedNeighbors.size() - 1) { + neighbors.add(orderedNeighbors.get(index + 1)); + } + return neighbors; } /** 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 index 7e613d7..4b56e5a 100644 --- 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 @@ -86,11 +86,19 @@ public class StreetAddressOrderTest { @Test public void testCreateError() { StreetAddressOrder test = new StreetAddressOrder(); - test.createError(new Node(new LatLon(0, 0)).save()); + test.createError(new Node(new LatLon(0, 0)).save(), Collections.emptyList()); assertTrue(test.getErrors().isEmpty()); - test.createError(new Node(new LatLon(0, 0))); + test.createError(new Node(new LatLon(0, 0)), Collections.emptyList()); assertEquals(1, test.getErrors().size()); + + test.clear(); + test.createError(new Node(new LatLon(0, 0)), Collections.singleton(new Node(new LatLon(1, 1)))); + assertEquals(1, test.getErrors().get(0).getHighlighted().size()); + + test.clear(); + test.createError(new Node(new LatLon(0, 0)), Collections.singleton(new Node(new LatLon(1, 1)).save())); + assertTrue(test.getErrors().get(0).getHighlighted().isEmpty()); } @Test @@ -157,4 +165,29 @@ public class StreetAddressOrderTest { assertEquals(way1Centroid, StreetAddressOrder.getCentroid(relation1).getEastNorth()); } + @Test + public void testGetNeighbors() { + List primitives = new ArrayList<>(); + primitives.add(new Node(new LatLon(0, 0))); + assertTrue(StreetAddressOrder.getNeighbors(primitives.get(0), primitives).isEmpty()); + + primitives.add(new Node(new LatLon(1, 1))); + assertSame(primitives.get(1), StreetAddressOrder.getNeighbors(primitives.get(0), primitives).get(0)); + assertEquals(1, StreetAddressOrder.getNeighbors(primitives.get(1), primitives).size()); + + assertSame(primitives.get(0), StreetAddressOrder.getNeighbors(primitives.get(1), primitives).get(0)); + assertEquals(1, StreetAddressOrder.getNeighbors(primitives.get(1), primitives).size()); + + primitives.add(new Node(new LatLon(2, 2))); + assertSame(primitives.get(1), StreetAddressOrder.getNeighbors(primitives.get(0), primitives).get(0)); + assertEquals(1, StreetAddressOrder.getNeighbors(primitives.get(0), primitives).size()); + + assertSame(primitives.get(0), StreetAddressOrder.getNeighbors(primitives.get(1), primitives).get(0)); + assertSame(primitives.get(2), StreetAddressOrder.getNeighbors(primitives.get(1), primitives).get(1)); + assertEquals(2, StreetAddressOrder.getNeighbors(primitives.get(1), primitives).size()); + + assertSame(primitives.get(1), StreetAddressOrder.getNeighbors(primitives.get(2), primitives).get(0)); + assertEquals(1, StreetAddressOrder.getNeighbors(primitives.get(2), primitives).size()); + } + }