Fix some artifact issues

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

src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/GetDataRunnable.java

@@ -14,6 +14,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Optional;
+import java.util.TreeMap;
 import java.util.concurrent.RecursiveTask;
 import java.util.stream.Collectors;
 
@@ -26,12 +27,15 @@ import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.Tag;
 import org.openstreetmap.josm.data.osm.UploadPolicy;
+import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.data.osm.WaySegment;
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor.CancelListener;
 import org.openstreetmap.josm.io.IllegalDataException;
 import org.openstreetmap.josm.plugins.mapwithai.MapWithAIPlugin;
 import org.openstreetmap.josm.plugins.mapwithai.commands.MergeDuplicateWays;
+import org.openstreetmap.josm.tools.Geometry;
 import org.openstreetmap.josm.tools.HttpClient;
 import org.openstreetmap.josm.tools.HttpClient.Response;
 import org.openstreetmap.josm.tools.Logging;
@@ -53,6 +57,9 @@ public class GetDataRunnable extends RecursiveTask<DataSet> implements CancelLis
 
     private static final int MAX_NUMBER_OF_BBOXES_TO_PROCESS = 1;
     private static final String SERVER_ID_KEY = "server_id";
+    private static final int DEFAULT_TIMEOUT = 50_000; // 50 seconds
+
+    private static final double ARTIFACT_ANGLE = 0.1745; // 10 degrees in radians
 
     /**
      * @param bbox    The initial bbox to get data from (don't reduce beforehand --
@@ -110,6 +117,9 @@ public class GetDataRunnable extends RecursiveTask<DataSet> implements CancelLis
                 removeCommonTags(dataSet);
                 mergeNodes(dataSet);
                 cleanupDataSet(dataSet);
+                mergeWays(dataSet);
+                dataSet.getWays().parallelStream().filter(way -> !way.isDeleted())
+                .forEach(GetDataRunnable::cleanupArtifacts);
                 for (int i = 0; i < 5; i++) {
                     new MergeDuplicateWays(dataSet).executeCommand();
                 }
@@ -177,9 +187,13 @@ public class GetDataRunnable extends RecursiveTask<DataSet> implements CancelLis
             bbox.addPrimitive(n1, 0.001);
             final List<Node> nearbyNodes = dataSet.searchNodes(bbox).parallelStream()
                     .filter(node -> !node.isDeleted() && !node.equals(n1)
-                            && n1.getKeys().equals(node.getKeys())
+                            && ((n1.getKeys().equals(node.getKeys()) || n1.getKeys().isEmpty()
+                                    || node.getKeys().isEmpty())
                                     && n1.getCoor().greatCircleDistance(node.getCoor()) < MapWithAIPreferenceHelper
-                            .getMaxNodeDistance())
+                                    .getMaxNodeDistance()
+                                    || !n1.getKeys().isEmpty() && n1.getKeys().equals(node.getKeys())
+                                    && n1.getCoor().greatCircleDistance(
+                                            node.getCoor()) < MapWithAIPreferenceHelper.getMaxNodeDistance() * 10))
                     .collect(Collectors.toList());
             final Command mergeCommand = MergeNodesAction.mergeNodes(nearbyNodes, n1);
             if (mergeCommand != null) {
@@ -189,6 +203,102 @@ public class GetDataRunnable extends RecursiveTask<DataSet> implements CancelLis
         }
     }
 
+    private static void mergeWays(DataSet dataSet) {
+        final List<Way> ways = dataSet.getWays().parallelStream().filter(way -> !way.isDeleted()).collect(Collectors.toList());
+        for (int i = 0; i < ways.size(); i++) {
+            final Way way1 = ways.get(i);
+            final BBox bbox = new BBox();
+            bbox.addPrimitive(way1, 0.001);
+            final List<Way> nearbyWays = dataSet.searchWays(bbox).parallelStream().filter(way -> way.getNodes().parallelStream().filter(node -> way1.getNodes().contains(node)).count() > 1).collect(Collectors.toList());
+            way1.getNodePairs(false);
+            nearbyWays.parallelStream().flatMap(way2 -> checkWayDuplications(way1, way2).entrySet().parallelStream())
+            .forEach(GetDataRunnable::addMissingElement);
+        }
+    }
+
+    protected static void addMissingElement(Map.Entry<WaySegment, List<WaySegment>> entry) {
+        Way way = entry.getKey().way;
+        Way waySegmentWay = entry.getKey().toWay();
+        Node toAdd = entry.getValue().parallelStream()
+                .flatMap(seg -> Arrays.asList(seg.getFirstNode(), seg.getSecondNode()).parallelStream())
+                .filter(node -> !waySegmentWay.containsNode(node)).findAny().orElse(null);
+        if (toAdd != null
+                && Geometry.getDistance(waySegmentWay, toAdd) < MapWithAIPreferenceHelper.getMaxNodeDistance() * 10) {
+            way.addNode(entry.getKey().lowerIndex + 1, toAdd);
+        }
+        for (int i = 0; i < way.getNodesCount() - 2; i++) {
+            Node node0 = way.getNode(i);
+            Node node3 = way.getNode(i + 2);
+            if (node0.equals(node3)) {
+                List<Node> nodes = way.getNodes();
+                nodes.remove(i + 2); // SonarLint doesn't like this (if it was i instead of i + 2, it would be an
+                // issue)
+                way.setNodes(nodes);
+            }
+        }
+    }
+
+    protected static void cleanupArtifacts(Way way) {
+        for (int i = 0; i < way.getNodesCount() - 2; i++) {
+            Node node0 = way.getNode(i);
+            Node node1 = way.getNode(i + 1);
+            Node node2 = way.getNode(i + 2);
+            double angle = Geometry.getCornerAngle(node0.getEastNorth(), node1.getEastNorth(), node2.getEastNorth());
+            if (angle < ARTIFACT_ANGLE) {
+                List<Node> nodes = way.getNodes();
+                nodes.remove(i + 1); // not an issue since I'm adding it back
+                nodes.add(i + 2, node1);
+            }
+        }
+        if (way.getNodesCount() == 2 && way.getDataSet() != null) {
+            BBox tBBox = new BBox();
+            tBBox.addPrimitive(way, 0.001);
+            if (way.getDataSet().searchWays(tBBox).parallelStream()
+                    .filter(tWay -> !way.equals(tWay) && !tWay.isDeleted()).anyMatch(
+                            tWay -> Geometry.getDistance(way, tWay) < MapWithAIPreferenceHelper.getMaxNodeDistance())) {
+                way.setDeleted(true);
+            }
+        }
+    }
+    protected static Map<WaySegment, List<WaySegment>> checkWayDuplications(Way way1, Way way2) {
+        List<WaySegment> waySegments1 = way1.getNodePairs(false).stream()
+                .map(pair -> WaySegment.forNodePair(way1, pair.a, pair.b)).collect(Collectors.toList());
+        List<WaySegment> waySegments2 = way2.getNodePairs(false).stream()
+                .map(pair -> WaySegment.forNodePair(way2, pair.a, pair.b)).collect(Collectors.toList());
+        Map<WaySegment, List<WaySegment>> partials = new TreeMap<>();
+        for (WaySegment segment1 : waySegments1) {
+            boolean same = false;
+            boolean first = false;
+            boolean second = false;
+            List<WaySegment> replacements = new ArrayList<>();
+            for (WaySegment segment2 : waySegments2) {
+                same = segment1.isSimilar(segment2);
+                if (same) {
+                    break;
+                }
+                if (Math.max(Geometry.getDistance(way1, segment2.getFirstNode()), Geometry.getDistance(way1,
+                        segment2.getSecondNode())) < MapWithAIPreferenceHelper.getMaxNodeDistance() * 10) {
+                    if (!first && (segment1.getFirstNode().equals(segment2.getFirstNode())
+                            || segment1.getFirstNode().equals(segment2.getSecondNode()))) {
+                        replacements.add(segment2);
+                        first = true;
+                    } else if (!second && (segment1.getSecondNode().equals(segment2.getFirstNode())
+                            || segment1.getSecondNode().equals(segment2.getSecondNode()))) {
+                        replacements.add(segment2);
+                        second = true;
+                    }
+                }
+            }
+            if (same) {
+                continue;
+            }
+            if (replacements.size() == 2) {
+                partials.put(segment1, replacements);
+            }
+        }
+        return partials;
+    }
+
     /**
      * Actually get the data
      *
@@ -197,6 +307,7 @@ public class GetDataRunnable extends RecursiveTask<DataSet> implements CancelLis
      * @return A dataset with the data from the bbox
      */
     private static DataSet getDataReal(BBox bbox, ProgressMonitor monitor) {
+        // Logging.error(bbox.toStringCSV(","));
         InputStream inputStream = null;
         final DataSet dataSet = new DataSet();
         String urlString = MapWithAIPreferenceHelper.getMapWithAIUrl();
@@ -210,6 +321,7 @@ public class GetDataRunnable extends RecursiveTask<DataSet> implements CancelLis
             final URL url = new URL(urlString.replace("{bbox}", bbox.toStringCSV(",")).replace("{crop_bbox}",
                     DetectTaskingManagerUtils.getTaskingManagerBBox().toStringCSV(",")));
             client = HttpClient.create(url);
+            client.setReadTimeout(DEFAULT_TIMEOUT);
             final StringBuilder defaultUserAgent = new StringBuilder();
             defaultUserAgent.append(client.getHeaders().get("User-Agent"));
             if (defaultUserAgent.toString().trim().length() == 0) {

src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIAction.java

@@ -43,7 +43,8 @@ public class MapWithAIAction extends JosmAction {
         if (isEnabled()) {
             final boolean hasLayer = MapWithAIDataUtils.getLayer(false) != null;
             final List<OsmDataLayer> osmLayers = MainApplication.getLayerManager().getLayersOfType(OsmDataLayer.class)
-                    .stream().filter(layer -> !(layer instanceof MapWithAILayer)).collect(Collectors.toList());
+                    .stream().filter(layer -> !(layer instanceof MapWithAILayer)).filter(Layer::isVisible)
+                    .collect(Collectors.toList());
             final OsmDataLayer layer = getOsmLayer(osmLayers);
             if (layer != null && MapWithAIDataUtils.getMapWithAIData(MapWithAIDataUtils.getLayer(true), layer)) {
                 createMessageDialog();

src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/MergeDuplicateWays.java

@@ -92,7 +92,8 @@ public class MergeDuplicateWays extends Command {
 
     public static void filterDataSet(DataSet dataSet, List<Command> commands) {
         final List<Way> ways = new ArrayList<>(
-                dataSet.getWays().parallelStream().filter(prim -> !prim.isIncomplete()).collect(Collectors.toList()));
+                dataSet.getWays().parallelStream().filter(prim -> !prim.isIncomplete() && !prim.isDeleted())
+                        .collect(Collectors.toList()));
         for (int i = 0; i < ways.size(); i++) {
             final Way way1 = ways.get(i);
             final Collection<Way> nearbyWays = dataSet.searchWays(way1.getBBox()).parallelStream()

test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/GetDataRunnableTest.java

@@ -0,0 +1,50 @@
+// License: GPL. For details, see LICENSE file.
+package org.openstreetmap.josm.plugins.mapwithai.backend;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+
+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.Node;
+import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.data.osm.WaySegment;
+import org.openstreetmap.josm.testutils.JOSMTestRules;
+
+public class GetDataRunnableTest {
+    @Rule
+    public JOSMTestRules rule = new JOSMTestRules().projection();
+
+    @Test
+    public void testAddMissingElement() {
+        Way way1 = TestUtils.newWay("", new Node(new LatLon(-5.7117803, 34.5011898)),
+                new Node(new LatLon(-5.7111915, 34.5013994)), new Node(new LatLon(-5.7104175, 34.5016354)));
+        Way way2 = new Way(way1);
+        way2.addNode(1, new Node(new LatLon(-5.7115826, 34.5012438)));
+        Map<WaySegment, List<WaySegment>> map = GetDataRunnable.checkWayDuplications(way1, way2);
+        GetDataRunnable.addMissingElement(map.entrySet().iterator().next());
+
+        assertEquals(4, way1.getNodesCount());
+        assertEquals(4, way2.getNodesCount());
+
+        way1.removeNode(way1.getNode(1));
+
+        List<Node> nodes = way2.getNodes();
+        Collections.reverse(nodes);
+        way2.setNodes(nodes);
+
+        map = GetDataRunnable.checkWayDuplications(way1, way2);
+        GetDataRunnable.addMissingElement(map.entrySet().iterator().next());
+
+        assertEquals(4, way1.getNodesCount());
+        assertEquals(4, way2.getNodesCount());
+        assertTrue(way1.getNodes().containsAll(way2.getNodes()));
+    }
+
+}