Add test for stub ends

Signed-off-by: Taylor Smock <taylor.smock@kaart.com>

src/main/java/org/openstreetmap/josm/plugins/mapwithai/MapWithAIPlugin.java

@@ -18,6 +18,7 @@ import javax.swing.JMenuItem;
 
 import org.openstreetmap.josm.actions.JosmAction;
 import org.openstreetmap.josm.data.validation.OsmValidator;
+import org.openstreetmap.josm.data.validation.Test;
 import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.gui.MainMenu;
 import org.openstreetmap.josm.gui.MapFrame;
@@ -37,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.StubEndsTest;
 import org.openstreetmap.josm.plugins.mapwithai.frontend.MapWithAIDownloadReader;
 import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.tools.Destroyable;
@@ -62,6 +64,9 @@ public final class MapWithAIPlugin extends Plugin implements Destroyable {
         MENU_ENTRIES.put(MergeDuplicateWaysAction.class, true);
     }
 
+    private final static List<Class<? extends Test>> VALIDATORS = Arrays.asList(RoutingIslandsTest.class,
+            ConnectingNodeInformationTest.class, StubEndsTest.class);
+
     public MapWithAIPlugin(PluginInformation info) {
         super(info);
 
@@ -81,12 +86,11 @@ public final class MapWithAIPlugin extends Plugin implements Destroyable {
             }
         }
 
-        if (!OsmValidator.getAllAvailableTestClasses().contains(RoutingIslandsTest.class)) {
+        VALIDATORS.forEach(clazz -> {
-            OsmValidator.addTest(RoutingIslandsTest.class);
+            if (!OsmValidator.getAllAvailableTestClasses().contains(clazz)) {
-        }
+                OsmValidator.addTest(clazz);
-        if (!OsmValidator.getAllAvailableTestClasses().contains(ConnectingNodeInformationTest.class)) {
+            }
-            OsmValidator.addTest(ConnectingNodeInformationTest.class);
+        });
-        }
 
         if (!Config.getPref().getKeySet().contains(PAINTSTYLE_PREEXISTS)) {
             Config.getPref().putBoolean(PAINTSTYLE_PREEXISTS, MapWithAIDataUtils.checkIfMapWithAIPaintStyleExists());
@@ -164,8 +168,7 @@ public final class MapWithAIPlugin extends Plugin implements Destroyable {
 
         destroyables.forEach(Destroyable::destroy);
         DownloadDialog.removeDownloadSource(mapWithAIDownloadReader);
-        OsmValidator.removeTest(RoutingIslandsTest.class);
+        VALIDATORS.forEach(OsmValidator::removeTest);
-        OsmValidator.removeTest(ConnectingNodeInformationTest.class);
         DownloadListener.destroyAll();
     }
 }

src/main/java/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/StubEndsTest.java

