Modify

Opened 3 months ago

Closed 3 months ago

Last modified 2 months ago

#17412 closed defect (fixed)

Purge Node doesn't update validator tree

Reported by: GerdP Owned by: team
Priority: normal Milestone: 19.03
Component: Core validator Version:
Keywords: template_report Cc:

Description

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

Attachments (4)

sample.osm (1.4 KB) - added by GerdP 3 months ago.
17412.patch (1.3 KB) - added by GerdP 3 months ago.
Possible solution, please review
17412-v2.patch (3.9 KB) - added by GerdP 3 months ago.
sample_loosing position_in_list when updating building outlines.zip (9.2 MB) - added by SanderH 3 months ago.

Change History (19)

Changed 3 months ago by GerdP

Attachment: sample.osm added

Changed 3 months ago by GerdP

Attachment: 17412.patch added

Possible solution, please review

Changed 3 months ago by GerdP

Attachment: 17412-v2.patch added

comment:1 Changed 3 months ago by 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?

comment:2 Changed 3 months ago by GerdP

Resolution: fixed
Status: newclosed

In 14849/josm:

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

comment:3 Changed 3 months ago by 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.

comment:4 Changed 3 months ago by 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?

comment:5 Changed 3 months ago by GerdP

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

comment:6 Changed 3 months ago by 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.

comment:7 Changed 3 months ago by 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.

comment:8 in reply to:  6 Changed 3 months ago by 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.

comment:9 Changed 2 months ago by GerdP

In 14856/josm:

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

comment:10 Changed 2 months ago by 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.

comment:11 Changed 2 months ago by GerdP

In 14878/josm:

see #17412: revert undintended functional change made in r14860

comment:12 in reply to:  10 Changed 2 months ago by 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

comment:13 Changed 2 months ago by GerdP

Milestone: 19.03

comment:14 Changed 2 months ago by 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.

comment:15 Changed 2 months ago by 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.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.