From 3cf03a23662e4e23d894bececa617bba974703de Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Thu, 9 Apr 2020 13:36:52 -0600 Subject: [PATCH] Fix an issue where a deleted node was used in a subsequent command Signed-off-by: Taylor Smock --- .../conflation/AbstractConflationCommand.java | 8 ++++++++ .../commands/conflation/MergeAddressBuildings.java | 6 ++++++ .../commands/conflation/MergeBuildingAddress.java | 4 ++++ .../commands/CreateConnectionsCommand.java | 14 ++++++++++---- .../commands/MovePrimitiveDataSetCommand.java | 12 +++++++----- 5 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/commands/conflation/AbstractConflationCommand.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/commands/conflation/AbstractConflationCommand.java index 8a4a64f..88c4dc9 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/commands/conflation/AbstractConflationCommand.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/commands/conflation/AbstractConflationCommand.java @@ -5,6 +5,7 @@ import static org.openstreetmap.josm.tools.I18n.tr; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -146,4 +147,11 @@ public abstract class AbstractConflationCommand extends Command { * @return {@code true} if the key should not exist in OpenStreetMap */ public abstract boolean keyShouldNotExistInOSM(); + + /** + * @return Conflation commands that conflict with this conflation command + */ + public Collection> conflictedCommands() { + return Collections.emptyList(); + } } diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/commands/conflation/MergeAddressBuildings.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/commands/conflation/MergeAddressBuildings.java index a6434ed..aab1df8 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/commands/conflation/MergeAddressBuildings.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/commands/conflation/MergeAddressBuildings.java @@ -6,6 +6,7 @@ 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; @@ -102,4 +103,9 @@ public class MergeAddressBuildings extends AbstractConflationCommand { public boolean keyShouldNotExistInOSM() { return false; } + + @Override + public Collection> conflictedCommands() { + return Collections.singleton(MergeBuildingAddress.class); + } } diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/commands/conflation/MergeBuildingAddress.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/commands/conflation/MergeBuildingAddress.java index 5acd2b9..426481b 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/commands/conflation/MergeBuildingAddress.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/commands/conflation/MergeBuildingAddress.java @@ -129,4 +129,8 @@ public class MergeBuildingAddress extends AbstractConflationCommand { return false; } + @Override + public Collection> conflictedCommands() { + return Collections.singleton(MergeAddressBuildings.class); + } } diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/CreateConnectionsCommand.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/CreateConnectionsCommand.java index a56ce87..221aa34 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/CreateConnectionsCommand.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/CreateConnectionsCommand.java @@ -59,8 +59,7 @@ public class CreateConnectionsCommand extends Command { command.executeCommand(); } if (undoCommands != null) { - GuiHelper.runInEDTAndWait(() -> undoCommands.executeCommand()); - GuiHelper.runInEDTAndWait(() -> UndoRedoHandler.getInstance().add(undoCommands, false)); + GuiHelper.runInEDTAndWait(() -> UndoRedoHandler.getInstance().add(undoCommands)); } return true; } @@ -88,6 +87,7 @@ public class CreateConnectionsCommand extends Command { final List undoable = new ArrayList<>(); final Collection realPrimitives = collection.stream().map(dataSet::getPrimitiveById) .filter(Objects::nonNull).collect(Collectors.toList()); + List> runCommands = new ArrayList<>(); for (final Class abstractCommandClass : getConflationCommands()) { final AbstractConflationCommand abstractCommand; try { @@ -97,18 +97,24 @@ public class CreateConnectionsCommand extends Command { Logging.debug(e); continue; } + // If there are conflicting commands, don't add it. + if (runCommands.parallelStream().anyMatch(c -> abstractCommand.conflictedCommands().contains(c))) { + continue; + } final Collection tPrimitives = new TreeSet<>(); abstractCommand.getInterestedTypes() .forEach(clazz -> tPrimitives.addAll(Utils.filteredCollection(realPrimitives, clazz))); - final Command actualCommand = abstractCommand.getCommand(tPrimitives.stream() - .filter(prim -> prim.hasKey(abstractCommand.getKey())).collect(Collectors.toList())); + final Command actualCommand = abstractCommand.getCommand( + tPrimitives.stream().filter(prim -> prim.hasKey(abstractCommand.getKey()) && !prim.isDeleted()) + .collect(Collectors.toList())); if (Objects.nonNull(actualCommand)) { if (abstractCommand.allowUndo()) { undoable.add(actualCommand); } else { permanent.add(actualCommand); } + runCommands.add(abstractCommand.getClass()); } } diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/MovePrimitiveDataSetCommand.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/MovePrimitiveDataSetCommand.java index a1811fd..270f76b 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/MovePrimitiveDataSetCommand.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/MovePrimitiveDataSetCommand.java @@ -33,7 +33,7 @@ import org.openstreetmap.josm.tools.Logging; * @author Taylor Smock */ public class MovePrimitiveDataSetCommand extends Command { - private SequenceCommand command; + private Command command; public MovePrimitiveDataSetCommand(DataSet to, DataSet from, Collection primitives) { super(to); @@ -70,7 +70,7 @@ public class MovePrimitiveDataSetCommand extends Command { * @param selection The primitives to move * @return The command that does the actual move */ - public static SequenceCommand moveCollection(DataSet from, DataSet to, Collection selection) { + public static Command moveCollection(DataSet from, DataSet to, Collection selection) { return moveCollection(from, to, selection, new HashSet<>()); } @@ -84,7 +84,7 @@ public class MovePrimitiveDataSetCommand extends Command { * if any positive ids are available) * @return The command that does the actual move */ - public static SequenceCommand moveCollection(DataSet from, DataSet to, Collection selection, + public static Command moveCollection(DataSet from, DataSet to, Collection selection, Collection primitiveData) { final List commands = new ArrayList<>(); @@ -121,14 +121,16 @@ public class MovePrimitiveDataSetCommand extends Command { } }); } + Command delete; if (!removeKeyCommand.isEmpty()) { SequenceCommand sequence = new SequenceCommand("Temporary Command", removeKeyCommand); sequence.executeCommand(); // This *must* be executed for the delete command to get everything. - commands.add(DeleteCommand.delete(selection, true, true)); + delete = DeleteCommand.delete(selection, true, true); sequence.undoCommand(); } else { - commands.add(DeleteCommand.delete(selection, true, true)); + delete = DeleteCommand.delete(selection, true, true); } + commands.add(delete); return new SequenceCommand(trn("Move {0} OSM Primitive between data sets", "Move {0} OSM Primitives between data sets", selection.size(), selection.size()), commands);