From b87214b7ef8294c64261a8a1d66669c5e71bd231 Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Wed, 2 Oct 2019 11:39:11 -0600 Subject: [PATCH] Check for random cases to ensure that they don't break Signed-off-by: Taylor Smock --- .../commands/DeletePrimitivesCommand.java | 3 ++ .../commands/MovePrimitiveDataSetCommand.java | 18 ++++--- .../MovePrimitiveDataSetCommandTest.java | 49 ++++++++++++++++++- 3 files changed, 61 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/openstreetmap/josm/plugins/rapid/commands/DeletePrimitivesCommand.java b/src/main/java/org/openstreetmap/josm/plugins/rapid/commands/DeletePrimitivesCommand.java index 9b54067..2447cec 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/rapid/commands/DeletePrimitivesCommand.java +++ b/src/main/java/org/openstreetmap/josm/plugins/rapid/commands/DeletePrimitivesCommand.java @@ -96,6 +96,9 @@ public class DeletePrimitivesCommand extends Command { @Override public void fillModifiedData(Collection modified, Collection deleted, Collection added) { + for (Command command : commands) { + command.fillModifiedData(modified, deleted, added); + } deleted.addAll(removed); } diff --git a/src/main/java/org/openstreetmap/josm/plugins/rapid/commands/MovePrimitiveDataSetCommand.java b/src/main/java/org/openstreetmap/josm/plugins/rapid/commands/MovePrimitiveDataSetCommand.java index 320577d..b58fff8 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/rapid/commands/MovePrimitiveDataSetCommand.java +++ b/src/main/java/org/openstreetmap/josm/plugins/rapid/commands/MovePrimitiveDataSetCommand.java @@ -26,7 +26,6 @@ public class MovePrimitiveDataSetCommand extends Command { private final DataSet from; private final Collection primitives; private SequenceCommand command; - List commands; public MovePrimitiveDataSetCommand(DataSet to, DataSet from, Collection primitives) { super(to); @@ -34,15 +33,14 @@ public class MovePrimitiveDataSetCommand extends Command { this.from = from; this.primitives = primitives; command = null; - commands = new ArrayList<>(); } @Override public boolean executeCommand() { - if (to.equals(from)) - return false; command = moveCollection(from, to, primitives); - command.executeCommand(); + if (command != null) { + command.executeCommand(); + } return true; } /** @@ -53,11 +51,13 @@ public class MovePrimitiveDataSetCommand extends Command { * @param selection The primitives to move */ public SequenceCommand moveCollection(DataSet from, DataSet to, Collection selection) { - if (from == null || to.isLocked() || from.isLocked()) { + if (from == null || to.isLocked() || from.isLocked() || to.equals(from)) { Logging.error("{0}: Cannot move primitives from {1} to {2}", RapiDPlugin.NAME, from, to); return null; } + List commands = new ArrayList<>(); + Collection allNeededPrimitives = new ArrayList<>(); RapiDDataUtils.addPrimitivesToCollection(allNeededPrimitives, selection); @@ -71,7 +71,9 @@ public class MovePrimitiveDataSetCommand extends Command { @Override public void undoCommand() { - command.undoCommand(); + if (command != null) { + command.undoCommand(); + } } @Override @@ -82,6 +84,6 @@ public class MovePrimitiveDataSetCommand extends Command { @Override public void fillModifiedData(Collection modified, Collection deleted, Collection added) { - modified.addAll(primitives); + command.fillModifiedData(modified, deleted, added); } } diff --git a/test/unit/org/openstreetmap/josm/plugins/rapid/commands/MovePrimitiveDataSetCommandTest.java b/test/unit/org/openstreetmap/josm/plugins/rapid/commands/MovePrimitiveDataSetCommandTest.java index 50234c5..7984f40 100644 --- a/test/unit/org/openstreetmap/josm/plugins/rapid/commands/MovePrimitiveDataSetCommandTest.java +++ b/test/unit/org/openstreetmap/josm/plugins/rapid/commands/MovePrimitiveDataSetCommandTest.java @@ -1,6 +1,9 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.plugins.rapid.commands; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import org.junit.Assert; @@ -10,8 +13,8 @@ import org.openstreetmap.josm.TestUtils; 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.OsmPrimitive; import org.openstreetmap.josm.data.osm.Way; -import org.openstreetmap.josm.plugins.rapid.commands.MovePrimitiveDataSetCommand; import org.openstreetmap.josm.testutils.JOSMTestRules; public class MovePrimitiveDataSetCommandTest { @@ -20,6 +23,9 @@ public class MovePrimitiveDataSetCommandTest { @Test public void testMovePrimitives() { + Collection added = new ArrayList<>(); + Collection modified = new ArrayList<>(); + Collection deleted = new ArrayList<>(); DataSet to = new DataSet(); DataSet from = new DataSet(); Way way1 = TestUtils.newWay("highway=tertiary", new Node(new LatLon(0, 0)), new Node(new LatLon(0.1, 0.1))); @@ -32,6 +38,10 @@ public class MovePrimitiveDataSetCommandTest { Assert.assertEquals(4, from.allPrimitives().size()); move.executeCommand(); + move.fillModifiedData(modified, deleted, added); + Assert.assertEquals(3, deleted.size()); + Assert.assertEquals(3, added.size()); + Assert.assertEquals(0, modified.size()); Assert.assertEquals(1, from.allPrimitives().size()); Assert.assertEquals(3, to.allPrimitives().size()); Assert.assertEquals(to, way1.getDataSet()); @@ -43,7 +53,14 @@ public class MovePrimitiveDataSetCommandTest { way1.firstNode().put("highway", "stop"); + modified.clear(); + added.clear(); + deleted.clear(); move.executeCommand(); + move.fillModifiedData(modified, deleted, added); + Assert.assertEquals(3, deleted.size()); + Assert.assertEquals(3, added.size()); + Assert.assertEquals(0, modified.size()); Assert.assertEquals(1, from.allPrimitives().size()); Assert.assertEquals(3, to.allPrimitives().size()); Assert.assertEquals(to, way1.getDataSet()); @@ -52,5 +69,35 @@ public class MovePrimitiveDataSetCommandTest { Assert.assertEquals(0, to.allPrimitives().size()); Assert.assertEquals(4, from.allPrimitives().size()); Assert.assertEquals(from, way1.getDataSet()); + + for (DataSet ds : Arrays.asList(from, to)) { + ds.lock(); + move.executeCommand(); + Assert.assertEquals(0, to.allPrimitives().size()); + Assert.assertEquals(4, from.allPrimitives().size()); + Assert.assertEquals(from, way1.getDataSet()); + move.undoCommand(); + ds.unlock(); + } + + move = new MovePrimitiveDataSetCommand(to, null, Collections.singleton(way1)); + move.executeCommand(); + Assert.assertEquals(0, to.allPrimitives().size()); + Assert.assertEquals(4, from.allPrimitives().size()); + Assert.assertEquals(from, way1.getDataSet()); + move.undoCommand(); + + move = new MovePrimitiveDataSetCommand(to, to, Collections.singleton(way1)); + move.executeCommand(); + Assert.assertEquals(0, to.allPrimitives().size()); + Assert.assertEquals(4, from.allPrimitives().size()); + Assert.assertEquals(from, way1.getDataSet()); + move.undoCommand(); + } + + @Test + public void testDescription() { + Assert.assertNotNull(new MovePrimitiveDataSetCommand(new DataSet(), new DataSet(), Collections.emptyList()) + .getDescriptionText()); } }