Ticket #18189: 18189.patch

src/org/openstreetmap/josm/actions/JoinNodeWayAction.java

@@ -18 +18 @@
 import java.util.SortedSet;
 import java.util.TreeSet;
 
+import javax.swing.JOptionPane;
+
 import org.openstreetmap.josm.command.ChangeCommand;
 import org.openstreetmap.josm.command.Command;
 import org.openstreetmap.josm.command.MoveCommand;
@@ -32 +34 @@
 import org.openstreetmap.josm.data.projection.ProjectionRegistry;
 import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.gui.MapView;
+import org.openstreetmap.josm.gui.Notification;
 import org.openstreetmap.josm.tools.Geometry;
 import org.openstreetmap.josm.tools.MultiMap;
 import org.openstreetmap.josm.tools.Shortcut;
@@ -125 +128 @@
         }
 
         // Execute phase: traverse the structure "data" and finally put the nodes into place
+        Map<Node,EastNorth> moved = new HashMap<>();
         for (Map.Entry<Way, MultiMap<Integer, Node>> entry : data.entrySet()) {
             final Way w = entry.getKey();
             final MultiMap<Integer, Node> innerEntry = entry.getValue();
@@ -142 +146 @@
                                 w.getNode(segmentIndex).getEastNorth(),
                                 w.getNode(segmentIndex+1).getEastNorth(),
                                 node.getEastNorth());
-                        MoveCommand c = new MoveCommand(
-                                node, ProjectionRegistry.getProjection().eastNorth2latlon(newPosition));
-                        // Avoid moving a given node several times at the same position in case of overlapping ways
-                        if (!cmds.contains(c)) {
-                            cmds.add(c);
+                        EastNorth prevMove = moved.get(node);
+                        if (prevMove != null) {
+                            if (!prevMove.equalsEpsilon(newPosition, 1e-7)) {
+                                new Notification(tr("Multiple target ways, no common point found. Nothing was changed."))
+                                        .setIcon(JOptionPane.INFORMATION_MESSAGE)
+                                        .show();
+                                return;
+                            }
+                            continue;
                         }
+                        MoveCommand c = new MoveCommand(node,
+                                ProjectionRegistry.getProjection().eastNorth2latlon(newPosition));
+                        cmds.add(c);
+                        moved.put(node, newPosition);
                     }
                 }
                 List<Node> nodesToAdd = new LinkedList<>();

test/unit/org/openstreetmap/josm/actions/JoinNodeWayActionTest.java

@@ +1 @@
+// License: GPL. For details, see LICENSE file.
+package org.openstreetmap.josm.actions;
+
+import static org.junit.Assert.assertTrue;
+
+import java.util.Arrays;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.openstreetmap.josm.data.coor.EastNorth;
+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.gui.MainApplication;
+import org.openstreetmap.josm.gui.layer.OsmDataLayer;
+import org.openstreetmap.josm.testutils.JOSMTestRules;
+import org.openstreetmap.josm.tools.Geometry;
+
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+
+/**
+ * Unit tests for class {@link JoinNodeWayAction}.
+ */
+public final class JoinNodeWayActionTest {
+
+    /**
+     * Setup test.
+     */
+    @Rule
+    @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
+    public JOSMTestRules test = new JOSMTestRules().projection().main().preferences();
+
+    /**
+     * Test case: Move node onto two overlapping ways
+     * see #18189 moveontoway.osm
+     */
+    @Test
+    public void testTicket18189() {
+        DataSet dataSet = new DataSet();
+        OsmDataLayer layer = new OsmDataLayer(dataSet, OsmDataLayer.createNewName(), null);
+        MainApplication.getLayerManager().addLayer(layer);
+        Node n1 = new Node(new EastNorth(1, 1));
+        Node n2 = new Node(new EastNorth(5, 5));
+        Node n3 = new Node(new EastNorth(1, 1.000000001)); // very small difference
+        Node n4 = new Node(new EastNorth(5, 5));
+        Node n5 = new Node(new EastNorth(3, 3.1)); // node to move
+
+        dataSet.addPrimitive(n1);
+        dataSet.addPrimitive(n2);
+        dataSet.addPrimitive(n3);
+        dataSet.addPrimitive(n4);
+        dataSet.addPrimitive(n5);
+
+        Way w1 = new Way();
+        w1.setNodes(Arrays.asList(n1, n2));
+        dataSet.addPrimitive(w1);
+        Way w2 = new Way();
+        w2.setNodes(Arrays.asList(n3, n4));
+        dataSet.addPrimitive(w2);
+
+        dataSet.addSelected(n5);
+        EastNorth expected = Geometry.closestPointToSegment(n1.getEastNorth(), n2.getEastNorth(), n5.getEastNorth());
+
+        JoinNodeWayAction action = JoinNodeWayAction.createMoveNodeOntoWayAction();
+        action.setEnabled(true);
+        action.actionPerformed(null);
+        // Make sure the node was only moved once
+        assertTrue("Node n5 wasn't added to way w1.", w1.containsNode(n5));
+        assertTrue("Node n5 wasn't added to way w2.", w2.containsNode(n5));
+        assertTrue("Node was moved too unexpected position", n5.getEastNorth().equalsEpsilon(expected, 1e-7));
+    }
+}