[josm-dev] [PATCH 05/24] make Commands able to fail

Dave Hansen dave at sr71.net
Sat May 3 20:15:05 BST 2008


If one command in a sequence fails, we can roll back all the
previously successful ones.

This is always the result of a programming error.  But, it
allows us to recover from such errors gracefully.

---

 core-dave/src/org/openstreetmap/josm/command/AddCommand.java             |    3 
 core-dave/src/org/openstreetmap/josm/command/ChangeCommand.java          |    3 
 core-dave/src/org/openstreetmap/josm/command/ChangePropertyCommand.java  |    3 
 core-dave/src/org/openstreetmap/josm/command/Command.java                |   13 +++
 core-dave/src/org/openstreetmap/josm/command/ConflictResolveCommand.java |    3 
 core-dave/src/org/openstreetmap/josm/command/DeleteCommand.java          |    3 
 core-dave/src/org/openstreetmap/josm/command/MoveCommand.java            |    3 
 core-dave/src/org/openstreetmap/josm/command/RotateCommand.java          |    3 
 core-dave/src/org/openstreetmap/josm/command/SequenceCommand.java        |   33 ++++++++--
 9 files changed, 53 insertions(+), 14 deletions(-)

diff -puN src/org/openstreetmap/josm/command/AddCommand.java~Command src/org/openstreetmap/josm/command/AddCommand.java
--- core/src/org/openstreetmap/josm/command/AddCommand.java~Command	2008-05-03 12:08:39.000000000 -0700
+++ core-dave/src/org/openstreetmap/josm/command/AddCommand.java	2008-05-03 12:08:39.000000000 -0700
@@ -43,8 +43,9 @@ public class AddCommand extends Command 
 		this.ds = Main.main.editLayer().data;
 	}
 