@@ -0,0 +1,119 @@
+// License: GPL. For details, see LICENSE file.
+package org.openstreetmap.josm.plugins.mapwithai.data.validation.tests;
+
+import static org.openstreetmap.josm.tools.I18n.tr;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import org.openstreetmap.josm.command.ChangeCommand;
+import org.openstreetmap.josm.command.DeleteCommand;
+import org.openstreetmap.josm.command.SequenceCommand;
+import org.openstreetmap.josm.data.osm.Node;
+import org.openstreetmap.josm.data.osm.Way;
+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.TestError.Builder;
+import org.openstreetmap.josm.gui.progress.ProgressMonitor;
+import org.openstreetmap.josm.plugins.mapwithai.MapWithAIPlugin;
+import org.openstreetmap.josm.spi.preferences.Config;
+
+public class StubEndsTest extends Test {
+    private static final String HIGHWAY = "highway";
+    private static final List<String> BAD_HIGHWAYS = Arrays.asList("services", "rest_area");
+    private double max_length = Config.getPref().getDouble(MapWithAIPlugin.NAME + ".stubendlength", 10);
+
+    public StubEndsTest() {
+        super(tr("Stub Ends"), tr("Look for short ends on ways"));
+    }
+
+    @Override
+    public void startTest(ProgressMonitor monitor) {
+        super.startTest(monitor);
+        max_length = Config.getPref().getDouble(MapWithAIPlugin.NAME + ".stubendlength", 10);
+    }
+
+    @Override
+    public void visit(Way way) {
+        if (way.hasTag(HIGHWAY) && !BAD_HIGHWAYS.contains(way.get(HIGHWAY)) && !way.isClosed()) {
+            checkEnds(way);
+        }
+    }
+
+    private void checkEnds(Way way) {
+        List<Node> nodesToFirstConnection = new ArrayList<>();
+        double distanceToFirstConnection = distanceToFirstConnection(way, nodesToFirstConnection);
+        if (distanceToFirstConnection < max_length && !nodesToFirstConnection.isEmpty()) {
+            errors.add(createError(way, nodesToFirstConnection, distanceToFirstConnection));
+        }
+
+        List<Node> nodesToLastConnection = new ArrayList<>();
+        double distanceToLastConnection = distanceToLastConnection(way, nodesToLastConnection);
+        if (distanceToLastConnection < max_length && !nodesToLastConnection.isEmpty()) {
+            errors.add(createError(way, nodesToLastConnection, distanceToLastConnection));
+        }
+    }
+
+    private TestError createError(Way way, List<Node> nodes, double distance) {
+        Builder error = TestError.builder(this, Severity.ERROR, 333300239).message(tr("Stub end ({0}m)", distance))
+                .primitives(way).highlight(nodes);
+        if (way.isNew()) {
+            Way tWay = new Way(way);
+            List<Node> tNodes = tWay.getNodes();
+            boolean reversed = false;
+            if (tWay.lastNode().equals(nodes.get(0))) {
+                reversed = true;
+                Collections.reverse(tNodes);
+            }
+            for (Node node : nodes) {
+                if (tNodes.get(0).equals(node)) {
+                    tNodes.remove(0);
+                }
+            }
+            if (reversed) {
+                Collections.reverse(tNodes);
+            }
+            List<Node> nodesToDelete = nodes.stream().filter(node -> !node.hasKeys())
+                    .filter(node -> node.getReferrers().size() == 1).collect(Collectors.toList());
+            tWay.setNodes(tNodes);
+            nodesToDelete.removeAll(tNodes);
+            error.fix(() -> new SequenceCommand(tr("Remove stub ends"), new ChangeCommand(way, tWay),
+                    DeleteCommand.delete(nodesToDelete)));
+        }
+        return error.build();
+    }
+
+    private static double distanceToFirstConnection(Way way, List<Node> nodesToFirstConnection) {
+        return distanceToConnection(way, nodesToFirstConnection, way.getNodes());
+    }
+
+    private static double distanceToLastConnection(Way way, List<Node> nodesToLastConnection) {
+        List<Node> nodes = way.getNodes();
+        Collections.reverse(nodes);
+        return distanceToConnection(way, nodesToLastConnection, nodes);
+    }
+
+    private static double distanceToConnection(Way way, List<Node> nodesToConnection, List<Node> nodeOrder) {
+        double distance = 0;
+        Node previous = nodeOrder.get(0);
+        for (Node node : nodeOrder) {
+            List<Way> connectingWays = previous.getReferrers().parallelStream().filter(Way.class::isInstance)
+                    .map(Way.class::cast).filter(tWay -> !tWay.equals(way))
+                    .filter(tWay -> tWay.hasTag(HIGHWAY) && !BAD_HIGHWAYS.contains(tWay.get(HIGHWAY)))
+                    .collect(Collectors.toList());
+            if (!node.equals(previous) && connectingWays.isEmpty()) {
+                nodesToConnection.add(previous);
+                distance += node.getCoor().greatCircleDistance(previous.getCoor());
+                previous = node;
+            }
+            if (!connectingWays.isEmpty()) {
+                break;
+            }
+        }
+        return distance;
+    }
+}

test/unit/org/openstreetmap/josm/plugins/mapwithai/data/validation/tests/StubEndsTestTest.java

@@ -0,0 +1,75 @@
+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.assertTrue;
+
+import java.util.Collections;
+import java.util.List;
+
+import org.junit.Before;
+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.Way;
+import org.openstreetmap.josm.testutils.JOSMTestRules;
+
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
+public class StubEndsTestTest {
+    @Rule
+    @SuppressFBWarnings("URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
+    public JOSMTestRules test = new JOSMTestRules().projection();
+    private Way nonStaticWay;
+    private Way staticWay;
+    private StubEndsTest tester;
+
+    @Before
+    public void setUp() {
+        staticWay = TestUtils.newWay("highway=residential", new Node(new LatLon(0, 0)),
+                new Node(new LatLon(0.01, 0.01)));
+        DataSet ds = new DataSet();
+        staticWay.getNodes().forEach(ds::addPrimitive);
+        ds.addPrimitive(staticWay);
+
+        nonStaticWay = TestUtils.newWay("highway=residential", new Node(new LatLon(0.010001, 0.010001)),
+                staticWay.lastNode(), new Node(new LatLon(1, 2)));
+        nonStaticWay.getNodes().stream().filter(node -> node.getDataSet() == null).forEach(ds::addPrimitive);
+        ds.addPrimitive(nonStaticWay);
+
+        tester = new StubEndsTest();
+    }
+
+    @Test
+    public void testStartEnd() {
+        tester.visit(staticWay);
+        assertTrue(tester.getErrors().isEmpty());
+
+        tester.visit(nonStaticWay);
+        assertFalse(tester.getErrors().isEmpty());
+        Node toDelete = nonStaticWay.getNode(0);
+        tester.getErrors().get(0).getFix().executeCommand();
+        assertTrue(toDelete.isDeleted());
+        assertEquals(2, nonStaticWay.getNodesCount());
+    }
+
+    @Test
+    public void testEndEnd() {
+        List<Node> nodes = nonStaticWay.getNodes();
+        Collections.reverse(nodes);
+        nonStaticWay.setNodes(nodes);
+
+        tester.visit(staticWay);
+        assertTrue(tester.getErrors().isEmpty());
+
+        tester.visit(nonStaticWay);
+        assertFalse(tester.getErrors().isEmpty());
+        Node toDelete = nonStaticWay.lastNode();
+        tester.getErrors().get(0).getFix().executeCommand();
+        assertTrue(toDelete.isDeleted());
+        assertEquals(2, nonStaticWay.getNodesCount());
+    }
+}