Index: /trunk/src/org/openstreetmap/josm/actions/JoinAreasAction.java =================================================================== --- /trunk/src/org/openstreetmap/josm/actions/JoinAreasAction.java (revision 12827) +++ /trunk/src/org/openstreetmap/josm/actions/JoinAreasAction.java (revision 12828) @@ -25,5 +25,4 @@ import org.openstreetmap.josm.Main; import org.openstreetmap.josm.actions.ReverseWayAction.ReverseWayResult; -import org.openstreetmap.josm.actions.SplitWayAction.SplitWayResult; import org.openstreetmap.josm.command.AddCommand; import org.openstreetmap.josm.command.ChangeCommand; @@ -31,4 +30,5 @@ import org.openstreetmap.josm.command.DeleteCommand; import org.openstreetmap.josm.command.SequenceCommand; +import org.openstreetmap.josm.command.SplitWayCommand; import org.openstreetmap.josm.data.UndoRedoHandler; import org.openstreetmap.josm.data.coor.EastNorth; @@ -1019,10 +1019,10 @@ if (chunks.size() > 1) { - SplitWayResult split = SplitWayAction.splitWay(way, chunks, - Collections.emptyList(), SplitWayAction.Strategy.keepFirstChunk()); + SplitWayCommand split = SplitWayCommand.splitWay(way, chunks, + Collections.emptyList(), SplitWayCommand.Strategy.keepFirstChunk()); if (split != null) { //execute the command, we need the results - cmds.add(split.getCommand()); + cmds.add(split); commitCommands(marktr("Split ways into fragments")); Index: /trunk/src/org/openstreetmap/josm/actions/SplitWayAction.java =================================================================== --- /trunk/src/org/openstreetmap/josm/actions/SplitWayAction.java (revision 12827) +++ /trunk/src/org/openstreetmap/josm/actions/SplitWayAction.java (revision 12828) @@ -11,13 +11,8 @@ import java.awt.event.KeyEvent; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.HashSet; import java.util.Iterator; -import java.util.LinkedList; import java.util.List; -import java.util.Optional; -import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; @@ -30,8 +25,6 @@ import org.openstreetmap.josm.Main; -import org.openstreetmap.josm.command.AddCommand; -import org.openstreetmap.josm.command.ChangeCommand; import org.openstreetmap.josm.command.Command; -import org.openstreetmap.josm.command.SequenceCommand; +import org.openstreetmap.josm.command.SplitWayCommand; import org.openstreetmap.josm.data.osm.DefaultNameFormatter; import org.openstreetmap.josm.data.osm.Node; @@ -39,5 +32,4 @@ import org.openstreetmap.josm.data.osm.PrimitiveId; import org.openstreetmap.josm.data.osm.Relation; -import org.openstreetmap.josm.data.osm.RelationMember; import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.data.osm.WaySegment; @@ -47,5 +39,4 @@ import org.openstreetmap.josm.gui.Notification; import org.openstreetmap.josm.gui.layer.OsmDataLayer; -import org.openstreetmap.josm.tools.CheckParameterUtil; import org.openstreetmap.josm.tools.GBC; import org.openstreetmap.josm.tools.Shortcut; @@ -63,5 +54,7 @@ * @see SplitWayAction#splitWay * @see SplitWayAction#split - */ + * @deprecated To be removed end of 2017. Use {@link SplitWayCommand} instead + */ + @Deprecated public static class SplitWayResult { private final Command command; @@ -81,4 +74,15 @@ this.originalWay = originalWay; this.newWays = newWays; + } + + /** + * @param command The command to be performed to split the way (which is saved for later retrieval with {@link #getCommand}) + * @since 12828 + */ + protected SplitWayResult(SplitWayCommand command) { + this.command = command; + this.newSelection = command.getNewSelection(); + this.originalWay = command.getOriginalWay(); + this.newWays = command.getNewWays(); } @@ -194,5 +198,5 @@ // Finally, applicableWays contains only one perfect way final Way selectedWay = applicableWays.get(0); - final List> wayChunks = buildSplitChunks(selectedWay, selectedNodes); + final List> wayChunks = SplitWayCommand.buildSplitChunks(selectedWay, selectedNodes); if (wayChunks != null) { List selectedRelations = OsmPrimitive.getFilteredList(selection, Relation.class); @@ -202,5 +206,5 @@ final List newWays = createNewWaysFromChunks(selectedWay, wayChunks); - final Way wayToKeep = Strategy.keepLongestChunk().determineWayToKeep(newWays); + final Way wayToKeep = SplitWayCommand.Strategy.keepLongestChunk().determineWayToKeep(newWays); if (ExpertToggleAction.isExpert() && !selectedWay.isNew()) { @@ -214,9 +218,5 @@ } if (wayToKeep != null) { - final SplitWayResult result = doSplitWay(selectedWay, wayToKeep, newWays, sel); - MainApplication.undoRedo.add(result.getCommand()); - if (!result.getNewSelection().isEmpty()) { - getLayerManager().getEditDataSet().setSelected(result.getNewSelection()); - } + doSplitWay(selectedWay, wayToKeep, newWays, sel); } } @@ -294,9 +294,5 @@ toggleSaveState(); // necessary since #showDialog() does not handle it due to the non-modal dialog if (getValue() == 1) { - SplitWayResult result = doSplitWay(selectedWay, list.getSelectedValue(), newWays, selection); - MainApplication.undoRedo.add(result.getCommand()); - if (!result.getNewSelection().isEmpty()) { - MainApplication.getLayerManager().getEditDataSet().setSelected(result.getNewSelection()); - } + doSplitWay(selectedWay, list.getSelectedValue(), newWays, selection); } } @@ -322,5 +318,7 @@ * @since 8954 * @since 10599 (functional interface) - */ + * @deprecated to be removed end of 2017. Use {@link org.openstreetmap.josm.command.SplitWayCommand.Strategy} instead + */ + @Deprecated @FunctionalInterface public interface Strategy { @@ -339,13 +337,5 @@ */ static Strategy keepLongestChunk() { - return wayChunks -> { - Way wayToKeep = null; - for (Way i : wayChunks) { - if (wayToKeep == null || i.getNodesCount() > wayToKeep.getNodesCount()) { - wayToKeep = i; - } - } - return wayToKeep; - }; + return SplitWayCommand.Strategy.keepLongestChunk()::determineWayToKeep; } @@ -355,5 +345,5 @@ */ static Strategy keepFirstChunk() { - return wayChunks -> wayChunks.iterator().next(); + return SplitWayCommand.Strategy.keepFirstChunk()::determineWayToKeep; } } @@ -406,63 +396,9 @@ * @param splitPoints the nodes where the way is split. Must not be null. * @return the list of chunks - */ + * @deprecated To be removed end of 2017. Use {@link SplitWayCommand#buildSplitChunks} instead + */ + @Deprecated public static List> buildSplitChunks(Way wayToSplit, List splitPoints) { - CheckParameterUtil.ensureParameterNotNull(wayToSplit, "wayToSplit"); - CheckParameterUtil.ensureParameterNotNull(splitPoints, "splitPoints"); - - Set nodeSet = new HashSet<>(splitPoints); - List> wayChunks = new LinkedList<>(); - List currentWayChunk = new ArrayList<>(); - wayChunks.add(currentWayChunk); - - Iterator it = wayToSplit.getNodes().iterator(); - while (it.hasNext()) { - Node currentNode = it.next(); - boolean atEndOfWay = currentWayChunk.isEmpty() || !it.hasNext(); - currentWayChunk.add(currentNode); - if (nodeSet.contains(currentNode) && !atEndOfWay) { - currentWayChunk = new ArrayList<>(); - currentWayChunk.add(currentNode); - wayChunks.add(currentWayChunk); - } - } - - // Handle circular ways specially. - // If you split at a circular way at two nodes, you just want to split - // it at these points, not also at the former endpoint. - // So if the last node is the same first node, join the last and the - // first way chunk. - List lastWayChunk = wayChunks.get(wayChunks.size() - 1); - if (wayChunks.size() >= 2 - && wayChunks.get(0).get(0) == lastWayChunk.get(lastWayChunk.size() - 1) - && !nodeSet.contains(wayChunks.get(0).get(0))) { - if (wayChunks.size() == 2) { - new Notification( - tr("You must select two or more nodes to split a circular way.")) - .setIcon(JOptionPane.WARNING_MESSAGE) - .show(); - return null; - } - lastWayChunk.remove(lastWayChunk.size() - 1); - lastWayChunk.addAll(wayChunks.get(0)); - wayChunks.remove(wayChunks.size() - 1); - wayChunks.set(0, lastWayChunk); - } - - if (wayChunks.size() < 2) { - if (wayChunks.get(0).get(0) == wayChunks.get(0).get(wayChunks.get(0).size() - 1)) { - new Notification( - tr("You must select two or more nodes to split a circular way.")) - .setIcon(JOptionPane.WARNING_MESSAGE) - .show(); - } else { - new Notification( - tr("The way cannot be split at the selected nodes. (Hint: Select nodes in the middle of the way.)")) - .setIcon(JOptionPane.WARNING_MESSAGE) - .show(); - } - return null; - } - return wayChunks; + return SplitWayCommand.buildSplitChunks(wayToSplit, splitPoints); } @@ -472,14 +408,9 @@ * @param wayChunks the way chunks * @return the new way objects - */ + * @deprecated To be removed end of 2017. Use {@link SplitWayCommand#createNewWaysFromChunks} instead + */ + @Deprecated protected static List createNewWaysFromChunks(Way way, Iterable> wayChunks) { - final List newWays = new ArrayList<>(); - for (List wayChunk : wayChunks) { - Way wayToAdd = new Way(); - wayToAdd.setKeys(way.getKeys()); - wayToAdd.setNodes(wayChunk); - newWays.add(wayToAdd); - } - return newWays; + return SplitWayCommand.createNewWaysFromChunks(way, wayChunks); } @@ -518,5 +449,7 @@ * @return the result from the split operation * @since 12718 - */ + * @deprecated to be removed end of 2017. Use {@link #splitWay(Way, List, Collection)} instead + */ + @Deprecated public static SplitWayResult splitWay(Way way, List> wayChunks, Collection selection) { @@ -565,206 +498,22 @@ * @return the result from the split operation * @since 12718 - */ + * @deprecated to be removed end of 2017. Use {@link SplitWayCommand#splitWay} instead + */ + @Deprecated public static SplitWayResult splitWay(Way way, List> wayChunks, Collection selection, Strategy splitStrategy) { - // build a list of commands, and also a new selection list - final List newSelection = new ArrayList<>(selection.size() + wayChunks.size()); - newSelection.addAll(selection); - - // Create all potential new ways - final List newWays = createNewWaysFromChunks(way, wayChunks); - - // Determine which part reuses the existing way - final Way wayToKeep = splitStrategy.determineWayToKeep(newWays); - - return wayToKeep != null ? doSplitWay(way, wayToKeep, newWays, newSelection) : null; - } - - static SplitWayResult doSplitWay(Way way, Way wayToKeep, List newWays, List newSelection) { - - Collection commandList = new ArrayList<>(newWays.size()); - Collection nowarnroles = Main.pref.getCollection("way.split.roles.nowarn", - Arrays.asList("outer", "inner", "forward", "backward", "north", "south", "east", "west")); - + SplitWayCommand cmd = SplitWayCommand.splitWay(way, wayChunks, selection, x -> splitStrategy.determineWayToKeep(x)); + return cmd != null ? new SplitWayResult(cmd) : null; + } + + static void doSplitWay(Way way, Way wayToKeep, List newWays, List newSelection) { final MapFrame map = MainApplication.getMap(); final boolean isMapModeDraw = map != null && map.mapMode == map.mapModeDraw; - - // Change the original way - final Way changedWay = new Way(way); - changedWay.setNodes(wayToKeep.getNodes()); - commandList.add(new ChangeCommand(way, changedWay)); - if (!isMapModeDraw && !newSelection.contains(way)) { - newSelection.add(way); - } - final int indexOfWayToKeep = newWays.indexOf(wayToKeep); - newWays.remove(wayToKeep); - - if (!isMapModeDraw) { - newSelection.addAll(newWays); - } - for (Way wayToAdd : newWays) { - commandList.add(new AddCommand(way.getDataSet(), wayToAdd)); - } - - boolean warnmerole = false; - boolean warnme = false; - // now copy all relations to new way also - - for (Relation r : OsmPrimitive.getFilteredList(way.getReferrers(), Relation.class)) { - if (!r.isUsable()) { - continue; - } - Relation c = null; - String type = Optional.ofNullable(r.get("type")).orElse(""); - - int ic = 0; - int ir = 0; - List relationMembers = r.getMembers(); - for (RelationMember rm: relationMembers) { - if (rm.isWay() && rm.getMember() == way) { - boolean insert = true; - if ("restriction".equals(type) || "destination_sign".equals(type)) { - /* this code assumes the restriction is correct. No real error checking done */ - String role = rm.getRole(); - if ("from".equals(role) || "to".equals(role)) { - OsmPrimitive via = findVia(r, type); - List nodes = new ArrayList<>(); - if (via != null) { - if (via instanceof Node) { - nodes.add((Node) via); - } else if (via instanceof Way) { - nodes.add(((Way) via).lastNode()); - nodes.add(((Way) via).firstNode()); - } - } - Way res = null; - for (Node n : nodes) { - if (changedWay.isFirstLastNode(n)) { - res = way; - } - } - if (res == null) { - for (Way wayToAdd : newWays) { - for (Node n : nodes) { - if (wayToAdd.isFirstLastNode(n)) { - res = wayToAdd; - } - } - } - if (res != null) { - if (c == null) { - c = new Relation(r); - } - c.addMember(new RelationMember(role, res)); - c.removeMembersFor(way); - insert = false; - } - } else { - insert = false; - } - } else if (!"via".equals(role)) { - warnme = true; - } - } else if (!("route".equals(type)) && !("multipolygon".equals(type))) { - warnme = true; - } - if (c == null) { - c = new Relation(r); - } - - if (insert) { - if (rm.hasRole() && !nowarnroles.contains(rm.getRole())) { - warnmerole = true; - } - - Boolean backwards = null; - int k = 1; - while (ir - k >= 0 || ir + k < relationMembers.size()) { - if ((ir - k >= 0) && relationMembers.get(ir - k).isWay()) { - Way w = relationMembers.get(ir - k).getWay(); - if ((w.lastNode() == way.firstNode()) || w.firstNode() == way.firstNode()) { - backwards = Boolean.FALSE; - } else if ((w.firstNode() == way.lastNode()) || w.lastNode() == way.lastNode()) { - backwards = Boolean.TRUE; - } - break; - } - if ((ir + k < relationMembers.size()) && relationMembers.get(ir + k).isWay()) { - Way w = relationMembers.get(ir + k).getWay(); - if ((w.lastNode() == way.firstNode()) || w.firstNode() == way.firstNode()) { - backwards = Boolean.TRUE; - } else if ((w.firstNode() == way.lastNode()) || w.lastNode() == way.lastNode()) { - backwards = Boolean.FALSE; - } - break; - } - k++; - } - - int j = ic; - final List waysToAddBefore = newWays.subList(0, indexOfWayToKeep); - for (Way wayToAdd : waysToAddBefore) { - RelationMember em = new RelationMember(rm.getRole(), wayToAdd); - j++; - if (Boolean.TRUE.equals(backwards)) { - c.addMember(ic + 1, em); - } else { - c.addMember(j - 1, em); - } - } - final List waysToAddAfter = newWays.subList(indexOfWayToKeep, newWays.size()); - for (Way wayToAdd : waysToAddAfter) { - RelationMember em = new RelationMember(rm.getRole(), wayToAdd); - j++; - if (Boolean.TRUE.equals(backwards)) { - c.addMember(ic, em); - } else { - c.addMember(j, em); - } - } - ic = j; - } - } - ic++; - ir++; - } - - if (c != null) { - commandList.add(new ChangeCommand(r.getDataSet(), r, c)); - } - } - if (warnmerole) { - new Notification( - tr("A role based relation membership was copied to all new ways.
You should verify this and correct it when necessary.")) - .setIcon(JOptionPane.WARNING_MESSAGE) - .show(); - } else if (warnme) { - new Notification( - tr("A relation membership was copied to all new ways.
You should verify this and correct it when necessary.")) - .setIcon(JOptionPane.WARNING_MESSAGE) - .show(); - } - - return new SplitWayResult( - new SequenceCommand( - /* for correct i18n of plural forms - see #9110 */ - trn("Split way {0} into {1} part", "Split way {0} into {1} parts", newWays.size() + 1, - way.getDisplayName(DefaultNameFormatter.getInstance()), newWays.size() + 1), - commandList - ), - newSelection, - way, - newWays - ); - } - - static OsmPrimitive findVia(Relation r, String type) { - for (RelationMember rmv : r.getMembers()) { - if (("restriction".equals(type) && "via".equals(rmv.getRole())) - || ("destination_sign".equals(type) && rmv.hasRole("sign", "intersection"))) { - return rmv.getMember(); - } - } - return null; + final SplitWayCommand result = SplitWayCommand.doSplitWay(way, wayToKeep, newWays, !isMapModeDraw ? newSelection : null); + MainApplication.undoRedo.add(result); + List newSel = result.getNewSelection(); + if (newSel != null && !newSel.isEmpty()) { + MainApplication.getLayerManager().getEditDataSet().setSelected(newSel); + } } @@ -806,5 +555,7 @@ * @return the result from the split operation * @since 12718 - */ + * @deprecated to be removed end of 2017. Use {@link #splitWay(Way, List, Collection)} instead + */ + @Deprecated public static SplitWayResult split(Way way, List atNodes, Collection selection) { List> chunks = buildSplitChunks(way, atNodes); Index: /trunk/src/org/openstreetmap/josm/command/DeleteCommand.java =================================================================== --- /trunk/src/org/openstreetmap/josm/command/DeleteCommand.java (revision 12827) +++ /trunk/src/org/openstreetmap/josm/command/DeleteCommand.java (revision 12828) @@ -22,6 +22,4 @@ import javax.swing.Icon; -import org.openstreetmap.josm.actions.SplitWayAction; -import org.openstreetmap.josm.actions.SplitWayAction.SplitWayResult; import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.DefaultNameFormatter; @@ -659,6 +657,5 @@ return new ChangeCommand(ws.way, wnew); } else { - SplitWayResult split = SplitWayAction.splitWay(ws.way, Arrays.asList(n1, n2), Collections.emptyList()); - return split != null ? split.getCommand() : null; + return SplitWayCommand.splitWay(ws.way, Arrays.asList(n1, n2), Collections.emptyList()); } } Index: /trunk/src/org/openstreetmap/josm/command/SplitWayCommand.java =================================================================== --- /trunk/src/org/openstreetmap/josm/command/SplitWayCommand.java (revision 12828) +++ /trunk/src/org/openstreetmap/josm/command/SplitWayCommand.java (revision 12828) @@ -0,0 +1,472 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.command; + +import static org.openstreetmap.josm.tools.I18n.tr; +import static org.openstreetmap.josm.tools.I18n.trn; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.function.Consumer; + +import org.openstreetmap.josm.Main; +import org.openstreetmap.josm.data.osm.DefaultNameFormatter; +import org.openstreetmap.josm.data.osm.Node; +import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.PrimitiveId; +import org.openstreetmap.josm.data.osm.Relation; +import org.openstreetmap.josm.data.osm.RelationMember; +import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.tools.CheckParameterUtil; +import org.openstreetmap.josm.tools.Logging; + +/** + * Splits a way into multiple ways (all identical except for their node list). + * + * Ways are just split at the selected nodes. The nodes remain in their + * original order. Selected nodes at the end of a way are ignored. + * + * @since 12828 ({@code SplitWayAction} converted to a {@link Command}) + */ +public class SplitWayCommand extends SequenceCommand { + + private static Consumer warningNotifier = msg -> Logging.warn(msg); + + /** + * Sets the global warning notifier. + * @param notifier warning notifier in charge of displaying warning message, if any. Must not be null + */ + public static void setWarningNotifier(Consumer notifier) { + warningNotifier = Objects.requireNonNull(notifier); + } + + private final List newSelection; + private final Way originalWay; + private final List newWays; + + /** + * Create a new {@code SplitWayCommand}. + * @param name The description text + * @param commandList The sequence of commands that should be executed. + * @param newSelection The new list of selected primitives ids (which is saved for later retrieval with {@link #getNewSelection}) + * @param originalWay The original way being split (which is saved for later retrieval with {@link #getOriginalWay}) + * @param newWays The resulting new ways (which is saved for later retrieval with {@link #getOriginalWay}) + */ + public SplitWayCommand(String name, Collection commandList, + List newSelection, Way originalWay, List newWays) { + super(name, commandList); + this.newSelection = newSelection; + this.originalWay = originalWay; + this.newWays = newWays; + } + + /** + * Replies the new list of selected primitives ids + * @return The new list of selected primitives ids + */ + public List getNewSelection() { + return newSelection; + } + + /** + * Replies the original way being split + * @return The original way being split + */ + public Way getOriginalWay() { + return originalWay; + } + + /** + * Replies the resulting new ways + * @return The resulting new ways + */ + public List getNewWays() { + return newWays; + } + + /** + * Determines which way chunk should reuse the old id and its history + */ + @FunctionalInterface + public interface Strategy { + + /** + * Determines which way chunk should reuse the old id and its history. + * + * @param wayChunks the way chunks + * @return the way to keep + */ + Way determineWayToKeep(Iterable wayChunks); + + /** + * Returns a strategy which selects the way chunk with the highest node count to keep. + * @return strategy which selects the way chunk with the highest node count to keep + */ + static Strategy keepLongestChunk() { + return wayChunks -> { + Way wayToKeep = null; + for (Way i : wayChunks) { + if (wayToKeep == null || i.getNodesCount() > wayToKeep.getNodesCount()) { + wayToKeep = i; + } + } + return wayToKeep; + }; + } + + /** + * Returns a strategy which selects the first way chunk. + * @return strategy which selects the first way chunk + */ + static Strategy keepFirstChunk() { + return wayChunks -> wayChunks.iterator().next(); + } + } + + /** + * Splits the nodes of {@code wayToSplit} into a list of node sequences + * which are separated at the nodes in {@code splitPoints}. + * + * This method displays warning messages if {@code wayToSplit} and/or + * {@code splitPoints} aren't consistent. + * + * Returns null, if building the split chunks fails. + * + * @param wayToSplit the way to split. Must not be null. + * @param splitPoints the nodes where the way is split. Must not be null. + * @return the list of chunks + */ + public static List> buildSplitChunks(Way wayToSplit, List splitPoints) { + CheckParameterUtil.ensureParameterNotNull(wayToSplit, "wayToSplit"); + CheckParameterUtil.ensureParameterNotNull(splitPoints, "splitPoints"); + + Set nodeSet = new HashSet<>(splitPoints); + List> wayChunks = new LinkedList<>(); + List currentWayChunk = new ArrayList<>(); + wayChunks.add(currentWayChunk); + + Iterator it = wayToSplit.getNodes().iterator(); + while (it.hasNext()) { + Node currentNode = it.next(); + boolean atEndOfWay = currentWayChunk.isEmpty() || !it.hasNext(); + currentWayChunk.add(currentNode); + if (nodeSet.contains(currentNode) && !atEndOfWay) { + currentWayChunk = new ArrayList<>(); + currentWayChunk.add(currentNode); + wayChunks.add(currentWayChunk); + } + } + + // Handle circular ways specially. + // If you split at a circular way at two nodes, you just want to split + // it at these points, not also at the former endpoint. + // So if the last node is the same first node, join the last and the + // first way chunk. + List lastWayChunk = wayChunks.get(wayChunks.size() - 1); + if (wayChunks.size() >= 2 + && wayChunks.get(0).get(0) == lastWayChunk.get(lastWayChunk.size() - 1) + && !nodeSet.contains(wayChunks.get(0).get(0))) { + if (wayChunks.size() == 2) { + warningNotifier.accept(tr("You must select two or more nodes to split a circular way.")); + return null; + } + lastWayChunk.remove(lastWayChunk.size() - 1); + lastWayChunk.addAll(wayChunks.get(0)); + wayChunks.remove(wayChunks.size() - 1); + wayChunks.set(0, lastWayChunk); + } + + if (wayChunks.size() < 2) { + if (wayChunks.get(0).get(0) == wayChunks.get(0).get(wayChunks.get(0).size() - 1)) { + warningNotifier.accept( + tr("You must select two or more nodes to split a circular way.")); + } else { + warningNotifier.accept( + tr("The way cannot be split at the selected nodes. (Hint: Select nodes in the middle of the way.)")); + } + return null; + } + return wayChunks; + } + + /** + * Creates new way objects for the way chunks and transfers the keys from the original way. + * @param way the original way whose keys are transferred + * @param wayChunks the way chunks + * @return the new way objects + */ + public static List createNewWaysFromChunks(Way way, Iterable> wayChunks) { + final List newWays = new ArrayList<>(); + for (List wayChunk : wayChunks) { + Way wayToAdd = new Way(); + wayToAdd.setKeys(way.getKeys()); + wayToAdd.setNodes(wayChunk); + newWays.add(wayToAdd); + } + return newWays; + } + + /** + * Splits the way {@code way} into chunks of {@code wayChunks} and replies + * the result of this process in an instance of {@link SplitWayCommand}. + * + * Note that changes are not applied to the data yet. You have to + * submit the command first, i.e. {@code Main.main.undoredo.add(result)}. + * + * @param way the way to split. Must not be null. + * @param wayChunks the list of way chunks into the way is split. Must not be null. + * @param selection The list of currently selected primitives + * @return the result from the split operation + */ + public static SplitWayCommand splitWay(Way way, List> wayChunks, Collection selection) { + return splitWay(way, wayChunks, selection, Strategy.keepLongestChunk()); + } + + /** + * Splits the way {@code way} into chunks of {@code wayChunks} and replies + * the result of this process in an instance of {@link SplitWayCommand}. + * The {@link org.openstreetmap.josm.actions.SplitWayAction.Strategy} is used to determine which + * way chunk should reuse the old id and its history. + * + * Note that changes are not applied to the data yet. You have to + * submit the command first, i.e. {@code Main.main.undoredo.add(result)}. + * + * @param way the way to split. Must not be null. + * @param wayChunks the list of way chunks into the way is split. Must not be null. + * @param selection The list of currently selected primitives + * @param splitStrategy The strategy used to determine which way chunk should reuse the old id and its history + * @return the result from the split operation + */ + public static SplitWayCommand splitWay(Way way, List> wayChunks, + Collection selection, Strategy splitStrategy) { + // build a list of commands, and also a new selection list + final List newSelection = new ArrayList<>(selection.size() + wayChunks.size()); + newSelection.addAll(selection); + + // Create all potential new ways + final List newWays = createNewWaysFromChunks(way, wayChunks); + + // Determine which part reuses the existing way + final Way wayToKeep = splitStrategy.determineWayToKeep(newWays); + + return wayToKeep != null ? doSplitWay(way, wayToKeep, newWays, newSelection) : null; + } + + /** + * Effectively constructs the {@link SplitWayCommand}. + * This method is only public for {@code SplitWayAction}. + * + * @param way the way to split. Must not be null. + * @param wayToKeep way chunk which should reuse the old id and its history + * @param newWays potential new ways + * @param newSelection new selection list to update (optional: can be null) + * @return the {@code SplitWayCommand} + */ + public static SplitWayCommand doSplitWay(Way way, Way wayToKeep, List newWays, List newSelection) { + + Collection commandList = new ArrayList<>(newWays.size()); + Collection nowarnroles = Main.pref.getCollection("way.split.roles.nowarn", + Arrays.asList("outer", "inner", "forward", "backward", "north", "south", "east", "west")); + + //final MapFrame map = MainApplication.getMap(); + //final boolean isMapModeDraw = map != null && map.mapMode == map.mapModeDraw; + + // Change the original way + final Way changedWay = new Way(way); + changedWay.setNodes(wayToKeep.getNodes()); + commandList.add(new ChangeCommand(way, changedWay)); + if (/*!isMapModeDraw &&*/ newSelection != null && !newSelection.contains(way)) { + newSelection.add(way); + } + final int indexOfWayToKeep = newWays.indexOf(wayToKeep); + newWays.remove(wayToKeep); + + if (/*!isMapModeDraw &&*/ newSelection != null) { + newSelection.addAll(newWays); + } + for (Way wayToAdd : newWays) { + commandList.add(new AddCommand(way.getDataSet(), wayToAdd)); + } + + boolean warnmerole = false; + boolean warnme = false; + // now copy all relations to new way also + + for (Relation r : OsmPrimitive.getFilteredList(way.getReferrers(), Relation.class)) { + if (!r.isUsable()) { + continue; + } + Relation c = null; + String type = Optional.ofNullable(r.get("type")).orElse(""); + + int ic = 0; + int ir = 0; + List relationMembers = r.getMembers(); + for (RelationMember rm: relationMembers) { + if (rm.isWay() && rm.getMember() == way) { + boolean insert = true; + if ("restriction".equals(type) || "destination_sign".equals(type)) { + /* this code assumes the restriction is correct. No real error checking done */ + String role = rm.getRole(); + if ("from".equals(role) || "to".equals(role)) { + OsmPrimitive via = findVia(r, type); + List nodes = new ArrayList<>(); + if (via != null) { + if (via instanceof Node) { + nodes.add((Node) via); + } else if (via instanceof Way) { + nodes.add(((Way) via).lastNode()); + nodes.add(((Way) via).firstNode()); + } + } + Way res = null; + for (Node n : nodes) { + if (changedWay.isFirstLastNode(n)) { + res = way; + } + } + if (res == null) { + for (Way wayToAdd : newWays) { + for (Node n : nodes) { + if (wayToAdd.isFirstLastNode(n)) { + res = wayToAdd; + } + } + } + if (res != null) { + if (c == null) { + c = new Relation(r); + } + c.addMember(new RelationMember(role, res)); + c.removeMembersFor(way); + insert = false; + } + } else { + insert = false; + } + } else if (!"via".equals(role)) { + warnme = true; + } + } else if (!("route".equals(type)) && !("multipolygon".equals(type))) { + warnme = true; + } + if (c == null) { + c = new Relation(r); + } + + if (insert) { + if (rm.hasRole() && !nowarnroles.contains(rm.getRole())) { + warnmerole = true; + } + + Boolean backwards = null; + int k = 1; + while (ir - k >= 0 || ir + k < relationMembers.size()) { + if ((ir - k >= 0) && relationMembers.get(ir - k).isWay()) { + Way w = relationMembers.get(ir - k).getWay(); + if ((w.lastNode() == way.firstNode()) || w.firstNode() == way.firstNode()) { + backwards = Boolean.FALSE; + } else if ((w.firstNode() == way.lastNode()) || w.lastNode() == way.lastNode()) { + backwards = Boolean.TRUE; + } + break; + } + if ((ir + k < relationMembers.size()) && relationMembers.get(ir + k).isWay()) { + Way w = relationMembers.get(ir + k).getWay(); + if ((w.lastNode() == way.firstNode()) || w.firstNode() == way.firstNode()) { + backwards = Boolean.TRUE; + } else if ((w.firstNode() == way.lastNode()) || w.lastNode() == way.lastNode()) { + backwards = Boolean.FALSE; + } + break; + } + k++; + } + + int j = ic; + final List waysToAddBefore = newWays.subList(0, indexOfWayToKeep); + for (Way wayToAdd : waysToAddBefore) { + RelationMember em = new RelationMember(rm.getRole(), wayToAdd); + j++; + if (Boolean.TRUE.equals(backwards)) { + c.addMember(ic + 1, em); + } else { + c.addMember(j - 1, em); + } + } + final List waysToAddAfter = newWays.subList(indexOfWayToKeep, newWays.size()); + for (Way wayToAdd : waysToAddAfter) { + RelationMember em = new RelationMember(rm.getRole(), wayToAdd); + j++; + if (Boolean.TRUE.equals(backwards)) { + c.addMember(ic, em); + } else { + c.addMember(j, em); + } + } + ic = j; + } + } + ic++; + ir++; + } + + if (c != null) { + commandList.add(new ChangeCommand(r.getDataSet(), r, c)); + } + } + if (warnmerole) { + warningNotifier.accept( + tr("A role based relation membership was copied to all new ways.
You should verify this and correct it when necessary.")); + } else if (warnme) { + warningNotifier.accept( + tr("A relation membership was copied to all new ways.
You should verify this and correct it when necessary.")); + } + + return new SplitWayCommand( + /* for correct i18n of plural forms - see #9110 */ + trn("Split way {0} into {1} part", "Split way {0} into {1} parts", newWays.size() + 1, + way.getDisplayName(DefaultNameFormatter.getInstance()), newWays.size() + 1), + commandList, + newSelection, + way, + newWays + ); + } + + static OsmPrimitive findVia(Relation r, String type) { + for (RelationMember rmv : r.getMembers()) { + if (("restriction".equals(type) && "via".equals(rmv.getRole())) + || ("destination_sign".equals(type) && rmv.hasRole("sign", "intersection"))) { + return rmv.getMember(); + } + } + return null; + } + + /** + * Splits the way {@code way} at the nodes in {@code atNodes} and replies + * the result of this process in an instance of {@link SplitWayCommand}. + * + * Note that changes are not applied to the data yet. You have to + * submit the command first, i.e. {@code Main.main.undoredo.add(result)}. + * + * Replies null if the way couldn't be split at the given nodes. + * + * @param way the way to split. Must not be null. + * @param atNodes the list of nodes where the way is split. Must not be null. + * @param selection The list of currently selected primitives + * @return the result from the split operation + */ + public static SplitWayCommand split(Way way, List atNodes, Collection selection) { + List> chunks = buildSplitChunks(way, atNodes); + return chunks != null ? splitWay(way, chunks, selection) : null; + } +} Index: /trunk/src/org/openstreetmap/josm/gui/MainApplication.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/MainApplication.java (revision 12827) +++ /trunk/src/org/openstreetmap/josm/gui/MainApplication.java (revision 12828) @@ -78,4 +78,5 @@ import org.openstreetmap.josm.actions.search.SearchAction; import org.openstreetmap.josm.command.DeleteCommand; +import org.openstreetmap.josm.command.SplitWayCommand; import org.openstreetmap.josm.data.Bounds; import org.openstreetmap.josm.data.UndoRedoHandler; @@ -1098,4 +1099,5 @@ MessageNotifier.setNotifierCallback(MainApplication::notifyNewMessages); DeleteCommand.setDeletionCallback(DeleteAction.defaultDeletionCallback); + SplitWayCommand.setWarningNotifier(msg -> new Notification(msg).setIcon(JOptionPane.WARNING_MESSAGE).show()); FileWatcher.registerLoader(SourceType.MAP_PAINT_STYLE, MapPaintStyleLoader::reloadStyle); FileWatcher.registerLoader(SourceType.TAGCHECKER_RULE, MapCSSTagChecker::reloadRule); Index: /trunk/test/unit/org/openstreetmap/josm/actions/SplitWayActionTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/actions/SplitWayActionTest.java (revision 12827) +++ /trunk/test/unit/org/openstreetmap/josm/actions/SplitWayActionTest.java (revision 12828) @@ -2,24 +2,15 @@ package org.openstreetmap.josm.actions; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; -import java.util.ArrayList; import java.util.Arrays; -import java.util.Iterator; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.openstreetmap.josm.actions.SplitWayAction.Strategy; import org.openstreetmap.josm.data.coor.EastNorth; -import org.openstreetmap.josm.data.coor.LatLon; import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.Node; -import org.openstreetmap.josm.data.osm.OsmPrimitive; -import org.openstreetmap.josm.data.osm.Relation; -import org.openstreetmap.josm.data.osm.RelationMember; import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.gui.MainApplication; @@ -112,110 +103,3 @@ } } - - /** - * Unit test of {@link SplitWayAction#findVia}. - */ - @Test - public void testFindVia() { - // empty relation - assertNull(SplitWayAction.findVia(new Relation(), null)); - // restriction relation without via member - Relation r = new Relation(); - r.addMember(new RelationMember("", new Node())); - assertNull(SplitWayAction.findVia(r, "restriction")); - // restriction relation with via member - r = new Relation(); - OsmPrimitive via = new Node(); - r.addMember(new RelationMember("via", via)); - assertEquals(via, SplitWayAction.findVia(r, "restriction")); - // destination_sign relation without sign nor intersection - r = new Relation(); - r.addMember(new RelationMember("", new Node())); - assertNull(SplitWayAction.findVia(r, "destination_sign")); - // destination_sign with sign - r = new Relation(); - via = new Node(); - r.addMember(new RelationMember("sign", via)); - assertEquals(via, SplitWayAction.findVia(r, "destination_sign")); - // destination_sign with intersection - r = new Relation(); - via = new Node(); - r.addMember(new RelationMember("intersection", via)); - assertEquals(via, SplitWayAction.findVia(r, "destination_sign")); - } - - /** - * Unit tests of route relations. - */ - @Test - public void testRouteRelation() { - doTestRouteRelation(false, 0); - doTestRouteRelation(false, 1); - doTestRouteRelation(false, 2); - doTestRouteRelation(false, 3); - doTestRouteRelation(true, 0); - doTestRouteRelation(true, 1); - doTestRouteRelation(true, 2); - doTestRouteRelation(true, 3); - } - - void doTestRouteRelation(final boolean wayIsReversed, final int indexOfWayToKeep) { - final DataSet dataSet = new DataSet(); - final Node n1 = new Node(new LatLon(1, 0)); - final Node n2 = new Node(new LatLon(2, 0)); - final Node n3 = new Node(new LatLon(3, 0)); - final Node n4 = new Node(new LatLon(4, 0)); - final Node n5 = new Node(new LatLon(5, 0)); - final Node n6 = new Node(new LatLon(6, 0)); - final Node n7 = new Node(new LatLon(7, 0)); - final Way w1 = new Way(); - final Way w2 = new Way(); - final Way w3 = new Way(); - final Relation route = new Relation(); - for (OsmPrimitive p : Arrays.asList(n1, n2, n3, n4, n5, n6, n7, w1, w2, w3, route)) { - dataSet.addPrimitive(p); - } - w1.setNodes(Arrays.asList(n1, n2)); - w2.setNodes(wayIsReversed - ? Arrays.asList(n6, n5, n4, n3, n2) - : Arrays.asList(n2, n3, n4, n5, n6) - ); - w3.setNodes(Arrays.asList(n6, n7)); - route.put("type", "route"); - route.addMember(new RelationMember("", w1)); - route.addMember(new RelationMember("", w2)); - route.addMember(new RelationMember("", w3)); - dataSet.setSelected(Arrays.asList(w2, n3, n4, n5)); - - final Strategy strategy = wayChunks -> { - final Iterator it = wayChunks.iterator(); - for (int i = 0; i < indexOfWayToKeep; i++) { - it.next(); - } - return it.next(); - }; - final SplitWayAction.SplitWayResult result = SplitWayAction.splitWay( - w2, SplitWayAction.buildSplitChunks(w2, Arrays.asList(n3, n4, n5)), new ArrayList(), strategy); - MainApplication.undoRedo.add(result.getCommand()); - - assertEquals(6, route.getMembersCount()); - assertEquals(w1, route.getMemberPrimitivesList().get(0)); - assertEquals(w3, route.getMemberPrimitivesList().get(5)); - assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(0)), n1); - assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(0)), n2); - assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(1)), n2); - assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(1)), n3); - assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(2)), n3); - assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(2)), n4); - assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(3)), n4); - assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(3)), n5); - assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(4)), n5); - assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(4)), n6); - assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(5)), n6); - assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(5)), n7); - } - - static void assertFirstLastNodeIs(Way way, Node node) { - assertTrue("First/last node of " + way + " should be " + node, node.equals(way.firstNode()) || node.equals(way.lastNode())); - } } Index: /trunk/test/unit/org/openstreetmap/josm/command/SplitWayCommandTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/command/SplitWayCommandTest.java (revision 12828) +++ /trunk/test/unit/org/openstreetmap/josm/command/SplitWayCommandTest.java (revision 12828) @@ -0,0 +1,145 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.command; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Iterator; + +import org.junit.Rule; +import org.junit.Test; +import org.openstreetmap.josm.command.SplitWayCommand.Strategy; +import org.openstreetmap.josm.data.coor.LatLon; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.Node; +import org.openstreetmap.josm.data.osm.OsmPrimitive; +import org.openstreetmap.josm.data.osm.Relation; +import org.openstreetmap.josm.data.osm.RelationMember; +import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.testutils.JOSMTestRules; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + +/** + * Unit tests for class {@link SplitWayCommand}. + */ +public final class SplitWayCommandTest { + + /** + * Setup test. + */ + @Rule + @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") + public JOSMTestRules test = new JOSMTestRules().main().projection(); + + /** + * Unit test of {@link SplitWayCommand#findVia}. + */ + @Test + public void testFindVia() { + // empty relation + assertNull(SplitWayCommand.findVia(new Relation(), null)); + // restriction relation without via member + Relation r = new Relation(); + r.addMember(new RelationMember("", new Node())); + assertNull(SplitWayCommand.findVia(r, "restriction")); + // restriction relation with via member + r = new Relation(); + OsmPrimitive via = new Node(); + r.addMember(new RelationMember("via", via)); + assertEquals(via, SplitWayCommand.findVia(r, "restriction")); + // destination_sign relation without sign nor intersection + r = new Relation(); + r.addMember(new RelationMember("", new Node())); + assertNull(SplitWayCommand.findVia(r, "destination_sign")); + // destination_sign with sign + r = new Relation(); + via = new Node(); + r.addMember(new RelationMember("sign", via)); + assertEquals(via, SplitWayCommand.findVia(r, "destination_sign")); + // destination_sign with intersection + r = new Relation(); + via = new Node(); + r.addMember(new RelationMember("intersection", via)); + assertEquals(via, SplitWayCommand.findVia(r, "destination_sign")); + } + + /** + * Unit tests of route relations. + */ + @Test + public void testRouteRelation() { + doTestRouteRelation(false, 0); + doTestRouteRelation(false, 1); + doTestRouteRelation(false, 2); + doTestRouteRelation(false, 3); + doTestRouteRelation(true, 0); + doTestRouteRelation(true, 1); + doTestRouteRelation(true, 2); + doTestRouteRelation(true, 3); + } + + void doTestRouteRelation(final boolean wayIsReversed, final int indexOfWayToKeep) { + final DataSet dataSet = new DataSet(); + final Node n1 = new Node(new LatLon(1, 0)); + final Node n2 = new Node(new LatLon(2, 0)); + final Node n3 = new Node(new LatLon(3, 0)); + final Node n4 = new Node(new LatLon(4, 0)); + final Node n5 = new Node(new LatLon(5, 0)); + final Node n6 = new Node(new LatLon(6, 0)); + final Node n7 = new Node(new LatLon(7, 0)); + final Way w1 = new Way(); + final Way w2 = new Way(); + final Way w3 = new Way(); + final Relation route = new Relation(); + for (OsmPrimitive p : Arrays.asList(n1, n2, n3, n4, n5, n6, n7, w1, w2, w3, route)) { + dataSet.addPrimitive(p); + } + w1.setNodes(Arrays.asList(n1, n2)); + w2.setNodes(wayIsReversed + ? Arrays.asList(n6, n5, n4, n3, n2) + : Arrays.asList(n2, n3, n4, n5, n6) + ); + w3.setNodes(Arrays.asList(n6, n7)); + route.put("type", "route"); + route.addMember(new RelationMember("", w1)); + route.addMember(new RelationMember("", w2)); + route.addMember(new RelationMember("", w3)); + dataSet.setSelected(Arrays.asList(w2, n3, n4, n5)); + + final Strategy strategy = wayChunks -> { + final Iterator it = wayChunks.iterator(); + for (int i = 0; i < indexOfWayToKeep; i++) { + it.next(); + } + return it.next(); + }; + final SplitWayCommand result = SplitWayCommand.splitWay( + w2, SplitWayCommand.buildSplitChunks(w2, Arrays.asList(n3, n4, n5)), new ArrayList(), strategy); + MainApplication.undoRedo.add(result); + + assertEquals(6, route.getMembersCount()); + assertEquals(w1, route.getMemberPrimitivesList().get(0)); + assertEquals(w3, route.getMemberPrimitivesList().get(5)); + assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(0)), n1); + assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(0)), n2); + assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(1)), n2); + assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(1)), n3); + assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(2)), n3); + assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(2)), n4); + assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(3)), n4); + assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(3)), n5); + assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(4)), n5); + assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(4)), n6); + assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(5)), n6); + assertFirstLastNodeIs(((Way) route.getMemberPrimitivesList().get(5)), n7); + } + + static void assertFirstLastNodeIs(Way way, Node node) { + assertTrue("First/last node of " + way + " should be " + node, node.equals(way.firstNode()) || node.equals(way.lastNode())); + } +}