From f1f69dacac4d1f8bb040947a33963031899bb2bf Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Thu, 23 Feb 2023 06:23:12 -0700 Subject: [PATCH] See #22762: Don't merge new address node with deleted address node Signed-off-by: Taylor Smock --- .../backend/MapWithAIMoveAction.java | 6 +- .../commands/MergeBuildingAddress.java | 2 +- .../backend/MapWithAIMoveActionTest.java | 65 +++++++++---------- .../CreateConnectionsCommandTest.java | 3 + 4 files changed, 37 insertions(+), 39 deletions(-) diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIMoveAction.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIMoveAction.java index 7ac36f4..aad4c95 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIMoveAction.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIMoveAction.java @@ -83,9 +83,9 @@ public class MapWithAIMoveAction extends JosmAction { nodes.stream().map(Node::getReferrers).forEach(ds::addSelected); final Collection selected = limitCollection(ds, maxAddition); final OsmDataLayer editLayer = getOsmDataLayer(); - if ((editLayer != null && !selected.isEmpty() - && (MapWithAIDataUtils.getAddedObjects() < maxAddition * MAX_ADD_MULTIPLIER)) - || (maxAddition == 0 && ExpertToggleAction.isExpert())) { + if (editLayer != null && !selected.isEmpty() + && (MapWithAIDataUtils.getAddedObjects() < maxAddition * MAX_ADD_MULTIPLIER + || (maxAddition == 0 && ExpertToggleAction.isExpert()))) { final MapWithAIAddCommand command = new MapWithAIAddCommand(mapWithAI, editLayer, selected); GuiHelper.runInEDTAndWait(() -> UndoRedoHandler.getInstance().add(command)); if (MapWithAIPreferenceHelper.isSwitchLayers()) { diff --git a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/MergeBuildingAddress.java b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/MergeBuildingAddress.java index c2b4bf4..82b9e52 100644 --- a/src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/MergeBuildingAddress.java +++ b/src/main/java/org/openstreetmap/josm/plugins/mapwithai/commands/MergeBuildingAddress.java @@ -82,7 +82,7 @@ public class MergeBuildingAddress extends AbstractConflationCommand { toCheck.addAll(affectedDataSet.searchRelations(bbox)); toCheck.addAll(affectedDataSet.searchNodes(bbox)); }); - List possibleDuplicates = toCheck.stream().filter(prim -> prim.hasTag(KEY)) + List possibleDuplicates = toCheck.stream().filter(prim -> !prim.isDeleted() && prim.hasTag(KEY)) .filter(prim -> prim.get(KEY).equals(node.get(KEY))) .filter(prim -> !prim.equals(node) && !this.possiblyAffectedPrimitives.contains(prim)) .collect(Collectors.toList()); diff --git a/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIMoveActionTest.java b/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIMoveActionTest.java index c6c1ef8..3388c22 100644 --- a/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIMoveActionTest.java +++ b/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/backend/MapWithAIMoveActionTest.java @@ -8,14 +8,9 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; -import java.util.ArrayList; import java.util.Collections; -import java.util.List; import java.util.Objects; -import java.util.logging.Handler; -import java.util.logging.LogRecord; import org.awaitility.Awaitility; import org.awaitility.Durations; @@ -28,6 +23,8 @@ import org.openstreetmap.josm.command.DeleteCommand; import org.openstreetmap.josm.data.UndoRedoHandler; import org.openstreetmap.josm.data.coor.LatLon; import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.INode; +import org.openstreetmap.josm.data.osm.IPrimitive; import org.openstreetmap.josm.data.osm.Node; import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.gui.MainApplication; @@ -39,6 +36,7 @@ import org.openstreetmap.josm.plugins.mapwithai.commands.DuplicateCommand; import org.openstreetmap.josm.plugins.mapwithai.testutils.MapWithAIPluginMock; import org.openstreetmap.josm.plugins.mapwithai.testutils.MapWithAITestRules; import org.openstreetmap.josm.plugins.mapwithai.testutils.MissingConnectionTagsMocker; +import org.openstreetmap.josm.plugins.mapwithai.testutils.annotations.LoggingHandler; import org.openstreetmap.josm.plugins.mapwithai.testutils.annotations.Wiremock; import org.openstreetmap.josm.spi.preferences.Config; import org.openstreetmap.josm.testutils.JOSMTestRules; @@ -289,6 +287,7 @@ class MapWithAIMoveActionTest { * features */ @Test + @LoggingHandler void testNonRegression22760() { final Node originalAddrNode = TestUtils.newNode("addr:city=Topeka\naddr:housenumber=1824\naddr:postcode=66615\n" + "addr:street=Southwest Stutley Road\nbuilding=residential\nlbcs:activity:code=1100\n" @@ -305,13 +304,10 @@ class MapWithAIMoveActionTest { final Node mwaiAddrNode = TestUtils.newNode( "addr:city=Unincorporated\n" + "addr:housenumber=1824\n" + "addr:postcode=66615\n" + "addr:state=KS\n" + "addr:street=Southwest Stutley Road\n" + "building=yes\n" + "source=esri_USDOT_Kansas"); - final DataSet osm = new DataSet(); - final DataSet mwai = new DataSet(); - final OsmDataLayer osmLayer = new OsmDataLayer(osm, "testNonRegression22760", null); - final MapWithAILayer mapWithAILayer = new MapWithAILayer(mwai, "testNonRegression22760", null); - MainApplication.getLayerManager().addLayer(osmLayer); - MainApplication.getLayerManager().addLayer(mapWithAILayer); - MainApplication.getLayerManager().setActiveLayer(mapWithAILayer); + final DataSet osm = this.osmLayer.getDataSet(); + final DataSet mwai = this.mapWithAIData; + osm.clear(); + mwai.clear(); originalAddrNode.setCoor(new LatLon(39.0331818, -95.7910286)); originalAddrNode.setOsmId(2081834687, 3); mwaiAddrNode.setCoor(new LatLon(39.0331883, -95.7910057)); @@ -328,30 +324,8 @@ class MapWithAIMoveActionTest { GuiHelper.runInEDTAndWait(() -> mwai.setSelected(mwai.allPrimitives())); } - final List logs = new ArrayList<>(); - final Handler testHandler = new Handler() { - @Override - public void publish(LogRecord record) { - logs.add(record); - } + this.moveAction.actionPerformed(null); - @Override - public void flush() { - // Do nothing - } - - @Override - public void close() { - // Do nothing - } - }; - Logging.getLogger().addHandler(testHandler); - try { - this.moveAction.actionPerformed(null); - assertAll(logs.stream().map(logRecord -> () -> fail(logRecord.getThrown()))); - } finally { - Logging.getLogger().removeHandler(testHandler); - } assertEquals(2, osm.getWays().size(), "Both buildings should be added, even though one is a duplicate. " + "If deduplicate buildings in the future, this test should be updated."); final Way actualWay = osm.getWays().stream().filter(way -> way.getNumKeys() > 3).findFirst() @@ -363,4 +337,25 @@ class MapWithAIMoveActionTest { assertAll(osm.getNodes().stream().filter(node -> !node.isDeleted()) .map(node -> () -> assertTrue(node.getParentWays().contains(actualWay)))); } + + @Test + @LoggingHandler + void testDeletedAddressAddingAddress() { + this.mapWithAIData.clear(); + this.osmLayer.getDataSet().clear(); + final DataSet osm = this.osmLayer.getDataSet(); + final Node osmAddr = TestUtils.newNode("addr:street=Test addr:housenumber=1"); + final Node mwaiAddr = TestUtils.newNode("addr:street=Test addr:housenumber=1 addr:city=Unknown"); + osm.addPrimitive(osmAddr); + this.mapWithAIData.addPrimitive(mwaiAddr); + UndoRedoHandler.getInstance().add(DeleteCommand.delete(Collections.singletonList(osmAddr), true, true)); + this.mapWithAIData.setSelected(mwaiAddr); + this.moveAction.actionPerformed(null); + assertEquals(1, osm.allNonDeletedPrimitives().size()); + final IPrimitive addr = osm.allNonDeletedPrimitives().iterator().next(); + assertTrue(addr instanceof INode); + assertEquals("Unknown", addr.get("addr:city")); + assertEquals("1", addr.get("addr:housenumber")); + assertEquals("Test", addr.get("addr:street")); + } } diff --git a/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/commands/CreateConnectionsCommandTest.java b/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/commands/CreateConnectionsCommandTest.java index 303562f..cdaebcd 100644 --- a/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/commands/CreateConnectionsCommandTest.java +++ b/src/test/unit/org/openstreetmap/josm/plugins/mapwithai/commands/CreateConnectionsCommandTest.java @@ -220,6 +220,9 @@ class CreateConnectionsCommandTest { DeleteCommand.delete(Collections.singletonList(addr)).executeCommand(); Command actualCommand = new CreateConnectionsCommand(ds, Collections.singleton(building1.save())); actualCommand.executeCommand(); + final Way building = ds.getWays().iterator().next(); + assertNull(building.get("addr:street")); + assertNull(building.get("addr:housenumber")); } /**