FIXUP: Add a test (and fix an issue) whereby simplified ways could cause a crash

Signed-off-by: Taylor Smock <taylor.smock@kaart.com>
pull/1/head
Taylor Smock 2020-08-14 08:59:09 -06:00
rodzic ed83ff4e1e
commit 251f968df8
Nie znaleziono w bazie danych klucza dla tego podpisu
ID klucza GPG: 625F6A74A3E4311A
2 zmienionych plików z 137 dodań i 45 usunięć

Wyświetl plik

@ -24,8 +24,84 @@ import org.openstreetmap.josm.tools.Utils;
public class OverNodedWays extends AbstractConflationCommand {
private double threshold;
private int acceptableRemovalPercentage;
private class PostponedOverNodedWayCommand extends Command {
private Command realCommand;
protected PostponedOverNodedWayCommand(DataSet data) {
super(data);
}
@Override
public boolean executeCommand() {
if (realCommand == null) {
double threshold = Config.getPref().getDouble("mapwithai.conflation.simplifyway", 0.5);
int acceptableRemovalPercentage = Config.getPref()
.getInt("mapwithai.conflation.simplifywaynodepercentagerequired", 20);
Layer current = MainApplication.getLayerManager().getActiveLayer();
Collection<Way> ways = Utils.filteredCollection(possiblyAffectedPrimitives, Way.class);
if (!ways.isEmpty()) {
DataSet ds = this.getAffectedDataSet();
Layer toSwitch = MainApplication.getLayerManager().getLayersOfType(AbstractOsmDataLayer.class)
.stream().filter(d -> ds.equals(d.getDataSet())).findAny().orElse(null);
if (toSwitch != null) {
MainApplication.getLayerManager().setActiveLayer(toSwitch);
}
}
Collection<Command> commands = new ArrayList<>();
for (Way way : ways) {
Command command = SimplifyWayAction.createSimplifyCommand(way, threshold);
if (command == null) {
continue;
}
int count = Utils
.filteredCollection(new ArrayList<>(command.getParticipatingPrimitives()), Node.class)
.size();
if ((count / (double) way.getNodesCount()) * 100 > acceptableRemovalPercentage) {
AutoScaleAction.zoomTo(Collections.singleton(way));
double length = SimplifyWayAction.askSimplifyWays(
tr("You are about to simplify {0} way with a total length of {1}.", 1, way.getLength()),
true);
command = SimplifyWayAction.createSimplifyCommand(way, length);
if (command != null) {
commands.add(command);
}
}
}
if (current != null) {
MainApplication.getLayerManager().setActiveLayer(current);
}
if (!commands.isEmpty()) {
realCommand = SequenceCommand.wrapIfNeeded(tr("Simplify ways"), commands);
realCommand.executeCommand();
}
} else {
realCommand.executeCommand();
}
return true;
}
@Override
public void undoCommand() {
if (realCommand != null) {
realCommand.undoCommand();
}
}
@Override
public String getDescriptionText() {
return realCommand != null ? realCommand.getDescriptionText() : "Command not run";
}
@Override
public void fillModifiedData(Collection<OsmPrimitive> modified, Collection<OsmPrimitive> deleted,
Collection<OsmPrimitive> added) {
if (realCommand != null) {
realCommand.fillModifiedData(modified, deleted, added);
}
}
}
public OverNodedWays(DataSet data) {
super(data);
@ -49,46 +125,7 @@ public class OverNodedWays extends AbstractConflationCommand {
@Override
public Command getRealCommand() {
threshold = Config.getPref().getDouble("mapwithai.conflation.simplifyway", 0.5);
acceptableRemovalPercentage = Config.getPref().getInt("mapwithai.conflation.simplifywaynodepercentagerequired",
20);
Layer current = MainApplication.getLayerManager().getActiveLayer();
Collection<Way> ways = Utils.filteredCollection(possiblyAffectedPrimitives, Way.class);
if (!ways.isEmpty()) {
DataSet ds = this.getAffectedDataSet();
Layer toSwitch = MainApplication.getLayerManager().getLayersOfType(AbstractOsmDataLayer.class).stream()
.filter(d -> ds.equals(d.getDataSet())).findAny().orElse(null);
if (toSwitch != null) {
MainApplication.getLayerManager().setActiveLayer(toSwitch);
}
}
Collection<Command> commands = new ArrayList<>();
for (Way way : ways) {
Command command = SimplifyWayAction.createSimplifyCommand(way, threshold);
if (command == null) {
continue;
}
int count = Utils.filteredCollection(new ArrayList<>(command.getParticipatingPrimitives()), Node.class)
.size();
if ((count / (double) way.getNodesCount()) * 100 > this.acceptableRemovalPercentage) {
AutoScaleAction.zoomTo(Collections.singleton(way));
double length = SimplifyWayAction.askSimplifyWays(
tr("You are about to simplify {0} way with a total length of {1}.", 1, way.getLength()), true);
command = SimplifyWayAction.createSimplifyCommand(way, length);
if (command != null) {
commands.add(command);
}
}
}
if (current != null) {
MainApplication.getLayerManager().setActiveLayer(current);
}
if (commands.size() == 1) {
return commands.iterator().next();
} else if (!commands.isEmpty()) {
return new SequenceCommand(tr("Simplify ways"), commands);
}
return null;
return new PostponedOverNodedWayCommand(this.getAffectedDataSet());
}
@Override

Wyświetl plik

@ -8,6 +8,7 @@ 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 java.util.Collections;
import java.util.concurrent.Future;
import org.awaitility.Awaitility;
@ -26,11 +27,14 @@ import org.openstreetmap.josm.gui.MainApplication;
import org.openstreetmap.josm.gui.Notification;
import org.openstreetmap.josm.gui.layer.OsmDataLayer;
import org.openstreetmap.josm.gui.util.GuiHelper;
import org.openstreetmap.josm.plugins.mapwithai.backend.commands.conflation.ConnectedCommand;
import org.openstreetmap.josm.plugins.mapwithai.backend.commands.conflation.DuplicateCommand;
import org.openstreetmap.josm.plugins.mapwithai.commands.ConnectedCommand;
import org.openstreetmap.josm.plugins.mapwithai.commands.DuplicateCommand;
import org.openstreetmap.josm.plugins.mapwithai.testutils.MapWithAITestRules;
import org.openstreetmap.josm.plugins.mapwithai.testutils.MissingConnectionTagsMocker;
import org.openstreetmap.josm.spi.preferences.Config;
import org.openstreetmap.josm.testutils.JOSMTestRules;
import org.openstreetmap.josm.testutils.mockers.WindowMocker;
import org.openstreetmap.josm.tools.Logging;
import org.openstreetmap.josm.tools.Territories;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
@ -46,7 +50,8 @@ public class MapWithAIMoveActionTest {
@Rule
@SuppressFBWarnings("URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
public JOSMTestRules test = new JOSMTestRules().preferences().main().projection().territories();
public JOSMTestRules test = new MapWithAITestRules().wiremock().preferences().main().projection().territories()
.assertionsInEDT();
@Before
public void setUp() {
@ -205,4 +210,54 @@ public class MapWithAIMoveActionTest {
assertDoesNotThrow(() -> UndoRedoHandler.getInstance().undo());
}
}
@Test
public void testAddSimplifiedWay() {
Node ma1 = new Node(new LatLon(39.1210737, -108.6162804));
Node ma2 = new Node(new LatLon(39.1210363, -108.6162804));
Node ma3 = new Node(new LatLon(39.1210196, -108.6162804));
Node ma4 = new Node(new LatLon(39.1209364, -108.6162804));
Node ma5 = new Node(new LatLon(39.1208031, -108.6163033));
ma5.put("dupe", "n7041074564");
Way ma1w = TestUtils.newWay("highway=residential mapwithai:source=MapWithAI source=digitalglobe", ma1, ma2, ma3,
ma4, ma5);
Node osm1 = new Node(new LatLon(39.1208025, -108.6173585));
osm1.setOsmId(176255324L, 5);
Node osm2 = TestUtils.newNode("maxspeed=\"35 mph\" traffic_sign:forward=yes traffic_sign=maxspeed");
osm2.setCoor(new LatLon(39.1208031, -108.6163033));
osm2.setOsmId(7041074564L, 1);
Node osm3 = TestUtils.newNode("crossing=uncontrolled highway=crossing");
osm3.setCoor(new LatLon(39.1208035, -108.6155962));
osm3.setOsmId(7050673431L, 1);
Way osm1w = TestUtils.newWay(
"highway=unclassified lanes=2 maxspeed=\"35 mph\" name=\"H Road\" ref=H surface=asphalt", osm1, osm2,
osm3);
osm1w.setOsmId(753597166L, 6);
DataSet osm = new DataSet();
osm1w.getNodes().forEach(osm::addPrimitive);
osm.addPrimitive(osm1w);
DataSet mapwithai = new DataSet();
ma1w.getNodes().forEach(mapwithai::addPrimitive);
mapwithai.addPrimitive(ma1w);
OsmDataLayer osmDataLayer = new OsmDataLayer(osm, "OSM Layer", null);
MapWithAILayer mapwithaiLayer = new MapWithAILayer(mapwithai, "MapWithAI", null);
MainApplication.getLayerManager().addLayer(osmDataLayer);
MainApplication.getLayerManager().addLayer(mapwithaiLayer);
MainApplication.getLayerManager().setActiveLayer(mapwithaiLayer);
Territories.initialize();
mapwithai.setSelected(Collections.singleton(ma1w));
Config.getPref().put("simplify-way.auto.remember", "yes");
Logging.clearLastErrorAndWarnings();
assertDoesNotThrow(() -> this.moveAction.actionPerformed(null));
assertTrue(Logging.getLastErrorAndWarnings().isEmpty(),
Logging.getLastErrorAndWarnings().isEmpty() ? "" : Logging.getLastErrorAndWarnings().get(0));
assertTrue(osm.allPrimitives().size() > 4);
assertTrue(osm.allPrimitives().stream().anyMatch(p -> p.hasTag("source", "digitalglobe")));
}
}