Fix #22649: IAE: node is already deleted

Signed-off-by: Taylor Smock <tsmock@meta.com>
pull/15/head v1.10.3
Taylor Smock 2023-01-16 07:08:53 -07:00
rodzic 1c4b249284
commit 00835859c5
Nie znaleziono w bazie danych klucza dla tego podpisu
ID klucza GPG: 233BB2E466604E27
6 zmienionych plików z 177 dodań i 13 usunięć

Wyświetl plik

@ -79,6 +79,7 @@ public class MergeAddressBuildings extends AbstractConflationCommand {
private static Collection<? extends Command> mergeAddressBuilding(DataSet affectedDataSet, OsmPrimitive object) {
final List<IPrimitive> toCheck = new ArrayList<>(affectedDataSet.searchNodes(object.getBBox()));
toCheck.removeIf(IPrimitive::isDeleted);
final Collection<IPrimitive> nodesInside = Geometry.filterInsideAnyPolygon(toCheck, object);
final List<Node> nodesWithAddresses = nodesInside.stream().filter(Node.class::isInstance).map(Node.class::cast)

Wyświetl plik

@ -22,6 +22,9 @@ import org.openstreetmap.josm.testutils.JOSMTestRules;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
/**
* Test class for {@link ConnectedCommand}
*/
@NoExceptions
class ConnectedCommandTest {
/**
@ -29,7 +32,7 @@ class ConnectedCommandTest {
*/
@RegisterExtension
@SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
JOSMTestRules rule = new JOSMTestRules().projection();
static JOSMTestRules rule = new JOSMTestRules().projection();
@Test
void testVariousConditions() {

Wyświetl plik

@ -16,6 +16,7 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.openstreetmap.josm.TestUtils;
import org.openstreetmap.josm.command.Command;
import org.openstreetmap.josm.command.DeleteCommand;
import org.openstreetmap.josm.command.SequenceCommand;
import org.openstreetmap.josm.data.coor.LatLon;
import org.openstreetmap.josm.data.osm.DataSet;
@ -26,6 +27,7 @@ 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.mapwithai.testutils.MapWithAITestRules;
import org.openstreetmap.josm.plugins.mapwithai.testutils.annotations.LoggingHandler;
import org.openstreetmap.josm.plugins.mapwithai.testutils.annotations.MapWithAISources;
import org.openstreetmap.josm.plugins.mapwithai.testutils.annotations.Wiremock;
import org.openstreetmap.josm.testutils.JOSMTestRules;
@ -33,7 +35,7 @@ import org.openstreetmap.josm.testutils.JOSMTestRules;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
/**
* Tests for {@link CreateConnections}
* Tests for {@link CreateConnectionsCommand}
*
* @author Taylor Smock
*/
@ -43,7 +45,7 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
class CreateConnectionsCommandTest {
@RegisterExtension
@SuppressFBWarnings("URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
JOSMTestRules test = new MapWithAITestRules().projection();
static JOSMTestRules test = new MapWithAITestRules().projection();
/**
* Test method for
@ -109,7 +111,7 @@ class CreateConnectionsCommandTest {
/**
* Test method for
* {@link CreateConnectionsCommand#addNodesToWay(Node, Node, Node, Node)}.
* {@link ConnectedCommand#addNodesToWay(Node, Way, Node, Node)}.
*/
@Test
void testAddNodesToWay() {
@ -144,7 +146,7 @@ class CreateConnectionsCommandTest {
}
/**
* Test method for {@link CreateConnectionsCommand#replaceNode(Node, Node)}.
* Test method for {@link DuplicateCommand#replaceNode(Node, Node)}.
*/
@Test
void testReplaceNode() {
@ -201,6 +203,25 @@ class CreateConnectionsCommandTest {
}
@Test
@LoggingHandler
void testDeletedAddressAddingBuilding() {
DataSet ds = new DataSet();
Node addr = new Node(new LatLon(0, 0));
MainApplication.getLayerManager().addLayer(new OsmDataLayer(ds, "testDeletedAddress", null));
addr.put("addr:street", "Test");
addr.put("addr:housenumber", "1");
Way building1 = TestUtils.newWay("building=yes", new Node(new LatLon(0.00001, 0.00001)),
new Node(new LatLon(0.00001, -0.00001)), new Node(new LatLon(-0.00001, -0.00001)),
new Node(new LatLon(-0.00001, 0.00001)));
ds.addPrimitive(addr);
ds.addPrimitiveRecursive(building1);
building1.addNode(building1.firstNode());
DeleteCommand.delete(Collections.singletonList(addr)).executeCommand();
Command actualCommand = new CreateConnectionsCommand(ds, Collections.singleton(building1.save()));
actualCommand.executeCommand();
}
/**
* Test method for {@link CreateConnectionsCommand#getDescriptionText()}.
*/

Wyświetl plik

@ -12,6 +12,7 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.openstreetmap.josm.TestUtils;
import org.openstreetmap.josm.command.Command;
import org.openstreetmap.josm.command.DeleteCommand;
import org.openstreetmap.josm.data.coor.LatLon;
import org.openstreetmap.josm.data.osm.DataSet;
import org.openstreetmap.josm.data.osm.Node;
@ -22,13 +23,16 @@ import org.openstreetmap.josm.testutils.JOSMTestRules;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
/**
* Test class for {@link MergeBuildingAddress}
*/
class MergeBuildingAddressTest {
/**
* Setup test.
*/
@RegisterExtension
@SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
JOSMTestRules rule = new JOSMTestRules().projection();
static JOSMTestRules rule = new JOSMTestRules().projection();
@Test
void testSingleAddress() {
@ -144,4 +148,22 @@ class MergeBuildingAddressTest {
assertEquals("Test", building1.get("addr:street"));
assertEquals("1", building1.get("addr:housenumber"));
}
@Test
void testDeletedBuilding() {
DataSet ds = new DataSet();
Node addr = new Node(new LatLon(0, 0));
addr.put("addr:street", "Test");
addr.put("addr:housenumber", "1");
Way building1 = TestUtils.newWay("building=yes", new Node(new LatLon(0.00001, 0.00001)),
new Node(new LatLon(0.00001, -0.00001)), new Node(new LatLon(-0.00001, -0.00001)),
new Node(new LatLon(-0.00001, 0.00001)));
ds.addPrimitive(addr);
ds.addPrimitiveRecursive(building1);
building1.addNode(building1.firstNode());
MergeBuildingAddress conflation = new MergeBuildingAddress(ds);
DeleteCommand.delete(Collections.singletonList(building1)).executeCommand();
Command command = conflation.getCommand(Collections.singletonList(addr));
assertNull(command);
}
}

Wyświetl plik

@ -6,14 +6,15 @@ import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.openstreetmap.josm.tools.I18n.tr;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.openstreetmap.josm.TestUtils;
import org.openstreetmap.josm.command.Command;
import org.openstreetmap.josm.command.DeleteCommand;
import org.openstreetmap.josm.data.coor.LatLon;
import org.openstreetmap.josm.data.osm.DataSet;
import org.openstreetmap.josm.data.osm.Node;
@ -23,6 +24,7 @@ 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.mapwithai.commands.MergeAddressBuildings;
import org.openstreetmap.josm.plugins.mapwithai.testutils.annotations.LoggingHandler;
import org.openstreetmap.josm.testutils.JOSMTestRules;
import org.openstreetmap.josm.testutils.annotations.BasicPreferences;
@ -33,7 +35,7 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
class MergeAddressBuildingsTest {
@RegisterExtension
@SuppressFBWarnings("URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
JOSMTestRules test = new JOSMTestRules().projection().main();
static JOSMTestRules test = new JOSMTestRules().projection().main();
private MergeAddressBuildings command;
private DataSet ds;
@ -71,11 +73,11 @@ class MergeAddressBuildingsTest {
command = new MergeAddressBuildings(ds);
address.setCoor(new LatLon(0.5, 0.5));
Command newCommand = command.getCommand(Arrays.asList(building));
Command newCommand = command.getCommand(Collections.singletonList(building));
assertNull(newCommand);
building.addNode(building.firstNode());
newCommand = command.getCommand(Arrays.asList(building));
newCommand = command.getCommand(Collections.singletonList(building));
newCommand.executeCommand();
@ -85,21 +87,39 @@ class MergeAddressBuildingsTest {
address.setCoor(new LatLon(-1, -1));
newCommand = command.getCommand(Arrays.asList(building));
newCommand = command.getCommand(Collections.singletonList(building));
assertNull(newCommand);
assertEquals(6, ds.allNonDeletedPrimitives().size());
address.setCoor(new LatLon(.75, .75));
Node address2 = new Node(new LatLon(0.25, 0.25));
address.getKeys().forEach((key, value) -> address2.put(key, value));
address.getKeys().forEach(address2::put);
ds.addPrimitive(address2);
newCommand = command.getCommand(Arrays.asList(building));
newCommand = command.getCommand(Collections.singletonList(building));
assertNull(newCommand);
assertEquals(7, ds.allNonDeletedPrimitives().size());
}
@Test
@LoggingHandler
void testDeletedAddress() {
Node addr = new Node(new LatLon(0, 0));
MainApplication.getLayerManager().addLayer(new OsmDataLayer(ds, "testDeletedAddress", null));
addr.put("addr:street", "Test");
addr.put("addr:housenumber", "1");
Way building1 = TestUtils.newWay("building=yes", new Node(new LatLon(0.00001, 0.00001)),
new Node(new LatLon(0.00001, -0.00001)), new Node(new LatLon(-0.00001, -0.00001)),
new Node(new LatLon(-0.00001, 0.00001)));
ds.addPrimitive(addr);
ds.addPrimitiveRecursive(building1);
building1.addNode(building1.firstNode());
DeleteCommand.delete(Collections.singletonList(addr)).executeCommand();
Command actualCommand = this.command.getCommand(Collections.singletonList(building1));
assertNull(actualCommand);
}
@Test
void testGetDescriptionText() {
assertEquals(tr("Merge added buildings with existing address nodes"), command.getDescriptionText());

Wyświetl plik

@ -0,0 +1,97 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.plugins.mapwithai.testutils.annotations;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.junit.jupiter.api.Assertions.fail;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.logging.Handler;
import java.util.logging.Level;
import java.util.logging.LogRecord;
import java.util.logging.Logger;
import java.util.stream.Stream;
import org.junit.jupiter.api.extension.AfterEachCallback;
import org.junit.jupiter.api.extension.BeforeEachCallback;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.openstreetmap.josm.tools.Logging;
@Target({ ElementType.TYPE, ElementType.METHOD, ElementType.PARAMETER })
@Retention(RetentionPolicy.RUNTIME)
@ExtendWith(LoggingHandler.LogHandler.class)
public @interface LoggingHandler {
class LogHandler implements AfterEachCallback, BeforeEachCallback {
private static class TestLogHandler extends Handler {
final Map<Level, List<LogRecord>> errorList = new HashMap<>();
@Override
public void publish(LogRecord record) {
errorList.computeIfAbsent(record.getLevel(), level -> new ArrayList<>()).add(record);
}
@Override
public void flush() {
// Do nothing
}
@Override
public void close() {
// Do nothing
}
}
@Override
public void afterEach(ExtensionContext context) {
final Logger logger = Logging.getLogger();
final Handler[] testHandlers = logger.getHandlers();
final ExtensionContext.Store store = getStore(context);
final Handler[] originalHandlers = store.get(logger, Handler[].class);
for (Handler handler : testHandlers) {
logger.removeHandler(handler);
}
for (Handler handler : originalHandlers) {
logger.addHandler(handler);
}
final Map<Level, List<LogRecord>> recordedErrors = store.get(TestLogHandler.class,
TestLogHandler.class).errorList;
final List<LogRecord> issues = new ArrayList<>();
if (recordedErrors.containsKey(Level.SEVERE)) {
issues.addAll(recordedErrors.get(Level.SEVERE));
}
if (recordedErrors.containsKey(Level.WARNING)) {
issues.addAll(recordedErrors.get(Level.WARNING));
}
assertAll(Stream.concat(
issues.stream().filter(logRecord -> logRecord.getThrown() != null)
.map(logRecord -> fail(logRecord.getThrown())),
issues.stream().filter(logRecord -> logRecord.getThrown() == null)
.map(logRecord -> fail(logRecord.getMessage()))));
}
@Override
public void beforeEach(ExtensionContext context) {
final Logger logger = Logging.getLogger();
final Handler[] originalHandlers = logger.getHandlers();
final ExtensionContext.Store store = getStore(context);
store.put(logger, originalHandlers);
for (Handler handler : originalHandlers) {
logger.removeHandler(handler);
}
final TestLogHandler testLogHandler = new TestLogHandler();
logger.addHandler(testLogHandler);
store.put(TestLogHandler.class, testLogHandler);
}
private static ExtensionContext.Store getStore(ExtensionContext context) {
return context.getStore(ExtensionContext.Namespace.create(Handler.class));
}
}
}