From 93f619499c5c08bc77a4dbf59f347cc2e69c5ac7 Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Wed, 2 Oct 2019 10:34:48 -0600 Subject: [PATCH] Create explicit tests for creating connections Signed-off-by: Taylor Smock --- .../commands/CreateConnectionsCommand.java | 40 ++++- .../CreateConnectionsCommandTest.java | 143 ++++++++++++++++++ 2 files changed, 176 insertions(+), 7 deletions(-) create mode 100644 test/unit/org/openstreetmap/josm/plugins/rapid/commands/CreateConnectionsCommandTest.java diff --git a/src/main/java/org/openstreetmap/josm/plugins/rapid/commands/CreateConnectionsCommand.java b/src/main/java/org/openstreetmap/josm/plugins/rapid/commands/CreateConnectionsCommand.java index 45022c0..85ba158 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/rapid/commands/CreateConnectionsCommand.java +++ b/src/main/java/org/openstreetmap/josm/plugins/rapid/commands/CreateConnectionsCommand.java @@ -18,6 +18,7 @@ import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.OsmPrimitiveType; import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.plugins.rapid.RapiDPlugin; +import org.openstreetmap.josm.tools.Geometry; import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.Utils; @@ -67,8 +68,11 @@ public class CreateConnectionsCommand extends Command { for (int i = 0; i < primitiveConnections.length / 3; i++) { if (primitiveConnections[i] instanceof Way && primitiveConnections[i + 1] instanceof Node && primitiveConnections[i + 2] instanceof Node) { - changedKeyList.add(addNodesToWay(node, (Way) primitiveConnections[i], - (Node) primitiveConnections[i + 1], (Node) primitiveConnections[i + 2])); + Command addNodesToWayCommand = addNodesToWay(node, (Way) primitiveConnections[i], + (Node) primitiveConnections[i + 1], (Node) primitiveConnections[i + 2]); + if (addNodesToWayCommand != null) { + changedKeyList.add(addNodesToWayCommand); + } } else { Logging.error("{0}: {1}, {2}: {3}, {4}: {5}", i, primitiveConnections[i].getClass(), i + 1, primitiveConnections[i + 1].getClass(), i + 2, primitiveConnections[i + 2].getClass()); @@ -83,7 +87,10 @@ public class CreateConnectionsCommand extends Command { Logging.error("RapiD: dupe connection connected to more than one node? (dupe={0})", node.get(DUPE_KEY)); } - changedKeyList.add(replaceNode(node, (Node) primitiveConnections[0])); + Command replaceCommand = replaceNode(node, (Node) primitiveConnections[0]); + if (replaceCommand != null) { + changedKeyList.add(replaceCommand); + } } } if (!changedKeyList.isEmpty()) @@ -126,11 +133,31 @@ public class CreateConnectionsCommand extends Command { * @param second The second node in a waysegemnt */ public static Command addNodesToWay(Node toAddNode, Way way, Node first, Node second) { - return new AddNodeToWayCommand(toAddNode, way, first, second); + Command tCommand = null; + Way tWay = new Way(); + tWay.addNode(first); + tWay.addNode(second); + double distance = Geometry.getDistanceWayNode(tWay, toAddNode); + if (distance < 5) { + tCommand = new AddNodeToWayCommand(toAddNode, way, first, second); + } + return tCommand; } + /** + * Replace nodes that are in the same location + * + * @param original The original node (the one to replace) + * @param newNode The node that is replacing the original node + * @return A command that replaces the node, or null if they are not at the same + * location. + */ public static Command replaceNode(Node original, Node newNode) { - return MergeNodesAction.mergeNodes(Arrays.asList(original), newNode, newNode); + Command tCommand = null; + if (original.getCoor().equalsEpsilon(newNode.getCoor())) { + tCommand = MergeNodesAction.mergeNodes(Arrays.asList(original), newNode, newNode); + } + return tCommand; } @Override @@ -141,7 +168,6 @@ public class CreateConnectionsCommand extends Command { @Override public void fillModifiedData(Collection modified, Collection deleted, Collection added) { - // Don't touch the collectioins, since we haven't modified anything -- the - // subcommands should do that. + command.fillModifiedData(modified, deleted, added); } } diff --git a/test/unit/org/openstreetmap/josm/plugins/rapid/commands/CreateConnectionsCommandTest.java b/test/unit/org/openstreetmap/josm/plugins/rapid/commands/CreateConnectionsCommandTest.java new file mode 100644 index 0000000..b3a70c1 --- /dev/null +++ b/test/unit/org/openstreetmap/josm/plugins/rapid/commands/CreateConnectionsCommandTest.java @@ -0,0 +1,143 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.plugins.rapid.commands; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; + +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.openstreetmap.josm.TestUtils; +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.OsmPrimitive; +import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.testutils.JOSMTestRules; + +/** + * Tests for {@link CreateConnections} + * + * @author Taylor Smock + */ +public class CreateConnectionsCommandTest { + @Rule + public JOSMTestRules test = new JOSMTestRules().projection(); + + /** + * Test method for + * {@link CreateConnectionsCommand#createConnections(DataSet, Collection)}. + */ + @Test + public void testCreateConnections() { + Node node1 = new Node(new LatLon(0, 0)); + Node node2 = new Node(new LatLon(1, 0)); + Node node3 = new Node(new LatLon(0.5, 0)); + Node dupe = new Node(new LatLon(0, 0)); + Way way = TestUtils.newWay("highway=residential", node1, node2); + Collection added = new ArrayList<>(); + Collection modified = new ArrayList<>(); + Collection deleted = new ArrayList<>(); + DataSet dataSet = new DataSet(node1, node2, node3, dupe, way); + + Command createConnections = new CreateConnectionsCommand(dataSet, Collections.singleton(node3)); + createConnections.executeCommand(); + + Assert.assertFalse(dataSet.isModified()); + createConnections.undoCommand(); + Assert.assertFalse(dataSet.isModified()); + + node3.put(CreateConnectionsCommand.CONN_KEY, + "w" + way.getUniqueId() + ",n" + node1.getUniqueId() + ",n" + node2.getUniqueId()); + createConnections = new CreateConnectionsCommand(dataSet, Collections.singleton(node3)); + createConnections.executeCommand(); + Assert.assertTrue(dataSet.isModified()); + Assert.assertEquals(3, way.getNodesCount()); + createConnections.fillModifiedData(modified, deleted, added); + Assert.assertEquals(3, modified.size()); // 3 since we remove the key from the node + Assert.assertTrue(deleted.isEmpty()); + Assert.assertTrue(added.isEmpty()); + createConnections.undoCommand(); + Assert.assertFalse(dataSet.isModified()); + Assert.assertEquals(2, way.getNodesCount()); + + 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()); + modified.clear(); + createConnections.fillModifiedData(modified, deleted, added); + Assert.assertEquals(1, modified.size()); + Assert.assertTrue(deleted.isEmpty()); + Assert.assertTrue(added.isEmpty()); + createConnections.undoCommand(); + Assert.assertFalse(dataSet.isModified()); + Assert.assertEquals(2, way.getNodesCount()); + } + + /** + * Test method for + * {@link CreateConnectionsCommand#addNodesToWay(Node, Node, Node, Node)}. + */ + @Test + public void testAddNodesToWay() { + Node node1 = new Node(new LatLon(0, 0)); + Node node2 = new Node(new LatLon(1, 0)); + Node node3 = new Node(new LatLon(0.5, 0)); + Way way = TestUtils.newWay("highway=residential", node1, node2); + new DataSet(node1, node2, node3, way); + Command addNodeToWayCommand = CreateConnectionsCommand.addNodesToWay(node3, way, node1, node2); + Assert.assertEquals(2, way.getNodesCount()); + addNodeToWayCommand.executeCommand(); + Assert.assertEquals(3, way.getNodesCount()); + addNodeToWayCommand.undoCommand(); + Assert.assertEquals(2, way.getNodesCount()); + + node2.setCoor(new LatLon(1, 0.1)); + addNodeToWayCommand = CreateConnectionsCommand.addNodesToWay(node3, way, node1, node2); + Assert.assertNull(addNodeToWayCommand); + + node2.setCoor(new LatLon(1, 0.01)); + addNodeToWayCommand = CreateConnectionsCommand.addNodesToWay(node3, way, node1, node2); + Assert.assertNull(addNodeToWayCommand); + + node2.setCoor(new LatLon(1, 0.00008)); + addNodeToWayCommand = CreateConnectionsCommand.addNodesToWay(node3, way, node1, node2); + addNodeToWayCommand.executeCommand(); + Assert.assertEquals(3, way.getNodesCount()); + addNodeToWayCommand.undoCommand(); + Assert.assertEquals(2, way.getNodesCount()); + } + + /** + * Test method for {@link CreateConnectionsCommand#replaceNode(Node, Node)}. + */ + @Test + public void testReplaceNode() { + Node node1 = new Node(new LatLon(0, 0)); + Node node2 = new Node(new LatLon(0, 0)); + new DataSet(node1, node2); + Command replaceNodeCommand = CreateConnectionsCommand.replaceNode(node1, node2); + replaceNodeCommand.executeCommand(); + Assert.assertTrue(node1.isDeleted()); + replaceNodeCommand.undoCommand(); + Assert.assertFalse(node1.isDeleted()); + + node2.setCoor(new LatLon(0.1, 0.1)); + Assert.assertNull(CreateConnectionsCommand.replaceNode(node1, node2)); + } + + /** + * Test method for {@link CreateConnectionsCommand#getDescriptionText()}. + */ + @Test + public void testGetDescriptionText() { + String text = new CreateConnectionsCommand(new DataSet(), null).getDescriptionText(); + Assert.assertNotNull(text); + Assert.assertFalse(text.isEmpty()); + } + +}