From f1d2e064c6217a6e09732a55fb3dc83760393338 Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Thu, 3 Oct 2019 14:16:52 -0600 Subject: [PATCH] More checkstyle and error prone fixes Signed-off-by: Taylor Smock --- .../josm/plugins/rapid/RapiDPreferences.java | 4 +- .../plugins/rapid/backend/RapiDDataUtils.java | 77 +++++++++++-------- .../rapid/backend/RapiDMoveAction.java | 2 +- .../rapid/commands/AddNodeToWayCommand.java | 53 ++++++++++--- .../commands/CreateConnectionsCommand.java | 4 +- .../commands/MovePrimitiveDataSetCommand.java | 30 ++++---- .../plugins/rapid/RapiDPreferencesTest.java | 8 +- .../rapid/backend/RapiDMoveActionTest.java | 2 +- .../commands/AddNodeToWayCommandTest.java | 2 +- .../MovePrimitiveDataSetCommandTest.java | 18 ++++- .../rapid/commands/RapiDAddComandTest.java | 22 +++--- 11 files changed, 141 insertions(+), 81 deletions(-) diff --git a/src/main/java/org/openstreetmap/josm/plugins/rapid/RapiDPreferences.java b/src/main/java/org/openstreetmap/josm/plugins/rapid/RapiDPreferences.java index 2acd3cb..e6e7285 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/rapid/RapiDPreferences.java +++ b/src/main/java/org/openstreetmap/josm/plugins/rapid/RapiDPreferences.java @@ -39,7 +39,7 @@ public class RapiDPreferences implements SubPreferenceSetting { final JLabel switchLayer = new JLabel(tr("Automatically switch layers")); final JLabel maximumAddition = new JLabel(tr("Maximum features (add)")); final JPanel container = new JPanel(new GridBagLayout()); - container.setAlignmentY(JPanel.TOP_ALIGNMENT); + container.setAlignmentY(Component.TOP_ALIGNMENT); final GridBagConstraints constraints = new GridBagConstraints(); possibleRapidApiUrl.setEditable(true); @@ -53,7 +53,7 @@ public class RapiDPreferences implements SubPreferenceSetting { } possibleRapidApiUrl.setSelectedItem(RapiDDataUtils.getRapiDURL()); - switchLayerCheckBox.setSelected(RapiDDataUtils.getSwitchLayers()); + switchLayerCheckBox.setSelected(RapiDDataUtils.isSwitchLayers()); constraints.gridx = 0; constraints.gridy = 0; diff --git a/src/main/java/org/openstreetmap/josm/plugins/rapid/backend/RapiDDataUtils.java b/src/main/java/org/openstreetmap/josm/plugins/rapid/backend/RapiDDataUtils.java index c5dc033..b3e440b 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/rapid/backend/RapiDDataUtils.java +++ b/src/main/java/org/openstreetmap/josm/plugins/rapid/backend/RapiDDataUtils.java @@ -86,45 +86,58 @@ public final class RapiDDataUtils { @Override public void run() { - final DataSet temporaryDataSet = getDataReal(bbox); + final DataSet temporaryDataSet = getDataReal(getBbox()); synchronized (RapiDDataUtils.GetDataRunnable.class) { - dataSet.mergeFrom(temporaryDataSet); + getDataSet().mergeFrom(temporaryDataSet); } } - } + /** + * @return The {@code BBox} associated with this object + */ + public BBox getBbox() { + return bbox; + } - private static DataSet getDataReal(BBox bbox) { - InputStream inputStream = null; - final DataSet dataSet = new DataSet(); - final String urlString = getRapiDURL(); - try { - final URL url = new URL(urlString.replace("{bbox}", bbox.toStringCSV(","))); - final HttpClient client = HttpClient.create(url); - final StringBuilder defaultUserAgent = new StringBuilder(); - defaultUserAgent.append(client.getHeaders().get("User-Agent")); - if (defaultUserAgent.toString().trim().length() == 0) { - defaultUserAgent.append("JOSM"); - } - defaultUserAgent.append(tr("/ {0} {1}", RapiDPlugin.NAME, RapiDPlugin.getVersionInfo())); - client.setHeader("User-Agent", defaultUserAgent.toString()); - Logging.debug("{0}: Getting {1}", RapiDPlugin.NAME, client.getURL().toString()); - final Response response = client.connect(); - inputStream = response.getContent(); - dataSet.mergeFrom(OsmReader.parseDataSet(inputStream, null)); - response.disconnect(); - } catch (UnsupportedOperationException | IllegalDataException | IOException e) { - Logging.debug(e); - } finally { - if (inputStream != null) { - try { - inputStream.close(); - } catch (final IOException e) { - Logging.debug(e); + /** + * @return The {@code DataSet} associated with this object + */ + public DataSet getDataSet() { + return dataSet; + } + + private static DataSet getDataReal(BBox bbox) { + InputStream inputStream = null; + final DataSet dataSet = new DataSet(); + final String urlString = getRapiDURL(); + try { + final URL url = new URL(urlString.replace("{bbox}", bbox.toStringCSV(","))); + final HttpClient client = HttpClient.create(url); + final StringBuilder defaultUserAgent = new StringBuilder(); + defaultUserAgent.append(client.getHeaders().get("User-Agent")); + if (defaultUserAgent.toString().trim().length() == 0) { + defaultUserAgent.append("JOSM"); + } + defaultUserAgent.append(tr("/ {0} {1}", RapiDPlugin.NAME, RapiDPlugin.getVersionInfo())); + client.setHeader("User-Agent", defaultUserAgent.toString()); + Logging.debug("{0}: Getting {1}", RapiDPlugin.NAME, client.getURL().toString()); + final Response response = client.connect(); + inputStream = response.getContent(); + dataSet.mergeFrom(OsmReader.parseDataSet(inputStream, null)); + response.disconnect(); + } catch (UnsupportedOperationException | IllegalDataException | IOException e) { + Logging.debug(e); + } finally { + if (inputStream != null) { + try { + inputStream.close(); + } catch (final IOException e) { + Logging.debug(e); + } } } + return dataSet; } - return dataSet; } /** @@ -263,7 +276,7 @@ public final class RapiDDataUtils { /** * @return {@code true} if we want to automatically switch layers */ - public static boolean getSwitchLayers() { + public static boolean isSwitchLayers() { return Config.getPref().getBoolean(RapiDPlugin.NAME.concat(".autoswitchlayers"), true); } diff --git a/src/main/java/org/openstreetmap/josm/plugins/rapid/backend/RapiDMoveAction.java b/src/main/java/org/openstreetmap/josm/plugins/rapid/backend/RapiDMoveAction.java index 59a804f..04b4c9e 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/rapid/backend/RapiDMoveAction.java +++ b/src/main/java/org/openstreetmap/josm/plugins/rapid/backend/RapiDMoveAction.java @@ -50,7 +50,7 @@ public class RapiDMoveAction extends JosmAction { if (editLayer != null) { final RapiDAddCommand command = new RapiDAddCommand(rapid, editLayer, selected); UndoRedoHandler.getInstance().add(command); - if (RapiDDataUtils.getSwitchLayers()) { + if (RapiDDataUtils.isSwitchLayers()) { MainApplication.getLayerManager().setActiveLayer(editLayer); final DataSet editable = editLayer.getDataSet(); editable.setSelected( diff --git a/src/main/java/org/openstreetmap/josm/plugins/rapid/commands/AddNodeToWayCommand.java b/src/main/java/org/openstreetmap/josm/plugins/rapid/commands/AddNodeToWayCommand.java index 963aad3..fdc0ddf 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/rapid/commands/AddNodeToWayCommand.java +++ b/src/main/java/org/openstreetmap/josm/plugins/rapid/commands/AddNodeToWayCommand.java @@ -14,35 +14,36 @@ import org.openstreetmap.josm.data.osm.Way; public class AddNodeToWayCommand extends Command { private final Node toAddNode; private final Way way; - private final Node first; - private final Node second; + private final Node firstNode; + private final Node secondNode; /** * Add a node to a way in an undoable manner * - * @param toAddNode The node to add - * @param way The way to add the node to - * @param first The node that comes before the node to add - * @param second The node that comes after the node to add + * @param toAddNode The node to add + * @param way The way to add the node to + * @param firstNode The node that comes before the node to add + * @param secondNode The node that comes after the node to add */ public AddNodeToWayCommand(Node toAddNode, Way way, Node first, Node second) { super(way.getDataSet()); this.toAddNode = toAddNode; this.way = way; - this.first = first; - this.second = second; + this.firstNode = first; + this.secondNode = second; } @Override public boolean executeCommand() { - final int index = Math.max(way.getNodes().indexOf(first), way.getNodes().indexOf(second)); - way.addNode(index, toAddNode); + final int index = Math.max(getWay().getNodes().indexOf(getFirstNode()), + getWay().getNodes().indexOf(getSecondNode())); + getWay().addNode(index, getToAddNode()); return true; } @Override public void undoCommand() { - way.removeNode(toAddNode); + getWay().removeNode(getToAddNode()); } @Override @@ -53,6 +54,34 @@ public class AddNodeToWayCommand extends Command { @Override public void fillModifiedData(Collection modified, Collection deleted, Collection added) { - modified.addAll(Arrays.asList(toAddNode, way)); + modified.addAll(Arrays.asList(getToAddNode(), getWay())); + } + + /** + * @return {@link Node} to add to {@link Way} + */ + public Node getToAddNode() { + return toAddNode; + } + + /** + * @return {@link Way} that we are adding a {@link Node} to + */ + public Way getWay() { + return way; + } + + /** + * @return {@link Node} that we are adding another {@link Node} after. + */ + public Node getFirstNode() { + return firstNode; + } + + /** + * @return {@link Node} that we are adding another {@link Node} before. + */ + public Node getSecondNode() { + return secondNode; } } 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 1b128d2..6314a46 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 @@ -150,11 +150,11 @@ public class CreateConnectionsCommand extends Command { missingPrimitives.put(i, new Pair<>(id, type)); } } - getMissingPrimitives(dataSet, primitiveConnections, missingPrimitives); + obtainMissingPrimitives(dataSet, primitiveConnections, missingPrimitives); return primitiveConnections; } - private static void getMissingPrimitives(DataSet dataSet, OsmPrimitive[] primitiveConnections, + private static void obtainMissingPrimitives(DataSet dataSet, OsmPrimitive[] primitiveConnections, Map> missingPrimitives) { final Map ids = missingPrimitives.entrySet().stream().collect(Collectors .toMap(entry -> new SimplePrimitiveId(entry.getValue().a, entry.getValue().b), Entry::getKey)); 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 bc08bbc..35ecc5b 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 @@ -25,14 +25,13 @@ public class MovePrimitiveDataSetCommand extends Command { private final DataSet to; private final DataSet from; private final Collection primitives; - private SequenceCommand command; + private SequenceCommand command = null; public MovePrimitiveDataSetCommand(DataSet to, DataSet from, Collection primitives) { super(to); this.to = to; this.from = from; this.primitives = primitives; - command = null; } @Override @@ -52,22 +51,23 @@ public class MovePrimitiveDataSetCommand extends Command { * @param selection The primitives to move */ public SequenceCommand moveCollection(DataSet from, DataSet to, Collection selection) { + SequenceCommand returnCommand = null; 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; + } else { + final List commands = new ArrayList<>(); + + final Collection allNeededPrimitives = new ArrayList<>(); + RapiDDataUtils.addPrimitivesToCollection(allNeededPrimitives, selection); + + commands.add(new DeletePrimitivesCommand(from, selection, true)); + final AddPrimitivesCommand addPrimitivesCommand = new AddPrimitivesCommand(to, allNeededPrimitives, selection); + commands.add(addPrimitivesCommand); + + returnCommand = new SequenceCommand(trn("Move {0} OSM Primitive between data sets", + "Move {0} OSM Primitives between data sets", selection.size(), selection.size()), commands); } - - final List commands = new ArrayList<>(); - - final Collection allNeededPrimitives = new ArrayList<>(); - RapiDDataUtils.addPrimitivesToCollection(allNeededPrimitives, selection); - - commands.add(new DeletePrimitivesCommand(from, selection, true)); - final AddPrimitivesCommand addPrimitivesCommand = new AddPrimitivesCommand(to, allNeededPrimitives, selection); - commands.add(addPrimitivesCommand); - - return new SequenceCommand(trn("Move {0} OSM Primitive between data sets", - "Move {0} OSM Primitives between data sets", selection.size(), selection.size()), commands); + return returnCommand; } @Override diff --git a/test/unit/org/openstreetmap/josm/plugins/rapid/RapiDPreferencesTest.java b/test/unit/org/openstreetmap/josm/plugins/rapid/RapiDPreferencesTest.java index 5cf4c35..d9f625f 100644 --- a/test/unit/org/openstreetmap/josm/plugins/rapid/RapiDPreferencesTest.java +++ b/test/unit/org/openstreetmap/josm/plugins/rapid/RapiDPreferencesTest.java @@ -43,16 +43,16 @@ public class RapiDPreferencesTest { Assert.assertEquals(tabs + 1, pane.getPluginPreference().getTabPane().getTabCount()); Assert.assertEquals(pane.getPluginPreference(), preferences.getTabPreferenceSetting(pane)); - final boolean switchLayers = RapiDDataUtils.getSwitchLayers(); + final boolean switchLayers = RapiDDataUtils.isSwitchLayers(); Assert.assertEquals(switchLayers, preferences.getSwitchLayerCheckBox().isSelected()); preferences.ok(); - Assert.assertEquals(switchLayers, RapiDDataUtils.getSwitchLayers()); + Assert.assertEquals(switchLayers, RapiDDataUtils.isSwitchLayers()); preferences.getSwitchLayerCheckBox().setSelected(!switchLayers); - Assert.assertNotEquals(!switchLayers, RapiDDataUtils.getSwitchLayers()); + Assert.assertNotEquals(!switchLayers, RapiDDataUtils.isSwitchLayers()); preferences.ok(); - Assert.assertEquals(!switchLayers, RapiDDataUtils.getSwitchLayers()); + Assert.assertEquals(!switchLayers, RapiDDataUtils.isSwitchLayers()); final Object tmp = preferences.getMaximumAdditionSpinner().getModel(); SpinnerNumberModel spinnerModel = null; diff --git a/test/unit/org/openstreetmap/josm/plugins/rapid/backend/RapiDMoveActionTest.java b/test/unit/org/openstreetmap/josm/plugins/rapid/backend/RapiDMoveActionTest.java index 4441bb1..0ed186d 100644 --- a/test/unit/org/openstreetmap/josm/plugins/rapid/backend/RapiDMoveActionTest.java +++ b/test/unit/org/openstreetmap/josm/plugins/rapid/backend/RapiDMoveActionTest.java @@ -22,7 +22,7 @@ public class RapiDMoveActionTest { public JOSMTestRules test = new JOSMTestRules().preferences().main().projection(); @Before - public void setup() { + public void setUp() { moveAction = new RapiDMoveAction(); } diff --git a/test/unit/org/openstreetmap/josm/plugins/rapid/commands/AddNodeToWayCommandTest.java b/test/unit/org/openstreetmap/josm/plugins/rapid/commands/AddNodeToWayCommandTest.java index 8874997..a73fa83 100644 --- a/test/unit/org/openstreetmap/josm/plugins/rapid/commands/AddNodeToWayCommandTest.java +++ b/test/unit/org/openstreetmap/josm/plugins/rapid/commands/AddNodeToWayCommandTest.java @@ -14,7 +14,7 @@ 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;; +import org.openstreetmap.josm.testutils.JOSMTestRules; public class AddNodeToWayCommandTest { private Node toAdd; 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 7cbecaa..a730be7 100644 --- a/test/unit/org/openstreetmap/josm/plugins/rapid/commands/MovePrimitiveDataSetCommandTest.java +++ b/test/unit/org/openstreetmap/josm/plugins/rapid/commands/MovePrimitiveDataSetCommandTest.java @@ -34,7 +34,7 @@ public class MovePrimitiveDataSetCommandTest { from.addPrimitive(way1); from.addPrimitive(new Node(new LatLon(-0.1, 0.1))); - MovePrimitiveDataSetCommand move = new MovePrimitiveDataSetCommand(to, from, Collections.singleton(way1)); + final MovePrimitiveDataSetCommand move = new MovePrimitiveDataSetCommand(to, from, Collections.singleton(way1)); Assert.assertEquals(0, to.allPrimitives().size()); Assert.assertEquals(4, from.allPrimitives().size()); @@ -51,6 +51,22 @@ public class MovePrimitiveDataSetCommandTest { Assert.assertEquals(0, to.allPrimitives().size()); Assert.assertEquals(4, from.allPrimitives().size()); Assert.assertEquals(from, way1.getDataSet()); + } + + @Test + public void testMovePrimitivesAdditionalData() { + final Collection added = new ArrayList<>(); + final Collection modified = new ArrayList<>(); + final Collection deleted = new ArrayList<>(); + final DataSet to = new DataSet(); + final DataSet from = new DataSet(); + final Way way1 = TestUtils.newWay("highway=tertiary", new Node(new LatLon(0, 0)), + new Node(new LatLon(0.1, 0.1))); + way1.getNodes().stream().forEach(node -> from.addPrimitive(node)); + from.addPrimitive(way1); + from.addPrimitive(new Node(new LatLon(-0.1, 0.1))); + + MovePrimitiveDataSetCommand move = new MovePrimitiveDataSetCommand(to, from, Collections.singleton(way1)); way1.firstNode().put("highway", "stop"); diff --git a/test/unit/org/openstreetmap/josm/plugins/rapid/commands/RapiDAddComandTest.java b/test/unit/org/openstreetmap/josm/plugins/rapid/commands/RapiDAddComandTest.java index 7ac3983..be58179 100644 --- a/test/unit/org/openstreetmap/josm/plugins/rapid/commands/RapiDAddComandTest.java +++ b/test/unit/org/openstreetmap/josm/plugins/rapid/commands/RapiDAddComandTest.java @@ -16,6 +16,8 @@ import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.testutils.JOSMTestRules; public class RapiDAddComandTest { + private final static String HIGHWAY_RESIDENTIAL = "highway=residential"; + @Rule public JOSMTestRules test = new JOSMTestRules(); @@ -23,10 +25,10 @@ public class RapiDAddComandTest { public void testMoveCollection() { final DataSet ds1 = new DataSet(); final DataSet ds2 = new DataSet(); - final Way way1 = TestUtils.newWay("highway=residential", new Node(new LatLon(0, 0)), + final Way way1 = TestUtils.newWay(HIGHWAY_RESIDENTIAL, new Node(new LatLon(0, 0)), new Node(new LatLon(0, 0.1))); - final Way way2 = TestUtils.newWay("highway=residential", new Node(new LatLon(-0.1, -0.2)), way1.firstNode()); - final Way way3 = TestUtils.newWay("highway=residential", new Node(new LatLon(65, 65)), + final Way way2 = TestUtils.newWay(HIGHWAY_RESIDENTIAL, new Node(new LatLon(-0.1, -0.2)), way1.firstNode()); + final Way way3 = TestUtils.newWay(HIGHWAY_RESIDENTIAL, new Node(new LatLon(65, 65)), new Node(new LatLon(66, 66))); for (final Way way : Arrays.asList(way1, way2, way3)) { for (final Node node : way.getNodes()) { @@ -68,14 +70,14 @@ public class RapiDAddComandTest { @Test public void testCreateConnections() { final DataSet ds1 = new DataSet(); - final Way way1 = TestUtils.newWay("highway=residential", new Node(new LatLon(0, 0)), + final Way way1 = TestUtils.newWay(HIGHWAY_RESIDENTIAL, new Node(new LatLon(0, 0)), new Node(new LatLon(0, 0.15))); - final Way way2 = TestUtils.newWay("highway=residential", new Node(new LatLon(0, 0.05)), + final Way way2 = TestUtils.newWay(HIGHWAY_RESIDENTIAL, new Node(new LatLon(0, 0.05)), new Node(new LatLon(0.05, 0.2))); way2.firstNode().put("conn", "w".concat(Long.toString(way1.getUniqueId())).concat(",n") - .concat(Long.toString(way1.firstNode().getUniqueId())).concat(",n") - .concat(Long.toString(way1.lastNode().getUniqueId()))); + .concat(Long.toString(way1.firstNode().getUniqueId())).concat(",n") + .concat(Long.toString(way1.lastNode().getUniqueId()))); way1.getNodes().forEach(node -> ds1.addPrimitive(node)); way2.getNodes().forEach(node -> ds1.addPrimitive(node)); ds1.addPrimitive(way2); @@ -84,7 +86,7 @@ public class RapiDAddComandTest { Assert.assertEquals(3, way1.getNodesCount()); Assert.assertFalse(way1.isFirstLastNode(way2.firstNode())); - final Way way3 = TestUtils.newWay("highway=residential", new Node(new LatLon(0, 0)), + final Way way3 = TestUtils.newWay(HIGHWAY_RESIDENTIAL, new Node(new LatLon(0, 0)), new Node(new LatLon(-0.1, -0.1))); way3.firstNode().put("dupe", "n".concat(Long.toString(way1.firstNode().getUniqueId()))); way3.getNodes().forEach(node -> ds1.addPrimitive(node)); @@ -100,9 +102,9 @@ public class RapiDAddComandTest { public void testCreateConnectionsUndo() { final DataSet osmData = new DataSet(); final DataSet rapidData = new DataSet(); - final Way way1 = TestUtils.newWay("highway=residential", new Node(new LatLon(0, 0)), + final Way 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)), + final Way 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));