Index: /applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/actions/SplitRoundaboutAction.java =================================================================== --- /applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/actions/SplitRoundaboutAction.java (revision 33403) +++ /applications/editors/josm/plugins/pt_assistant/src/org/openstreetmap/josm/plugins/pt_assistant/actions/SplitRoundaboutAction.java (revision 33404) @@ -4,8 +4,11 @@ import java.awt.event.ActionEvent; +import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; @@ -33,5 +36,4 @@ private static final String actionName = "Split Roundabout"; - private static final long serialVersionUID = 8912249304286025356L; @@ -42,4 +44,5 @@ @Override public void actionPerformed(ActionEvent e) { + Way roundabout = (Way) getLayerManager().getEditDataSet().getSelected().iterator().next(); @@ -70,10 +73,9 @@ { //make the roundabout round, if requested - int result = JOptionPane.showOptionDialog(Main.parent, + if(Main.pref.getBoolean("pt_assistant.roundabout-splitter.alignalways") || + JOptionPane.YES_OPTION == JOptionPane.showOptionDialog(Main.parent, tr("Do you want to make the roundabout round?"), tr("Roundabout round"), JOptionPane.YES_NO_OPTION, JOptionPane.QUESTION_MESSAGE, - null, null, null); - - if(result == JOptionPane.YES_OPTION) { + null, null, null)) { new AlignInCircleAction().actionPerformed(null); } @@ -86,7 +88,12 @@ getLayerManager().getEditDataSet().setSelected(splitNodes); new SplitWayAction().actionPerformed(null); - Collection splitWays = getLayerManager().getEditDataSet().getSelected(); + Collection splitWays = getLayerManager().getEditDataSet().getSelectedWays(); //update the relations. + updateRelations(savedPositions, splitNodes, splitWays); + } + + public void updateRelations(Map savedPositions, + List splitNodes, Collection splitWays) { savedPositions.forEach((r, i) -> { Way previous = r.getMember(i-1).getWay(); @@ -137,6 +144,8 @@ //split only on the nodes which might be the //entry or exit point for some public transport route - private List getSplitNodes(Way roundabout) { - List splitNodes = roundabout.getNodes(); + public List getSplitNodes(Way roundabout) { + Set noDuplicateSplitNodes = new HashSet<>(roundabout.getNodes()); + List splitNodes = new ArrayList<>(noDuplicateSplitNodes); + splitNodes.removeIf(n -> { List parents = n.getParentWays(); @@ -159,5 +168,5 @@ //save the position of the roundabout inside each public transport route //it is contained in - private Map getSavedPositions(Way roundabout) { + public Map getSavedPositions(Way roundabout) { Map savedPositions = new HashMap<>(); Index: /applications/editors/josm/plugins/pt_assistant/test/data/roundabout.osm =================================================================== --- /applications/editors/josm/plugins/pt_assistant/test/data/roundabout.osm (revision 33404) +++ /applications/editors/josm/plugins/pt_assistant/test/data/roundabout.osm (revision 33404) @@ -0,0 +1,4639 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Index: /applications/editors/josm/plugins/pt_assistant/test/unit/org/openstreetmap/josm/plugins/pt_assistant/AbstractTest.java =================================================================== --- /applications/editors/josm/plugins/pt_assistant/test/unit/org/openstreetmap/josm/plugins/pt_assistant/AbstractTest.java (revision 33403) +++ /applications/editors/josm/plugins/pt_assistant/test/unit/org/openstreetmap/josm/plugins/pt_assistant/AbstractTest.java (revision 33404) @@ -37,4 +37,5 @@ public static final String PATH_TO_PLATFORM_AS_WAY = "test/data/route-with-platform-as-way.osm"; + public static final String PATH_TO_ROUNDABOUT = "test/data/roundabout.osm"; public static final String PATH_TO_ROUNDABOUT_ONEWAY = "test/data/duesseldorf_roundabout.osm"; @@ -55,5 +56,4 @@ public static final String PATH_TO_STOP_AREA_MANY_PLATFORMS = "test/data/stop-area-many-platforms.osm"; - public static final String PATH_TO_SEGMENT_TEST = "test/data/segment-test.osm"; Index: /applications/editors/josm/plugins/pt_assistant/test/unit/org/openstreetmap/josm/plugins/pt_assistant/actions/SplitRoundaboutTest.java =================================================================== --- /applications/editors/josm/plugins/pt_assistant/test/unit/org/openstreetmap/josm/plugins/pt_assistant/actions/SplitRoundaboutTest.java (revision 33404) +++ /applications/editors/josm/plugins/pt_assistant/test/unit/org/openstreetmap/josm/plugins/pt_assistant/actions/SplitRoundaboutTest.java (revision 33404) @@ -0,0 +1,135 @@ +package org.openstreetmap.josm.plugins.pt_assistant.actions; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.util.Collection; +import java.util.List; +import java.util.Map; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.openstreetmap.josm.JOSMFixture; +import org.openstreetmap.josm.Main; +import org.openstreetmap.josm.actions.SplitWayAction; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.Node; +import org.openstreetmap.josm.data.osm.OsmPrimitiveType; +import org.openstreetmap.josm.data.osm.Relation; +import org.openstreetmap.josm.data.osm.SimplePrimitiveId; +import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.gui.layer.OsmDataLayer; +import org.openstreetmap.josm.io.IllegalDataException; +import org.openstreetmap.josm.io.OsmReader; +import org.openstreetmap.josm.plugins.pt_assistant.AbstractTest; + +public class SplitRoundaboutTest extends AbstractTest { + + DataSet ds; + OsmDataLayer layer; + SplitRoundaboutAction action; + SplitWayAction swa; + Way r1, r2, r3; + + @Before + public void init() throws FileNotFoundException, IllegalDataException { + + new JOSMFixture("../../JOSM/test/config/unit-josm.home").init(true); + + ds = OsmReader.parseDataSet(new FileInputStream(AbstractTest.PATH_TO_ROUNDABOUT), null); + layer = new OsmDataLayer(ds, OsmDataLayer.createNewName(), null); + Main.getLayerManager().addLayer(layer); + + Main.pref.put("pt_assistant.roundabout-splitter.alignalways", true); + action = new SplitRoundaboutAction(); + swa = Main.main.menu.splitWay; + swa.setEnabled(true); + r1 = (Way) ds.getPrimitiveById(new SimplePrimitiveId(293302077L, OsmPrimitiveType.WAY)); + r2 = (Way) ds.getPrimitiveById(new SimplePrimitiveId(205833435L, OsmPrimitiveType.WAY)); + r3 = (Way) ds.getPrimitiveById(new SimplePrimitiveId(25739002L, OsmPrimitiveType.WAY)); + } + + @Test + public void test1() { + + Map savedPositions = action.getSavedPositions(r1); + List splitNodes = action.getSplitNodes(r1); + assertEquals(4, splitNodes.size()); + ds.setSelected(splitNodes); + swa.actionPerformed(null); + + Collection splitWays = ds.getSelectedWays(); + action.updateRelations(savedPositions, splitNodes, splitWays); + + splitWays.forEach(w -> { + if (w.firstNode().getUniqueId() == 267843779L && w.lastNode().getUniqueId() == 2968718407L) + assertEquals(w.getReferrers().size(), 5); + else if (w.firstNode().getUniqueId() == 2968718407L && w.lastNode().getUniqueId() == 2383688231L) + assertEquals(w.getReferrers().size(), 0); + else if (w.firstNode().getUniqueId() == 2383688231L && w.lastNode().getUniqueId() == 267843741L) + assertEquals(w.getReferrers().size(), 5); + else if (w.firstNode().getUniqueId() == 267843741L && w.lastNode().getUniqueId() == 267843779L) + assertEquals(w.getReferrers().size(), 0); + else + fail(); + }); + } + + @Test + public void test2() { + Map savedPositions = action.getSavedPositions(r2); + List splitNodes = action.getSplitNodes(r2); + assertEquals(4, splitNodes.size()); + ds.setSelected(splitNodes); + swa.actionPerformed(null); + + Collection splitWays = ds.getSelectedWays(); + action.updateRelations(savedPositions, splitNodes, splitWays); + + splitWays.forEach(w -> { + if(w.firstNode().getUniqueId() == 2158181809L && w.lastNode().getUniqueId() == 2158181798L) + assertEquals(w.getReferrers().size(), 8); + else if (w.firstNode().getUniqueId() == 2158181798L && w.lastNode().getUniqueId() == 2158181789L) + assertEquals(w.getReferrers().size(), 0); + else if (w.firstNode().getUniqueId() == 2158181789L && w.lastNode().getUniqueId() == 2158181803L) + assertEquals(w.getReferrers().size(), 8); + else if (w.firstNode().getUniqueId() == 2158181803L && w.lastNode().getUniqueId() == 2158181809L) + assertEquals(w.getReferrers().size(), 0); + else + fail(); + }); + } + + @Test + public void test3() { + Map savedPositions = action.getSavedPositions(r3); + List splitNodes = action.getSplitNodes(r3); + assertEquals(4, splitNodes.size()); + ds.setSelected(splitNodes); + swa.actionPerformed(null); + + Collection splitWays = ds.getSelectedWays(); + action.updateRelations(savedPositions, splitNodes, splitWays); + + splitWays.forEach(w -> { + if(w.firstNode().getUniqueId() == 280697532L && w.lastNode().getUniqueId() == 280697452L) + assertEquals(w.getReferrers().size(), 0); + else if (w.firstNode().getUniqueId() == 280697452L && w.lastNode().getUniqueId() == 280697591L) + assertEquals(w.getReferrers().size(), 2); + else if (w.firstNode().getUniqueId() == 280697591L && w.lastNode().getUniqueId() == 280697534L) + assertEquals(w.getReferrers().size(), 0); + else if (w.firstNode().getUniqueId() == 280697534L && w.lastNode().getUniqueId() == 280697532L) + assertEquals(w.getReferrers().size(), 1); + else + fail(); + }); + } + + @After + public void end() { + Main.getLayerManager().removeLayer(layer); + } +}