-	@Override public void executeCommand() {
+	@Override public boolean executeCommand() {
 		osm.visit(new AddVisitor(ds));
+		return true;
 	}
 
 	@Override public void undoCommand() {
diff -puN src/org/openstreetmap/josm/command/ChangeCommand.java~Command src/org/openstreetmap/josm/command/ChangeCommand.java
--- core/src/org/openstreetmap/josm/command/ChangeCommand.java~Command	2008-05-03 12:08:39.000000000 -0700
+++ core-dave/src/org/openstreetmap/josm/command/ChangeCommand.java	2008-05-03 12:08:39.000000000 -0700
@@ -28,10 +28,11 @@ public class ChangeCommand extends Comma
 		this.newOsm = newOsm;
     }
 
-	@Override public void executeCommand() {
+	@Override public boolean executeCommand() {
 	    super.executeCommand();
 	    osm.cloneFrom(newOsm);
 	    osm.modified = true;
+		return true;
     }
 
 	@Override public void fillModifiedData(Collection<OsmPrimitive> modified, Collection<OsmPrimitive> deleted, Collection<OsmPrimitive> added) {
diff -puN src/org/openstreetmap/josm/command/ChangePropertyCommand.java~Command src/org/openstreetmap/josm/command/ChangePropertyCommand.java
--- core/src/org/openstreetmap/josm/command/ChangePropertyCommand.java~Command	2008-05-03 12:08:39.000000000 -0700
+++ core-dave/src/org/openstreetmap/josm/command/ChangePropertyCommand.java	2008-05-03 12:08:39.000000000 -0700
@@ -51,7 +51,7 @@ public class ChangePropertyCommand exten
 		this.value = value;
 	}
 	
-	@Override public void executeCommand() {
+	@Override public boolean executeCommand() {
 		super.executeCommand(); // save old
 		if (value == null) {
 			for (OsmPrimitive osm : objects) {
@@ -64,6 +64,7 @@ public class ChangePropertyCommand exten
 				osm.put(key, value);
 			}
 		}
+		return true;
 	}
 
 	@Override public void fillModifiedData(Collection<OsmPrimitive> modified, Collection<OsmPrimitive> deleted, Collection<OsmPrimitive> added) {
diff -puN src/org/openstreetmap/josm/command/Command.java~Command src/org/openstreetmap/josm/command/Command.java
--- core/src/org/openstreetmap/josm/command/Command.java~Command	2008-05-03 12:08:39.000000000 -0700
+++ core-dave/src/org/openstreetmap/josm/command/Command.java	2008-05-03 12:08:39.000000000 -0700
@@ -9,6 +9,8 @@ import java.util.Map.Entry;
 
 import javax.swing.tree.MutableTreeNode;
 
+import org.openstreetmap.josm.Main;
+import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
@@ -46,16 +48,24 @@ abstract public class Command {
 
 	private CloneVisitor orig; 
 
+	protected DataSet ds;
+
+	public Command() {
+		this.ds = Main.main.editLayer().data;
+	}
 	/**
 	 * Executes the command on the dataset. This implementation will remember all
 	 * primitives returned by fillModifiedData for restoring them on undo.
 	 */
-	public void executeCommand() {
+	public boolean did_execute = false;
+	public boolean executeCommand() {
+		did_execute = true;
 		orig = new CloneVisitor();
 		Collection<OsmPrimitive> all = new HashSet<OsmPrimitive>();
 		fillModifiedData(all, all, all);
 		for (OsmPrimitive osm : all)
 			osm.visit(orig);
+		return true;
 	}
 
 	/**
@@ -70,7 +80,6 @@ abstract public class Command {
 			e.getKey().cloneFrom(e.getValue());
 	}
 
-
 	/**
 	 * Called, when a layer has been removed to have the command remove itself from
 	 * any buffer if it is not longer applicable to the dataset (e.g. it was part of
diff -puN src/org/openstreetmap/josm/command/ConflictResolveCommand.java~Command src/org/openstreetmap/josm/command/ConflictResolveCommand.java
--- core/src/org/openstreetmap/josm/command/ConflictResolveCommand.java~Command	2008-05-03 12:08:39.000000000 -0700
+++ core-dave/src/org/openstreetmap/josm/command/ConflictResolveCommand.java	2008-05-03 12:08:39.000000000 -0700
@@ -35,7 +35,7 @@ public class ConflictResolveCommand exte
 		conflictDialog = Main.map.conflictDialog;
 	}
 
-	@Override public void executeCommand() {
+	@Override public boolean executeCommand() {
 		super.executeCommand();
 
 		origAllConflicts = new HashMap<OsmPrimitive, OsmPrimitive>(conflictDialog.conflicts);
@@ -56,6 +56,7 @@ public class ConflictResolveCommand exte
 				conflictDialog.conflicts.remove(k);
 			conflictDialog.rebuildList();
  		}
+		return true;
 	}
 
 	@Override public void undoCommand() {
diff -puN src/org/openstreetmap/josm/command/DeleteCommand.java~Command src/org/openstreetmap/josm/command/DeleteCommand.java
--- core/src/org/openstreetmap/josm/command/DeleteCommand.java~Command	2008-05-03 12:08:39.000000000 -0700
+++ core-dave/src/org/openstreetmap/josm/command/DeleteCommand.java	2008-05-03 12:08:39.000000000 -0700
@@ -29,11 +29,12 @@ public class DeleteCommand extends Comma
 		this.data = data;
 	}
 
-	@Override public void executeCommand() {
+	@Override public boolean executeCommand() {
 		super.executeCommand();
 		for (OsmPrimitive osm : data) {
 			osm.delete(true);
 		}
+		return true;
 	}
 	
 	@Override public void fillModifiedData(Collection<OsmPrimitive> modified, Collection<OsmPrimitive> deleted, Collection<OsmPrimitive> added) {
diff -puN src/org/openstreetmap/josm/command/MoveCommand.java~Command src/org/openstreetmap/josm/command/MoveCommand.java
--- core/src/org/openstreetmap/josm/command/MoveCommand.java~Command	2008-05-03 12:08:39.000000000 -0700
+++ core-dave/src/org/openstreetmap/josm/command/MoveCommand.java	2008-05-03 12:08:39.000000000 -0700
@@ -94,12 +94,13 @@ public class MoveCommand extends Command
 		this.y += y;
 	}
 	
-	@Override public void executeCommand() {
+	@Override public boolean executeCommand() {
 		for (Node n : objects) {
 			n.eastNorth = new EastNorth(n.eastNorth.east()+x, n.eastNorth.north()+y);
 			n.coor = Main.proj.eastNorth2latlon(n.eastNorth);
 			n.modified = true;
 		}
+		return true;
 	}
 
 	@Override public void undoCommand() {
diff -puN src/org/openstreetmap/josm/command/RotateCommand.java~Command src/org/openstreetmap/josm/command/RotateCommand.java
--- core/src/org/openstreetmap/josm/command/RotateCommand.java~Command	2008-05-03 12:08:39.000000000 -0700
+++ core-dave/src/org/openstreetmap/josm/command/RotateCommand.java	2008-05-03 12:08:39.000000000 -0700
@@ -112,8 +112,9 @@ public class RotateCommand extends Comma
 		}
 	}
 	
-	@Override public void executeCommand() {
+	@Override public boolean executeCommand() {
 		rotateNodes(true);
+		return true;
 	}
 
 	@Override public void undoCommand() {
diff -puN src/org/openstreetmap/josm/command/SequenceCommand.java~Command src/org/openstreetmap/josm/command/SequenceCommand.java
--- core/src/org/openstreetmap/josm/command/SequenceCommand.java~Command	2008-05-03 12:08:39.000000000 -0700
+++ core-dave/src/org/openstreetmap/josm/command/SequenceCommand.java	2008-05-03 12:08:39.000000000 -0700
@@ -9,6 +9,7 @@ import java.util.Collection;
 import javax.swing.tree.DefaultMutableTreeNode;
 import javax.swing.tree.MutableTreeNode;
 
+import org.openstreetmap.josm.Main;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 
 /**
@@ -22,7 +23,9 @@ public class SequenceCommand extends Com
 	 * The command sequenz to be executed.
 	 */
 	private Command[] sequence;
+	private boolean sequence_complete;
 	private final String name;
+	public boolean continueOnError = false;
 
 	/**
 	 * Create the command by specifying the list of commands to execute.
@@ -41,14 +44,34 @@ public class SequenceCommand extends Com
 		this(name, Arrays.asList(sequenz));
 	}
 	
-	@Override public void executeCommand() {
-		for (Command c : sequence)
-			c.executeCommand();
+	public int executed_commands = 0;
+	@Override public boolean executeCommand() {
+		for (int i=0; i < sequence.length; i++) {
+			Command c = sequence[i];
+			boolean result = c.executeCommand();
+			if (!result)
+				Main.debug("SequenceCommand, executing command[" + i + "] " +  c + " result: " + result);
+			if (!result && !continueOnError) {
+				this.undoCommands(i-1);
+				return false;
+			}
+		}
+		sequence_complete = true;
+		return true;
 	}
 
+	private void undoCommands(int start) {
+		// We probably aborted this halfway though the
+		// execution sequence because of a sub-command
+		// error.  We already undid the sub-commands.
+		if (!sequence_complete)
+			return;
+		for (int i = start; i >= 0; --i)
+			sequence[i].undoCommand();
+	}
+
 	@Override public void undoCommand() {
-		for (int i = sequence.length-1; i >= 0; --i)
-			sequence[i].undoCommand();
+		this.undoCommands(sequence.length-1);
 	}
 
 	@Override public void fillModifiedData(Collection<OsmPrimitive> modified, Collection<OsmPrimitive> deleted, Collection<OsmPrimitive> added) {
_




More information about the josm-dev mailing list