[josm-dev] [PATCH] Some core changes to support validator plugin

[Gabriel Ebner]

On Tue, Nov 20, 2007 at 06:01:25PM -0800, Dave Hansen wrote:
> 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.

We had a problem like that in DrawAction, when the user tries to insert
a node into a way and connect the same way to that node at once.  I worked
around it by keeping a mapping from old ways to changed ways.  (BTW, if you
want to create, say, a relation with the changed way as a member, use the old
way, not the changed way.)

And this is just one of the problems with JOSM's current command handling.
E.g. NavigatableComponent.getNearest* will always work on the old dataset.  So
if you want to do some sophisticated dataset mangling, you'll have to keep
track of what you've been doing, or call Main.undoRedo.add every once in a
while.

This also means you can't easily compose existing actions, as you would need
to pass all that state around.

I've been thinking about changing JOSM's command system to commit changes
directly to the dataset, and have a separate commit to combine them into a
sequence with a name, or do a rollback upon failure.

  Gabriel.

> 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.

So if I understand you correctly, this patch doesn't actually fix the problem?

  Gabriel.