Better client-side conflation

Instead of performing missing conflation in a parallel code path, we now
add the appropriate tags prior to the rest of the conflation taking
place.

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

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

@@ -27,7 +27,6 @@ import org.openstreetmap.josm.gui.layer.OsmDataLayer;
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.gui.progress.ProgressMonitor;
 import org.openstreetmap.josm.gui.progress.swing.PleaseWaitProgressMonitor;
-import org.openstreetmap.josm.plugins.mapwithai.MapWithAIPlugin;
 import org.openstreetmap.josm.tools.Pair;
 
 public abstract class AbstractConflationCommand extends Command {
@@ -83,22 +82,10 @@ public abstract class AbstractConflationCommand extends Command {
         final OsmPrimitive[] primitiveConnections = new OsmPrimitive[connections.length];
         for (int i = 0; i < connections.length; i++) {
             final String member = connections[i];
-            final char firstChar = member.charAt(0);
+            SimplePrimitiveId primitiveId = SimplePrimitiveId.fromString(member);
-            OsmPrimitiveType type = null;
+            primitiveConnections[i] = dataSet.getPrimitiveById(primitiveId);
-            if (firstChar == 'w') {
-                type = OsmPrimitiveType.WAY;
-            } else if (firstChar == 'n') {
-                type = OsmPrimitiveType.NODE;
-            } else if (firstChar == 'r') {
-                type = OsmPrimitiveType.RELATION;
-            } else {
-                throw new IllegalArgumentException(
-                        tr("{0}: We don't know how to handle {1} types", MapWithAIPlugin.NAME, firstChar));
-            }
-            final long id = Long.parseLong(member.substring(1));
-            primitiveConnections[i] = dataSet.getPrimitiveById(id, type);
             if (primitiveConnections[i] == null) {
-                missingPrimitives.put(i, new Pair<>(id, type));
+                missingPrimitives.put(i, new Pair<>(primitiveId.getUniqueId(), primitiveId.getType()));
             }
         }
         obtainMissingPrimitives(dataSet, primitiveConnections, missingPrimitives);

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

@@ -8,14 +8,16 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Objects;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import javax.swing.JOptionPane;
 
 import org.openstreetmap.josm.actions.AutoScaleAction;
-import org.openstreetmap.josm.command.ChangeCommand;
+import org.openstreetmap.josm.command.ChangePropertyCommand;
 import org.openstreetmap.josm.command.Command;
 import org.openstreetmap.josm.command.SequenceCommand;
 import org.openstreetmap.josm.data.osm.BBox;
@@ -100,11 +102,15 @@ public class MissingConnectionTags extends AbstractConflationCommand {
             MainApplication.getLayerManager().setActiveLayer(current);
         }
         GuiHelper.runInEDT(() -> getAffectedDataSet().setSelected(selection));
-        commands.forEach(Command::undoCommand);
+        if (commands.size() == 1) {
-        return commands.isEmpty() ? null : new SequenceCommand(tr("Perform missing conflation steps"), commands);
+            return commands.iterator().next();
+        } else if (!commands.isEmpty()) {
+            return new SequenceCommand(tr("Perform missing conflation steps"), commands);
+        }
+        return null;
     }
 
-    private void fixErrors(String prefKey, Collection<Command> commands, Collection<TestError> issues) {
+    protected void fixErrors(String prefKey, Collection<Command> commands, Collection<TestError> issues) {
         for (TestError issue : issues) {
             issue.getHighlighted();
             if (!issue.isFixable() || issue.getPrimitives().parallelStream().anyMatch(IPrimitive::isDeleted)) {
@@ -147,7 +153,26 @@ public class MissingConnectionTags extends AbstractConflationCommand {
                 way.getDataSet().searchNodes(searchBBox).stream().filter(MissingConnectionTags::noConflationKey)
                         .forEach(duplicateNodeTest::visit);
                 duplicateNodeTest.endTest();
-                issues.addAll(duplicateNodeTest.getErrors().stream().distinct().collect(Collectors.toList()));
+                if (duplicateNodeTest.getErrors().isEmpty()) {
+                    continue;
+                }
+                List<OsmPrimitive> dupeNodes = duplicateNodeTest.getErrors().stream()
+                        .filter(e -> e.getPrimitives().contains(node)).flatMap(e -> e.getPrimitives().stream())
+                        .distinct().filter(p -> !p.isDeleted() && !p.equals(node)).collect(Collectors.toList());
+                if (dupeNodes.isEmpty()) {
+                    continue;
+                }
+                List<String> dupes = duplicateNodeTest.getErrors().stream()
+                        .filter(e -> e.getPrimitives().contains(node)).flatMap(e -> e.getPrimitives().stream())
+                        .distinct().filter(p -> !p.isDeleted() && !p.equals(node)).map(OsmPrimitive::getPrimitiveId)
+                        .map(Object::toString).collect(Collectors.toList());
+
+                TestError initial = duplicateNodeTest.getErrors().get(0);
+                List<OsmPrimitive> prims = new ArrayList<>(dupeNodes);
+                prims.add(node);
+                issues.add(TestError.builder(initial.getTester(), initial.getSeverity(), initial.getCode())
+                        .message(initial.getMessage()).primitives(prims)
+                        .fix(() -> new ChangePropertyCommand(node, "dupe", String.join(",", dupes))).build());
                 duplicateNodeTest.clear();
             }
         }
@@ -175,7 +200,7 @@ public class MissingConnectionTags extends AbstractConflationCommand {
                     continue;
                 }
                 TestError.Builder fixError = TestError.builder(error.getTester(), error.getSeverity(), error.getCode())
-                        .primitives(error.getPrimitives()).fix(createIntersectionCommandSupplier(error, way))
+                        .primitives(error.getPrimitives()).fix(createIntersectionCommandSupplier(error, way, precision))
                         .message(error.getMessage());
                 seenFix.addAll(error.getPrimitives());
                 issues.add(fixError.build());
@@ -185,7 +210,7 @@ public class MissingConnectionTags extends AbstractConflationCommand {
         return issues;
     }
 
-    private Supplier<Command> createIntersectionCommandSupplier(TestError error, Way way) {
+    private static Supplier<Command> createIntersectionCommandSupplier(TestError error, Way way, double precision) {
         Collection<Node> nodes = Geometry.addIntersections(error.getPrimitives().stream().filter(Way.class::isInstance)
                 .map(Way.class::cast).filter(w -> w.hasKey(HIGHWAY)).collect(Collectors.toList()), false,
                 new ArrayList<>());
@@ -226,12 +251,10 @@ public class MissingConnectionTags extends AbstractConflationCommand {
     private static Command createAddNodeCommand(Way way, Node node, double precision) {
         if (Geometry.getDistance(node, way) < precision) {
             WaySegment seg = Geometry.getClosestWaySegment(way, node);
-            int index1 = way.getNodes().indexOf(seg.getFirstNode());
+            if (seg != null) {
-            int index2 = way.getNodes().indexOf(seg.getSecondNode());
+                return new ChangePropertyCommand(node, "conn",
-            if (index2 - index1 == 1) {
+                        String.join(",", Stream.of(way, seg.getFirstNode(), seg.getSecondNode())
-                Way newWay = new Way(way);
+                                .map(p -> p.getPrimitiveId().toString()).collect(Collectors.toList())));
-                newWay.addNode(index2, node);
-                return new ChangeCommand(way, newWay);
             }
         }
         return null;
@@ -247,10 +270,15 @@ public class MissingConnectionTags extends AbstractConflationCommand {
         // precision is in meters
         double precision = p == 0 ? 1 : p;
 
+        Collection<OsmPrimitive> primsAndChildren = new ArrayList<>(possiblyAffectedPrimitives);
+        possiblyAffectedPrimitives.stream().filter(Way.class::isInstance).map(Way.class::cast).map(Way::getNodes)
+                .forEach(primsAndChildren::addAll);
         Collection<TestError> issues = new ArrayList<>();
         for (TestError issue : unconnectedWays.getErrors()) {
-            if (issue.isFixable()) {
+            if (issue.isFixable() || issue.getPrimitives().stream().noneMatch(t -> primsAndChildren.contains(t))) {
-                issues.add(issue);
+                if (issue.isFixable() && issue.getPrimitives().stream().anyMatch(t -> primsAndChildren.contains(t))) {
+                    issues.add(issue);
+                }
                 continue;
             }
             Way way = Utils.filteredCollection(new ArrayList<>(issue.getPrimitives()), Way.class).stream().findAny()

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

@@ -39,11 +39,11 @@ public class CreateConnectionsCommand extends Command {
     private Command undoCommands;
     private static final LinkedHashSet<Class<? extends AbstractConflationCommand>> CONFLATION_COMMANDS = new LinkedHashSet<>();
     static {
+        CONFLATION_COMMANDS.add(MissingConnectionTags.class);
         CONFLATION_COMMANDS.add(ConnectedCommand.class);
         CONFLATION_COMMANDS.add(DuplicateCommand.class);
         CONFLATION_COMMANDS.add(MergeAddressBuildings.class);
         CONFLATION_COMMANDS.add(MergeBuildingAddress.class);
-        CONFLATION_COMMANDS.add(MissingConnectionTags.class);
         CONFLATION_COMMANDS.add(OverNodedWays.class);
     }
 
@@ -89,8 +89,6 @@ public class CreateConnectionsCommand extends Command {
     public static List<Command> createConnections(DataSet dataSet, Collection<PrimitiveData> collection) {
         final List<Command> permanent = new ArrayList<>();
         final List<Command> undoable = new ArrayList<>();
-        final Collection<OsmPrimitive> realPrimitives = collection.stream().map(dataSet::getPrimitiveById)
-                .filter(Objects::nonNull).collect(Collectors.toList());
         List<Class<? extends AbstractConflationCommand>> runCommands = new ArrayList<>();
         for (final Class<? extends AbstractConflationCommand> abstractCommandClass : getConflationCommands()) {
             final AbstractConflationCommand abstractCommand;
@@ -105,6 +103,8 @@ public class CreateConnectionsCommand extends Command {
             if (runCommands.parallelStream().anyMatch(c -> abstractCommand.conflictedCommands().contains(c))) {
                 continue;
             }
+            final Collection<OsmPrimitive> realPrimitives = collection.stream().map(dataSet::getPrimitiveById)
+                    .filter(Objects::nonNull).collect(Collectors.toList());
             final Collection<OsmPrimitive> tPrimitives = new TreeSet<>();
             abstractCommand.getInterestedTypes()
                     .forEach(clazz -> tPrimitives.addAll(Utils.filteredCollection(realPrimitives, clazz)));

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

@@ -7,12 +7,6 @@ import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import java.util.Arrays;
-import java.util.HashMap;
-import java.util.Map;
-
-import javax.swing.JOptionPane;
-
 import org.awaitility.Awaitility;
 import org.awaitility.Durations;
 import org.junit.Before;
@@ -31,12 +25,9 @@ import org.openstreetmap.josm.gui.layer.OsmDataLayer;
 import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.plugins.mapwithai.backend.commands.conflation.ConnectedCommand;
 import org.openstreetmap.josm.plugins.mapwithai.backend.commands.conflation.DuplicateCommand;
+import org.openstreetmap.josm.plugins.mapwithai.testutils.MissingConnectionTagsMocker;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
-import org.openstreetmap.josm.testutils.mockers.JOptionPaneSimpleMocker;
 import org.openstreetmap.josm.testutils.mockers.WindowMocker;
-import org.openstreetmap.josm.tools.I18n;
-
-import com.google.common.collect.ImmutableMap;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import mockit.Mock;
@@ -65,6 +56,9 @@ public class MapWithAIMoveActionTest {
         way2.getNodes().forEach(node -> osmData.addPrimitive(node));
         osmData.addPrimitive(way2);
         mapWithAIData.addPrimitive(way1);
+        way2.setOsmId(1, 1);
+        way2.firstNode().setOsmId(1, 1);
+        way2.lastNode().setOsmId(2, 1);
 
         osmLayer = new OsmDataLayer(osmData, "osm", null);
         final MapWithAILayer mapWithAILayer = new MapWithAILayer(mapWithAIData, "MapWithAI", null);
@@ -75,14 +69,16 @@ public class MapWithAIMoveActionTest {
 
     @Test
     public void testMoveAction() {
-        new JOptionPaneSimpleMocker(ImmutableMap.of("Sequence: Merge 2 nodes", JOptionPane.NO_OPTION));
+        new MissingConnectionTagsMocker();
 
         mapWithAIData.addSelected(way1);
         moveAction.actionPerformed(null);
         assertEquals(osmLayer, MainApplication.getLayerManager().getActiveLayer(),
                 "Current layer should be the OMS layer");
         assertNotNull(osmLayer.getDataSet().getPrimitiveById(way1), "way1 should have been added to the OSM layer");
-        UndoRedoHandler.getInstance().undo();
+        while (UndoRedoHandler.getInstance().hasUndoCommands()) {
+            UndoRedoHandler.getInstance().undo();
+        }
         assertNull(osmLayer.getDataSet().getPrimitiveById(way1), "way1 should have been removed from the OSM layer");
     }
 
@@ -93,6 +89,7 @@ public class MapWithAIMoveActionTest {
 
     @Test
     public void testConflationDupeKeyRemoval() {
+        new MissingConnectionTagsMocker();
         mapWithAIData.unlock();
         way1.lastNode().put(DuplicateCommand.KEY, "n" + Long.toString(way2.lastNode().getUniqueId()));
         mapWithAIData.lock();
@@ -117,6 +114,7 @@ public class MapWithAIMoveActionTest {
 
     @Test
     public void testConflationConnKeyRemoval() {
+        new MissingConnectionTagsMocker();
         mapWithAIData.unlock();
         way1.lastNode().put(ConnectedCommand.KEY, "w" + Long.toString(way2.getUniqueId()) + ",n"
                 + Long.toString(way2.lastNode().getUniqueId()) + ",n" + Long.toString(way2.firstNode().getUniqueId()));
@@ -150,13 +148,7 @@ public class MapWithAIMoveActionTest {
     public void testMaxAddNotification() {
         TestUtils.assumeWorkingJMockit();
         new WindowMocker();
-        Map<String, Object> map = new HashMap<>();
+        new MissingConnectionTagsMocker();
-        for (String text : Arrays.asList(I18n.marktr("Sequence: Merge {0} nodes"), I18n.marktr("Delete {0} nodes"))) {
-            for (int i = 0; i < 10; i++) {
-                map.computeIfAbsent(I18n.tr(text, i), (t) -> JOptionPane.NO_OPTION);
-            }
-        }
-        new JOptionPaneSimpleMocker(map);
 
         NotificationMocker notification = new NotificationMocker();
         DataSet ds = MapWithAIDataUtils.getLayer(true).getDataSet();

test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/commands/conflation/ConnectedCommandTest.java

@@ -40,6 +40,11 @@ public class ConnectedCommandTest {
 
         assertThrows(NullPointerException.class, () -> command.getCommand(Collections.singletonList(toAdd)));
 
+        // SimplePrimitiveId doesn't like negative ids
+        way.setOsmId(1, 1);
+        way.firstNode().setOsmId(1, 1);
+        way.lastNode().setOsmId(2, 1);
+
         toAdd.put(command.getKey(),
                 "w" + way.getUniqueId() + ",n" + way.firstNode().getUniqueId() + ",n" + way.lastNode().getUniqueId());
 

test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/commands/conflation/DuplicateCommandTest.java

@@ -38,6 +38,8 @@ public class DuplicateCommandTest {
 
         assertNull(dupe.getCommand(Collections.singleton(dupe1)));
 
+        // SimplePrimitiveId doesn't understand negative ids
+        dupe2.setOsmId(2, 1);
         dupe1.put(dupe.getKey(), "n" + dupe2.getUniqueId());
 
         Command command = dupe.getCommand(Collections.singleton(dupe1));
@@ -50,7 +52,7 @@ public class DuplicateCommandTest {
         assertTrue(dupe1.hasKey(dupe.getKey()));
         assertFalse(dupe2.hasKey(dupe.getKey()));
 
-        Command deleteDupe2 = DeleteCommand.delete(Collections.singleton(dupe2));
+        Command deleteDupe2 = DeleteCommand.delete(Collections.singleton(dupe2), true, true);
         deleteDupe2.executeCommand();
         command = dupe.getCommand(Collections.singleton(dupe1));
         command.executeCommand();

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

@@ -65,6 +65,11 @@ public class CreateConnectionsCommandTest {
         createConnections.undoCommand();
         assertFalse(dataSet.isModified(), "DataSet shouldn't be modified yet");
 
+        // SimplePrimitiveId doesn't like negative ids
+        way.setOsmId(1, 1);
+        way.firstNode().setOsmId(1, 1);
+        way.lastNode().setOsmId(2, 1);
+
         node3.put(ConnectedCommand.KEY,
                 "w" + way.getUniqueId() + ",n" + node1.getUniqueId() + ",n" + node2.getUniqueId());
         createConnections = new CreateConnectionsCommand(dataSet, Collections.singleton(node3.save()));

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

@@ -16,8 +16,6 @@ import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.Future;
 
-import javax.swing.JOptionPane;
-
 import org.awaitility.Awaitility;
 import org.awaitility.Durations;
 import org.junit.Before;
@@ -40,15 +38,13 @@ import org.openstreetmap.josm.gui.layer.OsmDataLayer;
 import org.openstreetmap.josm.gui.progress.NullProgressMonitor;
 import org.openstreetmap.josm.plugins.mapwithai.backend.commands.conflation.ConnectedCommand;
 import org.openstreetmap.josm.plugins.mapwithai.testutils.MapWithAITestRules;
+import org.openstreetmap.josm.plugins.mapwithai.testutils.MissingConnectionTagsMocker;
 import org.openstreetmap.josm.plugins.mapwithai.testutils.PleaseWaitDialogMocker;
 import org.openstreetmap.josm.plugins.mapwithai.testutils.SwingUtilitiesMocker;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
-import org.openstreetmap.josm.testutils.mockers.JOptionPaneSimpleMocker;
 import org.openstreetmap.josm.testutils.mockers.WindowMocker;
 import org.openstreetmap.josm.tools.Logging;
 
-import com.google.common.collect.ImmutableMap;
-
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
 public class MapWithAIAddComandTest {
@@ -111,6 +107,11 @@ public class MapWithAIAddComandTest {
                 new Node(new LatLon(0, 0.15)));
         final Way way2 = TestUtils.newWay(HIGHWAY_RESIDENTIAL, new Node(new LatLon(0, 0.05)),
                 new Node(new LatLon(0.05, 0.2)));
+        // SimplePrimitiveId doesn't understand negative ids
+        way1.setOsmId(1, 1);
+        way1.firstNode().setOsmId(1, 1);
+        way1.lastNode().setOsmId(2, 1);
+
         way2.firstNode().put("conn",
                 "w".concat(Long.toString(way1.getUniqueId())).concat(",n")
                         .concat(Long.toString(way1.firstNode().getUniqueId())).concat(",n")
@@ -137,7 +138,7 @@ public class MapWithAIAddComandTest {
 
     @Test
     public void testCreateConnectionsUndo() {
-        new JOptionPaneSimpleMocker(ImmutableMap.of("Sequence: Merge 2 nodes", JOptionPane.NO_OPTION));
+        new MissingConnectionTagsMocker();
 
         final DataSet osmData = new DataSet();
         final DataSet mapWithAIData = new DataSet();
@@ -150,6 +151,7 @@ public class MapWithAIAddComandTest {
         osmData.addPrimitive(way2);
         mapWithAIData.addPrimitive(way1);
         mapWithAIData.setSelected(way1);
+        way2.lastNode().setOsmId(1, 1);
 
         MapWithAIAddCommand command = new MapWithAIAddCommand(mapWithAIData, osmData, mapWithAIData.getSelected());
         command.executeCommand();
@@ -270,6 +272,9 @@ public class MapWithAIAddComandTest {
                 new Node(new LatLon(3.4188681, 102.0562935)));
         Way original = TestUtils.newWay("highway=tertiary", new Node(new LatLon(3.4185368, 102.0560268)),
                 new Node(new LatLon(3.4187717, 102.0558451)));
+        original.setOsmId(1, 1);
+        original.firstNode().setOsmId(1, 1);
+        original.lastNode().setOsmId(2, 1);
         String connectedValue = "w" + Long.toString(original.getUniqueId()) + ",n"
                 + Long.toString(original.firstNode().getUniqueId()) + ",n"
                 + Long.toString(original.lastNode().getUniqueId());

test/unit/org/openstreetmap/josm/plugins/mapwithai/commands/conflation/cleanup/MissingConnectionTagsTest.java

@@ -2,11 +2,13 @@
 package org.openstreetmap.josm.plugins.mapwithai.commands.conflation.cleanup;
 
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertSame;
+import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotSame;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
 
 import javax.swing.JOptionPane;
 
@@ -19,16 +21,15 @@ import org.openstreetmap.josm.command.Command;
 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.SimplePrimitiveId;
 import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.gui.layer.OsmDataLayer;
 import org.openstreetmap.josm.plugins.mapwithai.backend.commands.conflation.cleanup.MissingConnectionTags;
+import org.openstreetmap.josm.plugins.mapwithai.testutils.MissingConnectionTagsMocker;
 import org.openstreetmap.josm.testutils.JOSMTestRules;
-import org.openstreetmap.josm.testutils.mockers.JOptionPaneSimpleMocker;
 import org.openstreetmap.josm.testutils.mockers.WindowMocker;
 
-import com.google.common.collect.ImmutableMap;
-
 import mockit.integration.TestRunnerDecorator;
 
 public class MissingConnectionTagsTest {
@@ -36,7 +37,6 @@ public class MissingConnectionTagsTest {
     public JOSMTestRules josmTestRules = new JOSMTestRules().projection().main();
     private DataSet ds;
     private MissingConnectionTags missing;
-    private JOptionPaneSimpleMocker joptionMocker;
 
     @Before
     public void setUp() {
@@ -66,7 +66,9 @@ public class MissingConnectionTagsTest {
     @Test
     public void testDupeNode() {
         new WindowMocker();
-        joptionMocker = new JOptionPaneSimpleMocker(ImmutableMap.of("Sequence: Merge 2 nodes", JOptionPane.OK_OPTION));
+        Map<String, Object> actions = new HashMap<>();
+        actions.put("Set dupe=node 1 for node 'node'", JOptionPane.YES_OPTION);
+        new MissingConnectionTagsMocker(actions);
         Node node11 = new Node(LatLon.ZERO);
         Node node21 = new Node(LatLon.ZERO);
         Node node12 = new Node(LatLon.NORTH_POLE);
@@ -77,14 +79,22 @@ public class MissingConnectionTagsTest {
             way.getNodes().forEach(ds::addPrimitive);
             ds.addPrimitive(way);
         }
-        assertNotSame(way1.firstNode(), way2.firstNode());
+        way1.firstNode().setOsmId(1, 1);
+        assertFalse(way2.firstNode().hasKey("dupe"));
         Command command = missing.getCommand(Collections.singleton(way2));
+        // The dupe key has to appear after making the command, since the command
+        // was immediately run.
+        assertTrue(way2.firstNode().hasKey("dupe"));
+        // The change command will always replace its "undo" after every execution
+        command.undoCommand();
+        assertFalse(way2.firstNode().hasKey("dupe"));
         for (int i = 0; i < 10; i++) {
-            assertNotSame(way1.firstNode(), way2.firstNode());
             command.executeCommand();
-            assertSame(way1.firstNode(), way2.firstNode());
+            assertTrue(way2.firstNode().hasKey("dupe"));
+            assertEquals(way1.firstNode().getOsmPrimitiveId(),
+                    SimplePrimitiveId.fromString(way2.firstNode().get("dupe")));
             command.undoCommand();
-            assertNotSame(way1.firstNode(), way2.firstNode());
+            assertFalse(way2.firstNode().hasKey("dupe"));
         }
     }
 

test/unit/org/openstreetmap/josm/plugins/mapwithai/testutils/MissingConnectionTagsMocker.java

@@ -0,0 +1,74 @@
+// License: GPL. For details, see LICENSE file.
+package org.openstreetmap.josm.plugins.mapwithai.testutils;
+
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+
+import javax.swing.JOptionPane;
+
+import org.openstreetmap.josm.command.Command;
+import org.openstreetmap.josm.data.validation.TestError;
+import org.openstreetmap.josm.plugins.mapwithai.backend.commands.conflation.cleanup.MissingConnectionTags;
+import org.openstreetmap.josm.testutils.mockers.JOptionPaneSimpleMocker;
+
+import mockit.Invocation;
+import mockit.Mock;
+import mockit.MockUp;
+
+/**
+ * This mocks the fixErrors to avoid creating a ConditionalOptionPane dialog The
+ * default is {@link JOptionPane#NO_OPTION}
+ *
+ * @author Taylor Smock
+ *
+ */
+public class MissingConnectionTagsMocker extends MockUp<MissingConnectionTags> {
+    private final JOptionPaneSimpleMocker joptionPaneMocker;
+    private int defaultOption = JOptionPane.NO_OPTION;
+    private final Map<String, Object> map;
+
+    /**
+     * Only use the default option for joptionpanes
+     */
+    public MissingConnectionTagsMocker() {
+        this(new HashMap<>());
+    }
+
+    /**
+     * Initialize with a map of results
+     *
+     * @param map See {@link JOptionPaneSimpleMocker#JOptionPaneSimpleMocker(Map)}
+     */
+    public MissingConnectionTagsMocker(Map<String, Object> map) {
+        joptionPaneMocker = new JOptionPaneSimpleMocker(map);
+        this.map = map;
+    }
+
+    @Mock
+    protected void fixErrors(Invocation inv, String prefKey, Collection<Command> commands,
+            Collection<TestError> issues) {
+        issues.stream().filter(TestError::isFixable).map(t -> t.getFix().getDescriptionText())
+                .forEach(m -> map.putIfAbsent(m, defaultOption));
+        inv.proceed(prefKey, commands, issues);
+    }
+
+    /**
+     * Set the default option
+     *
+     * @param jOptionPaneOption Use one of the {@link JOptionPane#getOptions()}
+     *                          integers
+     */
+    public void setDefaultOption(int jOptionPaneOption) {
+        defaultOption = jOptionPaneOption;
+    }
+
+    /**
+     * Get the mocker being used for the option pane
+     *
+     * @return The JOptionPaneSimpleMocker
+     */
+    public JOptionPaneSimpleMocker getJOptionPaneSimpleMocker() {
+        return joptionPaneMocker;
+    }
+}