Purge Node doesn't update validator tree

[GerdP]

What steps will reproduce the problem?

1. Load attached sample.osm
2. Run Validator
3. Purge one of the end nodes
4. Confirm removal of other objects

What is the expected result?

All errros in Validator Result window should disappear

What happens instead?

Nothing is removed, a double click on the line Warnings produces a traceback

2019-03-03 15:10:23.963 WARNING: JOSM expected to find primitive [node -115428] in dataset but it is not there. Please report this at https://josm.openstreetmap.de. This is not a critical error, it should be safe to continue in your work.
2019-03-03 15:10:23.965 SEVERE: java.lang.Exception
java.lang.Exception
	at org.openstreetmap.josm.data.osm.DataSet.getPrimitiveByIdChecked(DataSet.java:727)
	at java.util.stream.ReferencePipeline$3$1.accept(Unknown Source)
	at java.util.HashMap$KeySpliterator.forEachRemaining(Unknown Source)
	at java.util.stream.AbstractPipeline.copyInto(Unknown Source)
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(Unknown Source)
	at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(Unknown Source)
	at java.util.stream.AbstractPipeline.evaluate(Unknown Source)
	at java.util.stream.ReferencePipeline.collect(Unknown Source)
	at org.openstreetmap.josm.data.osm.DataSelectionListener$SelectionReplaceEvent.<init>(DataSelectionListener.java:135)
	at org.openstreetmap.josm.data.osm.DataSet.lambda$5(DataSet.java:615)
	at org.openstreetmap.josm.data.osm.DataSet.doSelectionChange(DataSet.java:679)
	at org.openstreetmap.josm.data.osm.DataSet.setSelected(DataSet.java:615)
	at org.openstreetmap.josm.data.osm.DataSet.setSelected(DataSet.java:606)
	at org.openstreetmap.josm.gui.dialogs.ValidatorDialog$MouseEventHandler.mouseClicked(ValidatorDialog.java:489)
	at java.awt.AWTEventMulticaster.mouseClicked(Unknown Source)
	at java.awt.Component.processMouseEvent(Unknown Source)
	at javax.swing.JComponent.processMouseEvent(Unknown Source)
	at java.awt.Component.processEvent(Unknown Source)
	at java.awt.Container.processEvent(Unknown Source)
	at java.awt.Component.dispatchEventImpl(Unknown Source)
	at java.awt.Container.dispatchEventImpl(Unknown Source)
	at java.awt.Component.dispatchEvent(Unknown Source)
	at java.awt.LightweightDispatcher.retargetMouseEvent(Unknown Source)
	at java.awt.LightweightDispatcher.processMouseEvent(Unknown Source)
	at java.awt.LightweightDispatcher.dispatchEvent(Unknown Source)
	at java.awt.Container.dispatchEventImpl(Unknown Source)
	at java.awt.Window.dispatchEventImpl(Unknown Source)
	at java.awt.Component.dispatchEvent(Unknown Source)
	at java.awt.EventQueue.dispatchEventImpl(Unknown Source)
	at java.awt.EventQueue.access$500(Unknown Source)
	at java.awt.EventQueue$3.run(Unknown Source)
	at java.awt.EventQueue$3.run(Unknown Source)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(Unknown Source)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(Unknown Source)
	at java.awt.EventQueue$4.run(Unknown Source)
	at java.awt.EventQueue$4.run(Unknown Source)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(Unknown Source)
	at java.awt.EventQueue.dispatchEvent(Unknown Source)
	at java.awt.EventDispatchThread.pumpOneEventForFilters(Unknown Source)
	at java.awt.EventDispatchThread.pumpEventsForFilter(Unknown Source)
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(Unknown Source)
	at java.awt.EventDispatchThread.pumpEvents(Unknown Source)
	at java.awt.EventDispatchThread.pumpEvents(Unknown Source)
	at java.awt.EventDispatchThread.run(Unknown Source)

Please provide any additional information below. Attach a screenshot if possible.

Build-Date:2019-03-03 15:03:19
Revision:14828
Is-Local-Build:true

Identification: JOSM/1.5 (14828 SVN en) Windows 10 64-Bit
OS Build number: Windows 10 Home 1809 (17763)
Memory Usage: 815 MB / 1792 MB (342 MB allocated, but free)
Java version: 1.8.0_201-b09, Oracle Corporation, Java HotSpot(TM) 64-Bit Server VM
Screen: \Display0 1920x1080
Maximum Screen Size: 1920x1080
VM arguments: [-agentlib:jdwp=transport=dt_socket,suspend=y,address=localhost:65391, -ea, -javaagent:D:\portable_prgs\eclipse\configuration\org.eclipse.osgi\237\0\.cp\lib\javaagent-shaded.jar, -Dfile.encoding=UTF-8]
Program arguments: [--debug]
Dataset consistency test: No problems found

