Changeset 1656 in josm


Ignore:
Timestamp:
2009-06-08T21:30:19+02:00 (12 years ago)
Author:
Gubaer
Message:

fixed #2707: Deleting a way with new nodes produces invalid osm file

File:
1 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/org/openstreetmap/josm/command/DeleteCommand.java

    r1640 r1656  
    119119    public static Command deleteWithReferences(Collection<? extends OsmPrimitive> selection) {
    120120        CollectBackReferencesVisitor v = new CollectBackReferencesVisitor(Main.ds);
    121         for (OsmPrimitive osm : selection)
     121        for (OsmPrimitive osm : selection) {
    122122            osm.visit(v);
     123        }
    123124        v.data.addAll(selection);
    124125        if (v.data.isEmpty())
     
    141142            }
    142143        }
    143         if (role.length() > 0) {
    144             return new ExtendedDialog(Main.parent, 
    145                         tr("Conflicting relation"),
    146                         tr("Selection \"{0}\" is used by relation \"{1}\" with role {2}.\nDelete from relation?",
     144        if (role.length() > 0)
     145            return new ExtendedDialog(Main.parent,
     146                    tr("Conflicting relation"),
     147                    tr("Selection \"{0}\" is used by relation \"{1}\" with role {2}.\nDelete from relation?",
    147148                            s.name, n.name, role),
    148                         new String[] {tr("Delete from relation"), tr("Cancel")},
    149                         new String[] {"dialogs/delete.png", "cancel.png"}).getValue(); 
    150         } else {
    151             return new ExtendedDialog(Main.parent, 
    152                         tr("Conflicting relation"),
    153                         tr("Selection \"{0}\" is used by relation \"{1}\".\nDelete from relation?",
     149                            new String[] {tr("Delete from relation"), tr("Cancel")},
     150                            new String[] {"dialogs/delete.png", "cancel.png"}).getValue();
     151        else
     152            return new ExtendedDialog(Main.parent,
     153                    tr("Conflicting relation"),
     154                    tr("Selection \"{0}\" is used by relation \"{1}\".\nDelete from relation?",
    154155                            s.name, n.name),
    155                         new String[] {tr("Delete from relation"), tr("Cancel")},
    156                         new String[] {"dialogs/delete.png", "cancel.png"}).getValue(); 
    157         }
     156                            new String[] {tr("Delete from relation"), tr("Cancel")},
     157                            new String[] {"dialogs/delete.png", "cancel.png"}).getValue();
    158158    }
    159159
     
    210210            osm.visit(v);
    211211            for (OsmPrimitive ref : v.data) {
    212                 if (del.contains(ref))
     212                if (del.contains(ref)) {
    213213                    continue;
     214                }
    214215                if (ref instanceof Way) {
    215216                    waysToBeChanged.add((Way) ref);
     
    217218                    if (testRelation((Relation) ref, osm) == 1) {
    218219                        Collection<OsmPrimitive> relset = relationsToBeChanged.get(ref);
    219                         if (relset == null)
     220                        if (relset == null) {
    220221                            relset = new HashSet<OsmPrimitive>();
     222                        }
    221223                        relset.add(osm);
    222224                        relationsToBeChanged.put(ref, relset);
    223225                    } else
    224226                        return null;
    225                 } else {
     227                } else
    226228                    return null;
    227                 }
    228229            }
    229230        }
     
    239240                w.visit(v);
    240241                for (OsmPrimitive ref : v.data) {
    241                     if (del.contains(ref))
     242                    if (del.contains(ref)) {
    242243                        continue;
     244                    }
    243245                    if (ref instanceof Relation) {
    244246                        Boolean found = false;
    245247                        Collection<OsmPrimitive> relset = relationsToBeChanged.get(ref);
    246                         if (relset == null)
     248                        if (relset == null) {
    247249                            relset = new HashSet<OsmPrimitive>();
    248                         else {
     250                        } else {
    249251                            for (OsmPrimitive m : relset) {
    250252                                if (m == w) {
     
    261263                                return null;
    262264                        }
    263                     } else {
     265                    } else
    264266                        return null;
    265                     }
    266267                }
    267268            } else {
     
    288289        }
    289290
    290         if (!del.isEmpty())
     291        // #2707: ways to be deleted can include new nodes (with node.id == 0).
     292        // Remove them from the way before the way is deleted. Otherwise the
     293        // deleted way is saved (or sent to the API) with a dangling reference to a node
     294        // Example:
     295        //  <node id='2' action='delete' visible='true' version='1' ... />
     296        //  <node id='1' action='delete' visible='true' version='1' ... />
     297        //  <!-- missing node with id -1 because new deleted nodes are not persisted -->
     298        //   <way id='3' action='delete' visible='true' version='1'>
     299        //     <nd ref='1' />
     300        //     <nd ref='-1' />  <!-- heres the problem -->
     301        //     <nd ref='2' />
     302        //   </way>
     303        for (OsmPrimitive primitive : del) {
     304            if (! (primitive instanceof Way)) {
     305                continue;
     306            }
     307            Way w = (Way)primitive;
     308            if (w.id == 0) { // new ways with id == 0 are fine,
     309                continue;    // process existing ways only
     310            }
     311            Way wnew = new Way(w);
     312            ArrayList<Node> nodesToStrip = new ArrayList<Node>();
     313            // lookup new nodes which have been added to the set of deleted
     314            // nodes ...
     315            for (Node n : wnew.nodes) {
     316                if (n.id == 0 && del.contains(n)) {
     317                    nodesToStrip.add(n);
     318                }
     319            }
     320            // .. and remove them from the way
     321            //
     322            wnew.nodes.removeAll(nodesToStrip);
     323            if (!nodesToStrip.isEmpty()) {
     324                cmds.add(new ChangeCommand(w,wnew));
     325            }
     326        }
     327
     328        if (!del.isEmpty()) {
    291329            cmds.add(new DeleteCommand(del));
     330        }
    292331
    293332        return new SequenceCommand(tr("Delete"), cmds);
     
    300339        n2.addAll(ws.way.nodes.subList(ws.lowerIndex + 1, ws.way.nodes.size()));
    301340
    302         if (n1.size() < 2 && n2.size() < 2) {
     341        if (n1.size() < 2 && n2.size() < 2)
    303342            return new DeleteCommand(Collections.singleton(ws.way));
    304         }
    305343
    306344        Way wnew = new Way(ws.way);
     
    343381                        JPanel msg = new JPanel(new GridBagLayout());
    344382                        msg.add(new JLabel(
    345                             "<html>" +
    346                             // leave message in one tr() as there is a grammatical connection.
    347                             tr("You are about to delete nodes outside of the area you have downloaded." +
    348                             "<br>" +
    349                             "This can cause problems because other objects (that you don't see) might use them." +
    350                             "<br>" +
    351                             "Do you really want to delete?") + "</html>"));
     383                                "<html>" +
     384                                // leave message in one tr() as there is a grammatical connection.
     385                                tr("You are about to delete nodes outside of the area you have downloaded." +
     386                                        "<br>" +
     387                                        "This can cause problems because other objects (that you don't see) might use them." +
     388                                        "<br>" +
     389                                "Do you really want to delete?") + "</html>"));
    352390                        return DontShowAgainInfo.show("delete_outside_nodes", msg, false, JOptionPane.YES_NO_OPTION, JOptionPane.YES_OPTION);
    353391                    }
Note: See TracChangeset for help on using the changeset viewer.