Fix an issue where a deleted node was used in a subsequent command

Signed-off-by: Taylor Smock <taylor.smock@kaart.com>
pull/1/head
Taylor Smock 2020-04-09 13:36:52 -06:00
rodzic cc3d89442d
commit 3cf03a2366
Nie znaleziono w bazie danych klucza dla tego podpisu
ID klucza GPG: 625F6A74A3E4311A
5 zmienionych plików z 35 dodań i 9 usunięć

Wyświetl plik

@ -5,6 +5,7 @@ import static org.openstreetmap.josm.tools.I18n.tr;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Map.Entry; 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 * @return {@code true} if the key should not exist in OpenStreetMap
*/ */
public abstract boolean keyShouldNotExistInOSM(); public abstract boolean keyShouldNotExistInOSM();
/**
* @return Conflation commands that conflict with this conflation command
*/
public Collection<Class<? extends AbstractConflationCommand>> conflictedCommands() {
return Collections.emptyList();
}
} }

Wyświetl plik

@ -6,6 +6,7 @@ import static org.openstreetmap.josm.tools.I18n.tr;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collection; import java.util.Collection;
import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.stream.Collectors; import java.util.stream.Collectors;
@ -102,4 +103,9 @@ public class MergeAddressBuildings extends AbstractConflationCommand {
public boolean keyShouldNotExistInOSM() { public boolean keyShouldNotExistInOSM() {
return false; return false;
} }
@Override
public Collection<Class<? extends AbstractConflationCommand>> conflictedCommands() {
return Collections.singleton(MergeBuildingAddress.class);
}
} }

Wyświetl plik

@ -129,4 +129,8 @@ public class MergeBuildingAddress extends AbstractConflationCommand {
return false; return false;
} }
@Override
public Collection<Class<? extends AbstractConflationCommand>> conflictedCommands() {
return Collections.singleton(MergeAddressBuildings.class);
}
} }

Wyświetl plik

@ -59,8 +59,7 @@ public class CreateConnectionsCommand extends Command {
command.executeCommand(); command.executeCommand();
} }
if (undoCommands != null) { if (undoCommands != null) {
GuiHelper.runInEDTAndWait(() -> undoCommands.executeCommand()); GuiHelper.runInEDTAndWait(() -> UndoRedoHandler.getInstance().add(undoCommands));
GuiHelper.runInEDTAndWait(() -> UndoRedoHandler.getInstance().add(undoCommands, false));
} }
return true; return true;
} }
@ -88,6 +87,7 @@ public class CreateConnectionsCommand extends Command {
final List<Command> undoable = new ArrayList<>(); final List<Command> undoable = new ArrayList<>();
final Collection<OsmPrimitive> realPrimitives = collection.stream().map(dataSet::getPrimitiveById) final Collection<OsmPrimitive> realPrimitives = collection.stream().map(dataSet::getPrimitiveById)
.filter(Objects::nonNull).collect(Collectors.toList()); .filter(Objects::nonNull).collect(Collectors.toList());
List<Class<? extends AbstractConflationCommand>> runCommands = new ArrayList<>();
for (final Class<? extends AbstractConflationCommand> abstractCommandClass : getConflationCommands()) { for (final Class<? extends AbstractConflationCommand> abstractCommandClass : getConflationCommands()) {
final AbstractConflationCommand abstractCommand; final AbstractConflationCommand abstractCommand;
try { try {
@ -97,18 +97,24 @@ public class CreateConnectionsCommand extends Command {
Logging.debug(e); Logging.debug(e);
continue; continue;
} }
// If there are conflicting commands, don't add it.
if (runCommands.parallelStream().anyMatch(c -> abstractCommand.conflictedCommands().contains(c))) {
continue;
}
final Collection<OsmPrimitive> tPrimitives = new TreeSet<>(); final Collection<OsmPrimitive> tPrimitives = new TreeSet<>();
abstractCommand.getInterestedTypes() abstractCommand.getInterestedTypes()
.forEach(clazz -> tPrimitives.addAll(Utils.filteredCollection(realPrimitives, clazz))); .forEach(clazz -> tPrimitives.addAll(Utils.filteredCollection(realPrimitives, clazz)));
final Command actualCommand = abstractCommand.getCommand(tPrimitives.stream() final Command actualCommand = abstractCommand.getCommand(
.filter(prim -> prim.hasKey(abstractCommand.getKey())).collect(Collectors.toList())); tPrimitives.stream().filter(prim -> prim.hasKey(abstractCommand.getKey()) && !prim.isDeleted())
.collect(Collectors.toList()));
if (Objects.nonNull(actualCommand)) { if (Objects.nonNull(actualCommand)) {
if (abstractCommand.allowUndo()) { if (abstractCommand.allowUndo()) {
undoable.add(actualCommand); undoable.add(actualCommand);
} else { } else {
permanent.add(actualCommand); permanent.add(actualCommand);
} }
runCommands.add(abstractCommand.getClass());
} }
} }

Wyświetl plik

@ -33,7 +33,7 @@ import org.openstreetmap.josm.tools.Logging;
* @author Taylor Smock * @author Taylor Smock
*/ */
public class MovePrimitiveDataSetCommand extends Command { public class MovePrimitiveDataSetCommand extends Command {
private SequenceCommand command; private Command command;
public MovePrimitiveDataSetCommand(DataSet to, DataSet from, Collection<OsmPrimitive> primitives) { public MovePrimitiveDataSetCommand(DataSet to, DataSet from, Collection<OsmPrimitive> primitives) {
super(to); super(to);
@ -70,7 +70,7 @@ public class MovePrimitiveDataSetCommand extends Command {
* @param selection The primitives to move * @param selection The primitives to move
* @return The command that does the actual move * @return The command that does the actual move
*/ */
public static SequenceCommand moveCollection(DataSet from, DataSet to, Collection<OsmPrimitive> selection) { public static Command moveCollection(DataSet from, DataSet to, Collection<OsmPrimitive> selection) {
return moveCollection(from, to, selection, new HashSet<>()); return moveCollection(from, to, selection, new HashSet<>());
} }
@ -84,7 +84,7 @@ public class MovePrimitiveDataSetCommand extends Command {
* if any positive ids are available) * if any positive ids are available)
* @return The command that does the actual move * @return The command that does the actual move
*/ */
public static SequenceCommand moveCollection(DataSet from, DataSet to, Collection<OsmPrimitive> selection, public static Command moveCollection(DataSet from, DataSet to, Collection<OsmPrimitive> selection,
Collection<PrimitiveData> primitiveData) { Collection<PrimitiveData> primitiveData) {
final List<Command> commands = new ArrayList<>(); final List<Command> commands = new ArrayList<>();
@ -121,14 +121,16 @@ public class MovePrimitiveDataSetCommand extends Command {
} }
}); });
} }
Command delete;
if (!removeKeyCommand.isEmpty()) { if (!removeKeyCommand.isEmpty()) {
SequenceCommand sequence = new SequenceCommand("Temporary Command", removeKeyCommand); SequenceCommand sequence = new SequenceCommand("Temporary Command", removeKeyCommand);
sequence.executeCommand(); // This *must* be executed for the delete command to get everything. 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(); sequence.undoCommand();
} else { } 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", return new SequenceCommand(trn("Move {0} OSM Primitive between data sets",
"Move {0} OSM Primitives between data sets", selection.size(), selection.size()), commands); "Move {0} OSM Primitives between data sets", selection.size(), selection.size()), commands);