From b07707cc22d535acca47c2556aa269f71cadfa55 Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Mon, 22 Jun 2020 13:50:19 -0600 Subject: [PATCH] FIXUP: Ensure that we don't try to merge deleted ways This fixes #81. Signed-off-by: Taylor Smock --- .../commands/MergeDuplicateWays.java | 11 ++++-- .../commands/MergeDuplicateWaysTest.java | 37 +++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/MergeDuplicateWays.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/MergeDuplicateWays.java index 8efad4a..f82bc92 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/MergeDuplicateWays.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/MergeDuplicateWays.java @@ -63,10 +63,14 @@ public class MergeDuplicateWays extends Command { public MergeDuplicateWays(DataSet data, List ways) { super(data); - this.ways = ways; + this.ways = ways.stream().filter(MergeDuplicateWays::nonDeletedWay).collect(Collectors.toList()); this.commands = new ArrayList<>(); } + private static boolean nonDeletedWay(Way way) { + return !way.isDeleted() && way.getNodes().stream().noneMatch(OsmPrimitive::isDeleted); + } + @Override public boolean executeCommand() { if (commands.isEmpty() || (command == null)) { @@ -125,7 +129,7 @@ public class MergeDuplicateWays extends Command { for (int i = 0; i < ways.size(); i++) { final Way way1 = ways.get(i); final Collection nearbyWays = dataSet.searchWays(way1.getBBox()).parallelStream() - .filter(way -> !way.isDeleted()).collect(Collectors.toList()); + .filter(MergeDuplicateWays::nonDeletedWay).collect(Collectors.toList()); nearbyWays.remove(way1); for (final Way way2 : nearbyWays) { final Command command = checkForDuplicateWays(way1, way2); @@ -150,7 +154,8 @@ public class MergeDuplicateWays extends Command { * @param commands A list of commands to add to */ public static void checkForDuplicateWays(Way way, List commands) { - final Collection nearbyWays = way.getDataSet().searchWays(way.getBBox()); + final Collection nearbyWays = way.getDataSet().searchWays(way.getBBox()).stream() + .filter(MergeDuplicateWays::nonDeletedWay).collect(Collectors.toList()); nearbyWays.remove(way); for (final Way way2 : nearbyWays) { if (!way2.isDeleted()) { diff --git a/test/unit/org/openstreetmap/josm/plugins/mapwithai/commands/MergeDuplicateWaysTest.java b/test/unit/org/openstreetmap/josm/plugins/mapwithai/commands/MergeDuplicateWaysTest.java index 1b385db..beff46f 100644 --- a/test/unit/org/openstreetmap/josm/plugins/mapwithai/commands/MergeDuplicateWaysTest.java +++ b/test/unit/org/openstreetmap/josm/plugins/mapwithai/commands/MergeDuplicateWaysTest.java @@ -1,6 +1,7 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.plugins.mapwithai.commands; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -315,4 +316,40 @@ public class MergeDuplicateWaysTest { assertSame(matchPair.a.b, MergeDuplicateWays.nodeInCompressed(testNode, testSet), "If a node has a pairing, then the paired node should be returned"); } + + /** + * Non-regression test for #81. + */ + @Test + public void testDeletedNode() { + Way way1 = TestUtils.newWay("highway=residential", new Node(LatLon.ZERO), new Node(LatLon.NORTH_POLE)); + Way way2 = TestUtils.newWay("highway=residential", new Node(LatLon.ZERO), new Node(LatLon.NORTH_POLE)); + DataSet ds = new DataSet(); + List ways = Arrays.asList(way1, way2); + for (Way way : ways) { + way.getNodes().forEach(ds::addPrimitive); + ds.addPrimitive(way); + } + way2.firstNode().setDeleted(true); + assertDoesNotThrow(() -> new MergeDuplicateWays(ds, ways).executeCommand()); + } + + /** + * Non-regression test for #81. + */ + @Test + public void testDeletedWay() { + Way way1 = TestUtils.newWay("highway=residential", new Node(LatLon.ZERO), new Node(LatLon.NORTH_POLE)); + Way way2 = TestUtils.newWay("highway=residential", new Node(LatLon.ZERO), new Node(LatLon.NORTH_POLE)); + DataSet ds = new DataSet(); + List ways = Arrays.asList(way1, way2); + for (Way way : ways) { + way.getNodes().forEach(ds::addPrimitive); + ds.addPrimitive(way); + } + way2.setDeleted(true); + assertDoesNotThrow(() -> new MergeDuplicateWays(ds, ways).executeCommand()); + } }