[josm-dev] [PATCH] Some core changes to support validator plugin
Dave Hansen
dave at sr71.net
Wed Nov 21 02:01:25 GMT 2007
I've enhanced the validator plugin to check for motorways intersecting
with other non-motorways. The TIGER data creates a lot of these, and we
need to find some way to fix them up other than by hand.
One problem is that the "fix" code tries to make multiple ChangeCommands
to the same way in the same set of actions. So, what happens is:
1. copy way to newWay
2. replace node[x] in newWay with a new node.
3. set up ChangeCommand(), which will overwrite way with
newWay's contents later
We repeat that sequence of events when we have "fixed" the same way in
two different places. So, we have multiple *distinct* copies of "way"
with independent changes. Since we simply copy data in ChangeCommand(),
one of these commands effectively overwrites the other one. This isn't
*too* bad, but the code ends up leaving orphan nodes around (the new
one from the command that got overwritten). I think this behavior is
pretty unfortunate, and I think it may affect the DuplicateNodes
validator check as well as mine.
So, I've modified the OsmPrimitive to keep a version counter. There may
be a better way to do this, but I think this demonstrates the concept at
least! :) ChangeCommands will not execute actions on objects with
versions different from when the ChangeCommand was created. I also
modified Command.executeCommand() to return a success/failure boolean
and modified 'SequenceCommand' to roll back changes if any commands in a
sequence fail.
In practice, you may have to press the validator "Fix" button a few
times when you have conflicts like this. In the future, perhaps we can
do this in a more automated way.
Comments on this approach are very welcome!
--- src/org/openstreetmap/josm/data/osm/OsmPrimitive.java (revision 466)
+++ src/org/openstreetmap/josm/data/osm/OsmPrimitive.java (working copy)
@@ -114,6 +114,12 @@
public boolean incomplete = false;
/**
+ * This is to help determine when individual objects are modified.
+ * When making changes, a user may increment this count.
+ */
+ public int version = 0;
+
+ /**
* Contains a list of "uninteresting" keys that do not make an object
* "tagged".
*/
@@ -185,6 +191,7 @@
* @param value The value for the key.
*/
public final void put(String key, String value) {
+ this.version++;
if (value == null)
remove(key);
else {
@@ -198,6 +205,7 @@
* Remove the given key from the list.
*/
public final void remove(String key) {
+ this.version++;
if (keys != null) {
keys.remove(key);
if (keys.isEmpty())
@@ -227,6 +235,7 @@
* use this only in the data initializing phase
*/
public void cloneFrom(OsmPrimitive osm) {
+ this.version++;
keys = osm.keys == null ? null : new HashMap<String, String>(osm.keys);
id = osm.id;
modified = osm.modified;
Index: src/org/openstreetmap/josm/command/ConflictResolveCommand.java
===================================================================
--- src/org/openstreetmap/josm/command/ConflictResolveCommand.java (revision 466)
+++ src/org/openstreetmap/josm/command/ConflictResolveCommand.java (working copy)
@@ -35,7 +35,7 @@
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 @@
conflictDialog.conflicts.remove(k);
conflictDialog.rebuildList();
}
+ return true;
}
@Override public void undoCommand() {
Index: src/org/openstreetmap/josm/command/DeleteCommand.java
===================================================================
--- src/org/openstreetmap/josm/command/DeleteCommand.java (revision 466)
+++ src/org/openstreetmap/josm/command/DeleteCommand.java (working copy)
@@ -30,11 +30,12 @@
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) {
Index: src/org/openstreetmap/josm/command/ChangeCommand.java
===================================================================
--- src/org/openstreetmap/josm/command/ChangeCommand.java (revision 466)
+++ src/org/openstreetmap/josm/command/ChangeCommand.java (working copy)
@@ -21,17 +21,22 @@
public class ChangeCommand extends Command {
private final OsmPrimitive osm;
+ private final int osm_orig_version;
private final OsmPrimitive newOsm;
public ChangeCommand(OsmPrimitive osm, OsmPrimitive newOsm) {
this.osm = osm;
+ this.osm_orig_version = osm.version;
this.newOsm = newOsm;
}
- @Override public void executeCommand() {
+ @Override public boolean executeCommand() {
super.executeCommand();
+ if (osm.version != this.osm_orig_version)
+ return false;
osm.cloneFrom(newOsm);
osm.modified = true;
+ return true;
}
@Override public void fillModifiedData(Collection<OsmPrimitive> modified, Collection<OsmPrimitive> deleted, Collection<OsmPrimitive> added) {
Index: src/org/openstreetmap/josm/command/MoveCommand.java
===================================================================
--- src/org/openstreetmap/josm/command/MoveCommand.java (revision 466)
+++ src/org/openstreetmap/josm/command/MoveCommand.java (working copy)
@@ -94,12 +94,13 @@
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() {
Index: src/org/openstreetmap/josm/command/AddCommand.java
===================================================================
--- src/org/openstreetmap/josm/command/AddCommand.java (revision 466)
+++ src/org/openstreetmap/josm/command/AddCommand.java (working copy)
@@ -44,8 +44,9 @@
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() {
Index: src/org/openstreetmap/josm/command/SequenceCommand.java
===================================================================
--- src/org/openstreetmap/josm/command/SequenceCommand.java (revision 466)
+++ src/org/openstreetmap/josm/command/SequenceCommand.java (working copy)
@@ -23,6 +23,7 @@
*/
private Command[] sequence;
private final String name;
+ public boolean continueOnError = false;
/**
* Create the command by specifying the list of commands to execute.
@@ -41,14 +42,25 @@
this(name, Arrays.asList(sequenz));
}
- @Override public void executeCommand() {
- for (Command c : sequence)
- c.executeCommand();
+ @Override public boolean executeCommand() {
+ for (int i=0; i < sequence.length; i++) {
+ Command c = sequence[i];
+ boolean result = c.executeCommand();
+ if (!result && !continueOnError) {
+ this.undoCommands(i-1);
+ return false;
+ }
+ }
+ return true;
}
+ private void undoCommands(int start) {
+ 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) {
Index: src/org/openstreetmap/josm/command/ChangePropertyCommand.java
===================================================================
--- src/org/openstreetmap/josm/command/ChangePropertyCommand.java (revision 466)
+++ src/org/openstreetmap/josm/command/ChangePropertyCommand.java (working copy)
@@ -44,7 +44,7 @@
this.value = value;
}
- @Override public void executeCommand() {
+ @Override public boolean executeCommand() {
super.executeCommand(); // save old
if (value == null) {
for (OsmPrimitive osm : objects) {
@@ -57,6 +57,7 @@
osm.put(key, value);
}
}
+ return true;
}
@Override public void fillModifiedData(Collection<OsmPrimitive> modified, Collection<OsmPrimitive> deleted, Collection<OsmPrimitive> added) {
Index: src/org/openstreetmap/josm/command/RotateCommand.java
===================================================================
--- src/org/openstreetmap/josm/command/RotateCommand.java (revision 466)
+++ src/org/openstreetmap/josm/command/RotateCommand.java (working copy)
@@ -112,8 +112,9 @@
}
}
- @Override public void executeCommand() {
+ @Override public boolean executeCommand() {
rotateNodes(true);
+ return true;
}
@Override public void undoCommand() {
Index: src/org/openstreetmap/josm/command/Command.java
===================================================================
--- src/org/openstreetmap/josm/command/Command.java (revision 466)
+++ src/org/openstreetmap/josm/command/Command.java (working copy)
@@ -50,12 +50,13 @@
* 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 executeCommand() {
orig = new CloneVisitor();
Collection<OsmPrimitive> all = new HashSet<OsmPrimitive>();
fillModifiedData(all, all, all);
for (OsmPrimitive osm : all)
osm.visit(orig);
+ return true;
}
/**
-- Dave
-- Dave
More information about the josm-dev
mailing list