Plugins:
+ apache-commons (34506)
+ austriaaddresshelper (57)
+ buildings_tools (34904)
+ download_along (34869)
+ ejml (34389)
+ geotools (34513)
+ jaxb (34678)
+ jts (34524)
+ measurement (34867)
+ o5m (34867)
+ opendata (34899)
+ pbf (34867)
+ poly (34867)
+ reltoolbox (34867)
+ reverter (34867)
+ undelete (34883)
+ utilsplugin2 (34867)

Last errors/warnings:
- W: No configuration settings found.  Using hardcoded default values for all pools.
- W: JOSM expected to find primitive [node -115411] in dataset but it is not there. Please report this at https://josm.openstreetmap.de. This is not a critical error, it should be safe to continue in your work.
- E: java.lang.Exception

[GerdP]

Possible solution, please review

[GerdP]

v2 fixes possible performance problems introduced by the first patch. There is one major open problem:
It seems when I purge a new node (negative id) no event is produced. I guess this is not intended?

[GerdP]

In 14849/josm:

fix #17412: Update validator tree when primitives are purged or removed and existing errors refer to those primitives

[GerdP]

It seems when I purge a new node (negative id) no event is produced. I guess this is not intended?

Forget that, I was wrong. When a single node is purged I see the expected PrimitivesRemovedEvent. When multiple nodes are purged a DataChangedEvent event might be produced and my mistake was to assume that a purged primitive has the atrribute OsmPrimitive::isDeleted. Instead, those primitives are found with p.getDataSet() == null.

[SanderH]

Nice to see the validator clean up immediately, but the the focus of the selected item is lost now.
I often go through the issue list and fix some with keyboard shortcuts only and that doesn't really work now when having to scroll down through the list or having to reach out to the mouse.

Can you have the focus put on the next issue in the validation results?

[GerdP]

Please open a new ticket and explain more detailed what you are missing or what should be improved.

[GerdP]

Maybe you mean this?
If you select an entry in the error list for e.g. a duplicated node and press Enter the related primitves are selected.
If you now press M the error is fixed and the selected entry is removed. In r14824 the entry is not removed and remains selected which allows to press the Down or Up key to select the next error.
I have no idea yet how to fix this but I understand that this is a regression.

[GerdP]

I don't know if there is a common way to handle this. I see two very different approaches:
1)Instead of removing errors and rebuilding the tree we might simply flag errors as invalid/fixed or whatever and render them in a different way. Advantage: The tree structure is not changed. Disadvantage: Some methods have to be changed to recognize the new flag.
2) If one or more nodes are selected in the tree when the list of errors is changed, try to select the nearest error which still exists after the tree is rebuild. The question is how to calculate this "nearest error" as there are several special cases to consider.

[SanderH]

Replying to GerdP:

Maybe you mean this?
If you select an entry in the error list for e.g. a duplicated node and press Enter the related primitves are selected.
If you now press M the error is fixed and the selected entry is removed. In r14824 the entry is not removed and remains selected which allows to press the Down or Up key to select the next error.
I have no idea yet how to fix this but I understand that this is a regression.

Yes, that's pretty much what I mean. I've recorded a small sample showing the issue for when updating building outlines with new geometries to show it in more detail. See attachments.

[GerdP]

In 14856/josm:

see #17412 if validator tree is rebuild, try to re-select the error row that was selected before.

[SanderH]

Thanks, it's a lot better now.

Only thing which remains is that after an 'validation category' becomes empty, the tree does a full collapse on the top level instead of remaining expanded.

[GerdP]

In 14878/josm:

see #17412: revert undintended functional change made in r14860

[GerdP]

Replying to SanderH:

Thanks, it's a lot better now.

Only thing which remains is that after an 'validation category' becomes empty, the tree does a full collapse on the top level instead of remaining expanded.

Thanks for the hint. I removed a few lines of code because I thought they were not needed. Now I learned what they are good for :O

[GerdP]

In 14892/josm:

see #17412: Improve re-selection of tree element when the tree is rebuild

• If an item for a TestError was selected and the new tree contains a similar entry select this new entry. This works also when the entry is at very different position in the tree.
• If multiple edit layers have validation results, don't try to use the selection of one layer when switching to another and don't try to expand the same rows.

[GerdP]

In 14907/josm:

see #17412: improve code which identifies previously expanded rows
Old code did not work when text was created by concatenating message and description, for example "suspicious tag combination - maxspeed on suspicious object (1)"
New code is simpler and a bit faster.

[Don-vip]

Ticket #16387 has been marked as a duplicate of this ticket.