Changeset 5287 in josm

Timestamp:

2012-06-20T21:57:46+02:00 (12 years ago)

Author:

Don-vip

Message:

fix #5764, fix #5773 - Do not create delete commands for objects that already have been deleted, silently drop the related error in validator dialog

Location:

trunk/src/org/openstreetmap/josm

Files:

• data/validation/Test.java (modified) (2 diffs)
• data/validation/tests/DuplicateNode.java (modified) (1 diff)
• data/validation/tests/DuplicatedWayNodes.java (modified) (2 diffs)
• data/validation/tests/UntaggedNode.java (modified) (2 diffs)
• data/validation/tests/UntaggedWay.java (modified) (2 diffs)
• gui/dialogs/ValidatorDialog.java (modified) (2 diffs)

trunk/src/org/openstreetmap/josm/data/validation/Test.java

@@ -12 +12 @@
 import javax.swing.JPanel;
 
+import org.openstreetmap.josm.Main;
 import org.openstreetmap.josm.command.Command;
+import org.openstreetmap.josm.command.DeleteCommand;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
@@ -223 +225 @@
         return progressMonitor.isCanceled();
     }
+    
+    /**
+     * Build a Delete command on all primitives that have not yet been deleted manually by user, or by another error fix.
+     * If all primitives have already been deleted, null is returned.
+     * @param primitives The primitives wanted for deletion
+     * @return a Delete command on all primitives that have not yet been deleted, or null otherwise
+     */
+    protected final Command deletePrimitivesIfNeeded(Collection<? extends OsmPrimitive> primitives) {
+        Collection<OsmPrimitive> primitivesToDelete = new ArrayList<OsmPrimitive>();
+        for (OsmPrimitive p : primitives) {
+            if (!p.isDeleted()) {
+                primitivesToDelete.add(p);
+            }
+        }
+        if (!primitivesToDelete.isEmpty()) {
+            return DeleteCommand.delete(Main.map.mapView.getEditLayer(), primitivesToDelete);
+        } else {
+            return null;
+        }
+    }
 }

trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicateNode.java

@@ -373 +373 @@
         LinkedHashSet<Node> nodes = new LinkedHashSet<Node>(OsmPrimitive.getFilteredList(sel, Node.class));
 
-        // Use first existing node or first node if all nodes are new
-        Node target = null;
-        for (Node n: nodes) {
-            if (!n.isNew()) {
-                target = n;
-                break;
-            }
-        }
-        if (target == null) {
-            target = nodes.iterator().next();
-        }
-
-        if (DeleteCommand.checkAndConfirmOutlyingDelete(Main.main.getCurrentDataSet().getDataSourceArea(), nodes, Collections.singleton(target)))
-            return MergeNodesAction.mergeNodes(Main.main.getEditLayer(), nodes, target);
+        // Filter nodes that have already been deleted (see #5764 and #5773)
+        for (Iterator<Node> it = nodes.iterator(); it.hasNext();) {
+            if (it.next().isDeleted()) {
+                it.remove();
+            }
+        }
+
+        // Merge only if at least 2 nodes remain
+        if (nodes.size() >= 2) {
+            // Use first existing node or first node if all nodes are new
+            Node target = null;
+            for (Node n: nodes) {
+                if (!n.isNew()) {
+                    target = n;
+                    break;
+                }
+            }
+            if (target == null) {
+                target = nodes.iterator().next();
+            }
+    
+            if (DeleteCommand.checkAndConfirmOutlyingDelete(Main.main.getCurrentDataSet().getDataSourceArea(), nodes, Collections.singleton(target)))
+                return MergeNodesAction.mergeNodes(Main.main.getEditLayer(), nodes, target);
+        }
 
         return null;// undoRedo handling done in mergeNodes

trunk/src/org/openstreetmap/josm/data/validation/tests/DuplicatedWayNodes.java

@@ -7 +7 @@
 import java.util.Collections;
 
-import org.openstreetmap.josm.Main;
 import org.openstreetmap.josm.command.ChangeCommand;
 import org.openstreetmap.josm.command.Command;
-import org.openstreetmap.josm.command.DeleteCommand;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.Way;
@@ -61 +59 @@
         if (wnew.getNodesCount() < 2)
             // Empty way, delete
-            return DeleteCommand.delete(Main.map.mapView.getEditLayer(), Collections.singleton(w));
+            return deletePrimitivesIfNeeded(Collections.singleton(w));
         else
             return new ChangeCommand(w, wnew);

trunk/src/org/openstreetmap/josm/data/validation/tests/UntaggedNode.java

@@ -8 +8 @@
 import java.util.Map;
 
-import org.openstreetmap.josm.Main;
 import org.openstreetmap.josm.command.Command;
-import org.openstreetmap.josm.command.DeleteCommand;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
@@ -110 +108 @@
     @Override
     public Command fixError(TestError testError) {
-        return DeleteCommand.delete(Main.map.mapView.getEditLayer(), testError.getPrimitives());
+        return deletePrimitivesIfNeeded(testError.getPrimitives());
     }
 

trunk/src/org/openstreetmap/josm/data/validation/tests/UntaggedWay.java

@@ -11 +11 @@
 import org.openstreetmap.josm.Main;
 import org.openstreetmap.josm.command.Command;
-import org.openstreetmap.josm.command.DeleteCommand;
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.RelationMember;
@@ -142 +141 @@
     @Override
     public Command fixError(TestError testError) {
-        return DeleteCommand.delete(Main.map.mapView.getEditLayer(), testError.getPrimitives());
+        return deletePrimitivesIfNeeded(testError.getPrimitives());
     }
 }

trunk/src/org/openstreetmap/josm/gui/dialogs/ValidatorDialog.java

@@ -25 +25 @@
 import javax.swing.JOptionPane;
 import javax.swing.JPopupMenu;
-import javax.swing.KeyStroke;
 import javax.swing.SwingUtilities;
 import javax.swing.event.TreeSelectionEvent;
@@ -570 +569 @@
                     if (this.canceled)
                         return;
-                    final Command fixCommand = error.getFix();
-                    if (fixCommand != null) {
-                        SwingUtilities.invokeAndWait(new Runnable() {
-                            @Override
-                            public void run() {
-                                Main.main.undoRedo.addNoRedraw(fixCommand);
-                            }
-                        });
+                    if (error.isFixable()) {
+                        final Command fixCommand = error.getFix();
+                        if (fixCommand != null) {
+                            SwingUtilities.invokeAndWait(new Runnable() {
+                                @Override
+                                public void run() {
+                                    Main.main.undoRedo.addNoRedraw(fixCommand);
+                                }
+                            });
+                        }
+                        // It is wanted to ignore an error if it said fixable, even if fixCommand was null
+                        // This is to fix #5764 and #5773: a delete command, for example, may be null if all concerned primitives have already been deleted
                         error.setIgnored(true);
                     }