Changeset 18935 in josm for trunk/test/unit/org/openstreetmap/josm/actions/SimplifyWayActionTest.java

Timestamp:

2024-01-15T17:20:18+01:00 (5 months ago)

Author:

taylor.smock

Message:

Fix #23399: Simplify way crashes

This was primarily caused by selection updates occurring while the commands were generated. The fixes could be any of the following:

• Synchronized collections
• New list
• Avoiding selection updates

The last option was taken for the following reasons:

• Avoiding a StackOverflow exception when many ways are being simplified
• Reduced memory allocations (>11GB to <250MB)
• Reduced CPU cycles

File:

• trunk/test/unit/org/openstreetmap/josm/actions/SimplifyWayActionTest.java (modified) (3 diffs)

trunk/test/unit/org/openstreetmap/josm/actions/SimplifyWayActionTest.java

@@ -2 +2 @@
 package org.openstreetmap.josm.actions;
 
+import static org.junit.jupiter.api.Assertions.assertAll;
+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;
+import static org.openstreetmap.josm.tools.I18n.tr;
 
 import java.io.IOException;
@@ -25 +29 @@
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.gui.ExtendedDialog;
 import org.openstreetmap.josm.gui.MainApplication;
+import org.openstreetmap.josm.gui.layer.OsmDataLayer;
+import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.io.IllegalDataException;
 import org.openstreetmap.josm.io.OsmReader;
 import org.openstreetmap.josm.testutils.annotations.Main;
 import org.openstreetmap.josm.testutils.annotations.Projection;
+import org.openstreetmap.josm.testutils.mockers.ExtendedDialogMocker;
+import org.openstreetmap.josm.testutils.mockers.HelpAwareOptionPaneMocker;
 import org.openstreetmap.josm.tools.Utils;
 
@@ -100 +109 @@
         assertEquals(Collections.singleton(n1), deleteCommands.iterator().next().getParticipatingPrimitives());
     }
+
+    /**
+     * Non-regression test for #23399
+     */
+    @Test
+    void testNonRegression23399() {
+        TestUtils.assumeWorkingJMockit();
+        new ExtendedDialogMocker(Collections.singletonMap("Simplify way", "Simplify")) {
+            @Override
+            protected String getString(ExtendedDialog instance) {
+                return instance.getTitle();
+            }
+        };
+        new HelpAwareOptionPaneMocker(Collections.singletonMap(
+                tr("The selection contains {0} ways. Are you sure you want to simplify them all?", 1000), "Yes"));
+        final ArrayList<Way> ways = new ArrayList<>(1000);
+        final DataSet ds = new DataSet();
+        for (int i = 0; i < 1000; i++) {
+            final Way way = TestUtils.newWay("", new Node(new LatLon(0, 0)), new Node(new LatLon(0, 0.001)),
+                    new Node(new LatLon(0, 0.002)));
+            ways.add(way);
+            ds.addPrimitiveRecursive(way);
+        }
+        MainApplication.getLayerManager().addLayer(new OsmDataLayer(ds, "SimplifyWayActionTest#testNonRegression23399", null));
+        GuiHelper.runInEDTAndWait(() -> ds.setSelected(ds.allPrimitives()));
+        assertEquals(ds.allPrimitives().size(), ds.getAllSelected().size());
+        assertDoesNotThrow(() -> GuiHelper.runInEDTAndWaitWithException(() -> action.actionPerformed(null)));
+        assertAll(ways.stream().map(way -> () -> assertEquals(2, way.getNodesCount())));
+        assertAll(ds.getAllSelected().stream().map(p -> () -> assertFalse(p.isDeleted())));
+        assertEquals(3000, ds.getAllSelected().size());
+    }
 }