Skip to content

Commit

Permalink
Fix #20908: IllegalStateException: JOSM expected to find primitive in…
Browse files Browse the repository at this point in the history
… dataset after undoing a Parallel mode action

The test acts (roughly) like a user would, and performs click and drags via a new
utility class (`MapModeUtils`) which has various methods for performing mouse
actions on the map view.

git-svn-id: https://josm.openstreetmap.de/svn/trunk@19227 0c6e7542-c601-0410-84e7-c038aed88b3b
  • Loading branch information
taylor.smock committed Sep 19, 2024
1 parent 0ca2991 commit 900fd93
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@
import java.awt.Point;
import java.awt.event.KeyEvent;
import java.awt.event.MouseEvent;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.EnumMap;
import java.util.EnumSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
Expand Down Expand Up @@ -286,7 +288,9 @@ public void mousePressed(MouseEvent e) {

// Since the created way is left selected, we need to unselect again here
if (pWays != null && pWays.getWays() != null) {
getLayerManager().getEditDataSet().clearSelection(pWays.getWays());
final List<Way> ways = new ArrayList<>(pWays.getWays());
ways.removeIf(w -> w.getDataSet() == null);
getLayerManager().getEditDataSet().clearSelection(ways);
pWays = null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,40 +4,69 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.openstreetmap.josm.TestUtils;
import org.openstreetmap.josm.actions.mapmode.ParallelWayAction.Mode;
import org.openstreetmap.josm.actions.mapmode.ParallelWayAction.Modifier;
import org.openstreetmap.josm.data.UndoRedoHandler;
import org.openstreetmap.josm.data.coor.LatLon;
import org.openstreetmap.josm.data.osm.DataSet;
import org.openstreetmap.josm.gui.MainApplication;
import org.openstreetmap.josm.gui.MapFrame;
import org.openstreetmap.josm.gui.layer.OsmDataLayer;
import org.openstreetmap.josm.testutils.MapModeUtils;
import org.openstreetmap.josm.testutils.annotations.Main;
import org.openstreetmap.josm.testutils.annotations.Projection;
import org.openstreetmap.josm.tools.Logging;

/**
* Unit tests for class {@link ParallelWayAction}.
*/
@Main
@Projection
class ParallelWayActionTest {
private MapFrame map;
private ParallelWayAction mapMode;
private DataSet dataSet;

@BeforeEach
void setup() {
OsmDataLayer layer = new OsmDataLayer(new DataSet(), "ParallelWayActionTest", null);
MainApplication.getLayerManager().addLayer(layer);
this.map = MainApplication.getMap();
this.mapMode = new ParallelWayAction(this.map);
this.dataSet = layer.getDataSet();
}

/**
* Unit test of {@link ParallelWayAction#enterMode} and {@link ParallelWayAction#exitMode}.
*/
@Test
void testMode() {
OsmDataLayer layer = new OsmDataLayer(new DataSet(), "", null);
try {
MainApplication.getLayerManager().addLayer(layer);
MapFrame map = MainApplication.getMap();
ParallelWayAction mapMode = new ParallelWayAction(map);
MapMode oldMapMode = map.mapMode;
assertTrue(map.selectMapMode(mapMode));
assertEquals(mapMode, map.mapMode);
assertTrue(map.selectMapMode(oldMapMode));
} finally {
MainApplication.getLayerManager().removeLayer(layer);
}
final MapMode oldMapMode = this.map.mapMode;
assertTrue(this.map.selectMapMode(mapMode));
assertEquals(this.mapMode, this.map.mapMode);
assertTrue(this.map.selectMapMode(oldMapMode));
}

/**
* Non-regression test for <a href="https://josm.openstreetmap.de/ticket/20908">#20908</a>
*/
@Test
void testNonRegression20908() {
Logging.clearLastErrorAndWarnings();
this.map.selectMapMode(this.map.mapModeDraw);
MapModeUtils.clickAt(LatLon.ZERO);
MapModeUtils.clickAt(2, new LatLon(0.0001, 0));
assertEquals(3, this.dataSet.allPrimitives().size());
this.map.selectMapMode(mapMode);
MapModeUtils.dragFromTo(new LatLon(0.00005, 0), new LatLon(0.00005, 0.0001));
assertEquals(6, this.dataSet.allPrimitives().size());
UndoRedoHandler.getInstance().undo();
assertEquals(3, this.dataSet.allPrimitives().size());
this.map.mapMode.mousePressed(MapModeUtils.mouseClickAt(new LatLon(0.00005, 0.0001)));
assertTrue(Logging.getLastErrorAndWarnings().isEmpty(), String.join("\n", Logging.getLastErrorAndWarnings()));
}

/**
Expand Down
103 changes: 103 additions & 0 deletions test/unit/org/openstreetmap/josm/testutils/MapModeUtils.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// License: GPL. For details, see LICENSE file.
package org.openstreetmap.josm.testutils;

import static org.junit.jupiter.api.Assertions.assertEquals;

import java.awt.event.MouseEvent;

import org.awaitility.Awaitility;
import org.awaitility.Durations;
import org.openstreetmap.josm.data.coor.ILatLon;
import org.openstreetmap.josm.data.coor.LatLon;
import org.openstreetmap.josm.gui.MainApplication;
import org.openstreetmap.josm.gui.util.GuiHelper;

/**
* Utils for doing stuff in the {@link org.openstreetmap.josm.actions.mapmode.MapMode}
*/
public final class MapModeUtils {
private MapModeUtils() {
// Hide the constructor
}

/**
* Click at a specified lat/lon
* Note that we use {@link org.openstreetmap.josm.actions.mapmode.MapMode} from {@link org.openstreetmap.josm.gui.MapFrame}
* from {@link MainApplication#getMap()}.
* @param coordinates The coordinates to click at (lat, lon, lat, lon, ...)
*/
public static void clickAt(double... coordinates) {
assertEquals(0, coordinates.length % 2, "coordinates must be a multiple of 2");
for (int i = 0; i < coordinates.length; i += 2) {
clickAt(new LatLon(coordinates[i], coordinates[i + 1]));
}
}

/**
* Click at a specified lat/lon
* Note that we use {@link org.openstreetmap.josm.actions.mapmode.MapMode} from {@link org.openstreetmap.josm.gui.MapFrame}
* from {@link MainApplication#getMap()}.
* @param coordinates The coordinates to click at
*/
public static void clickAt(ILatLon... coordinates) {
assertEquals(0, coordinates.length % 2, "coordinates must be a multiple of 2");
for (ILatLon coordinate : coordinates) {
clickAt(coordinate);
}
}

/**
* Click at a specified lat/lon
* Note that we use {@link org.openstreetmap.josm.actions.mapmode.MapMode} from {@link org.openstreetmap.josm.gui.MapFrame}
* from {@link MainApplication#getMap()}.
* @param location The location to click at
*/
public static void clickAt(ILatLon location) {
clickAt(1, location);
}

/**
* Click at a specified lat/lon
* Note that we use {@link org.openstreetmap.josm.actions.mapmode.MapMode} from {@link org.openstreetmap.josm.gui.MapFrame}
* from {@link MainApplication#getMap()}.
* @param location The location to click at
* @param times The number of times to click
*/
public static void clickAt(int times, ILatLon location) {
for (int i = 0; i < times; i++) {
final var click = mouseClickAt(location);
MainApplication.getMap().mapMode.mousePressed(click);
MainApplication.getMap().mapMode.mouseReleased(click);
GuiHelper.runInEDTAndWait(() -> { /* Sync UI thread */ });
}
}

/**
* Perform a click-n-drag operation
* @param from The originating point
* @param to The end point
*/
public static void dragFromTo(ILatLon from, ILatLon to) {
MainApplication.getMap().mapMode.mousePressed(mouseClickAt(from));
// Some actions wait a period of time to avoid accidental dragging.
Awaitility.await().pollDelay(Durations.FIVE_HUNDRED_MILLISECONDS).atLeast(Durations.FIVE_HUNDRED_MILLISECONDS).until(() -> true);
MainApplication.getMap().mapMode.mouseDragged(mouseClickAt(from));
MainApplication.getMap().mapMode.mouseDragged(mouseClickAt(to));
MainApplication.getMap().mapMode.mouseReleased(mouseClickAt(to));
GuiHelper.runInEDTAndWait(() -> { /* Sync UI thread */ });
}

/**
* Create the click event
* @param location The location for the click event
* @return The click event
*/
public static MouseEvent mouseClickAt(ILatLon location) {
final var mapView = MainApplication.getMap().mapView;
mapView.zoomTo(mapView.getCenter(), 0.005);
mapView.zoomTo(location);
final var point = mapView.getPoint(location);
return new MouseEvent(MainApplication.getMap(), Long.hashCode(System.currentTimeMillis()),
System.currentTimeMillis(), 0, point.x, point.y, 1, false, MouseEvent.BUTTON1);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,17 @@ static Stream<Arguments> testExplainBadRequest() {
new OsmApiException(HttpURLConnection.HTTP_BAD_REQUEST, "You requested too many nodes", ""))
);
}

/**
* Test of {@link ExceptionUtil#explainBadRequest} method.
*/
@ParameterizedTest(name = "{1}")
@MethodSource
@SuppressWarnings("unchecked")
void testExplainBadRequest(Supplier<String> message, Object exception) {
assertEquals(message.get(), ExceptionUtil.explainBadRequest(exception instanceof Supplier ? ((Supplier<OsmApiException>) exception).get() : (OsmApiException) exception));
assertEquals(message.get(), ExceptionUtil.explainBadRequest(
exception instanceof Supplier ? ((Supplier<OsmApiException>) exception).get() : (OsmApiException) exception
));
}

/**
Expand Down

0 comments on commit 900fd93

Please sign in to comment.