Changeset 36217 in osm


Ignore:
Timestamp:
2024-03-03T10:25:25+01:00 (7 months ago)
Author:
GerdP
Message:

fix #23521: fix some memory leaks

  • dispose dialogs
  • either avoid to create clones of ways or relations or use setNodes(null) / setMembers(null)
  • replaces most ChangeCommand instances by better specialized alternatives
  • add some comments
  • fix some checkstyle / sonar issues
Location:
applications/editors/josm/plugins/reltoolbox/src/relcontext
Files:
14 edited

Legend:

Unmodified
Added
Removed

  • applications/editors/josm/plugins/reltoolbox/src/relcontext/RelContextDialog.java

    r36136 r36217  
    1818import java.awt.event.MouseEvent;
    1919import java.awt.event.MouseListener;
    20 import java.beans.PropertyChangeEvent;
    21 import java.beans.PropertyChangeListener;
    2220import java.io.BufferedReader;
    2321import java.io.InputStream;
    … …  
    5149import javax.swing.ListSelectionModel;
    5250import javax.swing.SwingUtilities;
    53 import javax.swing.event.ListSelectionEvent;
    54 import javax.swing.event.ListSelectionListener;
    5551import javax.swing.table.DefaultTableCellRenderer;
    5652import javax.swing.table.DefaultTableModel;
    … …  
    111107
    112108    private final DefaultTableModel relationsData;
    113     private final ChosenRelation chosenRelation;
     109    private final transient ChosenRelation chosenRelation;
    114110    private final JPanel chosenRelationPanel;
    115111    private final ChosenRelationPopupMenu popupMenu;
    … …  
    413409    @Override
    414410    public void destroy() {
     411        chosenRelation.removeChosenRelationListener(this);
    415412        enterRoleAction.destroy();
    416413        findRelationAction.destroy();
    … …  
    487484
    488485        Object answer = optionPane.getValue();
     486        dlg.dispose();
    489487        if (answer == null || answer == JOptionPane.UNINITIALIZED_VALUE
    490488                || (answer instanceof Integer && (Integer) answer != JOptionPane.OK_OPTION))
    … …  
    545543            for (int i = 0; i < r.getMembersCount(); i++) {
    546544                RelationMember m = r.getMember(i);
    547                 if (selected.contains(m.getMember())) {
    548                     if (!role.equals(m.getRole())) {
    549                         commands.add(new ChangeRelationMemberRoleCommand(r, i, role));
    550                     }
     545                if (selected.contains(m.getMember()) && !role.equals(m.getRole())) {
     546                    commands.add(new ChangeRelationMemberRoleCommand(r, i, role));
    551547                }
    552548            }
    553549            if (!commands.isEmpty()) {
    554                 //                UndoRedoHandler.getInstance().add(new ChangeCommand(chosenRelation.get(), r));
    555550                UndoRedoHandler.getInstance().add(new SequenceCommand(tr("Change relation member roles to {0}", role), commands));
    556551            }

  • applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/AddRemoveMemberAction.java

    r36102 r36217  
    1010
    1111import org.openstreetmap.josm.actions.JosmAction;
    12 import org.openstreetmap.josm.command.ChangeCommand;
     12import org.openstreetmap.josm.command.ChangeMembersCommand;
    1313import org.openstreetmap.josm.command.Command;
    1414import org.openstreetmap.josm.data.UndoRedoHandler;
    … …  
    8282
    8383        if (!r.getMemberPrimitives().equals(rel.get().getMemberPrimitives())) {
    84             UndoRedoHandler.getInstance().add(new ChangeCommand(rel.get(), r));
     84            UndoRedoHandler.getInstance().add(new ChangeMembersCommand(rel.get(), r.getMembers()));
    8585        }
     86        r.setMembers(null); // See #19885
    8687    }
    8788
    … …  
    9697        if (firstNode != null && !firstNode.equals(lastNode)) {
    9798            for (int i = 0; i < r.getMembersCount(); i++) {
    98                 if (r.getMember(i).getType().equals(OsmPrimitiveType.WAY)) {
     99                if (r.getMember(i).getType() == OsmPrimitiveType.WAY) {
    99100                    Way rw = (Way) r.getMember(i).getMember();
    100101                    Node firstNodeR = rw.firstNode();
    … …  
    133134
    134135    protected void updateIcon() {
    135         // todo: change icon based on selection
    136136        final int state; // 0=unknown, 1=add, 2=remove, 3=both
    137137        DataSet ds = getLayerManager().getEditDataSet();
    138         if (ds == null || ds.getSelected() == null
    139                 || ds.getSelected().isEmpty() || rel == null || rel.get() == null) {
     138        if (ds == null || ds.getSelected().isEmpty() || rel == null || rel.get() == null) {
    140139            state = 0;
    141140        } else {
    … …  
    156155            }
    157156        }
    158         GuiHelper.runInEDT(new Runnable() {
    159             @Override
    160             public void run() {
    161                 if (state == 0) {
    162                     putValue(LARGE_ICON_KEY, ImageProvider.get("relcontext", "addremove"));
    163                 } else {
    164                     String iconName = state == 1 ? "add" : state == 2 ? "remove" : "addremove";
    165                     putValue(NAME, null);
    166                     putValue(LARGE_ICON_KEY, ImageProvider.get("relcontext", iconName));
    167                 }
     157        GuiHelper.runInEDT(() -> {
     158            if (state == 0) {
     159                putValue(LARGE_ICON_KEY, ImageProvider.get("relcontext", "addremove"));
     160            } else {
     161                String iconName = state == 1 ? "add" : state == 2 ? "remove" : "addremove";
     162                putValue(NAME, null);
     163                putValue(LARGE_ICON_KEY, ImageProvider.get("relcontext", iconName));
    168164            }
    169165        });

  • applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/ClearChosenRelationAction.java

    r36102 r36217  
    1616
    1717public class ClearChosenRelationAction extends AbstractAction implements ChosenRelationListener {
    18     private final ChosenRelation rel;
     18    private final transient ChosenRelation rel;
    1919
    2020    public ClearChosenRelationAction(ChosenRelation rel) {

  • applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/CreateMultipolygonAction.java

    r36102 r36217  
    77import java.awt.GridBagLayout;
    88import java.awt.event.ActionEvent;
    9 import java.awt.event.ActionListener;
    109import java.awt.event.KeyEvent;
    1110import java.util.ArrayList;
    1211import java.util.Arrays;
    1312import java.util.Collection;
     13import java.util.Collections;
    1414import java.util.HashMap;
    1515import java.util.HashSet;
    1616import java.util.List;
    1717import java.util.Map;
     18import java.util.Map.Entry;
    1819import java.util.Set;
    1920import java.util.TreeSet;
    … …  
    2829import org.openstreetmap.josm.actions.JosmAction;
    2930import org.openstreetmap.josm.command.AddCommand;
    30 import org.openstreetmap.josm.command.ChangeCommand;
    3131import org.openstreetmap.josm.command.ChangePropertyCommand;
    3232import org.openstreetmap.josm.command.Command;
    … …  
    4040import org.openstreetmap.josm.data.osm.Relation;
    4141import org.openstreetmap.josm.data.osm.RelationMember;
     42import org.openstreetmap.josm.data.osm.TagMap;
    4243import org.openstreetmap.josm.data.osm.Way;
    4344import org.openstreetmap.josm.gui.MainApplication;
    … …  
    5657public class CreateMultipolygonAction extends JosmAction {
    5758    private static final String PREF_MULTIPOLY = "reltoolbox.multipolygon.";
    58     protected ChosenRelation chRel;
     59    protected transient ChosenRelation chRel;
    5960
    6061    public CreateMultipolygonAction(ChosenRelation chRel) {
    … …  
    7273    public static boolean getDefaultPropertyValue(String property) {
    7374        switch (property) {
    74             case "boundary":
    75             case "alltags":
    76             case "allowsplit":
    77                 return false;
    78             case "boundaryways":
    79             case "tags":
    80             case "single":
    81                 return true;
     75        case "boundary":
     76        case "alltags":
     77        case "allowsplit":
     78            return false;
     79        case "boundaryways":
     80        case "tags":
     81        case "single":
     82            return true;
    8283        }
    8384        throw new IllegalArgumentException(property);
    8485    }
    8586
    86     private boolean getPref(String property) {
     87    private static boolean getPref(String property) {
    8788        return Config.getPref().getBoolean(PREF_MULTIPOLY + property, getDefaultPropertyValue(property));
    8889    }
    … …  
    9798            if (getPref("allowsplit") || selectedWays.size() == 1) {
    9899                if (SplittingMultipolygons.canProcess(selectedWays)) {
    99                     rels = SplittingMultipolygons.process(ds.getSelectedWays());
     100                    rels = SplittingMultipolygons.process(selectedWays);
    100101                }
    101102            } else {
    … …  
    201202        for (OsmPrimitive p : getLayerManager().getEditDataSet().getSelected()) {
    202203            String role = null;
    203             if (p.getType().equals(OsmPrimitiveType.RELATION)) {
     204            if (p.getType() == OsmPrimitiveType.RELATION) {
    204205                role = "subarea";
    205             } else if (p.getType().equals(OsmPrimitiveType.NODE)) {
     206            } else if (p.getType() == OsmPrimitiveType.NODE) {
    206207                Node n = (Node) p;
    207208                if (!n.isIncomplete()) {
    … …  
    319320        if (isBoundary || !getPref("alltags")) {
    320321            for (RelationMember m : relation.getMembers()) {
    321                 if (m.hasRole() && m.getRole().equals("outer") && m.isWay()) {
     322                if (m.hasRole() && "outer".equals(m.getRole()) && m.isWay()) {
    322323                    for (String key : values.keySet()) {
    323324                        if (!m.getWay().hasKey(key) && !relation.hasKey(key)) {
    … …  
    337338        }
    338339
    339         if (values.containsKey("natural") && values.get("natural").equals("coastline")) {
     340        if ("coastline".equals(values.get("natural"))) {
    340341            values.remove("natural");
    341342        }
    … …  
    356357        boolean moveTags = getPref("tags");
    357358
    358         for (String key : values.keySet()) {
     359        for (Entry<String, String> entry : values.entrySet()) {
     360            String key = entry.getKey();
    359361            List<OsmPrimitive> affectedWays = new ArrayList<>();
    360             String value = values.get(key);
     362            String value = entry.getValue();
    361363
    362364            for (Way way : innerWays) {
    … …  
    375377            }
    376378
    377             if (affectedWays.size() > 0) {
     379            if (!affectedWays.isEmpty()) {
    378380                commands.add(new ChangePropertyCommand(affectedWays, key, null));
    379381            }
    … …  
    386388            }
    387389            boolean fixed = false;
    388             Relation r2 = new Relation(relation);
    389             for (String key : values.keySet()) {
    390                 if (!r2.hasKey(key) && !key.equals("area")
    391                         && (!isBoundary || key.equals("admin_level") || key.equals("name"))) {
    392                     if (relation.isNew()) {
    393                         relation.put(key, values.get(key));
     390            TagMap tags = relation.getKeys();
     391            for (Entry<String, String> e: values.entrySet()) {
     392                final String key = e.getKey();
     393                final String val = e.getValue();
     394                if (!tags.containsKey(key) && !"area".equals(key)
     395                        && (!isBoundary || "admin_level".equals(key) || "name".equals(key))) {
     396                    if (relation.getDataSet() == null) {
     397                        relation.put(key, val);
    394398                    } else {
    395                         r2.put(key, values.get(key));
     399                        tags.put(key, val);
    396400                    }
    397401                    fixed = true;
    398402                }
    399403            }
    400             if (fixed && !relation.isNew()) {
    401                 commands.add(new ChangeCommand(relation, r2));
     404            if (fixed && relation.getDataSet() != null) {
     405                commands.add(new ChangePropertyCommand(Collections.singleton(relation), tags));
    402406            }
    403407        }
    … …  
    444448        dlg.setModalityType(ModalityType.DOCUMENT_MODAL);
    445449
    446         name.addActionListener(new ActionListener() {
    447             @Override
    448             public void actionPerformed(ActionEvent e) {
    449                 dlg.setVisible(false);
    450                 optionPane.setValue(JOptionPane.OK_OPTION);
    451             }
     450        name.addActionListener(e -> {
     451            dlg.setVisible(false);
     452            optionPane.setValue(JOptionPane.OK_OPTION);
    452453        });
    453454
    … …  
    455456
    456457        Object answer = optionPane.getValue();
     458        dlg.dispose();
    457459        if (answer == null || answer == JOptionPane.UNINITIALIZED_VALUE
    458460                || (answer instanceof Integer && (Integer) answer != JOptionPane.OK_OPTION))
    459461            return false;
    460462
    461         String admin_level = admin.getText().trim();
    462         String new_name = name.getText().trim();
    463         if (admin_level.equals("10") || (admin_level.length() == 1 && Character.isDigit(admin_level.charAt(0)))) {
    464             rel.put("admin_level", admin_level);
    465             Config.getPref().put(PREF_MULTIPOLY + "lastadmin", admin_level);
    466         }
    467         if (new_name.length() > 0) {
    468             rel.put("name", new_name);
     463        String adminLevel = admin.getText().trim();
     464        String newName = name.getText().trim();
     465        if ("10".equals(adminLevel) || (adminLevel.length() == 1 && Character.isDigit(adminLevel.charAt(0)))) {
     466            rel.put("admin_level", adminLevel);
     467            Config.getPref().put(PREF_MULTIPOLY + "lastadmin", adminLevel);
     468        }
     469        if (!newName.isEmpty()) {
     470            rel.put("name", newName);
    469471        }
    470472        return true;

  • applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/CreateRelationAction.java

    r36102 r36217  
    127127
    128128        Object answer = optionPane.getValue();
     129        dlg.dispose();
    129130        if (answer == null || answer == JOptionPane.UNINITIALIZED_VALUE
    130131                || (answer instanceof Integer && (Integer) answer != JOptionPane.OK_OPTION))

  • applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/FindRelationAction.java

    r36102 r36217  
    123123
    124124        Object answer = optionPane.getValue();
     125        dlg.dispose();
     126       
    125127        if (answer == null || answer == JOptionPane.UNINITIALIZED_VALUE
    126128                || (answer instanceof Integer && (Integer) answer != JOptionPane.OK_OPTION))

  • applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/ReconstructPolygonAction.java

    r36142 r36217  
    88import java.util.ArrayList;
    99import java.util.Arrays;
     10import java.util.Collection;
    1011import java.util.Collections;
    1112import java.util.HashSet;
    … …  
    1920import org.openstreetmap.josm.command.AddCommand;
    2021import org.openstreetmap.josm.command.ChangeCommand;
     22import org.openstreetmap.josm.command.ChangeMembersCommand;
    2123import org.openstreetmap.josm.command.Command;
    2224import org.openstreetmap.josm.command.DeleteCommand;
    … …  
    4345 */
    4446public class ReconstructPolygonAction extends JosmAction implements ChosenRelationListener {
    45     private final ChosenRelation rel;
     47    private final transient ChosenRelation rel;
    4648
    4749    private static final List<String> IRRELEVANT_KEYS = Arrays.asList("source", "created_by", "note");
    4850
     51    /**
     52     * Reconstruct one or more polygons from multipolygon relation.
     53     * @param rel the multipolygon relation
     54     */
    4955    public ReconstructPolygonAction(ChosenRelation rel) {
    5056        super(tr("Reconstruct polygon"), "dialogs/filter", tr("Reconstruct polygon from multipolygon relation"),
    5157                Shortcut.registerShortcut("reltoolbox:reconstructpoly", tr("Relation Toolbox: {0}",
    5258                        tr("Reconstruct polygon from multipolygon relation")),
    53                         KeyEvent.CHAR_UNDEFINED, Shortcut.NONE), false);
     59                        KeyEvent.CHAR_UNDEFINED, Shortcut.NONE), false, false);
    5460        this.rel = rel;
    5561        rel.addChosenRelationListener(this);
    … …  
    7278        if (wont) {
    7379            JOptionPane.showMessageDialog(MainApplication.getMainFrame(),
    74                     tr("Multipolygon must consist only of ways with one referring relation"),
     80                    tr("Multipolygon must consist only of ways"),
    7581                    tr("Reconstruct polygon"), JOptionPane.ERROR_MESSAGE);
    7682            return;
    … …  
    8591
    8692        rel.clear();
    87         List<OsmPrimitive> newSelection = new ArrayList<>();
    8893        List<Command> commands = new ArrayList<>();
    8994        Command relationDeleteCommand = DeleteCommand.delete(Collections.singleton(r), true, true);
    … …  
    108113                // this ring has inner rings, so we leave a multipolygon in
    109114                // place and don't reconstruct the rings.
     115                List<RelationMember> members = new ArrayList<>();
    110116                Relation n;
     117                for (Way w : p.ways) {
     118                    members.add(new RelationMember("outer", w));
     119                }
     120                for (JoinedPolygon i : myInnerWays) {
     121                    for (Way w : i.ways) {
     122                        members.add(new RelationMember("inner", w));
     123                    }
     124                }
    111125                if (relationReused) {
    112126                    n = new Relation();
    113127                    n.setKeys(r.getKeys());
    114                 } else {
    115                     n = new Relation(r);
    116                     n.setMembers(null);
    117                 }
    118                 for (Way w : p.ways) {
    119                     n.addMember(new RelationMember("outer", w));
    120                 }
    121                 for (JoinedPolygon i : myInnerWays) {
    122                     for (Way w : i.ways) {
    123                         n.addMember(new RelationMember("inner", w));
    124                     }
    125                 }
    126                 if (relationReused) {
     128                    n.setMembers(members);
    127129                    commands.add(new AddCommand(ds, n));
    128130                } else {
    129131                    relationReused = true;
    130                     commands.add(new ChangeCommand(r, n));
    131                 }
    132                 newSelection.add(n);
     132                    commands.add(new ChangeMembersCommand(r, members));
     133                }
    133134                continue;
    134135            }
    … …  
    186187            result.addNode(result.firstNode());
    187188            result.setKeys(tags);
    188             newSelection.add(candidateWay == null ? result : candidateWay);
    189189            commands.add(candidateWay == null ? new AddCommand(ds, result) : new ChangeCommand(candidateWay, result));
    190190        }
    … …  
    195195            commands.add(0, relationDeleteCommand);
    196196        }
    197 
    198197        UndoRedoHandler.getInstance().add(new SequenceCommand(tr("Reconstruct polygons from relation {0}",
    199198                r.getDisplayName(DefaultNameFormatter.getInstance())), commands));
     199        Collection<? extends OsmPrimitive> newSelection = UndoRedoHandler.getInstance().getLastCommand().getParticipatingPrimitives();
     200        newSelection.removeIf(p -> p.isDeleted());
    200201        ds.setSelected(newSelection);
    201202    }

  • applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/ReconstructRouteAction.java

    r36102 r36217  
    1313import javax.swing.AbstractAction;
    1414
    15 import org.openstreetmap.josm.command.ChangeCommand;
    16 import org.openstreetmap.josm.command.Command;
     15import org.openstreetmap.josm.command.ChangeMembersCommand;
    1716import org.openstreetmap.josm.data.UndoRedoHandler;
    1817import org.openstreetmap.josm.data.coor.EastNorth;
    … …  
    3534 */
    3635public class ReconstructRouteAction extends AbstractAction implements ChosenRelationListener {
    37     private final ChosenRelation rel;
    38 
     36    private final transient ChosenRelation rel;
     37
     38    /**
     39     * Reconstruct route relation to scheme of public_transport.
     40     * @param rel chosen relation
     41     */
    3942    public ReconstructRouteAction(ChosenRelation rel) {
    4043        super(tr("Reconstruct route"));
    … …  
    4952    public void actionPerformed(ActionEvent e) {
    5053        Relation r = rel.get();
    51         Relation recRel = new Relation(r);
    52         recRel.removeMembersFor(recRel.getMemberPrimitives());
     54        List<RelationMember> recMembers = new ArrayList<>();
    5355
    5456        Map<OsmPrimitive, RelationMember> stopMembers = new LinkedHashMap<>();
    … …  
    118120                    nIndex += increment) {
    119121                Node refNode = w.getNode(nIndex);
    120                 if (PublicTransportHelper.isNodeStop(refNode)) {
    121                     if (stopMembers.containsKey(refNode)) {
    122                         recRel.addMember(stopMembers.get(refNode));
    123                         stopMembers.remove(refNode);
    124                         String stopName = PublicTransportHelper.getNameViaStoparea(refNode);
    125                         if (stopName == null) {
    126                             stopName = "";
     122                if (!(PublicTransportHelper.isNodeStop(refNode) && stopMembers.containsKey(refNode)))
     123                    continue;
     124                recMembers.add(stopMembers.get(refNode));
     125                stopMembers.remove(refNode);
     126                String stopName = PublicTransportHelper.getNameViaStoparea(refNode);
     127                if (stopName == null) {
     128                    stopName = "";
     129                }
     130                boolean existsPlatform = platformMembers.containsKey(stopName);
     131                if (!existsPlatform) {
     132                    stopName = ""; // find of the nameless
     133                }
     134                if (existsPlatform || platformMembers.containsKey(stopName)) {
     135                    List<RelationMember> lMember = platformMembers.get(stopName);
     136                    if (lMember.size() == 1) {
     137                        recMembers.add(lMember.get(0));
     138                        lMember.remove(0);
     139                    } else {
     140                        // choose closest
     141                        RelationMember candidat = getClosestPlatform(lMember, refNode);
     142                        if (candidat != null) {
     143                            recMembers.add(candidat);
     144                            lMember.remove(candidat);
    127145                        }
    128                         boolean existsPlatform = platformMembers.containsKey(stopName);
    129                         if (!existsPlatform) {
    130                             stopName = ""; // find of the nameless
    131                         }
    132                         if (existsPlatform || platformMembers.containsKey(stopName)) {
    133                             List<RelationMember> lMember = platformMembers.get(stopName);
    134                             if (lMember.size() == 1) {
    135                                 recRel.addMember(lMember.get(0));
    136                                 lMember.remove(0);
    137                             } else {
    138                                 // choose closest
    139                                 RelationMember candidat = getClosestPlatform(lMember, refNode);
    140                                 if (candidat != null) {
    141                                     recRel.addMember(candidat);
    142                                     lMember.remove(candidat);
    143                                 }
    144                             }
    145                             if (lMember.isEmpty()) {
    146                                 platformMembers.remove(stopName);
    147                             }
    148                         }
     146                    }
     147                    if (lMember.isEmpty()) {
     148                        platformMembers.remove(stopName);
    149149                    }
    150150                }
    … …  
    153153
    154154        for (RelationMember stop : stopMembers.values()) {
    155             recRel.addMember(stop);
     155            recMembers.add(stop);
    156156            String stopName = PublicTransportHelper.getNameViaStoparea(stop);
    157157            boolean existsPlatform = platformMembers.containsKey(stopName);
    … …  
    162162                List<RelationMember> lMember = platformMembers.get(stopName);
    163163                if (lMember.size() == 1) {
    164                     recRel.addMember(lMember.get(0));
     164                    recMembers.add(lMember.get(0));
    165165                    lMember.remove(0);
    166166                } else {
    … …  
    168168                    RelationMember candidat = getClosestPlatform(lMember, stop.getNode());
    169169                    if (candidat != null) {
    170                         recRel.addMember(candidat);
     170                        recMembers.add(candidat);
    171171                        lMember.remove(candidat);
    172172                    }
    … …  
    180180        for (List<RelationMember> lPlatforms : platformMembers.values()) {
    181181            for (RelationMember platform : lPlatforms) {
    182                 recRel.addMember(platform);
     182                recMembers.add(platform);
    183183            }
    184184        }
    185185
    186186        for (RelationMember route : routeMembers) {
    187             recRel.addMember(route);
     187            recMembers.add(route);
    188188        }
    189189        for (RelationMember wtf : wtfMembers) {
    190             recRel.addMember(wtf);
    191         }
    192         Command command = new ChangeCommand(r, recRel);
    193         UndoRedoHandler.getInstance().add(command);
     190            recMembers.add(wtf);
     191        }
     192        UndoRedoHandler.getInstance().add(new ChangeMembersCommand(r, recMembers));
    194193    }
    195194
    196195    private static final double maxSqrDistBetweenStopAndPlatform = 2000; // ~ 26m
    197     private RelationMember getClosestPlatform(List<RelationMember> members, Node stop) {
     196   
     197    private static RelationMember getClosestPlatform(List<RelationMember> members, Node stop) {
    198198        if (stop == null || members.isEmpty()) return null;
    199199        double maxDist = maxSqrDistBetweenStopAndPlatform;
    … …  
    229229    }
    230230
    231     private boolean isSuitableRelation(Relation newRelation) {
     231    private static boolean isSuitableRelation(Relation newRelation) {
    232232        return !(newRelation == null || !"route".equals(newRelation.get("type")) || newRelation.getMembersCount() == 0);
    233233    }

  • applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/SplittingMultipolygons.java

    r36102 r36217  
    1212import java.util.List;
    1313import java.util.Map;
     14import java.util.Map.Entry;
    1415import java.util.Objects;
    1516
    … …  
    351352                if (p instanceof Way && !p.equals(ring)) {
    352353                    for (OsmPrimitive r : p.getReferrers()) {
    353                         if (r instanceof Relation && r.hasKey("type") && r.get("type").equals("multipolygon")) {
     354                        if (r instanceof Relation && "multipolygon".equals(r.get("type"))) {
    354355                            if (touchingWays.containsKey(p)) {
    355356                                touchingWays.put((Way) p, Boolean.TRUE);
    … …  
    365366
    366367        List<TheRing> otherWays = new ArrayList<>();
    367         for (Way w : touchingWays.keySet()) {
    368             if (touchingWays.get(w)) {
    369                 otherWays.add(new TheRing(w));
     368        for (Entry<Way, Boolean> e : touchingWays.entrySet()) {
     369            if (Boolean.TRUE.equals(e.getValue())) {
     370                otherWays.add(new TheRing(e.getKey()));
    370371            }
    371372        }

  • applications/editors/josm/plugins/reltoolbox/src/relcontext/actions/TheRing.java

    r36134 r36217  
    1515import org.openstreetmap.josm.command.AddCommand;
    1616import org.openstreetmap.josm.command.ChangeCommand;
     17import org.openstreetmap.josm.command.ChangeMembersCommand;
     18import org.openstreetmap.josm.command.ChangeNodesCommand;
     19import org.openstreetmap.josm.command.ChangePropertyCommand;
    1720import org.openstreetmap.josm.command.Command;
    1821import org.openstreetmap.josm.command.DeleteCommand;
    … …  
    330333    public List<Command> getCommands(boolean createMultipolygon, Map<Relation, Relation> relationChangeMap) {
    331334        Way sourceCopy = new Way(source);
     335        Map<String, String> tagsToRemove = new HashMap<>();
    332336        if (createMultipolygon) {
    333337            Collection<String> linearTags = Config.getPref().getList(PREF_MULTIPOLY + "lineartags",
    … …  
    335339            relation = new Relation();
    336340            relation.put("type", "multipolygon");
    337             for (String key : sourceCopy.keySet()) {
    338                 if (linearTags.contains(key)) {
     341            for (String key : source.keySet()) {
     342                if (linearTags.contains(key)
     343                        || ("natural".equals(key) && "coastline".equals(source.get("natural"))))
    339344                    continue;
    340                 }
    341                 if ("natural".equals(key) && "coastline".equals(sourceCopy.get("natural"))) {
    342                     continue;
    343                 }
    344                 relation.put(key, sourceCopy.get(key));
     345               
     346                relation.put(key, source.get(key));
    345347                sourceCopy.remove(key);
     348                tagsToRemove.put(key, null);
    346349            }
    347350        }
    … …  
    354357                Relation rel;
    355358                if (relationChangeMap != null) {
    356                     if (relationChangeMap.containsKey(p)) {
    357                         rel = relationChangeMap.get(p);
    358                     } else {
     359                    rel = relationChangeMap.get(p);
     360                    if (rel == null) {
    359361                        rel = new Relation((Relation) p);
    360362                        relationChangeMap.put((Relation) p, rel);
    … …  
    362364                } else {
    363365                    rel = new Relation((Relation) p);
    364                     relationCommands.add(new ChangeCommand(p, rel));
     366                    relationCommands.add(new ChangeCommand(p, rel)); // should not happen
    365367                }
    366368                for (int i = 0; i < rel.getMembersCount(); i++) {
    … …  
    384386                if (createMultipolygon || !seg.getWayNodes().equals(source.getNodes())) {
    385387                    sourceCopy.setNodes(seg.getWayNodes());
    386                     commands.add(new ChangeCommand(source, sourceCopy));
     388                    if (!tagsToRemove.isEmpty()) {
     389                        commands.add(new ChangePropertyCommand(Collections.singleton(source), tagsToRemove));
     390                    }
     391                    if (!sourceCopy.getNodes().equals(source.getNodes()))
     392                        commands.add(new ChangeNodesCommand(source, sourceCopy.getNodes()));
    387393                }
    388394                foundOwnWay = true;
    … …  
    398404            }
    399405        }
     406        sourceCopy.setNodes(null); // see #19885
    400407        if (!foundOwnWay) {
    401408            final Command deleteCommand = DeleteCommand.delete(Collections.singleton(source));
    … …  
    413420    public static void updateCommandsWithRelations(List<Command> commands, Map<Relation, Relation> relationCache) {
    414421        for (Map.Entry<Relation, Relation> entry : relationCache.entrySet()) {
    415             commands.add(new ChangeCommand(entry.getKey(), entry.getValue()));
     422            Relation oldRel = entry.getKey();
     423            Relation newRel = entry.getValue();
     424            if (oldRel.getKeys().equals(newRel.getKeys())) {
     425                commands.add(new ChangeMembersCommand(oldRel, newRel.getMembers()));
     426                newRel.setMembers(null); // see #19885
     427            } else {
     428                commands.add(new ChangeCommand(oldRel, newRel)); // should not happen
     429            }
    416430        }
    417431    }

  • applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/AssociatedStreetFixer.java

    r36103 r36217  
    55
    66import java.util.ArrayList;
     7import java.util.Collections;
    78import java.util.HashMap;
    89import java.util.List;
    910import java.util.Map;
    1011
    11 import org.openstreetmap.josm.command.ChangeCommand;
     12import org.openstreetmap.josm.command.ChangeMembersCommand;
     13import org.openstreetmap.josm.command.ChangePropertyCommand;
    1214import org.openstreetmap.josm.command.Command;
    1315import org.openstreetmap.josm.command.SequenceCommand;
    … …  
    2325public class AssociatedStreetFixer extends RelationFixer {
    2426
     27    private static final String ADDR_HOUSENUMBER = "addr:housenumber";
     28    private static final String BUILDING = "building";
     29    private static final String HOUSE = "house";
     30    private static final String STREET = "street";
     31
    2532    public AssociatedStreetFixer() {
    2633        super("associatedStreet");
    … …  
    3037    public boolean isRelationGood(Relation rel) {
    3138        for (RelationMember m : rel.getMembers()) {
    32             if (m.getType().equals(OsmPrimitiveType.NODE) && !"house".equals(m.getRole())) {
     39            if (m.getType() == OsmPrimitiveType.NODE && !HOUSE.equals(m.getRole())) {
    3340                setWarningMessage(tr("Node without ''house'' role found"));
    3441                return false;
    3542            }
    36             if (m.getType().equals(OsmPrimitiveType.WAY) && !("house".equals(m.getRole()) || "street".equals(m.getRole()))) {
     43            if (m.getType() == OsmPrimitiveType.WAY && !(HOUSE.equals(m.getRole()) || STREET.equals(m.getRole()))) {
    3744                setWarningMessage(tr("Way without ''house'' or ''street'' role found"));
    3845                return false;
    3946            }
    40             if (m.getType().equals(OsmPrimitiveType.RELATION) && !"house".equals(m.getRole())) {
     47            if (m.getType() == OsmPrimitiveType.RELATION && !HOUSE.equals(m.getRole())) {
    4148                setWarningMessage(tr("Relation without ''house'' role found"));
    4249                return false;
    … …  
    5461        }
    5562        for (RelationMember m : rel.getMembers()) {
    56             if ("street".equals(m.getRole()) && !streetName.equals(m.getWay().get("name"))) {
     63            if (STREET.equals(m.getRole()) && !streetName.equals(m.getWay().get("name"))) {
    5764                String anotherName = m.getWay().get("name");
    5865                if (anotherName != null && !anotherName.isEmpty()) {
    … …  
    7279        // name - check which name is most used in street members and add to relation
    7380        // copy this name to the other street members (???)
    74         Relation rel = new Relation(source);
     81        List<RelationMember> members = source.getMembers();
    7582        boolean fixed = false;
    7683
    77         for (int i = 0; i < rel.getMembersCount(); i++) {
    78             RelationMember m = rel.getMember(i);
    79 
     84        for (int i = 0; i < members.size(); i++) {
     85            RelationMember m = members.get(i);
    8086            if (m.isNode()) {
    8187                Node node = m.getNode();
    82                 if (!"house".equals(m.getRole()) &&
    83                         (node.hasKey("building") || node.hasKey("addr:housenumber"))) {
     88                if (!HOUSE.equals(m.getRole()) &&
     89                        (node.hasKey(BUILDING) || node.hasKey(ADDR_HOUSENUMBER))) {
    8490                    fixed = true;
    85                     rel.setMember(i, new RelationMember("house", node));
     91                    members.set(i, new RelationMember(HOUSE, node));
    8692                }
    8793            } else if (m.isWay()) {
    8894                Way way = m.getWay();
    89                 if (!"street".equals(m.getRole()) && way.hasKey("highway")) {
     95                if (!STREET.equals(m.getRole()) && way.hasKey("highway")) {
    9096                    fixed = true;
    91                     rel.setMember(i, new RelationMember("street", way));
    92                 } else if (!"house".equals(m.getRole()) &&
    93                         (way.hasKey("building") || way.hasKey("addr:housenumber"))) {
     97                    members.set(i, new RelationMember(STREET, way));
     98                } else if (!HOUSE.equals(m.getRole()) &&
     99                        (way.hasKey(BUILDING) || way.hasKey(ADDR_HOUSENUMBER))) {
    94100                    fixed = true;
    95                     rel.setMember(i, new RelationMember("house", way));
     101                    members.set(i, new RelationMember(HOUSE, way));
    96102                }
    97103            } else if (m.isRelation()) {
    98104                Relation relation = m.getRelation();
    99                 if (!"house".equals(m.getRole()) &&
    100                         (relation.hasKey("building") || relation.hasKey("addr:housenumber") || "multipolygon".equals(relation.get("type")))) {
     105                if (!HOUSE.equals(m.getRole()) &&
     106                        (relation.hasKey(BUILDING) || relation.hasKey(ADDR_HOUSENUMBER) || "multipolygon".equals(relation.get("type")))) {
    101107                    fixed = true;
    102                     rel.setMember(i, new RelationMember("house", relation));
     108                    members.set(i, new RelationMember(HOUSE, relation));
    103109                }
    104110            }
    … …  
    107113        // fill relation name
    108114        Map<String, Integer> streetNames = new HashMap<>();
    109         for (RelationMember m : rel.getMembers()) {
    110             if ("street".equals(m.getRole()) && m.isWay()) {
     115        for (RelationMember m : members) {
     116            if (STREET.equals(m.getRole()) && m.isWay()) {
    111117                String name = m.getWay().get("name");
    112118                if (name == null || name.isEmpty()) {
    113119                    continue;
    114120                }
    115 
    116121                Integer count = streetNames.get(name);
    117 
    118122                streetNames.put(name, count != null ? count + 1 : 1);
    119123            }
    … …  
    128132        }
    129133
    130         if (!rel.hasKey("name") && !commonName.isEmpty()) {
    131             fixed = true;
    132             rel.put("name", commonName);
    133         } else {
    134             commonName = ""; // set empty common name - if we already have name on relation, do not overwrite it
     134        Map<String, String> nameTag = new HashMap<>();
     135        if (!source.hasKey("name") && !commonName.isEmpty()) {
     136            nameTag.put("name", commonName);
    135137        }
    136138
    137139        List<Command> commandList = new ArrayList<>();
     140        final DataSet ds = Utils.firstNonNull(source.getDataSet(), MainApplication.getLayerManager().getEditDataSet());
    138141        if (fixed) {
    139             final DataSet ds = Utils.firstNonNull(source.getDataSet(), MainApplication.getLayerManager().getEditDataSet());
    140             commandList.add(new ChangeCommand(ds, source, rel));
     142            commandList.add(new ChangeMembersCommand(ds, source, members));
     143        }
     144        if (!nameTag.isEmpty()) {
     145            commandList.add(new ChangePropertyCommand(ds, Collections.singleton(source), nameTag));
    141146        }
    142147
    143         /*if (!commonName.isEmpty())
    144         // fill common name to streets
    145         for (RelationMember m : rel.getMembers())
    146             if ("street".equals(m.getRole()) && m.isWay()) {
    147                 String name = m.getWay().get("name");
    148                 if (commonName.equals(name)) continue;
    149 
    150                 // TODO: ask user if he really wants to overwrite street name??
    151 
    152                 Way oldWay = m.getWay();
    153                 Way newWay = new Way(oldWay);
    154                 newWay.put("name", commonName);
    155 
    156                 commandList.add(new ChangeCommand(MainApplication.getLayerManager().getEditDataSet(), oldWay, newWay));
    157             }
    158          */
    159148        // return results
    160         if (commandList.isEmpty()) {
     149        if (commandList.isEmpty())
    161150            return null;
    162         }
    163151        return SequenceCommand.wrapIfNeeded(tr("fix associatedStreet relation"), commandList);
    164152    }

  • applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/BoundaryFixer.java

    r36103 r36217  
    44import static org.openstreetmap.josm.tools.I18n.tr;
    55
    6 import org.openstreetmap.josm.command.ChangeCommand;
     6import java.util.ArrayList;
     7import java.util.List;
     8
     9import org.openstreetmap.josm.command.ChangeMembersCommand;
    710import org.openstreetmap.josm.command.Command;
    811import org.openstreetmap.josm.data.osm.DataSet;
    … …  
    3740    public boolean isRelationGood(Relation rel) {
    3841        for (RelationMember m : rel.getMembers()) {
    39             if (m.getType().equals(OsmPrimitiveType.RELATION) && !"subarea".equals(m.getRole())) {
     42            if (m.getType() == OsmPrimitiveType.RELATION && !"subarea".equals(m.getRole())) {
    4043                setWarningMessage(tr("Relation without ''subarea'' role found"));
    4144                return false;
    4245            }
    43             if (m.getType().equals(OsmPrimitiveType.NODE) && !("label".equals(m.getRole()) || "admin_centre".equals(m.getRole()))) {
     46            if (m.getType() == OsmPrimitiveType.NODE && !("label".equals(m.getRole()) || "admin_centre".equals(m.getRole()))) {
    4447                setWarningMessage(tr("Node without ''label'' or ''admin_centre'' role found"));
    4548                return false;
    4649            }
    47             if (m.getType().equals(OsmPrimitiveType.WAY) && !("outer".equals(m.getRole()) || "inner".equals(m.getRole()))) {
     50            if (m.getType() == OsmPrimitiveType.WAY && !("outer".equals(m.getRole()) || "inner".equals(m.getRole()))) {
    4851                setWarningMessage(tr("Way without ''inner'' or ''outer'' role found"));
    4952                return false;
    … …  
    5659    @Override
    5760    public Command fixRelation(Relation rel) {
    58         Relation r = rel;
    59         Relation rr = fixMultipolygonRoles(r);
    60         boolean fixed = false;
    61         if (rr != null) {
    62             fixed = true;
    63             r = rr;
     61        List<RelationMember> members = fixMultipolygonRoles(rel.getMembers());
     62        if (members.isEmpty()) {
     63            members = rel.getMembers();
    6464        }
    65         rr = fixBoundaryRoles(r);
    66         if (rr != null) {
    67             fixed = true;
    68             r = rr;
    69         }
    70         if (fixed) {
     65        members = fixBoundaryRoles(members);
     66        if (!members.equals(rel.getMembers())) {
    7167            final DataSet ds = Utils.firstNonNull(rel.getDataSet(), MainApplication.getLayerManager().getEditDataSet());
    72             return new ChangeCommand(ds, rel, r);
     68            return new ChangeMembersCommand(ds, rel, members);
    7369        }
    7470        return null;
    7571    }
    7672
    77     private Relation fixBoundaryRoles(Relation source) {
    78         Relation r = new Relation(source);
    79         boolean fixed = false;
    80         for (int i = 0; i < r.getMembersCount(); i++) {
    81             RelationMember m = r.getMember(i);
     73    /**
     74     * Possibly change roles of non-way members.
     75     * @param origMembers original list of relation members
     76     * @return either the original and unmodified list or a new one with at least one new item
     77     */
     78    private static List<RelationMember> fixBoundaryRoles(List<RelationMember> origMembers) {
     79        List<RelationMember> members = origMembers;
     80        for (int i = 0; i < members.size(); i++) {
     81            RelationMember m = members.get(i);
    8282            String role = null;
    8383            if (m.isRelation()) {
    8484                role = "subarea";
    85             } else if (m.isNode()) {
     85            } else if (m.isNode() && !m.getMember().isIncomplete()) {
    8686                Node n = (Node) m.getMember();
    87                 if (!n.isIncomplete()) {
    88                     if (n.hasKey("place")) {
    89                         String place = n.get("place");
    90                         if (place.equals("state") || place.equals("country") ||
    91                                 place.equals("county") || place.equals("region")) {
    92                             role = "label";
    93                         } else {
    94                             role = "admin_centre";
    95                         }
     87                if (n.hasKey("place")) {
     88                    String place = n.get("place");
     89                    if ("state".equals(place) || "country".equals(place) ||
     90                            "county".equals(place) || "region".equals(place)) {
     91                        role = "label";
    9692                    } else {
    97                         role = "label";
     93                        role = "admin_centre";
    9894                    }
     95                } else {
     96                    role = "label";
    9997                }
    10098            }
    10199            if (role != null && !role.equals(m.getRole())) {
    102                 r.setMember(i, new RelationMember(role, m.getMember()));
    103                 fixed = true;
     100                if (members == origMembers) {
     101                    members = new ArrayList<>(origMembers); // don't modify original list
     102                }
     103                members.set(i, new RelationMember(role, m.getMember()));
    104104            }
    105105        }
    106         return fixed ? r : null;
     106        return members;
    107107    }
    108108}

  • applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/MultipolygonFixer.java

    r36103 r36217  
    55
    66import java.util.ArrayList;
    7 import java.util.Collection;
     7import java.util.Collections;
    88import java.util.HashSet;
     9import java.util.List;
    910import java.util.Set;
     11import java.util.stream.Collectors;
    1012
    11 import org.openstreetmap.josm.command.ChangeCommand;
     13import org.openstreetmap.josm.command.ChangeMembersCommand;
    1214import org.openstreetmap.josm.command.Command;
    1315import org.openstreetmap.josm.data.osm.DataSet;
    … …  
    2931    }
    3032
    31     protected MultipolygonFixer(String...types) {
     33    protected MultipolygonFixer(String... types) {
    3234        super(types);
    3335    }
    … …  
    3638    public boolean isRelationGood(Relation rel) {
    3739        for (RelationMember m : rel.getMembers()) {
    38             if (m.getType().equals(OsmPrimitiveType.WAY) && !("outer".equals(m.getRole()) || "inner".equals(m.getRole()))) {
     40            if (m.getType() == OsmPrimitiveType.WAY && !("outer".equals(m.getRole()) || "inner".equals(m.getRole()))) {
    3941                setWarningMessage(tr("Way without ''inner'' or ''outer'' role found"));
    4042                return false;
    … …  
    4749    @Override
    4850    public Command fixRelation(Relation rel) {
    49         Relation rr = fixMultipolygonRoles(rel);
    50         if (rr != null) {
     51        List<RelationMember> members = fixMultipolygonRoles(rel.getMembers());
     52        if (!members.equals(rel.getMembers())) {
    5153            final DataSet ds = Utils.firstNonNull(rel.getDataSet(), MainApplication.getLayerManager().getEditDataSet());
    52             return new ChangeCommand(ds, rel, rr);
     54            return new ChangeMembersCommand(ds, rel, members);
    5355        }
    5456        return null;
    … …  
    5658
    5759    /**
    58      * Basically, created multipolygon from scratch, and if successful, replace roles with new ones.
     60     * Basically, create member list of a multipolygon from scratch, and if
     61     * successful, replace roles with new ones.
     62     *
     63     * @param origMembers original list of relation members
     64     * @return either the original and unmodified list or a new one with at least
     65     *         one new item or the empty list if the way members don't build a
     66     *         correct multipolygon
    5967     */
    60     protected Relation fixMultipolygonRoles(Relation source) {
    61         Collection<Way> ways = new ArrayList<>(source.getMemberPrimitives(Way.class));
     68    protected List<RelationMember> fixMultipolygonRoles(final List<RelationMember> origMembers) {
     69        List<Way> ways = origMembers.stream().filter(m -> m.isWay()).map(m -> m.getWay()).collect(Collectors.toList());
     70
    6271        MultipolygonBuilder mpc = new MultipolygonBuilder();
    6372        String error = mpc.makeFromWays(ways);
    6473        if (error != null)
    65             return null;
     74            return Collections.emptyList();
    6675
    67         Relation r = new Relation(source);
    68         boolean fixed = false;
    6976        Set<Way> outerWays = new HashSet<>();
    7077        for (MultipolygonBuilder.JoinedPolygon poly : mpc.outerWays) {
    … …  
    7582            innerWays.addAll(poly.ways);
    7683        }
    77         for (int i = 0; i < r.getMembersCount(); i++) {
    78             RelationMember m = r.getMember(i);
     84        List<RelationMember> members = origMembers;
     85        for (int i = 0; i < members.size(); i++) {
     86            RelationMember m = members.get(i);
    7987            if (m.isWay()) {
    8088                final Way way = m.getWay();
    … …  
    8694                }
    8795                if (role != null && !role.equals(m.getRole())) {
    88                     r.setMember(i, new RelationMember(role, way));
    89                     fixed = true;
     96                    if (members == origMembers) {
     97                        members = new ArrayList<>(origMembers); // don't modify original list
     98                    }
     99                    members.set(i, new RelationMember(role, way));
    90100                }
    91101            }
    92102        }
    93         return fixed ? r : null;
     103        return members;
    94104    }
    95105}

  • applications/editors/josm/plugins/reltoolbox/src/relcontext/relationfix/PublicTransportFixer.java

    r36103 r36217  
    44import static org.openstreetmap.josm.tools.I18n.tr;
    55
    6 import org.openstreetmap.josm.command.ChangeCommand;
     6import java.util.ArrayList;
     7import java.util.List;
     8
     9import org.openstreetmap.josm.command.ChangeMembersCommand;
    710import org.openstreetmap.josm.command.Command;
    811import org.openstreetmap.josm.data.osm.DataSet;
    … …  
    1114import org.openstreetmap.josm.data.osm.RelationMember;
    1215import org.openstreetmap.josm.gui.MainApplication;
     16import org.openstreetmap.josm.tools.Utils;
    1317
    14 import org.openstreetmap.josm.tools.Utils;
    1518import relcontext.actions.PublicTransportHelper;
    1619
    … …  
    2629    }
    2730
    28     /*protected PublicTransportFixer(String...types) {
    29         super(types);
    30     }*/
    31 
    3231    @Override
    3332    public boolean isRelationGood(Relation rel) {
    3433        for (RelationMember m : rel.getMembers()) {
    35             if (m.getType().equals(OsmPrimitiveType.NODE)
     34            if (m.getType() == OsmPrimitiveType.NODE
    3635                    && !(m.getRole().startsWith(PublicTransportHelper.STOP) || m.getRole().startsWith(PublicTransportHelper.PLATFORM))) {
    3736                setWarningMessage(tr("Node without ''stop'' or ''platform'' role found"));
    3837                return false;
    3938            }
    40             if (m.getType().equals(OsmPrimitiveType.WAY)
     39            if (m.getType() == OsmPrimitiveType.WAY
    4140                    && PublicTransportHelper.isWayPlatform(m)
    4241                    && !m.getRole().startsWith(PublicTransportHelper.PLATFORM)) {
    … …  
    4948    }
    5049
    51     /*@Override
    52     public boolean isFixerApplicable(Relation rel) {
    53         return true;
    54     }*/
    55 
    5650    @Override
    5751    public Command fixRelation(Relation rel) {
    58         Relation r = rel;
    59         Relation rr = fixStopPlatformRole(r);
    60         boolean fixed = false;
    61         if (rr != null) {
    62             fixed = true;
    63             r = rr;
    64         }
    65         if (fixed) {
     52        List<RelationMember> members = fixStopPlatformRole(rel.getMembers());
     53        if (!members.equals(rel.getMembers())) {
    6654            final DataSet ds = Utils.firstNonNull(rel.getDataSet(), MainApplication.getLayerManager().getEditDataSet());
    67             return new ChangeCommand(ds, rel, r);
     55            return new ChangeMembersCommand(ds, rel, rel.getMembers());
    6856        }
    6957        return null;
    7058    }
    7159
    72     private Relation fixStopPlatformRole(Relation source) {
    73         Relation r = new Relation(source);
    74         boolean fixed = false;
    75         for (int i = 0; i < r.getMembersCount(); i++) {
    76             RelationMember m = r.getMember(i);
     60    /**
     61     * Fix roles of members.
     62     * @param origMembers original list of relation members
     63     * @return either the original and unmodified list or a new one with at least one new item
     64     */
     65    private static List<RelationMember> fixStopPlatformRole(List<RelationMember> origMembers) {
     66        List<RelationMember> members = origMembers;
     67        for (int i = 0; i < members.size(); i++) {
     68            RelationMember m = members.get(i);
    7769            String role = PublicTransportHelper.getRoleByMember(m);
    7870
    7971            if (role != null && !m.getRole().startsWith(role)) {
    80                 r.setMember(i, new RelationMember(role, m.getMember()));
    81                 fixed = true;
     72                if (members == origMembers) {
     73                    members = new ArrayList<>(origMembers);
     74                }
     75                members.set(i, new RelationMember(role, m.getMember()));
    8276            }
    8377        }
    84         return fixed ? r : null;
     78        return members;
    8579    }
    8680}
Note: See TracChangeset for help on using the changeset viewer.