Fix issue where dupe node information wasn't deleted

Signed-off-by: Taylor Smock <taylor.smock@kaart.com>
pull/1/head
Taylor Smock 2019-10-07 15:53:35 -06:00
rodzic 89cc8fe53c
commit cec488fb64
Nie znaleziono w bazie danych klucza dla tego podpisu
ID klucza GPG: 625F6A74A3E4311A
5 zmienionych plików z 85 dodań i 24 usunięć

Wyświetl plik

@ -83,8 +83,6 @@ public class RapiDAction extends JosmAction {
// TODO remove bounds that are already downloaded
if (rapidBounds.parallelStream().filter(bound::equals).count() == 0) {
final DataSet newData = RapiDDataUtils.getData(bound.toBBox());
/* Microsoft buildings don't have a source, so we add one */
RapiDDataUtils.addSourceTags(newData, "building", "Microsoft");
synchronized (LAYER_LOCK) {
layer.unlock();
layer.mergeFrom(newData);

Wyświetl plik

@ -78,10 +78,7 @@ public class CreateConnectionsCommand extends Command {
changedKeyList.addAll(connectedCommand(dataSet, node));
}
if (node.hasKey(DUPE_KEY)) {
final Command replaceCommand = duplicateNode(dataSet, node);
if (replaceCommand != null) {
changedKeyList.add(replaceCommand);
}
changedKeyList.addAll(duplicateNode(dataSet, node));
}
}
if (!changedKeyList.isEmpty()) {
@ -106,17 +103,34 @@ public class CreateConnectionsCommand extends Command {
primitiveConnections[i + 1].getClass(), i + 2, primitiveConnections[i + 2].getClass());
}
}
Logging.debug("RapiD: Removing conn from {0} in {1}", node, dataSet.getName());
commands.add(new ChangePropertyCommand(node, CONN_KEY, null));
return commands;
}
private static Command duplicateNode(DataSet dataSet, Node node) {
private static List<Command> duplicateNode(DataSet dataSet, Node node) {
final OsmPrimitive[] primitiveConnections = getPrimitives(dataSet, node.get(DUPE_KEY));
if (primitiveConnections.length != 1) {
Logging.error("RapiD: dupe connection connected to more than one node? (dupe={0})", node.get(DUPE_KEY));
Logging.error("{0}: {3} connection connected to more than one node? ({3}={1})", RapiDPlugin.NAME,
node.get(DUPE_KEY), DUPE_KEY);
}
return replaceNode(node, (Node) primitiveConnections[0]);
List<Command> commands = new ArrayList<>();
if (primitiveConnections[0] instanceof Node) {
Node replaceNode = (Node) primitiveConnections[0];
Command tCommand = replaceNode(node, replaceNode);
if (tCommand != null) {
commands.add(tCommand);
if (replaceNode.hasKey(DUPE_KEY)) {
String key = replaceNode.get(DUPE_KEY);
commands.add(new ChangePropertyCommand(replaceNode, DUPE_KEY, key));
} else {
replaceNode.put(DUPE_KEY, "empty_value"); // This is needed to actually have a command.
commands.add(new ChangePropertyCommand(replaceNode, DUPE_KEY, null));
replaceNode.remove(DUPE_KEY);
}
}
}
return commands;
}
/**

Wyświetl plik

@ -4,7 +4,9 @@ package org.openstreetmap.josm.plugins.rapid.commands;
import static org.openstreetmap.josm.tools.I18n.tr;
import java.awt.EventQueue;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import org.openstreetmap.josm.command.Command;
import org.openstreetmap.josm.command.SequenceCommand;
@ -12,6 +14,7 @@ import org.openstreetmap.josm.data.osm.DataSet;
import org.openstreetmap.josm.data.osm.OsmPrimitive;
import org.openstreetmap.josm.gui.layer.OsmDataLayer;
import org.openstreetmap.josm.plugins.rapid.RapiDPlugin;
import org.openstreetmap.josm.plugins.rapid.backend.RapiDDataUtils;
import org.openstreetmap.josm.plugins.rapid.backend.RapiDLayer;
import org.openstreetmap.josm.tools.Logging;
@ -70,7 +73,9 @@ public class RapiDAddCommand extends Command implements Runnable {
rapid.unlock();
}
final Command tCommand = new MovePrimitiveDataSetCommand(editable, rapid, primitives);
final Command createConnectionsCommand = createConnections(editable, primitives);
List<OsmPrimitive> allPrimitives = new ArrayList<>();
RapiDDataUtils.addPrimitivesToCollection(allPrimitives, primitives);
final Command createConnectionsCommand = createConnections(editable, allPrimitives);
command = new SequenceCommand(getDescriptionText(), tCommand, createConnectionsCommand);
command.executeCommand();

Wyświetl plik

@ -13,10 +13,15 @@ import org.openstreetmap.josm.data.osm.Node;
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.rapid.commands.CreateConnectionsCommand;
import org.openstreetmap.josm.testutils.JOSMTestRules;
public class RapiDMoveActionTest {
RapiDMoveAction moveAction;
DataSet rapidData;
OsmDataLayer osmLayer;
Way way1;
Way way2;
@Rule
public JOSMTestRules test = new JOSMTestRules().preferences().main().projection();
@ -24,34 +29,67 @@ public class RapiDMoveActionTest {
@Before
public void setUp() {
moveAction = new RapiDMoveAction();
}
@Test
public void testMoveAction() {
final DataSet osmData = new DataSet();
final DataSet rapidData = new DataSet();
final Way way1 = TestUtils.newWay("highway=residential", new Node(new LatLon(0, 0)),
rapidData = new DataSet();
way1 = TestUtils.newWay("highway=residential", new Node(new LatLon(0, 0)),
new Node(new LatLon(0.1, 0.1)));
final Way way2 = TestUtils.newWay("highway=residential", new Node(new LatLon(-0.1, -0.1)),
way2 = TestUtils.newWay("highway=residential", new Node(new LatLon(-0.1, -0.1)),
new Node(new LatLon(0.1, 0.1)));
way1.getNodes().forEach(node -> rapidData.addPrimitive(node));
way2.getNodes().forEach(node -> osmData.addPrimitive(node));
osmData.addPrimitive(way2);
rapidData.addPrimitive(way1);
final OsmDataLayer osmLayer = new OsmDataLayer(osmData, "osm", null);
final RapiDLayer rapidLayer = new RapiDLayer(RapiDDataUtils.getData(RapiDDataUtilsTest.getTestBBox()), "rapid",
osmLayer = new OsmDataLayer(osmData, "osm", null);
final RapiDLayer rapidLayer = new RapiDLayer(rapidData, "rapid",
null);
MainApplication.getLayerManager().addLayer(osmLayer);
MainApplication.getLayerManager().addLayer(rapidLayer);
MainApplication.getLayerManager().setActiveLayer(rapidLayer);
}
@Test
public void testMoveAction() {
rapidData.addSelected(way1);
moveAction.actionPerformed(null);
Assert.assertEquals(osmLayer, MainApplication.getLayerManager().getActiveLayer());
Assert.assertSame(osmLayer.getDataSet(), way1.getDataSet());
UndoRedoHandler.getInstance().undo();
Assert.assertNotSame(osmLayer.getDataSet(), way1.getDataSet());
}
@Test
public void testConflationDupeKeyRemoval() {
rapidData.unlock();
way1.lastNode().put(CreateConnectionsCommand.DUPE_KEY, "n" + Long.toString(way2.lastNode().getUniqueId()));
rapidData.lock();
rapidData.addSelected(way1);
moveAction.actionPerformed(null);
Assert.assertEquals(osmLayer, MainApplication.getLayerManager().getActiveLayer());
Assert.assertFalse(way2.lastNode().hasKey(CreateConnectionsCommand.DUPE_KEY));
Assert.assertFalse(way1.lastNode().hasKey(CreateConnectionsCommand.DUPE_KEY));
UndoRedoHandler.getInstance().undo();
Assert.assertFalse(way2.lastNode().hasKey(CreateConnectionsCommand.DUPE_KEY));
Assert.assertTrue(way1.lastNode().hasKey(CreateConnectionsCommand.DUPE_KEY));
}
@Test
public void testConflationConnKeyRemoval() {
rapidData.unlock();
way1.lastNode().put(CreateConnectionsCommand.CONN_KEY, "w" + Long.toString(way2.getUniqueId()) + ",n"
+ Long.toString(way2.lastNode().getUniqueId()) + ",n" + Long.toString(way2.firstNode().getUniqueId()));
rapidData.lock();
rapidData.addSelected(way1);
moveAction.actionPerformed(null);
Assert.assertFalse(way2.lastNode().hasKey(CreateConnectionsCommand.CONN_KEY));
Assert.assertFalse(way2.firstNode().hasKey(CreateConnectionsCommand.CONN_KEY));
Assert.assertFalse(way2.getNode(1).hasKey(CreateConnectionsCommand.CONN_KEY));
Assert.assertFalse(way1.lastNode().hasKey(CreateConnectionsCommand.CONN_KEY));
UndoRedoHandler.getInstance().undo();
Assert.assertFalse(way2.lastNode().hasKey(CreateConnectionsCommand.CONN_KEY));
Assert.assertTrue(way1.lastNode().hasKey(CreateConnectionsCommand.CONN_KEY));
}
}

Wyświetl plik

@ -45,7 +45,8 @@ public class CreateConnectionsCommandTest {
final Collection<OsmPrimitive> deleted = new ArrayList<>();
final DataSet dataSet = new DataSet(node1, node2, node3, dupe, way);
Command createConnections = new CreateConnectionsCommand(dataSet, Collections.singleton(node3));
CreateConnectionsCommand createConnections = new CreateConnectionsCommand(dataSet,
Collections.singleton(node3));
createConnections.executeCommand();
Assert.assertFalse(dataSet.isModified());
@ -58,6 +59,7 @@ public class CreateConnectionsCommandTest {
createConnections.executeCommand();
Assert.assertTrue(dataSet.isModified());
Assert.assertEquals(3, way.getNodesCount());
Assert.assertFalse(node3.hasKey(CreateConnectionsCommand.CONN_KEY));
createConnections.fillModifiedData(modified, deleted, added);
Assert.assertEquals(3, modified.size()); // 3 since we remove the key from the node
Assert.assertTrue(deleted.isEmpty());
@ -65,18 +67,22 @@ public class CreateConnectionsCommandTest {
createConnections.undoCommand();
Assert.assertFalse(dataSet.isModified());
Assert.assertEquals(2, way.getNodesCount());
Assert.assertTrue(node3.hasKey(CreateConnectionsCommand.CONN_KEY));
dupe.put(CreateConnectionsCommand.DUPE_KEY, "n" + node1.getUniqueId());
createConnections = new CreateConnectionsCommand(dataSet, Collections.singleton(dupe));
createConnections.executeCommand();
Assert.assertTrue(dataSet.isModified());
Assert.assertEquals(2, way.getNodesCount());
Assert.assertFalse(node1.hasKey(CreateConnectionsCommand.DUPE_KEY));
modified.clear();
createConnections.fillModifiedData(modified, deleted, added);
Assert.assertEquals(1, modified.size());
Assert.assertEquals(2, modified.size());
Assert.assertTrue(deleted.isEmpty());
Assert.assertTrue(added.isEmpty());
createConnections.undoCommand();
Assert.assertFalse(node1.hasKey(CreateConnectionsCommand.DUPE_KEY));
Assert.assertTrue(dupe.hasKey(CreateConnectionsCommand.DUPE_KEY));
Assert.assertFalse(dataSet.isModified());
Assert.assertEquals(2, way.getNodesCount());
}