Changeset 3055 in josm for trunk/src/org/openstreetmap

Timestamp:

2010-02-27T16:31:10+01:00 (15 years ago)

Author:

jttt

Message:

In DeleteCommand remove references first and then do the deleting (to prevent delete referenced inconsistency problem while command is being executed)

File:

• trunk/src/org/openstreetmap/josm/command/DeleteCommand.java (modified) (5 diffs)

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

@@ -2 +2 @@
 package org.openstreetmap.josm.command;
 
+import static org.openstreetmap.josm.tools.I18n.marktr;
 import static org.openstreetmap.josm.tools.I18n.tr;
 import static org.openstreetmap.josm.tools.I18n.trn;
-import static org.openstreetmap.josm.tools.I18n.marktr;
 
 import java.awt.GridBagLayout;
@@ -11 +11 @@
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
+import java.util.Map.Entry;
 
 import javax.swing.JLabel;
@@ -28 +31 @@
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.OsmPrimitiveType;
+import org.openstreetmap.josm.data.osm.PrimitiveData;
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.RelationToChildReference;
@@ -48 +52 @@
      */
     private final Collection<? extends OsmPrimitive> toDelete;
+    private final Map<OsmPrimitive, PrimitiveData> clonedPrimitives = new HashMap<OsmPrimitive, PrimitiveData>();
 
     /**
@@ -107 +112 @@
     }
 
-    protected void removeNewNodesFromDeletedWay(Way w) {
-        // #2707: ways to be deleted can include new nodes (with node.id == 0).
-        // Remove them from the way before the way is deleted. Otherwise the
-        // deleted way is saved (or sent to the API) with a dangling reference to a node
-        // Example:
-        // <node id='2' action='delete' visible='true' version='1' ... />
-        // <node id='1' action='delete' visible='true' version='1' ... />
-        // <!-- missing node with id -1 because new deleted nodes are not persisted -->
-        // <way id='3' action='delete' visible='true' version='1'>
-        // <nd ref='1' />
-        // <nd ref='-1' /> <!-- here's the problem -->
-        // <nd ref='2' />
-        // </way>
-        if (w.isNew())
-            return; // process existing ways only
-        List<Node> nodesToKeep = new ArrayList<Node>();
-        // lookup new nodes which have been added to the set of deleted
-        // nodes ...
-        Iterator<Node> it = nodesToKeep.iterator();
-        while(it.hasNext()) {
-            Node n = it.next();
-            if (n.isNew()) {
-                it.remove();
-            }
-        }
-        w.setNodes(nodesToKeep);
-    }
-
     @Override
     public boolean executeCommand() {
-        super.executeCommand();
-        for (OsmPrimitive osm : toDelete) {
+        // Make copy and remove all references (to prevent inconsistent dataset (delete referenced) while command is executed)
+        for (OsmPrimitive osm: toDelete) {
+            if (osm.isDeleted())
+                throw new IllegalArgumentException(osm.toString() + " is already deleted");
+            clonedPrimitives.put(osm, osm.save());
+
+            if (osm instanceof Way) {
+                ((Way) osm).setNodes(null);
+            } else if (osm instanceof Relation) {
+                ((Relation) osm).setMembers(null);
+            }
+        }
+
+        for (OsmPrimitive osm: toDelete) {
             osm.setDeleted(true);
-            if (osm instanceof Way) {
-                removeNewNodesFromDeletedWay((Way)osm);
-            }
-        }
+        }
+
         return true;
+    }
+
+    @Override
+    public void undoCommand() {
+        for (OsmPrimitive osm: toDelete) {
+            osm.setDeleted(false);
+        }
+
+        for (Entry<OsmPrimitive, PrimitiveData> entry: clonedPrimitives.entrySet()) {
+            entry.getKey().load(entry.getValue());
+        }
     }