Fix https://josm.openstreetmap.de/ticket/18351

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

src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/commands/conflation/DuplicateCommand.java

@@ -6,7 +6,9 @@ import static org.openstreetmap.josm.tools.I18n.tr;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
+import java.util.stream.Collectors;
 
 import org.openstreetmap.josm.actions.MergeNodesAction;
 import org.openstreetmap.josm.command.ChangePropertyCommand;
@@ -85,9 +87,17 @@ public class DuplicateCommand extends AbstractConflationCommand {
     @Override
     public Command getRealCommand() {
         final List<Command> commands = new ArrayList<>();
-        possiblyAffectedPrimitives.stream().filter(Node.class::isInstance).map(Node.class::cast)
+        for (Node tNode : possiblyAffectedPrimitives.stream().filter(Node.class::isInstance).map(Node.class::cast)
-        .filter(node -> node.hasKey(DUPE_KEY))
+                .distinct().filter(node -> node.hasKey(DUPE_KEY)).collect(Collectors.toList())) {
-        .forEach(node -> commands.addAll(duplicateNode(getAffectedDataSet(), node)));
+            List<Command> tCommands = duplicateNode(getAffectedDataSet(), tNode);
+            // We have to execute the command to avoid duplicating the command later. Undo
+            // occurs later, so that the state doesn't actually change.
+            tCommands.forEach(Command::executeCommand);
+            commands.addAll(tCommands);
+        }
+        Collections.reverse(commands);
+        commands.forEach(Command::undoCommand);
+        Collections.reverse(commands);
         Command returnCommand = null;
         if (!commands.isEmpty()) {
             returnCommand = new SequenceCommand(getDescriptionText(), commands);

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

@@ -64,11 +64,10 @@ public class CreateConnectionsCommand extends Command {
      *                   connecting to
      * @param collection The primitives with connection information (currently only
      *                   checks Nodes)
-     * @return A {@link SequenceCommand} to create connections with
+     * @return A {@link Command} to create connections with
      */
-    public static SequenceCommand createConnections(DataSet dataSet, Collection<OsmPrimitive> collection) {
+    public static Command createConnections(DataSet dataSet, Collection<OsmPrimitive> collection) {
         final List<Command> changedKeyList = new ArrayList<>();
-        SequenceCommand returnSequence = null;
         final Collection<OsmPrimitive> realPrimitives = collection.stream().map(dataSet::getPrimitiveById)
                 .filter(Objects::nonNull).collect(Collectors.toList());
         for (final Class<? extends AbstractConflationCommand> abstractCommandClass : getConflationCommands()) {
@@ -90,9 +89,14 @@ public class CreateConnectionsCommand extends Command {
                 changedKeyList.add(actualCommand);
             }
         }
-        if (!changedKeyList.isEmpty()) {
+
+        Command returnSequence = null;
+        if (changedKeyList.size() == 1) {
+            returnSequence = changedKeyList.get(0);
+        } else if (!changedKeyList.isEmpty()) {
             returnSequence = new SequenceCommand(getRealDescriptionText(), changedKeyList);
         }
+
         return returnSequence;
     }
 

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

@@ -85,11 +85,12 @@ public class MapWithAIAddCommand extends Command implements Runnable {
                 if (lock != null) {
                     lock.lock();
                 }
-                final Command movePrimitivesCommand = new MovePrimitiveDataSetCommand(editable, mapWithAI, primitives);
+                if (command == null) {// needed for undo/redo (don't create a new command)
                     final List<OsmPrimitive> allPrimitives = new ArrayList<>();
                     MapWithAIDataUtils.addPrimitivesToCollection(allPrimitives, primitives);
+                    final Command movePrimitivesCommand = new MovePrimitiveDataSetCommand(editable, mapWithAI,
+                            primitives);
                     final Command createConnectionsCommand = createConnections(editable, allPrimitives);
-                if (command == null) { // needed for undo/redo (don't create a new command)
                     command = new SequenceCommand(getDescriptionText(), movePrimitivesCommand, createConnectionsCommand);
                 }
                 command.executeCommand();

test/resources/wiremock/mappings/06_nodes-b2cf8a41-bcb3-4275-890b-8ca5a16180b4.json

@@ -0,0 +1,25 @@
+{
+  "id" : "b2cf8a41-bcb3-4275-890b-8ca5a16180b4",
+  "name" : "06_nodes",
+  "request" : {
+    "url" : "/0.6/nodes?nodes=176220609",
+    "method" : "GET"
+  },
+  "response" : {
+    "status" : 200,
+    "body" : "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<osm version=\"0.6\" generator=\"CGImap 0.7.5 (31804 thorn-03.openstreetmap.org)\" copyright=\"OpenStreetMap and contributors\" attribution=\"http://www.openstreetmap.org/copyright\" license=\"http://opendatacommons.org/licenses/odbl/1-0/\">\n <node id=\"176220609\" visible=\"true\" version=\"3\" changeset=\"8532383\" timestamp=\"2011-06-24T12:36:19Z\" user=\"nm7s9\" uid=\"12434\" lat=\"39.0339521\" lon=\"-108.4874581\"/>\n</osm>\n",
+    "headers" : {
+      "Date" : "Fri, 22 Nov 2019 15:30:17 GMT",
+      "Server" : "Apache/2.4.29 (Ubuntu)",
+      "Strict-Transport-Security" : [ "max-age=31536000; includeSubDomains; preload", "max-age=31536000; includeSubDomains; preload" ],
+      "Expect-CT" : [ "max-age=0, report-uri=\"https://openstreetmap.report-uri.com/r/d/ct/reportOnly\"", "max-age=0, report-uri=\"https://openstreetmap.report-uri.com/r/d/ct/reportOnly\"" ],
+      "Cache-Control" : "private, max-age=0, must-revalidate",
+      "Content-Type" : "text/xml; charset=utf-8",
+      "Keep-Alive" : "timeout=5, max=98",
+      "Connection" : "Keep-Alive"
+    }
+  },
+  "uuid" : "b2cf8a41-bcb3-4275-890b-8ca5a16180b4",
+  "persistent" : true,
+  "insertionIndex" : 19
+}

test/resources/wiremock/mappings/06_nodes-dc4006d6-1db6-48f9-a047-b0885fc88d2a.json

@@ -0,0 +1,25 @@
+{
+  "id" : "dc4006d6-1db6-48f9-a047-b0885fc88d2a",
+  "name" : "06_nodes",
+  "request" : {
+    "url" : "/0.6/nodes?nodes=176232378",
+    "method" : "GET"
+  },
+  "response" : {
+    "status" : 200,
+    "body" : "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<osm version=\"0.6\" generator=\"CGImap 0.7.5 (31803 thorn-03.openstreetmap.org)\" copyright=\"OpenStreetMap and contributors\" attribution=\"http://www.openstreetmap.org/copyright\" license=\"http://opendatacommons.org/licenses/odbl/1-0/\">\n <node id=\"176232378\" visible=\"true\" version=\"3\" changeset=\"8548635\" timestamp=\"2011-06-26T05:36:58Z\" user=\"nm7s9\" uid=\"12434\" lat=\"39.0292629\" lon=\"-108.4875117\"/>\n</osm>\n",
+    "headers" : {
+      "Date" : "Fri, 22 Nov 2019 15:30:17 GMT",
+      "Server" : "Apache/2.4.29 (Ubuntu)",
+      "Strict-Transport-Security" : [ "max-age=31536000; includeSubDomains; preload", "max-age=31536000; includeSubDomains; preload" ],
+      "Expect-CT" : [ "max-age=0, report-uri=\"https://openstreetmap.report-uri.com/r/d/ct/reportOnly\"", "max-age=0, report-uri=\"https://openstreetmap.report-uri.com/r/d/ct/reportOnly\"" ],
+      "Cache-Control" : "private, max-age=0, must-revalidate",
+      "Content-Type" : "text/xml; charset=utf-8",
+      "Keep-Alive" : "timeout=5, max=99",
+      "Connection" : "Keep-Alive"
+    }
+  },
+  "uuid" : "dc4006d6-1db6-48f9-a047-b0885fc88d2a",
+  "persistent" : true,
+  "insertionIndex" : 18
+}

test/resources/wiremock/mappings/capabilities-4bf5d84c-f2bd-44c3-b777-06fbdcbff249.json

@@ -0,0 +1,38 @@
+{
+  "id" : "4bf5d84c-f2bd-44c3-b777-06fbdcbff249",
+  "name" : "capabilities",
+  "request" : {
+    "url" : "/capabilities",
+    "method" : "GET"
+  },
+  "response" : {
+    "status" : 200,
+    "body" : "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<osm version=\"0.6\" generator=\"OpenStreetMap server\" copyright=\"OpenStreetMap and contributors\" attribution=\"http://www.openstreetmap.org/copyright\" license=\"http://opendatacommons.org/licenses/odbl/1-0/\">\n  <api>\n    <version minimum=\"0.6\" maximum=\"0.6\"/>\n    <area maximum=\"0.25\"/>\n    <note_area maximum=\"25\"/>\n    <tracepoints per_page=\"5000\"/>\n    <waynodes maximum=\"2000\"/>\n    <changesets maximum_elements=\"10000\"/>\n    <timeout seconds=\"300\"/>\n    <status database=\"online\" api=\"online\" gpx=\"online\"/>\n  </api>\n  <policy>\n    <imagery>\n      <blacklist regex=\".*\\.google(apis)?\\..*/(vt|kh)[\\?/].*([xyz]=.*){3}.*\"/>\n      <blacklist regex=\"http://xdworld\\.vworld\\.kr:8080/.*\"/>\n      <blacklist regex=\".*\\.here\\.com[/:].*\"/>\n    </imagery>\n  </policy>\n</osm>\n",
+    "headers" : {
+      "Date" : "Fri, 22 Nov 2019 15:30:17 GMT",
+      "Server" : "Apache/2.4.29 (Ubuntu)",
+      "Cache-Control" : "max-age=0, private, must-revalidate",
+      "Vary" : "Origin,Accept-Encoding",
+      "X-Permitted-Cross-Domain-Policies" : "none",
+      "X-XSS-Protection" : "1; mode=block",
+      "X-Request-Id" : "Xdf-Cf9mfoQXu7q7IFuX2gAAAUU",
+      "X-Download-Options" : "noopen",
+      "X-Runtime" : "0.011692",
+      "X-Frame-Options" : "sameorigin",
+      "X-Content-Type-Options" : "nosniff",
+      "Content-Security-Policy" : "default-src 'self'; child-src 'self'; connect-src 'self' piwik.openstreetmap.org; font-src 'none'; form-action 'self'; frame-ancestors 'self'; frame-src 'self'; img-src 'self' data: www.gravatar.com *.wp.com *.tile.openstreetmap.org *.tile.thunderforest.com *.openstreetmap.fr piwik.openstreetmap.org https://openstreetmap-user-avatars.s3.dualstack.eu-west-1.amazonaws.com; manifest-src 'self'; media-src 'none'; object-src 'self'; script-src 'self' piwik.openstreetmap.org; style-src 'self'; worker-src 'none'",
+      "X-Powered-By" : "Phusion Passenger 6.0.4",
+      "Strict-Transport-Security" : "max-age=31536000; includeSubDomains; preload",
+      "Expect-CT" : "max-age=0, report-uri=\"https://openstreetmap.report-uri.com/r/d/ct/reportOnly\"",
+      "Upgrade" : "h2",
+      "ETag" : "W/\"0d683cbc36a97f94c7e1bcc861adc22a-gzip\"",
+      "Status" : "200 OK",
+      "Keep-Alive" : "timeout=5, max=100",
+      "Connection" : "Keep-Alive",
+      "Content-Type" : "application/xml; charset=utf-8"
+    }
+  },
+  "uuid" : "4bf5d84c-f2bd-44c3-b777-06fbdcbff249",
+  "persistent" : true,
+  "insertionIndex" : 17
+}

test/unit/org/openstreetmap/josm/plugins/mapwithai/commands/CreateConnectionsCommandTest.java

@@ -7,6 +7,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
@@ -26,7 +27,7 @@ import org.openstreetmap.josm.plugins.mapwithai.backend.commands.conflation.Dupl
 import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
 
-import com.github.tomakehurst.wiremock.junit.WireMockRule;
+import com.github.tomakehurst.wiremock.WireMockServer;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
@@ -40,12 +41,17 @@ public class CreateConnectionsCommandTest {
     @SuppressFBWarnings("URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
     public JOSMTestRules test = new JOSMTestRules().projection();
 
-    @Rule
+    WireMockServer wireMock = new WireMockServer(options().usingFilesUnderDirectory("test/resources/wiremock"));
-    public WireMockRule wireMockRule = new WireMockRule(options().usingFilesUnderDirectory("test/resources/wiremock"));
 
     @Before
     public void setUp() {
-        Config.getPref().put("osm-server.url", wireMockRule.baseUrl());
+        wireMock.start();
+        Config.getPref().put("osm-server.url", wireMock.baseUrl());
+    }
+
+    @After
+    public void tearDown() {
+        wireMock.stop();
     }
 
     /**
@@ -201,5 +207,4 @@ public class CreateConnectionsCommandTest {
         Assert.assertNotNull(text);
         Assert.assertFalse(text.isEmpty());
     }
-
 }

test/unit/org/openstreetmap/josm/plugins/mapwithai/commands/MapWithAIAddComandTest.java

@@ -1,10 +1,15 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.plugins.mapwithai.commands;
 
+import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
 import java.util.Arrays;
 import java.util.Collections;
 
+import org.junit.After;
 import org.junit.Assert;
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.openstreetmap.josm.TestUtils;
@@ -13,10 +18,14 @@ import org.openstreetmap.josm.data.UndoRedoHandler;
 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.OsmPrimitiveType;
 import org.openstreetmap.josm.data.osm.Tag;
 import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
 
+import com.github.tomakehurst.wiremock.WireMockServer;
+
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
 public class MapWithAIAddComandTest {
@@ -26,6 +35,19 @@ public class MapWithAIAddComandTest {
     @SuppressFBWarnings("URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
     public JOSMTestRules test = new JOSMTestRules().projection();
 
+    WireMockServer wireMock = new WireMockServer(options().usingFilesUnderDirectory("test/resources/wiremock"));
+
+    @Before
+    public void setUp() {
+        wireMock.start();
+        Config.getPref().put("osm-server.url", wireMock.baseUrl());
+    }
+
+    @After
+    public void tearDown() {
+        wireMock.stop();
+    }
+
     @Test
     public void testMoveCollection() {
         final DataSet ds1 = new DataSet();
@@ -170,4 +192,25 @@ public class MapWithAIAddComandTest {
         UndoRedoHandler.getInstance().undo(UndoRedoHandler.getInstance().getUndoCommands().size());
         UndoRedoHandler.getInstance().redo(UndoRedoHandler.getInstance().getRedoCommands().size());
     }
+
+    /**
+     * https://josm.openstreetmap.de/ticket/18351
+     */
+    @Test
+    public void testRegression18351() {
+        Way way = TestUtils.newWay("highway=residential mapwithai:source=MapWithAI source=digitalglobe",
+                new Node(new LatLon(39.0339521, -108.4874581)), new Node(new LatLon(39.0292629, -108.4875117)));
+        way.firstNode().put("dupe", "n176220609");
+        way.getNode(way.getNodesCount() - 1).put("dupe", "n176232378");
+        DataSet mapWithAIData = new DataSet();
+        DataSet osmData = new DataSet();
+        way.getNodes().forEach(mapWithAIData::addPrimitive);
+        mapWithAIData.addPrimitive(way);
+        mapWithAIData.addSelected(way);
+        MapWithAIAddCommand command = new MapWithAIAddCommand(mapWithAIData, osmData, mapWithAIData.getSelected());
+        command.executeCommand();
+        assertNotNull(osmData.getPrimitiveById(176232378, OsmPrimitiveType.NODE));
+        assertNotNull(osmData.getPrimitiveById(176220609, OsmPrimitiveType.NODE));
+        assertNotNull(osmData.getPrimitiveById(way));
+    }
 }