Ignore:
Timestamp:
2009-10-25T12:05:31+01:00 (14 years ago)
Author:
Gubaer
Message:

fixed #3762: Deleted relation still referenced when deleting former member
Clean up of Delete command. New: only one confirmation dialog for all parent relations of deleted objects, see help?.
Improved infrastructure for context-sensitive help, improved internal help browser.

Location:
trunk/src/org/openstreetmap/josm/command
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/org/openstreetmap/josm/command/Command.java

    r2284 r2308  
    33
    44import java.util.Collection;
     5import static org.openstreetmap.josm.tools.I18n.tr;
    56import java.util.HashMap;
    67import java.util.HashSet;
    … …  
    5859     * Creates a new command in the context of a specific data layer
    5960     *
    60      * @param layer the data layer
     61     * @param layer the data layer. Must not be null.
     62     * @throws IllegalArgumentException thrown if layer is null
    6163     */
    62     public Command(OsmDataLayer layer) {
     64    public Command(OsmDataLayer layer) throws IllegalArgumentException {
     65        if (layer == null)
     66            throw new IllegalArgumentException(tr("Parameter ''{0}'' must not be null", "layer"));
    6367        this.layer = layer;
    6468    }

  • trunk/src/org/openstreetmap/josm/command/DeleteCommand.java

    r2296 r2308  
    1010import java.util.Collection;
    1111import java.util.Collections;
    12 import java.util.HashMap;
    1312import java.util.HashSet;
    1413import java.util.Iterator;
    … …  
    2423
    2524import org.openstreetmap.josm.Main;
     25import org.openstreetmap.josm.data.osm.BackreferencedDataSet;
    2626import org.openstreetmap.josm.data.osm.Node;
    2727import org.openstreetmap.josm.data.osm.OsmPrimitive;
    … …  
    3131import org.openstreetmap.josm.data.osm.Way;
    3232import org.openstreetmap.josm.data.osm.WaySegment;
     33import org.openstreetmap.josm.data.osm.BackreferencedDataSet.RelationToChildReference;
    3334import org.openstreetmap.josm.data.osm.visitor.CollectBackReferencesVisitor;
    3435import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil;
    3536import org.openstreetmap.josm.gui.DefaultNameFormatter;
    3637import org.openstreetmap.josm.gui.ExtendedDialog;
     38import org.openstreetmap.josm.gui.actionsupport.DeleteFromRelationConfirmationDialog;
    3739import org.openstreetmap.josm.gui.layer.OsmDataLayer;
    3840import org.openstreetmap.josm.tools.ImageProvider;
    3941
     42import sun.swing.BakedArrayList;
     43
    4044/**
    4145 * A command to delete a number of primitives from the dataset.
    42  * @author imi
     46
    4347 */
    4448public class DeleteCommand extends Command {
    … …  
    4953
    5054    /**
    51      * Constructor for a collection of data
     55     * Constructor. Deletes a collection of primitives in the current edit layer.
     56     *
     57     * @param data the primitives to delete
    5258     */
    5359    public DeleteCommand(Collection<? extends OsmPrimitive> data) {
    54         super();
     60        if (data == null) {
     61            data = Collections.emptyList();
     62        }
    5563        this.toDelete = data;
     64    }
     65
     66    /**
     67     * Constructor. Deletes a single primitive in the current edit layer.
     68     *
     69     * @param data  the primitive to delete. Must not be null.
     70     * @throws IllegalArgumentException thrown if data is null
     71     */
     72    public DeleteCommand(OsmPrimitive data) throws IllegalArgumentException {
     73        if (data == null)
     74            throw new IllegalArgumentException(tr("Parameter ''{0}'' must not be null", "data"));
     75        this.toDelete = Collections.singleton(data);
    5676    }
    5777
    … …  
    5979     * Constructor for a single data item. Use the collection constructor to delete multiple
    6080     * objects.
    61      */
    62     public DeleteCommand(OsmPrimitive data) {
    63         this.toDelete = Collections.singleton(data);
    64     }
    65 
    66     /**
    67      * Constructor for a single data item. Use the collection constructor to delete multiple
    68      * objects.
    69      *
    70      * @param layer the layer context for deleting this primitive
    71      * @param data the primitive to delete
    72      */
    73     public DeleteCommand(OsmDataLayer layer, OsmPrimitive data) {
     81     *
     82     * @param layer the layer context for deleting this primitive. Must not be null.
     83     * @param data the primitive to delete. Must not be null.
     84     * @throws IllegalArgumentException thrown if data is null
     85     * @throws IllegalArgumentException thrown if layer is null
     86     */
     87    public DeleteCommand(OsmDataLayer layer, OsmPrimitive data) throws IllegalArgumentException {
    7488        super(layer);
     89        if (data == null)
     90            throw new IllegalArgumentException(tr("Parameter ''{0}'' must not be null", "data"));
    7591        this.toDelete = Collections.singleton(data);
    7692    }
    … …  
    8096     * a specific layer
    8197     *
    82      * @param layer the layer context for deleting these primitives
     98     * @param layer the layer context for deleting these primitives. Must not be null.
    8399     * @param data the primitives to delete
    84      */
    85     public DeleteCommand(OsmDataLayer layer, Collection<? extends OsmPrimitive> data) {
     100     * @throws IllegalArgumentException thrown if layer is null
     101     */
     102    public DeleteCommand(OsmDataLayer layer, Collection<? extends OsmPrimitive> data) throws IllegalArgumentException{
    86103        super(layer);
     104        if (data == null) {
     105            data = Collections.emptyList();
     106        }
    87107        this.toDelete = data;
     108    }
     109
     110    protected void removeNewNodesFromDeletedWay(Way w) {
     111        // #2707: ways to be deleted can include new nodes (with node.id == 0).
     112        // Remove them from the way before the way is deleted. Otherwise the
     113        // deleted way is saved (or sent to the API) with a dangling reference to a node
     114        // Example:
     115        // <node id='2' action='delete' visible='true' version='1' ... />
     116        // <node id='1' action='delete' visible='true' version='1' ... />
     117        // <!-- missing node with id -1 because new deleted nodes are not persisted -->
     118        // <way id='3' action='delete' visible='true' version='1'>
     119        // <nd ref='1' />
     120        // <nd ref='-1' /> <!-- here's the problem -->
     121        // <nd ref='2' />
     122        // </way>
     123        if (w.isNew())
     124            return; // process existing ways only
     125        List<Node> nodesToKeep = new ArrayList<Node>();
     126        // lookup new nodes which have been added to the set of deleted
     127        // nodes ...
     128        Iterator<Node> it = nodesToKeep.iterator();
     129        while(it.hasNext()) {
     130            Node n = it.next();
     131            if (n.isNew()) {
     132                it.remove();
     133            }
     134        }
     135        w.setNodes(nodesToKeep);
    88136    }
    89137
    … …  
    93141        for (OsmPrimitive osm : toDelete) {
    94142            osm.setDeleted(true);
     143            if (osm instanceof Way) {
     144                removeNewNodesFromDeletedWay((Way)osm);
     145            }
    95146        }
    96147        return true;
    … …  
    109160            String msg = "";
    110161            switch(OsmPrimitiveType.from(primitive)) {
    111             case NODE: msg = "Delete node {0}"; break;
    112             case WAY: msg = "Delete way {0}"; break;
    113             case RELATION:msg = "Delete relation {0}"; break;
     162                case NODE: msg = "Delete node {0}"; break;
     163                case WAY: msg = "Delete way {0}"; break;
     164                case RELATION:msg = "Delete relation {0}"; break;
    114165            }
    115166
    … …  
    130181            apiname = t.getAPIName();
    131182            switch(t) {
    132             case NODE: msg = trn("Delete {0} node", "Delete {0} nodes", toDelete.size(), toDelete.size()); break;
    133             case WAY: msg = trn("Delete {0} way", "Delete {0} ways", toDelete.size(), toDelete.size()); break;
    134             case RELATION: msg = trn("Delete {0} relation", "Delete {0} relations", toDelete.size(), toDelete.size()); break;
     183                case NODE: msg = trn("Delete {0} node", "Delete {0} nodes", toDelete.size(), toDelete.size()); break;
     184                case WAY: msg = trn("Delete {0} way", "Delete {0} ways", toDelete.size(), toDelete.size()); break;
     185                case RELATION: msg = trn("Delete {0} relation", "Delete {0} relations", toDelete.size(), toDelete.size()); break;
    135186            }
    136187        }
    … …  
    156207     * If a way is deleted, only the way and no nodes are deleted.
    157208     *
    158      * @param layer
     209     * @param layer the {@see OsmDataLayer} in whose context primitives are deleted. Must not be null.
    159210     * @param selection The list of all object to be deleted.
    160      * @param simulate  Set to true if the user should not be bugged with additional dialogs
     211     * @param silent  Set to true if the user should not be bugged with additional dialogs
    161212     * @return command A command to perform the deletions, or null of there is nothing to delete.
    162      */
    163     public static Command deleteWithReferences(OsmDataLayer layer, Collection<? extends OsmPrimitive> selection, boolean simulate) {
     213     * @throws IllegalArgumentException thrown if layer is null
     214     */
     215    public static Command deleteWithReferences(OsmDataLayer layer, Collection<? extends OsmPrimitive> selection, boolean silent) throws IllegalArgumentException {
     216        if (layer == null)
     217            throw new IllegalArgumentException(tr("Parameter ''{0}'' must not be null", "layer"));
     218        if (selection == null || selection.isEmpty()) return null;
    164219        CollectBackReferencesVisitor v = new CollectBackReferencesVisitor(layer.data);
    165220        v.initialize();
    … …  
    170225        if (v.getData().isEmpty())
    171226            return null;
    172         if (!checkAndConfirmOutlyingDeletes(layer,v.getData()) && !simulate)
     227        if (!checkAndConfirmOutlyingDeletes(layer,v.getData()) && !silent)
    173228            return null;
    174229        return new DeleteCommand(layer,v.getData());
    … …  
    233288     *    <li>it is not referred to by other non-deleted primitives outside of  <code>primitivesToDelete</code></li>
    234289     * <ul>
     290     * @param backreferences backreference data structure
    235291     * @param layer  the layer in whose context primitives are deleted
    236292     * @param primitivesToDelete  the primitives to delete
    … …  
    238294     * can be deleted too
    239295     */
    240     protected static Collection<Node> computeNodesToDelete(OsmDataLayer layer, Collection<OsmPrimitive> primitivesToDelete) {
     296    protected static Collection<Node> computeNodesToDelete(BackreferencedDataSet backreferences, OsmDataLayer layer, Collection<OsmPrimitive> primitivesToDelete) {
    241297        Collection<Node> nodesToDelete = new HashSet<Node>();
    242         CollectBackReferencesVisitor v = new CollectBackReferencesVisitor(layer.data, false);
    243         for (OsmPrimitive osm : primitivesToDelete) {
    244             if (! (osm instanceof Way) ) {
    245                 continue;
    246             }
    247             for (Node n : ((Way) osm).getNodes()) {
     298        //CollectBackReferencesVisitor v = new CollectBackReferencesVisitor(layer.data, false);
     299        for (Way way : OsmPrimitive.getFilteredList(primitivesToDelete, Way.class)) {
     300            for (Node n : way.getNodes()) {
    248301                if (n.isTagged()) {
    249302                    continue;
    250303                }
    251                 v.initialize();
    252                 n.visit(v);
    253                 Collection<OsmPrimitive> referringPrimitives = v.getData();
     304                //v.initialize();
     305                //n.visit(v);
     306                Collection<OsmPrimitive> referringPrimitives = backreferences.getParents(n);
    254307                referringPrimitives.removeAll(primitivesToDelete);
    255308                int count = 0;
    … …  
    276329     * they are part of a relation, inform the user and do not delete.
    277330     *
    278      * @param layer the {@see OsmDataLayer} in whose context a primitive the primitives are deleted
    279      * @param selection The objects to delete.
     331     * @param layer the {@see OsmDataLayer} in whose context the primitives are deleted
     332     * @param selection the objects to delete.
    280333     * @param alsoDeleteNodesInWay <code>true</code> if nodes should be deleted as well
    281      * @param simulate Set to true if the user should not be bugged with additional questions
    282334     * @return command a command to perform the deletions, or null if there is nothing to delete.
    283335     */
    284336    public static Command delete(OsmDataLayer layer, Collection<? extends OsmPrimitive> selection,
    285337            boolean alsoDeleteNodesInWay) {
    286         return delete(layer, selection, alsoDeleteNodesInWay, false);
    287     }
    288 
     338        return delete(layer, selection, alsoDeleteNodesInWay, false /* not silent */);
     339    }
     340
     341    /**
     342     * Try to delete all given primitives.
     343     *
     344     * If a node is used by a way, it's removed from that way. If a node or a way is used by a
     345     * relation, inform the user and do not delete.
     346     *
     347     * If this would cause ways with less than 2 nodes to be created, delete these ways instead. If
     348     * they are part of a relation, inform the user and do not delete.
     349     *
     350     * @param layer the {@see OsmDataLayer} in whose context the primitives are deleted
     351     * @param selection the objects to delete.
     352     * @param alsoDeleteNodesInWay <code>true</code> if nodes should be deleted as well
     353     * @param silent set to true if the user should not be bugged with additional questions
     354     * @return command a command to perform the deletions, or null if there is nothing to delete.
     355     */
    289356    public static Command delete(OsmDataLayer layer, Collection<? extends OsmPrimitive> selection,
    290             boolean alsoDeleteNodesInWay, boolean simulate) {
    291         if (selection.isEmpty())
     357            boolean alsoDeleteNodesInWay, boolean silent) {
     358        if (selection == null || selection.isEmpty())
    292359            return null;
    293360
    294         Collection<OsmPrimitive> primitivesToDelete = new HashSet<OsmPrimitive>(selection);
     361        BackreferencedDataSet backreferences = new BackreferencedDataSet(layer.data);
     362        backreferences.build();
     363
     364        Set<OsmPrimitive> primitivesToDelete = new HashSet<OsmPrimitive>(selection);
    295365        Collection<Way> waysToBeChanged = new HashSet<Way>();
    296         HashMap<OsmPrimitive, Collection<OsmPrimitive>> relationsToBeChanged = new HashMap<OsmPrimitive, Collection<OsmPrimitive>>();
    297366
    298367        if (alsoDeleteNodesInWay) {
    299368            // delete untagged nodes only referenced by primitives in primitivesToDelete,
    300369            // too
    301             Collection<Node> nodesToDelete = computeNodesToDelete(layer, primitivesToDelete);
     370            Collection<Node> nodesToDelete = computeNodesToDelete(backreferences, layer, primitivesToDelete);
    302371            primitivesToDelete.addAll(nodesToDelete);
    303372        }
    304373
    305         if (!simulate && !checkAndConfirmOutlyingDeletes(layer,primitivesToDelete))
     374        if (!silent && !checkAndConfirmOutlyingDeletes(layer,primitivesToDelete))
    306375            return null;
    307376
    308         CollectBackReferencesVisitor v = new CollectBackReferencesVisitor(layer.data, false);
    309         for (OsmPrimitive osm : primitivesToDelete) {
    310             v.initialize();
    311             osm.visit(v);
    312             for (OsmPrimitive ref : v.getData()) {
    313                 if (primitivesToDelete.contains(ref)) {
    314                     continue;
    315                 }
    316                 if (ref instanceof Way) {
    317                     waysToBeChanged.add((Way) ref);
    318                 } else if (ref instanceof Relation) {
    319                     if (testRelation((Relation) ref, osm, simulate) == 1) {
    320                         Collection<OsmPrimitive> relset = relationsToBeChanged.get(ref);
    321                         if (relset == null) {
    322                             relset = new HashSet<OsmPrimitive>();
    323                         }
    324                         relset.add(osm);
    325                         relationsToBeChanged.put(ref, relset);
    326                     } else
    327                         return null;
    328                 } else
    329                     return null;
    330             }
    331         }
     377        waysToBeChanged.addAll(OsmPrimitive.getFilteredSet(backreferences.getParents(primitivesToDelete), Way.class));
    332378
    333379        Collection<Command> cmds = new LinkedList<Command>();
    … …  
    337383            if (wnew.getNodesCount() < 2) {
    338384                primitivesToDelete.add(w);
    339 
    340                 v.initialize();
    341                 w.visit(v);
    342                 for (OsmPrimitive ref : v.getData()) {
    343                     if (primitivesToDelete.contains(ref)) {
    344                         continue;
    345                     }
    346                     if (ref instanceof Relation) {
    347                         Boolean found = false;
    348                         Collection<OsmPrimitive> relset = relationsToBeChanged.get(ref);
    349                         if (relset == null) {
    350                             relset = new HashSet<OsmPrimitive>();
    351                         } else {
    352                             for (OsmPrimitive m : relset) {
    353                                 if (m == w) {
    354                                     found = true;
    355                                     break;
    356                                 }
    357                             }
    358                         }
    359                         if (!found) {
    360                             if (testRelation((Relation) ref, w, simulate) == 1) {
    361                                 relset.add(w);
    362                                 relationsToBeChanged.put(ref, relset);
    363                             } else
    364                                 return null;
    365                         }
    366                     } else
    367                         return null;
    368                 }
    369385            } else {
    370386                cmds.add(new ChangeCommand(w, wnew));
    … …  
    372388        }
    373389
    374         Iterator<OsmPrimitive> iterator = relationsToBeChanged.keySet().iterator();
     390        // get a confirmation that the objects to delete can be removed from their parent
     391        // relations
     392        //
     393        if (!silent) {
     394            Set<RelationToChildReference> references = backreferences.getRelationToChildReferences(primitivesToDelete);
     395            Iterator<RelationToChildReference> it = references.iterator();
     396            while(it.hasNext()) {
     397                RelationToChildReference ref = it.next();
     398                if (ref.getParent().isDeleted()) {
     399                    it.remove();
     400                }
     401            }
     402            if (!references.isEmpty()) {
     403                DeleteFromRelationConfirmationDialog dialog = DeleteFromRelationConfirmationDialog.getInstance();
     404                dialog.getModel().populate(references);
     405                dialog.setVisible(true);
     406                if (dialog.isCanceled())
     407                    return null;
     408            }
     409        }
     410
     411        // remove the objects from their parent relations
     412        //
     413        Iterator<Relation> iterator = OsmPrimitive.getFilteredSet(backreferences.getParents(primitivesToDelete), Relation.class).iterator();
    375414        while (iterator.hasNext()) {
    376             Relation cur = (Relation) iterator.next();
     415            Relation cur = iterator.next();
    377416            Relation rel = new Relation(cur);
    378             for (OsmPrimitive osm : relationsToBeChanged.get(cur)) {
    379                 rel.removeMembersFor(osm);
    380             }
     417            rel.removeMembersFor(primitivesToDelete);
    381418            cmds.add(new ChangeCommand(cur, rel));
    382419        }
    383420
    384         // #2707: ways to be deleted can include new nodes (with node.id == 0).
    385         // Remove them from the way before the way is deleted. Otherwise the
    386         // deleted way is saved (or sent to the API) with a dangling reference to a node
    387         // Example:
    388         // <node id='2' action='delete' visible='true' version='1' ... />
    389         // <node id='1' action='delete' visible='true' version='1' ... />
    390         // <!-- missing node with id -1 because new deleted nodes are not persisted -->
    391         // <way id='3' action='delete' visible='true' version='1'>
    392         // <nd ref='1' />
    393         // <nd ref='-1' /> <!-- heres the problem -->
    394         // <nd ref='2' />
    395         // </way>
    396         for (OsmPrimitive primitive : primitivesToDelete) {
    397             if (!(primitive instanceof Way)) {
    398                 continue;
    399             }
    400             Way w = (Way) primitive;
    401             if (w.isNew()) { // new ways with id == 0 are fine,
    402                 continue; // process existing ways only
    403             }
    404             Way wnew = new Way(w);
    405             List<Node> nodesToKeep = new ArrayList<Node>();
    406             // lookup new nodes which have been added to the set of deleted
    407             // nodes ...
    408             for (Node n : wnew.getNodes()) {
    409                 if (!n.isNew() || !primitivesToDelete.contains(n)) {
    410                     nodesToKeep.add(n);
    411                 }
    412             }
    413             // .. and remove them from the way
    414             //
    415             wnew.setNodes(nodesToKeep);
    416             if (nodesToKeep.size() < w.getNodesCount()) {
    417                 cmds.add(new ChangeCommand(w, wnew));
    418             }
    419         }
    420 
     421        // build the delete command
     422        //
    421423        if (!primitivesToDelete.isEmpty()) {
    422424            cmds.add(new DeleteCommand(layer,primitivesToDelete));
    … …  
    427429
    428430    public static Command deleteWaySegment(OsmDataLayer layer, WaySegment ws) {
    429         if (ws.way.getNodesCount() < 3) {
    430             // If the way contains less than three nodes, it can't have more
    431             // than one segment, so the way should be deleted.
    432 
     431        if (ws.way.getNodesCount() < 3)
    433432            return new DeleteCommand(layer, Collections.singleton(ws.way));
    434         }
    435433
    436434        if (ws.way.firstNode() == ws.way.lastNode()) {
Note: See TracChangeset for help on using the changeset viewer.